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

refactor: rate limit policy enforced status controller #603

Merged
merged 2 commits into from
May 9, 2024

Conversation

KevFan
Copy link
Contributor

@KevFan KevFan commented May 2, 2024

Description

Closes: #587

Introduces a new controller for reconciling enforcement status for RLPs as with the default / overrides work, the enforecement status of multiple RLPs need to be calculated from a singular RLP event.

Superseds #591

  • Fixes RLP GW policy status is not updated when Route RLP is deleted and vice versa

Verification

Code changes are tested with the integration test changes, so passing integration tests should be sufficient.

If you want verify manually:

  • Checkout this branch
  • Deploy
make local-setup
  • Deploy kuadrant
kubectl apply  -f - <<EOF               
---                                     
apiVersion: kuadrant.io/v1beta1
kind: Kuadrant
metadata:
  name: kuadrant-sample
spec: {}
EOF
  • Deploy toystore
kubectl apply -f examples/toystore/toystore.yaml
kubectl wait --timeout=300s --for=condition=Available deployment toystore
  • Create HTTP Route
kubectl apply -f - <<EOF
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: toystore
spec:
  parentRefs:
  - name: istio-ingressgateway
    namespace: istio-system
  hostnames:
  - api.toystore.com
  rules:
  - matches:
    - method: GET
      path:
        type: PathPrefix
        value: "/toys"
    backendRefs:
    - name: toystore
      port: 80
  - matches: # it has to be a separate HTTPRouteRule so we do not rate limit other endpoints
    - method: POST
      path:
        type: Exact
        value: "/toys"
    backendRefs:
    - name: toystore
      port: 80
EOF
  • Watch RLP statues
watch -n 0.1 "kubectl get ratelimitpolicy -A -o yaml | yq '.items[].status'"
  • Create GW RLP with defaults
kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: gw-rlp
  namespace: istio-system
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: istio-ingressgateway
  defaults:
    limits:
      "gateway":
        rates:
        - limit: 5
          duration: 10
          unit: second
EOF
  • Verify GW Policy is accepted and enforced

image

  • Port forward gateway service
kubectl port-forward -n istio-system service/istio-ingressgateway-istio 9080:80 2>&1 >/dev/null &
export GATEWAY_URL=localhost:9080
  • Ensure HTTP Route is rate limit at 5 requests per 10 seconds
while :; do curl --write-out '%{http_code}\n' --silent --output /dev/null -H 'Host: api.toystore.com' http://$GATEWAY_URL/toys -X POST | grep -E --color "\b(429)\b|$"; sleep 1; done
  • Create HTTPRoute RLP with implicit default limits
kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: toystore
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  limits:
    "route":
      rates:
      - limit: 8
        duration: 10
        unit: second
EOF
  • Verify Route policy is accepted and enforced
  • Verify GW policy is accepted and not enforced - overridden by route policy

image

  • Ensure HTTP Route is rate limted at 8 requests per second (Route RLP defaults takes precedence over gateway defaults) (Note: you might need to port forward the gateway service again)
while :; do curl --write-out '%{http_code}\n' --silent --output /dev/null -H 'Host: api.toystore.com' http://$GATEWAY_URL/toys -X POST | grep -E --color "\b(429)\b|$"; sleep 1; done
  • Delete route RLP
kubectl delete ratelimitpolicy toystore
  • Verify GW policy goes back to enforced true

image

  • Ensure HTTP Route is rate limits at 5 requests per second again (GW RLP defaults)
while :; do curl --write-out '%{http_code}\n' --silent --output /dev/null -H 'Host: api.toystore.com' http://$GATEWAY_URL/toys -X POST | grep -E --color "\b(429)\b|$"; sleep 1; done

@KevFan KevFan force-pushed the rlp-enforced-refactor branch 2 times, most recently from 4bf6588 to 037a9d0 Compare May 3, 2024 10:50
@KevFan KevFan force-pushed the rlp-enforced-refactor branch from 037a9d0 to 87532cd Compare May 3, 2024 15:36
@KevFan KevFan force-pushed the rlp-enforced-refactor branch from 87532cd to ae8f4b6 Compare May 3, 2024 16:04
@KevFan KevFan changed the title [wip] refactor: rate limit policy enforced status controller refactor: rate limit policy enforced status controller May 7, 2024
@KevFan KevFan marked this pull request as ready for review May 7, 2024 11:20
@KevFan KevFan requested a review from a team as a code owner May 7, 2024 11:20
Copy link
Contributor

@guicassolato guicassolato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Fun fact – In the verification steps, I actually got the Enforced status condition added to the RLP before the Accepted one. It was of course a matter of milliseconds, but enough to spot it while watching the status of the policy.

I guess it's nothing to worry about, but it does mean RFC 0004 is no longer being strictly implemented.

cc @didierofrivia

@KevFan KevFan merged commit b1c1f4e into Kuadrant:main May 9, 2024
14 checks passed
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 this pull request may close these issues.

GW RLP status not updated when Route RLP is deleted
2 participants