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

@@ -413,12 +413,19 @@
 	gatewayNN types.NamespacedName, gwPolicy *egv1a1.BackendTrafficPolicy,
 	route RouteContext, xdsIR resource.XdsIRMap, resources *resource.Resources,
 ) error {
-	mergedPolicy, err := mergeBackendTrafficPolicy(policy, gwPolicy)
+	tfGW, errGW := t.buildTrafficFeatures(gwPolicy, resources)
+	if errGW != nil {
+		return errGW
+	}
+	tfRoute, errRoute := t.buildTrafficFeatures(policy, resources)
+	if errRoute != nil {
+		return errRoute
+	}
+	mergedTF, err := utils.MergeTF(tfGW, tfRoute, *policy.Spec.MergeType)
 	if err != nil {
 		return fmt.Errorf("error merging policies: %w", err)
 	}
-	tf, errs := t.buildTrafficFeatures(mergedPolicy, resources)
-	if tf == nil {
+	if mergedTF == nil {
 		// should not happen
 		return nil
 	}
@@ -429,7 +436,7 @@
 		// should not happen.
 		return nil
 	}
-	applyTrafficFeatureToRoute(route, tf, errs, mergedPolicy, x)
+	applyTrafficFeatureToRoute(route, mergedTF, nil, policy, x)
 
 	return nil
 }
@@ -477,7 +484,6 @@
 				}
 
 				r.Traffic = tf.DeepCopy()
-				r.Traffic.Name = irTrafficName(policy)
 
 				if localTo, err := buildClusterSettingsTimeout(policy.Spec.ClusterSettings); err == nil {
 					r.Traffic.Timeout = localTo
@@ -494,14 +500,6 @@
 	}
 }
 
-func mergeBackendTrafficPolicy(routePolicy, gwPolicy *egv1a1.BackendTrafficPolicy) (*egv1a1.BackendTrafficPolicy, error) {
-	if routePolicy.Spec.MergeType == nil || gwPolicy == nil {
-		return routePolicy.DeepCopy(), nil
-	}
-
-	return utils.Merge[*egv1a1.BackendTrafficPolicy](gwPolicy, routePolicy, *routePolicy.Spec.MergeType)
-}
-
 func (t *Translator) buildTrafficFeatures(policy *egv1a1.BackendTrafficPolicy, resources *resource.Resources) (*ir.TrafficFeatures, error) {
 	var (
 		rl          *ir.RateLimit
@@ -683,7 +681,6 @@
 			}
 
 			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)
@@ -792,8 +789,7 @@
 	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)),
 		},
 	}
 
@@ -804,6 +800,8 @@
 		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
@@ -816,6 +814,7 @@
 			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