[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

[Daniel Pittman]

The following commit has been merged in the experimental branch:
commit a03790d82a2c190d6f00ee7677617a7be04aa85d
Author: Daniel Pittman <daniel at puppetlabs.com>
Date:   Tue Apr 5 18:39:19 2011 -0700

    (#6972) Handle ca-location in the certificate string.
    
    This ports the existing certificate location configuration to be a string
    option, and then uses that to change the configuration.  This will leak state
    between calls, which is somewhat unavoidable, but should at least get the
    basic stuff right for the CLI.
    
    We eventually need the CA string to be supported by a stateless internal CA
    implementation that allows us to do the right thing  overall.
    
    Reviewed-By: Dan Bode <dan at puppetlabs.com>

diff --git a/lib/puppet/application/certificate.rb b/lib/puppet/application/certificate.rb
index f4b13ff..eacb830 100644
--- a/lib/puppet/application/certificate.rb
+++ b/lib/puppet/application/certificate.rb
@@ -1,18 +1,10 @@
 require 'puppet/application/indirection_base'
 
 class Puppet::Application::Certificate < Puppet::Application::IndirectionBase
-
-  # Luke used to call this --ca but that's taken by the global boolean --ca.
-  # Since these options map CA terminology to indirector terminology, it's
-  # now called --ca-location.
-  option "--ca-location CA_LOCATION" do |arg|
-    Puppet::SSL::Host.ca_location = arg.to_sym
-  end
-
   def setup
-
-    unless Puppet::SSL::Host.ca_location
-      raise ArgumentError, "You must have a CA location specified; use --ca-location to specify the location (remote, local, only)"
+    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
@@ -23,5 +15,4 @@ class Puppet::Application::Certificate < Puppet::Application::IndirectionBase
 
     super
   end
-
 end
diff --git a/lib/puppet/string/certificate.rb b/lib/puppet/string/certificate.rb
index b231caf..fdb0bc9 100644
--- a/lib/puppet/string/certificate.rb
+++ b/lib/puppet/string/certificate.rb
@@ -2,9 +2,24 @@ require 'puppet/string/indirector'
 require 'puppet/ssl/host'
 
 Puppet::String::Indirector.define(:certificate, '0.0.1') do
+  # REVISIT: This should use a pre-invoke hook to run the common code that
+  # needs to happen before we invoke any action; that would be much nicer than
+  # the "please repeat yourself" stuff found in here right now.
+  #
+  # option "--ca-location LOCATION" do
+  #   type [:whatever, :location, :symbols]
+  #   hook :before do |value|
+  #     Puppet::SSL::Host.ca_location = value
+  #   end
+  # end
+  #
+  # ...but should I pass the arguments as well?
+  # --daniel 2011-04-05
+  option "--ca-location LOCATION"
 
   action :generate do
     when_invoked do |name, options|
+      Puppet::SSL::Host.ca_location = options[:ca_location].to_sym
       host = Puppet::SSL::Host.new(name)
       host.generate_certificate_request
       host.certificate_request.class.indirection.save(host.certificate_request)
@@ -13,6 +28,7 @@ Puppet::String::Indirector.define(:certificate, '0.0.1') do
 
   action :list do
     when_invoked do |options|
+      Puppet::SSL::Host.ca_location = options[:ca_location].to_sym
       Puppet::SSL::Host.indirection.search("*", {
         :for => :certificate_request,
       }).map { |h| h.inspect }
@@ -21,6 +37,7 @@ Puppet::String::Indirector.define(:certificate, '0.0.1') do
 
   action :sign do
     when_invoked do |name, options|
+      Puppet::SSL::Host.ca_location = options[:ca_location].to_sym
       host = Puppet::SSL::Host.new(name)
       host.desired_state = 'signed'
       Puppet::SSL::Host.indirection.save(host)
diff --git a/spec/unit/application/certificate_spec.rb b/spec/unit/application/certificate_spec.rb
index 6666f54..3d2215d 100755
--- a/spec/unit/application/certificate_spec.rb
+++ b/spec/unit/application/certificate_spec.rb
@@ -4,13 +4,17 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/application/certificate'
 
 describe Puppet::Application::Certificate do
-  it "should be a subclass of Puppet::Application::IndirectionBase" do
-    Puppet::Application::Certificate.superclass.should equal(
-      Puppet::Application::IndirectionBase
-    )
+  it "should have a 'ca-location' option" do
+    # REVISIT: This is delegated from the string, and we will have a test
+    # there, so is this actually a valuable test?
+    subject.command_line.stubs(:args).returns %w{list}
+    subject.preinit
+    subject.should respond_to(:handle_ca_location)
   end
 
-  it "should have a 'ca' option" do
-    Puppet::Application::Certificate.new.should respond_to(:handle_ca_location)
+  it "should accept the ca-location option" do
+    subject.command_line.stubs(:args).returns %w{--ca-location local list}
+    subject.preinit and subject.parse_options and subject.setup
+    subject.arguments.should == [{ :ca_location => "local" }]
   end
 end
diff --git a/spec/unit/string/certificate_spec.rb b/spec/unit/string/certificate_spec.rb
index f6d5368..4afd581 100755
--- a/spec/unit/string/certificate_spec.rb
+++ b/spec/unit/string/certificate_spec.rb
@@ -1,6 +1,14 @@
-#!/usr/bin/env ruby
-
-require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+require 'puppet/ssl/host'
 
 describe Puppet::String[:certificate, '0.0.1'] do
+  it "should have a ca-location option" do
+    subject.should be_option :ca_location
+  end
+
+  it "should set the ca location when invoked" do
+    pending "The string itself is broken in this release."
+    Puppet::SSL::Host.expects(:ca_location=).with(:foo)
+    Puppet::SSL::Host.indirection.expects(:search)
+    subject.list :ca_location => :foo
+  end
 end

-- 
Puppet packaging for Debian