From f8189e22d767ff2e5ccf5f906f5f88652627bb34 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Thu, 3 Sep 2020 12:43:58 -0700 Subject: Fix nil deep_merging While this behavior was fixed for attributes a long, long time ago it turns out we don't use this deep_merge mixin for constructing the deep merge of attribute precedence levels any more. This bug however surfaces because we still use deep_merge to merge json attributes into normal attributes. That means that it is not possible to pass a nil / null value in through -j and have it override a normal attribute which is pulled back in from the node object on the chef-server. So this fixes that behavior to be consistent with the behavior of attributes that we've had for several years now. Signed-off-by: Lamont Granquist --- lib/chef/mixin/deep_merge.rb | 12 ------------ spec/unit/mixin/deep_merge_spec.rb | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/chef/mixin/deep_merge.rb b/lib/chef/mixin/deep_merge.rb index 300ae1a31f..ad9c9e89dd 100644 --- a/lib/chef/mixin/deep_merge.rb +++ b/lib/chef/mixin/deep_merge.rb @@ -75,14 +75,7 @@ class Chef # @api private # def deep_merge!(source, dest) - # if dest doesn't exist, then simply copy source to it - if dest.nil? - dest = source; return dest - end - case source - when nil - dest when Hash if dest.is_a?(Hash) source.each do |src_key, src_value| @@ -147,11 +140,6 @@ class Chef end end merge_onto - - # If merge_with is nil, don't replace merge_onto - elsif merge_with.nil? - merge_onto - # In all other cases, replace merge_onto with merge_with else merge_with diff --git a/spec/unit/mixin/deep_merge_spec.rb b/spec/unit/mixin/deep_merge_spec.rb index 2122008616..c6681b3d16 100644 --- a/spec/unit/mixin/deep_merge_spec.rb +++ b/spec/unit/mixin/deep_merge_spec.rb @@ -236,6 +236,14 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do @dm.deep_merge!(hash_src, hash_dst) expect(hash_dst).to eq({ "item" => "orange" }) end + + it "should overwrite a string with a nil when merging nil values" do + hash_src = { "item" => nil } + hash_dst = { "item" => "orange" } + @dm.deep_merge!(hash_src, hash_dst) + expect(hash_dst).to eq({ "item" => nil }) + end + end # deep_merge! # Chef specific @@ -338,5 +346,12 @@ describe Chef::Mixin::DeepMerge do merge_with_hash = { "top_level_a" => 2, "top_level_b" => true } @dm.hash_only_merge(merge_ee_hash, merge_with_hash) end + + it "should overwrite a string with a nil when merging nil values" do + hash_src = { "item" => nil } + hash_dst = { "item" => "orange" } + merged_result = @dm.hash_only_merge(hash_dst, hash_src) + expect(merged_result).to eq({ "item" => nil }) + end end end -- cgit v1.2.1