Support NoMatchingListenerHostname reason in HTTPRoute status (nginx#290)

If an HTTPRoute references a listener with the hostname that does not
match any hostnames of the HTTPRoute, NGINX Kubernetes Gateway will
set the condition "Accepted" with the status "False" and the reason
"NoMatchingListenerHostname" in the status of the HTTPRoute.

The commit introduces a new type RouteCondition. It is used to capture
status-related conditions when building the graph. It will be used to
capture other conditions too, including when building configuration.

Note: the implementation required implementing a few additional
conditions. In one case, to wait until the corresponding reasons
(v1beta1.RouteConditionReason) are added to the Gateway API and in the
other case, to not increase the scope, those conditions are left to be
implemented (FIXMEs).

Author: miledxz

docs/gateway-api-compatibility.md

@@ -96,7 +96,9 @@ Fields:
   * `parents`
 	* `parentRef` - supported.
 	* `controllerName` - supported.
-	* `conditions` - partially supported.
+	* `conditions` - partially supported. Supported (Condition/Status/Reason):
+    	*  `Accepted/True/Accepted`
+    	*  `Accepted/False/NoMatchingListenerHostname`
 
 ### TLSRoute
 

internal/state/change_processor_test.go

@@ -2,6 +2,7 @@ package state_test
 
 import (
 	"context"
+	"sort"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -17,6 +18,7 @@ import (
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship/relationshipfakes"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/statefakes"
@@ -228,6 +230,24 @@ var _ = Describe("ChangeProcessor", func() {
 
 				gw2 = createGatewayWithTLSListener("gateway-2")
 			})
+
+			assertStatuses := func(expected, result state.Statuses) {
+				sortConditions := func(statuses state.HTTPRouteStatuses) {
+					for _, status := range statuses {
+						for _, ps := range status.ParentStatuses {
+							sort.Slice(ps.Conditions, func(i, j int) bool {
+								return ps.Conditions[i].Type < ps.Conditions[j].Type
+							})
+						}
+					}
+				}
+
+				sortConditions(expected.HTTPRouteStatuses)
+				sortConditions(result.HTTPRouteStatuses)
+
+				ExpectWithOffset(1, helpers.Diff(expected, result)).To(BeEmpty())
+			}
+
 			When("no upsert has occurred", func() {
 				It("returns empty configuration and statuses", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
@@ -279,8 +299,18 @@ var _ = Describe("ChangeProcessor", func() {
 								{Namespace: "test", Name: "hr-1"}: {
 									ObservedGeneration: hr1.Generation,
 									ParentStatuses: map[string]state.ParentStatus{
-										"listener-80-1":  {Attached: false},
-										"listener-443-1": {Attached: false},
+										"listener-80-1": {
+											Conditions: append(
+												conditions.NewDefaultRouteConditions(),
+												conditions.NewRouteTODO("GatewayClass is invalid or doesn't exist"),
+											),
+										},
+										"listener-443-1": {
+											Conditions: append(
+												conditions.NewDefaultRouteConditions(),
+												conditions.NewRouteTODO("GatewayClass is invalid or doesn't exist"),
+											),
+										},
 									},
 								},
 							},
@@ -289,7 +319,7 @@ var _ = Describe("ChangeProcessor", func() {
 						changed, conf, statuses := processor.Process(context.TODO())
 						Expect(changed).To(BeTrue())
 						Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-						Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+						assertStatuses(expectedStatuses, statuses)
 					})
 				})
 			})
@@ -367,8 +397,12 @@ var _ = Describe("ChangeProcessor", func() {
 							{Namespace: "test", Name: "hr-1"}: {
 								ObservedGeneration: hr1.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: true},
-									"listener-443-1": {Attached: true},
+									"listener-80-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
+									"listener-443-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
 								},
 							},
 						},
@@ -377,7 +411,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the first HTTPRoute without a generation changed is processed", func() {
@@ -465,8 +499,12 @@ var _ = Describe("ChangeProcessor", func() {
 							{Namespace: "test", Name: "hr-1"}: {
 								ObservedGeneration: hr1Updated.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: true},
-									"listener-443-1": {Attached: true},
+									"listener-80-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
+									"listener-443-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
 								},
 							},
 						},
@@ -475,7 +513,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				},
 				)
 			})
@@ -564,8 +602,12 @@ var _ = Describe("ChangeProcessor", func() {
 							{Namespace: "test", Name: "hr-1"}: {
 								ObservedGeneration: hr1Updated.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: true},
-									"listener-443-1": {Attached: true},
+									"listener-80-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
+									"listener-443-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
 								},
 							},
 						},
@@ -574,7 +616,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the GatewayClass update without generation change is processed", func() {
@@ -662,8 +704,12 @@ var _ = Describe("ChangeProcessor", func() {
 							{Namespace: "test", Name: "hr-1"}: {
 								ObservedGeneration: hr1Updated.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: true},
-									"listener-443-1": {Attached: true},
+									"listener-80-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
+									"listener-443-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
 								},
 							},
 						},
