Making the first pass at using requests instead of

specifying the terminus class.  The individual ssl
classes now work, but the ssl host class doesn't yet.

13 files changed, 47 insertions, 165 deletions

diff --git a/lib/puppet/indirector/certificate_request/ca_file.rb b/lib/puppet/indirector/certificate_request/ca_file.rb
index 24c262ef3..8c43f18d8 100644
--- a/lib/puppet/indirector/certificate_request/ca_file.rb
+++ b/lib/puppet/indirector/certificate_request/ca_file.rb
@@ -6,9 +6,9 @@ class Puppet::SSL::CertificateRequest::CaFile < Puppet::Indirector::SslFile
 
     store_in :csrdir
 
-    def save(instance, *args)
+    def save(request)
         result = super
-        Puppet.notice "%s has a waiting certificate request" % instance.name
+        Puppet.notice "%s has a waiting certificate request" % request.key
         result
     end
 end
diff --git a/lib/puppet/indirector/key/file.rb b/lib/puppet/indirector/key/file.rb
index 03e94ed2d..41d30a2d4 100644
--- a/lib/puppet/indirector/key/file.rb
+++ b/lib/puppet/indirector/key/file.rb
@@ -11,24 +11,24 @@ class Puppet::SSL::Key::File < Puppet::Indirector::SslFile
     end
 
     # Remove the public key, in addition to the private key
-    def destroy(key, options = {})
+    def destroy(request)
         super
 
-        return unless FileTest.exist?(public_key_path(key.name))
+        return unless FileTest.exist?(public_key_path(request.key))
 
         begin
-            File.unlink(public_key_path(key.name))
+            File.unlink(public_key_path(request.key))
         rescue => detail
-            raise Puppet::Error, "Could not remove %s public key: %s" % [key.name, detail]
+            raise Puppet::Error, "Could not remove %s public key: %s" % [request.key, detail]
         end
     end
 
     # Save the public key, in addition to the private key.
-    def save(key, options = {})
+    def save(request)
         super
 
         begin
-            File.open(public_key_path(key.name), "w") { |f| f.print key.content.public_key.to_pem }
+            File.open(public_key_path(request.key), "w") { |f| f.print request.instance.content.public_key.to_pem }
         rescue => detail
             raise Puppet::Error, "Could not write %s: %s" % [key, detail]
         end
diff --git a/lib/puppet/indirector/ssl_file.rb b/lib/puppet/indirector/ssl_file.rb
index 17cb0a144..c66d71e91 100644
--- a/lib/puppet/indirector/ssl_file.rb
+++ b/lib/puppet/indirector/ssl_file.rb
@@ -25,46 +25,46 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
     end
 
     # Remove our file.
-    def destroy(file, options = {})
-        path = path(file.name)
-        raise Puppet::Error.new("File %s does not exist; cannot destroy" % [file]) unless FileTest.exist?(path)
+    def destroy(request)
+        path = path(request.key)
+        raise Puppet::Error.new("File %s does not exist; cannot destroy" % [request.key]) unless FileTest.exist?(path)
 
         begin
             File.unlink(path)
         rescue => detail
-            raise Puppet::Error, "Could not remove %s: %s" % [file, detail]
+            raise Puppet::Error, "Could not remove %s: %s" % [request.key, detail]
         end
     end
 
     # Find the file on disk, returning an instance of the model.
-    def find(name, options = {})
-        path = path(name)
+    def find(request)
+        path = path(request.key)
 
         return nil unless FileTest.exist?(path)
 
-        result = model.new(name)
+        result = model.new(request.key)
         result.read(path)
         result
     end
 
     # Save our file to disk.
-    def save(file, options = {})
-        path = path(file.name)
+    def save(request)
+        path = path(request.key)
         dir = File.dirname(path)
 
-        raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [file, dir]) unless FileTest.directory?(dir)
-        raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [file, dir]) unless FileTest.writable?(dir)
+        raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [request.key, dir]) unless FileTest.directory?(dir)
+        raise Puppet::Error.new("Cannot save %s; parent directory %s does not exist" % [request.key, dir]) unless FileTest.writable?(dir)
 
         begin
-            File.open(path, "w") { |f| f.print file.to_s }
+            File.open(path, "w") { |f| f.print request.instance.to_s }
         rescue => detail
-            raise Puppet::Error, "Could not write %s: %s" % [file, detail]
+            raise Puppet::Error, "Could not write %s: %s" % [request.key, detail]
         end
     end
 
     # Search for more than one file.  At this point, it just returns
     # an instance for every file in the directory.
