diff options
-rw-r--r-- | lib/gitlab/ci/external/mapper.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/ci/external/processor.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/external/mapper_spec.rb | 6 | ||||
-rw-r--r-- | 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 |