Change attribute whitelist filter syntax to match Ohai

6 files changed, 125 insertions, 72 deletions

diff --git a/DOC_CHANGES.md b/DOC_CHANGES.md
index 2959cd63ea..e288c71972 100644
--- a/DOC_CHANGES.md
+++ b/DOC_CHANGES.md
@@ -52,24 +52,23 @@ If your automatic attribute data looks like
     "map - autohome" => {
       "size" => "10mb"
     }
+  },
+  "network" => {
+    "interfaces" => {
+      "eth0" => {...},
+      "eth1" => {...},
+    }
   }
 }
 ````
 and your config file looks like
 ````
-automatic_attribute_whitelist =
-  {
-    "filesystem" => {
-      "/dev/disk0s2" => true
-    }
-  }
+automatic_attribute_whitelist = ["network/interfaces/eth0"]
 ````
-then the entire `map - autohome` subtree will not be saved by the node.
+then the entire `filesystem` and `eth1` subtrees will not be saved by the node. To save the `/dev/disk0s2` subtree, you must write `automatic_attribute_whitelist = [ ["filesystem", "/dev/disk0s2"] ]`.
 
-If your config file looks like `automatic_attribute_whitelist = {}`, then none of your automatic attribute data will be saved by the node.
+If your config file looks like `automatic_attribute_whitelist = []`, then none of your automatic attribute data will be saved by the node.
 
 The default behavior is for the node to save all the attribute data. This can be ensured by setting your whitelist filter to `nil`.
 
-Note that only the keys in this has will be used. If the values are anything other than a hash, they are ignored. You cannot magically morph these config options into a blacklist by putting `false` as a value in the whitelist.
-
-We recommend only using `automatic_attribute_whitelist` to reduce the size of the system data being stored for nodes, and discourage the use of the other attribute whitelists except by advanced users. 
+We recommend only using `automatic_attribute_whitelist` to reduce the size of the system data being stored for nodes, and discourage the use of the other attribute whitelists except by advanced users.
diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index 92042d5248..66dd4cc77e 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -558,29 +558,9 @@ class Chef
     # the number of threads will help.
     default :cookbook_sync_threads, 10
 
-    # A whitelisted map of attribute keys you want sent over the wire when node
-    # data is saved. We will only use the keys in this hash to whitelist attributes;
-    # if the value are anything than another hash, we will ignore them. (You can't
-    # magically morph into a blacklist with "false" as a value in the whitelist.)
-    #
-    # If your data looks like:
-    # { "filesystem" => {
-    #     "/dev/disk0s2" => {
-    #       "size" => "10mb"
-    #     },
-    #     "map - autohome' => {
-    #       "size" => "10mb"
-    #     }
-    #   }
-    # }
-    # And your whitelist looks like:
-    # { "filesystem" => {
-    #     "/dev/disk0s2" => true
-    #   }
-    # }
-    # The entire "map - autohome" subtree will be dropped.
-    #
-    # The default setting is nil, which collects all data. Setting to {} will not
+    # A whitelisted array of attributes you want sent over the wire when node
+    # data is saved.
+    # The default setting is nil, which collects all data. Setting to [] will not
     # collect any data for save.
     default :automatic_attribute_whitelist, nil
     default :default_attribute_whitelist, nil
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 782ecc3fd8..37253ed7b3 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -329,5 +329,8 @@ class Chef
         super "Unable to acquire lock. Waited #{duration} seconds for #{blocking_pid} to release."
       end
     end
+
+    # Raise when a whitelisted attribute can't be found in the data.
+    class WhitelistAttributeNotFound < RuntimeError; end
   end
 end
diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 0a3a4f4297..17ec1d0f0a 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -34,6 +34,7 @@ require 'chef/node/attribute'
 require 'chef/mash'
 require 'chef/json_compat'
 require 'chef/search/query'
+require 'chef/whitelist'
 
 class Chef
   class Node
@@ -546,29 +547,17 @@ class Chef
     private
 
     def data_for_save
-      Chef::Log.info("Whitelisting node attributes for save")
       data = for_json
       ["automatic", "default", "normal", "override"].each do |level|
         whitelist_config_option = "#{level}_attribute_whitelist".to_sym
         whitelist = Chef::Config[whitelist_config_option]
-        unless whitelist.nil? # nil is default, saves everything
-          data[level] = apply_whitelist_filter(data[level], whitelist)
+        unless whitelist.nil? # nil => save everything
+          Chef::Log.info("Whitelisting #{level} node attributes for save.")
+          data[level] = Chef::Whitelist.filter(data[level], whitelist)
         end
       end
       data
     end
 
-    def apply_whitelist_filter(data, whitelist)
-      return nil if data.nil?
-
-      new_data = data.reject { |k, v| !whitelist.keys.include? k }
-      whitelist.each do |k, v|
-        if v.kind_of? Hash
-          new_data[k] = apply_whitelist_filter(new_data[k], v)
-        end
-      end
-      new_data
-    end
-
   end
 end
