Merge pull request #1152 from howardjohn/conformance/route-error

conformance: allow error status in Route

conformance/tests/httproute-disallowed-kind.go

@@ -44,12 +44,8 @@ var HTTPRouteDisallowedKind = suite.ConformanceTest{
 		routeName := types.NamespacedName{Name: "disallowed-kind", Namespace: "gateway-conformance-infra"}
 		gwName := types.NamespacedName{Name: "tlsroutes-only", Namespace: "gateway-conformance-infra"}
 
-		// TODO: Determine if this is actually what we want. It is likely
-		// preferable to have status set with some kind of warning/error message
-		// but that is also unlikely to be universally achievable.
 		t.Run("Route should not have Parents set in status", func(t *testing.T) {
-			parents := []v1alpha2.RouteParentStatus{}
-			kubernetes.HTTPRouteMustHaveParents(t, suite.Client, routeName, parents, true, 60)
+			kubernetes.HTTPRouteMustHaveNoAcceptedParents(t, suite.Client, routeName, 60)
 		})
 
 		t.Run("Gateway should have 0 Routes attached", func(t *testing.T) {

conformance/tests/httproute-invalid-cross-namespace-parent-ref.go

@@ -41,12 +41,8 @@ var HTTPRouteInvalidCrossNamespaceParentRef = suite.ConformanceTest{
 		routeName := types.NamespacedName{Name: "invalid-cross-namespace-parent-ref", Namespace: "gateway-conformance-web-backend"}
 		gwName := types.NamespacedName{Name: "same-namespace", Namespace: "gateway-conformance-infra"}
 
-		// TODO: Determine if this is actually what we want. It is likely
-		// preferable to have status set with some kind of warning/error message
-		// but that is also unlikely to be universally achievable.
 		t.Run("Route should not have Parents set in status", func(t *testing.T) {
-			parents := []v1alpha2.RouteParentStatus{}
-			kubernetes.HTTPRouteMustHaveParents(t, suite.Client, routeName, parents, true, 60)
+			kubernetes.HTTPRouteMustHaveNoAcceptedParents(t, suite.Client, routeName, 60)
 		})
 
 		t.Run("Gateway should have 0 Routes attached", func(t *testing.T) {

conformance/utils/kubernetes/helpers.go

@@ -32,7 +32,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
 
@@ -177,6 +176,47 @@ func WaitForGatewayAddress(t *testing.T, client client.Client, gwName types.Name
 	return net.JoinHostPort(ipAddr, port), waitErr
 }
 
+// HTTPRouteMustHaveNoAcceptedParents waits for the specified HTTPRoute to have either no parents
+// or a single parent that is not accepted. This is used to validate HTTPRoute errors.
+func HTTPRouteMustHaveNoAcceptedParents(t *testing.T, client client.Client, routeName types.NamespacedName, seconds int) {
+	t.Helper()
+
+	var actual []v1alpha2.RouteParentStatus
+	waitFor := time.Duration(seconds) * time.Second
+	emptyChecked := false
+	waitErr := wait.PollImmediate(1*time.Second, waitFor, func() (bool, error) {
+		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+		defer cancel()
+
+		route := &v1alpha2.HTTPRoute{}
+		err := client.Get(ctx, routeName, route)
+		if err != nil {
+			return false, fmt.Errorf("error fetching HTTPRoute: %w", err)
+		}
+
+		actual = route.Status.Parents
+
+		if len(actual) == 0 {
+			// For empty status, we need to distinguish between "correctly did not set" and "hasn't set yet"
+			// Ensure we iterate at least two times (taking advantage of the 1s poll delay) to give it some time.
+			if !emptyChecked {
+				emptyChecked = true
+				return false, nil
+			}
+			return true, nil
+		}
+		if len(actual) > 1 {
+			// Only expect one parent
+			return false, nil
+		}
+		return conditionsMatch(t, []metav1.Condition{{
+			Type:   string(v1alpha2.RouteConditionAccepted),
+			Status: "False",
+		}}, actual[0].Conditions), nil
+	})
+	require.NoErrorf(t, waitErr, "error waiting for HTTPRoute to have no accepted parents")
+}
+
 // HTTPRouteMustHaveParents waits for the specified HTTPRoute to have parents
 // in status that match the expected parents. This will cause the test to halt
 // if the specified timeout is exceeded.

go.mod

@@ -5,8 +5,8 @@ go 1.18
 require (
 	github.com/ahmetb/gen-crd-api-reference-docs v0.3.0
 	github.com/lithammer/dedent v1.1.0
-	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/stretchr/testify v1.7.0
+	golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4
 	k8s.io/api v0.22.4
 	k8s.io/apiextensions-apiserver v0.22.4
 	k8s.io/apimachinery v0.22.4
@@ -47,10 +47,10 @@ require (
 	github.com/modern-go/reflect2 v1.0.1 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/spf13/cobra v1.2.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 // indirect
-	golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4 // indirect
 	golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57 // indirect
 	golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f // indirect
 	golang.org/x/oauth2 v0.0.0-20210402161424-2e8d93401602 // indirect