Change ExternalFile to retrieve local file from repository instead of GitLab project

CE mirror of 03c6094997023d9c8875ced421a6c9ef39a4af44

12 files changed, 93 insertions, 52 deletions

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