Merge branch 'ci-variables-builder-switch' into 'master'

Move contextable variables definitions into the variables builder

See merge request gitlab-org/gitlab!75254

app/models/ci/pipeline.rb

@@ -1292,6 +1292,12 @@ module Ci
       end
     end
 
+    def use_variables_builder_definitions?
+      strong_memoize(:use_variables_builder_definitions) do
+        ::Feature.enabled?(:ci_use_variables_builder_definitions, project, default_enabled: :yaml)
+      end
+    end
+
     private
 
     def add_message(severity, content)

app/models/concerns/ci/contextable.rb

@@ -13,6 +13,8 @@ module Ci
       track_duration do
         variables = pipeline.variables_builder.scoped_variables(self, environment: environment, dependencies: dependencies)
 
+        next variables if pipeline.use_variables_builder_definitions?
+
         variables.concat(project.predefined_variables)
         variables.concat(pipeline.predefined_variables)
         variables.concat(runner.predefined_variables) if runnable? && runner
@@ -60,49 +62,27 @@ module Ci
     end
 
     def user_variables
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        break variables if user.blank?
-
-        variables.append(key: 'GITLAB_USER_ID', value: user.id.to_s)
-        variables.append(key: 'GITLAB_USER_EMAIL', value: user.email)
-        variables.append(key: 'GITLAB_USER_LOGIN', value: user.username)
-        variables.append(key: 'GITLAB_USER_NAME', value: user.name)
-      end
+      pipeline.variables_builder.user_variables(user)
     end
 
     def kubernetes_variables
-      ::Gitlab::Ci::Variables::Collection.new.tap do |collection|
-        # Should get merged with the cluster kubeconfig in deployment_variables, see
-        # https://gitlab.com/gitlab-org/gitlab/-/issues/335089
-        template = ::Ci::GenerateKubeconfigService.new(self).execute
-
-        if template.valid?
-          collection.append(key: 'KUBECONFIG', value: template.to_yaml, public: false, file: true)
-        end
-      end
+      pipeline.variables_builder.kubernetes_variables(self)
     end
 
     def deployment_variables(environment:)
-      return [] unless environment
-
-      project.deployment_variables(
-        environment: environment,
-        kubernetes_namespace: expanded_kubernetes_namespace
-      )
+      pipeline.variables_builder.deployment_variables(job: self, environment: environment)
     end
 
     def secret_instance_variables
-      project.ci_instance_variables_for(ref: git_ref)
+      pipeline.variables_builder.secret_instance_variables(ref: git_ref)
     end
 
     def secret_group_variables(environment: expanded_environment_name)
-      return [] unless project.group
-
-      project.group.ci_variables_for(git_ref, project, environment: environment)
+      pipeline.variables_builder.secret_group_variables(environment: environment, ref: git_ref)
     end
 
     def secret_project_variables(environment: expanded_environment_name)
-      project.ci_variables_for(ref: git_ref, environment: environment)
+      pipeline.variables_builder.secret_project_variables(environment: environment, ref: git_ref)
     end
   end
 end

config/feature_flags/development/ci_use_variables_builder_definitions.yml

+---
+name: ci_use_variables_builder_definitions
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75254
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/349049
+milestone: '14.7'
+type: development
+group: group::pipeline execution
+default_enabled: false

lib/gitlab/ci/variables/builder.rb

@@ -13,12 +13,76 @@ module Gitlab
         def scoped_variables(job, environment:, dependencies:)
           Gitlab::Ci::Variables::Collection.new.tap do |variables|
             variables.concat(predefined_variables(job))
