From aa18bbf581debbf8106ff50586fad8eb873d1349 Mon Sep 17 00:00:00 2001
From: Laura Montemayor <lmontemayor@gitlab.com>
Date: Fri, 16 Sep 2022 17:32:17 +0200
Subject: [PATCH 1/6] Updates Config::Entry::Variable value to support array

In order to support an array of values for a variable, we need to make
sure the entry class for variables supports an array of values, not just
a string.
---
 app/graphql/types/ci/config_variable_type.rb   | 10 +++++++++-
 doc/api/graphql/reference/index.md             |  2 +-
 lib/gitlab/ci/config/entry/variable.rb         |  8 ++++++--
 .../gitlab/ci/config/entry/variable_spec.rb    | 18 ++++++++++++------
 .../api/graphql/ci/config_variables_spec.rb    |  4 ++--
 spec/support/gitlab_stubs/gitlab_ci.yml        |  2 +-
 6 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/app/graphql/types/ci/config_variable_type.rb b/app/graphql/types/ci/config_variable_type.rb
index 87ae026c2c1daf..8a6836c2a052f2 100644
--- a/app/graphql/types/ci/config_variable_type.rb
+++ b/app/graphql/types/ci/config_variable_type.rb
@@ -14,9 +14,17 @@ class ConfigVariableType < BaseObject # rubocop:disable Graphql/AuthorizeTypes
             null: true,
             description: 'Description for the CI/CD config variable.'
 
-      field :value, GraphQL::Types::String,
+      field :value, [GraphQL::Types::String],
             null: true,
             description: 'Value of the variable.'
