Merge branch 'mb-ci-group-instance-group-project-secret-variables' into 'master'

Reuse variables builder for Ci::Config rules evaluation

See merge request gitlab-org/gitlab!79935

app/models/ci/pipeline.rb

@@ -838,6 +838,8 @@ module Ci
     def predefined_commit_variables
       strong_memoize(:predefined_commit_variables) do
         Gitlab::Ci::Variables::Collection.new.tap do |variables|
+          next variables unless sha.present?
+
           variables.append(key: 'CI_COMMIT_SHA', value: sha)
           variables.append(key: 'CI_COMMIT_SHORT_SHA', value: short_sha)
           variables.append(key: 'CI_COMMIT_BEFORE_SHA', value: before_sha)

config/feature_flags/development/ci_variables_builder_config_variables.yml

+---
+name: ci_variables_builder_config_variables
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79935
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/358791
+milestone: '14.10'
+type: development
+group: group::pipeline execution
+default_enabled: false

lib/gitlab/ci/config.rb

@@ -6,6 +6,8 @@ module Gitlab
     # Base GitLab CI Configuration facade
     #
     class Config
+      include Gitlab::Utils::StrongMemoize
+
       ConfigError = Class.new(StandardError)
       TIMEOUT_SECONDS = 30.seconds
       TIMEOUT_MESSAGE = 'Resolving config took longer than expected'
@@ -22,6 +24,11 @@ module Gitlab
       def initialize(config, project: nil, pipeline: nil, sha: nil, user: nil, parent_pipeline: nil, source: nil, logger: nil)
         @logger = logger || ::Gitlab::Ci::Pipeline::Logger.new(project: project)
         @source_ref_path = pipeline&.source_ref_path
+        @project = project
+
+        if use_config_variables?
+          pipeline ||= ::Ci::Pipeline.new(project: project, sha: sha, user: user, source: source)
+        end
 
         @context = self.logger.instrument(:config_build_context) do
           build_context(project: project, pipeline: pipeline, sha: sha, user: user, parent_pipeline: parent_pipeline)
@@ -155,6 +162,10 @@ module Gitlab
       end
 
       def build_variables_without_instrumentation(project:, pipeline:)
+        if use_config_variables?
+          return pipeline.variables_builder.config_variables
+        end
+
         Gitlab::Ci::Variables::Collection.new.tap do |variables|
           break variables unless project
 
@@ -184,6 +195,12 @@ module Gitlab
         Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error, @context.sentry_payload)
       end
 
+      def use_config_variables?
+        strong_memoize(:use_config_variables) do
+          ::Feature.enabled?(:ci_variables_builder_config_variables, @project, default_enabled: :yaml)
+        end
+      end
+
       # Overridden in EE
       def rescue_errors
         RESCUE_ERRORS

lib/gitlab/ci/variables/builder.rb

@@ -10,7 +10,7 @@ module Gitlab
           @pipeline = pipeline
           @instance_variables_builder = Builder::Instance.new
           @project_variables_builder = Builder::Project.new(project)
-          @group_variables_builder = Builder::Group.new(project.group)
+          @group_variables_builder = Builder::Group.new(project&.group)
         end
 
         def scoped_variables(job, environment:, dependencies:)
@@ -32,6 +32,20 @@ module Gitlab
           end
         end
 
+        def config_variables
+          Gitlab::Ci::Variables::Collection.new.tap do |variables|
+            break variables unless project
+
+            variables.concat(project.predefined_variables)
+            variables.concat(pipeline.predefined_variables)
+            variables.concat(secret_instance_variables)
+            variables.concat(secret_group_variables(environment: nil))
+            variables.concat(secret_project_variables(environment: nil))
+            variables.concat(pipeline.variables)
+            variables.concat(pipeline_schedule_variables)
+          end
+        end
+
         def kubernetes_variables(environment:, job:)
           ::Gitlab::Ci::Variables::Collection.new.tap do |collection|
             # NOTE: deployment_variables will be removed as part of cleanup for

spec/lib/gitlab/ci/variables/builder_spec.rb

@@ -501,4 +501,115 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
       end
     end
   end
