Set Accepted condition type on Gateway status (#633)

* Set Accepted condition type on Gateway status We always report the Accepted condition on the Gateway status. Its value depends on the validity of its spec and its Listeners. If the Gateway spec is invalid, we report the Accepted/False/Invalid Gateway condition but do not report Listener statuses. In all other cases, the Listener statuses will be reported. For routes that reference an invalid Gateway, we set the Route condition Accepted/False/InvalidGateway. * Reduce the complexity of bindRouteToListeners The gocyclo linter complained about the complexity of the bindRouteToListeners function. This commit refactors this function to reduce its complexity.
nginx · May 17, 2023 · be51e36 · be51e36
1 parent e4cb606
commit be51e36
Show file tree

Hide file tree

Showing 18 changed files with 830 additions and 548 deletions.
diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md
@@ -71,7 +71,13 @@ Fields:
 	* `name` - supported.
 	* `supportedKinds` - not supported.
 	* `attachedRoutes` - supported.
-	* `conditions` - partially supported.
+	* `conditions` - Supported (Condition/Status/Reason):
+      * `Accepted/True/Accepted`
+      * `Accepted/True/ListenersNotValid`
+      * `Accepted/False/Invalid`
+      * `Accepted/False/ListenersNotValid`
+      * `Accepted/False/UnsupportedValue`: Custom reason for when a value of a field in a Gateway is invalid or not supported.
+      * `Accepted/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway.
 
 ### HTTPRoute
 

diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go
diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go
@@ -8,20 +8,32 @@ import (
 )
 
 const (
-	// RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener.
-	RouteReasonInvalidListener v1beta1.RouteConditionReason = "InvalidListener"
-
 	// ListenerReasonUnsupportedValue is used with the "Accepted" condition when a value of a field in a Listener
 	// is invalid or not supported.
 	ListenerReasonUnsupportedValue v1beta1.ListenerConditionReason = "UnsupportedValue"
 
-	// ListenerReasonNoValidGatewayClass is used with the "Accepted" condition when there is no valid GatewayClass
-	// in the cluster.
-	ListenerReasonNoValidGatewayClass v1beta1.ListenerConditionReason = "NoValidGatewayClass"
-
 	// RouteReasonBackendRefUnsupportedValue is used with the "ResolvedRefs" condition when one of the
 	// Route rules has a backendRef with an unsupported value.
 	RouteReasonBackendRefUnsupportedValue = "UnsupportedValue"
+
+	// RouteReasonInvalidGateway is used with the "Accepted" (false) condition when the Gateway the Route
+	// references is invalid.
+	RouteReasonInvalidGateway = "InvalidGateway"
+
+	// RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener.
+	RouteReasonInvalidListener v1beta1.RouteConditionReason = "InvalidListener"
+
+	// GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from,
+	// and we ignored the resource in question and picked another Gateway as the winner.
+	// This reason is used with GatewayConditionAccepted (false).
+	GatewayReasonGatewayConflict v1beta1.GatewayConditionReason = "GatewayConflict"
+
+	// GatewayMessageGatewayConflict is message that describes GatewayReasonGatewayConflict.
+	GatewayMessageGatewayConflict = "The resource is ignored due to a conflicting Gateway resource"
+
+	// GatewayReasonUnsupportedValue is used with GatewayConditionAccepted (false) when a value of a field in a Gateway
+	// is invalid or not supported.
+	GatewayReasonUnsupportedValue v1beta1.GatewayConditionReason = "UnsupportedValue"
 )
 
 // Condition defines a condition to be reported in the status of resources.
@@ -64,6 +76,16 @@ func DeduplicateConditions(conds []Condition) []Condition {
 	return result
 }
 
+// NewTODO returns a Condition that can be used as a placeholder for a condition that is not yet implemented.
+func NewTODO(msg string) Condition {
+	return Condition{
+		Type:    "TODO",
+		Status:  metav1.ConditionTrue,
+		Reason:  "TODO",
+		Message: fmt.Sprintf("The condition for this has not been implemented yet: %s", msg),
+	}
+}
+
 // NewDefaultRouteConditions returns the default conditions that must be present in the status of an HTTPRoute.
 func NewDefaultRouteConditions() []Condition {
 	return []Condition{
@@ -103,16 +125,6 @@ func NewRouteUnsupportedValue(msg string) Condition {
 	}
 }
 
-// NewTODO returns a Condition that can be used as a placeholder for a condition that is not yet implemented.
-func NewTODO(msg string) Condition {
-	return Condition{
-		Type:    "TODO",
-		Status:  metav1.ConditionTrue,
-		Reason:  "TODO",
-		Message: fmt.Sprintf("The condition for this has not been implemented yet: %s", msg),
-	}
-}
-
 // NewRouteInvalidListener returns a Condition that indicates that the HTTPRoute is not accepted because of an
 // invalid listener.
 func NewRouteInvalidListener() Condition {
@@ -124,6 +136,80 @@ func NewRouteInvalidListener() Condition {
 	}
 }
 
+// NewRouteResolvedRefs returns a Condition that indicates that all the references on the Route are resolved.
+func NewRouteResolvedRefs() Condition {
+	return Condition{
+		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Status:  metav1.ConditionTrue,
+		Reason:  string(v1beta1.RouteReasonResolvedRefs),
+		Message: "All references are resolved",
+	}
+}
+
+// NewRouteBackendRefInvalidKind returns a Condition that indicates that the Route has a backendRef with an
+// invalid kind.
+func NewRouteBackendRefInvalidKind(msg string) Condition {
+	return Condition{
+		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1beta1.RouteReasonInvalidKind),
+		Message: msg,
+	}
+}
+
+// NewRouteBackendRefRefNotPermitted returns a Condition that indicates that the Route has a backendRef that
+// is not permitted.
+func NewRouteBackendRefRefNotPermitted(msg string) Condition {
+	return Condition{
+		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1beta1.RouteReasonRefNotPermitted),
+		Message: msg,
+	}
+}
+
+// NewRouteBackendRefRefBackendNotFound returns a Condition that indicates that the Route has a backendRef that
+// points to non-existing backend.
+func NewRouteBackendRefRefBackendNotFound(msg string) Condition {
+	return Condition{
+		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1beta1.RouteReasonBackendNotFound),
+		Message: msg,
+	}
+}
+
+// NewRouteBackendRefUnsupportedValue returns a Condition that indicates that the Route has a backendRef with
+// an unsupported value.
+func NewRouteBackendRefUnsupportedValue(msg string) Condition {
+	return Condition{
+		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Status:  metav1.ConditionFalse,
+		Reason:  RouteReasonBackendRefUnsupportedValue,
+		Message: msg,
+	}
+}
+
+// NewRouteInvalidGateway returns a Condition that indicates that the Route is not Accepted because the Gateway it
+// references is invalid.
+func NewRouteInvalidGateway() Condition {
+	return Condition{
+		Type:    string(v1beta1.RouteConditionAccepted),
+		Status:  metav1.ConditionFalse,
+		Reason:  RouteReasonInvalidGateway,
+		Message: "Gateway is invalid",
+	}
+}
+
+// NewDefaultListenerConditions returns the default Conditions that must be present in the status of a Listener.
+func NewDefaultListenerConditions() []Condition {
+	return []Condition{
+		NewListenerAccepted(),
+		NewListenerResolvedRefs(),
+		NewListenerNoConflicts(),
+	}
+}
+
 // NewListenerPortUnavailable returns a Condition that indicates a port is unavailable in a Listener.
 func NewListenerPortUnavailable(msg string) Condition {
 	return Condition{
@@ -164,15 +250,6 @@ func NewListenerNoConflicts() Condition {
 	}
 }
 
-// NewDefaultListenerConditions returns the default Conditions that must be present in the status of a Listener.
-func NewDefaultListenerConditions() []Condition {
-	return []Condition{
-		NewListenerAccepted(),
-		NewListenerResolvedRefs(),
-		NewListenerNoConflicts(),
-	}
-}
-
 // NewListenerUnsupportedValue returns a Condition that indicates that a field of a Listener has an unsupported value.
 // Unsupported means that the value is not supported by the implementation or invalid.
 func NewListenerUnsupportedValue(msg string) Condition {
@@ -230,89 +307,95 @@ func NewListenerUnsupportedProtocol(msg string) Condition {
 	}
 }
 
-// NewListenerNoValidGatewayClass returns a Condition that indicates that the Listener is not accepted because
-// there is no valid GatewayClass.
-func NewListenerNoValidGatewayClass(msg string) Condition {
-	return Condition{
-		Type:    string(v1beta1.ListenerConditionAccepted),
-		Status:  metav1.ConditionFalse,
-		Reason:  string(ListenerReasonNoValidGatewayClass),
-		Message: msg,
+// NewDefaultGatewayClassConditions returns the default Conditions that must be present in the status of a GatewayClass.
+func NewDefaultGatewayClassConditions() []Condition {
+	return []Condition{
+		{
+			Type:    string(v1beta1.GatewayClassConditionStatusAccepted),
+			Status:  metav1.ConditionTrue,
+			Reason:  string(v1beta1.GatewayClassReasonAccepted),
+			Message: "GatewayClass is accepted",
+		},
 	}
 }
 
-// NewRouteBackendRefInvalidKind returns a Condition that indicates that the Route has a backendRef with an
-// invalid kind.
-func NewRouteBackendRefInvalidKind(msg string) Condition {
+// NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters.
+func NewGatewayClassInvalidParameters(msg string) Condition {
 	return Condition{
-		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Type:    string(v1beta1.GatewayClassConditionStatusAccepted),
 		Status:  metav1.ConditionFalse,
-		Reason:  string(v1beta1.RouteReasonInvalidKind),
+		Reason:  string(v1beta1.GatewayClassReasonInvalidParameters),
 		Message: msg,
 	}
 }
 
-// NewRouteBackendRefRefNotPermitted returns a Condition that indicates that the Route has a backendRef that
-// is not permitted.
-func NewRouteBackendRefRefNotPermitted(msg string) Condition {
-	return Condition{
-		Type:    string(v1beta1.RouteConditionResolvedRefs),
-		Status:  metav1.ConditionFalse,
-		Reason:  string(v1beta1.RouteReasonRefNotPermitted),
-		Message: msg,
+// NewDefaultGatewayConditions returns the default Condition that must be present in the status of a Gateway.
+func NewDefaultGatewayConditions() []Condition {
+	return []Condition{
+		NewGatewayAccepted(),
 	}
 }
 
-// NewRouteBackendRefRefBackendNotFound returns a Condition that indicates that the Route has a backendRef that
-// points to non-existing backend.
-func NewRouteBackendRefRefBackendNotFound(msg string) Condition {
+// NewGatewayAccepted returns a Condition that indicates the Gateway is accepted.
+func NewGatewayAccepted() Condition {
 	return Condition{
-		Type:    string(v1beta1.RouteConditionResolvedRefs),
-		Status:  metav1.ConditionFalse,
-		Reason:  string(v1beta1.RouteReasonBackendNotFound),
-		Message: msg,
+		Type:    string(v1beta1.GatewayConditionAccepted),
+		Status:  metav1.ConditionTrue,
+		Reason:  string(v1beta1.GatewayReasonAccepted),
+		Message: "Gateway is accepted",
 	}
 }
 
-// NewRouteBackendRefUnsupportedValue returns a Condition that indicates that the Route has a backendRef with
-// an unsupported value.
-func NewRouteBackendRefUnsupportedValue(msg string) Condition {
+// NewGatewayConflict returns a Condition that indicates the Gateway has a conflict with another Gateway.
+func NewGatewayConflict() Condition {
 	return Condition{
-		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Type:    string(v1beta1.GatewayConditionAccepted),
 		Status:  metav1.ConditionFalse,
-		Reason:  RouteReasonBackendRefUnsupportedValue,
-		Message: msg,
+		Reason:  string(GatewayReasonGatewayConflict),
+		Message: GatewayMessageGatewayConflict,
 	}
 }
 
-// NewRouteResolvedRefs returns a Condition that indicates that all the references on the Route are resolved.
-func NewRouteResolvedRefs() Condition {
+// NewGatewayAcceptedListenersNotValid returns a Condition that indicates the Gateway is accepted,
+// but has at least one listener that is invalid.
+func NewGatewayAcceptedListenersNotValid() Condition {
 	return Condition{
-		Type:    string(v1beta1.RouteConditionResolvedRefs),
+		Type:    string(v1beta1.GatewayConditionAccepted),
 		Status:  metav1.ConditionTrue,
-		Reason:  string(v1beta1.RouteReasonResolvedRefs),
-		Message: "All references are resolved",
+		Reason:  string(v1beta1.GatewayReasonListenersNotValid),
+		Message: "Gateway has at least one valid listener",
 	}
 }
 
-// NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters.
-func NewGatewayClassInvalidParameters(msg string) Condition {
+// NewGatewayNotAcceptedListenersNotValid returns a Condition that indicates the Gateway is not accepted,
+// because all listeners are invalid.
+func NewGatewayNotAcceptedListenersNotValid() Condition {
 	return Condition{
-		Type:    string(v1beta1.GatewayClassConditionStatusAccepted),
+		Type:    string(v1beta1.GatewayConditionAccepted),
 		Status:  metav1.ConditionFalse,
-		Reason:  string(v1beta1.GatewayClassReasonInvalidParameters),
+		Reason:  string(v1beta1.GatewayReasonListenersNotValid),
+		Message: "Gateway has no valid listeners",
+	}
+}
+
+// NewGatewayInvalid returns a Condition that indicates the Gateway is not accepted because it is
+// semantically or syntactically invalid. The provided message contains the details of why the Gateway is invalid.
+func NewGatewayInvalid(msg string) Condition {
+	return Condition{
+		Type:    string(v1beta1.GatewayConditionAccepted),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1beta1.GatewayReasonInvalid),
 		Message: msg,
 	}
 }
 
-// NewDefaultGatewayClassConditions returns the default Conditions that must be present in the status of a GatewayClass.
-func NewDefaultGatewayClassConditions() []Condition {
-	return []Condition{
-		{
-			Type:    string(v1beta1.GatewayClassConditionStatusAccepted),
-			Status:  metav1.ConditionTrue,
-			Reason:  string(v1beta1.GatewayClassReasonAccepted),
-			Message: "GatewayClass is accepted",
-		},
+// NewGatewayUnsupportedValue returns a Condition that indicates that a field of the Gateway has an unsupported value.
+// Unsupported means that the value is not supported by the implementation or invalid.
+func NewGatewayUnsupportedValue(msg string) Condition {
+	return Condition{
+		Type:    string(v1beta1.GatewayConditionAccepted),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(GatewayReasonUnsupportedValue),
+		Message: msg,
 	}
 }
diff --git a/internal/state/graph/gateway.go b/internal/state/graph/gateway.go
@@ -4,10 +4,12 @@ import (
 	"sort"
 
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/gateway-api/apis/v1beta1"
 
 	nkgsort "github.com/nginxinc/nginx-kubernetes-gateway/internal/sort"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets"
 )
 
@@ -17,6 +19,10 @@ type Gateway struct {
 	Source *v1beta1.Gateway
 	// Listeners include the listeners of the Gateway.
 	Listeners map[string]*Listener
+	// Conditions holds the conditions for the Gateway.
+	Conditions []conditions.Condition
+	// Valid indicates whether the Gateway Spec is valid.
+	Valid bool
 }
 
 // processedGateways holds the resources that belong to NKG.
@@ -89,8 +95,38 @@ func buildGateway(gw *v1beta1.Gateway, secretMemoryMgr secrets.SecretDiskMemoryM
 		return nil
 	}
 
+	conds := validateGateway(gw, gc)
+
+	if len(conds) > 0 {
+		return &Gateway{
+			Source:     gw,
+			Valid:      false,
+			Conditions: conds,
+		}
+	}
+
 	return &Gateway{
 		Source:    gw,
-		Listeners: buildListeners(gw, secretMemoryMgr, gc),
+		Listeners: buildListeners(gw, secretMemoryMgr),
+		Valid:     true,
+	}
+}
+
+func validateGateway(gw *v1beta1.Gateway, gc *GatewayClass) []conditions.Condition {
+	var conds []conditions.Condition
+
+	if gc == nil {
+		conds = append(conds, conditions.NewGatewayInvalid("GatewayClass doesn't exist"))
+	} else if !gc.Valid {
+		conds = append(conds, conditions.NewGatewayInvalid("GatewayClass is invalid"))
+	}
+
+	if len(gw.Spec.Addresses) > 0 {
+		path := field.NewPath("spec", "addresses")
+		valErr := field.Forbidden(path, "addresses are not supported")
+
+		conds = append(conds, conditions.NewGatewayUnsupportedValue(valErr.Error()))
 	}
+
+	return conds
 }