Masterminds / sprig

Useful template functions for Go templates.
http://masterminds.github.io/sprig/
MIT License
4.07k stars 423 forks source link

Merging a dict over other types fails silently #394

Open forquare opened 4 months ago

forquare commented 4 months ago

Hi 👋

I'm seeing this issue in a Helm Library Chart, my helm version is:

version.BuildInfo{Version:"v3.12.1", GitCommit:"f32a527a060157990e2aa86bf45010dfb3cc8b8d", GitTreeState:"clean", GoVersion:"go1.20.4"}

However, I believe the issue is likely in spring in how it uses mergo (note this issue over on mergo: Mergo not overriding destination with source).

I use mustMergeOverwrite to merge two structures of unknown contents - i.e. the values are provided once the chart is in use and I won't know the structures contents when I'm writing the library chart.

I have noticed that in most cases I get the desired result: values in the source overwrite values in the destination even if the type changes.

By "even if the type changes" I mean that if a key in the destination has a string value, but in the source it has an int value, the source gets precedence and the result of the merge is that they key now has the int value from the source.

However, this is not the case if the key in the destination is any type other than a map and the key in the source is a map. In this case the result is that the destination has precedence and the value to the key remains as specified in the source.

While this breaks things for my users on occasion, I can usually spot this and we work around the problem. While I would love the behaviour to be changed such that merging a dict over another type results in the value being a dict, a big part of the problem at the moment is that this happens silently - I would expect an error at the least.

I have an extremely basic example over at helm playground which demonstrates the issue.
Note that all "toDict" values in the dest dict retain their destination values in the output while all of the other merges happen as expected.

I'm struggling to see someone relying on this specific behaviour, and it seems like a bug to me.