+
+  describe '#config_variables' do
+    subject(:config_variables) { builder.config_variables }
+
+    context 'without project' do
+      before do
+        pipeline.update!(project_id: nil)
+      end
+
+      it { expect(config_variables.size).to eq(0) }
+    end
+
+    context 'without repository' do
+      let(:project) { create(:project) }
+      let(:pipeline) { build(:ci_pipeline, ref: nil, sha: nil, project: project) }
+
+      it { expect(config_variables['CI_COMMIT_SHA']).to be_nil }
+    end
+
+    context 'with protected variables' do
+      let_it_be(:instance_variable) do
+        create(:ci_instance_variable, :protected, key: 'instance_variable')
+      end
+
+      let_it_be(:group_variable) do
+        create(:ci_group_variable, :protected, group: group, key: 'group_variable')
+      end
+
+      let_it_be(:project_variable) do
+        create(:ci_variable, :protected, project: project, key: 'project_variable')
+      end
+
+      it 'does not include protected variables' do
+        expect(config_variables[instance_variable.key]).to be_nil
+        expect(config_variables[group_variable.key]).to be_nil
+        expect(config_variables[project_variable.key]).to be_nil
+      end
+    end
+
+    context 'with scoped variables' do
+      let_it_be(:scoped_group_variable) do
+        create(:ci_group_variable,
+          group: group,
+          key: 'group_variable',
+          value: 'scoped',
+          environment_scope: 'scoped')
+      end
+
+      let_it_be(:group_variable) do
+        create(:ci_group_variable,
+          group: group,
+          key: 'group_variable',
+          value: 'unscoped')
+      end
+
+      let_it_be(:scoped_project_variable) do
+        create(:ci_variable,
+          project: project,
+          key: 'project_variable',
+          value: 'scoped',
+          environment_scope: 'scoped')
+      end
+
+      let_it_be(:project_variable) do
+        create(:ci_variable,
+          project: project,
+          key: 'project_variable',
+          value: 'unscoped')
+      end
+
+      it 'does not include scoped variables' do
+        expect(config_variables.to_hash[group_variable.key]).to eq('unscoped')
+        expect(config_variables.to_hash[project_variable.key]).to eq('unscoped')
+      end
+    end
+
+    context 'variables ordering' do
+      def var(name, value)
+        { key: name, value: value.to_s, public: true, masked: false }
+      end
+
+      before do
+        allow(pipeline.project).to receive(:predefined_variables) { [var('A', 1), var('B', 1)] }
+        allow(pipeline).to receive(:predefined_variables) { [var('B', 2), var('C', 2)] }
+        allow(builder).to receive(:secret_instance_variables) { [var('C', 3), var('D', 3)] }
+        allow(builder).to receive(:secret_group_variables) { [var('D', 4), var('E', 4)] }
+        allow(builder).to receive(:secret_project_variables) { [var('E', 5), var('F', 5)] }
+        allow(pipeline).to receive(:variables) { [var('F', 6), var('G', 6)] }
+        allow(pipeline).to receive(:pipeline_schedule) { double(job_variables: [var('G', 7), var('H', 7)]) }
+      end
+
+      it 'returns variables in order depending on resource hierarchy' do
+        expect(config_variables.to_runner_variables).to eq(
+          [var('A', 1), var('B', 1),
+           var('B', 2), var('C', 2),
+           var('C', 3), var('D', 3),
+           var('D', 4), var('E', 4),
+           var('E', 5), var('F', 5),
+           var('F', 6), var('G', 6),
+           var('G', 7), var('H', 7)])
+      end
+
+      it 'overrides duplicate keys depending on resource hierarchy' do
+        expect(config_variables.to_hash).to match(
+          'A' => '1', 'B' => '2',
+          'C' => '3', 'D' => '4',
+          'E' => '5', 'F' => '6',
+          'G' => '7', 'H' => '7')
+      end
+    end
+  end
 end

spec/models/ci/pipeline_spec.rb

@@ -1168,6 +1168,28 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
         end
       end
     end
+
+    context 'without a commit' do
+      let(:pipeline) { build(:ci_empty_pipeline, :created, sha: nil) }
+
+      it 'does not expose commit variables' do
+        expect(subject.to_hash.keys)
+          .not_to include(
+            'CI_COMMIT_SHA',
+            'CI_COMMIT_SHORT_SHA',
+            'CI_COMMIT_BEFORE_SHA',
+            'CI_COMMIT_REF_NAME',
+            'CI_COMMIT_REF_SLUG',
+            'CI_COMMIT_BRANCH',
+            'CI_COMMIT_TAG',
+            'CI_COMMIT_MESSAGE',
+            'CI_COMMIT_TITLE',
+            'CI_COMMIT_DESCRIPTION',
+            'CI_COMMIT_REF_PROTECTED',
+            'CI_COMMIT_TIMESTAMP',
+            'CI_COMMIT_AUTHOR')
+      end
+    end
   end
 
   describe '#protected_ref?' do