+
+            next variables unless pipeline.use_variables_builder_definitions?
+
+            variables.concat(project.predefined_variables)
+            variables.concat(pipeline.predefined_variables)
+            variables.concat(job.runner.predefined_variables) if job.runnable? && job.runner
+            variables.concat(kubernetes_variables(job))
+            variables.concat(deployment_variables(environment: environment, job: job))
+            variables.concat(job.yaml_variables)
+            variables.concat(user_variables(job.user))
+            variables.concat(job.dependency_variables) if dependencies
+            variables.concat(secret_instance_variables(ref: job.git_ref))
+            variables.concat(secret_group_variables(environment: environment, ref: job.git_ref))
+            variables.concat(secret_project_variables(environment: environment, ref: job.git_ref))
+            variables.concat(job.trigger_request.user_variables) if job.trigger_request
+            variables.concat(pipeline.variables)
+            variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule
+          end
+        end
+
+        def kubernetes_variables(job)
+          ::Gitlab::Ci::Variables::Collection.new.tap do |collection|
+            # Should get merged with the cluster kubeconfig in deployment_variables, see
+            # https://gitlab.com/gitlab-org/gitlab/-/issues/335089
+            template = ::Ci::GenerateKubeconfigService.new(job).execute
+
+            if template.valid?
+              collection.append(key: 'KUBECONFIG', value: template.to_yaml, public: false, file: true)
+            end
           end
         end
 
+        def deployment_variables(environment:, job:)
+          return [] unless environment
+
+          project.deployment_variables(
+            environment: environment,
+            kubernetes_namespace: job.expanded_kubernetes_namespace
+          )
+        end
+
+        def user_variables(user)
+          Gitlab::Ci::Variables::Collection.new.tap do |variables|
+            break variables if user.blank?
+
+            variables.append(key: 'GITLAB_USER_ID', value: user.id.to_s)
+            variables.append(key: 'GITLAB_USER_EMAIL', value: user.email)
+            variables.append(key: 'GITLAB_USER_LOGIN', value: user.username)
+            variables.append(key: 'GITLAB_USER_NAME', value: user.name)
+          end
+        end
+
+        def secret_instance_variables(ref:)
+          project.ci_instance_variables_for(ref: ref)
+        end
+
+        def secret_group_variables(environment:, ref:)
+          return [] unless project.group
+
+          project.group.ci_variables_for(ref, project, environment: environment)
+        end
+
+        def secret_project_variables(environment:, ref:)
+          project.ci_variables_for(ref: ref, environment: environment)
+        end
+
         private
 
         attr_reader :pipeline
+        delegate :project, to: :pipeline
 
         def predefined_variables(job)
           Gitlab::Ci::Variables::Collection.new.tap do |variables|

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

@@ -3,25 +3,201 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Variables::Builder do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+  let_it_be(:user) { project.owner }
+  let_it_be(:job) do
+    create(:ci_build,
+      pipeline: pipeline,
+      user: user,
+      yaml_variables: [{ key: 'YAML_VARIABLE', value: 'value' }]
+    )
+  end
+
   let(:builder) { described_class.new(pipeline) }
-  let(:pipeline) { create(:ci_pipeline) }
-  let(:job) { create(:ci_build, pipeline: pipeline) }
 
   describe '#scoped_variables' do
     let(:environment) { job.expanded_environment_name }
     let(:dependencies) { true }
