fix default/override attribute blacklists and whitelists

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>

3 files changed, 52 insertions, 58 deletions

diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 87418b5732..2123d71068 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -455,13 +455,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
@@ -472,10 +469,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
@@ -490,11 +487,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 4c056ca349..0901fa2682 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -1469,13 +1469,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" },
@@ -1486,12 +1485,13 @@ describe Chef::Node do
                   "eth1" => {},
                 },
               },
-            },
-            "default" => {}, "normal" => {}, "override" => {}
-          }
+            }
+          node.automatic = {}
+          node.normal = {}
+          node.override = {}
 
           selected_data = {
-            "automatic" => {
+            "default" => {
               "filesystem" => {
                 "/dev/disk0s2" => { "size" => "10mb" },
               },
@@ -1501,12 +1501,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
 
@@ -1515,8 +1514,7 @@ describe Chef::Node do
             "foo/bar/baz",
           ]
 
-          data = {
-            "default" => {
+          node.default = {
               "foo" => {
                 "bar" => {
                   "baz" => false,
@@ -1525,8 +1523,11 @@ describe Chef::Node do
                   "stuff" => true,
                 },
               },
-            },
-          }
+            }
+
+          node.automatic = {}
+          node.normal = {}
+          node.override = {}
 
           selected_data = {
             "default" => {
@@ -1539,44 +1540,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" },
@@ -1587,12 +1585,13 @@ describe Chef::Node do
                   "eth1" => {},
                 },
               },
-            },
-            "default" => {}, "normal" => {}, "override" => {}
-          }
+            }
+          node.automatic = {}
+          node.normal = {}
+          node.override = {}
 
           selected_data = {
-            "automatic" => {
+            "default" => {
               "filesystem" => {
                 "map - autohome" => { "size" => "10mb" },
               },
@@ -1602,40 +1601,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