From 53b621dc4cc28538c2fd14d8ff50e8d48c86c18e Mon Sep 17 00:00:00 2001 From: liuhy Date: Fri, 10 Jul 2026 00:49:52 +0800 Subject: [PATCH 1/4] feat: add optional name field to RateLimitRule for improved observability This enhancement allows users to provide meaningful names for rate limit rules, making rate limit keys more identifiable in metrics and dashboards. API Changes: - Add optional 'name' field to RateLimitRule with validation - Pattern: [a-zA-Z0-9._-]+ (alphanumeric, dots, underscores, hyphens) - Max length: 64 characters - Cannot be purely numeric (CEL validation at field level) - Must be unique within a policy (CEL validation at Rules array level) IR Translation: - When name is set: //rule/ - When name is not set: //rule/ (existing behavior) Testing: - Add test case for named rules (including shared rules) - Add CEL validation tests for valid names, duplicates, and numeric names - All existing tests continue to pass Benefits: - Improved observability: meaningful keys in metrics/dashboards - Stability: keys unchanged when rule ordering changes - Backward compatible: optional field preserves existing behavior Resolves: envoyproxy/gateway#9151 Signed-off-by: liuhy Co-Authored-By: Claude --- api/v1alpha1/ratelimit_types.go | 18 ++ api/v1alpha1/zz_generated.deepcopy.go | 5 + ....envoyproxy.io_backendtrafficpolicies.yaml | 49 ++++ ....envoyproxy.io_backendtrafficpolicies.yaml | 49 ++++ internal/gatewayapi/backendtrafficpolicy.go | 8 +- internal/gatewayapi/helpers.go | 5 +- ...icpolicy-with-ratelimit-named-rule.in.yaml | 77 +++++ ...cpolicy-with-ratelimit-named-rule.out.yaml | 272 ++++++++++++++++++ ...icpolicy-with-ratelimit-rule-names.in.yaml | 70 +++++ ...cpolicy-with-ratelimit-rule-names.out.yaml | 266 +++++++++++++++++ ...icy_ratelimitrule_merge.jsonmerge.out.yaml | 1 + ...atelimitrule_merge.strategicmerge.out.yaml | 1 + .../global-shared-named-rule.yaml | 34 +++ .../global-shared-named-rule.routes.yaml | 12 + .../global-shared-named-rule.yaml | 26 ++ ...ed-ratelimit-rules-backendtrafficpolicy.md | 1 + site/content/en/latest/api/extension_types.md | 1 + .../backendtrafficpolicy_test.go | 173 +++++++++++ 18 files changed, 1063 insertions(+), 5 deletions(-) create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.in.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.out.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.out.yaml create mode 100644 internal/xds/translator/testdata/in/ratelimit-config/global-shared-named-rule.yaml create mode 100644 internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.routes.yaml create mode 100644 internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.yaml create mode 100644 release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index d10af75594..6624d21837 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/api/v1alpha1/ratelimit_types.go @@ -55,6 +55,7 @@ type GlobalRateLimit struct { // to rate limit the request. // // +kubebuilder:validation:MaxItems=256 + // +kubebuilder:validation:XValidation:rule="self.all(r, !has(r.name) || self.filter(r2, has(r2.name) && r2.name == r.name).size() == 1)", message="rate limit rule names must be unique within the global rules slice" Rules []RateLimitRule `json:"rules"` } @@ -69,6 +70,7 @@ type LocalRateLimit struct { // +kubebuilder:validation:MaxItems=16 // +kubebuilder:validation:XValidation:rule="self.all(r, !has(r.cost) || !has(r.cost.response))", message="response cost is not supported for Local Rate Limits" // +kubebuilder:validation:XValidation:rule="self.all(r, !has(r.limit.fromMetadata))", message="limit fromMetadata is not supported for Local Rate Limits" + // +kubebuilder:validation:XValidation:rule="self.all(r, !has(r.name) || self.filter(r2, has(r2.name) && r2.name == r.name).size() == 1)", message="rate limit rule names must be unique within the local rules slice" Rules []RateLimitRule `json:"rules"` } @@ -92,6 +94,22 @@ const ( // RateLimitRule defines the semantics for matching attributes // from the incoming requests, and setting limits for them. type RateLimitRule struct { + // Name is a user-facing name for this rule that can be used for debugging + // and observability. When set, the rate limit key will include this name, + // making it easier to identify in metrics and dashboards. + // The name must be unique within a policy and should be a stable identifier + // that won't change when the rule order changes. + // + // When name is set, the rate limit key format becomes: + // //rule/ + // When name is not set, the format remains: + // //rule/ + // + // +optional + // +kubebuilder:validation:MaxLength=64 + // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9._-]+$` + // +kubebuilder:validation:XValidation:rule="!self.matches('^[0-9]+$')", message="rate limit rule name must not be purely numeric to avoid collision with auto-generated index-based rule names" + 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 diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7245a49145..25880bf06c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -7351,6 +7351,11 @@ func (in *RateLimitRedisSettings) DeepCopy() *RateLimitRedisSettings { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RateLimitRule) DeepCopyInto(out *RateLimitRule) { *out = *in + if in.Name != nil { + in, out := &in.Name, &out.Name + *out = new(string) + **out = **in + } if in.ClientSelectors != nil { in, out := &in.ClientSelectors, &out.ClientSelectors *out = make([]RateLimitSelectCondition, len(*in)) diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 575c1136c5..bc827bdd50 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -1847,6 +1847,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -1876,6 +1896,11 @@ spec: type: object maxItems: 256 type: array + x-kubernetes-validations: + - message: rate limit rule names must be unique within the + global rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) required: - rules type: object @@ -2258,6 +2283,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -2293,6 +2338,10 @@ spec: - message: limit fromMetadata is not supported for Local Rate Limits rule: self.all(r, !has(r.limit.fromMetadata)) + - message: rate limit rule names must be unique within the + local rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) type: object type: description: |- diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 7572080368..93bcf87b48 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -1846,6 +1846,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -1875,6 +1895,11 @@ spec: type: object maxItems: 256 type: array + x-kubernetes-validations: + - message: rate limit rule names must be unique within the + global rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) required: - rules type: object @@ -2257,6 +2282,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -2292,6 +2337,10 @@ spec: - message: limit fromMetadata is not supported for Local Rate Limits rule: self.all(r, !has(r.limit.fromMetadata)) + - message: rate limit rule names must be unique within the + local rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) type: object type: description: |- diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 933ba491fb..1c7861587c 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -1422,8 +1422,8 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) (* if err != nil { return nil, err } - // Set the Name field as //rule/ - irRule.Name = irRuleName(policy.Namespace, policy.Name, i) + // Set the Name field as //rule/ + irRule.Name = irRuleName(policy.Namespace, policy.Name, i, rule.Name) irRules = append(irRules, irRule) } @@ -1460,8 +1460,8 @@ func (t *Translator) buildGlobalRateLimit(policy *egv1a1.BackendTrafficPolicy) ( if err != nil { return nil, err } - // Set the Name field as //rule/ - irRules[i].Name = irRuleName(policy.Namespace, policy.Name, i) + // Set the Name field as //rule/ + irRules[i].Name = irRuleName(policy.Namespace, policy.Name, i, rule.Name) } return rateLimit, nil diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go index deef7d41b6..28ddc15b21 100644 --- a/internal/gatewayapi/helpers.go +++ b/internal/gatewayapi/helpers.go @@ -517,7 +517,10 @@ func irDestinationSettingName(destName string, backendIdx int) string { return fmt.Sprintf("%s/backend/%d", destName, backendIdx) } -func irRuleName(policyNamespace, policyName string, ruleIndex int) string { +func irRuleName(policyNamespace, policyName string, ruleIndex int, ruleName *string) string { + if ruleName != nil && *ruleName != "" { + return fmt.Sprintf("%s/%s/rule/%s", policyNamespace, policyName, *ruleName) + } return fmt.Sprintf("%s/%s/rule/%d", policyNamespace, policyName, ruleIndex) } diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.in.yaml new file mode 100644 index 0000000000..d4752e7537 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.in.yaml @@ -0,0 +1,77 @@ +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: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/" + backendRefs: + - name: service-1 + port: 8080 +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: default + name: policy-for-route + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + rateLimit: + type: Global + global: + rules: + # Named shared rule -> key uses the name: default/policy-for-route/rule/by-ip + - name: by-ip + clientSelectors: + - sourceCIDR: + type: "Distinct" + value: "0.0.0.0/0" + limit: + requests: 20 + unit: Hour + shared: true + # Named non-shared rule -> key uses the name: default/policy-for-route/rule/by-org + - name: by-org + clientSelectors: + - headers: + - name: x-org-id + type: Distinct + limit: + requests: 30 + unit: Hour + # Unnamed rule -> key falls back to the index: default/policy-for-route/rule/2 + - clientSelectors: + - headers: + - name: x-team + value: blue + limit: + requests: 40 + unit: Hour diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.out.yaml new file mode 100644 index 0000000000..8fefd4dc3c --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-named-rule.out.yaml @@ -0,0 +1,272 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: policy-for-route + namespace: default + spec: + rateLimit: + global: + rules: + - clientSelectors: + - sourceCIDR: + type: Distinct + value: 0.0.0.0/0 + limit: + requests: 20 + unit: Hour + name: by-ip + shared: true + - clientSelectors: + - headers: + - name: x-org-id + type: Distinct + limit: + requests: 30 + unit: Hour + name: by-org + - clientSelectors: + - headers: + - name: x-team + value: blue + limit: + requests: 40 + unit: Hour + type: Global + targetRef: + 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: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + reason: DeprecatedField + status: "True" + type: Warning + controllerName: gateway.envoyproxy.io/gatewayclass-controller +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: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + 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 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + envoyClientCertificate: + certificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUREVENDQWZXZ0F3SUJBZ0lVRUZNaFA5ZUo5WEFCV3NRNVptNmJSazJjTE5Rd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0ZqRVVNQklHQTFVRUF3d0xabTl2TG1KaGNpNWpiMjB3SGhjTk1qUXdNakk1TURrek1ERXdXaGNOTXpRdwpNakkyTURrek1ERXdXakFXTVJRd0VnWURWUVFEREF0bWIyOHVZbUZ5TG1OdmJUQ0NBU0l3RFFZSktvWklodmNOCkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFKbEk2WXhFOVprQ1BzNnBDUXhickNtZWl4OVA1RGZ4OVJ1NUxENFQKSm1kVzdJS2R0UVYvd2ZMbXRzdTc2QithVGRDaldlMEJUZmVPT1JCYlIzY1BBRzZFbFFMaWNsUVVydW4zcStncwpKcEsrSTdjSStqNXc4STY4WEg1V1E3clZVdGJ3SHBxYncrY1ZuQnFJVU9MaUlhdGpJZjdLWDUxTTF1RjljZkVICkU0RG5jSDZyYnI1OS9SRlpCc2toeHM1T3p3Sklmb2hreXZGd2V1VHd4Sy9WcGpJKzdPYzQ4QUJDWHBOTzlEL3EKRWgrck9hdWpBTWNYZ0hRSVRrQ2lpVVRjVW82TFNIOXZMWlB0YXFmem9acTZuaE1xcFc2NUUxcEF3RjNqeVRUeAphNUk4SmNmU0Zqa2llWjIwTFVRTW43TThVNHhIamFvL2d2SDBDQWZkQjdSTFUyc0NBd0VBQWFOVE1GRXdIUVlEClZSME9CQllFRk9SQ0U4dS8xRERXN2loWnA3Y3g5dFNtUG02T01COEdBMVVkSXdRWU1CYUFGT1JDRTh1LzFERFcKN2loWnA3Y3g5dFNtUG02T01BOEdBMVVkRXdFQi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQgpBRnQ1M3pqc3FUYUg1YThFMmNodm1XQWdDcnhSSzhiVkxNeGl3TkdqYm1FUFJ6K3c2TngrazBBOEtFY0lEc0tjClNYY2k1OHU0b1didFZKQmx6YS9adWpIUjZQMUJuT3BsK2FveTc4NGJiZDRQMzl3VExvWGZNZmJCQ20xdmV2aDkKQUpLbncyWnRxcjRta2JMY3hFcWxxM3NCTEZBUzlzUUxuS05DZTJjR0xkVHAyYm9HK3FjZ3lRZ0NJTTZmOEVNdgpXUGlmQ01NR3V6Sy9HUkY0YlBPL1lGNDhld0R1M1VlaWgwWFhkVUFPRTlDdFVhOE5JaGMxVVBhT3pQcnRZVnFyClpPR2t2L0t1K0I3OGg4U0VzTzlYclFjdXdiT25KeDZLdFIrYWV5a3ZBcFhDUTNmWkMvYllLQUFSK1A4QUpvUVoKYndJVW1YaTRnajVtK2JLUGhlK2lyK0U9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0= + name: envoy-gateway-system/envoy + privateKey: '[redacted]' + 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: gateway.envoyproxy.io + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + policies: + - kind: BackendTrafficPolicy + name: policy-for-route + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / + traffic: + rateLimit: + global: + rules: + - cidrMatch: + cidr: 0.0.0.0/0 + distinct: true + invert: false + isIPv6: false + maskLen: 0 + headerMatches: [] + limit: + requests: 20 + unit: Hour + name: default/policy-for-route/rule/by-ip + shared: true + - headerMatches: + - distinct: true + name: x-org-id + limit: + requests: 30 + unit: Hour + name: default/policy-for-route/rule/by-org + - headerMatches: + - distinct: false + exact: blue + name: x-team + limit: + requests: 40 + unit: Hour + name: default/policy-for-route/rule/2 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml new file mode 100644 index 0000000000..cd270f3d58 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml @@ -0,0 +1,70 @@ +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 + rules: + - matches: + - path: + value: "/" + backendRefs: + - name: service-1 + port: 8080 +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: default + name: policy-for-route + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + rateLimit: + type: Global + global: + rules: + - name: basic-limit + clientSelectors: + - headers: + - name: x-user-id + value: one + limit: + requests: 10 + unit: Minute + - name: premium-limit + clientSelectors: + - headers: + - name: x-user-id + value: premium + limit: + requests: 100 + unit: Minute + - clientSelectors: + - sourceCIDR: + type: "Distinct" + value: 192.168.0.0/16 + limit: + requests: 50 + unit: Hour \ No newline at end of file diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.out.yaml new file mode 100644 index 0000000000..99a99bf03e --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.out.yaml @@ -0,0 +1,266 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: policy-for-route + namespace: default + spec: + rateLimit: + global: + rules: + - clientSelectors: + - headers: + - name: x-user-id + value: one + limit: + requests: 10 + unit: Minute + name: basic-limit + - clientSelectors: + - headers: + - name: x-user-id + value: premium + limit: + requests: 100 + unit: Minute + name: premium-limit + - clientSelectors: + - sourceCIDR: + type: Distinct + value: 192.168.0.0/16 + limit: + requests: 50 + unit: Hour + type: Global + targetRef: + 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 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + reason: DeprecatedField + status: "True" + type: Warning + controllerName: gateway.envoyproxy.io/gatewayclass-controller +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 + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + 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 +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 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + envoyClientCertificate: + certificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUREVENDQWZXZ0F3SUJBZ0lVRUZNaFA5ZUo5WEFCV3NRNVptNmJSazJjTE5Rd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0ZqRVVNQklHQTFVRUF3d0xabTl2TG1KaGNpNWpiMjB3SGhjTk1qUXdNakk1TURrek1ERXdXaGNOTXpRdwpNakkyTURrek1ERXdXakFXTVJRd0VnWURWUVFEREF0bWIyOHVZbUZ5TG1OdmJUQ0NBU0l3RFFZSktvWklodmNOCkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFKbEk2WXhFOVprQ1BzNnBDUXhickNtZWl4OVA1RGZ4OVJ1NUxENFQKSm1kVzdJS2R0UVYvd2ZMbXRzdTc2QithVGRDaldlMEJUZmVPT1JCYlIzY1BBRzZFbFFMaWNsUVVydW4zcStncwpKcEsrSTdjSStqNXc4STY4WEg1V1E3clZVdGJ3SHBxYncrY1ZuQnFJVU9MaUlhdGpJZjdLWDUxTTF1RjljZkVICkU0RG5jSDZyYnI1OS9SRlpCc2toeHM1T3p3Sklmb2hreXZGd2V1VHd4Sy9WcGpJKzdPYzQ4QUJDWHBOTzlEL3EKRWgrck9hdWpBTWNYZ0hRSVRrQ2lpVVRjVW82TFNIOXZMWlB0YXFmem9acTZuaE1xcFc2NUUxcEF3RjNqeVRUeAphNUk4SmNmU0Zqa2llWjIwTFVRTW43TThVNHhIamFvL2d2SDBDQWZkQjdSTFUyc0NBd0VBQWFOVE1GRXdIUVlEClZSME9CQllFRk9SQ0U4dS8xRERXN2loWnA3Y3g5dFNtUG02T01COEdBMVVkSXdRWU1CYUFGT1JDRTh1LzFERFcKN2loWnA3Y3g5dFNtUG02T01BOEdBMVVkRXdFQi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQgpBRnQ1M3pqc3FUYUg1YThFMmNodm1XQWdDcnhSSzhiVkxNeGl3TkdqYm1FUFJ6K3c2TngrazBBOEtFY0lEc0tjClNYY2k1OHU0b1didFZKQmx6YS9adWpIUjZQMUJuT3BsK2FveTc4NGJiZDRQMzl3VExvWGZNZmJCQ20xdmV2aDkKQUpLbncyWnRxcjRta2JMY3hFcWxxM3NCTEZBUzlzUUxuS05DZTJjR0xkVHAyYm9HK3FjZ3lRZ0NJTTZmOEVNdgpXUGlmQ01NR3V6Sy9HUkY0YlBPL1lGNDhld0R1M1VlaWgwWFhkVUFPRTlDdFVhOE5JaGMxVVBhT3pQcnRZVnFyClpPR2t2L0t1K0I3OGg4U0VzTzlYclFjdXdiT25KeDZLdFIrYWV5a3ZBcFhDUTNmWkMvYllLQUFSK1A4QUpvUVoKYndJVW1YaTRnajVtK2JLUGhlK2lyK0U9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0= + name: envoy-gateway-system/envoy + privateKey: '[redacted]' + 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 + policies: + - kind: BackendTrafficPolicy + name: policy-for-route + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: / + traffic: + rateLimit: + global: + rules: + - headerMatches: + - distinct: false + exact: one + name: x-user-id + limit: + requests: 10 + unit: Minute + name: default/policy-for-route/rule/basic-limit + - headerMatches: + - distinct: false + exact: premium + name: x-user-id + limit: + requests: 100 + unit: Minute + name: default/policy-for-route/rule/premium-limit + - cidrMatch: + cidr: 192.168.0.0/16 + distinct: true + invert: false + isIPv6: false + maskLen: 16 + headerMatches: [] + limit: + requests: 50 + unit: Hour + name: default/policy-for-route/rule/2 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.jsonmerge.out.yaml b/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.jsonmerge.out.yaml index edf869191d..23dcf528b0 100644 --- a/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.jsonmerge.out.yaml +++ b/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.jsonmerge.out.yaml @@ -22,6 +22,7 @@ spec: limit: requests: 21 unit: Hour + name: two shared: false type: Global status: diff --git a/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.strategicmerge.out.yaml b/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.strategicmerge.out.yaml index edf869191d..23dcf528b0 100644 --- a/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.strategicmerge.out.yaml +++ b/internal/utils/testdata/backendtrafficpolicy_ratelimitrule_merge.strategicmerge.out.yaml @@ -22,6 +22,7 @@ spec: limit: requests: 21 unit: Hour + name: two shared: false type: Global status: diff --git a/internal/xds/translator/testdata/in/ratelimit-config/global-shared-named-rule.yaml b/internal/xds/translator/testdata/in/ratelimit-config/global-shared-named-rule.yaml new file mode 100644 index 0000000000..4ab198459f --- /dev/null +++ b/internal/xds/translator/testdata/in/ratelimit-config/global-shared-named-rule.yaml @@ -0,0 +1,34 @@ +http: +- name: "first-listener" + address: "0.0.0.0" + port: 10080 + hostnames: + - "*" + path: + mergeSlashes: true + escapedSlashesAction: UnescapeAndRedirect + routes: + - name: "first-route" + traffic: + rateLimit: + global: + rules: + # A shared rule whose name carries a meaningful suffix instead of the + # array index. stripRuleIndexSuffix must still recover the domain + # "test-namespace/test-policy-1" by cutting at "/rule/". + - name: "test-namespace/test-policy-1/rule/by-user" + headerMatches: + - name: "x-user-id" + distinct: true + limit: + requests: 5 + unit: second + shared: true + pathMatch: + exact: "foo/bar" + destination: + name: "first-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 diff --git a/internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.routes.yaml b/internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.routes.yaml new file mode 100644 index 0000000000..38a0abc8f5 --- /dev/null +++ b/internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.routes.yaml @@ -0,0 +1,12 @@ +rateLimitsByDomain: + test-namespace/test-policy-1: + - actions: + - ActionSpecifier: + GenericKey: + descriptor_key: test-namespace/test-policy-1/rule/by-user + descriptor_value: test-namespace/test-policy-1/rule/by-user + - ActionSpecifier: + RequestHeaders: + descriptor_key: rule-0-match-0 + header_name: x-user-id +routeName: first-route diff --git a/internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.yaml b/internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.yaml new file mode 100644 index 0000000000..290db85bd6 --- /dev/null +++ b/internal/xds/translator/testdata/out/ratelimit-config/global-shared-named-rule.yaml @@ -0,0 +1,26 @@ +name: test-namespace/test-policy-1 +domain: test-namespace/test-policy-1 +descriptors: + - key: test-namespace/test-policy-1/rule/by-user + value: test-namespace/test-policy-1/rule/by-user + rate_limit: null + descriptors: + - key: rule-0-match-0 + value: "" + rate_limit: + requests_per_unit: 5 + unit: SECOND + unlimited: false + name: "" + replaces: [] + descriptors: [] + shadow_mode: false + quota_mode: false + detailed_metric: false + value_to_metric: false + share_threshold: false + shadow_mode: false + quota_mode: false + detailed_metric: false + value_to_metric: false + share_threshold: false diff --git a/release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md b/release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md new file mode 100644 index 0000000000..3e6fa9c993 --- /dev/null +++ b/release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md @@ -0,0 +1 @@ +Added an optional `name` field to `RateLimitRule` in BackendTrafficPolicy. When set, the global rate-limit metric key uses the provided name (e.g. `//rule/`) instead of the array index (e.g. `.../rule/0`), making rate-limit metrics easier to identify and stable across rule reordering. Unset `name` preserves the existing index-based behavior. diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 66d6c6cd5a..94cbf32eca 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -5264,6 +5264,7 @@ _Appears in:_ | Field | Type | Required | Default | Description | | --- | --- | --- | --- | --- | +| `name` | _string_ | false | | Name is a user-facing name for this rule that can be used for debugging
and observability. When set, the rate limit key will include this name,
making it easier to identify in metrics and dashboards.
The name must be unique within a policy and should be a stable identifier
that won't change when the rule order changes.
When name is set, the rate limit key format becomes:
//rule/
When name is not set, the format remains:
//rule/ | | `clientSelectors` | _[RateLimitSelectCondition](#ratelimitselectcondition) array_ | false | | 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
and its limit to be applied.
If no client selectors are specified, the rule applies to all traffic of
the targeted Route.
If the policy targets a Gateway, the rule applies to each Route of the Gateway.
Please note that each Route has its own rate limit counters. For example,
if a Gateway has two Routes, and the policy has a rule with limit 10rps,
each Route will have its own 10rps limit. | | `limit` | _[RateLimitValue](#ratelimitvalue)_ | true | | Limit holds the rate limit values.
This limit is applied for traffic flows when the selectors
compute to True, causing the request to be counted towards the limit.
The limit is enforced and the request is ratelimited, i.e. a response with
429 HTTP status code is sent back to the client when
the selected requests have reached the limit. | | `cost` | _[RateLimitCost](#ratelimitcost)_ | false | | Cost specifies the cost of requests and responses for the rule.
This is optional and if not specified, the default behavior is to reduce the rate limit counters by 1 on
the request path and do not reduce the rate limit counters on the response path. | diff --git a/test/cel-validation/backendtrafficpolicy_test.go b/test/cel-validation/backendtrafficpolicy_test.go index 6bd00eb1c0..58a2f6effb 100644 --- a/test/cel-validation/backendtrafficpolicy_test.go +++ b/test/cel-validation/backendtrafficpolicy_test.go @@ -17,6 +17,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ptr "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -3726,6 +3727,178 @@ func TestBackendTrafficPolicyTarget(t *testing.T) { }, wantErrors: []string{}, }, + { + desc: "valid named global rate limit rule is accepted", + mutate: func(btp *egv1a1.BackendTrafficPolicy) { + btp.Spec = egv1a1.BackendTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + }, + }, + }, + RateLimit: &egv1a1.RateLimitSpec{ + Global: &egv1a1.GlobalRateLimit{ + Rules: []egv1a1.RateLimitRule{ + { + Name: ptr.To("by-ip"), + Limit: egv1a1.RateLimitValue{ + Requests: 10, + Unit: "Minute", + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{}, + }, + { + desc: "duplicate global rate limit rule names are rejected", + mutate: func(btp *egv1a1.BackendTrafficPolicy) { + btp.Spec = egv1a1.BackendTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + }, + }, + }, + RateLimit: &egv1a1.RateLimitSpec{ + Global: &egv1a1.GlobalRateLimit{ + Rules: []egv1a1.RateLimitRule{ + { + Name: ptr.To("dup"), + Limit: egv1a1.RateLimitValue{ + Requests: 10, + Unit: "Minute", + }, + }, + { + Name: ptr.To("dup"), + Limit: egv1a1.RateLimitValue{ + Requests: 20, + Unit: "Minute", + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{ + "rate limit rule names must be unique within the global rules slice", + }, + }, + { + desc: "duplicate local rate limit rule names are rejected", + mutate: func(btp *egv1a1.BackendTrafficPolicy) { + btp.Spec = egv1a1.BackendTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + }, + }, + }, + RateLimit: &egv1a1.RateLimitSpec{ + Local: &egv1a1.LocalRateLimit{ + Rules: []egv1a1.RateLimitRule{ + { + Name: ptr.To("dup"), + Limit: egv1a1.RateLimitValue{ + Requests: 10, + Unit: "Minute", + }, + }, + { + Name: ptr.To("dup"), + Limit: egv1a1.RateLimitValue{ + Requests: 20, + Unit: "Minute", + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{ + "rate limit rule names must be unique within the local rules slice", + }, + }, + { + desc: "purely numeric global rate limit rule name is rejected", + mutate: func(btp *egv1a1.BackendTrafficPolicy) { + btp.Spec = egv1a1.BackendTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + }, + }, + }, + RateLimit: &egv1a1.RateLimitSpec{ + Global: &egv1a1.GlobalRateLimit{ + Rules: []egv1a1.RateLimitRule{ + { + Name: ptr.To("0"), + Limit: egv1a1.RateLimitValue{ + Requests: 10, + Unit: "Minute", + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{ + "rate limit rule name must not be purely numeric", + }, + }, + { + desc: "global rate limit rule name containing slash is rejected", + mutate: func(btp *egv1a1.BackendTrafficPolicy) { + btp.Spec = egv1a1.BackendTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + }, + }, + }, + RateLimit: &egv1a1.RateLimitSpec{ + Global: &egv1a1.GlobalRateLimit{ + Rules: []egv1a1.RateLimitRule{ + { + Name: ptr.To("a/b"), + Limit: egv1a1.RateLimitValue{ + Requests: 10, + Unit: "Minute", + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{ + `spec.rateLimit.global.rules[0].name: Invalid value:`, + }, + }, } for _, tc := range cases { From c7ba8d136ccaa2a8c3ce4d4be66350a4137b6015 Mon Sep 17 00:00:00 2001 From: liuhy Date: Fri, 10 Jul 2026 20:38:23 +0800 Subject: [PATCH 2/4] release-notes: correct scope of named RateLimitRule to local and global The release note for #9151 previously mentioned only the global rate-limit metric key, but the change to irRuleName affects both the local and global rate-limit rule keys. Update the note to reflect the actual scope. Co-Authored-By: Claude Signed-off-by: liuhy --- .../9151-named-ratelimit-rules-backendtrafficpolicy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md b/release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md index 3e6fa9c993..2001fd0ade 100644 --- a/release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md +++ b/release-notes/current/new_features/9151-named-ratelimit-rules-backendtrafficpolicy.md @@ -1 +1 @@ -Added an optional `name` field to `RateLimitRule` in BackendTrafficPolicy. When set, the global rate-limit metric key uses the provided name (e.g. `//rule/`) instead of the array index (e.g. `.../rule/0`), making rate-limit metrics easier to identify and stable across rule reordering. Unset `name` preserves the existing index-based behavior. +Added an optional `name` field to `RateLimitRule` in BackendTrafficPolicy. When set, both local and global rate-limit rule keys use the provided name (e.g. `//rule/`) instead of the array index (e.g. `.../rule/0`), making rate-limit rule keys and metrics easier to identify and stable across rule reordering. Unset `name` preserves the existing index-based behavior. From 7f71d3dd22ed5cec0bb7e5b1ce5dba1199c29608 Mon Sep 17 00:00:00 2001 From: liuhy Date: Fri, 10 Jul 2026 20:42:53 +0800 Subject: [PATCH 3/4] fix: resolve CI lint and gen-check failures for RateLimitRule name The CEL validation test used ptr.To from k8s.io/utils/ptr, which is forbidden by forbidigo (use new() builtin or a typed helper instead). Replace the 7 ptr.To(...) call sites with a small file-local ptrString helper that matches the existing SectionNamePtr/ObjectNamePtr convention, and drop the now-unused ptr import. The new ratelimit-rule-names testdata YAML was missing a trailing newline, which fails yamllint's new-line-at-end-of-file rule (part of make lint / gen-check). Add the trailing newline. Co-Authored-By: Claude Signed-off-by: liuhy --- ...icpolicy-with-ratelimit-rule-names.in.yaml | 2 +- .../backendtrafficpolicy_test.go | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml index cd270f3d58..8711096bcb 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-rule-names.in.yaml @@ -67,4 +67,4 @@ backendTrafficPolicies: value: 192.168.0.0/16 limit: requests: 50 - unit: Hour \ No newline at end of file + unit: Hour diff --git a/test/cel-validation/backendtrafficpolicy_test.go b/test/cel-validation/backendtrafficpolicy_test.go index 58a2f6effb..7fa6af4c0a 100644 --- a/test/cel-validation/backendtrafficpolicy_test.go +++ b/test/cel-validation/backendtrafficpolicy_test.go @@ -17,13 +17,19 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ptr "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" ) +// ptrString returns a pointer to the given string. Used in place of the +// forbidden k8s.io/utils/ptr.To for *string literal values in test cases. +func ptrString(s string) *string { + v := s + return &v +} + func TestBackendTrafficPolicyTarget(t *testing.T) { ctx := context.Background() baseBTP := egv1a1.BackendTrafficPolicy{ @@ -3744,7 +3750,7 @@ func TestBackendTrafficPolicyTarget(t *testing.T) { Global: &egv1a1.GlobalRateLimit{ Rules: []egv1a1.RateLimitRule{ { - Name: ptr.To("by-ip"), + Name: ptrString("by-ip"), Limit: egv1a1.RateLimitValue{ Requests: 10, Unit: "Minute", @@ -3774,14 +3780,14 @@ func TestBackendTrafficPolicyTarget(t *testing.T) { Global: &egv1a1.GlobalRateLimit{ Rules: []egv1a1.RateLimitRule{ { - Name: ptr.To("dup"), + Name: ptrString("dup"), Limit: egv1a1.RateLimitValue{ Requests: 10, Unit: "Minute", }, }, { - Name: ptr.To("dup"), + Name: ptrString("dup"), Limit: egv1a1.RateLimitValue{ Requests: 20, Unit: "Minute", @@ -3813,14 +3819,14 @@ func TestBackendTrafficPolicyTarget(t *testing.T) { Local: &egv1a1.LocalRateLimit{ Rules: []egv1a1.RateLimitRule{ { - Name: ptr.To("dup"), + Name: ptrString("dup"), Limit: egv1a1.RateLimitValue{ Requests: 10, Unit: "Minute", }, }, { - Name: ptr.To("dup"), + Name: ptrString("dup"), Limit: egv1a1.RateLimitValue{ Requests: 20, Unit: "Minute", @@ -3852,7 +3858,7 @@ func TestBackendTrafficPolicyTarget(t *testing.T) { Global: &egv1a1.GlobalRateLimit{ Rules: []egv1a1.RateLimitRule{ { - Name: ptr.To("0"), + Name: ptrString("0"), Limit: egv1a1.RateLimitValue{ Requests: 10, Unit: "Minute", @@ -3884,7 +3890,7 @@ func TestBackendTrafficPolicyTarget(t *testing.T) { Global: &egv1a1.GlobalRateLimit{ Rules: []egv1a1.RateLimitRule{ { - Name: ptr.To("a/b"), + Name: ptrString("a/b"), Limit: egv1a1.RateLimitValue{ Requests: 10, Unit: "Minute", From 5d5c7d36ae5a2f15f72faafec15367da17bd6566 Mon Sep 17 00:00:00 2001 From: liuhy Date: Fri, 10 Jul 2026 21:55:29 +0800 Subject: [PATCH 4/4] test: sync helm CRD snapshots for RateLimitRule name field Regenerate the gateway-crds-helm golden snapshots so they reflect the new optional RateLimitRule.name schema and its CEL validations (non-numeric name, unique names within global/local rule slices) added in api/v1alpha1/ratelimit_types.go. The charts CRD sources were already regenerated; only the test/helm/gateway-crds-helm/*.out.yaml snapshots were stale, causing gen-check (helm-template) to fail. Signed-off-by: liuhy Co-Authored-By: Claude --- test/helm/gateway-crds-helm/all.out.yaml | 49 +++++++++++++++++++ test/helm/gateway-crds-helm/e2e.out.yaml | 49 +++++++++++++++++++ .../envoy-gateway-crds.out.yaml | 49 +++++++++++++++++++ 3 files changed, 147 insertions(+) diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index 31f9836eeb..7ddec4c478 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -26464,6 +26464,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -26493,6 +26513,11 @@ spec: type: object maxItems: 256 type: array + x-kubernetes-validations: + - message: rate limit rule names must be unique within the + global rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) required: - rules type: object @@ -26875,6 +26900,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -26910,6 +26955,10 @@ spec: - message: limit fromMetadata is not supported for Local Rate Limits rule: self.all(r, !has(r.limit.fromMetadata)) + - message: rate limit rule names must be unique within the + local rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) type: object type: description: |- diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index 02b6065f79..d8cf99f642 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -2402,6 +2402,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -2431,6 +2451,11 @@ spec: type: object maxItems: 256 type: array + x-kubernetes-validations: + - message: rate limit rule names must be unique within the + global rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) required: - rules type: object @@ -2813,6 +2838,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -2848,6 +2893,10 @@ spec: - message: limit fromMetadata is not supported for Local Rate Limits rule: self.all(r, !has(r.limit.fromMetadata)) + - message: rate limit rule names must be unique within the + local rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) type: object type: description: |- 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 0b1d7764f0..5c4597ffa5 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -2402,6 +2402,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -2431,6 +2451,11 @@ spec: type: object maxItems: 256 type: array + x-kubernetes-validations: + - message: rate limit rule names must be unique within the + global rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) required: - rules type: object @@ -2813,6 +2838,26 @@ spec: - requests - unit type: object + name: + description: |- + Name is a user-facing name for this rule that can be used for debugging + and observability. When set, the rate limit key will include this name, + making it easier to identify in metrics and dashboards. + The name must be unique within a policy and should be a stable identifier + that won't change when the rule order changes. + + When name is set, the rate limit key format becomes: + //rule/ + When name is not set, the format remains: + //rule/ + maxLength: 64 + pattern: ^[a-zA-Z0-9._-]+$ + type: string + x-kubernetes-validations: + - message: rate limit rule name must not be purely numeric + to avoid collision with auto-generated index-based + rule names + rule: '!self.matches(''^[0-9]+$'')' shadowMode: description: |- ShadowMode indicates whether this rate-limit rule runs in shadow mode. @@ -2848,6 +2893,10 @@ spec: - message: limit fromMetadata is not supported for Local Rate Limits rule: self.all(r, !has(r.limit.fromMetadata)) + - message: rate limit rule names must be unique within the + local rules slice + rule: self.all(r, !has(r.name) || self.filter(r2, has(r2.name) + && r2.name == r.name).size() == 1) type: object type: description: |-