[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 2.6.5-303-gfcfa26a

[Daniel Pittman]

The following commit has been merged in the upstream branch:
commit de6a2052c2aeda1cd76ba828936a9d6f0ac7e907
Author: Daniel Pittman <daniel at rimspace.net>
Date:   Tue Feb 22 11:18:19 2011 -0800

    (#5552) Clean up subcommand handling inside puppet cert.
    
    We now have a regular, testable mechanism for handling the legacy '--' version
    of subcommands, as well as a modern bareword subcommand pattern.  This makes
    it sensible to test command handling and avoid regressions.
    
    We identified a few quirks in the command line as part of this process.
    
    Pair-With: Jesse Wolfe <jesse at puppetlabs.com>
    Signed-off-by: Daniel Pittman <daniel at puppetlabs.com>

diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb
index 467b0c8..c8aad18 100644
--- a/lib/puppet/application/cert.rb
+++ b/lib/puppet/application/cert.rb
@@ -5,17 +5,19 @@ class Puppet::Application::Cert < Puppet::Application
   should_parse_config
   run_mode :master
 
-  attr_accessor :cert_mode, :all, :ca, :digest, :signed
+  attr_accessor :all, :ca, :digest, :signed
 
-  def find_mode(opt)
-    require 'puppet/ssl/certificate_authority'
-    modes = Puppet::SSL::CertificateAuthority::Interface::INTERFACE_METHODS
-    tmp = opt.sub("--", '').to_sym
-    @cert_mode = modes.include?(tmp) ? tmp : nil
+  def subcommand
+    @subcommand
+  end
+  def subcommand=(name)
+    # Handle the nasty, legacy mapping of "clean" to "destroy".
+    sub = name.to_sym
+    @subcommand = (sub == :clean ? :destroy : sub)
   end
 
   option("--clean", "-c") do
-    @cert_mode = :destroy
+    self.subcommand = "destroy"
   end
 
   option("--all", "-a") do
@@ -37,7 +39,7 @@ class Puppet::Application::Cert < Puppet::Application
   require 'puppet/ssl/certificate_authority/interface'
   Puppet::SSL::CertificateAuthority::Interface::INTERFACE_METHODS.reject {|m| m == :destroy }.each do |method|
     option("--#{method}", "-#{method.to_s[0,1]}") do
-      find_mode("--#{method}")
+      self.subcommand = method
     end
   end
 
@@ -54,8 +56,8 @@ class Puppet::Application::Cert < Puppet::Application
       hosts = command_line.args.collect { |h| h.downcase }
     end
     begin
-      @ca.apply(:revoke, :to => hosts) if @cert_mode == :destroy
-      @ca.apply(@cert_mode, :to => hosts, :digest => @digest)
+      @ca.apply(:revoke, :to => hosts) if subcommand == :destroy
+      @ca.apply(subcommand, :to => hosts, :digest => @digest)
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       puts detail.to_s
@@ -64,11 +66,12 @@ class Puppet::Application::Cert < Puppet::Application
   end
 
   def setup
+    require 'puppet/ssl/certificate_authority'
     exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
     Puppet::Util::Log.newdestination :console
 
-    if [:generate, :destroy].include? @cert_mode
+    if [:generate, :destroy].include? subcommand
       Puppet::SSL::Host.ca_location = :local
     else
       Puppet::SSL::Host.ca_location = :only
@@ -82,4 +85,11 @@ class Puppet::Application::Cert < Puppet::Application
       exit(23)
     end
   end
+
+  def parse_options
+    # handle the bareword subcommand pattern.
+    result = super
+    self.subcommand ||= self.command_line.args.shift
+    result
+  end
 end
diff --git a/spec/unit/application/cert_spec.rb b/spec/unit/application/cert_spec.rb
index 4663fc9..2f57d07 100755
--- a/spec/unit/application/cert_spec.rb
+++ b/spec/unit/application/cert_spec.rb
@@ -51,7 +51,7 @@ describe Puppet::Application::Cert do
 
   it "should set cert_mode to :destroy for --clean" do
     @cert_app.handle_clean(0)
-    @cert_app.cert_mode.should == :destroy
+    @cert_app.subcommand.should == :destroy
   end
 
   it "should set all to true for --all" do
@@ -68,7 +68,7 @@ describe Puppet::Application::Cert do
     it "should set cert_mode to #{method} with option --#{method}" do
       @cert_app.send("handle_#{method}".to_sym, nil)
 
-      @cert_app.cert_mode.should == method
+      @cert_app.subcommand.should == method
     end
   end
 
@@ -114,19 +114,19 @@ describe Puppet::Application::Cert do
     end
 
     it "should set the ca_location to :local if the cert_mode is generate" do
-      @cert_app.find_mode('--generate')
+      @cert_app.subcommand = 'generate'
       Puppet::SSL::Host.expects(:ca_location=).with(:local)
       @cert_app.setup
     end
 
     it "should set the ca_location to :local if the cert_mode is destroy" do
-      @cert_app.find_mode('--destroy')
+      @cert_app.subcommand = 'destroy'
       Puppet::SSL::Host.expects(:ca_location=).with(:local)
       @cert_app.setup
     end
 
     it "should set the ca_location to :only if the cert_mode is print" do
-      @cert_app.find_mode('--print')
+      @cert_app.subcommand = 'print'
       Puppet::SSL::Host.expects(:ca_location=).with(:only)
       @cert_app.setup
     end
@@ -171,24 +171,54 @@ describe Puppet::Application::Cert do
       @cert_app.main
     end
 
-    it "should delegate to ca.apply with current set cert_mode" do
-      @cert_app.cert_mode = "currentmode"
+    it "should revoke cert if cert_mode is clean" do
+      @cert_app.subcommand = :destroy
       @cert_app.command_line.stubs(:args).returns(["host"])
 
-      @ca.expects(:apply).with { |cert_mode,to| cert_mode == "currentmode" }
+      @ca.expects(:apply).with { |cert_mode,to| cert_mode == :revoke }
+      @ca.expects(:apply).with { |cert_mode,to| cert_mode == :destroy }
 
       @cert_app.main
     end
+  end
 
-    it "should revoke cert if cert_mode is clean" do
-      @cert_app.cert_mode = :destroy
-      @cert_app.command_line.stubs(:args).returns(["host"])
+  describe "when identifying subcommands" do
+    before :each do
+      @cert_app.all = false
+      @ca = stub_everything 'ca'
+      @cert_app.ca = @ca
+    end
 
-      @ca.expects(:apply).with { |cert_mode,to| cert_mode == :revoke }
-      @ca.expects(:apply).with { |cert_mode,to| cert_mode == :destroy }
+    %w{list revoke generate sign print verify fingerprint}.each do |cmd|
+      short = cmd[0,1]
+      [cmd, "--#{cmd}", "-#{short}"].each do |option|
+        # In our command line '-v' was eaten by 'verbose', so we can't consume
+        # it here; this is a special case from our otherwise standard
+        # processing. --daniel 2011-02-22
+        next if option == "-v"
 
-      @cert_app.main
+        it "should recognise '#{option}'" do
+          args = [option, "fun.example.com"]
+
+          @cert_app.command_line.stubs(:args).returns(args)
+          @cert_app.parse_options
+          @cert_app.subcommand.should == cmd.to_sym
+
+          args.should == ["fun.example.com"]
+        end
+      end
     end
 
+    %w{clean --clean -c}.each do |ugly|
+      it "should recognise the '#{ugly}' option as destroy" do
+        args = [ugly, "fun.example.com"]
+
+        @cert_app.command_line.stubs(:args).returns(args)
+        @cert_app.parse_options
+        @cert_app.subcommand.should == :destroy
+
+        args.should == ["fun.example.com"]
+      end
+    end
   end
 end

-- 
Puppet packaging for Debian