Skip to content

Commit

Permalink
Handle empty maps properly using old logic (not explicitly set) and n…
Browse files Browse the repository at this point in the history
…ew logic (declared as {})
  • Loading branch information
paul1r committed Nov 28, 2024
1 parent 4ca0a18 commit 7c33142
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 5 deletions.
44 changes: 40 additions & 4 deletions pkg/validation/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,18 @@ func (l *Limits) UnmarshalYAML(unmarshal func(interface{}) error) error {
return errors.Wrap(err, "cloning limits (unmarshaling)")
}
}

// Check if we're getting an empty map for RulerRemoteWriteHeaders
var raw map[string]interface{}
if err := unmarshal(&raw); err != nil {
return err
}

if v, ok := raw["ruler_remote_write_headers"]; ok && v == nil {
l.RulerRemoteWriteHeaders = OverwriteMarshalingStringMap{m: nil}
}

// Now unmarshal the full config
if err := unmarshal((*plain)(l)); err != nil {
return err
}
Expand Down Expand Up @@ -1205,14 +1217,38 @@ func (sm OverwriteMarshalingStringMap) MarshalYAML() (interface{}, error) {
return sm.m, nil
}

// UnmarshalYAML implements yaml.Unmarshaler.
func (sm *OverwriteMarshalingStringMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
var def map[string]string
var raw interface{}
if err := unmarshal(&raw); err != nil {
return err
}

err := unmarshal(&def)
if err != nil {
// If we get a nil value or empty map, set a nil map
if raw == nil {
sm.m = nil
return nil
}

// Check for empty map in different YAML representations
switch v := raw.(type) {
case map[interface{}]interface{}:
if len(v) == 0 {
sm.m = nil
return nil
}
case map[string]interface{}:
if len(v) == 0 {
sm.m = nil
return nil
}
}

// Otherwise try to unmarshal as a map
var def map[string]string
if err := unmarshal(&def); err != nil {
return err
}
sm.m = def

return nil
}
20 changes: 19 additions & 1 deletion pkg/validation/limits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,24 @@ ruler_remote_write_headers:
desc: "empty map overrides defaults",
yaml: `
ruler_remote_write_headers:
`,
exp: Limits{
DiscoverServiceName: []string{},
LogLevelFields: []string{},
// Rest from new defaults
StreamRetention: []StreamRetention{
{
Period: model.Duration(24 * time.Hour),
Selector: `{a="b"}`,
},
},
OTLPConfig: defaultOTLPConfig,
},
},
{
desc: "explicitly set empty map overrides defaults",
yaml: `
ruler_remote_write_headers: {} # Explicitly set an empty map
`,
exp: Limits{
DiscoverServiceName: []string{},
Expand Down Expand Up @@ -318,7 +336,7 @@ query_timeout: 5m
var out Limits
decoder := yaml.NewDecoder(bytes.NewReader([]byte(tc.yaml)))
decoder.KnownFields(true)
err := decoder.Decode(out)
err := decoder.Decode(&out)
require.NoError(t, err)
require.Equal(t, tc.exp, out)
})
Expand Down

0 comments on commit 7c33142

Please sign in to comment.