Skip to content

Commit

Permalink
Make the option WithErrorUnused enabled by default when unmarshalin…
Browse files Browse the repository at this point in the history
…g configuration (#9154)

**Description:**
Make the option `WithErrorUnused` enabled by default when unmarshaling
configuration
The option `WithErrorUnused` is now enabled by default, and a new option
`WithIgnoreUnused` is introduced to ignore
errors about unused fields.

**Link to tracking Issue:**
This relates to #7102 to some extent.

**Testing:**
N/A

**Documentation:**
N/A
  • Loading branch information
atoulme committed Dec 22, 2023
1 parent 87ea438 commit 6155cc2
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 12 deletions.
27 changes: 27 additions & 0 deletions .chloggen/make_error_unused_default.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# 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: Make the option `WithErrorUnused` enabled by default when unmarshaling configuration

# One or more tracking issues or pull requests related to the change
issues: [7102]

# (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: |
The option `WithErrorUnused` is now enabled by default, and a new option `WithIgnoreUnused` is introduced to ignore
errors about unused fields.
# 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: []
2 changes: 1 addition & 1 deletion component/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func UnmarshalConfig(conf *confmap.Conf, intoCfg Config) error {
return cu.Unmarshal(conf)
}

return conf.Unmarshal(intoCfg, confmap.WithErrorUnused())
return conf.Unmarshal(intoCfg)
}

// ConfigValidator defines an optional interface for configurations to implement to do validation.
Expand Down
18 changes: 14 additions & 4 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,25 @@ type UnmarshalOption interface {
}

type unmarshalOption struct {
errorUnused bool
ignoreUnused 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).
// (extra keys). This option is enabled by default and can be disabled with `WithIgnoreUnused`.
// Deprecated: this is now enabled by default. Use `WithIgnoreUnused` to disable.
func WithErrorUnused() UnmarshalOption {
return unmarshalOptionFunc(func(uo *unmarshalOption) {
uo.errorUnused = true
uo.ignoreUnused = false
})
}

// WithIgnoreUnused sets an option to ignore errors if existing
// keys in the original Conf were unused in the decoding process
// (extra keys).
func WithIgnoreUnused() UnmarshalOption {
return unmarshalOptionFunc(func(uo *unmarshalOption) {
uo.ignoreUnused = true
})
}

Expand All @@ -76,7 +86,7 @@ func (l *Conf) Unmarshal(result any, opts ...UnmarshalOption) error {
for _, opt := range opts {
opt.apply(&set)
}
return decodeConfig(l, result, set.errorUnused)
return decodeConfig(l, result, !set.ignoreUnused)
}

type marshalOption struct{}
Expand Down
3 changes: 2 additions & 1 deletion confmap/confmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ func TestUnmarshalWithErrorUnused(t *testing.T) {
"string": "this is a string",
}
conf := NewFromStringMap(stringMap)
assert.Error(t, conf.Unmarshal(&TestIDConfig{}, WithErrorUnused()))
assert.Error(t, conf.Unmarshal(&TestIDConfig{}))
assert.NoError(t, conf.Unmarshal(&TestIDConfig{}, WithIgnoreUnused()))
}

type TestConfig struct {
Expand Down
2 changes: 1 addition & 1 deletion exporter/loggingexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
return fmt.Errorf("'loglevel' and 'verbosity' are incompatible. Use only 'verbosity' instead")
}

if err := conf.Unmarshal(cfg, confmap.WithErrorUnused()); err != nil {
if err := conf.Unmarshal(cfg); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion otelcol/internal/configunmarshaler/configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func NewConfigs[F component.Factory](factories map[component.Type]F) *Configs[F]

func (c *Configs[F]) Unmarshal(conf *confmap.Conf) error {
rawCfgs := make(map[component.ID]map[string]any)
if err := conf.Unmarshal(&rawCfgs, confmap.WithErrorUnused()); err != nil {
if err := conf.Unmarshal(&rawCfgs); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion otelcol/unmarshaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) {
},
}

return cfg, v.Unmarshal(&cfg, confmap.WithErrorUnused())
return cfg, v.Unmarshal(&cfg)
}
4 changes: 2 additions & 2 deletions otelcol/unmarshaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func TestPipelineConfigUnmarshalError(t *testing.T) {
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
pips := new(pipelines.Config)
err := tt.conf.Unmarshal(&pips, confmap.WithErrorUnused())
err := tt.conf.Unmarshal(&pips)
require.Error(t, err)
assert.Contains(t, err.Error(), tt.expectError)
})
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestServiceUnmarshalError(t *testing.T) {

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
err := tt.conf.Unmarshal(&service.Config{}, confmap.WithErrorUnused())
err := tt.conf.Unmarshal(&service.Config{})
require.Error(t, err)
assert.Contains(t, err.Error(), tt.expectError)
})
Expand Down
2 changes: 1 addition & 1 deletion receiver/otlpreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (cfg *Config) Validate() error {
// Unmarshal a confmap.Conf into the config struct.
func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
// first load the config normally
err := conf.Unmarshal(cfg, confmap.WithErrorUnused())
err := conf.Unmarshal(cfg)
if err != nil {
return err
}
Expand Down

0 comments on commit 6155cc2

Please sign in to comment.