diff options
author | Noah Kantrowitz <noah@coderanger.net> | 2016-04-04 16:50:05 -0400 |
---|---|---|
committer | Thom May <thom@chef.io> | 2017-12-18 19:28:04 +0000 |
commit | 3b97e4a703356394a799677723a27432103ee611 (patch) | |
tree | f176a50c6d1715a9f5608148c4bcf2ae033ed4c0 | |
parent | abdfb00bac4cf6c920462afe8dd3d6f86ad5d974 (diff) | |
download | chef-3b97e4a703356394a799677723a27432103ee611.tar.gz |
Slight improvements to validation failures. Shows the failing command if possible and doesn't show anything for sensitive resources to be safe.
Refs https://github.com/chef/chef/issues/4793 and ping @chef/client-core for review.
-rw-r--r-- | lib/chef/provider/file.rb | 2 | ||||
-rw-r--r-- | lib/chef/resource/file/verification.rb | 10 | ||||
-rw-r--r-- | spec/support/shared/unit/provider/file.rb | 10 | ||||
-rw-r--r-- | spec/unit/resource/file/verification_spec.rb | 22 |
4 files changed, 38 insertions, 6 deletions
diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb index fea77dd7ea..86e7e5a5c4 100644 --- a/lib/chef/provider/file.rb +++ b/lib/chef/provider/file.rb @@ -339,7 +339,7 @@ class Chef if tempfile new_resource.verify.each do |v| if ! v.verify(tempfile.path) - raise Chef::Exceptions::ValidationFailed.new "Proposed content for #{new_resource.path} failed verification #{v}" + raise Chef::Exceptions::ValidationFailed.new "Proposed content for #{new_resource.path} failed verification #{new_resource.sensitive ? '[sensitive]' : v}" end end end diff --git a/lib/chef/resource/file/verification.rb b/lib/chef/resource/file/verification.rb index 079e6dc201..cfbdfd5263 100644 --- a/lib/chef/resource/file/verification.rb +++ b/lib/chef/resource/file/verification.rb @@ -119,6 +119,16 @@ class Chef v = verification_class.new(@parent_resource, @command, @command_opts, &@block) v.verify(path, opts) end + + def to_s + if @block + '<Proc>' + elsif @command.is_a?(Symbol) + "#{@command.inspect} (#{Chef::Resource::File::Verification.lookup(@command).name})" + elsif @command.is_a?(String) + @command + end + end end end end diff --git a/spec/support/shared/unit/provider/file.rb b/spec/support/shared/unit/provider/file.rb index b58159fcc9..d508e76b24 100644 --- a/spec/support/shared/unit/provider/file.rb +++ b/spec/support/shared/unit/provider/file.rb @@ -476,7 +476,15 @@ shared_examples_for Chef::Provider::File do allow(File).to receive(:directory?).with("C:\\Windows\\system32/cmd.exe").and_return(false) provider.new_resource.verify windows? ? "REM" : "true" provider.new_resource.verify windows? ? "cmd.exe /c exit 1" : "false" - expect { provider.send(:do_validate_content) }.to raise_error(Chef::Exceptions::ValidationFailed) + expect { provider.send(:do_validate_content) }.to raise_error(Chef::Exceptions::ValidationFailed, "Proposed content for #{provider.new_resource.path} failed verification #{windows? ? "cmd.exe /c exit 1" : "false"}") + end + + it "does not show verification for sensitive resources" do + allow(File).to receive(:directory?).with("C:\\Windows\\system32/cmd.exe").and_return(false) + provider.new_resource.verify windows? ? "REM" : "true" + provider.new_resource.verify windows? ? "cmd.exe /c exit 1" : "false" + provider.new_resource.sensitive true + expect { provider.send(:do_validate_content) }.to raise_error(Chef::Exceptions::ValidationFailed, "Proposed content for #{provider.new_resource.path} failed verification [sensitive]") end end end diff --git a/spec/unit/resource/file/verification_spec.rb b/spec/unit/resource/file/verification_spec.rb index feacd715a4..8e850395d8 100644 --- a/spec/unit/resource/file/verification_spec.rb +++ b/spec/unit/resource/file/verification_spec.rb @@ -66,6 +66,11 @@ describe Chef::Resource::File::Verification do v = Chef::Resource::File::Verification.new(parent_resource, nil, {}, &f_block) expect(v.verify(temp_path)).to eq(false) end + + it "responds to to_s" do + v = Chef::Resource::File::Verification.new(parent_resource, nil, {}) { } + expect(v.to_s).to eq("<Proc>") + end end context "with a verification command(String)" do @@ -110,23 +115,32 @@ describe Chef::Resource::File::Verification do v = Chef::Resource::File::Verification.new(parent_resource, "true", {}) expect(v.verify(temp_path)).to eq(true) end + + it "responds to to_s" do + v = Chef::Resource::File::Verification.new(parent_resource, "some command --here", {}) + expect(v.to_s).to eq("some command --here") + end end context "with a named verification(Symbol)" do + let(:registered_verification) { double("registered_verification") } + subject { described_class.new(parent_resource, :cats, {}) } before(:each) do class Chef::Resource::File::Verification::Turtle < Chef::Resource::File::Verification provides :cats def verify(path, opts) end end + allow(Chef::Resource::File::Verification::Turtle).to receive(:new).and_return(registered_verification) end it "delegates to the registered verification" do - registered_verification = double() - allow(Chef::Resource::File::Verification::Turtle).to receive(:new).and_return(registered_verification) - v = Chef::Resource::File::Verification.new(parent_resource, :cats, {}) expect(registered_verification).to receive(:verify).with(temp_path, {}) - v.verify(temp_path, {}) + subject.verify(temp_path, {}) + end + + it "responds to to_s" do + expect(subject.to_s).to eq(":cats (Chef::Resource::File::Verification::Turtle)") end end end |