Merge branch 'config_entry_script_similar_to_commands' into 'master'

Allow strings and nested arrays of strings for before_script and after_script

See merge request gitlab-org/gitlab!74792

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

@@ -21,7 +21,7 @@ module Gitlab
             validates :config, allowed_keys: ALLOWED_KEYS
           end
 
-          entry :before_script, Entry::Script,
+          entry :before_script, Entry::Commands,
             description: 'Script that will be executed before each job.',
             inherit: true
 
@@ -33,7 +33,7 @@ module Gitlab
             description: 'Docker images that will be linked to the container.',
             inherit: true
 
-          entry :after_script, Entry::Script,
+          entry :after_script, Entry::Commands,
             description: 'Script that will be executed after each job.',
             inherit: true
 

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

@@ -45,7 +45,7 @@ module Gitlab
             end
           end
 
-          entry :before_script, Entry::Script,
+          entry :before_script, Entry::Commands,
             description: 'Global before script overridden in this job.',
             inherit: true
 
@@ -58,7 +58,7 @@ module Gitlab
             inherit: false,
             deprecation: { deprecated: '9.0', warning: '14.8', removed: '15.0' }
 
-          entry :after_script, Entry::Script,
+          entry :after_script, Entry::Commands,
             description: 'Commands that will be executed when finishing job.',
             inherit: true
 

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

@@ -32,7 +32,7 @@ module Gitlab
             description: 'List of external YAML files to include.',
             reserved: true
 
-          entry :before_script, Entry::Script,
+          entry :before_script, Entry::Commands,
             description: 'Script that will be executed before each job.',
             reserved: true
 
@@ -44,7 +44,7 @@ module Gitlab
             description: 'Docker images that will be linked to the container.',
             reserved: true
 
-          entry :after_script, Entry::Script,
+          entry :after_script, Entry::Commands,
             description: 'Script that will be executed after each job.',
             reserved: true
 

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

-# frozen_string_literal: true
-
-module Gitlab
-  module Ci
-    class Config
-      module Entry
-        ##
-        # Entry that represents a script.
-        #
-        class Script < ::Gitlab::Config::Entry::Node
-          include ::Gitlab::Config::Entry::Validatable
-
-          validations do
-            validates :config, nested_array_of_strings: true
-          end
-
-          def value
-            config.flatten(1)
-          end
-        end
-      end
-    end
-  end
-end

lib/gitlab/config/entry/validators.rb

@@ -278,20 +278,6 @@ module Gitlab
           end
         end
 
-        class NestedArrayOfStringsValidator < ArrayOfStringsOrStringValidator
-          def validate_each(record, attribute, value)
-            unless validate_nested_array_of_strings(value)
-              record.errors.add(attribute, 'should be an array containing strings and arrays of strings')
-            end
-          end
-
-          private
-
-          def validate_nested_array_of_strings(values)
-            values.is_a?(Array) && values.all? { |element| validate_array_of_strings_or_string(element) }
-          end
-        end
-
         class StringOrNestedArrayOfStringsValidator < ActiveModel::EachValidator
           include LegacyValidationHelpers
           include NestedArrayHelpers

lib/gitlab/web_ide/config/entry/terminal.rb

@@ -28,7 +28,7 @@ module Gitlab
             end
           end
 
-          entry :before_script, ::Gitlab::Ci::Config::Entry::Script,
+          entry :before_script, ::Gitlab::Ci::Config::Entry::Commands,
             description: 'Global before script overridden in this job.'
 
           entry :script, ::Gitlab::Ci::Config::Entry::Commands,

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

@@ -354,9 +354,9 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do
       root.compose!
     end
 
-    context 'when before script is not an array' do
+    context 'when before script is a number' do
       let(:hash) do
-        { before_script: 'ls' }
+        { before_script: 123 }
       end
 
       describe '#valid?' do
@@ -368,7 +368,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do
       describe '#errors' do
         it 'reports errors from child nodes' do
           expect(root.errors)