+    let(:predefined_variables) do
+      [
+        { key: 'CI_JOB_NAME',
+          value: job.name },
+        { key: 'CI_JOB_STAGE',
+          value: job.stage },
+        { key: 'CI_NODE_TOTAL',
+          value: '1' },
+        { key: 'CI_BUILD_NAME',
+          value: job.name },
+        { key: 'CI_BUILD_STAGE',
+          value: job.stage },
+        { key: 'CI',
+          value: 'true' },
+        { key: 'GITLAB_CI',
+          value: 'true' },
+        { key: 'CI_SERVER_URL',
+          value: Gitlab.config.gitlab.url },
+        { key: 'CI_SERVER_HOST',
+          value: Gitlab.config.gitlab.host },
+        { key: 'CI_SERVER_PORT',
+          value: Gitlab.config.gitlab.port.to_s },
+        { key: 'CI_SERVER_PROTOCOL',
+          value: Gitlab.config.gitlab.protocol },
+        { key: 'CI_SERVER_NAME',
+          value: 'GitLab' },
+        { key: 'CI_SERVER_VERSION',
+          value: Gitlab::VERSION },
+        { key: 'CI_SERVER_VERSION_MAJOR',
+          value: Gitlab.version_info.major.to_s },
+        { key: 'CI_SERVER_VERSION_MINOR',
+          value: Gitlab.version_info.minor.to_s },
+        { key: 'CI_SERVER_VERSION_PATCH',
+          value: Gitlab.version_info.patch.to_s },
+        { key: 'CI_SERVER_REVISION',
+          value: Gitlab.revision },
+        { key: 'GITLAB_FEATURES',
+          value: project.licensed_features.join(',') },
+        { key: 'CI_PROJECT_ID',
+          value: project.id.to_s },
+        { key: 'CI_PROJECT_NAME',
+          value: project.path },
+        { key: 'CI_PROJECT_TITLE',
+          value: project.title },
+        { key: 'CI_PROJECT_PATH',
+          value: project.full_path },
+        { key: 'CI_PROJECT_PATH_SLUG',
+          value: project.full_path_slug },
+        { key: 'CI_PROJECT_NAMESPACE',
+          value: project.namespace.full_path },
+        { key: 'CI_PROJECT_ROOT_NAMESPACE',
+          value: project.namespace.root_ancestor.path },
+        { key: 'CI_PROJECT_URL',
+          value: project.web_url },
+        { key: 'CI_PROJECT_VISIBILITY',
+          value: "private" },
+        { key: 'CI_PROJECT_REPOSITORY_LANGUAGES',
+          value: project.repository_languages.map(&:name).join(',').downcase },
+        { key: 'CI_PROJECT_CLASSIFICATION_LABEL',
+          value: project.external_authorization_classification_label },
+        { key: 'CI_DEFAULT_BRANCH',
+          value: project.default_branch },
+        { key: 'CI_CONFIG_PATH',
+          value: project.ci_config_path_or_default },
+        { key: 'CI_PAGES_DOMAIN',
+          value: Gitlab.config.pages.host },
+        { key: 'CI_PAGES_URL',
+          value: project.pages_url },
+        { key: 'CI_API_V4_URL',
+          value: API::Helpers::Version.new('v4').root_url },
+        { key: 'CI_PIPELINE_IID',
+          value: pipeline.iid.to_s },
+        { key: 'CI_PIPELINE_SOURCE',
+          value: pipeline.source },
+        { key: 'CI_PIPELINE_CREATED_AT',
+          value: pipeline.created_at.iso8601 },
+        { key: 'CI_COMMIT_SHA',
+          value: job.sha },
+        { key: 'CI_COMMIT_SHORT_SHA',
+          value: job.short_sha },
+        { key: 'CI_COMMIT_BEFORE_SHA',
+          value: job.before_sha },
+        { key: 'CI_COMMIT_REF_NAME',
+          value: job.ref },
+        { key: 'CI_COMMIT_REF_SLUG',
+          value: job.ref_slug },
+        { key: 'CI_COMMIT_BRANCH',
+          value: job.ref },
+        { key: 'CI_COMMIT_MESSAGE',
+          value: pipeline.git_commit_message },
+        { key: 'CI_COMMIT_TITLE',
+          value: pipeline.git_commit_title },
+        { key: 'CI_COMMIT_DESCRIPTION',
+          value: pipeline.git_commit_description },
+        { key: 'CI_COMMIT_REF_PROTECTED',
+          value: (!!pipeline.protected_ref?).to_s },
+        { key: 'CI_COMMIT_TIMESTAMP',
+          value: pipeline.git_commit_timestamp },
+        { key: 'CI_COMMIT_AUTHOR',
+          value: pipeline.git_author_full_text },
+        { key: 'CI_BUILD_REF',
+          value: job.sha },
+        { key: 'CI_BUILD_BEFORE_SHA',
+          value: job.before_sha },
+        { key: 'CI_BUILD_REF_NAME',
+          value: job.ref },
+        { key: 'CI_BUILD_REF_SLUG',
+          value: job.ref_slug },
+        { key: 'YAML_VARIABLE',
+          value: 'value' },
+        { key: 'GITLAB_USER_ID',
+          value: user.id.to_s },
+        { key: 'GITLAB_USER_EMAIL',
+         value: user.email },
+        { key: 'GITLAB_USER_LOGIN',
+         value: user.username },
+        { key: 'GITLAB_USER_NAME',
+         value: user.name }
+      ].map { |var| var.merge(public: true, masked: false) }
+    end
 
     subject { builder.scoped_variables(job, environment: environment, dependencies: dependencies) }
 
