From b6a67edc7ea4c42767136b2360b9dc3939be242e Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Wed, 14 Sep 2011 19:34:16 +0200 Subject: [PATCH 1/4] Fix #7982 - puppet device doesn't reset all cached attributes This leads to strange errors like using the previous device SSL cert for the next device. Signed-off-by: Brice Figureau --- lib/puppet/application/device.rb | 1 + lib/puppet/ssl/host.rb | 4 ++++ spec/unit/application/device_spec.rb | 5 +++++ spec/unit/ssl/host_spec.rb | 8 +++++++- 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/puppet/application/device.rb b/lib/puppet/application/device.rb index 977c5c02391..d0b93873e33 100644 --- a/lib/puppet/application/device.rb +++ b/lib/puppet/application/device.rb @@ -196,6 +196,7 @@ def main Puppet.settings.set_value(:vardir, vardir, :cli) Puppet.settings.set_value(:confdir, confdir, :cli) Puppet.settings.set_value(:certname, certname, :cli) + Puppet::SSL::Host.reset end end end diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb index a06b1e275b9..28a0819d781 100644 --- a/lib/puppet/ssl/host.rb +++ b/lib/puppet/ssl/host.rb @@ -34,6 +34,10 @@ def self.localhost @localhost end + def self.reset + @localhost = nil + end + # This is the constant that people will use to mark that a given host is # a certificate authority. def self.ca_name diff --git a/spec/unit/application/device_spec.rb b/spec/unit/application/device_spec.rb index f88c0c3d925..43048fb55e9 100755 --- a/spec/unit/application/device_spec.rb +++ b/spec/unit/application/device_spec.rb @@ -338,6 +338,11 @@ @device.main end + it "should expire all cached attributes" do + Puppet::SSL::Host.expects(:reset).twice + + @device.main + end end end end diff --git a/spec/unit/ssl/host_spec.rb b/spec/unit/ssl/host_spec.rb index 226acdecddb..6551c8fe944 100755 --- a/spec/unit/ssl/host_spec.rb +++ b/spec/unit/ssl/host_spec.rb @@ -22,7 +22,7 @@ after do # Cleaned out any cached localhost instance. - Puppet::SSL::Host.instance_variable_set(:@localhost, nil) + Puppet::SSL::Host.reset Puppet::SSL::Host.ca_location = :none end @@ -65,6 +65,12 @@ Puppet::SSL::Host.should respond_to(:localhost) end + it "should allow to reset localhost" do + previous_host = Puppet::SSL::Host.localhost + Puppet::SSL::Host.reset + Puppet::SSL::Host.localhost.should_not == previous_host + end + it "should generate the certificate for the localhost instance if no certificate is available" do host = stub 'host', :key => nil Puppet::SSL::Host.expects(:new).returns host From 1d3a3a757ded47dc1195c91e42b580777c4dabbb Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Wed, 14 Sep 2011 19:40:54 +0200 Subject: [PATCH 2/4] Fix #9164 - allow '-' in device certificate names The regex was a little bit too strict and disallowed certnames containing a dash. Signed-off-by: Brice Figureau --- lib/puppet/util/network_device/config.rb | 2 +- spec/unit/util/network_device/config_spec.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/puppet/util/network_device/config.rb b/lib/puppet/util/network_device/config.rb index 17f4e254ce9..c1bc9c86a2c 100644 --- a/lib/puppet/util/network_device/config.rb +++ b/lib/puppet/util/network_device/config.rb @@ -51,7 +51,7 @@ def parse when /^\s*$/ # skip blank lines count += 1 next - when /^\[([\w.]+)\]\s*$/ # [device.fqdn] + when /^\[([\w.-]+)\]\s*$/ # [device.fqdn] name = $1 name.chomp! raise ConfigurationError, "Duplicate device found at line #{count}, already found at #{device.line}" if devices.include?(name) diff --git a/spec/unit/util/network_device/config_spec.rb b/spec/unit/util/network_device/config_spec.rb index d9bd3d97997..8b246777d30 100755 --- a/spec/unit/util/network_device/config_spec.rb +++ b/spec/unit/util/network_device/config_spec.rb @@ -78,6 +78,13 @@ lambda { @config.read }.should raise_error end + it "should accept device certname containing dashes" do + @fd.stubs(:each).yields('[router-1.puppetlabs.com]') + + @config.read + @config.devices.should include('router-1.puppetlabs.com') + end + it "should create a new device for each found device line" do @fd.stubs(:each).multiple_yields('[router.puppetlabs.com]', '[swith.puppetlabs.com]') From 759547511364f900d6ece94e6311c8e54cff86cd Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Wed, 14 Sep 2011 19:41:53 +0200 Subject: [PATCH 3/4] Fix device.conf error reporting The ConfigurationError class doesn't exist and during parsing of device.conf if an error is encountered it wasn't able to report it correctly. Signed-off-by: Brice Figureau --- lib/puppet/util/network_device/config.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/puppet/util/network_device/config.rb b/lib/puppet/util/network_device/config.rb index c1bc9c86a2c..afb9a150ba5 100644 --- a/lib/puppet/util/network_device/config.rb +++ b/lib/puppet/util/network_device/config.rb @@ -54,7 +54,7 @@ def parse when /^\[([\w.-]+)\]\s*$/ # [device.fqdn] name = $1 name.chomp! - raise ConfigurationError, "Duplicate device found at line #{count}, already found at #{device.line}" if devices.include?(name) + raise Puppet::Error, "Duplicate device found at line #{count}, already found at #{device.line}" if devices.include?(name) device = OpenStruct.new device.name = name device.line = count @@ -63,7 +63,7 @@ def parse when /^\s*(type|url)\s+(.+)$/ parse_directive(device, $1, $2, count) else - raise ConfigurationError, "Invalid line #{count}: #{line}" + raise Puppet::Error, "Invalid line #{count}: #{line}" end count += 1 } @@ -85,8 +85,7 @@ def parse_directive(device, var, value, count) when "url" device.url = value else - raise ConfigurationError, - "Invalid argument '#{var}' at line #{count}" + raise Puppet::Error, "Invalid argument '#{var}' at line #{count}" end end From ae74c68128647c9c0b4a71a09d6ff915c665b6b3 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Sat, 17 Sep 2011 15:29:53 +0200 Subject: [PATCH 4/4] Fix failing SSL Host test introduced by b6a67edc The fix for #7982 added a test that was failing when run as part of the full suite: 1) Puppet::SSL::Host should allow to reset localhost Failure/Error: previous_host = Puppet::SSL::Host.localhost Puppet::Error: Cannot save maynard.lan; parent directory /var/folders/hq/ hhqxfhws68bc_s23f2ktyx0m0000gp/T/ssl_host_testing20110914-2703-fzokxv-0/ ssl/private_keys does not exist # ./lib/puppet/indirector/ssl_file.rb:95:in `save' # ./lib/puppet/indirector/key/file.rb:34:in `save' # ./lib/puppet/indirector/indirection.rb:265:in `save' # ./lib/puppet/ssl/host.rb:147:in `generate_key' # ./lib/puppet/ssl/host.rb:176:in `certificate' # ./lib/puppet/ssl/host.rb:32:in `localhost' # ./spec/unit/ssl/host_spec.rb:69 This is because during the ssl_file terminus initialization in ca_spec the vardir and ssldir directories are created through the use of Puppet.settings.use. Unfortunately those "use" are reset after each test but not the fact that the terminus was initialized. Thus when running the host_spec tests, no "implicit" Puppet.settings.use are run when the terminus is first accessed. The fix is to add an explicit Puppet.settings.use to those tests. Signed-off-by: Brice Figureau --- spec/unit/ssl/host_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/unit/ssl/host_spec.rb b/spec/unit/ssl/host_spec.rb index 6551c8fe944..80f19f7c374 100755 --- a/spec/unit/ssl/host_spec.rb +++ b/spec/unit/ssl/host_spec.rb @@ -16,6 +16,7 @@ dir = tmpdir("ssl_host_testing") Puppet.settings[:confdir] = dir Puppet.settings[:vardir] = dir + Puppet.settings.use :main, :ssl @host = Puppet::SSL::Host.new("myname") end