From 82ed2a0909823807f3ece2cb29bfc501293361a0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 6 Sep 2017 18:57:07 +0200 Subject: [PATCH] Improve config source handling code --- app/models/ci/build.rb | 1 + app/models/ci/pipeline.rb | 19 ++++++++++--------- app/models/project.rb | 14 +++++++------- app/models/project_auto_devops.rb | 6 ++++++ spec/models/ci/pipeline_spec.rb | 14 +++++++++----- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 28c16d4037f..c439997d636 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -217,6 +217,7 @@ module Ci variables += runner.predefined_variables if runner variables += project.container_registry_variables variables += project.deployment_variables 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 diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index e72843d085e..0e1b3b29e07 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -325,15 +325,11 @@ module Ci end def set_config_source - self.config_source = - if project - case - when ci_yaml_from_repo then :repository_source - when implied_ci_yaml_file then :auto_devops_source - end - else - :unknown_source - end + if ci_yaml_from_repo + self.config_source = :repository_source + elsif implied_ci_yaml_file + self.config_source = :auto_devops_source + end end def config_processor @@ -461,12 +457,17 @@ module Ci private def ci_yaml_from_repo + return unless project + return unless sha + project.repository.gitlab_ci_yml_for(sha, ci_yaml_file_path) rescue GRPC::NotFound, Rugged::ReferenceError, GRPC::Internal nil end def implied_ci_yaml_file + return unless project + if project.auto_devops_enabled? Gitlab::Template::GitlabCiYmlTemplate.find('Auto-DevOps').content end diff --git a/app/models/project.rb b/app/models/project.rb index 9c1a2a15d2a..f8cc27d5569 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1390,7 +1390,7 @@ class Project < ActiveRecord::Base end def predefined_variables - 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 }, @@ -1398,12 +1398,6 @@ class Project < ActiveRecord::Base { key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: web_url, public: true } ] - - if auto_devops_enabled? && auto_devops&.domain - variables << { key: 'AUTO_DEVOPS_DOMAIN', value: auto_devops.domain, public: true } - end - - variables end def container_registry_variables @@ -1440,6 +1434,12 @@ class Project < ActiveRecord::Base deployment_service.predefined_variables end + def auto_devops_variables + return [] unless auto_devops_enabled? + + auto_devops&.variables || [] + end + def append_or_update_attribute(name, value) old_values = public_send(name.to_s) # rubocop:disable GitlabSecurity/PublicSend diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index d39273216d6..c54e421957b 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -2,4 +2,10 @@ class ProjectAutoDevops < ActiveRecord::Base belongs_to :project validates :domain, presence: true, hostname: { allow_numeric_hostname: true }, if: :enabled? + + def variables + variables = [] + variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain, public: true } if domain.present? + variables + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ff8f476d482..8b36fffd808 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -832,10 +832,10 @@ describe Ci::Pipeline, :mailer do describe '#ci_yaml_file' do let(:implied_yml) { Gitlab::Template::GitlabCiYmlTemplate.find('Auto-DevOps').content } - before { pipeline.detect_ci_yaml_file } - context 'the source is unknown' do - before { pipeline.unknown_source! } + before do + pipeline.unknown_source! + end it 'returns the configuration if found' do allow(pipeline.project.repository).to receive(:gitlab_ci_yml_for) @@ -854,7 +854,9 @@ describe Ci::Pipeline, :mailer do end context 'the source is the repository' do - before { pipeline.repository_source! } + before do + pipeline.repository_source! + end it 'returns the configuration if found' do allow(pipeline.project.repository).to receive(:gitlab_ci_yml_for) @@ -867,7 +869,9 @@ describe Ci::Pipeline, :mailer do end context 'when the source is auto_devops_source' do - before { pipeline.auto_devops_source! } + before do + pipeline.auto_devops_source! + end it 'finds the implied config' do allow_any_instance_of(ApplicationSetting)