Use TaskConfigValidator for RunnerTask

CHANGELOG.md

@@ -3,6 +3,7 @@
 *Enhancements*
 - Officially support Kubernetes 1.15 ([#546](https://github.com/Shopify/kubernetes-deploy/pull/546))
 - Make sure that we only declare a Service of type LoadBalancer as deployed after its IP address is published. [#547](https://github.com/Shopify/kubernetes-deploy/pull/547)
+- Add more validations to `RunnerTask`. [#554](https://github.com/Shopify/kubernetes-deploy/pull/554)
 
 
 *Bug Fixes*

lib/kubernetes-deploy/runner_task.rb

@@ -8,6 +8,7 @@
 require 'kubernetes-deploy/resource_watcher'
 require 'kubernetes-deploy/kubernetes_resource'
 require 'kubernetes-deploy/kubernetes_resource/pod'
+require 'kubernetes-deploy/runner_task_config_validator'
 
 module KubernetesDeploy
   class RunnerTask
@@ -35,7 +36,11 @@ def run!(task_template:, entrypoint:, args:, env_vars: [], verify_result: true)
       @logger.reset
 
       @logger.phase_heading("Initializing task")
-      validate_configuration(task_template, args)
+
+      @logger.info("Validating configuration")
+      verify_config!(task_template, args)
+      @logger.info("Using namespace '#{@namespace}' in context '#{@context}'")
+
       pod = build_pod(task_template, entrypoint, args, env_vars, verify_result)
       validate_pod(pod)
 
@@ -107,41 +112,14 @@ def record_status_once(pod)
       @logger.summary.add_paragraph(warning)
     end
 
-    def validate_configuration(task_template, args)
-      @logger.info("Validating configuration")
-      errors = []
-
-      if task_template.blank?
-        errors << "Task template name can't be nil"
-      end
-
-      if @namespace.blank?
-        errors << "Namespace can't be empty"
-      end
-
-      if args.blank?
-        errors << "Args can't be nil"
-      end
-
-      begin
-        kubeclient.get_namespace(@namespace) if @namespace.present?
-        @logger.info("Using namespace '#{@namespace}' in context '#{@context}'")
-
-      rescue Kubeclient::ResourceNotFoundError
-        errors << "Namespace was not found"
-      rescue Kubeclient::HttpError
-        errors << "Could not connect to kubernetes cluster"
-      rescue KubernetesDeploy::KubeclientBuilder::ContextMissingError
-        errors << "Could not connect to kubernetes cluster - context invalid"
-      end
-
-      unless errors.empty?
+    def verify_config!(task_template, args)
+      task_config_validator = RunnerTaskConfigValidator.new(task_template, args, @task_config, kubectl,
+        kubeclient_builder)
+      unless task_config_validator.valid?
         @logger.summary.add_action("Configuration invalid")
-        @logger.summary.add_paragraph(errors.map { |err| "- #{err}" }.join("\n"))
-        raise TaskConfigurationError, "Configuration invalid: #{errors.join(', ')}"
+        @logger.summary.add_paragraph([task_config_validator.errors].map { |err| "- #{err}" }.join("\n"))
+        raise KubernetesDeploy::TaskConfigurationError
       end
-
-      TaskConfigValidator.new(@task_config, kubectl, kubeclient_builder, only: [:validate_server_version]).valid?
     end
 
     def get_template(template_name)

lib/kubernetes-deploy/runner_task_config_validator.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+module KubernetesDeploy
+  class RunnerTaskConfigValidator < TaskConfigValidator
+    def initialize(template, args, *arguments)
+      super(*arguments)
+      @template = template
+      @args = args
+      @validations += %i(validate_template validate_args)
+    end
+
+    private
+
+    def validate_args
+      if @args.blank?
+        @errors << "Args can't be nil"
+      end
+    end
+
+    def validate_template
+      if @template.blank?
+        @errors << "Task template name can't be nil"
+      end
+    end
+  end
+end

test/integration-serial/serial_task_run_test.rb

@@ -13,7 +13,6 @@ def test_run_without_verify_result_fails_if_pod_was_not_created
     # Sketchy, but stubbing the kubeclient doesn't work (and wouldn't be concurrency-friendly)
     # Finding a way to reliably trigger a create failure would be much better, if possible
     mock = mock()
-    mock.expects(:get_namespace)
     template = kubeclient.get_pod_template('hello-cloud-template-runner', @namespace)
     mock.expects(:get_pod_template).returns(template)
     mock.expects(:create_pod).raises(Kubeclient::HttpError.new("409", "Pod with same name exists", {}))

test/integration/runner_task_test.rb

@@ -179,15 +179,15 @@ def test_run_bang_raises_exceptions_as_well_as_printing_failure
   end
 
   def test_run_fails_if_context_is_invalid
-    task_runner = build_task_runner(context: "missing")
+    task_runner = build_task_runner(context: "unknown")
     assert_task_run_failure(task_runner.run(run_params))
 
     assert_logs_match_all([
       "Initializing task",
       "Validating configuration",
       "Result: FAILURE",
       "Configuration invalid",
-      "- Could not connect to kubernetes cluster - context invalid",
+      "Context unknown missing from your kubeconfig file(s)",
     ], in_order: true)
   end
 
@@ -200,10 +200,54 @@ def test_run_fails_if_namespace_is_missing
       "Validating configuration",
       "Result: FAILURE",
       "Configuration invalid",
-      "- Namespace was not found",
+      "Could not find Namespace:",
     ], in_order: true)
   end
 
+  def test_run_fails_if_args_are_missing
+    task_runner = build_task_runner
+    result = task_runner.run(task_template: 'hello-cloud-template-runner',
+      entrypoint: ['/bin/sh', '-c'],
+      args: nil,
+      env_vars: ["MY_CUSTOM_VARIABLE=MITTENS"])
+    assert_task_run_failure(result)
+
+    assert_logs_match_all([
+      "Initializing task",
+      "Validating configuration",
+      "Result: FAILURE",
+      "Configuration invalid",
+      "Args can't be nil",
+    ], in_order: true)
+  end
+
+  def test_run_fails_if_task_template_is_blank
+    task_runner = build_task_runner
+    result = task_runner.run(task_template: '',
+      entrypoint: ['/bin/sh', '-c'],
+      args: nil,
+      env_vars: ["MY_CUSTOM_VARIABLE=MITTENS"])
+    assert_task_run_failure(result)
+
+    assert_logs_match_all([
+      "Initializing task",
+      "Validating configuration",
+      "Result: FAILURE",
+      "Configuration invalid",
+      "Task template name can't be nil",
+    ], in_order: true)
+  end
+
+  def test_run_bang_fails_if_task_template_or_args_are_invalid
+    task_runner = build_task_runner
+    assert_raises(KubernetesDeploy::TaskConfigurationError) do
+      task_runner.run!(task_template: '',
+        entrypoint: ['/bin/sh', '-c'],
+        args: nil,
+        env_vars: ["MY_CUSTOM_VARIABLE=MITTENS"])
+    end
+  end
+
   def test_run_with_template_missing
     task_runner = build_task_runner
     assert_task_run_failure(task_runner.run(run_params))