Fix #1886 - Add node cleanup capability

Here is a changeset that adds a new action to the puppet node face.

This application removes all traces of a node on the puppetmaster
(including certs, cached facts and nodes, reports, and storedconfig
entries).

Furthermore it is capable of unexporting 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.

Usage:
puppet node clean [--unexport] <host> [<host2> ...]

To achieve this we add different destroy methods to the different
parts of the indirector. So for example for yaml indirections we
already offer read access for the yaml, this changeset adds
the destroy handler which only removes the yaml file for
a request. This can be used to remove cached entries.

This work is based on the initial work of Brice Figureau
<brice-puppet@daysofwonder.com>

7 files changed, 500 insertions, 20 deletions

diff --git a/lib/puppet/face/node/clean.rb b/lib/puppet/face/node/clean.rb
new file mode 100644
index 000000000..10d6239ba
--- /dev/null
+++ b/lib/puppet/face/node/clean.rb
@@ -0,0 +1,154 @@
+Puppet::Indirector::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
+
+      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
+    end
+  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
+    else
+      Puppet.info "Not managing %s certs as this host is not a CA" % node
+    end
+  end
+
+  # clean facts for +host+
+  def clean_cached_facts(node)
+    Puppet::Node::Facts.indirection.destroy(node)
+    Puppet.info "%s's facts removed" % node
+  end
+
+  # clean cached node +host+
+  def clean_cached_node(node)
+    Puppet::Node.indirection.destroy(node)
+    Puppet.info "%s's cached node removed" % node
+  end
+
+  # clean node reports for +host+
+  def clean_reports(node)
+    Puppet::Transaction::Report.indirection.destroy(node)
+    Puppet.info "%s's reports removed" % node
+  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 %s in storedconfigs." % node
+      return
+    end
+
+    if do_unexport
+      unexport(rails_node)
+      Puppet.notice "Force %s's exported resources to absent" % node
+      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
+    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("%s has been marked as \"absent\"" % resource.name)
+      end
+    end
+  end
+
+  def ca
+    @ca ||= Puppet::SSL::CertificateAuthority.instance
+  end
+
+  def environment
+    @environemnt ||= 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')
+  end
+end
\ No newline at end of file
diff --git a/lib/puppet/indirector/report/processor.rb b/lib/puppet/indirector/report/processor.rb
index 81b379eb8..7bdadcb36 100644
--- a/lib/puppet/indirector/report/processor.rb
+++ b/lib/puppet/indirector/report/processor.rb
@@ -14,6 +14,12 @@ 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.
@@ -21,23 +27,17 @@ class Puppet::Transaction::Report::Processor < Puppet::Indirector::Code
   # compatible and that's good enough for now.
   def process(report)
     Puppet.debug "Recieved report to process from #{report.host}"
-    return if Puppet[:reports] == "none"
-
-    reports.each do |name|
-      Puppet.debug "Processing report from #{report.host} with processor #{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}'"
+    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
@@ -47,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/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/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/spec/unit/face/node_spec.rb b/spec/unit/face/node_spec.rb
index 027a4cce0..4ba1d134c 100755
--- a/spec/unit/face/node_spec.rb
+++ b/spec/unit/face/node_spec.rb
@@ -4,4 +4,260 @@ require 'puppet/face'
 
 describe Puppet::Face[:node, '0.0.1'] do
   it "REVISIT: really should have some tests"
+
+  describe "clean action" do
+    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 { subject.clean(['hostname', 'hostname2'],{}) }.should_not raise_error
+      expect { subject.clean(['hostname', 'hostname2', 'hostname3'],{:unexport => true}) }.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
+        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)
+
+          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 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)
+        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::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 }
+
+          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
+          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 unkown 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/indirector/report/processor_spec.rb b/spec/unit/indirector/report/processor_spec.rb
index fbc70a104..fda0d90fd 100755
--- a/spec/unit/indirector/report/processor_spec.rb
+++ b/spec/unit/indirector/report/processor_spec.rb
@@ -15,15 +15,21 @@ 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 => mock("report"), :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[:reports] = 'none'
 
@@ -34,9 +40,24 @@ describe Puppet::Transaction::Report::Processor, " when saving a report" do
     @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
 
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