Merge pull request #6576 from cma-arnold/master

Hide sensitive properties in converge_if_changed.

2 files changed, 80 insertions, 18 deletions

diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index 1ebdfa6feb..9e9013b24e 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -250,7 +250,13 @@ class Chef
           properties_str = if new_resource.sensitive
                              specified_properties.join(", ")
                            else
-                             specified_properties.map { |p| "#{p}=#{new_resource.send(p).inspect}" }.join(", ")
+                             specified_properties.map do |property|
+                               "#{property}=" << if new_resource.class.properties[property].sensitive?
+                                                   "(suppressed sensitive property)"
+                                                 else
+                                                   new_resource.send(property).inspect
+                                                 end
+                             end.join(", ")
                            end
           Chef::Log.debug("Skipping update of #{new_resource}: has not changed any of the specified properties #{properties_str}.")
           return false
@@ -259,7 +265,7 @@ class Chef
         # Print the pretty green text and run the block
         property_size = modified.map { |p| p.size }.max
         modified.map! do |p|
-          properties_str = if new_resource.sensitive
+          properties_str = if new_resource.sensitive || new_resource.class.properties[p].sensitive?
                              "(suppressed sensitive property)"
                            else
                              "#{new_resource.send(p).inspect} (was #{current_resource.send(p).inspect})"
@@ -274,7 +280,7 @@ class Chef
         property_size = properties.map { |p| p.size }.max
         created = properties.map do |property|
           default = " (default value)" unless new_resource.property_is_set?(property)
-          properties_str = if new_resource.sensitive
+          properties_str = if new_resource.sensitive || new_resource.class.properties[property].sensitive?
                              "(suppressed sensitive property)"
                            else
                              new_resource.send(property).inspect
diff --git a/spec/integration/recipes/resource_converge_if_changed_spec.rb b/spec/integration/recipes/resource_converge_if_changed_spec.rb
index 89d831ddec..f0ba4822a7 100644
--- a/spec/integration/recipes/resource_converge_if_changed_spec.rb
+++ b/spec/integration/recipes/resource_converge_if_changed_spec.rb
@@ -17,7 +17,7 @@ describe "Resource::ActionClass#converge_if_changed" do
   before { Namer.current_index += 1 }
   before { Namer.incrementing_value = 0 }
 
-  context "when the resource has identity, state and control properties" do
+  context "when the resource has identity, state, control, and sensitive properties" do
     let(:resource_name) { :"converge_if_changed_dsl#{Namer.current_index}" }
     let(:resource_class) do
       result = Class.new(Chef::Resource) do
@@ -28,6 +28,7 @@ describe "Resource::ActionClass#converge_if_changed" do
         property :control1, desired_state: false, default: "default_control1"
         property :state1, default: "default_state1"
         property :state2, default: "default_state2"
+        property :sensitive1, default: "default_dontprintme", sensitive: true
         attr_accessor :converged
         def initialize(*args)
           super
@@ -54,6 +55,7 @@ describe "Resource::ActionClass#converge_if_changed" do
           resource_class.load_current_value do
             state1 "current_state1"
             state2 "current_state2"
+            sensitive1 "current_dontprintme"
           end
         end
 
@@ -134,6 +136,26 @@ EOM
           end
         end
 
+        context "and sensitive1 is set to a new value" do
+          let(:converge_recipe) do
+            <<-EOM
+              #{resource_name} 'blah' do
+                sensitive1 'new_dontprintme'
+              end
+            EOM
+          end
+
+          it "the resource updates sensitive1" do
+            expect(resource.converged).to eq 1
+            expect(resource.updated?).to be_truthy
+            expect(converged_recipe.stdout).to eq <<-EOM
+* #{resource_name}[blah] action create
+  - update default_identity1
+  -   set sensitive1 to (suppressed sensitive property)
+EOM
+          end
+        end
+
         context "and state1 is set to its current value but state2 is set to a new value" do
           let(:converge_recipe) do
             <<-EOM
@@ -244,19 +266,21 @@ EOM
             expect(converged_recipe.stdout).to eq <<-EOM
 * #{resource_name}[blah] action create
   - create default_identity1
-  -   set identity1 to "default_identity1" (default value)
-  -   set state1    to "default_state1" (default value)
-  -   set state2    to "default_state2" (default value)
+  -   set identity1  to "default_identity1" (default value)
+  -   set state1     to "default_state1" (default value)
+  -   set state2     to "default_state2" (default value)
+  -   set sensitive1 to (suppressed sensitive property) (default value)
 EOM
           end
         end
 
