diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2019-04-18 21:56:13 -0700 |
---|---|---|
committer | Lamont Granquist <lamont@scriptkiddie.org> | 2019-04-18 22:07:45 -0700 |
commit | 0656a02111ad32ccccdc92e29d53a32b2c8dd66e (patch) | |
tree | 1cd3e7e5143d1f5c2ad77fa813f5ac7ccaf4063c | |
parent | c36403d741d7d51159fac3233e7eb5801eb44db4 (diff) | |
download | chef-lcg/fix-the-blacklist.tar.gz |
fix default/override attribute blacklists and whitelistslcg/fix-the-blacklist
looks like these have been broken for some time, the tests were only
testing automatic level (which has no "combined" hash version) and
the specs were injecting a plain old hash, so were failing to catch
this error on multiple different levels.
since the automatic level worked and since mostly people use automatic
blacklisting to remove junk from ohai it was very uncommon to hit
this bug.
Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
-rw-r--r-- | lib/chef/node.rb | 21 | ||||
-rw-r--r-- | lib/chef/whitelist.rb | 2 | ||||
-rw-r--r-- | spec/unit/node_spec.rb | 87 |
3 files changed, 52 insertions, 58 deletions
diff --git a/lib/chef/node.rb b/lib/chef/node.rb index c945bb640f..021cf33eb8 100644 --- a/lib/chef/node.rb +++ b/lib/chef/node.rb @@ -480,13 +480,10 @@ class Chef # Transform the node to a Hash def to_hash - index_hash = Hash.new + index_hash = attributes.to_hash index_hash["chef_type"] = "node" index_hash["name"] = name index_hash["chef_environment"] = chef_environment - attribute.each do |key, value| - index_hash[key] = value - end index_hash["recipe"] = run_list.recipe_names if run_list.recipe_names.length > 0 index_hash["role"] = run_list.role_names if run_list.role_names.length > 0 index_hash["run_list"] = run_list.run_list_items @@ -497,10 +494,10 @@ class Chef display = {} display["name"] = name display["chef_environment"] = chef_environment - display["automatic"] = automatic_attrs - display["normal"] = normal_attrs - display["default"] = attributes.combined_default - display["override"] = attributes.combined_override + display["automatic"] = attributes.automatic.to_hash + display["normal"] = attributes.normal.to_hash + display["default"] = attributes.combined_default.to_hash + display["override"] = attributes.combined_override.to_hash display["run_list"] = run_list.run_list_items display end @@ -515,11 +512,11 @@ class Chef "name" => name, "chef_environment" => chef_environment, "json_class" => self.class.name, - "automatic" => attributes.automatic, - "normal" => attributes.normal, + "automatic" => attributes.automatic.to_hash, + "normal" => attributes.normal.to_hash, "chef_type" => "node", - "default" => attributes.combined_default, - "override" => attributes.combined_override, + "default" => attributes.combined_default.to_hash, + "override" => attributes.combined_override.to_hash, # Render correctly for run_list items so malformed json does not result "run_list" => @primary_runlist.run_list.map { |item| item.to_s }, } diff --git a/lib/chef/whitelist.rb b/lib/chef/whitelist.rb index 58d0bd70c6..c94ffcaed2 100644 --- a/lib/chef/whitelist.rb +++ b/lib/chef/whitelist.rb @@ -45,7 +45,7 @@ class Chef all_data = data filtered_data = new_data parts[0..-2].each do |part| - unless all_data[part] + unless all_data.key?(part) Chef::Log.warn("Could not find whitelist attribute #{item}.") return nil end diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb index 752871e2e9..3225f912dc 100644 --- a/spec/unit/node_spec.rb +++ b/spec/unit/node_spec.rb @@ -1461,13 +1461,12 @@ describe Chef::Node do context "with whitelisted attributes configured" do it "should only save whitelisted attributes (and subattributes)" do - Chef::Config[:automatic_attribute_whitelist] = [ + Chef::Config[:default_attribute_whitelist] = [ ["filesystem", "/dev/disk0s2"], "network/interfaces/eth0", ] - data = { - "automatic" => { + node.default = { "filesystem" => { "/dev/disk0s2" => { "size" => "10mb" }, "map - autohome" => { "size" => "10mb" }, @@ -1478,12 +1477,13 @@ describe Chef::Node do "eth1" => {}, }, }, - }, - "default" => {}, "normal" => {}, "override" => {} - } + } + node.automatic = {} + node.normal = {} + node.override = {} selected_data = { - "automatic" => { + "default" => { "filesystem" => { "/dev/disk0s2" => { "size" => "10mb" }, }, @@ -1493,12 +1493,11 @@ describe Chef::Node do }, }, }, - "default" => {}, "normal" => {}, "override" => {} + "automatic" => {}, "normal" => {}, "override" => {} } node.name("picky-monkey") - allow(node).to receive(:for_json).and_return(data) - expect(@rest).to receive(:put).with("nodes/picky-monkey", selected_data).and_return("foo") + expect(@rest).to receive(:put).with("nodes/picky-monkey", hash_including(selected_data)).and_return("foo") node.save end @@ -1507,8 +1506,7 @@ describe Chef::Node do "foo/bar/baz", ] - data = { - "default" => { + node.default = { "foo" => { "bar" => { "baz" => false, @@ -1517,8 +1515,11 @@ describe Chef::Node do "stuff" => true, }, }, - }, - } + } + + node.automatic = {} + node.normal = {} + node.override = {} selected_data = { "default" => { @@ -1531,44 +1532,41 @@ describe Chef::Node do } node.name("falsey-monkey") - allow(node).to receive(:for_json).and_return(data) - expect(@rest).to receive(:put).with("nodes/falsey-monkey", selected_data).and_return("foo") + expect(@rest).to receive(:put).with("nodes/falsey-monkey", hash_including(selected_data)).and_return("foo") node.save end it "should not save any attributes if the whitelist is empty" do - Chef::Config[:automatic_attribute_whitelist] = [] + Chef::Config[:default_attribute_whitelist] = [] - data = { - "automatic" => { + node.default = { "filesystem" => { "/dev/disk0s2" => { "size" => "10mb" }, "map - autohome" => { "size" => "10mb" }, }, - }, - "default" => {}, "normal" => {}, "override" => {} - } + } + node.automatic = {} + node.normal = {} + node.override = {} selected_data = { "automatic" => {}, "default" => {}, "normal" => {}, "override" => {} } node.name("picky-monkey") - allow(node).to receive(:for_json).and_return(data) - expect(@rest).to receive(:put).with("nodes/picky-monkey", selected_data).and_return("foo") + expect(@rest).to receive(:put).with("nodes/picky-monkey", hash_including(selected_data)).and_return("foo") node.save end end context "with blacklisted attributes configured" do it "should only save non-blacklisted attributes (and subattributes)" do - Chef::Config[:automatic_attribute_blacklist] = [ + Chef::Config[:default_attribute_blacklist] = [ ["filesystem", "/dev/disk0s2"], "network/interfaces/eth0", ] - data = { - "automatic" => { + node.default = { "filesystem" => { "/dev/disk0s2" => { "size" => "10mb" }, "map - autohome" => { "size" => "10mb" }, @@ -1579,12 +1577,13 @@ describe Chef::Node do "eth1" => {}, }, }, - }, - "default" => {}, "normal" => {}, "override" => {} - } + } + node.automatic = {} + node.normal = {} + node.override = {} selected_data = { - "automatic" => { + "default" => { "filesystem" => { "map - autohome" => { "size" => "10mb" }, }, @@ -1594,40 +1593,38 @@ describe Chef::Node do }, }, }, - "default" => {}, "normal" => {}, "override" => {} + "automatic" => {}, "normal" => {}, "override" => {} } node.name("picky-monkey") - allow(node).to receive(:for_json).and_return(data) - expect(@rest).to receive(:put).with("nodes/picky-monkey", selected_data).and_return("foo") + expect(@rest).to receive(:put).with("nodes/picky-monkey", hash_including(selected_data)).and_return("foo") node.save end - it "should save all attributes if the blacklist is empty" do - Chef::Config[:automatic_attribute_blacklist] = [] + it "should save all attributes if the blacklist is empty" do + Chef::Config[:default_attribute_blacklist] = [] - data = { - "automatic" => { + node.default = { "filesystem" => { "/dev/disk0s2" => { "size" => "10mb" }, "map - autohome" => { "size" => "10mb" }, }, - }, - "default" => {}, "normal" => {}, "override" => {} - } + } + node.automatic = {} + node.normal = {} + node.override = {} selected_data = { - "automatic" => { + "default" => { "filesystem" => { "/dev/disk0s2" => { "size" => "10mb" }, "map - autohome" => { "size" => "10mb" }, }, }, - "default" => {}, "normal" => {}, "override" => {} + "automatic" => {}, "normal" => {}, "override" => {} } node.name("picky-monkey") - allow(node).to receive(:for_json).and_return(data) - expect(@rest).to receive(:put).with("nodes/picky-monkey", selected_data).and_return("foo") + expect(@rest).to receive(:put).with("nodes/picky-monkey", hash_including(selected_data)).and_return("foo") node.save end end |