diff --git a/lib/chef/whitelist.rb b/lib/chef/whitelist.rb
new file mode 100644
index 0000000000..ad52215f11
--- /dev/null
+++ b/lib/chef/whitelist.rb
@@ -0,0 +1,82 @@
+
+require 'chef/exceptions'
+
+class Chef
+  class Whitelist
+
+    # filter takes two arguments - the data you want to filter, and a whitelisted array
+    # of keys you want included. You can capture a subtree of the data to filter by
+    # providing a "/"-delimited string of keys. If some key includes "/"-characters,
+    # you must provide an array of keys instead.
+    #
+    # Whitelist.filter(
+    #   { "filesystem" => {
+    #       "/dev/disk" => {
+    #         "size" => "10mb"
+    #       },
+    #       "map - autohome" => {
+    #         "size" => "10mb"
+    #       }
+    #     },
+    #     "network" => {
+    #       "interfaces" => {
+    #         "eth0" => {...},
+    #         "eth1" => {...}
+    #       }
+    #     }
+    #   },
+    #   ["network/interfaces/eth0", ["filesystem", "/dev/disk"]])
+    # will capture the eth0 and /dev/disk subtrees.
+    def self.filter(data, whitelist=nil)
+      return data if whitelist.nil?
+
+      new_data = {}
+      whitelist.each do |item|
+        self.add_data(data, new_data, item)
+      end
+      new_data
+    end
+
+    private
+
+    # Walk the data has according to the keys provided by the whitelisted item
+    # and add the data to the whitelisting result.
+    def self.add_data(data, new_data, item)
+      parts = self.to_array(item)
+
+      all_data = data
+      filtered_data = new_data
+      parts[0..-2].each do |part|
+        unless all_data[part]
+          Chef::Log.warn("Could not find whitelist attribute #{item}.")
+          return nil
+        end
+
+        filtered_data[part] ||= {}
+        filtered_data = filtered_data[part]
+        all_data = all_data[part]
+      end
+
+      unless all_data[parts[-1]]
+        Chef::Log.warn("Could not find whitelist attribute #{item}.")
+        return nil
+      end
+
+      filtered_data[parts[-1]] = all_data[parts[-1]]
+      new_data
+    end
+
+    # Accepts a String or an Array, and returns an Array of String keys that
+    # are used to traverse the data hash. Strings are split on "/", Arrays are
+    # assumed to contain exact keys (that is, Array elements will not be split
+    # by "/").
+    def self.to_array(item)
+      return item if item.kind_of? Array
+
+      parts = item.split("/")
+      parts.shift if !parts.empty? && parts[0].empty?
+      parts
+    end
+
+  end
+end
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index 7c8dba0b12..21a978a9c9 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -852,41 +852,40 @@ describe Chef::Node do
       end
 
       context "with whitelisted attributes configured" do
-        before do
-          @defaults = {}
-          ["automatic", "default", "normal", "override"].each do |level|
-            option_key = "#{level}_attribute_whitelist".to_sym
-            @defaults[level] = Chef::Config[option_key]
-          end
-        end
-
-        after do
-          @defaults.each do |level, setting|
-            option_key = "#{level}_attribute_whitelist".to_sym
-            Chef::Config[option_key] = setting
-          end
-        end
-
         it "should only save whitelisted attributes (and subattributes)" do
-          Chef::Config[:automatic_attribute_whitelist] = {
-            "filesystem" => { "/dev/disk0s2" => true }
-          }
+          Chef::Config[:automatic_attribute_whitelist] = [
+            ["filesystem", "/dev/disk0s2"],
+            "network/interfaces/eth0"
+          ]
 
           data = {
             "automatic" => {
               "filesystem" => {
                 "/dev/disk0s2"   => { "size" => "10mb" },
                 "map - autohome" => { "size" => "10mb" }
+              },
+              "network" => {
+                "interfaces" => {
+                  "eth0" => {},
+                  "eth1" => {}
+                }
               }
-            }
+            },
+            "default" => {}, "normal" => {}, "override" => {}
           }
 
           selected_data = {
             "automatic" => {
               "filesystem" => {
                 "/dev/disk0s2" => { "size" => "10mb" }
+              },
+              "network" => {
+                "interfaces" => {
+                  "eth0" => {}
+                }
               }
-            }
+            },
+            "default" => {}, "normal" => {}, "override" => {}
           }
 
           node.name("picky-monkey")
@@ -896,7 +895,7 @@ describe Chef::Node do
         end
 
         it "should not save any attributes if the whitelist is empty" do
-          Chef::Config[:automatic_attribute_whitelist] = {}
+          Chef::Config[:automatic_attribute_whitelist] = []
 
           data = {
             "automatic" => {
@@ -904,11 +903,12 @@ describe Chef::Node do
                 "/dev/disk0s2"   => { "size" => "10mb" },
                 "map - autohome" => { "size" => "10mb" }
               }
-            }
+            },
+            "default" => {}, "normal" => {}, "override" => {}
           }
 
           selected_data = {
-            "automatic" => {}
+            "automatic" => {}, "default" => {}, "normal" => {}, "override" => {}
           }
 
           node.name("picky-monkey")