diff --git a/acceptance/tests/resource/registry/should_tolerate_mixed_case.rb b/acceptance/tests/resource/registry/should_tolerate_mixed_case.rb new file mode 100644 index 00000000..4bdd472b --- /dev/null +++ b/acceptance/tests/resource/registry/should_tolerate_mixed_case.rb @@ -0,0 +1,161 @@ +require 'pathname' +require Pathname.new(__FILE__).dirname.dirname.dirname.dirname + 'lib/systest/util/registry' +# Include our utility methods in the singleton class of the test case instance. +class << self + include Systest::Util::Registry +end + +test_name "Registry Value Management (Mixed Case)" + +# JJM - The whole purpose of this test is to exercise situations where the user +# specifies registry valies in a case-insensitive but case-preserving way. +# This has particular "gotchas" with regard to autorequire. +# +# Note how SUBKEY1 is capitalized in some resources but downcased in other +# resources. On windows these refer to the same thing, and case is preserved. +# In puppet, however, resource namevars are case sensisitve unless otherwise +# noted. + +# Generate a unique key name +keyname = "PuppetLabsTest_MixedCase_#{randomstring(8)}" +# This is the keypath we'll use for this entire test. We will actually create this key and delete it. +vendor_path = "HKLM\\Software\\Vendor" +keypath = "#{vendor_path}\\#{keyname}" + +master_manifest_content = < "fact_phase: $fact_phase" } + +registry_key { '#{vendor_path}': ensure => present } +registry_key { '32:#{vendor_path}': ensure => present } + +class phase1 { + Registry_key { ensure => present } + registry_key { '#{keypath}': } + registry_key { '#{keypath}\\SUBKEY1': } + # NOTE THE DIFFERENCE IN CASE IN SubKey1 and SUBKEY1 above + registry_key { '#{keypath}\\SubKey1\\SUBKEY2': } + registry_key { '32:#{keypath}': } + registry_key { '32:#{keypath}\\SUBKEY1': } + registry_key { '32:#{keypath}\\SubKey1\\SUBKEY2': } + + # The Default Value + # NOTE THE DIFFERENCE IN CASE IN SubKey1 and SUBKEY1 above + registry_value { '#{keypath}\\SubKey1\\\\': + data => "Default Data", + } + + # String Values + registry_value { '#{keypath}\\SubKey1\\ValueString1': + data => "Should be a string", + } + registry_value { '#{keypath}\\SubKey1\\ValueString2': + type => string, + data => "Should be a string", + } + registry_value { '#{keypath}\\SubKey1\\ValueString3': + ensure => present, + type => string, + data => "Should be a string", + } + registry_value { '#{keypath}\\SubKey1\\ValueString4': + data => "Should be a string", + type => string, + ensure => present, + } + registry_value { '#{keypath}\\SubKey1\\SubKey2\\ValueString1': + data => "Should be a string", + } + registry_value { '#{keypath}\\SubKey1\\SubKey2\\ValueString2': + type => string, + data => "Should be a string", + } + registry_value { '#{keypath}\\SubKey1\\SubKey2\\ValueString3': + ensure => present, + type => string, + data => "Should be a string", + } + registry_value { '#{keypath}\\SubKey1\\SubKey2\\ValueString4': + data => "Should be a string", + type => string, + ensure => present, + } + + # The Default Value + # NOTE THE DIFFERENCE IN CASE IN SubKey1 and SUBKEY1 above + registry_value { '32:#{keypath}\\SubKey1\\\\': + data => "Default Data", + } + + # String Values + registry_value { '32:#{keypath}\\SubKey1\\ValueString1': + data => "Should be a string", + } + registry_value { '32:#{keypath}\\SubKey1\\ValueString2': + type => string, + data => "Should be a string", + } + registry_value { '32:#{keypath}\\SubKey1\\ValueString3': + ensure => present, + type => string, + data => "Should be a string", + } + registry_value { '32:#{keypath}\\SubKey1\\ValueString4': + data => "Should be a string", + type => string, + ensure => present, + } + registry_value { '32:#{keypath}\\SubKey1\\SubKey2\\ValueString1': + data => "Should be a string", + } + registry_value { '32:#{keypath}\\SubKey1\\SubKey2\\ValueString2': + type => string, + data => "Should be a string", + } + registry_value { '32:#{keypath}\\SubKey1\\SubKey2\\ValueString3': + ensure => present, + type => string, + data => "Should be a string", + } + registry_value { '32:#{keypath}\\SubKey1\\SubKey2\\ValueString4': + data => "Should be a string", + type => string, + ensure => present, + } +} + +case $fact_phase { + default: { include phase1 } +} +HERE + +# Setup the master to use the modules specified in the --modules option +setup_master master_manifest_content + +step "Start the master" do + with_master_running_on(master, master_options) do + # A set of keys we expect Puppet to create + phase1_resources_created = [ + /Registry_key\[HKLM.Software.Vendor.PuppetLabsTest\w+\].ensure: created/, + /Registry_key\[HKLM.Software.Vendor.PuppetLabsTest\w+\\SUBKEY1\].ensure: created/, + /Registry_key\[32:HKLM.Software.Vendor.PuppetLabsTest\w+\].ensure: created/, + /Registry_key\[32:HKLM.Software.Vendor.PuppetLabsTest\w+\\SUBKEY1\].ensure: created/, + /Registry_value\[HKLM.Software.Vendor.PuppetLabsTest\w+\\SubKey1\\\\\].ensure: created/, + /Registry_value\[HKLM.Software.Vendor.PuppetLabsTest\w+\\SubKey1\\ValueString1\].ensure: created/, + /Registry_value\[HKLM.Software.Vendor.PuppetLabsTest\w+\\SubKey1\\ValueString2\].ensure: created/, + /Registry_value\[HKLM.Software.Vendor.PuppetLabsTest\w+\\SubKey1\\ValueString3\].ensure: created/, + /Registry_value\[HKLM.Software.Vendor.PuppetLabsTest\w+\\SubKey1\\ValueString4\].ensure: created/, + ] + + windows_agents.each do |agent| + this_agent_args = agent_args % get_test_file_path(agent, agent_lib_dir) + + step "Registry Tolerate Mixed Case Values - Phase 1.a - Create some values" + on agent, puppet_agent(this_agent_args, :environment => { 'FACTER_FACT_PHASE' => '1' }), :acceptable_exit_codes => agent_exit_codes do + phase1_resources_created.each do |val_re| + assert_match(val_re, result.stdout, "Expected output to contain #{val_re.inspect}.") + end + assert_no_match(/err:/, result.stdout, "Expected no error messages.") + end + end + end +end diff --git a/lib/puppet/type/registry_key.rb b/lib/puppet/type/registry_key.rb index 2f1455b1..a5711ec0 100644 --- a/lib/puppet/type/registry_key.rb +++ b/lib/puppet/type/registry_key.rb @@ -20,7 +20,12 @@ def self.title_patterns Puppet::Modules::Registry::RegistryKeyPath.new(path).valid? end munge do |path| - Puppet::Modules::Registry::RegistryKeyPath.new(path).canonical + canonical = Puppet::Modules::Registry::RegistryKeyPath.new(path).canonical + # 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 end end @@ -59,8 +64,11 @@ def self.title_patterns autorequire(:registry_key) do req = [] path = Puppet::Modules::Registry::RegistryKeyPath.new(value(:path)) - if found = path.enum_for(:ascend).find { |p| catalog.resource(:registry_key, p.to_s) } - req << found.to_s + # 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. + if found = path.enum_for(:ascend).find { |p| catalog.resource(:registry_key, p.to_s.downcase) } + req << found.to_s.downcase end req end diff --git a/lib/puppet/type/registry_value.rb b/lib/puppet/type/registry_value.rb index 0533e839..ed67570d 100644 --- a/lib/puppet/type/registry_value.rb +++ b/lib/puppet/type/registry_value.rb @@ -20,7 +20,12 @@ def self.title_patterns Puppet::Modules::Registry::RegistryValuePath.new(path).valid? end munge do |path| - Puppet::Modules::Registry::RegistryValuePath.new(path).canonical + canonical = Puppet::Modules::Registry::RegistryValuePath.new(path).canonical + # 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 end end @@ -97,8 +102,11 @@ def change_to_s(currentvalue, newvalue) autorequire(:registry_key) do req = [] path = Puppet::Modules::Registry::RegistryKeyPath.new(value(:path)) - if found = path.enum_for(:ascend).find { |p| catalog.resource(:registry_key, p.to_s) } - req << found.to_s + # 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. + if found = path.enum_for(:ascend).find { |p| catalog.resource(:registry_key, p.to_s.downcase) } + req << found.to_s.downcase end req end diff --git a/spec/unit/puppet/type/registry_key_spec.rb b/spec/unit/puppet/type/registry_key_spec.rb index 296c4581..ea9b11d3 100644 --- a/spec/unit/puppet/type/registry_key_spec.rb +++ b/spec/unit/puppet/type/registry_key_spec.rb @@ -66,11 +66,17 @@ context "purging" do 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 + before :each do key[:purge_values] = true catalog.add_resource(key) - catalog.add_resource(Puppet::Type.type(:registry_value).new(:path => "#{key[:path]}\\val1")) - catalog.add_resource(Puppet::Type.type(:registry_value).new(:path => "#{key[:path]}\\val2")) + catalog.add_resource(Puppet::Type.type(:registry_value).new(:path => "#{key[:path]}\\val1", :catalog => catalog)) + catalog.add_resource(Puppet::Type.type(:registry_value).new(:path => "#{key[:path]}\\val2", :catalog => catalog)) end it "should return an empty array if the key doesn't have any values" do @@ -92,4 +98,36 @@ end end end + + describe "#autorequire" do + let :the_catalog do + Puppet::Resource::Catalog.new + end + + let(:the_resource_name) { 'HKLM\Software\Vendor\PuppetLabs' } + + let :the_resource do + # JJM Holy cow this is an intertangled mess. ;) + resource = Puppet::Type.type(:registry_key).new(:name => the_resource_name, :catalog => the_catalog) + the_catalog.add_resource resource + resource + end + + it 'Should initialize the catalog instance variable' do + the_resource.catalog.must be the_catalog + end + + it 'Should allow case insensitive lookup using the downcase path' do + the_resource.must be the_catalog.resource(:registry_key, the_resource_name.downcase) + end + + it 'Should preserve the case of the user specified path' do + the_resource.must be the_catalog.resource(:registry_key, the_resource_name) + end + + it 'Should return the same resource regardless of the alias used' do + the_resource.must be the_catalog.resource(:registry_key, the_resource_name) + the_resource.must be the_catalog.resource(:registry_key, the_resource_name.downcase) + end + end end