Update HTTPRoute BackendRef docs to clarify error behavior (#1243)

* Update HTTPRoute BackendRef docs to clarify error behavior Signed-off-by: Nick Young <ynick@vmware.com> * Add new ResolvedRefs reasons Signed-off-by: Nick Young <ynick@vmware.com> * Add more invalid BackendRef conformance tests Signed-off-by: Nick Young <ynick@vmware.com> * Update conformance tests Signed-off-by: Nick Young <ynick@vmware.com> * Fix PR comments and update v1beta1 resources Signed-off-by: Nick Young <ynick@vmware.com> * Update generated YAML Signed-off-by: Nick Young <ynick@vmware.com> * Update conformance tests with Reasons Signed-off-by: Nick Young <ynick@vmware.com> * Fix spelling Signed-off-by: Nick Young <ynick@vmware.com> * UFix PR comments Signed-off-by: Nick Young <ynick@vmware.com> * Fix some more PR comments Signed-off-by: Nick Young <ynick@vmware.com> * Update helpers with PR feedback Signed-off-by: Nick Young <ynick@vmware.com> * Fixing initialization for new conformance tests Co-authored-by: Rob Scott <robertjscott@google.com>
kubernetes-sigs · Jul 12, 2022 · 30f2983 · 30f2983
1 parent 5e8421c
commit 30f2983
Show file tree

Hide file tree

Showing 16 changed files with 414 additions and 194 deletions.
diff --git a/apis/v1alpha2/httproute_types.go b/apis/v1alpha2/httproute_types.go
@@ -202,24 +202,25 @@ type HTTPRouteRule struct {
 	// BackendRefs defines the backend(s) where matching requests should be
 	// sent.
 	//
-	// A 500 status code MUST be returned if there are no BackendRefs or filters
-	// specified that would result in a response being sent.
+	// Failure behavior here depends on how many BackendRefs are specified and
+	// how many are invalid.
 	//
-	// A BackendRef is considered invalid when it refers to:
+	// If *all* entries in BackendRefs are invalid, and there are also no filters
+	// specified in this route rule, *all* traffic which matches this rule MUST
+	// receive a 500 status code.
 	//
-	// * an unknown or unsupported kind of resource
-	// * a resource that does not exist
-	// * a resource in another namespace when the reference has not been
-	//   explicitly allowed by a ReferenceGrant (or equivalent concept).
+	// See the HTTPBackendRef definition for the rules about what makes a single
+	// HTTPBackendRef invalid.
 	//
-	// When a BackendRef is invalid, 500 status codes MUST be returned for
+	// When a HTTPBackendRef is invalid, 500 status codes MUST be returned for
 	// requests that would have otherwise been routed to an invalid backend. If
 	// multiple backends are specified, and some are invalid, the proportion of
 	// requests that would otherwise have been routed to an invalid backend
 	// MUST receive a 500 status code.
 	//
-	// When a BackendRef refers to a Service that has no ready endpoints, it is
-	// recommended to return a 503 status code.
+	// For example, if two backends are specified with equal weights, and one is
+	// invalid, 50 percent of traffic must receive a 500. Implementations may
+	// choose how that 50 percent is determined.
 	//
 	// Support: Core for Kubernetes Service
 	//
@@ -933,21 +934,31 @@ type HTTPRequestMirrorFilter struct {
 type HTTPBackendRef struct {
 	// BackendRef is a reference to a backend to forward matched requests to.
 	//
-	// If the referent cannot be found, this HTTPBackendRef is invalid and must
-	// be dropped from the Gateway. The controller must ensure the
-	// "ResolvedRefs" condition on the Route is set to `status: False` and not
-	// configure this backend in the underlying implementation.
+	// A BackendRef can be invalid for the following reasons. In all cases, the
+	// implementation MUST ensure the `ResolvedRefs` Condition on the Route
+	// is set to `status: False`, with a Reason and Message that indicate
+	// what is the cause of the error.
 	//
-	// If there is a cross-namespace reference to an *existing* object
-	// that is not covered by a ReferenceGrant, the controller must ensure the
-	// "ResolvedRefs"  condition on the Route is set to `status: False`,
-	// with the "RefNotPermitted" reason and not configure this backend in the
-	// underlying implementation.
+	// A BackendRef is invalid if:
 	//
-	// In either error case, the Message of the `ResolvedRefs` Condition
-	// should be used to provide more detail about the problem.
+	// * It refers to an unknown or unsupported kind of resource. In this
+	//   case, the Reason must be set to `InvalidKind` and Message of the
+	//   Condition must explain which kind of resource is unknown or unsupported.
+	//
+	// * It refers to a resource that does not exist. In this case, the Reason must
+	//   be set to `BackendNotFound` and the Message of the Condition must explain
+	//   which resource does not exist.
 	//
-	// Support: Custom
+	// * It refers a resource in another namespace when the reference has not been
+	//   explicitly allowed by a ReferenceGrant (or equivalent concept). In this
+	//   case, the Reason must be set to `RefNotPermitted` and the Message of the
+	//   Condition must explain which cross-namespace reference is not allowed.
+	//
+	// Support: Core for Kubernetes Service
+	//
+	// Support: Custom for any other resource
+	//
+	// Support for weight: Core
 	//
 	// +optional
 	BackendRef `json:",inline"`

diff --git a/apis/v1alpha2/shared_types.go b/apis/v1alpha2/shared_types.go
@@ -246,6 +246,15 @@ const (
 	// another namespace, where the object in the other namespace does
 	// not have a ReferenceGrant explicitly allowing the reference.
 	RouteReasonRefNotPermitted RouteConditionReason = "RefNotPermitted"
+
+	// This reason is used with the "ResolvedRefs" condition when
+	// one of the Route's rules has a reference to an unknown or unsupported
+	// Group and/or Kind.
+	RouteReasonInvalidKind RouteConditionReason = "InvalidKind"
+
+	// This reason is used with the "ResolvedRefs" condition when one of the
+	// Route's rules has a reference to a resource that does not exist.
+	RouteReasonBackendNotFound RouteConditionReason = "BackendNotFound"
 )
 
 // RouteParentStatus describes the status of a route with respect to an

diff --git a/apis/v1beta1/httproute_types.go b/apis/v1beta1/httproute_types.go
@@ -201,24 +201,25 @@ type HTTPRouteRule struct {
 	// BackendRefs defines the backend(s) where matching requests should be
 	// sent.
 	//
-	// A 500 status code MUST be returned if there are no BackendRefs or filters
-	// specified that would result in a response being sent.
+	// Failure behavior here depends on how many BackendRefs are specified and
+	// how many are invalid.
 	//
-	// A BackendRef is considered invalid when it refers to:
+	// If *all* entries in BackendRefs are invalid, and there are also no filters
+	// specified in this route rule, *all* traffic which matches this rule MUST
+	// receive a 500 status code.
 	//
-	// * an unknown or unsupported kind of resource
-	// * a resource that does not exist
-	// * a resource in another namespace when the reference has not been
-	//   explicitly allowed by a ReferenceGrant (or equivalent concept).
+	// See the HTTPBackendRef definition for the rules about what makes a single
+	// HTTPBackendRef invalid.
 	//
-	// When a BackendRef is invalid, 500 status codes MUST be returned for
+	// When a HTTPBackendRef is invalid, 500 status codes MUST be returned for
 	// requests that would have otherwise been routed to an invalid backend. If
 	// multiple backends are specified, and some are invalid, the proportion of
 	// requests that would otherwise have been routed to an invalid backend
 	// MUST receive a 500 status code.
 	//
-	// When a BackendRef refers to a Service that has no ready endpoints, it is
-	// recommended to return a 503 status code.
+	// For example, if two backends are specified with equal weights, and one is
+	// invalid, 50 percent of traffic must receive a 500. Implementations may
+	// choose how that 50 percent is determined.
 	//
 	// Support: Core for Kubernetes Service
 	//
@@ -932,21 +933,31 @@ type HTTPRequestMirrorFilter struct {
 type HTTPBackendRef struct {
 	// BackendRef is a reference to a backend to forward matched requests to.
 	//
-	// If the referent cannot be found, this HTTPBackendRef is invalid and must
-	// be dropped from the Gateway. The controller must ensure the
-	// "ResolvedRefs" condition on the Route is set to `status: False` and not
-	// configure this backend in the underlying implementation.
+	// A BackendRef can be invalid for the following reasons. In all cases, the
+	// implementation MUST ensure the `ResolvedRefs` Condition on the Route
+	// is set to `status: False`, with a Reason and Message that indicate
+	// what is the cause of the error.
 	//
-	// If there is a cross-namespace reference to an *existing* object
-	// that is not covered by a ReferenceGrant, the controller must ensure the
-	// "ResolvedRefs"  condition on the Route is set to `status: False`,
-	// with the "RefNotPermitted" reason and not configure this backend in the
-	// underlying implementation.
+	// A BackendRef is invalid if:
 	//
-	// In either error case, the Message of the `ResolvedRefs` Condition
-	// should be used to provide more detail about the problem.
+	// * It refers to an unknown or unsupported kind of resource. In this
+	//   case, the Reason must be set to `InvalidKind` and Message of the
+	//   Condition must explain which kind of resource is unknown or unsupported.
+	//
+	// * It refers to a resource that does not exist. In this case, the Reason must
+	//   be set to `BackendNotFound` and the Message of the Condition must explain
+	//   which resource does not exist.
 	//
-	// Support: Custom
+	// * It refers a resource in another namespace when the reference has not been
+	//   explicitly allowed by a ReferenceGrant (or equivalent concept). In this
+	//   case, the Reason must be set to `RefNotPermitted` and the Message of the
+	//   Condition must explain which cross-namespace reference is not allowed.
+	//
+	// Support: Core for Kubernetes Service
+	//
+	// Support: Custom for any other resource
+	//
+	// Support for weight: Core
 	//
 	// +optional
 	BackendRef `json:",inline"`

diff --git a/apis/v1beta1/shared_types.go b/apis/v1beta1/shared_types.go
@@ -246,6 +246,15 @@ const (
 	// another namespace, where the object in the other namespace does
 	// not have a ReferenceGrant explicitly allowing the reference.
 	RouteReasonRefNotPermitted RouteConditionReason = "RefNotPermitted"
+
+	// This reason is used with the "ResolvedRefs" condition when
+	// one of the Route's rules has a reference to an unknown or unsupported
+	// Group and/or Kind.
+	RouteReasonInvalidKind RouteConditionReason = "InvalidKind"
+
+	// This reason is used with the "ResolvedRefs" condition when one of the
+	// Route's rules has a reference to a resource that does not exist.
+	RouteReasonBackendNotFound RouteConditionReason = "BackendNotFound"
 )
 
 // RouteParentStatus describes the status of a route with respect to an

diff --git a/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml b/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml