[release-1.1] Backport CI improvements (#3417)

* Fix CEL tests for v1.28+ (#3325)

* feat: validate CRDs on specific k8s versions (#3316)

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* finalize verify->crds-validation migration (#3330)

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* Fixing kind v1.30 image ref (#3329)

---------

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>
Co-authored-by: Rob Scott <robertjscott@google.com>

Makefile

@@ -83,6 +83,11 @@ test:
 	cd "conformance/echo-basic" && go test -race -cover ./...
 	cd "gwctl" && go test -race -cover ./...
 
+# Run tests for CRDs validation
+.PHONY: test.crds-validation
+test.crds-validation:
+	./hack/test-crds-validation.sh $(VERSION)
+
 # Run conformance tests against controller implementation
 .PHONY: conformance
 conformance:

hack/verify-crds-kind.sh → hack/test-crds-validation.sh

@@ -41,15 +41,49 @@ cleanup() {
 
 trap cleanup INT TERM EXIT
 
+# TODO(mlavacca): find a good way to keep this dependency up to date.
+KIND_VERSION="v0.24.0"
+
+# list of kind images taken from https://github.com/kubernetes-sigs/kind/releases/tag/v0.24.0.
+# they need to be updated when kind is updated.
+KIND_IMAGES=(
+  "kindest/node:v1.27.17@sha256:3fd82731af34efe19cd54ea5c25e882985bafa2c9baefe14f8deab1737d9fabe"
+  "kindest/node:v1.28.13@sha256:45d319897776e11167e4698f6b14938eb4d52eb381d9e3d7a9086c16c69a8110"
+  "kindest/node:v1.29.8@sha256:d46b7aa29567e93b27f7531d258c372e829d7224b25e3fc6ffdefed12476d3aa"
+  "kindest/node:v1.30.4@sha256:976ea815844d5fa93be213437e3ff5754cd599b040946b5cca43ca45c2047114"
+  "kindest/node:v1.31.0@sha256:53df588e04085fd41ae12de0c3fe4c72f7013bba32a20e7325357a1ac94ba865"
+)
+
+if [ "$#" -gt 1 ]; then
+    echo "Error: Too many arguments provided. Only 1 argument is allowed."
+    exit 1
+fi
+
+DEFAULT_INDEX=$((1))
+
+if [ "$#" -eq 1 ]; then
+  # Check if the argument is a valid number between 1 and 5
+  if ! [[ "$1" =~ ^[1-5] ]]; then
+      echo "Error: Argument is not a valid integer between 1 and 5."
+      exit 1
+  fi
+  INDEX=$(($1))
+else
+  INDEX=$((DEFAULT_INDEX))
+fi
+
+K8S_IMAGE=${KIND_IMAGES[$((INDEX-1))]}
+echo "Using Kubernetes image: ${K8S_IMAGE}"
+
 # For exit code
 res=0
 
 # Install kind
-(cd $GOPATH && go install sigs.k8s.io/kind@v0.20.0) || res=$?
+(cd "${GOPATH}" && go install sigs.k8s.io/kind@${KIND_VERSION}) || res=$?
 
 # Create cluster
 KIND_CREATE_ATTEMPTED=true
-kind create cluster --name "${CLUSTER_NAME}"
+kind create cluster --name "${CLUSTER_NAME}" --image "${K8S_IMAGE}" || res=$?
 
 # Verify CEL validation
 for CHANNEL in experimental standard; do

hack/verify-all.sh

@@ -53,10 +53,12 @@ fi
 
 EXCLUDE="verify-all.sh"
 
+SCRIPTS=$(find "${SCRIPT_ROOT}"/hack -name "verify-*.sh")
+
 ret=0
-for t in `ls $SCRIPT_ROOT/hack/verify-*.sh`
+for t in $SCRIPTS;
 do
-  if is-excluded $t ; then
+  if is-excluded "${t}" ; then
     echo "Skipping $t"
     continue
   fi

pkg/test/cel/backendlbpolicy_test.go

@@ -22,7 +22,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -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)
 		}
 	}

pkg/test/cel/backendtlspolicy_test.go

@@ -22,7 +22,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -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)
 		}
 	}

pkg/test/cel/gateway_test.go

@@ -19,7 +19,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -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)
 				}
 			}

pkg/test/cel/gatewayclass_test.go

@@ -19,7 +19,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -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)
 			}
 		})

pkg/test/cel/grpcroute_test.go

@@ -22,7 +22,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -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)
 		}
 	}

pkg/test/cel/httproute_test.go

@@ -19,7 +19,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -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"),
@@ -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"),
 						},
 					},
@@ -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"),
 						},
 					},
@@ -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"),
 						},
 					},
@@ -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"),
 						},
 					},
@@ -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"),
 			},
 		},
@@ -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"),
 			},
 		},
@@ -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)
 		}
 	}

pkg/test/cel/main_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"strings"
 	"testing"
 
 	v1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -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)
+}