diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-01-14 13:55:51 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-03-01 17:55:53 +0300 |
commit | c78861bc4268049777bd279f5de5981ec4e78019 (patch) | |
tree | 64af983e404f84c3d9c7fd5cadaf6d6b4a3154ea /spec | |
parent | c9ecc71ab91b0b55f9aba632f9e7b305191a458c (diff) |
Allow to recursively expand includes
This change introduces a support for nesting the includes,
allowing to evaluate them in context of the target,
by properly respecting the relative inclusions and user permissions
of another projects, or templates.
Diffstat (limited to 'spec')
7 files changed, 231 insertions, 34 deletions
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 1a6b3587599..fa39b32d7ab 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', nil) } + let(:context) { described_class::Context.new(nil, 'HEAD', nil, Set.new) } let(:test_class) do Class.new(described_class) do @@ -79,4 +79,20 @@ describe Gitlab::Ci::Config::External::File::Base do end end end + + describe '#to_hash' do + context 'with includes' do + let(:location) { 'some/file/config.yml' } + let(:content) { 'include: { template: Bash.gitlab-ci.yml }'} + + before do + allow_any_instance_of(test_class) + .to receive(:content).and_return(content) + end + + it 'does expand hash to include the template' do + expect(subject.to_hash).to include(:before_script) + end + end + end 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 ff67a765da0..dc14b07287e 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -4,8 +4,10 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Local do set(:project) { create(:project, :repository) } + set(:user) { create(:user) } - let(:context) { described_class::Context.new(project, '12345', nil) } + let(:sha) { '12345' } + let(:context) { described_class::Context.new(project, sha, user, Set.new) } let(:params) { { local: location } } let(:local_file) { described_class.new(params, context) } @@ -103,4 +105,36 @@ describe Gitlab::Ci::Config::External::File::Local do expect(local_file.error_message).to eq("Local file `#{location}` does not exist!") end end + + describe '#expand_context' do + let(:location) { 'location.yml' } + + subject { local_file.send(:expand_context) } + + it 'inherits project, user and sha' do + is_expected.to include(user: user, project: project, sha: sha) + end + end + + describe '#to_hash' do + context 'properly includes another local file in the same repository' do + let(:location) { 'some/file/config.yml' } + let(:content) { 'include: { local: another-config.yml }'} + + let(:another_location) { 'another-config.yml' } + let(:another_content) { 'rspec: JOB' } + + before do + allow(project.repository).to receive(:blob_data_at).with(sha, location) + .and_return(content) + + allow(project.repository).to receive(:blob_data_at).with(sha, another_location) + .and_return(another_content) + end + + it 'does expand hash to include the template' do + expect(local_file.to_hash).to include(:rspec) + end + end + end end diff --git a/spec/lib/gitlab/ci/config/external/file/project_spec.rb b/spec/lib/gitlab/ci/config/external/file/project_spec.rb index 11809adcaf6..6e89bb1b30f 100644 --- a/spec/lib/gitlab/ci/config/external/file/project_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/project_spec.rb @@ -3,12 +3,13 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Project do + set(:context_project) { create(:project) } 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) } + let(:context) { described_class::Context.new(context_project, '12345', context_user, Set.new) } + let(:project_file) { described_class.new(params, context) } before do project.add_developer(user) @@ -19,7 +20,7 @@ describe Gitlab::Ci::Config::External::File::Project do let(:params) { { file: 'file.yml', project: 'project' } } it 'should return true' do - expect(subject).to be_matching + expect(project_file).to be_matching end end @@ -27,7 +28,7 @@ describe Gitlab::Ci::Config::External::File::Project do let(:params) { { file: 'file.yml' } } it 'should return false' do - expect(subject).not_to be_matching + expect(project_file).not_to be_matching end end @@ -35,7 +36,7 @@ describe Gitlab::Ci::Config::External::File::Project do let(:params) { { project: 'project' } } it 'should return false' do - expect(subject).not_to be_matching + expect(project_file).not_to be_matching end end @@ -43,7 +44,7 @@ describe Gitlab::Ci::Config::External::File::Project do let(:params) { {} } it 'should return false' do - expect(subject).not_to be_matching + expect(project_file).not_to be_matching end end end @@ -61,15 +62,15 @@ describe Gitlab::Ci::Config::External::File::Project do end it 'should return true' do - expect(subject).to be_valid + expect(project_file).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!") + expect(project_file).not_to be_valid + expect(project_file.error_message).to include("Project `#{project.full_path}` not found or access denied!") end end end @@ -86,7 +87,7 @@ describe Gitlab::Ci::Config::External::File::Project do end it 'should return true' do - expect(subject).to be_valid + expect(project_file).to be_valid end end @@ -102,8 +103,8 @@ describe Gitlab::Ci::Config::External::File::Project do 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!") + expect(project_file).not_to be_valid + expect(project_file.error_message).to include("Project `#{project.full_path}` file `/file.yml` is empty!") end end @@ -113,8 +114,8 @@ describe Gitlab::Ci::Config::External::File::Project do 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!") + expect(project_file).not_to be_valid + expect(project_file.error_message).to include("Project `#{project.full_path}` reference `I-Do-Not-Exist` does not exist!") end end @@ -124,8 +125,8 @@ describe Gitlab::Ci::Config::External::File::Project do 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!") + expect(project_file).not_to be_valid + expect(project_file.error_message).to include("Project `#{project.full_path}` file `/invalid-file.yml` does not exist!") end end @@ -135,12 +136,22 @@ describe Gitlab::Ci::Config::External::File::Project do 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!') + expect(project_file).not_to be_valid + expect(project_file.error_message).to include('Included file `/invalid-file` does not have YAML extension!') end end end + describe '#expand_context' do + let(:params) { { file: 'file.yml', project: project.full_path, ref: 'master' } } + + subject { project_file.send(:expand_context) } + + it 'inherits user, and target project and sha' do + is_expected.to include(user: user, project: project, sha: project.commit('master').id) + end + end + private def stub_project_blob(ref, path) 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 3e0fda9c308..c5b32c29759 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', nil) } + let(:context) { described_class::Context.new(nil, '12345', nil, Set.new) } 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' } @@ -181,4 +181,14 @@ describe Gitlab::Ci::Config::External::File::Remote do end end end + + describe '#expand_context' do + let(:params) { { remote: 'http://remote' } } + + subject { remote_file.send(:expand_context) } + + it 'drops all parameters' do + is_expected.to include(user: nil, project: nil, sha: nil) + end + end end diff --git a/spec/lib/gitlab/ci/config/external/file/template_spec.rb b/spec/lib/gitlab/ci/config/external/file/template_spec.rb index 1fb5655309a..8ecaf4800f8 100644 --- a/spec/lib/gitlab/ci/config/external/file/template_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/template_spec.rb @@ -3,18 +3,21 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Template do - let(:context) { described_class::Context.new(nil, '12345') } + set(:project) { create(:project) } + set(:user) { create(:user) } + + let(:context) { described_class::Context.new(project, '12345', user, Set.new) } let(:template) { 'Auto-DevOps.gitlab-ci.yml' } let(:params) { { template: template } } - subject { described_class.new(params, context) } + let(:template_file) { 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 + expect(template_file).to be_matching end end @@ -22,7 +25,7 @@ describe Gitlab::Ci::Config::External::File::Template do let(:params) { { template: nil } } it 'should return false' do - expect(subject).not_to be_matching + expect(template_file).not_to be_matching end end @@ -30,7 +33,7 @@ describe Gitlab::Ci::Config::External::File::Template do let(:params) { {} } it 'should return false' do - expect(subject).not_to be_matching + expect(template_file).not_to be_matching end end end @@ -40,7 +43,7 @@ describe Gitlab::Ci::Config::External::File::Template do let(:template) { 'Auto-DevOps.gitlab-ci.yml' } it 'should return true' do - expect(subject).to be_valid + expect(template_file).to be_valid end end @@ -48,8 +51,8 @@ describe Gitlab::Ci::Config::External::File::Template 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!') + expect(template_file).not_to be_valid + expect(template_file.error_message).to include('Template file `Template.yml` is not a valid location!') end end @@ -57,14 +60,14 @@ describe Gitlab::Ci::Config::External::File::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!') + expect(template_file).not_to be_valid + expect(template_file.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) } + let(:template_name) { template_file.send(:template_name) } context 'when template does end with .gitlab-ci.yml' do let(:template) { 'my-template.gitlab-ci.yml' } @@ -90,4 +93,14 @@ describe Gitlab::Ci::Config::External::File::Template do end end end + + describe '#expand_context' do + let(:location) { 'location.yml' } + + subject { template_file.send(:expand_context) } + + it 'drops all parameters' do + is_expected.to include(user: nil, project: nil, sha: nil) + 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 4cab4961b0f..136974569de 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -9,6 +9,7 @@ describe Gitlab::Ci::Config::External::Mapper do 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(:expandset) { Set.new } let(:file_content) do <<~HEREDOC @@ -21,7 +22,7 @@ describe Gitlab::Ci::Config::External::Mapper do end describe '#process' do - subject { described_class.new(values, project: project, sha: '123456', user: user).process } + subject { described_class.new(values, project: project, sha: '123456', user: user, expandset: expandset).process } context "when single 'include' keyword is defined" do context 'when the string is a local file' do @@ -141,5 +142,37 @@ describe Gitlab::Ci::Config::External::Mapper do expect(subject).to be_empty end end + + context "when duplicate 'include' is defined" do + let(:values) do + { include: [ + { 'local' => local_file }, + { 'local' => local_file } + ], + image: 'ruby:2.2' } + end + + it 'raises an exception' do + expect { subject }.to raise_error(described_class::DuplicateIncludesError) + end + end + + context "when too many 'includes' are defined" do + let(:values) do + { include: [ + { 'local' => local_file }, + { 'remote' => remote_url } + ], + image: 'ruby:2.2' } + end + + before do + stub_const("#{described_class}::MAX_INCLUDES", 1) + end + + it 'raises an exception' do + expect { subject }.to raise_error(described_class::TooManyIncludesError) + end + end end end diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 1ac58139b25..3f6f6d7c5d9 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -4,15 +4,20 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Processor do set(:project) { create(:project, :repository) } + set(:another_project) { create(:project, :repository) } set(:user) { create(:user) } - let(:processor) { described_class.new(values, project: project, sha: '12345', user: user) } + let(:expandset) { Set.new } + let(:sha) { '12345' } + let(:processor) { described_class.new(values, project: project, sha: '12345', user: user, expandset: expandset) } before do project.add_developer(user) end describe "#perform" do + subject { processor.perform } + context 'when no external files defined' do let(:values) { { image: 'ruby:2.2' } } @@ -190,5 +195,80 @@ describe Gitlab::Ci::Config::External::Processor do expect(processor.perform[:image]).to eq('ruby:2.2') end end + + context "when a nested includes are defined" do + let(:values) do + { + include: [ + { local: '/local/file.yml' } + ], + image: 'ruby:2.2' + } + end + + before do + allow(project.repository).to receive(:blob_data_at).with('12345', '/local/file.yml') do + <<~HEREDOC + include: + - template: Ruby.gitlab-ci.yml + - remote: http://my.domain.com/config.yml + - project: #{another_project.full_path} + file: /templates/my-workflow.yml + HEREDOC + end + + allow_any_instance_of(Repository).to receive(:blob_data_at).with(another_project.commit.id, '/templates/my-workflow.yml') do + <<~HEREDOC + include: + - local: /templates/my-build.yml + HEREDOC + end + + allow_any_instance_of(Repository).to receive(:blob_data_at).with(another_project.commit.id, '/templates/my-build.yml') do + <<~HEREDOC + my_build: + script: echo Hello World + HEREDOC + end + + WebMock.stub_request(:get, 'http://my.domain.com/config.yml').to_return(body: 'remote_build: { script: echo Hello World }') + end + + context 'when project is public' do + before do + another_project.update!(visibility: 'public') + end + + it 'properly expands all includes' do + is_expected.to include(:my_build, :remote_build, :rspec) + end + end + + context 'when user is reporter of another project' do + before do + another_project.add_reporter(user) + end + + it 'properly expands all includes' do + is_expected.to include(:my_build, :remote_build, :rspec) + end + end + + context 'when user is not allowed' do + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Ci::Config::External::Processor::IncludeError, /not found or access denied/) + end + end + + context 'when too many includes is included' do + before do + stub_const('Gitlab::Ci::Config::External::Mapper::MAX_INCLUDES', 1) + end + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Ci::Config::External::Processor::IncludeError, /Maximum of 1 nested/) + end + end + end end end |