Merge branch...

Merge branch '218173-ci-resource_group-not-working-when-using-ci_environment_name-directly' into 'master'

Allow predefined environment variables in resource_group

See merge request gitlab-org/gitlab!67876

app/models/ci/build.rb

@@ -90,6 +90,10 @@ module Ci
       end
     end
 
+    def persisted_environment=(environment)
+      strong_memoize(:persisted_environment) { environment }
+    end
+
     serialize :options # rubocop:disable Cop/ActiveRecordSerialize
     serialize :yaml_variables, Gitlab::Serializer::Ci::Variables # rubocop:disable Cop/ActiveRecordSerialize
 

app/models/concerns/ci/contextable.rb

@@ -33,13 +33,13 @@ module Ci
     #
     def simple_variables
       strong_memoize(:simple_variables) do
-        scoped_variables(environment: nil).to_runner_variables
+        scoped_variables(environment: nil)
       end
     end
 
     def simple_variables_without_dependencies
       strong_memoize(:variables_without_dependencies) do
-        scoped_variables(environment: nil, dependencies: false).to_runner_variables
+        scoped_variables(environment: nil, dependencies: false)
       end
     end
 

config/feature_flags/development/env_vars_resource_group.yml

+---
+name: env_vars_resource_group
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67876
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339024
+milestone: '14.3'
+type: development
+group: group::release
+default_enabled: false

doc/ci/variables/where_variables_can_be_used.md

@@ -26,7 +26,7 @@ There are two places defined variables can be used. On the:
 |:-------------------------------------------|:-----------------|:-----------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `environment:url`                          | yes              | GitLab                 | The variable expansion is made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab.<br/><br/>Supported are all variables defined for a job (project/group variables, variables from `.gitlab-ci.yml`, variables from triggers, variables from pipeline schedules).<br/><br/>Not supported are variables defined in the GitLab Runner `config.toml` and variables created in the job's `script`. |
 | `environment:name`                         | yes              | GitLab                 | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/><br/>- Variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`).<br/>- Any other variables related to environment (currently only `CI_ENVIRONMENT_URL`).<br/>- [Persisted variables](#persisted-variables).                                                                                 |
-| `resource_group`                           | yes              | GitLab                 | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/><br/>- Variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`).<br/>- Any other variables related to environment (currently only `CI_ENVIRONMENT_URL`).<br/>- [Persisted variables](#persisted-variables).                                                                                 |
+| `resource_group`                           | yes              | GitLab                 | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/>- `CI_ENVIRONMENT_URL`<br/>- [Persisted variables](#persisted-variables)                                                                                 |
 | `include`                                  | yes              | GitLab                 | The variable expansion is made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab. <br/><br/>Predefined project variables are supported: `GITLAB_FEATURES`, `CI_DEFAULT_BRANCH`, and all variables that start with `CI_PROJECT_` (for example `CI_PROJECT_NAME`).                                                                                                            |
 | `variables`                                | yes              | GitLab/Runner          | The variable expansion is first made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab, and then any unrecognized or unavailable variables are expanded by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism).                                                                                                   |
 | `image`                                    | yes              | Runner                 | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism)                                                                                                                                                                                                                                                                                   |

lib/gitlab/ci/pipeline/seed/build.rb

@@ -118,6 +118,8 @@ module Gitlab
               return { environment: nil }
             end
 
