Actually, I just spent the couple of minutes, and it turns out that the handling is almost identical between the two implementations. So, we are pretty much doing the same thing you were.
Daniel On Tue, Feb 22, 2011 at 13:59, Daniel Pittman <[email protected]> wrote: > 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 > -- ⎋ 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.
