Allow empty strings in -o to result in empty override run listlcg/empty-override-runlist

Previously this was not possible.  Now you can do this:

```
chef-client -o "" ./foo.rb
```

And it will only run foo.rb and not any of the recipes in your
run_list at all.  Node will not be saved at the end.

Useful to one-shot something like keyrotation (which still needs
keys and needs to talk to the chef server API, so this is a
different use case from simple chef-apply).

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

5 files changed, 102 insertions, 12 deletions

diff --git a/lib/chef/application.rb b/lib/chef/application.rb
index a63d8218f4..817cdf051d 100644
--- a/lib/chef/application.rb
+++ b/lib/chef/application.rb
@@ -1,7 +1,7 @@
 #
 # Author:: AJ Christensen (<aj@chef.io>)
 # Author:: Mark Mzyk (mmzyk@chef.io)
-# Copyright:: Copyright 2008-2018, Chef Software Inc.
+# Copyright:: Copyright 2008-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -260,7 +260,6 @@ class Chef
 
       Chef::LocalMode.with_server_connectivity do
         override_runlist = config[:override_runlist]
-        override_runlist ||= [] if specific_recipes.size > 0
         @chef_client = Chef::Client.new(
           @chef_client_json,
           override_runlist: override_runlist,
diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 9b67f27f14..c945bb640f 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -300,13 +300,37 @@ class Chef
       @primary_runlist
     end
 
-    attr_writer :override_runlist
+    # This boolean can be useful to determine if an override_runlist is set, it can be true
+    # even if the override_runlist is empty.
+    #
+    # (Mutators can set the override_runlist so any non-empty override_runlist is considered set)
+    #
+    # @return [Boolean] if the override run list has been set
+    def override_runlist_set?
+      !!@override_runlist_set || !override_runlist.empty?
+    end
+
+    # Accessor for override_runlist (this cannot set an empty override run list)
+    #
+    # @params args [Array] override run list to set
+    # @return [Chef::RunList] the override run list
     def override_runlist(*args)
-      args.length > 0 ? @override_runlist.reset!(args) : @override_runlist
+      return @override_runlist if args.length == 0
+      @override_runlist_set = true
+      @override_runlist.reset!(args)
+    end
+
+    # Setter for override_runlist which allows setting an empty override run list and marking it to be used
+    #
+    # @params array [Array] override run list to set
+    # @return [Chef::RunList] the override run list
+    def override_runlist=(array)
+      @override_runlist_set = true
+      @override_runlist.reset!(array)
     end
 
     def select_run_list
-      @override_runlist.empty? ? @primary_runlist : @override_runlist
+      override_runlist_set? ? @override_runlist : @primary_runlist
     end
 
     # Returns an Array of roles and recipes, in the order they will be applied.
diff --git a/lib/chef/policy_builder/expand_node_object.rb b/lib/chef/policy_builder/expand_node_object.rb
index dda4a2b4c3..eea5a37edd 100644
--- a/lib/chef/policy_builder/expand_node_object.rb
+++ b/lib/chef/policy_builder/expand_node_object.rb
@@ -214,7 +214,7 @@ class Chef
       # override_runlist was provided. Chef::Client uses this to decide whether
       # to do the final node save at the end of the run or not.
       def temporary_policy?
-        !node.override_runlist.empty?
+        node.override_runlist_set?
       end
 
       ########################################
@@ -222,9 +222,9 @@ class Chef
       ########################################
 
       def setup_run_list_override
-        runlist_override_sanity_check!
-        unless override_runlist.empty?
-          node.override_runlist(*override_runlist)
+        unless override_runlist.nil?
+          runlist_override_sanity_check!
+          node.override_runlist = override_runlist
           Chef::Log.warn "Run List override has been provided."
           Chef::Log.warn "Original Run List: [#{node.primary_runlist}]"
           Chef::Log.warn "Overridden Run List: [#{node.run_list}]"
diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb
index afe94f854e..006839be3f 100644
--- a/spec/integration/client/client_spec.rb
+++ b/spec/integration/client/client_spec.rb
@@ -233,6 +233,7 @@ describe "chef-client" do
             THECONSTANT = '1'
           end
         EOM
+
         file "arbitrary.rb", <<~EOM
           file #{path_to('tempfile.txt').inspect} do
             content ::Blah::THECONSTANT
@@ -245,6 +246,72 @@ describe "chef-client" do
         expect(IO.read(path_to("tempfile.txt"))).to eq("1")
       end
 
+      it "should run recipes specified directly on the command line AFTER recipes in the run list (without an override_runlist this time)" do
+        file "config/client.rb", <<~EOM
+          local_mode true
+          client_key #{path_to('mykey.pem').inspect}
+          cookbook_path #{path_to('cookbooks').inspect}
+        EOM
+
+        file "config/dna.json", <<~EOM
+          {
+            "run_list": [ "recipe[x::constant_definition]" ]
+          }
+        EOM
+
+        file "cookbooks/x/recipes/constant_definition.rb", <<~EOM
+          class ::Blah
+            THECONSTANT = '1'
+          end
+        EOM
+
+        file "arbitrary.rb", <<~EOM
+          file #{path_to('tempfile.txt').inspect} do
+            content ::Blah::THECONSTANT
+          end
+        EOM
+
+        result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -j \"#{path_to('config/dna.json')}\" arbitrary.rb", cwd: path_to(""))
+        result.error!
+
+        expect(IO.read(path_to("tempfile.txt"))).to eq("1")
+      end
+
+      it "an override_runlist of an empty string should allow a recipe specified directly on the command line to be the only one run" do
+        file "config/client.rb", <<~EOM
+          local_mode true
+          client_key #{path_to('mykey.pem').inspect}
+          cookbook_path #{path_to('cookbooks').inspect}
+          class ::Blah
+            THECONSTANT = "1"
+          end
+        EOM
+
+        file "config/dna.json", <<~EOM
+          {
+            "run_list": [ "recipe[x::constant_definition]" ]
+          }
+        EOM
+
+        file "cookbooks/x/recipes/constant_definition.rb", <<~EOM
+          class ::Blah
+            THECONSTANT = "2"
+          end
+        EOM
+
+        file "arbitrary.rb", <<~EOM
+          raise "this test failed" unless ::Blah::THECONSTANT == "1"
+          file #{path_to('tempfile.txt').inspect} do
+            content ::Blah::THECONSTANT
+          end
+        EOM
+
+        result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -j \"#{path_to('config/dna.json')}\" -o \"\" arbitrary.rb", cwd: path_to(""))
+        result.error!
+
+        expect(IO.read(path_to("tempfile.txt"))).to eq("1")
+      end
+
     end
 
     it "should complete with success when passed the -z flag" do
diff --git a/spec/unit/policy_builder/expand_node_object_spec.rb b/spec/unit/policy_builder/expand_node_object_spec.rb
index 122c960c11..d622bfcbb4 100644
--- a/spec/unit/policy_builder/expand_node_object_spec.rb
+++ b/spec/unit/policy_builder/expand_node_object_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Daniel DeLeo (<dan@chef.io>)
-# Copyright:: Copyright 2014-2017, Chef Software Inc.
+# Copyright:: Copyright 2014-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -42,7 +42,7 @@ describe Chef::PolicyBuilder::ExpandNodeObject do
       expect(policy_builder).to respond_to(:finish_load_node)
     end
 
-    it "implements  a build_node method" do
+    it "implements a build_node method" do
       expect(policy_builder).to respond_to(:build_node)
     end
 
@@ -224,7 +224,7 @@ describe Chef::PolicyBuilder::ExpandNodeObject do
       end
 
       it "reports that a temporary policy is being used" do
-        expect(policy_builder.temporary_policy?).to be_truthy
+        expect(policy_builder.temporary_policy?).to be true
       end
 
     end