Load external files in config

CE mirror of 8e03a6619be44fdaf19a6c13284ea8e51377b311

13 files changed, 89 insertions, 196 deletions

diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 46dad59eb8c..94b2fbc082b 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -1,15 +1,17 @@
 module Gitlab
   module Ci
-    ##
+    #
     # Base GitLab CI Configuration facade
     #
     class Config
       ConfigError = Class.new(StandardError)
 
       def initialize(config, opts = {})
-        @config = Config::Extendable
+        initial_config = Config::Extendable
           .new(build_config(config, opts))
           .to_hash
+        processor = ::Gitlab::Ci::ExternalFiles::Processor.new(initial_config)
+        @config = processor.perform
 
         @global = Entry::Global.new(@config)
         @global.compose!
diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb
index 401e566de77..a4ec8f0ff2f 100644
--- a/lib/gitlab/ci/config/entry/global.rb
+++ b/lib/gitlab/ci/config/entry/global.rb
@@ -33,15 +33,11 @@ module Gitlab
           entry :cache, Entry::Cache,
             description: 'Configure caching between build jobs.'
 
-          entry :includes, Entry::Includes,
-            description: 'External GitlLab Ci files'
-
           helpers :before_script, :image, :services, :after_script,
-                  :variables, :stages, :types, :cache, :jobs, :includes
+                  :variables, :stages, :types, :cache, :jobs
 
           def compose!(_deps = nil)
             super(self) do
-              append_external_files!
               compose_jobs!
               compose_deprecated_entries!
             end
@@ -49,10 +45,6 @@ module Gitlab
 
           private
 
-          def append_external_files!
-            return if includes_value.nil?
-          end
-
           def compose_jobs!
             factory = Entry::Factory.new(Entry::Jobs)
               .value(@config.except(*self.class.nodes.keys))
diff --git a/lib/gitlab/ci/config/entry/includes.rb b/lib/gitlab/ci/config/entry/includes.rb
deleted file mode 100644
index c7a8c7960e8..00000000000
--- a/lib/gitlab/ci/config/entry/includes.rb
+++ /dev/null
@@ -1,22 +0,0 @@
-module Gitlab
-  module Ci
-    class Config
-      module Entry
-        ##
-        # Entry that represents a Docker image.
-        #
-        class Includes < Node
-          include Validatable
-
-          validations do
-            validates :config, array_or_string: true, external_file: true, allow_nil: true
-          end
-
-          def value
-            Array(@config)
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb
index 5b4d5d53821..b3c889ee92f 100644
--- a/lib/gitlab/ci/config/entry/validators.rb
+++ b/lib/gitlab/ci/config/entry/validators.rb
@@ -60,38 +60,6 @@ module Gitlab
             end
           end
 
-          class ArrayOrStringValidator < ActiveModel::EachValidator
-            def validate_each(record, attribute, value)
-              unless value.is_a?(Array) || value.is_a?(String)
-                record.errors.add(attribute, 'should be an array or a string')
-              end
-            end
-          end
-
-          class ExternalFileValidator < ActiveModel::EachValidator
-            def validate_each(record, attribute, value)
-              if value.is_a?(Array)
-                value.each do |path|
-                  validate_external_file(path, record, attribute)
-                end
-              else
-                validate_external_file(value, record, attribute)
-              end
-            end
-
-            private
-
-            def validate_external_file(value, record, attribute)
-              unless valid_url?(value)
-                record.errors.add(attribute, 'should be a valid local or remote file')
-              end
-            end
-
-            def valid_url?(value)
-              Gitlab::UrlSanitizer.valid?(value) || File.exists?("#{Rails.root}/#{value}")
-            end
-          end
-
           class KeyValidator < ActiveModel::EachValidator
             include LegacyValidationHelpers
 
diff --git a/lib/gitlab/ci/external_files/external_file.rb b/lib/gitlab/ci/external_files/external_file.rb
index 9bad8231c42..47d6d1c0fdc 100644
--- a/lib/gitlab/ci/external_files/external_file.rb
+++ b/lib/gitlab/ci/external_files/external_file.rb
@@ -4,7 +4,6 @@ module Gitlab
   module Ci
     module ExternalFiles
       class ExternalFile
-
         def initialize(value)
           @value = value
         end
