diff options
author | Lamont Granquist <454857+lamont-granquist@users.noreply.github.com> | 2021-07-16 12:29:58 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-07-16 12:29:58 -0700 |
commit | d8ad5a60dda218f4808da8990e2f08a9eadcc005 (patch) | |
tree | 15b3a8247fd014572c3d07b02280fcb955539071 | |
parent | 1eaa82c5dce935aa891e9beb4aa390da00073359 (diff) | |
parent | 31c051b79dc17bf4687e35ac12a824f608598ff1 (diff) | |
download | chef-d8ad5a60dda218f4808da8990e2f08a9eadcc005.tar.gz |
Merge pull request #11803 from chef/lcg/policyfile-run-list
-rw-r--r-- | chef-config/lib/chef-config/config.rb | 10 | ||||
-rw-r--r-- | lib/chef/policy_builder/policyfile.rb | 52 | ||||
-rw-r--r-- | spec/unit/policy_builder/policyfile_spec.rb | 159 |
3 files changed, 160 insertions, 61 deletions
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/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/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 |