DRY project-level predefined variables

5 files changed, 26 insertions, 20 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index d4b5d964846..a3a289c3a12 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -257,12 +257,10 @@ module Ci
         variables.concat(project.predefined_variables)
         variables.concat(pipeline.predefined_variables)
         variables.concat(runner.predefined_variables) if runner
-        variables.concat(project.container_registry_variables)
         variables.concat(project.deployment_variables) if has_environment?
-        variables.concat(project.auto_devops_variables)
         variables.concat(yaml_variables)
         variables.concat(user_variables)
-        variables.concat(project.group.secret_variables_for(ref, project).map(&:to_runner_variable)) if project.group
+        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)
diff --git a/app/models/project.rb b/app/models/project.rb
index 9ca859c5879..85a4d570e9a 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1581,21 +1581,21 @@ class Project < ActiveRecord::Base
       variables.append(key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path, public: true)
       variables.append(key: 'CI_PROJECT_URL', value: web_url, public: true)
       variables.append(key: 'CI_PROJECT_VISIBILITY', value: visibility, public: true)
+      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, public: true)
 
-    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, public: true)
+      end
     end
-
-    variables
   end
 
   def secret_variables_for(ref:, environment: nil)
@@ -1624,7 +1624,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)
diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb
index 112ed7ed434..519bd2fe2ac 100644
--- a/app/models/project_auto_devops.rb
+++ b/app/models/project_auto_devops.rb
@@ -14,9 +14,11 @@ 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|
+      variables.append(key: 'AUTO_DEVOPS_DOMAIN',
+                       value: domain.presence || instance_domain,
+                       public: true) if has_domain?
+    end
   end
 end
diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb
index d6540e86fc5..1ed07cedd55 100644
--- a/lib/gitlab/ci/variables/collection/item.rb
+++ b/lib/gitlab/ci/variables/collection/item.rb
@@ -14,6 +14,10 @@ module Gitlab
             }
           end
 
+          def [](key)
+            @variable.fetch(key)
+          end
+
           def ==(other)
             to_hash == self.class.fabricate(other).to_hash
           end
@@ -39,6 +43,8 @@ module Gitlab
               self.new(resource.to_hash)
             when ::Ci::PipelineVariable
               self.new(resource.to_hash)
+            when ::Ci::GroupVariable
+              self.new(resource.to_hash)
             when self
               resource.dup
             else
diff --git a/spec/models/project_auto_devops_spec.rb b/spec/models/project_auto_devops_spec.rb
index 296b91a771c..7545c0797e9 100644
--- a/spec/models/project_auto_devops_spec.rb
+++ b/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