Report proper conditions when GatewayClass is invalid or doesn't exist (

#576) * Refactor listener validation * Report proper conditions when GatewayClass is invalid or doesn't exist If GatewayClass is invalid or doesn't exist: - For Gateway Listeners: NKG will make every listener invalid and report Accepted condition with status False and reason NoValidGatewayClass in every listener status. - For HTTPRoutes: An HTTPRoute will not be able to attach to any listener, because they will be invalid. This is already handled: NKG will report Accepted condition with status False and reason InvalidListener. Fixes #307 * Introduce gateway_listener.go - move all listener-related code there
nginxinc · Apr 17, 2023 · ae6bec8 · ae6bec8
1 parent 79a8078
commit ae6bec8
Show file tree

Hide file tree

Showing 9 changed files with 813 additions and 862 deletions.
diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go
@@ -316,18 +316,20 @@ var _ = Describe("ChangeProcessor", func() {
 								ObservedGeneration: gw1.Generation,
 								ListenerStatuses: map[string]state.ListenerStatus{
 									"listener-80-1": {
-										AttachedRoutes: 1,
-										Conditions: append(
-											conditions.NewDefaultListenerConditions(),
-											conditions.NewTODO("GatewayClass is invalid or doesn't exist"),
-										),
+										AttachedRoutes: 0,
+										Conditions: []conditions.Condition{
+											conditions.NewListenerResolvedRefs(),
+											conditions.NewListenerNoConflicts(),
+											conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"),
+										},
 									},
 									"listener-443-1": {
-										AttachedRoutes: 1,
-										Conditions: append(
-											conditions.NewDefaultListenerConditions(),
-											conditions.NewTODO("GatewayClass is invalid or doesn't exist"),
-										),
+										AttachedRoutes: 0,
+										Conditions: []conditions.Condition{
+											conditions.NewListenerResolvedRefs(),
+											conditions.NewListenerNoConflicts(),
+											conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"),
+										},
 									},
 								},
 							},
@@ -339,18 +341,16 @@ var _ = Describe("ChangeProcessor", func() {
 										{
 											GatewayNsName: client.ObjectKeyFromObject(gw1),
 											SectionName:   helpers.GetPointer[v1beta1.SectionName]("listener-80-1"),
-											Conditions: append(
-												conditions.NewDefaultRouteConditions(),
-												conditions.NewTODO("GatewayClass is invalid or doesn't exist"),
-											),
+											Conditions: []conditions.Condition{
+												conditions.NewRouteInvalidListener(),
+											},
 										},
 										{
 											GatewayNsName: client.ObjectKeyFromObject(gw1),
 											SectionName:   helpers.GetPointer[v1beta1.SectionName]("listener-443-1"),
-											Conditions: append(
-												conditions.NewDefaultRouteConditions(),
-												conditions.NewTODO("GatewayClass is invalid or doesn't exist"),
-											),
+											Conditions: []conditions.Condition{
+												conditions.NewRouteInvalidListener(),
+											},
 										},
 									},
 								},
@@ -1219,17 +1219,19 @@ var _ = Describe("ChangeProcessor", func() {
 							ListenerStatuses: map[string]state.ListenerStatus{
 								"listener-80-1": {
 									AttachedRoutes: 0,
-									Conditions: append(
-										conditions.NewDefaultListenerConditions(),
-										conditions.NewTODO("GatewayClass is invalid or doesn't exist"),
-									),
+									Conditions: []conditions.Condition{
+										conditions.NewListenerResolvedRefs(),
+										conditions.NewListenerNoConflicts(),
+										conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"),
+									},
 								},
 								"listener-443-1": {
 									AttachedRoutes: 0,
-									Conditions: append(
-										conditions.NewDefaultListenerConditions(),
-										conditions.NewTODO("GatewayClass is invalid or doesn't exist"),
-									),
+									Conditions: []conditions.Condition{
+										conditions.NewListenerResolvedRefs(),
+										conditions.NewListenerNoConflicts(),
+										conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"),
+									},
 								},
 							},
 						},

diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go
@@ -15,6 +15,10 @@ const (
 	// 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"
@@ -129,27 +133,42 @@ func NewListenerPortUnavailable(msg string) Condition {
 	}
 }
 
+// NewListenerAccepted returns a Condition that indicates that the Listener is accepted.
+func NewListenerAccepted() Condition {
+	return Condition{
+		Type:    string(v1beta1.ListenerConditionAccepted),
+		Status:  metav1.ConditionTrue,
+		Reason:  string(v1beta1.ListenerReasonAccepted),
+		Message: "Listener is accepted",
+	}
+}
+
+// NewListenerResolvedRefs returns a Condition that indicates that all references in a Listener are resolved.
+func NewListenerResolvedRefs() Condition {
+	return Condition{
+		Type:    string(v1beta1.ListenerConditionResolvedRefs),
+		Status:  metav1.ConditionTrue,
+		Reason:  string(v1beta1.ListenerReasonResolvedRefs),
+		Message: "All references are resolved",
+	}
+}
+
+// NewListenerNoConflicts returns a Condition that indicates that there are no conflicts in a Listener.
+func NewListenerNoConflicts() Condition {
+	return Condition{
+		Type:    string(v1beta1.ListenerConditionConflicted),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1beta1.ListenerReasonNoConflicts),
+		Message: "No conflicts",
+	}
+}
+
 // NewDefaultListenerConditions returns the default Conditions that must be present in the status of a Listener.
 func NewDefaultListenerConditions() []Condition {
 	return []Condition{
-		{
-			Type:    string(v1beta1.ListenerConditionAccepted),
-			Status:  metav1.ConditionTrue,
-			Reason:  string(v1beta1.ListenerReasonAccepted),
-			Message: "Listener is accepted",
-		},
-		{
-			Type:    string(v1beta1.ListenerReasonResolvedRefs),
-			Status:  metav1.ConditionTrue,
-			Reason:  string(v1beta1.ListenerReasonResolvedRefs),
-			Message: "All references are resolved",
-		},
-		{
-			Type:    string(v1beta1.ListenerConditionConflicted),
-			Status:  metav1.ConditionFalse,
-			Reason:  string(v1beta1.ListenerReasonNoConflicts),
-			Message: "No conflicts",
-		},
+		NewListenerAccepted(),
+		NewListenerResolvedRefs(),
+		NewListenerNoConflicts(),
 	}
 }
 
@@ -220,6 +239,17 @@ 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,
+	}
+}
+
 // NewRouteBackendRefInvalidKind returns a Condition that indicates that the Route has a backendRef with an
 // invalid kind.
 func NewRouteBackendRefInvalidKind(msg string) Condition {