(#7122) Enforce call arity on actions in the CLI wrapper.

We had a problem, previously, in the generic translation of command line
arguments to Ruby method calls: we could mistake the options, added by the CLI
wrapper, for a positional argument to the action method.

This was caused by a combination of factors, but primarily that the wrapper
methods for actions are designed to present a friendly, helpful Ruby API for
internal use.  Consequently, they have a default value if you don't wish to
pass options.

Unfortunately, this meant that the options that the CLI *always* passed could
be treated as a positional argument instead, and the default set of options
added to the back of the call.

To resolve this we now check the number of positional arguments in the CLI
wrapper, and raise an exception if they are mismatched.  This makes the
generic CLI handling do the right thing in adapting the command line
arguments to the Ruby API.

(As an aside, we would have had a similar-but-different failure mode if we
 type-checked positional arguments: these calls would have failed with an
 invalid argument validation error.)

Reviewed-By: Max Martin <max@puppetlabs.com>

4 files changed, 16 insertions, 9 deletions

diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index d6ca28d64..d8c970157 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -206,11 +206,11 @@ describe Puppet::Application::FaceBase do
 
       app.render_as = :json
       app.face      = Puppet::Face[:basetest, '0.0.1']
-      app.arguments = []
+      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
+      app.action    = app.face.get_action :return_true
       expect { app.main }.to exit_with 0
     end
 
diff --git a/spec/unit/application/indirection_base_spec.rb b/spec/unit/application/indirection_base_spec.rb
index 833fb424e..e0a9bebb4 100755
--- a/spec/unit/application/indirection_base_spec.rb
+++ b/spec/unit/application/indirection_base_spec.rb
@@ -28,10 +28,11 @@ describe Puppet::Application::IndirectionBase do
     Puppet::Indirector::Indirection.expects(:instance).
       with(:testindirection).returns(terminus)
 
-    subject.command_line.instance_variable_set('@args', %w{--terminus foo save})
+    subject.command_line.instance_variable_set('@args', %w{--terminus foo save bar})
 
     # Not a very nice thing. :(
     $stderr.stubs(:puts)
+    Puppet.stubs(:err)
 
     expect { subject.run }.to exit_with 0
   end
diff --git a/spec/unit/face/facts_spec.rb b/spec/unit/face/facts_spec.rb
index 06b229aa1..27b5b9e3d 100755
--- a/spec/unit/face/facts_spec.rb
+++ b/spec/unit/face/facts_spec.rb
@@ -9,9 +9,15 @@ describe Puppet::Face[:facts, '0.0.1'] do
 
   describe "when uploading" do
     it "should set the terminus_class to :facter"
+    it "should set the cache_class to :rest"
+    it "should find the current certname"
+  end
 
-    it "should set the cach_eclass to :rest"
+  describe "#find" do
+    it { should be_action :find }
 
-    it "should find the current certname"
+    it "should fail without a key" do
+      expect { subject.find }.to raise_error ArgumentError, /wrong number of arguments/
+    end
   end
 end
diff --git a/spec/unit/face/indirector_spec.rb b/spec/unit/face/indirector_spec.rb
index bb06fcfe2..e7dd44f3d 100755
--- a/spec/unit/face/indirector_spec.rb
+++ b/spec/unit/face/indirector_spec.rb
@@ -34,12 +34,12 @@ describe Puppet::Face::Indirector do
     end
 
     it "should call the indirection method with options when the '#{method}' action is invoked" do
-      subject.indirection.expects(method).with(:test, "myargs", {})
-      subject.send(method, :test, "myargs")
+      subject.indirection.expects(method).with(:test, {})
+      subject.send(method, :test)
     end
     it "should forward passed options" do
-      subject.indirection.expects(method).with(:test, "action", {'one'=>'1'})
-      subject.send(method, :test, 'action', {'one'=>'1'})
+      subject.indirection.expects(method).with(:test, {'one'=>'1'})
+      subject.send(method, :test, {'one'=>'1'})
     end
   end