-    def search(options = {})
+    def search(request)
         dir = collection_directory
         Dir.entries(dir).reject { |file| file !~ /\.pem$/ }.collect do |file|
             name = file.sub(/\.pem$/, '')
diff --git a/lib/puppet/ssl/base.rb b/lib/puppet/ssl/base.rb
index 674330373..ab040152d 100644
--- a/lib/puppet/ssl/base.rb
+++ b/lib/puppet/ssl/base.rb
@@ -1,5 +1,4 @@
 require 'puppet/ssl'
-require 'puppet/ssl/indirection_hooks'
 
 # The base class for wrapping SSL instances.
 class Puppet::SSL::Base
diff --git a/lib/puppet/ssl/certificate.rb b/lib/puppet/ssl/certificate.rb
index 697b2e785..9b1e2a79a 100644
--- a/lib/puppet/ssl/certificate.rb
+++ b/lib/puppet/ssl/certificate.rb
@@ -1,12 +1,16 @@
 require 'puppet/ssl/base'
 
-# Manage certificates themselves.
+# Manage certificates themselves.  This class has no
+# 'generate' method because the CA is responsible
+# for turning CSRs into certificates; we can only
+# retrieve them from the CA (or not, as is often
+# the case).
 class Puppet::SSL::Certificate < Puppet::SSL::Base
     # This is defined from the base class
     wraps OpenSSL::X509::Certificate
 
     extend Puppet::Indirector
-    indirects :certificate, :extend => Puppet::SSL::IndirectionHooks
+    indirects :certificate
 
     # Indicate where we should get our signed certs from.
     def self.ca_is(dest)
@@ -22,24 +26,4 @@ class Puppet::SSL::Certificate < Puppet::SSL::Base
             :local
         end
     end
-
-    # Request a certificate from our CA.
-    def generate(request)
-        if self.class.ca_location == :local
-            terminus = :ca_file
-        else
-            terminus = :rest
-        end
-
-        # Save our certificate request.
-        request.save :in => terminus
-
-        # And see if we can retrieve the certificate.
-        if cert = self.class.find(name, :in => terminus)
-            @content = cert.content
-            return true
-        else
-            return false
-        end
-    end
 end
diff --git a/lib/puppet/ssl/certificate_request.rb b/lib/puppet/ssl/certificate_request.rb
index e8cbbbade..4ca6d9899 100644
--- a/lib/puppet/ssl/certificate_request.rb
+++ b/lib/puppet/ssl/certificate_request.rb
@@ -5,7 +5,7 @@ class Puppet::SSL::CertificateRequest < Puppet::SSL::Base
     wraps OpenSSL::X509::Request
 
     extend Puppet::Indirector
-    indirects :certificate_request, :extend => Puppet::SSL::IndirectionHooks
+    indirects :certificate_request
 
     # How to create a certificate request with our system defaults.
     def generate(key)
diff --git a/lib/puppet/ssl/indirection_hooks.rb b/lib/puppet/ssl/indirection_hooks.rb
deleted file mode 100644
index c2a3442c0..000000000
--- a/lib/puppet/ssl/indirection_hooks.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-#
-#  Created by Luke Kanies on 2008-3-10.
-#  Copyright (c) 2008. All rights reserved.
-
-require 'uri'
-require 'puppet/ssl'
-
-# This module is used to pick the appropriate terminus
-# in certificate indirections.  This is necessary because
-# we need the ability to choose between interacting with the CA
-# or the local certs.
-module Puppet::SSL::IndirectionHooks
-    # Pick an appropriate terminus based on what's specified, defaulting to :file.
-    def select_terminus(full_uri, options = {})
-        return options[:to] || options[:in] || :file
-    end
-end
diff --git a/lib/puppet/ssl/key.rb b/lib/puppet/ssl/key.rb
index 65294ac00..124d4c2d7 100644
--- a/lib/puppet/ssl/key.rb
+++ b/lib/puppet/ssl/key.rb
@@ -6,7 +6,7 @@ class Puppet::SSL::Key < Puppet::SSL::Base
     wraps OpenSSL::PKey::RSA
 
     extend Puppet::Indirector
-    indirects :key, :terminus_class => :file
+    indirects :key
 
     attr_reader :password_file
 
diff --git a/spec/unit/indirector/key/file.rb b/spec/unit/indirector/key/file.rb
index add80b26c..a7297d522 100755
--- a/spec/unit/indirector/key/file.rb
+++ b/spec/unit/indirector/key/file.rb
@@ -34,6 +34,8 @@ describe Puppet::SSL::Key::File do
             @real_key = stub 'sslkey', :public_key => @public_key
 
             @key = stub 'key', :name => "myname", :content => @real_key