@@ -672,7 +718,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("no changes are captured", func() {
@@ -763,8 +809,12 @@ var _ = Describe("ChangeProcessor", func() {
 							{Namespace: "test", Name: "hr-1"}: {
 								ObservedGeneration: hr1Updated.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: true},
-									"listener-443-1": {Attached: true},
+									"listener-80-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
+									"listener-443-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
 								},
 							},
 						},
@@ -773,7 +823,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the second HTTPRoute is upserted", func() {
@@ -853,15 +903,29 @@ var _ = Describe("ChangeProcessor", func() {
 							{Namespace: "test", Name: "hr-1"}: {
 								ObservedGeneration: hr1Updated.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: true},
-									"listener-443-1": {Attached: true},
+									"listener-80-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
+									"listener-443-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
 								},
 							},
 							{Namespace: "test", Name: "hr-2"}: {
 								ObservedGeneration: hr2.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: false},
-									"listener-443-1": {Attached: false},
+									"listener-80-1": {
+										Conditions: append(
+											conditions.NewDefaultRouteConditions(),
+											conditions.NewRouteTODO("Gateway is ignored"),
+										),
+									},
+									"listener-443-1": {
+										Conditions: append(
+											conditions.NewDefaultRouteConditions(),
+											conditions.NewRouteTODO("Gateway is ignored"),
+										),
+									},
 								},
 							},
 						},
@@ -870,7 +934,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the first Gateway is deleted", func() {
@@ -949,8 +1013,12 @@ var _ = Describe("ChangeProcessor", func() {
 							{Namespace: "test", Name: "hr-2"}: {
 								ObservedGeneration: hr2.Generation,
 								ParentStatuses: map[string]state.ParentStatus{
-									"listener-80-1":  {Attached: true},
-									"listener-443-1": {Attached: true},
+									"listener-80-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
+									"listener-443-1": {
+										Conditions: conditions.NewDefaultRouteConditions(),
+									},
 								},
 							},
 						},
@@ -959,7 +1027,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the second HTTPRoute is deleted", func() {
@@ -1003,7 +1071,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the GatewayClass is deleted", func() {
@@ -1035,7 +1103,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the second Gateway is deleted", func() {
@@ -1054,7 +1122,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 			When("the first HTTPRoute is deleted", func() {
@@ -1073,7 +1141,7 @@ var _ = Describe("ChangeProcessor", func() {
 					changed, conf, statuses := processor.Process(context.TODO())
 					Expect(changed).To(BeTrue())
 					Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
-					Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+					assertStatuses(expectedStatuses, statuses)
 				})
 			})
 		})

internal/state/conditions/httproute.go

@@ -0,0 +1,72 @@
+package conditions
+
+import (
+	"fmt"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+)
+
+// RouteCondition defines a condition to be reported in the status of an HTTPRoute.
+type RouteCondition struct {
+	Type    v1beta1.RouteConditionType
+	Status  metav1.ConditionStatus
+	Reason  v1beta1.RouteConditionReason
+	Message string
+}
+
+// DeduplicateRouteConditions removes duplicate conditions based on the condition type.
+// The last condition wins.
+func DeduplicateRouteConditions(conds []RouteCondition) []RouteCondition {
+	uniqueConds := make(map[v1beta1.RouteConditionType]RouteCondition)
+
+	for _, cond := range conds {
+		uniqueConds[cond.Type] = cond
+	}
+
+	result := make([]RouteCondition, 0, len(uniqueConds))
+
+	for _, cond := range uniqueConds {
+		result = append(result, cond)
+	}
+
+	return result
+}
+
+// NewDefaultRouteConditions returns the default conditions that must be present in the status of an HTTPRoute.
+func NewDefaultRouteConditions() []RouteCondition {
+	return []RouteCondition{
+		NewRouteAccepted(),
+	}
+}
+
+// NewRouteNoMatchingListenerHostname returns a RouteCondition that indicates that the hostname of the listener
+// does not match the hostnames of the HTTPRoute.
+func NewRouteNoMatchingListenerHostname() RouteCondition {
+	return RouteCondition{
+		Type:    v1beta1.RouteConditionAccepted,
+		Status:  metav1.ConditionFalse,
+		Reason:  v1beta1.RouteReasonNoMatchingListenerHostname,
+		Message: "Listener hostname does not match the HTTPRoute hostnames",
+	}
+}
+
+// NewRouteAccepted returns a RouteCondition that indicates that the HTTPRoute is accepted.
+func NewRouteAccepted() RouteCondition {
+	return RouteCondition{
+		Type:    v1beta1.RouteConditionAccepted,
+		Status:  metav1.ConditionTrue,
+		Reason:  v1beta1.RouteReasonAccepted,
+		Message: "The route is accepted",
+	}
+}
+
+// NewRouteTODO returns a RouteCondition that can be used as a placeholder for a condition that is not yet implemented.
+func NewRouteTODO(msg string) RouteCondition {
+	return RouteCondition{
+		Type:    "TODO",
+		Status:  metav1.ConditionTrue,
+		Reason:  "TODO",
+		Message: fmt.Sprintf("The condition for this has not been implemented yet: %s", msg),
+	}
+}