Flatten CI config rules

Flattens CI config rules. Nested rules arrays are now valid, this allows
including rules groups with !reference.

Changelog: changed

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

@@ -31,7 +31,7 @@ module Gitlab
 
               with_options allow_nil: true do
                 validates :extends, array_of_strings_or_string: true
-                validates :rules, array_of_hashes: true
+                validates :rules, nested_array_of_hashes: true
                 validates :resource_group, type: String
               end
             end

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

@@ -13,7 +13,7 @@ module Gitlab
           end
 
           def value
-            @config
+            [@config].flatten
           end
 
           def composable_class

lib/gitlab/config/entry/validators.rb

@@ -78,10 +78,26 @@ module Gitlab
           include LegacyValidationHelpers
 
           def validate_each(record, attribute, value)
-            unless value.is_a?(Array) && value.map { |hsh| hsh.is_a?(Hash) }.all?
+            unless validate_array_of_hashes(value)
               record.errors.add(attribute, 'should be an array of hashes')
             end
           end
+
+          private
+
+          def validate_array_of_hashes(value)
+            value.is_a?(Array) && value.all? { |obj| obj.is_a?(Hash) }
+          end
+        end
+
+        class NestedArrayOfHashesValidator < ArrayOfHashesValidator
+          include NestedArrayHelpers
+
+          def validate_each(record, attribute, value)
+            unless validate_nested_array(value, 1, &method(:validate_array_of_hashes))
+              record.errors.add(attribute, 'should be an array containing hashes and arrays of hashes')
+            end
+          end
         end
 
         class ArrayOrStringValidator < ActiveModel::EachValidator

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

@@ -17,6 +17,10 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do
   describe '.new' do
     subject { entry }
 
+    before do
+      subject.compose!
+    end
+
     context 'with a list of rule rule' do
       let(:config) do
         [{ if: '$THIS == "that"', when: 'never' }]
@@ -24,14 +28,6 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do
 
       it { is_expected.to be_a(described_class) }
       it { is_expected.to be_valid }
-
-      context 'when composed' do
-        before do
-          subject.compose!
-        end
-
-        it { is_expected.to be_valid }
-      end
     end
 
     context 'with a list of two rules' do
@@ -42,21 +38,34 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do
         ]
       end
 
-      it { is_expected.to be_a(described_class) }
       it { is_expected.to be_valid }
+    end
 
-      context 'when composed' do
-        before do
-          subject.compose!
-        end
+    context 'with a single rule object' do
+      let(:config) do
+        { if: '$SKIP', when: 'never' }
+      end
 
-        it { is_expected.to be_valid }
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'with nested rules' do
+      let(:config) do
+        [
+          { if: '$THIS == "that"', when: 'always' },
+          [{ if: '$SKIP', when: 'never' }]
+        ]
       end
+
+      it { is_expected.to be_valid }
     end
 
-    context 'with a single rule object' do
+    context 'with rules nested more than one level' do
       let(:config) do
-        { if: '$SKIP', when: 'never' }
+        [
+          { if: '$THIS == "that"', when: 'always' },
+          [{ if: '$SKIP', when: 'never' }, [{ if: '$THIS == "other"', when: 'aways' }]]
+        ]
       end
 
       it { is_expected.not_to be_valid }
@@ -90,7 +99,36 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do
         { if: '$SKIP', when: 'never' }
       end
 
-      it { is_expected.to eq(config) }
+      it { is_expected.to eq([config]) }
+    end
+
+    context 'with nested rules' do
+      let(:first_rule) { { if: '$THIS == "that"', when: 'always' } }
+      let(:second_rule) { { if: '$SKIP', when: 'never' } }
+
+      let(:config) do
+        [
+          first_rule,
+          [second_rule]
+        ]
+      end
+
+      it { is_expected.to contain_exactly(first_rule, second_rule) }
+    end
+
+    context 'with rules nested more than one level' do
+      let(:first_rule) { { if: '$THIS == "that"', when: 'always' } }
+      let(:second_rule) { { if: '$SKIP', when: 'never' } }
+      let(:third_rule) { { if: '$THIS == "other"', when: 'aways' } }
+
+      let(:config) do
+        [
+          first_rule,
+          [second_rule, [third_rule]]
+        ]
+      end
+
+      it { is_expected.to contain_exactly(first_rule, second_rule, third_rule) }
     end
   end
 

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -2777,7 +2777,7 @@ module Gitlab
             expect(subject.valid?).to eq(false)
             expect(subject.errors).to contain_exactly(
               'jobs:rspec config contains unknown keys: bad_tags',
-              'jobs:rspec rules should be an array of hashes')
+              'jobs:rspec rules should be an array containing hashes and arrays of hashes')
           end
         end