Merge pull request #2851 from Igorshp/ssh_attribute

Prioritise manual ssh attribute over defaults

2 files changed, 25 insertions, 28 deletions

diff --git a/spec/functional/knife/ssh_spec.rb b/spec/functional/knife/ssh_spec.rb
index 5b8ad6f368..6608d05771 100644
--- a/spec/functional/knife/ssh_spec.rb
+++ b/spec/functional/knife/ssh_spec.rb
@@ -165,7 +165,7 @@ describe Chef::Knife::Ssh do
 
       it "uses the ssh_attribute" do
         @knife.run
-        expect(@knife.config[:attribute]).to eq("ec2.public_hostname")
+        expect(@knife.get_ssh_attribute(Chef::Node.new)).to eq("ec2.public_hostname")
       end
     end
 
@@ -177,7 +177,7 @@ describe Chef::Knife::Ssh do
 
       it "uses the default" do
         @knife.run
-        expect(@knife.config[:attribute]).to eq("fqdn")
+        expect(@knife.get_ssh_attribute(Chef::Node.new)).to eq("fqdn")
       end
     end
 
diff --git a/spec/unit/knife/ssh_spec.rb b/spec/unit/knife/ssh_spec.rb
index a838a21edc..723280bead 100644
--- a/spec/unit/knife/ssh_spec.rb
+++ b/spec/unit/knife/ssh_spec.rb
@@ -28,10 +28,10 @@ describe Chef::Knife::Ssh do
   before do
     @knife = Chef::Knife::Ssh.new
     @knife.merge_configs
-    @knife.config[:attribute] = "fqdn"
     @node_foo = Chef::Node.new
     @node_foo.automatic_attrs[:fqdn] = "foo.example.org"
     @node_foo.automatic_attrs[:ipaddress] = "10.0.0.1"
+
     @node_bar = Chef::Node.new
     @node_bar.automatic_attrs[:fqdn] = "bar.example.org"
     @node_bar.automatic_attrs[:ipaddress] = "10.0.0.2"
@@ -52,15 +52,15 @@ describe Chef::Knife::Ssh do
       def self.should_return_specified_attributes
         it "returns an array of the attributes specified on the command line OR config file, if only one is set" do
           @knife.config[:attribute] = "ipaddress"
-          @knife.config[:attribute_from_cli] = "ipaddress"
+          Chef::Config[:knife][:ssh_attribute] = "ipaddress" # this value will be in the config file
           configure_query([@node_foo, @node_bar])
           expect(@knife).to receive(:session_from_list).with([['10.0.0.1', nil], ['10.0.0.2', nil]])
           @knife.configure_session
         end
 
         it "returns an array of the attributes specified on the command line even when a config value is set" do
-          @knife.config[:attribute] = "config_file" # this value will be the config file
-          @knife.config[:attribute_from_cli] = "ipaddress" # this is the value of the command line via #configure_attribute
+          Chef::Config[:knife][:ssh_attribute] = "config_file" # this value will be in the config file
+          @knife.config[:attribute] = "ipaddress" # this is the value of the command line via #configure_attribute
           configure_query([@node_foo, @node_bar])
           expect(@knife).to receive(:session_from_list).with([['10.0.0.1', nil], ['10.0.0.2', nil]])
           @knife.configure_session
@@ -83,7 +83,6 @@ describe Chef::Knife::Ssh do
           @node_foo.automatic_attrs[:cloud][:public_hostname] = "ec2-10-0-0-1.compute-1.amazonaws.com"
           @node_bar.automatic_attrs[:cloud][:public_hostname] = "ec2-10-0-0-2.compute-1.amazonaws.com"
         end
-
         it "returns an array of cloud public hostnames" do
           configure_query([@node_foo, @node_bar])
           expect(@knife).to receive(:session_from_list).with([
@@ -150,42 +149,40 @@ describe Chef::Knife::Ssh do
     end
   end
 
-  describe "#configure_attribute" do
+  describe "#get_ssh_attribute" do
+    # Order of precedence for ssh target
+    # 1) command line attribute
+    # 2) configuration file
+    # 3) cloud attribute
+    # 4) fqdn
     before do
       Chef::Config[:knife][:ssh_attribute] = nil
       @knife.config[:attribute] = nil
+      @node_foo.automatic_attrs[:cloud][:public_hostname] = "ec2-10-0-0-1.compute-1.amazonaws.com"
+      @node_bar.automatic_attrs[:cloud][:public_hostname] = ''
     end
 
     it "should return fqdn by default" do
-      @knife.configure_attribute
-      expect(@knife.config[:attribute]).to eq("fqdn")
+      expect(@knife.get_ssh_attribute(Chef::Node.new)).to eq("fqdn")
     end
 
-    it "should return the value set in the configuration file" do
-      Chef::Config[:knife][:ssh_attribute] = "config_file"
-      @knife.configure_attribute
-      expect(@knife.config[:attribute]).to eq("config_file")
+    it "should return cloud.public_hostname attribute if available" do
+      expect(@knife.get_ssh_attribute(@node_foo)).to eq("cloud.public_hostname")
     end
 
-    it "should return the value set on the command line" do
+    it "should favor to attribute_from_cli over config file and cloud" do 
       @knife.config[:attribute] = "command_line"
-      @knife.configure_attribute
-      expect(@knife.config[:attribute]).to eq("command_line")
+      Chef::Config[:knife][:ssh_attribute] = "config_file"
+      expect( @knife.get_ssh_attribute(@node_foo)).to eq("command_line")
     end
 
-    it "should set attribute_from_cli to the value of attribute from the command line" do
-      @knife.config[:attribute] = "command_line"
-      @knife.configure_attribute
-      expect(@knife.config[:attribute]).to eq("command_line")
-      expect(@knife.config[:attribute_from_cli]).to eq("command_line")
+    it "should favor config file over cloud and default" do 
+      Chef::Config[:knife][:ssh_attribute] = "config_file"
+      expect( @knife.get_ssh_attribute(@node_foo)).to eq("config_file")
     end
 
-    it "should prefer the command line over the config file for the value of attribute_from_cli" do
-      Chef::Config[:knife][:ssh_attribute] = "config_file"
-      @knife.config[:attribute] = "command_line"
-      @knife.configure_attribute
-      expect(@knife.config[:attribute]).to eq("command_line")
-      expect(@knife.config[:attribute_from_cli]).to eq("command_line")
+    it "should return fqdn if cloud.hostname is empty" do
+          expect( @knife.get_ssh_attribute(@node_bar)).to eq("fqdn")
     end
   end