Merge pull request #1431 from opscode/CHEF-3811

add whitelist config options for attributes saved by the node

7 files changed, 222 insertions, 8 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c3556f6e67..696ca05e93 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@
 * chef-full template gets knife options to override install script url, add wget/curl cli options, and custom install commands (CHEF-4697)
 * knife now bootstraps node with the latest current version of chef-client. (CHEF-4911)
 * Add a threaded download queue for synchronizing cookbooks in parallel. (CHEF-4423)
+* Add config options for attribute whitelisting in node.save. (CHEF-3811)
 
 ## Last Release: 11.12.0 RC1 (03/31/2014)
 * SIGTERM will once-more kill a non-daemonized chef-client (CHEF-5172)
diff --git a/DOC_CHANGES.md b/DOC_CHANGES.md
index 00d15f7b18..e288c71972 100644
--- a/DOC_CHANGES.md
+++ b/DOC_CHANGES.md
@@ -37,3 +37,38 @@ You can now modify the chef-full template with the following options in `knife b
 ### Parallelize cookbook synchronization
 
 You can now synchronize your cookbooks faster by parallelizing the process. You can specify the number of helper threads in your config file with `cookbook_sync_threads NUM_THREADS`. The default is 10. Increasing `NUM_THREADS` can result in gateway errors from the chef server (namely 503 and 504). If you are experiencing these often, consider decreasing `NUM_THREADS` to fewer than default.
+
+### New chef config options: Whitelisting for the attributes saved by the node
+
+You can now whitelist attributes that will be saved by the node by providing a hash with the keys you want to include. Whitelist filters are described for each attribute level: `automatic_attribute_whitelist`, `default_attribute_whitelist`, `normal_attribute_whitelist`, and `override_attribute_whitelist`.
+
+If your automatic attribute data looks like
+````
+{
+  "filesystem" => {
+    "/dev/disk0s2" => {
+      "size" => "10mb"
+    },
+    "map - autohome" => {
+      "size" => "10mb"
+    }
+  },
+  "network" => {
+    "interfaces" => {
+      "eth0" => {...},
+      "eth1" => {...},
+    }
+  }
+}
+````
+and your config file looks like
+````
+automatic_attribute_whitelist = ["network/interfaces/eth0"]
+````
+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.
+
+The default behavior is for the node to save all the attribute data. This can be ensured by setting your whitelist filter to `nil`.
+
+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 35b07c24ea..66dd4cc77e 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -558,6 +558,15 @@ class Chef
     # the number of threads will help.
     default :cookbook_sync_threads, 10
 
+    # 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
+    default :normal_attribute_whitelist, nil
+    default :override_attribute_whitelist, nil
+
     # If installed via an omnibus installer, this gives the path to the
     # "embedded" directory which contains all of the software packaged with
     # omnibus. This is used to locate the cacert.pem file on windows.
diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 6061dbe615..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
@@ -520,18 +521,18 @@ class Chef
         if Chef::Config[:why_run]
           Chef::Log.warn("In whyrun mode, so NOT performing node save.")
         else
-          chef_server_rest.put_rest("nodes/#{name}", self)
+          chef_server_rest.put_rest("nodes/#{name}", data_for_save)
         end
       rescue Net::HTTPServerException => e
         raise e unless e.response.code == "404"
-        chef_server_rest.post_rest("nodes", self)
+        chef_server_rest.post_rest("nodes", data_for_save)
       end
       self
     end
 
     # Create the node via the REST API
     def create
-      chef_server_rest.post_rest("nodes", self)
+      chef_server_rest.post_rest("nodes", data_for_save)
       self
     end
 
@@ -543,5 +544,20 @@ class Chef
       self.name <=> other_node.name
     end
 
+    private
+
+    def data_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 => save everything
+          Chef::Log.info("Whitelisting #{level} node attributes for save.")
+          data[level] = Chef::Whitelist.filter(data[level], whitelist)
+        end
+      end
+      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/client_spec.rb b/spec/unit/client_spec.rb
index 6aa98843c0..36a0e9fc1b 100644
--- a/spec/unit/client_spec.rb
+++ b/spec/unit/client_spec.rb
@@ -253,9 +253,11 @@ describe Chef::Client do
       end
 
       def stub_for_node_save
+        node.stub(:data_for_save).and_return(node.for_json)
+
         # --Client#save_updated_node
         Chef::REST.should_receive(:new).with(Chef::Config[:chef_server_url]).and_return(http_node_save)
-        http_node_save.should_receive(:put_rest).with("nodes/#{fqdn}", node).and_return(true)
+        http_node_save.should_receive(:put_rest).with("nodes/#{fqdn}", node.for_json).and_return(true)
       end
 
       def stub_for_run
@@ -614,4 +616,3 @@ describe Chef::Client do
 
   end
 end
-
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index 832e10f645..21a978a9c9 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -814,22 +814,25 @@ describe Chef::Node do
     describe "save" do
       it "should update a node if it already exists" do
         node.name("monkey")
-        @rest.should_receive(:put_rest).with("nodes/monkey", node).and_return("foo")
+        node.stub(:data_for_save).and_return({})
+        @rest.should_receive(:put_rest).with("nodes/monkey", {}).and_return("foo")
         node.save
       end
 
       it "should not try and create if it can update" do
         node.name("monkey")
-        @rest.should_receive(:put_rest).with("nodes/monkey", node).and_return("foo")
+        node.stub(:data_for_save).and_return({})
+        @rest.should_receive(:put_rest).with("nodes/monkey", {}).and_return("foo")
         @rest.should_not_receive(:post_rest)
         node.save
       end
 
       it "should create if it cannot update" do
         node.name("monkey")
+        node.stub(:data_for_save).and_return({})
         exception = double("404 error", :code => "404")
         @rest.should_receive(:put_rest).and_raise(Net::HTTPServerException.new("foo", exception))
-        @rest.should_receive(:post_rest).with("nodes", node)
+        @rest.should_receive(:post_rest).with("nodes", {})
         node.save
       end
 
@@ -847,6 +850,73 @@ describe Chef::Node do
           node.save
         end
       end
+
+      context "with whitelisted attributes configured" do
+        it "should only save whitelisted attributes (and subattributes)" do
+          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")
+          node.stub(:for_json).and_return(data)
+          @rest.should_receive(:put_rest).with("nodes/picky-monkey", 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] = []
+
+          data = {
+            "automatic" => {
+              "filesystem" => {
+                "/dev/disk0s2"   => { "size" => "10mb" },
+                "map - autohome" => { "size" => "10mb" }
+              }
+            },
+            "default" => {}, "normal" => {}, "override" => {}
+          }
+
+          selected_data = {
+            "automatic" => {}, "default" => {}, "normal" => {}, "override" => {}
+          }
+
+          node.name("picky-monkey")
+          node.stub(:for_json).and_return(data)
+          @rest.should_receive(:put_rest).with("nodes/picky-monkey", selected_data).and_return("foo")
+          node.save
+        end
+      end
     end
   end