Skip to content

Commit

Permalink
Fix invalid template identification for partials
Browse files Browse the repository at this point in the history
  • Loading branch information
KnVerey committed Apr 6, 2018
1 parent 5a92f95 commit aca0780
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 20 deletions.
18 changes: 10 additions & 8 deletions lib/kubernetes-deploy/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
module KubernetesDeploy
class Renderer
class InvalidPartialError < InvalidTemplateError
attr_reader :parents, :content, :filename
attr_accessor :parents, :content, :filename
def initialize(msg, parents: [], content: nil, filename:)
@parents = parents
super(msg, content: content, filename: filename)
end
end
class PartialNotFound < InvalidPartialError; end
class PartialNotFound < InvalidTemplateError; end

def initialize(current_sha:, template_dir:, logger:, bindings: {})
@current_sha = current_sha
Expand All @@ -35,9 +35,9 @@ 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 InvalidTemplateError.new(err.message,
filename: "#{err.filename} (partial included from: #{all_parents.join(' -> ')})", content: err.content)
err.parents = err.parents.dup.unshift(filename)
err.filename = "#{err.filename} (partial included from: #{err.parents.join(' -> ')})"
raise err
rescue StandardError => err
raise InvalidTemplateError.new(err.message, filename: filename, content: raw_template)
end
Expand All @@ -60,10 +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.new(err.message, filename: err.filename)
# get the filename from the first parent, not the missing partial itself
raise err if err.filename == partial
raise InvalidPartialError.new(err.message, filename: partial, content: expanded_template || template)
rescue InvalidPartialError => err
parents = err.parents.dup.unshift(File.basename(partial_path))
raise InvalidPartialError.new(err.message, parents: parents, filename: err.filename, content: err.content)
err.parents = err.parents.dup.unshift(File.basename(partial_path))
raise err
rescue StandardError => err
raise InvalidPartialError.new(err.message, filename: partial_path, content: expanded_template || template)
end
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= partial 'does-not-exist' %>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= partial 'parent-with-missing-child' %>

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= partial 'does-not-exist' %>
5 changes: 5 additions & 0 deletions test/helpers/fixture_deploy_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ def deploy_raw_fixtures(set, wait: true, bindings: {}, subset: nil)
success = false
if subset
Dir.mktmpdir("fixture_dir") do |target_dir|
partials_dir = File.join(fixture_path(set), 'partials')
if File.directory?(partials_dir)
FileUtils.copy_entry(partials_dir, File.join(target_dir, 'partials'))
end

subset.each do |file|
FileUtils.copy_entry(File.join(fixture_path(set), file), File.join(target_dir, file))
end
Expand Down
30 changes: 22 additions & 8 deletions test/integration/kubernetes_deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def test_success_with_unrecognized_resource_type
end

def test_invalid_yaml_fails_fast
refute deploy_dir(fixture_path("invalid"))
assert_deploy_failure(deploy_dir(fixture_path("invalid")))
assert_logs_match_all([
"Failed to render and parse template",
"Invalid template: yaml-error.yml",
Expand All @@ -152,7 +152,7 @@ def test_invalid_yaml_fails_fast
end

def test_invalid_yaml_in_partial_prints_helpful_error
refute deploy_raw_fixtures("invalid-partials")
assert_deploy_failure(deploy_raw_fixtures("invalid-partials"))
included_from = "partial included from: include-invalid-partial.yml.erb"
assert_logs_match_all([
"Result: FAILURE",
Expand All @@ -172,18 +172,32 @@ def test_invalid_yaml_in_partial_prints_helpful_error
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"
def test_missing_partial_correctly_identifies_invalid_template
assert_deploy_failure(deploy_raw_fixtures("missing-partials", subset: ["parent-with-missing-child.yml.erb"]))

assert_logs_match_all([
"Result: FAILURE",
"Failed to render and parse template",
"Invalid template: missing (#{included_from})",
"Invalid template: parent-with-missing-child.yml.erb", # the thing with the invalid `partial` call in it
"> Error message:",
%r{Could not find partial 'missing' in any of.*fixtures/missing-partials/partials:.*/fixtures/partials}
%r{Could not find partial 'does-not-exist' in any of .*fixture_dir[^/]*/partials:.*/partials},
"> Template content:",
"<%= partial 'does-not-exist' %>",
], in_order: true)
end

refute_logs_match("Template content")
def test_missing_nested_partial_correctly_identifies_invalid_template_and_its_parents
assert_deploy_failure(deploy_raw_fixtures("missing-partials", subset: ["parent-with-missing-grandchild.yml.erb"]))

assert_logs_match_all([
"Result: FAILURE",
"Failed to render and parse template",
"Invalid template: parent-with-missing-child (partial included from: parent-with-missing-grandchild.yml.erb)",
"> Error message:",
%r{Could not find partial 'does-not-exist' in any of .*fixture_dir[^/]*/partials:.*/partials},
"> Template content:",
"<%= partial 'does-not-exist' %>"
], in_order: true)
end

def test_invalid_k8s_spec_that_is_valid_yaml_fails_fast_and_prints_template
Expand Down
4 changes: 2 additions & 2 deletions test/unit/kubernetes-deploy/renderer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ def test_non_existent_partial_raises
end
base = "Could not find partial 'foobarbaz' in any of"
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
assert_equal "including-non-existent-partial.yaml.erb", err.filename
assert_equal "---\n<%= partial 'foobarbaz' %>\n", err.content
end

def test_nesting_fields
Expand Down

0 comments on commit aca0780

Please sign in to comment.