From 95b296f8ac8578e142efd6a60a582be4da5b09be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 7 Sep 2018 21:18:26 +0200 Subject: Change ExternalFile to retrieve local file from repository instead of GitLab project CE mirror of 03c6094997023d9c8875ced421a6c9ef39a4af44 --- app/models/blob_viewer/gitlab_ci_yml.rb | 2 +- app/models/repository.rb | 4 +++ lib/gitlab/ci/config.rb | 2 +- lib/gitlab/ci/external_files/external_file.rb | 21 ++++++----- lib/gitlab/ci/external_files/mapper.rb | 16 +++++---- lib/gitlab/ci/external_files/processor.rb | 4 +-- lib/gitlab/ci/yaml_processor.rb | 6 ++-- .../ci/external_files/.gitlab-ci-template-2.yml | 8 ----- spec/lib/gitlab/ci/config_spec.rb | 16 +++++++-- .../gitlab/ci/external_files/external_file_spec.rb | 10 ++++-- spec/lib/gitlab/ci/external_files/mapper_spec.rb | 42 ++++++++++++++++------ .../lib/gitlab/ci/external_files/processor_spec.rb | 14 ++++---- 12 files changed, 93 insertions(+), 52 deletions(-) delete mode 100644 spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml diff --git a/app/models/blob_viewer/gitlab_ci_yml.rb b/app/models/blob_viewer/gitlab_ci_yml.rb index 1a86f04b1b9..f5e4367fc09 100644 --- a/app/models/blob_viewer/gitlab_ci_yml.rb +++ b/app/models/blob_viewer/gitlab_ci_yml.rb @@ -15,7 +15,7 @@ module BlobViewer prepare! - @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data) + @validation_message = Gitlab::Ci::YamlProcessor.validation_message(blob.data, project) end def valid? diff --git a/app/models/repository.rb b/app/models/repository.rb index 929d28b9d88..4b479f047ed 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -980,6 +980,10 @@ class Repository blob_data_at(sha, '.gitlab/route-map.yml') end + def fetch_file_for(sha, path_to_file) + blob_data_at(sha, path_to_file) + end + def gitlab_ci_yml_for(sha, path = '.gitlab-ci.yml') blob_data_at(sha, path) end diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index f665ace7c74..caa3a7c3c86 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -12,7 +12,7 @@ module Gitlab .to_hash if project.present? - processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config) + processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config, project) @config = processor.perform else @config = initial_config diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb index 47d6d1c0fdc..45c6ca3fbfe 100644 --- a/lib/gitlab/ci/external_files/external_file.rb +++ b/lib/gitlab/ci/external_files/external_file.rb @@ -4,33 +4,38 @@ module Gitlab module Ci module ExternalFiles class ExternalFile - def initialize(value) + def initialize(value, project) @value = value + @project = project end def content if remote_url? open(value).read else - File.read(base_path) + local_file_content end end def valid? - remote_url? || File.exist?(base_path) + remote_url? || local_file_content end private - attr_reader :value - - def base_path - "#{Rails.root}/#{value}" - end + attr_reader :value, :project def remote_url? ::Gitlab::UrlSanitizer.valid?(value) end + + def local_file_content + project.repository.fetch_file_for(sha, value) + end + + def sha + @sha ||= project.repository.commit.sha + end end end end diff --git a/lib/gitlab/ci/external_files/mapper.rb b/lib/gitlab/ci/external_files/mapper.rb index 4b5d2ddd6a1..2d40e296546 100644 --- a/lib/gitlab/ci/external_files/mapper.rb +++ b/lib/gitlab/ci/external_files/mapper.rb @@ -2,12 +2,12 @@ module Gitlab module Ci module ExternalFiles class Mapper - def self.fetch_paths(values) - paths = values.fetch(:includes, []) - normalize_paths(paths) + def initialize(values, project) + @paths = values.fetch(:includes, []) + @project = project end - def self.normalize_paths(paths) + def process if paths.is_a?(String) [build_external_file(paths)] else @@ -15,8 +15,12 @@ module Gitlab end end - def self.build_external_file(path) - ::Gitlab::Ci::ExternalFiles::ExternalFile.new(path) + private + + attr_reaer :paths, :project + + def build_external_file(path) + ::Gitlab::Ci::ExternalFiles::ExternalFile.new(path, project) end end end diff --git a/lib/gitlab/ci/external_files/processor.rb b/lib/gitlab/ci/external_files/processor.rb index 221b6f58b98..ac51db05d8e 100644 --- a/lib/gitlab/ci/external_files/processor.rb +++ b/lib/gitlab/ci/external_files/processor.rb @@ -4,9 +4,9 @@ module Gitlab class Processor ExternalFileError = Class.new(StandardError) - def initialize(values) + def initialize(values, project) @values = values - @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.fetch_paths(values) + @external_files = ::Gitlab::Ci::ExternalFiles::Mapper.fetch_paths(values, project).process end def perform diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 702bcd3802d..0f79fbede9f 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -73,13 +73,13 @@ module Gitlab end end - def self.validation_message(content, opts = {}) + def self.validation_message(content, project = nil, opts = {}) return 'Please provide content of .gitlab-ci.yml' if content.blank? begin - Gitlab::Ci::YamlProcessor.new(content, opts) + Gitlab::Ci::YamlProcessor.new(content, project, opts) nil - rescue ValidationError => e + rescue ValidationError, ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError => e e.message end end diff --git a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml b/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml deleted file mode 100644 index b341cca8946..00000000000 --- a/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml +++ /dev/null @@ -1,8 +0,0 @@ -variables: - # AUTO_DEVOPS_DOMAIN is the application deployment domain and should be set as a variable at the group or project level. - - AUTO_DEVOPS_DOMAIN: domain.example.com - POSTGRES_USER: user - POSTGRES_PASSWORD: testing-password - POSTGRES_ENABLED: "true" - POSTGRES_DB: $CI_ENVIRONMENT_SLUG diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 57354e12aa3..b1c801ff052 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -127,6 +127,17 @@ describe Gitlab::Ci::Config do end context "when yml has valid 'includes' defined" do + let(:http_file_content) do + <<~HEREDOC + variables: + AUTO_DEVOPS_DOMAIN: domain.example.com + POSTGRES_USER: user + POSTGRES_PASSWORD: testing-password + POSTGRES_ENABLED: "true" + POSTGRES_DB: $CI_ENVIRONMENT_SLUG + HEREDOC + end + let(:local_file_content) { File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") } let(:yml) do <<-EOS includes: @@ -139,7 +150,8 @@ describe Gitlab::Ci::Config do end before do - allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(yml) + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(local_file_content) + allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(http_file_content) end it 'should return a composed hash' do @@ -167,7 +179,7 @@ describe Gitlab::Ci::Config do end end - context "when config has invalid 'includes' defined" do + context "when yml has invalid 'includes' defined" do let(:yml) do <<-EOS includes: invalid diff --git a/spec/lib/gitlab/ci/external_files/external_file_spec.rb b/spec/lib/gitlab/ci/external_files/external_file_spec.rb index 33e5e0b3b77..b2aeb0ac67a 100644 --- a/spec/lib/gitlab/ci/external_files/external_file_spec.rb +++ b/spec/lib/gitlab/ci/external_files/external_file_spec.rb @@ -1,12 +1,17 @@ require 'rails_helper' describe Gitlab::Ci::ExternalFiles::ExternalFile do - let(:external_file) { described_class.new(value) } + let(:project) { create(:project, :repository) } + let(:external_file) { described_class.new(value, project) } describe "#valid?" do context 'when is a valid remote url' do let(:value) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + before do + allow_any_instance_of(described_class).to receive(:local_file_content).and_return("image: 'ruby2:2'") + end + it 'should return true' do expect(external_file.valid?).to be_truthy end @@ -54,8 +59,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' } before do - allow(File).to receive(:exists?).and_return(true) - allow(File).to receive(:read).and_return(external_file_content) + allow_any_instance_of(described_class).to receive(:local_file_content).and_return(external_file_content) end it 'should return the content of the file' do diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb index 57cf5e74cdc..4bdb6c200c4 100644 --- a/spec/lib/gitlab/ci/external_files/mapper_spec.rb +++ b/spec/lib/gitlab/ci/external_files/mapper_spec.rb @@ -1,36 +1,56 @@ require 'rails_helper' describe Gitlab::Ci::ExternalFiles::Mapper do - describe '.fetch_paths' do - context 'when includes is defined as string' do - let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } } + let(:project) { create(:project, :repository) } + + describe '#process' do + subject { described_class.new(values, project).process } + + context 'when includes keyword is defined as string' do + let(:values) do + { + includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', + image: 'ruby:2.2' + } + end it 'returns an array' do - expect(described_class.fetch_paths(values)).to be_an(Array) + expect(subject).to be_an(Array) end it 'returns ExternalFile instances' do - expect(described_class.fetch_paths(values).first).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) + expect(subject.first).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile) end end context 'when includes is defined as an array' do - let(:values) { { includes: ['https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', '/vendor/gitlab-ci-yml/template.yml'], image: 'ruby:2.2' } } + let(:values) do + { + includes: + [ + 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', + '/vendor/gitlab-ci-yml/template.yml' + ], + image: 'ruby:2.2' + } + end + it 'returns an array' do - expect(described_class.fetch_paths(values)).to be_an(Array) + expect(subject).to be_an(Array) end it 'returns ExternalFile instances' do - paths = described_class.fetch_paths(values) - expect(paths).to all(be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile)) + expect(subject).to all(be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile)) end end context 'when includes is not defined' do - let(:values) { { image: 'ruby:2.2' } } + let(:values) do + { image: 'ruby:2.2' } + end it 'returns an empty array' do - expect(described_class.fetch_paths(values)).to be_empty + expect(subject).to be_empty end end end diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external_files/processor_spec.rb index 7a0374a4bce..f009ece8821 100644 --- a/spec/lib/gitlab/ci/external_files/processor_spec.rb +++ b/spec/lib/gitlab/ci/external_files/processor_spec.rb @@ -1,7 +1,8 @@ require 'rails_helper' describe Gitlab::Ci::ExternalFiles::Processor do - let(:processor) { described_class.new(values) } + let(:project) { create(:project, :repository) } + let(:processor) { described_class.new(values, project) } describe "#perform" do context 'when no external files defined' do @@ -77,8 +78,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do end before do - allow(File).to receive(:exists?).and_return(true) - allow(File).to receive(:read).and_return(external_file_content) + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(external_file_content) end it 'should append the file to the values' do @@ -95,7 +95,6 @@ describe Gitlab::Ci::ExternalFiles::Processor do let(:external_files) do [ "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml", - "/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml", 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' ] end @@ -111,11 +110,13 @@ describe Gitlab::Ci::ExternalFiles::Processor do end before do + file_content = File.read("#{Rails.root}/spec/ee/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml") + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(file_content) allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(remote_file_content) end it 'should append the files to the values' do - expect(processor.perform.keys).to match_array([:image, :variables, :stages, :before_script, :rspec]) + expect(processor.perform.keys).to match_array([:image, :stages, :before_script, :rspec]) end it "should remove the 'includes' keyword" do @@ -129,8 +130,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do let(:external_file_content) { 'invalid content file ////' } before do - allow(File).to receive(:exists?).and_return(true) - allow(File).to receive(:read).and_return(external_file_content) + allow_any_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile).to receive(:local_file_content).and_return(external_file_content) end it 'should raise an error' do -- cgit v1.2.1