From 6854f89132a7e950675536be02573f77e3781673 Mon Sep 17 00:00:00 2001 From: Martin Chodur Date: Thu, 29 Feb 2024 18:07:57 +0100 Subject: [PATCH] Fus fix interval params validation (#51) * fix: validation of the hasAllowedSourceTenants params Signed-off-by: Martin Chodur * fix: change hasAllowedEvaluationInterval params from time.Duration to Prometheus model.Duration Signed-off-by: Martin Chodur * chore: changelog Signed-off-by: Martin Chodur --------- Signed-off-by: Martin Chodur --- CHANGELOG.md | 1 + examples/human_readable.html | 2 +- examples/human_readable.md | 2 +- examples/validation.yaml | 1 - pkg/validator/group.go | 29 ++++++++++++++--------------- pkg/validator/validator_test.go | 10 +++++----- 6 files changed, 22 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a84268..647d97d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +- Fixed param validation of the `hasAllowedEvaluationInterval` validator, if the `maximum` was not set. ## [v2.8.0] - 2024-02-29 - Added new param `ignoreTemplatedValues` to the `labelHasAllowedValue` validator to ignore templated values in the label. diff --git a/examples/human_readable.html b/examples/human_readable.html index 26c1837..fea5480 100644 --- a/examples/human_readable.html +++ b/examples/human_readable.html @@ -48,7 +48,7 @@

check-metric-name

check-groups

  • Group does not have other source_tenants than: tenant1, tenant2, k8s
    • -
  • Group evaluation interval is between 20s and 10m0s if set
    • +
  • Group evaluation interval is between 20s and 106751d23h47m16s854ms if set
  • Group has valid partial_response_strategy (one of warn or abort) if set
diff --git a/examples/human_readable.md b/examples/human_readable.md index 5a2be52..acc9ca2 100644 --- a/examples/human_readable.md +++ b/examples/human_readable.md @@ -33,7 +33,7 @@ Validation rules: check-groups - Group does not have other `source_tenants` than: `tenant1`, `tenant2`, `k8s` - - Group evaluation interval is between `20s` and `10m0s` if set + - Group evaluation interval is between `20s` and `106751d23h47m16s854ms` if set - Group has valid partial_response_strategy (one of `warn` or `abort`) if set another-checks diff --git a/examples/validation.yaml b/examples/validation.yaml index 7534a98..67bf2d1 100644 --- a/examples/validation.yaml +++ b/examples/validation.yaml @@ -93,6 +93,5 @@ validationRules: - type: hasAllowedEvaluationInterval params: minimum: "20s" - maximum: "10m" intervalMustBeSet: false - type: hasValidPartialStrategy diff --git a/pkg/validator/group.go b/pkg/validator/group.go index 37e2cbe..43cf41b 100644 --- a/pkg/validator/group.go +++ b/pkg/validator/group.go @@ -3,10 +3,10 @@ package validator import ( "fmt" "strings" - "time" "github.com/fusakla/promruval/v2/pkg/prometheus" "github.com/fusakla/promruval/v2/pkg/unmarshaler" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/model/rulefmt" "golang.org/x/exp/slices" "gopkg.in/yaml.v3" @@ -45,29 +45,28 @@ func (h hasAllowedSourceTenants) Validate(group unmarshaler.RuleGroup, _ rulefmt func newHasAllowedEvaluationInterval(paramsConfig yaml.Node) (Validator, error) { params := struct { - MinimumEvaluationInterval time.Duration `yaml:"minimum"` - MaximumEvaluationInterval time.Duration `yaml:"maximum"` - MustBeSet bool `yaml:"intervalMustBeSet"` + Minimum model.Duration `yaml:"minimum"` + Maximum model.Duration `yaml:"maximum"` + MustBeSet bool `yaml:"intervalMustBeSet"` }{} if err := paramsConfig.Decode(¶ms); err != nil { return nil, err } - if params.MinimumEvaluationInterval > params.MaximumEvaluationInterval { + if params.Maximum == 0 { + params.Maximum = model.Duration(1<<63 - 1) + } + if params.Minimum > params.Maximum { return nil, fmt.Errorf("minimum is greater than maximum") } - if params.MaximumEvaluationInterval == 0 && params.MinimumEvaluationInterval == 0 { + if params.Maximum == 0 && params.Minimum == 0 { return nil, fmt.Errorf("at least one of the `minimum` or `maximum` must be set") } - if params.MaximumEvaluationInterval == 0 { - params.MaximumEvaluationInterval = time.Duration(1<<63 - 1) - - } - return &hasAllowedEvaluationInterval{minimum: params.MinimumEvaluationInterval, maximum: params.MaximumEvaluationInterval, mustBeSet: params.MustBeSet}, nil + return &hasAllowedEvaluationInterval{minimum: params.Minimum, maximum: params.Maximum, mustBeSet: params.MustBeSet}, nil } type hasAllowedEvaluationInterval struct { - minimum time.Duration - maximum time.Duration + minimum model.Duration + maximum model.Duration mustBeSet bool } @@ -88,10 +87,10 @@ func (h hasAllowedEvaluationInterval) Validate(group unmarshaler.RuleGroup, _ ru } return []error{} } - if h.minimum != 0 && time.Duration(group.Interval) < h.minimum { + if h.minimum != 0 && group.Interval < h.minimum { return []error{fmt.Errorf("evaluation interval %s is less than `%s`", group.Interval, h.minimum)} } - if h.maximum != 0 && time.Duration(group.Interval) > h.maximum { + if h.maximum != 0 && group.Interval > h.maximum { return []error{fmt.Errorf("evaluation interval %s is greater than `%s`", group.Interval, h.maximum)} } return []error{} diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index 4114763..1f9da7d 100644 --- a/pkg/validator/validator_test.go +++ b/pkg/validator/validator_test.go @@ -211,11 +211,11 @@ var testCases = []struct { {name: "allowedSourceTenantsAndGroupSourceTenantsWithMultipleTenantsAndOneIsMissing", validator: hasAllowedSourceTenants{allowedSourceTenants: []string{"tenant1", "tenant2"}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant1", "tenant3"}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 1}, // hasAllowedEvaluationInterval - {name: "validInterval", validator: hasAllowedEvaluationInterval{minimum: time.Second, maximum: time.Minute, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Minute)}, expectedErrors: 0}, - {name: "unsetRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: time.Second, maximum: time.Minute, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 1}, - {name: "unsetNotRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: time.Second, maximum: time.Minute, mustBeSet: false}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 0}, - {name: "tooShortInterval", validator: hasAllowedEvaluationInterval{minimum: time.Minute, maximum: time.Hour, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Second)}, expectedErrors: 1}, - {name: "tooHighInterval", validator: hasAllowedEvaluationInterval{minimum: time.Minute, maximum: time.Hour, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Hour * 2)}, expectedErrors: 1}, + {name: "validInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Second), maximum: model.Duration(time.Minute), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Minute)}, expectedErrors: 0}, + {name: "unsetRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Second), maximum: model.Duration(time.Minute), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 1}, + {name: "unsetNotRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Second), maximum: model.Duration(time.Minute), mustBeSet: false}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 0}, + {name: "tooShortInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Minute), maximum: model.Duration(time.Hour), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Second)}, expectedErrors: 1}, + {name: "tooHighInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Minute), maximum: model.Duration(time.Hour), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Hour * 2)}, expectedErrors: 1}, // hasValidPartialStrategy {name: "validPartialStrategy", validator: hasValidPartialStrategy{}, group: unmarshaler.RuleGroup{PartialResponseStrategy: "warn"}, expectedErrors: 0},