Unify render errors and handle an edge case

.rubocop.yml

@@ -3,3 +3,9 @@ inherit_from:
 
 AllCops:
   TargetRubyVersion: 2.3
+
+Style/TrailingCommaInArrayLiteral:
+  Enabled: false
+
+Style/TrailingCommaInHashLiteral:
+  Enabled: false

lib/kubernetes-deploy/deploy_task.rb

@@ -225,24 +225,25 @@ def discover_resources
         end
       end
       resources
+    rescue InvalidTemplateError => e
+      debug_msg = paragraph_for_invalid_templates(e.message, filenames: [e.filename], content: e.content)
+      @logger.summary.add_paragraph(debug_msg)
+      raise FatalDeploymentError, "Failed to render and parse template"
     end
 
     def split_templates(filename)
       file_content = File.read(File.join(@template_dir, filename))
       rendered_content = @renderer.render_template(filename, file_content)
       YAML.load_stream(rendered_content) do |doc|
-        yield doc unless doc.blank?
+        next if doc.blank?
+        unless doc.is_a?(Hash)
+          raise InvalidTemplateError.new("Template is not a valid Kubernetes manifest",
+            filename: filename, content: doc)
+        end
+        yield doc
       end
     rescue Psych::SyntaxError => e
-      debug_msg = <<~INFO
-        Error message: #{e}
-
-        Template content:
-        ---
-      INFO
-      debug_msg += rendered_content
-      @logger.summary.add_paragraph(debug_msg)
-      raise FatalDeploymentError, "Template '#{filename}' cannot be parsed"
+      raise InvalidTemplateError.new(e, filename: filename, content: rendered_content)
     end
 
     def record_invalid_template(err, file_paths:, original_filenames: nil)
@@ -252,14 +253,18 @@ def record_invalid_template(err, file_paths:, original_filenames: nil)
         contents << File.read(file_path)
         template_names << File.basename(file_path) unless original_filenames
       end.join("\n")
-      template_list = template_names.compact.join(", ").presence || "See error message"
+      debug_msg = paragraph_for_invalid_templates(err, filenames: template_names, content: file_content)
+      @logger.summary.add_paragraph(debug_msg)
+    end
 
-      debug_msg = ColorizedString.new("Invalid #{'template'.pluralize(template_names.length)}: #{template_list}\n").red
-      debug_msg += "> Error from kubectl:\n#{indent_four(err)}"
-      if file_content.present?
-        debug_msg += "\n> Rendered template content:\n#{indent_four(file_content)}"
+    def paragraph_for_invalid_templates(err, filenames:, content:)
+      template_list = filenames.compact.join(", ").presence || "See error message"
+      debug_msg = ColorizedString.new("Invalid #{'template'.pluralize(filenames.length)}: #{template_list}\n").red
+      debug_msg += "> Error message:\n#{indent_four(err)}"
+      if content.present?
+        debug_msg += "\n> Template content:\n#{indent_four(content)}"
       end
-      @logger.summary.add_paragraph(debug_msg)
+      debug_msg
     end
 
     def indent_four(str)

lib/kubernetes-deploy/errors.rb

@@ -3,6 +3,15 @@ module KubernetesDeploy
   class FatalDeploymentError < StandardError; end
   class KubectlError < StandardError; end
 
+  class InvalidTemplateError < FatalDeploymentError
+    attr_reader :filename, :content
+    def initialize(err, filename:, content: nil)
+      @filename = filename
+      @content = content
+      super(err)
+    end
+  end
+
   class NamespaceNotFoundError < FatalDeploymentError
     def initialize(name, context)
       super("Namespace `#{name}` not found in context `#{context}`")

lib/kubernetes-deploy/renderer.rb

@@ -7,11 +7,11 @@
 
 module KubernetesDeploy
   class Renderer
-    class InvalidPartialError < FatalDeploymentError
-      attr_reader :parents
-      def initialize(msg, parents = [])
+    class InvalidPartialError < InvalidTemplateError
+      attr_reader :parents, :content, :filename
+      def initialize(msg, parents: [], content: nil, filename:)
         @parents = parents
