Predefined variables collections / EE

app/models/ci/build.rb

@@ -255,23 +255,23 @@ module Ci
     # All variables, including those dependent on environment, which could
     # contain unexpanded variables.
     def variables(environment: persisted_environment)
-      variables = predefined_variables
-      variables += project.predefined_variables
-      variables += pipeline.predefined_variables
-      variables += runner.predefined_variables if runner
-      variables += project.container_registry_variables
-      variables += project.deployment_variables(environment: environment) if has_environment?
-      variables += project.auto_devops_variables
-      variables += yaml_variables
-      variables += user_variables
-      variables += project.group.secret_variables_for(ref, project).map(&:to_runner_variable) if project.group
-      variables += secret_variables(environment: environment)
-      variables += trigger_request.user_variables if trigger_request
-      variables += pipeline.variables.map(&:to_runner_variable)
-      variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule
-      variables += persisted_environment_variables if environment
-
-      variables
+      collection = Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        variables.concat(predefined_variables)
+        variables.concat(project.predefined_variables)
+        variables.concat(pipeline.predefined_variables)
+        variables.concat(runner.predefined_variables) if runner
+        variables.concat(project.deployment_variables(environment: environment)) if has_environment?
+        variables.concat(yaml_variables)
+        variables.concat(user_variables)
+        variables.concat(project.group.secret_variables_for(ref, project)) if project.group
+        variables.concat(secret_variables(environment: environment))
+        variables.concat(trigger_request.user_variables) if trigger_request
+        variables.concat(pipeline.variables)
+        variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule
+        variables.concat(persisted_environment_variables) if environment
+      end
+
+      collection.to_runner_variables
     end
 
     def features
@@ -439,14 +439,14 @@ module Ci
     end
 
     def user_variables
-      return [] if user.blank?
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        return variables if user.blank?
 
-      [
-        { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true },
-        { key: 'GITLAB_USER_EMAIL', value: user.email, public: true },
-        { key: 'GITLAB_USER_LOGIN', value: user.username, public: true },
-        { key: 'GITLAB_USER_NAME', value: user.name, public: true }
-      ]
+        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_variables(environment: persisted_environment)
@@ -549,60 +549,57 @@ module Ci
     CI_REGISTRY_USER = 'gitlab-ci-token'.freeze
 
     def predefined_variables
-      variables = [
-        { 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: id.to_s, public: true },
-        { key: 'CI_JOB_NAME', value: name, public: true },
-        { key: 'CI_JOB_STAGE', value: stage, public: true },
-        { key: 'CI_JOB_TOKEN', value: token, public: false },
-        { key: 'CI_COMMIT_SHA', value: sha, public: true },
-        { key: 'CI_COMMIT_REF_NAME', value: ref, public: true },
-        { key: 'CI_COMMIT_REF_SLUG', value: ref_slug, public: true },
-        { key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER, public: true },
-        { key: 'CI_REGISTRY_PASSWORD', value: token, public: false },
-        { key: 'CI_REPOSITORY_URL', value: repo_url, public: false }
-      ]
-
-      variables << { key: "CI_COMMIT_TAG", value: ref, public: true } if tag?
-      variables << { key: "CI_PIPELINE_TRIGGERED", value: 'true', public: true } if trigger_request
-      variables << { key: "CI_JOB_MANUAL", value: 'true', public: true } if action?
-      variables.concat(legacy_variables)
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        variables.append(key: 'CI', value: 'true')
+        variables.append(key: 'GITLAB_CI', value: 'true')
+        variables.append(key: 'GITLAB_FEATURES', value: project.namespace.features.join(','))
+        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)
+        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)
+        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)
+        variables.append(key: 'CI_REGISTRY_PASSWORD', value: token, public: false)
+        variables.append(key: 'CI_REPOSITORY_URL', value: repo_url, public: false)
+        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?
+        variables.concat(legacy_variables)
+      end
     end
 
     def persisted_environment_variables
-      return [] unless persisted_environment
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        return variables unless persisted_environment
 
