Merge pull request #1218 from opscode/lcg/save-CHEF-2418

WIP: attempt to save CHEF-2418

2 files changed, 124 insertions, 6 deletions

diff --git a/lib/chef/knife/ssh.rb b/lib/chef/knife/ssh.rb
index 837710b932..83c1735b4a 100644
--- a/lib/chef/knife/ssh.rb
+++ b/lib/chef/knife/ssh.rb
@@ -64,12 +64,12 @@ class Chef
         :long => "--ssh-user USERNAME",
         :description => "The ssh username"
 
-      option :ssh_password,
+      option :ssh_password_ng,
         :short => "-P [PASSWORD]",
         :long => "--ssh-password [PASSWORD]",
         :description => "The ssh password - will prompt if flag is specified but no password is given",
-        # default to a value that can not be a password (boolean) 
-        # so we can effectively test if this parameter was specified 
+        # default to a value that can not be a password (boolean)
+        # so we can effectively test if this parameter was specified
         # without a vlaue
         :default => false
 
@@ -436,16 +436,27 @@ class Chef
                              Chef::Config[:knife][:ssh_user])
       end
 
+      # This is a bit overly complicated because of the way we want knife ssh to work with -P causing a password prompt for
+      # the user, but we have to be conscious that this code gets included in knife bootstrap and knife * server create as
+      # well.  We want to change the semantics so that the default is false and 'nil' means -P without an argument on the
+      # command line.  But the other utilities expect nil to be the default and we can't prompt in that case. So we effectively
+      # use ssh_password_ng to determine if we're coming from knife ssh or from the other utilities.  The other utilties can
+      # also be patched to use ssh_password_ng easily as long they follow the convention that the default is false.
       def configure_password
-        if config[:ssh_password].nil?
+        if config.has_key?(:ssh_password_ng) && config[:ssh_password_ng].nil?
           # If the parameter is called on the command line with no value
-          # it will set :ssh_password = nil
+          # it will set :ssh_password_ng = nil
           # This is where we want to trigger a prompt for password
           config[:ssh_password] = get_password
         else
+          # if ssh_password_ng is false then it has not been set at all, and we may be in knife ec2 and still
+          # using an old config[:ssh_password].  this is backwards compatibility.  all knife cloud plugins should
+          # be updated to use ssh_password_ng with a default of false and ssh_password should be retired, (but
+          # we'll still need to use the ssh_password out of knife.rb if we find that).
+          ssh_password = config.has_key?(:ssh_password_ng) ? config[:ssh_password_ng] : config[:ssh_password]
           # Otherwise, the password has either been specified on the command line,
           # in knife.rb, or key based auth will be attempted
-          config[:ssh_password] = get_stripped_unfrozen_value(config[:ssh_password] ||
+          config[:ssh_password] = get_stripped_unfrozen_value(ssh_password ||
                              Chef::Config[:knife][:ssh_password])
         end
       end
diff --git a/spec/unit/knife/ssh_spec.rb b/spec/unit/knife/ssh_spec.rb
index 0d67f33ee7..01dd742da0 100644
--- a/spec/unit/knife/ssh_spec.rb
+++ b/spec/unit/knife/ssh_spec.rb
@@ -280,4 +280,111 @@ describe Chef::Knife::Ssh do
       end
     end
   end
+
+  describe "#configure_password" do
+    before do
+      @knife.config.delete(:ssh_password_ng)
+      @knife.config.delete(:ssh_password)
+    end
+
+    context "when setting ssh_password_ng from knife ssh" do
+      # in this case ssh_password_ng exists, but ssh_password does not
+      it "should prompt for a password when ssh_passsword_ng is nil"  do
+        @knife.config[:ssh_password_ng] = nil
+        @knife.should_receive(:get_password).and_return("mysekretpassw0rd")
+        @knife.configure_password
+        @knife.config[:ssh_password].should == "mysekretpassw0rd"
+      end
+
+      it "should set ssh_password to false if ssh_password_ng is false"  do
+        @knife.config[:ssh_password_ng] = false
+        @knife.should_not_receive(:get_password)
+        @knife.configure_password
+        @knife.config[:ssh_password].should be_false
+      end
+
+      it "should set ssh_password to ssh_password_ng if we set a password" do
+        @knife.config[:ssh_password_ng] = "mysekretpassw0rd"
+        @knife.should_not_receive(:get_password)
+        @knife.configure_password
+        @knife.config[:ssh_password].should == "mysekretpassw0rd"
+      end
+    end
+
+    context "when setting ssh_password from knife bootstrap / knife * server create" do
+      # in this case ssh_password exists, but ssh_password_ng does not
+      it "should set ssh_password to nil when ssh_password is nil" do
+        @knife.config[:ssh_password] = nil
+        @knife.should_not_receive(:get_password)
+        @knife.configure_password
+        @knife.config[:ssh_password].should be_nil
+      end
+
+      it "should set ssh_password to false when ssh_password is false" do
+        @knife.config[:ssh_password] = false
+        @knife.should_not_receive(:get_password)
+        @knife.configure_password
+        @knife.config[:ssh_password].should be_false
+      end
+
+      it "should set ssh_password to ssh_password if we set a password" do
+        @knife.config[:ssh_password] = "mysekretpassw0rd"
+        @knife.should_not_receive(:get_password)
+        @knife.configure_password
+        @knife.config[:ssh_password].should == "mysekretpassw0rd"
+      end
+    end
+    context "when setting ssh_password in the config variable" do
+      before(:each) do
+        Chef::Config[:knife][:ssh_password] = "my_knife_passw0rd"
+      end
+      context "when setting ssh_password_ng from knife ssh" do
+        # in this case ssh_password_ng exists, but ssh_password does not
+        it "should prompt for a password when ssh_passsword_ng is nil"  do
+          @knife.config[:ssh_password_ng] = nil
+          @knife.should_receive(:get_password).and_return("mysekretpassw0rd")
+          @knife.configure_password
+          @knife.config[:ssh_password].should == "mysekretpassw0rd"
+        end
+
+        it "should set ssh_password to the configured knife.rb value if ssh_password_ng is false"  do
+          @knife.config[:ssh_password_ng] = false
+          @knife.should_not_receive(:get_password)
+          @knife.configure_password
+          @knife.config[:ssh_password].should == "my_knife_passw0rd"
+        end
+
+        it "should set ssh_password to ssh_password_ng if we set a password" do
+          @knife.config[:ssh_password_ng] = "mysekretpassw0rd"
+          @knife.should_not_receive(:get_password)
+          @knife.configure_password
+          @knife.config[:ssh_password].should == "mysekretpassw0rd"
+        end
+      end
+
+      context "when setting ssh_password from knife bootstrap / knife * server create" do
+        # in this case ssh_password exists, but ssh_password_ng does not
+        it "should set ssh_password to the configured knife.rb value when ssh_password is nil" do
+          @knife.config[:ssh_password] = nil
+          @knife.should_not_receive(:get_password)
+          @knife.configure_password
+          @knife.config[:ssh_password].should == "my_knife_passw0rd"
+        end
+
+        it "should set ssh_password to the configured knife.rb value when ssh_password is false" do
+          @knife.config[:ssh_password] = false
+          @knife.should_not_receive(:get_password)
+          @knife.configure_password
+          @knife.config[:ssh_password].should == "my_knife_passw0rd"
+        end
+
+        it "should set ssh_password to ssh_password if we set a password" do
+          @knife.config[:ssh_password] = "mysekretpassw0rd"
+          @knife.should_not_receive(:get_password)
+          @knife.configure_password
+          @knife.config[:ssh_password].should == "mysekretpassw0rd"
+        end
+      end
+    end
+  end
 end