-        super(msg)
+        super(msg, content: content, filename: filename)
       end
     end
     class PartialNotFound < InvalidPartialError; end
@@ -36,10 +36,10 @@ def render_template(filename, raw_template)
       ERB.new(raw_template, nil, '-').result(erb_binding)
     rescue InvalidPartialError => err
       all_parents = err.parents.dup.unshift(filename)
-      raise FatalDeploymentError, "#{err.message} (included from: #{all_parents.join(' -> ')})"
+      raise InvalidTemplateError.new(err.message,
+        filename: "#{err.filename} (partial included from: #{all_parents.join(' -> ')})", content: err.content)
     rescue StandardError => err
-      report_template_invalid(err.message, raw_template)
-      raise FatalDeploymentError, "Template '#{filename}' cannot be rendered"
+      raise InvalidTemplateError.new(err.message, filename: filename, content: raw_template)
     end
 
     def render_partial(partial, locals)
@@ -60,12 +60,12 @@ def render_partial(partial, locals)
       # Note that JSON is a subset of YAML.
       JSON.generate(docs.children.first.to_ruby)
     rescue PartialNotFound => err
-      raise InvalidPartialError, err.message
+      raise InvalidPartialError.new(err.message, filename: err.filename)
     rescue InvalidPartialError => err
-      raise InvalidPartialError.new(err.message, err.parents.dup.unshift(File.basename(partial_path)))
+      parents = err.parents.dup.unshift(File.basename(partial_path))
+      raise InvalidPartialError.new(err.message, parents: parents, filename: err.filename, content: err.content)
     rescue StandardError => err
-      report_template_invalid(err.message, expanded_template)
-      raise InvalidPartialError, "Template '#{partial_path}' cannot be rendered"
+      raise InvalidPartialError.new(err.message, filename: partial_path, content: expanded_template || template)
     end
 
     private
@@ -91,12 +91,8 @@ def find_partial(name)
           return partial_path if File.exist?(partial_path)
         end
       end
-      raise PartialNotFound, "Could not find partial '#{name}' in any of #{@partials_dirs.join(':')}"
-    end
-
-    def report_template_invalid(message, content)
-      @logger.summary.add_paragraph("Error from renderer:\n  #{message.tr("\n", ' ')}")
-      @logger.summary.add_paragraph("Rendered template content:\n#{content}")
+      raise PartialNotFound.new("Could not find partial '#{name}' in any of #{@partials_dirs.join(':')}",
+        filename: name)
     end
 
     class TemplateContext

test/fixtures/invalid/wrong-extension-erb.yml

@@ -0,0 +1,12 @@
+<% (0..2).each do |n| %>
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: <%= "test#{n}" %>
+  labels:
+    name: <%= "test#{n}" %>
+    app: hello-cloud
+data:
+  test<%= n %>: <%= n * n %>
+<% end %>

test/helpers/fixture_deploy_helper.rb

@@ -40,8 +40,19 @@ def deploy_fixtures(set, subset: nil, **args) # extra args are passed through to
     success
   end
 
-  def deploy_raw_fixtures(set, wait: true, bindings: {})
-    deploy_dir(fixture_path(set), wait: wait, bindings: bindings)
+  def deploy_raw_fixtures(set, wait: true, bindings: {}, subset: nil)
+    success = false
+    if subset
+      Dir.mktmpdir("fixture_dir") do |target_dir|
+        subset.each do |file|
+          FileUtils.copy_entry(File.join(fixture_path(set), file), File.join(target_dir, file))
+        end
+        success = deploy_dir(target_dir, wait: wait, bindings: bindings)
+      end
+    else
+      success = deploy_dir(fixture_path(set), wait: wait, bindings: bindings)
+    end
+    success
   end
 
   def deploy_dir_without_profiling(dir, wait: true, allow_protected_ns: false, prune: true, bindings: {},

test/integration/kubernetes_deploy_test.rb

@@ -136,41 +136,47 @@ def test_success_with_unrecognized_resource_type
   def test_invalid_yaml_fails_fast
     refute deploy_dir(fixture_path("invalid"))
     assert_logs_match_all([
-      /Template 'yaml-error.yml' cannot be parsed/,
-      /datapoint1: value1:/
-    ])
+      "Failed to render and parse template",
+      "Invalid template: yaml-error.yml",
+      "mapping values are not allowed in this context",
+      "> Template content:",
+      "datapoint1: value1:"
+    ], in_order: true)
   end
 
   def test_invalid_yaml_in_partial_prints_helpful_error
     refute deploy_raw_fixtures("invalid-partials")
