CHANGELOG

@@ -1,4 +1,8 @@
-2012-05-16 - Jeff McCune <jeff@puppetlabs.com>
+0.1.1 - 2012-05-21 - Jeff McCune <jeff@puppetlabs.com>
+ * (#14517) Improve error handling when writing values (27223db)
+ * (#14572) Fix management of the default value (f29bdc5)
+
+0.1.0 - 2012-05-16 - Jeff McCune <jeff@puppetlabs.com>
  * (#14529) Add registry::value defined type (bf44208)
 
 2012-05-16 - Josh Cooper <josh+github@puppetlabs.com>

Modulefile

@@ -1,5 +1,5 @@
 name    'puppetlabs-registry'
-version '0.1.0'
+version '0.1.1'
 source 'git://github.com/puppetlabs/puppetlabs-registry.git'
 author 'puppetlabs'
 license 'Apache License, Version 2.0'

acceptance/tests/resource/registry/should_manage_values.rb

@@ -292,6 +292,15 @@ class phase1 {
         end
         assert_no_match(/err:/, result.stdout, "Expected no error messages.")
       end
+
+      step "Registry Values - Phase 3 - Check the default value (#14572)"
+      # (#14572) This test uses the 64 bit version of reg.exe to read the
+      # default value of a registry key.  It should contain the string shown in
+      # val_re.
+      on agent, "/cygdrive/c/windows/sysnative/reg.exe query '#{keypath}\\Subkey1'" do
+        val_re = /\(Default\)    REG_SZ    Default Data phase=2/i
+        assert_match(val_re, result.stdout, "Expected output to contain #{val_re.inspect}.")
+      end
     end
   end
 end

lib/puppet/modules/registry.rb

@@ -30,6 +30,13 @@ def canonical
       filter_path[:canonical]
     end
 
+    # This method is meant to help setup aliases so autorequire can sort itself
+    # out in a case insensitive but preserving manner.  It returns an array of
+    # resource identifiers.
+    def aliases
+      [canonical.downcase]
+    end
+
     def access
       filter_path[:access]
     end
@@ -104,30 +111,45 @@ def filter_path
                 raise ArgumentError, "Invalid registry key: #{path}"
               end
 
-      result[:subkey] = captures[3]
+      result[:trailing_path] = captures[3]
+
+      result[:trailing_path].gsub!(/^\\/, '')
 
-      if result[:subkey].empty?
+      if result[:trailing_path].empty?
         result[:canonical] = "#{result[:prefix]}#{result[:root].to_s}"
       else
         # Leading backslash is not part of the subkey name
-        result[:subkey].sub!(/^\\(.*)$/, '\1')
-        result[:canonical] = "#{result[:prefix]}#{result[:root].to_s}\\#{result[:subkey]}"
+        result[:canonical] = "#{result[:prefix]}#{result[:root].to_s}\\#{result[:trailing_path]}"
       end
 
       @filter_path_memo = result
     end
-
   end
 
   class RegistryKeyPath < RegistryPathBase
     def subkey
-      filter_path[:subkey]
+      filter_path[:trailing_path]
     end
   end
 
   class RegistryValuePath < RegistryPathBase
+    def canonical
+      # This method gets called in the type and the provider.  We need to
+      # preserve the trailing backslash for the provider, otherwise it won't
+      # think this is a default value.
+      if default?
+        filter_path[:canonical] << "\\"
+      else
+        filter_path[:canonical]
+      end
+    end
+
     def subkey
-      filter_path[:subkey].gsub(/\\#{filter_path[:valuename]}/, '')
+      if default?
+        filter_path[:trailing_path]
+      else
+        filter_path[:trailing_path].gsub(/^(.*)\\.*$/, '\1')
+      end
     end
 
     def valuename

lib/puppet/provider/registry_value/registry.rb

@@ -16,10 +16,23 @@ def self.instances
   def exists?
     Puppet.debug("Checking the existence of registry value: #{self}")
     found = false
-    hive.open(subkey, Win32::Registry::KEY_READ | access) do |reg|
-      type = [0].pack('L')
-      size = [0].pack('L')
-      found = reg_query_value_ex_a.call(reg.hkey, valuename, 0, type, 0, size) == 0
+    begin
+      hive.open(subkey, Win32::Registry::KEY_READ | access) do |reg|
+        type = [0].pack('L')
+        size = [0].pack('L')
+        found = reg_query_value_ex_a.call(reg.hkey, valuename, 0, type, 0, size) == 0
+      end
+    rescue Win32::Registry::Error => detail
+      case detail.code
+      when 2
+        # Code 2 is the error message for "The system cannot find the file specified."
+        # http://msdn.microsoft.com/en-us/library/windows/desktop/ms681382.aspx
+        found = false
+      else
+        error = Puppet::Error.new("Unexpected exception from Win32 API. detail: (#{detail.message}) ERROR CODE: #{detail.code}. Puppet Error ID: D4B679E4-0E22-48D5-80EF-96AAEC0282B9")
+        error.set_backtrace detail.backtrace
+        raise error
+      end
     end
     found
   end
@@ -30,6 +43,8 @@ def create
   end
 
   def flush
+    # REVISIT - This concept of flush seems different than package provider's
+    # concept of flush.
     Puppet.debug("Flushing registry value: #{self}")
     return if resource[:ensure] == :absent
     write_value
@@ -122,16 +137,25 @@ def reg_query_value_ex_a
     @@reg_query_value_ex_a ||= Win32API.new('advapi32', 'RegQueryValueEx', 'LPLPPP', 'L')
   end
 
-  # def to_s
-  #   "#{valuepath.hkey.keyname}\\#{valuepath.subkey}\\#{valuepath.valuename}"
-  # end
-
   private
 
   def write_value
-    hive.open(subkey, Win32::Registry::KEY_ALL_ACCESS | access) do |reg|
-      ary = to_native(resource[:type], resource[:data])
-      reg.write(valuename, ary[0], ary[1])
+    begin
+      hive.open(subkey, Win32::Registry::KEY_ALL_ACCESS | access) do |reg|
+        ary = to_native(resource[:type], resource[:data])
+        reg.write(valuename, ary[0], ary[1])
+      end
+    rescue Win32::Registry::Error => detail
+      error = case detail.code
+      when 2
+        # Code 2 is the error message for "The system cannot find the file specified."
+        # http://msdn.microsoft.com/en-us/library/windows/desktop/ms681382.aspx
+        Puppet::Error.new("Cannot write to the registry. The parent key does not exist. detail: (#{detail.message}) Puppet Error ID: AC99C7C6-98D6-4E91-A75E-970F4064BF95")
+      else
+        Puppet::Error.new("Unexpected exception from Win32 API. detail: (#{detail.message}). ERROR CODE: #{detail.code}. Puppet Error ID: F46C6AE2-C711-48F9-86D6-5D50E1988E48")
+      end
+      error.set_backtrace detail.backtrace
+      raise error
     end
   end
 

lib/puppet/type/registry_key.rb

@@ -35,12 +35,18 @@ def self.title_patterns
       Puppet::Modules::Registry::RegistryKeyPath.new(path).valid?
     end
     munge do |path|
-      canonical = Puppet::Modules::Registry::RegistryKeyPath.new(path).canonical
+      reg_path = Puppet::Modules::Registry::RegistryKeyPath.new(path)
       # Windows is case insensitive and case preserving.  We deal with this by
       # aliasing resources to their downcase values.  This is inspired by the
       # munge block in the alias metaparameter.
-      @resource.catalog.alias(@resource, canonical.downcase) if @resource.catalog.respond_to? :alias
-      canonical
+      if @resource.catalog
+        reg_path.aliases.each do |alt_name|
+          @resource.catalog.alias(@resource, alt_name)
+        end
+      else
+        Puppet.debug "Resource has no associated catalog.  Aliases are not being set for #{@resource.to_s}"
+      end
+      reg_path.canonical
     end
   end
 
@@ -102,7 +108,7 @@ def eval_generate
     # create absent registry_value resources for the complement
     resources = []
     (is_values - should_values).each do |name|
-      resources << Puppet::Type.type(:registry_value).new(:path => "#{self[:path]}\\#{name}", :ensure => :absent)
+      resources << Puppet::Type.type(:registry_value).new(:path => "#{self[:path]}\\#{name}", :ensure => :absent, :catalog => catalog)
     end
     resources
   end

lib/puppet/type/registry_value.rb

@@ -31,12 +31,18 @@ def self.title_patterns
       Puppet::Modules::Registry::RegistryValuePath.new(path).valid?
     end
     munge do |path|
-      canonical = Puppet::Modules::Registry::RegistryValuePath.new(path).canonical
+      reg_path = Puppet::Modules::Registry::RegistryValuePath.new(path)
       # Windows is case insensitive and case preserving.  We deal with this by
       # aliasing resources to their downcase values.  This is inspired by the
       # munge block in the alias metaparameter.
-      @resource.catalog.alias(@resource, canonical.downcase) if @resource.catalog.respond_to? :alias
-      canonical
+      if @resource.catalog
+        reg_path.aliases.each do |alt_name|
+          @resource.catalog.alias(@resource, alt_name)
+        end
+      else
+        Puppet.debug "Resource has no associated catalog.  Aliases are not being set for #{@resource.to_s}"
+      end
+      reg_path.canonical
     end
   end
 
@@ -111,7 +117,9 @@ def change_to_s(currentvalue, newvalue)
   # Autorequire the nearest ancestor registry_key found in the catalog.
   autorequire(:registry_key) do
     req = []
-    path = Puppet::Modules::Registry::RegistryKeyPath.new(value(:path))
+    # This is a value path and not a key path because it's based on the path of
+    # the value resource.
+    path = Puppet::Modules::Registry::RegistryValuePath.new(value(:path))
     # It is important to match against the downcase value of the path because
     # other resources are expected to alias themselves to the downcase value so
     # that we respect the case insensitive and preserving nature of Windows.

spec/unit/puppet/type/registry_key_spec.rb

@@ -5,7 +5,13 @@
 require 'puppet/type/registry_key'
 
 describe Puppet::Type.type(:registry_key) do
-  let (:key) { Puppet::Type.type(:registry_key).new(:name => 'HKLM\Software') }
+  let (:catalog) do Puppet::Resource::Catalog.new end
+
+  # This is overridden here so we get a consistent association with the key
+  # and a catalog using memoized let methods.
+  let (:key) do
+    Puppet::Type.type(:registry_key).new(:name => 'HKLM\Software', :catalog => catalog)
+  end
 
   [:ensure].each do |property|
     it "should have a #{property} property" do

spec/unit/puppet/type/registry_value_spec.rb

@@ -3,6 +3,8 @@
 require 'puppet/type/registry_value'
 
 describe Puppet::Type.type(:registry_value) do
+  let (:catalog) do Puppet::Resource::Catalog.new end
+
   [:ensure, :type, :data].each do |property|
     it "should have a #{property} property" do
       described_class.attrclass(property).ancestors.should be_include(Puppet::Property)
@@ -20,36 +22,36 @@
 
     %w[hklm\propname hklm\software\propname].each do |path|
       it "should accept #{path}" do
-        described_class.new(:path => path)
+        described_class.new(:path => path, :catalog => catalog)
       end
     end
 
     %w[hklm\\ hklm\software\\ hklm\software\vendor\\].each do |path|
       it "should accept the unnamed (default) value: #{path}" do
-        described_class.new(:path => path)
+        described_class.new(:path => path, :catalog => catalog)
       end
     end
 
     it "should strip trailling slashes from unnamed values" do
-      described_class.new(:path => 'hklm\\software\\\\')
+      described_class.new(:path => 'hklm\\software\\\\', :catalog => catalog)
     end
 
     %w[HKEY_DYN_DATA\\ HKEY_PERFORMANCE_DATA\name].each do |path|
       it "should reject #{path} as unsupported" do
-        expect { described_class.new(:path => path) }.to raise_error(Puppet::Error, /Unsupported/)
+        expect { described_class.new(:path => path, :catalog => catalog) }.to raise_error(Puppet::Error, /Unsupported/)
       end
     end
 
     %[hklm hkcr unknown\\name unknown\\subkey\\name].each do |path|
       it "should reject #{path} as invalid" do
         pending 'wrong message'
-        expect { described_class.new(:path => path) }.should raise_error(Puppet::Error, /Invalid registry key/)
+        expect { described_class.new(:path => path, :catalog => catalog) }.should raise_error(Puppet::Error, /Invalid registry key/)
       end
     end
 
     %w[HKLM\\name HKEY_LOCAL_MACHINE\\name hklm\\name].each do |root|
       it "should canonicalize root key #{root}" do
-        value = described_class.new(:path => root)
+        value = described_class.new(:path => root, :catalog => catalog)
         value[:path].should == 'hklm\name'
       end
     end
@@ -60,12 +62,12 @@
     it 'should be case-insensitive'
     it 'should autorequire ancestor keys'
     it 'should support 32-bit values' do
-      value = described_class.new(:path => '32:hklm\software\foo')
+      value = described_class.new(:path => '32:hklm\software\foo', :catalog => catalog)
     end
   end
 
   describe "type property" do
-    let (:value) { described_class.new(:path => 'hklm\software\foo') }
+    let (:value) { described_class.new(:path => 'hklm\software\foo', :catalog => catalog) }
 
     [:string, :array, :dword, :qword, :binary, :expand].each do |type|
       it "should support a #{type.to_s} type" do
@@ -80,7 +82,7 @@
   end
 
   describe "data property" do
-    let (:value) { described_class.new(:path => 'hklm\software\foo') }
+    let (:value) { described_class.new(:path => 'hklm\software\foo', :catalog => catalog) }
 
     context "string data" do
       ['', 'foobar'].each do |data|