-      variables = persisted_environment.predefined_variables
+        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 << { key: 'CI_ENVIRONMENT_URL', value: environment_url, public: true } if environment_url
-
-      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
-      variables = [
-        { key: 'CI_BUILD_ID', value: id.to_s, public: true },
-        { key: 'CI_BUILD_TOKEN', value: token, public: false },
-        { key: 'CI_BUILD_REF', value: sha, public: true },
-        { key: 'CI_BUILD_BEFORE_SHA', value: before_sha, public: true },
-        { key: 'CI_BUILD_REF_NAME', value: ref, public: true },
-        { key: 'CI_BUILD_REF_SLUG', value: ref_slug, public: true },
-        { key: 'CI_BUILD_NAME', value: name, public: true },
-        { key: 'CI_BUILD_STAGE', value: stage, public: true }
-      ]
-
-      variables << { key: "CI_BUILD_TAG", value: ref, public: true } if tag?
-      variables << { key: "CI_BUILD_TRIGGERED", value: 'true', public: true } if trigger_request
-      variables << { key: "CI_BUILD_MANUAL", value: 'true', public: true } if action?
-      variables
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        variables.append(key: 'CI_BUILD_ID', value: id.to_s)
+        variables.append(key: 'CI_BUILD_TOKEN', value: token, public: false)
+        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)
+        variables.append(key: 'CI_BUILD_REF_SLUG', value: ref_slug)
+        variables.append(key: 'CI_BUILD_NAME', value: name)
+        variables.append(key: 'CI_BUILD_STAGE', value: stage)
+        variables.append(key: "CI_BUILD_TAG", value: ref) if tag?
+        variables.append(key: "CI_BUILD_TRIGGERED", value: 'true') if trigger_request
+        variables.append(key: "CI_BUILD_MANUAL", value: 'true') if action?
+      end
     end
 
     def environment_url

app/models/project.rb

@@ -1586,29 +1586,31 @@ class Project < ActiveRecord::Base
   end
 
   def predefined_variables
-    [
-      { key: 'CI_PROJECT_ID', value: id.to_s, public: true },
-      { key: 'CI_PROJECT_NAME', value: path, public: true },
-      { key: 'CI_PROJECT_PATH', value: full_path, public: true },
-      { key: 'CI_PROJECT_PATH_SLUG', value: full_path_slug, public: true },
-      { key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path, public: true },
-      { key: 'CI_PROJECT_URL', value: web_url, public: true },
-      { key: 'CI_PROJECT_VISIBILITY', value: Gitlab::VisibilityLevel.string_level(visibility_level), public: true }
-    ]
+    visibility = Gitlab::VisibilityLevel.string_level(visibility_level)
+
+    Gitlab::Ci::Variables::Collection.new.tap do |variables|
+      variables.append(key: 'CI_PROJECT_ID', value: id.to_s)
+      variables.append(key: 'CI_PROJECT_NAME', value: path)
+      variables.append(key: 'CI_PROJECT_PATH', value: full_path)
+      variables.append(key: 'CI_PROJECT_PATH_SLUG', value: full_path_slug)
+      variables.append(key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path)
+      variables.append(key: 'CI_PROJECT_URL', value: web_url)
+      variables.append(key: 'CI_PROJECT_VISIBILITY', value: visibility)
+      variables.concat(container_registry_variables)
+      variables.concat(auto_devops_variables)
+    end
   end
 
   def container_registry_variables
-    return [] unless Gitlab.config.registry.enabled
+    Gitlab::Ci::Variables::Collection.new.tap do |variables|
+      return variables unless Gitlab.config.registry.enabled
 
-    variables = [
-      { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true }
-    ]
+      variables.append(key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port)
 
-    if container_registry_enabled?
-      variables << { key: 'CI_REGISTRY_IMAGE', value: container_registry_url, public: true }
+      if container_registry_enabled?
+        variables.append(key: 'CI_REGISTRY_IMAGE', value: container_registry_url)
+      end
     end
-
-    variables
   end
 
   def secret_variables_for(ref:, environment: nil)
@@ -1635,7 +1637,7 @@ class Project < ActiveRecord::Base
   def auto_devops_variables
     return [] unless auto_devops_enabled?
 
-    (auto_devops || build_auto_devops)&.variables
+    (auto_devops || build_auto_devops)&.predefined_variables
   end
 
   def append_or_update_attribute(name, value)

app/models/project_auto_devops.rb

@@ -14,9 +14,12 @@ class ProjectAutoDevops < ActiveRecord::Base
     domain.present? || instance_domain.present?
   end
 
-  def variables
-    variables = []
-    variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain.presence || instance_domain, public: true } if has_domain?
-    variables
+  def predefined_variables
+    Gitlab::Ci::Variables::Collection.new.tap do |variables|
+      if has_domain?
+        variables.append(key: 'AUTO_DEVOPS_DOMAIN',
+                         value: domain.presence || instance_domain)
+      end
+    end
   end
 end

app/models/project_services/kubernetes_service.rb

@@ -107,19 +107,15 @@ class KubernetesService < DeploymentService
   def predefined_variables
     config = YAML.dump(kubeconfig)
 
