From ab115fd3d58dc2716ba22ba4f4adbb623858776f Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Fri, 1 Nov 2024 14:45:00 +0800 Subject: [PATCH] fix: Route with multiple parents has incorrect namespace in parentRef status (#4592) fix route status wrong ns Signed-off-by: Huabing Zhao (cherry picked from commit 7285dda6ba2727d10423887dc2262bad2711f80b) Signed-off-by: Huabing Zhao --- internal/gatewayapi/contexts.go | 52 +++- ...ultiple-gateways-from-different-ns.in.yaml | 55 ++++ ...ltiple-gateways-from-different-ns.out.yaml | 249 ++++++++++++++++++ ...ith-multiple-gateways-from-same-ns.in.yaml | 54 ++++ ...th-multiple-gateways-from-same-ns.out.yaml | 247 +++++++++++++++++ release-notes/current.yaml | 1 + 6 files changed, 646 insertions(+), 12 deletions(-) create mode 100644 internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.out.yaml diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index fbd4c588f9b..7bcf321d3a2 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -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++ { @@ -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) 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()), @@ -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, @@ -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. diff --git a/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.in.yaml b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.in.yaml new file mode 100644 index 00000000000..12aa992ef44 --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.in.yaml @@ -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 diff --git a/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.out.yaml b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.out.yaml new file mode 100644 index 00000000000..ba2f58b8667 --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.out.yaml @@ -0,0 +1,249 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-a + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + hostname: '*.a.example.com' + name: default + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: default + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-b + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + hostname: '*.b.example.com' + name: default + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: default + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + 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 + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-a + namespace: default + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-b +infraIR: + default/gateway-a: + proxy: + listeners: + - address: null + name: default/gateway-a/default + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-a + gateway.envoyproxy.io/owning-gateway-namespace: default + name: default/gateway-a + envoy-gateway/gateway-b: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-b/default + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-b + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-b +xdsIR: + default/gateway-a: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*.a.example.com' + isHTTP2: false + metadata: + kind: Gateway + name: gateway-a + namespace: default + sectionName: default + name: default/gateway-a/default + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - directResponse: + statusCode: 500 + headerMatches: + - distinct: false + exact: GET + name: :method + hostname: targeted.a.example.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: targeted-route + namespace: envoy-gateway + name: httproute/envoy-gateway/targeted-route/rule/0/match/0/targeted_a_example_com + pathMatch: + distinct: false + name: "" + prefix: /toy + envoy-gateway/gateway-b: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*.b.example.com' + isHTTP2: false + metadata: + kind: Gateway + name: gateway-b + namespace: envoy-gateway + sectionName: default + name: envoy-gateway/gateway-b/default + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - directResponse: + statusCode: 500 + headerMatches: + - distinct: false + exact: GET + name: :method + hostname: targeted.b.example.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: targeted-route + namespace: envoy-gateway + name: httproute/envoy-gateway/targeted-route/rule/0/match/0/targeted_b_example_com + pathMatch: + distinct: false + name: "" + prefix: /toy diff --git a/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.in.yaml b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.in.yaml new file mode 100644 index 00000000000..6c9aa71d29c --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.in.yaml @@ -0,0 +1,54 @@ +gateways: + - apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-a + namespace: envoy-gateway + 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 + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-b + rules: + - matches: + - method: GET + path: + type: PathPrefix + value: /toy diff --git a/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.out.yaml b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.out.yaml new file mode 100644 index 00000000000..4e6bef64b9e --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-same-ns.out.yaml @@ -0,0 +1,247 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-a + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + hostname: '*.a.example.com' + name: default + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: default + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-b + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + hostname: '*.b.example.com' + name: default + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: default + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + 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 + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-b + rules: + - matches: + - method: GET + path: + type: PathPrefix + value: /toy + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-a + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-b +infraIR: + envoy-gateway/gateway-a: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-a/default + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-a + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-a + envoy-gateway/gateway-b: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-b/default + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-b + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-b +xdsIR: + envoy-gateway/gateway-a: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*.a.example.com' + isHTTP2: false + metadata: + kind: Gateway + name: gateway-a + namespace: envoy-gateway + sectionName: default + name: envoy-gateway/gateway-a/default + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - directResponse: + statusCode: 500 + headerMatches: + - distinct: false + exact: GET + name: :method + hostname: targeted.a.example.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: targeted-route + namespace: envoy-gateway + name: httproute/envoy-gateway/targeted-route/rule/0/match/0/targeted_a_example_com + pathMatch: + distinct: false + name: "" + prefix: /toy + envoy-gateway/gateway-b: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*.b.example.com' + isHTTP2: false + metadata: + kind: Gateway + name: gateway-b + namespace: envoy-gateway + sectionName: default + name: envoy-gateway/gateway-b/default + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - directResponse: + statusCode: 500 + headerMatches: + - distinct: false + exact: GET + name: :method + hostname: targeted.b.example.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: targeted-route + namespace: envoy-gateway + name: httproute/envoy-gateway/targeted-route/rule/0/match/0/targeted_b_example_com + pathMatch: + distinct: false + name: "" + prefix: /toy diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 1268ce35b0f..b2d9b889bed 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 + Route with multiple parents has incorrect namespace in parentRef status # Enhancements that improve performance. performance improvements: |