@@ -18,7 +17,7 @@ module Gitlab
         end
 
         def valid?
-          remote_url? || File.exists?(base_path) 
+          remote_url? || File.exist?(base_path)
         end
 
         private
diff --git a/lib/gitlab/ci/external_files/mapper.rb b/lib/gitlab/ci/external_files/mapper.rb
index 5deb2f5a2e5..4b5d2ddd6a1 100644
--- a/lib/gitlab/ci/external_files/mapper.rb
+++ b/lib/gitlab/ci/external_files/mapper.rb
@@ -2,14 +2,11 @@ module Gitlab
   module Ci
     module ExternalFiles
       class Mapper
-
         def self.fetch_paths(values)
           paths = values.fetch(:includes, [])
           normalize_paths(paths)
         end
 
-        private
-
         def self.normalize_paths(paths)
           if paths.is_a?(String)
             [build_external_file(paths)]
diff --git a/lib/gitlab/ci/external_files/processor.rb b/lib/gitlab/ci/external_files/processor.rb
index f38bc7633b5..221b6f58b98 100644
--- a/lib/gitlab/ci/external_files/processor.rb
+++ b/lib/gitlab/ci/external_files/processor.rb
@@ -25,7 +25,7 @@ module Gitlab
         attr_reader :values, :external_files
 
         def validate_external_file(external_file)
-          unless external_file.valid? 
+          unless external_file.valid?
             raise ExternalFileError, 'External files should be a valid local or remote file'
           end
         end
@@ -39,7 +39,6 @@ module Gitlab
           values.delete(:includes)
           values
         end
-
       end
     end
   end
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
index ff623b95be8..1860ed79bfd 100644
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -13,7 +13,7 @@ describe Gitlab::Ci::Config::Entry::Global do
         expect(described_class.nodes.keys)
           .to match_array(%i[before_script image services
                              after_script variables stages
-                             types cache includes])
+                             types cache])
       end
     end
   end
@@ -42,7 +42,7 @@ describe Gitlab::Ci::Config::Entry::Global do
         end
 
         it 'creates node object for each entry' do
-          expect(global.descendants.count).to eq 9
+          expect(global.descendants.count).to eq 8
         end
 
         it 'creates node object using valid class' do
@@ -189,7 +189,7 @@ describe Gitlab::Ci::Config::Entry::Global do
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.descendants.count).to eq 9
+          expect(global.descendants.count).to eq 8
         end
 
         it 'contains unspecified nodes' do
diff --git a/spec/lib/gitlab/ci/config/entry/includes_spec.rb b/spec/lib/gitlab/ci/config/entry/includes_spec.rb
deleted file mode 100644
index d72503535ed..00000000000
--- a/spec/lib/gitlab/ci/config/entry/includes_spec.rb
+++ /dev/null
@@ -1,97 +0,0 @@
-require 'rails_helper'
-
-describe Gitlab::Ci::Config::Entry::Includes do
-  let(:entry) { described_class.new(config) }
-
-  shared_examples 'valid external file' do
-    it 'should be valid' do
-      expect(entry).to be_valid
-    end
-
-    it 'should not return any error' do
-      expect(entry.errors).to be_empty
-    end
-  end
-
-  shared_examples 'invalid external file' do
-    it 'should not be valid' do
-      expect(entry).not_to be_valid
-    end
-
-    it 'should return an error' do
-      expect(entry.errors.first).to match(/should be a valid local or remote file/)
-    end
-  end
-
-  describe "#valid?" do
-    context 'with no external file given' do
-      let(:config) { nil }
-
-      it_behaves_like 'valid external file'
-    end
-
-    context 'with multiple external files' do
-      let(:config) { %w(https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-2.yml) }
-
-      it_behaves_like 'valid external file'
-    end
-
-    context 'with just one external file' do
-      let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } 
-
-      it_behaves_like 'valid external file'
-    end
-
-    context 'when they contain valid URLs' do
-      let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } 
-
-      it_behaves_like 'valid external file'
-    end
-
-    context 'when they contain valid relative URLs' do
-      let(:config) { '/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml' } 
-
-      it_behaves_like 'valid external file'
-    end
-
-    context 'when they not contain valid URLs' do
-      let(:config) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
-
-      it_behaves_like 'invalid external file'
-    end
-
-    context 'when they not contain valid relative URLs' do
-      let(:config) { '/vendor/gitlab-ci-yml/non-existent-file.yml' }
-
-      it_behaves_like 'invalid external file' 
-    end
-  end
-
-  describe "#value" do
-    context 'with multiple external files' do
-      let(:config) { %w(https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-2.yml) }
-      it 'should return an array' do
-        expect(entry.value).to be_an(Array)
-        expect(entry.value.count).to eq(2)
-      end
-    end
-
-    context 'with just one external file' do
-      let(:config) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
-
-      it 'should return an array' do
-        expect(entry.value).to be_an(Array)
-        expect(entry.value.count).to eq(1)
-      end
-    end
-
-    context 'with no external file given' do
-      let(:config) { nil }
-
-      it 'should return an empty array' do
-        expect(entry.value).to be_an(Array)
-        expect(entry.value).to be_empty
-      end
-    end
-  end
-end
diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index 5a78ce783dd..99896b9be5d 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -124,4 +124,61 @@ describe Gitlab::Ci::Config do
       end
     end
   end
