Skip to content

Commit 3b7d066

Browse files
mx-psievan-bradley
andauthored
[configoptional] Add a confmap.Marshaler implementation (#13196)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adds a `confmap.Marshaler` implementation for `configoptional.Optional` --------- Co-authored-by: Evan Bradley <[email protected]>
1 parent e7b140f commit 3b7d066

File tree

7 files changed

+191
-5
lines changed

7 files changed

+191
-5
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: confmap
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Distinguish between empty and nil values when marshaling `confmap.Conf` structs."
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13196]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [api]
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: configoptional
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Add a confmap.Marshaler implementation for configoptional.Optional"
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13196]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [api]

config/configoptional/optional.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,28 @@ func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error {
164164
o.flavor = someFlavor
165165
return nil
166166
}
167+
168+
var _ confmap.Marshaler = (*Optional[any])(nil)
169+
170+
// Marshal the Optional value into the configuration.
171+
// If the Optional is None or Default, it does not marshal anything.
172+
// If the Optional is Some, it marshals the value into the configuration.
173+
//
174+
// T must be derefenceable to a type with struct kind.
175+
// Scalar values are not supported.
176+
func (o Optional[T]) Marshal(conf *confmap.Conf) error {
177+
if err := assertStructKind[T](); err != nil {
178+
return err
179+
}
180+
181+
if o.flavor == noneFlavor || o.flavor == defaultFlavor {
182+
// Optional is None or Default, do not marshal anything.
183+
return conf.Marshal(map[string]any(nil))
184+
}
185+
186+
if err := conf.Marshal(o.value); err != nil {
187+
return fmt.Errorf("configoptional: failed to marshal Optional value: %w", err)
188+
}
189+
190+
return nil
191+
}

config/configoptional/optional_test.go

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package configoptional
55

