Reconcile differences in lib/gitlab/ci

lib/gitlab/ci/config.rb

@@ -7,14 +7,18 @@ module Gitlab
       ConfigError = Class.new(StandardError)
 
       def initialize(config, opts = {})
-        @config = Config::Extendable
-          .new(build_config(config, opts))
-          .to_hash
-
-        @global = Entry::Global.new(@config)
-        @global.compose!
-      rescue Loader::FormatError, Extendable::ExtensionError => e
-        raise Config::ConfigError, e.message
+        begin
+          @config = Config::Extendable
+            .new(build_config(config, opts))
+            .to_hash
+
+          @global = Entry::Global.new(@config)
+          @global.compose!
+        rescue Loader::FormatError, Extendable::ExtensionError => e
+          raise Config::ConfigError, e.message
+        end
+      rescue ::Gitlab::Ci::External::Processor::FileError => e
+        raise ::Gitlab::Ci::YamlProcessor::ValidationError, e.message
       end
 
       def valid?
@@ -64,6 +68,8 @@ module Gitlab
         @global.jobs_value
       end
 
+      private
+
       def build_config(config, opts = {})
         initial_config = Loader.new(config).load!
         project = opts.fetch(:project, nil)
@@ -76,8 +82,8 @@ module Gitlab
       end
 
       def process_external_files(config, project, opts)
-        sha = opts.fetch(:sha, project.repository.commit.sha)
-          ::Gitlab::Ci::External::Processor.new(config, project, sha).perform
+        sha = opts.fetch(:sha) { project.repository.root_ref_sha }
+        ::Gitlab::Ci::External::Processor.new(config, project, sha).perform
       end
     end
   end

lib/gitlab/ci/yaml_processor.rb

@@ -79,7 +79,7 @@ module Gitlab
         begin
           Gitlab::Ci::YamlProcessor.new(content, opts)
           nil
-        rescue ValidationError, ::Gitlab::Ci::ExternalFiles::Processor::ExternalFileError => e
+        rescue ValidationError => e
           e.message
         end
       end

spec/lib/gitlab/ci/config_spec.rb

@@ -3,13 +3,12 @@ require 'fast_spec_helper'
 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, sha: '12345' })
+    described_class.new(yml)
   end
 
   context 'when config is valid' do
-    let(:gitlab_ci_yml) do
+    let(:yml) do
       <<-EOS
         image: ruby:2.2
 
@@ -126,7 +125,11 @@ describe Gitlab::Ci::Config do
     end
   end
 
-  context "when gitlab_ci_yml has valid 'include' defined" do
+  context "when using 'include' directive" do
+    let(:project) { create(:project, :repository) }
+    let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
+    let(:local_location) { 'ee/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml' }
+
     let(:remote_file_content) do
       <<~HEREDOC
       variables:
@@ -137,83 +140,221 @@ describe Gitlab::Ci::Config do
         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(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
+
+    let(:local_file_content) do
+      File.read(Rails.root.join(local_location))
+    end
+
     let(:gitlab_ci_yml) do
-      <<-EOS
-        include:
-          - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml
-          - #{remote_location}
+      <<~HEREDOC
+      include:
+        - #{local_location}
+        - #{remote_location}
 
-        image: ruby:2.2
-      EOS
+      image: ruby:2.2
+      HEREDOC
+    end
+
+    let(:config) do
+      described_class.new(gitlab_ci_yml, project: project, sha: '12345')
     end
 
     before do
-      allow_any_instance_of(::Gitlab::Ci::External::File::Local).to receive(:local_file_content).and_return(local_file_content)
-      WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content)
-    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
-      }
+      WebMock.stub_request(:get, remote_location)
+        .to_return(body: remote_file_content)
 
-      expect(config.to_hash).to eq(composed_hash)
+      allow(project.repository)
+        .to receive(:blob_data_at).and_return(local_file_content)
     end
-  end
 
-  context "when gitlab_ci.yml has invalid 'include' defined"  do
-    let(:gitlab_ci_yml) do
-      <<-EOS
-      include: invalid
-      EOS
+    context "when gitlab_ci_yml has valid 'include' defined" do
+      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
 