+
+
+  context "when yml has valid 'includes' defined" do
+    let(:yml) do
+      <<-EOS
+        includes:
+          - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml
+          - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-2.yml
+          - https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml
+
+        image: ruby:2.2
+      EOS
+    end
+
+    before do
+      allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(yml)
+    end
+
+    it 'should return a composed hash' do
+      before_script_values = [
+        "apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs", "ruby -v",
+        "which ruby",
+        "gem install bundler --no-ri --no-rdoc",
+        "bundle install --jobs $(nproc)  \"${FLAGS[@]}\""
+      ]
+      variables = {
+        AUTO_DEVOPS_DOMAIN: "domain.example.com",
+        POSTGRES_USER: "user",
+        POSTGRES_PASSWORD: "testing-password",
+        POSTGRES_ENABLED: "true",
+        POSTGRES_DB: "$CI_ENVIRONMENT_SLUG"
+      }
+      composed_hash = {
+        before_script: before_script_values,
+        image: "ruby:2.2",
+        rspec: { script: ["bundle exec rspec"] },
+        variables: variables
+      }
+
+      expect(config.to_hash).to eq(composed_hash)
+    end
+  end
+
+  context "when config has invalid 'includes' defined"  do
+    let(:yml) do
+      <<-EOS
+      includes: invalid
+      EOS
+    end
+
+    it 'raises error' do
+      expect { config }.to raise_error(
+        ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError,
+        /External files should be a valid local or remote file/
+      )
+    end
+  end
 end
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 80468f63435..33e5e0b3b77 100644
--- a/spec/lib/gitlab/ci/external_files/external_file_spec.rb
+++ b/spec/lib/gitlab/ci/external_files/external_file_spec.rb
@@ -13,7 +13,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile 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(:value) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
 
       it 'should return false' do
         expect(external_file.valid?).to be_falsy
@@ -39,7 +39,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do
   end
 
   describe "#content" do
-    let(:external_file_content) {
+    let(:external_file_content) do
       <<-HEREDOC
         before_script:
           - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs
@@ -48,7 +48,7 @@ describe Gitlab::Ci::ExternalFiles::ExternalFile do
           - gem install bundler --no-ri --no-rdoc
           - bundle install --jobs $(nproc)  "${FLAGS[@]}"
       HEREDOC
-    }
+    end
 
     context 'with a local file' do
       let(:value) { '/vendor/gitlab-ci-yml/non-existent-file.yml' }
diff --git a/spec/lib/gitlab/ci/external_files/mapper_spec.rb b/spec/lib/gitlab/ci/external_files/mapper_spec.rb
index dfbf8bfa098..57cf5e74cdc 100644
--- a/spec/lib/gitlab/ci/external_files/mapper_spec.rb
+++ b/spec/lib/gitlab/ci/external_files/mapper_spec.rb
@@ -3,7 +3,7 @@ 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(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } }
 
       it 'returns an array' do
         expect(described_class.fetch_paths(values)).to be_an(Array)
