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

v1alpha2 validation fix/update #831

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,5 @@ issues:
- gocyclo
- errcheck
- dupl
exclude:
- Using the variable on range scope `tc` in function literal
21 changes: 9 additions & 12 deletions apis/v1alpha1/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,13 +405,11 @@ func TestValidateHTTPRoute(t *testing.T) {
errCount: 0,
},
}
for _, tt := range tests {
// copy variable to avoid scope problems with ranges
tt := tt
t.Run(tt.name, func(t *testing.T) {
errs := ValidateHTTPRoute(&tt.hRoute)
if len(errs) != tt.errCount {
t.Errorf("ValidateHTTPRoute() got %v errors, want %v errors", len(errs), tt.errCount)
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
errs := ValidateHTTPRoute(&tc.hRoute)
if len(errs) != tc.errCount {
t.Errorf("ValidateHTTPRoute() got %v errors, want %v errors", len(errs), tc.errCount)
}
})
}
Expand Down Expand Up @@ -490,11 +488,10 @@ func TestValidateGatewayClassUpdate(t *testing.T) {
want: nil,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
if got := ValidateGatewayClassUpdate(tt.args.oldClass, tt.args.newClass); !reflect.DeepEqual(got, tt.want) {
t.Errorf("ValidateGatewayClassUpdate() = %v, want %v", got, tt.want)
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := ValidateGatewayClassUpdate(tc.args.oldClass, tc.args.newClass); !reflect.DeepEqual(got, tc.want) {
t.Errorf("ValidateGatewayClassUpdate() = %v, want %v", got, tc.want)
}
})
}
Expand Down
1 change: 0 additions & 1 deletion apis/v1alpha2/validation/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ func TestValidateGateway(t *testing.T) {
}

for name, tc := range testCases {
tc := tc
t.Run(name, func(t *testing.T) {
gw := baseGateway.DeepCopy()
tc.mutate(gw)
Expand Down
9 changes: 4 additions & 5 deletions apis/v1alpha2/validation/gatewayclass_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,10 @@ func TestValidateGatewayClassUpdate(t *testing.T) {
want: nil,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
if got := ValidateGatewayClassUpdate(tt.args.oldClass, tt.args.newClass); !reflect.DeepEqual(got, tt.want) {
t.Errorf("ValidateGatewayClassUpdate() = %v, want %v", got, tt.want)
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := ValidateGatewayClassUpdate(tc.args.oldClass, tc.args.newClass); !reflect.DeepEqual(got, tc.want) {
t.Errorf("ValidateGatewayClassUpdate() = %v, want %v", got, tc.want)
}
})
}
Expand Down
33 changes: 31 additions & 2 deletions apis/v1alpha2/validation/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ func ValidateHTTPRoute(route *gatewayv1a2.HTTPRoute) field.ErrorList {
// validateHTTPRouteSpec validates that required fields of spec are set according to the
// HTTPRoute specification.
func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) field.ErrorList {
return validateHTTPRouteUniqueFilters(spec.Rules, path.Child("rules"))
if errList := validateHTTPRouteUniqueFilters(spec.Rules, path.Child("rules")); len(errList) > 0 {
return errList
}

return nil
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
}

// validateHTTPRouteUniqueFilters validates whether each core and extended filter
Expand All @@ -55,7 +59,7 @@ func validateHTTPRouteUniqueFilters(rules []gatewayv1a2.HTTPRouteRule, path *fie
}
// custom filters don't have any validation
for _, key := range repeatableHTTPRouteFilters {
counts[key] = 0
delete(counts, key)
}

for filterType, count := range counts {
Expand All @@ -64,7 +68,32 @@ func validateHTTPRouteUniqueFilters(rules []gatewayv1a2.HTTPRouteRule, path *fie
}
}

if errList := validateHTTPBackendUniqueFilters(rule.BackendRefs, path, i); len(errList) > 0 {
errs = append(errs, errList...)
}
}

return errs
}

func validateHTTPBackendUniqueFilters(ref []gatewayv1a2.HTTPBackendRef, path *field.Path, i int) field.ErrorList {
var errs field.ErrorList

for _, bkr := range ref {
counts := map[gatewayv1a2.HTTPRouteFilterType]int{}
for _, filter := range bkr.Filters {
counts[filter.Type]++
}

for _, key := range repeatableHTTPRouteFilters {
delete(counts, key)
}

for filterType, count := range counts {
if count > 1 {
errs = append(errs, field.Invalid(path.Index(i).Child("BackendRefs"), filterType, "cannot be used multiple times in the same backend"))
}
}
}
return errs
}
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
Loading