-    it 'returns the expected variables' do
-      keys = %w[CI_JOB_NAME
-                CI_JOB_STAGE
-                CI_NODE_TOTAL
-                CI_BUILD_NAME
-                CI_BUILD_STAGE]
+    it { is_expected.to be_instance_of(Gitlab::Ci::Variables::Collection) }
+
+    it { expect(subject.to_runner_variables).to eq(predefined_variables) }
+
+    context 'variables ordering' do
+      def var(name, value)
+        { key: name, value: value.to_s, public: true, masked: false }
+      end
+
+      before do
+        allow(builder).to receive(:predefined_variables) { [var('A', 1), var('B', 1)] }
+        allow(project).to receive(:predefined_variables) { [var('B', 2), var('C', 2)] }
+        allow(pipeline).to receive(:predefined_variables) { [var('C', 3), var('D', 3)] }
+        allow(job).to receive(:runner) { double(predefined_variables: [var('D', 4), var('E', 4)]) }
+        allow(builder).to receive(:kubernetes_variables) { [var('E', 5), var('F', 5)] }
+        allow(builder).to receive(:deployment_variables) { [var('F', 6), var('G', 6)] }
+        allow(job).to receive(:yaml_variables) { [var('G', 7), var('H', 7)] }
+        allow(builder).to receive(:user_variables) { [var('H', 8), var('I', 8)] }
+        allow(job).to receive(:dependency_variables) { [var('I', 9), var('J', 9)] }
+        allow(builder).to receive(:secret_instance_variables) { [var('J', 10), var('K', 10)] }
+        allow(builder).to receive(:secret_group_variables) { [var('K', 11), var('L', 11)] }
+        allow(builder).to receive(:secret_project_variables) { [var('L', 12), var('M', 12)] }
+        allow(job).to receive(:trigger_request) { double(user_variables: [var('M', 13), var('N', 13)]) }
+        allow(pipeline).to receive(:variables) { [var('N', 14), var('O', 14)] }
+        allow(pipeline).to receive(:pipeline_schedule) { double(job_variables: [var('O', 15), var('P', 15)]) }
+      end
+
+      it 'returns variables in order depending on resource hierarchy' do
+        expect(subject.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),
+           var('H', 8), var('I', 8),
+           var('I', 9), var('J', 9),
+           var('J', 10), var('K', 10),
+           var('K', 11), var('L', 11),
+           var('L', 12), var('M', 12),
+           var('M', 13), var('N', 13),
+           var('N', 14), var('O', 14),
+           var('O', 15), var('P', 15)])
+      end
 
-      subject.map { |env| env[:key] }.tap do |names|
-        expect(names).to include(*keys)
+      it 'overrides duplicate keys depending on resource hierarchy' do
+        expect(subject.to_hash).to match(
+          'A' => '1', 'B' => '2',
+          'C' => '3', 'D' => '4',
+          'E' => '5', 'F' => '6',
+          'G' => '7', 'H' => '8',
+          'I' => '9', 'J' => '10',
+          'K' => '11', 'L' => '12',
+          'M' => '13', 'N' => '14',
+          'O' => '15', 'P' => '15')
       end
     end
   end

spec/models/ci/build_spec.rb

@@ -2824,7 +2824,7 @@ RSpec.describe Ci::Build do
             allow(build).to receive(:dependency_variables) { [job_dependency_var] }
             allow(build).to receive(:dependency_proxy_variables) { [dependency_proxy_var] }
 
-            allow(build.project)
+            allow(build.pipeline.project)
               .to receive(:predefined_variables) { [project_pre_var] }
 
             project.variables.create!(key: 'secret', value: 'value')
@@ -3124,7 +3124,7 @@ RSpec.describe Ci::Build do
 
       context 'when the branch is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
+          allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -3132,7 +3132,7 @@ RSpec.describe Ci::Build do
 
       context 'when the tag is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
+          allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -3171,7 +3171,7 @@ RSpec.describe Ci::Build do
 
       context 'when the branch is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
+          allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -3179,7 +3179,7 @@ RSpec.describe Ci::Build do
 
       context 'when the tag is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
+          allow(build.pipeline.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -3566,6 +3566,20 @@ RSpec.describe Ci::Build do
 
       build.scoped_variables
     end
+
+    context 'when variables builder is used' do
+      it 'returns the same variables' do
+        build.user = create(:user)
+
+        allow(build.pipeline).to receive(:use_variables_builder_definitions?).and_return(false)
+        legacy_variables = build.scoped_variables.to_hash
+
+        allow(build.pipeline).to receive(:use_variables_builder_definitions?).and_return(true)
+        new_variables = build.scoped_variables.to_hash
+
+        expect(new_variables).to eq(legacy_variables)
+      end
+    end
   end
 
   describe '#simple_variables_without_dependencies' do
@@ -3578,7 +3592,8 @@ RSpec.describe Ci::Build do
 
   shared_examples "secret CI variables" do
     context 'when ref is branch' do
-      let(:build) { create(:ci_build, ref: 'master', tag: false, project: project) }
+      let(:pipeline) { create(:ci_pipeline, project: project) }
+      let(:build) { create(:ci_build, ref: 'master', tag: false, pipeline: pipeline, project: project) }
 
       context 'when ref is protected' do
         before do
@@ -3594,7 +3609,8 @@ RSpec.describe Ci::Build do
     end
 
     context 'when ref is tag' do
-      let(:build) { create(:ci_build, ref: 'v1.1.0', tag: true, project: project) }
+      let(:pipeline) { create(:ci_pipeline, project: project) }
+      let(:build) { create(:ci_build, ref: 'v1.1.0', tag: true, pipeline: pipeline, project: project) }
 
       context 'when ref is protected' do
         before do
@@ -3692,8 +3708,6 @@ RSpec.describe Ci::Build do
         .and_return(project_variables)
     end
 
-    it { is_expected.to eq(project_variables) }
-
     context 'environment is nil' do
       let(:environment) { nil }
 
@@ -3701,6 +3715,35 @@ RSpec.describe Ci::Build do
     end
   end
 
+  describe '#user_variables' do
+    subject { build.user_variables.to_hash }
+
+    context 'with user' do
+      let(:expected_variables) do
+        {
+          'GITLAB_USER_EMAIL' => user.email,
+          'GITLAB_USER_ID' => user.id.to_s,
+          'GITLAB_USER_LOGIN' => user.username,
+          'GITLAB_USER_NAME' => user.name
+        }
+      end
+
+      before do
+        build.user = user
+      end
+
+      it { is_expected.to eq(expected_variables) }
+    end
+
+    context 'without user' do
+      before do
+        expect(build).to receive(:user).and_return(nil)
+      end
+
+      it { is_expected.to be_empty }
+    end
+  end
+
   describe '#any_unmet_prerequisites?' do
     let(:build) { create(:ci_build, :created) }