fix: merge SecurityPolicy authorization.rules by name across hierarchy

api/v1alpha1/authorization_types.go

@@ -20,8 +20,21 @@ 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. 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.
+	//
+	// +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.

charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml

@@ -178,6 +178,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.

charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml

@@ -177,6 +177,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.

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,27 @@ func (t *Translator) processSecurityPolicyForRoute(
 					fmt.Sprintf("Merged with policy %s/%s", parentPolicy.Namespace, parentPolicy.Name),
 					policy.Generation,
 				)
+
+				// 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("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,
+					)
+				}
 			}
 		}
 	}
@@ -2576,15 +2597,45 @@ 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 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, nil
-	}
-	mergedPolicy, err := utils.Merge[*egv1a1.SecurityPolicy](parentPolicy, routePolicy, *routePolicy.Spec.MergeType)
+		return routePolicy, nil, false, nil
+	}
+	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 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
+	}
+	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), authzRulesMergeFellBack, 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 mergedPolicy, buildSecurityPolicyOwners(routePolicy, parentPolicy), nil
+	return false
 }
 
 // ownerOf returns route if routeOwns(route) is true, otherwise parent.

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)
 

internal/gatewayapi/status/policy.go

@@ -25,6 +25,12 @@ 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 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"
 )
 
 type PolicyResolveError struct {