diff --git a/app/graphql/types/ci/config_variable_type.rb b/app/graphql/types/ci/config_variable_type.rb
index 5b5890fd5a56c806618c3ec8a632aaf260d77011..020af5b2444625a2d6431dd8a595c17e61e02c45 100644
--- a/app/graphql/types/ci/config_variable_type.rb
+++ b/app/graphql/types/ci/config_variable_type.rb
@@ -19,6 +19,7 @@ class ConfigVariableType < BaseObject # rubocop:disable Graphql/AuthorizeTypes
description: 'Value of the variable.'
field :value_options, [GraphQL::Types::String],
+ hash_key: :options,
null: true,
description: 'Value options for the variable.'
end
diff --git a/lib/gitlab/ci/config/entry/root.rb b/lib/gitlab/ci/config/entry/root.rb
index 1e6dbb22209bd084285a1b5e16ee9f072da1ec5b..a3d57ab6ac6b12f07e15dbb7e6ab416062fb91a8 100644
--- a/lib/gitlab/ci/config/entry/root.rb
+++ b/lib/gitlab/ci/config/entry/root.rb
@@ -50,7 +50,7 @@ class Root < ::Gitlab::Config::Entry::Node
entry :variables, Entry::Variables,
description: 'Environment variables that will be used.',
- metadata: { allowed_value_data: %i[value description expand], allow_array_value: true },
+ metadata: { allowed_value_data: %i[value description expand options] },
reserved: true
entry :stages, Entry::Stages,
diff --git a/lib/gitlab/ci/config/entry/variable.rb b/lib/gitlab/ci/config/entry/variable.rb
index 16091758916e26b121d10847cec69c6b9a17646f..decb568ffc920efc4b14eacb31d836f5fe7abd32 100644
--- a/lib/gitlab/ci/config/entry/variable.rb
+++ b/lib/gitlab/ci/config/entry/variable.rb
@@ -10,7 +10,6 @@ module Entry
class Variable < ::Gitlab::Config::Entry::Simplifiable
strategy :SimpleVariable, if: -> (config) { SimpleVariable.applies_to?(config) }
strategy :ComplexVariable, if: -> (config) { ComplexVariable.applies_to?(config) }
- strategy :ComplexArrayVariable, if: -> (config) { ComplexArrayVariable.applies_to?(config) }
class SimpleVariable < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
@@ -41,20 +40,24 @@ def value_with_prefill_data
class ComplexVariable < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
+ include ::Gitlab::Config::Entry::Attributable
class << self
def applies_to?(config)
- config.is_a?(Hash) && !config[:value].is_a?(Array)
+ config.is_a?(Hash)
end
end
+ attributes :value, :description, :expand, :options, prefix: :config
+
validations do
validates :key, alphanumeric: true
- validates :config_value, alphanumeric: true, allow_nil: false, if: :config_value_defined?
- validates :config_description, alphanumeric: true, allow_nil: false, if: :config_description_defined?
- validates :config_expand, boolean: true,
- allow_nil: false,
- if: -> { ci_raw_variables_in_yaml_config_enabled? && config_expand_defined? }
+ validates :config_value, alphanumeric: true, allow_nil: true
+ validates :config_description, alphanumeric: true, allow_nil: true
+ validates :config_expand, boolean: true, allow_nil: true, if: -> {
+ ci_raw_variables_in_yaml_config_enabled?
+ }
+ validates :config_options, array_of_strings: true, allow_nil: true
validate do
allowed_value_data = Array(opt(:allowed_value_data))
@@ -66,91 +69,43 @@ def applies_to?(config)
else
errors.add(:config, "must be a string")
end
+
+ if config_options.present? && config_options.exclude?(config_value)
+ errors.add(:config, 'value must be present in options')
+ end
end
end
def value
+ # Needed since the `Entry::Node` provides `value` (which is current hash)
config_value.to_s
end
def value_with_data
if ci_raw_variables_in_yaml_config_enabled?
{
- value: value,
- raw: (!config_expand if config_expand_defined?)
+ value: config_value.to_s,
+ raw: (!config_expand if has_config_expand?)
}.compact
else
{
- value: value
+ value: config_value.to_s
}.compact
end
end
def value_with_prefill_data
value_with_data.merge(
- description: config_description
+ description: config_description,
+ options: config_options
).compact
end
- def config_value
- @config[:value]
- end
-
- def config_description
- @config[:description]
- end
-
- def config_expand
- @config[:expand]
- end
-
- def config_value_defined?
- config.key?(:value)
- end
-
- def config_description_defined?
- config.key?(:description)
- end
-
- def config_expand_defined?
- config.key?(:expand)
- end
-
def ci_raw_variables_in_yaml_config_enabled?
YamlProcessor::FeatureFlags.enabled?(:ci_raw_variables_in_yaml_config)
end
end
- class ComplexArrayVariable < ComplexVariable
- include ::Gitlab::Config::Entry::Validatable
-
- class << self
- def applies_to?(config)
- config.is_a?(Hash) && config[:value].is_a?(Array)
- end
- end
-
- validations do
- validates :config_value, array_of_strings: true, allow_nil: false, if: :config_value_defined?
-
- validate do
- next if opt(:allow_array_value)
-
- errors.add(:config, 'value must be an alphanumeric string')
- end
- end
-
- def value
- config_value.first
- end
-
- def value_with_prefill_data
- super.merge(
- value_options: config_value
- ).compact
- end
- end
-
class UnknownStrategy < ::Gitlab::Config::Entry::Node
def errors
["variable definition must be either a string or a hash"]
diff --git a/lib/gitlab/ci/config/entry/variables.rb b/lib/gitlab/ci/config/entry/variables.rb
index ef4f74b9f56e0ff25a3e01e190728fe9a8627e96..e338bce3109a92d56f062ffa301627ba5b88677e 100644
--- a/lib/gitlab/ci/config/entry/variables.rb
+++ b/lib/gitlab/ci/config/entry/variables.rb
@@ -42,7 +42,7 @@ def composable_class(_name, _config)
end
def composable_metadata
- { allowed_value_data: opt(:allowed_value_data), allow_array_value: opt(:allow_array_value) }
+ { allowed_value_data: opt(:allowed_value_data) }
end
end
end
diff --git a/lib/gitlab/config/entry/attributable.rb b/lib/gitlab/config/entry/attributable.rb
index d266d5218de06ce86286e2fead4c0555dc5f07e6..c8ad25215748c2f6f16bd31b95b8b0e65f9bac41 100644
--- a/lib/gitlab/config/entry/attributable.rb
+++ b/lib/gitlab/config/entry/attributable.rb
@@ -7,19 +7,21 @@ module Attributable
extend ActiveSupport::Concern
class_methods do
- def attributes(*attributes)
+ def attributes(*attributes, prefix: nil)
attributes.flatten.each do |attribute|
- if method_defined?(attribute)
- raise ArgumentError, "Method '#{attribute}' already defined in '#{name}'"
+ attribute_method = prefix ? "#{prefix}_#{attribute}" : attribute
+
+ if method_defined?(attribute_method)
+ raise ArgumentError, "Method '#{attribute_method}' already defined in '#{name}'"
end
- define_method(attribute) do
+ define_method(attribute_method) do
return unless config.is_a?(Hash)
config[attribute]
end
- define_method("has_#{attribute}?") do
+ define_method("has_#{attribute_method}?") do
config.is_a?(Hash) && config.key?(attribute)
end
end
diff --git a/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js b/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js
index 3e699b93fd3ba52ce0edadcb2142ee3d6ccdaab7..2360dd7d103c827a7cc6fd3b44a6bf449eec2665 100644
--- a/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js
+++ b/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js
@@ -295,11 +295,11 @@ describe('Pipeline New Form', () => {
expect(dropdownItems.at(2).text()).toBe(valueOptions[2]);
});
- it('variables with multiple predefined values sets the first option as the default', () => {
+ it('variable with multiple predefined values sets value as the default', () => {
const dropdown = findValueDropdowns().at(0);
const { valueOptions } = mockYamlVariables[2];
- expect(dropdown.props('text')).toBe(valueOptions[0]);
+ expect(dropdown.props('text')).toBe(valueOptions[1]);
});
});
diff --git a/spec/frontend/pipeline_new/mock_data.js b/spec/frontend/pipeline_new/mock_data.js
index e95a65171fcf3001016e1d3683c67641db6c8422..2af0ef4d7c46aa2b15ceb0d157615c3f3522617f 100644
--- a/spec/frontend/pipeline_new/mock_data.js
+++ b/spec/frontend/pipeline_new/mock_data.js
@@ -83,7 +83,7 @@ export const mockYamlVariables = [
{
description: 'This is a variable with predefined values.',
key: 'VAR_WITH_OPTIONS',
- value: 'development',
+ value: 'staging',
valueOptions: ['development', 'staging', 'production'],
},
];
@@ -105,7 +105,7 @@ export const mockYamlVariablesWithoutDesc = [
{
description: null,
key: 'VAR_WITH_OPTIONS',
- value: 'development',
+ value: 'staging',
valueOptions: ['development', 'staging', 'production'],
},
];
diff --git a/spec/lib/gitlab/ci/config/entry/root_spec.rb b/spec/lib/gitlab/ci/config/entry/root_spec.rb
index b9a705a4e1f5e54edf69075e41bdbc7286cd9c8f..c40589104cd0aaf2d100637f4d35710576710392 100644
--- a/spec/lib/gitlab/ci/config/entry/root_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/root_spec.rb
@@ -38,7 +38,7 @@
variables: {
VAR: 'root',
VAR2: { value: 'val 2', description: 'this is var 2' },
- VAR3: { value: %w[val3 val3b], description: 'this is var 3' }
+ VAR3: { value: 'val3', options: %w[val3 val4 val5], description: 'this is var 3 and some options' }
},
after_script: ['make clean'],
stages: %w(build pages release),
@@ -326,6 +326,42 @@
end
end
+ context 'when variables have `options` data' do
+ before do
+ root.compose!
+ end
+
+ context 'and the value is in the `options` array' do
+ let(:hash) do
+ {
+ variables: { 'VAR' => { value: 'val1', options: %w[val1 val2] } },
+ rspec: { script: 'bin/rspec' }
+ }
+ end
+
+ it 'returns correct value' do
+ expect(root.variables_entry.value_with_data).to eq(
+ 'VAR' => { value: 'val1' }
+ )
+
+ expect(root.variables_value).to eq('VAR' => 'val1')
+ end
+ end
+
+ context 'and the value is not in the `options` array' do
+ let(:hash) do
+ {
+ variables: { 'VAR' => { value: 'val', options: %w[val1 val2] } },
+ rspec: { script: 'bin/rspec' }
+ }
+ end
+
+ it 'returns an error' do
+ expect(root.errors).to contain_exactly('variables:var config value must be present in options')
+ end
+ end
+ end
+
context 'when variables have "expand" data' do
let(:hash) do
{
diff --git a/spec/lib/gitlab/ci/config/entry/variable_spec.rb b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
index d7023072312fc69813d993df8469ed03418db6be..97b06c8b1a55653be1cbec964753afdf310f2ad5 100644
--- a/spec/lib/gitlab/ci/config/entry/variable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
@@ -306,48 +306,48 @@
end
end
end
- end
- describe 'ComplexArrayVariable' do
- context 'when allow_array_value metadata is false' do
- let(:config) { { value: %w[value value2], description: 'description' } }
- let(:metadata) { { allow_array_value: false } }
+ context 'when config is a hash with options' do
+ context 'when there is no metadata' do
+ let(:config) { { value: 'value', options: %w[value value2] } }
+ let(:metadata) { {} }
- describe '#valid?' do
- it { is_expected.not_to be_valid }
- end
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
- describe '#errors' do
- subject(:errors) { entry.errors }
+ describe '#errors' do
+ subject(:errors) { entry.errors }
- it { is_expected.to include 'var1 config value must be an alphanumeric string' }
+ it { is_expected.to include 'var1 config must be a string' }
+ end
end
- end
- context 'when allow_array_value metadata is true' do
- let(:config) { { value: %w[value value2], description: 'description' } }
- let(:metadata) { { allowed_value_data: %i[value description], allow_array_value: true } }
+ context 'when options are allowed' do
+ let(:config) { { value: 'value', options: %w[value value2] } }
+ let(:metadata) { { allowed_value_data: %i[value options] } }
- describe '#valid?' do
- it { is_expected.to be_valid }
- end
+ describe '#valid?' do
+ it { is_expected.to be_valid }
+ end
- describe '#value' do
- subject(:value) { entry.value }
+ describe '#value' do
+ subject(:value) { entry.value }
- it { is_expected.to eq('value') }
- end
+ it { is_expected.to eq('value') }
+ end
- describe '#value_with_data' do
- subject(:value_with_data) { entry.value_with_data }
+ describe '#value_with_data' do
+ subject(:value_with_data) { entry.value_with_data }
- it { is_expected.to eq(value: 'value') }
- end
+ it { is_expected.to eq(value: 'value') }
+ end
- describe '#value_with_prefill_data' do
- subject(:value_with_prefill_data) { entry.value_with_prefill_data }
+ describe '#value_with_prefill_data' do
+ subject(:value_with_prefill_data) { entry.value_with_prefill_data }
- it { is_expected.to eq(value: 'value', description: 'description', value_options: %w[value value2]) }
+ it { is_expected.to eq(value: 'value', options: %w[value value2]) }
+ end
end
end
end
diff --git a/spec/lib/gitlab/ci/config/entry/variables_spec.rb b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
index 609e4422d5c18298e37a1c1dd944259d31109240..e7dbc78729db2078174c980ee83f54e0f7834d06 100644
--- a/spec/lib/gitlab/ci/config/entry/variables_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
@@ -116,8 +116,8 @@
it_behaves_like 'invalid config', /variable_1 config must be a string/
end
- context 'when metadata has allow_array_value and allowed_value_data' do
- let(:metadata) { { allowed_value_data: %i[value description], allow_array_value: true } }
+ context 'when metadata has the allowed_value_data key' do
+ let(:metadata) { { allowed_value_data: %i[value description options] } }
let(:result) do
{ 'VARIABLE_1' => 'value' }
@@ -143,17 +143,15 @@
end
end
- context 'when entry config value has key-value pair and value is an array' do
+ context 'when entry config value has options' do
let(:config) do
- { 'VARIABLE_1' => { value: %w[value1 value2], description: 'variable 1' } }
+ { 'VARIABLE_1' => {
+ value: 'value1', options: %w[value1 value2], description: 'variable 1'
+ } }
end
- context 'when there is no allowed_value_data metadata' do
- it_behaves_like 'invalid config', /variable_1 config value must be an alphanumeric string/
- end
-
- context 'when metadata has allow_array_value and allowed_value_data' do
- let(:metadata) { { allowed_value_data: %i[value description], allow_array_value: true } }
+ context 'when metadata has allowed_value_data' do
+ let(:metadata) { { allowed_value_data: %i[value description options] } }
let(:result) do
{ 'VARIABLE_1' => 'value1' }
@@ -172,7 +170,7 @@
describe '#value_with_prefill_data' do
it 'returns variable with prefill data' do
expect(entry.value_with_prefill_data).to eq(
- 'VARIABLE_1' => { value: 'value1', value_options: %w[value1 value2], description: 'variable 1' }
+ 'VARIABLE_1' => { value: 'value1', options: %w[value1 value2], description: 'variable 1' }
)
end
end
@@ -234,14 +232,6 @@
it_behaves_like 'invalid config', /variable_1 config uses invalid data keys: hello/
end
- context 'when entry config value has hash with nil description' do
- let(:config) do
- { 'VARIABLE_1' => { value: 'value 1', description: nil } }
- end
-
- it_behaves_like 'invalid config', /variable_1 config description must be an alphanumeric string/
- end
-
context 'when entry config value has hash without description' do
let(:config) do
{ 'VARIABLE_1' => { value: 'value 1' } }
diff --git a/spec/lib/gitlab/config/entry/attributable_spec.rb b/spec/lib/gitlab/config/entry/attributable_spec.rb
index 8a207bddaae43491ade2834eab41895aa889c583..0a2f8ac2c3a669bf1b7b16000482e566484c5aa2 100644
--- a/spec/lib/gitlab/config/entry/attributable_spec.rb
+++ b/spec/lib/gitlab/config/entry/attributable_spec.rb
@@ -10,10 +10,11 @@
end
let(:instance) { node.new }
+ let(:prefix) { nil }
before do
- node.class_eval do
- attributes :name, :test
+ node.class_exec(prefix) do |pre|
+ attributes :name, :test, prefix: pre
end
end
@@ -24,6 +25,17 @@
.and_return({ name: 'some name', test: 'some test' })
end
+ context 'and is provided a prefix' do
+ let(:prefix) { :pre }
+
+ it 'returns the value of config' do
+ expect(instance).to have_pre_name
+ expect(instance.pre_name).to eq 'some name'
+ expect(instance).to have_pre_test
+ expect(instance.pre_test).to eq 'some test'
+ end
+ end
+
it 'returns the value of config' do
expect(instance).to have_name
expect(instance.name).to eq 'some name'
diff --git a/spec/support/gitlab_stubs/gitlab_ci.yml b/spec/support/gitlab_stubs/gitlab_ci.yml
index 945235917654b3d1276e743318e78db054eef723..dbbfe044e35dda65743ed4e2a41e114fd6ea25ca 100644
--- a/spec/support/gitlab_stubs/gitlab_ci.yml
+++ b/spec/support/gitlab_stubs/gitlab_ci.yml
@@ -12,7 +12,8 @@ variables:
description: 'value of KEY_VALUE_VAR'
DB_NAME: postgres
ENVIRONMENT_VAR:
- value: ['env var value', 'env var value2']
+ value: 'env var value'
+ options: ['env var value', 'env var value2']
description: 'env var description'
stages: