diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 19d93c6e785..0ffd1791791 100644 --- a/lib/gitlab/ci/config.rb +++ b/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 + 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 + @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 diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 67f5d2d6ae4..5d1864ae9e2 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/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 diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 83a30de199f..baa4793e677 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/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(:gitlab_ci_yml) do - <<-EOS - include: - - /spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml - - #{remote_location} - image: ruby:2.2 - EOS + let(:local_file_content) do + File.read(Rails.root.join(local_location)) + end + + let(:gitlab_ci_yml) do + <<~HEREDOC + include: + - #{local_location} + - #{remote_location} + + 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) + WebMock.stub_request(:get, remote_location) + .to_return(body: remote_file_content) + + allow(project.repository) + .to receive(:blob_data_at).and_return(local_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 - } + 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 - - context "when gitlab_ci.yml has invalid 'include' defined" do - let(:gitlab_ci_yml) do - <<-EOS - include: invalid - EOS + 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" - ) - end - end + context "when gitlab_ci.yml has invalid 'include' defined" do + let(:gitlab_ci_yml) do + <<~HEREDOC + include: invalid + HEREDOC + 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 + it 'raises error YamlProcessor validationError' do + expect { config }.to raise_error( + ::Gitlab::Ci::YamlProcessor::ValidationError, + "Local file 'invalid' is not valid." + ) + end + end + + 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 - 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 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 + + 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