+
+            @request = stub 'request', :key => "myname", :instance => @key
         end
 
         it "should save the public key when saving the private key" do
@@ -49,7 +51,7 @@ describe Puppet::SSL::Key::File do
 
             fh.expects(:print).with "my pem"
 
-            @searcher.save(@key)
+            @searcher.save(@request)
         end
 
         it "should destroy the public key when destroying the private key" do
@@ -58,7 +60,7 @@ describe Puppet::SSL::Key::File do
             FileTest.expects(:exist?).with(@publickey).returns true
             File.expects(:unlink).with(@publickey)
 
-            @searcher.destroy(@key)
+            @searcher.destroy(@request)
         end
 
         it "should not fail if the public key does not exist when deleting the private key" do
@@ -68,7 +70,7 @@ describe Puppet::SSL::Key::File do
             FileTest.expects(:exist?).with(@publickey).returns false
             File.expects(:unlink).with(@publickey).never
 
-            @searcher.destroy(@key)
+            @searcher.destroy(@request)
         end
     end
 end
diff --git a/spec/unit/indirector/ssl_file.rb b/spec/unit/indirector/ssl_file.rb
index 90faf1533..31f82856e 100755
--- a/spec/unit/indirector/ssl_file.rb
+++ b/spec/unit/indirector/ssl_file.rb
@@ -40,6 +40,8 @@ describe Puppet::Indirector::SslFile do
 
             @cert = stub 'certificate', :name => "myname"
             @certpath = File.join(@path, "myname" + ".pem")
+
+            @request = stub 'request', :key => @cert.name, :instance => @cert
         end
         
         describe "when choosing the location for certificates" do
@@ -55,7 +57,7 @@ describe Puppet::Indirector::SslFile do
                 end
 
                 it "should return nil" do
-                    @searcher.find(@cert.name).should be_nil
+                    @searcher.find(@request).should be_nil
                 end
             end
 
@@ -71,7 +73,7 @@ describe Puppet::Indirector::SslFile do
 
                     model.expects(:new).with("myname").returns cert
                     cert.expects(:read).with(@certpath)
-                    @searcher.find("myname").should equal(cert)
+                    @searcher.find(@request).should equal(cert)
                 end
             end
         end
@@ -79,13 +81,13 @@ describe Puppet::Indirector::SslFile do
         describe "when saving certificates to disk" do
             it "should fail if the directory is absent" do
                 FileTest.expects(:directory?).with(File.dirname(@certpath)).returns false
-                lambda { @searcher.save(@cert) }.should raise_error(Puppet::Error)
+                lambda { @searcher.save(@request) }.should raise_error(Puppet::Error)
             end
 
             it "should fail if the directory is not writeable" do
                 FileTest.stubs(:directory?).returns true
                 FileTest.expects(:writable?).with(File.dirname(@certpath)).returns false
-                lambda { @searcher.save(@cert) }.should raise_error(Puppet::Error)
+                lambda { @searcher.save(@request) }.should raise_error(Puppet::Error)
             end
 
             it "should save to the path the output of converting the certificate to a string" do
@@ -99,8 +101,7 @@ describe Puppet::Indirector::SslFile do
 
                 fh.expects(:print).with("mycert")
 
-                @searcher.save(@cert)
-
+                @searcher.save(@request)
             end
         end
 
@@ -111,7 +112,7 @@ describe Puppet::Indirector::SslFile do
                 end
 
                 it "should fail" do
-                    lambda { @searcher.destroy(@cert) }.should raise_error(Puppet::Error)
+                    lambda { @searcher.destroy(@request) }.should raise_error(Puppet::Error)
                 end
             end
 
@@ -122,7 +123,7 @@ describe Puppet::Indirector::SslFile do
 
                 it "should unlink the certificate file" do
                     File.expects(:unlink).with(@certpath)
-                    @searcher.destroy(@cert)
+                    @searcher.destroy(@request)
                 end
             end
         end
@@ -141,7 +142,7 @@ describe Puppet::Indirector::SslFile do
                 @model.expects(:new).with("one").returns one
                 @model.expects(:new).with("two").returns two
 
-                @searcher.search.should == [one, two]
+                @searcher.search(@request).should == [one, two]
             end
 
             it "should read each certificate in using the model's :read method" do
@@ -152,7 +153,7 @@ describe Puppet::Indirector::SslFile do
 
                 @model.expects(:new).with("one").returns one
 
