Add proper validation errors to config module, enable soft-fail on start

This commit is contained in:
Daniel 2022-03-01 15:28:33 +01:00
parent 7d144dae89
commit bdd1bc2d86
10 changed files with 125 additions and 70 deletions

View file

@ -2,6 +2,7 @@ package config
import (
"encoding/json"
"fmt"
"testing"
"github.com/safing/portbase/log"
@ -13,7 +14,11 @@ func parseAndReplaceConfig(jsonData string) error {
return err
}
return replaceConfig(m)
validationErrors := replaceConfig(m)
if len(validationErrors) > 0 {
return fmt.Errorf("%d errors, first: %w", len(validationErrors), validationErrors[0])
}
return nil
}
func parseAndReplaceDefaultConfig(jsonData string) error {
@ -22,7 +27,11 @@ func parseAndReplaceDefaultConfig(jsonData string) error {
return err
}
return replaceDefaultConfig(m)
validationErrors := replaceDefaultConfig(m)
if len(validationErrors) > 0 {
return fmt.Errorf("%d errors, first: %w", len(validationErrors), validationErrors[0])
}
return nil
}
func quickRegister(t *testing.T, key string, optType OptionType, defaultValue interface{}) {

View file

@ -64,7 +64,7 @@ func start() error {
return err
}
err = loadConfig()
err = loadConfig(false)
if err != nil && !os.IsNotExist(err) {
return err
}

View file

@ -6,6 +6,7 @@ import (
"regexp"
"sync"
"github.com/mitchellh/copystructure"
"github.com/tidwall/sjson"
"github.com/safing/portbase/database/record"
@ -290,6 +291,15 @@ func (option *Option) GetAnnotation(key string) (interface{}, bool) {
return val, ok
}
// copyOrNil returns a copy of the option, or nil if copying failed.
func (option *Option) copyOrNil() *Option {
copied, err := copystructure.Copy(option)
if err != nil {
return nil
}
return copied.(*Option) //nolint:forcetypeassert
}
// Export expors an option to a Record.
func (option *Option) Export() (record.Record, error) {
option.Lock()

View file

@ -2,16 +2,32 @@ package config
import (
"encoding/json"
"fmt"
"io/ioutil"
"path"
"strings"
"sync"
"github.com/safing/portbase/log"
)
var configFilePath string
var (
configFilePath string
func loadConfig() error {
loadedConfigValidationErrors []*ValidationError
loadedConfigValidationErrorsLock sync.Mutex
)
// GetLoadedConfigValidationErrors returns the encountered validation errors
// from the last time loading config from disk.
func GetLoadedConfigValidationErrors() []*ValidationError {
loadedConfigValidationErrorsLock.Lock()
defer loadedConfigValidationErrorsLock.Unlock()
return loadedConfigValidationErrors
}
func loadConfig(requireValidConfig bool) error {
// check if persistence is configured
if configFilePath == "" {
return nil
@ -29,7 +45,17 @@ func loadConfig() error {
return err
}
return replaceConfig(newValues)
validationErrors := replaceConfig(newValues)
if requireValidConfig && len(validationErrors) > 0 {
return fmt.Errorf("encountered %d validation errors during config loading", len(validationErrors))
}
// Save validation errors.
loadedConfigValidationErrorsLock.Lock()
defer loadedConfigValidationErrorsLock.Unlock()
loadedConfigValidationErrors = validationErrors
return nil
}
// saveConfig saves the current configuration to file.

View file

@ -92,9 +92,10 @@ func Register(option *Option) error {
}
}
option.activeFallbackValue, err = validateValue(option, option.DefaultValue)
if err != nil {
return fmt.Errorf("config: invalid default value: %w", err)
var vErr *ValidationError
option.activeFallbackValue, vErr = validateValue(option, option.DefaultValue)
if vErr != nil {
return fmt.Errorf("config: invalid default value: %w", vErr)
}
optionsLock.Lock()

View file

@ -2,7 +2,6 @@ package config
import (
"errors"
"fmt"
"sync"
"github.com/tevino/abool"
@ -39,9 +38,8 @@ func signalChanges() {
}
// replaceConfig sets the (prioritized) user defined config.
func replaceConfig(newValues map[string]interface{}) error {
var firstErr error
var errCnt int
func replaceConfig(newValues map[string]interface{}) []*ValidationError {
var validationErrors []*ValidationError
// RLock the options because we are not adding or removing
// options from the registration but rather only update the
@ -59,10 +57,7 @@ func replaceConfig(newValues map[string]interface{}) error {
if err == nil {
option.activeValue = valueCache
} else {
errCnt++
if firstErr == nil {
firstErr = err
}
validationErrors = append(validationErrors, err)
}
}
@ -72,20 +67,12 @@ func replaceConfig(newValues map[string]interface{}) error {
signalChanges()
if firstErr != nil {
if errCnt > 0 {
return fmt.Errorf("encountered %d errors, first was: %w", errCnt, firstErr)
}
return firstErr
}
return nil
return validationErrors
}
// replaceDefaultConfig sets the (fallback) default config.
func replaceDefaultConfig(newValues map[string]interface{}) error {
var firstErr error
var errCnt int
func replaceDefaultConfig(newValues map[string]interface{}) []*ValidationError {
var validationErrors []*ValidationError
// RLock the options because we are not adding or removing
// options from the registration but rather only update the
@ -103,10 +90,7 @@ func replaceDefaultConfig(newValues map[string]interface{}) error {
if err == nil {
option.activeDefaultValue = valueCache
} else {
errCnt++
if firstErr == nil {
firstErr = err
}
validationErrors = append(validationErrors, err)
}
}
handleOptionUpdate(option, true)
@ -115,14 +99,7 @@ func replaceDefaultConfig(newValues map[string]interface{}) error {
signalChanges()
if firstErr != nil {
if errCnt > 0 {
return fmt.Errorf("encountered %d errors, first was: %w", errCnt, firstErr)
}
return firstErr
}
return nil
return validationErrors
}
// SetConfigOption sets a single value in the (prioritized) user defined config.
@ -140,9 +117,8 @@ func setConfigOption(key string, value interface{}, push bool) (err error) {
if value == nil {
option.activeValue = nil
} else {
var valueCache *valueCache
valueCache, err = validateValue(option, value)
if err == nil {
valueCache, vErr := validateValue(option, value)
if vErr == nil {
option.activeValue = valueCache
}
}
@ -180,9 +156,8 @@ func setDefaultConfigOption(key string, value interface{}, push bool) (err error
if value == nil {
option.activeDefaultValue = nil
} else {
var valueCache *valueCache
valueCache, err = validateValue(option, value)
if err == nil {
valueCache, vErr := validateValue(option, value)
if vErr == nil {
option.activeDefaultValue = valueCache
}
}

View file

@ -24,9 +24,9 @@ func TestLayersGetters(t *testing.T) { //nolint:paralleltest
t.Fatal(err)
}
err = replaceConfig(mapData)
if err != nil {
t.Fatal(err)
validationErrors := replaceConfig(mapData)
if len(validationErrors) > 0 {
t.Fatalf("%d errors, first: %s", len(validationErrors), validationErrors[0].Error())
}
// Test missing values

View file

@ -61,15 +61,18 @@ func isAllowedPossibleValue(opt *Option, value interface{}) error {
}
}
return fmt.Errorf("value is not allowed")
return errors.New("value is not allowed")
}
// validateValue ensures that value matches the expected type of option.
// It does not create a copy of the value!
func validateValue(option *Option, value interface{}) (*valueCache, error) { //nolint:gocyclo
func validateValue(option *Option, value interface{}) (*valueCache, *ValidationError) { //nolint:gocyclo
if option.OptType != OptTypeStringArray {
if err := isAllowedPossibleValue(option, value); err != nil {
return nil, fmt.Errorf("validation of option %s failed for %v: %w", option.Key, value, err)
return nil, &ValidationError{
Option: option.copyOrNil(),
Err: err,
}
}
}
@ -79,11 +82,11 @@ func validateValue(option *Option, value interface{}) (*valueCache, error) { //n
switch v := value.(type) {
case string:
if option.OptType != OptTypeString {
return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
}
if option.compiledRegex != nil {
if !option.compiledRegex.MatchString(v) {
return nil, fmt.Errorf("validation of option %s failed: string \"%s\" did not match validation regex for option", option.Key, v)
return nil, invalid(option, "did not match validation regex")
}
}
validated = &valueCache{stringVal: v}
@ -92,28 +95,28 @@ func validateValue(option *Option, value interface{}) (*valueCache, error) { //n
for pos, entry := range v {
s, ok := entry.(string)
if !ok {
return nil, fmt.Errorf("validation of option %s failed: element %+v (#%d) is not a string", option.Key, entry, pos+1)
return nil, invalid(option, "entry #%d is not a string", pos+1)
}
vConverted[pos] = s
}
// Call validation function again with converted value.
var err error
validated, err = validateValue(option, vConverted)
if err != nil {
return nil, err
var vErr *ValidationError
validated, vErr = validateValue(option, vConverted)
if vErr != nil {
return nil, vErr
}
case []string:
if option.OptType != OptTypeStringArray {
return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
}
if option.compiledRegex != nil {
for pos, entry := range v {
if !option.compiledRegex.MatchString(entry) {
return nil, fmt.Errorf("validation of option %s failed: string \"%s\" (#%d) did not match validation regex", option.Key, entry, pos+1)
return nil, invalid(option, "entry #%d did not match validation regex", pos+1)
}
if err := isAllowedPossibleValue(option, entry); err != nil {
return nil, fmt.Errorf("validation of option %s failed: string %q (#%d) is not allowed", option.Key, entry, pos+1)
return nil, invalid(option, "entry #%d is not allowed", pos+1)
}
}
}
@ -121,12 +124,12 @@ func validateValue(option *Option, value interface{}) (*valueCache, error) { //n
case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, float32, float64:
// uint64 is omitted, as it does not fit in a int64
if option.OptType != OptTypeInt {
return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
}
if option.compiledRegex != nil {
// we need to use %v here so we handle float and int correctly.
if !option.compiledRegex.MatchString(fmt.Sprintf("%v", v)) {
return nil, fmt.Errorf("validation of option %s failed: number \"%d\" did not match validation regex", option.Key, v)
return nil, invalid(option, "did not match validation regex")
}
}
switch v := value.(type) {
@ -153,25 +156,25 @@ func validateValue(option *Option, value interface{}) (*valueCache, error) { //n
if math.Remainder(float64(v), 1) == 0 {
validated = &valueCache{intVal: int64(v)}
} else {
return nil, fmt.Errorf("failed to convert float32 to int64 for option %s, got value %+v", option.Key, v)
return nil, invalid(option, "failed to convert float32 to int64")
}
case float64:
// convert if float has no decimals
if math.Remainder(v, 1) == 0 {
validated = &valueCache{intVal: int64(v)}
} else {
return nil, fmt.Errorf("failed to convert float64 to int64 for option %s, got value %+v", option.Key, v)
return nil, invalid(option, "failed to convert float64 to int64")
}
default:
return nil, errors.New("internal error")
return nil, invalid(option, "internal error")
}
case bool:
if option.OptType != OptTypeBool {
return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
}
validated = &valueCache{boolVal: v}
default:
return nil, fmt.Errorf("invalid option value type for option %s: %T", option.Key, value)
return nil, invalid(option, "invalid option value type: %T", value)
}
// Check if there is an additional function to validate the value.
@ -190,9 +193,35 @@ func validateValue(option *Option, value interface{}) (*valueCache, error) { //n
err = option.ValidationFunc(validated.boolVal)
}
if err != nil {
return nil, err
return nil, &ValidationError{
Option: option.copyOrNil(),
Err: err,
}
}
}
return validated, nil
}
// ValidationError error holds details about a config option value validation error.
type ValidationError struct {
Option *Option
Err error
}
// Error returns the formatted error.
func (ve *ValidationError) Error() string {
return fmt.Sprintf("validation of %s failed: %s", ve.Option.Key, ve.Err)
}
// Unwrap returns the wrapped error.
func (ve *ValidationError) Unwrap() error {
return ve.Err
}
func invalid(option *Option, format string, a ...interface{}) *ValidationError {
return &ValidationError{
Option: option.copyOrNil(),
Err: fmt.Errorf(format, a...),
}
}

1
go.mod
View file

@ -20,6 +20,7 @@ require (
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1
github.com/hashicorp/go-version v1.4.0
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/seehuhn/fortuna v1.0.1
github.com/shirou/gopsutil v3.21.11+incompatible
github.com/stretchr/testify v1.6.1

4
go.sum
View file

@ -67,8 +67,12 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw=
github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s=
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ=
github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=