-    variables = [
-      { key: 'KUBE_URL', value: api_url, public: true },
-      { key: 'KUBE_TOKEN', value: token, public: false },
-      { key: 'KUBE_NAMESPACE', value: actual_namespace, public: true },
-      { key: 'KUBECONFIG', value: config, public: false, file: true }
-    ]
-
-    if ca_pem.present?
-      variables << { key: 'KUBE_CA_PEM', value: ca_pem, public: true }
-      variables << { key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true }
+    Gitlab::Ci::Variables::Collection.new.tap do |collection|
+      collection.append(key: 'KUBE_URL', value: api_url, public: true)
+      collection.append(key: 'KUBE_TOKEN', value: token, public: false)
+      collection.append(key: 'KUBE_NAMESPACE', value: actual_namespace, public: true)
+      collection.append(key: 'KUBECONFIG', value: config, public: false, file: true)
+
+      collection.append(key: 'KUBE_CA_PEM', value: ca_pem, public: true)
+      collection.append(key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true)
     end
-
-    variables
   end
 
   # Constructs a list of terminals from the reactive cache

lib/gitlab/ci/variables/collection.rb

+module Gitlab
+  module Ci
+    module Variables
+      class Collection
+        include Enumerable
+
+        def initialize(variables = [])
+          @variables = []
+
+          variables.each { |variable| self.append(variable) }
+        end
+
+        def append(resource)
+          @variables.append(Collection::Item.fabricate(resource))
+        end
+
+        def concat(resources)
+          resources.each { |variable| self.append(variable) }
+        end
+
+        def each
+          @variables.each { |variable| yield variable }
+        end
+
+        def +(other)
+          self.class.new.tap do |collection|
+            self.each { |variable| collection.append(variable) }
+            other.each { |variable| collection.append(variable) }
+          end
+        end
+
+        def to_runner_variables
+          self.map(&:to_hash)
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/variables/collection/item.rb

+module Gitlab
+  module Ci
+    module Variables
+      class Collection
+        class Item
+          def initialize(**options)
+            @variable = {
+              key: options.fetch(:key),
+              value: options.fetch(:value),
+              public: options.fetch(:public, true),
+              file: options.fetch(:files, false)
+            }
+          end
+
+          def [](key)
+            @variable.fetch(key)
+          end
+
+          def ==(other)
+            to_hash == self.class.fabricate(other).to_hash
+          end
+
+          ##
+          # If `file: true` has been provided we expose it, otherwise we
+          # don't expose `file` attribute at all (stems from what the runner
+          # expects).
+          #
+          def to_hash
+            @variable.reject do |hash_key, hash_value|
+              hash_key == :file && hash_value == false
+            end
+          end
+
+          def self.fabricate(resource)
+            case resource
+            when Hash
+              self.new(resource)
+            when ::HasVariable
+              self.new(resource.to_runner_variable)
+            when self
+              resource.dup
+            else
+              raise ArgumentError, "Unknown `#{resource.class}` variable resource!"
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/variables/collection/item_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Variables::Collection::Item do
+  let(:variable) do
+    { key: 'VAR', value: 'something', public: true }
+  end
+
+  describe '.fabricate' do
+    it 'supports using a hash' do
+      resource = described_class.fabricate(variable)
+
+      expect(resource).to be_a(described_class)
+      expect(resource).to eq variable
+    end
+
+    it 'supports using an active record resource' do
+      variable = create(:ci_variable, key: 'CI_VAR', value: '123')
+      resource = described_class.fabricate(variable)
+
+      expect(resource).to be_a(described_class)
+      expect(resource).to eq(key: 'CI_VAR', value: '123', public: false)
+    end
+
+    it 'supports using another collection item' do
+      item = described_class.new(**variable)
+
+      resource = described_class.fabricate(item)
+
+      expect(resource).to be_a(described_class)
+      expect(resource).to eq variable
+      expect(resource.object_id).not_to eq item.object_id
+    end
+  end
+
+  describe '#==' do
+    it 'compares a hash representation of a variable' do
+      expect(described_class.new(**variable) == variable).to be true
+    end
+  end
+
+  describe '#[]' do
+    it 'behaves like a hash accessor' do
+      item = described_class.new(**variable)
+
+      expect(item[:key]).to eq 'VAR'
+    end
+  end
+
+  describe '#to_hash' do
+    it 'returns a hash representation of a collection item' do
+      expect(described_class.new(**variable).to_hash).to eq variable
+    end
+  end
+end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Variables::Collection do
+  describe '.new' do
+    it 'can be initialized with an array' do
+      variable = { key: 'VAR', value: 'value', public: true }
+
+      collection = described_class.new([variable])
+
+      expect(collection.first.to_hash).to eq variable
+    end
+
+    it 'can be initialized without an argument' do
+      expect(subject).to be_none
+    end
+  end
+
+  describe '#append' do
+    it 'appends a hash' do
+      subject.append(key: 'VARIABLE', value: 'something')
+
+      expect(subject).to be_one
+    end
+
+    it 'appends a Ci::Variable' do
+      subject.append(build(:ci_variable))
+
+      expect(subject).to be_one
+    end
+
+    it 'appends an internal resource' do
+      collection = described_class.new([{ key: 'TEST', value: 1 }])
+
+      subject.append(collection.first)
+
+      expect(subject).to be_one
+    end
+  end
+
+  describe '#concat' do
+    it 'appends all elements from an array' do
+      collection = described_class.new([{ key: 'VAR_1', value: '1' }])
+      variables = [{ key: 'VAR_2', value: '2' }, { key: 'VAR_3', value: '3' }]
+
+      collection.concat(variables)
+
+      expect(collection).to include(key: 'VAR_1', value: '1', public: true)
+      expect(collection).to include(key: 'VAR_2', value: '2', public: true)
+      expect(collection).to include(key: 'VAR_3', value: '3', public: true)
+    end
+
+    it 'appends all elements from other collection' do
+      collection = described_class.new([{ key: 'VAR_1', value: '1' }])
+      additional = described_class.new([{ key: 'VAR_2', value: '2' },
+                                        { key: 'VAR_3', value: '3' }])
+
+      collection.concat(additional)
+
+      expect(collection).to include(key: 'VAR_1', value: '1', public: true)
+      expect(collection).to include(key: 'VAR_2', value: '2', public: true)
+      expect(collection).to include(key: 'VAR_3', value: '3', public: true)
+    end
+  end
+
+  describe '#+' do
+    it 'makes it possible to combine with an array' do
+      collection = described_class.new([{ key: 'TEST', value: 1 }])
+      variables = [{ key: 'TEST', value: 'something' }]
+
+      expect((collection + variables).count).to eq 2
+    end
+
+    it 'makes it possible to combine with another collection' do
+      collection = described_class.new([{ key: 'TEST', value: 1 }])
+      other = described_class.new([{ key: 'TEST', value: 2 }])
+
+      expect((collection + other).count).to eq 2
+    end
+  end
+
+  describe '#to_runner_variables' do
+    it 'creates an array of hashes in a runner-compatible format' do
+      collection = described_class.new([{ key: 'TEST', value: 1 }])
+
+      expect(collection.to_runner_variables)
+        .to eq [{ key: 'TEST', value: 1, public: true }]
+    end
+  end
+end

