Merge pull request #10302 from NaomiReeves/ncr_config_exit_code

Signed-off-by: Tim Smith <tsmith@chef.io>

5 files changed, 32 insertions, 12 deletions

diff --git a/lib/chef/application.rb b/lib/chef/application.rb
index 322c10460d..257f993bbe 100644
--- a/lib/chef/application.rb
+++ b/lib/chef/application.rb
@@ -96,7 +96,11 @@ class Chef
     # Parse configuration (options and config file)
     def configure_chef
       parse_options
-      load_config_file
+      begin
+        load_config_file
+      rescue Exception => e
+        Chef::Application.fatal!(e.message, Chef::Exceptions::ConfigurationError.new)
+      end
       chef_config.export_proxies
       chef_config.init_openssl
       File.umask chef_config[:umask]
@@ -151,8 +155,6 @@ class Chef
 
     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
 
     # Set the specific recipes to Chef::Config if the recipes are valid
@@ -352,7 +354,8 @@ class Chef
       logger.fatal("Configuration error #{error.class}: #{error.message}")
       filtered_trace = error.backtrace.grep(/#{Regexp.escape(config_file_path)}/)
       filtered_trace.each { |line| logger.fatal("  " + line ) }
-      Chef::Application.fatal!("Aborting due to error in '#{config_file_path}'", error)
+      raise Chef::Exceptions::ConfigurationError.new("Aborting due to error in '#{config_file_path}': #{error}")
+      # Chef::Application.fatal!("Aborting due to error in '#{config_file_path}'", Chef::Exceptions::ConfigurationError.new(error))
     end
 
     # This is a hook for testing
diff --git a/lib/chef/application/exit_code.rb b/lib/chef/application/exit_code.rb
index 3235bfadeb..ee0621f5ce 100644
--- a/lib/chef/application/exit_code.rb
+++ b/lib/chef/application/exit_code.rb
@@ -36,6 +36,7 @@ class Chef
         REBOOT_NEEDED: 37,
         REBOOT_FAILED: 41,
         # 42 was used by audit mode and should not be reused
+        CONFIG_FAILURE: 43,
         CLIENT_UPGRADED: 213,
       }.freeze
 
@@ -79,6 +80,8 @@ class Chef
             VALID_RFC_062_EXIT_CODES[:REBOOT_NEEDED]
           elsif reboot_failed?(exception)
             VALID_RFC_062_EXIT_CODES[:REBOOT_FAILED]
+          elsif configuration_failure?(exception)
+            VALID_RFC_062_EXIT_CODES[:CONFIG_FAILURE]
           elsif client_upgraded?(exception)
             VALID_RFC_062_EXIT_CODES[:CLIENT_UPGRADED]
           else
@@ -104,6 +107,12 @@ class Chef
           end
         end
 
+        def configuration_failure?(exception)
+          resolve_exception_array(exception).any? do |e|
+            e.is_a? Chef::Exceptions::ConfigurationError
+          end
+        end
+
         def client_upgraded?(exception)
           resolve_exception_array(exception).any? do |e|
             e.is_a? Chef::Exceptions::ClientUpgraded
diff --git a/spec/unit/application/client_spec.rb b/spec/unit/application/client_spec.rb
index 004debf5ad..6977e4f108 100644
--- a/spec/unit/application/client_spec.rb
+++ b/spec/unit/application/client_spec.rb
@@ -192,7 +192,7 @@ describe Chef::Application::Client, "reconfigure" do
 
       context "with an empty value" do
         it "should terminate with message" do
-          expect(Chef::Application).to receive(:fatal!).with('Unparsable config option ""').and_raise("so ded")
+          expect(Chef::Application).to receive(:fatal!).with('Unparsable config option ""', ChefConfig::ConfigurationError.new).and_raise("so ded")
           ARGV.replace(["--config-option", ""])
           expect { app.reconfigure }.to raise_error "so ded"
         end
@@ -200,7 +200,7 @@ describe Chef::Application::Client, "reconfigure" do
 
       context "with an invalid value" do
         it "should terminate with message" do
-          expect(Chef::Application).to receive(:fatal!).with('Unparsable config option "asdf"').and_raise("so ded")
+          expect(Chef::Application).to receive(:fatal!).with('Unparsable config option "asdf"', ChefConfig::ConfigurationError.new).and_raise("so ded")
           ARGV.replace(["--config-option", "asdf"])
           expect { app.reconfigure }.to raise_error "so ded"
         end
diff --git a/spec/unit/application/exit_code_spec.rb b/spec/unit/application/exit_code_spec.rb
index 6b025938a6..7ede9fb86d 100644
--- a/spec/unit/application/exit_code_spec.rb
+++ b/spec/unit/application/exit_code_spec.rb
@@ -61,6 +61,10 @@ describe Chef::Application::ExitCode do
       expect(valid_rfc_exit_codes.include?(41)).to eq(true)
     end
 
+    it "validates a CONFIG_FAILURE return code of 43" do
+      expect(valid_rfc_exit_codes.include?(43)).to eq(true)
+    end
+
     it "validates a CLIENT_UPGRADED return code of 213" do
       expect(valid_rfc_exit_codes.include?(213)).to eq(true)
     end
@@ -113,6 +117,12 @@ describe Chef::Application::ExitCode do
       expect(exit_codes.normalize_exit_code(runtime_error)).to eq(37)
     end
 
+    it "returns CONFIG_FAILURE when a configuration exception is specified" do
+      config_error = Chef::Exceptions::ConfigurationError.new("BOOM")
+      runtime_error = Chef::Exceptions::RunFailedWrappingError.new(config_error)
+      expect(exit_codes.normalize_exit_code(runtime_error)).to eq(43)
+    end
+
     it "returns CLIENT_UPGRADED when the client was upgraded during converge" do
       client_upgraded_error = Chef::Exceptions::ClientUpgraded.new("BOOM")
       runtime_error = Chef::Exceptions::RunFailedWrappingError.new(client_upgraded_error)
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index aa916d0793..b84f0c99f2 100644
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -491,16 +491,14 @@ describe Chef::Application do
 
   describe "configuration errors" do
     before do
-      expect(Process).to receive(:exit)
+      allow(Process).to receive(:exit).and_return(true)
     end
 
     def raises_informative_fatals_on_configure_chef
       config_file_regexp = Regexp.new @app.config[:config_file]
-      expect(Chef::Log).to receive(:fatal)
-        .with(/Configuration error/)
-      expect(Chef::Log).to receive(:fatal)
-        .with(config_file_regexp)
-        .at_least(1).times
+      expect(Chef::Log).to receive(:fatal).with(/Configuration error/)
+      expect(Chef::Log).to receive(:fatal).with(config_file_regexp)
+      expect(Process).to receive(:exit).with(43).and_return(true)
       @app.configure_chef
     end