+    included_from = "partial included from: include-invalid-partial.yml.erb"
     assert_logs_match_all([
       "Result: FAILURE",
-      %r{Template '.*/partials/invalid.yml.erb' cannot be rendered \(included from: include-invalid-partial.yml.erb\)},
-      "Error from renderer:",
-      "  (<unknown>): mapping values are not allowed in this context",
-      "Rendered template content:",
+      "Failed to render and parse template",
+      %r{Invalid template: .*/partials/invalid.yml.erb \(#{included_from}\)},
+      "> Error message:",
+      "(<unknown>): mapping values are not allowed in this context",
+      "> Template content:",
       "containers:",
       "- name: invalid-container",
       "notField: notval:"
     ], in_order: true)
 
     # make sure we're not displaying duplicate errors
     refute_logs_match("Template 'include-invalid-partial.yml.erb' cannot be rendered")
-    assert_logs_match("Rendered template content:", 1)
-    assert_logs_match("Error from renderer:", 1)
+    assert_logs_match("Template content:", 1)
+    assert_logs_match("Error message:", 1)
   end
 
   def test_missing_nested_partial_prints_helpful_error
     refute deploy_raw_fixtures("missing-partials")
+    included_from = "partial included from: include-missing-partials.yml.erb -> nest-missing-partial.yml.erb"
     assert_logs_match_all([
       "Result: FAILURE",
-      %r{Could not find partial 'missing' in any of.*fixtures/missing-partials/partials:.*/fixtures/partials},
-      "(included from: include-missing-partials.yml.erb -> nest-missing-partial.yml.erb)"
+      "Failed to render and parse template",
+      "Invalid template: missing (#{included_from})",
+      "> Error message:",
+      %r{Could not find partial 'missing' in any of.*fixtures/missing-partials/partials:.*/fixtures/partials}
     ], in_order: true)
 
-    # make sure we're not displaying empty errors from the parents
-    refute_logs_match("Rendered template content:")
-    refute_logs_match("Error from renderer:")
+    refute_logs_match("Template content")
   end
 
   def test_invalid_k8s_spec_that_is_valid_yaml_fails_fast_and_prints_template
@@ -184,19 +190,19 @@ def test_invalid_k8s_spec_that_is_valid_yaml_fails_fast_and_prints_template
       assert_logs_match_all([
         "Template validation failed",
         /Invalid template: ConfigMap-hello-cloud-configmap-data.*yml/,
-        "> Error from kubectl:",
+        "> Error message:",
         "error validating data: found invalid field myKey for v1.ObjectMeta",
-        "> Rendered template content:",
+        "> Template content:",
         "      myKey: uhOh"
       ], in_order: true)
     else
       assert_logs_match_all([
         "Template validation failed",
         /Invalid template: ConfigMap-hello-cloud-configmap-data.*yml/,
-        "> Error from kubectl:",
+        "> Error message:",
         "error validating data: ValidationError(ConfigMap.metadata): \
 unknown field \"myKey\" in io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
-        "> Rendered template content:",
+        "> Template content:",
         "      myKey: uhOh"
       ], in_order: true)
     end
@@ -231,7 +237,7 @@ def test_multiple_invalid_k8s_specs_fails_on_apply_and_prints_template
       /Invalid templates: Service-web.*\.yml, Deployment-web.*\.yml/,
       "Error from server (Invalid): error when creating",
       "Error from server (Invalid): error when creating", # once for deployment, once for svc
-      "> Rendered template content:",
+      "> Template content:",
       "        targetPort: http_test_is_really_long_and_invalid_chars", # error in svc template
       "              name: http_test_is_really_long_and_invalid_chars" # error in deployment template
     ], in_order: true)
