diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb index 7a5ce3400..ea5ba4aaf 100644 --- a/lib/puppet/application/face_base.rb +++ b/lib/puppet/application/face_base.rb @@ -1,232 +1,246 @@ require 'puppet/application' require 'puppet/face' require 'optparse' require 'pp' class Puppet::Application::FaceBase < 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("--render-as FORMAT") do |format| self.render_as = format.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 :face, :action, :type, :arguments, :render_as def render_as=(format) if format == :json then @render_as = Puppet::Network::FormatHandler.format(:pson) else @render_as = Puppet::Network::FormatHandler.format(format) end @render_as or raise ArgumentError, "I don't know how to render '#{format}'" end def render(result) # Invoke the rendering hook supplied by the user, if appropriate. if hook = action.when_rendering(render_as.name) result = hook.call(result) end render_as.render(result) end def preinit super Signal.trap(:INT) do $stderr.puts "Cancelling Face" exit(0) end end def parse_options # 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. # REVISIT: 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 @face = Puppet::Face[@type, :current] # 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 + action_name = nil + index = -1 + until action_name or (index += 1) >= command_line.args.length do item = command_line.args[index] if item =~ /^-/ then option = @face.options.find do |name| item =~ /^-+#{name.to_s.gsub(/[-_]/, '[-_]')}(?:[ =].*)?$/ end if option then option = @face.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 elsif option = find_application_argument(item) then - index += 1 if (option[:argument] and option[:optional]) + index += 1 if (option[:argument] and not option[:optional]) else raise OptionParser::InvalidOption.new(item.sub(/=.*$/, '')) end else - @action = @face.get_action(item.to_sym) + # Stash away the requested action name for later, and try to fetch the + # action object it represents; if this is an invalid action name that + # will be nil, and handled later. + action_name = item.to_sym + @action = @face.get_action(action_name) end end if @action.nil? if @action = @face.get_default_action() then @is_default_action = true else - Puppet.err "#{face.name} does not have a default action, and no action was given" - Puppet.err Puppet::Face[:help, :current].help(@face.name) + # REVISIT: ...and this horror thanks to our log setup, which doesn't + # initialize destinations until the setup method, which we will never + # reach. We could also just print here, but that is actually a little + # uglier and nastier in the long term, in which we should do log setup + # earlier if at all possible. --daniel 2011-05-31 + Puppet::Util::Log.newdestination(:console) + + face = @face.name + action = action_name.nil? ? 'default' : "'#{action_name}'" + msg = "'#{face}' has no #{action} action. See `puppet help #{face}`." + Puppet.err(msg) + exit false end end # Now 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 # ...and invoke our parent to parse all the command line options. super 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 find_application_argument(item) self.class.option_parser_commands.each do |options, function| options.each do |option| next unless option =~ /^-/ pattern = /^#{option.sub('[no-]', '').sub(/[ =].*$/, '')}(?:[ =].*)?$/ next unless pattern.match(item) return { :argument => option =~ /[ =]/, :optional => option =~ /[ =]\[/ } end end return nil # not 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 face action. --daniel 2011-04-04 # Of course, now that we have default actions, we should leave the # "action" name on if we didn't actually consume it when we found our # action. @arguments.delete_at(0) unless @is_default_action # 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 # If we don't have a rendering format, set one early. self.render_as ||= (@action.render_as || :console) end def main status = false # Call the method associated with the provided action (e.g., 'find'). unless @action puts Puppet::Face[:help, :current].help(@face.name) raise "#{face} does not respond to action #{arguments.first}" end # We need to do arity checking here because this is generic code # calling generic methods – that have argument defaulting. We need to # make sure we don't accidentally pass the options as the first # argument to a method that takes one argument. eg: # # puppet facts find # => options => {} # @arguments => [{}] # => @face.send :bar, {} # # def face.bar(argument, options = {}) # => bar({}, {}) # oops! we thought the options were the # # positional argument!! # # We could also fix this by making it mandatory to pass the options on # every call, but that would make the Ruby API much more annoying to # work with; having the defaulting is a much nicer convention to have. # # We could also pass the arguments implicitly, by having a magic # 'options' method that was visible in the scope of the action, which # returned the right stuff. # # That sounds attractive, but adds complications to all sorts of # things, especially when you think about how to pass options when you # are writing Ruby code that calls multiple faces. Especially if # faces are involved in that. ;) # # --daniel 2011-04-27 if (arity = @action.positional_arg_count) > 0 unless (count = arguments.length) == arity then s = arity == 2 ? '' : 's' raise ArgumentError, "puppet #{@face.name} #{@action.name} takes #{arity-1} argument#{s}, but you gave #{count-1}" end end result = @face.send(@action.name, *arguments) puts render(result) unless result.nil? status = true rescue Exception => detail puts detail.backtrace if Puppet[:trace] Puppet.err detail.to_s Puppet.err "Try 'puppet help #{@face.name} #{@action.name}' for usage" ensure exit status end end diff --git a/spec/lib/puppet/face/basetest.rb b/spec/lib/puppet/face/basetest.rb index 9398f5b2d..e5c9a9dc5 100755 --- a/spec/lib/puppet/face/basetest.rb +++ b/spec/lib/puppet/face/basetest.rb @@ -1,41 +1,46 @@ require 'puppet/face' Puppet::Face.define(:basetest, '0.0.1') do copyright "Puppet Labs", 2011 license "Apache 2 license; see COPYING" summary "This is just so tests don't fail" option "--[no-]boolean" option "--mandatory ARGUMENT" action :foo do option("--action") when_invoked do |*args| args.length end end action :return_true do summary "just returns true" when_invoked do |options| true end end action :return_false do summary "just returns false" when_invoked do |options| false end end action :return_nil do summary "just returns nil" when_invoked do |options| nil end end action :raise do summary "just raises an exception" when_invoked do |options| raise ArgumentError, "your failure" end end action :with_s_rendering_hook do summary "has a rendering hook for 's'" when_invoked do |options| "this is not the hook you are looking for" end when_rendering :s do |value| "you invoked the 's' rendering hook" end end + + action :count_args do + summary "return the count of arguments given" + when_invoked do |*args| args.length - 1 end + end end diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb index 3318e061e..0a4a86be6 100755 --- a/spec/unit/application/face_base_spec.rb +++ b/spec/unit/application/face_base_spec.rb @@ -1,332 +1,360 @@ #!/usr/bin/env rspec require 'spec_helper' require 'puppet/application/face_base' require 'tmpdir' class Puppet::Application::FaceBase::Basetest < Puppet::Application::FaceBase end describe Puppet::Application::FaceBase do let :app do app = Puppet::Application::FaceBase::Basetest.new 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 "#parse_options" do before :each do app.command_line.stubs(:args).returns %w{} end describe "with just an action" do before :all do # We have to stub Signal.trap to avoid a crazy mess where we take # over signal handling and make it impossible to cancel the test # suite run. # # It would be nice to fix this elsewhere, but it is actually hard to # capture this in rspec 2.5 and all. :( --daniel 2011-04-08 Signal.stubs(:trap) app.command_line.stubs(:args).returns %w{foo} app.preinit app.parse_options end it "should set the face based on the type" do app.face.name.should == :basetest end it "should find the action" do app.action.should be app.action.name.should == :foo end end + it "should stop if the first thing found is not an action" do + app.command_line.stubs(:args).returns %w{banana count_args} + expect { app.run }.to exit_with 1 + @logs.first.message.should =~ /has no 'banana' action/ + end + it "should use the default action if not given any arguments" do app.command_line.stubs(:args).returns [] action = stub(:options => [], :render_as => nil) Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action) app.stubs(:main) app.run app.action.should == action app.arguments.should == [ { } ] end it "should use the default action if not given a valid one" do app.command_line.stubs(:args).returns %w{bar} action = stub(:options => [], :render_as => nil) Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action) app.stubs(:main) app.run app.action.should == action app.arguments.should == [ 'bar', { } ] end it "should have no action if not given a valid one and there is no default action" do app.command_line.stubs(:args).returns %w{bar} Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil) app.stubs(:main) expect { app.run }.to exit_with 1 - @logs.first.message.should =~ /does not have a default action/ + @logs.first.message.should =~ /has no 'bar' action./ + end + + [%w{something_I_cannot_do}, + %w{something_I_cannot_do argument}].each do |input| + it "should report unknown actions nicely" do + app.command_line.stubs(:args).returns input + Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil) + app.stubs(:main) + expect { app.run }.to exit_with 1 + @logs.first.message.should =~ /has no 'something_I_cannot_do' action/ + end + end + + [%w{something_I_cannot_do --unknown-option}, + %w{something_I_cannot_do argument --unknown-option}].each do |input| + it "should report unknown actions even if there are unknown options" do + app.command_line.stubs(:args).returns input + Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil) + app.stubs(:main) + expect { app.run }.to exit_with 1 + @logs.first.message.should =~ /has no 'something_I_cannot_do' action/ + end 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; app.parse_options }. to raise_error OptionParser::InvalidOption, /invalid 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; app.parse_options }. to raise_error OptionParser::InvalidOption, /invalid 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; app.parse_options }. to raise_error OptionParser::InvalidOption, /invalid option: --bar/ end it "should fail if an unknown option is after the action" do app.command_line.stubs(:args).returns %w{foo --bar} expect { app.preinit; app.parse_options }. to raise_error OptionParser::InvalidOption, /invalid 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 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 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; app.parse_options }. to raise_error OptionParser::InvalidOption, /invalid 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 it "should handle application-level options", :'fails_on_ruby_1.9.2' => true do - app.command_line.stubs(:args).returns %w{basetest --verbose return_true} + app.command_line.stubs(:args).returns %w{--verbose return_true} app.preinit app.parse_options app.face.name.should == :basetest 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 app.parse_options 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 app.parse_options app.setup app.arguments.should == ['bar', 'baz', 'quux', { :mandatory => "--bar" }] end end describe "#main" do before :each do app.stubs(:puts) # don't dump text to screen. app.face = Puppet::Face[:basetest, '0.0.1'] app.action = app.face.get_action(:foo) app.arguments = ["myname", "myarg"] end it "should send the specified verb and name to the face" do app.face.expects(:foo).with(*app.arguments) expect { app.main }.to exit_with 0 end it "should lookup help when it cannot do anything else" do app.action = nil Puppet::Face[:help, :current].expects(:help).with(:basetest) expect { app.main }.to exit_with 1 end it "should use its render method to render any result" do app.expects(:render).with(app.arguments.length + 1) expect { app.main }.to exit_with 0 end end describe "error reporting" do before :each do app.stubs(:puts) # don't dump text to screen. app.render_as = :json app.face = Puppet::Face[:basetest, '0.0.1'] app.arguments = [{}] # we always have options in there... end it "should exit 0 when the action returns true" do app.action = app.face.get_action :return_true expect { app.main }.to exit_with 0 end it "should exit 0 when the action returns false" do app.action = app.face.get_action :return_false expect { app.main }.to exit_with 0 end it "should exit 0 when the action returns nil" do app.action = app.face.get_action :return_nil expect { app.main }.to exit_with 0 end it "should exit non-0 when the action raises" do app.action = app.face.get_action :return_raise expect { app.main }.not_to exit_with 0 end end describe "#render" do before :each do app.face = Puppet::Face[:basetest, '0.0.1'] app.action = app.face.get_action(:foo) end context "default rendering" do before :each do app.setup end ["hello", 1, 1.0].each do |input| it "should just return a #{input.class.name}" do app.render(input).should == input end end [[1, 2], ["one"], [{ 1 => 1 }]].each do |input| it "should render #{input.class} using JSON" do app.render(input).should == input.to_pson.chomp end end it "should render a non-trivially-keyed Hash with using JSON" do hash = { [1,2] => 3, [2,3] => 5, [3,4] => 7 } app.render(hash).should == hash.to_pson.chomp end it "should render a {String,Numeric}-keyed Hash into a table" do object = Object.new hash = { "one" => 1, "two" => [], "three" => {}, "four" => object, 5 => 5, 6.0 => 6 } # Gotta love ASCII-betical sort order. Hope your objects are better # structured for display than my test one is. --daniel 2011-04-18 app.render(hash).should == < { "1" => '1' * 40, "2" => '2' * 40, '3' => '3' * 40 }, "text" => { "a" => 'a' * 40, 'b' => 'b' * 40, 'c' => 'c' * 40 } } app.render(hash).should == <"1111111111111111111111111111111111111111", "2"=>"2222222222222222222222222222222222222222", "3"=>"3333333333333333333333333333333333333333"} text {"a"=>"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "b"=>"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "c"=>"cccccccccccccccccccccccccccccccccccccccc"} EOT end it "should invoke the action rendering hook while rendering" do app.action.set_rendering_method_for(:console, proc { |value| "bi-winning!" }) app.render("bi-polar?").should == "bi-winning!" end it "should render JSON when asked for json" do app.render_as = :json json = app.render({ :one => 1, :two => 2 }) json.should =~ /"one":\s*1\b/ json.should =~ /"two":\s*2\b/ PSON.parse(json).should == { "one" => 1, "two" => 2 } end end it "should fail early if asked to render an invalid format" do - app.command_line.stubs(:args).returns %w{--render-as interpretive-dance help help} + app.command_line.stubs(:args).returns %w{--render-as interpretive-dance return_true} # We shouldn't get here, thanks to the exception, and our expectation on # it, but this helps us fail if that slips up and all. --daniel 2011-04-27 Puppet::Face[:help, :current].expects(:help).never expect { expect { app.run }.to exit_with 1 }.to have_printed(/I don't know how to render 'interpretive-dance'/) end it "should work if asked to render a NetworkHandler format" do - app.command_line.stubs(:args).returns %w{dummy find dummy --render-as yaml} + app.command_line.stubs(:args).returns %w{count_args a b c --render-as yaml} expect { expect { app.run }.to exit_with 0 }.to have_printed(/--- 3/) end it "should invoke when_rendering hook 's' when asked to render-as 's'" do app.command_line.stubs(:args).returns %w{with_s_rendering_hook --render-as s} app.action = app.face.get_action(:with_s_rendering_hook) expect { expect { app.run }.to exit_with 0 }.to have_printed(/you invoked the 's' rendering hook/) end end end