From b650e6786f9757d3c867fd8344d80dcabdc55606 Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 2 Jun 2026 17:36:47 -0500 Subject: [PATCH 1/3] fix: lighter validation w/ ozzo-validation --- go.mod | 9 +- go.sum | 27 ++-- internal/conf.go | 36 ++--- internal/conf_validate.go | 203 +++++++++++++++++++++++ internal/conf_validate_test.go | 259 ++++++++++++++++++++++++++++++ internal/status/stat-server.go | 8 +- testdata/upd_test_short.yaml | 24 --- testdata/upd_test_short_noda.yaml | 18 --- 8 files changed, 492 insertions(+), 92 deletions(-) create mode 100644 internal/conf_validate.go create mode 100644 internal/conf_validate_test.go delete mode 100644 testdata/upd_test_short.yaml delete mode 100644 testdata/upd_test_short_noda.yaml diff --git a/go.mod b/go.mod index c1263e9..e038daa 100644 --- a/go.mod +++ b/go.mod @@ -3,22 +3,15 @@ module github.com/hugoh/upd go 1.25.0 require ( - github.com/go-playground/validator/v10 v10.30.3 + github.com/go-ozzo/ozzo-validation/v4 v4.3.0 github.com/stretchr/testify v1.11.1 github.com/urfave/cli/v3 v3.9.0 ) require ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect - github.com/gabriel-vasile/mimetype v1.4.13 // indirect - github.com/go-playground/locales v0.14.1 // indirect - github.com/go-playground/universal-translator v0.18.1 // indirect github.com/kr/pretty v0.3.1 // indirect - github.com/leodido/go-urn v1.4.0 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect - golang.org/x/crypto v0.52.0 // indirect - golang.org/x/sys v0.45.0 // indirect - golang.org/x/text v0.37.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect ) diff --git a/go.sum b/go.sum index ec3eb9a..572f3ec 100644 --- a/go.sum +++ b/go.sum @@ -1,16 +1,11 @@ +github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496 h1:zV3ejI06GQ59hwDQAvmK1qxOQGB3WuVTRoY0okPTAv0= +github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496/go.mod h1:oGkLhpf+kjZl6xBf758TQhh5XrAeiJv/7FRz/2spLIg= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/gabriel-vasile/mimetype v1.4.13 h1:46nXokslUBsAJE/wMsp5gtO500a4F3Nkz9Ufpk2AcUM= -github.com/gabriel-vasile/mimetype v1.4.13/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s= -github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= -github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= -github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= -github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= -github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= -github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= -github.com/go-playground/validator/v10 v10.30.3 h1:4MU6YkEwx7GbcPJOZxrtbu+QfF3pJLJuaYTeAH0DYy8= -github.com/go-playground/validator/v10 v10.30.3/go.mod h1:4Axh7oCNGcoGkqLoE4YWt6n20mcEIsPRlB7vPk3lpyc= +github.com/go-ozzo/ozzo-validation/v4 v4.3.0 h1:byhDUpfEwjsVQb1vBunvIjh2BHQ9ead57VkAEY4V+Es= +github.com/go-ozzo/ozzo-validation/v4 v4.3.0/go.mod h1:2NKgrcHl3z6cJs+3Oo940FPRiTzuqKbvfrL2RxCj6Ew= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= @@ -20,26 +15,22 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= -github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/urfave/cli/v3 v3.9.0 h1:AV9lIiPv3ukYnxunaCUsHnEozptYmDN2F0+yWqLMn/c= github.com/urfave/cli/v3 v3.9.0/go.mod h1:ysVLtOEmg2tOy6PknnYVhDoouyC/6N42TMeoMzskhso= -golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= -golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= -golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= -golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= -golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/conf.go b/internal/conf.go index 5a2a500..10e5ddd 100644 --- a/internal/conf.go +++ b/internal/conf.go @@ -66,7 +66,6 @@ import ( "strings" "time" - "github.com/go-playground/validator/v10" "github.com/hugoh/upd/internal/check" "github.com/hugoh/upd/internal/logger" "github.com/hugoh/upd/internal/logic" @@ -91,26 +90,26 @@ var ConfigFileUsed string type Configuration struct { Checks struct { Every struct { - Normal types.Duration `validate:"required,gt=0"` - Down types.Duration `validate:"required,gt=0"` - } `validate:"required"` + Normal types.Duration + Down types.Duration + } List struct { - Ordered []string `validate:"dive,uri"` - Shuffled []string `validate:"dive,uri"` - } `validate:"required"` - TimeOut types.Duration `validate:"required,gt=0"` - } `validate:"required"` + Ordered []string + Shuffled []string + } + TimeOut types.Duration + } DownAction struct { Exec string Every struct { - After types.Duration `validate:"omitempty,gt=0"` - Repeat types.Duration `validate:"omitempty,gt=0"` - BackoffLimit types.Duration `validate:"omitempty,gte=0" yaml:"expBackoffLimit"` + After types.Duration + Repeat types.Duration + BackoffLimit types.Duration `yaml:"expBackoffLimit"` } - StopExec string `yaml:"stopExec" validate:"omitempty"` //nolint:tagalign - } `validate:"omitempty" yaml:"downAction"` - Stats status.StatServerConfig `validate:"omitempty"` - LogLevel string `validate:"omitempty,oneof=trace debug info warn" yaml:"logLevel"` + StopExec string `yaml:"stopExec"` + } `yaml:"downAction"` + Stats status.StatServerConfig + LogLevel string `yaml:"logLevel"` } func configError(msg string, path string, err error) (*Configuration, error) { @@ -161,10 +160,7 @@ func ReadConf(cfgFile string) (*Configuration, error) { return configError("Unable to parse the config", cfgFile, err) } - validate := validator.New() - - err = validate.Struct(&conf) - if err != nil { + if err := conf.Validate(); err != nil { return configError("Missing required attributes", cfgFile, err) } diff --git a/internal/conf_validate.go b/internal/conf_validate.go new file mode 100644 index 0000000..b804181 --- /dev/null +++ b/internal/conf_validate.go @@ -0,0 +1,203 @@ +package internal + +import ( + "errors" + "fmt" + "net/url" + "reflect" + "time" + + validation "github.com/go-ozzo/ozzo-validation/v4" + "github.com/hugoh/upd/internal/types" +) + +var ( + errDurationMustBePositive = errors.New("must be greater than 0") + errInvalidURI = errors.New("must be a valid URI") + errNotDuration = errors.New("value is not a types.Duration") + errNotString = errors.New("value is not a string") +) + +// Validate checks that the configuration has all required fields with valid values. +func (c Configuration) Validate() error { + if err := validation.ValidateStruct(&c, + validation.Field(&c.Checks, validation.Required, + validation.By(c.validateChecks), + ), + validation.Field(&c.DownAction, + validation.When(!reflect.ValueOf(c.DownAction).IsZero(), + validation.By(c.validateDownAction), + ), + ), + validation.Field(&c.Stats, + validation.When(!reflect.ValueOf(c.Stats).IsZero(), + validation.By(c.validateStats), + ), + ), + validation.Field(&c.LogLevel, + validation.When(c.LogLevel != "", + validation.In("trace", "debug", "info", "warn"), + ), + ), + ); err != nil { + return fmt.Errorf("configuration: %w", err) + } + + return nil +} + +func (c Configuration) validateChecks(_ any) error { + var errs []error + + if err := c.validateEvery(nil); err != nil { + errs = append(errs, fmt.Errorf("every: %w", err)) + } + + if err := c.validateList(nil); err != nil { + errs = append(errs, fmt.Errorf("list: %w", err)) + } + + if err := validation.Validate(c.Checks.TimeOut, validation.Required, + validation.By(validatePositiveDuration), + ); err != nil { + errs = append(errs, fmt.Errorf("timeout: %w", err)) + } + + return errors.Join(errs...) +} + +func (c Configuration) validateEvery(_ any) error { + var errs []error + + if err := validation.Validate(c.Checks.Every.Normal, validation.Required, + validation.By(validatePositiveDuration), + ); err != nil { + errs = append(errs, fmt.Errorf("normal: %w", err)) + } + + if err := validation.Validate(c.Checks.Every.Down, validation.Required, + validation.By(validatePositiveDuration), + ); err != nil { + errs = append(errs, fmt.Errorf("down: %w", err)) + } + + return errors.Join(errs...) +} + +func (c Configuration) validateList(_ any) error { + var errs []error + + if len(c.Checks.List.Ordered) > 0 { + if err := validation.Validate(c.Checks.List.Ordered, + validation.Each(validation.By(validateURI)), + ); err != nil { + errs = append(errs, fmt.Errorf("ordered: %w", err)) + } + } + + if len(c.Checks.List.Shuffled) > 0 { + if err := validation.Validate(c.Checks.List.Shuffled, + validation.Each(validation.By(validateURI)), + ); err != nil { + errs = append(errs, fmt.Errorf("shuffled: %w", err)) + } + } + + return errors.Join(errs...) +} + +func (c Configuration) validateDownAction(_ any) error { + var errs []error + + if c.DownAction.Every.After != 0 { + if err := validation.Validate(c.DownAction.Every.After, + validation.By(validatePositiveDuration), + ); err != nil { + errs = append(errs, fmt.Errorf("after: %w", err)) + } + } + + if c.DownAction.Every.Repeat != 0 { + if err := validation.Validate(c.DownAction.Every.Repeat, + validation.By(validatePositiveDuration), + ); err != nil { + errs = append(errs, fmt.Errorf("repeat: %w", err)) + } + } + + if c.DownAction.Every.BackoffLimit != 0 { + if err := validation.Validate(c.DownAction.Every.BackoffLimit, + validation.Min(time.Duration(0)), + ); err != nil { + errs = append(errs, fmt.Errorf("expBackoffLimit: %w", err)) + } + } + + return errors.Join(errs...) +} + +func (c Configuration) validateStats(_ any) error { + var errs []error + + if c.Stats.Port != 0 { + if err := validation.Validate(c.Stats.Port, + validation.Min(1), + validation.Max(65535), //nolint:mnd // well-known max port + ); err != nil { + errs = append(errs, fmt.Errorf("port: %w", err)) + } + } + + if c.Stats.ReadTimeout != 0 { + if err := validation.Validate(c.Stats.ReadTimeout, + validation.Min(time.Duration(0)), + ); err != nil { + errs = append(errs, fmt.Errorf("readTimeout: %w", err)) + } + } + + if c.Stats.WriteTimeout != 0 { + if err := validation.Validate(c.Stats.WriteTimeout, + validation.Min(time.Duration(0)), + ); err != nil { + errs = append(errs, fmt.Errorf("writeTimeout: %w", err)) + } + } + + if c.Stats.IdleTimeout != 0 { + if err := validation.Validate(c.Stats.IdleTimeout, + validation.Min(time.Duration(0)), + ); err != nil { + errs = append(errs, fmt.Errorf("idleTimeout: %w", err)) + } + } + + return errors.Join(errs...) +} + +func validatePositiveDuration(value any) error { + d, ok := value.(types.Duration) + if !ok { + return errNotDuration + } + + if time.Duration(d) <= 0 { + return errDurationMustBePositive + } + + return nil +} + +func validateURI(value any) error { + s, ok := value.(string) + if !ok { + return errNotString + } + + _, err := url.ParseRequestURI(s) + if err != nil { + return errInvalidURI + } + + return nil +} diff --git a/internal/conf_validate_test.go b/internal/conf_validate_test.go new file mode 100644 index 0000000..5b8063f --- /dev/null +++ b/internal/conf_validate_test.go @@ -0,0 +1,259 @@ +package internal + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeTestConfig(t *testing.T, content string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "upd.yaml") + + err := os.WriteFile(path, []byte(content), 0o600) + require.NoError(t, err) + + return path +} + +func TestValidate_missingChecks(t *testing.T) { + path := writeTestConfig(t, `logLevel: debug`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_normalZero(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 0s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_downZero(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 0s + list: + ordered: + - http://example.com/ + timeout: 2000ms`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_timeoutZero(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 0s`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_orderedInvalidURI(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + - "://invalid" + timeout: 2000ms`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_shuffledInvalidURI(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + shuffled: + - http://example.com/ + - "://invalid" + timeout: 2000ms`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_afterNegative(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +downAction: + exec: "echo" + every: + after: -5s + repeat: 300s`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_repeatNegative(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +downAction: + exec: "echo" + every: + after: 60s + repeat: -5s`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_backoffLimitNegative(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +downAction: + exec: "echo" + every: + after: 60s + repeat: 300s + expBackoffLimit: -5s`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_portTooHigh(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +downAction: + exec: "echo" + every: + after: 60s + repeat: 300s +stats: + port: 99999`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_readTimeoutNegative(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +stats: + port: 8080 + readTimeout: -5s`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_writeTimeoutNegative(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +stats: + port: 8080 + writeTimeout: -5s`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_idleTimeoutNegative(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +stats: + port: 8080 + idleTimeout: -5s`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} + +func TestValidate_logLevelInvalid(t *testing.T) { + path := writeTestConfig(t, `checks: + every: + normal: 120s + down: 20s + list: + ordered: + - http://example.com/ + timeout: 2000ms +logLevel: invalid`) + + _, err := ReadConf(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "Missing required attributes") +} diff --git a/internal/status/stat-server.go b/internal/status/stat-server.go index 05a5791..15dff63 100644 --- a/internal/status/stat-server.go +++ b/internal/status/stat-server.go @@ -25,12 +25,12 @@ const ( // StatServerConfig holds configuration for the statistics HTTP server. type StatServerConfig struct { - Port int `validate:"omitempty,min=1,max=65535"` + Port int Reports []time.Duration Retention time.Duration - ReadTimeout time.Duration `validate:"omitempty,gte=0" yaml:"readTimeout"` //nolint:tagalign - WriteTimeout time.Duration `validate:"omitempty,gte=0" yaml:"writeTimeout"` //nolint:tagalign - IdleTimeout time.Duration `validate:"omitempty,gte=0" yaml:"idleTimeout"` //nolint:tagalign + ReadTimeout time.Duration `yaml:"readTimeout"` + WriteTimeout time.Duration `yaml:"writeTimeout"` + IdleTimeout time.Duration `yaml:"idleTimeout"` } // UnmarshalYAML implements the yaml.Unmarshaler interface for StatServerConfig, diff --git a/testdata/upd_test_short.yaml b/testdata/upd_test_short.yaml deleted file mode 100644 index 4b123e5..0000000 --- a/testdata/upd_test_short.yaml +++ /dev/null @@ -1,24 +0,0 @@ -checks: - every: - normal: 120s - down: 20s - list: - shuffled: - # From https://en.wikipedia.org/wiki/Captive_portal - - http://captive.apple.com/hotspot-detect.html - - http://connectivitycheck.gstatic.com/generate_204 - - http://clients3.google.com/generate_204 - - http://www.msftconnecttest.com/connecttest.txt - - dns://1.1.1.1/ - - dns://1.0.0.1/ - - dns://8.8.8.8/ - - dns://8.8.4.4/ - timeout: 10ms -downAction: - exec: cowsay - every: - after: 1s - repeat: 1s - expBackoffLimit: 3s -# Options = debug, info, warn, error -logLevel: debug diff --git a/testdata/upd_test_short_noda.yaml b/testdata/upd_test_short_noda.yaml deleted file mode 100644 index 1ab0ad2..0000000 --- a/testdata/upd_test_short_noda.yaml +++ /dev/null @@ -1,18 +0,0 @@ -checks: - every: - normal: 120s - down: 20s - list: - ordered: - # From https://en.wikipedia.org/wiki/Captive_portal - - http://captive.apple.com/hotspot-detect.html - - http://connectivitycheck.gstatic.com/generate_204 - - http://clients3.google.com/generate_204 - - http://www.msftconnecttest.com/connecttest.txt - - dns://1.1.1.1/ - - dns://1.0.0.1/ - - dns://8.8.8.8/ - - dns://8.8.4.4/ - timeout: 10ms -# Options = debug, info, warn, error -logLevel: debug From 58a87d2420605cb330c4c985cfa5f319769fbdfb Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 2 Jun 2026 17:36:47 -0500 Subject: [PATCH 2/3] fix: even lighter validation --- go.mod | 1 - go.sum | 9 -- internal/conf.go | 13 ++- internal/conf_test.go | 8 +- internal/conf_validate.go | 207 ++++++++++++++------------------------ 5 files changed, 86 insertions(+), 152 deletions(-) diff --git a/go.mod b/go.mod index e038daa..1fa6c21 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/hugoh/upd go 1.25.0 require ( - github.com/go-ozzo/ozzo-validation/v4 v4.3.0 github.com/stretchr/testify v1.11.1 github.com/urfave/cli/v3 v3.9.0 ) diff --git a/go.sum b/go.sum index 572f3ec..92d9b3b 100644 --- a/go.sum +++ b/go.sum @@ -1,11 +1,6 @@ -github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496 h1:zV3ejI06GQ59hwDQAvmK1qxOQGB3WuVTRoY0okPTAv0= -github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496/go.mod h1:oGkLhpf+kjZl6xBf758TQhh5XrAeiJv/7FRz/2spLIg= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/go-ozzo/ozzo-validation/v4 v4.3.0 h1:byhDUpfEwjsVQb1vBunvIjh2BHQ9ead57VkAEY4V+Es= -github.com/go-ozzo/ozzo-validation/v4 v4.3.0/go.mod h1:2NKgrcHl3z6cJs+3Oo940FPRiTzuqKbvfrL2RxCj6Ew= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= @@ -16,14 +11,11 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/urfave/cli/v3 v3.9.0 h1:AV9lIiPv3ukYnxunaCUsHnEozptYmDN2F0+yWqLMn/c= @@ -31,6 +23,5 @@ github.com/urfave/cli/v3 v3.9.0/go.mod h1:ysVLtOEmg2tOy6PknnYVhDoouyC/6N42TMeoMz gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/conf.go b/internal/conf.go index 10e5ddd..a1deddf 100644 --- a/internal/conf.go +++ b/internal/conf.go @@ -79,6 +79,11 @@ const ( DefaultConfig = ".upd.yaml" // DefaultDNSPort is the default DNS port. DefaultDNSPort = "53" + + logLevelTrace = "trace" + logLevelDebug = "debug" + logLevelInfo = "info" + logLevelWarn = "warn" ) // ConfigFileUsed stores the path of the active configuration file for debugging purposes. @@ -331,13 +336,13 @@ func (c Configuration) logSetup() { var level slog.Level switch c.LogLevel { - case "trace": + case logLevelTrace: level = slog.LevelDebug - 4 //nolint:mnd // slog doesn't have LevelTrace, use LevelDebug - 4 - case "debug": + case logLevelDebug: level = slog.LevelDebug - case "info": + case logLevelInfo: level = slog.LevelInfo - case "warn": + case logLevelWarn: level = slog.LevelWarn default: logger.L.Error("[Config] Unknown loglevel", "loglevel", c.LogLevel) diff --git a/internal/conf_test.go b/internal/conf_test.go index 542fe45..0ae8d47 100644 --- a/internal/conf_test.go +++ b/internal/conf_test.go @@ -230,10 +230,10 @@ func TestLogSetup(t *testing.T) { name string logLevel string }{ - {"trace level", "trace"}, - {"debug level", "debug"}, - {"info level", "info"}, - {"warn level", "warn"}, + {"trace level", logLevelTrace}, + {"debug level", logLevelDebug}, + {"info level", logLevelInfo}, + {"warn level", logLevelWarn}, {"empty defaults to warn", ""}, } diff --git a/internal/conf_validate.go b/internal/conf_validate.go index b804181..4df83cc 100644 --- a/internal/conf_validate.go +++ b/internal/conf_validate.go @@ -7,197 +7,136 @@ import ( "reflect" "time" - validation "github.com/go-ozzo/ozzo-validation/v4" "github.com/hugoh/upd/internal/types" ) var ( errDurationMustBePositive = errors.New("must be greater than 0") errInvalidURI = errors.New("must be a valid URI") - errNotDuration = errors.New("value is not a types.Duration") - errNotString = errors.New("value is not a string") + errMustNotBeNegative = errors.New("must not be negative") + errPortOutOfRange = errors.New("must be between 1 and 65535") + errInvalidLogLevel = errors.New("must be one of: trace, debug, info, warn") ) -// Validate checks that the configuration has all required fields with valid values. -func (c Configuration) Validate() error { - if err := validation.ValidateStruct(&c, - validation.Field(&c.Checks, validation.Required, - validation.By(c.validateChecks), - ), - validation.Field(&c.DownAction, - validation.When(!reflect.ValueOf(c.DownAction).IsZero(), - validation.By(c.validateDownAction), - ), - ), - validation.Field(&c.Stats, - validation.When(!reflect.ValueOf(c.Stats).IsZero(), - validation.By(c.validateStats), - ), - ), - validation.Field(&c.LogLevel, - validation.When(c.LogLevel != "", - validation.In("trace", "debug", "info", "warn"), - ), - ), - ); err != nil { - return fmt.Errorf("configuration: %w", err) +func appendErr(errs []error, key string, err error) []error { + if err != nil { + return append(errs, fmt.Errorf("%s: %w", key, err)) } - return nil + return errs } -func (c Configuration) validateChecks(_ any) error { +// Validate checks that the configuration has all required fields with valid values. +func (c Configuration) Validate() error { var errs []error - if err := c.validateEvery(nil); err != nil { - errs = append(errs, fmt.Errorf("every: %w", err)) + errs = appendErr(errs, "checks", c.validateChecks()) + + if !reflect.ValueOf(c.DownAction).IsZero() { + errs = appendErr(errs, "downAction", c.validateDownAction()) } - if err := c.validateList(nil); err != nil { - errs = append(errs, fmt.Errorf("list: %w", err)) + if !reflect.ValueOf(c.Stats).IsZero() { + errs = appendErr(errs, "stats", c.validateStats()) } - if err := validation.Validate(c.Checks.TimeOut, validation.Required, - validation.By(validatePositiveDuration), - ); err != nil { - errs = append(errs, fmt.Errorf("timeout: %w", err)) + if c.LogLevel != "" { + errs = appendErr(errs, "logLevel", validateLogLevel(c.LogLevel)) } return errors.Join(errs...) } -func (c Configuration) validateEvery(_ any) error { +func (c Configuration) validateChecks() error { var errs []error - if err := validation.Validate(c.Checks.Every.Normal, validation.Required, - validation.By(validatePositiveDuration), - ); err != nil { - errs = append(errs, fmt.Errorf("normal: %w", err)) - } - - if err := validation.Validate(c.Checks.Every.Down, validation.Required, - validation.By(validatePositiveDuration), - ); err != nil { - errs = append(errs, fmt.Errorf("down: %w", err)) - } + errs = appendErr(errs, "every.normal", validatePositiveDuration(c.Checks.Every.Normal)) + errs = appendErr(errs, "every.down", validatePositiveDuration(c.Checks.Every.Down)) + errs = appendErr(errs, "timeout", validatePositiveDuration(c.Checks.TimeOut)) + errs = appendErr(errs, "list.ordered", validateURIs(c.Checks.List.Ordered)) + errs = appendErr(errs, "list.shuffled", validateURIs(c.Checks.List.Shuffled)) return errors.Join(errs...) } -func (c Configuration) validateList(_ any) error { +func (c Configuration) validateDownAction() error { var errs []error - if len(c.Checks.List.Ordered) > 0 { - if err := validation.Validate(c.Checks.List.Ordered, - validation.Each(validation.By(validateURI)), - ); err != nil { - errs = append(errs, fmt.Errorf("ordered: %w", err)) - } - } - - if len(c.Checks.List.Shuffled) > 0 { - if err := validation.Validate(c.Checks.List.Shuffled, - validation.Each(validation.By(validateURI)), - ); err != nil { - errs = append(errs, fmt.Errorf("shuffled: %w", err)) - } - } + errs = appendErr(errs, "every.after", checkPositive(time.Duration(c.DownAction.Every.After))) + errs = appendErr(errs, "every.repeat", checkPositive(time.Duration(c.DownAction.Every.Repeat))) + errs = appendErr( + errs, + "every.expBackoffLimit", + checkNonNegative(time.Duration(c.DownAction.Every.BackoffLimit)), + ) return errors.Join(errs...) } -func (c Configuration) validateDownAction(_ any) error { +func (c Configuration) validateStats() error { var errs []error - if c.DownAction.Every.After != 0 { - if err := validation.Validate(c.DownAction.Every.After, - validation.By(validatePositiveDuration), - ); err != nil { - errs = append(errs, fmt.Errorf("after: %w", err)) - } - } - - if c.DownAction.Every.Repeat != 0 { - if err := validation.Validate(c.DownAction.Every.Repeat, - validation.By(validatePositiveDuration), - ); err != nil { - errs = append(errs, fmt.Errorf("repeat: %w", err)) - } - } - - if c.DownAction.Every.BackoffLimit != 0 { - if err := validation.Validate(c.DownAction.Every.BackoffLimit, - validation.Min(time.Duration(0)), - ); err != nil { - errs = append(errs, fmt.Errorf("expBackoffLimit: %w", err)) - } - } + errs = appendErr(errs, "port", validatePort(c.Stats.Port)) + errs = appendErr(errs, "readTimeout", checkNonNegative(c.Stats.ReadTimeout)) + errs = appendErr(errs, "writeTimeout", checkNonNegative(c.Stats.WriteTimeout)) + errs = appendErr(errs, "idleTimeout", checkNonNegative(c.Stats.IdleTimeout)) return errors.Join(errs...) } -func (c Configuration) validateStats(_ any) error { - var errs []error - - if c.Stats.Port != 0 { - if err := validation.Validate(c.Stats.Port, - validation.Min(1), - validation.Max(65535), //nolint:mnd // well-known max port - ); err != nil { - errs = append(errs, fmt.Errorf("port: %w", err)) - } - } - - if c.Stats.ReadTimeout != 0 { - if err := validation.Validate(c.Stats.ReadTimeout, - validation.Min(time.Duration(0)), - ); err != nil { - errs = append(errs, fmt.Errorf("readTimeout: %w", err)) - } +func validatePositiveDuration(d types.Duration) error { + if time.Duration(d) <= 0 { + return errDurationMustBePositive } - if c.Stats.WriteTimeout != 0 { - if err := validation.Validate(c.Stats.WriteTimeout, - validation.Min(time.Duration(0)), - ); err != nil { - errs = append(errs, fmt.Errorf("writeTimeout: %w", err)) - } - } + return nil +} - if c.Stats.IdleTimeout != 0 { - if err := validation.Validate(c.Stats.IdleTimeout, - validation.Min(time.Duration(0)), - ); err != nil { - errs = append(errs, fmt.Errorf("idleTimeout: %w", err)) - } +func checkPositive(d time.Duration) error { + if d < 0 { + return errDurationMustBePositive } - return errors.Join(errs...) + return nil } -func validatePositiveDuration(value any) error { - d, ok := value.(types.Duration) - if !ok { - return errNotDuration +func checkNonNegative(d time.Duration) error { + if d < 0 { + return errMustNotBeNegative } - if time.Duration(d) <= 0 { - return errDurationMustBePositive + return nil +} + +func validatePort(port int) error { + if port < 0 || port > 65535 { + return errPortOutOfRange } return nil } -func validateURI(value any) error { - s, ok := value.(string) - if !ok { - return errNotString +func validateLogLevel(level string) error { + switch level { + case logLevelTrace, logLevelDebug, logLevelInfo, logLevelWarn: + return nil + default: + return errInvalidLogLevel } +} - _, err := url.ParseRequestURI(s) - if err != nil { - return errInvalidURI +func validateURIs(uris []string) error { + if len(uris) == 0 { + return nil } - return nil + var errs []error + + for i, s := range uris { + if _, err := url.ParseRequestURI(s); err != nil { + errs = append(errs, fmt.Errorf("[%d]: %w", i, errInvalidURI)) + } + } + + return errors.Join(errs...) } From c253313c8e2dd2d29fe8e81ff070953758e42343 Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 2 Jun 2026 17:36:47 -0500 Subject: [PATCH 3/3] fix(security): Go upgrade --- mise.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mise.toml b/mise.toml index 2919131..bef2d83 100644 --- a/mise.toml +++ b/mise.toml @@ -1,5 +1,5 @@ [tools] -go = "1.26.3" +go = "1.26.4" "go:github.com/vladopajic/go-test-coverage/v2" = "2.18.8" "go:golang.org/x/vuln/cmd/govulncheck" = "1.3.0" golangci-lint = "2.12.2" @@ -7,7 +7,7 @@ goreleaser = "2.16.0" cocogitto = "7.0.0" gotestsum = "1.13.0" hk = "1.46.0" -rumdl = "0.2.4" +rumdl = "0.2.5" zizmor = "1.25.2" dprint = "0.54.0" ghalint = "1.5.6"