Move original config initialization to constructorKNIFE-EC2-567/handle-unmerged-case

This moves initialization of @original_config out of
merge_configs and into the constructor.

A common access pattern for plugins running knife commands is:

    cmd = KnifeCommand.new
    cmd.config[:something] = value
    cmd.run

This bypasses the `Knife.run` class method, which does extra config
initialization - including merge_configs.

When that happens, `config_source` will now return `nil` as the
source instead of exploding.

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>

2 files changed, 35 insertions, 3 deletions

diff --git a/lib/chef/knife.rb b/lib/chef/knife.rb
index a5f56180fc..20322fe61e 100644
--- a/lib/chef/knife.rb
+++ b/lib/chef/knife.rb
@@ -317,6 +317,10 @@ class Chef
         exit 1
       end
 
+      # Grab a copy before config merge occurs, so that we can later identify
+      # whare a given config value is sourced from.
+      @original_config = config.dup
+
       # copy Mixlib::CLI over so that it can be configured in config.rb/knife.rb
       # config file
       Chef::Config[:verbosity] = config[:verbosity] if config[:verbosity]
@@ -356,8 +360,9 @@ class Chef
     #  config_file_settings - Chef::Config[:knife] sub-hash
     #  config               - mixlib-cli settings (accessor from the mixin)
     def merge_configs
-      # This is the config after user CLI options have been evaluated, and it contains only
-      # user-supplied values.
+      # Update our original_config - if someone has created a knife command
+      # instance directly, they are likely ot have set cmd.config values directly
+      # as well, at which point our saved original config is no longer up to date.
       @original_config = config.dup
       # other code may have a handle to the config object, so use Hash#replace to deliberately
       # update-in-place.
@@ -373,7 +378,9 @@ class Chef
     #   - :cli - this was explicitly provided on the CLI
     #   - :config - this came from Chef::Config[:knife]
     #   - :cli_default - came from a declared CLI `option`'s `default` value.
-    #   - nil - if they key does not exist
+    #   - nil - if the key could not be found in any source.
+    #           This can happen when it is invalid, or has been
+    #           set directly into #config without then calling #merge_config
     def config_source(key)
       return :cli if @original_config.include? key
       return :config if config_file_settings.key? key
diff --git a/spec/unit/knife_spec.rb b/spec/unit/knife_spec.rb
index 6fcb831531..080b2ffa29 100644
--- a/spec/unit/knife_spec.rb
+++ b/spec/unit/knife_spec.rb
@@ -280,6 +280,31 @@ describe Chef::Knife do
       expect(other_deps_loaded).to be_truthy
     end
 
+    describe "working with unmerged configuration in #config_source" do
+      let(:command) { KnifeSpecs::TestYourself.new([]) }
+
+      before do
+        KnifeSpecs::TestYourself.option(:opt_with_default,
+                                        short: "-D VALUE",
+                                        default: "default-value")
+      end
+      # This supports a use case used by plugins, where the pattern
+      # seems to follow:
+      #   cmd = KnifeCommand.new
+      #   cmd.config[:config_key] = value
+      #   cmd.run
+      #
+      # This bypasses Knife::run and the `merge_configs` call it
+      # performs - config_source should break when that happens.
+      context "when config is fed in directly without a merge" do
+        it "retains the value but returns nil as a config source" do
+          command.config[:test1] = "value"
+          expect(command.config[:test1]).to eq "value"
+          expect(command.config_source(:test1)).to eq nil
+        end
+      end
+
+    end
     describe "merging configuration options" do
       before do
         KnifeSpecs::TestYourself.option(:opt_with_default,