From a8c50960264d3242a417e5261781ee3649a4e4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Wed, 2 Jan 2019 12:51:15 +0100 Subject: [PATCH] Allow to include templates This rewrites a syntax to allow include of templates. This also normalises the syntax used by include: feature --- changelogs/unreleased/include-templates.yml | 5 + doc/ci/yaml/README.md | 56 ++++++- lib/gitlab/ci/config.rb | 2 +- lib/gitlab/ci/config/external/file/base.rb | 16 +- lib/gitlab/ci/config/external/file/local.rb | 9 +- lib/gitlab/ci/config/external/file/remote.rb | 6 + .../ci/config/external/file/template.rb | 51 +++++++ lib/gitlab/ci/config/external/mapper.rb | 54 ++++++- lib/gitlab/ci/config/external/processor.rb | 6 +- .../ci/config/external/file/base_spec.rb | 36 ++++- .../ci/config/external/file/local_spec.rb | 33 ++++- .../ci/config/external/file/remote_spec.rb | 30 +++- .../ci/config/external/file/template_spec.rb | 93 ++++++++++++ .../gitlab/ci/config/external/mapper_spec.rb | 138 ++++++++++++------ .../ci/config/external/processor_spec.rb | 5 +- spec/lib/gitlab/ci/config_spec.rb | 17 +++ 16 files changed, 480 insertions(+), 77 deletions(-) create mode 100644 changelogs/unreleased/include-templates.yml create mode 100644 lib/gitlab/ci/config/external/file/template.rb create mode 100644 spec/lib/gitlab/ci/config/external/file/template_spec.rb diff --git a/changelogs/unreleased/include-templates.yml b/changelogs/unreleased/include-templates.yml new file mode 100644 index 00000000000..5601cd185e9 --- /dev/null +++ b/changelogs/unreleased/include-templates.yml @@ -0,0 +1,5 @@ +--- +title: Allow to include templates in gitlab-ci.yml +merge_request: 23495 +author: +type: added diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index c74f5e5b3f9..a0f0009305b 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1649,6 +1649,7 @@ test: > Behaviour expanded in GitLab 10.8 to allow more flexible overriding. > [Moved](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21603) to GitLab Core in 11.4 +> In GitLab 11.7, support for including [GitLab-supplied templates](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/lib/gitlab/ci/templates) directly [was added](https://gitlab.com/gitlab-org/gitlab-ce/issues/53445). Using the `include` keyword, you can allow the inclusion of external YAML files. @@ -1688,6 +1689,13 @@ are both valid cases: include: '/templates/.after-script-template.yml' ``` +```yaml +# Single string + +include: + file: '/templates/.after-script-template.yml' +``` + ```yaml # Array @@ -1696,9 +1704,27 @@ include: - '/templates/.after-script-template.yml' ``` +```yaml +# Array mixed syntax + +include: + - 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml' + - '/templates/.after-script-template.yml' + - template: Auto-DevOps.gitlab-ci.yml +``` + +```yaml +# Array + +include: + - remote: 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml' + - local: '/templates/.after-script-template.yml' + - template: Auto-DevOps.gitlab-ci.yml +``` + --- -`include` supports two types of files: +`include` supports three types of files: - **local** to the same repository, referenced by using full paths in the same repository, with `/` being the root directory. For example: @@ -1708,6 +1734,14 @@ include: include: '/templates/.gitlab-ci-template.yml' ``` + Or using: + + ```yaml + # Within the repository + include: + local: '/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 @@ -1720,9 +1754,18 @@ include: using the full URL. For example: ```yaml + # File sourced from outside repository include: 'https://gitlab.com/awesome-project/raw/master/.gitlab-ci-template.yml' ``` + Or using: + + ```yaml + # File sourced from outside repository + include: + remote: '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. @@ -1731,6 +1774,17 @@ include: you may need to enable the **Allow requests to the local network from hooks and services** checkbox located in the **Settings > Network > Outbound requests** section within the **Admin area**. +- **template** included with GitLab. For example: + + ```yaml + # File sourced from GitLab's template collection + include: + template: Auto-DevOps.gitlab-ci.yml + ``` + + NOTE: **Note:** + Templates included this way are sourced from [lib/gitlab/ci/templates](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/lib/gitlab/ci/templates). + --- diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 6333799a491..11e0352975d 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -83,7 +83,7 @@ module Gitlab def process_external_files(config, project, opts) sha = opts.fetch(:sha) { project.repository.root_ref_sha } - Config::External::Processor.new(config, project, sha).perform + Config::External::Processor.new(config, project: project, sha: sha).perform end end end diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index ee4ea9bbb1d..2ac6656a703 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -8,20 +8,26 @@ module Gitlab class Base include Gitlab::Utils::StrongMemoize - attr_reader :location, :opts, :errors + attr_reader :location, :params, :context, :errors YAML_WHITELIST_EXTENSION = /.+\.(yml|yaml)$/i.freeze - def initialize(location, opts = {}) - @location = location - @opts = opts + Context = Struct.new(:project, :sha) + + def initialize(params, context) + @params = params + @context = context @errors = [] validate! end + def matching? + location.present? + end + def invalid_extension? - !::File.basename(location).match(YAML_WHITELIST_EXTENSION) + location.nil? || !::File.basename(location).match?(YAML_WHITELIST_EXTENSION) end def valid? diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index 2a256aff65c..2535d178ba8 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -8,11 +8,8 @@ module Gitlab class Local < Base include Gitlab::Utils::StrongMemoize - attr_reader :project, :sha - - def initialize(location, opts = {}) - @project = opts.fetch(:project) - @sha = opts.fetch(:sha) + def initialize(params, context) + @location = params[:local] super end @@ -32,7 +29,7 @@ module Gitlab end def fetch_local_content - project.repository.blob_data_at(sha, location) + context.project.repository.blob_data_at(context.sha, location) end end end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb index 86fa5ad8800..567a86c47e5 100644 --- a/lib/gitlab/ci/config/external/file/remote.rb +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -8,6 +8,12 @@ module Gitlab class Remote < Base include Gitlab::Utils::StrongMemoize + def initialize(params, context) + @location = params[:remote] + + super + end + def content strong_memoize(:content) { fetch_remote_content } end diff --git a/lib/gitlab/ci/config/external/file/template.rb b/lib/gitlab/ci/config/external/file/template.rb new file mode 100644 index 00000000000..54f4cf74c4d --- /dev/null +++ b/lib/gitlab/ci/config/external/file/template.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module External + module File + class Template < Base + attr_reader :location, :project + + SUFFIX = '.gitlab-ci.yml'.freeze + + def initialize(params, context) + @location = params[:template] + + super + end + + def content + strong_memoize(:content) { fetch_template_content } + end + + private + + def validate_location! + super + + unless template_name_valid? + errors.push("Template file `#{location}` is not a valid location!") + end + end + + def template_name + return unless template_name_valid? + + location.first(-SUFFIX.length) + end + + def template_name_valid? + location.to_s.end_with?(SUFFIX) + end + + def fetch_template_content + Gitlab::Template::GitlabCiYmlTemplate.find(template_name, project)&.content + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index def3563e505..74bd927da39 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -5,25 +5,63 @@ module Gitlab class Config module External class Mapper - def initialize(values, project, sha) - @locations = Array(values.fetch(:include, [])) + include Gitlab::Utils::StrongMemoize + + FILE_CLASSES = [ + External::File::Remote, + External::File::Template, + External::File::Local + ].freeze + + AmbigiousSpecificationError = Class.new(StandardError) + + def initialize(values, project:, sha:) + @locations = Array.wrap(values.fetch(:include, [])) @project = project @sha = sha end def process - locations.map { |location| build_external_file(location) } + locations + .compact + .map(&method(:normalize_location)) + .map(&method(:select_first_matching)) end private - attr_reader :locations, :project, :sha + attr_reader :locations, :project, :sha, :user - def build_external_file(location) - if ::Gitlab::UrlSanitizer.valid?(location) - External::File::Remote.new(location) + # convert location if String to canonical form + def normalize_location(location) + if location.is_a?(String) + normalize_location_string(location) else - External::File::Local.new(location, project: project, sha: sha) + location.deep_symbolize_keys + end + end + + def normalize_location_string(location) + if ::Gitlab::UrlSanitizer.valid?(location) + { remote: location } + else + { local: location } + end + end + + def select_first_matching(location) + matching = FILE_CLASSES.map do |file_class| + file_class.new(location, context) + end.select(&:matching?) + + raise AmbigiousSpecificationError, "Include `#{location.to_json}` needs to match exactly one accessor!" unless matching.one? + + matching.first + end + + def context + strong_memoize(:context) do + External::File::Base::Context.new(project, sha) end end end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb index eae0bdeb644..1d310b29dc8 100644 --- a/lib/gitlab/ci/config/external/processor.rb +++ b/lib/gitlab/ci/config/external/processor.rb @@ -7,10 +7,12 @@ module Gitlab class Processor IncludeError = Class.new(StandardError) - def initialize(values, project, sha) + def initialize(values, project:, sha:) @values = values - @external_files = External::Mapper.new(values, project, sha).process + @external_files = External::Mapper.new(values, project: project, sha: sha).process @content = {} + rescue External::Mapper::AmbigiousSpecificationError => e + raise IncludeError, e.message end def perform diff --git a/spec/lib/gitlab/ci/config/external/file/base_spec.rb b/spec/lib/gitlab/ci/config/external/file/base_spec.rb index 2e92d5204d6..ada8775c489 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -3,13 +3,43 @@ require 'fast_spec_helper' describe Gitlab::Ci::Config::External::File::Base do - subject { described_class.new(location) } + let(:context) { described_class::Context.new(nil, 'HEAD') } + + let(:test_class) do + Class.new(described_class) do + def initialize(params, context = {}) + @location = params + + super + end + end + end + + subject { test_class.new(location, context) } before do - allow_any_instance_of(described_class) + allow_any_instance_of(test_class) .to receive(:content).and_return('key: value') end + describe '#matching?' do + context 'when a location is present' do + let(:location) { 'some-location' } + + it 'should return true' do + expect(subject).to be_matching + end + end + + context 'with a location is missing' do + let(:location) { nil } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + end + describe '#valid?' do context 'when location is not a YAML file' do let(:location) { 'some/file.txt' } @@ -39,7 +69,7 @@ describe Gitlab::Ci::Config::External::File::Base do let(:location) { 'some/file/config.yml' } before do - allow_any_instance_of(described_class) + allow_any_instance_of(test_class) .to receive(:content).and_return('invalid_syntax') end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 645db642e29..83be43e240b 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -3,8 +3,37 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Local do - let(:project) { create(:project, :repository) } - let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } + set(:project) { create(:project, :repository) } + + let(:context) { described_class::Context.new(project, '12345') } + let(:params) { { local: location } } + let(:local_file) { described_class.new(params, context) } + + describe '#matching?' do + context 'when a local is specified' do + let(:params) { { local: 'file' } } + + it 'should return true' do + expect(local_file).to be_matching + end + end + + context 'with a missing local' do + let(:params) { { local: nil } } + + it 'should return false' do + expect(local_file).not_to be_matching + end + end + + context 'with a missing local key' do + let(:params) { {} } + + it 'should return false' do + expect(local_file).not_to be_matching + end + end + end describe '#valid?' do context 'when is a valid local path' do diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index eaf621e4140..319e7137f9f 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Remote do - let(:remote_file) { described_class.new(location) } + let(:context) { described_class::Context.new(nil, '12345') } + let(:params) { { remote: location } } + let(:remote_file) { described_class.new(params, context) } let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:remote_file_content) do <<~HEREDOC @@ -15,6 +17,32 @@ describe Gitlab::Ci::Config::External::File::Remote do HEREDOC end + describe '#matching?' do + context 'when a remote is specified' do + let(:params) { { remote: 'http://remote' } } + + it 'should return true' do + expect(remote_file).to be_matching + end + end + + context 'with a missing remote' do + let(:params) { { remote: nil } } + + it 'should return false' do + expect(remote_file).not_to be_matching + end + end + + context 'with a missing remote key' do + let(:params) { {} } + + it 'should return false' do + expect(remote_file).not_to be_matching + end + end + end + describe "#valid?" do context 'when is a valid remote url' do before do diff --git a/spec/lib/gitlab/ci/config/external/file/template_spec.rb b/spec/lib/gitlab/ci/config/external/file/template_spec.rb new file mode 100644 index 00000000000..1fb5655309a --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/file/template_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Config::External::File::Template do + let(:context) { described_class::Context.new(nil, '12345') } + let(:template) { 'Auto-DevOps.gitlab-ci.yml' } + let(:params) { { template: template } } + + subject { described_class.new(params, context) } + + describe '#matching?' do + context 'when a template is specified' do + let(:params) { { template: 'some-template' } } + + it 'should return true' do + expect(subject).to be_matching + end + end + + context 'with a missing template' do + let(:params) { { template: nil } } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + + context 'with a missing template key' do + let(:params) { {} } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + end + + describe "#valid?" do + context 'when is a valid template name' do + let(:template) { 'Auto-DevOps.gitlab-ci.yml' } + + it 'should return true' do + expect(subject).to be_valid + end + end + + context 'with invalid template name' do + let(:template) { 'Template.yml' } + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include('Template file `Template.yml` is not a valid location!') + end + end + + context 'with a non-existing template' do + let(:template) { 'I-Do-Not-Have-This-Template.gitlab-ci.yml' } + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include('Included file `I-Do-Not-Have-This-Template.gitlab-ci.yml` is empty or does not exist!') + end + end + end + + describe '#template_name' do + let(:template_name) { subject.send(:template_name) } + + context 'when template does end with .gitlab-ci.yml' do + let(:template) { 'my-template.gitlab-ci.yml' } + + it 'returns template name' do + expect(template_name).to eq('my-template') + end + end + + context 'when template is nil' do + let(:template) { nil } + + it 'returns nil' do + expect(template_name).to be_nil + end + end + + context 'when template does not end with .gitlab-ci.yml' do + let(:template) { 'my-template' } + + it 'returns nil' do + expect(template_name).to be_nil + end + end + end +end diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index 5b236fe99f1..e27d2cd9422 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -3,78 +3,101 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Mapper do - let(:project) { create(:project, :repository) } + set(:project) { create(:project, :repository) } + + let(:local_file) { '/lib/gitlab/ci/templates/non-existent-file.yml' } + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:template_file) { 'Auto-DevOps.gitlab-ci.yml' } + let(:file_content) do <<~HEREDOC image: 'ruby:2.2' HEREDOC end - describe '#process' do - subject { described_class.new(values, project, '123456').process } + before do + WebMock.stub_request(:get, remote_url).to_return(body: file_content) + end - context "when 'include' keyword is defined as string" do + describe '#process' do + subject { described_class.new(values, project: project, sha: '123456').process } + + context "when single 'include' keyword is defined" do context 'when the string is a local file' do let(:values) do - { - include: '/lib/gitlab/ci/templates/non-existent-file.yml', - image: 'ruby:2.2' - } - end - - it 'returns an array' do - expect(subject).to be_an(Array) + { include: local_file, + image: 'ruby:2.2' } end it 'returns File instances' do - expect(subject.first) - .to be_an_instance_of(Gitlab::Ci::Config::External::File::Local) + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Local)) + end + end + + context 'when the key is a local file hash' do + let(:values) do + { include: { 'local' => local_file }, + image: 'ruby:2.2' } + end + + it 'returns File instances' do + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Local)) end 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: remote_url, - image: 'ruby:2.2' - } - end - - before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) - end - - it 'returns an array' do - expect(subject).to be_an(Array) + { include: remote_url, image: 'ruby:2.2' } end it 'returns File instances' do - expect(subject.first) - .to be_an_instance_of(Gitlab::Ci::Config::External::File::Remote) + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Remote)) + end + end + + context 'when the key is a remote file hash' do + let(:values) do + { include: { 'remote' => remote_url }, + image: 'ruby:2.2' } + end + + it 'returns File instances' do + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Remote)) + end + end + + context 'when the key is a template file hash' do + let(:values) do + { include: { 'template' => template_file }, + image: 'ruby:2.2' } + end + + it 'returns File instances' do + expect(subject).to contain_exactly( + an_instance_of(Gitlab::Ci::Config::External::File::Template)) + end + end + + context 'when the key is a hash of file and remote' do + let(:values) do + { include: { 'local' => local_file, 'remote' => remote_url }, + image: 'ruby:2.2' } + end + + it 'returns ambigious specification error' do + expect { subject }.to raise_error(described_class::AmbigiousSpecificationError) end end 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: - [ - remote_url, - '/lib/gitlab/ci/templates/template.yml' - ], - image: 'ruby:2.2' - } - end - - before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) - end - - it 'returns an array' do - expect(subject).to be_an(Array) + { include: [remote_url, local_file], + image: 'ruby:2.2' } end it 'returns Files instances' do @@ -83,6 +106,29 @@ describe Gitlab::Ci::Config::External::Mapper do end end + context "when 'include' is defined as an array of hashes" do + let(:values) do + { include: [{ remote: remote_url }, { local: local_file }], + image: 'ruby:2.2' } + end + + it 'returns Files instances' do + expect(subject).to all(respond_to(:valid?)) + expect(subject).to all(respond_to(:content)) + end + + context 'when it has ambigious match' do + let(:values) do + { include: [{ remote: remote_url, local: local_file }], + image: 'ruby:2.2' } + end + + it 'returns ambigious specification error' do + expect { subject }.to raise_error(described_class::AmbigiousSpecificationError) + end + end + end + context "when 'include' is not defined" do let(:values) do { diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index dbd28e9745c..d2d4fbc5115 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Processor do - let(:project) { create(:project, :repository) } - let(:processor) { described_class.new(values, project, '12345') } + set(:project) { create(:project, :repository) } + + let(:processor) { described_class.new(values, project: project, sha: '12345') } describe "#perform" do context 'when no external files defined' do diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index ea6f1e20014..49988468d1a 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -205,6 +205,23 @@ describe Gitlab::Ci::Config do end end + context "when gitlab_ci.yml has ambigious 'include' defined" do + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + remote: http://url + local: /local/file.yml + HEREDOC + end + + it 'raises error YamlProcessor validationError' do + expect { config }.to raise_error( + described_class::ConfigError, + 'Include `{"remote":"http://url","local":"/local/file.yml"}` needs to match exactly one accessor!' + ) + end + end + describe 'external file version' do context 'when external local file SHA is defined' do it 'is using a defined value' do