From de993648c7893863a209c2023b57e08c3f414dfc Mon Sep 17 00:00:00 2001
From: Michael Hashizume <michael.hashizume@puppet.com>
Date: Fri, 19 Apr 2024 11:11:46 -0700
Subject: [PATCH] Update validate_data_hash to return modified hash

Puppet::Pops::Lookup::ModuleDataProvider#validate_data_hash is a method
that is supposed to prune a data hash of any keys that are not prefixed
with a given module's name. However prior to this commit it incorrectly
took the data hash, cloned it, operated on the clone, then returned the
unchanged original hash.

This commit updates validate_data_hash to instead return the modified
hash, and updates the warning message generated when a key is pruned to
include the key.
---
 lib/puppet/pops/lookup/module_data_provider.rb |  3 +--
 spec/unit/functions/lookup_fixture_spec.rb     | 12 ++++++------
 spec/unit/pops/lookup/lookup_spec.rb           |  1 +
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/puppet/pops/lookup/module_data_provider.rb b/lib/puppet/pops/lookup/module_data_provider.rb
index bca3b3f2241..8f480e81f56 100644
--- a/lib/puppet/pops/lookup/module_data_provider.rb
+++ b/lib/puppet/pops/lookup/module_data_provider.rb
@@ -51,9 +51,8 @@ def validate_data_hash(data_hash)
       next memo if k == LOOKUP_OPTIONS || k.start_with?(module_prefix)
 
       msg = "#{yield} must use keys qualified with the name of the module"
-      memo = memo.clone if memo.equal?(data_hash)
       memo.delete(k)
-      Puppet.warning("Module '#{module_name}': #{msg}")
+      Puppet.warning("Module '#{module_name}': #{msg}; got #{k}")
       memo
     end
     data_hash
diff --git a/spec/unit/functions/lookup_fixture_spec.rb b/spec/unit/functions/lookup_fixture_spec.rb
index cff86df030f..8d4a2c25197 100644
--- a/spec/unit/functions/lookup_fixture_spec.rb
+++ b/spec/unit/functions/lookup_fixture_spec.rb
@@ -377,7 +377,7 @@ def compile_and_get_notifications(code)
         expect { compiler.compile }.to raise_error(Puppet::ParseError, /did not find a value for the name 'bad_data::b'/)
       end
       warnings = logs.select { |log| log.level == :warning }.map { |log| log.message }
-      expect(warnings).to include("Module 'bad_data': Value returned from deprecated API function 'bad_data::data' must use keys qualified with the name of the module")
+      expect(warnings).to include("Module 'bad_data': Value returned from deprecated API function 'bad_data::data' must use keys qualified with the name of the module; got b")
     end
 
     it 'will succeed finding prefixed keys even when a key in the function provided module data is not prefixed' do
@@ -391,7 +391,7 @@ def compile_and_get_notifications(code)
         expect(resources).to include('module_c')
       end
       warnings = logs.select { |log| log.level == :warning }.map { |log| log.message }
-      expect(warnings).to include("Module 'bad_data': Value returned from deprecated API function 'bad_data::data' must use keys qualified with the name of the module")
+      expect(warnings).to include("Module 'bad_data': Value returned from deprecated API function 'bad_data::data' must use keys qualified with the name of the module; got b")
     end
 
     it 'will resolve global, environment, and module correctly' do
@@ -422,8 +422,8 @@ def compile_and_get_notifications(code)
       Puppet::Util::Log.with_destination(Puppet::Test::LogCollector.new(logs)) do
         compiler.compile
       end
-      warnings = logs.select { |log| log.level == :warning }.map { |log| log.message }
-      expect(warnings).to include("Module 'bad_data': Value returned from deprecated API function 'bad_data::data' must use keys qualified with the name of the module")
+      warnings = logs.filter_map { |log| log.message if log.level == :warning }
+      expect(warnings).to include("Module 'bad_data': Value returned from deprecated API function 'bad_data::data' must use keys qualified with the name of the module; got b")
     end
 
     it 'a warning will be logged when key in the hiera provided module data is not prefixed' do
@@ -432,8 +432,8 @@ def compile_and_get_notifications(code)
       Puppet::Util::Log.with_destination(Puppet::Test::LogCollector.new(logs)) do
         compiler.compile
       end
-      warnings = logs.select { |log| log.level == :warning }.map { |log| log.message }
-      expect(warnings).to include("Module 'hieraprovider': Value returned from data_hash function 'json_data', when using location '#{environmentpath}/production/modules/hieraprovider/data/first.json', must use keys qualified with the name of the module")
+      warnings = logs.filter_map { |log| log.message if log.level == :warning }
+      expect(warnings).to include("Module 'hieraprovider': Value returned from data_hash function 'json_data', when using location '#{environmentpath}/production/modules/hieraprovider/data/first.json', must use keys qualified with the name of the module; got test::param_b")
     end
   end
 
diff --git a/spec/unit/pops/lookup/lookup_spec.rb b/spec/unit/pops/lookup/lookup_spec.rb
index 3e7b6de29b6..6cacbc1dcf5 100644
--- a/spec/unit/pops/lookup/lookup_spec.rb
+++ b/spec/unit/pops/lookup/lookup_spec.rb
@@ -28,6 +28,7 @@ module Lookup
           a: a (from global)
           d: d (from global)
           mod::e: mod::e (from global)
+          other::y: should be removed
           YAML
       }
     }