diff options
| -rw-r--r-- | app/models/blob_viewer/gitlab_ci_yml.rb | 8 | ||||
| -rw-r--r-- | app/models/ci/pipeline.rb | 2 | ||||
| -rw-r--r-- | app/views/projects/blob/viewers/_gitlab_ci_yml.html.haml | 4 | ||||
| -rw-r--r-- | lib/gitlab/ci/config.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/ci/external/file/local.rb | 18 | ||||
| -rw-r--r-- | lib/gitlab/ci/external/file/remote.rb | 18 | ||||
| -rw-r--r-- | lib/gitlab/ci/external/mapper.rb | 17 | ||||
| -rw-r--r-- | lib/gitlab/ci/external/processor.rb | 10 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config_spec.rb | 3 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/external/file/local_spec.rb | 9 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/external/file/remote_spec.rb | 12 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/external/mapper_spec.rb | 12 | ||||
| -rw-r--r-- | 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 |