@@ -15,21 +15,19 @@ describe Gitlab::Ci::ExternalFiles::Mapper do
     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) { { includes: ['https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', '/vendor/gitlab-ci-yml/template.yml'], image: 'ruby:2.2' } }
       it 'returns an array' do
         expect(described_class.fetch_paths(values)).to be_an(Array)
       end
 
       it 'returns ExternalFile instances' do
         paths = described_class.fetch_paths(values)
-        paths.each do |path|
-          expect(path).to be_an_instance_of(::Gitlab::Ci::ExternalFiles::ExternalFile)
-        end
+        expect(paths).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) { { image: 'ruby:2.2' } }
 
       it 'returns an empty array' do
         expect(described_class.fetch_paths(values)).to be_empty
diff --git a/spec/lib/gitlab/ci/external_files/processor_spec.rb b/spec/lib/gitlab/ci/external_files/processor_spec.rb
index 51ed14de4cf..7a0374a4bce 100644
--- a/spec/lib/gitlab/ci/external_files/processor_spec.rb
+++ b/spec/lib/gitlab/ci/external_files/processor_spec.rb
@@ -13,7 +13,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
     end
 
     context 'when an invalid local file is defined' do
-      let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2'} }
+      let(:values) { { includes: '/vendor/gitlab-ci-yml/non-existent-file.yml', image: 'ruby:2.2' } }
 
       it 'should raise an error' do
         expect { processor.perform }.to raise_error(described_class::ExternalFileError)
@@ -21,7 +21,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
     end
 
     context 'when an invalid remote file is defined' do
-      let(:values) { { includes: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2'} }
+      let(:values) { { includes: 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } }
 
       it 'should raise an error' do
         expect { processor.perform }.to raise_error(described_class::ExternalFileError)
@@ -30,7 +30,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
 
     context 'with a valid remote external file is defined' do
       let(:values) { { includes: 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml', image: 'ruby:2.2' } }
-      let(:external_file_content) {
+      let(:external_file_content) do
         <<-HEREDOC
         before_script:
           - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs
@@ -47,7 +47,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
           script:
             - bundle exec rubocop
         HEREDOC
-      }
+      end
 
       before do
         allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(external_file_content)
@@ -55,7 +55,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
 
       it 'should append the file to the values' do
         output = processor.perform
-        expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop]) 
+        expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop])
       end
 
       it "should remove the 'includes' keyword" do
@@ -64,8 +64,8 @@ describe Gitlab::Ci::ExternalFiles::Processor do
     end
 
     context 'with a valid local external file is defined' do
-      let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml' , image: 'ruby:2.2'} }
-      let(:external_file_content) {
+      let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } }
+      let(:external_file_content) do
         <<-HEREDOC
         before_script:
           - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs
@@ -74,7 +74,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
           - gem install bundler --no-ri --no-rdoc
           - bundle install --jobs $(nproc)  "${FLAGS[@]}"
         HEREDOC
-      }
+      end
 
       before do
         allow(File).to receive(:exists?).and_return(true)
@@ -83,7 +83,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
 
       it 'should append the file to the values' do
         output = processor.perform
-        expect(output.keys).to match_array([:image, :before_script]) 
+        expect(output.keys).to match_array([:image, :before_script])
       end
 
       it "should remove the 'includes' keyword" do
@@ -92,23 +92,23 @@ describe Gitlab::Ci::ExternalFiles::Processor do
     end
 
     context 'with multiple external files are defined' do
-      let(:external_files) { 
+      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'
         ]
-      }
-      let(:values) { { includes: external_files, image: 'ruby:2.2'} }
+      end
+      let(:values) { { includes: external_files, image: 'ruby:2.2' } }
 
-      let(:remote_file_content) {
+      let(:remote_file_content) do
         <<-HEREDOC
         stages:
           - build
           - review
           - cleanup
         HEREDOC
-      }
+      end
 
       before do
         allow_any_instance_of(Kernel).to receive_message_chain(:open, :read).and_return(remote_file_content)
@@ -124,7 +124,7 @@ describe Gitlab::Ci::ExternalFiles::Processor do
     end
 
     context 'when external files are defined but not valid' do
-      let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2'} }
+      let(:values) { { includes: '/vendor/gitlab-ci-yml/template.yml', image: 'ruby:2.2' } }
 
       let(:external_file_content) { 'invalid content file ////' }