Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: HTTPRoute status only shows one parent when targeting multiple Gateways from different GatewayClasses #4587

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/gatewayapi/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
74 changes: 63 additions & 11 deletions internal/provider/kubernetes/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import (
"context"
"fmt"
"reflect"

kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -18,6 +19,7 @@
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"
Expand Down Expand Up @@ -74,7 +76,7 @@
panic(err)
}
hCopy := h.DeepCopy()
hCopy.Status.Parents = val.Parents
hCopy.Status.Parents = mergeRouteParentStatus(h.Namespace, h.Status.Parents, val.Parents)

Check warning on line 79 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L79

Added line #L79 was not covered by tests
return hCopy
}),
})
Expand All @@ -97,15 +99,15 @@
NamespacedName: key,
Resource: new(gwapiv1.GRPCRoute),
Mutator: MutatorFunc(func(obj client.Object) client.Object {
h, ok := obj.(*gwapiv1.GRPCRoute)
g, ok := obj.(*gwapiv1.GRPCRoute)

Check warning on line 102 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L102

Added line #L102 was not covered by tests
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

Check warning on line 110 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L108-L110

Added lines #L108 - L110 were not covered by tests
}),
})
},
Expand Down Expand Up @@ -136,7 +138,7 @@
panic(err)
}
tCopy := t.DeepCopy()
tCopy.Status.Parents = val.Parents
tCopy.Status.Parents = mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents)

Check warning on line 141 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L141

Added line #L141 was not covered by tests
return tCopy
}),
})
Expand Down Expand Up @@ -168,7 +170,7 @@
panic(err)
}
tCopy := t.DeepCopy()
tCopy.Status.Parents = val.Parents
tCopy.Status.Parents = mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents)

Check warning on line 173 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L173

Added line #L173 was not covered by tests
return tCopy
}),
})
Expand All @@ -193,15 +195,15 @@
NamespacedName: key,
Resource: new(gwapiv1a2.UDPRoute),
Mutator: MutatorFunc(func(obj client.Object) client.Object {
t, ok := obj.(*gwapiv1a2.UDPRoute)
u, ok := obj.(*gwapiv1a2.UDPRoute)

Check warning on line 198 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L198

Added line #L198 was not covered by tests
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

Check warning on line 206 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L204-L206

Added lines #L204 - L206 were not covered by tests
}),
})
},
Expand Down Expand Up @@ -469,6 +471,56 @@
}
}

// 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 {
Expand Down
Loading