Skip to content

Fix #7982 and some assorted network devices problems #113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeffmccune merged 4 commits into from
Sep 18, 2011
Merged

Fix #7982 and some assorted network devices problems #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6a67ed
Fix #7982 - puppet device doesn't reset all cached attributes
Sep 14, 2011
1d3a3a7
Fix #9164 - allow '-' in device certificate names
Sep 14, 2011
7595475
Fix device.conf error reporting
Sep 14, 2011
ae74c68
Fix failing SSL Host test introduced by b6a67edc
Sep 17, 2011
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/puppet/application/device.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/puppet/ssl/host.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions lib/puppet/util/network_device/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ 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)
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
Expand All @@ -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
}
Expand All @@ -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

Expand Down
5 changes: 5 additions & 0 deletions spec/unit/application/device_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 8 additions & 1 deletion spec/unit/ssl/host_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@
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

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

Expand Down Expand Up @@ -65,6 +66,12 @@
Puppet::SSL::Host.should respond_to(:localhost)
end

it "should allow to reset localhost" do
previous_host = Puppet::SSL::Host.localhost
Copy link
Copy Markdown
Contributor

@jeffmccune jeffmccune Sep 15, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masterzen, when I run the rspec tests I'm getting:

Failures:

  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

Finished in 86.56 seconds
13046 examples, 1 failure, 90 pending

Failed examples:

rspec ./spec/unit/ssl/host_spec.rb:68 # Puppet::SSL::Host should allow to reset localhost
rspec spec/**/*_spec.rb  80.95s user 7.84s system 97% cpu 1:31.54 total

Do you have any idea of how to make this more robust? Unfortunately none jump out at me at the moment.

Copy link
Copy Markdown
Contributor Author

@masterzen masterzen Sep 15, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my bad. The test passes fine when run isolated but combined in the whole suite it fails. Something prevents the directory to be created apparently.

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
Expand Down
7 changes: 7 additions & 0 deletions spec/unit/util/network_device/config_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]')

Expand Down