This includes various style changes, and assorted fixes to testing. Paired-With: Matt Robinson
Signed-off-by: Pieter van de Bruggen <pie...@puppetlabs.com> --- Local-branch: feature/2.7.x/1886 lib/puppet/face/ca.rb | 30 +++--- lib/puppet/face/node/clean.rb | 111 ++++++++++----------- spec/integration/node/facts_spec.rb | 2 +- spec/unit/face/node_spec.rb | 132 +++++++++++++------------ spec/unit/indirector/report/processor_spec.rb | 2 +- 5 files changed, 139 insertions(+), 138 deletions(-) diff --git a/lib/puppet/face/ca.rb b/lib/puppet/face/ca.rb index e643530..00591d6 100644 --- a/lib/puppet/face/ca.rb +++ b/lib/puppet/face/ca.rb @@ -6,21 +6,21 @@ Puppet::Face.define(:ca, '0.1.0') do summary "Local Puppet Certificate Authority management." - description <<TEXT -This provides local management of the Puppet Certificate Authority. + 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 + 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 <<-end -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. - end + 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." @@ -37,12 +37,12 @@ verification failure reported. option "--subject PATTERN" do summary "Only list if the subject matches PATTERN." - description <<TEXT -Only include certificates or requests where 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 + PATTERN is interpreted as a regular expression, allowing complex + filtering of the content. + TEXT end when_invoked do |options| diff --git a/lib/puppet/face/node/clean.rb b/lib/puppet/face/node/clean.rb index 10d6239..a4df1bf 100644 --- a/lib/puppet/face/node/clean.rb +++ b/lib/puppet/face/node/clean.rb @@ -1,29 +1,29 @@ -Puppet::Indirector::Face.define(:node, '0.0.1') do +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 + 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 @@ -39,82 +39,78 @@ EOT 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 - begin - nodes.each do |node| - node = node.downcase - clean_cert(node) - clean_cached_facts(node) - clean_cached_node(node) - clean_reports(node) - clean_storeconfigs(node,options[:unexport]) - end - rescue => detail - puts detail.backtrace if Puppet[:trace] - puts detail.to_s - end + 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) + clean_storeconfigs(node, unexport) + end + # clean signed cert for +host+ def clean_cert(node) - if Puppet::SSL::Host.ca_location == :local - ca.apply(:revoke, :to => [node]) - ca.apply(:destroy, :to => [node]) - Puppet.info "%s certificates removed from ca" % 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 %s certs as this host is not a CA" % node + 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 "%s's facts removed" % node + Puppet.info "#{node}'s facts removed" end # clean cached node +host+ def clean_cached_node(node) Puppet::Node.indirection.destroy(node) - Puppet.info "%s's cached node removed" % 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 "%s's reports removed" % node + Puppet.info "#{node}'s reports removed" end # clean storeconfig for +node+ - def clean_storeconfigs(node,do_unexport=false) + 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 %s in storedconfigs." % node + Puppet.notice "No entries found for #{node} in storedconfigs." return end if do_unexport unexport(rails_node) - Puppet.notice "Force %s's exported resources to absent" % 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 "%s storeconfigs removed" % node + 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] + 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 @@ -122,7 +118,7 @@ EOT if ensure_param = resource.param_values.find( :first, - :conditions => [ 'param_name_id = ?', param_name.id] + :conditions => [ 'param_name_id = ?', param_name.id ] ) line = ensure_param.line.to_i Puppet::Rails::ParamValue.delete(ensure_param.id); @@ -134,21 +130,22 @@ EOT :line => line, :param_name => param_name ) - Puppet.info("%s has been marked as \"absent\"" % resource.name) + Puppet.info("#{resource.name} has been marked as \"absent\"") end end end - def ca - @ca ||= Puppet::SSL::CertificateAuthority.instance - end - def environment - @environemnt ||= Puppet::Node::Environment.new + @environment ||= Puppet::Node::Environment.new end def type_is_ensurable(resource) - (type=Puppet::Type.type(resource.restype)) && type.validattr?(:ensure) || \ - (type = environment.known_resource_types.find_definition('',resource.restype)) && type.arguments.keys.include?('ensure') + 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 \ No newline at end of file +end diff --git a/spec/integration/node/facts_spec.rb b/spec/integration/node/facts_spec.rb index f54d7f9..e87a0bd 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/unit/face/node_spec.rb b/spec/unit/face/node_spec.rb index 4ba1d13..7151353 100755 --- a/spec/unit/face/node_spec.rb +++ b/spec/unit/face/node_spec.rb @@ -3,9 +3,42 @@ 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'], + "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 @@ -19,8 +52,13 @@ describe Puppet::Face[:node, '0.0.1'] do end it "should accept more than one node name" do - expect { subject.clean(['hostname', 'hostname2'],{}) }.should_not raise_error - expect { subject.clean(['hostname', 'hostname2', 'hostname3'],{:unexport => true}) }.should_not raise_error + 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 @@ -29,20 +67,13 @@ describe Puppet::Face[:node, '0.0.1'] do end context "clean action" do - subject { Puppet::Face[:node, 'current'] } + subject { Puppet::Face[:node, :current] } before :each do Puppet::Util::Log.stubs(:newdestination) Puppet::Util::Log.stubs(:level=) end describe "during setup" do - before :each do - Puppet::Log.stubs(:newdestination) - Puppet::Log.stubs(:level=) - Puppet::Node::Facts.indirection.stubs(:terminus_class=) - Puppet::Node.stubs(:cache_class=) - end - 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) @@ -63,43 +94,16 @@ describe Puppet::Face[:node, '0.0.1'] do 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') + 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 running" do - - before :each do - @host = 'hostname' - Puppet.stubs(:info) - [ "cert", "cached_facts", "cached_node", "reports" ].each do |m| - subject.stubs("clean_#{m}".to_sym).with(@host) - end - subject.stubs(:clean_storeconfigs).with(@host,nil) - end - - [ "cert", "cached_facts", "cached_node", "reports" ].each do |m| - it "should clean #{m.sub('_',' ')}" do - subject.expects("clean_#{m}".to_sym).with(@host) - - subject.clean(@host) - end - end - - it "should clean storeconfigs" do - subject.expects(:clean_storeconfigs).with(@host,nil) - - subject.clean(@host) + 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 @@ -110,16 +114,16 @@ describe Puppet::Face[:node, '0.0.1'] do end it "should send the :destroy order to the ca if we are a CA" do - Puppet::SSL::Host.stubs(:ca_location).returns(:local) - @ca.expects(:apply).with { |cert_mode,to| cert_mode == :revoke } - @ca.expects(:apply).with { |cert_mode,to| cert_mode == :destroy } - + 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::Host.stubs(:ca_location).returns(:none) - @ca.expects(:apply).never + Puppet::SSL::CertificateAuthority.stubs(:ca?).returns(false) + @ca.expects(:revoke).never + @ca.expects(:destroy).never subject.clean_cert(@host) end end @@ -162,20 +166,20 @@ describe Puppet::Face[:node, '0.0.1'] do it "should connect to the database" do Puppet::Rails.expects(:connect) - subject.clean_storeconfigs(@host,false) + 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) + 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) + subject.clean_storeconfigs(@host, false) end end @@ -197,7 +201,7 @@ describe Puppet::Face[:node, '0.0.1'] do 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) + subject.clean_storeconfigs(@host, true) end describe "with an exported native type" do @@ -207,21 +211,21 @@ describe Puppet::Face[:node, '0.0.1'] do end it "should test a native type for ensure as an attribute" do - subject.clean_storeconfigs(@host,true) + 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) + 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) + subject.clean_storeconfigs(@host, true) end end @@ -229,14 +233,14 @@ describe Puppet::Face[:node, '0.0.1'] 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) + 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 unkown type" do Puppet::Type.stubs(:type).returns(nil) - Puppet::Resource::TypeCollection.any_instance.expects(:find_definition).with('',"File").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 @@ -244,17 +248,17 @@ describe Puppet::Face[:node, '0.0.1'] do 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::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) + 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::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) + subject.clean_storeconfigs(@host, true) end end end diff --git a/spec/unit/indirector/report/processor_spec.rb b/spec/unit/indirector/report/processor_spec.rb index fda0d90..c64cc7e 100755 --- a/spec/unit/indirector/report/processor_spec.rb +++ b/spec/unit/indirector/report/processor_spec.rb @@ -26,7 +26,7 @@ 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 => mock("report"), :key => 'node' + @request = stub 'request', :instance => stub("report", :host => 'hostname'), :key => 'node' end it "should not save the report if reports are set to 'none'" do -- 1.7.5.1 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to puppet-dev@googlegroups.com. To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.