From 193b2306b2c901e0ce0c8536f0b5c0e8f3de0100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?VILLERS=20Micka=C3=ABl?= Date: Thu, 21 May 2026 10:15:14 +0200 Subject: [PATCH 1/6] fix: merge SecurityPolicy authorization.rules by name across hierarchy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Authorization.Rules` slice on `SecurityPolicySpec` was missing the Kubernetes strategic-merge annotations (`patchMergeKey` / `patchStrategy`), causing `mergeType: StrategicMerge` (and `JSONMerge` by spec) to replace the entire parent rule set instead of merging element-wise by name. This is the same gap fixed by #6951 for `Compression` in `BackendTrafficPolicy` and by #5915 for the rate-limit rules. With this change, a route-level SecurityPolicy with `mergeType: StrategicMerge` and its own `authorization.rules` is merged with the Gateway-level parent SecurityPolicy by rule `name`: - rules with the same name in child and parent: child overrides parent - rules only present in the child: appended to parent's rule list - rules without an explicit `name` fall back to slice-replace behavior Added an end-to-end translator testdata case (`securitypolicy-with-merge-authorization-rules`) demonstrating the concatenation of parent and child rules in the resulting IR. Fixes #9053 Signed-off-by: VILLERS Mickaël --- api/v1alpha1/authorization_types.go | 12 +- ...ateway.envoyproxy.io_securitypolicies.yaml | 7 + ...ateway.envoyproxy.io_securitypolicies.yaml | 7 + ...icy-with-merge-authorization-rules.in.yaml | 72 +++++ ...cy-with-merge-authorization-rules.out.yaml | 274 ++++++++++++++++++ release-notes/current.yaml | 2 +- 6 files changed, 372 insertions(+), 2 deletions(-) create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.in.yaml create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.out.yaml diff --git a/api/v1alpha1/authorization_types.go b/api/v1alpha1/authorization_types.go index 1e52ffacec..42342e03f8 100644 --- a/api/v1alpha1/authorization_types.go +++ b/api/v1alpha1/authorization_types.go @@ -20,8 +20,18 @@ type Authorization struct { // For example, if there are two rules: the first rule allows the request // and the second rule denies it, when a request matches both rules, it will be allowed. // + // When this `SecurityPolicy` is merged with another `SecurityPolicy` via + // `mergeType: StrategicMerge`, rules are merged by their `name` field — + // child rules with the same name as parent rules override the parent's + // configuration, and child rules with new names are concatenated with the + // parent's. Rules without an explicit `name` cannot be merged by name and + // fall back to the slice-replace behavior. + // + // +patchMergeKey=name + // +patchStrategy=merge + // // +optional - Rules []AuthorizationRule `json:"rules,omitempty"` + Rules []AuthorizationRule `json:"rules,omitempty" patchMergeKey:"name" patchStrategy:"merge"` // DefaultAction defines the default action to be taken if no rules match. // If not specified, the default action is Deny. diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml index 4d618a9f25..3e7f09932d 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -178,6 +178,13 @@ spec: For example, if there are two rules: the first rule allows the request and the second rule denies it, when a request matches both rules, it will be allowed. + + When this `SecurityPolicy` is merged with another `SecurityPolicy` via + `mergeType: StrategicMerge`, rules are merged by their `name` field — + child rules with the same name as parent rules override the parent's + configuration, and child rules with new names are concatenated with the + parent's. Rules without an explicit `name` cannot be merged by name and + fall back to the slice-replace behavior. items: description: AuthorizationRule defines a single authorization rule. diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 47686dab2d..b3032dddd2 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -177,6 +177,13 @@ spec: For example, if there are two rules: the first rule allows the request and the second rule denies it, when a request matches both rules, it will be allowed. + + When this `SecurityPolicy` is merged with another `SecurityPolicy` via + `mergeType: StrategicMerge`, rules are merged by their `name` field — + child rules with the same name as parent rules override the parent's + configuration, and child rules with new names are concatenated with the + parent's. Rules without an explicit `name` cannot be merged by name and + fall back to the slice-replace behavior. items: description: AuthorizationRule defines a single authorization rule. diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.in.yaml new file mode 100644 index 0000000000..7d29d9f626 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.in.yaml @@ -0,0 +1,72 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: policy-for-gateway + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http + authorization: + defaultAction: Allow + rules: + - name: parent-internal + action: Allow + principal: + clientCIDRs: + - 10.0.0.0/8 +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-for-route + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + authorization: + defaultAction: Deny + rules: + - name: child-specific + action: Allow + principal: + clientCIDRs: + - 1.2.3.4/32 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.out.yaml new file mode 100644 index 0000000000..284d5102fd --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules.out.yaml @@ -0,0 +1,274 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + 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: http + 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: + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + 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: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-for-route + namespace: default + spec: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: child-specific + principal: + clientCIDRs: + - 1.2.3.4/32 + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Merged with policy envoy-gateway/policy-for-gateway + reason: Merged + status: "True" + type: Merged + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-for-gateway + namespace: envoy-gateway + spec: + authorization: + defaultAction: Allow + rules: + - action: Allow + name: parent-internal + principal: + clientCIDRs: + - 10.0.0.0/8 + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: 'This policy is being merged by other securityPolicies for these + routes: [default/httproute-1]' + reason: Merged + status: "True" + type: Merged + controllerName: gateway.envoyproxy.io/gatewayclass-controller +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /foo + security: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: child-specific + principal: + clientCIDRs: + - cidr: 1.2.3.4/32 + distinct: false + invert: false + isIPv6: false + maskLen: 32 + - action: Allow + name: parent-internal + principal: + clientCIDRs: + - cidr: 10.0.0.0/8 + distinct: false + invert: false + isIPv6: false + maskLen: 8 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 4fa4db5bfc..49adc48c07 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -18,7 +18,7 @@ bug fixes: | Fixed missing deprecated field warning in ClientTrafficPolicy and SecurityPolicy. Fixed ClientTrafficPolicy TLS cipher validation rejecting supported IANA/RFC cipher suite names. Fixed TLS secrets with non-canonical PEM formatting (e.g. unusual line endings) being passed verbatim to Envoy, which could cause BoringSSL errors such as `BAD_END_LINE`. Cert and key PEM data is now re-encoded to a canonical form before being delivered as xDS resources. - + Fixed SecurityPolicy `authorization.rules` not being merged across hierarchy levels when using `mergeType: StrategicMerge` by adding the missing `patchMergeKey` and `patchStrategy` annotations on the Rules slice. # Enhancements that improve performance. performance improvements: | From 9f97f618eaf0c92174cbf2beaabdd99edac7090b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?VILLERS=20Micka=C3=ABl?= Date: Thu, 21 May 2026 10:24:42 +0200 Subject: [PATCH 2/6] test: cover SecurityPolicy merge with unnamed authorization rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a translator testdata case exercising three combinations of unnamed rules (parent unnamed / child named, parent named / child unnamed, both unnamed) under `mergeType: StrategicMerge`. The expected IR in `.out.yaml` confirms that rules without an explicit `name` cannot benefit from the new keyed merge and silently fall back to the pre-existing slice-replace behavior: the child's `authorization.rules` fully replaces the parent's, exactly as before the previous commit. No merge error is raised; child policies still report `Accepted: True` in their status. Existing users that omit `name` therefore see no behavior change. Documents the regression-free fallback flagged in PR review. Signed-off-by: VILLERS Mickaël --- ...-merge-authorization-rules-unnamed.in.yaml | 182 +++++++ ...merge-authorization-rules-unnamed.out.yaml | 500 ++++++++++++++++++ 2 files changed, 682 insertions(+) create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml new file mode 100644 index 0000000000..af044e9be0 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml @@ -0,0 +1,182 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-parent-unnamed + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/parent-unnamed" + backendRefs: + - name: service-1 + port: 8080 +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-child-unnamed + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/child-unnamed" + backendRefs: + - name: service-2 + port: 8080 +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-both-unnamed + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/both-unnamed" + backendRefs: + - name: service-3 + port: 8080 +securityPolicies: +# CASE A: parent has UNNAMED rule, child has NAMED rule. Tests whether parent's +# nameless rule is preserved / dropped / errors when merge key 'name' is missing. +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: parent-unnamed-rule + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-parent-unnamed + authorization: + defaultAction: Allow + rules: + - action: Allow + principal: + clientCIDRs: + - 10.0.0.0/8 +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: child-named-rule-A + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-parent-unnamed + authorization: + defaultAction: Deny + rules: + - name: child-specific + action: Allow + principal: + clientCIDRs: + - 1.2.3.4/32 +# CASE B: parent has NAMED rule, child has UNNAMED rule. Tests whether child's +# nameless rule is appended / dropped / errors with merge key 'name' missing. +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: parent-named-rule + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-child-unnamed + authorization: + defaultAction: Allow + rules: + - name: parent-internal + action: Allow + principal: + clientCIDRs: + - 10.0.0.0/8 +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: child-unnamed-rule + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-child-unnamed + authorization: + defaultAction: Deny + rules: + - action: Allow + principal: + clientCIDRs: + - 1.2.3.4/32 +# CASE C: both parent and child have UNNAMED rules. Tests the worst case for +# regression — pre-fix behavior was full slice-replace; we want to confirm we +# don't break it. +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: parent-unnamed-rule-C + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-both-unnamed + authorization: + defaultAction: Allow + rules: + - action: Allow + principal: + clientCIDRs: + - 10.0.0.0/8 +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: child-unnamed-rule-C + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-both-unnamed + authorization: + defaultAction: Deny + rules: + - action: Allow + principal: + clientCIDRs: + - 1.2.3.4/32 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml new file mode 100644 index 0000000000..0c69f39540 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml @@ -0,0 +1,500 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 3 + 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: http + 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: + name: httproute-parent-unnamed + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /parent-unnamed + 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: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-child-unnamed + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-2 + port: 8080 + matches: + - path: + value: /child-unnamed + 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: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-both-unnamed + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-3 + port: 8080 + matches: + - path: + value: /both-unnamed + 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: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: parent-unnamed-rule + namespace: envoy-gateway + spec: + authorization: + defaultAction: Allow + rules: + - action: Allow + principal: + clientCIDRs: + - 10.0.0.0/8 + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-parent-unnamed + status: + ancestors: null +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: child-named-rule-A + namespace: default + spec: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: child-specific + principal: + clientCIDRs: + - 1.2.3.4/32 + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-parent-unnamed + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: parent-named-rule + namespace: envoy-gateway + spec: + authorization: + defaultAction: Allow + rules: + - action: Allow + name: parent-internal + principal: + clientCIDRs: + - 10.0.0.0/8 + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-child-unnamed + status: + ancestors: null +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: child-unnamed-rule + namespace: default + spec: + authorization: + defaultAction: Deny + rules: + - action: Allow + principal: + clientCIDRs: + - 1.2.3.4/32 + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-child-unnamed + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: parent-unnamed-rule-C + namespace: envoy-gateway + spec: + authorization: + defaultAction: Allow + rules: + - action: Allow + principal: + clientCIDRs: + - 10.0.0.0/8 + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-both-unnamed + status: + ancestors: null +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: child-unnamed-rule-C + namespace: default + spec: + authorization: + defaultAction: Deny + rules: + - action: Allow + principal: + clientCIDRs: + - 1.2.3.4/32 + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-both-unnamed + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-parent-unnamed + namespace: default + name: httproute/default/httproute-parent-unnamed/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-parent-unnamed/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-parent-unnamed + namespace: default + name: httproute/default/httproute-parent-unnamed/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /parent-unnamed + security: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: child-specific + principal: + clientCIDRs: + - cidr: 1.2.3.4/32 + distinct: false + invert: false + isIPv6: false + maskLen: 32 + - destination: + metadata: + kind: HTTPRoute + name: httproute-child-unnamed + namespace: default + name: httproute/default/httproute-child-unnamed/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-2 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-child-unnamed/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-child-unnamed + namespace: default + name: httproute/default/httproute-child-unnamed/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /child-unnamed + security: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: securitypolicy/default/child-unnamed-rule/authorization/rule/0 + principal: + clientCIDRs: + - cidr: 1.2.3.4/32 + distinct: false + invert: false + isIPv6: false + maskLen: 32 + - destination: + metadata: + kind: HTTPRoute + name: httproute-both-unnamed + namespace: default + name: httproute/default/httproute-both-unnamed/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-3 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-both-unnamed/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-both-unnamed + namespace: default + name: httproute/default/httproute-both-unnamed/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /both-unnamed + security: + authorization: + defaultAction: Deny + rules: + - action: Allow + name: securitypolicy/default/child-unnamed-rule-C/authorization/rule/0 + principal: + clientCIDRs: + - cidr: 1.2.3.4/32 + distinct: false + invert: false + isIPv6: false + maskLen: 32 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 From 5ce4a3f5694750fbe293d60dbc2972dfa51267d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?VILLERS=20Micka=C3=ABl?= Date: Thu, 21 May 2026 10:29:07 +0200 Subject: [PATCH 3/6] docs: regenerate API reference for Authorization.Rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: VILLERS Mickaël --- site/content/en/latest/api/extension_types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 8a8bd691ea..dcbb77ae61 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -259,7 +259,7 @@ _Appears in:_ | Field | Type | Required | Default | Description | | --- | --- | --- | --- | --- | -| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed. | +| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed.
When this `SecurityPolicy` is merged with another `SecurityPolicy` via
`mergeType: StrategicMerge`, rules are merged by their `name` field —
child rules with the same name as parent rules override the parent's
configuration, and child rules with new names are concatenated with the
parent's. Rules without an explicit `name` cannot be merged by name and
fall back to the slice-replace behavior. | | `defaultAction` | _[AuthorizationAction](#authorizationaction)_ | false | | DefaultAction defines the default action to be taken if no rules match.
If not specified, the default action is Deny. | From db5e949305deace6595175f3d18fe31c821ac9b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?VILLERS=20Micka=C3=ABl?= Date: Fri, 22 May 2026 15:36:03 +0200 Subject: [PATCH 4/6] fix: graceful fallback when authorization rules omit name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strategic merge of `authorization.rules` is keyed by the rule's `name`; if any rule on either side omits its name, `strategicpatch.StrategicMergePatch` would return `does not contain declared merge key: name` and reject the child policy. Pre-existing policies that omit `name` were happily slice-replaced in EG 1.8.0 — rejecting them now would be a regression. `mergeSecurityPolicy` now detects unnamed rules and transparently switches the merge to `JSONMerge` for that operation only, which replaces the `rules` slice wholesale (matching the pre-keyed-merge behavior) while still recursively merging the other authorization fields (`defaultAction`, etc.). Existing test cases retained; the `securitypolicy-with-merge-authorization-rules-unnamed` testdata is also restructured per review feedback so the parent SP targets the Gateway (canonical hierarchy) instead of the same HTTPRoute as the child. Signed-off-by: VILLERS Mickaël --- api/v1alpha1/authorization_types.go | 5 +- ...ateway.envoyproxy.io_securitypolicies.yaml | 5 +- ...ateway.envoyproxy.io_securitypolicies.yaml | 5 +- internal/gatewayapi/securitypolicy.go | 29 +- ...-merge-authorization-rules-unnamed.in.yaml | 81 +++- ...merge-authorization-rules-unnamed.out.yaml | 454 ++++++++++++++---- site/content/en/latest/api/extension_types.md | 2 +- 7 files changed, 460 insertions(+), 121 deletions(-) diff --git a/api/v1alpha1/authorization_types.go b/api/v1alpha1/authorization_types.go index 42342e03f8..5a7ca50b35 100644 --- a/api/v1alpha1/authorization_types.go +++ b/api/v1alpha1/authorization_types.go @@ -24,8 +24,9 @@ type Authorization struct { // `mergeType: StrategicMerge`, rules are merged by their `name` field — // child rules with the same name as parent rules override the parent's // configuration, and child rules with new names are concatenated with the - // parent's. Rules without an explicit `name` cannot be merged by name and - // fall back to the slice-replace behavior. + // parent's. If any rule on either side omits `name`, the controller + // transparently falls back to `JSONMerge` for this merge operation, which + // slice-replaces the rules array (matching the pre-keyed-merge behavior). // // +patchMergeKey=name // +patchStrategy=merge diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml index 3e7f09932d..17a7009032 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -183,8 +183,9 @@ spec: `mergeType: StrategicMerge`, rules are merged by their `name` field — child rules with the same name as parent rules override the parent's configuration, and child rules with new names are concatenated with the - parent's. Rules without an explicit `name` cannot be merged by name and - fall back to the slice-replace behavior. + parent's. If any rule on either side omits `name`, the controller + transparently falls back to `JSONMerge` for this merge operation, which + slice-replaces the rules array (matching the pre-keyed-merge behavior). items: description: AuthorizationRule defines a single authorization rule. diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index b3032dddd2..772b127f4a 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -182,8 +182,9 @@ spec: `mergeType: StrategicMerge`, rules are merged by their `name` field — child rules with the same name as parent rules override the parent's configuration, and child rules with new names are concatenated with the - parent's. Rules without an explicit `name` cannot be merged by name and - fall back to the slice-replace behavior. + parent's. If any rule on either side omits `name`, the controller + transparently falls back to `JSONMerge` for this merge operation, which + slice-replaces the rules array (matching the pre-keyed-merge behavior). items: description: AuthorizationRule defines a single authorization rule. diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 463042508a..dbd0d132fa 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -2580,13 +2580,40 @@ func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv if routePolicy.Spec.MergeType == nil || parentPolicy == nil { return routePolicy, nil, nil } - mergedPolicy, err := utils.Merge[*egv1a1.SecurityPolicy](parentPolicy, routePolicy, *routePolicy.Spec.MergeType) + mergeType := *routePolicy.Spec.MergeType + // Strategic merge of authorization.rules is keyed by the rule's `name`. If + // any rule on either side omits its name, strategic merge would fail with + // `does not contain declared merge key: name`. Fall back to JSONMerge — + // which slice-replaces the rules array but still recursively merges other + // fields — to preserve the pre-keyed-merge behavior for unnamed rules + // instead of rejecting the child policy. + if mergeType == egv1a1.StrategicMerge && hasUnnamedAuthorizationRule(parentPolicy, routePolicy) { + mergeType = egv1a1.JSONMerge + } + mergedPolicy, err := utils.Merge[*egv1a1.SecurityPolicy](parentPolicy, routePolicy, mergeType) if err != nil { return nil, nil, err } return mergedPolicy, buildSecurityPolicyOwners(routePolicy, parentPolicy), nil } +// hasUnnamedAuthorizationRule reports whether any of the given policies declares +// an authorization rule whose `name` is empty. Used to skip the keyed strategic +// merge path on the Authorization.Rules slice. +func hasUnnamedAuthorizationRule(policies ...*egv1a1.SecurityPolicy) bool { + for _, p := range policies { + if p == nil || p.Spec.Authorization == nil { + continue + } + for _, r := range p.Spec.Authorization.Rules { + if r.Name == nil || *r.Name == "" { + return true + } + } + } + return false +} + // ownerOf returns route if routeOwns(route) is true, otherwise parent. // Use this when ownership of a merged field is determined by a single predicate. func ownerOf( diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml index af044e9be0..3617a2dd4e 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.in.yaml @@ -3,13 +3,44 @@ gateways: kind: Gateway metadata: namespace: envoy-gateway - name: gateway-1 + name: gateway-A spec: gatewayClassName: envoy-gateway-class listeners: - name: http protocol: HTTP port: 80 + hostname: a.example.com + allowedRoutes: + namespaces: + from: All +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-B + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + hostname: b.example.com + allowedRoutes: + namespaces: + from: All +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-C + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + hostname: c.example.com allowedRoutes: namespaces: from: All @@ -18,16 +49,16 @@ httpRoutes: kind: HTTPRoute metadata: namespace: default - name: httproute-parent-unnamed + name: httproute-A spec: parentRefs: - namespace: envoy-gateway - name: gateway-1 + name: gateway-A sectionName: http rules: - matches: - path: - value: "/parent-unnamed" + value: "/foo" backendRefs: - name: service-1 port: 8080 @@ -35,16 +66,16 @@ httpRoutes: kind: HTTPRoute metadata: namespace: default - name: httproute-child-unnamed + name: httproute-B spec: parentRefs: - namespace: envoy-gateway - name: gateway-1 + name: gateway-B sectionName: http rules: - matches: - path: - value: "/child-unnamed" + value: "/foo" backendRefs: - name: service-2 port: 8080 @@ -52,16 +83,16 @@ httpRoutes: kind: HTTPRoute metadata: namespace: default - name: httproute-both-unnamed + name: httproute-C spec: parentRefs: - namespace: envoy-gateway - name: gateway-1 + name: gateway-C sectionName: http rules: - matches: - path: - value: "/both-unnamed" + value: "/foo" backendRefs: - name: service-3 port: 8080 @@ -72,12 +103,12 @@ securityPolicies: kind: SecurityPolicy metadata: namespace: envoy-gateway - name: parent-unnamed-rule + name: parent-A spec: targetRefs: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-parent-unnamed + kind: Gateway + name: gateway-A authorization: defaultAction: Allow rules: @@ -89,13 +120,13 @@ securityPolicies: kind: SecurityPolicy metadata: namespace: default - name: child-named-rule-A + name: child-A spec: mergeType: StrategicMerge targetRefs: - group: gateway.networking.k8s.io kind: HTTPRoute - name: httproute-parent-unnamed + name: httproute-A authorization: defaultAction: Deny rules: @@ -110,12 +141,12 @@ securityPolicies: kind: SecurityPolicy metadata: namespace: envoy-gateway - name: parent-named-rule + name: parent-B spec: targetRefs: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-child-unnamed + kind: Gateway + name: gateway-B authorization: defaultAction: Allow rules: @@ -128,13 +159,13 @@ securityPolicies: kind: SecurityPolicy metadata: namespace: default - name: child-unnamed-rule + name: child-B spec: mergeType: StrategicMerge targetRefs: - group: gateway.networking.k8s.io kind: HTTPRoute - name: httproute-child-unnamed + name: httproute-B authorization: defaultAction: Deny rules: @@ -149,12 +180,12 @@ securityPolicies: kind: SecurityPolicy metadata: namespace: envoy-gateway - name: parent-unnamed-rule-C + name: parent-C spec: targetRefs: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-both-unnamed + kind: Gateway + name: gateway-C authorization: defaultAction: Allow rules: @@ -166,13 +197,13 @@ securityPolicies: kind: SecurityPolicy metadata: namespace: default - name: child-unnamed-rule-C + name: child-C spec: mergeType: StrategicMerge targetRefs: - group: gateway.networking.k8s.io kind: HTTPRoute - name: httproute-both-unnamed + name: httproute-C authorization: defaultAction: Deny rules: diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml index 0c69f39540..6ce28546ab 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml @@ -2,7 +2,7 @@ gateways: - apiVersion: gateway.networking.k8s.io/v1 kind: Gateway metadata: - name: gateway-1 + name: gateway-A namespace: envoy-gateway spec: gatewayClassName: envoy-gateway-class @@ -10,12 +10,93 @@ gateways: - allowedRoutes: namespaces: from: All + hostname: a.example.com name: http port: 80 protocol: HTTP status: listeners: - - attachedRoutes: 3 + - 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: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-B + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + hostname: b.example.com + name: http + 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: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-C + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + hostname: c.example.com + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 conditions: - lastTransitionTime: null message: Sending translated listener configuration to the data plane @@ -42,11 +123,11 @@ httpRoutes: - apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: - name: httproute-parent-unnamed + name: httproute-A namespace: default spec: parentRefs: - - name: gateway-1 + - name: gateway-A namespace: envoy-gateway sectionName: http rules: @@ -55,7 +136,7 @@ httpRoutes: port: 8080 matches: - path: - value: /parent-unnamed + value: /foo status: parents: - conditions: @@ -71,17 +152,17 @@ httpRoutes: type: ResolvedRefs controllerName: gateway.envoyproxy.io/gatewayclass-controller parentRef: - name: gateway-1 + name: gateway-A namespace: envoy-gateway sectionName: http - apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: - name: httproute-child-unnamed + name: httproute-B namespace: default spec: parentRefs: - - name: gateway-1 + - name: gateway-B namespace: envoy-gateway sectionName: http rules: @@ -90,7 +171,7 @@ httpRoutes: port: 8080 matches: - path: - value: /child-unnamed + value: /foo status: parents: - conditions: @@ -106,17 +187,17 @@ httpRoutes: type: ResolvedRefs controllerName: gateway.envoyproxy.io/gatewayclass-controller parentRef: - name: gateway-1 + name: gateway-B namespace: envoy-gateway sectionName: http - apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: - name: httproute-both-unnamed + name: httproute-C namespace: default spec: parentRefs: - - name: gateway-1 + - name: gateway-C namespace: envoy-gateway sectionName: http rules: @@ -125,7 +206,7 @@ httpRoutes: port: 8080 matches: - path: - value: /both-unnamed + value: /foo status: parents: - conditions: @@ -141,14 +222,50 @@ httpRoutes: type: ResolvedRefs controllerName: gateway.envoyproxy.io/gatewayclass-controller parentRef: - name: gateway-1 + name: gateway-C namespace: envoy-gateway sectionName: http infraIR: - envoy-gateway/gateway-1: + envoy-gateway/gateway-A: + proxy: + listeners: + - name: envoy-gateway/gateway-A/http + 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 + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-A + namespace: envoy-gateway-system + envoy-gateway/gateway-B: + proxy: + listeners: + - name: envoy-gateway/gateway-B/http + 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 + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-B + namespace: envoy-gateway-system + envoy-gateway/gateway-C: proxy: listeners: - - name: envoy-gateway/gateway-1/http + - name: envoy-gateway/gateway-C/http ports: - containerPort: 10080 name: http-80 @@ -156,44 +273,63 @@ infraIR: servicePort: 80 metadata: labels: - gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-name: gateway-C gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway ownerReference: kind: GatewayClass name: envoy-gateway-class - name: envoy-gateway/gateway-1 + name: envoy-gateway/gateway-C namespace: envoy-gateway-system securityPolicies: - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: - name: parent-unnamed-rule - namespace: envoy-gateway + name: child-A + namespace: default spec: authorization: - defaultAction: Allow + defaultAction: Deny rules: - action: Allow + name: child-specific principal: clientCIDRs: - - 10.0.0.0/8 + - 1.2.3.4/32 + mergeType: StrategicMerge targetRefs: - group: gateway.networking.k8s.io kind: HTTPRoute - name: httproute-parent-unnamed + name: httproute-A status: - ancestors: null + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-A + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Merged with policy envoy-gateway/parent-A + reason: Merged + status: "True" + type: Merged + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: - name: child-named-rule-A + name: child-B namespace: default spec: authorization: defaultAction: Deny rules: - action: Allow - name: child-specific principal: clientCIDRs: - 1.2.3.4/32 @@ -201,16 +337,21 @@ securityPolicies: targetRefs: - group: gateway.networking.k8s.io kind: HTTPRoute - name: httproute-parent-unnamed + name: httproute-B status: ancestors: - ancestorRef: group: gateway.networking.k8s.io kind: Gateway - name: gateway-1 + name: gateway-B namespace: envoy-gateway sectionName: http conditions: + - lastTransitionTime: null + message: Merged with policy envoy-gateway/parent-B + reason: Merged + status: "True" + type: Merged - lastTransitionTime: null message: Policy has been accepted. reason: Accepted @@ -220,110 +361,155 @@ securityPolicies: - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: - name: parent-named-rule - namespace: envoy-gateway + name: child-C + namespace: default spec: authorization: - defaultAction: Allow + defaultAction: Deny rules: - action: Allow - name: parent-internal principal: clientCIDRs: - - 10.0.0.0/8 + - 1.2.3.4/32 + mergeType: StrategicMerge targetRefs: - group: gateway.networking.k8s.io kind: HTTPRoute - name: httproute-child-unnamed + name: httproute-C status: - ancestors: null + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-C + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Merged with policy envoy-gateway/parent-C + reason: Merged + status: "True" + type: Merged + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: - name: child-unnamed-rule - namespace: default + name: parent-A + namespace: envoy-gateway spec: authorization: - defaultAction: Deny + defaultAction: Allow rules: - action: Allow principal: clientCIDRs: - - 1.2.3.4/32 - mergeType: StrategicMerge + - 10.0.0.0/8 targetRefs: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-child-unnamed + kind: Gateway + name: gateway-A status: ancestors: - ancestorRef: group: gateway.networking.k8s.io kind: Gateway - name: gateway-1 + name: gateway-A namespace: envoy-gateway - sectionName: http conditions: - lastTransitionTime: null message: Policy has been accepted. reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: 'This policy is being merged by other securityPolicies for these + routes: [default/httproute-A]' + reason: Merged + status: "True" + type: Merged controllerName: gateway.envoyproxy.io/gatewayclass-controller - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: - name: parent-unnamed-rule-C + name: parent-B namespace: envoy-gateway spec: authorization: defaultAction: Allow rules: - action: Allow + name: parent-internal principal: clientCIDRs: - 10.0.0.0/8 targetRefs: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-both-unnamed + kind: Gateway + name: gateway-B status: - ancestors: null + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-B + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: 'This policy is being merged by other securityPolicies for these + routes: [default/httproute-B]' + reason: Merged + status: "True" + type: Merged + controllerName: gateway.envoyproxy.io/gatewayclass-controller - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: - name: child-unnamed-rule-C - namespace: default + name: parent-C + namespace: envoy-gateway spec: authorization: - defaultAction: Deny + defaultAction: Allow rules: - action: Allow principal: clientCIDRs: - - 1.2.3.4/32 - mergeType: StrategicMerge + - 10.0.0.0/8 targetRefs: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-both-unnamed + kind: Gateway + name: gateway-C status: ancestors: - ancestorRef: group: gateway.networking.k8s.io kind: Gateway - name: gateway-1 + name: gateway-C namespace: envoy-gateway - sectionName: http conditions: - lastTransitionTime: null message: Policy has been accepted. reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: 'This policy is being merged by other securityPolicies for these + routes: [default/httproute-C]' + reason: Merged + status: "True" + type: Merged controllerName: gateway.envoyproxy.io/gatewayclass-controller xdsIR: - envoy-gateway/gateway-1: + envoy-gateway/gateway-A: accessLog: json: - path: /dev/stdout @@ -331,10 +517,10 @@ xdsIR: proxyServiceCluster: metadata: kind: Service - name: envoy-envoy-gateway-gateway-1-196ae069 + name: envoy-envoy-gateway-gateway-A-55bef863 namespace: envoy-gateway-system sectionName: "8080" - name: envoy-gateway/gateway-1 + name: envoy-gateway/gateway-A settings: - addressType: IP endpoints: @@ -343,22 +529,22 @@ xdsIR: zone: zone1 metadata: kind: Service - name: envoy-envoy-gateway-gateway-1-196ae069 + name: envoy-envoy-gateway-gateway-A-55bef863 namespace: envoy-gateway-system sectionName: "8080" - name: envoy-gateway/gateway-1 + name: envoy-gateway/gateway-A protocol: TCP http: - address: 0.0.0.0 externalPort: 80 hostnames: - - '*' + - a.example.com metadata: kind: Gateway - name: gateway-1 + name: gateway-A namespace: envoy-gateway sectionName: http - name: envoy-gateway/gateway-1/http + name: envoy-gateway/gateway-A/http path: escapedSlashesAction: UnescapeAndRedirect mergeSlashes: true @@ -367,9 +553,9 @@ xdsIR: - destination: metadata: kind: HTTPRoute - name: httproute-parent-unnamed + name: httproute-A namespace: default - name: httproute/default/httproute-parent-unnamed/rule/0 + name: httproute/default/httproute-A/rule/0 settings: - addressType: IP endpoints: @@ -380,20 +566,20 @@ xdsIR: name: service-1 namespace: default sectionName: "8080" - name: httproute/default/httproute-parent-unnamed/rule/0/backend/0 + name: httproute/default/httproute-A/rule/0/backend/0 protocol: HTTP weight: 1 - hostname: '*' + hostname: a.example.com isHTTP2: false metadata: kind: HTTPRoute - name: httproute-parent-unnamed + name: httproute-A namespace: default - name: httproute/default/httproute-parent-unnamed/rule/0/match/0/* + name: httproute/default/httproute-A/rule/0/match/0/a_example_com pathMatch: distinct: false name: "" - prefix: /parent-unnamed + prefix: /foo security: authorization: defaultAction: Deny @@ -407,12 +593,58 @@ xdsIR: invert: false isIPv6: false maskLen: 32 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 + envoy-gateway/gateway-B: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-B-93731b94 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-B + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-B-93731b94 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-B + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - b.example.com + metadata: + kind: Gateway + name: gateway-B + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-B/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: - destination: metadata: kind: HTTPRoute - name: httproute-child-unnamed + name: httproute-B namespace: default - name: httproute/default/httproute-child-unnamed/rule/0 + name: httproute/default/httproute-B/rule/0 settings: - addressType: IP endpoints: @@ -423,26 +655,26 @@ xdsIR: name: service-2 namespace: default sectionName: "8080" - name: httproute/default/httproute-child-unnamed/rule/0/backend/0 + name: httproute/default/httproute-B/rule/0/backend/0 protocol: HTTP weight: 1 - hostname: '*' + hostname: b.example.com isHTTP2: false metadata: kind: HTTPRoute - name: httproute-child-unnamed + name: httproute-B namespace: default - name: httproute/default/httproute-child-unnamed/rule/0/match/0/* + name: httproute/default/httproute-B/rule/0/match/0/b_example_com pathMatch: distinct: false name: "" - prefix: /child-unnamed + prefix: /foo security: authorization: defaultAction: Deny rules: - action: Allow - name: securitypolicy/default/child-unnamed-rule/authorization/rule/0 + name: securitypolicy/default/child-B/authorization/rule/0 principal: clientCIDRs: - cidr: 1.2.3.4/32 @@ -450,12 +682,58 @@ xdsIR: invert: false isIPv6: false maskLen: 32 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 + envoy-gateway/gateway-C: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-C-e2859049 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-C + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-C-e2859049 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-C + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - c.example.com + metadata: + kind: Gateway + name: gateway-C + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-C/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: - destination: metadata: kind: HTTPRoute - name: httproute-both-unnamed + name: httproute-C namespace: default - name: httproute/default/httproute-both-unnamed/rule/0 + name: httproute/default/httproute-C/rule/0 settings: - addressType: IP endpoints: @@ -466,26 +744,26 @@ xdsIR: name: service-3 namespace: default sectionName: "8080" - name: httproute/default/httproute-both-unnamed/rule/0/backend/0 + name: httproute/default/httproute-C/rule/0/backend/0 protocol: HTTP weight: 1 - hostname: '*' + hostname: c.example.com isHTTP2: false metadata: kind: HTTPRoute - name: httproute-both-unnamed + name: httproute-C namespace: default - name: httproute/default/httproute-both-unnamed/rule/0/match/0/* + name: httproute/default/httproute-C/rule/0/match/0/c_example_com pathMatch: distinct: false name: "" - prefix: /both-unnamed + prefix: /foo security: authorization: defaultAction: Deny rules: - action: Allow - name: securitypolicy/default/child-unnamed-rule-C/authorization/rule/0 + name: securitypolicy/default/child-C/authorization/rule/0 principal: clientCIDRs: - cidr: 1.2.3.4/32 diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index dcbb77ae61..c1013370a8 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -259,7 +259,7 @@ _Appears in:_ | Field | Type | Required | Default | Description | | --- | --- | --- | --- | --- | -| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed.
When this `SecurityPolicy` is merged with another `SecurityPolicy` via
`mergeType: StrategicMerge`, rules are merged by their `name` field —
child rules with the same name as parent rules override the parent's
configuration, and child rules with new names are concatenated with the
parent's. Rules without an explicit `name` cannot be merged by name and
fall back to the slice-replace behavior. | +| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed.
When this `SecurityPolicy` is merged with another `SecurityPolicy` via
`mergeType: StrategicMerge`, rules are merged by their `name` field —
child rules with the same name as parent rules override the parent's
configuration, and child rules with new names are concatenated with the
parent's. If any rule on either side omits `name`, the controller
transparently falls back to `JSONMerge` for this merge operation, which
slice-replaces the rules array (matching the pre-keyed-merge behavior). | | `defaultAction` | _[AuthorizationAction](#authorizationaction)_ | false | | DefaultAction defines the default action to be taken if no rules match.
If not specified, the default action is Deny. | From 66a21da797a2a1c07c99c995c38bced1afbf6100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?VILLERS=20Mickae=CC=88l?= Date: Wed, 27 May 2026 11:56:16 +0200 Subject: [PATCH 5/6] feat: warn when authorization.rules merge falls back to JSONMerge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a SecurityPolicy with mergeType: StrategicMerge is merged across the Gateway -> Route hierarchy and any authorization rule omits its `name`, the controller already falls back to JSONMerge (slice-replace) so the child policy is not rejected. That fallback was silent. Surface it as a Warning condition (reason AuthorizationRulesMergeFallback) on the SecurityPolicy via SetWarningForPolicyAncestor, so users can notice that element-wise merging did not happen and learn how to enable it. mergeSecurityPolicy now reports whether the fallback occurred; the field doc comment, CRDs, API reference and release note are updated accordingly, and a unit case plus the existing translator testdata assert the warning. Addresses review feedback on #9054. Signed-off-by: VILLERS Mickaël --- api/v1alpha1/authorization_types.go | 7 +-- ...ateway.envoyproxy.io_securitypolicies.yaml | 7 +-- ...ateway.envoyproxy.io_securitypolicies.yaml | 7 +-- internal/gatewayapi/securitypolicy.go | 30 ++++++++--- internal/gatewayapi/securitypolicy_test.go | 51 +++++++++++++++++-- internal/gatewayapi/status/policy.go | 5 ++ ...merge-authorization-rules-unnamed.out.yaml | 24 +++++++++ release-notes/current.yaml | 2 +- site/content/en/latest/api/extension_types.md | 2 +- 9 files changed, 115 insertions(+), 20 deletions(-) diff --git a/api/v1alpha1/authorization_types.go b/api/v1alpha1/authorization_types.go index 5a7ca50b35..cdb875195b 100644 --- a/api/v1alpha1/authorization_types.go +++ b/api/v1alpha1/authorization_types.go @@ -24,9 +24,10 @@ type Authorization struct { // `mergeType: StrategicMerge`, rules are merged by their `name` field — // child rules with the same name as parent rules override the parent's // configuration, and child rules with new names are concatenated with the - // parent's. If any rule on either side omits `name`, the controller - // transparently falls back to `JSONMerge` for this merge operation, which - // slice-replaces the rules array (matching the pre-keyed-merge behavior). + // parent's. If any rule on either side omits `name`, the controller falls + // back to `JSONMerge` for this merge operation, which slice-replaces the + // rules array (matching the pre-keyed-merge behavior) and surfaces a + // `Warning` condition on the SecurityPolicy. // // +patchMergeKey=name // +patchStrategy=merge diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml index 17a7009032..33d7768072 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -183,9 +183,10 @@ spec: `mergeType: StrategicMerge`, rules are merged by their `name` field — child rules with the same name as parent rules override the parent's configuration, and child rules with new names are concatenated with the - parent's. If any rule on either side omits `name`, the controller - transparently falls back to `JSONMerge` for this merge operation, which - slice-replaces the rules array (matching the pre-keyed-merge behavior). + parent's. If any rule on either side omits `name`, the controller falls + back to `JSONMerge` for this merge operation, which slice-replaces the + rules array (matching the pre-keyed-merge behavior) and surfaces a + `Warning` condition on the SecurityPolicy. items: description: AuthorizationRule defines a single authorization rule. diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 772b127f4a..395838ebac 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -182,9 +182,10 @@ spec: `mergeType: StrategicMerge`, rules are merged by their `name` field — child rules with the same name as parent rules override the parent's configuration, and child rules with new names are concatenated with the - parent's. If any rule on either side omits `name`, the controller - transparently falls back to `JSONMerge` for this merge operation, which - slice-replaces the rules array (matching the pre-keyed-merge behavior). + parent's. If any rule on either side omits `name`, the controller falls + back to `JSONMerge` for this merge operation, which slice-replaces the + rules array (matching the pre-keyed-merge behavior) and surfaces a + `Warning` condition on the SecurityPolicy. items: description: AuthorizationRule defines a single authorization rule. diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index dbd0d132fa..bd78a645b4 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -439,7 +439,7 @@ func (t *Translator) processSecurityPolicyForRoute( } // Merge with parent policy - mergedPolicy, owners, err := mergeSecurityPolicy(policy, parentPolicy) + mergedPolicy, owners, authzRulesMergeFellBack, err := mergeSecurityPolicy(policy, parentPolicy) if err != nil { status.SetConditionForPolicyAncestor(&policy.Status, &ancestorRef, @@ -498,6 +498,22 @@ func (t *Translator) processSecurityPolicyForRoute( fmt.Sprintf("Merged with policy %s/%s", parentPolicy.Namespace, parentPolicy.Name), policy.Generation, ) + + // Surface a warning when StrategicMerge was downgraded to JSONMerge + // for authorization.rules because a rule omits its `name` merge key, + // so the slice-replace fallback is not silent. + if authzRulesMergeFellBack { + status.SetWarningForPolicyAncestor(&policy.Status, + &ancestorRef, + t.GatewayControllerName, + status.PolicyReasonAuthorizationRulesMergeFallback, + fmt.Sprintf("authorization.rules were merged with policy %s/%s using JSONMerge (slice-replace) "+ + "instead of the requested StrategicMerge because one or more rules omit the `name` field "+ + "used as the merge key; set a unique name on every authorization rule to enable element-wise merging", + parentPolicy.Namespace, parentPolicy.Name), + policy.Generation, + ) + } } } } @@ -2576,9 +2592,10 @@ func policyOwnerOr(owner, fallback *egv1a1.SecurityPolicy) *egv1a1.SecurityPolic } // mergeSecurityPolicy merges a route-level SecurityPolicy with a parent (Gateway/Listener) SecurityPolicy. -func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv1a1.SecurityPolicy, *securityPolicyOwners, error) { +// It also reports whether the requested StrategicMerge fell back to JSONMerge for authorization rules. +func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv1a1.SecurityPolicy, *securityPolicyOwners, bool, error) { if routePolicy.Spec.MergeType == nil || parentPolicy == nil { - return routePolicy, nil, nil + return routePolicy, nil, false, nil } mergeType := *routePolicy.Spec.MergeType // Strategic merge of authorization.rules is keyed by the rule's `name`. If @@ -2587,14 +2604,15 @@ func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv // which slice-replaces the rules array but still recursively merges other // fields — to preserve the pre-keyed-merge behavior for unnamed rules // instead of rejecting the child policy. - if mergeType == egv1a1.StrategicMerge && hasUnnamedAuthorizationRule(parentPolicy, routePolicy) { + authzRulesMergeFellBack := mergeType == egv1a1.StrategicMerge && hasUnnamedAuthorizationRule(parentPolicy, routePolicy) + if authzRulesMergeFellBack { mergeType = egv1a1.JSONMerge } mergedPolicy, err := utils.Merge[*egv1a1.SecurityPolicy](parentPolicy, routePolicy, mergeType) if err != nil { - return nil, nil, err + return nil, nil, false, err } - return mergedPolicy, buildSecurityPolicyOwners(routePolicy, parentPolicy), nil + return mergedPolicy, buildSecurityPolicyOwners(routePolicy, parentPolicy), authzRulesMergeFellBack, nil } // hasUnnamedAuthorizationRule reports whether any of the given policies declares diff --git a/internal/gatewayapi/securitypolicy_test.go b/internal/gatewayapi/securitypolicy_test.go index 189db2ff32..6c4c9559ea 100644 --- a/internal/gatewayapi/securitypolicy_test.go +++ b/internal/gatewayapi/securitypolicy_test.go @@ -1695,6 +1695,7 @@ func TestMergeSecurityPolicy(t *testing.T) { parentPolicy *egv1a1.SecurityPolicy wantSpec egv1a1.SecurityPolicySpec wantErr bool + wantFellBack bool }{ { name: "merge with StrategicMerge - different fields", @@ -1902,11 +1903,54 @@ func TestMergeSecurityPolicy(t *testing.T) { }, }, }, + { + name: "fallback to JSONMerge when an authorization rule omits its name", + routePolicy: &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "route-policy", Namespace: "default"}, + Spec: egv1a1.SecurityPolicySpec{ + MergeType: new(egv1a1.StrategicMerge), + Authorization: &egv1a1.Authorization{ + DefaultAction: new(egv1a1.AuthorizationActionDeny), + Rules: []egv1a1.AuthorizationRule{{ + // No Name: forces the keyed strategic merge to fall back. + Action: egv1a1.AuthorizationActionAllow, + Principal: egv1a1.Principal{ClientCIDRs: []egv1a1.CIDR{"1.2.3.4/32"}}, + }}, + }, + }, + }, + parentPolicy: &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "gateway-policy", Namespace: "default"}, + Spec: egv1a1.SecurityPolicySpec{ + Authorization: &egv1a1.Authorization{ + DefaultAction: new(egv1a1.AuthorizationActionAllow), + Rules: []egv1a1.AuthorizationRule{{ + Name: new("parent-internal"), + Action: egv1a1.AuthorizationActionAllow, + Principal: egv1a1.Principal{ClientCIDRs: []egv1a1.CIDR{"10.0.0.0/8"}}, + }}, + }, + }, + }, + // JSONMerge slice-replaces rules with the route's array while the + // route's DefaultAction overrides the parent's. + wantSpec: egv1a1.SecurityPolicySpec{ + MergeType: new(egv1a1.StrategicMerge), + Authorization: &egv1a1.Authorization{ + DefaultAction: new(egv1a1.AuthorizationActionDeny), + Rules: []egv1a1.AuthorizationRule{{ + Action: egv1a1.AuthorizationActionAllow, + Principal: egv1a1.Principal{ClientCIDRs: []egv1a1.CIDR{"1.2.3.4/32"}}, + }}, + }, + }, + wantFellBack: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, _, err := mergeSecurityPolicy(tt.routePolicy, tt.parentPolicy) + got, _, fellBack, err := mergeSecurityPolicy(tt.routePolicy, tt.parentPolicy) if (err != nil) != tt.wantErr { t.Errorf("mergeSecurityPolicy() error = %v, wantErr %v", err, tt.wantErr) return @@ -1918,6 +1962,7 @@ func TestMergeSecurityPolicy(t *testing.T) { require.Equal(t, tt.wantSpec.BasicAuth, got.Spec.BasicAuth, "BasicAuth should match") require.Equal(t, tt.wantSpec.CORS, got.Spec.CORS, "CORS should match") require.Equal(t, tt.wantSpec.Authorization, got.Spec.Authorization, "Authorization should match") + require.Equal(t, tt.wantFellBack, fellBack, "authorization rules merge fallback should match") } }) } @@ -2012,7 +2057,7 @@ func Test_securityPolicyOwnerChoose(t *testing.T) { }, } - _, owners, err := mergeSecurityPolicy(routePolicy, parentPolicy) + _, owners, _, err := mergeSecurityPolicy(routePolicy, parentPolicy) require.NoError(t, err) require.NotNil(t, owners) @@ -2053,7 +2098,7 @@ func Test_securityPolicyOwnerChoose(t *testing.T) { Spec: egv1a1.SecurityPolicySpec{MergeType: new(egv1a1.StrategicMerge)}, } - _, owners, err := mergeSecurityPolicy(routePolicy, parentPolicy) + _, owners, _, err := mergeSecurityPolicy(routePolicy, parentPolicy) require.NoError(t, err) require.NotNil(t, owners) diff --git a/internal/gatewayapi/status/policy.go b/internal/gatewayapi/status/policy.go index 6322e42928..7ee424491d 100644 --- a/internal/gatewayapi/status/policy.go +++ b/internal/gatewayapi/status/policy.go @@ -25,6 +25,11 @@ const ( // PolicyReasonMultipleWarnings is used with the "Warning" condition when multiple warning // messages need to be surfaced on the same ancestor. PolicyReasonMultipleWarnings gwapiv1.PolicyConditionReason = "Warnings" + + // PolicyReasonAuthorizationRulesMergeFallback is used with the "Warning" condition when a + // SecurityPolicy requesting StrategicMerge falls back to JSONMerge for authorization rules + // because one or more rules omit the `name` field used as the strategic-merge key. + PolicyReasonAuthorizationRulesMergeFallback gwapiv1.PolicyConditionReason = "AuthorizationRulesMergeFallback" ) type PolicyResolveError struct { diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml index 6ce28546ab..bab44fab6a 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml @@ -314,6 +314,14 @@ securityPolicies: reason: Merged status: "True" type: Merged + - lastTransitionTime: null + message: authorization.rules were merged with policy envoy-gateway/parent-A + using JSONMerge (slice-replace) instead of the requested StrategicMerge + because one or more rules omit the `name` field used as the merge key; set + a unique name on every authorization rule to enable element-wise merging + reason: AuthorizationRulesMergeFallback + status: "True" + type: Warning - lastTransitionTime: null message: Policy has been accepted. reason: Accepted @@ -352,6 +360,14 @@ securityPolicies: reason: Merged status: "True" type: Merged + - lastTransitionTime: null + message: authorization.rules were merged with policy envoy-gateway/parent-B + using JSONMerge (slice-replace) instead of the requested StrategicMerge + because one or more rules omit the `name` field used as the merge key; set + a unique name on every authorization rule to enable element-wise merging + reason: AuthorizationRulesMergeFallback + status: "True" + type: Warning - lastTransitionTime: null message: Policy has been accepted. reason: Accepted @@ -390,6 +406,14 @@ securityPolicies: reason: Merged status: "True" type: Merged + - lastTransitionTime: null + message: authorization.rules were merged with policy envoy-gateway/parent-C + using JSONMerge (slice-replace) instead of the requested StrategicMerge + because one or more rules omit the `name` field used as the merge key; set + a unique name on every authorization rule to enable element-wise merging + reason: AuthorizationRulesMergeFallback + status: "True" + type: Warning - lastTransitionTime: null message: Policy has been accepted. reason: Accepted diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 49adc48c07..7e24e52200 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -18,7 +18,7 @@ bug fixes: | Fixed missing deprecated field warning in ClientTrafficPolicy and SecurityPolicy. Fixed ClientTrafficPolicy TLS cipher validation rejecting supported IANA/RFC cipher suite names. Fixed TLS secrets with non-canonical PEM formatting (e.g. unusual line endings) being passed verbatim to Envoy, which could cause BoringSSL errors such as `BAD_END_LINE`. Cert and key PEM data is now re-encoded to a canonical form before being delivered as xDS resources. - Fixed SecurityPolicy `authorization.rules` not being merged across hierarchy levels when using `mergeType: StrategicMerge` by adding the missing `patchMergeKey` and `patchStrategy` annotations on the Rules slice. + Fixed SecurityPolicy `authorization.rules` not being merged across hierarchy levels when using `mergeType: StrategicMerge` by adding the missing `patchMergeKey` and `patchStrategy` annotations on the Rules slice. When a rule omits `name`, the merge falls back to `JSONMerge` (slice-replace) and reports a `Warning` condition on the SecurityPolicy. # Enhancements that improve performance. performance improvements: | diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index c1013370a8..f004fff003 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -259,7 +259,7 @@ _Appears in:_ | Field | Type | Required | Default | Description | | --- | --- | --- | --- | --- | -| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed.
When this `SecurityPolicy` is merged with another `SecurityPolicy` via
`mergeType: StrategicMerge`, rules are merged by their `name` field —
child rules with the same name as parent rules override the parent's
configuration, and child rules with new names are concatenated with the
parent's. If any rule on either side omits `name`, the controller
transparently falls back to `JSONMerge` for this merge operation, which
slice-replaces the rules array (matching the pre-keyed-merge behavior). | +| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed.
When this `SecurityPolicy` is merged with another `SecurityPolicy` via
`mergeType: StrategicMerge`, rules are merged by their `name` field —
child rules with the same name as parent rules override the parent's
configuration, and child rules with new names are concatenated with the
parent's. If any rule on either side omits `name`, the controller falls
back to `JSONMerge` for this merge operation, which slice-replaces the
rules array (matching the pre-keyed-merge behavior) and surfaces a
`Warning` condition on the SecurityPolicy. | | `defaultAction` | _[AuthorizationAction](#authorizationaction)_ | false | | DefaultAction defines the default action to be taken if no rules match.
If not specified, the default action is Deny. | From d2e0c780d8c36c2f08a2835c68b12f74ebfe45ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?VILLERS=20Micka=C3=ABl?= Date: Thu, 28 May 2026 15:21:23 +0200 Subject: [PATCH 6/6] docs: clarify that JSONMerge fallback affects the whole policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on #9054 from @kkk777-7: the previous warning message ("authorization.rules were merged with policy X/Y using JSONMerge") suggested only authorization.rules were slice-replaced. In fact the fallback switches the entire merge operation to JSONMerge, so every spec slice (extAuth.contextExtensions, jwt.providers, oidc.provider.backendRefs, ...) is replaced rather than strategic-merged. Reword the Warning message, the AuthorizationRulesMergeFallback constant doc, the mergeSecurityPolicy function comment, the Authorization.Rules field doc and the release note so users see what the downgrade actually costs them. Regenerated CRDs, API reference and helm test data accordingly. Also regenerate internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml to include the parent rule that the new keyed merge now correctly preserves alongside the child rule — fixes the TestMergePolicy/securitypolicy_all coverage-test failure introduced by 4d67f8d. Signed-off-by: VILLERS Mickaël --- api/v1alpha1/authorization_types.go | 7 +++-- ...ateway.envoyproxy.io_securitypolicies.yaml | 7 +++-- ...ateway.envoyproxy.io_securitypolicies.yaml | 7 +++-- internal/gatewayapi/securitypolicy.go | 28 ++++++++++------- internal/gatewayapi/status/policy.go | 5 ++-- ...merge-authorization-rules-unnamed.out.yaml | 30 +++++++++++-------- ...securitypolicy_all.strategicmerge.out.yaml | 7 +++++ release-notes/current.yaml | 2 +- site/content/en/latest/api/extension_types.md | 2 +- test/helm/gateway-crds-helm/all.out.yaml | 10 +++++++ test/helm/gateway-crds-helm/e2e.out.yaml | 10 +++++++ .../envoy-gateway-crds.out.yaml | 10 +++++++ 12 files changed, 89 insertions(+), 36 deletions(-) diff --git a/api/v1alpha1/authorization_types.go b/api/v1alpha1/authorization_types.go index cdb875195b..033be27014 100644 --- a/api/v1alpha1/authorization_types.go +++ b/api/v1alpha1/authorization_types.go @@ -25,9 +25,10 @@ type Authorization struct { // child rules with the same name as parent rules override the parent's // configuration, and child rules with new names are concatenated with the // parent's. If any rule on either side omits `name`, the controller falls - // back to `JSONMerge` for this merge operation, which slice-replaces the - // rules array (matching the pre-keyed-merge behavior) and surfaces a - // `Warning` condition on the SecurityPolicy. + // back to `JSONMerge` for the entire merge operation (not just this field): + // every spec slice is slice-replaced instead of strategic-merged, matching + // the pre-keyed-merge behavior. A `Warning` condition with reason + // `AuthorizationRulesMergeFallback` is surfaced on the SecurityPolicy. // // +patchMergeKey=name // +patchStrategy=merge diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml index 33d7768072..52b48d2b74 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -184,9 +184,10 @@ spec: child rules with the same name as parent rules override the parent's configuration, and child rules with new names are concatenated with the parent's. If any rule on either side omits `name`, the controller falls - back to `JSONMerge` for this merge operation, which slice-replaces the - rules array (matching the pre-keyed-merge behavior) and surfaces a - `Warning` condition on the SecurityPolicy. + back to `JSONMerge` for the entire merge operation (not just this field): + every spec slice is slice-replaced instead of strategic-merged, matching + the pre-keyed-merge behavior. A `Warning` condition with reason + `AuthorizationRulesMergeFallback` is surfaced on the SecurityPolicy. items: description: AuthorizationRule defines a single authorization rule. diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 395838ebac..df968fc892 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -183,9 +183,10 @@ spec: child rules with the same name as parent rules override the parent's configuration, and child rules with new names are concatenated with the parent's. If any rule on either side omits `name`, the controller falls - back to `JSONMerge` for this merge operation, which slice-replaces the - rules array (matching the pre-keyed-merge behavior) and surfaces a - `Warning` condition on the SecurityPolicy. + back to `JSONMerge` for the entire merge operation (not just this field): + every spec slice is slice-replaced instead of strategic-merged, matching + the pre-keyed-merge behavior. A `Warning` condition with reason + `AuthorizationRulesMergeFallback` is surfaced on the SecurityPolicy. items: description: AuthorizationRule defines a single authorization rule. diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index bd78a645b4..13a7e6cb01 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -499,17 +499,22 @@ func (t *Translator) processSecurityPolicyForRoute( policy.Generation, ) - // Surface a warning when StrategicMerge was downgraded to JSONMerge - // for authorization.rules because a rule omits its `name` merge key, - // so the slice-replace fallback is not silent. + // Surface a warning when the requested StrategicMerge was downgraded + // to JSONMerge because an authorization rule omits its `name` merge + // key. The downgrade applies to the entire policy spec — not only + // `authorization.rules` — so the user sees that other slices + // (e.g. `extAuth.contextExtensions`, `jwt.providers`) are also + // slice-replaced instead of strategic-merged. if authzRulesMergeFellBack { status.SetWarningForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, status.PolicyReasonAuthorizationRulesMergeFallback, - fmt.Sprintf("authorization.rules were merged with policy %s/%s using JSONMerge (slice-replace) "+ - "instead of the requested StrategicMerge because one or more rules omit the `name` field "+ - "used as the merge key; set a unique name on every authorization rule to enable element-wise merging", + fmt.Sprintf("policy was merged with %s/%s using JSONMerge (slice-replace for all fields) "+ + "instead of the requested StrategicMerge because one or more `authorization.rules` "+ + "omit the `name` field used as the strategic-merge key; this affects every spec field, "+ + "not only `authorization.rules`. Set a unique `name` on every authorization rule to "+ + "keep the requested StrategicMerge semantics", parentPolicy.Namespace, parentPolicy.Name), policy.Generation, ) @@ -2592,7 +2597,8 @@ func policyOwnerOr(owner, fallback *egv1a1.SecurityPolicy) *egv1a1.SecurityPolic } // mergeSecurityPolicy merges a route-level SecurityPolicy with a parent (Gateway/Listener) SecurityPolicy. -// It also reports whether the requested StrategicMerge fell back to JSONMerge for authorization rules. +// It also reports whether the requested StrategicMerge for the whole policy fell back to JSONMerge +// because an authorization rule omits the `name` strategic-merge key. func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv1a1.SecurityPolicy, *securityPolicyOwners, bool, error) { if routePolicy.Spec.MergeType == nil || parentPolicy == nil { return routePolicy, nil, false, nil @@ -2600,10 +2606,10 @@ func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv mergeType := *routePolicy.Spec.MergeType // Strategic merge of authorization.rules is keyed by the rule's `name`. If // any rule on either side omits its name, strategic merge would fail with - // `does not contain declared merge key: name`. Fall back to JSONMerge — - // which slice-replaces the rules array but still recursively merges other - // fields — to preserve the pre-keyed-merge behavior for unnamed rules - // instead of rejecting the child policy. + // `does not contain declared merge key: name`. Fall back to JSONMerge for + // the entire policy — JSONMerge slice-replaces every array (including + // authorization.rules), matching the pre-keyed-merge behavior for unnamed + // rules — instead of rejecting the child policy. authzRulesMergeFellBack := mergeType == egv1a1.StrategicMerge && hasUnnamedAuthorizationRule(parentPolicy, routePolicy) if authzRulesMergeFellBack { mergeType = egv1a1.JSONMerge diff --git a/internal/gatewayapi/status/policy.go b/internal/gatewayapi/status/policy.go index 7ee424491d..ff3e652920 100644 --- a/internal/gatewayapi/status/policy.go +++ b/internal/gatewayapi/status/policy.go @@ -27,8 +27,9 @@ const ( PolicyReasonMultipleWarnings gwapiv1.PolicyConditionReason = "Warnings" // PolicyReasonAuthorizationRulesMergeFallback is used with the "Warning" condition when a - // SecurityPolicy requesting StrategicMerge falls back to JSONMerge for authorization rules - // because one or more rules omit the `name` field used as the strategic-merge key. + // SecurityPolicy requesting StrategicMerge falls back to JSONMerge for the entire policy + // because one or more authorization rules omit the `name` field used as the + // strategic-merge key. The downgrade affects every spec field, not only `authorization.rules`. PolicyReasonAuthorizationRulesMergeFallback gwapiv1.PolicyConditionReason = "AuthorizationRulesMergeFallback" ) diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml index bab44fab6a..014a199f4f 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-authorization-rules-unnamed.out.yaml @@ -315,10 +315,12 @@ securityPolicies: status: "True" type: Merged - lastTransitionTime: null - message: authorization.rules were merged with policy envoy-gateway/parent-A - using JSONMerge (slice-replace) instead of the requested StrategicMerge - because one or more rules omit the `name` field used as the merge key; set - a unique name on every authorization rule to enable element-wise merging + message: policy was merged with envoy-gateway/parent-A using JSONMerge (slice-replace + for all fields) instead of the requested StrategicMerge because one or more + `authorization.rules` omit the `name` field used as the strategic-merge + key; this affects every spec field, not only `authorization.rules`. Set + a unique `name` on every authorization rule to keep the requested StrategicMerge + semantics reason: AuthorizationRulesMergeFallback status: "True" type: Warning @@ -361,10 +363,12 @@ securityPolicies: status: "True" type: Merged - lastTransitionTime: null - message: authorization.rules were merged with policy envoy-gateway/parent-B - using JSONMerge (slice-replace) instead of the requested StrategicMerge - because one or more rules omit the `name` field used as the merge key; set - a unique name on every authorization rule to enable element-wise merging + message: policy was merged with envoy-gateway/parent-B using JSONMerge (slice-replace + for all fields) instead of the requested StrategicMerge because one or more + `authorization.rules` omit the `name` field used as the strategic-merge + key; this affects every spec field, not only `authorization.rules`. Set + a unique `name` on every authorization rule to keep the requested StrategicMerge + semantics reason: AuthorizationRulesMergeFallback status: "True" type: Warning @@ -407,10 +411,12 @@ securityPolicies: status: "True" type: Merged - lastTransitionTime: null - message: authorization.rules were merged with policy envoy-gateway/parent-C - using JSONMerge (slice-replace) instead of the requested StrategicMerge - because one or more rules omit the `name` field used as the merge key; set - a unique name on every authorization rule to enable element-wise merging + message: policy was merged with envoy-gateway/parent-C using JSONMerge (slice-replace + for all fields) instead of the requested StrategicMerge because one or more + `authorization.rules` omit the `name` field used as the strategic-merge + key; this affects every spec field, not only `authorization.rules`. Set + a unique `name` on every authorization rule to keep the requested StrategicMerge + semantics reason: AuthorizationRulesMergeFallback status: "True" type: Warning diff --git a/internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml b/internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml index 8eeabc5212..d2ba7fb62b 100644 --- a/internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml +++ b/internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml @@ -18,6 +18,13 @@ spec: - name: x-role values: - route + - action: Allow + name: parent-rule + principal: + headers: + - name: x-role + values: + - parent basicAuth: users: name: users-route diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 7e24e52200..7e48b0db36 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -18,7 +18,7 @@ bug fixes: | Fixed missing deprecated field warning in ClientTrafficPolicy and SecurityPolicy. Fixed ClientTrafficPolicy TLS cipher validation rejecting supported IANA/RFC cipher suite names. Fixed TLS secrets with non-canonical PEM formatting (e.g. unusual line endings) being passed verbatim to Envoy, which could cause BoringSSL errors such as `BAD_END_LINE`. Cert and key PEM data is now re-encoded to a canonical form before being delivered as xDS resources. - Fixed SecurityPolicy `authorization.rules` not being merged across hierarchy levels when using `mergeType: StrategicMerge` by adding the missing `patchMergeKey` and `patchStrategy` annotations on the Rules slice. When a rule omits `name`, the merge falls back to `JSONMerge` (slice-replace) and reports a `Warning` condition on the SecurityPolicy. + Fixed SecurityPolicy `authorization.rules` not being merged across hierarchy levels when using `mergeType: StrategicMerge` by adding the missing `patchMergeKey` and `patchStrategy` annotations on the Rules slice. If any authorization rule omits `name`, the controller falls back to `JSONMerge` for the entire policy (every spec field is slice-replaced, not only `authorization.rules`) and surfaces a `Warning` condition with reason `AuthorizationRulesMergeFallback` on the SecurityPolicy. # Enhancements that improve performance. performance improvements: | diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index f004fff003..0470426cf1 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -259,7 +259,7 @@ _Appears in:_ | Field | Type | Required | Default | Description | | --- | --- | --- | --- | --- | -| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed.
When this `SecurityPolicy` is merged with another `SecurityPolicy` via
`mergeType: StrategicMerge`, rules are merged by their `name` field —
child rules with the same name as parent rules override the parent's
configuration, and child rules with new names are concatenated with the
parent's. If any rule on either side omits `name`, the controller falls
back to `JSONMerge` for this merge operation, which slice-replaces the
rules array (matching the pre-keyed-merge behavior) and surfaces a
`Warning` condition on the SecurityPolicy. | +| `rules` | _[AuthorizationRule](#authorizationrule) array_ | false | | Rules defines a list of authorization rules.
These rules are evaluated in order, the first matching rule will be applied,
and the rest will be skipped.
For example, if there are two rules: the first rule allows the request
and the second rule denies it, when a request matches both rules, it will be allowed.
When this `SecurityPolicy` is merged with another `SecurityPolicy` via
`mergeType: StrategicMerge`, rules are merged by their `name` field —
child rules with the same name as parent rules override the parent's
configuration, and child rules with new names are concatenated with the
parent's. If any rule on either side omits `name`, the controller falls
back to `JSONMerge` for the entire merge operation (not just this field):
every spec slice is slice-replaced instead of strategic-merged, matching
the pre-keyed-merge behavior. A `Warning` condition with reason
`AuthorizationRulesMergeFallback` is surfaced on the SecurityPolicy. | | `defaultAction` | _[AuthorizationAction](#authorizationaction)_ | false | | DefaultAction defines the default action to be taken if no rules match.
If not specified, the default action is Deny. | diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index e0f84d9c12..059c710f51 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -50183,6 +50183,16 @@ spec: For example, if there are two rules: the first rule allows the request and the second rule denies it, when a request matches both rules, it will be allowed. + + When this `SecurityPolicy` is merged with another `SecurityPolicy` via + `mergeType: StrategicMerge`, rules are merged by their `name` field — + child rules with the same name as parent rules override the parent's + configuration, and child rules with new names are concatenated with the + parent's. If any rule on either side omits `name`, the controller falls + back to `JSONMerge` for the entire merge operation (not just this field): + every spec slice is slice-replaced instead of strategic-merged, matching + the pre-keyed-merge behavior. A `Warning` condition with reason + `AuthorizationRulesMergeFallback` is surfaced on the SecurityPolicy. items: description: AuthorizationRule defines a single authorization rule. diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index fb6087113e..1b59314157 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -28156,6 +28156,16 @@ spec: For example, if there are two rules: the first rule allows the request and the second rule denies it, when a request matches both rules, it will be allowed. + + When this `SecurityPolicy` is merged with another `SecurityPolicy` via + `mergeType: StrategicMerge`, rules are merged by their `name` field — + child rules with the same name as parent rules override the parent's + configuration, and child rules with new names are concatenated with the + parent's. If any rule on either side omits `name`, the controller falls + back to `JSONMerge` for the entire merge operation (not just this field): + every spec slice is slice-replaced instead of strategic-merged, matching + the pre-keyed-merge behavior. A `Warning` condition with reason + `AuthorizationRulesMergeFallback` is surfaced on the SecurityPolicy. items: description: AuthorizationRule defines a single authorization rule. diff --git a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml index 9a6ff0b16d..96e5c8cc9e 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -28156,6 +28156,16 @@ spec: For example, if there are two rules: the first rule allows the request and the second rule denies it, when a request matches both rules, it will be allowed. + + When this `SecurityPolicy` is merged with another `SecurityPolicy` via + `mergeType: StrategicMerge`, rules are merged by their `name` field — + child rules with the same name as parent rules override the parent's + configuration, and child rules with new names are concatenated with the + parent's. If any rule on either side omits `name`, the controller falls + back to `JSONMerge` for the entire merge operation (not just this field): + every spec slice is slice-replaced instead of strategic-merged, matching + the pre-keyed-merge behavior. A `Warning` condition with reason + `AuthorizationRulesMergeFallback` is surfaced on the SecurityPolicy. items: description: AuthorizationRule defines a single authorization rule.