[confmap] fix panic when assigning nil maps (#13118)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Do not panic on assigning nil maps to non-nil maps

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes #13117

<!--Describe what testing was performed and which tests were added.-->
#### Testing
unit tests

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
This commit is contained in:
Antoine Toulme 2025-06-03 01:54:01 -07:00 committed by GitHub
parent e45c0e524c
commit 72878a1e97
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 99 additions and 2 deletions

View File

@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: confmap
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Do not panic on assigning nil maps to non-nil maps
# One or more tracking issues or pull requests related to the change
issues: [13117]
# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []

View File

@ -261,7 +261,7 @@ func decodeConfig(m *Conf, result any, errorUnused bool, skipTopLevelUnmarshaler
// after the main unmarshaler hook is called,
// we unmarshal the embedded structs if present to merge with the result:
unmarshalerEmbeddedStructsHookFunc(),
zeroSliceHookFunc(),
zeroSliceAndMapHookFunc(),
),
}
decoder, err := mapstructure.NewDecoder(dc)
@ -595,7 +595,12 @@ type Marshaler interface {
//
// 4. configuration have no `keys` field specified, the output should be default config
// - for example, input is {}, then output is Config{ Keys: ["a", "b"]}
func zeroSliceHookFunc() mapstructure.DecodeHookFuncValue {
//
// This hook is also used to solve https://github.com/open-telemetry/opentelemetry-collector/issues/13117.
// Since v0.127.0, we decode nil values to avoid creating empty map objects.
// The nil value is not well understood when layered on top of a default map non-nil value.
// The fix is to avoid the assignment and return the previous value.
func zeroSliceAndMapHookFunc() mapstructure.DecodeHookFuncValue {
return safeWrapDecodeHookFunc(func(from reflect.Value, to reflect.Value) (any, error) {
if to.CanSet() && to.Kind() == reflect.Slice && from.Kind() == reflect.Slice {
if !from.IsNil() {
@ -603,6 +608,11 @@ func zeroSliceHookFunc() mapstructure.DecodeHookFuncValue {
to.Set(reflect.MakeSlice(to.Type(), from.Len(), from.Cap()))
}
}
if to.CanSet() && to.Kind() == reflect.Map && from.Kind() == reflect.Map {
if from.IsNil() {
return to.Interface(), nil
}
}
return from.Interface(), nil
})

View File

@ -1089,3 +1089,65 @@ func TestConfDelete(t *testing.T) {
})
}
}
type structWithConfigOpaqueMap struct {
Headers map[string]string `mapstructure:"headers"`
}
func TestMapMerge(t *testing.T) {
tests := []struct {
name string
initial map[string]string
added map[string]string
expected map[string]string
}{
{
name: "both nil",
initial: nil,
added: nil,
expected: nil,
},
{
name: "nil map",
initial: map[string]string{},
added: nil,
expected: map[string]string{},
},
{
name: "initialized",
initial: map[string]string{
"foo": "bar",
},
added: nil,
expected: map[string]string{
"foo": "bar",
},
},
{
name: "both",
initial: map[string]string{
"foo": "bar",
},
added: map[string]string{
"foobar": "bar",
},
expected: map[string]string{
"foo": "bar",
"foobar": "bar",
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
s := structWithConfigOpaqueMap{
Headers: test.initial,
}
c := NewFromStringMap(map[string]any{
"headers": test.added,
})
require.NoError(t, c.Unmarshal(&s))
assert.Equal(t, test.expected, s.Headers)
})
}
}