Commit 0165cfaa authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Grzegorz Bizon

Re-define default only except policy

parent 6b68d82f
---
title: Define the default value for only/except policies
merge_request: 23531
merge_request: 23765
author:
type: changed
# frozen_string_literal: true
module Gitlab
module Ci
class Config
module Entry
##
# Entry that represents an only/except trigger policy for the job.
#
class ExceptPolicy < Policy
def self.default
end
end
end
end
end
end
......@@ -16,6 +16,13 @@ module Gitlab
dependencies before_script after_script variables
environment coverage retry parallel extends].freeze
DEFAULT_ONLY_POLICY = {
refs: %w(branches tags)
}.freeze
DEFAULT_EXCEPT_POLICY = {
}.freeze
validations do
validates :config, allowed_keys: ALLOWED_KEYS
validates :config, presence: true
......@@ -65,10 +72,10 @@ module Gitlab
entry :services, Entry::Services,
description: 'Services that will be used to execute this job.'
entry :only, Entry::OnlyPolicy,
entry :only, Entry::Policy,
description: 'Refs policy this job will be executed for.'
entry :except, Entry::ExceptPolicy,
entry :except, Entry::Policy,
description: 'Refs policy this job will be executed for.'
entry :variables, Entry::Variables,
......@@ -154,8 +161,8 @@ module Gitlab
services: services_value,
stage: stage_value,
cache: cache_value,
only: only_value,
except: except_value,
only: DEFAULT_ONLY_POLICY.deep_merge(only_value.to_h),
except: DEFAULT_EXCEPT_POLICY.deep_merge(except_value.to_h),
variables: variables_defined? ? variables_value : nil,
environment: environment_defined? ? environment_value : nil,
environment_name: environment_defined? ? environment_value[:name] : nil,
......
# frozen_string_literal: true
module Gitlab
module Ci
class Config
module Entry
##
# Entry that represents an only/except trigger policy for the job.
#
class OnlyPolicy < Policy
def self.default
{ refs: %w[branches tags] }
end
end
end
end
end
end
......@@ -5,9 +5,12 @@ module Gitlab
class Config
module Entry
##
# Base class for OnlyPolicy and ExceptPolicy
# Entry that represents an only/except trigger policy for the job.
#
class Policy < ::Gitlab::Config::Entry::Simplifiable
strategy :RefsPolicy, if: -> (config) { config.is_a?(Array) }
strategy :ComplexPolicy, if: -> (config) { config.is_a?(Hash) }
class RefsPolicy < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
......@@ -63,16 +66,6 @@ module Gitlab
def self.default
end
##
# Class-level execution won't be inherited by subclasses by default.
# Therefore, we need to explicitly execute that for OnlyPolicy and ExceptPolicy
def self.inherited(klass)
super
klass.strategy :RefsPolicy, if: -> (config) { config.is_a?(Array) }
klass.strategy :ComplexPolicy, if: -> (config) { config.is_a?(Hash) }
end
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Ci::Config::Entry::ExceptPolicy do
let(:entry) { described_class.new(config) }
it_behaves_like 'correct only except policy'
describe '.default' do
it 'does not have a default value' do
expect(described_class.default).to be_nil
end
end
end
......@@ -161,7 +161,8 @@ describe Gitlab::Ci::Config::Entry::Global do
variables: { 'VAR' => 'value' },
ignore: false,
after_script: ['make clean'],
only: { refs: %w[branches tags] } },
only: { refs: %w[branches tags] },
except: {} },
spinach: { name: :spinach,
before_script: [],
script: %w[spinach],
......@@ -173,7 +174,8 @@ describe Gitlab::Ci::Config::Entry::Global do
variables: {},
ignore: false,
after_script: ['make clean'],
only: { refs: %w[branches tags] } }
only: { refs: %w[branches tags] },
except: {} }
)
end
end
......
......@@ -259,7 +259,8 @@ describe Gitlab::Ci::Config::Entry::Job do
stage: 'test',
ignore: false,
after_script: %w[cleanup],
only: { refs: %w[branches tags] })
only: { refs: %w[branches tags] },
except: {})
end
end
end
......
......@@ -68,13 +68,15 @@ describe Gitlab::Ci::Config::Entry::Jobs do
commands: 'rspec',
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] } },
only: { refs: %w[branches tags] },
except: {} },
spinach: { name: :spinach,
script: %w[spinach],
commands: 'spinach',
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] } })
only: { refs: %w[branches tags] },
except: {} })
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Ci::Config::Entry::OnlyPolicy do
let(:entry) { described_class.new(config) }
it_behaves_like 'correct only except policy'
describe '.default' do
it 'haa a default value' do
expect(described_class.default).to eq( { refs: %w[branches tags] } )
end
end
end
require 'spec_helper'
require 'fast_spec_helper'
require_dependency 'active_model'
describe Gitlab::Ci::Config::Entry::Policy do
let(:entry) { described_class.new(config) }
context 'when using simplified policy' do
describe 'validations' do
context 'when entry config value is valid' do
context 'when config is a branch or tag name' do
let(:config) { %w[master feature/branch] }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
describe '#value' do
it 'returns refs hash' do
expect(entry.value).to eq(refs: config)
end
end
end
context 'when config is a regexp' do
let(:config) { ['/^issue-.*$/'] }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
end
context 'when config is a special keyword' do
let(:config) { %w[tags triggers branches] }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
end
end
context 'when entry value is not valid' do
let(:config) { [1] }
describe '#errors' do
it 'saves errors' do
expect(entry.errors)
.to include /policy config should be an array of strings or regexps/
end
end
end
end
end
context 'when using complex policy' do
context 'when specifying refs policy' do
let(:config) { { refs: ['master'] } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(refs: %w[master])
end
end
context 'when specifying kubernetes policy' do
let(:config) { { kubernetes: 'active' } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(kubernetes: 'active')
end
end
context 'when specifying invalid kubernetes policy' do
let(:config) { { kubernetes: 'something' } }
it 'reports an error about invalid policy' do
expect(entry.errors).to include /unknown value: something/
end
end
context 'when specifying valid variables expressions policy' do
let(:config) { { variables: ['$VAR == null'] } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(config)
end
end
context 'when specifying variables expressions in invalid format' do
let(:config) { { variables: '$MY_VAR' } }
it 'reports an error about invalid format' do
expect(entry.errors).to include /should be an array of strings/
end
end
context 'when specifying invalid variables expressions statement' do
let(:config) { { variables: ['$MY_VAR =='] } }
it 'reports an error about invalid statement' do
expect(entry.errors).to include /invalid expression syntax/
end
end
context 'when specifying invalid variables expressions token' do
let(:config) { { variables: ['$MY_VAR == 123'] } }
it 'reports an error about invalid expression' do
expect(entry.errors).to include /invalid expression syntax/
end
end
context 'when using invalid variables expressions regexp' do
let(:config) { { variables: ['$MY_VAR =~ /some ( thing/'] } }
it 'reports an error about invalid expression' do
expect(entry.errors).to include /invalid expression syntax/
end
end
context 'when specifying a valid changes policy' do
let(:config) { { changes: %w[some/* paths/**/*.rb] } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(config)
end
end
context 'when changes policy is invalid' do
let(:config) { { changes: [1, 2] } }
it 'returns errors' do
expect(entry.errors).to include /changes should be an array of strings/
end
end
context 'when specifying unknown policy' do
let(:config) { { refs: ['master'], invalid: :something } }
it 'returns error about invalid key' do
expect(entry.errors).to include /unknown keys: invalid/
end
end
context 'when policy is empty' do
let(:config) { {} }
it 'is not a valid configuration' do
expect(entry.errors).to include /can't be blank/
end
end
end
context 'when policy strategy does not match' do
let(:config) { 'string strategy' }
it 'returns information about errors' do
expect(entry.errors)
.to include /has to be either an array of conditions or a hash/
end
end
describe '.default' do
it 'does not have a default value' do
expect(described_class.default).to be_nil
......
......@@ -840,6 +840,37 @@ describe Ci::CreatePipelineService do
end
end
context "when config uses variables for only keyword" do
let(:config) do
{
build: {
stage: 'build',
script: 'echo',
only: {
variables: %w($CI)
}
}
}
end
context 'when merge request is specified' do
let(:merge_request) do
create(:merge_request,
source_project: project,
source_branch: ref_name,
target_project: project,
target_branch: 'master')
end
it 'does not create a merge request pipeline' do
expect(pipeline).not_to be_persisted
expect(pipeline.errors[:base])
.to eq(['No stages / jobs for this pipeline.'])
end
end
end
context "when config has 'except: [tags]'" do
let(:config) do
{
......
# frozen_string_literal: true
shared_examples 'correct only except policy' do
context 'when using simplified policy' do
describe 'validations' do
context 'when entry config value is valid' do
context 'when config is a branch or tag name' do
let(:config) { %w[master feature/branch] }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
describe '#value' do
it 'returns refs hash' do
expect(entry.value).to eq(refs: config)
end
end
end
context 'when config is a regexp' do
let(:config) { ['/^issue-.*$/'] }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
end
context 'when config is a special keyword' do
let(:config) { %w[tags triggers branches] }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
end
end
context 'when entry value is not valid' do
let(:config) { [1] }
describe '#errors' do
it 'saves errors' do
expect(entry.errors)
.to include /policy config should be an array of strings or regexps/
end
end
end
end
end
context 'when using complex policy' do
context 'when specifying refs policy' do
let(:config) { { refs: ['master'] } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(refs: %w[master])
end
end
context 'when specifying kubernetes policy' do
let(:config) { { kubernetes: 'active' } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(kubernetes: 'active')
end
end
context 'when specifying invalid kubernetes policy' do
let(:config) { { kubernetes: 'something' } }
it 'reports an error about invalid policy' do
expect(entry.errors).to include /unknown value: something/
end
end
context 'when specifying valid variables expressions policy' do
let(:config) { { variables: ['$VAR == null'] } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(config)
end
end
context 'when specifying variables expressions in invalid format' do
let(:config) { { variables: '$MY_VAR' } }
it 'reports an error about invalid format' do
expect(entry.errors).to include /should be an array of strings/
end
end
context 'when specifying invalid variables expressions statement' do
let(:config) { { variables: ['$MY_VAR =='] } }
it 'reports an error about invalid statement' do
expect(entry.errors).to include /invalid expression syntax/
end
end
context 'when specifying invalid variables expressions token' do
let(:config) { { variables: ['$MY_VAR == 123'] } }
it 'reports an error about invalid expression' do
expect(entry.errors).to include /invalid expression syntax/
end
end
context 'when using invalid variables expressions regexp' do
let(:config) { { variables: ['$MY_VAR =~ /some ( thing/'] } }
it 'reports an error about invalid expression' do
expect(entry.errors).to include /invalid expression syntax/
end
end
context 'when specifying a valid changes policy' do
let(:config) { { changes: %w[some/* paths/**/*.rb] } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(config)
end
end
context 'when changes policy is invalid' do
let(:config) { { changes: [1, 2] } }
it 'returns errors' do
expect(entry.errors).to include /changes should be an array of strings/
end
end
context 'when specifying unknown policy' do
let(:config) { { refs: ['master'], invalid: :something } }
it 'returns error about invalid key' do
expect(entry.errors).to include /unknown keys: invalid/
end
end
context 'when policy is empty' do
let(:config) { {} }
it 'is not a valid configuration' do
expect(entry.errors).to include /can't be blank/
end
end
end
context 'when policy strategy does not match' do
let(:config) { 'string strategy' }
it 'returns information about errors' do
expect(entry.errors)
.to include /has to be either an array of conditions or a hash/
end
end
end
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