Extract build variables that depend on persistence

2 files changed, 39 insertions, 31 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 7e724de9e77..f8755e1a4ff 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -292,7 +292,9 @@ module Ci
     # All variables, including persisted environment variables.
     #
     def variables
-      scoped_variables
+      Gitlab::Ci::Variables::Collection.new
+        .concat(persisted_variables)
+        .concat(scoped_variables)
         .concat(persisted_environment_variables)
         .to_runner_variables
     end
@@ -579,6 +581,21 @@ module Ci
 
     CI_REGISTRY_USER = 'gitlab-ci-token'.freeze
 
+    def persisted_variables
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        return variables unless persisted?
+
+        variables
+          .append(key: 'CI_JOB_ID', value: id.to_s)
+          .append(key: 'CI_JOB_TOKEN', value: token, public: false)
+          .append(key: 'CI_BUILD_ID', value: id.to_s)
+          .append(key: 'CI_BUILD_TOKEN', value: token, public: false)
+          .append(key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER)
+          .append(key: 'CI_REGISTRY_PASSWORD', value: token, public: false)
+          .append(key: 'CI_REPOSITORY_URL', value: repo_url, public: false)
+      end
+    end
+
     def predefined_variables
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
         variables.append(key: 'CI', value: 'true')
@@ -587,16 +604,11 @@ module Ci
         variables.append(key: 'CI_SERVER_NAME', value: 'GitLab')
         variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION)
         variables.append(key: 'CI_SERVER_REVISION', value: Gitlab::REVISION)
-        variables.append(key: 'CI_JOB_ID', value: id.to_s) if persisted?
         variables.append(key: 'CI_JOB_NAME', value: name)
         variables.append(key: 'CI_JOB_STAGE', value: stage)
-        variables.append(key: 'CI_JOB_TOKEN', value: token, public: false) if persisted?
         variables.append(key: 'CI_COMMIT_SHA', value: sha)
         variables.append(key: 'CI_COMMIT_REF_NAME', value: ref)
         variables.append(key: 'CI_COMMIT_REF_SLUG', value: ref_slug)
-        variables.append(key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER) if persisted?
-        variables.append(key: 'CI_REGISTRY_PASSWORD', value: token, public: false) if persisted?
-        variables.append(key: 'CI_REPOSITORY_URL', value: repo_url, public: false) if persisted?
         variables.append(key: "CI_COMMIT_TAG", value: ref) if tag?
         variables.append(key: "CI_PIPELINE_TRIGGERED", value: 'true') if trigger_request
         variables.append(key: "CI_JOB_MANUAL", value: 'true') if action?
@@ -604,26 +616,8 @@ module Ci
       end
     end
 
-    def persisted_environment_variables
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        return variables unless persisted? && persisted_environment.present?
-
-        variables.concat(persisted_environment.predefined_variables)
-
-        # Here we're passing unexpanded environment_url for runner to expand,
-        # and we need to make sure that CI_ENVIRONMENT_NAME and
-        # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
-        variables.append(key: 'CI_ENVIRONMENT_URL', value: environment_url) if environment_url
-      end
-    end
-
     def legacy_variables
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        if persisted?
-          variables.append(key: 'CI_BUILD_ID', value: id.to_s)
-          variables.append(key: 'CI_BUILD_TOKEN', value: token, public: false)
-        end
-
         variables.append(key: 'CI_BUILD_REF', value: sha)
         variables.append(key: 'CI_BUILD_BEFORE_SHA', value: before_sha)
         variables.append(key: 'CI_BUILD_REF_NAME', value: ref)
@@ -636,6 +630,19 @@ module Ci
       end
     end
 
+    def persisted_environment_variables
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        return variables unless persisted? && persisted_environment.present?
+
+        variables.concat(persisted_environment.predefined_variables)
+
+        # Here we're passing unexpanded environment_url for runner to expand,
+        # and we need to make sure that CI_ENVIRONMENT_NAME and
+        # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
+        variables.append(key: 'CI_ENVIRONMENT_URL', value: environment_url) if environment_url
+      end
+    end
+
     def environment_url
       options&.dig(:environment, :url) || persisted_environment&.external_url
     end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 42b40ff91fc..4b9ddb3c831 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1463,24 +1463,24 @@ describe Ci::Build do
     let(:container_registry_enabled) { false }
     let(:predefined_variables) do
       [
+        { key: 'CI_JOB_ID', value: build.id.to_s, public: true },
+        { key: 'CI_JOB_TOKEN', value: build.token, public: false },
+        { key: 'CI_BUILD_ID', value: build.id.to_s, public: true },
+        { key: 'CI_BUILD_TOKEN', value: build.token, public: false },
+        { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true },
+        { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false },
+        { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false },
         { key: 'CI', value: 'true', public: true },
         { key: 'GITLAB_CI', value: 'true', public: true },
         { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true },
         { key: 'CI_SERVER_NAME', value: 'GitLab', public: true },
         { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true },
         { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true },
-        { key: 'CI_JOB_ID', value: build.id.to_s, public: true },
         { key: 'CI_JOB_NAME', value: 'test', public: true },
         { key: 'CI_JOB_STAGE', value: 'test', public: true },
-        { key: 'CI_JOB_TOKEN', value: build.token, public: false },
         { key: 'CI_COMMIT_SHA', value: build.sha, public: true },
         { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true },
         { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true },
-        { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true },
-        { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false },
-        { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false },
-        { key: 'CI_BUILD_ID', value: build.id.to_s, public: true },
-        { key: 'CI_BUILD_TOKEN', value: build.token, public: false },
         { key: 'CI_BUILD_REF', value: build.sha, public: true },
         { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true },
         { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true },
@@ -1945,6 +1945,7 @@ describe Ci::Build do
         before do
           allow(build).to receive(:predefined_variables) { [build_pre_var] }
           allow(build).to receive(:yaml_variables) { [build_yaml_var] }
+          allow(build).to receive(:persisted_variables) { [] }
 
           allow_any_instance_of(Project)
             .to receive(:predefined_variables) { [project_pre_var] }