-            .to include 'before_script config should be an array containing strings and arrays of strings'
+            .to include 'before_script config should be a string or a nested array of strings up to 10 levels deep'
         end
       end
     end

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

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Ci::Config::Entry::Script do
-  let(:entry) { described_class.new(config) }
-
-  describe 'validations' do
-    context 'when entry config value is array of strings' do
-      let(:config) { %w(ls pwd) }
-
-      describe '#value' do
-        it 'returns array of strings' do
-          expect(entry.value).to eq config
-        end
-      end
-
-      describe '#errors' do
-        it 'does not append errors' do
-          expect(entry.errors).to be_empty
-        end
-      end
-
-      describe '#valid?' do
-        it 'is valid' do
-          expect(entry).to be_valid
-        end
-      end
-    end
-
-    context 'when entry config value is array of arrays of strings' do
-      let(:config) { [['ls'], ['pwd', 'echo 1']] }
-
-      describe '#value' do
-        it 'returns array of strings' do
-          expect(entry.value).to eq ['ls', 'pwd', 'echo 1']
-        end
-      end
-
-      describe '#errors' do
-        it 'does not append errors' do
-          expect(entry.errors).to be_empty
-        end
-      end
-
-      describe '#valid?' do
-        it 'is valid' do
-          expect(entry).to be_valid
-        end
-      end
-    end
-
-    context 'when entry config value is array containing strings and arrays of strings' do
-      let(:config) { ['ls', ['pwd', 'echo 1']] }
-
-      describe '#value' do
-        it 'returns array of strings' do
-          expect(entry.value).to eq ['ls', 'pwd', 'echo 1']
-        end
-      end
-
-      describe '#errors' do
-        it 'does not append errors' do
-          expect(entry.errors).to be_empty
-        end
-      end
-
-      describe '#valid?' do
-        it 'is valid' do
-          expect(entry).to be_valid
-        end
-      end
-    end
-
-    context 'when entry value is string' do
-      let(:config) { 'ls' }
-
-      describe '#errors' do
-        it 'saves errors' do
-          expect(entry.errors)
-            .to include 'script config should be an array containing strings and arrays of strings'
-        end
-      end
-
-      describe '#valid?' do
-        it 'is not valid' do
-          expect(entry).not_to be_valid
-        end
-      end
-    end
-
-    context 'when entry value is multi-level nested array' do
-      let(:config) { [['ls', ['echo 1']], 'pwd'] }
-
-      describe '#errors' do
-        it 'saves errors' do
-          expect(entry.errors)
-            .to include 'script config should be an array containing strings and arrays of strings'
-        end
-      end
-
-      describe '#valid?' do
-        it 'is not valid' do
-          expect(entry).not_to be_valid
-        end
-      end
-    end
-  end
-end

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -710,16 +710,16 @@ module Gitlab
             end
           end
 
-          context 'when script is array of arrays of strings' do
+          context 'when script is nested arrays of strings' do
             let(:config) do
               {
-                before_script: [["global script", "echo 1"], ["ls"], "pwd"],
+                before_script: [[["global script"], "echo 1"], "echo 2", ["ls"], "pwd"],
                 test: { script: ["script"] }
               }
             end
 
             it "return commands with scripts concatenated" do
-              expect(subject[:options][:before_script]).to eq(["global script", "echo 1", "ls", "pwd"])
+              expect(subject[:options][:before_script]).to eq(["global script", "echo 1", "echo 2", "ls", "pwd"])
             end
           end
         end
@@ -737,15 +737,15 @@ module Gitlab
             end
           end
 
-          context 'when script is array of arrays of strings' do
+          context 'when script is nested arrays of strings' do
             let(:config) do
               {
-                test: { script: [["script"], ["echo 1"], "ls"] }
+                test: { script: [[["script"], "echo 1", "echo 2"], "ls"] }
               }
             end
 
             it "return commands with scripts concatenated" do
-              expect(subject[:options][:script]).to eq(["script", "echo 1", "ls"])
+              expect(subject[:options][:script]).to eq(["script", "echo 1", "echo 2", "ls"])
             end
           end
         end
@@ -790,16 +790,16 @@ module Gitlab
             end
           end
 
-          context 'when script is array of arrays of strings' do
+          context 'when script is nested arrays of strings' do
             let(:config) do
               {
-                after_script: [["global script", "echo 1"], ["ls"], "pwd"],
+                after_script: [[["global script"], "echo 1"], "echo 2", ["ls"], "pwd"],
                 test: { script: ["script"] }
               }
             end
 
             it "return after_script in options" do