-    it 'raises error YamlProcessor ValidationError' do
-      expect { config }.to raise_error(
-        ::Gitlab::Ci::YamlProcessor::ValidationError,
-        "External file: 'invalid' should be a valid local or remote file"
-      )
+    context "when gitlab_ci.yml has invalid 'include' defined"  do
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
+          include: invalid
+        HEREDOC
+      end
+
+      it 'raises error YamlProcessor validationError' do
+        expect { config }.to raise_error(
+          ::Gitlab::Ci::YamlProcessor::ValidationError,
+          "Local file 'invalid' is not valid."
+        )
+      end
     end
-  end
 
-  context "when both external files and gitlab_ci.yml defined the same key" do
-    let(:remote_location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
-    let(:gitlab_ci_yml) do
-      <<~HEREDOC
+    describe 'external file version' do
+      context 'when external local file SHA is defined' do
+        it 'is using a defined value' do
+          expect(project.repository).to receive(:blob_data_at)
+            .with('eeff1122', local_location)
+
+          described_class.new(gitlab_ci_yml, project: project, sha: 'eeff1122')
+        end
+      end
+
+      context 'when external local file SHA is not defined' do
+        it 'is using latest SHA on the default branch' do
+          expect(project.repository).to receive(:root_ref_sha)
+
+          described_class.new(gitlab_ci_yml, project: project)
+        end
+      end
+    end
+
+    context "when both external files and gitlab_ci.yml defined the same key" do
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
         include:
           - #{remote_location}
 
         image: ruby:2.2
-      HEREDOC
-    end
+        HEREDOC
+      end
 
-    let(:remote_file_content) do
-      <<~HEREDOC
+      let(:remote_file_content) do
+        <<~HEREDOC
         image: php:5-fpm-alpine
-      HEREDOC
+        HEREDOC
+      end
+
+      it 'should take precedence' do
+        expect(config.to_hash).to eq({ image: 'ruby:2.2' })
+      end
+    end
+
+    context "when both external files and gitlab_ci.yml define a dictionary of distinct variables" do
+      let(:remote_file_content) do
+        <<~HEREDOC
+        variables:
+          A: 'alpha'
+          B: 'beta'
+        HEREDOC
+      end
+
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
+        include:
+          - #{remote_location}
+
+        variables:
+          C: 'gamma'
+          D: 'delta'
+        HEREDOC
+      end
+
+      it 'should merge the variables dictionaries' do
+        expect(config.to_hash).to eq({ variables: { A: 'alpha', B: 'beta', C: 'gamma', D: 'delta' } })
+      end
+    end
+
+    context "when both external files and gitlab_ci.yml define a dictionary of overlapping variables" do
+      let(:remote_file_content) do
+        <<~HEREDOC
+        variables:
+          A: 'alpha'
+          B: 'beta'
+          C: 'omnicron'
+        HEREDOC
+      end
+
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
+        include:
+          - #{remote_location}
+
+        variables:
+          C: 'gamma'
+          D: 'delta'
+        HEREDOC
+      end
+
+      it 'later declarations should take precedence' do
+        expect(config.to_hash).to eq({ variables: { A: 'alpha', B: 'beta', C: 'gamma', D: 'delta' } })
+      end
     end
 
-    it 'should take precedence' do
-      WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content)
-      expect(config.to_hash).to eq({ image: 'ruby:2.2' })
+    context 'when both external files and gitlab_ci.yml define a job' do
+      let(:remote_file_content) do
+        <<~HEREDOC
+        job1:
+          script:
+          - echo 'hello from remote file'
+        HEREDOC
+      end
+
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
+        include:
+          - #{remote_location}
+
+        job1:
+          variables:
+            VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+        HEREDOC
+      end
+
+      it 'merges the jobs' do
+        expect(config.to_hash).to eq({
+          job1: {
+            script: ["echo 'hello from remote file'"],
+            variables: {
+              VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+            }
+          }
+        })
+      end
+
+      context 'when the script key is in both' do
+        let(:gitlab_ci_yml) do
+          <<~HEREDOC
+          include:
+            - #{remote_location}
+
+          job1:
+            script:
+            - echo 'hello from main file'
+            variables:
+              VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+          HEREDOC
+        end
+
+        it 'uses the script from the gitlab_ci.yml' do
+          expect(config.to_hash).to eq({
+            job1: {
+              script: ["echo 'hello from main file'"],
+              variables: {
+                VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+              }
+            }
+          })
+        end
+      end
     end
   end
 end