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: Route with multiple parents has incorrect namespace in parentRef status #4592

Open
wants to merge 2 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
52 changes: 40 additions & 12 deletions internal/gatewayapi/contexts.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,21 +238,26 @@ func GetRouteStatus(route RouteContext) *gwapiv1.RouteStatus {
return &rs
}

// GetRouteParentContext returns RouteParentContext by using the Route
// objects' ParentReference.
// GetRouteParentContext returns RouteParentContext by using the Route objects' ParentReference.
// It creates a new RouteParentContext and add a new RouteParentStatus to the Route's Status if the ParentReference is not found.
func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentReference) *RouteParentContext {
rv := reflect.ValueOf(route).Elem()
pr := rv.FieldByName("ParentRefs")

// If the ParentRefs field is nil, initialize it.
if pr.IsNil() {
mm := reflect.MakeMap(reflect.TypeOf(map[gwapiv1.ParentReference]*RouteParentContext{}))
pr.Set(mm)
}

// If the RouteParentContext is already in the RouteContext, return it.
if p := pr.MapIndex(reflect.ValueOf(forParentRef)); p.IsValid() && !p.IsZero() {
ctx := p.Interface().(*RouteParentContext)
return ctx
}

// Verify that the ParentReference is present in the Route.Spec.ParentRefs.
// This is just a sanity check, the parentRef should always be present, otherwise it's a programming error.
var parentRef *gwapiv1.ParentReference
specParentRefs := rv.FieldByName("Spec").FieldByName("ParentRefs")
for i := 0; i < specParentRefs.Len(); i++ {
Expand All @@ -266,25 +271,19 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
panic("parentRef not found")
}

// Find the parent in the Route's Status.
routeParentStatusIdx := -1
defaultNamespace := gwapiv1.Namespace(metav1.NamespaceDefault)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the root cause of the bug.

statusParents := rv.FieldByName("Status").FieldByName("Parents")

for i := 0; i < statusParents.Len(); i++ {
p := statusParents.Index(i).FieldByName("ParentRef").Interface().(gwapiv1.ParentReference)
// For those non-v1 routes, their underlying type of `ParentReference` is v1 as well.
// So we can skip upgrading these routes for simplicity.
if forParentRef.Namespace == nil {
forParentRef.Namespace = &defaultNamespace
}
if p.Namespace == nil {
p.Namespace = &defaultNamespace
}
if reflect.DeepEqual(p, forParentRef) {
if isParentRefEqual(p, *parentRef, route.GetNamespace()) {
routeParentStatusIdx = i
break
}
}

// If the parent is not found in the Route's Status, create a new RouteParentStatus and add it to the Route's Status.
if routeParentStatusIdx == -1 {
rParentStatus := gwapiv1a2.RouteParentStatus{
ControllerName: gwapiv1a2.GatewayController(rv.FieldByName("GatewayControllerName").String()),
Expand All @@ -294,6 +293,7 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
routeParentStatusIdx = statusParents.Len() - 1
}

// Also add the RouteParentContext to the RouteContext.
ctx := &RouteParentContext{
ParentReference: parentRef,
routeParentStatusIdx: routeParentStatusIdx,
Expand All @@ -304,6 +304,34 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
return ctx
}

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)
}

// RouteParentContext wraps a ParentReference and provides helper methods for
// setting conditions and other status information on the associated
// HTTPRoute, TLSRoute etc.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
gateways:
- apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
name: gateway-a
namespace: default
spec:
gatewayClassName: envoy-gateway-class
listeners:
- name: default
port: 80
protocol: HTTP
hostname: '*.a.example.com'
allowedRoutes:
namespaces:
from: All
- apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
name: gateway-b
namespace: envoy-gateway
spec:
gatewayClassName: envoy-gateway-class
listeners:
- name: default
port: 80
protocol: HTTP
hostname: '*.b.example.com'
allowedRoutes:
namespaces:
from: All
httpRoutes:
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
name: targeted-route
namespace: envoy-gateway
spec:
hostnames:
- targeted.a.example.com
- targeted.b.example.com
parentRefs:
- group: gateway.networking.k8s.io
kind: Gateway
name: gateway-a
namespace: default
- group: gateway.networking.k8s.io
kind: Gateway
name: gateway-b
rules:
- matches:
- method: GET
path:
type: PathPrefix
value: /toy
Loading