Add options to confmap Marshal/Unmarshal, deprecat UnmarshalExact (#6231

)

Officially this can be considered breaking change because of a use in a `var foo func(interface{}) error = conf.Unmarshal`, not in a normal calling usage, but this is an edgecase that we can except and move forward with this.

Signed-off-by: Bogdan <[email protected]>

CHANGELOG.md

@@ -35,6 +35,7 @@
 - Deprecate `pcommon.Map.PutString` in favor of `pcommon.Map.PutStr` (#6210)
 - Deprecate `pcommon.NewValueString` in favor of `pcommon.NewValueStr` (#6209)
 - Deprecate `pmetric.MetricAggregationTemporality` enum type in favor of `pmetric.AggregationTemporality` (#6249)
+- Deprecate `confmap.Conf.UnmarshalExact` in favor of `confmap.Conf.Unmarshal(.., WithErrorUnused)` (#6231)
 
 ### 💡 Enhancements 💡
 

config/common.go

@@ -48,5 +48,5 @@ func unmarshal(componentSection *confmap.Conf, intoCfg interface{}) error {
 		return cu.Unmarshal(componentSection)
 	}
 
-	return componentSection.UnmarshalExact(intoCfg)
+	return componentSection.Unmarshal(intoCfg, confmap.WithErrorUnused())
 }

confmap/confmap.go

@@ -57,19 +57,52 @@ func (l *Conf) AllKeys() []string {
 	return l.k.Keys()
 }
 
-// Unmarshal unmarshalls the config into a struct.
+type UnmarshalOption interface {
+	apply(*unmarshalOption)
+}
+
+type unmarshalOption struct {
+	errorUnused bool
+}
+
+// WithErrorUnused sets an option to error when there are existing
+// keys in the original Conf that were unused in the decoding process
+// (extra keys).
+func WithErrorUnused() UnmarshalOption {
+	return unmarshalOptionFunc(func(uo *unmarshalOption) {
+		uo.errorUnused = true
+	})
+}
+
+type unmarshalOptionFunc func(*unmarshalOption)
+
+func (fn unmarshalOptionFunc) apply(set *unmarshalOption) {
+	fn(set)
+}
+
+// Unmarshal unmarshalls the config into a struct using the given options.
 // Tags on the fields of the structure must be properly set.
-func (l *Conf) Unmarshal(result interface{}) error {
-	return decodeConfig(l, result, false)
+func (l *Conf) Unmarshal(result interface{}, opts ...UnmarshalOption) error {
+	set := unmarshalOption{}
+	for _, opt := range opts {
+		opt.apply(&set)
+	}
+	return decodeConfig(l, result, set.errorUnused)
 }
 
-// UnmarshalExact unmarshalls the config into a struct, erroring if a field is nonexistent.
+// Deprecated: [v0.62.0] use Unmarshal.
 func (l *Conf) UnmarshalExact(result interface{}) error {
-	return decodeConfig(l, result, true)
+	return l.Unmarshal(result, WithErrorUnused())
+}
+
+type marshalOption struct{}
+
+type MarshalOption interface {
+	apply(*marshalOption)
 }
 
 // Marshal encodes the config and merges it into the Conf.
-func (l *Conf) Marshal(rawVal interface{}) error {
+func (l *Conf) Marshal(rawVal interface{}, _ ...MarshalOption) error {
 	enc := encoder.New(encoderConfig(rawVal))
 	data, err := enc.Encode(rawVal)
 	if err != nil {

confmap/confmap_test.go

@@ -118,7 +118,7 @@ func TestExpandNilStructPointersHookFunc(t *testing.T) {
 	conf := NewFromStringMap(stringMap)
 	cfg := &TestConfig{}
 	assert.Nil(t, cfg.Struct)
-	assert.NoError(t, conf.UnmarshalExact(cfg))
+	assert.NoError(t, conf.Unmarshal(cfg))
 	assert.Nil(t, cfg.Boolean)
 	// assert.False(t, *cfg.Boolean)
 	assert.Nil(t, cfg.Struct)
@@ -144,7 +144,7 @@ func TestExpandNilStructPointersHookFuncDefaultNotNilConfigNil(t *testing.T) {
 		Struct:    s1,
 		MapStruct: map[string]*Struct{"struct": s2},
 	}
-	assert.NoError(t, conf.UnmarshalExact(cfg))
+	assert.NoError(t, conf.Unmarshal(cfg))
 	assert.NotNil(t, cfg.Boolean)
 	assert.True(t, *cfg.Boolean)
 	assert.NotNil(t, cfg.Struct)
@@ -154,6 +154,15 @@ func TestExpandNilStructPointersHookFuncDefaultNotNilConfigNil(t *testing.T) {
 	assert.Equal(t, &Struct{}, cfg.MapStruct["struct"])
 }
 
+func TestUnmarshalWithErrorUnused(t *testing.T) {
+	stringMap := map[string]interface{}{
+		"boolean": true,
+		"string":  "this is a string",
+	}
+	conf := NewFromStringMap(stringMap)
+	assert.Error(t, conf.Unmarshal(&TestIDConfig{}, WithErrorUnused()))
+}
+
 type TestConfig struct {
 	Boolean   *bool              `mapstructure:"boolean"`
 	Struct    *Struct            `mapstructure:"struct"`
@@ -208,11 +217,6 @@ func TestMapKeyStringToMapKeyTextUnmarshalerHookFunc(t *testing.T) {
 	}
 	conf := NewFromStringMap(stringMap)
 
-	cfgExact := &TestIDConfig{}
-	assert.NoError(t, conf.UnmarshalExact(cfgExact))
-	assert.True(t, cfgExact.Boolean)
-	assert.Equal(t, map[TestID]string{"string": "this is a string"}, cfgExact.Map)
-
 	cfg := &TestIDConfig{}
 	assert.NoError(t, conf.Unmarshal(cfg))
 	assert.True(t, cfg.Boolean)
@@ -229,9 +233,6 @@ func TestMapKeyStringToMapKeyTextUnmarshalerHookFuncDuplicateID(t *testing.T) {
 	}
 	conf := NewFromStringMap(stringMap)
 
-	cfgExact := &TestIDConfig{}
-	assert.Error(t, conf.UnmarshalExact(cfgExact))
-
 	cfg := &TestIDConfig{}
 	assert.Error(t, conf.Unmarshal(cfg))
 }
@@ -245,9 +246,6 @@ func TestMapKeyStringToMapKeyTextUnmarshalerHookFuncErrorUnmarshal(t *testing.T)
 	}
 	conf := NewFromStringMap(stringMap)
 
-	cfgExact := &TestIDConfig{}
-	assert.Error(t, conf.UnmarshalExact(cfgExact))
-
 	cfg := &TestIDConfig{}
 	assert.Error(t, conf.Unmarshal(cfg))
 }
@@ -326,7 +324,7 @@ type testConfig struct {
 }
 
 func (tc *testConfig) Unmarshal(component *Conf) error {
-	if err := component.UnmarshalExact(tc); err != nil {
+	if err := component.Unmarshal(tc); err != nil {
 		return err
 	}
 	tc.Another += " is only called directly"
@@ -338,7 +336,7 @@ type nextConfig struct {
 }
 
 func (nc *nextConfig) Unmarshal(component *Conf) error {
-	if err := component.UnmarshalExact(nc); err != nil {
+	if err := component.Unmarshal(nc); err != nil {
 		return err
 	}
 	nc.String += " is called"
@@ -357,11 +355,6 @@ func TestUnmarshaler(t *testing.T) {
 	assert.NoError(t, cfgMap.Unmarshal(tc))
 	assert.Equal(t, "make sure this", tc.Another)
 	assert.Equal(t, "make sure this is called", tc.Next.String)
-
-	tce := &testConfig{}
-	assert.NoError(t, cfgMap.UnmarshalExact(tce))
-	assert.Equal(t, "make sure this", tce.Another)
-	assert.Equal(t, "make sure this is called", tce.Next.String)
 }
 
 func TestDirectUnmarshaler(t *testing.T) {
@@ -383,7 +376,7 @@ type testErrConfig struct {
 }
 
 func (tc *testErrConfig) Unmarshal(component *Conf) error {
-	return component.UnmarshalExact(tc)
+	return component.Unmarshal(tc)
 }
 
 type errConfig struct {
@@ -406,8 +399,4 @@ func TestUnmarshalerErr(t *testing.T) {
 	tc := &testErrConfig{}
 	assert.EqualError(t, cfgMap.Unmarshal(tc), expectErr)
 	assert.Empty(t, tc.Err.Foo)
-
-	tce := &testErrConfig{}
-	assert.EqualError(t, cfgMap.UnmarshalExact(tce), expectErr)
-	assert.Empty(t, tc.Err.Foo)
 }

receiver/otlpreceiver/config.go

@@ -61,7 +61,7 @@ func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
 		return errors.New("empty config for OTLP receiver")
 	}
 	// first load the config normally
-	err := componentParser.UnmarshalExact(cfg)
+	err := componentParser.Unmarshal(cfg, confmap.WithErrorUnused())
 	if err != nil {
 		return err
 	}

service/internal/configunmarshaler/defaultunmarshaler.go

@@ -92,7 +92,7 @@ func (ConfigUnmarshaler) Unmarshal(v *confmap.Conf, factories component.Factorie
 
 	// Unmarshal top level sections and validate.
 	rawCfg := configSettings{}
-	if err := v.UnmarshalExact(&rawCfg); err != nil {
+	if err := v.Unmarshal(&rawCfg, confmap.WithErrorUnused()); err != nil {
 		return nil, configError{
 			error: fmt.Errorf("error reading top level configuration sections: %w", err),
 			code:  errUnmarshalTopLevelStructure,
@@ -185,7 +185,7 @@ func unmarshalService(srvRaw map[string]interface{}) (config.Service, error) {
 		},
 	}
 
-	if err := confmap.NewFromStringMap(srvRaw).UnmarshalExact(&srv); err != nil {
+	if err := confmap.NewFromStringMap(srvRaw).Unmarshal(&srv, confmap.WithErrorUnused()); err != nil {
 		return srv, fmt.Errorf("error reading service configuration: %w", err)
 	}