-        context "and state1 and state2 are set" do
+        context "and state1, state2, and sensitive1 are set" do
           let(:converge_recipe) do
             <<-EOM
               #{resource_name} 'blah' do
                 state1 'new_state1'
                 state2 'new_state2'
+                sensitive1 'new_dontprintme'
               end
             EOM
           end
@@ -267,9 +291,10 @@ EOM
             expect(converged_recipe.stdout).to eq <<-EOM
 * #{resource_name}[blah] action create
   - create default_identity1
-  -   set identity1 to "default_identity1" (default value)
-  -   set state1    to "new_state1"
-  -   set state2    to "new_state2"
+  -   set identity1  to "default_identity1" (default value)
+  -   set state1     to "new_state1"
+  -   set state2     to "new_state2"
+  -   set sensitive1 to (suppressed sensitive property)
 EOM
           end
         end
@@ -291,9 +316,10 @@ EOM
             expect(converged_recipe.stdout).to eq <<-EOM
 * #{resource_name}[blah] action create
   - create default_identity1
-  -   set identity1 to (suppressed sensitive property) (default value)
-  -   set state1    to (suppressed sensitive property)
-  -   set state2    to (suppressed sensitive property)
+  -   set identity1  to (suppressed sensitive property) (default value)
+  -   set state1     to (suppressed sensitive property)
+  -   set state2     to (suppressed sensitive property)
+  -   set sensitive1 to (suppressed sensitive property) (default value)
 EOM
           end
         end
@@ -309,6 +335,9 @@ EOM
           converge_if_changed :state2 do
             new_resource.converged += 1
           end
+          converge_if_changed :sensitive1 do
+            new_resource.converged += 1
+          end
         end
       end
 
@@ -415,6 +444,26 @@ EOM
 EOM
           end
         end
+
+        context "and sensitive1 is set to a new value" do
+          let(:converge_recipe) do
+            <<-EOM
+              #{resource_name} 'blah' do
+                sensitive1 'new_dontprintme'
+              end
+            EOM
+          end
+
+          it "the resource updates sensitive1" do
+            expect(resource.converged).to eq 1
+            expect(resource.updated?).to be_truthy
+            expect(converged_recipe.stdout).to eq <<-EOM
+* #{resource_name}[blah] action create
+  - update default_identity1
+  -   set sensitive1 to (suppressed sensitive property)
+EOM
+          end
+        end
       end
 
       context "and no current_resource" do
@@ -430,7 +479,7 @@ EOM
           end
 
           it "the resource is created" do
-            expect(resource.converged).to eq 2
+            expect(resource.converged).to eq 3
             expect(resource.updated?).to  be_truthy
             expect(converged_recipe.stdout).to eq <<-EOM
 * #{resource_name}[blah] action create
@@ -438,22 +487,25 @@ EOM
   -   set state1 to "default_state1" (default value)
   - create default_identity1
   -   set state2 to "default_state2" (default value)
+  - create default_identity1
+  -   set sensitive1 to (suppressed sensitive property) (default value)
 EOM
           end
         end
 
-        context "and state1 and state2 are set to new values" do
+        context "and state1, state2, and sensitive1 are set to new values" do
           let(:converge_recipe) do
             <<-EOM
               #{resource_name} 'blah' do
                 state1 'new_state1'
                 state2 'new_state2'
+                sensitive1 'new_dontprintme'
               end
             EOM
           end
 
           it "the resource is created" do
-            expect(resource.converged).to eq 2
+            expect(resource.converged).to eq 3
             expect(resource.updated?).to  be_truthy
             expect(converged_recipe.stdout).to eq <<-EOM
 * #{resource_name}[blah] action create
@@ -461,6 +513,8 @@ EOM
   -   set state1 to "new_state1"
   - create default_identity1
   -   set state2 to "new_state2"
+  - create default_identity1
+  -   set sensitive1 to (suppressed sensitive property)
 EOM
           end
         end
@@ -477,7 +531,7 @@ EOM
           end
 
           it "the resource is created" do
-            expect(resource.converged).to eq 2
+            expect(resource.converged).to eq 3
             expect(resource.updated?).to be_truthy
             expect(converged_recipe.stdout).to eq <<-EOM
 * #{resource_name}[blah] action create
@@ -485,6 +539,8 @@ EOM
   -   set state1 to (suppressed sensitive property)
   - create default_identity1
   -   set state2 to (suppressed sensitive property)
+  - create default_identity1
+  -   set sensitive1 to (suppressed sensitive property) (default value)
 EOM
           end
         end