Merge branch 'refactor/2.7.x/8561-and-7290-improved-option-handling' into 2.7.x

2 files changed, 68 insertions, 37 deletions

diff --git a/lib/puppet/indirector/face.rb b/lib/puppet/indirector/face.rb
index ead3f4b46..adb6b688b 100644
--- a/lib/puppet/indirector/face.rb
+++ b/lib/puppet/indirector/face.rb
@@ -48,16 +48,26 @@ class Puppet::Indirector::Face < Puppet::Face
     return result
   end
 
+  option "--extra HASH" do
+    summary "Extra arguments to pass to the indirection request"
+    description <<-end
+      A terminus can take additional arguments to refine the operation, which
+      are passed as an arbitrary hash to the back-end.  Anything passed as
+      the extra value is just send direct to the back-end.
+    end
+    default_to do Hash.new end
+  end
+
   action :destroy do
     summary "Delete an object."
     arguments "<key>"
-    when_invoked { |key, options| call_indirection_method(:destroy, key, options) }
+    when_invoked {|key, options| call_indirection_method :destroy, key, options[:extra] }
   end
 
   action :find do
     summary "Retrieve an object by name."
     arguments "<key>"
-    when_invoked { |key, options| call_indirection_method(:find, key, options) }
+    when_invoked {|key, options| call_indirection_method :find, key, options[:extra] }
   end
 
   action :save do
@@ -68,13 +78,13 @@ class Puppet::Indirector::Face < Puppet::Face
       currently accept data from STDIN, save actions cannot currently be invoked
       from the command line.
     EOT
-    when_invoked { |key, options| call_indirection_method(:save, key, options) }
+    when_invoked {|key, options| call_indirection_method :save, key, options[:extra] }
   end
 
   action :search do
     summary "Search for an object or retrieve multiple objects."
     arguments "<query>"
-    when_invoked { |key, options| call_indirection_method(:search, key, options) }
+    when_invoked {|key, options| call_indirection_method :search, key, options[:extra] }
   end
 
   # Print the configuration for the current terminus class
@@ -86,11 +96,11 @@ class Puppet::Indirector::Face < Puppet::Face
       run mode with the '--mode' option.
     EOT
 
-    when_invoked do |*args|
+    when_invoked do |options|
       if t = indirection.terminus_class
-        puts "Run mode '#{Puppet.run_mode.name}': #{t}"
+        "Run mode '#{Puppet.run_mode.name}': #{t}"
       else
-        $stderr.puts "No default terminus class for run mode '#{Puppet.run_mode.name}'"
+        "No default terminus class for run mode '#{Puppet.run_mode.name}'"
       end
     end
   end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index ce9c60b49..bd47a36ea 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -196,15 +196,12 @@ class Puppet::Interface::Action
     wrapper = <<WRAPPER
 def #{@name}(#{decl.join(", ")})
   #{optn}
-  args = #{args}
-  options = args.last
-
-  action = get_action(#{name.inspect})
-  action.add_default_args(args)
-  action.validate_args(args)
-  __invoke_decorations(:before, action, args, options)
+  args    = #{args}
+  action  = get_action(#{name.inspect})
+  args   << action.validate_and_clean(args.pop)
+  __invoke_decorations(:before, action, args, args.last)
   rval = self.__send__(#{internal_name.inspect}, *args)
-  __invoke_decorations(:after, action, args, options)
+  __invoke_decorations(:after, action, args, args.last)
   return rval
 end
 WRAPPER
@@ -254,35 +251,59 @@ WRAPPER
     option
   end
 
-  def add_default_args(args)
-    options.map {|x| get_option(x) }.each do |option|
-      if option.has_default? and not option.aliases.any? {|x| args.last.has_key? x}
-        args.last[option.name] = option.default
+  def validate_and_clean(original)
+    # The final set of arguments; effectively a hand-rolled shallow copy of
+    # the original, which protects the caller from the surprises they might
+    # get if they passed us a hash and we mutated it...
+    result = {}
+
+    # Check for multiple aliases for the same option, and canonicalize the
+    # name of the argument while we are about it.
+    overlap = Hash.new do |h, k| h[k] = [] end
+    unknown = []
+    original.keys.each do |name|
+      if option = get_option(name) then
+        canonical = option.name
+        if result.has_key? canonical
+          overlap[canonical] << name
+        else
+          result[canonical] = original[name]
+        end
+      else
+        unknown << name
       end
     end
-  end
 
-  def validate_args(args)
-    # Check for multiple aliases for the same option...
-    args.last.keys.each do |name|
-      # #7290: If this isn't actually an option, ignore it for now.  We should
-      # probably fail, but that wasn't our API, and I don't want to perturb
-      # behaviour this late in the RC cycle. --daniel 2011-04-29
-      if option = get_option(name) then
-        overlap = (option.aliases & args.last.keys)
-        unless overlap.length == 1 then
-          raise ArgumentError, "Multiple aliases for the same option passed: #{overlap.join(', ')}"
-        end
+    unless overlap.empty?
+      msg = overlap.map {|k, v| "(#{k}, #{v.sort.join(', ')})" }.join(", ")
+      raise ArgumentError, "Multiple aliases for the same option passed: #{msg}"
+    end
+
+    unless unknown.empty?
+      msg = unknown.sort.join(", ")
+      raise ArgumentError, "Unknown options passed: #{msg}"
+    end
+
+    # Inject default arguments and check for missing mandating options.
+    missing = []
+    options.map {|x| get_option(x) }.each do |option|
+      name = option.name
+      next if result.has_key? name
+
+      if option.has_default?
+        result[name] = option.default
+      elsif option.required?
+        missing << name
       end
     end
 
-    # Check for missing mandatory options.
-    required = options.map do |name|
-      get_option(name)
-    end.select(&:required?).collect(&:name) - args.last.keys
+    unless missing.empty?
+      msg = missing.sort.join(', ')
+      raise ArgumentError, "The following options are required: #{msg}"
+    end
 
-    return if required.empty?
-    raise ArgumentError, "The following options are required: #{required.join(', ')}"
+    # All done.
+    return result
   end
 
   ########################################################################