+            build.persisted_environment = environment
+
             {
               deployment: Seed::Deployment.new(build, environment).to_resource,
               metadata_attributes: {

lib/gitlab/ci/pipeline/seed/processable/resource_group.rb

@@ -28,7 +28,16 @@ module Gitlab
 
             def expanded_resource_group_key
               strong_memoize(:expanded_resource_group_key) do
-                ExpandVariables.expand(resource_group_key, -> { processable.simple_variables })
+                ExpandVariables.expand(resource_group_key, -> { variables })
+              end
+            end
+
+            def variables
+              processable.simple_variables.tap do |variables|
+                # Adding persisted environment variables
+                if Feature.enabled?(:env_vars_resource_group) && processable.persisted_environment.present?
+                  variables.concat(processable.persisted_environment.predefined_variables)
+                end
               end
             end
           end

spec/lib/gitlab/ci/pipeline/seed/build_spec.rb

@@ -490,12 +490,21 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
       end
 
       context 'when job belongs to a resource group' do
-        let(:attributes) { { name: 'rspec', ref: 'master', resource_group_key: 'iOS' } }
+        let(:resource_group) { 'iOS' }
+        let(:attributes) { { name: 'rspec', ref: 'master', resource_group_key: resource_group, environment: 'production' }}
 
         it 'returns a job with resource group' do
           expect(subject.resource_group).not_to be_nil
           expect(subject.resource_group.key).to eq('iOS')
         end
+
+        context 'when resource group has $CI_ENVIRONMENT_NAME in it' do
+          let(:resource_group) { 'test/$CI_ENVIRONMENT_NAME' }
+
+          it 'expands environment name' do
+            expect(subject.resource_group.key).to eq('test/production')
+          end
+        end
       end
     end
 

spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb

@@ -127,6 +127,32 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
           end
         end
       end
+
+      context 'when resource group key includes a variable' do
+        let(:config) do
+          <<~YAML
+          instrumentation_test:
+            stage: test
+            resource_group: $CI_ENVIRONMENT_NAME
+            trigger:
+              include: path/to/child.yml
+              strategy: depend
+          YAML
+        end
+
+        it 'ignores the resource group keyword because it fails to expand the variable', :aggregate_failures do
+          pipeline = create_pipeline!
+          Ci::InitialPipelineProcessWorker.new.perform(pipeline.id)
+
+          test = pipeline.statuses.find_by(name: 'instrumentation_test')
+          expect(pipeline).to be_created_successfully
+          expect(pipeline.triggered_pipelines).not_to be_exist
+          expect(project.resource_groups.count).to eq(0)
+          expect(test).to be_a Ci::Bridge
+          expect(test).to be_pending
+          expect(test.resource_group).to be_nil
+        end
+      end
     end
   end
 

spec/services/ci/create_pipeline_service_spec.rb

@@ -991,6 +991,74 @@ RSpec.describe Ci::CreatePipelineService do
       end
     end
 
+    context 'when resource group is defined for review app deployment' do
+      before do
+        config = YAML.dump(
+          review_app: {
+            stage: 'test',
+            script: 'deploy',
+            environment: {
+              name: 'review/$CI_COMMIT_REF_SLUG',
+              on_stop: 'stop_review_app'
+            },
+            resource_group: '$CI_ENVIRONMENT_NAME'
+          },
+          stop_review_app: {
+            stage: 'test',
+            script: 'stop',
+            when: 'manual',
+            environment: {
+              name: 'review/$CI_COMMIT_REF_SLUG',
+              action: 'stop'
+            },
+            resource_group: '$CI_ENVIRONMENT_NAME'
+          }
+        )
+
+        stub_ci_pipeline_yaml_file(config)
+      end
+
+      it 'persists the association correctly' do
+        result = execute_service.payload
+        deploy_job = result.builds.find_by_name!(:review_app)
+        stop_job = result.builds.find_by_name!(:stop_review_app)
+
+        expect(result).to be_persisted
+        expect(deploy_job.resource_group.key).to eq('review/master')
+        expect(stop_job.resource_group.key).to eq('review/master')
+        expect(project.resource_groups.count).to eq(1)
+      end
+
+      it 'initializes scoped variables only once for each build' do
+        # Bypassing `stub_build` hack because it distrubs the expectations below.
+        allow_next_instances_of(Gitlab::Ci::Build::Context::Build, 2) do |build_context|
+          allow(build_context).to receive(:variables) { Gitlab::Ci::Variables::Collection.new }
+        end
+
+        expect_next_instances_of(::Ci::Build, 2) do |ci_build|
+          expect(ci_build).to receive(:scoped_variables).once.and_call_original
+        end
+
+        expect(execute_service.payload).to be_created_successfully
+      end
+
+      context 'when the env_vars_resource_group feature flag is disabled' do
+        before do
+          stub_feature_flags(env_vars_resource_group: false)
+        end
+
+        it 'does not create a resource group because its key contains an invalid character' do
+          result = execute_service.payload
+          deploy_job = result.builds.find_by_name!(:review_app)
+          stop_job = result.builds.find_by_name!(:stop_review_app)
+          expect(result).to be_persisted
+          expect(deploy_job.resource_group).to be_nil
+          expect(stop_job.resource_group).to be_nil
+          expect(project.resource_groups.count).to eq(0)
+        end
+      end
+    end
+
     context 'with timeout' do
       context 'when builds with custom timeouts are configured' do
         before do