66
import (
7+
"fmt"
78
"testing"
89

910
"github.com/stretchr/testify/assert"
@@ -339,10 +340,11 @@ func confFromYAML(t *testing.T, yaml string) *confmap.Conf {
339340
return conf
340341
}
341342

342-
func TestComparePointer(t *testing.T) {
343+
func TestComparePointerUnmarshal(t *testing.T) {
343344
tests := []struct {
344345
yaml string
345346
}{
347+
{yaml: ""},
346348
{yaml: "sub: "},
347349
{yaml: "sub: null"},
348350
{yaml: "sub: {}"},
@@ -365,3 +367,96 @@ func TestComparePointer(t *testing.T) {
365367
})
366368
}
367369
}
370+
371+
func TestOptionalMarshal(t *testing.T) {
372+
tests := []struct {
373+
name string
374+
value Config[Sub]
375+
expected map[string]any
376+
}{
377+
{
378+
name: "none (zero value)",
379+
value: Config[Sub]{},
380+
expected: map[string]any{"sub": nil},
381+
},
382+
{
383+
name: "none",
384+
value: Config[Sub]{Sub1: None[Sub]()},
385+
expected: map[string]any{"sub": nil},
386+
},
387+
{
388+
name: "default",
389+
value: Config[Sub]{Sub1: Default(subDefault)},
390+
expected: map[string]any{"sub": nil},
391+
},
392+
{
393+
name: "some",
394+
value: Config[Sub]{Sub1: Some(Sub{
395+
Foo: "bar",
396+
})},
397+
expected: map[string]any{
398+
"sub": map[string]any{
399+
"foo": "bar",
400+
},
401+
},
402+
},
403+
}
404+
405+
for _, test := range tests {
406+
t.Run(test.name, func(t *testing.T) {
407+
conf := confmap.New()
408+
require.NoError(t, conf.Marshal(test.value))
409+
assert.Equal(t, test.expected, conf.ToStringMap())
410+
})
411+
}
412+
}
413+
414+
func TestComparePointerMarshal(t *testing.T) {
415+
type Wrap[T any] struct {
416+
// Note: passes without requiring "squash".
417+
Sub1 T `mapstructure:"sub"`
418+
}
419+
420+
type WrapOmitEmpty[T any] struct {
421+
// Note: passes without requiring "squash", except with Default-flavored Optional values.
422+
Sub1 T `mapstructure:"sub,omitempty"`
423+
}
424+
425+
tests := []struct {
426+
pointer *Sub
427+
optional Optional[Sub]
428+
skipOmitEmpty bool
429+
}{
430+
{pointer: nil, optional: None[Sub]()},
431+
{pointer: nil, optional: Default(subDefault), skipOmitEmpty: true}, // does not work with omitempty
432+
{pointer: &Sub{Foo: "bar"}, optional: Some(Sub{Foo: "bar"})},
433+
}
434+
for _, test := range tests {
435+
t.Run(fmt.Sprintf("%v vs %v", test.pointer, test.optional), func(t *testing.T) {
436+
wrapPointer := Wrap[*Sub]{Sub1: test.pointer}
437+
confPointer := confmap.NewFromStringMap(nil)
438+
require.NoError(t, confPointer.Marshal(wrapPointer))
439+
440+
wrapOptional := Wrap[Optional[Sub]]{Sub1: test.optional}
441+
confOptional := confmap.NewFromStringMap(nil)
442+
require.NoError(t, confOptional.Marshal(wrapOptional))
443+
444+
assert.Equal(t, confPointer.ToStringMap(), confOptional.ToStringMap())
445+
})
446+
447+
if test.skipOmitEmpty {
448+
continue
449+
}
450+
t.Run(fmt.Sprintf("%v vs %v (omitempty)", test.pointer, test.optional), func(t *testing.T) {
451+
wrapPointer := WrapOmitEmpty[*Sub]{Sub1: test.pointer}
452+
confPointer := confmap.NewFromStringMap(nil)
453+
require.NoError(t, confPointer.Marshal(wrapPointer))
454+
455+
wrapOptional := WrapOmitEmpty[Optional[Sub]]{Sub1: test.optional}
456+
confOptional := confmap.NewFromStringMap(nil)
457+
require.NoError(t, confOptional.Marshal(wrapOptional))
458+
459+
assert.Equal(t, confPointer.ToStringMap(), confOptional.ToStringMap())
460+
})
461+
}
462+
}

confmap/confmap.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,11 +580,22 @@ func marshalerHookFunc(orig any) mapstructure.DecodeHookFuncValue {
580580
if !ok {
581581
return from.Interface(), nil
582582
}
583-
conf := New()
583+
conf := NewFromStringMap(nil)
584584
if err := marshaler.Marshal(conf); err != nil {
585585
return nil, err
586586
}
587-
return conf.ToStringMap(), nil
587+
588+
stringMap := conf.ToStringMap()
589+
if stringMap == nil {
590+
// If conf is still nil after marshaling, we want to encode it as an untyped nil
591+
// instead of a map-typed nil. This ensures the value is a proper null value
592+
// in the final marshaled output instead of an empty map. We hit this case
593+
// when marshaling wrapper structs that have no direct representation
594+
// in the marshaled output that aren't tagged with "squash" on the fields
595+
// they're used on.
596+
return nil, nil
597+
}
598+
return stringMap, nil
588599
})
589600
}
590601

confmap/internal/mapstructure/encoder.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,12 @@ func (e *Encoder) encodeMap(value reflect.Value) (any, error) {
165165
Kind: value.Kind(),
166166
}
167167
}
168-
result := make(map[string]any)
168+
169+
var result map[string]any
170+
if value.Len() > 0 || !value.IsNil() {
171+
result = make(map[string]any)
172+
}
173+
169174
iterator := value.MapRange()
170175
for iterator.Next() {
171176
encoded, err := e.encode(iterator.Key())

service/config_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestConfmapMarshalConfig(t *testing.T) {
8787
Telemetry: defaultTelConfig,
8888
}))
8989
assert.Equal(t, map[string]any{
90-
"pipelines": map[string]any{},
90+
"pipelines": map[string]any(nil),
9191
"telemetry": map[string]any{
9292
"logs": map[string]any{
9393
"encoding": "console",

0 commit comments

Comments
 (0)