Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: PC-13758 Add limits for recurring events #533

Merged
merged 12 commits into from
Sep 12, 2024
Merged
6 changes: 6 additions & 0 deletions cspell.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@ ignorePaths:
- "docs/mock_example/mocks/*"
words:
- 00u2y4e4atkzaYkXP4x8
- BYHOUR
- BYMINUTE
- BYSECOND
- CONNECTIONFAILURES
- CONNECTIONSESTABLISHED
- CONNECTIONSUCCESSES
- DTSTART
- DzpxcSRh
- MAXRTT
- TIMEMAX
Expand All @@ -51,6 +55,7 @@ words:
- awsiam
- budgetadjustment
- byday
- bysecond
- clientcreds
- codeowners
- consumergroup
Expand All @@ -65,6 +70,7 @@ words:
- devel
- dnssec
- doublestar
- dtstart
- dynatrace
- endef
- enduml
Expand Down
28 changes: 27 additions & 1 deletion manifest/v1alpha/budgetadjustment/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package budgetadjustment
import (
"time"

"github.com/pkg/errors"
"github.com/teambition/rrule-go"

"github.com/nobl9/govy/pkg/govy"
Expand Down Expand Up @@ -46,7 +47,8 @@ var specValidation = govy.New(
Required().
Rules(rules.DurationPrecision(time.Minute)),
govy.Transform(func(s Spec) string { return s.Rrule }, rrule.StrToRRule).
WithName("rrule"),
WithName("rrule").
Rules(atLeastHourlyFreq),
govy.For(func(s Spec) Filters { return s.Filters }).
WithName("filters").
Include(filtersValidationRule),
Expand All @@ -72,3 +74,27 @@ var sloValidationRule = govy.New(
Required().
Rules(rules.StringDNSLabel()),
)

var atLeastHourlyFreq = govy.NewRule(func(rule *rrule.RRule) error {
kubaceg marked this conversation as resolved.
Show resolved Hide resolved
if rule == nil {
return nil
}

if rule.Options.Count == 1 {
return nil
}

if rule.Options.Freq == rrule.MINUTELY && rule.Options.Interval < 60 {
return errors.New("interval must be at least 60 minutes for minutely frequency")
}

if rule.Options.Freq == rrule.SECONDLY && rule.Options.Interval < 3600 {
return errors.New("interval must be at least 3600 seconds for secondly frequency")
}

if len(rule.Options.Byminute) > 1 || len(rule.Options.Bysecond) > 0 {
return errors.New("byminute and bysecond are not supported")
}

return nil
})
167 changes: 167 additions & 0 deletions manifest/v1alpha/budgetadjustment/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/teambition/rrule-go"

validationV1Alpha "github.com/nobl9/nobl9-go/internal/manifest/v1alpha"

Expand Down Expand Up @@ -276,6 +277,61 @@ func TestValidate_Spec(t *testing.T) {
},
},
},
{
name: "invalid freq in rrule",
spec: Spec{
FirstEventStart: time.Now(),
Duration: "1m",
Rrule: "FREQ=MINUTELY;INTERVAL=2;COUNT=10",
Filters: Filters{
SLOs: []SLORef{{
Name: "test",
Project: "project",
}},
},
},
expectedErrors: []testutils.ExpectedError{
{
Prop: "spec.rrule",
Message: "interval must be at least 60 minutes for minutely frequency",
},
},
},
{
name: "rrule with dtstart trows transform error",
spec: Spec{
FirstEventStart: time.Now(),
Duration: "1m",
Rrule: "DTSTART:20240909T065900Z\\nRRULE:FREQ=MINUTELY;BYHOUR=6,8,9,10,11,12,13,14,15,16;COUNT=10",
Filters: Filters{
SLOs: []SLORef{{
Name: "test",
Project: "project",
}},
},
},
expectedErrors: []testutils.ExpectedError{
{
Prop: "spec.rrule",
Code: "transform",
},
},
},
{
name: "proper freq in rrule",
spec: Spec{
FirstEventStart: time.Now(),
Duration: "1m",
Rrule: "FREQ=HOURLY;INTERVAL=1",
Filters: Filters{
SLOs: []SLORef{{
Name: "test",
Project: "project",
}},
},
},
expectedErrors: []testutils.ExpectedError{},
},
{
name: "duplicate slo",
spec: Spec{
Expand Down Expand Up @@ -375,3 +431,114 @@ func validBudgetAdjustment() BudgetAdjustment {
},
}
}

