Move job artifacts configuration new CI config code

lib/ci/gitlab_ci_yaml_processor.rb

@@ -148,26 +148,6 @@ module Ci
           raise ValidationError, "#{name} job: artifacts unknown parameter #{key}"
         end
       end
-
-      if job[:artifacts][:name] && !validate_string(job[:artifacts][:name])
-        raise ValidationError, "#{name} job: artifacts:name parameter should be a string"
-      end
-
-      if job[:artifacts][:untracked] && !validate_boolean(job[:artifacts][:untracked])
-        raise ValidationError, "#{name} job: artifacts:untracked parameter should be an boolean"
-      end
-
-      if job[:artifacts][:paths] && !validate_array_of_strings(job[:artifacts][:paths])
-        raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings"
-      end
-
-      if job[:artifacts][:when] && !job[:artifacts][:when].in?(%w[on_success on_failure always])
-        raise ValidationError, "#{name} job: artifacts:when parameter should be on_success, on_failure or always"
-      end
-
-      if job[:artifacts][:expire_in] && !validate_duration(job[:artifacts][:expire_in])
-        raise ValidationError, "#{name} job: artifacts:expire_in parameter should be a duration"
-      end
     end
 
     def validate_job_dependencies!(name, job)

lib/gitlab/ci/config/node/artifacts.rb

+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a configuration of job artifacts.
+        #
+        class Artifacts < Entry
+          include Validatable
+          include Attributable
+
+          attributes :name, :untracked, :paths, :when, :expire_in
+
+          validations do
+            validates :config, type: Hash
+
+            with_options allow_nil: true do
+              validates :name, type: String
+              validates :untracked, boolean: true
+              validates :paths, array_of_strings: true
+              validates :when,
+                inclusion: { in: %w[on_success on_failure always],
+                             message: 'should be on_success, on_failure ' \
+                                      'or always' }
+              validates :expire_in, duration: true
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/config/node/attributable.rb

+module Gitlab
+  module Ci
+    class Config
+      module Node
+        module Attributable
+          extend ActiveSupport::Concern
+
+          class_methods do
+            def attributes(*attributes)
+              attributes.each do |attribute|
+                define_method(attribute) do
+                  return unless config.is_a?(Hash)
+
+                  config[attribute]
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -47,8 +47,12 @@ module Gitlab
           node :variables, Variables,
             description: 'Environment variables available for this job.'
 
+          node :artifacts, Artifacts,
+            description: 'Artifacts configuration for this job.'
+
           helpers :before_script, :script, :stage, :type, :after_script,
-                  :cache, :image, :services, :only, :except, :variables
+                  :cache, :image, :services, :only, :except, :variables,
+                  :artifacts
 
           def name
             @metadata[:name]
@@ -71,6 +75,7 @@ module Gitlab
               only: only,
               except: except,
               variables: variables_defined? ? variables : nil,
+              artifacts: artifacts,
               after_script: after_script }
           end
 

lib/gitlab/ci/config/node/validators.rb

@@ -33,6 +33,16 @@ module Gitlab
             end
           end
 
+          class DurationValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
+            def validate_each(record, attribute, value)
+              unless validate_duration(value)
+                record.errors.add(attribute, 'should be a duration')
+              end
+            end
+          end
+
           class RequiredValidator < ActiveModel::EachValidator
             def validate_each(record, attribute, value)
               if value.nil?

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -1138,42 +1138,42 @@ EOT
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { name: 1 } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:name parameter should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts name should be a string")
       end
 
       it "returns errors if job artifacts:when is not an a predefined value" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { when: 1 } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:when parameter should be on_success, on_failure or always")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts when should be on_success, on_failure or always")
       end
 
       it "returns errors if job artifacts:expire_in is not an a string" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { expire_in: 1 } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:expire_in parameter should be a duration")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts expire in should be a duration")
       end
 
       it "returns errors if job artifacts:expire_in is not an a valid duration" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { expire_in: "7 elephants" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:expire_in parameter should be a duration")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts expire in should be a duration")
       end
 
       it "returns errors if job artifacts:untracked is not an array of strings" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { untracked: "string" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:untracked parameter should be an boolean")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts untracked should be a boolean value")
       end
 
       it "returns errors if job artifacts:paths is not an array of strings" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { paths: "string" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:paths parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts paths should be an array of strings")
       end
 
       it "returns errors if cache:untracked is not an array of strings" do

spec/lib/gitlab/ci/config/node/artifacts_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Artifacts do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    context 'when entry config value is correct' do
+      let(:config) { { paths: %w[public/] } }
+
+      describe '#value' do
+        it 'returns image string' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not correct' do
+      let(:config) { { name: 10 } }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'artifacts name should be a string'
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/config/node/attributable_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Attributable do
+  let(:node) { Class.new }
+  let(:instance) { node.new }
+
+  before do
+    node.include(described_class)
+
+    node.class_eval do
+      attributes :name, :test
+    end
+  end
+
+  context 'config is a hash' do
+    before do
+      allow(instance)
+        .to receive(:config)
+        .and_return({ name: 'some name', test: 'some test' })
+    end
+
+    it 'returns the value of config' do
+      expect(instance.name).to eq 'some name'
+      expect(instance.test).to eq 'some test'
+    end
+
+    it 'returns no method error for unknown attributes' do
+      expect { instance.unknown }.to raise_error(NoMethodError)
+    end
+  end
+
+  context 'config is not a hash' do
+    before do
+      allow(instance)
+        .to receive(:config)
+        .and_return('some test')
+    end
+
+    it 'returns nil' do
+      expect(instance.test).to be_nil
+    end
+  end
+end