diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2018-12-01 12:39:13 +0100 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-01-07 09:38:05 +0100 |
commit | c4d615c9dcba6815d0e9d1b7b7de5b7528ac7c72 (patch) | |
tree | d5358e8d4dad18f7b92b2dea54b1b1b768e37313 | |
parent | b97b85c37e77e5d37705cb2d3a60161896585420 (diff) | |
download | gitlab-ce-c4d615c9dcba6815d0e9d1b7b7de5b7528ac7c72.tar.gz |
Allow to include files from another projects
This adds `project:, file:, ref:` specification support.
22 files changed, 314 insertions, 42 deletions
diff --git a/app/controllers/projects/ci/lints_controller.rb b/app/controllers/projects/ci/lints_controller.rb index 2090af0a111..d7a0b7ece14 100644 --- a/app/controllers/projects/ci/lints_controller.rb +++ b/app/controllers/projects/ci/lints_controller.rb @@ -8,7 +8,7 @@ class Projects::Ci::LintsController < Projects::ApplicationController def create @content = params[:content] - @error = Gitlab::Ci::YamlProcessor.validation_message(@content, yaml_processor_options) + @error = Gitlab::Ci::YamlProcessor.validation_message(@content, yaml_processor_options) @status = @error.blank? if @error.blank? @@ -24,6 +24,10 @@ class Projects::Ci::LintsController < Projects::ApplicationController private def yaml_processor_options - { project: @project, sha: project.repository.commit.sha } + { + project: @project, + user: current_user, + sha: project.repository.commit.sha + } end end diff --git a/app/models/blob_viewer/gitlab_ci_yml.rb b/app/models/blob_viewer/gitlab_ci_yml.rb index 655241c2808..11228e620c9 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, sha) + def validation_message(opts) return @validation_message if defined?(@validation_message) prepare! - @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, { project: project, sha: sha }) + @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, opts) end - def valid?(project, sha) - validation_message(project, sha).blank? + def valid?(opts) + validation_message(opts).blank? end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 01134e133db..d0027ad823c 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -496,7 +496,7 @@ module Ci return @config_processor if defined?(@config_processor) @config_processor ||= begin - ::Gitlab::Ci::YamlProcessor.new(ci_yaml_file, { project: project, sha: sha }) + ::Gitlab::Ci::YamlProcessor.new(ci_yaml_file, { project: project, sha: sha, user: user }) rescue Gitlab::Ci::YamlProcessor::ValidationError => e self.yaml_errors = e.message nil 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 5be7cc7f25a..61d67a88a5a 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, @commit.sha) +- if viewer.valid?(project: @project, sha: @commit.sha, user: @current_user) = icon('check fw') This GitLab CI configuration is valid. - else = icon('warning fw') This GitLab CI configuration is invalid: - = viewer.validation_message(@project, @commit.sha) + = viewer.validation_message(project: @project, sha: @commit.sha, user: @current_user) = link_to 'Learn more', help_page_path('ci/yaml/README') diff --git a/changelogs/unreleased/include-project.yml b/changelogs/unreleased/include-project.yml new file mode 100644 index 00000000000..c63ac490d21 --- /dev/null +++ b/changelogs/unreleased/include-project.yml @@ -0,0 +1,5 @@ +--- +title: Allow to include files from another projects in gitlab-ci.yml +merge_request: 24101 +author: +type: added diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index fdb57532770..3532185a0ac 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1649,7 +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). +> In GitLab 11.7, support for [including GitLab-supplied templates directly](https://gitlab.com/gitlab-org/gitlab-ce/issues/53445) and support for [including templates from another repository](https://gitlab.com/gitlab-org/gitlab-ce/issues/53903) was added. Using the `include` keyword, you can allow the inclusion of external YAML files. @@ -1724,7 +1724,7 @@ include: --- -`include` supports three types of files: +`include` supports four types of files: - **local** to the same repository, referenced by using full paths in the same repository, with `/` being the root directory. For example: @@ -1750,6 +1750,32 @@ include: NOTE: **Note:** We don't support the inclusion of local files through Git submodules paths. +- **file** from another repository, referenced by using full paths in the same + repository, with `/` being the root directory. For example: + + ```yaml + include: + project: 'my-group/my-project' + file: '/templates/.gitlab-ci-template.yml' + ``` + + You can also specify `ref:`. The default `ref:` is the `HEAD` of the project: + + ```yaml + include: + - project: 'my-group/my-project' + ref: master + file: '/templates/.gitlab-ci-template.yml' + + - project: 'my-group/my-project' + ref: v1.0.0 + file: '/templates/.gitlab-ci-template.yml' + + - project: 'my-group/my-project' + ref: 787123b47f14b552955ca2786bc9542ae66fee5b # git sha + file: '/templates/.gitlab-ci-template.yml' + ``` + - **remote** in a different location, accessed using HTTP/HTTPS, referenced using the full URL. For example: diff --git a/lib/api/lint.rb b/lib/api/lint.rb index 0342a4b6654..a7672021db0 100644 --- a/lib/api/lint.rb +++ b/lib/api/lint.rb @@ -8,7 +8,8 @@ module API requires :content, type: String, desc: 'Content of .gitlab-ci.yml' end post '/lint' do - error = Gitlab::Ci::YamlProcessor.validation_message(params[:content]) + error = Gitlab::Ci::YamlProcessor.validation_message(params[:content], + user: current_user) status 200 diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 11e0352975d..5875479183e 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -8,9 +8,9 @@ module Gitlab class Config ConfigError = Class.new(StandardError) - def initialize(config, opts = {}) + def initialize(config, project: nil, sha: nil, user: nil) @config = Config::Extendable - .new(build_config(config, opts)) + .new(build_config(config, project: project, sha: sha, user: user)) .to_hash @global = Entry::Global.new(@config) @@ -70,20 +70,21 @@ module Gitlab private - def build_config(config, opts = {}) + def build_config(config, project:, sha:, user:) initial_config = Gitlab::Config::Loader::Yaml.new(config).load! - project = opts.fetch(:project, nil) if project - process_external_files(initial_config, project, opts) + process_external_files(initial_config, project: project, sha: sha, user: user) else initial_config end end - def process_external_files(config, project, opts) - sha = opts.fetch(:sha) { project.repository.root_ref_sha } - Config::External::Processor.new(config, project: project, sha: sha).perform + def process_external_files(config, project:, sha:, user:) + Config::External::Processor.new(config, + project: project, + sha: sha || project.repository.root_ref_sha, + user: user).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 2ac6656a703..a747886093c 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -12,7 +12,7 @@ module Gitlab YAML_WHITELIST_EXTENSION = /.+\.(yml|yaml)$/i.freeze - Context = Struct.new(:project, :sha) + Context = Struct.new(:project, :sha, :user) def initialize(params, context) @params = params diff --git a/lib/gitlab/ci/config/external/file/project.rb b/lib/gitlab/ci/config/external/file/project.rb new file mode 100644 index 00000000000..e75540dbe5a --- /dev/null +++ b/lib/gitlab/ci/config/external/file/project.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module External + module File + class Project < Base + include Gitlab::Utils::StrongMemoize + + attr_reader :project_name, :ref_name + + def initialize(params, context = {}) + @location = params[:file] + @project_name = params[:project] + @ref_name = params[:ref] || 'HEAD' + + super + end + + def matching? + super && project_name.present? + end + + def content + strong_memoize(:content) { fetch_local_content } + end + + private + + def validate_content! + if !can_access_local_content? + errors.push("Project `#{project_name}` not found or access denied!") + elsif sha.nil? + errors.push("Project `#{project_name}` reference `#{ref_name}` does not exist!") + elsif content.nil? + errors.push("Project `#{project_name}` file `#{location}` does not exist!") + elsif content.blank? + errors.push("Project `#{project_name}` file `#{location}` is empty!") + end + end + + def project + strong_memoize(:project) do + ::Project.find_by_full_path(project_name) + end + end + + def can_access_local_content? + Ability.allowed?(context.user, :download_code, project) + end + + def fetch_local_content + return unless can_access_local_content? + return unless sha + + project.repository.blob_data_at(sha, location) + rescue GRPC::NotFound, GRPC::Internal + nil + end + + def sha + strong_memoize(:sha) do + project.commit(ref_name).try(:sha) + end + 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 74bd927da39..108bfd5eb43 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -10,15 +10,17 @@ module Gitlab FILE_CLASSES = [ External::File::Remote, External::File::Template, - External::File::Local + External::File::Local, + External::File::Project ].freeze AmbigiousSpecificationError = Class.new(StandardError) - def initialize(values, project:, sha:) + def initialize(values, project:, sha:, user:) @locations = Array.wrap(values.fetch(:include, [])) @project = project @sha = sha + @user = user end def process @@ -61,7 +63,7 @@ module Gitlab def context strong_memoize(:context) do - External::File::Base::Context.new(project, sha) + External::File::Base::Context.new(project, sha, user) end end end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb index 1d310b29dc8..69bc164a039 100644 --- a/lib/gitlab/ci/config/external/processor.rb +++ b/lib/gitlab/ci/config/external/processor.rb @@ -7,9 +7,9 @@ module Gitlab class Processor IncludeError = Class.new(StandardError) - def initialize(values, project:, sha:) + def initialize(values, project:, sha:, user:) @values = values - @external_files = External::Mapper.new(values, project: project, sha: sha).process + @external_files = External::Mapper.new(values, project: project, sha: sha, user: user).process @content = {} rescue External::Mapper::AmbigiousSpecificationError => e raise IncludeError, e.message diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 172926b8ab0..244658a44d3 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -10,7 +10,7 @@ module Gitlab attr_reader :cache, :stages, :jobs def initialize(config, opts = {}) - @ci_config = Gitlab::Ci::Config.new(config, opts) + @ci_config = Gitlab::Ci::Config.new(config, **opts) @config = @ci_config.to_hash unless @ci_config.valid? 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 ada8775c489..1a6b3587599 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -3,7 +3,7 @@ require 'fast_spec_helper' describe Gitlab::Ci::Config::External::File::Base do - let(:context) { described_class::Context.new(nil, 'HEAD') } + let(:context) { described_class::Context.new(nil, 'HEAD', nil) } let(:test_class) do Class.new(described_class) do 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 83be43e240b..ff67a765da0 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Local do set(:project) { create(:project, :repository) } - let(:context) { described_class::Context.new(project, '12345') } + let(:context) { described_class::Context.new(project, '12345', nil) } let(:params) { { local: location } } let(:local_file) { described_class.new(params, context) } diff --git a/spec/lib/gitlab/ci/config/external/file/project_spec.rb b/spec/lib/gitlab/ci/config/external/file/project_spec.rb new file mode 100644 index 00000000000..11809adcaf6 --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/file/project_spec.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Config::External::File::Project do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + + let(:context_user) { user } + let(:context) { described_class::Context.new(nil, '12345', context_user) } + let(:subject) { described_class.new(params, context) } + + before do + project.add_developer(user) + end + + describe '#matching?' do + context 'when a file and project is specified' do + let(:params) { { file: 'file.yml', project: 'project' } } + + it 'should return true' do + expect(subject).to be_matching + end + end + + context 'with only file is specified' do + let(:params) { { file: 'file.yml' } } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + + context 'with only project is specified' do + let(:params) { { project: 'project' } } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + + context 'with a missing local key' do + let(:params) { {} } + + it 'should return false' do + expect(subject).not_to be_matching + end + end + end + + describe '#valid?' do + context 'when a valid path is used' do + let(:params) do + { project: project.full_path, file: '/file.yml' } + end + + let(:root_ref_sha) { project.repository.root_ref_sha } + + before do + stub_project_blob(root_ref_sha, '/file.yml') { 'image: ruby:2.1' } + end + + it 'should return true' do + expect(subject).to be_valid + end + + context 'when user does not have permission to access file' do + let(:context_user) { create(:user) } + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include("Project `#{project.full_path}` not found or access denied!") + end + end + end + + context 'when a valid path with custom ref is used' do + let(:params) do + { project: project.full_path, ref: 'master', file: '/file.yml' } + end + + let(:ref_sha) { project.commit('master').sha } + + before do + stub_project_blob(ref_sha, '/file.yml') { 'image: ruby:2.1' } + end + + it 'should return true' do + expect(subject).to be_valid + end + end + + context 'when an empty file is used' do + let(:params) do + { project: project.full_path, file: '/file.yml' } + end + + let(:root_ref_sha) { project.repository.root_ref_sha } + + before do + stub_project_blob(root_ref_sha, '/file.yml') { '' } + end + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include("Project `#{project.full_path}` file `/file.yml` is empty!") + end + end + + context 'when non-existing ref is used' do + let(:params) do + { project: project.full_path, ref: 'I-Do-Not-Exist', file: '/file.yml' } + end + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include("Project `#{project.full_path}` reference `I-Do-Not-Exist` does not exist!") + end + end + + context 'when non-existing file is requested' do + let(:params) do + { project: project.full_path, file: '/invalid-file.yml' } + end + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include("Project `#{project.full_path}` file `/invalid-file.yml` does not exist!") + end + end + + context 'when file is not a yaml file' do + let(:params) do + { project: project.full_path, file: '/invalid-file' } + end + + it 'should return false' do + expect(subject).not_to be_valid + expect(subject.error_message).to include('Included file `/invalid-file` does not have YAML extension!') + end + end + end + + private + + def stub_project_blob(ref, path) + allow_any_instance_of(Repository) + .to receive(:blob_data_at) + .with(ref, path) { yield } + end +end 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 319e7137f9f..3e0fda9c308 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,7 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Remote do - let(:context) { described_class::Context.new(nil, '12345') } + let(:context) { described_class::Context.new(nil, '12345', nil) } 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' } diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index e27d2cd9422..4cab4961b0f 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Mapper do set(:project) { create(:project, :repository) } + set(:user) { create(:user) } 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' } @@ -20,7 +21,7 @@ describe Gitlab::Ci::Config::External::Mapper do end describe '#process' do - subject { described_class.new(values, project: project, sha: '123456').process } + subject { described_class.new(values, project: project, sha: '123456', user: user).process } context "when single 'include' keyword is defined" do context 'when the string is a local file' do diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index d2d4fbc5115..1ac58139b25 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -4,8 +4,13 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Processor do set(:project) { create(:project, :repository) } + set(:user) { create(:user) } - let(:processor) { described_class.new(values, project: project, sha: '12345') } + let(:processor) { described_class.new(values, project: project, sha: '12345', user: user) } + + before do + project.add_developer(user) + end 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 49988468d1a..cd6d2a2f343 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -1,8 +1,10 @@ require 'spec_helper' describe Gitlab::Ci::Config do + set(:user) { create(:user) } + let(:config) do - described_class.new(yml) + described_class.new(yml, project: nil, sha: nil, user: nil) end context 'when config is valid' do @@ -154,7 +156,7 @@ describe Gitlab::Ci::Config do end let(:config) do - described_class.new(gitlab_ci_yml, project: project, sha: '12345') + described_class.new(gitlab_ci_yml, project: project, sha: '12345', user: user) end before do @@ -228,7 +230,7 @@ describe Gitlab::Ci::Config do expect(project.repository).to receive(:blob_data_at) .with('eeff1122', local_location) - described_class.new(gitlab_ci_yml, project: project, sha: 'eeff1122') + described_class.new(gitlab_ci_yml, project: project, sha: 'eeff1122', user: user) end end @@ -236,7 +238,7 @@ describe Gitlab::Ci::Config 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) + described_class.new(gitlab_ci_yml, project: project, sha: nil, user: user) end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 441e8214181..1ee9e7a3ec7 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -3,10 +3,10 @@ require 'spec_helper' module Gitlab module Ci describe YamlProcessor do - subject { described_class.new(config) } + subject { described_class.new(config, user: nil) } describe '#build_attributes' do - subject { described_class.new(config).build_attributes(:rspec) } + subject { described_class.new(config, user: nil).build_attributes(:rspec) } describe 'attributes list' do let(:config) do diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb index 01c555a7a90..16bf947b493 100644 --- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb +++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb @@ -4,7 +4,9 @@ describe BlobViewer::GitlabCiYml do include FakeBlobHelpers include RepoHelpers - let(:project) { create(:project, :repository) } + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + 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 } @@ -14,12 +16,12 @@ describe BlobViewer::GitlabCiYml do it 'calls prepare! on the viewer' do expect(subject).to receive(:prepare!) - subject.validation_message(project, sha) + subject.validation_message(project: project, sha: sha, user: user) end context 'when the configuration is valid' do it 'returns nil' do - expect(subject.validation_message(project, sha)).to be_nil + expect(subject.validation_message(project: project, sha: sha, user: user)).to be_nil end end @@ -27,7 +29,7 @@ describe BlobViewer::GitlabCiYml do let(:data) { 'oof' } it 'returns the error message' do - expect(subject.validation_message(project, sha)).to eq('Invalid configuration format') + expect(subject.validation_message(project: project, sha: sha, user: user)).to eq('Invalid configuration format') end end end |