From cb3aead33cd5aebea7162f0979989efe8a789861 Mon Sep 17 00:00:00 2001 From: "Marc A. Paradise" Date: Wed, 8 May 2019 16:37:36 -0400 Subject: Move original config initialization to constructor 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 --- lib/chef/knife.rb | 13 ++++++++++--- spec/unit/knife_spec.rb | 25 +++++++++++++++++++++++++ 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, -- cgit v1.2.1