Merge branch 'chef:master' into update-unshare-error-message

12 files changed, 184 insertions, 83 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c2c0218f3..2b1d594207 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,17 +1,19 @@
 <!-- usage documentation: http://expeditor-docs.es.chef.io/configuration/changelog/ -->
 This changelog lists individual merged pull requests to Chef Infra Client and geared towards developers. For a list of significant changes per release see the [Chef Infra Client Release Notes](https://docs.chef.io/release_notes_client/).
 
-<!-- latest_release 17.3.37 -->
-## [v17.3.37](https://github.com/chef/chef/tree/v17.3.37) (2021-07-15)
+<!-- latest_release 17.3.39 -->
+## [v17.3.39](https://github.com/chef/chef/tree/v17.3.39) (2021-07-16)
 
 #### Merged Pull Requests
-- Bump omnibus from `5803fdc` to `6d109b6` in /omnibus [#11822](https://github.com/chef/chef/pull/11822) ([dependabot[bot]](https://github.com/dependabot[bot]))
+- Support command line setting of run_list with policyfiles [#11803](https://github.com/chef/chef/pull/11803) ([lamont-granquist](https://github.com/lamont-granquist))
 <!-- latest_release -->
 
 <!-- release_rollup since=17.2.29 -->
 ### Changes not yet released to stable
 
 #### Merged Pull Requests
+- Support command line setting of run_list with policyfiles [#11803](https://github.com/chef/chef/pull/11803) ([lamont-granquist](https://github.com/lamont-granquist)) <!-- 17.3.39 -->
+- Bump omnibus-software from `cdef22f` to `237c0a4` in /omnibus [#11827](https://github.com/chef/chef/pull/11827) ([dependabot[bot]](https://github.com/dependabot[bot])) <!-- 17.3.38 -->
 - Bump omnibus from `5803fdc` to `6d109b6` in /omnibus [#11822](https://github.com/chef/chef/pull/11822) ([dependabot[bot]](https://github.com/dependabot[bot])) <!-- 17.3.37 -->
 - Bump omnibus-software from `e9feb35` to `cdef22f` in /omnibus [#11815](https://github.com/chef/chef/pull/11815) ([dependabot[bot]](https://github.com/dependabot[bot])) <!-- 17.3.36 -->
 - Bump omnibus from `5c1b453` to `5803fdc` in /omnibus [#11814](https://github.com/chef/chef/pull/11814) ([dependabot[bot]](https://github.com/dependabot[bot])) <!-- 17.3.35 -->
diff --git a/Gemfile.lock b/Gemfile.lock
index 3fb69d81dd..43f7deaa1b 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -35,11 +35,11 @@ GIT
 PATH
   remote: .
   specs:
-    chef (17.3.37)
+    chef (17.3.39)
       addressable
       aws-sdk-secretsmanager (~> 1.46)
-      chef-config (= 17.3.37)
-      chef-utils (= 17.3.37)
+      chef-config (= 17.3.39)
+      chef-utils (= 17.3.39)
       chef-vault
       chef-zero (>= 14.0.11)
       diff-lcs (>= 1.2.4, < 1.4.0)
@@ -63,11 +63,11 @@ PATH
       train-core (~> 3.2, >= 3.2.28)
       train-winrm (>= 0.2.5)
       uuidtools (>= 2.1.5, < 3.0)
-    chef (17.3.37-universal-mingw32)
+    chef (17.3.39-universal-mingw32)
       addressable
       aws-sdk-secretsmanager (~> 1.46)
-      chef-config (= 17.3.37)
-      chef-utils (= 17.3.37)
+      chef-config (= 17.3.39)
+      chef-utils (= 17.3.39)
       chef-vault
       chef-zero (>= 14.0.11)
       diff-lcs (>= 1.2.4, < 1.4.0)
@@ -106,15 +106,15 @@ PATH
 PATH
   remote: chef-bin
   specs:
-    chef-bin (17.3.37)
-      chef (= 17.3.37)
+    chef-bin (17.3.39)
+      chef (= 17.3.39)
 
 PATH
   remote: chef-config
   specs:
-    chef-config (17.3.37)
+    chef-config (17.3.39)
       addressable
-      chef-utils (= 17.3.37)
+      chef-utils (= 17.3.39)
       fuzzyurl
       mixlib-config (>= 2.2.12, < 4.0)
       mixlib-shellout (>= 2.0, < 4.0)
@@ -123,7 +123,7 @@ PATH
 PATH
   remote: chef-utils
   specs:
-    chef-utils (17.3.37)
+    chef-utils (17.3.39)
       concurrent-ruby
 
 GEM
@@ -136,7 +136,7 @@ GEM
       mixlib-shellout (>= 2.0, < 4.0)
     ast (2.4.2)
     aws-eventstream (1.1.1)
-    aws-partitions (1.478.0)
+    aws-partitions (1.479.0)
     aws-sdk-core (3.117.0)
       aws-eventstream (~> 1, >= 1.0.2)
       aws-partitions (~> 1, >= 1.239.0)
diff --git a/VERSION b/VERSION
index 0d4ff4e5e9..25541528c7 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-17.3.37
\ No newline at end of file
+17.3.39
\ No newline at end of file
diff --git a/chef-bin/lib/chef-bin/version.rb b/chef-bin/lib/chef-bin/version.rb
index 1304cd1fdb..7223375e60 100644
--- a/chef-bin/lib/chef-bin/version.rb
+++ b/chef-bin/lib/chef-bin/version.rb
@@ -21,7 +21,7 @@
 
 module ChefBin
   CHEFBIN_ROOT = File.expand_path("..", __dir__)
-  VERSION = "17.3.37".freeze
+  VERSION = "17.3.39".freeze
 end
 
 #
diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index 11c510f297..3eb8c8475c 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -644,6 +644,16 @@ module ChefConfig
     # effect if `policy_document_native_api` is set to `false`.
     default :deployment_group, nil
 
+    # When using policyfiles you can optionally set it to read the node.run_list
+    # from the server and have that override the policyfile run_list or the
+    # named_run_list set in config.  With policyfiles there is no depsolving done
+    # on the run_list items so every item in the run_list must be in the set of
+    # cookbooks pushed to the node.  This enables flows where the node can change
+    # its run_list and have it persist or to bootstrap nodes with the -j flag.  If
+    # no run_list is set on the server node object then the configured named_run_list
+    # or run_list out of the policy is used.
+    default :policy_persist_run_list, false
+
     # Set these to enable SSL authentication / mutual-authentication
     # with the server
 
diff --git a/chef-config/lib/chef-config/version.rb b/chef-config/lib/chef-config/version.rb
index 839f1b930e..06aba4e23c 100644
--- a/chef-config/lib/chef-config/version.rb
+++ b/chef-config/lib/chef-config/version.rb
@@ -15,5 +15,5 @@
 
 module ChefConfig
   CHEFCONFIG_ROOT = File.expand_path("..", __dir__)
-  VERSION = "17.3.37".freeze
+  VERSION = "17.3.39".freeze
 end
diff --git a/chef-utils/lib/chef-utils/version.rb b/chef-utils/lib/chef-utils/version.rb
index 4d354d5e2e..436236b72e 100644
--- a/chef-utils/lib/chef-utils/version.rb
+++ b/chef-utils/lib/chef-utils/version.rb
@@ -16,5 +16,5 @@
 
 module ChefUtils
   CHEFUTILS_ROOT = File.expand_path("..", __dir__)
-  VERSION = "17.3.37"
+  VERSION = "17.3.39"
 end
diff --git a/knife/lib/chef/knife/version.rb b/knife/lib/chef/knife/version.rb
index fb85d2eb1c..b6a0302a76 100644
--- a/knife/lib/chef/knife/version.rb
+++ b/knife/lib/chef/knife/version.rb
@@ -17,7 +17,7 @@
 class Chef
   class Knife
     KNIFE_ROOT = File.expand_path("../..", __dir__)
-    VERSION = "17.3.37".freeze
+    VERSION = "17.3.39".freeze
   end
 end
 
diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb
index 16fdf49256..0603acf958 100644
--- a/lib/chef/policy_builder/policyfile.rb
+++ b/lib/chef/policy_builder/policyfile.rb
@@ -32,14 +32,8 @@ class Chef
     # Policyfile is a policy builder implementation that gets run
     # list and cookbook version information from a single document.
     #
-    # == Unsupported Options:
-    # * override_runlist:: This could potentially be integrated into the
-    # policyfile, or replaced with a similar feature that has different
-    # semantics.
-    # * specific_recipes:: put more design thought into this use case.
-    # * run_list in json_attribs:: would be ignored anyway, so it raises an error.
-    # * chef-solo:: not currently supported. Need more design thought around
-    # how this should work.
+    # Does not support legacy chef-solo or roles/environments.
+    #
     class Policyfile
 
       class UnsupportedFeature < StandardError; end
@@ -96,10 +90,6 @@ class Chef
           raise UnsupportedFeature, "Policyfile does not support chef-solo. Use #{ChefUtils::Dist::Infra::CLIENT} local mode instead."
         end
 
-        if json_attribs && json_attribs.key?("run_list")
-          raise UnsupportedFeature, "Policyfile does not support setting the run_list in json data."
-        end
-
         if Chef::Config[:environment] && !Chef::Config[:environment].chomp.empty?
           raise UnsupportedFeature, "Policyfile does not work with an Environment configured."
         end
@@ -147,6 +137,9 @@ class Chef
         expand_run_list
         apply_policyfile_attributes
 
+        if persistent_run_list_set?
+          Chef::Log.warn("The node.run_list setting is overriding the Policyfile run_list")
+        end
         Chef::Log.info("Run List is [#{run_list}]")
         Chef::Log.info("Run List expands to [#{run_list_with_versions_for_display(run_list).join(", ")}]")
 
@@ -203,7 +196,7 @@ class Chef
       #
       # @return [RunListExpansionIsh] A RunListExpansion duck-type.
       def expand_run_list
-        CookbookCacheCleaner.instance.skip_removal = true if named_run_list_requested?
+        validate_run_list!(run_list)
 
         node.run_list(run_list)
         node.automatic_attrs[:policy_revision] = revision_id
@@ -232,6 +225,18 @@ class Chef
 
       # @api private
       #
+      # Validate run_list against policyfile cookbooks
+      #
+      def validate_run_list!(run_list)
+        run_list.map do |recipe_spec|
+          cookbook, recipe = parse_recipe_spec(recipe_spec)
+          lock_data = cookbook_lock_for(cookbook)
+          raise PolicyfileError, "invalid run_list item '#{recipe_spec}' not in cookbook set of PolicyFile #{policyfile_location}" unless lock_data
+        end
+      end
+
+      # @api private
+      #
       # Generates an array of strings with recipe names including version and
       # identifier info.
       def run_list_with_versions_for_display(run_list)
@@ -278,7 +283,12 @@ class Chef
       def parse_recipe_spec(recipe_spec)
         rmatch = recipe_spec.to_s.match(/recipe\[([^:]+)::([^:]+)\]/)
         if rmatch.nil?
-          raise PolicyfileError, "invalid recipe specification #{recipe_spec} in Policyfile from #{policyfile_location}"
+          rmatch = recipe_spec.to_s.match(/recipe\[([^:]+)\]/)
+          if rmatch.nil?
+            raise PolicyfileError, "invalid recipe specification #{recipe_spec} in Policyfile from #{policyfile_location}"
+          else
+            [rmatch[1], "default"]
+          end
         else
           [rmatch[1], rmatch[2]]
         end
@@ -294,7 +304,11 @@ class Chef
       def run_list
         return override_runlist.map(&:to_s) if override_runlist
 
-        if named_run_list_requested?
+        if json_attribs["run_list"]
+          json_attribs["run_list"]
+        elsif persistent_run_list_set?
+          node.run_list
+        elsif named_run_list_requested?
           named_run_list || raise(ConfigurationError,
             "Policy '#{retrieved_policy_name}' revision '#{revision_id}' does not have named_run_list '#{named_run_list_name}'" +
             "(available named_run_lists: [#{available_named_run_lists.join(", ")}])")
@@ -450,7 +464,7 @@ class Chef
       # should be reduced to a single call.
       def cookbooks_to_sync
         @cookbook_to_sync ||= begin
-          events.cookbook_resolution_start(run_list_with_versions_for_display(policy["run_list"]))
+          events.cookbook_resolution_start(run_list_with_versions_for_display(run_list))
 
           cookbook_versions_by_name = cookbook_locks.inject({}) do |cb_map, (name, lock_data)|
             cb_map[name] = manifest_for(name, lock_data)
@@ -462,7 +476,7 @@ class Chef
         end
       rescue Exception => e
         # TODO: wrap/munge exception to provide helpful error output
-        events.cookbook_resolution_failed(run_list_with_versions_for_display(policy["run_list"]), e)
+        events.cookbook_resolution_failed(run_list_with_versions_for_display(run_list), e)
         raise
       end
 
@@ -532,6 +546,10 @@ class Chef
         (policy["named_run_lists"] || {}).keys
       end
 
+      def persistent_run_list_set?
+        Chef::Config[:policy_persist_run_list] && node.run_list && !node.run_list.empty?
+      end
+
       def named_run_list_requested?
         !!Chef::Config[:named_run_list]
       end
diff --git a/lib/chef/version.rb b/lib/chef/version.rb
index 28bcebd705..24e9f9452c 100644
--- a/lib/chef/version.rb
+++ b/lib/chef/version.rb
@@ -23,7 +23,7 @@ require_relative "version_string"
 
 class Chef
   CHEF_ROOT = File.expand_path("..", __dir__)
-  VERSION = Chef::VersionString.new("17.3.37")
+  VERSION = Chef::VersionString.new("17.3.39")
 end
 
 #
diff --git a/omnibus/Gemfile.lock b/omnibus/Gemfile.lock
index ccc43c8000..5d36d9a405 100644
--- a/omnibus/Gemfile.lock
+++ b/omnibus/Gemfile.lock
@@ -1,6 +1,6 @@
 GIT
   remote: https://github.com/chef/omnibus-software.git
-  revision: cdef22f0276e2e7272d3811bb459109b7e67a2bf
+  revision: 237c0a49b458da92cc6cdfbc488b8455f848f1c4
   branch: master
   specs:
     omnibus-software (4.0.0)
diff --git a/spec/unit/policy_builder/policyfile_spec.rb b/spec/unit/policy_builder/policyfile_spec.rb
index 63dc300f2a..d3ad6154c0 100644
--- a/spec/unit/policy_builder/policyfile_spec.rb
+++ b/spec/unit/policy_builder/policyfile_spec.rb
@@ -72,40 +72,40 @@ describe Chef::PolicyBuilder::Policyfile do
 
   let(:policyfile_default_attributes) do
     {
-        "policyfile_default_attr" => "policyfile_default_value",
-        "top_level_attr" => "hat",
-        "baseline_attr" => {
-          "one" => 1,
-          "two" => 2,
-          "deep" => {
-              "three" => 3,
-              "four" => [4],
-              "five" => [5],
-          },
+      "policyfile_default_attr" => "policyfile_default_value",
+      "top_level_attr" => "hat",
+      "baseline_attr" => {
+        "one" => 1,
+        "two" => 2,
+        "deep" => {
+          "three" => 3,
+          "four" => [4],
+          "five" => [5],
         },
-        "policy_group_value" => {
-          "baseline_attr" => {
-            "one" => 111,
-          },
+      },
+      "policy_group_value" => {
+        "baseline_attr" => {
+          "one" => 111,
         },
-      }
+      },
+    }
   end
 
   let(:policyfile_override_attributes) do
     {
-       "policyfile_override_attr" => "policyfile_override_value",
-       "baseline_attr" => {
-         "deep" => {
-           "three" => 333 },
-       },
-       "policy_group_value" => {
-         "top_level_attr" => "cat",
-         "baseline_attr" => {
-           "deep" => {
-             "four" => [444],
-           },
-         },
-       },
+      "policyfile_override_attr" => "policyfile_override_value",
+      "baseline_attr" => {
+        "deep" => {
+          "three" => 333 },
+      },
+      "policy_group_value" => {
+        "top_level_attr" => "cat",
+        "baseline_attr" => {
+          "deep" => {
+            "four" => [444],
+          },
+        },
+      },
     }
   end
 
@@ -162,14 +162,6 @@ describe Chef::PolicyBuilder::Policyfile do
       end
     end
 
-    context "when json_attribs contains a run_list" do
-      let(:json_attribs) { { "run_list" => [] } }
-
-      it "errors on create" do
-        expect { initialize_pb }.to raise_error(err_namespace::UnsupportedFeature)
-      end
-    end
-
     context "when an environment is configured" do
       before { Chef::Config[:environment] = "blurch" }
 
@@ -347,9 +339,9 @@ describe Chef::PolicyBuilder::Policyfile do
           expect { policy_builder.validate_policyfile }.to raise_error(err_namespace::PolicyfileError)
         end
 
-        it "errors if the policyfile json contains non-fully qualified recipe items" do
+        it "does not error if the policyfile json contains non-fully qualified recipe items" do
           parsed_policyfile_json["run_list"] = ["recipe[foo]"]
-          expect { policy_builder.validate_policyfile }.to raise_error(err_namespace::PolicyfileError)
+          expect { policy_builder.validate_policyfile }.not_to raise_error
         end
 
         it "errors if the policyfile doesn't have a run_list key" do
@@ -397,8 +389,8 @@ describe Chef::PolicyBuilder::Policyfile do
             {
               id: "_policy_node",
               run_list: [
-               { type: "recipe", name: "test::default", skipped: false, version: nil },
-               { type: "recipe", name: "test::other", skipped: false, version: nil },
+                { type: "recipe", name: "test::default", skipped: false, version: nil },
+                { type: "recipe", name: "test::other", skipped: false, version: nil },
               ],
             }
           end
@@ -670,10 +662,6 @@ describe Chef::PolicyBuilder::Policyfile do
               expect(node[:recipes]).to eq( ["example1::default"] )
             end
 
-            it "disables the cookbook cache cleaner" do
-              expect(Chef::CookbookCacheCleaner.instance.skip_removal).to be(true)
-            end
-
           end
         end
 
@@ -884,6 +872,89 @@ describe Chef::PolicyBuilder::Policyfile do
 
         end
       end
+
+      describe "selecting the run_list" do
+        let(:node) do
+          node = Chef::Node.new
+          node.name(node_name)
+          node
+        end
+
+        before do
+          allow(policy_builder).to receive(:node).and_return(node)
+        end
+
+        context "when json_attribs contains a run_list" do
+          let(:json_attribs) { { "run_list" => [ "recipe[something::default]" ] } }
+
+          it "reads the run_list from the json_attribs" do
+            expect(policy_builder.run_list).to eql(json_attribs["run_list"])
+          end
+
+          it "ignores the node.run_list" do
+            node.run_list.reset!("recipe[incorrect::incorrect]")
+            expect(policy_builder.run_list).to eql(json_attribs["run_list"])
+          end
+
+          it "ignores the node.run_list if the Chef::Config value is set" do
+            Chef::Config[:policy_persist_run_list] = true
+            node.run_list.reset!("recipe[incorrect::incorrect]")
+            expect(policy_builder.run_list).to eql(json_attribs["run_list"])
+          end
+        end
+
+        it "reads the run_list from the policyfile" do
+          expect(policy_builder.run_list).to eql(policyfile_run_list)
+        end
+
+        it "ignores the node.run_list by default" do
+          node.run_list.reset!("recipe[incorrect::incorrect]")
+          expect(policy_builder.run_list).to eql(policyfile_run_list)
+        end
+
+        it "uses the node.run_list if the Chef::Config value is set" do
+          Chef::Config[:policy_persist_run_list] = true
+          node.run_list.reset!("recipe[correct::default]")
+          expect(policy_builder.run_list).to eql(node.run_list)
+        end
+
+        it "does not use an empty node.run_list" do
+          Chef::Config[:policy_persist_run_list] = true
+          node.run_list.reset!
+          expect(policy_builder.run_list).to eql(policyfile_run_list)
+        end
+
+        context "with a valid named_run_list" do
+          let(:parsed_policyfile_json) do
+            basic_valid_policy_data.dup.tap do |p|
+              p["named_run_lists"] = {
+                "deploy-app" => [ "recipe[example1::default]" ],
+              }
+            end
+          end
+
+          it "uses the named_run_list over the policyfile" do
+            Chef::Config[:named_run_list] = "deploy-app"
+            expect(policy_builder.run_list).to eq([ "recipe[example1::default]" ])
+          end
+
+          it "is overridden if the run_list is persistent" do
+            Chef::Config[:named_run_list] = "deploy-app"
+            Chef::Config[:policy_persist_run_list] = true
+            node.run_list.reset!("recipe[correct::default]")
+            expect(policy_builder.run_list).to eql(node.run_list)
+          end
+
+          context "when json_attribs contains a run_list" do
+            let(:json_attribs) { { "run_list" => [ "recipe[something::default]" ] } }
+
+            it "overrides the named_run_list" do
+              expect(policy_builder.run_list).to eql(json_attribs["run_list"])
+            end
+          end
+
+        end
+      end
     end
   end
 end