This landed in 2.6.next as part of fixing an issue scheduled for
2.6.6.  Hoisting this support up to the next level seemed like a good
longer term investment, which I might tackle when I have some more
free time.

Daniel

On Tue, Feb 22, 2011 at 13:13, Luke Kanies <[email protected]> wrote:
> What branch would you expect to merge this into?
>
> If it's master, is it worth us running through what I've been doing in my 
> Interfaces work?  It seems pretty similar to this.
>
> On Feb 22, 2011, at 11:36 AM, Daniel Pittman wrote:
>
>> From: Daniel Pittman <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Daniel Pittman <[email protected]>
>> ---
>> lib/puppet/application/cert.rb     |   32 +++++++++++++-------
>> spec/unit/application/cert_spec.rb |   58 +++++++++++++++++++++++++++--------
>> 2 files changed, 65 insertions(+), 25 deletions(-)
>>
>> 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
>> --
>> 1.7.4.1
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "Puppet Developers" group.
>> To post to this group, send email to [email protected].
>> To unsubscribe from this group, send email to 
>> [email protected].
>> For more options, visit this group at 
>> http://groups.google.com/group/puppet-dev?hl=en.
>>
>
>
> --
> When your work speaks for itself, don't interrupt.
>        -- Henry J. Kaiser
> ---------------------------------------------------------------------
> Luke Kanies  -|-   http://puppetlabs.com   -|-   +1(615)594-8199
>
>
>
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Puppet Developers" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to 
> [email protected].
> For more options, visit this group at 
> http://groups.google.com/group/puppet-dev?hl=en.
>
>



-- 
⎋ Puppet Labs Developer – http://puppetlabs.com
✉ Daniel Pittman <[email protected]>
✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775
♲ Made with 100 percent post-consumer electrons

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to