-                @searcher.search
+                @searcher.search(@request)
             end
 
             it "should skip any files that do not match /\.pem$/" do
@@ -162,7 +163,7 @@ describe Puppet::Indirector::SslFile do
 
                 @model.expects(:new).with("one").returns one
 
-                @searcher.search
+                @searcher.search(@request)
             end
         end
     end
diff --git a/spec/unit/ssl/certificate.rb b/spec/unit/ssl/certificate.rb
index 18b432e9e..7aaf8534d 100755
--- a/spec/unit/ssl/certificate.rb
+++ b/spec/unit/ssl/certificate.rb
@@ -21,16 +21,6 @@ describe Puppet::SSL::Certificate do
         @class.indirection.name.should == :certificate
     end
 
-    it "should default to the :file terminus class" do
-        @class.indirection.terminus(:file).expects(:find).with "myname"
-        @class.find("myname")
-    end
-
-    it "should allow specification of a different terminus class" do
-        @class.indirection.terminus(:ca_file).expects(:find).with { |*args| args[0] == "myname" }
-        @class.find("myname", :in => :ca_file)
-    end
-
     it "should default to a local certificate authority" do
         @class.ca_location.should == :local
     end
@@ -79,66 +69,4 @@ describe Puppet::SSL::Certificate do
             @certificate.to_text.should == "certificatetext"
         end
     end
-
-    describe "when generating the certificate" do
-        before do
-            @cert = @class.new("test")
-            @request = mock 'request'
-        end
-
-        describe "from a local ca" do
-            before do
-                @class.stubs(:ca_location).returns :local
-            end
-
-            it "should save the certificate request to and try to find the cert in the :ca_file terminus" do
-                @request.expects(:save).with(:in => :ca_file)
-                @cert.class.expects(:find).with("test", :in => :ca_file)
-
-                @cert.generate(@request)
-            end
-        end
-
-        describe "from a remote ca" do
-            before do
-                @class.stubs(:ca_location).returns :remote
-            end
-
-            it "should save the certificate request to and try to find the cert in the :rest terminus" do
-                @request.expects(:save).with(:in => :rest)
-                @cert.class.expects(:find).with("test", :in => :rest)
-
-                @cert.generate(@request)
-            end
-        end
-
-        describe "successfully" do
-            it "should set its content to the content of the retrieved certificate" do
-                @request.stubs(:save)
-                newcert = mock 'newcert', :content => "realcert"
-                @cert.class.expects(:find).returns(newcert)
-
-                @cert.generate(@request)
-
-                @cert.content.should == "realcert"
-            end
-
-            it "should return true" do
-                @request.stubs(:save)
-                newcert = mock 'newcert', :content => "realcert"
-                @cert.class.expects(:find).returns(newcert)
-
-                @cert.generate(@request).should be_true
-            end
-        end
-
-        describe "unsuccessfully" do
-            it "should return false" do
-                @request.stubs(:save)
-                @cert.class.expects(:find).returns(nil)
-
-                @cert.generate(@request).should be_false
-            end
-        end
-    end
 end
diff --git a/spec/unit/ssl/certificate_request.rb b/spec/unit/ssl/certificate_request.rb
index 2b4545bee..19e72d65e 100755
--- a/spec/unit/ssl/certificate_request.rb
+++ b/spec/unit/ssl/certificate_request.rb
@@ -22,16 +22,6 @@ describe Puppet::SSL::CertificateRequest do
         @class.new("myname").name.should == "myname"
     end
 
-    it "should default to the :file terminus class" do
-        @class.indirection.terminus(:file).expects(:find).with "myname"
-        @class.find("myname")
-    end
-
-    it "should allow specification of a different terminus class" do
-        @class.indirection.terminus(:ca_file).expects(:find).with { |*args| args[0] == "myname" }
-        @class.find("myname", :in => :ca_file)
-    end
-
     describe "when managing instances" do
         before do
             @request = @class.new("myname")
diff --git a/spec/unit/ssl/key.rb b/spec/unit/ssl/key.rb
index 9955f468a..98b7e8a2b 100755
--- a/spec/unit/ssl/key.rb
+++ b/spec/unit/ssl/key.rb
@@ -17,11 +17,6 @@ describe Puppet::SSL::Key do
         @class.indirection.name.should == :key
     end
 
-    it "should default to the :file terminus class" do
-        @class.indirection.terminus(:file).expects(:find).with "myname"
-        @class.find("myname")
-    end
-
     describe "when managing instances" do
         before do
             @key = @class.new("myname")