Fix for #2890 (the cached certificates that would not die)

This patch implements the two-part suggestion from the ticket;

1) a client that receives a certificate that doesn't match its current
private key does not accept, store or use the certificate--instead it
removes any locally cached copies and acts as if the certificate had
never been found.

2) a puppetmaster that receives a csr from a client for whom it already
has a signed certificate now honors the request and considers it to
supercede any previously signed certificates.

In order to make the cache expiration work as expected, I changed a few
assumptions in the caching system:

* The expiration of a cached certificate is the earlier of the envelope
  expiration and the certificate's expiration, as opposed to just overriding
  the cache value
* Telling the cache to expire an item now removes it from the cache if
  possible, rather than just setting an expiration date in the past and
  hoping that somebody notices.

Signed-off-by: Markus Roberts <Markus@reality.com>

1 files changed, 29 insertions, 18 deletions

diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb
index 220aa24fe..ca2a412e3 100755
--- a/spec/unit/indirector/indirection.rb
+++ b/spec/unit/indirector/indirection.rb
@@ -536,7 +536,7 @@ describe Puppet::Indirector::Indirection do
                     @indirection.expire("/my/key")
                 end
 
-                it "should log that it is expiring any found instance" do
+                it "should log when expiring a found instance" do
                     @cache.expects(:find).returns @cached
                     @cache.stubs(:save)
 
@@ -545,33 +545,44 @@ describe Puppet::Indirector::Indirection do
                     @indirection.expire("/my/key")
                 end
 
-                it "should set the cached instance's expiration to a time in the past" do
-                    @cache.expects(:find).returns @cached
-                    @cache.stubs(:save)
+                describe "and the terminus supports removal of cache items with destroy" do
+                    it "should destroy the cached instance" do
+                        @cache.expects(:find).returns @cached
+                        @cache.expects(:destroy).with { |r| r.method == :destroy and r.key == "/my/key" }
+                        @cache.expects(:save).never
+                        @indirection.expire("/my/key")
+                    end
+                end
 
-                    @cached.expects(:expiration=).with { |t| t < Time.now }
+                describe "and the terminus does not support removal of cache items with destroy" do
+                    it "should set the cached instance's expiration to a time in the past" do
+                        @cache.expects(:find).returns @cached
+                        @cache.stubs(:save)
 
-                    @indirection.expire("/my/key")
-                end
+                        @cached.expects(:expiration=).with { |t| t < Time.now }
 
-                it "should save the now expired instance back into the cache" do
-                    @cache.expects(:find).returns @cached
+                        @indirection.expire("/my/key")
+                    end
 
-                    @cached.expects(:expiration=).with { |t| t < Time.now }
+                    it "should save the now expired instance back into the cache" do
+                        @cache.expects(:find).returns @cached
 
-                    @cache.expects(:save)
+                        @cached.expects(:expiration=).with { |t| t < Time.now }
 
-                    @indirection.expire("/my/key")
-                end
+                        @cache.expects(:save)
 
-                it "should use a request to save the expired resource to the cache" do
-                    @cache.expects(:find).returns @cached
+                        @indirection.expire("/my/key")
+                    end
 
-                    @cached.expects(:expiration=).with { |t| t < Time.now }
+                    it "should use a request to save the expired resource to the cache" do
+                        @cache.expects(:find).returns @cached
 
-                    @cache.expects(:save).with { |r| r.is_a?(Puppet::Indirector::Request) and r.instance == @cached and r.method == :save }.returns(@cached)
+                        @cached.expects(:expiration=).with { |t| t < Time.now }
 
-                    @indirection.expire("/my/key")
+                        @cache.expects(:save).with { |r| r.is_a?(Puppet::Indirector::Request) and r.instance == @cached and r.method == :save }.returns(@cached)
+
+                        @indirection.expire("/my/key")
+                    end
                 end
             end
         end