Skip to content

Commit

Permalink
cmd/icingadb: warn about unknown config options instead of rejecting …
Browse files Browse the repository at this point in the history
…them

This makes the change from #605 more suitable for the v1.1.1 bug fix release
because this way, no new fatal errors are added while keeping the helpful
information in a warning.
  • Loading branch information
julianbrost committed Aug 8, 2023
1 parent 401d086 commit d11f03d
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 26 deletions.
4 changes: 4 additions & 0 deletions cmd/icingadb/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ func run() int {
logger := logs.GetLogger()
defer logger.Sync()

if warn := cmd.Config.DecodeWarning; warn != nil {
logger.Warnf("ignoring unknown config option, this will become a fatal error in Icinga DB v1.2:\n\n%v", warn)
}

logger.Info("Starting Icinga DB")

db, err := cmd.Database(logs.GetChildLogger("database"))
Expand Down
21 changes: 16 additions & 5 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package config

import (
"bytes"
"crypto/tls"
"crypto/x509"
"fmt"
"github.com/creasty/defaults"
"github.com/goccy/go-yaml"
"github.com/jessevdk/go-flags"
Expand All @@ -17,6 +19,8 @@ type Config struct {
Redis Redis `yaml:"redis"`
Logging Logging `yaml:"logging"`
Retention Retention `yaml:"retention"`

DecodeWarning error `yaml:"-"`
}

// Validate checks constraints in the supplied configuration and returns an error if they are violated.
Expand Down Expand Up @@ -47,14 +51,13 @@ type Flags struct {

// FromYAMLFile returns a new Config value created from the given YAML config file.
func FromYAMLFile(name string) (*Config, error) {
f, err := os.Open(name)
f, err := os.ReadFile(name)
if err != nil {
return nil, errors.Wrap(err, "can't open YAML file "+name)
return nil, errors.Wrap(err, "can't read YAML file "+name)
}
defer f.Close()

c := &Config{}
d := yaml.NewDecoder(f, yaml.DisallowUnknownField())
d := yaml.NewDecoder(bytes.NewReader(f))

if err := defaults.Set(c); err != nil {
return nil, errors.Wrap(err, "can't set config defaults")
Expand All @@ -64,8 +67,16 @@ func FromYAMLFile(name string) (*Config, error) {
return nil, errors.Wrap(err, "can't parse YAML file "+name)
}

// Decode again with yaml.DisallowUnknownField() (like v1.2 will do) and issue a warning if it returns an error.
c.DecodeWarning = yaml.NewDecoder(bytes.NewReader(f), yaml.DisallowUnknownField()).Decode(&Config{})

if err := c.Validate(); err != nil {
return nil, errors.Wrap(err, "invalid configuration")
const msg = "invalid configuration"
if warn := c.DecodeWarning; warn != nil {
return nil, fmt.Errorf("%s: %w\n\nwarning: ignored unknown config option:\n\n%v", msg, err, warn)
} else {
return nil, errors.Wrap(err, msg)
}
}

return c, nil
Expand Down
50 changes: 29 additions & 21 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,33 +20,34 @@ redis:
host: 2001:db8::1
`

miniOutput := &Config{}
_ = defaults.Set(miniOutput)

miniOutput.Database.Host = "192.0.2.1"
miniOutput.Database.Database = "icingadb"
miniOutput.Database.User = "icingadb"
miniOutput.Database.Password = "icingadb"

miniOutput.Redis.Host = "2001:db8::1"
miniOutput.Logging.Output = logging.CONSOLE

subtests := []struct {
name string
input string
output *Config
warn bool
}{
{
name: "mini",
input: miniConf,
output: func() *Config {
c := &Config{}
_ = defaults.Set(c)

c.Database.Host = "192.0.2.1"
c.Database.Database = "icingadb"
c.Database.User = "icingadb"
c.Database.Password = "icingadb"

c.Redis.Host = "2001:db8::1"
c.Logging.Output = logging.CONSOLE

return c
}(),
name: "mini",
input: miniConf,
output: miniOutput,
warn: false,
},
{
name: "mini-with-unknown",
input: miniConf + "\nunknown: 42",
output: nil,
output: miniOutput,
warn: true,
},
}

Expand All @@ -58,12 +59,19 @@ redis:

require.NoError(t, os.WriteFile(tempFile.Name(), []byte(st.input), 0o600))

if actual, err := FromYAMLFile(tempFile.Name()); st.output == nil {
require.Error(t, err)
actual, err := FromYAMLFile(tempFile.Name())
require.NoError(t, err)

if st.warn {
require.Error(t, actual.DecodeWarning, "reading config should produce a warning")

// Reset the warning so that the following require.Equal() doesn't try to compare it.
actual.DecodeWarning = nil
} else {
require.NoError(t, err)
require.Equal(t, st.output, actual)
require.NoError(t, actual.DecodeWarning, "reading config should not produce a warning")
}

require.Equal(t, st.output, actual)
})
}
}

0 comments on commit d11f03d

Please sign in to comment.