Ensure extra config options are properly initialized

YAML is decoded by the structure of the YAML source document, not the Go
destination data structure. Therefore, the old code did not always call
UnmarshalYAML() on all sub-structs. Therefore, defaults were not always set but
zero values were used, resulting in all kind of strange behavior.

This commit changes the code so that it no longer relies on individual
UnmarshalYAML() functions to set the defaults for each sub-struct but instead
just sets all of them when creating the surrounding Config instance. It also
moves the config validation to separate Validate() functions.
This commit is contained in:
Julian Brost 2021-08-25 16:57:11 +02:00
parent a3875d82d2
commit be9054628a
5 changed files with 34 additions and 27 deletions

View file

@ -1,6 +1,7 @@
package config
import (
"github.com/creasty/defaults"
"github.com/jessevdk/go-flags"
"github.com/pkg/errors"
"gopkg.in/yaml.v3"
@ -13,6 +14,17 @@ type Config struct {
Redis Redis `yaml:"redis"`
}
// Validate checks constraints in the supplied configuration and returns an error if they are violated.
func (c *Config) Validate() error {
if err := c.Database.Validate(); err != nil {
return err
}
if err := c.Redis.Validate(); err != nil {
return err
}
return nil
}
// Flags defines CLI flags.
type Flags struct {
// Version decides whether to just print the version and exit.
@ -32,10 +44,18 @@ func FromYAMLFile(name string) (*Config, error) {
c := &Config{}
d := yaml.NewDecoder(f)
if err := defaults.Set(c); err != nil {
return nil, errors.Wrap(err, "can't set config defaults")
}
if err := d.Decode(c); err != nil {
return nil, errors.Wrap(err, "can't parse YAML file "+name)
}
if err := c.Validate(); err != nil {
return nil, errors.Wrap(err, "invalid configuration")
}
return c, nil
}

View file

@ -49,3 +49,8 @@ func (d *Database) Open(logger *zap.SugaredLogger) (*icingadb.DB, error) {
return icingadb.NewDb(db, logger, &d.Options), nil
}
// Validate checks constraints in the supplied database configuration and returns an error if they are violated.
func (d *Database) Validate() error {
return d.Options.Validate()
}

View file

@ -74,3 +74,8 @@ func dialWithLogging(logger *zap.SugaredLogger) func(context.Context, string, st
return
}
}
// Validate checks constraints in the supplied Redis configuration and returns an error if they are violated.
func (r *Redis) Validate() error {
return r.Options.Validate()
}

View file

@ -4,7 +4,6 @@ import (
"context"
"database/sql/driver"
"fmt"
"github.com/creasty/defaults"
"github.com/go-sql-driver/mysql"
"github.com/icinga/icingadb/internal"
"github.com/icinga/icingadb/pkg/backoff"
@ -56,24 +55,14 @@ type Options struct {
MaxRowsPerTransaction int `yaml:"MaxRowsPerTransaction" default:"8192"`
}
// UnmarshalYAML implements the yaml.Unmarshaler interface.
func (o *Options) UnmarshalYAML(unmarshal func(interface{}) error) error {
if err := defaults.Set(o); err != nil {
return errors.Wrap(err, "can't set default database config")
}
// Prevent recursion.
type self Options
if err := unmarshal((*self)(o)); err != nil {
return internal.CantUnmarshalYAML(err, o)
}
// Validate checks constraints in the supplied database options and returns an error if they are violated.
func (o *Options) Validate() error {
if o.MaxConnections == 0 {
return errors.New("max_connections cannot be 0. Configure a value greater than zero, or use -1 for no connection limit")
}
if o.MaxConnectionsPerTable < 1 {
return errors.New("max_connections_per_table must be at least 1")
}
return nil
}

View file

@ -2,9 +2,7 @@ package icingaredis
import (
"context"
"github.com/creasty/defaults"
"github.com/go-redis/redis/v8"
"github.com/icinga/icingadb/internal"
"github.com/icinga/icingadb/pkg/com"
"github.com/icinga/icingadb/pkg/common"
"github.com/icinga/icingadb/pkg/contracts"
@ -34,17 +32,8 @@ type Options struct {
HScanCount int `yaml:"hscan_count" default:"4096"`
}
// UnmarshalYAML implements the yaml.Unmarshaler interface.
func (o *Options) UnmarshalYAML(unmarshal func(interface{}) error) error {
if err := defaults.Set(o); err != nil {
return errors.Wrapf(err, "can't set defaults %#v", o)
}
// Prevent recursion.
type self Options
if err := unmarshal((*self)(o)); err != nil {
return internal.CantUnmarshalYAML(err, o)
}
// Validate checks constraints in the supplied Redis options and returns an error if they are violated.
func (o *Options) Validate() error {
if o.Timeout == 0 {
return errors.New("timeout cannot be 0. Configure a value greater than zero, or use -1 for no timeout")
}
@ -57,7 +46,6 @@ func (o *Options) UnmarshalYAML(unmarshal func(interface{}) error) error {
if o.HScanCount < 1 {
return errors.New("hscan_count must be at least 1")
}
return nil
}