Skip to content
Snippets Groups Projects

Fix search within Environments folder

Merged Fix search within Environments folder
427537-fix-environment-search into master
All threads resolved!
Merged Pam Artiaga requested to merge 427537-fix-environment-search into master
All threads resolved!

What does this MR do and why?

Searching for Environments within a folder does not always return all matches. (For more details of the problem, see related issue: Environment Search not returning all matches (#427537 - closed).)

This bug happens because the SQL query statement to search for Environments within a folder (i.e.: of the format folder/env-name) uses the ltrim function incorrectly.

The ltrim call below is supposed to trim out the folder and the slash (environment_type || '/') from the full environment name.

ltrim(environments.name, environments.environment_type || '/')

It assumes that ltrim trims by the exact environment folder. For example, with a full environment name "stg/static-files-1", the assumption is that it would trim out the leading stg/ alone. However, ltrim seems to evaluate each character in "stg/static-files-1" one by one and trim it from the start of the string until it finds a character that is not in "stg/".

To fix this, we need to do the ltrim calls below instead:

ltrim(ltrim(environments.name, environments.environment_type), '/')

So the first ltrim will stop at the /, and the second ltrim will trim out the /.

For a more detailed explanation, please see this comment thread.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Before After
env_folder_search_bug env_folder_search_fixed

Running the newly-introduced tests without the change results in an error

Rspec output 
$ bundle exec rspec spec/models/environment_spec.rb:338

Test environment set up in 2.817859 seconds
....FF....

Failures:

  1) Environment.for_name_like_within_folder when the environment folder is the same as the starting characters of the environment name returns a found name
     Failure/Error: is_expected.to contain_exactly(environment)
     
       expected collection contained:  [#<Environment id: 9, project_id: 5, name: "test/test-app", created_at: "2024-02-29 05:47:40.63753600..., merge_request_id: nil, cluster_agent_id: nil, kubernetes_namespace: nil, flux_resource_path: nil>]
       actual collection contained:    []
       the missing elements were:      [#<Environment id: 9, project_id: 5, name: "test/test-app", created_at: "2024-02-29 05:47:40.63753600..., merge_request_id: nil, cluster_agent_id: nil, kubernetes_namespace: nil, flux_resource_path: nil>]

<snip>

  2) Environment.for_name_like_within_folder when the environment folder has characters in the starting characters of the environment name returns a found name
     Failure/Error: is_expected.to contain_exactly(environment)
     
       expected collection contained:  [#<Environment id: 11, project_id: 6, name: "atr/test-app", created_at: "2024-02-29 05:47:41.13624900..., merge_request_id: nil, cluster_agent_id: nil, kubernetes_namespace: nil, flux_resource_path: nil>]
       actual collection contained:    []
       the missing elements were:      [#<Environment id: 11, project_id: 6, name: "atr/test-app", created_at: "2024-02-29 05:47:41.13624900..., merge_request_id: nil, cluster_agent_id: nil, kubernetes_namespace: nil, flux_resource_path: nil>]

<snip>

Finished in 11.97 seconds (files took 42.47 seconds to load)
10 examples, 2 failures

Failed examples:

rspec ./spec/models/environment_spec.rb:373 # Environment.for_name_like_within_folder when the environment folder is the same as the starting characters of the environment name returns a found name
rspec ./spec/models/environment_spec.rb:381 # Environment.for_name_like_within_folder when the environment folder has characters in the starting characters of the environment name returns a found name

Illustration of the ltrim function usage

Before (ltrim(environments.name, environments.environment_type || '/'))

Expand for SQL query 
> select name, environment_type, LOWER(ltrim(environments.name, environments.environment_type || '/')) as within_folder_name from environments where project_id=40;

             name              | environment_type | within_folder_name
-------------------------------+------------------+--------------------
 dev/static-files-1            | dev              | static-files-1
 dev/static-files-2            | dev              | static-files-2
 stg/static-files-1            | stg              | atic-files-1
 stg/static-files-2            | stg              | atic-files-2
 uat/static-files-1            | uat              | static-files-1
 uat/static-files-2            | uat              | static-files-2

After (ltrim(ltrim(environments.name, environments.environment_type), '/'))

Expand for SQL query 
> select name, environment_type, LOWER(ltrim(ltrim(environments.name, environments.environment_type), '/')) as within_folder_name from environments where project_id=40;

             name              | environment_type | within_folder_name
-------------------------------+------------------+--------------------
 dev/static-files-1            | dev              | static-files-1
 dev/static-files-2            | dev              | static-files-2
 stg/static-files-1            | stg              | static-files-1
 stg/static-files-2            | stg              | static-files-2
 uat/static-files-1            | uat              | static-files-1
 uat/static-files-2            | uat              | static-files-2

How to set up and validate locally

Setup

Create a project with the below environments:

dev/static-files-1
dev/static-files-2
stg/static-files-1
stg/static-files-2
uat/static-files-1
uat/static-files-2
Expand for the pipeline configuration to create the environments: 
deploy_dev_1:
  stage: deploy
  script: echo "deploying..."
  environment:
    name: dev/static-files-1
    action: start
  only:
    refs:
      - main

deploy_dev_2:
  stage: deploy
  script: echo "deploying..."
  environment:
    name: dev/static-files-2
    action: start
  only:
    refs:
      - main

deploy_stg_1:
  stage: deploy
  script: echo "deploying..."
  environment:
    name: stg/static-files-1
    action: start
  only:
    refs:
      - main

deploy_stg_2:
  stage: deploy
  script: echo "deploying..."
  environment:
    name: stg/static-files-2
    action: start
  only:
    refs:
      - main

deploy_uat_1:
  stage: deploy
  script: echo "deploying..."
  environment:
    name: uat/static-files-1
    action: start
  only:
    refs:
      - main

deploy_uat_2:
  stage: deploy
  script: echo "deploying..."
  environment:
    name: uat/static-files-2
    action: start
  only:
    refs:
      - main

Test Before

  1. Go to the Environments page of the project (Operate -> Environments)
  2. Search for "static"
  3. Only the environments under dev and uat would be in the result (see "before" screenshot above).

Test After

  1. Go to the Environments page of the project (Operate -> Environments)
  2. Search for "static"
  3. All environments with "static" in the name should be in the result (see "after" screenshot above).

Related to #427537 (closed)

Edited by Pam Artiaga

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Please register or sign in to reply