diff --git a/internal/state/graph/backend_refs_test.go b/internal/state/graph/backend_refs_test.go index 4826c9f34d..92f0cd8207 100644 --- a/internal/state/graph/backend_refs_test.go +++ b/internal/state/graph/backend_refs_test.go @@ -311,9 +311,11 @@ func TestAddBackendGroupsToRouteTest(t *testing.T) { sectionNameRefs := []ParentRef{ { - Idx: 0, - Gateway: types.NamespacedName{Namespace: "test", Name: "gateway"}, - Attached: true, + Idx: 0, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway"}, + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + }, }, } diff --git a/internal/state/graph/graph_test.go b/internal/state/graph/graph_test.go index be5c52346c..c7a4a93cca 100644 --- a/internal/state/graph/graph_test.go +++ b/internal/state/graph/graph_test.go @@ -180,9 +180,11 @@ func TestBuildGraph(t *testing.T) { Source: hr1, ParentRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw1), - Attached: true, + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw1), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + }, }, }, Rules: []Rule{createValidRuleWithBackendGroup(hr1Group)}, @@ -193,9 +195,11 @@ func TestBuildGraph(t *testing.T) { Source: hr3, ParentRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw1), - Attached: true, + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw1), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + }, }, }, Rules: []Rule{createValidRuleWithBackendGroup(hr3Group)}, diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index c2a50ba58d..1ccbf58d39 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -27,12 +27,20 @@ type Rule struct { // ParentRef describes a reference to a parent in an HTTPRoute. type ParentRef struct { - // FailedAttachmentCondition describes the failure of the attachment when Attached is false. - FailedAttachmentCondition conditions.Condition + // Attachment is the attachment status of the ParentRef. It could be nil. In that case, NGK didn't attempt to + // attach because of problems with the Route. + Attachment *ParentRefAttachmentStatus // Gateway is the NamespacedName of the referenced Gateway Gateway types.NamespacedName // Idx is the index of the corresponding ParentReference in the HTTPRoute. Idx int +} + +// ParentRefAttachmentStatus describes the attachment status of a ParentRef. +type ParentRefAttachmentStatus struct { + // FailedCondition is the condition that describes why the ParentRef is not attached to the Gateway. It is set + // when Attached is false. + FailedCondition conditions.Condition // Attached indicates if the ParentRef is attached to the Gateway. Attached bool } @@ -280,7 +288,10 @@ func bindRouteToListeners(r *Route, gw *Gateway) { } for i := 0; i < len(r.ParentRefs); i++ { + attachment := &ParentRefAttachmentStatus{} ref := &r.ParentRefs[i] + ref.Attachment = attachment + routeRef := r.Source.Spec.ParentRefs[ref.Idx] path := field.NewPath("spec").Child("parentRefs").Index(ref.Idx) @@ -289,13 +300,13 @@ func bindRouteToListeners(r *Route, gw *Gateway) { if routeRef.SectionName == nil || *routeRef.SectionName == "" { valErr := field.Required(path.Child("sectionName"), "cannot be empty") - ref.FailedAttachmentCondition = conditions.NewRouteUnsupportedValue(valErr.Error()) + attachment.FailedCondition = conditions.NewRouteUnsupportedValue(valErr.Error()) continue } if routeRef.Port != nil { valErr := field.Forbidden(path.Child("port"), "cannot be set") - ref.FailedAttachmentCondition = conditions.NewRouteUnsupportedValue(valErr.Error()) + attachment.FailedCondition = conditions.NewRouteUnsupportedValue(valErr.Error()) continue } @@ -304,7 +315,7 @@ func bindRouteToListeners(r *Route, gw *Gateway) { referencesWinningGw := ref.Gateway.Namespace == gw.Source.Namespace && ref.Gateway.Name == gw.Source.Name if !referencesWinningGw { - ref.FailedAttachmentCondition = conditions.NewTODO("Gateway is ignored") + attachment.FailedCondition = conditions.NewTODO("Gateway is ignored") continue } @@ -327,23 +338,23 @@ func bindRouteToListeners(r *Route, gw *Gateway) { if !exists { // FIXME(pleshakov): Add a proper condition once it is available in the Gateway API. // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/306 - ref.FailedAttachmentCondition = conditions.NewTODO("listener is not found") + attachment.FailedCondition = conditions.NewTODO("listener is not found") continue } if !l.Valid { - ref.FailedAttachmentCondition = conditions.NewRouteInvalidListener() + attachment.FailedCondition = conditions.NewRouteInvalidListener() continue } accepted := findAcceptedHostnames(l.Source.Hostname, r.Source.Spec.Hostnames) if len(accepted) == 0 { - ref.FailedAttachmentCondition = conditions.NewRouteNoMatchingListenerHostname() + attachment.FailedCondition = conditions.NewRouteNoMatchingListenerHostname() continue } - ref.Attached = true + attachment.Attached = true for _, h := range accepted { l.AcceptedHostnames[h] = struct{}{} diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 0ab760555c..5ca5c103e1 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -763,6 +763,12 @@ func TestBindRouteToListeners(t *testing.T) { } notValidRoute := &Route{ Valid: false, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + }, + }, } notValidListener := createModifiedListener(func(l *Listener) { @@ -773,12 +779,11 @@ func TestBindRouteToListeners(t *testing.T) { }) tests := []struct { - route *Route - gateway *Gateway - expectedRouteUnattachedSectionNameRefs map[string]conditions.Condition - expectedGatewayListeners map[string]*Listener - name string - expectedSectionNameRefs []ParentRef + route *Route + gateway *Gateway + expectedGatewayListeners map[string]*Listener + name string + expectedSectionNameRefs []ParentRef }{ { route: createNormalRoute(), @@ -790,9 +795,11 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: true, + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -817,12 +824,14 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: false, - FailedAttachmentCondition: conditions.NewRouteUnsupportedValue( - `spec.parentRefs[0].sectionName: Required value: cannot be empty`, - ), + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteUnsupportedValue( + `spec.parentRefs[0].sectionName: Required value: cannot be empty`, + ), + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -840,12 +849,14 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: false, - FailedAttachmentCondition: conditions.NewRouteUnsupportedValue( - `spec.parentRefs[0].sectionName: Required value: cannot be empty`, - ), + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteUnsupportedValue( + `spec.parentRefs[0].sectionName: Required value: cannot be empty`, + ), + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -863,12 +874,14 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: false, - FailedAttachmentCondition: conditions.NewRouteUnsupportedValue( - `spec.parentRefs[0].port: Forbidden: cannot be set`, - ), + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteUnsupportedValue( + `spec.parentRefs[0].port: Forbidden: cannot be set`, + ), + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -886,10 +899,12 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: false, - FailedAttachmentCondition: conditions.NewTODO("listener is not found"), + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewTODO("listener is not found"), + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -907,10 +922,12 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: false, - FailedAttachmentCondition: conditions.NewRouteInvalidListener(), + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteInvalidListener(), + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -928,10 +945,12 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: false, - FailedAttachmentCondition: conditions.NewRouteNoMatchingListenerHostname(), + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteNoMatchingListenerHostname(), + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -949,10 +968,12 @@ func TestBindRouteToListeners(t *testing.T) { }, expectedSectionNameRefs: []ParentRef{ { - Idx: 0, - Gateway: ignoredGwNsName, - Attached: false, - FailedAttachmentCondition: conditions.NewTODO("Gateway is ignored"), + Idx: 0, + Gateway: ignoredGwNsName, + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewTODO("Gateway is ignored"), + }, }, }, expectedGatewayListeners: map[string]*Listener{ @@ -968,7 +989,13 @@ func TestBindRouteToListeners(t *testing.T) { "listener-80-1": createListener(), }, }, - expectedRouteUnattachedSectionNameRefs: map[string]conditions.Condition{}, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: nil, + }, + }, expectedGatewayListeners: map[string]*Listener{ "listener-80-1": createListener(), }, diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 10a75757a7..50c2a7be1a 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -133,7 +133,7 @@ func buildStatuses(graph *graph.Graph) Statuses { for _, ref := range r.ParentRefs { failedAttachmentCondCount := 0 - if !ref.Attached { + if ref.Attachment != nil && !ref.Attachment.Attached { failedAttachmentCondCount = 1 } allConds := make([]conditions.Condition, 0, len(r.Conditions)+len(defaultConds)+failedAttachmentCondCount) @@ -143,7 +143,7 @@ func buildStatuses(graph *graph.Graph) Statuses { allConds = append(allConds, defaultConds...) allConds = append(allConds, r.Conditions...) if failedAttachmentCondCount == 1 { - allConds = append(allConds, ref.FailedAttachmentCondition) + allConds = append(allConds, ref.Attachment.FailedCondition) } routeRef := r.Source.Spec.ParentRefs[ref.Idx] diff --git a/internal/state/statuses_test.go b/internal/state/statuses_test.go index 5111a62dae..ad8e52fb36 100644 --- a/internal/state/statuses_test.go +++ b/internal/state/statuses_test.go @@ -15,8 +15,12 @@ import ( ) func TestBuildStatuses(t *testing.T) { - invalidCondition := conditions.Condition{ - Type: "Test", + invalidRouteCondition := conditions.Condition{ + Type: "TestInvalidRoute", + Status: metav1.ConditionTrue, + } + invalidAttachmentCondition := conditions.Condition{ + Type: "TestInvalidAttachment", Status: metav1.ConditionTrue, } @@ -37,7 +41,8 @@ func TestBuildStatuses(t *testing.T) { } routes := map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-1"}: { + {Namespace: "test", Name: "hr-valid"}: { + Valid: true, Source: &v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Generation: 3, @@ -57,15 +62,44 @@ func TestBuildStatuses(t *testing.T) { }, ParentRefs: []graph.ParentRef{ { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attached: true, + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &graph.ParentRefAttachmentStatus{ + Attached: true, + }, }, { - Idx: 1, - Gateway: client.ObjectKeyFromObject(gw), - Attached: false, - FailedAttachmentCondition: invalidCondition, + Idx: 1, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &graph.ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: invalidAttachmentCondition, + }, + }, + }, + }, + {Namespace: "test", Name: "hr-invalid"}: { + Valid: false, + Conditions: []conditions.Condition{invalidRouteCondition}, + Source: &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 3, + }, + Spec: v1beta1.HTTPRouteSpec{ + CommonRouteSpec: v1beta1.CommonRouteSpec{ + ParentRefs: []v1beta1.ParentReference{ + { + SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), + }, + }, + }, + }, + }, + ParentRefs: []graph.ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: nil, }, }, }, @@ -114,7 +148,7 @@ func TestBuildStatuses(t *testing.T) { {Namespace: "test", Name: "ignored-gateway"}: {ObservedGeneration: 1}, }, HTTPRouteStatuses: map[types.NamespacedName]HTTPRouteStatus{ - {Namespace: "test", Name: "hr-1"}: { + {Namespace: "test", Name: "hr-valid"}: { ObservedGeneration: 3, ParentStatuses: []ParentStatus{ { @@ -127,7 +161,20 @@ func TestBuildStatuses(t *testing.T) { SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-2"), Conditions: append( conditions.NewDefaultRouteConditions(), - invalidCondition, + invalidAttachmentCondition, + ), + }, + }, + }, + {Namespace: "test", Name: "hr-invalid"}: { + ObservedGeneration: 3, + ParentStatuses: []ParentStatus{ + { + GatewayNsName: client.ObjectKeyFromObject(gw), + SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), + Conditions: append( + conditions.NewDefaultRouteConditions(), + invalidRouteCondition, ), }, },