diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go index 1c1ecee7672..366a24b827e 100644 --- a/internal/gatewayapi/helpers.go +++ b/internal/gatewayapi/helpers.go @@ -86,6 +86,7 @@ var ( QueryParamMatchTypeDerefOr = ptr.Deref[gwapiv1.QueryParamMatchType] ) +// Deprecated: use k8s.io/utils/ptr ptr.Deref instead func NamespaceDerefOr(namespace *gwapiv1.Namespace, defaultNamespace string) string { if namespace != nil && *namespace != "" { return string(*namespace) diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go index c3d5553b0bf..a59eb82f75a 100644 --- a/internal/provider/kubernetes/status.go +++ b/internal/provider/kubernetes/status.go @@ -8,6 +8,7 @@ package kubernetes import ( "context" "fmt" + "reflect" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -18,6 +19,7 @@ import ( gwapiv1a3 "sigs.k8s.io/gateway-api/apis/v1alpha3" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/gatewayapi/resource" "github.com/envoyproxy/gateway/internal/gatewayapi/status" "github.com/envoyproxy/gateway/internal/message" "github.com/envoyproxy/gateway/internal/utils" @@ -74,7 +76,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext panic(err) } hCopy := h.DeepCopy() - hCopy.Status.Parents = val.Parents + hCopy.Status.Parents = mergeRouteParentStatus(h.Namespace, h.Status.Parents, val.Parents) return hCopy }), }) @@ -97,15 +99,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext NamespacedName: key, Resource: new(gwapiv1.GRPCRoute), Mutator: MutatorFunc(func(obj client.Object) client.Object { - h, ok := obj.(*gwapiv1.GRPCRoute) + g, ok := obj.(*gwapiv1.GRPCRoute) if !ok { err := fmt.Errorf("unsupported object type %T", obj) errChan <- err panic(err) } - hCopy := h.DeepCopy() - hCopy.Status.Parents = val.Parents - return hCopy + gCopy := g.DeepCopy() + gCopy.Status.Parents = mergeRouteParentStatus(g.Namespace, g.Status.Parents, val.Parents) + return gCopy }), }) }, @@ -136,7 +138,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext panic(err) } tCopy := t.DeepCopy() - tCopy.Status.Parents = val.Parents + tCopy.Status.Parents = mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents) return tCopy }), }) @@ -168,7 +170,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext panic(err) } tCopy := t.DeepCopy() - tCopy.Status.Parents = val.Parents + tCopy.Status.Parents = mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents) return tCopy }), }) @@ -193,15 +195,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext NamespacedName: key, Resource: new(gwapiv1a2.UDPRoute), Mutator: MutatorFunc(func(obj client.Object) client.Object { - t, ok := obj.(*gwapiv1a2.UDPRoute) + u, ok := obj.(*gwapiv1a2.UDPRoute) if !ok { err := fmt.Errorf("unsupported object type %T", obj) errChan <- err panic(err) } - tCopy := t.DeepCopy() - tCopy.Status.Parents = val.Parents - return tCopy + uCopy := u.DeepCopy() + uCopy.Status.Parents = mergeRouteParentStatus(u.Namespace, u.Status.Parents, val.Parents) + return uCopy }), }) }, @@ -469,6 +471,56 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext } } +// mergeRouteParentStatus merges the old and new RouteParentStatus. +// This is needed because the RouteParentStatus doesn't support strategic merge patch yet. +func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []gwapiv1.RouteParentStatus { + merged := make([]gwapiv1.RouteParentStatus, len(old)) + _ = copy(merged, old) + for _, parent := range new { + found := -1 + for i, existing := range old { + if isParentRefEqual(parent.ParentRef, existing.ParentRef, ns) { + found = i + break + } + } + if found >= 0 { + merged[found] = parent + } else { + merged = append(merged, parent) + } + } + return merged +} + +func isParentRefEqual(ref1, ref2 gwapiv1.ParentReference, routeNS string) bool { + defaultGroup := (*gwapiv1.Group)(&gwapiv1.GroupVersion.Group) + if ref1.Group == nil { + ref1.Group = defaultGroup + } + if ref2.Group == nil { + ref2.Group = defaultGroup + } + + defaultKind := gwapiv1.Kind(resource.KindGateway) + if ref1.Kind == nil { + ref1.Kind = &defaultKind + } + if ref2.Kind == nil { + ref2.Kind = &defaultKind + } + + // If the parent's namespace is not set, default to the namespace of the Route. + defaultNS := gwapiv1.Namespace(routeNS) + if ref1.Namespace == nil { + ref1.Namespace = &defaultNS + } + if ref2.Namespace == nil { + ref2.Namespace = &defaultNS + } + return reflect.DeepEqual(ref1, ref2) +} + func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *gwapiv1.Gateway) { // nil check for unit tests. if r.statusUpdater == nil { diff --git a/internal/provider/kubernetes/status_test.go b/internal/provider/kubernetes/status_test.go new file mode 100644 index 00000000000..5e81c46135e --- /dev/null +++ b/internal/provider/kubernetes/status_test.go @@ -0,0 +1,294 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package kubernetes + +import ( + "reflect" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func Test_mergeRouteParentStatus(t *testing.T) { + type args struct { + old []gwapiv1.RouteParentStatus + new []gwapiv1.RouteParentStatus + } + tests := []struct { + name string + args args + want []gwapiv1.RouteParentStatus + }{ + { + name: "merge old and new", + args: args{ + old: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + Namespace: ptr.To[gwapiv1.Namespace]("default"), + SectionName: ptr.To[gwapiv1.SectionName]("listener1"), + Port: ptr.To[gwapiv1.PortNumber](80), + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + }, + new: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + want: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + Namespace: ptr.To[gwapiv1.Namespace]("default"), + SectionName: ptr.To[gwapiv1.SectionName]("listener1"), + Port: ptr.To[gwapiv1.PortNumber](80), + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + + { + name: "override an existing parent", + args: args{ + old: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + Namespace: ptr.To[gwapiv1.Namespace]("default"), + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + }, + new: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + want: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + + { + name: "nothing changed", + args: args{ + old: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + new: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + want: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := mergeRouteParentStatus("default", tt.args.old, tt.args.new); !reflect.DeepEqual(got, tt.want) { + t.Errorf("mergeRouteParentStatus() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/release-notes/current.yaml b/release-notes/current.yaml index b2d9b889bed..3b54de7121e 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -16,6 +16,7 @@ new features: | bug fixes: | Only log endpoint configuration in verbose logging mode (`-v 4` or higher) The xDS translation failed when wasm http code source configured without a sha + HTTPRoute status only shows one parent when targeting multiple Gateways from different GatewayClasses Route with multiple parents has incorrect namespace in parentRef status # Enhancements that improve performance.