-              expect(subject[:options][:after_script]).to eq(["global script", "echo 1", "ls", "pwd"])
+              expect(subject[:options][:after_script]).to eq(["global script", "echo 1", "echo 2", "ls", "pwd"])
             end
           end
         end
@@ -2469,40 +2469,16 @@ module Gitlab
           it_behaves_like 'returns errors', 'jobs:rspec:tags config should be an array of strings'
         end
 
-        context 'returns errors if before_script parameter is invalid' do
-          let(:config) { YAML.dump({ before_script: "bundle update", rspec: { script: "test" } }) }
-
-          it_behaves_like 'returns errors', 'before_script config should be an array containing strings and arrays of strings'
-        end
-
         context 'returns errors if job before_script parameter is not an array of strings' do
           let(:config) { YAML.dump({ rspec: { script: "test", before_script: [10, "test"] } }) }
 
-          it_behaves_like 'returns errors', 'jobs:rspec:before_script config should be an array containing strings and arrays of strings'
-        end
-
-        context 'returns errors if job before_script parameter is multi-level nested array of strings' do
-          let(:config) { YAML.dump({ rspec: { script: "test", before_script: [["ls", ["pwd"]], "test"] } }) }
-
-          it_behaves_like 'returns errors', 'jobs:rspec:before_script config should be an array containing strings and arrays of strings'
-        end
-
-        context 'returns errors if after_script parameter is invalid' do
-          let(:config) { YAML.dump({ after_script: "bundle update", rspec: { script: "test" } }) }
-
-          it_behaves_like 'returns errors', 'after_script config should be an array containing strings and arrays of strings'
+          it_behaves_like 'returns errors', 'jobs:rspec:before_script config should be a string or a nested array of strings up to 10 levels deep'
         end
 
         context 'returns errors if job after_script parameter is not an array of strings' do
           let(:config) { YAML.dump({ rspec: { script: "test", after_script: [10, "test"] } }) }
 
-          it_behaves_like 'returns errors', 'jobs:rspec:after_script config should be an array containing strings and arrays of strings'
-        end
-
-        context 'returns errors if job after_script parameter is multi-level nested array of strings' do
-          let(:config) { YAML.dump({ rspec: { script: "test", after_script: [["ls", ["pwd"]], "test"] } }) }
-
-          it_behaves_like 'returns errors', 'jobs:rspec:after_script config should be an array containing strings and arrays of strings'
+          it_behaves_like 'returns errors', 'jobs:rspec:after_script config should be a string or a nested array of strings up to 10 levels deep'
         end
 
         context 'returns errors if image parameter is invalid' do

spec/lib/gitlab/config/entry/factory_spec.rb

@@ -5,8 +5,8 @@ require 'spec_helper'
 RSpec.describe Gitlab::Config::Entry::Factory do
   describe '#create!' do
     before do
-      stub_const('Script', Class.new(Gitlab::Config::Entry::Node))
-      Script.class_eval do
+      stub_const('Commands', Class.new(Gitlab::Config::Entry::Node))
+      Commands.class_eval do
         include Gitlab::Config::Entry::Validatable
 
         validations do
@@ -15,7 +15,7 @@ RSpec.describe Gitlab::Config::Entry::Factory do
       end
     end
 
-    let(:entry) { Script }
+    let(:entry) { Commands }
     let(:factory) { described_class.new(entry) }
 
     context 'when setting a concrete value' do

spec/lib/gitlab/web_ide/config/entry/global_spec.rb

@@ -108,7 +108,7 @@ RSpec.describe Gitlab::WebIde::Config::Entry::Global do
       describe '#errors' do
         it 'reports errors about missing script' do
           expect(global.errors)
-            .to include "terminal:before_script config should be an array containing strings and arrays of strings"
+            .to include "terminal:before_script config should be a string or a nested array of strings up to 10 levels deep"
         end
       end
     end

spec/lib/gitlab/web_ide/config_spec.rb

@@ -56,7 +56,7 @@ RSpec.describe Gitlab::WebIde::Config do
     end
 
     context 'when config logic is incorrect' do
-      let(:yml) { 'terminal: { before_script: "ls" }' }
+      let(:yml) { 'terminal: { before_script: 123 }' }
 
       describe '#valid?' do
         it 'is not valid' do