+
+      def value
+        if object[:value].is_a?(String)
+          Array.wrap(object[:value])
+        else
+          object[:value]
+        end
+      end
     end
   end
 end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index bd9905ca652a02..ee9b8b8be9b254 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -10404,7 +10404,7 @@ CI/CD config variables.
 | ---- | ---- | ----------- |
 | <a id="ciconfigvariabledescription"></a>`description` | [`String`](#string) | Description for the CI/CD config variable. |
 | <a id="ciconfigvariablekey"></a>`key` | [`String`](#string) | Name of the variable. |
-| <a id="ciconfigvariablevalue"></a>`value` | [`String`](#string) | Value of the variable. |
+| <a id="ciconfigvariablevalue"></a>`value` | [`[String!]`](#string) | Value of the variable. |
 
 ### `CiGroup`
 
diff --git a/lib/gitlab/ci/config/entry/variable.rb b/lib/gitlab/ci/config/entry/variable.rb
index 253888aadebeb3..c19dc4e329b619 100644
--- a/lib/gitlab/ci/config/entry/variable.rb
+++ b/lib/gitlab/ci/config/entry/variable.rb
@@ -45,7 +45,7 @@ def applies_to?(config)
 
             validations do
               validates :key, alphanumeric: true
-              validates :config_value, alphanumeric: true, allow_nil: false, if: :config_value_defined?
+              validates :config_value, array_of_strings_or_string: true, allow_nil: false, if: :config_value_defined?
               validates :config_description, alphanumeric: true, allow_nil: false, if: :config_description_defined?
 
               validate do
@@ -62,7 +62,11 @@ def applies_to?(config)
             end
 
             def value
-              config_value.to_s
+              if config_value.is_a?(Array)
+                config_value.map(&:to_s)
+              else
+                config_value.to_s
+              end
             end
 
             def value_with_data
diff --git a/spec/lib/gitlab/ci/config/entry/variable_spec.rb b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
index 744a89d4509510..e3716dc36b88ec 100644
--- a/spec/lib/gitlab/ci/config/entry/variable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
@@ -127,17 +127,23 @@
           end
         end
 
-        context 'when config value is an array' do
-          let(:config) { { value: ['value'], description: 'description' } }
+        context 'when config value is an array of values' do
+          let(:config) { { value: %w[value value2], description: 'description' } }
 
           describe '#valid?' do
-            it { is_expected.not_to be_valid }
+            it { is_expected.to be_valid }
           end
 
-          describe '#errors' do
-            subject(:errors) { entry.errors }
+          describe '#value' do
+            subject(:value) { entry.value }
+
+            it { is_expected.to eq(%w[value value2]) }
+          end
+
+          describe '#value_with_data' do
+            subject(:value_with_data) { entry.value_with_data }
 
-            it { is_expected.to include 'var1 config value must be an alphanumeric string' }
+            it { is_expected.to eq(value: %w[value value2], description: 'description') }
           end
         end
 
diff --git a/spec/requests/api/graphql/ci/config_variables_spec.rb b/spec/requests/api/graphql/ci/config_variables_spec.rb
index 2b5a5d0dc931c9..33f423ed944b6b 100644
--- a/spec/requests/api/graphql/ci/config_variables_spec.rb
+++ b/spec/requests/api/graphql/ci/config_variables_spec.rb
@@ -54,12 +54,12 @@
         expect(graphql_data.dig('project', 'ciConfigVariables')).to contain_exactly(
           {
             'key' => 'DB_NAME',
-            'value' => 'postgres',
+            'value' => ['postgres'],
             'description' => nil
           },
           {
             'key' => 'ENVIRONMENT_VAR',
-            'value' => 'env var value',
+            'value' => ['env var value', 'env var value2'],
             'description' => 'env var description'
           }
         )
diff --git a/spec/support/gitlab_stubs/gitlab_ci.yml b/spec/support/gitlab_stubs/gitlab_ci.yml
index b6a66cfa2c62f1..93f1581a115377 100644
--- a/spec/support/gitlab_stubs/gitlab_ci.yml
+++ b/spec/support/gitlab_stubs/gitlab_ci.yml
@@ -9,7 +9,7 @@ before_script:
 variables:
   DB_NAME: postgres
   ENVIRONMENT_VAR:
-    value: 'env var value'
+    value: ['env var value', 'env var value2']
     description: 'env var description'
 
 stages:
-- 
GitLab


From fc4a133dc8b71c30f04acf13530abbcb9245c637 Mon Sep 17 00:00:00 2001
From: Laura Montemayor <lmontemayor@gitlab.com>
Date: Wed, 5 Oct 2022 13:45:42 +0200
Subject: [PATCH 2/6] Updates GraphQL CiConfigType value field

The ciConfigVariableType value field will return either
an array of values when there are multiple values in the
Entry::Variable or a single value as a wrapped array.
---
 app/graphql/types/ci/config_variable_type.rb     | 8 +++-----
 lib/gitlab/ci/config/entry/variable.rb           | 6 +-----
 spec/lib/gitlab/ci/config/entry/variable_spec.rb | 4 ++--
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/app/graphql/types/ci/config_variable_type.rb b/app/graphql/types/ci/config_variable_type.rb
index 8a6836c2a052f2..c987a0c51b366e 100644
--- a/app/graphql/types/ci/config_variable_type.rb
+++ b/app/graphql/types/ci/config_variable_type.rb
@@ -19,11 +19,9 @@ class ConfigVariableType < BaseObject # rubocop:disable Graphql/AuthorizeTypes
             description: 'Value of the variable.'
 
       def value
-        if object[:value].is_a?(String)
-          Array.wrap(object[:value])
-        else
-          object[:value]
-        end
+        Gitlab::Json.parse(object[:value])
+      rescue StandardError
+        Array.wrap(object[:value])
       end
     end
   end
diff --git a/lib/gitlab/ci/config/entry/variable.rb b/lib/gitlab/ci/config/entry/variable.rb
index c19dc4e329b619..32676e636590f3 100644
--- a/lib/gitlab/ci/config/entry/variable.rb
+++ b/lib/gitlab/ci/config/entry/variable.rb
@@ -62,11 +62,7 @@ def applies_to?(config)
             end
 
             def value
-              if config_value.is_a?(Array)
-                config_value.map(&:to_s)
-              else
-                config_value.to_s
-              end
+              config_value.to_s
             end
 
             def value_with_data
diff --git a/spec/lib/gitlab/ci/config/entry/variable_spec.rb b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
index e3716dc36b88ec..99012806757586 100644
--- a/spec/lib/gitlab/ci/config/entry/variable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
@@ -137,13 +137,13 @@
           describe '#value' do
             subject(:value) { entry.value }
 
-            it { is_expected.to eq(%w[value value2]) }
+            it { is_expected.to eq(%w[value value2].to_s) }
           end
 
           describe '#value_with_data' do
             subject(:value_with_data) { entry.value_with_data }
 
-            it { is_expected.to eq(value: %w[value value2], description: 'description') }
+            it { is_expected.to eq(value: %w[value value2].to_s, description: 'description') }
           end
         end
 
-- 
GitLab


From e36066a277703544ae606aa61038ba4ca94509f2 Mon Sep 17 00:00:00 2001
From: Laura Montemayor <lmontemayor@gitlab.com>
Date: Thu, 6 Oct 2022 13:51:55 +0200
Subject: [PATCH 3/6] Updates variable entry in root with metadata

In order for arrays to be allowed only in top-level variables, a
metadata key `allow_array `was added in the variable keyword in
Entry::Root. The metadata is then used in Entry::Variable to keep either
an alphanumeric validation for other types of variables or allow an
array of strings for top-level variables.
---
 lib/gitlab/ci/config/entry/root.rb            |  2 +-
 lib/gitlab/ci/config/entry/variable.rb        | 11 ++++--
 lib/gitlab/ci/config/entry/variables.rb       |  2 +-
 .../gitlab/ci/config/entry/variable_spec.rb   | 38 ++++++++++++++-----
 .../gitlab/ci/config/entry/variables_spec.rb  | 26 +++++++++++++
 5 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/root.rb b/lib/gitlab/ci/config/entry/root.rb
index b1b42cad74c34f..64c89e41d631b2 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] },
+            metadata: { allowed_value_data: %i[value description], allow_array: true },
             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 32676e636590f3..4b49dbb06706df 100644
--- a/lib/gitlab/ci/config/entry/variable.rb
+++ b/lib/gitlab/ci/config/entry/variable.rb
@@ -45,7 +45,8 @@ def applies_to?(config)
 
             validations do
               validates :key, alphanumeric: true
-              validates :config_value, array_of_strings_or_string: true, allow_nil: false, if: :config_value_defined?
+              validates :config_value, array_of_strings_or_string: true, allow_nil: false, if: :array_in_config_value?
+              validates :config_value, alphanumeric: true, allow_nil: false, if: :string_in_config_value?
               validates :config_description, alphanumeric: true, allow_nil: false, if: :config_description_defined?
 
               validate do
@@ -77,8 +78,12 @@ def config_description
               @config[:description]
             end
 
-            def config_value_defined?
-              config.key?(:value)
+            def array_in_config_value?
+              config.key?(:value) && opt(:allow_array)
+            end
+
+            def string_in_config_value?
+              config.key?(:value) && !opt(:allow_array)
             end
 
             def config_description_defined?
diff --git a/lib/gitlab/ci/config/entry/variables.rb b/lib/gitlab/ci/config/entry/variables.rb
index 7fa0ee00066714..4149c1e1f4bb00 100644
--- a/lib/gitlab/ci/config/entry/variables.rb
+++ b/lib/gitlab/ci/config/entry/variables.rb
@@ -36,7 +36,7 @@ def composable_class(_name, _config)
           end
 
           def composable_metadata
-            { allowed_value_data: opt(:allowed_value_data) }
+            { allowed_value_data: opt(:allowed_value_data), allow_array: opt(:allow_array) }
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/entry/variable_spec.rb b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
index 99012806757586..07188849d88541 100644
--- a/spec/lib/gitlab/ci/config/entry/variable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
@@ -128,22 +128,40 @@
         end
 
         context 'when config value is an array of values' do
-          let(:config) { { value: %w[value value2], description: 'description' } }
+          context 'when allow_array metadata is false' do
+            let(:config) { { value: %w[value value2], description: 'description' } }
+            let(:metadata) { { allowed_value_data: %i[value description], allow_array: false } }
 
-          describe '#valid?' do
-            it { is_expected.to be_valid }
-          end
+            describe '#valid?' do
+              it { is_expected.not_to be_valid }
+            end
 
-          describe '#value' do
-            subject(:value) { entry.value }
+            describe '#errors' do
+              subject(:errors) { entry.errors }
 
-            it { is_expected.to eq(%w[value value2].to_s) }
+              it { is_expected.to include 'var1 config value must be an alphanumeric string' }
+            end
           end
 
-          describe '#value_with_data' do
-            subject(:value_with_data) { entry.value_with_data }
+          context 'when allow_array metadata is true' do
+            let(:config) { { value: %w[value value2], description: 'description' } }
+            let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+
+            describe '#valid?' do
+              it { is_expected.to be_valid }
+            end
+
+            describe '#value' do
+              subject(:value) { entry.value }
+
+              it { is_expected.to eq(%w[value value2].to_s) }
+            end
+
+            describe '#value_with_data' do
+              subject(:value_with_data) { entry.value_with_data }
 
-            it { is_expected.to eq(value: %w[value value2].to_s, description: 'description') }
+              it { is_expected.to eq(value: %w[value value2].to_s, description: 'description') }
+            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 ad7290d058960f..1e681df0d3f44a 100644
--- a/spec/lib/gitlab/ci/config/entry/variables_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
@@ -98,6 +98,32 @@
     it_behaves_like 'invalid config', /must be either a string or a hash/
   end
 
+  context 'when entry config value has key-value pair and value is an array' do
+    let(:config) do
+      { 'VARIABLE_1' => { value: %w[value1 value2], description: 'variable 1' } }
+    end
+
+    it_behaves_like 'invalid config', /variable_1 config must be a string/
+
+    context 'when metadata has allow_array and allowed_value_data' do
+      let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+
+      let(:result) do
+        { 'VARIABLE_1' => %w[value1 value2].to_s }
+      end
+
+      it_behaves_like 'valid config'
+
+      describe '#value_with_data' do
+        it 'returns variable with data' do
+          expect(entry.value_with_data).to eq(
+            'VARIABLE_1' => { value: %w[value1 value2].to_s, description: 'variable 1' }
+          )
+        end
+      end
+    end
+  end
+
   context 'when entry config value has key-value pair and hash' do
     let(:config) do
       { 'VARIABLE_1' => { value: 'value 1', description: 'variable 1' },
-- 
GitLab


From 47f5ce0fd34193d9a3d966cd942d29a5dc0a5013 Mon Sep 17 00:00:00 2001
From: Laura Montemayor <lmontemayor@gitlab.com>
Date: Thu, 6 Oct 2022 18:26:21 +0200
Subject: [PATCH 4/6] Adds a better rescue and spec

---
 app/graphql/types/ci/config_variable_type.rb  |  2 +-
 .../gitlab/ci/config/entry/variable_spec.rb   | 20 ++++++++++++
 .../gitlab/ci/config/entry/variables_spec.rb  | 32 ++++++++++++++++++-
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/app/graphql/types/ci/config_variable_type.rb b/app/graphql/types/ci/config_variable_type.rb
index c987a0c51b366e..180a87b80191f1 100644
--- a/app/graphql/types/ci/config_variable_type.rb
+++ b/app/graphql/types/ci/config_variable_type.rb
@@ -20,7 +20,7 @@ class ConfigVariableType < BaseObject # rubocop:disable Graphql/AuthorizeTypes
 
       def value
         Gitlab::Json.parse(object[:value])
-      rescue StandardError
+      rescue JSON::ParserError
         Array.wrap(object[:value])
       end
     end
diff --git a/spec/lib/gitlab/ci/config/entry/variable_spec.rb b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
index 07188849d88541..2ad5d10b40eda9 100644
--- a/spec/lib/gitlab/ci/config/entry/variable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
@@ -95,6 +95,26 @@
           it { is_expected.to eq(value: 'value', description: 'description') }
         end
 
+        context 'when allow_array metadata is true' do
+          let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+
+          describe '#valid?' do
+            it { is_expected.to be_valid }
+          end
+
+          describe '#value' do
+            subject(:value) { entry.value }
+
+            it { is_expected.to eq('value') }
+          end
+
+          describe '#value_with_data' do
+            subject(:value_with_data) { entry.value_with_data }
+
+            it { is_expected.to eq(value: 'value', description: 'description') }
+          end
+        end
+
         context 'when config value is a symbol' do
           let(:config) { { value: :value, description: 'description' } }
 
diff --git a/spec/lib/gitlab/ci/config/entry/variables_spec.rb b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
index 1e681df0d3f44a..d6599d23ffae3c 100644
--- a/spec/lib/gitlab/ci/config/entry/variables_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
@@ -98,12 +98,42 @@
     it_behaves_like 'invalid config', /must be either a string or a hash/
   end
 
+  context 'when entry config value has unallowed value key-value pair and value is a string' do
+    let(:config) do
+      { 'VARIABLE_1' => { value: 'value', description: 'variable 1' } }
+    end
+
+    context 'when there is no allowed_value_data metadata' do
+      it_behaves_like 'invalid config', /variable_1 config must be a string/
+    end
+
+    context 'when metadata has allow_array and allowed_value_data' do
+      let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+
+      let(:result) do
+        { 'VARIABLE_1' => 'value' }
+      end
+
+      it_behaves_like 'valid config'
+
+      describe '#value_with_data' do
+        it 'returns variable with data' do
+          expect(entry.value_with_data).to eq(
+            'VARIABLE_1' => { value: 'value', description: 'variable 1' }
+          )
+        end
+      end
+    end
+  end
+
   context 'when entry config value has key-value pair and value is an array' do
     let(:config) do
       { 'VARIABLE_1' => { value: %w[value1 value2], description: 'variable 1' } }
     end
 
-    it_behaves_like 'invalid config', /variable_1 config must be a string/
+    context 'when there is no allowed_value_data metadata' do
+      it_behaves_like 'invalid config', /variable_1 config must be a string/
+    end
 
     context 'when metadata has allow_array and allowed_value_data' do
       let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
-- 
GitLab


From b665c90c5106136175ddf59000fec945823a0804 Mon Sep 17 00:00:00 2001
From: Laura Montemayor <lmontemayor@gitlab.com>
Date: Tue, 11 Oct 2022 14:10:12 +0200
Subject: [PATCH 5/6] Adds ComplexArrayVariable class to Entry::Variable class

Updates the GraphQL type for ciConfigVariable to have either
a value that is a string or an array of values
---
 app/graphql/types/ci/config_variable_type.rb  | 10 +-
 doc/api/graphql/reference/index.md            |  3 +-
 lib/gitlab/ci/config/entry/variable.rb        | 42 ++++++--
 spec/lib/gitlab/ci/config/entry/root_spec.rb  |  8 +-
 .../gitlab/ci/config/entry/variable_spec.rb   | 98 +++++++++----------
 .../gitlab/ci/config/entry/variables_spec.rb  |  6 +-
 .../api/graphql/ci/config_variables_spec.rb   | 13 ++-
 spec/support/gitlab_stubs/gitlab_ci.yml       |  3 +
 8 files changed, 109 insertions(+), 74 deletions(-)

diff --git a/app/graphql/types/ci/config_variable_type.rb b/app/graphql/types/ci/config_variable_type.rb
index 180a87b80191f1..5b5890fd5a56c8 100644
--- a/app/graphql/types/ci/config_variable_type.rb
+++ b/app/graphql/types/ci/config_variable_type.rb
@@ -14,15 +14,13 @@ class ConfigVariableType < BaseObject # rubocop:disable Graphql/AuthorizeTypes
             null: true,
             description: 'Description for the CI/CD config variable.'
 
-      field :value, [GraphQL::Types::String],
+      field :value, GraphQL::Types::String,
             null: true,
             description: 'Value of the variable.'
 
-      def value
-        Gitlab::Json.parse(object[:value])
-      rescue JSON::ParserError
-        Array.wrap(object[:value])
-      end
+      field :value_options, [GraphQL::Types::String],
+            null: true,
+            description: 'Value options for the variable.'
     end
   end
 end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index ee9b8b8be9b254..f547326d818947 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -10404,7 +10404,8 @@ CI/CD config variables.
 | ---- | ---- | ----------- |
 | <a id="ciconfigvariabledescription"></a>`description` | [`String`](#string) | Description for the CI/CD config variable. |
 | <a id="ciconfigvariablekey"></a>`key` | [`String`](#string) | Name of the variable. |
-| <a id="ciconfigvariablevalue"></a>`value` | [`[String!]`](#string) | Value of the variable. |
+| <a id="ciconfigvariablevalue"></a>`value` | [`String`](#string) | Value of the variable. |
+| <a id="ciconfigvariablevalueoptions"></a>`valueOptions` | [`[String!]`](#string) | Value options for the variable. |
 
 ### `CiGroup`
 
diff --git a/lib/gitlab/ci/config/entry/variable.rb b/lib/gitlab/ci/config/entry/variable.rb
index 4b49dbb06706df..8fb841a268f621 100644
--- a/lib/gitlab/ci/config/entry/variable.rb
+++ b/lib/gitlab/ci/config/entry/variable.rb
@@ -10,6 +10,7 @@ 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
@@ -39,14 +40,13 @@ class ComplexVariable < ::Gitlab::Config::Entry::Node
 
             class << self
               def applies_to?(config)
-                config.is_a?(Hash)
+                config.is_a?(Hash) && !config[:value].is_a?(Array)
               end
             end
 
             validations do
               validates :key, alphanumeric: true
-              validates :config_value, array_of_strings_or_string: true, allow_nil: false, if: :array_in_config_value?
-              validates :config_value, alphanumeric: true, allow_nil: false, if: :string_in_config_value?
+              validates :config_value, alphanumeric: true, allow_nil: false, if: :config_value_defined?
               validates :config_description, alphanumeric: true, allow_nil: false, if: :config_description_defined?
 
               validate do
@@ -78,12 +78,8 @@ def config_description
               @config[:description]
             end
 
-            def array_in_config_value?
-              config.key?(:value) && opt(:allow_array)
-            end
-
-            def string_in_config_value?
-              config.key?(:value) && !opt(:allow_array)
+            def config_value_defined?
+              config.key?(:value)
             end
 
             def config_description_defined?
@@ -91,6 +87,34 @@ def config_description_defined?
             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)
+
+                errors.add(:config, 'value must be an alphanumeric string')
+              end
+            end
+
+            def value
+              config_value.first
+            end
+
+            def value_with_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/spec/lib/gitlab/ci/config/entry/root_spec.rb b/spec/lib/gitlab/ci/config/entry/root_spec.rb
index 64438a1594a46e..a55e13e7c2d9eb 100644
--- a/spec/lib/gitlab/ci/config/entry/root_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/root_spec.rb
@@ -34,7 +34,11 @@
           image: 'image:1.0',
           default: {},
           services: ['postgres:9.1', 'mysql:5.5'],
-          variables: { VAR: 'root', VAR2: { value: 'val 2', description: 'this is var 2' } },
+          variables: {
+            VAR: 'root',
+            VAR2: { value: 'val 2', description: 'this is var 2' },
+            VAR3: { value: %w[val3 val3b], description: 'this is var 3' }
+          },
           after_script: ['make clean'],
           stages: %w(build pages release),
           cache: { key: 'k', untracked: true, paths: ['public/'] },
@@ -83,7 +87,7 @@
         end
 
         it 'sets correct variables value' do
-          expect(root.variables_value).to eq('VAR' => 'root', 'VAR2' => 'val 2')
+          expect(root.variables_value).to eq('VAR' => 'root', 'VAR2' => 'val 2', 'VAR3' => 'val3')
         end
 
         describe '#leaf?' do
diff --git a/spec/lib/gitlab/ci/config/entry/variable_spec.rb b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
index 2ad5d10b40eda9..4921d4ac6edb42 100644
--- a/spec/lib/gitlab/ci/config/entry/variable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
@@ -95,26 +95,6 @@
           it { is_expected.to eq(value: 'value', description: 'description') }
         end
 
-        context 'when allow_array metadata is true' do
-          let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
-
-          describe '#valid?' do
-            it { is_expected.to be_valid }
-          end
-
-          describe '#value' do
-            subject(:value) { entry.value }
-
-            it { is_expected.to eq('value') }
-          end
-
-          describe '#value_with_data' do
-            subject(:value_with_data) { entry.value_with_data }
-
-            it { is_expected.to eq(value: 'value', description: 'description') }
-          end
-        end
-
         context 'when config value is a symbol' do
           let(:config) { { value: :value, description: 'description' } }
 
@@ -147,41 +127,17 @@
           end
         end
 
-        context 'when config value is an array of values' do
-          context 'when allow_array metadata is false' do
-            let(:config) { { value: %w[value value2], description: 'description' } }
-            let(:metadata) { { allowed_value_data: %i[value description], allow_array: false } }
-
-            describe '#valid?' do
-              it { is_expected.not_to be_valid }
-            end
+        context 'when config is an array' do
+          let(:config) { [] }
 
-            describe '#errors' do
-              subject(:errors) { entry.errors }
-
-              it { is_expected.to include 'var1 config value must be an alphanumeric string' }
-            end
+          describe '#valid?' do
+            it { is_expected.not_to be_valid }
           end
 
-          context 'when allow_array metadata is true' do
-            let(:config) { { value: %w[value value2], description: 'description' } }
-            let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
-
-            describe '#valid?' do
-              it { is_expected.to be_valid }
-            end
+          describe '#errors' do
+            subject(:errors) { entry.errors }
 
-            describe '#value' do
-              subject(:value) { entry.value }
-
-              it { is_expected.to eq(%w[value value2].to_s) }
-            end
-
-            describe '#value_with_data' do
-              subject(:value_with_data) { entry.value_with_data }
-
-              it { is_expected.to eq(value: %w[value value2].to_s, description: 'description') }
-            end
+            it { is_expected.to include 'variable definition must be either a string or a hash' }
           end
         end
 
@@ -253,4 +209,44 @@
       end
     end
   end
+
+  describe 'ComplexArrayVariable' do
+    context 'when config value is an array of values' do
+      context 'when allow_array metadata is false' do
+        let(:config) { { value: %w[value value2], description: 'description' } }
+        let(:metadata) { { allow_array: false } }
+
+        describe '#valid?' do
+          it { is_expected.not_to be_valid }
+        end
+
+        describe '#errors' do
+          subject(:errors) { entry.errors }
+
+          it { is_expected.to include 'var1 config value must be an alphanumeric string' }
+        end
+      end
+
+      context 'when allow_array metadata is true' do
+        let(:config) { { value: %w[value value2], description: 'description' } }
+        let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+
+        describe '#valid?' do
+          it { is_expected.to be_valid }
+        end
+
+        describe '#value' do
+          subject(:value) { entry.value }
+
+          it { is_expected.to eq('value') }
+        end
+
+        describe '#value_with_data' do
+          subject(:value_with_data) { entry.value_with_data }
+
+          it { is_expected.to eq(value: 'value', description: 'description', value_options: %w[value value2]) }
+        end
+      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 d6599d23ffae3c..9a4664160a0098 100644
--- a/spec/lib/gitlab/ci/config/entry/variables_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
@@ -132,14 +132,14 @@
     end
 
     context 'when there is no allowed_value_data metadata' do
-      it_behaves_like 'invalid config', /variable_1 config must be a string/
+      it_behaves_like 'invalid config', /variable_1 config value must be an alphanumeric string/
     end
 
     context 'when metadata has allow_array and allowed_value_data' do
       let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
 
       let(:result) do
-        { 'VARIABLE_1' => %w[value1 value2].to_s }
+        { 'VARIABLE_1' => 'value1' }
       end
 
       it_behaves_like 'valid config'
@@ -147,7 +147,7 @@
       describe '#value_with_data' do
         it 'returns variable with data' do
           expect(entry.value_with_data).to eq(
-            'VARIABLE_1' => { value: %w[value1 value2].to_s, description: 'variable 1' }
+            'VARIABLE_1' => { value: 'value1', value_options: %w[value1 value2], description: 'variable 1' }
           )
         end
       end
diff --git a/spec/requests/api/graphql/ci/config_variables_spec.rb b/spec/requests/api/graphql/ci/config_variables_spec.rb
index 33f423ed944b6b..17133d7ea6605a 100644
--- a/spec/requests/api/graphql/ci/config_variables_spec.rb
+++ b/spec/requests/api/graphql/ci/config_variables_spec.rb
@@ -23,6 +23,7 @@
           ciConfigVariables(sha: "#{sha}") {
             key
             value
+            valueOptions
             description
           }
         }
@@ -52,14 +53,22 @@
         post_graphql(query, current_user: user)
 
         expect(graphql_data.dig('project', 'ciConfigVariables')).to contain_exactly(
+          {
+            'key' => 'KEY_VALUE_VAR',
+            'value' => 'value x',
+            'valueOptions' => nil,
+            'description' => 'value of KEY_VALUE_VAR'
+          },
           {
             'key' => 'DB_NAME',
-            'value' => ['postgres'],
+            'value' => 'postgres',
+            'valueOptions' => nil,
             'description' => nil
           },
           {
             'key' => 'ENVIRONMENT_VAR',
-            'value' => ['env var value', 'env var value2'],
+            'value' => 'env var value',
+            'valueOptions' => ['env var value', 'env var value2'],
             'description' => 'env var description'
           }
         )
diff --git a/spec/support/gitlab_stubs/gitlab_ci.yml b/spec/support/gitlab_stubs/gitlab_ci.yml
index 93f1581a115377..945235917654b3 100644
--- a/spec/support/gitlab_stubs/gitlab_ci.yml
+++ b/spec/support/gitlab_stubs/gitlab_ci.yml
@@ -7,6 +7,9 @@ before_script:
   - bundle exec rake db:create
 
 variables:
+  KEY_VALUE_VAR:
+    value: 'value x'
+    description: 'value of KEY_VALUE_VAR'
   DB_NAME: postgres
   ENVIRONMENT_VAR:
     value: ['env var value', 'env var value2']
-- 
GitLab


From 8e8e1c448b9c57789658b314205bec2b7e64b802 Mon Sep 17 00:00:00 2001
From: Laura Montemayor <lmontemayor@gitlab.com>
Date: Wed, 12 Oct 2022 12:09:16 +0200
Subject: [PATCH 6/6] Renames allow_array to allow_array_value and removes some
 specs

---
 lib/gitlab/ci/config/entry/root.rb            |  2 +-
 lib/gitlab/ci/config/entry/variable.rb        |  2 +-
 lib/gitlab/ci/config/entry/variables.rb       |  2 +-
 .../gitlab/ci/config/entry/variable_spec.rb   | 62 +++++++------------
 .../gitlab/ci/config/entry/variables_spec.rb  |  8 +--
 5 files changed, 30 insertions(+), 46 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/root.rb b/lib/gitlab/ci/config/entry/root.rb
index 64c89e41d631b2..1d7d8617c74780 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], allow_array: true },
+            metadata: { allowed_value_data: %i[value description], allow_array_value: true },
             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 8fb841a268f621..54c153c8b07dfb 100644
--- a/lib/gitlab/ci/config/entry/variable.rb
+++ b/lib/gitlab/ci/config/entry/variable.rb
@@ -100,7 +100,7 @@ def applies_to?(config)
               validates :config_value, array_of_strings: true, allow_nil: false, if: :config_value_defined?
 
               validate do
-                next if opt(:allow_array)
+                next if opt(:allow_array_value)
 
                 errors.add(:config, 'value must be an alphanumeric string')
               end
diff --git a/lib/gitlab/ci/config/entry/variables.rb b/lib/gitlab/ci/config/entry/variables.rb
index 4149c1e1f4bb00..4430a11dda73bd 100644
--- a/lib/gitlab/ci/config/entry/variables.rb
+++ b/lib/gitlab/ci/config/entry/variables.rb
@@ -36,7 +36,7 @@ def composable_class(_name, _config)
           end
 
           def composable_metadata
-            { allowed_value_data: opt(:allowed_value_data), allow_array: opt(:allow_array) }
+            { allowed_value_data: opt(:allowed_value_data), allow_array_value: opt(:allow_array_value) }
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/entry/variable_spec.rb b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
index 4921d4ac6edb42..076a5b32e92ba6 100644
--- a/spec/lib/gitlab/ci/config/entry/variable_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variable_spec.rb
@@ -127,20 +127,6 @@
           end
         end
 
-        context 'when config is an array' do
-          let(:config) { [] }
-
-          describe '#valid?' do
-            it { is_expected.not_to be_valid }
-          end
-
-          describe '#errors' do
-            subject(:errors) { entry.errors }
-
-            it { is_expected.to include 'variable definition must be either a string or a hash' }
-          end
-        end
-
         context 'when config description is a symbol' do
           let(:config) { { value: 'value', description: :description } }
 
@@ -211,41 +197,39 @@
   end
 
   describe 'ComplexArrayVariable' do
-    context 'when config value is an array of values' do
-      context 'when allow_array metadata is false' do
-        let(:config) { { value: %w[value value2], description: 'description' } }
-        let(:metadata) { { allow_array: false } }
+    context 'when allow_array_value metadata is false' do
+      let(:config) { { value: %w[value value2], description: 'description' } }
+      let(:metadata) { { allow_array_value: false } }
 
-        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' }
-        end
+        it { is_expected.to include 'var1 config value must be an alphanumeric string' }
       end
+    end
 
-      context 'when allow_array metadata is true' do
-        let(:config) { { value: %w[value value2], description: 'description' } }
-        let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+    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 } }
 
-        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', description: 'description', value_options: %w[value value2]) }
-        end
+        it { is_expected.to eq(value: 'value', description: 'description', value_options: %w[value value2]) }
       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 9a4664160a0098..085f304094e4f1 100644
--- a/spec/lib/gitlab/ci/config/entry/variables_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
@@ -107,8 +107,8 @@
       it_behaves_like 'invalid config', /variable_1 config must be a string/
     end
 
-    context 'when metadata has allow_array and allowed_value_data' do
-      let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+    context 'when metadata has allow_array_value and allowed_value_data' do
+      let(:metadata) { { allowed_value_data: %i[value description], allow_array_value: true } }
 
       let(:result) do
         { 'VARIABLE_1' => 'value' }
@@ -135,8 +135,8 @@
       it_behaves_like 'invalid config', /variable_1 config value must be an alphanumeric string/
     end
 
-    context 'when metadata has allow_array and allowed_value_data' do
-      let(:metadata) { { allowed_value_data: %i[value description], allow_array: true } }
+    context 'when metadata has allow_array_value and allowed_value_data' do
+      let(:metadata) { { allowed_value_data: %i[value description], allow_array_value: true } }
 
       let(:result) do
         { 'VARIABLE_1' => 'value1' }
-- 
GitLab