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

Custom route-level clusterClientRatelimit in Ingress #1831

Closed
mortenbo opened this issue Aug 19, 2021 · 3 comments · Fixed by #1832
Closed

Custom route-level clusterClientRatelimit in Ingress #1831

mortenbo opened this issue Aug 19, 2021 · 3 comments · Fixed by #1832

Comments

@mortenbo
Copy link

mortenbo commented Aug 19, 2021

Is there a way in the ingress file to define custom clusterClientRatelimit for specific routes? This is where I'm at atm:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: ourservice-{{deploy_env}}
  namespace: {{namespace}}
  labels:
    app: ourservice
    env: {{deploy_env}}
  annotations:
    kubernetes.io/ingress.class: skipper
    zalando.org/skipper-filter: lifo(10, 10, "10s") -> clusterClientRatelimit("ourservice-{{deploy_env}}", 10, "10s")
    zalando.org/skipper-routes: |
      a: Path("/login") -> clusterClientRatelimit("ourservice-login", 10, "10m") -> <loopback>;
      b: Path("/signup") -> clusterClientRatelimit("ourservice-login", 10, "10m") -> <loopback>;
spec:
  rules:
  - host: ourservice-{{deploy_env}}.{{cluster_name}}
    http:
      paths:
      - backend:
          serviceName: ourservice-{{deploy_env}}
          servicePort: 4000

The issue here seems to be that the rate limiter is always on for those two routes. It never relays the requests to our backend when not having made more than 10 reqs within 10 minutes.

I've tried <loopback>, <dynamic>, http://127.0.0.1:4000, and more. Either I'm just doing it plain wrong (it's been hard for me figuring this out via the docs or open-source code in GH) or ingress is not even made for this case.

I hope some can shed some light on this. Thank you!

@AlexanderYastrebov
Copy link
Member

AlexanderYastrebov commented Aug 19, 2021

Hello.

To add ratelimit to the specific path/route you would either need two ingress definitions for the same backend and host, something like

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: app-login
  annotations:
    zalando.org/skipper-predicate: Path("/login")
    zalando.org/skipper-filter: clusterClientRatelimit("login-ratelimit", 1, "1m")
spec:
  rules:
  - host: app.example.org
    http:
      paths:
      - backend:
          serviceName: app-svc
          servicePort: 80
---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: app
spec:
  rules:
  - host: app.example.org
    http:
      paths:
      - backend:
          serviceName: app-svc
          servicePort: 80

or use Routegroups to define routes explicitly which might be more user-friendly than several ingresses

it's been hard for me figuring this out via the docs

This is something to improve for us in the docs, the closest thing is https://github.com/zalando/skipper/blob/master/docs/kubernetes/ingress-usage.md#ab-test

@AlexanderYastrebov
Copy link
Member

@mortenbo

The problem with <loopback> approach is that /login requests would go into a loop and most likely you would observe max loopbacks reached error in the logs.

The general hurdle with path-specific ratelimits and a single ingress is that zalando.org/skipper-filter or zalando.org/skipper-predicate is applied to all routes generated from the ingress definition while zalando.org/skipper-routes adds extra routes but there is no way to specify service backend by name.

I think with enough creativity it could be possible to make it work with <loopback> backend by modifying request (it should somehow avoid looping into the same route) or by using cluster-local service name as a network backend like "http://app-svc.default.cluster.local:80" (which has a drawback that requests would go through service while normally skipper uses service endpoints so e.g. skipper would not be able to apply loadbalancing algorithms) but the way to go is an extra ingress definition for specific path or a routegroup.

AlexanderYastrebov added a commit that referenced this issue Aug 19, 2021
Fixes #1831

Signed-off-by: Alexander Yastrebov <[email protected]>
@mortenbo
Copy link
Author

@AlexanderYastrebov Thanks a bunch. That did the trick. Have a nice day ☀️

AlexanderYastrebov added a commit that referenced this issue Aug 24, 2021
Fixes #1831

Signed-off-by: Alexander Yastrebov <[email protected]>
AlexanderYastrebov added a commit that referenced this issue Aug 30, 2021
Fixes #1831

Signed-off-by: Alexander Yastrebov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants