Knife bootstrap options cleanup

We have issue that are caused by old code before merging of hash values
were done correctly.

The `config` hash correctly merges all options and should always be
used.

Knife plugins should never touch Chef::Config[:knife] values (either
reading or writing from them).

The `knife_config` should be converted to the `config` hash since it
directly accesses Chef::Config[:knife] values.

The `config_value()` helper should no longer be used.  Very clearly most
people started to use that when they should just use the config hash
directly.  That was intended to be used only when a knife cli option
was being renamed and the former configuration value needed to be
used as well.  It has been cargo culted around as the way to access
config values, and that should really stop.

The DataBagSecretOption mixin has been cleaned up so that the cli
options read+write only to the config[:cl_secret] and
config[:cl_secret_file] values.  The config file values go into
config[:secret] and config[:secret_file].  The fact that those are
the merged values in the `config` hash doesn't matter since only
the cli should be writing to the first two and only the config
file should be writing to the latter two.  I don't know why it was
made so complicated to begin with, but if there's some hidden
chef-11.early backcompat there, then chef-16 deliberately breaks that.

The use of `locate_config_value` helpers in all knife plugins is also
discouraged (but they all implement those themselves), just use the
config hash, which has the correct hash merge ordering.  All of those
need to be deleted.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

1 files changed, 23 insertions, 19 deletions

diff --git a/spec/functional/knife/ssh_spec.rb b/spec/functional/knife/ssh_spec.rb
index 4fe919c3a3..4f4290f66d 100644
--- a/spec/functional/knife/ssh_spec.rb
+++ b/spec/functional/knife/ssh_spec.rb
@@ -50,8 +50,8 @@ describe Chef::Knife::Ssh do
   describe "identity file" do
     context "when knife[:ssh_identity_file] is set" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_identity_file] = "~/.ssh/aws.rsa"
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the ssh_identity_file" do
@@ -62,8 +62,8 @@ describe Chef::Knife::Ssh do
 
     context "when knife[:ssh_identity_file] is set and frozen" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_identity_file] = "~/.ssh/aws.rsa".freeze
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the ssh_identity_file" do
@@ -74,8 +74,8 @@ describe Chef::Knife::Ssh do
 
     context "when -i is provided" do
       before do
-        setup_knife(["-i ~/.ssh/aws.rsa", "*:*", "uptime"])
         Chef::Config[:knife][:ssh_identity_file] = nil
+        setup_knife(["-i ~/.ssh/aws.rsa", "*:*", "uptime"])
       end
 
       it "should use the value on the command line" do
@@ -85,6 +85,7 @@ describe Chef::Knife::Ssh do
 
       it "should override what is set in knife.rb" do
         Chef::Config[:knife][:ssh_identity_file] = "~/.ssh/other.rsa"
+        @knife.merge_configs
         @knife.run
         expect(@knife.config[:ssh_identity_file]).to eq("~/.ssh/aws.rsa")
       end
@@ -92,8 +93,8 @@ describe Chef::Knife::Ssh do
 
     context "when knife[:ssh_identity_file] is not provided]" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_identity_file] = nil
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the default" do
@@ -119,8 +120,8 @@ describe Chef::Knife::Ssh do
   describe "user" do
     context "when knife[:ssh_user] is set" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_user] = "ubuntu"
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the ssh_user" do
@@ -131,8 +132,8 @@ describe Chef::Knife::Ssh do
 
     context "when knife[:ssh_user] is set and frozen" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_user] = "ubuntu".freeze
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the ssh_user" do
@@ -143,8 +144,8 @@ describe Chef::Knife::Ssh do
 
     context "when -x is provided" do
       before do
-        setup_knife(["-x ubuntu", "*:*", "uptime"])
         Chef::Config[:knife][:ssh_user] = nil
+        setup_knife(["-x ubuntu", "*:*", "uptime"])
       end
 
       it "should use the value on the command line" do
@@ -154,6 +155,7 @@ describe Chef::Knife::Ssh do
 
       it "should override what is set in knife.rb" do
         Chef::Config[:knife][:ssh_user] = "root"
+        @knife.merge_configs
         @knife.run
         expect(@knife.config[:ssh_user]).to eq("ubuntu")
       end
@@ -161,8 +163,8 @@ describe Chef::Knife::Ssh do
 
     context "when knife[:ssh_user] is not provided]" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_user] = nil
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the default (current user)" do
@@ -175,8 +177,8 @@ describe Chef::Knife::Ssh do
   describe "attribute" do
     context "when knife[:ssh_attribute] is set" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_attribute] = "ec2.public_hostname"
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the ssh_attribute" do
@@ -187,8 +189,8 @@ describe Chef::Knife::Ssh do
 
     context "when knife[:ssh_attribute] is not provided" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_attribute] = nil
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the default" do
@@ -199,8 +201,8 @@ describe Chef::Knife::Ssh do
 
     context "when -a ec2.public_public_hostname is provided" do
       before do
-        setup_knife(["-a", "ec2.public_hostname", "*:*", "uptime"])
         Chef::Config[:knife][:ssh_attribute] = nil
+        setup_knife(["-a", "ec2.public_hostname", "*:*", "uptime"])
       end
 
       it "should use the value on the command line" do
@@ -211,6 +213,7 @@ describe Chef::Knife::Ssh do
       it "should override what is set in knife.rb" do
         # This is the setting imported from knife.rb
         Chef::Config[:knife][:ssh_attribute] = "fqdn"
+        @knife.merge_configs
         # Then we run knife with the -a flag, which sets the above variable
         setup_knife(["-a", "ec2.public_hostname", "*:*", "uptime"])
         @knife.run
@@ -222,8 +225,8 @@ describe Chef::Knife::Ssh do
   describe "prefix" do
     context "when knife[:prefix_attribute] is set" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:prefix_attribute] = "name"
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the prefix_attribute" do
@@ -234,8 +237,8 @@ describe Chef::Knife::Ssh do
 
     context "when knife[:prefix_attribute] is not provided" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:prefix_attribute] = nil
+        setup_knife(["*:*", "uptime"])
       end
 
       it "falls back to nil" do
@@ -246,8 +249,8 @@ describe Chef::Knife::Ssh do
 
     context "when --prefix-attribute ec2.public_public_hostname is provided" do
       before do
-        setup_knife(["--prefix-attribute", "ec2.public_hostname", "*:*", "uptime"])
         Chef::Config[:knife][:prefix_attribute] = nil
+        setup_knife(["--prefix-attribute", "ec2.public_hostname", "*:*", "uptime"])
       end
 
       it "should use the value on the command line" do
@@ -258,6 +261,7 @@ describe Chef::Knife::Ssh do
       it "should override what is set in knife.rb" do
         # This is the setting imported from knife.rb
         Chef::Config[:knife][:prefix_attribute] = "fqdn"
+        @knife.merge_configs
         # Then we run knife with the -b flag, which sets the above variable
         setup_knife(["--prefix-attribute", "ec2.public_hostname", "*:*", "uptime"])
         @knife.run
@@ -269,8 +273,8 @@ describe Chef::Knife::Ssh do
   describe "gateway" do
     context "when knife[:ssh_gateway] is set" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_gateway] = "user@ec2.public_hostname"
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the ssh_gateway" do
@@ -282,8 +286,8 @@ describe Chef::Knife::Ssh do
 
     context "when -G user@ec2.public_hostname is provided" do
       before do
-        setup_knife(["-G user@ec2.public_hostname", "*:*", "uptime"])
         Chef::Config[:knife][:ssh_gateway] = nil
+        setup_knife(["-G user@ec2.public_hostname", "*:*", "uptime"])
       end
 
       it "uses the ssh_gateway" do
@@ -295,9 +299,9 @@ describe Chef::Knife::Ssh do
 
     context "when knife[:ssh_gateway_identity] is set" do
       before do
-        setup_knife(["*:*", "uptime"])
         Chef::Config[:knife][:ssh_gateway] = "user@ec2.public_hostname"
         Chef::Config[:knife][:ssh_gateway_identity] = "~/.ssh/aws-gateway.rsa"
+        setup_knife(["*:*", "uptime"])
       end
 
       it "uses the ssh_gateway_identity file" do
@@ -309,9 +313,9 @@ describe Chef::Knife::Ssh do
 
     context "when -ssh-gateway-identity is provided and knife[:ssh_gateway] is set" do
       before do
-        setup_knife(["--ssh-gateway-identity", "~/.ssh/aws-gateway.rsa", "*:*", "uptime"])
         Chef::Config[:knife][:ssh_gateway] = "user@ec2.public_hostname"
         Chef::Config[:knife][:ssh_gateway_identity] = nil
+        setup_knife(["--ssh-gateway-identity", "~/.ssh/aws-gateway.rsa", "*:*", "uptime"])
       end
 
       it "uses the ssh_gateway_identity file" do
@@ -323,8 +327,8 @@ describe Chef::Knife::Ssh do
 
     context "when the gateway requires a password" do
       before do
-        setup_knife(["-G user@ec2.public_hostname", "*:*", "uptime"])
         Chef::Config[:knife][:ssh_gateway] = nil
+        setup_knife(["-G user@ec2.public_hostname", "*:*", "uptime"])
         allow(@knife.session).to receive(:via) do |host, user, options|
           raise Net::SSH::AuthenticationFailed unless options[:password]
         end