Merge branch 'ci-allow-deep-nesting-for-scripts' into 'master'

Change nesting level for CI script keyword

See merge request gitlab-org/gitlab!53737

changelogs/unreleased/ci-allow-deep-nesting-for-scripts.yml

+---
+title: Accept deeply nested arrays for CI script keyword
+merge_request: 53737
+author:
+type: changed

lib/gitlab/ci/config/entry/commands.rb

@@ -10,12 +10,14 @@ module Gitlab
         class Commands < ::Gitlab::Config::Entry::Node
           include ::Gitlab::Config::Entry::Validatable
 
+          MAX_NESTING_LEVEL = 10
+
           validations do
-            validates :config, string_or_nested_array_of_strings: true
+            validates :config, string_or_nested_array_of_strings: { max_level: MAX_NESTING_LEVEL }
           end
 
           def value
-            Array(@config).flatten(1)
+            Array(@config).flatten(MAX_NESTING_LEVEL)
           end
         end
       end

lib/gitlab/config/entry/validators.rb

@@ -268,17 +268,16 @@ module Gitlab
           end
         end
 
-        class StringOrNestedArrayOfStringsValidator < NestedArrayOfStringsValidator
-          def validate_each(record, attribute, value)
-            unless validate_string_or_nested_array_of_strings(value)
-              record.errors.add(attribute, 'should be a string or an array containing strings and arrays of strings')
-            end
-          end
+        class StringOrNestedArrayOfStringsValidator < ActiveModel::EachValidator
+          include LegacyValidationHelpers
+          include NestedArrayHelpers
 
-          private
+          def validate_each(record, attribute, value)
+            max_level = options.fetch(:max_level, 1)
 
-          def validate_string_or_nested_array_of_strings(values)
-            validate_string(values) || validate_nested_array_of_strings(values)
+            unless validate_string(value) || validate_nested_array(value, max_level, &method(:validate_string))
+              record.errors.add(attribute, "should be a string or a nested array of strings up to #{max_level} levels deep")
+            end
           end
         end
 

lib/gitlab/config/entry/validators/nested_array_helpers.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Config
+    module Entry
+      module Validators
+        # Include this module to validate deeply nested array of values
+        #
+        # class MyNestedValidator < ActiveModel::EachValidator
+        #   include NestedArrayHelpers
+        #
+        #   def validate_each(record, attribute, value)
+        #     max_depth = options.fetch(:max_depth, 1)
+        #
+        #     unless validate_nested_array(value, max_depth) { |v| v.is_a?(Integer) }
+        #       record.errors.add(attribute, "is invalid")
+        #     end
+        #   end
+        # end
+        #
+        module NestedArrayHelpers
+          def validate_nested_array(value, max_depth = 1, &validator_proc)
+            return false unless value.is_a?(Array)
+
+            validate_nested_array_recursively(value, max_depth, &validator_proc)
+          end
+
+          private
+
+          # rubocop: disable Performance/RedundantBlockCall
+          # Disables Rubocop rule for easier readability reasons.
+          def validate_nested_array_recursively(value, nesting_level, &validator_proc)
+            return true if validator_proc.call(value)
+            return false if nesting_level <= 0
+            return false unless value.is_a?(Array)
+
+            value.all? do |element|
+              validate_nested_array_recursively(element, nesting_level - 1, &validator_proc)
+            end
+          end
+          # rubocop: enable Performance/RedundantBlockCall
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/config/entry/commands_spec.rb

@@ -87,18 +87,20 @@ RSpec.describe Gitlab::Ci::Config::Entry::Commands do
     describe '#errors' do
       it 'saves errors' do
         expect(entry.errors)
-          .to include 'commands config should be a string or an array containing strings and arrays of strings'
+          .to include 'commands config should be a string or a nested array of strings up to 10 levels deep'
       end
     end
   end
 
   context 'when entry value is multi-level nested array' do
-    let(:config) { [['ls', ['echo 1']], 'pwd'] }
+    let(:config) do
+      ['ls 0', ['ls 1', ['ls 2', ['ls 3', ['ls 4', ['ls 5', ['ls 6', ['ls 7', ['ls 8', ['ls 9', ['ls 10']]]]]]]]]]]
+    end
 
     describe '#errors' do
       it 'saves errors' do
         expect(entry.errors)
-          .to include 'commands config should be a string or an array containing strings and arrays of strings'
+          .to include 'commands config should be a string or a nested array of strings up to 10 levels deep'
       end
     end
 

spec/lib/gitlab/config/entry/validators/nested_array_helpers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Config::Entry::Validators::NestedArrayHelpers do
+  let(:config_struct) do
+    Struct.new(:value, keyword_init: true) do
+      include ActiveModel::Validations
+      extend Gitlab::Config::Entry::Validators::NestedArrayHelpers
+
+      validates_each :value do |record, attr, value|
+        unless validate_nested_array(value, 2) { |v| v.is_a?(Integer) }
+          record.errors.add(attr, "is invalid")
+        end
+      end
+    end
+  end
+
+  describe '#validate_nested_array' do
+    let(:config) { config_struct.new(value: value) }
+
+    subject(:errors) { config.errors }
+
+    before do
+      config.valid?
+    end
+
+    context 'with valid values' do
+      context 'with arrays of integers' do
+        let(:value) { [10, 11] }
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'with nested arrays of integers' do
+        let(:value) { [10, [11, 12]] }
+
+        it { is_expected.to be_empty }
+      end
+    end
+
+    context 'with invalid values' do
+      subject(:error_messages) { errors.messages }
+
+      context 'with single integers' do
+        let(:value) { 10 }
+
+        it { is_expected.to eq({ value: ['is invalid'] }) }
+      end
+
+      context 'when it is nested over the limit' do
+        let(:value) { [10, [11, [12]]] }
+
+        it { is_expected.to eq({ value: ['is invalid'] }) }
+      end
+
+      context 'when a value in the array is not valid' do
+        let(:value) { [10, 11.5] }
+
+        it { is_expected.to eq({ value: ['is invalid'] }) }
+      end
+
+      context 'when a value in the nested array is not valid' do
+        let(:value) { [10, [11, 12.5]] }
+
+        it { is_expected.to eq({ value: ['is invalid'] }) }
+      end
+    end
+  end
+end