Add latest changes from gitlab-org/gitlab@master

14 files changed, 135 insertions, 43 deletions

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index 121c4bbfc32..9787e02643b 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -64,14 +64,6 @@ class ApplicationRecord < ActiveRecord::Base
   end
 
   def self.safe_find_or_create_by(*args, &block)
-    return optimized_safe_find_or_create_by(*args, &block) if Feature.enabled?(:optimize_safe_find_or_create_by, default_enabled: :yaml)
-
-    safe_ensure_unique(retries: 1) do # rubocop:disable Performance/ActiveRecordSubtransactionMethods
-      find_or_create_by(*args, &block)
-    end
-  end
-
-  def self.optimized_safe_find_or_create_by(*args, &block)
     record = find_by(*args)
     return record if record.present?
 
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 2df9f3bc75c..d7b93a616fc 100644
--- a/app/models/ci/build.rb
+++ b/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
 
diff --git a/app/models/concerns/ci/contextable.rb b/app/models/concerns/ci/contextable.rb
index bdba2d3e251..27a704c1de0 100644
--- a/app/models/concerns/ci/contextable.rb
+++ b/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
 
diff --git a/config/feature_flags/development/env_vars_resource_group.yml b/config/feature_flags/development/env_vars_resource_group.yml
new file mode 100644
index 00000000000..0b5fdb02da1
--- /dev/null
+++ b/config/feature_flags/development/env_vars_resource_group.yml
@@ -0,0 +1,8 @@
+---
+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
diff --git a/config/feature_flags/development/optimize_safe_find_or_create_by.yml b/config/feature_flags/development/optimize_safe_find_or_create_by.yml
deleted file mode 100644
index 6adedb5cece..00000000000
--- a/config/feature_flags/development/optimize_safe_find_or_create_by.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: optimize_safe_find_or_create_by
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68458
-rollout_issue_url: 
-milestone: '14.3'
-type: development
-group: group::database
-default_enabled: false
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 46506750e26..c081addb24a 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -7605,7 +7605,7 @@ Describes a rule for who can approve merge requests.
 | <a id="approvalruleapproved"></a>`approved` | [`Boolean`](#boolean) | Indicates if the rule is satisfied. |
 | <a id="approvalruleapprovedby"></a>`approvedBy` | [`UserCoreConnection`](#usercoreconnection) | List of users defined in the rule that approved the merge request. (see [Connections](#connections)) |
 | <a id="approvalrulecontainshiddengroups"></a>`containsHiddenGroups` | [`Boolean`](#boolean) | Indicates if the rule contains approvers from a hidden group. |
-| <a id="approvalruleeligibleapprovers"></a>`eligibleApprovers` | [`UserCoreConnection`](#usercoreconnection) | List of all users eligible to approve the merge request (defined explicitly and from associated groups). (see [Connections](#connections)) |
+| <a id="approvalruleeligibleapprovers"></a>`eligibleApprovers` | [`[UserCore!]`](#usercore) | List of all users eligible to approve the merge request (defined explicitly and from associated groups). |
 | <a id="approvalrulegroups"></a>`groups` | [`GroupConnection`](#groupconnection) | List of groups added as approvers for the rule. (see [Connections](#connections)) |
 | <a id="approvalruleid"></a>`id` | [`GlobalID!`](#globalid) | ID of the rule. |
 | <a id="approvalrulename"></a>`name` | [`String`](#string) | Name of the rule. |
diff --git a/doc/ci/variables/where_variables_can_be_used.md b/doc/ci/variables/where_variables_can_be_used.md
index 8009687dbca..c15e4c123dc 100644
--- a/doc/ci/variables/where_variables_can_be_used.md
+++ b/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)                                                                                                                                                                                                                                                                                   |
diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb
index a8043882c1a..934bf22d8ad 100644
--- a/lib/gitlab/ci/pipeline/seed/build.rb
+++ b/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: {
diff --git a/lib/gitlab/ci/pipeline/seed/processable/resource_group.rb b/lib/gitlab/ci/pipeline/seed/processable/resource_group.rb
index f8ea6d4184c..d0ecfff4f34 100644
--- a/lib/gitlab/ci/pipeline/seed/processable/resource_group.rb
+++ b/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
diff --git a/qa/qa/resource/merge_request.rb b/qa/qa/resource/merge_request.rb
index 8c313f5d518..93b5fc3ef6b 100644
--- a/qa/qa/resource/merge_request.rb
+++ b/qa/qa/resource/merge_request.rb
@@ -25,7 +25,7 @@ module QA
                  :state
 
       attribute :project do
-        Project.fabricate! do |resource|
+        Project.fabricate_via_api! do |resource|
           resource.name = 'project-with-merge-request'
         end
       end
diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index b211bc50f4b..0c28515b574 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/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
 
diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb
index 40e2aae0bad..7b9e70261fb 100644
--- a/spec/models/application_record_spec.rb
+++ b/spec/models/application_record_spec.rb
@@ -39,7 +39,7 @@ RSpec.describe ApplicationRecord do
 
     let(:suggestion_attributes) { attributes_for(:suggestion).merge!(note_id: note.id) }
 
-    shared_examples '.safe_find_or_create_by' do
+    describe '.safe_find_or_create_by' do
       it 'creates the suggestion avoiding race conditions' do
         existing_suggestion = double(:Suggestion)
 
@@ -63,7 +63,7 @@ RSpec.describe ApplicationRecord do
       end
     end
 
-    shared_examples '.safe_find_or_create_by!' do
+    describe '.safe_find_or_create_by!' do
       it 'creates a record using safe_find_or_create_by' do
         expect(Suggestion.safe_find_or_create_by!(suggestion_attributes))
           .to be_a(Suggestion)
@@ -88,24 +88,6 @@ RSpec.describe ApplicationRecord do
           .to raise_error(ActiveRecord::RecordNotFound)
       end
     end
-
-    context 'when optimized_safe_find_or_create_by is enabled' do
-      before do
-        stub_feature_flags(optimized_safe_find_or_create_by: true)
-      end
-
-      it_behaves_like '.safe_find_or_create_by'
-      it_behaves_like '.safe_find_or_create_by!'
-    end
-
-    context 'when optimized_safe_find_or_create_by is disabled' do
-      before do
-        stub_feature_flags(optimized_safe_find_or_create_by: false)
-      end
-
-      it_behaves_like '.safe_find_or_create_by'
-      it_behaves_like '.safe_find_or_create_by!'
-    end
   end
 
   describe '.underscore' do
diff --git a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb
index 6eb1315fff4..4326fa5533f 100644
--- a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb
+++ b/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
 
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index d5d65598589..4c9e7da83f9 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/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