Merge pull request #5268 from MsysTechnologiesllc/dh/registry_resource_idempotent_issue

Fix for idempotent issue for some registry types in registry_key resource

3 files changed, 48 insertions, 17 deletions

diff --git a/lib/chef/provider/registry_key.rb b/lib/chef/provider/registry_key.rb
index e516433ac8..5e8dbe9bd8 100644
--- a/lib/chef/provider/registry_key.rb
+++ b/lib/chef/provider/registry_key.rb
@@ -102,6 +102,9 @@ class Chef
         @new_resource.unscrubbed_values.each do |value|
           if @name_hash.has_key?(value[:name].downcase)
             current_value = @name_hash[value[:name].downcase]
+            if [:dword, :dword_big_endian, :qword].include? value[:type]
+              value[:data] = value[:data].to_i
+            end
             unless current_value[:type] == value[:type] && current_value[:data] == value[:data]
               converge_by("set value #{value}") do
                 registry.set_value(@new_resource.key, value)
diff --git a/spec/functional/resource/registry_spec.rb b/spec/functional/resource/registry_spec.rb
index e64b6697c5..8393e0a074 100644
--- a/spec/functional/resource/registry_spec.rb
+++ b/spec/functional/resource/registry_spec.rb
@@ -153,6 +153,16 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
       expect(@registry.data_exists?(reg_child, { :name => "Color", :type => :string, :data => "Orange" })).to eq(true)
     end
 
+    it "does not create the key if it already exists with same value and type but datatype of data differs" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "number", :type => :dword, :data => "12345" }])
+      @new_resource.run_action(:create)
+
+      expect(@new_resource).not_to be_updated_by_last_action
+      expect(@registry.key_exists?(reg_child)).to eq(true)
+      expect(@registry.data_exists?(reg_child, { :name => "number", :type => :dword, :data => 12344 })).to eq(true)
+    end
+
     it "creates a value if it does not exist" do
       @new_resource.key(reg_child)
       @new_resource.values([{ :name => "Mango", :type => :string, :data => "Yellow" }])
diff --git a/spec/unit/provider/registry_key_spec.rb b/spec/unit/provider/registry_key_spec.rb
index 41bc5b33d3..9e19d02d5e 100644
--- a/spec/unit/provider/registry_key_spec.rb
+++ b/spec/unit/provider/registry_key_spec.rb
@@ -19,23 +19,6 @@
 require "spec_helper"
 
 shared_examples_for "a registry key" do
-  before(:each) do
-    @node = Chef::Node.new
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, {}, @events)
-
-    @new_resource = Chef::Resource::RegistryKey.new("windows is fun", @run_context)
-    @new_resource.key keyname
-    @new_resource.values( testval1 )
-    @new_resource.recursive false
-
-    @provider = Chef::Provider::RegistryKey.new(@new_resource, @run_context)
-
-    allow(@provider).to receive(:running_on_windows!).and_return(true)
-    @double_registry = double(Chef::Win32::Registry)
-    allow(@provider).to receive(:registry).and_return(@double_registry)
-  end
-
   describe "when first created" do
   end
 
@@ -273,6 +256,23 @@ shared_examples_for "a registry key" do
 end
 
 describe Chef::Provider::RegistryKey do
+  before(:each) do
+    @node = Chef::Node.new
+    @events = Chef::EventDispatch::Dispatcher.new
+    @run_context = Chef::RunContext.new(@node, {}, @events)
+
+    @new_resource = Chef::Resource::RegistryKey.new("windows is fun", @run_context)
+    @new_resource.key keyname
+    @new_resource.values( testval1 )
+    @new_resource.recursive false
+
+    @provider = Chef::Provider::RegistryKey.new(@new_resource, @run_context)
+
+    allow(@provider).to receive(:running_on_windows!).and_return(true)
+    @double_registry = double(Chef::Win32::Registry)
+    allow(@provider).to receive(:registry).and_return(@double_registry)
+  end
+
   context "when the key data is safe" do
     let(:keyname) { 'HKLM\Software\Opscode\Testing\Safe' }
     let(:testval1) { { :name => "one", :type => :string, :data => "1" } }
@@ -292,4 +292,22 @@ describe Chef::Provider::RegistryKey do
 
     it_should_behave_like "a registry key"
   end
+
+  describe "action_create" do
+    context "when key exists and type matches" do
+      let(:keyname) { 'hklm\\software\\opscode\\testing\\dword' }
+      let(:dword_passed_as_integer) { { :name => "one", :type => :dword, :data => 12345 } }
+      let(:testval1) { { :name => "one", :type => :dword, :data => "12345" } }
+      before do
+        expect(@double_registry).to receive(:key_exists?).twice.with(keyname).and_return(true)
+      end
+
+      it "does not make a change for datatype of data value differing" do
+        expect(@double_registry).to receive(:get_values).with(keyname).and_return( dword_passed_as_integer )
+        expect(@double_registry).not_to receive(:set_value)
+        @provider.load_current_resource
+        @provider.action_create
+      end
+    end
+  end
 end