@@ -247,7 +253,7 @@ def test_invalid_k8s_spec_that_is_valid_yaml_but_has_no_template_path_in_error_p
       "Command failed: apply -f",
       "WARNING: Any resources not mentioned in the error below were likely created/updated.",
       "Invalid templates: See error message",
-      "> Error from kubectl:",
+      "> Error message:",
       '    The Service "web" is invalid:',
       'spec.ports[0].targetPort: Invalid value: "http_test_is_really_long_and_invalid_chars"'
     ], in_order: true)
@@ -415,7 +421,15 @@ def test_extra_bindings_should_be_rendered
 
   def test_deploy_fails_if_required_binding_not_present
     assert_deploy_failure(deploy_fixtures('collection-with-erb', subset: ["conf_map.yaml.erb"]))
-    assert_logs_match("Template 'conf_map.yaml.erb' cannot be rendered")
+    assert_logs_match_all([
+      "Result: FAILURE",
+      "Failed to render and parse template",
+      "Invalid template: conf_map.yaml.erb",
+      "> Error message:",
+      "undefined local variable or method `binding_test_a'",
+      "> Template content:",
+      'BINDING_TEST_A: "<%= binding_test_a %>"'
+    ], in_order: true)
   end
 
   def test_long_running_deployment
@@ -571,7 +585,7 @@ def test_output_when_switching_labels_incorrectly
       "Command failed: apply -f",
       "WARNING: Any resources not mentioned in the error below were likely created/updated.",
       "Invalid templates: See error message",
-      "> Error from kubectl:",
+      "> Error message:",
       /The Deployment "web" is invalid.*`selector` does not match template `labels`/
     ], in_order: true)
   end
@@ -865,4 +879,8 @@ def test_cronjobs_can_be_deployed
     cronjobs = FixtureSetAssertions::CronJobs.new(@namespace)
     cronjobs.assert_cronjob_present("my-cronjob")
   end
+
+  def test_friendly_error_on_misidentified_erb_file
+    assert_deploy_failure(deploy_raw_fixtures('invalid', subset: ['wrong-extension-erb.yml']))
+  end
 end

test/unit/kubernetes-deploy/renderer_test.rb

@@ -43,20 +43,23 @@ def test_can_render_template_with_correct_indentation
   end
 
   def test_invalid_partial_raises
-    err = assert_raises(KubernetesDeploy::FatalDeploymentError) do
+    err = assert_raises(KubernetesDeploy::InvalidTemplateError) do
       render('broken-partial-inclusion.yaml.erb')
     end
-    included_from = "included from: broken-partial-inclusion.yaml.erb -> broken.yml.erb"
-    assert_match %r{Template '.*/partials/simple.yaml.erb' cannot be rendered \(#{included_from}\)}, err.message
+    included_from = "partial included from: broken-partial-inclusion.yaml.erb -> broken.yml.erb"
+    assert_match "undefined local variable or method `foo'", err.message
+    assert_match %r{.*/partials/simple.yaml.erb \(#{included_from}\)}, err.filename
+    assert_equal "c: c3\nd: d4\nfoo: <%= foo %>\n", err.content
   end
 
   def test_non_existent_partial_raises
-    err = assert_raises(KubernetesDeploy::FatalDeploymentError) do
+    err = assert_raises(KubernetesDeploy::InvalidTemplateError) do
       render('including-non-existent-partial.yaml.erb')
     end
     base = "Could not find partial 'foobarbaz' in any of"
-    included_from = "included from: including-non-existent-partial.yaml.erb"
-    assert_match %r{#{base} .*/fixtures/for_unit_tests/partials:.*/fixtures/partials \(#{included_from}\)}, err.message
+    assert_match %r{#{base} .*/fixtures/for_unit_tests/partials:.*/fixtures/partials}, err.message
+    assert_equal "foobarbaz (partial included from: including-non-existent-partial.yaml.erb)", err.filename
+    assert_nil err.content
   end
 
   def test_nesting_fields