spec/models/ci/build_spec.rb

@@ -1855,10 +1855,10 @@ describe Ci::Build do
     end
 
     context 'returns variables in valid order' do
-      let(:build_pre_var) { { key: 'build', value: 'value' } }
-      let(:project_pre_var) { { key: 'project', value: 'value' } }
-      let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } }
-      let(:build_yaml_var) { { key: 'yaml', value: 'value' } }
+      let(:build_pre_var) { { key: 'build', value: 'value', public: true } }
+      let(:project_pre_var) { { key: 'project', value: 'value', public: true } }
+      let(:pipeline_pre_var) { { key: 'pipeline', value: 'value', public: true } }
+      let(:build_yaml_var) { { key: 'yaml', value: 'value', public: true } }
 
       before do
         allow(build).to receive(:predefined_variables) { [build_pre_var] }

spec/models/project_auto_devops_spec.rb

@@ -36,14 +36,14 @@ describe ProjectAutoDevops do
     end
   end
 
-  describe '#variables' do
+  describe '#predefined_variables' do
     let(:auto_devops) { build_stubbed(:project_auto_devops, project: project, domain: domain) }
 
     context 'when domain is defined' do
       let(:domain) { 'example.com' }
 
       it 'returns AUTO_DEVOPS_DOMAIN' do
-        expect(auto_devops.variables).to include(domain_variable)
+        expect(auto_devops.predefined_variables).to include(domain_variable)
       end
     end
 
@@ -55,7 +55,7 @@ describe ProjectAutoDevops do
           allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return('example.com')
         end
 
-        it { expect(auto_devops.variables).to include(domain_variable) }
+        it { expect(auto_devops.predefined_variables).to include(domain_variable) }
       end
 
       context 'when there is no instance domain specified' do
@@ -63,7 +63,7 @@ describe ProjectAutoDevops do
           allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return(nil)
         end
 
-        it { expect(auto_devops.variables).not_to include(domain_variable) }
+        it { expect(auto_devops.predefined_variables).not_to include(domain_variable) }
       end
     end