Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: RateLimit: fail when fill_interval is set above 60s #1626

Open
Ressetkk opened this issue Jan 20, 2025 · 0 comments
Open

bug: RateLimit: fail when fill_interval is set above 60s #1626

Ressetkk opened this issue Jan 20, 2025 · 0 comments
Assignees
Labels
area/api-gateway Issues or PRs related to api-gateway kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@Ressetkk
Copy link
Contributor

/kind bug
/area api-gateway

Description

When RateLimit is deployed, Istiod returns an error if fill_interval contains values below or above seconds.

2025-01-20T10:50:32.427568Z     warn    delta   ADS:LDS: ACK ERROR httpbin.test-1 Internal:Error adding/updating listener(s) virtualInbound: Unable to parse JSON as proto (INVALID_ARGUMENT: invalid JSON in   envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit @ descriptors[0].token_bucket.fill_interval: message google.protobuf.Duration, near   1:514 (offset 513): duration must end   with  a single 's'): {...}

Expected result
RateLimit configuration gets applied correctly.

Steps to reproduce

RateLimit CR

apiVersion: gateway.kyma-project.io/v1alpha1
kind: RateLimit
metadata:
  labels:
    app: httpbin
  name: ratelimit-path-sample
  namespace: test
spec:
  selectorLabels:
    app: httpbin
  enableResponseHeaders: true
  local:
    defaultBucket:
      maxTokens: 100
      tokensPerFill: 50
      fillInterval: 30s
    buckets:
      - path: /ip
        bucket:
          maxTokens: 1
          tokensPerFill: 1
          fillInterval: 60m

Troubleshooting
I believe we can't rely on go's default duration parse. All durations need to be converted to seconds. When EnvoyFilter contains ms, I got an error:

2025-01-20T13:13:42.256815Z     warn    delta   ADS:LDS: ACK ERROR httpbin.test-17 Internal:Error adding/updating listener(s) virtualInbound: Proto constraint validation failed (LocalRateLimitValidationError.Descriptors[0]: embedded message failed validation | caused by LocalRateLimitDescriptorValidationError.TokenBucket: embedded message failed validation | caused by TokenBucketValidationError.FillInterval: value must be greater than 0s): stat_prefix: "rate_limit"

That means we can only use seconds.

@Ressetkk Ressetkk added the kind/bug Categorizes issue or PR as related to a bug. label Jan 20, 2025
@kyma-bot kyma-bot added the area/api-gateway Issues or PRs related to api-gateway label Jan 20, 2025
@Ressetkk Ressetkk added this to the 2.11.0 milestone Jan 20, 2025
@Ressetkk Ressetkk self-assigned this Jan 20, 2025
@Ressetkk Ressetkk removed their assignment Jan 22, 2025
@videlov videlov assigned Ressetkk and unassigned videlov Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api-gateway Issues or PRs related to api-gateway kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants