Commit 14f06891 authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Alejandro Rodríguez

Merge branch 'fix/require-build-script-configuration-entry' into 'master'

Make job script a required configuration entry

## What does this MR do?

This MR makes a job script a required configuration entry.

## Does this MR meet the acceptance criteria?

- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing

## What are the relevant issue numbers?

Closes #24575

See merge request !7566
parent 8535196f
---
title: Make job script a required configuration entry
merge_request: 7566
author:
......@@ -4,12 +4,6 @@ module Gitlab
# Base GitLab CI Configuration facade
#
class Config
##
# Temporary delegations that should be removed after refactoring
#
delegate :before_script, :image, :services, :after_script, :variables,
:stages, :cache, :jobs, to: :@global
def initialize(config)
@config = Loader.new(config).load!
......@@ -28,6 +22,41 @@ module Gitlab
def to_hash
@config
end
##
# Temporary method that should be removed after refactoring
#
def before_script
@global.before_script_value
end
def image
@global.image_value
end
def services
@global.services_value
end
def after_script
@global.after_script_value
end
def variables
@global.variables_value
end
def stages
@global.stages_value
end
def cache
@global.cache_value
end
def jobs
@global.jobs_value
end
end
end
end
......@@ -66,8 +66,6 @@ module Gitlab
@entries[symbol].value
end
alias_method symbol.to_sym, "#{symbol}_value".to_sym
end
end
end
......
......@@ -13,12 +13,10 @@ module Gitlab
type stage when artifacts cache dependencies before_script
after_script variables environment]
attributes :tags, :allow_failure, :when, :dependencies
validations do
validates :config, allowed_keys: ALLOWED_KEYS
validates :config, presence: true
validates :script, presence: true
validates :name, presence: true
validates :name, type: Symbol
......@@ -77,6 +75,8 @@ module Gitlab
:cache, :image, :services, :only, :except, :variables,
:artifacts, :commands, :environment
attributes :script, :tags, :allow_failure, :when, :dependencies
def compose!(deps = nil)
super do
if type_defined? && !stage_defined?
......@@ -118,20 +118,20 @@ module Gitlab
def to_hash
{ name: name,
before_script: before_script,
script: script,
before_script: before_script_value,
script: script_value,
commands: commands,
image: image,
services: services,
stage: stage,
cache: cache,
only: only,
except: except,
variables: variables_defined? ? variables : nil,
environment: environment_defined? ? environment : nil,
environment_name: environment_defined? ? environment[:name] : nil,
artifacts: artifacts,
after_script: after_script }
image: image_value,
services: services_value,
stage: stage_value,
cache: cache_value,
only: only_value,
except: except_value,
variables: variables_defined? ? variables_value : nil,
environment: environment_defined? ? environment_value : nil,
environment_name: environment_defined? ? environment_value[:name] : nil,
artifacts: artifacts_value,
after_script: after_script_value }
end
end
end
......
......@@ -1124,8 +1124,8 @@ EOT
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:extra config should be a hash")
end
it "returns errors if there are unknown parameters that are hashes, but doesn't have a script" do
config = YAML.dump({ extra: { services: "test" } })
it "returns errors if services configuration is not correct" do
config = YAML.dump({ extra: { script: 'rspec', services: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:extra:services config should be an array of strings")
......
......@@ -60,9 +60,9 @@ describe Gitlab::Ci::Config::Entry::Global do
end
context 'when not composed' do
describe '#before_script' do
describe '#before_script_value' do
it 'returns nil' do
expect(global.before_script).to be nil
expect(global.before_script_value).to be nil
end
end
......@@ -82,40 +82,40 @@ describe Gitlab::Ci::Config::Entry::Global do
end
end
describe '#before_script' do
describe '#before_script_value' do
it 'returns correct script' do
expect(global.before_script).to eq ['ls', 'pwd']
expect(global.before_script_value).to eq ['ls', 'pwd']
end
end
describe '#image' do
describe '#image_value' do
it 'returns valid image' do
expect(global.image).to eq 'ruby:2.2'
expect(global.image_value).to eq 'ruby:2.2'
end
end
describe '#services' do
describe '#services_value' do
it 'returns array of services' do
expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
expect(global.services_value).to eq ['postgres:9.1', 'mysql:5.5']
end
end
describe '#after_script' do
describe '#after_script_value' do
it 'returns after script' do
expect(global.after_script).to eq ['make clean']
expect(global.after_script_value).to eq ['make clean']
end
end
describe '#variables' do
describe '#variables_value' do
it 'returns variables' do
expect(global.variables).to eq(VAR: 'value')
expect(global.variables_value).to eq(VAR: 'value')
end
end
describe '#stages' do
describe '#stages_value' do
context 'when stages key defined' do
it 'returns array of stages' do
expect(global.stages).to eq %w[build pages]
expect(global.stages_value).to eq %w[build pages]
end
end
......@@ -126,21 +126,21 @@ describe Gitlab::Ci::Config::Entry::Global do
end
it 'returns array of types as stages' do
expect(global.stages).to eq %w[test deploy]
expect(global.stages_value).to eq %w[test deploy]
end
end
end
describe '#cache' do
describe '#cache_value' do
it 'returns cache configuration' do
expect(global.cache)
expect(global.cache_value)
.to eq(key: 'k', untracked: true, paths: ['public/'])
end
end
describe '#jobs' do
describe '#jobs_value' do
it 'returns jobs configuration' do
expect(global.jobs).to eq(
expect(global.jobs_value).to eq(
rspec: { name: :rspec,
script: %w[rspec ls],
before_script: ['ls', 'pwd'],
......@@ -185,21 +185,21 @@ describe Gitlab::Ci::Config::Entry::Global do
end
end
describe '#variables' do
describe '#variables_value' do
it 'returns default value for variables' do
expect(global.variables).to eq({})
expect(global.variables_value).to eq({})
end
end
describe '#stages' do
describe '#stages_value' do
it 'returns an array of default stages' do
expect(global.stages).to eq %w[build test deploy]
expect(global.stages_value).to eq %w[build test deploy]
end
end
describe '#cache' do
describe '#cache_value' do
it 'returns correct cache definition' do
expect(global.cache).to eq(key: 'a')
expect(global.cache_value).to eq(key: 'a')
end
end
end
......@@ -217,9 +217,9 @@ describe Gitlab::Ci::Config::Entry::Global do
{ variables: nil, rspec: { script: 'rspec' } }
end
describe '#variables' do
describe '#variables_value' do
it 'undefined entry returns a default value' do
expect(global.variables).to eq({})
expect(global.variables_value).to eq({})
end
end
end
......@@ -245,9 +245,9 @@ describe Gitlab::Ci::Config::Entry::Global do
end
end
describe '#before_script' do
describe '#before_script_value' do
it 'returns nil' do
expect(global.before_script).to be_nil
expect(global.before_script_value).to be_nil
end
end
end
......
......@@ -19,8 +19,7 @@ describe Gitlab::Ci::Config::Entry::Job do
let(:entry) { described_class.new(config, name: ''.to_sym) }
it 'reports error' do
expect(entry.errors)
.to include "job name can't be blank"
expect(entry.errors).to include "job name can't be blank"
end
end
end
......@@ -56,6 +55,15 @@ describe Gitlab::Ci::Config::Entry::Job do
end
end
end
context 'when script is not provided' do
let(:config) { { stage: 'test' } }
it 'returns error about missing script entry' do
expect(entry).not_to be_valid
expect(entry.errors).to include "job script can't be blank"
end
end
end
end
......@@ -78,7 +86,7 @@ describe Gitlab::Ci::Config::Entry::Job do
before { entry.compose!(deps) }
let(:config) do
{ image: 'some_image', cache: { key: 'test' } }
{ script: 'rspec', image: 'some_image', cache: { key: 'test' } }
end
it 'overrides global config' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment