Merge pull request #12051 from chef/collector_16

Support attribute block/allow list in data collector

5 files changed, 73 insertions, 22 deletions

diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index 2f261b45a7..73b2246058 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -1007,7 +1007,7 @@ module ChefConfig
     default :blocked_normal_attributes, nil
     default :blocked_override_attributes, nil
 
-    # deprecated config options that will be removed in Chef Infra Client 17
+    # deprecated config options that will be removed in Chef Infra Client 18
     default :automatic_attribute_blacklist, nil
     default :default_attribute_blacklist, nil
     default :normal_attribute_blacklist, nil
diff --git a/lib/chef/data_collector/run_end_message.rb b/lib/chef/data_collector/run_end_message.rb
index 1900effa26..91cf21e643 100644
--- a/lib/chef/data_collector/run_end_message.rb
+++ b/lib/chef/data_collector/run_end_message.rb
@@ -51,7 +51,7 @@ class Chef
             "id" => run_status&.run_id,
             "message_version" => "1.1.0",
             "message_type" => "run_converge",
-            "node" => node || {},
+            "node" => node&.data_for_save || {},
             "node_name" => node&.name || data_collector.node_name,
             "organization_name" => organization,
             "resources" => all_action_records(action_collection),
diff --git a/lib/chef/deprecated.rb b/lib/chef/deprecated.rb
index c5466be92e..d2d851eda8 100644
--- a/lib/chef/deprecated.rb
+++ b/lib/chef/deprecated.rb
@@ -255,6 +255,10 @@ class Chef
       target 35
     end
 
+    class AttributeWhitelistConfiguration < Base
+      target 34
+    end
+
     class Generic < Base
       def url
         "https://docs.chef.io/chef_deprecations_client/"
diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index d569eeda38..cb09164001 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -687,6 +687,25 @@ class Chef
       name <=> other.name
     end
 
+    # Returns hash of node data with attributes based on whitelist/blacklist rules.
+    def data_for_save
+      data = for_json
+      %w{automatic default normal override}.each do |level|
+        allowlist = allowlist_or_whitelist_config(level)
+        unless allowlist.nil? # nil => save everything
+          logger.info("Allowing #{level} node attributes for save.")
+          data[level] = Chef::AttributeAllowlist.filter(data[level], allowlist)
+        end
+
+        blocklist = blocklist_or_blacklist_config(level)
+        unless blocklist.nil? # nil => remove nothing
+          logger.info("Blocking #{level} node attributes for save")
+          data[level] = Chef::AttributeBlocklist.filter(data[level], blocklist)
+        end
+      end
+      data
+    end
+
     private
 
     def save_without_policyfile_attrs
@@ -712,7 +731,7 @@ class Chef
     # @param [String] level the attribute level
     def allowlist_or_whitelist_config(level)
       if Chef::Config["#{level}_attribute_whitelist".to_sym]
-        Chef.deprecated(:attribute_blacklist_configuration, "Attribute whitelist configurations have been deprecated. Use the allowed_LEVEL_attribute configs instead")
+        Chef.deprecated(:attribute_whitelist_configuration, "Attribute whitelist configurations have been deprecated. Use the allowed_LEVEL_attribute configs instead")
         Chef::Config["#{level}_attribute_whitelist".to_sym]
       else
         Chef::Config["allowed_#{level}_attributes".to_sym]
@@ -732,24 +751,6 @@ class Chef
       end
     end
 
-    def data_for_save
-      data = for_json
-      %w{automatic default normal override}.each do |level|
-        allowlist = allowlist_or_whitelist_config(level)
-        unless allowlist.nil? # nil => save everything
-          logger.info("Allowing #{level} node attributes for save.")
-          data[level] = Chef::AttributeAllowlist.filter(data[level], allowlist)
-        end
-
-        blocklist = blocklist_or_blacklist_config(level)
-        unless blocklist.nil? # nil => remove nothing
-          logger.info("Blocking #{level} node attributes for save")
-          data[level] = Chef::AttributeBlocklist.filter(data[level], blocklist)
-        end
-      end
-      data
-    end
-
     # Returns a UUID that uniquely identifies this node for reporting reasons.
     #
     # The node is read in from disk if it exists, or it's generated if it does
diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb
index 63531663ee..24f8807d2e 100644
--- a/spec/unit/data_collector_spec.rb
+++ b/spec/unit/data_collector_spec.rb
@@ -142,11 +142,17 @@ describe Chef::DataCollector do
   def expect_converge_message(keys)
     keys["message_type"] = "run_converge"
     keys["message_version"] = "1.1.0"
+    # if (keys.key?("node") && !keys["node"].empty?)
+    #   expect(rest_client).to receive(:post) do |_a, hash, _b|
+    #     require 'pry'; binding.pry
+    #   end
+    # else
     expect(rest_client).to receive(:post).with(
       nil,
       hash_including(keys),
       { "Content-Type" => "application/json" }
     )
+    # end
   end
 
   def resource_has_diff(new_resource, status)
@@ -202,7 +208,7 @@ describe Chef::DataCollector do
     end
 
     it "has a node" do
-      expect_converge_message("node" => expected_node)
+      expect_converge_message("node" => expected_node.is_a?(Chef::Node) ? expected_node.data_for_save : expected_node)
       send_run_failed_or_completed_event
     end
 
@@ -808,6 +814,46 @@ describe Chef::DataCollector do
         it_behaves_like "sends a converge message"
       end
 
+      context "when node attributes are block-listed" do
+        let(:status) { "success" }
+        before do
+          Chef::Config[:blocked_default_attributes] = [
+            %w{secret key_to_the_kingdom},
+          ]
+          node.default = {
+            "secret" => { "key_to_the_kingdom" => "under the flower pot to the left of the drawbridge" },
+            "publicinfo" => { "num_flower_pots" => 18 },
+          }
+        end
+
+        it "payload should exclude blocked attributes" do
+          expect(rest_client).to receive(:post) do |_addr, hash, _headers|
+            expect(hash["node"]["default"]).to eq({ "secret" => {}, "publicinfo" => { "num_flower_pots" => 18 } })
+          end
+          send_run_failed_or_completed_event
+        end
+      end
+
+      context "when node attributes are allow-listed" do
+        let(:status) { "success" }
+        before do
+          Chef::Config[:allowed_default_attributes] = [
+            %w{public entrance},
+          ]
+          node.default = {
+            "public" => { "entrance" => "is the drawbridge" },
+            "secret" => { "entrance" => "is the tunnel" },
+          }
+        end
+
+        it "payload should include only allowed attributes" do
+          expect(rest_client).to receive(:post) do |_addr, hash, _headers|
+            expect(hash["node"]["default"]).to eq({ "public" => { "entrance" => "is the drawbridge" } })
+          end
+          send_run_failed_or_completed_event
+        end
+      end
+
     end
   end