diff --git a/lib/puppet/application/certificate.rb b/lib/puppet/application/certificate.rb index f4b13ffe0..eacb830b2 100644 --- a/lib/puppet/application/certificate.rb +++ b/lib/puppet/application/certificate.rb @@ -1,27 +1,18 @@ 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 if location == :local && !Puppet::SSL::CertificateAuthority.ca? self.class.run_mode("master") self.set_run_mode self.class.run_mode end super end - end diff --git a/lib/puppet/application/string_base.rb b/lib/puppet/application/string_base.rb index 76b0a46fd..09d02c079 100644 --- a/lib/puppet/application/string_base.rb +++ b/lib/puppet/application/string_base.rb @@ -1,130 +1,150 @@ require 'puppet/application' require 'puppet/string' class Puppet::Application::StringBase < Puppet::Application should_parse_config run_mode :agent option("--debug", "-d") do |arg| Puppet::Util::Log.level = :debug end option("--verbose", "-v") do Puppet::Util::Log.level = :info end option("--format FORMAT") do |arg| @format = arg.to_sym end option("--mode RUNMODE", "-r") do |arg| raise "Invalid run mode #{arg}; supported modes are user, agent, master" unless %w{user agent master}.include?(arg) self.class.run_mode(arg.to_sym) set_run_mode self.class.run_mode end attr_accessor :string, :action, :type, :arguments, :format attr_writer :exit_code # This allows you to set the exit code if you don't want to just exit # immediately but you need to indicate a failure. def exit_code @exit_code || 0 end # Override this if you need custom rendering. def render(result) render_method = Puppet::Network::FormatHandler.format(format).render_method if render_method == "to_pson" jj result exit(0) else result.send(render_method) end end def preinit super trap(:INT) do $stderr.puts "Cancelling String" exit(0) end # We need to parse enough of the command line out early, to identify what # the action is, so that we can obtain the full set of options to parse. # TODO: These should be configurable versions, through a global # '--version' option, but we don't implement that yet... --daniel 2011-03-29 @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym @string = Puppet::String[@type, :current] @format = @string.default_format # Now, walk the command line and identify the action. We skip over # arguments based on introspecting the action and all, and find the first # non-option word to use as the action. action = nil index = -1 until @action or (index += 1) >= command_line.args.length do item = command_line.args[index] if item =~ /^-/ then - option = @string.options.find { |a| item =~ /^-+#{a}\b/ } + option = @string.options.find do |name| + item =~ /^-+#{name.to_s.gsub(/[-_]/, '[-_]')}(?:[ =].*)?$/ + end if option then option = @string.get_option(option) # If we have an inline argument, just carry on. We don't need to # care about optional vs mandatory in that case because we do a real # parse later, and that will totally take care of raising the error # when we get there. --daniel 2011-04-04 if option.takes_argument? and !item.index('=') then index += 1 unless (option.optional_argument? and command_line.args[index + 1] =~ /^-/) end + elsif option = find_global_settings_argument(item) then + unless Puppet.settings.boolean? option.name then + # As far as I can tell, we treat non-bool options as always having + # a mandatory argument. --daniel 2011-04-05 + index += 1 # ...so skip the argument. + end else raise ArgumentError, "Unknown option #{item.sub(/=.*$/, '').inspect}" end else action = @string.get_action(item.to_sym) if action.nil? then raise ArgumentError, "#{@string} does not have an #{item.inspect} action!" end @action = action end end @action or raise ArgumentError, "No action given on the command line!" # Finally, we can interact with the default option code to build behaviour # around the full set of options we now know we support. @action.options.each do |option| option = @action.get_option(option) # make it the object. self.class.option(*option.optparse) # ...and make the CLI parse it. end end + def find_global_settings_argument(item) + Puppet.settings.each do |name, object| + object.optparse_args.each do |arg| + next unless arg =~ /^-/ + # sadly, we have to emulate some of optparse here... + pattern = /^#{arg.sub('[no-]', '').sub(/[ =].*$/, '')}(?:[ =].*)?$/ + pattern.match item and return object + end + end + return nil # nothing found. + end + def setup Puppet::Util::Log.newdestination :console @arguments = command_line.args # Note: because of our definition of where the action is set, we end up # with it *always* being the first word of the remaining set of command # line arguments. So, strip that off when we construct the arguments to # pass down to the string action. --daniel 2011-04-04 @arguments.delete_at(0) # We copy all of the app options to the end of the call; This allows each # action to read in the options. This replaces the older model where we # would invoke the action with options set as global state in the # interface object. --daniel 2011-03-28 @arguments << options end def main # Call the method associated with the provided action (e.g., 'find'). if result = @string.send(@action.name, *arguments) puts render(result) end exit(exit_code) end end diff --git a/lib/puppet/string/certificate.rb b/lib/puppet/string/certificate.rb index b231cafb1..e8773ae2e 100644 --- a/lib/puppet/string/certificate.rb +++ b/lib/puppet/string/certificate.rb @@ -1,30 +1,46 @@ 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) end end 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 } end end 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) end end - end diff --git a/lib/puppet/string/option.rb b/lib/puppet/string/option.rb index f499e4b95..be7bbb76e 100644 --- a/lib/puppet/string/option.rb +++ b/lib/puppet/string/option.rb @@ -1,80 +1,80 @@ class Puppet::String::Option attr_reader :parent attr_reader :name attr_reader :aliases attr_reader :optparse attr_accessor :desc def takes_argument? !!@argument end def optional_argument? !!@optional_argument end def initialize(parent, *declaration, &block) @parent = parent @optparse = [] # Collect and sort the arguments in the declaration. dups = {} declaration.each do |item| if item.is_a? String and item.to_s =~ /^-/ then unless item =~ /^-[a-z]\b/ or item =~ /^--[^-]/ then raise ArgumentError, "#{item.inspect}: long options need two dashes (--)" end @optparse << item # Duplicate checking... name = optparse_to_name(item) if dup = dups[name] then raise ArgumentError, "#{item.inspect}: duplicates existing alias #{dup.inspect} in #{@parent}" else dups[name] = item end else raise ArgumentError, "#{item.inspect} is not valid for an option argument" end end if @optparse.empty? then raise ArgumentError, "No option declarations found while building" end # Now, infer the name from the options; we prefer the first long option as # the name, rather than just the first option. @name = optparse_to_name(@optparse.find do |a| a =~ /^--/ end || @optparse.first) @aliases = @optparse.map { |o| optparse_to_name(o) } # Do we take an argument? If so, are we consistent about it, because # incoherence here makes our life super-difficult, and we can more easily # relax this rule later if we find a valid use case for it. --daniel 2011-03-30 @argument = @optparse.any? { |o| o =~ /[ =]/ } if @argument and not @optparse.all? { |o| o =~ /[ =]/ } then raise ArgumentError, "Option #{@name} is inconsistent about taking an argument" end # Is our argument optional? The rules about consistency apply here, also, # just like they do to taking arguments at all. --daniel 2011-03-30 @optional_argument = @optparse.any? { |o| o.include? "[" } if @optional_argument and not @optparse.all? { |o| o.include? "[" } then raise ArgumentError, "Option #{@name} is inconsistent about the argument being optional" end end # to_s and optparse_to_name are roughly mirrored, because they are used to # transform strings to name symbols, and vice-versa. This isn't a full # bidirectional transformation though. def to_s @name.to_s.tr('_', '-') end def optparse_to_name(declaration) - unless found = declaration.match(/^-+([^= ]+)/) or found.length != 1 then + unless found = declaration.match(/^-+(?:\[no-\])?([^ =]+)/) then raise ArgumentError, "Can't find a name in the declaration #{declaration.inspect}" end - name = found.captures.first.sub('[no-]', '').tr('-', '_') + name = found.captures.first.tr('-', '_') raise "#{name.inspect} is an invalid option name" unless name.to_s =~ /^[a-z]\w*$/ name.to_sym end end diff --git a/spec/unit/application/certificate_spec.rb b/spec/unit/application/certificate_spec.rb index 6666f54f7..3d2215ded 100755 --- a/spec/unit/application/certificate_spec.rb +++ b/spec/unit/application/certificate_spec.rb @@ -1,16 +1,20 @@ #!/usr/bin/env ruby 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/application/string_base_spec.rb b/spec/unit/application/string_base_spec.rb index cd24b6c49..3f8ae73b6 100755 --- a/spec/unit/application/string_base_spec.rb +++ b/spec/unit/application/string_base_spec.rb @@ -1,156 +1,185 @@ #!/usr/bin/env ruby require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb') require 'puppet/application/string_base' require 'tmpdir' class Puppet::Application::StringBase::Basetest < Puppet::Application::StringBase end describe Puppet::Application::StringBase do before :all do @dir = Dir.mktmpdir $LOAD_PATH.push(@dir) FileUtils.mkdir_p(File.join @dir, 'puppet', 'string') File.open(File.join(@dir, 'puppet', 'string', 'basetest.rb'), 'w') do |f| f.puts "Puppet::String.define(:basetest, '0.0.1')" end Puppet::String.define(:basetest, '0.0.1') do option("--[no-]boolean") option("--mandatory MANDATORY") option("--optional [OPTIONAL]") action :foo do option("--action") when_invoked { |*args| args.length } end end end after :all do FileUtils.remove_entry_secure @dir $LOAD_PATH.pop end let :app do app = Puppet::Application::StringBase::Basetest.new app.stubs(:exit) app.stubs(:puts) app.command_line.stubs(:subcommand_name).returns 'subcommand' Puppet::Util::Log.stubs(:newdestination) app end + describe "#find_global_settings_argument" do + it "should not match --ca to --ca-location" do + option = mock('ca option', :optparse_args => ["--ca"]) + Puppet.settings.expects(:each).yields(:ca, option) + + app.find_global_settings_argument("--ca-location").should be_nil + end + end + describe "#preinit" do before :each do app.command_line.stubs(:args).returns %w{} end describe "parsing the command line" do context "with just an action" do before :all do app.command_line.stubs(:args).returns %w{foo} app.preinit end it "should set the string based on the type" do app.string.name.should == :basetest end it "should set the format based on the string default" do app.format.should == :pson end it "should find the action" do app.action.should be app.action.name.should == :foo end end it "should fail if no action is given" do expect { app.preinit }. should raise_error ArgumentError, /No action given/ end it "should report a sensible error when options with = fail" do app.command_line.stubs(:args).returns %w{--action=bar foo} expect { app.preinit }. should raise_error ArgumentError, /Unknown option "--action"/ end it "should fail if an action option is before the action" do app.command_line.stubs(:args).returns %w{--action foo} expect { app.preinit }. should raise_error ArgumentError, /Unknown option "--action"/ end it "should fail if an unknown option is before the action" do app.command_line.stubs(:args).returns %w{--bar foo} expect { app.preinit }. should raise_error ArgumentError, /Unknown option "--bar"/ end it "should not fail if an unknown option is after the action" do app.command_line.stubs(:args).returns %w{foo --bar} app.preinit app.action.name.should == :foo app.string.should_not be_option :bar app.action.should_not be_option :bar end it "should accept --bar as an argument to a mandatory option after action" do app.command_line.stubs(:args).returns %w{foo --mandatory --bar} app.preinit and app.parse_options app.action.name.should == :foo app.options.should == { :mandatory => "--bar" } end it "should accept --bar as an argument to a mandatory option before action" do app.command_line.stubs(:args).returns %w{--mandatory --bar foo} app.preinit and app.parse_options app.action.name.should == :foo app.options.should == { :mandatory => "--bar" } end it "should not skip when --foo=bar is given" do app.command_line.stubs(:args).returns %w{--mandatory=bar --bar foo} expect { app.preinit }. should raise_error ArgumentError, /Unknown option "--bar"/ end + + { "boolean options before" => %w{--trace foo}, + "boolean options after" => %w{foo --trace} + }.each do |name, args| + it "should accept global boolean settings #{name} the action" do + app.command_line.stubs(:args).returns args + app.preinit && app.parse_options + Puppet[:trace].should be_true + end + end + + { "before" => %w{--syslogfacility user1 foo}, + " after" => %w{foo --syslogfacility user1} + }.each do |name, args| + it "should accept global settings with arguments #{name} the action" do + app.command_line.stubs(:args).returns args + app.preinit && app.parse_options + Puppet[:syslogfacility].should == "user1" + end + end end end describe "#setup" do it "should remove the action name from the arguments" do app.command_line.stubs(:args).returns %w{--mandatory --bar foo} app.preinit and app.parse_options and app.setup app.arguments.should == [{ :mandatory => "--bar" }] end it "should pass positional arguments" do app.command_line.stubs(:args).returns %w{--mandatory --bar foo bar baz quux} app.preinit and app.parse_options and app.setup app.arguments.should == ['bar', 'baz', 'quux', { :mandatory => "--bar" }] end end describe "#main" do before do app.string = Puppet::String[:basetest, '0.0.1'] app.action = app.string.get_action(:foo) app.format = :pson app.arguments = ["myname", "myarg"] end it "should send the specified verb and name to the string" do app.string.expects(:foo).with(*app.arguments) app.main end it "should use its render method to render any result" do app.expects(:render).with(app.arguments.length + 1) app.main end end end diff --git a/spec/unit/string/certificate_spec.rb b/spec/unit/string/certificate_spec.rb index f6d53688b..9fdc5aab8 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 "#6983: This is broken in the actual string..." + Puppet::SSL::Host.expects(:ca_location=).with(:foo) + Puppet::SSL::Host.indirection.expects(:save) + subject.sign :ca_location => :foo + end end