Implement `--config-option` CLI opt for knifedan/config-option-in-knife

The `--config-option` CLI option allows setting any `Chef::Config`
setting on the command line, which allows applications to be used
without a configuration file even if a desired config option does not
have a corresponding CLI option. This CLI option was present in the help
output for knife but was not actually implemented. Moving the behavior
into chef-config allows us to use it for both `chef-client` and `knife`.

Signed-off-by: Daniel DeLeo <dan@chef.io>

6 files changed, 159 insertions, 16 deletions

diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index f2db54aa17..d0fa87a5ad 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -32,6 +32,7 @@ require "mixlib/shellout"
 require "uri"
 require "addressable/uri"
 require "openssl"
+require "yaml"
 
 module ChefConfig
 
@@ -70,6 +71,25 @@ module ChefConfig
       event_handlers << logger
     end
 
+    def self.apply_extra_config_options(extra_config_options)
+      if extra_config_options
+        extra_parsed_options = extra_config_options.inject({}) do |memo, option|
+          # Sanity check value.
+          if option.empty? || !option.include?("=")
+            raise UnparsableConfigOption, "Unparsable config option #{option.inspect}"
+          end
+          # Split including whitespace if someone does truly odd like
+          # --config-option "foo = bar"
+          key, value = option.split(/\s*=\s*/, 2)
+          # Call to_sym because Chef::Config expects only symbol keys. Also
+          # runs a simple parse on the string for some common types.
+          memo[key.to_sym] = YAML.safe_load(value)
+          memo
+        end
+        merge!(extra_parsed_options)
+      end
+    end
+
     # Config file to load (client.rb, knife.rb, etc. defaults set differently in knife, chef-client, etc.)
     configurable(:config_file)
 
diff --git a/chef-config/lib/chef-config/exceptions.rb b/chef-config/lib/chef-config/exceptions.rb
index db10a5f364..23fd28f9c8 100644
--- a/chef-config/lib/chef-config/exceptions.rb
+++ b/chef-config/lib/chef-config/exceptions.rb
@@ -22,5 +22,6 @@ module ChefConfig
 
   class ConfigurationError < ArgumentError; end
   class InvalidPath < StandardError; end
+  class UnparsableConfigOption < StandardError; end
 
 end
diff --git a/chef-config/spec/unit/config_spec.rb b/chef-config/spec/unit/config_spec.rb
index 0ddb56cf0d..2cdf9af78f 100644
--- a/chef-config/spec/unit/config_spec.rb
+++ b/chef-config/spec/unit/config_spec.rb
@@ -68,6 +68,91 @@ RSpec.describe ChefConfig::Config do
     end
   end
 
+  describe "parsing arbitrary config from the CLI" do
+
+    def apply_config
+      described_class.apply_extra_config_options(extra_config_options)
+    end
+
+    context "when no arbitrary config is given" do
+
+      let(:extra_config_options) { nil }
+
+      it "succeeds" do
+        expect { apply_config }.to_not raise_error
+      end
+
+    end
+
+    context "when given a simple string option" do
+
+      let(:extra_config_options) { [ "node_name=bobotclown" ] }
+
+      it "applies the string option" do
+        apply_config
+        expect(described_class[:node_name]).to eq("bobotclown")
+      end
+
+    end
+
+    context "when given a blank value" do
+
+      let(:extra_config_options) { [ "http_retries=" ] }
+
+      it "sets the value to nil" do
+        # ensure the value is actually changed in the test
+        described_class[:http_retries] = 55
+        apply_config
+        expect(described_class[:http_retries]).to eq(nil)
+      end
+    end
+
+    context "when given spaces between `key = value`" do
+
+      let(:extra_config_options) { [ "node_name = bobo" ] }
+
+      it "handles the extra spaces and applies the config option" do
+        apply_config
+        expect(described_class[:node_name]).to eq("bobo")
+      end
+
+    end
+
+    context "when given an integer value" do
+
+      let(:extra_config_options) { [ "http_retries=9000" ] }
+
+      it "converts to a numeric type and applies the config option" do
+        apply_config
+        expect(described_class[:http_retries]).to eq(9000)
+      end
+
+    end
+
+    context "when given a boolean" do
+
+      let(:extra_config_options) { [ "boolean_thing=true" ] }
+
+      it "converts to a boolean type and applies the config option" do
+        apply_config
+        expect(described_class[:boolean_thing]).to eq(true)
+      end
+
+    end
+
+    context "when given input that is not in key=value form" do
+
+      let(:extra_config_options) { [ "http_retries:9000" ] }
+
+      it "raises UnparsableConfigOption" do
+        message = 'Unparsable config option "http_retries:9000"'
+        expect { apply_config }.to raise_error(ChefConfig::UnparsableConfigOption, message)
+      end
+
+    end
+
+  end
+
   describe "when configuring formatters" do
       # if TTY and not(force-logger)
       #   formatter = configured formatter or default formatter
