From c3e33f06c2920a9f032ee8166cccf8423bd24b78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 19:57:57 +0200 Subject: [PATCH 01/23] Build barebones for ExternalFiles libraries CE mirror of 30ca00f17034f654403ec7cb5dc370d1e5db8152 --- lib/gitlab/ci/config/entry/global.rb | 10 +- lib/gitlab/ci/config/entry/includes.rb | 22 +++ lib/gitlab/ci/config/entry/validators.rb | 32 ++++ lib/gitlab/ci/external_files/external_file.rb | 38 +++++ lib/gitlab/ci/external_files/mapper.rb | 27 ++++ lib/gitlab/ci/external_files/processor.rb | 46 ++++++ .../external_files/.gitlab-ci-template-1.yml | 10 ++ .../external_files/.gitlab-ci-template-2.yml | 8 + .../lib/gitlab/ci/config/entry/global_spec.rb | 6 +- .../gitlab/ci/config/entry/includes_spec.rb | 97 ++++++++++++ .../ci/external_files/external_file_spec.rb | 78 ++++++++++ .../gitlab/ci/external_files/mapper_spec.rb | 39 +++++ .../ci/external_files/processor_spec.rb | 141 ++++++++++++++++++ 13 files changed, 550 insertions(+), 4 deletions(-) create mode 100644 lib/gitlab/ci/config/entry/includes.rb create mode 100644 lib/gitlab/ci/external_files/external_file.rb create mode 100644 lib/gitlab/ci/external_files/mapper.rb create mode 100644 lib/gitlab/ci/external_files/processor.rb create mode 100644 spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml create mode 100644 spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml create mode 100644 spec/lib/gitlab/ci/config/entry/includes_spec.rb create mode 100644 spec/lib/gitlab/ci/external_files/external_file_spec.rb create mode 100644 spec/lib/gitlab/ci/external_files/mapper_spec.rb create mode 100644 spec/lib/gitlab/ci/external_files/processor_spec.rb diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb index a4ec8f0ff2f..401e566de77 100644 --- a/lib/gitlab/ci/config/entry/global.rb +++ b/lib/gitlab/ci/config/entry/global.rb @@ -33,11 +33,15 @@ module Gitlab entry :cache, Entry::Cache, description: 'Configure caching between build jobs.' + entry :includes, Entry::Includes, + description: 'External GitlLab Ci files' + helpers :before_script, :image, :services, :after_script, - :variables, :stages, :types, :cache, :jobs + :variables, :stages, :types, :cache, :jobs, :includes def compose!(_deps = nil) super(self) do + append_external_files! compose_jobs! compose_deprecated_entries! end @@ -45,6 +49,10 @@ module Gitlab private + def append_external_files! + return if includes_value.nil? + end + def compose_jobs! factory = Entry::Factory.new(Entry::Jobs) .value(@config.except(*self.class.nodes.keys)) diff --git a/lib/gitlab/ci/config/entry/includes.rb b/lib/gitlab/ci/config/entry/includes.rb new file mode 100644 index 00000000000..c7a8c7960e8 --- /dev/null +++ b/lib/gitlab/ci/config/entry/includes.rb @@ -0,0 +1,22 @@ +module Gitlab + module Ci + class Config + module Entry + ## + # Entry that represents a Docker image. + # + class Includes < Node + include Validatable + + validations do + validates :config, array_or_string: true, external_file: true, allow_nil: true + end + + def value + Array(@config) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb index b3c889ee92f..5b4d5d53821 100644 --- a/lib/gitlab/ci/config/entry/validators.rb +++ b/lib/gitlab/ci/config/entry/validators.rb @@ -60,6 +60,38 @@ module Gitlab end end + class ArrayOrStringValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + unless value.is_a?(Array) || value.is_a?(String) + record.errors.add(attribute, 'should be an array or a string') + end + end + end + + class ExternalFileValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + if value.is_a?(Array) + value.each do |path| + validate_external_file(path, record, attribute) + end + else + validate_external_file(value, record, attribute) + end + end + + private + + def validate_external_file(value, record, attribute) + unless valid_url?(value) + record.errors.add(attribute, 'should be a valid local or remote file') + end + end + + def valid_url?(value) + Gitlab::UrlSanitizer.valid?(value) || File.exists?("#{Rails.root}/#{value}") + end + end + class KeyValidator < ActiveModel::EachValidator include LegacyValidationHelpers diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb new file mode 100644 index 00000000000..9bad8231c42 --- /dev/null +++ b/lib/gitlab/ci/external_files/external_file.rb @@ -0,0 +1,38 @@ +require 'open-uri' + +module Gitlab + module Ci + module ExternalFiles + class ExternalFile + + def initialize(value) + @value = value + end + + def content + if remote_url? + open(value).read + else + File.read(base_path) + end + end + + def valid? + remote_url? || File.exists?(base_path) + end + + private + + attr_reader :value + + def base_path + "#{Rails.root}/#{value}" + end + + def remote_url? + ::Gitlab::UrlSanitizer.valid?(value) + end + end + end + end +end diff --git a/lib/gitlab/ci/external_files/mapper.rb b/lib/gitlab/ci/external_files/mapper.rb new file mode 100644 index 00000000000..5deb2f5a2e5 --- /dev/null +++ b/lib/gitlab/ci/external_files/mapper.rb @@ -0,0 +1,27 @@ +module Gitlab + module Ci + module ExternalFiles + class Mapper + + def self.fetch_paths(values) + paths = values.fetch(:includes, []) + normalize_paths(paths) + end + + private + + def self.normalize_paths(paths) + if paths.is_a?(String) + [build_external_file(paths)] + else + paths.map { |path| build_external_file(path) } + end + end + + def self.build_external_file(path) + ::Gitlab::Ci::ExternalFiles::ExternalFile.new(path) + end + end + end + end +end diff --git a/lib/gitlab/ci/external_files/processor.rb b/lib/gitlab/ci/external_files/processor.rb new file mode 100644 index 00000000000..f38bc7633b5 --- /dev/null +++ b/lib/gitlab/ci/external_files/processor.rb @@ -0,0 +1,46 @@ +module Gitlab + module Ci + module ExternalFiles + class Processor + ExternalFileError = Class.new(StandardError) + + def initialize(values) + @values = values + @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.fetch_paths(values) + end + + def perform + return values if external_files.empty? + + external_files.each do |external_file| + validate_external_file(external_file) + append_external_content(external_file) + end + + remove_include_keyword + end + + private + + attr_reader :values, :external_files + + def validate_external_file(external_file) + unless external_file.valid? + raise ExternalFileError, 'External files should be a valid local or remote file' + end + end + + def append_external_content(external_file) + external_values = ::Gitlab::Ci::Config::Loader.new(external_file.content).load! + @values.merge!(external_values) + end + + def remove_include_keyword + values.delete(:includes) + values + end + + end + end + end +end diff --git a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml new file mode 100644 index 00000000000..0bab94a7c2e --- /dev/null +++ b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml @@ -0,0 +1,10 @@ +before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + +rspec: + script: + - bundle exec rspec diff --git a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml new file mode 100644 index 00000000000..b341cca8946 --- /dev/null +++ b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml @@ -0,0 +1,8 @@ +variables: + # AUTO_DEVOPS_DOMAIN is the application deployment domain and should be set as a variable at the group or project level. + + AUTO_DEVOPS_DOMAIN: domain.example.com + POSTGRES_USER: user + POSTGRES_PASSWORD: testing-password + POSTGRES_ENABLED: "true" + POSTGRES_DB: $CI_ENVIRONMENT_SLUG diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb index 1860ed79bfd..ff623b95be8 100644 --- a/spec/lib/gitlab/ci/config/entry/global_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Ci::Config::Entry::Global do expect(described_class.nodes.keys) .to match_array(%i[before_script image services after_script variables stages - types cache]) + types cache includes]) end end end @@ -42,7 +42,7 @@ describe Gitlab::Ci::Config::Entry::Global do end it 'creates node object for each entry' do - expect(global.descendants.count).to eq 8 + expect(global.descendants.count).to eq 9 end it 'creates node object using valid class' do @@ -189,7 +189,7 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#nodes' do it 'instantizes all nodes' do - expect(global.descendants.count).to eq 8 + expect(global.descendants.count).to eq 9 end it 'contains unspecified nodes' do diff --git a/spec/lib/gitlab/ci/config/entry/includes_spec.rb b/spec/lib/gitlab/ci/config/entry/includes_spec.rb new file mode 100644 index 00000000000..d72503535ed --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/includes_spec.rb @@ -0,0 +1,97 @@ +require 'rails_helper' + +describe Gitlab::Ci::Config::Entry::Includes do + let(:entry) { described_class.new(config) } + + shared_examples 'valid external file' do + it 'should be valid' do + expect(entry).to be_valid + end + + it 'should not return any error' do + expect(entry.errors).to be_empty + end + end + + shared_examples 'invalid external file' do + it 'should not be valid' do + expect(entry).not_to be_valid + end + + it 'should return an error' do + expect(entry.errors.first).to match(/should be a valid local or remote file/) + end + end + + describe "#valid?" do + context 'with no external file given' do + let(:config) { nil } + + it_behaves_like 'valid external file' + end + + context 'with multiple external files' do + let(:config) { %w(https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-2.yml) } + + it_behaves_like 'valid external file' + end + + context 'with just one external file' do + let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it_behaves_like 'valid external file' + end + + context 'when they contain valid URLs' do + let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it_behaves_like 'valid external file' + end + + context 'when they contain valid relative URLs' do + let(:config) { '/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml' } + + it_behaves_like 'valid external file' + end + + context 'when they not contain valid URLs' do + let(:config) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it_behaves_like 'invalid external file' + end + + context 'when they not contain valid relative URLs' do + let(:config) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + + it_behaves_like 'invalid external file' + end + end + + describe "#value" do + context 'with multiple external files' do + let(:config) { %w(https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-2.yml) } + it 'should return an array' do + expect(entry.value).to be_an(Array) + expect(entry.value.count).to eq(2) + end + end + + context 'with just one external file' do + let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it 'should return an array' do + expect(entry.value).to be_an(Array) + expect(entry.value.count).to eq(1) + end + end + + context 'with no external file given' do + let(:config) { nil } + + it 'should return an empty array' do + expect(entry.value).to be_an(Array) + expect(entry.value).to be_empty + end + end + end +end diff --git a/spec/lib/gitlab/ci/external_files/external_file_spec.rb b/spec/lib/gitlab/ci/external_files/external_file_spec.rb new file mode 100644 index 00000000000..80468f63435 --- /dev/null +++ b/spec/lib/gitlab/ci/external_files/external_file_spec.rb @@ -0,0 +1,78 @@ +require 'rails_helper' + +describe Gitlab::Ci::ExternalFiles::ExternalFile do + let(:external_file) { described_class.new(value) } + + describe "#valid?" do + context 'when is a valid remote url' do + let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it 'should return true' do + expect(external_file.valid?).to be_truthy + end + end + + context 'when is not a valid remote url' do + let(:value) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it 'should return false' do + expect(external_file.valid?).to be_falsy + end + end + + context 'when is a valid local path' do + let(:value) { '/vendor/gitlab-ci-yml/existent-file.yml' } + + it 'should return true' do + allow(File).to receive(:exists?).and_return(true) + expect(external_file.valid?).to be_truthy + end + end + + context 'when is not a valid local path' do + let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + + it 'should return false' do + expect(external_file.valid?).to be_falsy + end + end + end + + describe "#content" do + let(:external_file_content) { + <<-HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + } + + context 'with a local file' do + let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + + before do + allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:read).and_return(external_file_content) + end + + it 'should return the content of the file' do + expect(external_file.content).to eq(external_file_content) + end + end + + context 'with a valid remote file' do + let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + before do + allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(external_file_content) + end + + it 'should return the content of the file' do + expect(external_file.content).to eq(external_file_content) + end + end + end +end diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb new file mode 100644 index 00000000000..dfbf8bfa098 --- /dev/null +++ b/spec/lib/gitlab/ci/external_files/mapper_spec.rb @@ -0,0 +1,39 @@ +require 'rails_helper' + +describe Gitlab::Ci::ExternalFiles::Mapper do + describe '.fetch_paths' do + context 'when includes is defined as string' do + let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2'} } + + it 'returns an array' do + expect(described_class.fetch_paths(values)).to be_an(Array) + end + + it 'returns ExternalFile instances' do + expect(described_class.fetch_paths(values).first).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) + end + end + + context 'when includes is defined as an array' do + let(:values) { { includes: ['https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', '/vendor/gitlab-ci-yml/template.yml'], image: 'ruby:2.2'} } + it 'returns an array' do + expect(described_class.fetch_paths(values)).to be_an(Array) + end + + it 'returns ExternalFile instances' do + paths = described_class.fetch_paths(values) + paths.each do |path| + expect(path).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) + end + end + end + + context 'when includes is not defined' do + let(:values) { { image: 'ruby:2.2'} } + + it 'returns an empty array' do + expect(described_class.fetch_paths(values)).to be_empty + end + end + end +end diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external_files/processor_spec.rb new file mode 100644 index 00000000000..51ed14de4cf --- /dev/null +++ b/spec/lib/gitlab/ci/external_files/processor_spec.rb @@ -0,0 +1,141 @@ +require 'rails_helper' + +describe Gitlab::Ci::ExternalFiles::Processor do + let(:processor) { described_class.new(values) } + + describe "#perform" do + context 'when no external files defined' do + let(:values) { { image: 'ruby:2.2' } } + + it 'should return the same values' do + expect(processor.perform).to eq(values) + end + end + + context 'when an invalid local file is defined' do + let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2'} } + + it 'should raise an error' do + expect { processor.perform }.to raise_error(described_class::ExternalFileError) + end + end + + context 'when an invalid remote file is defined' do + let(:values) { { includes: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2'} } + + it 'should raise an error' do + expect { processor.perform }.to raise_error(described_class::ExternalFileError) + end + end + + context 'with a valid remote external file is defined' do + let(:values) { { includes: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } + let(:external_file_content) { + <<-HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + + rspec: + script: + - bundle exec rspec + + rubocop: + script: + - bundle exec rubocop + HEREDOC + } + + before do + allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(external_file_content) + end + + it 'should append the file to the values' do + output = processor.perform + expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop]) + end + + it "should remove the 'includes' keyword" do + expect(processor.perform[:includes]).to be_nil + end + end + + context 'with a valid local external file is defined' do + let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml' , image: 'ruby:2.2'} } + let(:external_file_content) { + <<-HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + } + + before do + allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:read).and_return(external_file_content) + end + + it 'should append the file to the values' do + output = processor.perform + expect(output.keys).to match_array([:image, :before_script]) + end + + it "should remove the 'includes' keyword" do + expect(processor.perform[:includes]).to be_nil + end + end + + context 'with multiple external files are defined' do + let(:external_files) { + [ + "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml", + "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml", + 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' + ] + } + let(:values) { { includes: external_files, image: 'ruby:2.2'} } + + let(:remote_file_content) { + <<-HEREDOC + stages: + - build + - review + - cleanup + HEREDOC + } + + before do + allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(remote_file_content) + end + + it 'should append the files to the values' do + expect(processor.perform.keys).to match_array([:image, :variables, :stages, :before_script, :rspec]) + end + + it "should remove the 'includes' keyword" do + expect(processor.perform[:includes]).to be_nil + end + end + + context 'when external files are defined but not valid' do + let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2'} } + + let(:external_file_content) { 'invalid content file ////' } + + before do + allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:read).and_return(external_file_content) + end + + it 'should raise an error' do + expect { processor.perform }.to raise_error(Gitlab::Ci::Config::Loader::FormatError) + end + end + end +end From 5f33690ebbe19dd8698bf8b432a5e6740305f007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 20:53:28 +0200 Subject: [PATCH 02/23] Load external files in config CE mirror of 8e03a6619be44fdaf19a6c13284ea8e51377b311 --- lib/gitlab/ci/config.rb | 6 +- lib/gitlab/ci/config/entry/global.rb | 10 +- lib/gitlab/ci/config/entry/includes.rb | 22 ----- lib/gitlab/ci/config/entry/validators.rb | 32 ------ lib/gitlab/ci/external_files/external_file.rb | 3 +- lib/gitlab/ci/external_files/mapper.rb | 3 - lib/gitlab/ci/external_files/processor.rb | 3 +- .../lib/gitlab/ci/config/entry/global_spec.rb | 6 +- .../gitlab/ci/config/entry/includes_spec.rb | 97 ------------------- spec/lib/gitlab/ci/config_spec.rb | 57 +++++++++++ .../ci/external_files/external_file_spec.rb | 6 +- .../gitlab/ci/external_files/mapper_spec.rb | 10 +- .../ci/external_files/processor_spec.rb | 30 +++--- 13 files changed, 89 insertions(+), 196 deletions(-) delete mode 100644 lib/gitlab/ci/config/entry/includes.rb delete mode 100644 spec/lib/gitlab/ci/config/entry/includes_spec.rb diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 46dad59eb8c..94b2fbc082b 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -1,15 +1,17 @@ module Gitlab module Ci - ## + # # Base GitLab CI Configuration facade # class Config ConfigError = Class.new(StandardError) def initialize(config, opts = {}) - @config = Config::Extendable + initial_config = Config::Extendable .new(build_config(config, opts)) .to_hash + processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config) + @config = processor.perform @global = Entry::Global.new(@config) @global.compose! diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb index 401e566de77..a4ec8f0ff2f 100644 --- a/lib/gitlab/ci/config/entry/global.rb +++ b/lib/gitlab/ci/config/entry/global.rb @@ -33,15 +33,11 @@ module Gitlab entry :cache, Entry::Cache, description: 'Configure caching between build jobs.' - entry :includes, Entry::Includes, - description: 'External GitlLab Ci files' - helpers :before_script, :image, :services, :after_script, - :variables, :stages, :types, :cache, :jobs, :includes + :variables, :stages, :types, :cache, :jobs def compose!(_deps = nil) super(self) do - append_external_files! compose_jobs! compose_deprecated_entries! end @@ -49,10 +45,6 @@ module Gitlab private - def append_external_files! - return if includes_value.nil? - end - def compose_jobs! factory = Entry::Factory.new(Entry::Jobs) .value(@config.except(*self.class.nodes.keys)) diff --git a/lib/gitlab/ci/config/entry/includes.rb b/lib/gitlab/ci/config/entry/includes.rb deleted file mode 100644 index c7a8c7960e8..00000000000 --- a/lib/gitlab/ci/config/entry/includes.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Gitlab - module Ci - class Config - module Entry - ## - # Entry that represents a Docker image. - # - class Includes < Node - include Validatable - - validations do - validates :config, array_or_string: true, external_file: true, allow_nil: true - end - - def value - Array(@config) - end - end - end - end - end -end diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb index 5b4d5d53821..b3c889ee92f 100644 --- a/lib/gitlab/ci/config/entry/validators.rb +++ b/lib/gitlab/ci/config/entry/validators.rb @@ -60,38 +60,6 @@ module Gitlab end end - class ArrayOrStringValidator < ActiveModel::EachValidator - def validate_each(record, attribute, value) - unless value.is_a?(Array) || value.is_a?(String) - record.errors.add(attribute, 'should be an array or a string') - end - end - end - - class ExternalFileValidator < ActiveModel::EachValidator - def validate_each(record, attribute, value) - if value.is_a?(Array) - value.each do |path| - validate_external_file(path, record, attribute) - end - else - validate_external_file(value, record, attribute) - end - end - - private - - def validate_external_file(value, record, attribute) - unless valid_url?(value) - record.errors.add(attribute, 'should be a valid local or remote file') - end - end - - def valid_url?(value) - Gitlab::UrlSanitizer.valid?(value) || File.exists?("#{Rails.root}/#{value}") - end - end - class KeyValidator < ActiveModel::EachValidator include LegacyValidationHelpers diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb index 9bad8231c42..47d6d1c0fdc 100644 --- a/lib/gitlab/ci/external_files/external_file.rb +++ b/lib/gitlab/ci/external_files/external_file.rb @@ -4,7 +4,6 @@ module Gitlab module Ci module ExternalFiles class ExternalFile - def initialize(value) @value = value end @@ -18,7 +17,7 @@ module Gitlab end def valid? - remote_url? || File.exists?(base_path) + remote_url? || File.exist?(base_path) end private diff --git a/lib/gitlab/ci/external_files/mapper.rb b/lib/gitlab/ci/external_files/mapper.rb index 5deb2f5a2e5..4b5d2ddd6a1 100644 --- a/lib/gitlab/ci/external_files/mapper.rb +++ b/lib/gitlab/ci/external_files/mapper.rb @@ -2,14 +2,11 @@ module Gitlab module Ci module ExternalFiles class Mapper - def self.fetch_paths(values) paths = values.fetch(:includes, []) normalize_paths(paths) end - private - def self.normalize_paths(paths) if paths.is_a?(String) [build_external_file(paths)] diff --git a/lib/gitlab/ci/external_files/processor.rb b/lib/gitlab/ci/external_files/processor.rb index f38bc7633b5..221b6f58b98 100644 --- a/lib/gitlab/ci/external_files/processor.rb +++ b/lib/gitlab/ci/external_files/processor.rb @@ -25,7 +25,7 @@ module Gitlab attr_reader :values, :external_files def validate_external_file(external_file) - unless external_file.valid? + unless external_file.valid? raise ExternalFileError, 'External files should be a valid local or remote file' end end @@ -39,7 +39,6 @@ module Gitlab values.delete(:includes) values end - end end end diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb index ff623b95be8..1860ed79bfd 100644 --- a/spec/lib/gitlab/ci/config/entry/global_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Ci::Config::Entry::Global do expect(described_class.nodes.keys) .to match_array(%i[before_script image services after_script variables stages - types cache includes]) + types cache]) end end end @@ -42,7 +42,7 @@ describe Gitlab::Ci::Config::Entry::Global do end it 'creates node object for each entry' do - expect(global.descendants.count).to eq 9 + expect(global.descendants.count).to eq 8 end it 'creates node object using valid class' do @@ -189,7 +189,7 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#nodes' do it 'instantizes all nodes' do - expect(global.descendants.count).to eq 9 + expect(global.descendants.count).to eq 8 end it 'contains unspecified nodes' do diff --git a/spec/lib/gitlab/ci/config/entry/includes_spec.rb b/spec/lib/gitlab/ci/config/entry/includes_spec.rb deleted file mode 100644 index d72503535ed..00000000000 --- a/spec/lib/gitlab/ci/config/entry/includes_spec.rb +++ /dev/null @@ -1,97 +0,0 @@ -require 'rails_helper' - -describe Gitlab::Ci::Config::Entry::Includes do - let(:entry) { described_class.new(config) } - - shared_examples 'valid external file' do - it 'should be valid' do - expect(entry).to be_valid - end - - it 'should not return any error' do - expect(entry.errors).to be_empty - end - end - - shared_examples 'invalid external file' do - it 'should not be valid' do - expect(entry).not_to be_valid - end - - it 'should return an error' do - expect(entry.errors.first).to match(/should be a valid local or remote file/) - end - end - - describe "#valid?" do - context 'with no external file given' do - let(:config) { nil } - - it_behaves_like 'valid external file' - end - - context 'with multiple external files' do - let(:config) { %w(https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-2.yml) } - - it_behaves_like 'valid external file' - end - - context 'with just one external file' do - let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - it_behaves_like 'valid external file' - end - - context 'when they contain valid URLs' do - let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - it_behaves_like 'valid external file' - end - - context 'when they contain valid relative URLs' do - let(:config) { '/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml' } - - it_behaves_like 'valid external file' - end - - context 'when they not contain valid URLs' do - let(:config) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - it_behaves_like 'invalid external file' - end - - context 'when they not contain valid relative URLs' do - let(:config) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } - - it_behaves_like 'invalid external file' - end - end - - describe "#value" do - context 'with multiple external files' do - let(:config) { %w(https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-2.yml) } - it 'should return an array' do - expect(entry.value).to be_an(Array) - expect(entry.value.count).to eq(2) - end - end - - context 'with just one external file' do - let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - it 'should return an array' do - expect(entry.value).to be_an(Array) - expect(entry.value.count).to eq(1) - end - end - - context 'with no external file given' do - let(:config) { nil } - - it 'should return an empty array' do - expect(entry.value).to be_an(Array) - expect(entry.value).to be_empty - end - end - end -end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 5a78ce783dd..99896b9be5d 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -124,4 +124,61 @@ describe Gitlab::Ci::Config do end end end + + + context "when yml has valid 'includes' defined" do + let(:yml) do + <<-EOS + includes: + - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml + - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml + - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml + + image: ruby:2.2 + EOS + end + + before do + allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(yml) + end + + it 'should return a composed hash' do + before_script_values = [ + "apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs", "ruby -v", + "which ruby", + "gem install bundler --no-ri --no-rdoc", + "bundle install --jobs $(nproc) \"${FLAGS[@]}\"" + ] + variables = { + AUTO_DEVOPS_DOMAIN: "domain.example.com", + POSTGRES_USER: "user", + POSTGRES_PASSWORD: "testing-password", + POSTGRES_ENABLED: "true", + POSTGRES_DB: "$CI_ENVIRONMENT_SLUG" + } + composed_hash = { + before_script: before_script_values, + image: "ruby:2.2", + rspec: { script: ["bundle exec rspec"] }, + variables: variables + } + + expect(config.to_hash).to eq(composed_hash) + end + end + + context "when config has invalid 'includes' defined" do + let(:yml) do + <<-EOS + includes: invalid + EOS + end + + it 'raises error' do + expect { config }.to raise_error( + ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError, + /External files should be a valid local or remote file/ + ) + end + end end diff --git a/spec/lib/gitlab/ci/external_files/external_file_spec.rb b/spec/lib/gitlab/ci/external_files/external_file_spec.rb index 80468f63435..33e5e0b3b77 100644 --- a/spec/lib/gitlab/ci/external_files/external_file_spec.rb +++ b/spec/lib/gitlab/ci/external_files/external_file_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do end context 'when is not a valid remote url' do - let(:value) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:value) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } it 'should return false' do expect(external_file.valid?).to be_falsy @@ -39,7 +39,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do end describe "#content" do - let(:external_file_content) { + let(:external_file_content) do <<-HEREDOC before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs @@ -48,7 +48,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC - } + end context 'with a local file' do let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb index dfbf8bfa098..57cf5e74cdc 100644 --- a/spec/lib/gitlab/ci/external_files/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external_files/mapper_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe Gitlab::Ci::ExternalFiles::Mapper do describe '.fetch_paths' do context 'when includes is defined as string' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2'} } + let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } } it 'returns an array' do expect(described_class.fetch_paths(values)).to be_an(Array) @@ -15,21 +15,19 @@ describe Gitlab::Ci::ExternalFiles::Mapper do end context 'when includes is defined as an array' do - let(:values) { { includes: ['https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', '/vendor/gitlab-ci-yml/template.yml'], image: 'ruby:2.2'} } + let(:values) { { includes: ['https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', '/vendor/gitlab-ci-yml/template.yml'], image: 'ruby:2.2' } } it 'returns an array' do expect(described_class.fetch_paths(values)).to be_an(Array) end it 'returns ExternalFile instances' do paths = described_class.fetch_paths(values) - paths.each do |path| - expect(path).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) - end + expect(paths).to all(be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile)) end end context 'when includes is not defined' do - let(:values) { { image: 'ruby:2.2'} } + let(:values) { { image: 'ruby:2.2' } } it 'returns an empty array' do expect(described_class.fetch_paths(values)).to be_empty diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external_files/processor_spec.rb index 51ed14de4cf..7a0374a4bce 100644 --- a/spec/lib/gitlab/ci/external_files/processor_spec.rb +++ b/spec/lib/gitlab/ci/external_files/processor_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do end context 'when an invalid local file is defined' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2'} } + let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } } it 'should raise an error' do expect { processor.perform }.to raise_error(described_class::ExternalFileError) @@ -21,7 +21,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do end context 'when an invalid remote file is defined' do - let(:values) { { includes: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2'} } + let(:values) { { includes: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } it 'should raise an error' do expect { processor.perform }.to raise_error(described_class::ExternalFileError) @@ -30,7 +30,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do context 'with a valid remote external file is defined' do let(:values) { { includes: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } - let(:external_file_content) { + let(:external_file_content) do <<-HEREDOC before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs @@ -47,7 +47,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do script: - bundle exec rubocop HEREDOC - } + end before do allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(external_file_content) @@ -55,7 +55,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do it 'should append the file to the values' do output = processor.perform - expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop]) + expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop]) end it "should remove the 'includes' keyword" do @@ -64,8 +64,8 @@ describe Gitlab::Ci::ExternalFiles::Processor do end context 'with a valid local external file is defined' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml' , image: 'ruby:2.2'} } - let(:external_file_content) { + let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } + let(:external_file_content) do <<-HEREDOC before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs @@ -74,7 +74,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do - gem install bundler --no-ri --no-rdoc - bundle install --jobs $(nproc) "${FLAGS[@]}" HEREDOC - } + end before do allow(File).to receive(:exists?).and_return(true) @@ -83,7 +83,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do it 'should append the file to the values' do output = processor.perform - expect(output.keys).to match_array([:image, :before_script]) + expect(output.keys).to match_array([:image, :before_script]) end it "should remove the 'includes' keyword" do @@ -92,23 +92,23 @@ describe Gitlab::Ci::ExternalFiles::Processor do end context 'with multiple external files are defined' do - let(:external_files) { + let(:external_files) do [ "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml", "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml", 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' ] - } - let(:values) { { includes: external_files, image: 'ruby:2.2'} } + end + let(:values) { { includes: external_files, image: 'ruby:2.2' } } - let(:remote_file_content) { + let(:remote_file_content) do <<-HEREDOC stages: - build - review - cleanup HEREDOC - } + end before do allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(remote_file_content) @@ -124,7 +124,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do end context 'when external files are defined but not valid' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2'} } + let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } let(:external_file_content) { 'invalid content file ////' } From cd72189fc0127e9993eebaba9569f912394cc5c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 21:00:52 +0200 Subject: [PATCH 03/23] Import external files in Gitlab CI config CE mirror of a10b777f8cb1902bba3964a4cbad2b1dd6bce785 --- app/models/ci/pipeline.rb | 2 +- lib/gitlab/ci/config.rb | 11 ++++++++--- lib/gitlab/ci/yaml_processor.rb | 4 ++-- spec/lib/gitlab/ci/config_spec.rb | 4 ++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2955e0b2bca..f3a40204305 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -464,7 +464,7 @@ module Ci return @config_processor if defined?(@config_processor) @config_processor ||= begin - Gitlab::Ci::YamlProcessor.new(ci_yaml_file) + Gitlab::Ci::YamlProcessor.new(ci_yaml_file, project) rescue Gitlab::Ci::YamlProcessor::ValidationError => e self.yaml_errors = e.message nil diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 94b2fbc082b..f665ace7c74 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -6,12 +6,17 @@ module Gitlab class Config ConfigError = Class.new(StandardError) - def initialize(config, opts = {}) + def initialize(config, project = nil, opts = {}) initial_config = Config::Extendable .new(build_config(config, opts)) .to_hash - processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config) - @config = processor.perform + + if project.present? + processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config) + @config = processor.perform + else + @config = initial_config + end @global = Entry::Global.new(@config) @global.compose! diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 5d1864ae9e2..702bcd3802d 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -7,8 +7,8 @@ module Gitlab attr_reader :cache, :stages, :jobs - def initialize(config, opts = {}) - @ci_config = Gitlab::Ci::Config.new(config, opts) + def initialize(config, project = nil, opts = {}) + @ci_config = Gitlab::Ci::Config.new(config, project, opts) @config = @ci_config.to_hash unless @ci_config.valid? diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 99896b9be5d..57354e12aa3 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -3,8 +3,9 @@ require 'fast_spec_helper' require_dependency 'active_model' describe Gitlab::Ci::Config do + let(:project) { create(:project, :repository) } let(:config) do - described_class.new(yml) + described_class.new(yml, project) end context 'when config is valid' do @@ -125,7 +126,6 @@ describe Gitlab::Ci::Config do end end - context "when yml has valid 'includes' defined" do let(:yml) do <<-EOS From 95b296f8ac8578e142efd6a60a582be4da5b09be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 21:18:26 +0200 Subject: [PATCH 04/23] Change ExternalFile to retrieve local file from repository instead of GitLab project CE mirror of 03c6094997023d9c8875ced421a6c9ef39a4af44 --- app/models/blob_viewer/gitlab_ci_yml.rb | 2 +- app/models/repository.rb | 4 ++ lib/gitlab/ci/config.rb | 2 +- lib/gitlab/ci/external_files/external_file.rb | 21 ++++++---- lib/gitlab/ci/external_files/mapper.rb | 16 ++++--- lib/gitlab/ci/external_files/processor.rb | 4 +- lib/gitlab/ci/yaml_processor.rb | 6 +-- .../external_files/.gitlab-ci-template-2.yml | 8 ---- spec/lib/gitlab/ci/config_spec.rb | 16 ++++++- .../ci/external_files/external_file_spec.rb | 10 +++-- .../gitlab/ci/external_files/mapper_spec.rb | 42 ++++++++++++++----- .../ci/external_files/processor_spec.rb | 14 +++---- 12 files changed, 93 insertions(+), 52 deletions(-) delete mode 100644 spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml diff --git a/app/models/blob_viewer/gitlab_ci_yml.rb b/app/models/blob_viewer/gitlab_ci_yml.rb index 1a86f04b1b9..f5e4367fc09 100644 --- a/app/models/blob_viewer/gitlab_ci_yml.rb +++ b/app/models/blob_viewer/gitlab_ci_yml.rb @@ -15,7 +15,7 @@ module BlobViewer prepare! - @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data) + @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, project) end def valid? diff --git a/app/models/repository.rb b/app/models/repository.rb index 929d28b9d88..4b479f047ed 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -980,6 +980,10 @@ class Repository blob_data_at(sha, '.gitlab/route-map.yml') end + def fetch_file_for(sha, path_to_file) + blob_data_at(sha, path_to_file) + end + def gitlab_ci_yml_for(sha, path = '.gitlab-ci.yml') blob_data_at(sha, path) end diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index f665ace7c74..caa3a7c3c86 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -12,7 +12,7 @@ module Gitlab .to_hash if project.present? - processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config) + processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config, project) @config = processor.perform else @config = initial_config diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb index 47d6d1c0fdc..45c6ca3fbfe 100644 --- a/lib/gitlab/ci/external_files/external_file.rb +++ b/lib/gitlab/ci/external_files/external_file.rb @@ -4,33 +4,38 @@ module Gitlab module Ci module ExternalFiles class ExternalFile - def initialize(value) + def initialize(value, project) @value = value + @project = project end def content if remote_url? open(value).read else - File.read(base_path) + local_file_content end end def valid? - remote_url? || File.exist?(base_path) + remote_url? || local_file_content end private - attr_reader :value - - def base_path - "#{Rails.root}/#{value}" - end + attr_reader :value, :project def remote_url? ::Gitlab::UrlSanitizer.valid?(value) end + + def local_file_content + project.repository.fetch_file_for(sha, value) + end + + def sha + @sha ||= project.repository.commit.sha + end end end end diff --git a/lib/gitlab/ci/external_files/mapper.rb b/lib/gitlab/ci/external_files/mapper.rb index 4b5d2ddd6a1..2d40e296546 100644 --- a/lib/gitlab/ci/external_files/mapper.rb +++ b/lib/gitlab/ci/external_files/mapper.rb @@ -2,12 +2,12 @@ module Gitlab module Ci module ExternalFiles class Mapper - def self.fetch_paths(values) - paths = values.fetch(:includes, []) - normalize_paths(paths) + def initialize(values, project) + @paths = values.fetch(:includes, []) + @project = project end - def self.normalize_paths(paths) + def process if paths.is_a?(String) [build_external_file(paths)] else @@ -15,8 +15,12 @@ module Gitlab end end - def self.build_external_file(path) - ::Gitlab::Ci::ExternalFiles::ExternalFile.new(path) + private + + attr_reaer :paths, :project + + def build_external_file(path) + ::Gitlab::Ci::ExternalFiles::ExternalFile.new(path, project) end end end diff --git a/lib/gitlab/ci/external_files/processor.rb b/lib/gitlab/ci/external_files/processor.rb index 221b6f58b98..ac51db05d8e 100644 --- a/lib/gitlab/ci/external_files/processor.rb +++ b/lib/gitlab/ci/external_files/processor.rb @@ -4,9 +4,9 @@ module Gitlab class Processor ExternalFileError = Class.new(StandardError) - def initialize(values) + def initialize(values, project) @values = values - @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.fetch_paths(values) + @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.fetch_paths(values, project).process end def perform diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 702bcd3802d..0f79fbede9f 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -73,13 +73,13 @@ module Gitlab end end - def self.validation_message(content, opts = {}) + def self.validation_message(content, project = nil, opts = {}) return 'Please provide content of .gitlab-ci.yml' if content.blank? begin - Gitlab::Ci::YamlProcessor.new(content, opts) + Gitlab::Ci::YamlProcessor.new(content, project, opts) nil - rescue ValidationError => e + rescue ValidationError, ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError => e e.message end end diff --git a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml deleted file mode 100644 index b341cca8946..00000000000 --- a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml +++ /dev/null @@ -1,8 +0,0 @@ -variables: - # AUTO_DEVOPS_DOMAIN is the application deployment domain and should be set as a variable at the group or project level. - - AUTO_DEVOPS_DOMAIN: domain.example.com - POSTGRES_USER: user - POSTGRES_PASSWORD: testing-password - POSTGRES_ENABLED: "true" - POSTGRES_DB: $CI_ENVIRONMENT_SLUG diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 57354e12aa3..b1c801ff052 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -127,6 +127,17 @@ describe Gitlab::Ci::Config do end context "when yml has valid 'includes' defined" do + let(:http_file_content) do + <<~HEREDOC + variables: + AUTO_DEVOPS_DOMAIN: domain.example.com + POSTGRES_USER: user + POSTGRES_PASSWORD: testing-password + POSTGRES_ENABLED: "true" + POSTGRES_DB: $CI_ENVIRONMENT_SLUG + HEREDOC + end + let(:local_file_content) { File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") } let(:yml) do <<-EOS includes: @@ -139,7 +150,8 @@ describe Gitlab::Ci::Config do end before do - allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(yml) + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(local_file_content) + allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(http_file_content) end it 'should return a composed hash' do @@ -167,7 +179,7 @@ describe Gitlab::Ci::Config do end end - context "when config has invalid 'includes' defined" do + context "when yml has invalid 'includes' defined" do let(:yml) do <<-EOS includes: invalid diff --git a/spec/lib/gitlab/ci/external_files/external_file_spec.rb b/spec/lib/gitlab/ci/external_files/external_file_spec.rb index 33e5e0b3b77..b2aeb0ac67a 100644 --- a/spec/lib/gitlab/ci/external_files/external_file_spec.rb +++ b/spec/lib/gitlab/ci/external_files/external_file_spec.rb @@ -1,12 +1,17 @@ require 'rails_helper' describe Gitlab::Ci::ExternalFiles::ExternalFile do - let(:external_file) { described_class.new(value) } + let(:project) { create(:project, :repository) } + let(:external_file) { described_class.new(value, project) } describe "#valid?" do context 'when is a valid remote url' do let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + before do + allow_any_instance_of(described_class).to receive(:local_file_content).and_return("image: 'ruby2:2'") + end + it 'should return true' do expect(external_file.valid?).to be_truthy end @@ -54,8 +59,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } before do - allow(File).to receive(:exists?).and_return(true) - allow(File).to receive(:read).and_return(external_file_content) + allow_any_instance_of(described_class).to receive(:local_file_content).and_return(external_file_content) end it 'should return the content of the file' do diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb index 57cf5e74cdc..4bdb6c200c4 100644 --- a/spec/lib/gitlab/ci/external_files/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external_files/mapper_spec.rb @@ -1,36 +1,56 @@ require 'rails_helper' describe Gitlab::Ci::ExternalFiles::Mapper do - describe '.fetch_paths' do - context 'when includes is defined as string' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } } + let(:project) { create(:project, :repository) } + + describe '#process' do + subject { described_class.new(values, project).process } + + context 'when includes keyword is defined as string' do + let(:values) do + { + includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', + image: 'ruby:2.2' + } + end it 'returns an array' do - expect(described_class.fetch_paths(values)).to be_an(Array) + expect(subject).to be_an(Array) end it 'returns ExternalFile instances' do - expect(described_class.fetch_paths(values).first).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) + expect(subject.first).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) end end context 'when includes is defined as an array' do - let(:values) { { includes: ['https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', '/vendor/gitlab-ci-yml/template.yml'], image: 'ruby:2.2' } } + let(:values) do + { + includes: + [ + 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + '/vendor/gitlab-ci-yml/template.yml' + ], + image: 'ruby:2.2' + } + end + it 'returns an array' do - expect(described_class.fetch_paths(values)).to be_an(Array) + expect(subject).to be_an(Array) end it 'returns ExternalFile instances' do - paths = described_class.fetch_paths(values) - expect(paths).to all(be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile)) + expect(subject).to all(be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile)) end end context 'when includes is not defined' do - let(:values) { { image: 'ruby:2.2' } } + let(:values) do + { image: 'ruby:2.2' } + end it 'returns an empty array' do - expect(described_class.fetch_paths(values)).to be_empty + expect(subject).to be_empty end end end diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external_files/processor_spec.rb index 7a0374a4bce..f009ece8821 100644 --- a/spec/lib/gitlab/ci/external_files/processor_spec.rb +++ b/spec/lib/gitlab/ci/external_files/processor_spec.rb @@ -1,7 +1,8 @@ require 'rails_helper' describe Gitlab::Ci::ExternalFiles::Processor do - let(:processor) { described_class.new(values) } + let(:project) { create(:project, :repository) } + let(:processor) { described_class.new(values, project) } describe "#perform" do context 'when no external files defined' do @@ -77,8 +78,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do end before do - allow(File).to receive(:exists?).and_return(true) - allow(File).to receive(:read).and_return(external_file_content) + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(external_file_content) end it 'should append the file to the values' do @@ -95,7 +95,6 @@ describe Gitlab::Ci::ExternalFiles::Processor do let(:external_files) do [ "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml", - "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml", 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' ] end @@ -111,11 +110,13 @@ describe Gitlab::Ci::ExternalFiles::Processor do end before do + file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(file_content) allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(remote_file_content) end it 'should append the files to the values' do - expect(processor.perform.keys).to match_array([:image, :variables, :stages, :before_script, :rspec]) + expect(processor.perform.keys).to match_array([:image, :stages, :before_script, :rspec]) end it "should remove the 'includes' keyword" do @@ -129,8 +130,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do let(:external_file_content) { 'invalid content file ////' } before do - allow(File).to receive(:exists?).and_return(true) - allow(File).to receive(:read).and_return(external_file_content) + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(external_file_content) end it 'should raise an error' do From eca73d2b30a62876a3148bd1a8b1dfd6d48977fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 21:33:06 +0200 Subject: [PATCH 05/23] Address MR comments CE mirror of 1269dc47b7f8d1a9913de326c9bd356d3e603663 --- lib/gitlab/ci/config.rb | 20 +++++------ lib/gitlab/ci/external_files/external_file.rb | 8 +++-- lib/gitlab/ci/external_files/mapper.rb | 4 +-- lib/gitlab/ci/external_files/processor.rb | 6 ++-- spec/lib/gitlab/ci/config_spec.rb | 17 +++++---- .../ci/external_files/external_file_spec.rb | 14 +++++++- .../gitlab/ci/external_files/mapper_spec.rb | 10 +++--- .../ci/external_files/processor_spec.rb | 36 +++++++++++-------- 8 files changed, 67 insertions(+), 48 deletions(-) diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index caa3a7c3c86..4a0d67720a9 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -7,17 +7,10 @@ module Gitlab ConfigError = Class.new(StandardError) def initialize(config, project = nil, opts = {}) - initial_config = Config::Extendable + @config = Config::Extendable .new(build_config(config, opts)) .to_hash - if project.present? - processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config, project) - @config = processor.perform - else - @config = initial_config - end - @global = Entry::Global.new(@config) @global.compose! rescue Loader::FormatError, Extendable::ExtensionError => e @@ -72,8 +65,15 @@ module Gitlab end # 'opts' argument is used in EE see /ee/lib/ee/gitlab/ci/config.rb - def build_config(config, opts = {}) - Loader.new(config).load! + def build_config(config, project, opts = {}) + initial_config = Loader.new(config).load! + + if project.present? + processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config, project) + processor.perform + else + initial_config + end end end end diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb index 45c6ca3fbfe..f481e9b0a39 100644 --- a/lib/gitlab/ci/external_files/external_file.rb +++ b/lib/gitlab/ci/external_files/external_file.rb @@ -4,6 +4,8 @@ module Gitlab module Ci module ExternalFiles class ExternalFile + attr_reader :value, :project + def initialize(value, project) @value = value @project = project @@ -11,10 +13,12 @@ module Gitlab def content if remote_url? - open(value).read + HTTParty.get(value) else local_file_content end + rescue HTTParty::Error, Timeout::Error + nil end def valid? @@ -23,8 +27,6 @@ module Gitlab private - attr_reader :value, :project - def remote_url? ::Gitlab::UrlSanitizer.valid?(value) end diff --git a/lib/gitlab/ci/external_files/mapper.rb b/lib/gitlab/ci/external_files/mapper.rb index 2d40e296546..fa252cc3b9e 100644 --- a/lib/gitlab/ci/external_files/mapper.rb +++ b/lib/gitlab/ci/external_files/mapper.rb @@ -3,7 +3,7 @@ module Gitlab module ExternalFiles class Mapper def initialize(values, project) - @paths = values.fetch(:includes, []) + @paths = values.fetch(:include, []) @project = project end @@ -17,7 +17,7 @@ module Gitlab private - attr_reaer :paths, :project + attr_reader :paths, :project def build_external_file(path) ::Gitlab::Ci::ExternalFiles::ExternalFile.new(path, project) diff --git a/lib/gitlab/ci/external_files/processor.rb b/lib/gitlab/ci/external_files/processor.rb index ac51db05d8e..5657f025084 100644 --- a/lib/gitlab/ci/external_files/processor.rb +++ b/lib/gitlab/ci/external_files/processor.rb @@ -6,7 +6,7 @@ module Gitlab def initialize(values, project) @values = values - @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.fetch_paths(values, project).process + @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.new(values, project).process end def perform @@ -26,7 +26,7 @@ module Gitlab def validate_external_file(external_file) unless external_file.valid? - raise ExternalFileError, 'External files should be a valid local or remote file' + raise ExternalFileError, "External file: '#{external_file.value}' should be a valid local or remote file" end end @@ -36,7 +36,7 @@ module Gitlab end def remove_include_keyword - values.delete(:includes) + values.delete(:include) values end end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index b1c801ff052..6e1edf76c19 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -126,7 +126,7 @@ describe Gitlab::Ci::Config do end end - context "when yml has valid 'includes' defined" do + context "when yml has valid 'include' defined" do let(:http_file_content) do <<~HEREDOC variables: @@ -140,9 +140,8 @@ describe Gitlab::Ci::Config do let(:local_file_content) { File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") } let(:yml) do <<-EOS - includes: + include: - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml - - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml image: ruby:2.2 @@ -151,7 +150,7 @@ describe Gitlab::Ci::Config do before do allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(local_file_content) - allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(http_file_content) + allow(HTTParty).to receive(:get).and_return(http_file_content) end it 'should return a composed hash' do @@ -179,17 +178,17 @@ describe Gitlab::Ci::Config do end end - context "when yml has invalid 'includes' defined" do + context "when yml has invalid 'include' defined" do let(:yml) do <<-EOS - includes: invalid + include: invalid EOS end - it 'raises error' do + it 'raises error YamlProcessor ValidationError' do expect { config }.to raise_error( - ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError, - /External files should be a valid local or remote file/ + ::Gitlab::Ci::YamlProcessor::ValidationError, + "External file: 'invalid' should be a valid local or remote file" ) end end diff --git a/spec/lib/gitlab/ci/external_files/external_file_spec.rb b/spec/lib/gitlab/ci/external_files/external_file_spec.rb index b2aeb0ac67a..822ed2970bf 100644 --- a/spec/lib/gitlab/ci/external_files/external_file_spec.rb +++ b/spec/lib/gitlab/ci/external_files/external_file_spec.rb @@ -71,12 +71,24 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } before do - allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(external_file_content) + allow(HTTParty).to receive(:get).and_return(external_file_content) end it 'should return the content of the file' do expect(external_file.content).to eq(external_file_content) end end + + context 'with a timeout' do + let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + before do + allow(HTTParty).to receive(:get).and_raise(Timeout::Error) + end + + it 'should return nil' do + expect(external_file.content).to be_nil + end + end end end diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb index 4bdb6c200c4..ab1bc242c19 100644 --- a/spec/lib/gitlab/ci/external_files/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external_files/mapper_spec.rb @@ -6,10 +6,10 @@ describe Gitlab::Ci::ExternalFiles::Mapper do describe '#process' do subject { described_class.new(values, project).process } - context 'when includes keyword is defined as string' do + context "when 'include' keyword is defined as string" do let(:values) do { - includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', + include: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } end @@ -23,10 +23,10 @@ describe Gitlab::Ci::ExternalFiles::Mapper do end end - context 'when includes is defined as an array' do + context "when 'include' is defined as an array" do let(:values) do { - includes: + include: [ 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', '/vendor/gitlab-ci-yml/template.yml' @@ -44,7 +44,7 @@ describe Gitlab::Ci::ExternalFiles::Mapper do end end - context 'when includes is not defined' do + context "when 'include' is not defined" do let(:values) do { image: 'ruby:2.2' } end diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external_files/processor_spec.rb index f009ece8821..a3db566f428 100644 --- a/spec/lib/gitlab/ci/external_files/processor_spec.rb +++ b/spec/lib/gitlab/ci/external_files/processor_spec.rb @@ -14,23 +14,29 @@ describe Gitlab::Ci::ExternalFiles::Processor do end context 'when an invalid local file is defined' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } } + let(:values) { { include: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } } it 'should raise an error' do - expect { processor.perform }.to raise_error(described_class::ExternalFileError) + expect { processor.perform }.to raise_error( + described_class::ExternalFileError, + "External file: '/vendor/gitlab-ci-yml/non-existent-file.yml' should be a valid local or remote file" + ) end end context 'when an invalid remote file is defined' do - let(:values) { { includes: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } + let(:values) { { include: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } it 'should raise an error' do - expect { processor.perform }.to raise_error(described_class::ExternalFileError) + expect { processor.perform }.to raise_error( + described_class::ExternalFileError, + "External file: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' should be a valid local or remote file" + ) end end context 'with a valid remote external file is defined' do - let(:values) { { includes: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } + let(:values) { { include: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } let(:external_file_content) do <<-HEREDOC before_script: @@ -51,7 +57,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do end before do - allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(external_file_content) + allow(HTTParty).to receive(:get).and_return(external_file_content) end it 'should append the file to the values' do @@ -59,13 +65,13 @@ describe Gitlab::Ci::ExternalFiles::Processor do expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop]) end - it "should remove the 'includes' keyword" do - expect(processor.perform[:includes]).to be_nil + it "should remove the 'include' keyword" do + expect(processor.perform[:include]).to be_nil end end context 'with a valid local external file is defined' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } + let(:values) { { include: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } let(:external_file_content) do <<-HEREDOC before_script: @@ -86,7 +92,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do expect(output.keys).to match_array([:image, :before_script]) end - it "should remove the 'includes' keyword" do + it "should remove the 'include' keyword" do expect(processor.perform[:includes]).to be_nil end end @@ -98,7 +104,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' ] end - let(:values) { { includes: external_files, image: 'ruby:2.2' } } + let(:values) { { include: external_files, image: 'ruby:2.2' } } let(:remote_file_content) do <<-HEREDOC @@ -112,20 +118,20 @@ describe Gitlab::Ci::ExternalFiles::Processor do before do file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(file_content) - allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(remote_file_content) + allow(HTTParty).to receive(:get).and_return(remote_file_content) end it 'should append the files to the values' do expect(processor.perform.keys).to match_array([:image, :stages, :before_script, :rspec]) end - it "should remove the 'includes' keyword" do - expect(processor.perform[:includes]).to be_nil + it "should remove the 'include' keyword" do + expect(processor.perform[:include]).to be_nil end end context 'when external files are defined but not valid' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } + let(:values) { { include: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } let(:external_file_content) { 'invalid content file ////' } From 1a53f017b4c2106da3425f3dcbe40fb95fd44bbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 21:37:44 +0200 Subject: [PATCH 06/23] Make Repository#blob_data_at public CE mirror of 17de13ada1a98da060802c55889489a512183cd1 --- app/models/repository.rb | 20 ++-- doc/ci/yaml/README.md | 98 +++++++++++++++++++ lib/gitlab/ci/external_files/external_file.rb | 4 +- spec/lib/gitlab/ci/config_spec.rb | 22 +++++ .../ci/external_files/external_file_spec.rb | 2 +- .../gitlab/ci/external_files/mapper_spec.rb | 2 +- .../ci/external_files/processor_spec.rb | 2 +- 7 files changed, 132 insertions(+), 18 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 4b479f047ed..96d25e38cbe 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -980,10 +980,6 @@ class Repository blob_data_at(sha, '.gitlab/route-map.yml') end - def fetch_file_for(sha, path_to_file) - blob_data_at(sha, path_to_file) - end - def gitlab_ci_yml_for(sha, path = '.gitlab-ci.yml') blob_data_at(sha, path) end @@ -1003,14 +999,6 @@ class Repository remote_branch: merge_request.target_branch) end - def blob_data_at(sha, path) - blob = blob_at(sha, path) - return unless blob - - blob.load_all_data! - blob.data - end - def squash(user, merge_request) raw.squash(user, merge_request.id, branch: merge_request.target_branch, start_sha: merge_request.diff_start_sha, @@ -1019,6 +1007,14 @@ class Repository message: merge_request.title) end + def blob_data_at(sha, path) + blob = blob_at(sha, path) + return unless blob + + blob.load_all_data! + blob.data + end + private # TODO Generice finder, later split this on finders by Ref or Oid diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index d89705e8ead..418d5b88375 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -966,6 +966,104 @@ Additionally, if you have a job that unconditionally recreates the cache without reference to its previous contents, you can use `policy: push` in that job to skip the download step. +### include + +From 10.5 we can use `include` keyword to allow the inclusion of external yml files. + +```yaml +# Content of https://gitlab.com/awesome-project/raw/master/.gitlab-ci-before-script-template.yml +before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" +``` + +```yaml +include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-before-script-template.yml' + +rspec: + script: + - bundle exec rspec + +rubocop: + script: + - bundle exec rubocop +``` + +In the above example `.gitlab-ci-before-script-template.yml` content will be automatically fetched and evaluated as it were a single local file. + +`include` supports two types of files: + - **local** to the same repository, referenced using the relative path, or + - **remote** in a different location, accessed using HTTP(S) protocol, referenced using the full URL + +Also, `include` supports a single string or an array of different values, so + +```yaml +include: '/templates/.gitlab-ci-templates.yml' +``` + +and + +```yaml +include: + - 'https://gitlab.com/same-group/another-project/raw/master/.gitlab-ci-templates.yml' + - '/templates/.gitlab-ci-templates.yml' +``` + +are both valid use cases. + +#### Restrictions + +- We can only use files that are currently tracked by Git on the default repository branch, so when using a **local file** make sure it's on the latest commit of your default branch, otherwise feel free to use a remote location. +- Since external files defined on `include` are evaluated first, the configuration on this files will take precedence over the content of `.gitlab-ci.yml`, for example: + +```yaml +# Content of http://company.com/default-gitlab-ci.yml +image: php:5-fpm-alpine + +job2: + script: php -v +``` + + +```yaml +include: + - http://company.com/default-gitlab-ci.yml + +image: ruby:2.1 + +job1: + script: ruby -v +``` + +In this case both, `job1` and `job2` will be executed with `php:5-fpm-alpine` + + + +#### Examples + +**Example of local files included** + +```yaml +include: '/templates/.gitlab-ci-templates.yml' +``` + +**Example of remote files included** + +```yaml +include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-templates.yml' +``` + +**Example of multiple files included** + +```yaml +include: + - 'https://gitlab.com/same-group/another-project/raw/master/.gitlab-ci-templates.yml' + - '/templates/.gitlab-ci-templates.yml' +``` + ## `artifacts` > **Notes:** diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb index f481e9b0a39..4fffdc5f716 100644 --- a/lib/gitlab/ci/external_files/external_file.rb +++ b/lib/gitlab/ci/external_files/external_file.rb @@ -1,5 +1,3 @@ -require 'open-uri' - module Gitlab module Ci module ExternalFiles @@ -32,7 +30,7 @@ module Gitlab end def local_file_content - project.repository.fetch_file_for(sha, value) + project.repository.blob_data_at(sha, value) end def sha diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 6e1edf76c19..fe9d3b3a570 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -192,4 +192,26 @@ describe Gitlab::Ci::Config do ) end end + + context "when both external files and gitlab_ci defined the same key" do + let(:yml) do + <<~HEREDOC + include: + - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml + + image: ruby:2.2 + HEREDOC + end + + let(:http_file_content) do + <<~HEREDOC + image: php:5-fpm-alpine + HEREDOC + end + + it 'should take precedence' do + allow(HTTParty).to receive(:get).and_return(http_file_content) + expect(config.to_hash).to eq({ image: 'php:5-fpm-alpine' }) + end + end end diff --git a/spec/lib/gitlab/ci/external_files/external_file_spec.rb b/spec/lib/gitlab/ci/external_files/external_file_spec.rb index 822ed2970bf..b149b4200d8 100644 --- a/spec/lib/gitlab/ci/external_files/external_file_spec.rb +++ b/spec/lib/gitlab/ci/external_files/external_file_spec.rb @@ -1,4 +1,4 @@ -require 'rails_helper' +require 'fast_spec_helper' describe Gitlab::Ci::ExternalFiles::ExternalFile do let(:project) { create(:project, :repository) } diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb index ab1bc242c19..84957c1f6b4 100644 --- a/spec/lib/gitlab/ci/external_files/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external_files/mapper_spec.rb @@ -1,4 +1,4 @@ -require 'rails_helper' +require 'fast_spec_helper' describe Gitlab::Ci::ExternalFiles::Mapper do let(:project) { create(:project, :repository) } diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external_files/processor_spec.rb index a3db566f428..36eec0b90d3 100644 --- a/spec/lib/gitlab/ci/external_files/processor_spec.rb +++ b/spec/lib/gitlab/ci/external_files/processor_spec.rb @@ -1,4 +1,4 @@ -require 'rails_helper' +require 'fast_spec_helper' describe Gitlab::Ci::ExternalFiles::Processor do let(:project) { create(:project, :repository) } From 3149b5cfea4d8f14d69bb2520974f588454a0762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 22:03:05 +0200 Subject: [PATCH 07/23] Improve external architecture CE mirror of 4f17c7b2c30188302e6a73421acbf5a09fb2c823 --- app/models/blob_viewer/gitlab_ci_yml.rb | 8 +- app/models/ci/pipeline.rb | 6 +- .../blob/viewers/_gitlab_ci_yml.html.haml | 4 +- doc/ci/yaml/README.md | 16 ++-- lib/gitlab/ci/config.rb | 16 ++-- lib/gitlab/ci/external/file/local.rb | 35 +++++++ lib/gitlab/ci/external/file/remote.rb | 25 +++++ lib/gitlab/ci/external/mapper.rb | 31 ++++++ lib/gitlab/ci/external/processor.rb | 50 ++++++++++ lib/gitlab/ci/external_files/external_file.rb | 42 --------- lib/gitlab/ci/external_files/mapper.rb | 28 ------ lib/gitlab/ci/external_files/processor.rb | 45 --------- lib/gitlab/ci/yaml_processor.rb | 8 +- spec/lib/gitlab/ci/config_spec.rb | 21 +++-- .../lib/gitlab/ci/external/file/local_spec.rb | 54 +++++++++++ .../gitlab/ci/external/file/remote_spec.rb | 68 ++++++++++++++ spec/lib/gitlab/ci/external/mapper_spec.rb | 92 ++++++++++++++++++ .../processor_spec.rb | 52 +++++++--- .../ci/external_files/external_file_spec.rb | 94 ------------------- .../gitlab/ci/external_files/mapper_spec.rb | 57 ----------- spec/models/blob_viewer/gitlab_ci_yml_spec.rb | 6 +- spec/models/ci/pipeline_spec.rb | 2 +- 22 files changed, 445 insertions(+), 315 deletions(-) create mode 100644 lib/gitlab/ci/external/file/local.rb create mode 100644 lib/gitlab/ci/external/file/remote.rb create mode 100644 lib/gitlab/ci/external/mapper.rb create mode 100644 lib/gitlab/ci/external/processor.rb delete mode 100644 lib/gitlab/ci/external_files/external_file.rb delete mode 100644 lib/gitlab/ci/external_files/mapper.rb delete mode 100644 lib/gitlab/ci/external_files/processor.rb create mode 100644 spec/lib/gitlab/ci/external/file/local_spec.rb create mode 100644 spec/lib/gitlab/ci/external/file/remote_spec.rb create mode 100644 spec/lib/gitlab/ci/external/mapper_spec.rb rename spec/lib/gitlab/ci/{external_files => external}/processor_spec.rb (69%) delete mode 100644 spec/lib/gitlab/ci/external_files/external_file_spec.rb delete mode 100644 spec/lib/gitlab/ci/external_files/mapper_spec.rb diff --git a/app/models/blob_viewer/gitlab_ci_yml.rb b/app/models/blob_viewer/gitlab_ci_yml.rb index f5e4367fc09..54462d01b66 100644 --- a/app/models/blob_viewer/gitlab_ci_yml.rb +++ b/app/models/blob_viewer/gitlab_ci_yml.rb @@ -10,16 +10,16 @@ module BlobViewer self.file_types = %i(gitlab_ci) self.binary = false - def validation_message + def validation_message(project, ref) return @validation_message if defined?(@validation_message) prepare! - @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, project) + @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, { project: project, branch_name: ref }) end - def valid? - validation_message.blank? + def valid?(project, ref) + validation_message(project, ref).blank? end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index f3a40204305..27e0ade2722 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -464,7 +464,7 @@ module Ci return @config_processor if defined?(@config_processor) @config_processor ||= begin - Gitlab::Ci::YamlProcessor.new(ci_yaml_file, project) + initialize_yaml_processor rescue Gitlab::Ci::YamlProcessor::ValidationError => e self.yaml_errors = e.message nil @@ -474,6 +474,10 @@ module Ci end end + def initialize_yaml_processor + Gitlab::Ci::YamlProcessor.new(ci_yaml_file, { project: project, branch_name: ref }) + end + def ci_yaml_file_path if project.ci_config_path.blank? '.gitlab-ci.yml' diff --git a/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml b/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml index 28c5be6ebf3..6ef09278ee8 100644 --- a/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml +++ b/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml @@ -1,9 +1,9 @@ -- if viewer.valid? +- if viewer.valid?(@project, @ref) = icon('check fw') This GitLab CI configuration is valid. - else = icon('warning fw') This GitLab CI configuration is invalid: - = viewer.validation_message + = viewer.validation_message(@project, @ref) = link_to 'Learn more', help_page_path('ci/yaml/README') diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 418d5b88375..3ade7201177 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -981,6 +981,7 @@ before_script: ``` ```yaml +# Content of gitlab-ci.yml include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-before-script-template.yml' rspec: @@ -992,13 +993,13 @@ rubocop: - bundle exec rubocop ``` -In the above example `.gitlab-ci-before-script-template.yml` content will be automatically fetched and evaluated as it were a single local file. +In the above example `.gitlab-ci-before-script-template.yml` content will be automatically fetched and evaluated along with the content of `gitlab-ci.yml`. `include` supports two types of files: - **local** to the same repository, referenced using the relative path, or - - **remote** in a different location, accessed using HTTP(S) protocol, referenced using the full URL + - **remote** in a different location, accessed using HTTP protocol, referenced using the full URL -Also, `include` supports a single string or an array of different values, so +Also, `include` supports a single string or an array composed by different values, so ```yaml include: '/templates/.gitlab-ci-templates.yml' @@ -1016,11 +1017,12 @@ are both valid use cases. #### Restrictions -- We can only use files that are currently tracked by Git on the default repository branch, so when using a **local file** make sure it's on the latest commit of your default branch, otherwise feel free to use a remote location. -- Since external files defined on `include` are evaluated first, the configuration on this files will take precedence over the content of `.gitlab-ci.yml`, for example: +- We can only use files that are currently tracked by Git on the same branch and commit your configuration file is. In other words, when using a **local file** make sure it's on the latest commit of your branch. +- Since external files defined on `include` are evaluated first, the content on `gitlab-ci.yml` **will take precedence over the content of the external files**, for example: ```yaml # Content of http://company.com/default-gitlab-ci.yml + image: php:5-fpm-alpine job2: @@ -1029,6 +1031,8 @@ job2: ```yaml +# Content of gitlab-ci.yml + include: - http://company.com/default-gitlab-ci.yml @@ -1038,7 +1042,7 @@ job1: script: ruby -v ``` -In this case both, `job1` and `job2` will be executed with `php:5-fpm-alpine` +In this case both, `job1` and `job2` will be executed with `ruby:2.1` diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 4a0d67720a9..1779fbb2a39 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -6,7 +6,7 @@ module Gitlab class Config ConfigError = Class.new(StandardError) - def initialize(config, project = nil, opts = {}) + def initialize(config, opts = {}) @config = Config::Extendable .new(build_config(config, opts)) .to_hash @@ -64,17 +64,21 @@ module Gitlab @global.jobs_value end - # 'opts' argument is used in EE see /ee/lib/ee/gitlab/ci/config.rb - def build_config(config, project, opts = {}) + def build_config(config, opts = {}) initial_config = Loader.new(config).load! + project = opts.fetch(:project, nil) - if project.present? - processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config, project) - processor.perform + if project + process_external_files(initial_config, project, opts) else initial_config end end + + def process_external_files(config, project, opts) + branch_name = opts.fetch(:branch_name, project.default_branch) + ::Gitlab::Ci::External::Processor.new(config, project, branch_name).perform + end end end end diff --git a/lib/gitlab/ci/external/file/local.rb b/lib/gitlab/ci/external/file/local.rb new file mode 100644 index 00000000000..dc1ba923411 --- /dev/null +++ b/lib/gitlab/ci/external/file/local.rb @@ -0,0 +1,35 @@ +module Gitlab + module Ci + module External + module File + class Local + attr_reader :value, :project, :branch_name + + def initialize(value, project, branch_name) + @value = value + @project = project + @branch_name = branch_name + end + + def valid? + commit && local_file_content + end + + def content + local_file_content + end + + private + + def commit + @commit ||= project.repository.commit(branch_name) + end + + def local_file_content + @local_file_content ||= project.repository.blob_data_at(commit.sha, value) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/external/file/remote.rb b/lib/gitlab/ci/external/file/remote.rb new file mode 100644 index 00000000000..7d8cd1957ad --- /dev/null +++ b/lib/gitlab/ci/external/file/remote.rb @@ -0,0 +1,25 @@ +module Gitlab + module Ci + module External + module File + class Remote + attr_reader :value + + def initialize(value) + @value = value + end + + def valid? + ::Gitlab::UrlSanitizer.valid?(value) && content + end + + def content + HTTParty.get(value) + rescue HTTParty::Error, Timeout::Error + false + end + end + end + end + end +end diff --git a/lib/gitlab/ci/external/mapper.rb b/lib/gitlab/ci/external/mapper.rb new file mode 100644 index 00000000000..f2e5ec972df --- /dev/null +++ b/lib/gitlab/ci/external/mapper.rb @@ -0,0 +1,31 @@ +module Gitlab + module Ci + module External + class Mapper + def initialize(values, project, branch_name) + @paths = Array(values.fetch(:include, [])) + @project = project + @branch_name = branch_name + end + + def process + paths.map { |path| build_external_file(path) } + end + + private + + attr_reader :paths, :project, :branch_name + + def build_external_file(path) + remote_file = Gitlab::Ci::External::File::Remote.new(path) + + if remote_file.valid? + remote_file + else + ::Gitlab::Ci::External::File::Local.new(path, project, branch_name) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/external/processor.rb b/lib/gitlab/ci/external/processor.rb new file mode 100644 index 00000000000..5cae0166346 --- /dev/null +++ b/lib/gitlab/ci/external/processor.rb @@ -0,0 +1,50 @@ +module Gitlab + module Ci + module External + class Processor + FileError = Class.new(StandardError) + + def initialize(values, project, branch_name) + @values = values + @external_files = ::Gitlab::Ci::External::Mapper.new(values, project, branch_name).process + @content = {} + end + + def perform + return values if external_files.empty? + + external_files.each do |external_file| + validate_external_file(external_file) + @content.merge!(content_of(external_file)) + end + + append_external_content + remove_include_keyword + end + + private + + attr_reader :values, :external_files, :content + + def validate_external_file(external_file) + unless external_file.valid? + raise FileError, "External file: '#{external_file.value}' should be a valid local or remote file" + end + end + + def content_of(external_file) + ::Gitlab::Ci::Config::Loader.new(external_file.content).load! + end + + def append_external_content + @content.merge!(@values) + end + + def remove_include_keyword + content.delete(:include) + content + end + end + end + end +end diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb deleted file mode 100644 index 4fffdc5f716..00000000000 --- a/lib/gitlab/ci/external_files/external_file.rb +++ /dev/null @@ -1,42 +0,0 @@ -module Gitlab - module Ci - module ExternalFiles - class ExternalFile - attr_reader :value, :project - - def initialize(value, project) - @value = value - @project = project - end - - def content - if remote_url? - HTTParty.get(value) - else - local_file_content - end - rescue HTTParty::Error, Timeout::Error - nil - end - - def valid? - remote_url? || local_file_content - end - - private - - def remote_url? - ::Gitlab::UrlSanitizer.valid?(value) - end - - def local_file_content - project.repository.blob_data_at(sha, value) - end - - def sha - @sha ||= project.repository.commit.sha - end - end - end - end -end diff --git a/lib/gitlab/ci/external_files/mapper.rb b/lib/gitlab/ci/external_files/mapper.rb deleted file mode 100644 index fa252cc3b9e..00000000000 --- a/lib/gitlab/ci/external_files/mapper.rb +++ /dev/null @@ -1,28 +0,0 @@ -module Gitlab - module Ci - module ExternalFiles - class Mapper - def initialize(values, project) - @paths = values.fetch(:include, []) - @project = project - end - - def process - if paths.is_a?(String) - [build_external_file(paths)] - else - paths.map { |path| build_external_file(path) } - end - end - - private - - attr_reader :paths, :project - - def build_external_file(path) - ::Gitlab::Ci::ExternalFiles::ExternalFile.new(path, project) - end - end - end - end -end diff --git a/lib/gitlab/ci/external_files/processor.rb b/lib/gitlab/ci/external_files/processor.rb deleted file mode 100644 index 5657f025084..00000000000 --- a/lib/gitlab/ci/external_files/processor.rb +++ /dev/null @@ -1,45 +0,0 @@ -module Gitlab - module Ci - module ExternalFiles - class Processor - ExternalFileError = Class.new(StandardError) - - def initialize(values, project) - @values = values - @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.new(values, project).process - end - - def perform - return values if external_files.empty? - - external_files.each do |external_file| - validate_external_file(external_file) - append_external_content(external_file) - end - - remove_include_keyword - end - - private - - attr_reader :values, :external_files - - def validate_external_file(external_file) - unless external_file.valid? - raise ExternalFileError, "External file: '#{external_file.value}' should be a valid local or remote file" - end - end - - def append_external_content(external_file) - external_values = ::Gitlab::Ci::Config::Loader.new(external_file.content).load! - @values.merge!(external_values) - end - - def remove_include_keyword - values.delete(:include) - values - end - end - end - end -end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 0f79fbede9f..67f5d2d6ae4 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -7,8 +7,8 @@ module Gitlab attr_reader :cache, :stages, :jobs - def initialize(config, project = nil, opts = {}) - @ci_config = Gitlab::Ci::Config.new(config, project, opts) + def initialize(config, opts = {}) + @ci_config = Gitlab::Ci::Config.new(config, opts) @config = @ci_config.to_hash unless @ci_config.valid? @@ -73,11 +73,11 @@ module Gitlab end end - def self.validation_message(content, project = nil, opts = {}) + def self.validation_message(content, opts = {}) return 'Please provide content of .gitlab-ci.yml' if content.blank? begin - Gitlab::Ci::YamlProcessor.new(content, project, opts) + Gitlab::Ci::YamlProcessor.new(content, opts) nil rescue ValidationError, ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError => e e.message diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index fe9d3b3a570..1ac832e2b16 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -5,11 +5,11 @@ require_dependency 'active_model' describe Gitlab::Ci::Config do let(:project) { create(:project, :repository) } let(:config) do - described_class.new(yml, project) + described_class.new(gitlab_ci_yml, { project: project, branch_name: 'testing' }) end context 'when config is valid' do - let(:yml) do + let(:gitlab_ci_yml) do <<-EOS image: ruby:2.2 @@ -126,7 +126,7 @@ describe Gitlab::Ci::Config do end end - context "when yml has valid 'include' defined" do + context "when gitlab_ci_yml has valid 'include' defined" do let(:http_file_content) do <<~HEREDOC variables: @@ -149,7 +149,8 @@ describe Gitlab::Ci::Config do end before do - allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(local_file_content) + allow_any_instance_of(::Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') + allow_any_instance_of(::Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) allow(HTTParty).to receive(:get).and_return(http_file_content) end @@ -178,8 +179,8 @@ describe Gitlab::Ci::Config do end end - context "when yml has invalid 'include' defined" do - let(:yml) do + context "when gitlab_ci.yml has invalid 'include' defined" do + let(:gitlab_ci_yml) do <<-EOS include: invalid EOS @@ -193,11 +194,11 @@ describe Gitlab::Ci::Config do end end - context "when both external files and gitlab_ci defined the same key" do - let(:yml) do + context "when both external files and gitlab_ci.yml defined the same key" do + let(:gitlab_ci_yml) do <<~HEREDOC include: - - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml + - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.gitlab_ci_yml image: ruby:2.2 HEREDOC @@ -211,7 +212,7 @@ describe Gitlab::Ci::Config do it 'should take precedence' do allow(HTTParty).to receive(:get).and_return(http_file_content) - expect(config.to_hash).to eq({ image: 'php:5-fpm-alpine' }) + expect(config.to_hash).to eq({ image: 'ruby:2.2' }) end end end diff --git a/spec/lib/gitlab/ci/external/file/local_spec.rb b/spec/lib/gitlab/ci/external/file/local_spec.rb new file mode 100644 index 00000000000..9e1e49da7cb --- /dev/null +++ b/spec/lib/gitlab/ci/external/file/local_spec.rb @@ -0,0 +1,54 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::External::File::Local do + let(:project) { create(:project, :repository) } + let(:local_file) { described_class.new(value, project, 'testing') } + + describe "#valid?" do + context 'when is a valid local path' do + let(:value) { '/vendor/gitlab-ci-yml/existent-file.yml' } + + before do + allow_any_instance_of(described_class).to receive(:commit).and_return('12345') + allow_any_instance_of(described_class).to receive(:local_file_content).and_return("image: 'ruby2:2'") + end + + it 'should return true' do + expect(local_file.valid?).to be_truthy + end + end + + context 'when is not a valid local path' do + let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + + it 'should return false' do + expect(local_file.valid?).to be_falsy + end + end + + describe "#content" do + let(:local_file_content) do + <<~HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + end + + context 'with a local file' do + let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + + before do + allow_any_instance_of(described_class).to receive(:local_file_content).and_return(local_file_content) + end + + it 'should return the content of the file' do + expect(local_file.content).to eq(local_file_content) + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/external/file/remote_spec.rb b/spec/lib/gitlab/ci/external/file/remote_spec.rb new file mode 100644 index 00000000000..91b50dcf5fd --- /dev/null +++ b/spec/lib/gitlab/ci/external/file/remote_spec.rb @@ -0,0 +1,68 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::External::File::Remote do + let(:remote_file) { described_class.new(value) } + let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:remote_file_content) do + <<~HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + end + + describe "#valid?" do + context 'when is a valid remote url' do + before do + allow(HTTParty).to receive(:get).and_return(remote_file_content) + end + + it 'should return true' do + expect(remote_file.valid?).to be_truthy + end + end + + context 'when is not a valid remote url' do + let(:value) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it 'should return false' do + expect(remote_file.valid?).to be_falsy + end + end + + context 'with a timeout' do + before do + allow(HTTParty).to receive(:get).and_raise(Timeout::Error) + end + + it 'should be falsy' do + expect(remote_file.valid?).to be_falsy + end + end + end + + describe "#content" do + context 'with a valid remote file' do + before do + allow(HTTParty).to receive(:get).and_return(remote_file_content) + end + + it 'should return the content of the file' do + expect(remote_file.content).to eq(remote_file_content) + end + end + + context 'with a timeout' do + before do + allow(HTTParty).to receive(:get).and_raise(Timeout::Error) + end + + it 'should be falsy' do + expect(remote_file.content).to be_falsy + end + end + end +end diff --git a/spec/lib/gitlab/ci/external/mapper_spec.rb b/spec/lib/gitlab/ci/external/mapper_spec.rb new file mode 100644 index 00000000000..c752a23def6 --- /dev/null +++ b/spec/lib/gitlab/ci/external/mapper_spec.rb @@ -0,0 +1,92 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::External::Mapper do + let(:project) { create(:project, :repository) } + let(:file_content) do + <<~HEREDOC + image: 'ruby:2.2' + HEREDOC + end + + describe '#process' do + subject { described_class.new(values, project, 'testing').process } + + context "when 'include' keyword is defined as string" do + context 'when the string is a local file' do + let(:values) do + { + include: '/vendor/gitlab-ci-yml/non-existent-file.yml', + image: 'ruby:2.2' + } + end + + it 'returns an array' do + expect(subject).to be_an(Array) + end + + it 'returns File instances' do + expect(subject.first).to be_an_instance_of(::Gitlab::Ci::External::File::Local) + end + end + + context 'when the string is a remote file' do + let(:values) do + { + include: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + image: 'ruby:2.2' + } + end + + before do + allow(HTTParty).to receive(:get).and_return(file_content) + end + + it 'returns an array' do + expect(subject).to be_an(Array) + end + + it 'returns File instances' do + expect(subject.first).to be_an_instance_of(::Gitlab::Ci::External::File::Remote) + end + end + end + + context "when 'include' is defined as an array" do + let(:values) do + { + include: + [ + 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + '/vendor/gitlab-ci-yml/template.yml' + ], + image: 'ruby:2.2' + } + end + + before do + allow(HTTParty).to receive(:get).and_return(file_content) + end + + it 'returns an array' do + expect(subject).to be_an(Array) + end + + it 'returns Files instances' do + expect(subject).to all(respond_to(:valid?)) + expect(subject).to all(respond_to(:content)) + end + end + + context "when 'include' is not defined" do + let(:values) do + { + image: 'ruby:2.2' + } + end + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + end +end diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external/processor_spec.rb similarity index 69% rename from spec/lib/gitlab/ci/external_files/processor_spec.rb rename to spec/lib/gitlab/ci/external/processor_spec.rb index 36eec0b90d3..ba9e106706e 100644 --- a/spec/lib/gitlab/ci/external_files/processor_spec.rb +++ b/spec/lib/gitlab/ci/external/processor_spec.rb @@ -1,8 +1,8 @@ require 'fast_spec_helper' -describe Gitlab::Ci::ExternalFiles::Processor do +describe Gitlab::Ci::External::Processor do let(:project) { create(:project, :repository) } - let(:processor) { described_class.new(values, project) } + let(:processor) { described_class.new(values, project, 'testing') } describe "#perform" do context 'when no external files defined' do @@ -18,7 +18,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do it 'should raise an error' do expect { processor.perform }.to raise_error( - described_class::ExternalFileError, + described_class::FileError, "External file: '/vendor/gitlab-ci-yml/non-existent-file.yml' should be a valid local or remote file" ) end @@ -29,7 +29,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do it 'should raise an error' do expect { processor.perform }.to raise_error( - described_class::ExternalFileError, + described_class::FileError, "External file: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' should be a valid local or remote file" ) end @@ -72,7 +72,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do context 'with a valid local external file is defined' do let(:values) { { include: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } - let(:external_file_content) do + let(:local_file_content) do <<-HEREDOC before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs @@ -84,7 +84,8 @@ describe Gitlab::Ci::ExternalFiles::Processor do end before do - allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(external_file_content) + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) end it 'should append the file to the values' do @@ -93,7 +94,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do end it "should remove the 'include' keyword" do - expect(processor.perform[:includes]).to be_nil + expect(processor.perform[:include]).to be_nil end end @@ -104,7 +105,12 @@ describe Gitlab::Ci::ExternalFiles::Processor do 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' ] end - let(:values) { { include: external_files, image: 'ruby:2.2' } } + let(:values) do + { + include: external_files, + image: 'ruby:2.2' + } + end let(:remote_file_content) do <<-HEREDOC @@ -116,8 +122,9 @@ describe Gitlab::Ci::ExternalFiles::Processor do end before do - file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") - allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(file_content) + local_file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) allow(HTTParty).to receive(:get).and_return(remote_file_content) end @@ -133,15 +140,36 @@ describe Gitlab::Ci::ExternalFiles::Processor do context 'when external files are defined but not valid' do let(:values) { { include: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } } - let(:external_file_content) { 'invalid content file ////' } + let(:local_file_content) { 'invalid content file ////' } before do - allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(external_file_content) + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) end it 'should raise an error' do expect { processor.perform }.to raise_error(Gitlab::Ci::Config::Loader::FormatError) end end + + context "when both external files and values defined the same key" do + let(:values) do + { + include: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + image: 'ruby:2.2' + } + end + + let(:remote_file_content) do + <<~HEREDOC + image: php:5-fpm-alpine + HEREDOC + end + + it 'should take precedence' do + allow(HTTParty).to receive(:get).and_return(remote_file_content) + expect(processor.perform[:image]).to eq('ruby:2.2') + end + end end end diff --git a/spec/lib/gitlab/ci/external_files/external_file_spec.rb b/spec/lib/gitlab/ci/external_files/external_file_spec.rb deleted file mode 100644 index b149b4200d8..00000000000 --- a/spec/lib/gitlab/ci/external_files/external_file_spec.rb +++ /dev/null @@ -1,94 +0,0 @@ -require 'fast_spec_helper' - -describe Gitlab::Ci::ExternalFiles::ExternalFile do - let(:project) { create(:project, :repository) } - let(:external_file) { described_class.new(value, project) } - - describe "#valid?" do - context 'when is a valid remote url' do - let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - before do - allow_any_instance_of(described_class).to receive(:local_file_content).and_return("image: 'ruby2:2'") - end - - it 'should return true' do - expect(external_file.valid?).to be_truthy - end - end - - context 'when is not a valid remote url' do - let(:value) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - it 'should return false' do - expect(external_file.valid?).to be_falsy - end - end - - context 'when is a valid local path' do - let(:value) { '/vendor/gitlab-ci-yml/existent-file.yml' } - - it 'should return true' do - allow(File).to receive(:exists?).and_return(true) - expect(external_file.valid?).to be_truthy - end - end - - context 'when is not a valid local path' do - let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } - - it 'should return false' do - expect(external_file.valid?).to be_falsy - end - end - end - - describe "#content" do - let(:external_file_content) do - <<-HEREDOC - before_script: - - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - ruby -v - - which ruby - - gem install bundler --no-ri --no-rdoc - - bundle install --jobs $(nproc) "${FLAGS[@]}" - HEREDOC - end - - context 'with a local file' do - let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } - - before do - allow_any_instance_of(described_class).to receive(:local_file_content).and_return(external_file_content) - end - - it 'should return the content of the file' do - expect(external_file.content).to eq(external_file_content) - end - end - - context 'with a valid remote file' do - let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - before do - allow(HTTParty).to receive(:get).and_return(external_file_content) - end - - it 'should return the content of the file' do - expect(external_file.content).to eq(external_file_content) - end - end - - context 'with a timeout' do - let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - before do - allow(HTTParty).to receive(:get).and_raise(Timeout::Error) - end - - it 'should return nil' do - expect(external_file.content).to be_nil - end - end - end -end diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb deleted file mode 100644 index 84957c1f6b4..00000000000 --- a/spec/lib/gitlab/ci/external_files/mapper_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -require 'fast_spec_helper' - -describe Gitlab::Ci::ExternalFiles::Mapper do - let(:project) { create(:project, :repository) } - - describe '#process' do - subject { described_class.new(values, project).process } - - context "when 'include' keyword is defined as string" do - let(:values) do - { - include: '/vendor/gitlab-ci-yml/non-existent-file.yml', - image: 'ruby:2.2' - } - end - - it 'returns an array' do - expect(subject).to be_an(Array) - end - - it 'returns ExternalFile instances' do - expect(subject.first).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) - end - end - - context "when 'include' is defined as an array" do - let(:values) do - { - include: - [ - 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', - '/vendor/gitlab-ci-yml/template.yml' - ], - image: 'ruby:2.2' - } - end - - it 'returns an array' do - expect(subject).to be_an(Array) - end - - it 'returns ExternalFile instances' do - expect(subject).to all(be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile)) - end - end - - context "when 'include' is not defined" do - let(:values) do - { image: 'ruby:2.2' } - end - - it 'returns an empty array' do - expect(subject).to be_empty - end - end - end -end diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb index bed364a8c14..4c8a5aae83a 100644 --- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb +++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb @@ -12,12 +12,12 @@ describe BlobViewer::GitlabCiYml do it 'calls prepare! on the viewer' do expect(subject).to receive(:prepare!) - subject.validation_message + subject.validation_message(project, project.default_branch) end context 'when the configuration is valid' do it 'returns nil' do - expect(subject.validation_message).to be_nil + expect(subject.validation_message(project, project.default_branch)).to be_nil end end @@ -25,7 +25,7 @@ describe BlobViewer::GitlabCiYml do let(:data) { 'oof' } it 'returns the error message' do - expect(subject.validation_message).to eq('Invalid configuration format') + expect(subject.validation_message(project, project.default_branch)).to eq('Invalid configuration format') end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 14ccc2960bb..2216705c032 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1743,7 +1743,7 @@ describe Ci::Pipeline, :mailer do create(:ci_pipeline, config: { rspec: { script: 'rake test' } }) end - it 'does not containyaml errors' do + it 'does not contain yaml errors' do expect(pipeline).not_to have_yaml_errors end end From 49598c58ebca5807cc63eb7b17872018c6f48503 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 22:09:29 +0200 Subject: [PATCH 08/23] Update YAML include doc to make it more specific CE mirror or d1baf60ff99f6e56a003b8b6ba12c6aafce10659 --- doc/ci/yaml/README.md | 103 ++++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 44 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 3ade7201177..cf20f763a34 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -968,10 +968,10 @@ skip the download step. ### include -From 10.5 we can use `include` keyword to allow the inclusion of external yml files. +From 10.5 we can use `include` keyword to allow the inclusion of external YAML files. ```yaml -# Content of https://gitlab.com/awesome-project/raw/master/.gitlab-ci-before-script-template.yml +# Content of https://gitlab.com/awesome-project/raw/master/.before-script-template.yml before_script: - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - ruby -v @@ -982,7 +982,7 @@ before_script: ```yaml # Content of gitlab-ci.yml -include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-before-script-template.yml' +include: 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml' rspec: script: @@ -993,23 +993,34 @@ rubocop: - bundle exec rubocop ``` -In the above example `.gitlab-ci-before-script-template.yml` content will be automatically fetched and evaluated along with the content of `gitlab-ci.yml`. +In the above example `.before-script-template.yml` content will be automatically fetched and evaluated along with the content of `gitlab-ci.yml`. `include` supports two types of files: - - **local** to the same repository, referenced using the relative path, or - - **remote** in a different location, accessed using HTTP protocol, referenced using the full URL + +- **local** to the same repository, referenced using the paths in the same the repository, i.e: + +```yaml +# Within the repository +include: '/templates/.gitlab-ci-template.yml' +``` + +- **remote** in a different location, accessed using HTTP/HTTPS protocol, referenced using the full URL, i.e: + +```yaml +include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-template.yml' +``` Also, `include` supports a single string or an array composed by different values, so ```yaml -include: '/templates/.gitlab-ci-templates.yml' +include: '/templates/.gitlab-ci-template.yml' ``` and ```yaml include: - - 'https://gitlab.com/same-group/another-project/raw/master/.gitlab-ci-templates.yml' + - 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-templates.yml' - '/templates/.gitlab-ci-templates.yml' ``` @@ -1017,56 +1028,60 @@ are both valid use cases. #### Restrictions -- We can only use files that are currently tracked by Git on the same branch and commit your configuration file is. In other words, when using a **local file** make sure it's on the latest commit of your branch. -- Since external files defined on `include` are evaluated first, the content on `gitlab-ci.yml` **will take precedence over the content of the external files**, for example: +- We can only use files that are currently tracked by Git on the same branch your configuration file is. In other words, when using a **local file** make sure that both, `gitlab-ci.yml` and the local file are on the same branch. +- Since external files defined on `include` are evaluated first, the content on `gitlab-ci.yml` **will always take precedence over the content of the external files, no matters of the position of the `include` keyword, allowing to override values and functions with local definitions**, for example: ```yaml -# Content of http://company.com/default-gitlab-ci.yml +# Content of http://company.com/ruby-autodeploy-template.yml -image: php:5-fpm-alpine +variables: + KUBE_DOMAIN: example.com -job2: - script: php -v +build: + stage: build + script: + - command build + only: + - master + +deploy: + stage: deploy + script: + - command deploy + environment: + name: production + url: http://production.example.com + only: + - master ``` ```yaml # Content of gitlab-ci.yml -include: - - http://company.com/default-gitlab-ci.yml +include: 'http://company.com/ruby-autodeploy-template.yml' -image: ruby:2.1 +image: registry.gitlab.com/gitlab-examples/kubernetes-deploy -job1: - script: ruby -v +variables: + KUBE_DOMAIN: gitlab.domain.com + +stages: + - build + - deploy + +deploy: + stage: deploy + script: + - command deploy + environment: + name: production + url: http://gitlab.com + only: + - master ``` -In this case both, `job1` and `job2` will be executed with `ruby:2.1` - - - -#### Examples - -**Example of local files included** - -```yaml -include: '/templates/.gitlab-ci-templates.yml' -``` - -**Example of remote files included** - -```yaml -include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-templates.yml' -``` - -**Example of multiple files included** - -```yaml -include: - - 'https://gitlab.com/same-group/another-project/raw/master/.gitlab-ci-templates.yml' - - '/templates/.gitlab-ci-templates.yml' -``` +In this case, the variable `KUBE_DOMAIN` and the `deploy` job defined on `ruby-autodeploy-template.yml` will override by the ones defined on `gitlab-ci.yml`. ## `artifacts` From 1e816a972164c515bf99c39bd8880dd23e534c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 22:22:52 +0200 Subject: [PATCH 09/23] Address MR suggestions CE mirror of c4578b951e331fe8c75cd4f948ce74cec6587bad --- app/models/blob_viewer/gitlab_ci_yml.rb | 8 ++++---- app/models/ci/pipeline.rb | 2 +- .../blob/viewers/_gitlab_ci_yml.html.haml | 4 ++-- lib/gitlab/ci/config.rb | 4 ++-- lib/gitlab/ci/external/file/local.rb | 18 +++++++----------- lib/gitlab/ci/external/file/remote.rb | 18 +++++++++++------- lib/gitlab/ci/external/mapper.rb | 17 +++++++++-------- lib/gitlab/ci/external/processor.rb | 10 +++++----- spec/lib/gitlab/ci/config_spec.rb | 3 +-- spec/lib/gitlab/ci/external/file/local_spec.rb | 9 ++++----- .../lib/gitlab/ci/external/file/remote_spec.rb | 12 ++++++------ spec/lib/gitlab/ci/external/mapper_spec.rb | 12 +++++++----- spec/lib/gitlab/ci/external/processor_spec.rb | 15 +++++++-------- 13 files changed, 66 insertions(+), 66 deletions(-) diff --git a/app/models/blob_viewer/gitlab_ci_yml.rb b/app/models/blob_viewer/gitlab_ci_yml.rb index 54462d01b66..655241c2808 100644 --- a/app/models/blob_viewer/gitlab_ci_yml.rb +++ b/app/models/blob_viewer/gitlab_ci_yml.rb @@ -10,16 +10,16 @@ module BlobViewer self.file_types = %i(gitlab_ci) self.binary = false - def validation_message(project, ref) + def validation_message(project, sha) return @validation_message if defined?(@validation_message) prepare! - @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, { project: project, branch_name: ref }) + @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, { project: project, sha: sha }) end - def valid?(project, ref) - validation_message(project, ref).blank? + def valid?(project, sha) + validation_message(project, sha).blank? end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 27e0ade2722..bec0aff2ba0 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -475,7 +475,7 @@ module Ci end def initialize_yaml_processor - Gitlab::Ci::YamlProcessor.new(ci_yaml_file, { project: project, branch_name: ref }) + ::Gitlab::Ci::YamlProcessor.new(ci_yaml_file, { project: project, sha: sha }) end def ci_yaml_file_path diff --git a/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml b/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml index 6ef09278ee8..5be7cc7f25a 100644 --- a/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml +++ b/app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml @@ -1,9 +1,9 @@ -- if viewer.valid?(@project, @ref) +- if viewer.valid?(@project, @commit.sha) = icon('check fw') This GitLab CI configuration is valid. - else = icon('warning fw') This GitLab CI configuration is invalid: - = viewer.validation_message(@project, @ref) + = viewer.validation_message(@project, @commit.sha) = link_to 'Learn more', help_page_path('ci/yaml/README') diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 1779fbb2a39..19d93c6e785 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -76,8 +76,8 @@ module Gitlab end def process_external_files(config, project, opts) - branch_name = opts.fetch(:branch_name, project.default_branch) - ::Gitlab::Ci::External::Processor.new(config, project, branch_name).perform + sha = opts.fetch(:sha, project.repository.commit.sha) + ::Gitlab::Ci::External::Processor.new(config, project, sha).perform end end end diff --git a/lib/gitlab/ci/external/file/local.rb b/lib/gitlab/ci/external/file/local.rb index dc1ba923411..27e827222e5 100644 --- a/lib/gitlab/ci/external/file/local.rb +++ b/lib/gitlab/ci/external/file/local.rb @@ -3,16 +3,16 @@ module Gitlab module External module File class Local - attr_reader :value, :project, :branch_name + attr_reader :location, :project, :branch_name - def initialize(value, project, branch_name) - @value = value - @project = project - @branch_name = branch_name + def initialize(location, opts = {}) + @location = location + @project = opts.fetch(:project) + @sha = opts.fetch(:sha) end def valid? - commit && local_file_content + local_file_content end def content @@ -21,12 +21,8 @@ module Gitlab private - def commit - @commit ||= project.repository.commit(branch_name) - end - def local_file_content - @local_file_content ||= project.repository.blob_data_at(commit.sha, value) + @local_file_content ||= project.repository.blob_data_at(sha, location) end end end diff --git a/lib/gitlab/ci/external/file/remote.rb b/lib/gitlab/ci/external/file/remote.rb index 7d8cd1957ad..aa089860525 100644 --- a/lib/gitlab/ci/external/file/remote.rb +++ b/lib/gitlab/ci/external/file/remote.rb @@ -3,20 +3,24 @@ module Gitlab module External module File class Remote - attr_reader :value + attr_reader :location - def initialize(value) - @value = value + def initialize(location, opts = {}) + @location = location end def valid? - ::Gitlab::UrlSanitizer.valid?(value) && content + ::Gitlab::UrlSanitizer.valid?(location) && content end def content - HTTParty.get(value) - rescue HTTParty::Error, Timeout::Error - false + return @content if defined?(@content) + + @content ||= begin + HTTParty.get(location) + rescue HTTParty::Error, Timeout::Error + false + end end end end diff --git a/lib/gitlab/ci/external/mapper.rb b/lib/gitlab/ci/external/mapper.rb index f2e5ec972df..cc9be317ebe 100644 --- a/lib/gitlab/ci/external/mapper.rb +++ b/lib/gitlab/ci/external/mapper.rb @@ -2,27 +2,28 @@ module Gitlab module Ci module External class Mapper - def initialize(values, project, branch_name) - @paths = Array(values.fetch(:include, [])) + def initialize(values, project, sha) + @locations = Array(values.fetch(:include, [])) @project = project - @branch_name = branch_name + @sha = sha end def process - paths.map { |path| build_external_file(path) } + locations.map { |location| build_external_file(location) } end private - attr_reader :paths, :project, :branch_name + attr_reader :locations, :project, :sha - def build_external_file(path) - remote_file = Gitlab::Ci::External::File::Remote.new(path) + def build_external_file(location) + remote_file = Gitlab::Ci::External::File::Remote.new(location) if remote_file.valid? remote_file else - ::Gitlab::Ci::External::File::Local.new(path, project, branch_name) + options = { project: project, sha: sha } + Gitlab::Ci::External::File::Local.new(location, options) end end end diff --git a/lib/gitlab/ci/external/processor.rb b/lib/gitlab/ci/external/processor.rb index 5cae0166346..44dc3183367 100644 --- a/lib/gitlab/ci/external/processor.rb +++ b/lib/gitlab/ci/external/processor.rb @@ -4,9 +4,9 @@ module Gitlab class Processor FileError = Class.new(StandardError) - def initialize(values, project, branch_name) + def initialize(values, project, sha) @values = values - @external_files = ::Gitlab::Ci::External::Mapper.new(values, project, branch_name).process + @external_files = Gitlab::Ci::External::Mapper.new(values, project, sha).process @content = {} end @@ -18,7 +18,7 @@ module Gitlab @content.merge!(content_of(external_file)) end - append_external_content + append_inline_content remove_include_keyword end @@ -28,7 +28,7 @@ module Gitlab def validate_external_file(external_file) unless external_file.valid? - raise FileError, "External file: '#{external_file.value}' should be a valid local or remote file" + raise FileError, "External file: '#{external_file.location}' should be a valid local or remote file" end end @@ -36,7 +36,7 @@ module Gitlab ::Gitlab::Ci::Config::Loader.new(external_file.content).load! end - def append_external_content + def append_inline_content @content.merge!(@values) end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 1ac832e2b16..1ab5ccb63d0 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -5,7 +5,7 @@ require_dependency 'active_model' describe Gitlab::Ci::Config do let(:project) { create(:project, :repository) } let(:config) do - described_class.new(gitlab_ci_yml, { project: project, branch_name: 'testing' }) + described_class.new(gitlab_ci_yml, { project: project, sha: '12345' }) end context 'when config is valid' do @@ -149,7 +149,6 @@ describe Gitlab::Ci::Config do end before do - allow_any_instance_of(::Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') allow_any_instance_of(::Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) allow(HTTParty).to receive(:get).and_return(http_file_content) end diff --git a/spec/lib/gitlab/ci/external/file/local_spec.rb b/spec/lib/gitlab/ci/external/file/local_spec.rb index 9e1e49da7cb..8d3545d5009 100644 --- a/spec/lib/gitlab/ci/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/external/file/local_spec.rb @@ -2,14 +2,13 @@ require 'fast_spec_helper' describe Gitlab::Ci::External::File::Local do let(:project) { create(:project, :repository) } - let(:local_file) { described_class.new(value, project, 'testing') } + let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } describe "#valid?" do context 'when is a valid local path' do - let(:value) { '/vendor/gitlab-ci-yml/existent-file.yml' } + let(:location) { '/vendor/gitlab-ci-yml/existent-file.yml' } before do - allow_any_instance_of(described_class).to receive(:commit).and_return('12345') allow_any_instance_of(described_class).to receive(:local_file_content).and_return("image: 'ruby2:2'") end @@ -19,7 +18,7 @@ describe Gitlab::Ci::External::File::Local do end context 'when is not a valid local path' do - let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + let(:location) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } it 'should return false' do expect(local_file.valid?).to be_falsy @@ -39,7 +38,7 @@ describe Gitlab::Ci::External::File::Local do end context 'with a local file' do - let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + let(:location) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } before do allow_any_instance_of(described_class).to receive(:local_file_content).and_return(local_file_content) diff --git a/spec/lib/gitlab/ci/external/file/remote_spec.rb b/spec/lib/gitlab/ci/external/file/remote_spec.rb index 91b50dcf5fd..620c8e1001e 100644 --- a/spec/lib/gitlab/ci/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/external/file/remote_spec.rb @@ -1,8 +1,8 @@ require 'fast_spec_helper' describe Gitlab::Ci::External::File::Remote do - let(:remote_file) { described_class.new(value) } - let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:remote_file) { described_class.new(location) } + let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:remote_file_content) do <<~HEREDOC before_script: @@ -17,7 +17,7 @@ describe Gitlab::Ci::External::File::Remote do describe "#valid?" do context 'when is a valid remote url' do before do - allow(HTTParty).to receive(:get).and_return(remote_file_content) + WebMock.stub_request(:get, location).to_return(body: remote_file_content) end it 'should return true' do @@ -26,7 +26,7 @@ describe Gitlab::Ci::External::File::Remote do end context 'when is not a valid remote url' do - let(:value) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } it 'should return false' do expect(remote_file.valid?).to be_falsy @@ -47,11 +47,11 @@ describe Gitlab::Ci::External::File::Remote do describe "#content" do context 'with a valid remote file' do before do - allow(HTTParty).to receive(:get).and_return(remote_file_content) + WebMock.stub_request(:get, location).to_return(body: remote_file_content) end it 'should return the content of the file' do - expect(remote_file.content).to eq(remote_file_content) + expect(remote_file.content).to eql(remote_file_content) end end diff --git a/spec/lib/gitlab/ci/external/mapper_spec.rb b/spec/lib/gitlab/ci/external/mapper_spec.rb index c752a23def6..b1399ae5382 100644 --- a/spec/lib/gitlab/ci/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external/mapper_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Ci::External::Mapper do end describe '#process' do - subject { described_class.new(values, project, 'testing').process } + subject { described_class.new(values, project, '123456').process } context "when 'include' keyword is defined as string" do context 'when the string is a local file' do @@ -30,15 +30,16 @@ describe Gitlab::Ci::External::Mapper do end context 'when the string is a remote file' do + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:values) do { - include: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + include: remote_url, image: 'ruby:2.2' } end before do - allow(HTTParty).to receive(:get).and_return(file_content) + WebMock.stub_request(:get, remote_url).to_return(body: file_content) end it 'returns an array' do @@ -52,11 +53,12 @@ describe Gitlab::Ci::External::Mapper do end context "when 'include' is defined as an array" do + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:values) do { include: [ - 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + remote_url, '/vendor/gitlab-ci-yml/template.yml' ], image: 'ruby:2.2' @@ -64,7 +66,7 @@ describe Gitlab::Ci::External::Mapper do end before do - allow(HTTParty).to receive(:get).and_return(file_content) + WebMock.stub_request(:get, remote_url).to_return(body: file_content) end it 'returns an array' do diff --git a/spec/lib/gitlab/ci/external/processor_spec.rb b/spec/lib/gitlab/ci/external/processor_spec.rb index ba9e106706e..ca28558bad3 100644 --- a/spec/lib/gitlab/ci/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/external/processor_spec.rb @@ -2,7 +2,7 @@ require 'fast_spec_helper' describe Gitlab::Ci::External::Processor do let(:project) { create(:project, :repository) } - let(:processor) { described_class.new(values, project, 'testing') } + let(:processor) { described_class.new(values, project, '12345') } describe "#perform" do context 'when no external files defined' do @@ -36,7 +36,8 @@ describe Gitlab::Ci::External::Processor do end context 'with a valid remote external file is defined' do - let(:values) { { include: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:values) { { include: remote_url, image: 'ruby:2.2' } } let(:external_file_content) do <<-HEREDOC before_script: @@ -57,7 +58,7 @@ describe Gitlab::Ci::External::Processor do end before do - allow(HTTParty).to receive(:get).and_return(external_file_content) + WebMock.stub_request(:get, remote_url).to_return(body: external_file_content) end it 'should append the file to the values' do @@ -84,7 +85,6 @@ describe Gitlab::Ci::External::Processor do end before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) end @@ -99,10 +99,11 @@ describe Gitlab::Ci::External::Processor do end context 'with multiple external files are defined' do + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:external_files) do [ "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml", - 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' + remote_url ] end let(:values) do @@ -123,9 +124,8 @@ describe Gitlab::Ci::External::Processor do before do local_file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) - allow(HTTParty).to receive(:get).and_return(remote_file_content) + WebMock.stub_request(:get, remote_url).to_return(body: remote_file_content) end it 'should append the files to the values' do @@ -143,7 +143,6 @@ describe Gitlab::Ci::External::Processor do let(:local_file_content) { 'invalid content file ////' } before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:commit).and_return('12345') allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) end From c6f4647c353ba0bc6cf82ec79b58fc3f926786b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 22:26:21 +0200 Subject: [PATCH 10/23] Update yaml documentation examples CE mirror of b8d12ec36b50fdae966361b4d6acc4791ef1830c --- doc/ci/yaml/README.md | 74 ++++++++++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index cf20f763a34..d5c3400073a 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1032,56 +1032,80 @@ are both valid use cases. - Since external files defined on `include` are evaluated first, the content on `gitlab-ci.yml` **will always take precedence over the content of the external files, no matters of the position of the `include` keyword, allowing to override values and functions with local definitions**, for example: ```yaml -# Content of http://company.com/ruby-autodeploy-template.yml +# Content of http://company.com/autodevops-template.yml variables: - KUBE_DOMAIN: example.com + POSTGRES_USER: user + POSTGRES_PASSWORD: testing_password + POSTGRES_ENABLED: "true" + POSTGRES_DB: $CI_ENVIRONMENT_SLUG + KUBERNETES_VERSION: 1.8.6 + HELM_VERSION: 2.6.1 + CODECLIMATE_VERSION: 0.69.0 -build: - stage: build +production: + stage: production script: - - command build - only: - - master - -deploy: - stage: deploy - script: - - command deploy + - check_kube_domain + - install_dependencies + - download_chart + - ensure_namespace + - install_tiller + - create_secret + - deploy + - delete canary + - persist_environment_url environment: name: production - url: http://production.example.com + url: http://$CI_PROJECT_PATH_SLUG.$AUTO_DEVOPS_DOMAIN only: - - master + refs: + - master + kubernetes: active ``` - ```yaml # Content of gitlab-ci.yml -include: 'http://company.com/ruby-autodeploy-template.yml' +include: 'https://company.com/autodevops-template.yml' -image: registry.gitlab.com/gitlab-examples/kubernetes-deploy +image: alpine:latest variables: - KUBE_DOMAIN: gitlab.domain.com + POSTGRES_USER: root + POSTGRES_PASSWORD: secure_password + POSTGRES_DB: company_database stages: - build - - deploy + - test + - review + - dast + - staging + - canary + - production + - performance + - cleanup -deploy: - stage: deploy +production: + stage: production script: - - command deploy + - check_kube_domain + - install_dependencies + - download_chart + - ensure_namespace + - install_tiller + - create_secret + - deploy environment: name: production - url: http://gitlab.com + url: http://auto_devops_domain.com only: - - master + refs: + - master ``` -In this case, the variable `KUBE_DOMAIN` and the `deploy` job defined on `ruby-autodeploy-template.yml` will override by the ones defined on `gitlab-ci.yml`. +In this case, the variables `POSTGRES_USER`, `POSTGRES_PASSWORD` and `POSTGRES_DB` along with the `production` job defined on `autodevops-template.yml` will be overridden by the ones defined on `gitlab-ci.yml`. ## `artifacts` From 3af363ecb2cdc764bb9034e81c7159f65c19cb44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 22:28:07 +0200 Subject: [PATCH 11/23] Replace gitlab-ci.yml for .gitlab-ci.yml on yaml doc CE mirror of cfc0bf818d5c0fb5141832d4caf1fcc34de3e9f9 --- doc/ci/yaml/README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index d5c3400073a..fb867a857e3 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -981,7 +981,7 @@ before_script: ``` ```yaml -# Content of gitlab-ci.yml +# Content of .gitlab-ci.yml include: 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml' rspec: @@ -993,7 +993,7 @@ rubocop: - bundle exec rubocop ``` -In the above example `.before-script-template.yml` content will be automatically fetched and evaluated along with the content of `gitlab-ci.yml`. +In the above example `.before-script-template.yml` content will be automatically fetched and evaluated along with the content of `.gitlab-ci.yml`. `include` supports two types of files: @@ -1028,8 +1028,8 @@ are both valid use cases. #### Restrictions -- We can only use files that are currently tracked by Git on the same branch your configuration file is. In other words, when using a **local file** make sure that both, `gitlab-ci.yml` and the local file are on the same branch. -- Since external files defined on `include` are evaluated first, the content on `gitlab-ci.yml` **will always take precedence over the content of the external files, no matters of the position of the `include` keyword, allowing to override values and functions with local definitions**, for example: +- We can only use files that are currently tracked by Git on the same branch your configuration file is. In other words, when using a **local file** make sure that both, `.gitlab-ci.yml` and the local file are on the same branch. +- Since external files defined on `include` are evaluated first, the content on `.gitlab-ci.yml` **will always take precedence over the content of the external files, no matters of the position of the `include` keyword, allowing to override values and functions with local definitions**, for example: ```yaml # Content of http://company.com/autodevops-template.yml @@ -1065,7 +1065,7 @@ production: ``` ```yaml -# Content of gitlab-ci.yml +# Content of .gitlab-ci.yml include: 'https://company.com/autodevops-template.yml' @@ -1105,7 +1105,7 @@ production: - master ``` -In this case, the variables `POSTGRES_USER`, `POSTGRES_PASSWORD` and `POSTGRES_DB` along with the `production` job defined on `autodevops-template.yml` will be overridden by the ones defined on `gitlab-ci.yml`. +In this case, the variables `POSTGRES_USER`, `POSTGRES_PASSWORD` and `POSTGRES_DB` along with the `production` job defined on `autodevops-template.yml` will be overridden by the ones defined on `.gitlab-ci.yml`. ## `artifacts` From 6a8133b943a8e06571f5497bea0f36c236b2bf90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 22:37:37 +0200 Subject: [PATCH 12/23] Stub http request on specs intead of mocking HTTParty CE mirror of bb2a9fde8e6a4d1df13638fe336f641b9c72ef59 --- lib/gitlab/ci/external/file/remote.rb | 13 ++++++++----- spec/lib/gitlab/ci/config_spec.rb | 16 +++++++++------- spec/lib/gitlab/ci/external/processor_spec.rb | 17 +++++++++-------- spec/models/blob_viewer/gitlab_ci_yml_spec.rb | 10 ++++++---- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/lib/gitlab/ci/external/file/remote.rb b/lib/gitlab/ci/external/file/remote.rb index aa089860525..e728e3de77d 100644 --- a/lib/gitlab/ci/external/file/remote.rb +++ b/lib/gitlab/ci/external/file/remote.rb @@ -3,6 +3,7 @@ module Gitlab module External module File class Remote + include Gitlab::Utils::StrongMemoize attr_reader :location def initialize(location, opts = {}) @@ -16,11 +17,13 @@ module Gitlab def content return @content if defined?(@content) - @content ||= begin - HTTParty.get(location) - rescue HTTParty::Error, Timeout::Error - false - end + @content = strong_memoize(:content) do + begin + HTTParty.get(location) + rescue HTTParty::Error, Timeout::Error + false + end + end end end end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 1ab5ccb63d0..83a30de199f 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -127,7 +127,7 @@ describe Gitlab::Ci::Config do end context "when gitlab_ci_yml has valid 'include' defined" do - let(:http_file_content) do + let(:remote_file_content) do <<~HEREDOC variables: AUTO_DEVOPS_DOMAIN: domain.example.com @@ -138,11 +138,12 @@ describe Gitlab::Ci::Config do HEREDOC end let(:local_file_content) { File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") } - let(:yml) do + let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:gitlab_ci_yml) do <<-EOS include: - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml - - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml + - #{remote_location} image: ruby:2.2 EOS @@ -150,7 +151,7 @@ describe Gitlab::Ci::Config do before do allow_any_instance_of(::Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) - allow(HTTParty).to receive(:get).and_return(http_file_content) + WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content) end it 'should return a composed hash' do @@ -194,23 +195,24 @@ describe Gitlab::Ci::Config do end context "when both external files and gitlab_ci.yml defined the same key" do + let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:gitlab_ci_yml) do <<~HEREDOC include: - - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.gitlab_ci_yml + - #{remote_location} image: ruby:2.2 HEREDOC end - let(:http_file_content) do + let(:remote_file_content) do <<~HEREDOC image: php:5-fpm-alpine HEREDOC end it 'should take precedence' do - allow(HTTParty).to receive(:get).and_return(http_file_content) + WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content) expect(config.to_hash).to eq({ image: 'ruby:2.2' }) end end diff --git a/spec/lib/gitlab/ci/external/processor_spec.rb b/spec/lib/gitlab/ci/external/processor_spec.rb index ca28558bad3..588baf92bee 100644 --- a/spec/lib/gitlab/ci/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/external/processor_spec.rb @@ -36,8 +36,8 @@ describe Gitlab::Ci::External::Processor do end context 'with a valid remote external file is defined' do - let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:values) { { include: remote_url, image: 'ruby:2.2' } } + let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:values) { { include: remote_file, image: 'ruby:2.2' } } let(:external_file_content) do <<-HEREDOC before_script: @@ -58,7 +58,7 @@ describe Gitlab::Ci::External::Processor do end before do - WebMock.stub_request(:get, remote_url).to_return(body: external_file_content) + WebMock.stub_request(:get, remote_file).to_return(body: external_file_content) end it 'should append the file to the values' do @@ -99,11 +99,11 @@ describe Gitlab::Ci::External::Processor do end context 'with multiple external files are defined' do - let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:external_files) do [ "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml", - remote_url + remote_file ] end let(:values) do @@ -125,7 +125,7 @@ describe Gitlab::Ci::External::Processor do before do local_file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) - WebMock.stub_request(:get, remote_url).to_return(body: remote_file_content) + WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) end it 'should append the files to the values' do @@ -152,9 +152,10 @@ describe Gitlab::Ci::External::Processor do end context "when both external files and values defined the same key" do + let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:values) do { - include: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + include: remote_file, image: 'ruby:2.2' } end @@ -166,7 +167,7 @@ describe Gitlab::Ci::External::Processor do end it 'should take precedence' do - allow(HTTParty).to receive(:get).and_return(remote_file_content) + WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) expect(processor.perform[:image]).to eq('ruby:2.2') end end diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb index 4c8a5aae83a..48345b192d1 100644 --- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb +++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb @@ -2,22 +2,24 @@ require 'spec_helper' describe BlobViewer::GitlabCiYml do include FakeBlobHelpers + include RepoHelpers - let(:project) { build_stubbed(:project) } + let(:project) { build_stubbed(:project, :repository) } let(:data) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } let(:blob) { fake_blob(path: '.gitlab-ci.yml', data: data) } + let(:sha) { sample_commit.id } subject { described_class.new(blob) } describe '#validation_message' do it 'calls prepare! on the viewer' do expect(subject).to receive(:prepare!) - subject.validation_message(project, project.default_branch) + subject.validation_message(project, sha) end context 'when the configuration is valid' do it 'returns nil' do - expect(subject.validation_message(project, project.default_branch)).to be_nil + expect(subject.validation_message(project, sha)).to be_nil end end @@ -25,7 +27,7 @@ describe BlobViewer::GitlabCiYml do let(:data) { 'oof' } it 'returns the error message' do - expect(subject.validation_message(project, project.default_branch)).to eq('Invalid configuration format') + expect(subject.validation_message(project, sha)).to eq('Invalid configuration format') end end end From 797046e358bcf0fcd8cab413fcccad1614180aa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Sep 2018 12:09:14 +0200 Subject: [PATCH 13/23] Reconcile differences in lib/gitlab/ci/external/file --- lib/gitlab/ci/external/file/base.rb | 27 +++++++ lib/gitlab/ci/external/file/local.rb | 19 ++--- lib/gitlab/ci/external/file/remote.rb | 20 +++--- .../lib/gitlab/ci/external/file/local_spec.rb | 71 ++++++++++++------- .../gitlab/ci/external/file/remote_spec.rb | 52 ++++++++++++-- 5 files changed, 140 insertions(+), 49 deletions(-) create mode 100644 lib/gitlab/ci/external/file/base.rb diff --git a/lib/gitlab/ci/external/file/base.rb b/lib/gitlab/ci/external/file/base.rb new file mode 100644 index 00000000000..4591b3ec82e --- /dev/null +++ b/lib/gitlab/ci/external/file/base.rb @@ -0,0 +1,27 @@ +module Gitlab + module Ci + module External + module File + class Base + YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze + + def initialize(location, opts = {}) + @location = location + end + + def valid? + location.match(YAML_WHITELIST_EXTENSION) && content + end + + def content + raise NotImplementedError, 'content must be implemented and return a string or nil' + end + + def error_message + raise NotImplementedError, 'error_message must be implemented and return a string' + end + end + end + end + end +end diff --git a/lib/gitlab/ci/external/file/local.rb b/lib/gitlab/ci/external/file/local.rb index 27e827222e5..829cc9b3d19 100644 --- a/lib/gitlab/ci/external/file/local.rb +++ b/lib/gitlab/ci/external/file/local.rb @@ -2,27 +2,28 @@ module Gitlab module Ci module External module File - class Local - attr_reader :location, :project, :branch_name + class Local < Base + attr_reader :location, :project, :sha def initialize(location, opts = {}) - @location = location + super + @project = opts.fetch(:project) @sha = opts.fetch(:sha) end - def valid? - local_file_content + def content + @content ||= fetch_local_content end - def content - local_file_content + def error_message + "Local file '#{location}' is not valid." end private - def local_file_content - @local_file_content ||= project.repository.blob_data_at(sha, location) + def fetch_local_content + project.repository.blob_data_at(sha, location) end end end diff --git a/lib/gitlab/ci/external/file/remote.rb b/lib/gitlab/ci/external/file/remote.rb index e728e3de77d..b4294231265 100644 --- a/lib/gitlab/ci/external/file/remote.rb +++ b/lib/gitlab/ci/external/file/remote.rb @@ -2,29 +2,25 @@ module Gitlab module Ci module External module File - class Remote + class Remote < Base include Gitlab::Utils::StrongMemoize attr_reader :location - def initialize(location, opts = {}) - @location = location - end - - def valid? - ::Gitlab::UrlSanitizer.valid?(location) && content - end - def content return @content if defined?(@content) @content = strong_memoize(:content) do begin - HTTParty.get(location) - rescue HTTParty::Error, Timeout::Error - false + Gitlab::HTTP.get(location) + rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError + nil end end end + + def error_message + "Remote file '#{location}' is not valid." + end end end end diff --git a/spec/lib/gitlab/ci/external/file/local_spec.rb b/spec/lib/gitlab/ci/external/file/local_spec.rb index 8d3545d5009..a4099703ac9 100644 --- a/spec/lib/gitlab/ci/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/external/file/local_spec.rb @@ -1,15 +1,15 @@ -require 'fast_spec_helper' +require 'spec_helper' describe Gitlab::Ci::External::File::Local do let(:project) { create(:project, :repository) } let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } - describe "#valid?" do + describe '#valid?' do context 'when is a valid local path' do let(:location) { '/vendor/gitlab-ci-yml/existent-file.yml' } before do - allow_any_instance_of(described_class).to receive(:local_file_content).and_return("image: 'ruby2:2'") + allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return("image: 'ruby2:2'") end it 'should return true' do @@ -25,29 +25,52 @@ describe Gitlab::Ci::External::File::Local do end end - describe "#content" do - let(:local_file_content) do - <<~HEREDOC - before_script: - - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - ruby -v - - which ruby - - gem install bundler --no-ri --no-rdoc - - bundle install --jobs $(nproc) "${FLAGS[@]}" - HEREDOC - end + context 'when is not a yaml file' do + let(:location) { '/config/application.rb' } - context 'with a local file' do - let(:location) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } - - before do - allow_any_instance_of(described_class).to receive(:local_file_content).and_return(local_file_content) - end - - it 'should return the content of the file' do - expect(local_file.content).to eq(local_file_content) - end + it 'should return false' do + expect(local_file.valid?).to be_falsy end end end + + describe '#content' do + context 'with a a valid file' do + let(:local_file_content) do + <<~HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + end + let(:location) { '/vendor/gitlab-ci-yml/existent-file.yml' } + + before do + allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return(local_file_content) + end + + it 'should return the content of the file' do + expect(local_file.content).to eq(local_file_content) + end + end + + context 'with an invalid file' do + let(:location) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + + it 'should be nil' do + expect(local_file.content).to be_nil + end + end + end + + describe '#error_message' do + let(:location) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } + + it 'should return an error message' do + expect(local_file.error_message).to eq("Local file '#{location}' is not valid.") + end + end end diff --git a/spec/lib/gitlab/ci/external/file/remote_spec.rb b/spec/lib/gitlab/ci/external/file/remote_spec.rb index 620c8e1001e..314dcfec511 100644 --- a/spec/lib/gitlab/ci/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/external/file/remote_spec.rb @@ -1,4 +1,4 @@ -require 'fast_spec_helper' +require 'spec_helper' describe Gitlab::Ci::External::File::Remote do let(:remote_file) { described_class.new(location) } @@ -25,7 +25,7 @@ describe Gitlab::Ci::External::File::Remote do end end - context 'when is not a valid remote url' do + context 'with an irregular url' do let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } it 'should return false' do @@ -35,13 +35,29 @@ describe Gitlab::Ci::External::File::Remote do context 'with a timeout' do before do - allow(HTTParty).to receive(:get).and_raise(Timeout::Error) + allow(Gitlab::HTTP).to receive(:get).and_raise(Timeout::Error) end it 'should be falsy' do expect(remote_file.valid?).to be_falsy end end + + context 'when is not a yaml file' do + let(:location) { 'https://asdasdasdaj48ggerexample.com' } + + it 'should be falsy' do + expect(remote_file.valid?).to be_falsy + end + end + + context 'with an internal url' do + let(:location) { 'http://localhost:8080' } + + it 'should be falsy' do + expect(remote_file.valid?).to be_falsy + end + end end describe "#content" do @@ -57,12 +73,40 @@ describe Gitlab::Ci::External::File::Remote do context 'with a timeout' do before do - allow(HTTParty).to receive(:get).and_raise(Timeout::Error) + allow(Gitlab::HTTP).to receive(:get).and_raise(Timeout::Error) end it 'should be falsy' do expect(remote_file.content).to be_falsy end end + + context 'with an invalid remote url' do + let(:location) { 'https://asdasdasdaj48ggerexample.com' } + + before do + WebMock.stub_request(:get, location).to_raise(SocketError.new('Some HTTP error')) + end + + it 'should be nil' do + expect(remote_file.content).to be_nil + end + end + + context 'with an internal url' do + let(:location) { 'http://localhost:8080' } + + it 'should be nil' do + expect(remote_file.content).to be_nil + end + end + end + + describe "#error_message" do + let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it 'should return an error message' do + expect(remote_file.error_message).to eq("Remote file '#{location}' is not valid.") + end end end From 5e0ce238d2c19d11fac5dd40b922b01f134ffcb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Sep 2018 14:09:26 +0200 Subject: [PATCH 14/23] Reconcile differences in lib/gitlab/ci/external --- lib/gitlab/ci/external/mapper.rb | 6 ++--- lib/gitlab/ci/external/processor.rb | 8 +++---- spec/lib/gitlab/ci/external/mapper_spec.rb | 6 ++--- spec/lib/gitlab/ci/external/processor_spec.rb | 23 +++++++++++-------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/lib/gitlab/ci/external/mapper.rb b/lib/gitlab/ci/external/mapper.rb index cc9be317ebe..3c359efa803 100644 --- a/lib/gitlab/ci/external/mapper.rb +++ b/lib/gitlab/ci/external/mapper.rb @@ -17,10 +17,8 @@ module Gitlab attr_reader :locations, :project, :sha def build_external_file(location) - remote_file = Gitlab::Ci::External::File::Remote.new(location) - - if remote_file.valid? - remote_file + if ::Gitlab::UrlSanitizer.valid?(location) + Gitlab::Ci::External::File::Remote.new(location) else options = { project: project, sha: sha } Gitlab::Ci::External::File::Local.new(location, options) diff --git a/lib/gitlab/ci/external/processor.rb b/lib/gitlab/ci/external/processor.rb index 44dc3183367..22588867a08 100644 --- a/lib/gitlab/ci/external/processor.rb +++ b/lib/gitlab/ci/external/processor.rb @@ -15,7 +15,7 @@ module Gitlab external_files.each do |external_file| validate_external_file(external_file) - @content.merge!(content_of(external_file)) + @content.deep_merge!(content_of(external_file)) end append_inline_content @@ -28,16 +28,16 @@ module Gitlab def validate_external_file(external_file) unless external_file.valid? - raise FileError, "External file: '#{external_file.location}' should be a valid local or remote file" + raise FileError, external_file.error_message end end def content_of(external_file) - ::Gitlab::Ci::Config::Loader.new(external_file.content).load! + Gitlab::Ci::Config::Loader.new(external_file.content).load! end def append_inline_content - @content.merge!(@values) + @content.deep_merge!(@values) end def remove_include_keyword diff --git a/spec/lib/gitlab/ci/external/mapper_spec.rb b/spec/lib/gitlab/ci/external/mapper_spec.rb index b1399ae5382..89285c8ba45 100644 --- a/spec/lib/gitlab/ci/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external/mapper_spec.rb @@ -1,4 +1,4 @@ -require 'fast_spec_helper' +require 'spec_helper' describe Gitlab::Ci::External::Mapper do let(:project) { create(:project, :repository) } @@ -25,7 +25,7 @@ describe Gitlab::Ci::External::Mapper do end it 'returns File instances' do - expect(subject.first).to be_an_instance_of(::Gitlab::Ci::External::File::Local) + expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Local) end end @@ -47,7 +47,7 @@ describe Gitlab::Ci::External::Mapper do end it 'returns File instances' do - expect(subject.first).to be_an_instance_of(::Gitlab::Ci::External::File::Remote) + expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Remote) end end end diff --git a/spec/lib/gitlab/ci/external/processor_spec.rb b/spec/lib/gitlab/ci/external/processor_spec.rb index 588baf92bee..fc9a0748075 100644 --- a/spec/lib/gitlab/ci/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/external/processor_spec.rb @@ -1,4 +1,4 @@ -require 'fast_spec_helper' +require 'spec_helper' describe Gitlab::Ci::External::Processor do let(:project) { create(:project, :repository) } @@ -19,18 +19,23 @@ describe Gitlab::Ci::External::Processor do it 'should raise an error' do expect { processor.perform }.to raise_error( described_class::FileError, - "External file: '/vendor/gitlab-ci-yml/non-existent-file.yml' should be a valid local or remote file" + "Local file '/vendor/gitlab-ci-yml/non-existent-file.yml' is not valid." ) end end context 'when an invalid remote file is defined' do - let(:values) { { include: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } } + let(:remote_file) { 'http://doesntexist.com/.gitlab-ci-1.yml' } + let(:values) { { include: remote_file, image: 'ruby:2.2' } } + + before do + WebMock.stub_request(:get, remote_file).to_raise(SocketError.new('Some HTTP error')) + end it 'should raise an error' do expect { processor.perform }.to raise_error( described_class::FileError, - "External file: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' should be a valid local or remote file" + "Remote file '#{remote_file}' is not valid." ) end end @@ -85,7 +90,7 @@ describe Gitlab::Ci::External::Processor do end before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) end it 'should append the file to the values' do @@ -102,7 +107,7 @@ describe Gitlab::Ci::External::Processor do let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:external_files) do [ - "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml", + '/ee/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml', remote_file ] end @@ -123,8 +128,8 @@ describe Gitlab::Ci::External::Processor do end before do - local_file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) + local_file_content = File.read(Rails.root.join('ee/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml')) + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) end @@ -143,7 +148,7 @@ describe Gitlab::Ci::External::Processor do let(:local_file_content) { 'invalid content file ////' } before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) end it 'should raise an error' do From 776bca5a8f6c3cb714451066f90a98660ae0c8d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Sep 2018 14:19:09 +0200 Subject: [PATCH 15/23] Reconcile differences in doc/ci/yaml --- doc/ci/yaml/README.md | 322 ++++++++++++++++++++++++------------------ 1 file changed, 181 insertions(+), 141 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index fb867a857e3..d50093e8563 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -966,147 +966,6 @@ Additionally, if you have a job that unconditionally recreates the cache without reference to its previous contents, you can use `policy: push` in that job to skip the download step. -### include - -From 10.5 we can use `include` keyword to allow the inclusion of external YAML files. - -```yaml -# Content of https://gitlab.com/awesome-project/raw/master/.before-script-template.yml -before_script: - - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - ruby -v - - which ruby - - gem install bundler --no-ri --no-rdoc - - bundle install --jobs $(nproc) "${FLAGS[@]}" -``` - -```yaml -# Content of .gitlab-ci.yml -include: 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml' - -rspec: - script: - - bundle exec rspec - -rubocop: - script: - - bundle exec rubocop -``` - -In the above example `.before-script-template.yml` content will be automatically fetched and evaluated along with the content of `.gitlab-ci.yml`. - -`include` supports two types of files: - -- **local** to the same repository, referenced using the paths in the same the repository, i.e: - -```yaml -# Within the repository -include: '/templates/.gitlab-ci-template.yml' -``` - -- **remote** in a different location, accessed using HTTP/HTTPS protocol, referenced using the full URL, i.e: - -```yaml -include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-template.yml' -``` - -Also, `include` supports a single string or an array composed by different values, so - -```yaml -include: '/templates/.gitlab-ci-template.yml' -``` - -and - -```yaml -include: - - 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-templates.yml' - - '/templates/.gitlab-ci-templates.yml' -``` - -are both valid use cases. - -#### Restrictions - -- We can only use files that are currently tracked by Git on the same branch your configuration file is. In other words, when using a **local file** make sure that both, `.gitlab-ci.yml` and the local file are on the same branch. -- Since external files defined on `include` are evaluated first, the content on `.gitlab-ci.yml` **will always take precedence over the content of the external files, no matters of the position of the `include` keyword, allowing to override values and functions with local definitions**, for example: - -```yaml -# Content of http://company.com/autodevops-template.yml - -variables: - POSTGRES_USER: user - POSTGRES_PASSWORD: testing_password - POSTGRES_ENABLED: "true" - POSTGRES_DB: $CI_ENVIRONMENT_SLUG - KUBERNETES_VERSION: 1.8.6 - HELM_VERSION: 2.6.1 - CODECLIMATE_VERSION: 0.69.0 - -production: - stage: production - script: - - check_kube_domain - - install_dependencies - - download_chart - - ensure_namespace - - install_tiller - - create_secret - - deploy - - delete canary - - persist_environment_url - environment: - name: production - url: http://$CI_PROJECT_PATH_SLUG.$AUTO_DEVOPS_DOMAIN - only: - refs: - - master - kubernetes: active -``` - -```yaml -# Content of .gitlab-ci.yml - -include: 'https://company.com/autodevops-template.yml' - -image: alpine:latest - -variables: - POSTGRES_USER: root - POSTGRES_PASSWORD: secure_password - POSTGRES_DB: company_database - -stages: - - build - - test - - review - - dast - - staging - - canary - - production - - performance - - cleanup - -production: - stage: production - script: - - check_kube_domain - - install_dependencies - - download_chart - - ensure_namespace - - install_tiller - - create_secret - - deploy - environment: - name: production - url: http://auto_devops_domain.com - only: - refs: - - master -``` - -In this case, the variables `POSTGRES_USER`, `POSTGRES_PASSWORD` and `POSTGRES_DB` along with the `production` job defined on `autodevops-template.yml` will be overridden by the ones defined on `.gitlab-ci.yml`. - ## `artifacts` > **Notes:** @@ -1493,6 +1352,187 @@ test: retry: 2 ``` +## `include` + +> Introduced in [GitLab Edition Premium][ee] 10.5. +> Available for Starter, Premium and Ultimate [versions][gitlab-versions] since 10.6. +> Behaviour expanded in GitLab 10.8 to allow more flexible overriding +> Available for Libre since 11.4 + +Using the `include` keyword, you can allow the inclusion of external YAML files. + +In the following example, the content of `.before-script-template.yml` will be +automatically fetched and evaluated along with the content of `.gitlab-ci.yml`: + +```yaml +# Content of https://gitlab.com/awesome-project/raw/master/.before-script-template.yml + +before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" +``` + +```yaml +# Content of .gitlab-ci.yml + +include: 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml' + +rspec: + script: + - bundle exec rspec +``` + +You can define it either as a single string, or, in case you want to include +more than one files, an array of different values . The following examples +are both valid cases: + +```yaml +# Single string + +include: '/templates/.after-script-template.yml' +``` + +```yaml +# Array + +include: + - 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml' + - '/templates/.after-script-template.yml' +``` + +--- + +`include` supports two types of files: + +- **local** to the same repository, referenced by using full paths in the same + repository, with `/` being the root directory. For example: + + ```yaml + # Within the repository + include: '/templates/.gitlab-ci-template.yml' + ``` + + NOTE: **Note:** + You can only use files that are currently tracked by Git on the same branch + your configuration file is. In other words, when using a **local file**, make + sure that both `.gitlab-ci.yml` and the local file are on the same branch. + + NOTE: **Note:** + We don't support the inclusion of local files through Git submodules paths. + +- **remote** in a different location, accessed using HTTP/HTTPS, referenced + using the full URL. For example: + + ```yaml + include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-template.yml' + ``` + + NOTE: **Note:** + The remote file must be publicly accessible through a simple GET request, as we don't support authentication schemas in the remote URL. + +--- + + +Since GitLab 10.8 we are now recursively merging the files defined in `include` +with those in `.gitlab-ci.yml`. Files defined by `include` are always +evaluated first and recursively merged with the content of `.gitlab-ci.yml`, no +matter the position of the `include` keyword. You can take advantage of +recursive merging to customize and override details in included CI +configurations with local definitions. + +The following example shows specific YAML-defined variables and details of the +`production` job from an include file being customized in `.gitlab-ci.yml`. + +```yaml +# Content of https://company.com/autodevops-template.yml + +variables: + POSTGRES_USER: user + POSTGRES_PASSWORD: testing_password + POSTGRES_DB: $CI_ENVIRONMENT_SLUG + +production: + stage: production + script: + - install_dependencies + - deploy + environment: + name: production + url: https://$CI_PROJECT_PATH_SLUG.$AUTO_DEVOPS_DOMAIN + only: + - master +``` + +```yaml +# Content of .gitlab-ci.yml + +include: 'https://company.com/autodevops-template.yml' + +image: alpine:latest + +variables: + POSTGRES_USER: root + POSTGRES_PASSWORD: secure_password + +stages: + - build + - test + - production + +production: + environment: + url: https://domain.com +``` + +In this case, the variables `POSTGRES_USER` and `POSTGRES_PASSWORD` along +with the environment url of the `production` job defined in +`autodevops-template.yml` have been overridden by new values defined in +`.gitlab-ci.yml`. + +NOTE: **Note:** +Recursive includes are not supported meaning your external files +should not use the `include` keyword, as it will be ignored. + +Recursive merging lets you extend and override dictionary mappings, but +you cannot add or modify items to an included array. For example, to add +an additional item to the production job script, you must repeat the +existing script items. + +```yaml +# Content of https://company.com/autodevops-template.yml + +production: + stage: production + script: + - install_dependencies + - deploy +``` + +```yaml +# Content of .gitlab-ci.yml + +include: 'https://company.com/autodevops-template.yml' + +stages: + - production + +production: + script: + - install_depedencies + - deploy + - notify_owner +``` + +In this case, if `install_dependencies` and `deploy` were not repeated in +`.gitlab-ci.yml`, they would not be part of the script for the `production` +job in the combined CI configuration. + +NOTE: **Note:** +We currently do not support using YAML aliases across different YAML files +sourced by `include`. You must only refer to aliases in the same file. Instead +of using YAML anchors you can use [`extends` keyword](#extends). + ## `variables` > Introduced in GitLab Runner v0.5.0. From df16969c24c608682b8b46883a8e6e20147a3791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Sep 2018 14:55:36 +0200 Subject: [PATCH 16/23] Reconcile differences in lib/gitlab/ci --- lib/gitlab/ci/config.rb | 24 ++- lib/gitlab/ci/yaml_processor.rb | 2 +- spec/lib/gitlab/ci/config_spec.rb | 267 +++++++++++++++++++++++------- 3 files changed, 220 insertions(+), 73 deletions(-) diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 19d93c6e785..0ffd1791791 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -7,14 +7,18 @@ module Gitlab ConfigError = Class.new(StandardError) def initialize(config, opts = {}) - @config = Config::Extendable - .new(build_config(config, opts)) - .to_hash + begin + @config = Config::Extendable + .new(build_config(config, opts)) + .to_hash - @global = Entry::Global.new(@config) - @global.compose! - rescue Loader::FormatError, Extendable::ExtensionError => e - raise Config::ConfigError, e.message + @global = Entry::Global.new(@config) + @global.compose! + rescue Loader::FormatError, Extendable::ExtensionError => e + raise Config::ConfigError, e.message + end + rescue ::Gitlab::Ci::External::Processor::FileError => e + raise ::Gitlab::Ci::YamlProcessor::ValidationError, e.message end def valid? @@ -64,6 +68,8 @@ module Gitlab @global.jobs_value end + private + def build_config(config, opts = {}) initial_config = Loader.new(config).load! project = opts.fetch(:project, nil) @@ -76,8 +82,8 @@ module Gitlab end def process_external_files(config, project, opts) - sha = opts.fetch(:sha, project.repository.commit.sha) - ::Gitlab::Ci::External::Processor.new(config, project, sha).perform + sha = opts.fetch(:sha) { project.repository.root_ref_sha } + ::Gitlab::Ci::External::Processor.new(config, project, sha).perform end end end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 67f5d2d6ae4..5d1864ae9e2 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -79,7 +79,7 @@ module Gitlab begin Gitlab::Ci::YamlProcessor.new(content, opts) nil - rescue ValidationError, ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError => e + rescue ValidationError => e e.message end end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 83a30de199f..baa4793e677 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -3,13 +3,12 @@ require 'fast_spec_helper' require_dependency 'active_model' describe Gitlab::Ci::Config do - let(:project) { create(:project, :repository) } let(:config) do - described_class.new(gitlab_ci_yml, { project: project, sha: '12345' }) + described_class.new(yml) end context 'when config is valid' do - let(:gitlab_ci_yml) do + let(:yml) do <<-EOS image: ruby:2.2 @@ -126,7 +125,11 @@ describe Gitlab::Ci::Config do end end - context "when gitlab_ci_yml has valid 'include' defined" do + context "when using 'include' directive" do + let(:project) { create(:project, :repository) } + let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:local_location) { 'ee/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml' } + let(:remote_file_content) do <<~HEREDOC variables: @@ -137,83 +140,221 @@ describe Gitlab::Ci::Config do POSTGRES_DB: $CI_ENVIRONMENT_SLUG HEREDOC end - let(:local_file_content) { File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") } - let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:gitlab_ci_yml) do - <<-EOS - include: - - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml - - #{remote_location} - image: ruby:2.2 - EOS + let(:local_file_content) do + File.read(Rails.root.join(local_location)) + end + + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + - #{local_location} + - #{remote_location} + + image: ruby:2.2 + HEREDOC + end + + let(:config) do + described_class.new(gitlab_ci_yml, project: project, sha: '12345') end before do - allow_any_instance_of(::Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content) - WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content) + WebMock.stub_request(:get, remote_location) + .to_return(body: remote_file_content) + + allow(project.repository) + .to receive(:blob_data_at).and_return(local_file_content) end - it 'should return a composed hash' do - before_script_values = [ - "apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs", "ruby -v", - "which ruby", - "gem install bundler --no-ri --no-rdoc", - "bundle install --jobs $(nproc) \"${FLAGS[@]}\"" - ] - variables = { - AUTO_DEVOPS_DOMAIN: "domain.example.com", - POSTGRES_USER: "user", - POSTGRES_PASSWORD: "testing-password", - POSTGRES_ENABLED: "true", - POSTGRES_DB: "$CI_ENVIRONMENT_SLUG" - } - composed_hash = { - before_script: before_script_values, - image: "ruby:2.2", - rspec: { script: ["bundle exec rspec"] }, - variables: variables - } + context "when gitlab_ci_yml has valid 'include' defined" do + it 'should return a composed hash' do + before_script_values = [ + "apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs", "ruby -v", + "which ruby", + "gem install bundler --no-ri --no-rdoc", + "bundle install --jobs $(nproc) \"${FLAGS[@]}\"" + ] + variables = { + AUTO_DEVOPS_DOMAIN: "domain.example.com", + POSTGRES_USER: "user", + POSTGRES_PASSWORD: "testing-password", + POSTGRES_ENABLED: "true", + POSTGRES_DB: "$CI_ENVIRONMENT_SLUG" + } + composed_hash = { + before_script: before_script_values, + image: "ruby:2.2", + rspec: { script: ["bundle exec rspec"] }, + variables: variables + } - expect(config.to_hash).to eq(composed_hash) - end - end - - context "when gitlab_ci.yml has invalid 'include' defined" do - let(:gitlab_ci_yml) do - <<-EOS - include: invalid - EOS + expect(config.to_hash).to eq(composed_hash) + end end - it 'raises error YamlProcessor ValidationError' do - expect { config }.to raise_error( - ::Gitlab::Ci::YamlProcessor::ValidationError, - "External file: 'invalid' should be a valid local or remote file" - ) - end - end + context "when gitlab_ci.yml has invalid 'include' defined" do + let(:gitlab_ci_yml) do + <<~HEREDOC + include: invalid + HEREDOC + end - context "when both external files and gitlab_ci.yml defined the same key" do - let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:gitlab_ci_yml) do - <<~HEREDOC + it 'raises error YamlProcessor validationError' do + expect { config }.to raise_error( + ::Gitlab::Ci::YamlProcessor::ValidationError, + "Local file 'invalid' is not valid." + ) + end + end + + describe 'external file version' do + context 'when external local file SHA is defined' do + it 'is using a defined value' do + expect(project.repository).to receive(:blob_data_at) + .with('eeff1122', local_location) + + described_class.new(gitlab_ci_yml, project: project, sha: 'eeff1122') + end + end + + context 'when external local file SHA is not defined' do + it 'is using latest SHA on the default branch' do + expect(project.repository).to receive(:root_ref_sha) + + described_class.new(gitlab_ci_yml, project: project) + end + end + end + + context "when both external files and gitlab_ci.yml defined the same key" do + let(:gitlab_ci_yml) do + <<~HEREDOC include: - #{remote_location} image: ruby:2.2 - HEREDOC - end + HEREDOC + end - let(:remote_file_content) do - <<~HEREDOC + let(:remote_file_content) do + <<~HEREDOC image: php:5-fpm-alpine - HEREDOC + HEREDOC + end + + it 'should take precedence' do + expect(config.to_hash).to eq({ image: 'ruby:2.2' }) + end end - it 'should take precedence' do - WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content) - expect(config.to_hash).to eq({ image: 'ruby:2.2' }) + context "when both external files and gitlab_ci.yml define a dictionary of distinct variables" do + let(:remote_file_content) do + <<~HEREDOC + variables: + A: 'alpha' + B: 'beta' + HEREDOC + end + + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + - #{remote_location} + + variables: + C: 'gamma' + D: 'delta' + HEREDOC + end + + it 'should merge the variables dictionaries' do + expect(config.to_hash).to eq({ variables: { A: 'alpha', B: 'beta', C: 'gamma', D: 'delta' } }) + end + end + + context "when both external files and gitlab_ci.yml define a dictionary of overlapping variables" do + let(:remote_file_content) do + <<~HEREDOC + variables: + A: 'alpha' + B: 'beta' + C: 'omnicron' + HEREDOC + end + + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + - #{remote_location} + + variables: + C: 'gamma' + D: 'delta' + HEREDOC + end + + it 'later declarations should take precedence' do + expect(config.to_hash).to eq({ variables: { A: 'alpha', B: 'beta', C: 'gamma', D: 'delta' } }) + end + end + + context 'when both external files and gitlab_ci.yml define a job' do + let(:remote_file_content) do + <<~HEREDOC + job1: + script: + - echo 'hello from remote file' + HEREDOC + end + + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + - #{remote_location} + + job1: + variables: + VARIABLE_DEFINED_IN_MAIN_FILE: 'some value' + HEREDOC + end + + it 'merges the jobs' do + expect(config.to_hash).to eq({ + job1: { + script: ["echo 'hello from remote file'"], + variables: { + VARIABLE_DEFINED_IN_MAIN_FILE: 'some value' + } + } + }) + end + + context 'when the script key is in both' do + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + - #{remote_location} + + job1: + script: + - echo 'hello from main file' + variables: + VARIABLE_DEFINED_IN_MAIN_FILE: 'some value' + HEREDOC + end + + it 'uses the script from the gitlab_ci.yml' do + expect(config.to_hash).to eq({ + job1: { + script: ["echo 'hello from main file'"], + variables: { + VARIABLE_DEFINED_IN_MAIN_FILE: 'some value' + } + } + }) + end + end end end end From b293034998501de64dbdbbe500f9c34c6e8e2f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Sep 2018 18:00:32 +0200 Subject: [PATCH 17/23] Reconcile differences in spec/models/blob_viewer --- spec/models/blob_viewer/gitlab_ci_yml_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb index 48345b192d1..01c555a7a90 100644 --- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb +++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb @@ -4,7 +4,7 @@ describe BlobViewer::GitlabCiYml do include FakeBlobHelpers include RepoHelpers - let(:project) { build_stubbed(:project, :repository) } + let(:project) { create(:project, :repository) } let(:data) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } let(:blob) { fake_blob(path: '.gitlab-ci.yml', data: data) } let(:sha) { sample_commit.id } From a5604651aa9684b2f24ccd1337dcb4383c7ecae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Sep 2018 18:13:08 +0200 Subject: [PATCH 18/23] Add CHANGELOG --- ...external-files-in-gitlab-ci-yml-from-starter-to-libre.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/42861-move-include-external-files-in-gitlab-ci-yml-from-starter-to-libre.yml diff --git a/changelogs/unreleased/42861-move-include-external-files-in-gitlab-ci-yml-from-starter-to-libre.yml b/changelogs/unreleased/42861-move-include-external-files-in-gitlab-ci-yml-from-starter-to-libre.yml new file mode 100644 index 00000000000..171779817c8 --- /dev/null +++ b/changelogs/unreleased/42861-move-include-external-files-in-gitlab-ci-yml-from-starter-to-libre.yml @@ -0,0 +1,5 @@ +--- +title: Move including external files in .gitlab-ci.yml from Starter to Libre +merge_request: 21603 +author: +type: changed From 5b2c873cfdf8d1add7e147c5a72f8a1d0f99f775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Sep 2018 21:43:42 +0200 Subject: [PATCH 19/23] Fix references to ee .gitlab-ci.yml spec fixtures --- spec/lib/gitlab/ci/config_spec.rb | 2 +- spec/lib/gitlab/ci/external/processor_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index baa4793e677..b43aca8a354 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -128,7 +128,7 @@ describe Gitlab::Ci::Config do context "when using 'include' directive" do let(:project) { create(:project, :repository) } let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:local_location) { 'ee/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml' } + let(:local_location) { 'spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml' } let(:remote_file_content) do <<~HEREDOC diff --git a/spec/lib/gitlab/ci/external/processor_spec.rb b/spec/lib/gitlab/ci/external/processor_spec.rb index fc9a0748075..bf355ddb21d 100644 --- a/spec/lib/gitlab/ci/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/external/processor_spec.rb @@ -107,7 +107,7 @@ describe Gitlab::Ci::External::Processor do let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:external_files) do [ - '/ee/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml', + '/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml', remote_file ] end @@ -128,7 +128,7 @@ describe Gitlab::Ci::External::Processor do end before do - local_file_content = File.read(Rails.root.join('ee/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml')) + local_file_content = File.read(Rails.root.join('spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml')) allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) end From 2fafcf668ed25e2e794088c7f089c048372f3f10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 11 Sep 2018 16:22:37 +0200 Subject: [PATCH 20/23] Add link to CE move MR --- doc/ci/yaml/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index d50093e8563..0f9aae953f7 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1356,8 +1356,8 @@ test: > Introduced in [GitLab Edition Premium][ee] 10.5. > Available for Starter, Premium and Ultimate [versions][gitlab-versions] since 10.6. -> Behaviour expanded in GitLab 10.8 to allow more flexible overriding -> Available for Libre since 11.4 +> Behaviour expanded in GitLab 10.8 to allow more flexible overriding. +> Available for Libre since [11.4](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21603) Using the `include` keyword, you can allow the inclusion of external YAML files. From b470f975e18ce792e78026b7aaa2c4a4fa45e237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 11 Sep 2018 16:26:57 +0200 Subject: [PATCH 21/23] Add missing frozen string literal comments --- lib/gitlab/ci/external/file/base.rb | 2 ++ lib/gitlab/ci/external/file/local.rb | 2 ++ lib/gitlab/ci/external/file/remote.rb | 2 ++ lib/gitlab/ci/external/mapper.rb | 2 ++ lib/gitlab/ci/external/processor.rb | 2 ++ spec/lib/gitlab/ci/external/file/local_spec.rb | 2 ++ spec/lib/gitlab/ci/external/file/remote_spec.rb | 2 ++ spec/lib/gitlab/ci/external/mapper_spec.rb | 2 ++ spec/lib/gitlab/ci/external/processor_spec.rb | 2 ++ 9 files changed, 18 insertions(+) diff --git a/lib/gitlab/ci/external/file/base.rb b/lib/gitlab/ci/external/file/base.rb index 4591b3ec82e..f4da07b0b02 100644 --- a/lib/gitlab/ci/external/file/base.rb +++ b/lib/gitlab/ci/external/file/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab module Ci module External diff --git a/lib/gitlab/ci/external/file/local.rb b/lib/gitlab/ci/external/file/local.rb index 829cc9b3d19..1aa7f687507 100644 --- a/lib/gitlab/ci/external/file/local.rb +++ b/lib/gitlab/ci/external/file/local.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab module Ci module External diff --git a/lib/gitlab/ci/external/file/remote.rb b/lib/gitlab/ci/external/file/remote.rb index b4294231265..59bb3e8999e 100644 --- a/lib/gitlab/ci/external/file/remote.rb +++ b/lib/gitlab/ci/external/file/remote.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab module Ci module External diff --git a/lib/gitlab/ci/external/mapper.rb b/lib/gitlab/ci/external/mapper.rb index 3c359efa803..58bd6a19acf 100644 --- a/lib/gitlab/ci/external/mapper.rb +++ b/lib/gitlab/ci/external/mapper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab module Ci module External diff --git a/lib/gitlab/ci/external/processor.rb b/lib/gitlab/ci/external/processor.rb index 22588867a08..76cf3ce89f9 100644 --- a/lib/gitlab/ci/external/processor.rb +++ b/lib/gitlab/ci/external/processor.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab module Ci module External diff --git a/spec/lib/gitlab/ci/external/file/local_spec.rb b/spec/lib/gitlab/ci/external/file/local_spec.rb index a4099703ac9..3f32d81a827 100644 --- a/spec/lib/gitlab/ci/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/external/file/local_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Ci::External::File::Local do diff --git a/spec/lib/gitlab/ci/external/file/remote_spec.rb b/spec/lib/gitlab/ci/external/file/remote_spec.rb index 314dcfec511..b1819c8960b 100644 --- a/spec/lib/gitlab/ci/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/external/file/remote_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Ci::External::File::Remote do diff --git a/spec/lib/gitlab/ci/external/mapper_spec.rb b/spec/lib/gitlab/ci/external/mapper_spec.rb index 89285c8ba45..6270d27a36d 100644 --- a/spec/lib/gitlab/ci/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external/mapper_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Ci::External::Mapper do diff --git a/spec/lib/gitlab/ci/external/processor_spec.rb b/spec/lib/gitlab/ci/external/processor_spec.rb index bf355ddb21d..688c2b3c8aa 100644 --- a/spec/lib/gitlab/ci/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/external/processor_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Ci::External::Processor do From 7600a6469bbd2975c80b48e6bb9b95af1e40795f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 11 Sep 2018 16:29:48 +0200 Subject: [PATCH 22/23] Stack rescue blocks for Gitlab::Ci::Config#initialize --- lib/gitlab/ci/config.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 0ffd1791791..fe98d25af29 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -7,16 +7,14 @@ module Gitlab ConfigError = Class.new(StandardError) def initialize(config, opts = {}) - begin - @config = Config::Extendable - .new(build_config(config, opts)) - .to_hash + @config = Config::Extendable + .new(build_config(config, opts)) + .to_hash - @global = Entry::Global.new(@config) - @global.compose! - rescue Loader::FormatError, Extendable::ExtensionError => e - raise Config::ConfigError, e.message - end + @global = Entry::Global.new(@config) + @global.compose! + rescue Loader::FormatError, Extendable::ExtensionError => e + raise Config::ConfigError, e.message rescue ::Gitlab::Ci::External::Processor::FileError => e raise ::Gitlab::Ci::YamlProcessor::ValidationError, e.message end From e358ae16b45602666e959ada5a193c1cb4addb0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 12 Sep 2018 18:44:35 +0200 Subject: [PATCH 23/23] Inline initialize_yaml_processor --- app/models/ci/pipeline.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index bec0aff2ba0..8c075253400 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -464,7 +464,7 @@ module Ci return @config_processor if defined?(@config_processor) @config_processor ||= begin - initialize_yaml_processor + ::Gitlab::Ci::YamlProcessor.new(ci_yaml_file, { project: project, sha: sha }) rescue Gitlab::Ci::YamlProcessor::ValidationError => e self.yaml_errors = e.message nil @@ -474,10 +474,6 @@ module Ci end end - def initialize_yaml_processor - ::Gitlab::Ci::YamlProcessor.new(ci_yaml_file, { project: project, sha: sha }) - end - def ci_yaml_file_path if project.ci_config_path.blank? '.gitlab-ci.yml'