moved shared under rules

api/v1alpha1/ratelimit_types.go

@@ -49,19 +49,8 @@ type GlobalRateLimit struct {
 	// matches two rules, one rate limited and one not, the final decision will be
 	// to rate limit the request.
 	//
-	// +patchMergeKey:"name"
-	// +patchStrategy:"merge"
 	// +kubebuilder:validation:MaxItems=64
-	Rules []RateLimitRule `json:"rules" patchMergeKey:"name" patchStrategy:"merge"`
-
-	// Shared determines whether the rate limit rules apply across all the policy targets.
-	// If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
-	// Default: false.
-	//
-	// +optional
-	// +notImplementedHide
-	// +kubebuilder:default=false
-	Shared *bool `json:"shared,omitempty"`
+	Rules []RateLimitRule `json:"rules"`
 }
 
 // LocalRateLimit defines local rate limit configuration.
@@ -71,23 +60,15 @@ type LocalRateLimit struct {
 	// matches two rules, one with 10rps and one with 20rps, the final limit will
 	// be based on the rule with 10rps.
 	//
-	// +patchMergeKey:"name"
-	// +patchStrategy:"merge"
-	//
 	// +optional
 	// +kubebuilder:validation:MaxItems=16
 	// +kubebuilder:validation:XValidation:rule="self.all(foo, !has(foo.cost) || !has(foo.cost.response))", message="response cost is not supported for Local Rate Limits"
-	Rules []RateLimitRule `json:"rules" patchMergeKey:"name" patchStrategy:"merge"`
+	Rules []RateLimitRule `json:"rules"`
 }
 
 // RateLimitRule defines the semantics for matching attributes
 // from the incoming requests, and setting limits for them.
 type RateLimitRule struct {
-	// Name is the name of the rule. This is used to identify the rule
-	// in the Envoy configuration and as a unique identifier for merging.
-	//
-	// +optional
-	Name string `json:"name,omitempty"`
 	// ClientSelectors holds the list of select conditions to select
 	// specific clients using attributes from the traffic flow.
 	// All individual select conditions must hold True for this rule
@@ -118,6 +99,12 @@ type RateLimitRule struct {
 	//
 	// +optional
 	Cost *RateLimitCost `json:"cost,omitempty"`
+	// Shared determines whether this rate limit rule applies across all the policy targets.
+	// If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
+	// Default: false.
+	//
+	// +optional
+	Shared *bool `json:"shared,omitempty"`
 }
 
 type RateLimitCost struct {

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

@@ -948,23 +948,17 @@ spec:
                               - requests
                               - unit
                               type: object
-                            name:
+                            shared:
                               description: |-
-                                Name is the name of the rule. This is used to identify the rule
-                                in the Envoy configuration and as a unique identifier for merging.
-                              type: string
+                                Shared determines whether this rate limit rule applies across all the policy targets.
+                                If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
+                                Default: false.
+                              type: boolean
                           required:
                           - limit
                           type: object
                         maxItems: 64
                         type: array
-                      shared:
-                        default: false
-                        description: |-
-                          Shared determines whether the rate limit rules apply across all the policy targets.
-                          If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
-                          Default: false.
-                        type: boolean
                     required:
                     - rules
                     type: object
@@ -1203,11 +1197,12 @@ spec:
                               - requests
                               - unit
                               type: object
-                            name:
+                            shared:
                               description: |-
-                                Name is the name of the rule. This is used to identify the rule
-                                in the Envoy configuration and as a unique identifier for merging.
-                              type: string
+                                Shared determines whether this rate limit rule applies across all the policy targets.
+                                If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
+                                Default: false.
+                              type: boolean
                           required:
                           - limit
                           type: object

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

@@ -947,23 +947,17 @@ spec:
                               - requests
                               - unit
                               type: object
-                            name:
+                            shared:
                               description: |-
-                                Name is the name of the rule. This is used to identify the rule
-                                in the Envoy configuration and as a unique identifier for merging.
-                              type: string
+                                Shared determines whether this rate limit rule applies across all the policy targets.
+                                If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
+                                Default: false.
+                              type: boolean
                           required:
                           - limit
                           type: object
                         maxItems: 64
                         type: array
-                      shared:
-                        default: false
-                        description: |-
-                          Shared determines whether the rate limit rules apply across all the policy targets.
-                          If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
-                          Default: false.
-                        type: boolean
                     required:
                     - rules
                     type: object
@@ -1202,11 +1196,12 @@ spec:
                               - requests
                               - unit
                               type: object
-                            name:
+                            shared:
                               description: |-
-                                Name is the name of the rule. This is used to identify the rule
-                                in the Envoy configuration and as a unique identifier for merging.
-                              type: string
+                                Shared determines whether this rate limit rule applies across all the policy targets.
+                                If set to true, the rule is treated as a common bucket and is shared across all policy targets (xRoutes).
+                                Default: false.
+                              type: boolean
                           required:
                           - limit
                           type: object

internal/gatewayapi/backendtrafficpolicy.go

@@ -429,13 +429,33 @@ func (t *Translator) translateBackendTrafficPolicyForRouteWithMerge(
 	if err != nil {
 		return fmt.Errorf("error merging policies: %w", err)
 	}
+
+	// Build traffic features from the merged policy
 	tf, errs := t.buildTrafficFeatures(mergedPolicy, resources)
 	if tf == nil {
 		// should not happen
 		return nil
 	}
 
-	// Apply IR to relevant gateway routes
+	// Since GlobalRateLimit merge relies on IR auto-generated key: (<policy-ns>/<policy-name>/rule/<rule-index>)
+	// We can't simply merge the BTP's using utils.Merge() we need to specifically merge the GlobalRateLimit.Rules using IR fields.
+	// Since ir.TrafficFeatures is not a built-in Kubernetes API object with defined merging strategies and it does not support a deep merge (for lists/maps).
+	if policy.Spec.RateLimit != nil && gwPolicy.Spec.RateLimit != nil {
+		tfGW, _ := t.buildTrafficFeatures(gwPolicy, resources)
+		tfRoute, _ := t.buildTrafficFeatures(policy, resources)
+
+		if tfGW != nil && tfRoute != nil &&
+			tfGW.RateLimit != nil && tfRoute.RateLimit != nil {
+
+			mergedRL, err := utils.MergeRL(tfGW.RateLimit, tfRoute.RateLimit, *policy.Spec.MergeType)
+			if err != nil {
+				return fmt.Errorf("error merging rate limits: %w", err)
+			}
+			// Replace the rate limit in the merged features if successful
+			tf.RateLimit = mergedRL
+		}
+	}
+
 	x, ok := xdsIR[t.IRKey(gatewayNN)]
 	if !ok {
 		// should not happen.
@@ -489,7 +509,6 @@ func applyTrafficFeatureToRoute(route RouteContext,
 				}
 
 				r.Traffic = tf.DeepCopy()
-				r.Traffic.Name = irTrafficName(policy)
 
 				if localTo, err := buildClusterSettingsTimeout(policy.Spec.ClusterSettings); err == nil {
 					r.Traffic.Timeout = localTo
@@ -695,7 +714,6 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(
 			}
 
 			r.Traffic = tf.DeepCopy()
-			r.Traffic.Name = irTrafficName(policy)
 
 			// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
 			r.Traffic.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)
@@ -769,7 +787,7 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) (*
 	var err error
 	var irRule *ir.RateLimitRule
 	irRules := make([]*ir.RateLimitRule, 0)
-	for _, rule := range local.Rules {
+	for i, rule := range local.Rules {
 		// We don't process the rule without clientSelectors here because it's
 		// previously used as the default route-level limit.
 		if len(rule.ClientSelectors) == 0 {
@@ -780,6 +798,8 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) (*
 		if err != nil {
 			return nil, err
 		}
+		// Set the Name field as <policy-ns>/<policy-name>/rule/<rule-index>
+		irRule.Name = irRuleName(policy.Namespace, policy.Name, i)
 		irRules = append(irRules, irRule)
 	}
 
@@ -804,8 +824,7 @@ func (t *Translator) buildGlobalRateLimit(policy *egv1a1.BackendTrafficPolicy) (
 	global := policy.Spec.RateLimit.Global
 	rateLimit := &ir.RateLimit{
 		Global: &ir.GlobalRateLimit{
-			Rules:  make([]*ir.RateLimitRule, len(global.Rules)),
-			Shared: global.Shared,
+			Rules: make([]*ir.RateLimitRule, len(global.Rules)),
 		},
 	}
 
@@ -816,6 +835,8 @@ func (t *Translator) buildGlobalRateLimit(policy *egv1a1.BackendTrafficPolicy) (
 		if err != nil {
 			return nil, err
 		}
+		// Set the Name field as <policy-ns>/<policy-name>/rule/<rule-index>
+		irRules[i].Name = irRuleName(policy.Namespace, policy.Name, i)
 	}
 
 	return rateLimit, nil
@@ -828,6 +849,7 @@ func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) {
 			Unit:     ir.RateLimitUnit(rule.Limit.Unit),
 		},
 		HeaderMatches: make([]*ir.StringMatch, 0),
+		Shared:        rule.Shared,
 	}
 
 	for _, match := range rule.ClientSelectors {

internal/gatewayapi/helpers.go

@@ -408,6 +408,10 @@ func irDestinationSettingName(destName string, backendIdx int) string {
 	return fmt.Sprintf("%s/backend/%d", destName, backendIdx)
 }
 
+func irRuleName(policyNamespace, policyName string, ruleIndex int) string {
+	return fmt.Sprintf("%s/%s/rule/%d", policyNamespace, policyName, ruleIndex)
+}
+
 // irTLSConfigs produces a defaulted IR TLSConfig
 func irTLSConfigs(tlsSecrets ...*corev1.Secret) *ir.TLSConfig {
 	if len(tlsSecrets) == 0 {
@@ -450,11 +454,6 @@ func irTLSCACertName(namespace, name string) string {
 	return fmt.Sprintf("%s/%s/%s", namespace, name, caCertKey)
 }
 
-// Helper function to format the policy name and namespace
-func irTrafficName(policy *egv1a1.BackendTrafficPolicy) string {
-	return fmt.Sprintf("%s/%s", policy.Namespace, policy.Name)
-}
-
 func IsMergeGatewaysEnabled(resources *resource.Resources) bool {
 	return resources.EnvoyProxyForGatewayClass != nil && resources.EnvoyProxyForGatewayClass.Spec.MergeGateways != nil && *resources.EnvoyProxyForGatewayClass.Spec.MergeGateways
 }

internal/gatewayapi/testdata/backendtrafficpolicy-buffer-limit-out-of-range-error.out.yaml

@@ -284,7 +284,6 @@ xdsIR:
           namespace: default
         name: grpcroute/default/grpcroute-1/rule/0/match/-1/*
         traffic:
-          name: envoy-gateway/policy-for-gateway
           timeout:
             http:
               connectionIdleTimeout: 16s

internal/gatewayapi/testdata/backendtrafficpolicy-buffer-limit-with-invalid-value.out.yaml

@@ -284,7 +284,6 @@ xdsIR:
           namespace: default
         name: grpcroute/default/grpcroute-1/rule/0/match/-1/*
         traffic:
-          name: envoy-gateway/policy-for-gateway
           timeout:
             http:
               connectionIdleTimeout: 16s

internal/gatewayapi/testdata/backendtrafficpolicy-buffer-limit.out.yaml

@@ -284,7 +284,6 @@ xdsIR:
           namespace: default
         name: grpcroute/default/grpcroute-1/rule/0/match/-1/*
         traffic:
-          name: envoy-gateway/policy-for-gateway
           timeout:
             http:
               connectionIdleTimeout: 16s
@@ -340,7 +339,6 @@ xdsIR:
         traffic:
           backendConnection:
             bufferLimit: 100000000
-          name: default/policy-for-route
     readyListener:
       address: 0.0.0.0
       ipFamily: IPv4

internal/gatewayapi/testdata/backendtrafficpolicy-compression.out.yaml

@@ -171,7 +171,6 @@ xdsIR:
           compression:
           - type: Brotli
           - type: Gzip
-          name: default/policy-for-route
     readyListener:
       address: 0.0.0.0
       ipFamily: IPv4

internal/gatewayapi/testdata/backendtrafficpolicy-dns-lookup-family.out.yaml

@@ -449,7 +449,6 @@ xdsIR:
             dnsRefreshRate: 5s
             lookupFamily: IPv6
             respectDnsTtl: false
-          name: default/backend-traffic-policy
       - destination:
           name: grpcroute/default/grpcroute-1/rule/0
           settings:
@@ -495,7 +494,6 @@ xdsIR:
             dnsRefreshRate: 5s
             lookupFamily: IPv6
             respectDnsTtl: false
-          name: default/backend-traffic-policy
     readyListener:
       address: 0.0.0.0
       ipFamily: IPv4

internal/gatewayapi/testdata/backendtrafficpolicy-http-upgrade-spdy.out.yaml

@@ -169,7 +169,6 @@ xdsIR:
         traffic:
           httpUpgrade:
           - spdy/3.1
-          name: default/policy-for-route
     readyListener:
       address: 0.0.0.0
       ipFamily: IPv4

internal/gatewayapi/testdata/backendtrafficpolicy-http-upgrade-websocket.out.yaml

@@ -171,7 +171,6 @@ xdsIR:
           httpUpgrade:
           - websocket
           - spdy/3.1
-          name: default/policy-for-route
     readyListener:
       address: 0.0.0.0
       ipFamily: IPv4

internal/gatewayapi/testdata/backendtrafficpolicy-override-replace.out.yaml

@@ -313,7 +313,6 @@ xdsIR:
           loadBalancer:
             consistentHash:
               sourceIP: true
-          name: default/policy-for-route-1
       - destination:
           name: httproute/default/httproute-2/rule/0
           settings:
@@ -338,7 +337,6 @@ xdsIR:
         traffic:
           loadBalancer:
             random: {}
-          name: envoy-gateway/policy-for-gateway-1
           timeout:
             http:
               connectionIdleTimeout: 21s
@@ -366,8 +364,7 @@ xdsIR:
           distinct: false
           name: ""
           prefix: /baz
-        traffic:
-          name: default/policy-for-route-3
+        traffic: {}
     readyListener:
       address: 0.0.0.0
       ipFamily: IPv4