Skip to content

Commit

Permalink
Fix CEL tests for v1.28+ (kubernetes-sigs#3325)
Browse files Browse the repository at this point in the history
  • Loading branch information
robscott authored and mlavacca committed Oct 28, 2024
1 parent 4d0cf60 commit b7005fc
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 21 deletions.
3 changes: 1 addition & 2 deletions pkg/test/cel/backendlbpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package main
import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -122,7 +121,7 @@ func validateBackendLBPolicy(t *testing.T, lbPolicy *gatewayv1a2.BackendLBPolicy

var missingErrorStrings []string
for _, wantError := range wantErrors {
if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) {
if !celErrorStringMatches(err.Error(), wantError) {
missingErrorStrings = append(missingErrorStrings, wantError)
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/test/cel/backendtlspolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package main
import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -144,7 +143,7 @@ func validateBackendTLSPolicy(t *testing.T, route *gatewayv1a3.BackendTLSPolicy,

var missingErrorStrings []string
for _, wantError := range wantErrors {
if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) {
if !celErrorStringMatches(err.Error(), wantError) {
missingErrorStrings = append(missingErrorStrings, wantError)
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/test/cel/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -608,7 +607,7 @@ func TestValidateGateway(t *testing.T) {

var missingErrorStrings []string
for _, wantError := range tc.wantErrors {
if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) {
if !celErrorStringMatches(err.Error(), wantError) {
missingErrorStrings = append(missingErrorStrings, wantError)
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/test/cel/gatewayclass_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -72,7 +71,7 @@ func TestValidateGatewayClassUpdate(t *testing.T) {
if (tc.wantError != "") != (err != nil) {
t.Fatalf("Unexpected error while updating GatewayClass; got err=\n%v\n;want error=%v", err, tc.wantError != "")
}
if tc.wantError != "" && !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(tc.wantError)) {
if tc.wantError != "" && !celErrorStringMatches(err.Error(), tc.wantError) {
t.Fatalf("Unexpected error while updating GatewayClass; got err=\n%v\n;want substring within error=%q", err, tc.wantError)
}
})
Expand Down
3 changes: 1 addition & 2 deletions pkg/test/cel/grpcroute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package main
import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -425,7 +424,7 @@ func validateGRPCRoute(t *testing.T, route *gatewayv1.GRPCRoute, wantErrors []st

var missingErrorStrings []string
for _, wantError := range wantErrors {
if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) {
if !celErrorStringMatches(err.Error(), wantError) {
missingErrorStrings = append(missingErrorStrings, wantError)
}
}
Expand Down
23 changes: 12 additions & 11 deletions pkg/test/cel/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -75,7 +74,7 @@ func TestHTTPPathMatch(t *testing.T) {
},
{
name: "invalid type",
wantErrors: []string{"type must be one of ['Exact', 'PathPrefix', 'RegularExpression']"},
wantErrors: []string{"must be one of ['Exact', 'PathPrefix', 'RegularExpression']"},
path: &gatewayv1.HTTPPathMatch{
Type: ptrTo(gatewayv1.PathMatchType("FooBar")),
Value: ptrTo("/path"),
Expand Down Expand Up @@ -739,13 +738,13 @@ func TestHTTPRouteRule(t *testing.T) {
}},
},
{
name: "invalid URLRewrite filter because too many path matches",
name: "invalid URLRewrite filter because wrong path match type",
wantErrors: []string{"When using URLRewrite filter with path.replacePrefixMatch, exactly one PathPrefix match must be specified"},
rules: []gatewayv1.HTTPRouteRule{{
Matches: []gatewayv1.HTTPRouteMatch{
{
Path: &gatewayv1.HTTPPathMatch{
Type: ptrTo(gatewayv1.PathMatchType(gatewayv1.FullPathHTTPPathModifier)), // Incorrect Patch match Type for URLRewrite filter with ReplacePrefixMatch.
Type: ptrTo(gatewayv1.PathMatchRegularExpression), // Incorrect Path match Type for URLRewrite filter with ReplacePrefixMatch.
Value: ptrTo("/foo"),
},
},
Expand Down Expand Up @@ -813,13 +812,13 @@ func TestHTTPRouteRule(t *testing.T) {
}},
},
{
name: "invalid RequestRedirect filter because path match has type ReplaceFullPath",
name: "invalid RequestRedirect filter because path match has type RegularExpression",
wantErrors: []string{"When using RequestRedirect filter with path.replacePrefixMatch, exactly one PathPrefix match must be specified"},
rules: []gatewayv1.HTTPRouteRule{{
Matches: []gatewayv1.HTTPRouteMatch{
{
Path: &gatewayv1.HTTPPathMatch{
Type: ptrTo(gatewayv1.PathMatchType(gatewayv1.FullPathHTTPPathModifier)), // Incorrect Patch match Type for RequestRedirect filter with ReplacePrefixMatch.
Type: ptrTo(gatewayv1.PathMatchRegularExpression), // Incorrect Path match Type for RequestRedirect filter with ReplacePrefixMatch.
Value: ptrTo("/foo"),
},
},
Expand Down Expand Up @@ -907,13 +906,13 @@ func TestHTTPRouteRule(t *testing.T) {
}},
},
{
name: "invalid URLRewrite filter (within backendRefs) because path match has type ReplaceFullPath",
name: "invalid URLRewrite filter (within backendRefs) because path match has type RegularExpression",
wantErrors: []string{"Within backendRefs, When using URLRewrite filter with path.replacePrefixMatch, exactly one PathPrefix match must be specified"},
rules: []gatewayv1.HTTPRouteRule{{
Matches: []gatewayv1.HTTPRouteMatch{
{
Path: &gatewayv1.HTTPPathMatch{
Type: ptrTo(gatewayv1.PathMatchType(gatewayv1.FullPathHTTPPathModifier)), // Incorrect Patch match Type for URLRewrite filter with ReplacePrefixMatch.
Type: ptrTo(gatewayv1.PathMatchRegularExpression), // Incorrect Path match Type for URLRewrite filter with ReplacePrefixMatch.
Value: ptrTo("/foo"),
},
},
Expand Down Expand Up @@ -1011,13 +1010,13 @@ func TestHTTPRouteRule(t *testing.T) {
}},
},
{
name: "invalid RequestRedirect filter (within backendRefs) because path match has type ReplaceFullPath",
name: "invalid RequestRedirect filter (within backendRefs) because path match has type RegularExpression",
wantErrors: []string{"Within backendRefs, when using RequestRedirect filter with path.replacePrefixMatch, exactly one PathPrefix match must be specified"},
rules: []gatewayv1.HTTPRouteRule{{
Matches: []gatewayv1.HTTPRouteMatch{
{
Path: &gatewayv1.HTTPPathMatch{
Type: ptrTo(gatewayv1.PathMatchType(gatewayv1.FullPathHTTPPathModifier)), // Incorrect Patch match Type for RequestRedirect filter with ReplacePrefixMatch.
Type: ptrTo(gatewayv1.PathMatchRegularExpression), // Incorrect Path match Type for RequestRedirect filter with ReplacePrefixMatch.
Value: ptrTo("/foo"),
},
},
Expand Down Expand Up @@ -1318,6 +1317,7 @@ func TestHTTPPathModifier(t *testing.T) {
name: "type must be 'ReplaceFullPath' when replaceFullPath is set",
wantErrors: []string{"type must be 'ReplaceFullPath' when replaceFullPath is set"},
pathModifier: gatewayv1.HTTPPathModifier{
Type: gatewayv1.PrefixMatchHTTPPathModifier,
ReplaceFullPath: ptrTo("foo"),
},
},
Expand All @@ -1339,6 +1339,7 @@ func TestHTTPPathModifier(t *testing.T) {
name: "type must be 'ReplacePrefixMatch' when replacePrefixMatch is set",
wantErrors: []string{"type must be 'ReplacePrefixMatch' when replacePrefixMatch is set"},
pathModifier: gatewayv1.HTTPPathModifier{
Type: gatewayv1.FullPathHTTPPathModifier,
ReplacePrefixMatch: ptrTo("/foo"),
},
},
Expand Down Expand Up @@ -1384,7 +1385,7 @@ func validateHTTPRoute(t *testing.T, route *gatewayv1.HTTPRoute, wantErrors []st

var missingErrorStrings []string
for _, wantError := range wantErrors {
if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) {
if !celErrorStringMatches(err.Error(), wantError) {
missingErrorStrings = append(missingErrorStrings, wantError)
}
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/test/cel/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"os"
"path"
"strings"
"testing"

v1 "sigs.k8s.io/gateway-api/apis/v1"
Expand Down Expand Up @@ -59,3 +60,26 @@ func TestMain(m *testing.M) {
func ptrTo[T any](a T) *T {
return &a
}

func celErrorStringMatches(got, want string) bool {
gotL := strings.ToLower(got)
wantL := strings.ToLower(want)

// Starting in k8s v1.28, CEL error messages stopped adding spec and status prefixes to path names
wantLAdjusted := strings.ReplaceAll(wantL, "spec.", "")
wantLAdjusted = strings.ReplaceAll(wantLAdjusted, "status.", "")

// Enum validation messages changed in k8s v1.28:
// Before: must be one of ['Exact', 'PathPrefix', 'RegularExpression']
// After: supported values: "Exact", "PathPrefix", "RegularExpression"
if strings.Contains(wantLAdjusted, "must be one of") {
r := strings.NewReplacer(
"must be one of", "supported values:",
"[", "",
"]", "",
"'", "\"",
)
wantLAdjusted = r.Replace(wantLAdjusted)
}
return strings.Contains(gotL, wantL) || strings.Contains(gotL, wantLAdjusted)
}

0 comments on commit b7005fc

Please sign in to comment.