diff options
92 files changed, 2486 insertions, 355 deletions
diff --git a/acceptance/tests/jeff_append_to_array.rb b/acceptance/tests/jeff_append_to_array.rb index 415d59fe8..20f43665e 100644 --- a/acceptance/tests/jeff_append_to_array.rb +++ b/acceptance/tests/jeff_append_to_array.rb @@ -12,8 +12,9 @@ manifest = %q{ include parent::child } -apply_manifest_on(agents, manifest) do - stdout =~ /notice: parent array element/ or fail_test("parent missing") - stdout =~ /notice: child array element/ or fail_test("child missing") +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_match(/notice: parent array element/, stdout, "#{host}: parent missing") + assert_match(/notice: child array element/, stdout, "#{host}: child missing") + end end - diff --git a/acceptance/tests/puppet_apply_a_file_should_create_a_file_and_report_the_md5.rb b/acceptance/tests/puppet_apply_a_file_should_create_a_file_and_report_the_md5.rb index abb06fbaf..44338520a 100644 --- a/acceptance/tests/puppet_apply_a_file_should_create_a_file_and_report_the_md5.rb +++ b/acceptance/tests/puppet_apply_a_file_should_create_a_file_and_report_the_md5.rb @@ -6,10 +6,11 @@ manifest = "file{'#{file}': content => 'test'}" step "clean up #{file} for testing" on agents, "rm -f #{file}" -step "run the manifest and verify MD5 was printed" -apply_manifest_on(agents, manifest) do - fail_test "didn't find the content MD5 on output" unless - stdout.include? "defined content as '{md5}098f6bcd4621d373cade4e832627b4f6'" +step "Run the manifest and verify MD5 was printed" +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_match(/defined content as '{md5}098f6bcd4621d373cade4e832627b4f6'/, stdout, "#{host}: didn't find the content MD5 on output") + end end step "clean up #{file} after testing" diff --git a/acceptance/tests/puppet_apply_basics.rb b/acceptance/tests/puppet_apply_basics.rb index bbbdefc15..23c57d84d 100644 --- a/acceptance/tests/puppet_apply_basics.rb +++ b/acceptance/tests/puppet_apply_basics.rb @@ -5,11 +5,13 @@ test_name "Trivial puppet tests" step "check that puppet apply displays notices" -apply_manifest_on(agents, "notice 'Hello World'") do - stdout =~ /notice:.*Hello World/ or fail_test("missing notice!") +agents.each do |host| + apply_manifest_on(host, "notice 'Hello World'") do + assert_match(/notice:.*Hello World/, stdout, "#{host}: missing notice!") + end end step "verify help displays something for puppet master" on master, puppet_master("--help") do - stdout =~ /puppet master/ or fail_test("improper help output") + assert_match(/puppet master/, stdout, "improper help output") end diff --git a/acceptance/tests/puppet_apply_should_show_a_notice.rb b/acceptance/tests/puppet_apply_should_show_a_notice.rb index af6f41ca7..757d29bbf 100644 --- a/acceptance/tests/puppet_apply_should_show_a_notice.rb +++ b/acceptance/tests/puppet_apply_should_show_a_notice.rb @@ -1,5 +1,7 @@ test_name "puppet apply should show a notice" -apply_manifest_on(agents, "notice 'Hello World'") do - fail_test "the notice didn't show" unless - stdout =~ /notice: .*: Hello World/ + +agents.each do |host| + apply_manifest_on(host, "notice 'Hello World'") do + assert_match(/notice: .*: Hello World/, stdout, "#{host}: the notice didn't show") + end end diff --git a/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb b/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb index 127e943a9..9f0fdc5c7 100755 --- a/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb +++ b/acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb @@ -4,8 +4,7 @@ step "Validate services running agreement ralsh vs. OS service count" # ticket_4123_should_list_all_running_redhat.sh hosts.each do |host| - if host['platform'].include?('centos') or host['platform'].include?('redhat') - puts "XXX #{host['platform']}" + if host['platform'].include?('centos') or host['platform'].include?('rhel') run_script_on(host,'acceptance-tests/tests/resource/service/ticket_4123_should_list_all_running_redhat.sh') else skip_test "Test not supported on this plaform" diff --git a/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb b/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb index db96ad91c..13ad5ceac 100755 --- a/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb +++ b/acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.rb @@ -4,7 +4,7 @@ step "Validate disabled services agreement ralsh vs. OS service count" # ticket_4124_should_list_all_disabled.sh hosts.each do |host| - unless host['platform'].include? 'centos' or host['platform'].include? 'redhat' + unless host['platform'].include? 'centos' or host['platform'].include? 'rhel' skip_test "Test not supported on this plaform" else run_script_on(host,'acceptance-tests/tests/resource/service/ticket_4124_should_list_all_disabled.sh') diff --git a/acceptance/tests/ticket_3172_puppet_kick_with_hostnames_on_the_command_line.rb b/acceptance/tests/ticket_3172_puppet_kick_with_hostnames_on_the_command_line.rb index 436ce29fe..24dd7256b 100644 --- a/acceptance/tests/ticket_3172_puppet_kick_with_hostnames_on_the_command_line.rb +++ b/acceptance/tests/ticket_3172_puppet_kick_with_hostnames_on_the_command_line.rb @@ -2,6 +2,8 @@ test_name "#3172: puppet kick with hostnames on the command line" step "verify that we trigger our host" target = 'working.example.org' -on(agents, puppet_kick(target), :acceptable_exit_codes => [3]) { - fail_test "didn't trigger #{target}" unless stdout.include? "Triggering #{target}" -} +agents.each do |host| + on(host, puppet_kick(target), :acceptable_exit_codes => [3]) { + assert_match(/Triggering #{target}/, stdout, "didn't trigger #{target} on #{host}" ) + } +end diff --git a/acceptance/tests/ticket_4059_ralsh_can_change_settings.rb b/acceptance/tests/ticket_4059_ralsh_can_change_settings.rb index c97bbdbe6..83f5899f2 100644 --- a/acceptance/tests/ticket_4059_ralsh_can_change_settings.rb +++ b/acceptance/tests/ticket_4059_ralsh_can_change_settings.rb @@ -11,9 +11,10 @@ on(agents, puppet_resource(content)) do stdout.index('Host[example.com]/ensure: created') or fail_test("missing notice about host record creation") end -on(agents, "cat #{target}") do - stdout =~ /^127\.0\.0\.1\s+example\.com/ or - fail_test("missing host record in #{target}") +agents.each do |host| + on(host, "cat #{target}") do + assert_match(/^127\.0\.0\.1\s+example\.com/, stdout, "missing host record in #{target} on #{host}") + end end step "cleanup at the end of the test" diff --git a/acceptance/tests/ticket_4110_puppet_apply_should_not_create_a_user_that_already_exists.rb b/acceptance/tests/ticket_4110_puppet_apply_should_not_create_a_user_that_already_exists.rb index 9704250d9..147857362 100644 --- a/acceptance/tests/ticket_4110_puppet_apply_should_not_create_a_user_that_already_exists.rb +++ b/acceptance/tests/ticket_4110_puppet_apply_should_not_create_a_user_that_already_exists.rb @@ -1,5 +1,7 @@ test_name "#4110: puppet apply should not create a user that already exists" -apply_manifest_on(agents, "user { 'root': ensure => 'present' }") do - fail_test("we tried to create root on this host") if stdout =~ /created/ +agents.each do |host| + apply_manifest_on(host, "user { 'root': ensure => 'present' }") do + assert_no_match(/created/, stdout, "we tried to create root on #{host}" ) + end end diff --git a/acceptance/tests/ticket_4233_resource_with_a_newline.rb b/acceptance/tests/ticket_4233_resource_with_a_newline.rb index 7bb7dc3c3..11924b550 100644 --- a/acceptance/tests/ticket_4233_resource_with_a_newline.rb +++ b/acceptance/tests/ticket_4233_resource_with_a_newline.rb @@ -8,6 +8,9 @@ test_name "#4233: resource with a newline" # and 2.6.0 final to not return an error line. # Look for the line in the output and fail the test # if we find it. -apply_manifest_on(agents, 'exec { \'/bin/echo -e "\nHello World\n"\': }') do - stdout =~ /err:/ and fail_test("error report in output, sorry") + +agents.each do |host| + apply_manifest_on(host, 'exec { \'/bin/echo -e "\nHello World\n"\': }') do + assert_no_match(/err:/, stdout, "error report in output on #{host}") + end end diff --git a/acceptance/tests/ticket_4285_file_resource_fail_when_name_defined_instead_of_path.rb b/acceptance/tests/ticket_4285_file_resource_fail_when_name_defined_instead_of_path.rb index d2297fbc4..c1cdb0b9d 100644 --- a/acceptance/tests/ticket_4285_file_resource_fail_when_name_defined_instead_of_path.rb +++ b/acceptance/tests/ticket_4285_file_resource_fail_when_name_defined_instead_of_path.rb @@ -12,6 +12,8 @@ manifest = %q{ } } -apply_manifest_on(agents, manifest) do - fail_test "found the bug report output" if stdout =~ /Cannot alias/ +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_no_match(/Cannot alias/, stdout, "#{host}: found the bug report output") + end end diff --git a/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb b/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb index e9a17df36..f5a1c1685 100644 --- a/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb +++ b/acceptance/tests/ticket_4287_undefined_method_evaluate_match_when_function_call_used_in_an_if_statement.rb @@ -7,7 +7,8 @@ manifest = %q{ } } -apply_manifest_on(agents, manifest) do - fail_test "didn't get the expected notice" unless - stdout.include? 'notice: No issue here...' +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_match(/notice: No issue here.../, stdout, "didn't get the expected notice on #{host}") + end end diff --git a/acceptance/tests/ticket_4289_facter_should_recognize_OEL_operatingsystemrelease.rb b/acceptance/tests/ticket_4289_facter_should_recognize_OEL_operatingsystemrelease.rb index 653fcb274..cacb7db65 100644 --- a/acceptance/tests/ticket_4289_facter_should_recognize_OEL_operatingsystemrelease.rb +++ b/acceptance/tests/ticket_4289_facter_should_recognize_OEL_operatingsystemrelease.rb @@ -16,6 +16,6 @@ agents.each do |host| if stdout =~ /oel/i then step "test operatingsystemrelease fact on OEL host #{host}" on host, facter("operatingsystemrelease") - stdout =~ /^\d\.\d$/ or fail_test "operatingsystemrelease not as expected" + assert_match(/^\d\.\d$/, stdout, "operatingsystemrelease not as expected on #{host}") end end diff --git a/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb b/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb index 830da99b4..aa42fd401 100644 --- a/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb +++ b/acceptance/tests/ticket_4293_define_and_use_a_define_within_a_class.rb @@ -16,7 +16,8 @@ class foo { include foo PP -apply_manifest_on(agents, manifest) do - stdout =~ /notice.*?Foo::Do_notify.*?a_message_for_you/ or - fail_test("the notification didn't show up in stdout") +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_match(/notice.*?Foo::Do_notify.*?a_message_for_you/, stdout, "the notification didn't show up in stdout on #{host}") + end end diff --git a/acceptance/tests/ticket_5477_master_not_dectect_sitepp.rb b/acceptance/tests/ticket_5477_master_not_dectect_sitepp.rb index d8723b2ec..f774eca05 100644 --- a/acceptance/tests/ticket_5477_master_not_dectect_sitepp.rb +++ b/acceptance/tests/ticket_5477_master_not_dectect_sitepp.rb @@ -24,7 +24,10 @@ with_master_running_on(master, "--manifest #{manifest_file} --certdnsnames=\"pup sleep 3 step "Agent: puppet agent --test" - on agents, puppet_agent("--test --server #{master}"), :acceptable_exit_codes => [2] do - fail_test "Site.pp not detect at Master?" unless stdout.include? 'ticket_5477_notify' + + agents.each do |host| + on(host, puppet_agent("--test --server #{master}"), :acceptable_exit_codes => [2]) do + assert_match(/ticket_5477_notify/, stdout, "#{host}: Site.pp not detected on Puppet Master") + end end end diff --git a/acceptance/tests/ticket_6541_invalid_filebucket_files.rb b/acceptance/tests/ticket_6541_invalid_filebucket_files.rb index 25bcff452..33b985b67 100644 --- a/acceptance/tests/ticket_6541_invalid_filebucket_files.rb +++ b/acceptance/tests/ticket_6541_invalid_filebucket_files.rb @@ -4,23 +4,26 @@ apply_manifest_on(agents, manifest) test_name "verify invalid hashes should not change the file" manifest = "file { '/tmp/6541': content => '{md5}notahash' }" -apply_manifest_on(agents, manifest) do - fail_test "shouldn't have overwrote the file" if - stdout =~ /content changed/ +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_no_match(/content changed/, stdout, "#{host}: shouldn't have overwrote the file") + end end test_name "verify valid but unbucketed hashes should not change the file" manifest = "file { '/tmp/6541': content => '{md5}13ad7345d56b566a4408ffdcd877bc78' }" -apply_manifest_on(agents, manifest) do - fail_test "shouldn't have overwrote the file" if - stdout =~ /content changed/ +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_no_match(/content changed/, stdout, "#{host}: shouldn't have overwrote the file") + end end on(agents, puppet_filebucket("backup -l /dev/null") ) test_name "verify that an empty file can be retrieved from the filebucket" manifest = "file { '/tmp/6541': content => '{md5}d41d8cd98f00b204e9800998ecf8427e' }" -apply_manifest_on(agents, manifest) do - fail_test "shouldn't have overwrote the file" unless - stdout =~ /content changed '\{md5\}552e21cd4cd9918678e3c1a0df491bc3' to '\{md5\}d41d8cd98f00b204e9800998ecf8427e'/ +agents.each do |host| + apply_manifest_on(host, manifest) do + assert_match(/content changed '\{md5\}552e21cd4cd9918678e3c1a0df491bc3' to '\{md5\}d41d8cd98f00b204e9800998ecf8427e'/, stdout, "#{host}: shouldn't have overwrote the file") + end end diff --git a/acceptance/tests/ticket_6734_6256_5530_5503.rb b/acceptance/tests/ticket_6734_6256_5530_5503.rb index 72bb592d8..b5d7aaaf3 100644 --- a/acceptance/tests/ticket_6734_6256_5530_5503.rb +++ b/acceptance/tests/ticket_6734_6256_5530_5503.rb @@ -7,6 +7,6 @@ test_name "Tickets 6734 6256 5530 5503i Puppet Master fails to start" with_master_running_on(master) do step "Check permissions on puppet/rrd/" on master, "ls -l /var/lib/puppet | grep rrd | awk '{print $3\" \"$4}'" do - fail_test "puppet/rrd does not exist/wrong permission" unless stdout.include? 'puppet puppet' + assert_match(/puppet puppet/, stdout, "puppet/rrd does not exist/wrong permissions") end end diff --git a/acceptance/tests/ticket_6857_password-disclosure-when-changing-a-users-password.rb b/acceptance/tests/ticket_6857_password-disclosure-when-changing-a-users-password.rb new file mode 100644 index 000000000..d3b415691 --- /dev/null +++ b/acceptance/tests/ticket_6857_password-disclosure-when-changing-a-users-password.rb @@ -0,0 +1,33 @@ +test_name "#6857: redact password hashes when applying in noop mode" + +hosts_to_test = agents.reject do |agent| + if agent['platform'].match /(?:ubuntu|centos|debian|rhel)/ + result = on(agent, %q{ruby -e 'require "shadow" or raise'}, :silent => true) + result.exit_code != 0 + else + false + end +end +skip_test "No suitable hosts found" if hosts_to_test.empty? + +adduser_manifest = <<MANIFEST +user { 'passwordtestuser': + ensure => 'present', + password => 'apassword', +} +MANIFEST + +changepass_manifest = <<MANIFEST +user { 'passwordtestuser': + ensure => 'present', + password => 'newpassword', + noop => true, +} +MANIFEST + +apply_manifest_on(hosts_to_test, adduser_manifest ) +results = apply_manifest_on(hosts_to_test, changepass_manifest ) + +results.each do |result| + assert_match( /current_value \[old password hash redacted\], should be \[new password hash redacted\]/ , "#{result.host}: #{result.stdout}" ) +end diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb index f0442648b..ea7cbdfb5 100644 --- a/lib/puppet/application/agent.rb +++ b/lib/puppet/application/agent.rb @@ -187,10 +187,10 @@ configuration options can also be generated by running puppet agent with should always at least contain MD5, MD2, SHA1 and SHA256. * --detailed-exitcodes: - Provide transaction information via exit codes. If this is enabled, an - exit code of '2' means there were changes, and an exit code of '4' - means that there were failures during the transaction. This option - only makes sense in conjunction with --onetime. + Provide transaction information via exit codes. If this is enabled, an exit + code of '2' means there were changes, an exit code of '4' means there were + failures during the transaction, and an exit code of '6' means there were both + changes and failures. * --disable: Disable working on the local system. This puts a lock file in place, diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb index 5562a9b09..200309b7d 100644 --- a/lib/puppet/application/apply.rb +++ b/lib/puppet/application/apply.rb @@ -82,9 +82,10 @@ configuration options can also be generated by running puppet with Enable full debugging. * --detailed-exitcodes: - Provide transaction information via exit codes. If this is enabled, an - exit code of '2' means there were changes, and an exit code of '4' - means that there were failures during the transaction. + Provide transaction information via exit codes. If this is enabled, an exit + code of '2' means there were changes, an exit code of '4' means there were + failures during the transaction, and an exit code of '6' means there were both + changes and failures. * --help: Print this help message diff --git a/lib/puppet/application/ca.rb b/lib/puppet/application/ca.rb new file mode 100644 index 000000000..d1ec2502e --- /dev/null +++ b/lib/puppet/application/ca.rb @@ -0,0 +1,5 @@ +require 'puppet/application/face_base' + +class Puppet::Application::Ca < Puppet::Application::FaceBase + run_mode :master +end diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb index 162672b6a..330fba8bd 100644 --- a/lib/puppet/application/cert.rb +++ b/lib/puppet/application/cert.rb @@ -218,7 +218,8 @@ Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License if sub = self.command_line.args.shift then self.subcommand = sub else - help + puts help + exit end end result diff --git a/lib/puppet/application/certificate.rb b/lib/puppet/application/certificate.rb index eacb830b2..de5b2c499 100644 --- a/lib/puppet/application/certificate.rb +++ b/lib/puppet/application/certificate.rb @@ -2,11 +2,6 @@ require 'puppet/application/indirection_base' class Puppet::Application::Certificate < Puppet::Application::IndirectionBase def setup - unless options[:ca_location] - raise ArgumentError, "You must have a CA location specified;\n" + - "use --ca-location to specify the location (remote, local, only)" - end - location = Puppet::SSL::Host.ca_location if location == :local && !Puppet::SSL::CertificateAuthority.ca? self.class.run_mode("master") diff --git a/lib/puppet/application/device.rb b/lib/puppet/application/device.rb index 3e2dec98c..977c5c023 100644 --- a/lib/puppet/application/device.rb +++ b/lib/puppet/application/device.rb @@ -113,10 +113,10 @@ parameter, so you can specify '--server <servername>' as an argument. Enable full debugging. * --detailed-exitcodes: - Provide transaction information via exit codes. If this is enabled, an - exit code of '2' means there were changes, and an exit code of '4' means - that there were failures during the transaction. This option only makes - sense in conjunction with --onetime. + Provide transaction information via exit codes. If this is enabled, an exit + code of '2' means there were changes, an exit code of '4' means there were + failures during the transaction, and an exit code of '6' means there were both + changes and failures. * --help: Print this help message diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb index ea5ba4aaf..a111518f1 100644 --- a/lib/puppet/application/face_base.rb +++ b/lib/puppet/application/face_base.rb @@ -100,7 +100,8 @@ class Puppet::Application::FaceBase < Puppet::Application # action object it represents; if this is an invalid action name that # will be nil, and handled later. action_name = item.to_sym - @action = @face.get_action(action_name) + @action = Puppet::Face.find_action(@face.name, action_name) + @face = @action.face if @action end end diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index 30865cfc1..b5a4ac872 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -1,6 +1,4 @@ -require 'puppet' require 'puppet/application' -require 'puppet/file_bucket/dipper' class Puppet::Application::Inspect < Puppet::Application @@ -98,6 +96,11 @@ Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License Puppet::Resource::Catalog.indirection.terminus_class = :yaml end + def preinit + require 'puppet' + require 'puppet/file_bucket/dipper' + end + def run_command benchmark(:notice, "Finished inspection") do retrieval_starttime = Time.now diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 07442d0e9..2247634b3 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -466,7 +466,7 @@ module Puppet :desc => "The directory in which to store reports received from the client. Each client gets a separate subdirectory."}, - :reporturl => ["http://localhost:3000/reports", + :reporturl => ["http://localhost:3000/reports/upload", "The URL used by the http reports processor to send reports"], :fileserverconfig => ["$confdir/fileserver.conf", "Where the fileserver configuration is stored."], :strict_hostname_checking => [false, "Whether to only search for the complete diff --git a/lib/puppet/face/ca.rb b/lib/puppet/face/ca.rb new file mode 100644 index 000000000..00591d637 --- /dev/null +++ b/lib/puppet/face/ca.rb @@ -0,0 +1,233 @@ +require 'puppet/face' + +Puppet::Face.define(:ca, '0.1.0') do + copyright "Puppet Labs", 2011 + license "Apache 2 license; see COPYING" + + summary "Local Puppet Certificate Authority management." + + description <<-TEXT + This provides local management of the Puppet Certificate Authority. + + You can use this subcommand to sign outstanding certificate requests, list + and manage local certificates, and inspect the state of the CA. + TEXT + + action :list do + summary "List certificates and/or certificate requests." + + description <<-TEXT + This will list the current certificates and certificate signing requests + in the Puppet CA. You will also get the fingerprint, and any certificate + verification failure reported. + TEXT + + option "--[no-]all" do + summary "Include all certificates and requests." + end + + option "--[no-]pending" do + summary "Include pending certificate signing requests." + end + + option "--[no-]signed" do + summary "Include signed certificates." + end + + option "--subject PATTERN" do + summary "Only list if the subject matches PATTERN." + + description <<-TEXT + Only include certificates or requests where subject matches PATTERN. + + PATTERN is interpreted as a regular expression, allowing complex + filtering of the content. + TEXT + end + + when_invoked do |options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + pattern = options[:subject].nil? ? nil : + Regexp.new(options[:subject], Regexp::IGNORECASE) + + pending = options[:pending].nil? ? options[:all] : options[:pending] + signed = options[:signed].nil? ? options[:all] : options[:signed] + + # By default we list pending, so if nothing at all was requested... + unless pending or signed then pending = true end + + hosts = [] + + pending and hosts += ca.waiting? + signed and hosts += ca.list + + pattern and hosts = hosts.select {|hostname| pattern.match hostname } + + hosts.sort.map {|host| Puppet::SSL::Host.new(host) } + end + + when_rendering :console do |hosts| + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + length = hosts.map{|x| x.name.length }.max + 1 + + hosts.map do |host| + name = host.name.ljust(length) + if host.certificate_request then + " #{name} (#{host.certificate_request.fingerprint})" + else + begin + ca.verify(host.certificate) + "+ #{name} (#{host.certificate.fingerprint})" + rescue Puppet::SSL::CertificateAuthority::CertificateVerificationError => e + "- #{name} (#{host.certificate.fingerprint}) (#{e.to_s})" + end + end + end.join("\n") + end + end + + action :destroy do + when_invoked do |host, options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + ca.destroy host + end + end + + action :revoke do + when_invoked do |host, options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + begin + ca.revoke host + rescue ArgumentError => e + # This is a bit naff, but it makes the behaviour consistent with the + # destroy action. The underlying tools could be nicer for that sort + # of thing; they have fairly inconsistent reporting of failures. + raise unless e.to_s =~ /Could not find a serial number for / + "Nothing was revoked" + end + end + end + + action :generate do + when_invoked do |host, options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + begin + ca.generate host + rescue RuntimeError => e + if e.to_s =~ /already has a requested certificate/ + "#{host} already has a certificate request; use sign instead" + else + raise + end + rescue ArgumentError => e + if e.to_s =~ /A Certificate already exists for / + "#{host} already has a certificate" + else + raise + end + end + end + end + + action :sign do + when_invoked do |host, options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + begin + ca.sign host + rescue ArgumentError => e + if e.to_s =~ /Could not find certificate request/ + e.to_s + else + raise + end + end + end + end + + action :print do + when_invoked do |host, options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + ca.print host + end + end + + action :fingerprint do + option "--digest ALGORITHM" do + summary "The hash algorithm to use when displaying the fingerprint" + end + + when_invoked do |host, options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + begin + # I want the default from the CA, not to duplicate it, but passing + # 'nil' explicitly means that we don't get that. This works... + if options.has_key? :digest + ca.fingerprint host, options[:digest] + else + ca.fingerprint host + end + rescue ArgumentError => e + raise unless e.to_s =~ /Could not find a certificate or csr for/ + nil + end + end + end + + action :verify do + when_invoked do |host, options| + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? + unless ca = Puppet::SSL::CertificateAuthority.instance + raise "Unable to fetch the CA" + end + + begin + ca.verify host + { :host => host, :valid => true } + rescue ArgumentError => e + raise unless e.to_s =~ /Could not find a certificate for/ + { :host => host, :valid => false, :error => e.to_s } + rescue Puppet::SSL::CertificateAuthority::CertificateVerificationError => e + { :host => host, :valid => false, :error => e.to_s } + end + end + + when_rendering :console do |value| + if value[:valid] + nil + else + "Could not verify #{value[:host]}: #{value[:error]}" + end + end + end +end diff --git a/lib/puppet/face/certificate.rb b/lib/puppet/face/certificate.rb index 9a306da37..8019b6bea 100644 --- a/lib/puppet/face/certificate.rb +++ b/lib/puppet/face/certificate.rb @@ -6,7 +6,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do license "Apache 2 license; see COPYING" summary "Provide access to the CA for certificate management." - description <<-'EOT' + description <<-EOT This subcommand interacts with a local or remote Puppet certificate authority. Currently, its behavior is not a full superset of `puppet cert`; specifically, it is unable to mimic puppet cert's "clean" option, @@ -15,8 +15,9 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do EOT option "--ca-location LOCATION" do + required summary "Which certificate authority to use (local or remote)." - description <<-'EOT' + description <<-EOT Whether to act on the local certificate authority or one provided by a remote puppet master. Allowed values are 'local' and 'remote.' @@ -24,6 +25,9 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do EOT before_action do |action, args, options| + unless [:remote, :local, :only].include? options[:ca_location].to_sym + raise ArgumentError, "Valid values for ca-location are 'remote', 'local', 'only'." + end Puppet::SSL::Host.ca_location = options[:ca_location].to_sym end end @@ -32,7 +36,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do summary "Generate a new certificate signing request." arguments "<host>" returns "Nothing." - description <<-'EOT' + description <<-EOT Generates and submits a certificate signing request (CSR) for the specified host. This CSR will then have to be signed by a user with the proper authorization on the certificate authority. @@ -41,7 +45,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do primarily useful for requesting certificates for individual users and external applications. EOT - examples <<-'EOT' + examples <<-EOT Request a certificate for "somenode" from the site's CA: $ puppet certificate generate somenode.puppetlabs.lan --ca-location remote @@ -56,7 +60,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do action :list do summary "List all certificate signing requests." - returns <<-'EOT' + returns <<-EOT An array of #inspect output from CSR objects. This output is currently messy, but does contain the names of nodes requesting certificates. This action returns #inspect strings even when used @@ -73,10 +77,10 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do action :sign do summary "Sign a certificate signing request for HOST." arguments "<host>" - returns <<-'EOT' + returns <<-EOT A string that appears to be (but isn't) an x509 certificate. EOT - examples <<-'EOT' + examples <<-EOT Sign somenode.puppetlabs.lan's certificate: $ puppet certificate sign somenode.puppetlabs.lan --ca-location remote @@ -93,9 +97,9 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do find = get_action(:find) find.summary "Retrieve a certificate." find.arguments "<host>" - find.returns <<-'EOT' - An x509 SSL certificate. You will usually want to render this as a - string (--render-as s). + find.render_as = :s + find.returns <<-EOT + An x509 SSL certificate. Note that this action has a side effect of caching a copy of the certificate in Puppet's `ssldir`. @@ -105,7 +109,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do destroy.summary "Delete a certificate." destroy.arguments "<host>" destroy.returns "Nothing." - destroy.description <<-'EOT' + destroy.description <<-EOT Deletes a certificate. This action currently only works on the local CA. EOT diff --git a/lib/puppet/face/certificate_request.rb b/lib/puppet/face/certificate_request.rb index 774821f12..cf342d51a 100644 --- a/lib/puppet/face/certificate_request.rb +++ b/lib/puppet/face/certificate_request.rb @@ -5,7 +5,7 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do license "Apache 2 license; see COPYING" summary "Manage certificate requests." - description <<-'EOT' + description <<-EOT This subcommand retrieves and submits certificate signing requests (CSRs). EOT @@ -15,14 +15,12 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do find = get_action(:find) find.summary "Retrieve a single CSR." find.arguments "<host>" - find.returns <<-'EOT' + find.render_as = :s + find.returns <<-EOT A single certificate request. When used from the Ruby API, returns a Puppet::SSL::CertificateRequest object. - - RENDERING ISSUES: In most cases, you will want to render this as a string - ('--render-as s'). EOT - find.examples <<-'EOT' + find.examples <<-EOT Retrieve a single CSR from the puppet master's CA: $ puppet certificate_request find somenode.puppetlabs.lan --terminus rest @@ -31,10 +29,10 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do search = get_action(:search) search.summary "Retrieve all outstanding CSRs." search.arguments "<dummy_text>" - search.returns <<-'EOT' - A list of certificate requests; be sure to to render this as a string - ('--render-as s'). When used from the Ruby API, returns an array of - Puppet::SSL::CertificateRequest objects. + search.render_as = :s + search.returns <<-EOT + A list of certificate requests. When used from the Ruby API, returns an + array of Puppet::SSL::CertificateRequest objects. EOT search.short_description <<-EOT Retrieves all outstanding certificate signing requests. Due to a known bug, @@ -44,7 +42,7 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do Although this action always returns all CSRs, it requires a dummy search key; this is a known bug. EOT - search.examples <<-'EOT' + search.examples <<-EOT Retrieve all CSRs from the local CA (similar to 'puppet cert list'): $ puppet certificate_request search x --terminus ca diff --git a/lib/puppet/face/certificate_revocation_list.rb b/lib/puppet/face/certificate_revocation_list.rb index f58368f75..022323b29 100644 --- a/lib/puppet/face/certificate_revocation_list.rb +++ b/lib/puppet/face/certificate_revocation_list.rb @@ -5,7 +5,7 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do license "Apache 2 license; see COPYING" summary "Manage the list of revoked certificates." - description <<-'EOT' + description <<-EOT This subcommand is primarily for retrieving the certificate revocation list from the CA. EOT @@ -13,12 +13,10 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do find = get_action(:find) find.summary "Retrieve the certificate revocation list." find.arguments "<dummy_text>" - find.returns <<-'EOT' + find.render_as = :s + find.returns <<-EOT The certificate revocation list. When used from the Ruby API: returns an OpenSSL::X509::CRL object. - - RENDERING ISSUES: this should usually be rendered as a string - ('--render-as s'). EOT find.short_description <<-EOT Retrieves the certificate revocation list. Due to a known bug, this action @@ -28,7 +26,7 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do Although this action always returns the CRL from the specified terminus, it requires a dummy argument; this is a known bug. EOT - find.examples <<-'EXAMPLES' + find.examples <<-EXAMPLES Retrieve a copy of the puppet master's CRL: $ puppet certificate_revocation_list find crl --terminus rest @@ -38,7 +36,7 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do destroy.summary "Delete the certificate revocation list." destroy.arguments "<dummy_text>" destroy.returns "Nothing." - destroy.description <<-'EOT' + destroy.description <<-EOT Deletes the certificate revocation list. This cannot be done over REST, but it is possible to delete the locally cached copy or the local CA's copy of the CRL. diff --git a/lib/puppet/face/node/clean.rb b/lib/puppet/face/node/clean.rb new file mode 100644 index 000000000..d2852de04 --- /dev/null +++ b/lib/puppet/face/node/clean.rb @@ -0,0 +1,154 @@ +Puppet::Face.define(:node, '0.0.1') do + action(:clean) do + option "--[no-]unexport" do + summary "Unexport exported resources" + end + + summary "Clean up everything a puppetmaster knows about a node" + arguments "<host1> [<host2> ...]" + description <<-EOT + This includes + + * Signed certificates ($vardir/ssl/ca/signed/node.domain.pem) + * Cached facts ($vardir/yaml/facts/node.domain.yaml) + * Cached node stuff ($vardir/yaml/node/node.domain.yaml) + * Reports ($vardir/reports/node.domain) + * Stored configs: it can either remove all data from an host in your + storedconfig database, or with --unexport turn every exported resource + supporting ensure to absent so that any other host checking out their + config can remove those exported configurations. + + This will unexport exported resources of a + host, so that consumers of these resources can remove the exported + resources and we will safely remove the node from our + infrastructure. + EOT + + when_invoked do |*args| + nodes = args[0..-2] + options = args.last + raise "At least one node should be passed" if nodes.empty? || nodes == options + + # TODO: this is a hack and should be removed if faces provide the proper + # infrastructure to set the run mode. + require 'puppet/util/run_mode' + $puppet_application_mode = Puppet::Util::RunMode[:master] + + if Puppet::SSL::CertificateAuthority.ca? + Puppet::SSL::Host.ca_location = :local + else + Puppet::SSL::Host.ca_location = :none + end + + Puppet::Node::Facts.indirection.terminus_class = :yaml + Puppet::Node::Facts.indirection.cache_class = :yaml + Puppet::Node.indirection.terminus_class = :yaml + Puppet::Node.indirection.cache_class = :yaml + + nodes.each { |node| cleanup(node.downcase, options[:unexport]) } + end + end + + def cleanup(node, unexport) + clean_cert(node) + clean_cached_facts(node) + clean_cached_node(node) + clean_reports(node) + + # This is roughly functional, but seems to introduce order-dependent test + # failures; this can be re-added when those issues are resolved. + # clean_storeconfigs(node, unexport) + end + + # clean signed cert for +host+ + def clean_cert(node) + if Puppet::SSL::CertificateAuthority.ca? + Puppet::Face[:ca, :current].revoke(node) + Puppet::Face[:ca, :current].destroy(node) + Puppet.info "#{node} certificates removed from ca" + else + Puppet.info "Not managing #{node} certs as this host is not a CA" + end + end + + # clean facts for +host+ + def clean_cached_facts(node) + Puppet::Node::Facts.indirection.destroy(node) + Puppet.info "#{node}'s facts removed" + end + + # clean cached node +host+ + def clean_cached_node(node) + Puppet::Node.indirection.destroy(node) + Puppet.info "#{node}'s cached node removed" + end + + # clean node reports for +host+ + def clean_reports(node) + Puppet::Transaction::Report.indirection.destroy(node) + Puppet.info "#{node}'s reports removed" + end + + # clean storeconfig for +node+ + def clean_storeconfigs(node, do_unexport=false) + return unless Puppet[:storeconfigs] && Puppet.features.rails? + require 'puppet/rails' + Puppet::Rails.connect + unless rails_node = Puppet::Rails::Host.find_by_name(node) + Puppet.notice "No entries found for #{node} in storedconfigs." + return + end + + if do_unexport + unexport(rails_node) + Puppet.notice "Force #{node}'s exported resources to absent" + Puppet.warning "Please wait until all other hosts have checked out their configuration before finishing the cleanup with:" + Puppet.warning "$ puppet node clean #{node}" + else + rails_node.destroy + Puppet.notice "#{node} storeconfigs removed" + end + end + + def unexport(node) + # fetch all exported resource + query = {:include => {:param_values => :param_name}} + query[:conditions] = [ "exported=? AND host_id=?", true, node.id ] + Puppet::Rails::Resource.find(:all, query).each do |resource| + if type_is_ensurable(resource) + line = 0 + param_name = Puppet::Rails::ParamName.find_or_create_by_name("ensure") + + if ensure_param = resource.param_values.find( + :first, + :conditions => [ 'param_name_id = ?', param_name.id ] + ) + line = ensure_param.line.to_i + Puppet::Rails::ParamValue.delete(ensure_param.id); + end + + # force ensure parameter to "absent" + resource.param_values.create( + :value => "absent", + :line => line, + :param_name => param_name + ) + Puppet.info("#{resource.name} has been marked as \"absent\"") + end + end + end + + def environment + @environment ||= Puppet::Node::Environment.new + end + + def type_is_ensurable(resource) + if (type = Puppet::Type.type(resource.restype)) && type.validattr?(:ensure) + return true + else + type = environment.known_resource_types.find_definition('', resource.restype) + return true if type && type.arguments.keys.include?('ensure') + end + return false + end +end diff --git a/lib/puppet/face/status.rb b/lib/puppet/face/status.rb index bdb0c4d26..e8c87e98d 100644 --- a/lib/puppet/face/status.rb +++ b/lib/puppet/face/status.rb @@ -12,6 +12,7 @@ Puppet::Indirector::Face.define(:status, '0.0.1') do get_action(:search).summary "Invalid for this subcommand." find = get_action(:find) + find.default = true find.summary "Check status of puppet master server." find.arguments "<dummy_text>" find.returns <<-'EOT' diff --git a/lib/puppet/file_serving/configuration/parser.rb b/lib/puppet/file_serving/configuration/parser.rb index 334201d37..83b75e28f 100644 --- a/lib/puppet/file_serving/configuration/parser.rb +++ b/lib/puppet/file_serving/configuration/parser.rb @@ -24,9 +24,10 @@ class Puppet::FileServing::Configuration::Parser < Puppet::Util::LoadedFile when /^\s*$/; next # skip blank lines when /\[([-\w]+)\]/ mount = newmount($1) - when /^\s*(\w+)\s+(.+)$/ + when /^\s*(\w+)\s+(.+?)(\s*#.*)?$/ var = $1 value = $2 + value.strip! raise(ArgumentError, "Fileserver configuration file does not use '=' as a separator") if value =~ /^=/ case var when "path" @@ -58,12 +59,8 @@ class Puppet::FileServing::Configuration::Parser < Puppet::Util::LoadedFile begin mount.info "allowing #{val} access" mount.allow(val) - rescue AuthStoreError => detail - - raise ArgumentError.new( - detail.to_s, - - @count, file) + rescue Puppet::AuthStoreError => detail + raise ArgumentError.new(detail.to_s, @count, file) end } end @@ -75,12 +72,8 @@ class Puppet::FileServing::Configuration::Parser < Puppet::Util::LoadedFile begin mount.info "denying #{val} access" mount.deny(val) - rescue AuthStoreError => detail - - raise ArgumentError.new( - detail.to_s, - - @count, file) + rescue Puppet::AuthStoreError => detail + raise ArgumentError.new(detail.to_s, @count, file) end } end diff --git a/lib/puppet/indirector/face.rb b/lib/puppet/indirector/face.rb index ead3f4b46..adb6b688b 100644 --- a/lib/puppet/indirector/face.rb +++ b/lib/puppet/indirector/face.rb @@ -48,16 +48,26 @@ class Puppet::Indirector::Face < Puppet::Face return result end + option "--extra HASH" do + summary "Extra arguments to pass to the indirection request" + description <<-end + A terminus can take additional arguments to refine the operation, which + are passed as an arbitrary hash to the back-end. Anything passed as + the extra value is just send direct to the back-end. + end + default_to do Hash.new end + end + action :destroy do summary "Delete an object." arguments "<key>" - when_invoked { |key, options| call_indirection_method(:destroy, key, options) } + when_invoked {|key, options| call_indirection_method :destroy, key, options[:extra] } end action :find do summary "Retrieve an object by name." arguments "<key>" - when_invoked { |key, options| call_indirection_method(:find, key, options) } + when_invoked {|key, options| call_indirection_method :find, key, options[:extra] } end action :save do @@ -68,13 +78,13 @@ class Puppet::Indirector::Face < Puppet::Face currently accept data from STDIN, save actions cannot currently be invoked from the command line. EOT - when_invoked { |key, options| call_indirection_method(:save, key, options) } + when_invoked {|key, options| call_indirection_method :save, key, options[:extra] } end action :search do summary "Search for an object or retrieve multiple objects." arguments "<query>" - when_invoked { |key, options| call_indirection_method(:search, key, options) } + when_invoked {|key, options| call_indirection_method :search, key, options[:extra] } end # Print the configuration for the current terminus class @@ -86,11 +96,11 @@ class Puppet::Indirector::Face < Puppet::Face run mode with the '--mode' option. EOT - when_invoked do |*args| + when_invoked do |options| if t = indirection.terminus_class - puts "Run mode '#{Puppet.run_mode.name}': #{t}" + "Run mode '#{Puppet.run_mode.name}': #{t}" else - $stderr.puts "No default terminus class for run mode '#{Puppet.run_mode.name}'" + "No default terminus class for run mode '#{Puppet.run_mode.name}'" end end end diff --git a/lib/puppet/indirector/report/processor.rb b/lib/puppet/indirector/report/processor.rb index 88fe4b487..7bdadcb36 100644 --- a/lib/puppet/indirector/report/processor.rb +++ b/lib/puppet/indirector/report/processor.rb @@ -14,28 +14,30 @@ class Puppet::Transaction::Report::Processor < Puppet::Indirector::Code process(request.instance) end + def destroy(request) + processors do |mod| + mod.destroy(request.key) if mod.respond_to?(:destroy) + end + end + private # Process the report with each of the configured report types. # LAK:NOTE This isn't necessarily the best design, but it's backward # compatible and that's good enough for now. def process(report) - return if Puppet[:reports] == "none" - - reports.each do |name| - if mod = Puppet::Reports.report(name) - # We have to use a dup because we're including a module in the - # report. - newrep = report.dup - begin - newrep.extend(mod) - newrep.process - rescue => detail - puts detail.backtrace if Puppet[:trace] - Puppet.err "Report #{name} failed: #{detail}" - end - else - Puppet.warning "No report named '#{name}'" + Puppet.debug "Recieved report to process from #{report.host}" + processors do |mod| + Puppet.debug "Processing report from #{report.host} with processor #{mod}" + # We have to use a dup because we're including a module in the + # report. + newrep = report.dup + begin + newrep.extend(mod) + newrep.process + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.err "Report #{name} failed: #{detail}" end end end @@ -45,4 +47,15 @@ class Puppet::Transaction::Report::Processor < Puppet::Indirector::Code # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] x = Puppet[:reports].gsub(/(^\s+)|(\s+$)/, '').split(/\s*,\s*/) end + + def processors(&blk) + return if Puppet[:reports] == "none" + reports.each do |name| + if mod = Puppet::Reports.report(name) + yield(mod) + else + Puppet.warning "No report named '#{name}'" + end + end + end end diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 8018fe8e3..19daff51d 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -93,7 +93,9 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus http_connection.send(method, *args) rescue OpenSSL::SSL::SSLError => error - if error.message.include? "hostname was not match" + if error.message.include? "certificate verify failed" + raise Puppet::Error, "#{error.message}. This is often because the time is out of sync on the server or client" + elsif error.message.include? "hostname was not match" raise unless cert = peer_certs.find { |c| c.name !~ /^puppet ca/i } valid_certnames = [cert.name, *cert.alternate_names].uniq diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb index 23997e97a..7b12d25e2 100644 --- a/lib/puppet/indirector/yaml.rb +++ b/lib/puppet/indirector/yaml.rb @@ -47,6 +47,11 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus File.join(base, self.class.indirection_name.to_s, name.to_s + ext) end + def destroy(request) + file_path = path(request.key) + File.unlink(file_path) if File.exists?(file_path) + end + def search(request) Dir.glob(path(request.key,'')).collect do |file| YAML.load_file(file) diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb index 6be8b6930..eba99d6be 100644 --- a/lib/puppet/interface.rb +++ b/lib/puppet/interface.rb @@ -2,6 +2,7 @@ require 'puppet' require 'puppet/util/autoload' require 'puppet/interface/documentation' require 'prettyprint' +require 'semver' class Puppet::Interface include FullDocs @@ -63,6 +64,10 @@ class Puppet::Interface end face end + + def find_action(name, action, version = :current) + Puppet::Interface::FaceCollection.get_action_for_face(name, action, version) + end end def set_default_format(format) @@ -84,12 +89,12 @@ class Puppet::Interface attr_reader :name, :version def initialize(name, version, &block) - unless Puppet::Interface::FaceCollection.validate_version(version) + unless SemVer.valid?(version) raise ArgumentError, "Cannot create face #{name.inspect} with invalid version number '#{version}'!" end @name = Puppet::Interface::FaceCollection.underscorize(name) - @version = version + @version = SemVer.new(version) # The few bits of documentation we actually demand. The default license # is a favour to our end users; if you happen to get that in a core face diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 185302b07..60ddb2ca3 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -38,6 +38,7 @@ class Puppet::Interface::Action def to_s() "#{@face}##{@name}" end attr_reader :name + attr_reader :face attr_accessor :default def default? !!@default @@ -195,14 +196,12 @@ class Puppet::Interface::Action wrapper = <<WRAPPER def #{@name}(#{decl.join(", ")}) #{optn} - args = #{args} - options = args.last - - action = get_action(#{name.inspect}) - action.validate_args(args) - __invoke_decorations(:before, action, args, options) + args = #{args} + action = get_action(#{name.inspect}) + args << action.validate_and_clean(args.pop) + __invoke_decorations(:before, action, args, args.last) rval = self.__send__(#{internal_name.inspect}, *args) - __invoke_decorations(:after, action, args, options) + __invoke_decorations(:after, action, args, args.last) return rval end WRAPPER @@ -227,8 +226,9 @@ WRAPPER end end + @options << option.name + option.aliases.each do |name| - @options << name @options_hash[name] = option end @@ -251,27 +251,61 @@ WRAPPER option end - def validate_args(args) - # Check for multiple aliases for the same option... - args.last.keys.each do |name| - # #7290: If this isn't actually an option, ignore it for now. We should - # probably fail, but that wasn't our API, and I don't want to perturb - # behaviour this late in the RC cycle. --daniel 2011-04-29 + def validate_and_clean(original) + # The final set of arguments; effectively a hand-rolled shallow copy of + # the original, which protects the caller from the surprises they might + # get if they passed us a hash and we mutated it... + result = {} + + # Check for multiple aliases for the same option, and canonicalize the + # name of the argument while we are about it. + overlap = Hash.new do |h, k| h[k] = [] end + unknown = [] + original.keys.each do |name| if option = get_option(name) then - overlap = (option.aliases & args.last.keys) - unless overlap.length == 1 then - raise ArgumentError, "Multiple aliases for the same option passed: #{overlap.join(', ')}" + canonical = option.name + if result.has_key? canonical + overlap[canonical] << name + else + result[canonical] = original[name] end + elsif Puppet.settings.include? name + result[name] = original[name] + else + unknown << name + end + end + + unless overlap.empty? + msg = overlap.map {|k, v| "(#{k}, #{v.sort.join(', ')})" }.join(", ") + raise ArgumentError, "Multiple aliases for the same option passed: #{msg}" + end + + unless unknown.empty? + msg = unknown.sort.join(", ") + raise ArgumentError, "Unknown options passed: #{msg}" + end + + # Inject default arguments and check for missing mandating options. + missing = [] + options.map {|x| get_option(x) }.each do |option| + name = option.name + next if result.has_key? name + + if option.has_default? + result[name] = option.default + elsif option.required? + missing << name end end - # Check for missing mandatory options. - required = options.map do |name| - get_option(name) - end.select(&:required?).collect(&:name) - args.last.keys + unless missing.empty? + msg = missing.sort.join(', ') + raise ArgumentError, "The following options are required: #{msg}" + end - return if required.empty? - raise ArgumentError, "The following options are required: #{required.join(', ')}" + # All done. + return result end ######################################################################## diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb index fbf588d7d..5c9af4f96 100644 --- a/lib/puppet/interface/action_manager.rb +++ b/lib/puppet/interface/action_manager.rb @@ -7,13 +7,14 @@ module Puppet::Interface::ActionManager require 'puppet/interface/action_builder' @actions ||= {} - @default_action ||= nil raise "Action #{name} already defined for #{self}" if action?(name) + action = Puppet::Interface::ActionBuilder.build(self, name, &block) - if action.default - raise "Actions #{@default_action.name} and #{name} cannot both be default" if @default_action - @default_action = action + + if action.default and current = get_default_action + raise "Actions #{current.name} and #{name} cannot both be default" end + @actions[action.name] = action end @@ -61,7 +62,11 @@ module Puppet::Interface::ActionManager end def get_default_action - @default_action + default = actions.map {|x| get_action(x) }.select {|x| x.default } + if default.length > 1 + raise "The actions #{default.map(&:name).join(", ")} cannot all be default" + end + default.first end def action?(name) diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 12d3c56b1..b1f6ba398 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -1,8 +1,6 @@ require 'puppet/interface' module Puppet::Interface::FaceCollection - SEMVER_VERSION = /^(\d+)\.(\d+)\.(\d+)([A-Za-z][0-9A-Za-z-]*|)$/ - @faces = Hash.new { |hash, key| hash[key] = {} } def self.faces @@ -17,55 +15,36 @@ module Puppet::Interface::FaceCollection @faces.keys.select {|name| @faces[name].length > 0 } end - def self.validate_version(version) - !!(SEMVER_VERSION =~ version.to_s) - end - - def self.semver_to_array(v) - parts = SEMVER_VERSION.match(v).to_a[1..4] - parts[0..2] = parts[0..2].map { |e| e.to_i } - parts - end - - def self.cmp_semver(a, b) - a, b = [a, b].map do |x| semver_to_array(x) end - - cmp = a[0..2] <=> b[0..2] - if cmp == 0 - cmp = a[3] <=> b[3] - cmp = +1 if a[3].empty? && !b[3].empty? - cmp = -1 if b[3].empty? && !a[3].empty? - end - cmp + def self.[](name, version) + name = underscorize(name) + get_face(name, version) or load_face(name, version) end - def self.prefix_match?(desired, target) - # Can't meaningfully do a prefix match with current on either side. - return false if desired == :current - return false if target == :current - - # REVISIT: Should probably fail if the matcher is not valid. - prefix = desired.split('.').map {|x| x =~ /^\d+$/ and x.to_i } - have = semver_to_array(target) + def self.get_action_for_face(name, action_name, version) + name = underscorize(name) - while want = prefix.shift do - return false unless want == have.shift + # If the version they request specifically doesn't exist, don't search + # elsewhere. Usually this will start from :current and all... + return nil unless face = self[name, version] + unless action = face.get_action(action_name) + # ...we need to search for it bound to an o{lder,ther} version. Since + # we load all actions when the face is first references, this will be in + # memory in the known set of versions of the face. + (@faces[name].keys - [ :current ]).sort.reverse.each do |version| + break if action = @faces[name][version].get_action(action_name) + end end - return true - end - def self.[](name, version) - name = underscorize(name) - get_face(name, version) or load_face(name, version) + return action end # get face from memory, without loading. - def self.get_face(name, desired_version) + def self.get_face(name, pattern) return nil unless @faces.has_key? name + return @faces[name][:current] if pattern == :current - return @faces[name][:current] if desired_version == :current - - found = @faces[name].keys.select {|v| prefix_match?(desired_version, v) }.sort.last + versions = @faces[name].keys - [ :current ] + found = SemVer.find_matching(pattern, versions) return @faces[name][found] end @@ -77,9 +56,7 @@ module Puppet::Interface::FaceCollection # # We use require to avoid executing the code multiple times, like any # other Ruby library that we might want to use. --daniel 2011-04-06 - begin - require "puppet/face/#{name}" - + if safely_require name then # If we wanted :current, we need to index to find that; direct version # requests just work™ as they go. --daniel 2011-04-06 if version == :current then @@ -108,21 +85,35 @@ module Puppet::Interface::FaceCollection # versions here and return the last item in that set. # # --daniel 2011-04-06 - latest_ver = @faces[name].keys.sort {|a, b| cmp_semver(a, b) }.last + latest_ver = @faces[name].keys.sort.last @faces[name][:current] = @faces[name][latest_ver] end - rescue LoadError => e - raise unless e.message =~ %r{-- puppet/face/#{name}$} - # ...guess we didn't find the file; return a much better problem. - rescue SyntaxError => e - raise unless e.message =~ %r{puppet/face/#{name}\.rb:\d+: } - Puppet.err "Failed to load face #{name}:\n#{e}" - # ...but we just carry on after complaining. + end + + unless version == :current or get_face(name, version) + # Try an obsolete version of the face, if needed, to see if that helps? + safely_require name, version end return get_face(name, version) end + def self.safely_require(name, version = nil) + path = File.join 'puppet' ,'face', version.to_s, name.to_s + require path + true + + rescue LoadError => e + raise unless e.message =~ %r{-- #{path}$} + # ...guess we didn't find the file; return a much better problem. + nil + rescue SyntaxError => e + raise unless e.message =~ %r{#{path}\.rb:\d+: } + Puppet.err "Failed to load face #{name}:\n#{e}" + # ...but we just carry on after complaining. + nil + end + def self.register(face) @faces[underscorize(face.name)][face.version] = face end diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb index 3cd930acf..01f6f2307 100644 --- a/lib/puppet/interface/option.rb +++ b/lib/puppet/interface/option.rb @@ -6,6 +6,7 @@ class Puppet::Interface::Option def initialize(parent, *declaration, &block) @parent = parent @optparse = [] + @default = nil # Collect and sort the arguments in the declaration. dups = {} @@ -81,8 +82,26 @@ class Puppet::Interface::Option !!@required end + def has_default? + !!@default + end + + def default=(proc) + required and raise ArgumentError, "#{self} can't be optional and have a default value" + proc.is_a? Proc or raise ArgumentError, "default value for #{self} is a #{proc.class.name.inspect}, not a proc" + @default = proc + end + + def default + @default and @default.call + end + attr_reader :parent, :name, :aliases, :optparse attr_accessor :required + def required=(value) + has_default? and raise ArgumentError, "#{self} can't be optional and have a default value" + @required = value + end attr_accessor :before_action def before_action=(proc) diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb index 5676ec977..c87adc2c0 100644 --- a/lib/puppet/interface/option_builder.rb +++ b/lib/puppet/interface/option_builder.rb @@ -51,4 +51,17 @@ class Puppet::Interface::OptionBuilder def required(value = true) @option.required = value end + + def default_to(&block) + block or raise ArgumentError, "#{@option} default_to requires a block" + if @option.has_default? + raise ArgumentError, "#{@option} already has a default value" + end + # Ruby 1.8 treats a block without arguments as accepting any number; 1.9 + # gets this right, so we work around it for now... --daniel 2011-07-20 + unless block.arity == 0 or (RUBY_VERSION =~ /^1\.8/ and block.arity == -1) + raise ArgumentError, "#{@option} default_to block should not take any arguments" + end + @option.default = block + end end diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb index 326a91d92..a1f300e8e 100644 --- a/lib/puppet/interface/option_manager.rb +++ b/lib/puppet/interface/option_manager.rb @@ -26,8 +26,9 @@ module Puppet::Interface::OptionManager end end + @options << option.name + option.aliases.each do |name| - @options << name @options_hash[name] = option end diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb index 059591ed8..00468df96 100644 --- a/lib/puppet/module.rb +++ b/lib/puppet/module.rb @@ -42,7 +42,10 @@ class Puppet::Module def has_metadata? return false unless metadata_file - FileTest.exist?(metadata_file) + return false unless FileTest.exist?(metadata_file) + + metadata = PSON.parse File.read(metadata_file) + return metadata.is_a?(Hash) && !metadata.keys.empty? end def initialize(name, environment = nil) diff --git a/lib/puppet/network/authconfig.rb b/lib/puppet/network/authconfig.rb index 4ba89fa71..1e486a2f9 100644 --- a/lib/puppet/network/authconfig.rb +++ b/lib/puppet/network/authconfig.rb @@ -102,7 +102,7 @@ module Puppet name = $3 if $2 == "path" name.chomp! right = newrights.newright(name, count, @file) - when /^\s*(allow|deny|method|environment|auth(?:enticated)?)\s+(.+)$/ + when /^\s*(allow|deny|method|environment|auth(?:enticated)?)\s+(.+?)(\s*#.*)?$/ parse_right_directive(right, $1, $2, count) else raise ConfigurationError, "Invalid line #{count}: #{line}" @@ -130,6 +130,7 @@ module Puppet end def parse_right_directive(right, var, value, count) + value.strip! case var when "allow" modify_right(right, :allow, value, "allowing %s access", count) @@ -159,6 +160,7 @@ module Puppet def modify_right(right, method, value, msg, count) value.split(/\s*,\s*/).each do |val| begin + val.strip! right.info msg % val right.send(method, val) rescue AuthStoreError => detail diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb index 5b4b17a32..5da4cedef 100755 --- a/lib/puppet/network/handler/fileserver.rb +++ b/lib/puppet/network/handler/fileserver.rb @@ -269,6 +269,7 @@ class Puppet::Network::Handler value = $2 case var when "path" + raise FileServerError.new("No mount specified for argument #{var} #{value}") unless mount if mount.name == MODULES Puppet.warning "The '#{mount.name}' module can not have a path. Ignoring attempt to set it" else @@ -280,6 +281,7 @@ class Puppet::Network::Handler end end when "allow" + raise FileServerError.new("No mount specified for argument #{var} #{value}") unless mount value.split(/\s*,\s*/).each { |val| begin mount.info "allowing #{val} access" @@ -294,6 +296,7 @@ class Puppet::Network::Handler end } when "deny" + raise FileServerError.new("No mount specified for argument #{var} #{value}") unless mount value.split(/\s*,\s*/).each { |val| begin mount.info "denying #{val} access" diff --git a/lib/puppet/network/http/webrick.rb b/lib/puppet/network/http/webrick.rb index 54bcf30c2..52aec1bf1 100644 --- a/lib/puppet/network/http/webrick.rb +++ b/lib/puppet/network/http/webrick.rb @@ -40,7 +40,7 @@ class Puppet::Network::HTTP::WEBrick @listening = true @thread = Thread.new { @server.start { |sock| - raise "Client disconnected before connection could be established" unless IO.select([sock],nil,nil,0.1) + raise "Client disconnected before connection could be established" unless IO.select([sock],nil,nil,6.2) sock.accept @server.run(sock) } diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb index dfe8f85c4..7dcc81ef4 100644 --- a/lib/puppet/network/rest_authconfig.rb +++ b/lib/puppet/network/rest_authconfig.rb @@ -29,10 +29,15 @@ module Puppet @main end + def allowed?(request) + Puppet.deprecation_warning "allowed? should not be called for REST authorization - use check_authorization instead" + check_authorization(request) + end + # check wether this request is allowed in our ACL # raise an Puppet::Network::AuthorizedError if the request # is denied. - def allowed?(indirection, method, key, params) + def check_authorization(indirection, method, key, params) read # we're splitting the request in part because diff --git a/lib/puppet/network/rest_authorization.rb b/lib/puppet/network/rest_authorization.rb index 50f094e3e..d636d486a 100644 --- a/lib/puppet/network/rest_authorization.rb +++ b/lib/puppet/network/rest_authorization.rb @@ -16,7 +16,7 @@ module Puppet::Network # Verify that our client has access. def check_authorization(indirection, method, key, params) - authconfig.allowed?(indirection, method, key, params) + authconfig.check_authorization(indirection, method, key, params) end end end diff --git a/lib/puppet/parser/functions/create_resources.rb b/lib/puppet/parser/functions/create_resources.rb index 430f110b4..3b8bb3543 100644 --- a/lib/puppet/parser/functions/create_resources.rb +++ b/lib/puppet/parser/functions/create_resources.rb @@ -27,15 +27,16 @@ Takes two parameters: args[1].each do |title, params| raise ArgumentError, 'params should not contain title' if(params['title']) case type_of_resource - when :type - res = resource.hash2resource(params.merge(:title => title)) - catalog.add_resource(res) - when :define + # JJM The only difference between a type and a define is the call to instantiate_resource + # for a defined type. + when :type, :define p_resource = Puppet::Parser::Resource.new(type_name, title, :scope => self, :source => resource) params.merge(:name => title).each do |k,v| p_resource.set_parameter(k,v) end - resource.instantiate_resource(self, p_resource) + if type_of_resource == :define then + resource.instantiate_resource(self, p_resource) + end compiler.add_resource(self, p_resource) when :class klass = find_hostclass(title) diff --git a/lib/puppet/reports/store.rb b/lib/puppet/reports/store.rb index 625a263b3..997206ec4 100644 --- a/lib/puppet/reports/store.rb +++ b/lib/puppet/reports/store.rb @@ -41,5 +41,20 @@ Puppet::Reports.register_report(:store) do # Only testing cares about the return value file end + + # removes all reports for a given host + def self.destroy(host) + client = host.gsub("..",".") + dir = File.join(Puppet[:reportdir], client) + + if File.exists?(dir) + Dir.entries(dir).each do |file| + next if ['.','..'].include?(file) + file = File.join(dir, file) + File.unlink(file) if File.file?(file) + end + Dir.rmdir(dir) + end + end end diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb index 8eb4266db..be6302595 100644 --- a/lib/puppet/resource/catalog.rb +++ b/lib/puppet/resource/catalog.rb @@ -94,7 +94,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph resource.ref =~ /^(.+)\[/ class_name = $1 || resource.class.name - newref = [class_name, key] + newref = [class_name, key].flatten if key.is_a? String ref_string = "#{class_name}[#{key}]" @@ -107,7 +107,10 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph # isn't sufficient. if existing = @resource_table[newref] return if existing == resource - raise(ArgumentError, "Cannot alias #{resource.ref} to #{key.inspect}; resource #{newref.inspect} already exists") + resource_definition = " at #{resource.file}:#{resource.line}" if resource.file and resource.line + existing_definition = " at #{existing.file}:#{existing.line}" if existing.file and existing.line + msg = "Cannot alias #{resource.ref} to #{key.inspect}#{resource_definition}; resource #{newref.inspect} already defined#{existing_definition}" + raise ArgumentError, msg end @resource_table[newref] = resource @aliases[resource.ref] ||= [] @@ -436,7 +439,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph res = Puppet::Resource.new(nil, type) end title_key = [res.type, res.title.to_s] - uniqueness_key = [res.type, res.uniqueness_key] + uniqueness_key = [res.type, res.uniqueness_key].flatten @resource_table[title_key] || @resource_table[uniqueness_key] end diff --git a/lib/puppet/ssl/inventory.rb b/lib/puppet/ssl/inventory.rb index e094da100..c210fdc35 100644 --- a/lib/puppet/ssl/inventory.rb +++ b/lib/puppet/ssl/inventory.rb @@ -48,5 +48,7 @@ class Puppet::SSL::Inventory return Integer($1) end + + return nil end end diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 76c646baf..67401505d 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -42,7 +42,7 @@ module Puppet on the local host, whereas `agent` will connect to the puppet server that it received the manifest from. - See the [fileserver configuration documentation](http://projects.puppetlabs.com/projects/puppet/wiki/File_Serving_Configuration) for information on how to configure + See the [fileserver configuration documentation](http://docs.puppetlabs.com/guides/file_serving.html) for information on how to configure and use file services within Puppet. If you specify multiple file sources for a file, then the first @@ -154,7 +154,7 @@ module Puppet fail detail, "Could not retrieve file metadata for #{source}: #{detail}" end end - fail "Could not retrieve information from source(s) #{value.join(", ")}" unless result + fail "Could not retrieve information from environment #{Puppet[:environment]} source(s) #{value.join(", ")}" unless result result end diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index 572d5796d..c64bf69e8 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -168,6 +168,14 @@ module Puppet return "changed password" end end + + def is_to_s( currentvalue ) + return '[old password hash redacted]' + end + def should_to_s( newvalue ) + return '[new password hash redacted]' + end + end newproperty(:password_min_age, :required_features => :manages_password_age) do diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index ce9d4642b..ff09221a2 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -30,7 +30,6 @@ module Util end end - def self.synchronize_on(x,type) sync_object,users = 0,1 begin diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb index f243b8691..4559e9af3 100644 --- a/lib/puppet/util/settings.rb +++ b/lib/puppet/util/settings.rb @@ -721,7 +721,7 @@ if @config.include?(:run_mode) end Puppet::Util::SUIDManager.asuser(*chown) do - mode = obj.mode || 0640 + mode = obj.mode ? obj.mode.to_i : 0640 args << "w" if args.empty? args << mode diff --git a/lib/semver.rb b/lib/semver.rb new file mode 100644 index 000000000..ef9435abd --- /dev/null +++ b/lib/semver.rb @@ -0,0 +1,65 @@ +class SemVer + VERSION = /^v?(\d+)\.(\d+)\.(\d+)([A-Za-z][0-9A-Za-z-]*|)$/ + SIMPLE_RANGE = /^v?(\d+|[xX])(?:\.(\d+|[xX])(?:\.(\d+|[xX]))?)?$/ + + include Comparable + + def self.valid?(ver) + VERSION =~ ver + end + + def self.find_matching(pattern, versions) + versions.select { |v| v.matched_by?("#{pattern}") }.sort.last + end + + attr_reader :major, :minor, :tiny, :special + + def initialize(ver) + unless SemVer.valid?(ver) + raise ArgumentError.new("Invalid version string '#{ver}'!") + end + + @major, @minor, @tiny, @special = VERSION.match(ver).captures.map do |x| + # Because Kernel#Integer tries to interpret hex and octal strings, which + # we specifically do not want, and which cannot be overridden in 1.8.7. + Float(x).to_i rescue x + end + end + + def <=>(other) + other = SemVer.new("#{other}") unless other.is_a? SemVer + return self.major <=> other.major unless self.major == other.major + return self.minor <=> other.minor unless self.minor == other.minor + return self.tiny <=> other.tiny unless self.tiny == other.tiny + + return 0 if self.special == other.special + return 1 if self.special == '' + return -1 if other.special == '' + + return self.special <=> other.special + end + + def matched_by?(pattern) + # For the time being, this is restricted to exact version matches and + # simple range patterns. In the future, we should implement some or all of + # the comparison operators here: + # https://github.com/isaacs/node-semver/blob/d474801/semver.js#L340 + + case pattern + when SIMPLE_RANGE + pattern = SIMPLE_RANGE.match(pattern).captures + pattern[1] = @minor unless pattern[1] && pattern[1] !~ /x/i + pattern[2] = @tiny unless pattern[2] && pattern[2] !~ /x/i + [@major, @minor, @tiny] == pattern.map { |x| x.to_i } + when VERSION + self == SemVer.new(pattern) + else + false + end + end + + def inspect + "v#{@major}.#{@minor}.#{@tiny}#{@special}" + end + alias :to_s :inspect +end diff --git a/spec/integration/defaults_spec.rb b/spec/integration/defaults_spec.rb index 9bec769ab..8cf0e3e7b 100755 --- a/spec/integration/defaults_spec.rb +++ b/spec/integration/defaults_spec.rb @@ -275,6 +275,6 @@ describe "Puppet defaults" do describe "reporturl" do subject { Puppet.settings[:reporturl] } - it { should == "http://localhost:3000/reports" } + it { should == "http://localhost:3000/reports/upload" } end end diff --git a/spec/integration/network/rest_authconfig_spec.rb b/spec/integration/network/rest_authconfig_spec.rb new file mode 100644 index 000000000..d2f539cd4 --- /dev/null +++ b/spec/integration/network/rest_authconfig_spec.rb @@ -0,0 +1,145 @@ +require 'spec_helper' + +require 'puppet/network/rest_authconfig' + +RSpec::Matchers.define :allow do |params| + + match do |auth| + begin + auth.check_authorization(params[0], params[1], params[2], params[3]) + true + rescue Puppet::Network::AuthorizationError + false + end + end + + failure_message_for_should do |instance| + "expected #{params[3][:node]}/#{params[3][:ip]} to be allowed" + end + + failure_message_for_should_not do |instance| + "expected #{params[3][:node]}/#{params[3][:ip]} to be forbidden" + end +end + +describe Puppet::Network::RestAuthConfig do + include PuppetSpec::Files + + before(:each) do + Puppet[:rest_authconfig] = tmpfile('auth.conf') + end + + def add_rule(rule) + File.open(Puppet[:rest_authconfig],"w+") do |f| + f.print "path /test\n#{rule}\n" + end + @auth = Puppet::Network::RestAuthConfig.new(Puppet[:rest_authconfig], true) + end + + def add_regex_rule(regex, rule) + File.open(Puppet[:rest_authconfig],"w+") do |f| + f.print "path ~ #{regex}\n#{rule}\n" + end + @auth = Puppet::Network::RestAuthConfig.new(Puppet[:rest_authconfig], true) + end + + def request(args = {}) + { :ip => '10.1.1.1', :node => 'host.domain.com', :key => 'key', :authenticated => true }.each do |k,v| + args[k] ||= v + end + ['test', :find, args[:key], args] + end + + it "should support IPv4 address" do + add_rule("allow 10.1.1.1") + + @auth.should allow(request) + end + + it "should support CIDR IPv4 address" do + add_rule("allow 10.0.0.0/8") + + @auth.should allow(request) + end + + it "should support wildcard IPv4 address" do + add_rule("allow 10.1.1.*") + + @auth.should allow(request) + end + + it "should support IPv6 address" do + add_rule("allow 2001:DB8::8:800:200C:417A") + + @auth.should allow(request(:ip => '2001:DB8::8:800:200C:417A')) + end + + it "should support hostname" do + add_rule("allow host.domain.com") + + @auth.should allow(request) + end + + it "should support wildcard host" do + add_rule("allow *.domain.com") + + @auth.should allow(request) + end + + it "should support hostname backreferences" do + add_regex_rule('^/test/([^/]+)$', "allow $1.domain.com") + + @auth.should allow(request(:key => 'host')) + end + + it "should support opaque strings" do + add_rule("allow this-is-opaque@or-not") + + @auth.should allow(request(:node => 'this-is-opaque@or-not')) + end + + it "should support opaque strings and backreferences" do + add_regex_rule('^/test/([^/]+)$', "allow $1") + + @auth.should allow(request(:key => 'this-is-opaque@or-not', :node => 'this-is-opaque@or-not')) + end + + it "should support hostname ending with '.'" do + pending('bug #7589') + add_rule("allow host.domain.com.") + + @auth.should allow(request(:node => 'host.domain.com.')) + end + + it "should support hostname ending with '.' and backreferences" do + pending('bug #7589') + add_regex_rule('^/test/([^/]+)$',"allow $1") + + @auth.should allow(request(:node => 'host.domain.com.')) + end + + it "should support trailing whitespace" do + add_rule('allow host.domain.com ') + + @auth.should allow(request) + end + + it "should support inlined comments" do + add_rule('allow host.domain.com # will it work?') + + @auth.should allow(request) + end + + it "should deny non-matching host" do + add_rule("allow inexistant") + + @auth.should_not allow(request) + end + + it "should deny denied hosts" do + add_rule("deny host.domain.com") + + @auth.should_not allow(request) + end + +end
\ No newline at end of file diff --git a/spec/integration/node/facts_spec.rb b/spec/integration/node/facts_spec.rb index f54d7f9aa..e87a0bdeb 100755 --- a/spec/integration/node/facts_spec.rb +++ b/spec/integration/node/facts_spec.rb @@ -7,7 +7,7 @@ require 'spec_helper' describe Puppet::Node::Facts do describe "when using the indirector" do - after { Puppet::Util::Cacher.expire } + after(:each) { Puppet::Util::Cacher.expire } it "should expire any cached node instances when it is saved" do Puppet::Node::Facts.indirection.stubs(:terminus_class).returns :yaml diff --git a/spec/lib/puppet/face/1.0.0/huzzah.rb b/spec/lib/puppet/face/1.0.0/huzzah.rb new file mode 100755 index 000000000..8a1311704 --- /dev/null +++ b/spec/lib/puppet/face/1.0.0/huzzah.rb @@ -0,0 +1,8 @@ +require 'puppet/face' +Puppet::Face.define(:huzzah, '1.0.0') do + copyright "Puppet Labs", 2011 + license "Apache 2 license; see COPYING" + summary "life is a thing for celebration" + script :obsolete_in_core do |_| "you are in obsolete core now!" end + script :call_newer do |_| method_on_newer end +end diff --git a/spec/lib/puppet/face/huzzah.rb b/spec/lib/puppet/face/huzzah.rb index ab465d9e0..f3d18a797 100755 --- a/spec/lib/puppet/face/huzzah.rb +++ b/spec/lib/puppet/face/huzzah.rb @@ -4,4 +4,5 @@ Puppet::Face.define(:huzzah, '2.0.1') do license "Apache 2 license; see COPYING" summary "life is a thing for celebration" script :bar do |options| "is where beer comes from" end + script :call_older do |_| method_on_older end end diff --git a/spec/lib/puppet/face/huzzah/obsolete.rb b/spec/lib/puppet/face/huzzah/obsolete.rb new file mode 100644 index 000000000..1f717ea2f --- /dev/null +++ b/spec/lib/puppet/face/huzzah/obsolete.rb @@ -0,0 +1,6 @@ +Puppet::Face.define(:huzzah, '1.0.0') do + action :obsolete do + summary "This is an action on version 1.0.0 of the face" + when_invoked do |options| options end + end +end diff --git a/spec/shared_behaviours/things_that_declare_options.rb b/spec/shared_behaviours/things_that_declare_options.rb index ebf1b2071..ecdbfcaea 100755 --- a/spec/shared_behaviours/things_that_declare_options.rb +++ b/spec/shared_behaviours/things_that_declare_options.rb @@ -43,7 +43,7 @@ shared_examples_for "things that declare options" do option "-f" option "--baz" end - thing.options.should == [:foo, :bar, :b, :q, :quux, :f, :baz] + thing.options.should == [:foo, :bar, :quux, :f, :baz] end it "should detect conflicts in long options" do @@ -146,4 +146,117 @@ shared_examples_for "things that declare options" do end end end + + describe "#default_to" do + it "should not have a default value by default" do + option = add_options_to do option "--foo" end.get_option(:foo) + option.should_not be_has_default + end + + it "should accept a block for the default value" do + option = add_options_to do + option "--foo" do + default_to do + 12 + end + end + end.get_option(:foo) + + option.should be_has_default + end + + it "should invoke the block when asked for the default value" do + invoked = false + option = add_options_to do + option "--foo" do + default_to do + invoked = true + end + end + end.get_option(:foo) + + option.should be_has_default + option.default.should be_true + invoked.should be_true + end + + it "should return the value of the block when asked for the default" do + option = add_options_to do + option "--foo" do + default_to do + 12 + end + end + end.get_option(:foo) + + option.should be_has_default + option.default.should == 12 + end + + it "should invoke the block every time the default is requested" do + option = add_options_to do + option "--foo" do + default_to do + {} + end + end + end.get_option(:foo) + + first = option.default.object_id + second = option.default.object_id + third = option.default.object_id + + first.should_not == second + first.should_not == third + second.should_not == third + end + + it "should fail if the option has a default and is required" do + expect { + add_options_to do + option "--foo" do + required + default_to do 12 end + end + end + }.to raise_error ArgumentError, /can't be optional and have a default value/ + + expect { + add_options_to do + option "--foo" do + default_to do 12 end + required + end + end + }.to raise_error ArgumentError, /can't be optional and have a default value/ + end + + it "should fail if default_to has no block" do + expect { add_options_to do option "--foo" do default_to end end }. + to raise_error ArgumentError, /default_to requires a block/ + end + + it "should fail if default_to is invoked twice" do + expect { + add_options_to do + option "--foo" do + default_to do 12 end + default_to do "fun" end + end + end + }.to raise_error ArgumentError, /already has a default value/ + end + + [ "one", "one, two", "one, *two" ].each do |input| + it "should fail if the block has the wrong arity (#{input})" do + expect { + add_options_to do + option "--foo" do + eval "default_to do |#{input}| 12 end" + end + end + }.to raise_error ArgumentError, /should not take any arguments/ + end + end + end end diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb index c9555157c..489f4db36 100755 --- a/spec/unit/application/apply_spec.rb +++ b/spec/unit/application/apply_spec.rb @@ -134,7 +134,9 @@ describe Puppet::Application::Apply do Puppet[:postrun_command] = '' Puppet::Node::Facts.indirection.terminus_class = :memory + Puppet::Node::Facts.indirection.cache_class = :memory Puppet::Node.indirection.terminus_class = :memory + Puppet::Node.indirection.cache_class = :memory @facts = Puppet::Node::Facts.new(Puppet[:node_name_value]) Puppet::Node::Facts.indirection.save(@facts) diff --git a/spec/unit/application/cert_spec.rb b/spec/unit/application/cert_spec.rb index 7510f0783..300234c2b 100755 --- a/spec/unit/application/cert_spec.rb +++ b/spec/unit/application/cert_spec.rb @@ -208,5 +208,15 @@ describe Puppet::Application::Cert, :'fails_on_ruby_1.9.2' => true do args.should == ["fun.example.com"] end end + + it "should print help and exit if there is no subcommand" do + args = [] + @cert_app.command_line.stubs(:args).returns(args) + @cert_app.stubs(:help).returns("I called for help!") + @cert_app.expects(:puts).with("I called for help!") + + expect { @cert_app.parse_options }.to exit_with 0 + @cert_app.subcommand.should be_nil + end end end diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb index 0a4a86be6..bebc26210 100755 --- a/spec/unit/application/face_base_spec.rb +++ b/spec/unit/application/face_base_spec.rb @@ -55,6 +55,7 @@ describe Puppet::Application::FaceBase do it "should stop if the first thing found is not an action" do app.command_line.stubs(:args).returns %w{banana count_args} expect { app.run }.to exit_with 1 + @logs.first.should_not be_nil @logs.first.message.should =~ /has no 'banana' action/ end diff --git a/spec/unit/application/inspect_spec.rb b/spec/unit/application/inspect_spec.rb index 571683f37..77f36f438 100755 --- a/spec/unit/application/inspect_spec.rb +++ b/spec/unit/application/inspect_spec.rb @@ -12,6 +12,11 @@ describe Puppet::Application::Inspect do before :each do @inspect = Puppet::Application[:inspect] + @inspect.preinit + end + + it "should operate in agent run_mode" do + @inspect.class.run_mode.name.should == :agent end describe "during setup" do diff --git a/spec/unit/face/ca_spec.rb b/spec/unit/face/ca_spec.rb new file mode 100755 index 000000000..b8c82ce99 --- /dev/null +++ b/spec/unit/face/ca_spec.rb @@ -0,0 +1,355 @@ +#!/usr/bin/env rspec +require 'spec_helper' +require 'puppet/face' + +describe Puppet::Face[:ca, '0.1.0'] do + include PuppetSpec::Files + + before :each do + Puppet.run_mode.stubs(:master?).returns(true) + Puppet[:ca] = true + Puppet[:ssldir] = tmpdir("face-ca-ssldir") + + Puppet::SSL::Host.ca_location = :only + Puppet[:certificate_revocation] = true + + # This is way more intimate than I want to be with the implementation, but + # there doesn't seem any other way to test this. --daniel 2011-07-18 + Puppet::SSL::CertificateAuthority.stubs(:instance).returns( + # ...and this actually does the directory creation, etc. + Puppet::SSL::CertificateAuthority.new + ) + end + + def make_certs(csr_names, crt_names) + Array(csr_names).map do |name| + Puppet::SSL::Host.new(name).generate_certificate_request + end + + Array(crt_names).map do |name| + Puppet::SSL::Host.new(name).generate + end + end + + context "#verify" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:verify) end + + it "should not explode if there is no certificate" do + expect { + subject.verify('random-host').should == { + :host => 'random-host', :valid => false, + :error => 'Could not find a certificate for random-host' + } + }.should_not raise_error + end + + it "should not explode if there is only a CSR" do + make_certs('random-host', []) + expect { + subject.verify('random-host').should == { + :host => 'random-host', :valid => false, + :error => 'Could not find a certificate for random-host' + } + }.should_not raise_error + end + + it "should verify a signed certificate" do + make_certs([], 'random-host') + subject.verify('random-host').should == { + :host => 'random-host', :valid => true + } + end + + it "should not verify a revoked certificate" do + make_certs([], 'random-host') + subject.revoke('random-host') + + expect { + subject.verify('random-host').should == { + :host => 'random-host', :valid => false, + :error => 'certificate revoked' + } + }.should_not raise_error + end + + it "should verify a revoked certificate if CRL use was turned off" do + make_certs([], 'random-host') + subject.revoke('random-host') + + Puppet[:certificate_revocation] = false + subject.verify('random-host').should == { + :host => 'random-host', :valid => true + } + end + end + + context "#fingerprint" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:fingerprint) end + + it "should have a 'digest' option" do + action.should be_option :digest + end + + it "should not explode if there is no certificate" do + expect { + subject.fingerprint('random-host').should be_nil + }.should_not raise_error + end + + it "should fingerprint a CSR" do + make_certs('random-host', []) + expect { + subject.fingerprint('random-host').should =~ /^[0-9A-F:]+$/ + }.should_not raise_error + end + + it "should fingerprint a certificate" do + make_certs([], 'random-host') + subject.fingerprint('random-host').should =~ /^[0-9A-F:]+$/ + end + + %w{md5 MD5 sha1 ShA1 SHA1 RIPEMD160 sha256 sha512}.each do |digest| + it "should fingerprint with #{digest.inspect}" do + make_certs([], 'random-host') + subject.fingerprint('random-host', :digest => digest).should =~ /^[0-9A-F:]+$/ + end + + it "should fingerprint with #{digest.to_sym} as a symbol" do + make_certs([], 'random-host') + subject.fingerprint('random-host', :digest => digest.to_sym). + should =~ /^[0-9A-F:]+$/ + end + end + end + + context "#print" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:print) end + + it "should not explode if there is no certificate" do + expect { + subject.print('random-host').should be_nil + }.should_not raise_error + end + + it "should return nothing if there is only a CSR" do + make_certs('random-host', []) + expect { + subject.print('random-host').should be_nil + }.should_not raise_error + end + + it "should return the certificate content if there is a cert" do + make_certs([], 'random-host') + text = subject.print('random-host') + text.should be_an_instance_of String + text.should =~ /^Certificate:/ + text.should =~ /Issuer: CN=Puppet CA: / + text.should =~ /Subject: CN=random-host$/ + end + end + + context "#sign" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:sign) end + + it "should not explode if there is no CSR" do + expect { + subject.sign('random-host'). + should == 'Could not find certificate request for random-host' + }.should_not raise_error + end + + it "should not explode if there is a signed cert" do + make_certs([], 'random-host') + expect { + subject.sign('random-host'). + should == 'Could not find certificate request for random-host' + }.should_not raise_error + end + + it "should sign a CSR if one exists" do + make_certs('random-host', []) + subject.sign('random-host').should be_an_instance_of Puppet::SSL::Certificate + + list = subject.list(:signed => true) + list.length.should == 1 + list.first.name.should == 'random-host' + end + end + + context "#generate" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:generate) end + + it "should generate a certificate if requested" do + subject.list(:all => true).should == [] + + subject.generate('random-host') + + list = subject.list(:signed => true) + list.length.should == 1 + list.first.name.should == 'random-host' + end + + it "should not explode if a CSR with that name already exists" do + make_certs('random-host', []) + expect { + subject.generate('random-host').should =~ /already has a certificate request/ + }.should_not raise_error + end + + it "should not explode if the certificate with that name already exists" do + make_certs([], 'random-host') + expect { + subject.generate('random-host').should =~ /already has a certificate/ + }.should_not raise_error + end + end + + context "#revoke" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:revoke) end + + it "should not explode when asked to revoke something that doesn't exist" do + expect { subject.revoke('nonesuch') }.should_not raise_error + end + + it "should let the user know what went wrong" do + subject.revoke('nonesuch').should == 'Nothing was revoked' + end + + it "should revoke a certificate" do + make_certs([], 'random-host') + found = subject.list(:all => true, :subject => 'random-host') + subject.get_action(:list).when_rendering(:console).call(found). + should =~ /^\+ random-host/ + + subject.revoke('random-host') + + found = subject.list(:all => true, :subject => 'random-host') + subject.get_action(:list).when_rendering(:console).call(found). + should =~ /^- random-host \([:0-9A-F]+\) \(certificate revoked\)/ + end + end + + context "#destroy" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:destroy) end + + it "should not explode when asked to delete something that doesn't exist" do + expect { subject.destroy('nonesuch') }.should_not raise_error + end + + it "should let the user know if nothing was deleted" do + subject.destroy('nonesuch').should == "Nothing was deleted" + end + + it "should destroy a CSR, if we have one" do + make_certs('random-host', []) + subject.list(:pending => true, :subject => 'random-host').should_not == [] + + subject.destroy('random-host') + + subject.list(:pending => true, :subject => 'random-host').should == [] + end + + it "should destroy a certificate, if we have one" do + make_certs([], 'random-host') + subject.list(:signed => true, :subject => 'random-host').should_not == [] + + subject.destroy('random-host') + + subject.list(:signed => true, :subject => 'random-host').should == [] + end + + it "should tell the user something was deleted" do + make_certs([], 'random-host') + subject.list(:signed => true, :subject => 'random-host').should_not == [] + subject.destroy('random-host'). + should == "Deleted for random-host: Puppet::SSL::Certificate, Puppet::SSL::Key" + end + end + + context "#list" do + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:list) end + + context "options" do + subject { Puppet::Face[:ca, '0.1.0'].get_action(:list) } + it { should be_option :pending } + it { should be_option :signed } + it { should be_option :all } + it { should be_option :subject } + end + + context "with no hosts in CA" do + [:pending, :signed, :all].each do |type| + it "should return nothing for #{type}" do + subject.list(type => true).should == [] + end + + it "should not fail when a matcher is passed" do + expect { + subject.list(type => true, :subject => '.').should == [] + }.should_not raise_error + end + end + end + + context "with some hosts" do + csr_names = (1..3).map {|n| "csr-#{n}" } + crt_names = (1..3).map {|n| "crt-#{n}" } + all_names = csr_names + crt_names + + { + {} => csr_names, + { :pending => true } => csr_names, + + { :signed => true } => crt_names, + + { :all => true } => all_names, + { :pending => true, :signed => true } => all_names, + }.each do |input, expect| + it "should map #{input.inspect} to #{expect.inspect}" do + make_certs(csr_names, crt_names) + subject.list(input).map(&:name).should =~ expect + end + + ['', '.', '2', 'none'].each do |pattern| + filtered = expect.select {|x| Regexp.new(pattern).match(x) } + + it "should filter all hosts matching #{pattern.inspect} to #{filtered.inspect}" do + make_certs(csr_names, crt_names) + subject.list(input.merge :subject => pattern).map(&:name).should =~ filtered + end + end + end + + context "when_rendering :console" do + { [["csr1.local"], []] => '^ csr1.local ', + [[], ["crt1.local"]] => '^\+ crt1.local ', + [["csr2"], ["crt2"]] => ['^ csr2 ', '^\+ crt2 '] + }.each do |input, pattern| + it "should render #{input.inspect} to match #{pattern.inspect}" do + make_certs(*input) + text = action.when_rendering(:console).call(subject.list(:all => true)) + Array(pattern).each do |item| + text.should =~ Regexp.new(item) + end + end + end + end + end + end + + actions = %w{destroy list revoke generate sign print verify fingerprint} + actions.each do |action| + it { should be_action action } + it "should fail #{action} when not a CA" do + Puppet[:ca] = false + expect { + case subject.method(action).arity + when -1 then subject.send(action) + when -2 then subject.send(action, 'dummy') + else + raise "#{action} has arity #{subject.method(action).arity}" + end + }.should raise_error(/Not a CA/) + end + end +end diff --git a/spec/unit/face/certificate_spec.rb b/spec/unit/face/certificate_spec.rb index 0cb905b75..9291d7523 100755 --- a/spec/unit/face/certificate_spec.rb +++ b/spec/unit/face/certificate_spec.rb @@ -10,14 +10,26 @@ describe Puppet::Face[:certificate, '0.0.1'] do end it "should set the ca location when invoked" do - Puppet::SSL::Host.expects(:ca_location=).with(:foo) + Puppet::SSL::Host.expects(:ca_location=).with(:local) Puppet::SSL::Host.indirection.expects(:save) - subject.sign "hello, friend", :ca_location => :foo + subject.sign "hello, friend", :ca_location => :local end it "(#7059) should set the ca location when an inherited action is invoked" do - Puppet::SSL::Host.expects(:ca_location=).with(:foo) + Puppet::SSL::Host.expects(:ca_location=).with(:local) subject.indirection.expects(:find) - subject.find "hello, friend", :ca_location => :foo + subject.find "hello, friend", :ca_location => :local + end + + it "should validate the option as required" do + expect do + subject.find 'hello, friend' + end.to raise_exception ArgumentError, /required/i + end + + it "should validate the option as a supported value" do + expect do + subject.find 'hello, friend', :ca_location => :foo + end.to raise_exception ArgumentError, /valid values/i end end diff --git a/spec/unit/face/node_spec.rb b/spec/unit/face/node_spec.rb index 027a4cce0..6f6edc6cc 100755 --- a/spec/unit/face/node_spec.rb +++ b/spec/unit/face/node_spec.rb @@ -3,5 +3,265 @@ require 'spec_helper' require 'puppet/face' describe Puppet::Face[:node, '0.0.1'] do - it "REVISIT: really should have some tests" + describe '#cleanup' do + it "should clean everything" do + { + "cert" => ['hostname'], + "cached_facts" => ['hostname'], + "cached_node" => ['hostname'], + "reports" => ['hostname'], + + # Support for cleaning storeconfigs has been temporarily suspended. + # "storeconfigs" => ['hostname', :unexport] + }.each { |k, v| subject.expects("clean_#{k}".to_sym).with(*v) } + subject.cleanup('hostname', :unexport) + end + end + + describe 'when running #clean' do + before :each do + Puppet::Node::Facts.indirection.stubs(:terminus_class=) + Puppet::Node::Facts.indirection.stubs(:cache_class=) + Puppet::Node.stubs(:terminus_class=) + Puppet::Node.stubs(:cache_class=) + end + + it 'should invoke #cleanup' do + subject.expects(:cleanup).with('hostname', nil) + subject.clean('hostname') + end + end + + describe "clean action" do + before :each do + Puppet::Node::Facts.indirection.stubs(:terminus_class=) + Puppet::Node::Facts.indirection.stubs(:cache_class=) + Puppet::Node.stubs(:terminus_class=) + Puppet::Node.stubs(:cache_class=) + subject.stubs(:cleanup) + end + + it "should have a clean action" do + subject.should be_action :clean + end + + it "should not accept a call with no arguments" do + expect { subject.clean() }.should raise_error + end + + it "should accept a node name" do + expect { subject.clean('hostname') }.should_not raise_error + end + + it "should accept more than one node name" do + expect do + subject.clean('hostname', 'hostname2', {}) + end.should_not raise_error + + expect do + subject.clean('hostname', 'hostname2', 'hostname3', { :unexport => true }) + end.should_not raise_error + end + + it "should accept the option --unexport" do + expect { subject.help('hostname', :unexport => true) }. + should_not raise_error ArgumentError + end + + context "clean action" do + subject { Puppet::Face[:node, :current] } + before :each do + Puppet::Util::Log.stubs(:newdestination) + Puppet::Util::Log.stubs(:level=) + end + + describe "during setup" do + it "should set facts terminus and cache class to yaml" do + Puppet::Node::Facts.indirection.expects(:terminus_class=).with(:yaml) + Puppet::Node::Facts.indirection.expects(:cache_class=).with(:yaml) + + subject.clean('hostname') + end + + it "should run in master mode" do + subject.clean('hostname') + $puppet_application_mode.name.should == :master + end + + it "should set node cache as yaml" do + Puppet::Node.indirection.expects(:terminus_class=).with(:yaml) + Puppet::Node.indirection.expects(:cache_class=).with(:yaml) + + subject.clean('hostname') + end + + it "should manage the certs if the host is a CA" do + Puppet::SSL::CertificateAuthority.stubs(:ca?).returns(true) + Puppet::SSL::Host.expects(:ca_location=).with(:local) + subject.clean('hostname') + end + + it "should not manage the certs if the host is not a CA" do + Puppet::SSL::CertificateAuthority.stubs(:ca?).returns(false) + Puppet::SSL::Host.expects(:ca_location=).with(:none) + subject.clean('hostname') + end + end + + describe "when cleaning certificate" do + before :each do + Puppet::SSL::Host.stubs(:destroy) + @ca = mock() + Puppet::SSL::CertificateAuthority.stubs(:instance).returns(@ca) + end + + it "should send the :destroy order to the ca if we are a CA" do + Puppet::SSL::CertificateAuthority.stubs(:ca?).returns(true) + @ca.expects(:revoke).with(@host) + @ca.expects(:destroy).with(@host) + subject.clean_cert(@host) + end + + it "should not destroy the certs if we are not a CA" do + Puppet::SSL::CertificateAuthority.stubs(:ca?).returns(false) + @ca.expects(:revoke).never + @ca.expects(:destroy).never + subject.clean_cert(@host) + end + end + + describe "when cleaning cached facts" do + it "should destroy facts" do + @host = 'node' + Puppet::Node::Facts.indirection.expects(:destroy).with(@host) + + subject.clean_cached_facts(@host) + end + end + + describe "when cleaning cached node" do + it "should destroy the cached node" do + Puppet::Node::Yaml.any_instance.expects(:destroy) + subject.clean_cached_node(@host) + end + end + + describe "when cleaning archived reports" do + it "should tell the reports to remove themselves" do + Puppet::Transaction::Report.indirection.stubs(:destroy).with(@host) + + subject.clean_reports(@host) + end + end + + # describe "when cleaning storeconfigs entries for host", :if => Puppet.features.rails? do + # before :each do + # # Stub this so we don't need access to the DB + # require 'puppet/rails/host' + # + # Puppet.stubs(:[]).with(:storeconfigs).returns(true) + # + # Puppet::Rails.stubs(:connect) + # @rails_node = stub_everything 'rails_node' + # Puppet::Rails::Host.stubs(:find_by_name).returns(@rails_node) + # end + # + # it "should connect to the database" do + # Puppet::Rails.expects(:connect) + # subject.clean_storeconfigs(@host, false) + # end + # + # it "should find the right host entry" do + # Puppet::Rails::Host.expects(:find_by_name).with(@host).returns(@rails_node) + # subject.clean_storeconfigs(@host, false) + # end + # + # describe "without unexport" do + # it "should remove the host and it's content" do + # @rails_node.expects(:destroy) + # subject.clean_storeconfigs(@host, false) + # end + # end + # + # describe "with unexport" do + # before :each do + # @rails_node.stubs(:id).returns(1234) + # + # @type = stub_everything 'type' + # @type.stubs(:validattr?).with(:ensure).returns(true) + # + # @ensure_name = stub_everything 'ensure_name', :id => 23453 + # Puppet::Rails::ParamName.stubs(:find_or_create_by_name).returns(@ensure_name) + # + # @param_values = stub_everything 'param_values' + # @resource = stub_everything 'resource', :param_values => @param_values, :restype => "File" + # Puppet::Rails::Resource.stubs(:find).returns([@resource]) + # end + # + # it "should find all resources" do + # Puppet::Rails::Resource.expects(:find).with(:all, {:include => {:param_values => :param_name}, :conditions => ["exported=? AND host_id=?", true, 1234]}).returns([]) + # + # subject.clean_storeconfigs(@host, true) + # end + # + # describe "with an exported native type" do + # before :each do + # Puppet::Type.stubs(:type).returns(@type) + # @type.expects(:validattr?).with(:ensure).returns(true) + # end + # + # it "should test a native type for ensure as an attribute" do + # subject.clean_storeconfigs(@host, true) + # end + # + # it "should delete the old ensure parameter" do + # ensure_param = stub 'ensure_param', :id => 12345, :line => 12 + # @param_values.stubs(:find).returns(ensure_param) + # Puppet::Rails::ParamValue.expects(:delete).with(12345); + # subject.clean_storeconfigs(@host, true) + # end + # + # it "should add an ensure => absent parameter" do + # @param_values.expects(:create).with(:value => "absent", + # :line => 0, + # :param_name => @ensure_name) + # subject.clean_storeconfigs(@host, true) + # end + # end + # + # describe "with an exported definition" do + # it "should try to lookup a definition and test it for the ensure argument" do + # Puppet::Type.stubs(:type).returns(nil) + # definition = stub_everything 'definition', :arguments => { 'ensure' => 'present' } + # Puppet::Resource::TypeCollection.any_instance.expects(:find_definition).with('', "File").returns(definition) + # subject.clean_storeconfigs(@host, true) + # end + # end + # + # it "should not unexport the resource of an unknown type" do + # Puppet::Type.stubs(:type).returns(nil) + # Puppet::Resource::TypeCollection.any_instance.expects(:find_definition).with('', "File").returns(nil) + # Puppet::Rails::ParamName.expects(:find_or_create_by_name).never + # subject.clean_storeconfigs(@host) + # end + # + # it "should not unexport the resource of a not ensurable native type" do + # Puppet::Type.stubs(:type).returns(@type) + # @type.expects(:validattr?).with(:ensure).returns(false) + # Puppet::Resource::TypeCollection.any_instance.expects(:find_definition).with('', "File").returns(nil) + # Puppet::Rails::ParamName.expects(:find_or_create_by_name).never + # subject.clean_storeconfigs(@host, true) + # end + # + # it "should not unexport the resource of a not ensurable definition" do + # Puppet::Type.stubs(:type).returns(nil) + # definition = stub_everything 'definition', :arguments => { 'foobar' => 'someValue' } + # Puppet::Resource::TypeCollection.any_instance.expects(:find_definition).with('', "File").returns(definition) + # Puppet::Rails::ParamName.expects(:find_or_create_by_name).never + # subject.clean_storeconfigs(@host, true) + # end + # end + # end + end + end end diff --git a/spec/unit/file_serving/configuration/parser_spec.rb b/spec/unit/file_serving/configuration/parser_spec.rb index 3d6b3e234..5ccfc5075 100755 --- a/spec/unit/file_serving/configuration/parser_spec.rb +++ b/spec/unit/file_serving/configuration/parser_spec.rb @@ -118,6 +118,14 @@ describe Puppet::FileServing::Configuration::Parser do @parser.parse end + it "should support inline comments" do + mock_file_content "[one]\nallow something \# will it work?\n" + + @mount.expects(:info) + @mount.expects(:allow).with("something") + @parser.parse + end + it "should tell the mount to deny any deny values from the section" do mock_file_content "[one]\ndeny something\n" diff --git a/spec/unit/indirector/certificate_status/file_spec.rb b/spec/unit/indirector/certificate_status/file_spec.rb index ae03aa9cb..5451913a1 100755 --- a/spec/unit/indirector/certificate_status/file_spec.rb +++ b/spec/unit/indirector/certificate_status/file_spec.rb @@ -7,6 +7,10 @@ require 'tempfile' describe "Puppet::Indirector::CertificateStatus::File" do include PuppetSpec::Files + before :all do + Puppet::SSL::Host.configure_indirection(:file) + end + before do Puppet::SSL::CertificateAuthority.stubs(:ca?).returns true @terminus = Puppet::SSL::Host.indirection.terminus(:file) diff --git a/spec/unit/indirector/face_spec.rb b/spec/unit/indirector/face_spec.rb index 943ff7991..8e324f019 100755 --- a/spec/unit/indirector/face_spec.rb +++ b/spec/unit/indirector/face_spec.rb @@ -12,6 +12,8 @@ describe Puppet::Indirector::Face do instance end + it { should be_option :extra } + it "should be able to return a list of indirections" do Puppet::Indirector::Face.indirections.should be_include("catalog") end @@ -48,7 +50,7 @@ describe Puppet::Indirector::Face do end it "should forward passed options" do subject.indirection.expects(method).with(:test, {'one'=>'1'}) - subject.send(method, :test, {'one'=>'1'}) + subject.send(method, :test, :extra => {'one'=>'1'}) end end diff --git a/spec/unit/indirector/report/processor_spec.rb b/spec/unit/indirector/report/processor_spec.rb index bafbe6ee7..c64cc7eff 100755 --- a/spec/unit/indirector/report/processor_spec.rb +++ b/spec/unit/indirector/report/processor_spec.rb @@ -15,39 +15,62 @@ describe Puppet::Transaction::Report::Processor do it "should provide a method for saving reports" do Puppet::Transaction::Report::Processor.new.should respond_to(:save) end + + it "should provide a method for cleaning reports" do + Puppet::Transaction::Report::Processor.new.should respond_to(:destroy) + end + end -describe Puppet::Transaction::Report::Processor, " when saving a report" do +describe Puppet::Transaction::Report::Processor, " when processing a report" do before do Puppet.settings.stubs(:use) @reporter = Puppet::Transaction::Report::Processor.new + @request = stub 'request', :instance => stub("report", :host => 'hostname'), :key => 'node' end - it "should not process the report if reports are set to 'none'" do + it "should not save the report if reports are set to 'none'" do Puppet::Reports.expects(:report).never - Puppet.settings.expects(:value).with(:reports).returns("none") + Puppet[:reports] = 'none' - request = stub 'request', :instance => mock("report") + request = Puppet::Indirector::Request.new(:indirection_name, :head, "key") + report = Puppet::Transaction::Report.new('apply') + request.instance = report @reporter.save(request) end - it "should process the report with each configured report type" do + it "should save the report with each configured report type" do Puppet.settings.stubs(:value).with(:reports).returns("one,two") @reporter.send(:reports).should == %w{one two} + + Puppet::Reports.expects(:report).with('one') + Puppet::Reports.expects(:report).with('two') + + @reporter.save(@request) + end + + it "should destroy reports for each processor that responds to destroy" do + Puppet.settings.stubs(:value).with(:reports).returns("http,store") + http_report = mock() + store_report = mock() + store_report.expects(:destroy).with(@request.key) + Puppet::Reports.expects(:report).with('http').returns(http_report) + Puppet::Reports.expects(:report).with('store').returns(store_report) + @reporter.destroy(@request) end end describe Puppet::Transaction::Report::Processor, " when processing a report" do before do - Puppet.settings.stubs(:value).with(:reports).returns("one") + Puppet[:reports] = "one" Puppet.settings.stubs(:use) @reporter = Puppet::Transaction::Report::Processor.new @report_type = mock 'one' @dup_report = mock 'dupe report' @dup_report.stubs(:process) - @report = mock 'report' + @report = Puppet::Transaction::Report.new('apply') @report.expects(:dup).returns(@dup_report) @request = stub 'request', :instance => @report @@ -74,7 +97,7 @@ describe Puppet::Transaction::Report::Processor, " when processing a report" do end it "should not raise exceptions" do - Puppet.settings.stubs(:value).with(:trace).returns(false) + Puppet[:trace] = false @dup_report.expects(:process).raises(ArgumentError) proc { @reporter.save(@request) }.should_not raise_error end diff --git a/spec/unit/indirector/rest_spec.rb b/spec/unit/indirector/rest_spec.rb index ee0111a77..042b7ca16 100755 --- a/spec/unit/indirector/rest_spec.rb +++ b/spec/unit/indirector/rest_spec.rb @@ -90,42 +90,53 @@ describe Puppet::Indirector::REST do @rest_class.port.should == 543 end - describe "when making http requests" do - it "should provide a helpful error message when hostname was not match with server certificate" do - Puppet[:certdnsnames] = 'foo:bar:baz' - csr = OpenSSL::X509::Request.new - csr.subject = OpenSSL::X509::Name.new([['CN', 'not_my_server']]) - csr.public_key = OpenSSL::PKey::RSA.generate(Puppet[:keylength]).public_key - cert = Puppet::SSL::CertificateFactory.new('server', csr, csr, 14).result - - connection = Net::HTTP.new('my_server', 8140) - @searcher.stubs(:network).returns(connection) - ssl_context = OpenSSL::SSL::SSLContext.new - ssl_context.stubs(:current_cert).returns(cert) - connection.stubs(:get).with do - connection.verify_callback.call(true, ssl_context) - end.raises(OpenSSL::SSL::SSLError.new('hostname was not match with server certificate')) - - msg = /Server hostname 'my_server' did not match server certificate; expected one of (.+)/ - expect { @searcher.http_request(:get, stub('request')) }.to( - raise_error(Puppet::Error, msg) do |error| - error.message =~ msg - $1.split(', ').should =~ ['foo', 'bar', 'baz', 'not_my_server'] - end - ) - end - - it "should pass along the error message otherwise" do - connection = Net::HTTP.new('my_server', 8140) - @searcher.stubs(:network).returns(connection) - - connection.stubs(:get).raises(OpenSSL::SSL::SSLError.new('certificate verify failed')) - - expect do - @searcher.http_request(:get, stub('request')) - end.to raise_error(/certificate verify failed/) - end - end + describe "when making http requests" do + it "should provide a suggestive error message when certificate verify failed" do + connection = Net::HTTP.new('my_server', 8140) + @searcher.stubs(:network).returns(connection) + + connection.stubs(:get).raises(OpenSSL::SSL::SSLError.new('certificate verify failed')) + + expect do + @searcher.http_request(:get, stub('request')) + end.to raise_error(/This is often because the time is out of sync on the server or client/) + end + + it "should provide a helpful error message when hostname was not match with server certificate" do + Puppet[:certdnsnames] = 'foo:bar:baz' + csr = OpenSSL::X509::Request.new + csr.subject = OpenSSL::X509::Name.new([['CN', 'not_my_server']]) + csr.public_key = OpenSSL::PKey::RSA.generate(Puppet[:keylength]).public_key + cert = Puppet::SSL::CertificateFactory.new('server', csr, csr, 14).result + + connection = Net::HTTP.new('my_server', 8140) + @searcher.stubs(:network).returns(connection) + ssl_context = OpenSSL::SSL::SSLContext.new + ssl_context.stubs(:current_cert).returns(cert) + connection.stubs(:get).with do + connection.verify_callback.call(true, ssl_context) + end.raises(OpenSSL::SSL::SSLError.new('hostname was not match with server certificate')) + + msg = /Server hostname 'my_server' did not match server certificate; expected one of (.+)/ + expect { @searcher.http_request(:get, stub('request')) }.to( + raise_error(Puppet::Error, msg) do |error| + error.message =~ msg + $1.split(', ').should =~ ['foo', 'bar', 'baz', 'not_my_server'] + end + ) + end + + it "should pass along the error message otherwise" do + connection = Net::HTTP.new('my_server', 8140) + @searcher.stubs(:network).returns(connection) + + connection.stubs(:get).raises(OpenSSL::SSL::SSLError.new('some other message')) + + expect do + @searcher.http_request(:get, stub('request')) + end.to raise_error(/some other message/) + end + end describe "when deserializing responses" do it "should return nil if the response code is 404" do diff --git a/spec/unit/indirector/yaml_spec.rb b/spec/unit/indirector/yaml_spec.rb index c43dbcaf6..29f6d466f 100755 --- a/spec/unit/indirector/yaml_spec.rb +++ b/spec/unit/indirector/yaml_spec.rb @@ -154,5 +154,23 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do Dir.expects(:glob).with(:glob).returns [] @store.search(@request).should == [] end + + describe Puppet::Indirector::Yaml, " when destroying" do + it "should unlink the right yaml file if it exists" do + path = File.join("/what/ever", @store.class.indirection_name.to_s, @request.key.to_s + ".yaml") + File.expects(:exists?).with(path).returns true + File.expects(:unlink).with(path) + + @store.destroy(@request) + end + + it "should not unlink the yaml file if it does not exists" do + path = File.join("/what/ever", @store.class.indirection_name.to_s, @request.key.to_s + ".yaml") + File.expects(:exists?).with(path).returns false + File.expects(:unlink).with(path).never + + @store.destroy(@request) + end + end end end diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb index cf8d61d51..6b68eb149 100755 --- a/spec/unit/interface/action_spec.rb +++ b/spec/unit/interface/action_spec.rb @@ -121,6 +121,7 @@ describe Puppet::Interface::Action do let :face do Puppet::Interface.new(:ruby_api, '1.0.0') do action :bar do + option "--bar" when_invoked do |*args| args.last end @@ -138,8 +139,8 @@ describe Puppet::Interface::Action do options.should == { :bar => "beer" } end - it "should call #validate_args on the action when invoked" do - face.get_action(:bar).expects(:validate_args).with([1, :two, 'three', {}]) + it "should call #validate_and_clean on the action when invoked" do + face.get_action(:bar).expects(:validate_and_clean).with({}).returns({}) face.bar 1, :two, 'three' end end @@ -171,6 +172,30 @@ describe Puppet::Interface::Action do face.get_action(:foo).options.should =~ [:bar] end + describe "option aliases" do + let :option do action.get_option :bar end + let :action do face.get_action :foo end + let :face do + Puppet::Interface.new(:action_level_options, '0.0.1') do + action :foo do + when_invoked do |options| options end + option "--bar", "--foo", "-b" + end + end + end + + it "should only list options and not aliases" do + action.options.should =~ [:bar] + end + + it "should use the canonical option name when passed aliases" do + name = option.name + option.aliases.each do |input| + face.foo(input => 1).should == { name => 1 } + end + end + end + describe "with both face and action options" do let :face do Puppet::Interface.new(:action_level_options, '0.0.1') do @@ -426,12 +451,12 @@ describe Puppet::Interface::Action do end it "should be invoked when calling a child action" do - subject.on_child(:foo => true, :bar => true).should == :on_child + subject.on_child(:foo => true).should == :on_child subject.reported.should == [ :child_before ] end it "should be invoked when calling a parent action" do - subject.on_parent(:foo => true, :bar => true).should == :on_parent + subject.on_parent(:foo => true).should == :on_parent subject.reported.should == [ :child_before ] end end @@ -443,12 +468,12 @@ describe Puppet::Interface::Action do end it "should be invoked when calling a child action" do - subject.on_child(:foo => true, :bar => true).should == :on_child + subject.on_child(:foo => true).should == :on_child subject.reported.should == [ :parent_before ] end it "should be invoked when calling a parent action" do - subject.on_parent(:foo => true, :bar => true).should == :on_parent + subject.on_parent(:foo => true).should == :on_parent subject.reported.should == [ :parent_before ] end end @@ -524,10 +549,10 @@ describe Puppet::Interface::Action do it "should return the block if asked" end - context "#validate_args" do + context "#validate_and_clean" do subject do Puppet::Interface.new(:validate_args, '1.0.0') do - script :test do |options| true end + script :test do |options| options end end end @@ -541,5 +566,84 @@ describe Puppet::Interface::Action do expect { subject.test :foo => true, :f => true }. to raise_error ArgumentError, /Multiple aliases for the same option/ end + + it "should fail if an unknown option is passed" do + expect { subject.test :unknown => true }. + to raise_error ArgumentError, /Unknown options passed: unknown/ + end + + it "should report all the unknown options passed" do + expect { subject.test :unknown => true, :unseen => false }. + to raise_error ArgumentError, /Unknown options passed: unknown, unseen/ + end + + it "should accept 'global' options from settings" do + expect { + subject.test(:certname => "true").should == { :certname => "true" } + }.not_to raise_error + end + end + + context "default option values" do + subject do + Puppet::Interface.new(:default_option_values, '1.0.0') do + action :foo do + option "--foo" do end + option "--bar" do end + when_invoked do |options| options end + end + end + end + + let :action do subject.get_action :foo end + let :option do action.get_option :foo end + + it "should not add options without defaults" do + subject.foo.should == {} + end + + it "should not add options without defaults, if options are given" do + subject.foo(:bar => 1).should == { :bar => 1 } + end + + it "should add the option default value when set" do + option.default = proc { 12 } + subject.foo.should == { :foo => 12 } + end + + it "should add the option default value when set, if other options are given" do + option.default = proc { 12 } + subject.foo(:bar => 1).should == { :foo => 12, :bar => 1 } + end + + it "should invoke the same default proc every time called" do + option.default = proc { @foo ||= {} } + subject.foo[:foo].object_id.should == subject.foo[:foo].object_id + end + + [nil, 0, 1, true, false, {}, []].each do |input| + it "should not override a passed option (#{input.inspect})" do + option.default = proc { :fail } + subject.foo(:foo => input).should == { :foo => input } + end + end + end + + context "runtime manipulations" do + subject do + Puppet::Interface.new(:runtime_manipulations, '1.0.0') do + action :foo do + when_invoked do |options| options end + end + end + end + + let :action do subject.get_action :foo end + + it "should be the face default action if default is set true" do + subject.get_default_action.should be_nil + action.default = true + subject.get_default_action.should == action + end end end diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb index 4ad8787c5..514a624b1 100755 --- a/spec/unit/interface/face_collection_spec.rb +++ b/spec/unit/interface/face_collection_spec.rb @@ -25,39 +25,9 @@ describe Puppet::Interface::FaceCollection do @original_required.each {|f| $".push f unless $".include? f } end - describe "::prefix_match?" do - # want have - { ['1.0.0', '1.0.0'] => true, - ['1.0', '1.0.0'] => true, - ['1', '1.0.0'] => true, - ['1.0.0', '1.1.0'] => false, - ['1.0', '1.1.0'] => false, - ['1', '1.1.0'] => true, - ['1.0.1', '1.0.0'] => false, - }.each do |data, result| - it "should return #{result.inspect} for prefix_match?(#{data.join(', ')})" do - subject.prefix_match?(*data).should == result - end - end - end - - describe "::validate_version" do - { '10.10.10' => true, - '1.2.3.4' => false, - '10.10.10beta' => true, - '10.10' => false, - '123' => false, - 'v1.1.1' => false, - }.each do |input, result| - it "should#{result ? '' : ' not'} permit #{input.inspect}" do - subject.validate_version(input).should(result ? be_true : be_false) - end - end - end - describe "::[]" do before :each do - subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10 + subject.instance_variable_get("@faces")[:foo][SemVer.new('0.0.1')] = 10 end it "should return the face with the given name" do @@ -65,7 +35,8 @@ describe Puppet::Interface::FaceCollection do end it "should attempt to load the face if it isn't found" do - subject.expects(:require).with('puppet/face/bar') + subject.expects(:require).once.with('puppet/face/bar') + subject.expects(:require).once.with('puppet/face/0.0.1/bar') subject["bar", '0.0.1'] end @@ -75,13 +46,13 @@ describe Puppet::Interface::FaceCollection do end it "should return true if the face specified is registered" do - subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10 + subject.instance_variable_get("@faces")[:foo][SemVer.new('0.0.1')] = 10 subject["foo", '0.0.1'].should == 10 end it "should attempt to require the face if it is not registered" do subject.expects(:require).with do |file| - subject.instance_variable_get("@faces")[:bar]['0.0.1'] = true + subject.instance_variable_get("@faces")[:bar][SemVer.new('0.0.1')] = true file == 'puppet/face/bar' end subject["bar", '0.0.1'].should be_true @@ -94,7 +65,8 @@ describe Puppet::Interface::FaceCollection do it "should return false if the face file itself is missing" do subject.stubs(:require). - raises(LoadError, 'no such file to load -- puppet/face/bar') + raises(LoadError, 'no such file to load -- puppet/face/bar').then. + raises(LoadError, 'no such file to load -- puppet/face/0.0.1/bar') subject["bar", '0.0.1'].should be_false end @@ -127,11 +99,49 @@ describe Puppet::Interface::FaceCollection do end end + describe "::get_action_for_face" do + it "should return an action on the current face" do + Puppet::Face::FaceCollection.get_action_for_face(:huzzah, :bar, :current). + should be_an_instance_of Puppet::Interface::Action + end + + it "should return an action on an older version of a face" do + action = Puppet::Face::FaceCollection. + get_action_for_face(:huzzah, :obsolete, :current) + + action.should be_an_instance_of Puppet::Interface::Action + action.face.version.should == SemVer.new('1.0.0') + end + + it "should load the full older version of a face" do + action = Puppet::Face::FaceCollection. + get_action_for_face(:huzzah, :obsolete, :current) + + action.face.version.should == SemVer.new('1.0.0') + action.face.should be_action :obsolete_in_core + end + + it "should not add obsolete actions to the current version" do + action = Puppet::Face::FaceCollection. + get_action_for_face(:huzzah, :obsolete, :current) + + action.face.version.should == SemVer.new('1.0.0') + action.face.should be_action :obsolete_in_core + + current = Puppet::Face[:huzzah, :current] + current.version.should == SemVer.new('2.0.1') + current.should_not be_action :obsolete_in_core + current.should_not be_action :obsolete + end + end + describe "::register" do it "should store the face by name" do face = Puppet::Face.new(:my_face, '0.0.1') subject.register(face) - subject.instance_variable_get("@faces").should == {:my_face => {'0.0.1' => face}} + subject.instance_variable_get("@faces").should == { + :my_face => { face.version => face } + } end end diff --git a/spec/unit/interface/option_spec.rb b/spec/unit/interface/option_spec.rb index e77b46e79..e73561fba 100755 --- a/spec/unit/interface/option_spec.rb +++ b/spec/unit/interface/option_spec.rb @@ -97,4 +97,48 @@ describe Puppet::Interface::Option do end end end + + context "defaults" do + subject { Puppet::Interface::Option.new(face, "--foo") } + + it "should work sanely if member variables are used for state" do + subject.default = proc { @foo ||= 0; @foo += 1 } + subject.default.should == 1 + subject.default.should == 2 + subject.default.should == 3 + end + + context "with no default" do + it { should_not be_has_default } + its :default do should be_nil end + + it "should set a proc as default" do + expect { subject.default = proc { 12 } }.should_not raise_error + end + + [1, {}, [], Object.new, "foo"].each do |input| + it "should reject anything but a proc (#{input.class})" do + expect { subject.default = input }.to raise_error ArgumentError, /not a proc/ + end + end + end + + context "with a default" do + before :each do subject.default = proc { [:foo] } end + + it { should be_has_default } + its :default do should == [:foo] end + + it "should invoke the block every time" do + subject.default.object_id.should_not == subject.default.object_id + subject.default.should == subject.default + end + + it "should allow replacing the default proc" do + subject.default.should == [:foo] + subject.default = proc { :bar } + subject.default.should == :bar + end + end + end end diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb index 8bbbcc857..4ff71ac3d 100755 --- a/spec/unit/interface_spec.rb +++ b/spec/unit/interface_spec.rb @@ -126,14 +126,11 @@ describe Puppet::Interface do end it "should try to require faces that are not known" do - pending "mocking require causes random stack overflow" - subject::FaceCollection.expects(:require).with "puppet/face/foo" - subject[:foo, '0.0.1'] + subject::FaceCollection.expects(:load_face).with(:foo, :current) + subject::FaceCollection.expects(:load_face).with(:foo, '0.0.1') + expect { subject[:foo, '0.0.1'] }.to raise_error Puppet::Error end - it "should be able to load all actions in all search paths" - - it_should_behave_like "things that declare options" do def add_options_to(&block) subject.new(:with_options, '0.0.1', &block) @@ -174,6 +171,14 @@ describe Puppet::Interface do face.get_action(:foo).options.should =~ [:quux] face.get_action(:bar).options.should =~ [:quux] end + + it "should only list options and not aliases" do + face = subject.new(:face_options, '0.0.1') do + option "--bar", "-b", "--foo-bar" + end + face.options.should =~ [:bar] + end + end describe "with inherited options" do diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb index 8d38657f9..822c76296 100755 --- a/spec/unit/module_spec.rb +++ b/spec/unit/module_spec.rb @@ -505,12 +505,38 @@ describe Puppet::Module do mod.metadata_file.should == mod.metadata_file end - it "should know if it has a metadata file" do + it "should have metadata if it has a metadata file and its data is not empty" do FileTest.expects(:exist?).with(@module.metadata_file).returns true + File.stubs(:read).with(@module.metadata_file).returns "{\"foo\" : \"bar\"}" @module.should be_has_metadata end + it "should have metadata if it has a metadata file and its data is not empty" do + FileTest.expects(:exist?).with(@module.metadata_file).returns true + File.stubs(:read).with(@module.metadata_file).returns "{\"foo\" : \"bar\"}" + + @module.should be_has_metadata + end + + it "should not have metadata if has a metadata file and its data is empty" do + FileTest.expects(:exist?).with(@module.metadata_file).returns true + File.stubs(:read).with(@module.metadata_file).returns "/* ++-----------------------------------------------------------------------+ +| | +| ==> DO NOT EDIT THIS FILE! <== | +| | +| You should edit the `Modulefile` and run `puppet-module build` | +| to generate the `metadata.json` file for your releases. | +| | ++-----------------------------------------------------------------------+ +*/ + +{}" + + @module.should_not be_has_metadata + end + it "should know if it is missing a metadata file" do FileTest.expects(:exist?).with(@module.metadata_file).returns false @@ -528,16 +554,16 @@ describe Puppet::Module do Puppet::Module.new("yay") end - describe "when loading the medatada file", :if => Puppet.features.json? do + describe "when loading the medatada file", :if => Puppet.features.pson? do before do @data = { - :license => "GPL2", - :author => "luke", - :version => "1.0", - :source => "http://foo/", + :license => "GPL2", + :author => "luke", + :version => "1.0", + :source => "http://foo/", :puppetversion => "0.25" } - @text = @data.to_json + @text = @data.to_pson @module = Puppet::Module.new("foo") @module.stubs(:metadata_file).returns "/my/file" @@ -552,9 +578,12 @@ describe Puppet::Module do it "should fail if #{attr} is not present in the metadata file" do @data.delete(attr.to_sym) - @text = @data.to_json + @text = @data.to_pson File.stubs(:read).with("/my/file").returns @text - lambda { @module.load_metadata }.should raise_error(Puppet::Module::MissingMetadata) + lambda { @module.load_metadata }.should raise_error( + Puppet::Module::MissingMetadata, + "No #{attr} module metadata provided for foo" + ) end end diff --git a/spec/unit/network/authconfig_spec.rb b/spec/unit/network/authconfig_spec.rb index c47b2e0c5..ca94cc1ab 100755 --- a/spec/unit/network/authconfig_spec.rb +++ b/spec/unit/network/authconfig_spec.rb @@ -184,6 +184,29 @@ describe Puppet::Network::AuthConfig do @authconfig.read end + it "should strip whitespace around ACE" do + acl = stub 'acl', :info + + @fd.stubs(:each).multiple_yields('[puppetca]', ' allow 127.0.0.1 , 172.16.10.0 ') + @rights.stubs(:newright).with("[puppetca]", 1, 'dummy').returns(acl) + + acl.expects(:allow).with('127.0.0.1') + acl.expects(:allow).with('172.16.10.0') + + @authconfig.read + end + + it "should allow ACE inline comments" do + acl = stub 'acl', :info + + @fd.stubs(:each).multiple_yields('[puppetca]', ' allow 127.0.0.1 # will it work?') + @rights.stubs(:newright).with("[puppetca]", 1, 'dummy').returns(acl) + + acl.expects(:allow).with('127.0.0.1') + + @authconfig.read + end + it "should create an allow ACE on each subsequent allow" do acl = stub 'acl', :info diff --git a/spec/unit/network/handler/fileserver_spec.rb b/spec/unit/network/handler/fileserver_spec.rb index 08852634d..851736e76 100755 --- a/spec/unit/network/handler/fileserver_spec.rb +++ b/spec/unit/network/handler/fileserver_spec.rb @@ -25,6 +25,38 @@ describe Puppet::Network::Handler::FileServer do @mount = Puppet::Network::Handler::FileServer::Mount.new("some_path", @basedir) end + describe "when parsing the fileserver.conf" do + it "should create a valid mount when a valid conf is read" do + config_file = tmpfile('fileserver.conf') + mountdir = tmpdir('mountdir') + + conf_text = <<-HEREDOC + [mymount] + path #{mountdir} + allow anyone.com + deny nobody.com + HEREDOC + File.open(config_file, 'w') { |f| f.write conf_text } + + fs = Puppet::Network::Handler::FileServer.new(:Config => config_file) + mounts = fs.instance_variable_get(:@mounts) + mount = mounts["mymount"] + mount.path == mountdir + mount.instance_variable_get(:@declarations).map {|d| d.pattern}.should =~ [["com", "nobody"], ["com", "anyone"]] + end + + ['path', 'allow', 'deny'].each do |arg| + it "should error if config file doesn't specify a mount for #{arg} argument" do + config_file = tmpfile('fileserver.conf') + File.open(config_file, 'w') { |f| f.puts "#{arg} 127.0.0.1/24" } + + expect { + Puppet::Network::Handler::FileServer.new(:Config => config_file) + }.should raise_error(Puppet::Network::Handler::FileServerError, "No mount specified for argument #{arg} 127.0.0.1/24") + end + end + end + it "should list a single directory" do @mount.list("/", false, false).should == [["/", "directory"]] end diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb index e1403997f..bebbb874f 100755 --- a/spec/unit/network/rest_authconfig_spec.rb +++ b/spec/unit/network/rest_authconfig_spec.rb @@ -29,7 +29,7 @@ describe Puppet::Network::RestAuthConfig do params = {:ip => "127.0.0.1", :node => "me", :environment => :env, :authenticated => true} @acl.expects(:is_request_forbidden_and_why?).with("path", :save, "to/resource", params).returns(nil) - @authconfig.allowed?("path", :save, "to/resource", params) + @authconfig.check_authorization("path", :save, "to/resource", params) end describe "when defining an acl with mk_acl" do diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb index 8f4910af6..896167a2b 100755 --- a/spec/unit/resource/catalog_spec.rb +++ b/spec/unit/resource/catalog_spec.rb @@ -500,7 +500,7 @@ describe Puppet::Resource::Catalog, "when compiling" do lambda { @catalog.alias(@one, "other") }.should_not raise_error end - it "should create aliases for resources isomorphic resources whose names do not match their titles" do + it "should create aliases for isomorphic resources whose names do not match their titles" do resource = Puppet::Type::File.new(:title => "testing", :path => @basepath+"/something") @catalog.add_resource(resource) @@ -508,7 +508,7 @@ describe Puppet::Resource::Catalog, "when compiling" do @catalog.resource(:file, @basepath+"/something").should equal(resource) end - it "should not create aliases for resources non-isomorphic resources whose names do not match their titles" do + it "should not create aliases for non-isomorphic resources whose names do not match their titles" do resource = Puppet::Type.type(:exec).new(:title => "testing", :command => "echo", :path => %w{/bin /usr/bin /usr/local/bin}) @catalog.add_resource(resource) @@ -524,11 +524,6 @@ describe Puppet::Resource::Catalog, "when compiling" do @catalog.resource("notify", "other").should equal(@one) end - it "should ignore conflicting aliases that point to the aliased resource" do - @catalog.alias(@one, "other") - lambda { @catalog.alias(@one, "other") }.should_not raise_error - end - it "should fail to add an alias if the aliased name already exists" do @catalog.add_resource @one proc { @catalog.alias @two, "one" }.should raise_error(ArgumentError) @@ -582,6 +577,58 @@ describe Puppet::Resource::Catalog, "when compiling" do @catalog.create_resource :file, args @catalog.resource("File[/yay]").should equal(resource) end + + describe "when adding resources with multiple namevars" do + before :each do + Puppet::Type.newtype(:multiple) do + newparam(:color, :namevar => true) + newparam(:designation, :namevar => true) + + def self.title_patterns + [ [ + /^(\w+) (\w+)$/, + [ + [:color, lambda{|x| x}], + [:designation, lambda{|x| x}] + ] + ] ] + end + end + end + + it "should add an alias using the uniqueness key" do + @resource = Puppet::Type.type(:multiple).new(:title => "some resource", :color => "red", :designation => "5") + + @catalog.add_resource(@resource) + @catalog.resource(:multiple, "some resource").must == @resource + @catalog.resource("Multiple[some resource]").must == @resource + @catalog.resource("Multiple[red 5]").must == @resource + end + + it "should conflict with a resource with the same uniqueness key" do + @resource = Puppet::Type.type(:multiple).new(:title => "some resource", :color => "red", :designation => "5") + @other = Puppet::Type.type(:multiple).new(:title => "another resource", :color => "red", :designation => "5") + + @catalog.add_resource(@resource) + expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError, /Cannot alias Multiple\[another resource\] to \["red", "5"\].*resource \["Multiple", "red", "5"\] already defined/) + end + + it "should conflict when its uniqueness key matches another resource's title" do + @resource = Puppet::Type.type(:file).new(:title => "/tmp/foo") + @other = Puppet::Type.type(:file).new(:title => "another file", :path => "/tmp/foo") + + @catalog.add_resource(@resource) + expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError, /Cannot alias File\[another file\] to \["\/tmp\/foo"\].*resource \["File", "\/tmp\/foo"\] already defined/) + end + + it "should conflict when its uniqueness key matches the uniqueness key derived from another resource's title" do + @resource = Puppet::Type.type(:multiple).new(:title => "red leader") + @other = Puppet::Type.type(:multiple).new(:title => "another resource", :color => "red", :designation => "leader") + + @catalog.add_resource(@resource) + expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError, /Cannot alias Multiple\[another resource\] to \["red", "leader"\].*resource \["Multiple", "red", "leader"\] already defined/) + end + end end describe "when applying" do diff --git a/spec/unit/semver_spec.rb b/spec/unit/semver_spec.rb new file mode 100644 index 000000000..0e0457b6e --- /dev/null +++ b/spec/unit/semver_spec.rb @@ -0,0 +1,187 @@ +require 'spec_helper' +require 'semver' + +describe SemVer do + describe '::valid?' do + it 'should validate basic version strings' do + %w[ 0.0.0 999.999.999 v0.0.0 v999.999.999 ].each do |vstring| + SemVer.valid?(vstring).should be_true + end + end + + it 'should validate special version strings' do + %w[ 0.0.0foo 999.999.999bar v0.0.0a v999.999.999beta ].each do |vstring| + SemVer.valid?(vstring).should be_true + end + end + + it 'should fail to validate invalid version strings' do + %w[ nope 0.0foo 999.999 x0.0.0 z.z.z 1.2.3-beta 1.x.y ].each do |vstring| + SemVer.valid?(vstring).should be_false + end + end + end + + describe '::find_matching' do + before :all do + @versions = %w[ + 0.0.1 + 0.0.2 + 1.0.0rc1 + 1.0.0rc2 + 1.0.0 + 1.0.1 + 1.1.0 + 1.1.1 + 1.1.2 + 1.1.3 + 1.1.4 + 1.2.0 + 1.2.1 + 2.0.0rc1 + ].map { |v| SemVer.new(v) } + end + + it 'should match exact versions by string' do + @versions.each do |version| + SemVer.find_matching(version, @versions).should == version + end + end + + it 'should return nil if no versions match' do + %w[ 3.0.0 2.0.0rc2 1.0.0alpha ].each do |v| + SemVer.find_matching(v, @versions).should be_nil + end + end + + it 'should find the greatest match for partial versions' do + SemVer.find_matching('1.0', @versions).should == 'v1.0.1' + SemVer.find_matching('1.1', @versions).should == 'v1.1.4' + SemVer.find_matching('1', @versions).should == 'v1.2.1' + SemVer.find_matching('2', @versions).should == 'v2.0.0rc1' + SemVer.find_matching('2.1', @versions).should == nil + end + + + it 'should find the greatest match for versions with placeholders' do + SemVer.find_matching('1.0.x', @versions).should == 'v1.0.1' + SemVer.find_matching('1.1.x', @versions).should == 'v1.1.4' + SemVer.find_matching('1.x', @versions).should == 'v1.2.1' + SemVer.find_matching('1.x.x', @versions).should == 'v1.2.1' + SemVer.find_matching('2.x', @versions).should == 'v2.0.0rc1' + SemVer.find_matching('2.x.x', @versions).should == 'v2.0.0rc1' + SemVer.find_matching('2.1.x', @versions).should == nil + end + end + + describe 'instantiation' do + it 'should raise an exception when passed an invalid version string' do + expect { SemVer.new('invalidVersion') }.to raise_exception ArgumentError + end + + it 'should populate the appropriate fields for a basic version string' do + version = SemVer.new('1.2.3') + version.major.should == 1 + version.minor.should == 2 + version.tiny.should == 3 + version.special.should == '' + end + + it 'should populate the appropriate fields for a special version string' do + version = SemVer.new('3.4.5beta6') + version.major.should == 3 + version.minor.should == 4 + version.tiny.should == 5 + version.special.should == 'beta6' + end + end + + describe '#matched_by?' do + subject { SemVer.new('v1.2.3beta') } + + describe 'should match against' do + describe 'literal version strings' do + it { should be_matched_by('1.2.3beta') } + + it { should_not be_matched_by('1.2.3alpha') } + it { should_not be_matched_by('1.2.4beta') } + it { should_not be_matched_by('1.3.3beta') } + it { should_not be_matched_by('2.2.3beta') } + end + + describe 'partial version strings' do + it { should be_matched_by('1.2.3') } + it { should be_matched_by('1.2') } + it { should be_matched_by('1') } + end + + describe 'version strings with placeholders' do + it { should be_matched_by('1.2.x') } + it { should be_matched_by('1.x.3') } + it { should be_matched_by('1.x.x') } + it { should be_matched_by('1.x') } + end + end + end + + describe 'comparisons' do + describe 'against a string' do + it 'should just work' do + SemVer.new('1.2.3').should == '1.2.3' + end + end + + describe 'against a symbol' do + it 'should just work' do + SemVer.new('1.2.3').should == :'1.2.3' + end + end + + describe 'on a basic version (v1.2.3)' do + subject { SemVer.new('v1.2.3') } + + it { should == SemVer.new('1.2.3') } + + # Different major versions + it { should > SemVer.new('0.2.3') } + it { should < SemVer.new('2.2.3') } + + # Different minor versions + it { should > SemVer.new('1.1.3') } + it { should < SemVer.new('1.3.3') } + + # Different tiny versions + it { should > SemVer.new('1.2.2') } + it { should < SemVer.new('1.2.4') } + + # Against special versions + it { should > SemVer.new('1.2.3beta') } + it { should < SemVer.new('1.2.4beta') } + end + + describe 'on a special version (v1.2.3beta)' do + subject { SemVer.new('v1.2.3beta') } + + it { should == SemVer.new('1.2.3beta') } + + # Same version, final release + it { should < SemVer.new('1.2.3') } + + # Different major versions + it { should > SemVer.new('0.2.3') } + it { should < SemVer.new('2.2.3') } + + # Different minor versions + it { should > SemVer.new('1.1.3') } + it { should < SemVer.new('1.3.3') } + + # Different tiny versions + it { should > SemVer.new('1.2.2') } + it { should < SemVer.new('1.2.4') } + + # Against special versions + it { should > SemVer.new('1.2.3alpha') } + it { should < SemVer.new('1.2.3beta2') } + end + end +end diff --git a/spec/unit/type/user_spec.rb b/spec/unit/type/user_spec.rb index 71c9e1857..823b12f27 100755 --- a/spec/unit/type/user_spec.rb +++ b/spec/unit/type/user_spec.rb @@ -289,6 +289,14 @@ describe user do @password.change_to_s("other", "mypass").should_not be_include("mypass") end + it "should redact the password when displaying the old value" do + @password.is_to_s("currentpassword").should =~ /^\[old password hash redacted\]$/ + end + + it "should redact the password when displaying the new value" do + @password.should_to_s("newpassword").should =~ /^\[new password hash redacted\]$/ + end + it "should fail if a ':' is included in the password" do lambda { @password.should = "some:thing" }.should raise_error(Puppet::Error) end diff --git a/spec/unit/util/settings_spec.rb b/spec/unit/util/settings_spec.rb index aa50c8f3a..a2cd57a0c 100755 --- a/spec/unit/util/settings_spec.rb +++ b/spec/unit/util/settings_spec.rb @@ -1,5 +1,6 @@ #!/usr/bin/env rspec require 'spec_helper' +require 'ostruct' describe Puppet::Util::Settings do describe "when specifying defaults" do @@ -1104,4 +1105,14 @@ describe Puppet::Util::Settings do it "should cache the result" end + + describe "#writesub" do + it "should only pass valid arguments to File.open" do + settings = Puppet::Util::Settings.new + settings.stubs(:get_config_file_default).with(:privatekeydir).returns(OpenStruct.new(:mode => "750")) + + File.expects(:open).with("/path/to/keydir", "w", 750).returns true + settings.writesub(:privatekeydir, "/path/to/keydir") + end + end end |