From 898fc0147b44794ded1ce6d8bb82b88c3f134d17 Mon Sep 17 00:00:00 2001 From: RTann Date: Tue, 25 Oct 2022 13:42:01 -0700 Subject: [PATCH 1/5] ugh --- pkg/env/durationsetting.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/env/durationsetting.go b/pkg/env/durationsetting.go index 6c10305427d38..33929a8ba3957 100644 --- a/pkg/env/durationsetting.go +++ b/pkg/env/durationsetting.go @@ -1,6 +1,7 @@ package env import ( + "fmt" "os" "time" @@ -41,6 +42,10 @@ func (d *DurationSetting) DurationSetting() time.Duration { } func registerDurationSetting(envVar string, defaultDuration time.Duration) *DurationSetting { + if defaultDuration <= 0 { + panic(fmt.Sprintf("invalid default duration: %v <= 0", defaultDuration)) + } + s := &DurationSetting{ envVar: envVar, defaultDuration: defaultDuration, From f6896e375219699911ba79e6b81868be2a1af344 Mon Sep 17 00:00:00 2001 From: RTann Date: Tue, 25 Oct 2022 13:58:52 -0700 Subject: [PATCH 2/5] update --- pkg/env/durationsetting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/env/durationsetting.go b/pkg/env/durationsetting.go index 33929a8ba3957..de5d977cc2aa1 100644 --- a/pkg/env/durationsetting.go +++ b/pkg/env/durationsetting.go @@ -33,7 +33,7 @@ func (d *DurationSetting) DurationSetting() time.Duration { val := os.Getenv(d.envVar) if val != "" { dur, err := time.ParseDuration(val) - if err == nil { + if err == nil && dur >= 0 { return dur } log.Warnf("%s is not a valid environment variable for %s, using default value: %s", val, d.envVar, d.defaultDuration.String()) From 8ee042346542b0b81ad30c36192ab27524c64459 Mon Sep 17 00:00:00 2001 From: RTann Date: Tue, 25 Oct 2022 14:01:55 -0700 Subject: [PATCH 3/5] one more update --- pkg/env/durationsetting.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/env/durationsetting.go b/pkg/env/durationsetting.go index de5d977cc2aa1..9c8572087fe13 100644 --- a/pkg/env/durationsetting.go +++ b/pkg/env/durationsetting.go @@ -33,7 +33,8 @@ func (d *DurationSetting) DurationSetting() time.Duration { val := os.Getenv(d.envVar) if val != "" { dur, err := time.ParseDuration(val) - if err == nil && dur >= 0 { + // Only durations > 0 are meaningful and should be used. + if err == nil && dur > 0 { return dur } log.Warnf("%s is not a valid environment variable for %s, using default value: %s", val, d.envVar, d.defaultDuration.String()) From 241ce31a485145582f3d21095d585cb3c0c6902f Mon Sep 17 00:00:00 2001 From: RTann Date: Wed, 26 Oct 2022 14:12:28 -0700 Subject: [PATCH 4/5] allow zero --- pkg/env/alert_renotif_debounce.go | 2 +- pkg/env/durationsetting.go | 28 ++++++++++++++++++++++------ pkg/env/durationsetting_options.go | 22 ++++++++++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 pkg/env/durationsetting_options.go diff --git a/pkg/env/alert_renotif_debounce.go b/pkg/env/alert_renotif_debounce.go index 2b8d0f9a3314f..5a89a23a82332 100644 --- a/pkg/env/alert_renotif_debounce.go +++ b/pkg/env/alert_renotif_debounce.go @@ -5,5 +5,5 @@ var ( // between an alert being resolved, and a new alert being generated for the same deployment-policy pair, // such that notifications are sent for the new alert. // If it is set to 0 (the default), notifications are always sent, and there is no debouncing. - AlertRenotifDebounceDuration = registerDurationSetting("ROX_ALERT_RENOTIF_DEBOUNCE_DURATION", 0) + AlertRenotifDebounceDuration = registerDurationSetting("ROX_ALERT_RENOTIF_DEBOUNCE_DURATION", 0, WithDurationZeroAllowed()) ) diff --git a/pkg/env/durationsetting.go b/pkg/env/durationsetting.go index 9c8572087fe13..289fcdacae001 100644 --- a/pkg/env/durationsetting.go +++ b/pkg/env/durationsetting.go @@ -6,6 +6,7 @@ import ( "time" "github.com/stackrox/rox/pkg/logging" + "github.com/stackrox/rox/pkg/utils" ) var ( @@ -16,6 +17,7 @@ var ( type DurationSetting struct { envVar string defaultDuration time.Duration + opts durationSettingOpts } // EnvVar returns the string name of the environment variable @@ -33,25 +35,39 @@ func (d *DurationSetting) DurationSetting() time.Duration { val := os.Getenv(d.envVar) if val != "" { dur, err := time.ParseDuration(val) - // Only durations > 0 are meaningful and should be used. - if err == nil && dur > 0 { + if err == nil && validateDuration(dur, d.opts) == nil { return dur } - log.Warnf("%s is not a valid environment variable for %s, using default value: %s", val, d.envVar, d.defaultDuration.String()) + log.Warnf("%s is not a valid environment variable for %s, using default value: %v", val, d.envVar, d.defaultDuration) } return d.defaultDuration } -func registerDurationSetting(envVar string, defaultDuration time.Duration) *DurationSetting { - if defaultDuration <= 0 { - panic(fmt.Sprintf("invalid default duration: %v <= 0", defaultDuration)) +func registerDurationSetting(envVar string, defaultDuration time.Duration, options ...DurationSettingOption) *DurationSetting { + var opts durationSettingOpts + for _, o := range options { + o.apply(&opts) } + utils.CrashOnError(validateDuration(defaultDuration, opts)) + s := &DurationSetting{ envVar: envVar, defaultDuration: defaultDuration, + opts: opts, } Settings[s.EnvVar()] = s return s } + +func validateDuration(d time.Duration, opts durationSettingOpts) error { + if d < 0 { + return fmt.Errorf("invalid duration: %v < 0", d) + } + if !opts.zeroAllowed && d == 0 { + return fmt.Errorf("invalid duration: %v == 0", d) + } + + return nil +} diff --git a/pkg/env/durationsetting_options.go b/pkg/env/durationsetting_options.go new file mode 100644 index 0000000000000..2953c77e0d908 --- /dev/null +++ b/pkg/env/durationsetting_options.go @@ -0,0 +1,22 @@ +package env + +type durationSettingOpts struct { + zeroAllowed bool +} + +type DurationSettingOption interface { + apply(opts *durationSettingOpts) +} + +type durationSettingOptsFunc func(opts *durationSettingOpts) + +func (f durationSettingOptsFunc) apply(opts *durationSettingOpts) { + f(opts) +} + +// WithDurationZeroAllowed specifies the DurationSetting may have a value of 0. +func WithDurationZeroAllowed() DurationSettingOption { + return durationSettingOptsFunc(func(opts *durationSettingOpts) { + opts.zeroAllowed = true + }) +} From b1a99f16a0a009c1483c99a3b500e2b08b9513ee Mon Sep 17 00:00:00 2001 From: RTann Date: Wed, 26 Oct 2022 14:13:58 -0700 Subject: [PATCH 5/5] forgot to add this --- pkg/env/durationsetting_options.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/env/durationsetting_options.go b/pkg/env/durationsetting_options.go index 2953c77e0d908..7cc10fac6b95d 100644 --- a/pkg/env/durationsetting_options.go +++ b/pkg/env/durationsetting_options.go @@ -4,6 +4,8 @@ type durationSettingOpts struct { zeroAllowed bool } +// DurationSettingOption represents an option which may be specified +// for a DurationSetting environment variable. type DurationSettingOption interface { apply(opts *durationSettingOpts) }