func TestAtLeastHourlyFreq(t *testing.T) {
tests := []struct {
name string
rule string
expectedError string
}{
{
name: "nil rule returns nil error",
rule: "",
expectedError: "",
},
{
name: "frequency less than hourly returns error",
rule: "FREQ=MINUTELY;INTERVAL=1",
expectedError: "interval must be at least 60 minutes for minutely frequency",
},
{
name: "frequency less than hourly returns error",
rule: "FREQ=MINUTELY;INTERVAL=59;COUNT=10",
expectedError: "interval must be at least 60 minutes for minutely frequency",
},
{
name: "frequency less than hourly for single event returns no error",
rule: "FREQ=MINUTELY;INTERVAL=59;COUNT=1",
expectedError: "",
},
{
name: "single occurrence rrule returns no error",
rule: "FREQ=MINUTELY;COUNT=1",
expectedError: "",
},
{
name: "two times minutely occurrence rrule returns error",
rule: "FREQ=MINUTELY;COUNT=2",
expectedError: "interval must be at least 60 minutes for minutely frequency",
},
{
name: "hourly frequency returns no error",
rule: "FREQ=HOURLY;INTERVAL=1",
expectedError: "",
},
{
name: "daily frequency returns no error",
rule: "FREQ=DAILY;INTERVAL=1",
expectedError: "",
},
{
name: "frequency greater than hourly in minutes no error",
rule: "FREQ=MINUTELY;INTERVAL=61;COUNT=10",
expectedError: "",
},
{
name: "frequency greater than hourly in seconds no error",
rule: "FREQ=SECONDLY;INTERVAL=3600;COUNT=10",
expectedError: "",
},
{
name: "frequency greater than hourly in seconds no error",
rule: "FREQ=SECONDLY;INTERVAL=3600",
expectedError: "",
},
{
name: "frequency shorter than hourly in seconds returns error",
rule: "FREQ=SECONDLY;INTERVAL=3500;COUNT=10",
expectedError: "interval must be at least 3600 seconds for secondly frequency",
},
{
name: "minutely with by hour returns error",
rule: "FREQ=MINUTELY;BYHOUR=6,8,9,10,11,12,13,14,15,16;COUNT=10",
expectedError: "interval must be at least 60 minutes for minutely frequency",
},
{
name: "minutely with by hour returns error",
rule: "FREQ=MINUTELY;INTERVAL=10;BYHOUR=6,8,9,10,11,12,13,14,15,16",
expectedError: "interval must be at least 60 minutes for minutely frequency",
},
{
name: "hourly with by second returns error",
rule: "FREQ=HOURLY;BYSECOND=6,8,9,10,11,12,13,14,15,16",
expectedError: "byminute and bysecond are not supported",
},
{
name: "hourly with by minute returns no error",
rule: "FREQ=HOURLY;BYHOUR=6,8,9,10,11,12,13,14,15,16;BYMINUTE=6,8,9,10,11,12,13,14,15,16,59;COUNT=10",
expectedError: "byminute and bysecond are not supported",
},
{
name: "single by minute is supported",
rule: "FREQ=HOURLY;BYMINUTE=6;COUNT=10",
expectedError: "",
kubaceg marked this conversation as resolved.
Show resolved Hide resolved
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var rule *rrule.RRule
var err error
if tt.rule != "" {
rule, err = rrule.StrToRRule(tt.rule)
assert.NoError(t, err)
}
err = atLeastHourlyFreq.Validate(rule)
if tt.expectedError == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, tt.expectedError)
}
})
}
}
Loading