diff --git a/lib/chef/application.rb b/lib/chef/application.rb
index f9735a3769..7f15859c8f 100644
--- a/lib/chef/application.rb
+++ b/lib/chef/application.rb
@@ -28,7 +28,6 @@ require "mixlib/cli"
 require "tmpdir"
 require "rbconfig"
 require "chef/application/exit_code"
-require "yaml"
 
 class Chef
   class Application
@@ -111,20 +110,13 @@ class Chef
       end
       extra_config_options = config.delete(:config_option)
       Chef::Config.merge!(config)
-      if extra_config_options
-        extra_parsed_options = extra_config_options.inject({}) do |memo, option|
-          # Sanity check value.
-          Chef::Application.fatal!("Unparsable config option #{option.inspect}") if option.empty? || !option.include?("=")
-          # Split including whitespace if someone does truly odd like
-          # --config-option "foo = bar"
-          key, value = option.split(/\s*=\s*/, 2)
-          # Call to_sym because Chef::Config expects only symbol keys. Also
-          # runs a simple parse on the string for some common types.
-          memo[key.to_sym] = YAML.safe_load(value)
-          memo
-        end
-        Chef::Config.merge!(extra_parsed_options)
-      end
+      apply_extra_config_options(extra_config_options)
+    end
+
+    def apply_extra_config_options(extra_config_options)
+      Chef::Config.apply_extra_config_options(extra_config_options)
+    rescue ChefConfig::UnparsableConfigOption => e
+      Chef::Application.fatal!(e.message)
     end
 
     def set_specific_recipes
diff --git a/lib/chef/knife.rb b/lib/chef/knife.rb
index 0dbd02ceb4..b1b263bb6f 100644
--- a/lib/chef/knife.rb
+++ b/lib/chef/knife.rb
@@ -408,11 +408,25 @@ class Chef
       config_loader = self.class.load_config(config[:config_file])
       config[:config_file] = config_loader.config_location
 
+      # For CLI options like `--config-option key=value`. These have to get
+      # parsed and applied separately.
+      extra_config_options = config.delete(:config_option)
+
       merge_configs
       apply_computed_config
-      Chef::Config.export_proxies
+
       # This has to be after apply_computed_config so that Mixlib::Log is configured
       Chef::Log.info("Using configuration from #{config[:config_file]}") if config[:config_file]
+
+      begin
+        Chef::Config.apply_extra_config_options(extra_config_options)
+      rescue ChefConfig::UnparsableConfigOption => e
+        ui.error e.message
+        show_usage
+        exit(1)
+      end
+
+      Chef::Config.export_proxies
     end
 
     def show_usage
diff --git a/spec/unit/knife_spec.rb b/spec/unit/knife_spec.rb
index f0ec45d59a..9569526b2a 100644
--- a/spec/unit/knife_spec.rb
+++ b/spec/unit/knife_spec.rb
@@ -349,6 +349,37 @@ describe Chef::Knife do
         expect { knife.run_with_pretty_exceptions }.to raise_error(Exception)
       end
     end
+
+    describe "setting arbitrary configuration with --config-option" do
+
+      let(:stdout) { StringIO.new }
+
+      let(:stderr) { StringIO.new }
+
+      let(:stdin) { StringIO.new }
+
+      let(:ui) { Chef::Knife::UI.new(stdout, stderr, stdin, disable_editing: true) }
+
+      let(:subcommand) do
+        KnifeSpecs::TestYourself.options = Chef::Application::Knife.options.merge(KnifeSpecs::TestYourself.options)
+        KnifeSpecs::TestYourself.new(%w{--config-option badly_formatted_arg}).tap do |cmd|
+          cmd.ui = ui
+        end
+      end
+
+      it "sets arbitrary configuration via --config-option" do
+        Chef::Knife.run(%w{test yourself --config-option arbitrary_config_thing=hello}, Chef::Application::Knife.options)
+        expect(Chef::Config[:arbitrary_config_thing]).to eq("hello")
+      end
+
+      it "handles errors in arbitrary configuration" do
+        expect(subcommand).to receive(:exit).with(1)
+        subcommand.configure_chef
+        expect(stderr.string).to include("ERROR: Unparsable config option \"badly_formatted_arg\"")
+        expect(stdout.string).to include(subcommand.opt_parser.to_s)
+      end
+    end
+
   end
 
   describe "when first created" do