Extend wasm configuration for conditional multiple descriptors for rate limiting

[eguzki]

Limitations

• Currently, for rate limiting, the action data can only generate one descriptor.

actions:
      - extension: ratelimit-ext
        scope: rlp-ns-A/rlp-name-A
        data:
          - selector:
              selector: request.headers.My-Custom-Header
          - static:
              key: admin
              value: "1"

Additionally, only top level conditions used to conditionally include descriptor entries. And it is all or nothing.

• Only one rule can be activated per request. This is not a limitation per se. It is a limitation taking into account how kuadrant's control plane generates wasm configuration. For example, the following RLP targeting a simple HTTPRoute (routing traffic for GET /get:

  limits:
    limitA:
      rates:
        - duration: 10
          limit: 8
          unit: second
    limitB:
      rates:
        - duration: 5
          limit: 3
          unit: second

generates the following wasm configuration:

    rules:
    - conditions: # RULE for limit A
      - allOf:
          - operator: startswith
             selector: request.url_path
             value: /get
      actions:   
      - extension: ratelimit-ext
        scope: rlp-ns/rlp-name
        data:
        - static:
             key: limit.limitA__6fb66d73
             value: '1'
    - conditions: # RULE for limit B
      - allOf:
          - operator: startswith
             selector: request.url_path
             value: /get
      actions:   
      - extension: ratelimit-ext
        scope: rlp-ns/rlp-name
        data:
        - static:
             key: limit.limitB__6fb66d73
             value: '1'

This configuration will always activate the first rule making the second one shadowed (disabled effectively). However, the expected behavior of the user is to have both counters being increased. That requires that the descritor(s) sent would include key : "limit.limitA__6fb66d73", value: "1" and key : "limit.limitB__2726263", value: "1". However, the current implementation does not send both descriptor entries, only the one of the first rule. In practice, this leads to some limit to be enforced and not the other.

This is not a limitation per se, because the wasm config could be like the following and the desired behavior (activating multiple counters) would be achieved.

    rules:
          - actions:
              - data:
                  - static:
                      key: limit.limitA__6fb66d73
                      value: '1'
                  - static:
                      key: limit.limitB__2726263
                      value: '1'
                extension: limitador
                scope: kuadrant/rlp-name-a
            conditions:
              - allOf:
                  - operator: startswith
                    selector: request.url_path
                    value: /get

In short, one wasm rule, with one action and multiple static descriptor entries, one per policy limit.

Proposal

extensions:
  auth-ext:
    type: auth
    endpoint: auth-cluster
    failureMode: deny
  ratelimit-ext:
    type: ratelimit
    endpoint: ratelimit-cluster
    failureMode: deny
policies:
  - name: rlp-ns-A/rlp-name-A
    hostnames: [ "*.toystore.com" ]
    rules:
    - conditions: # TOP LEVEL CONDITIONS (from HTTPRouteRules)
      - allOf:
        - selector: request.url_path
          operator: startswith
          value: /get
        - selector: request.method
          operator: eq
          value: GET
      actions:
      - extension: ratelimit-ext
        scope: rlp-ns-A/rlp-name-A
        typed_config: 
           "@type": type.kuadrant.io/extensions.ratelimit.v1.Wasm
           config:
            actionRules: # two action rules, one per RLP limit A, other one for RLP limit B
            - conditions: # `when` rate limit conditions (from policy)
              - allOf:{}
              - allOf:{}
              descriptor:
              - static:
                  key: limit_a
                  value: "1"
             - conditions: # `when` rate limit conditions (from policy)
               - allOf:
                 - selector: request.headers.My-Custom-Header
                   operator: startswith
                   value: kuadrant
               - allOf:{}
              descriptor:
              - selector:
                   selector: request.headers.My-Custom-Header
               - static:
                   key: limit_b
                   value: "1"

[eguzki]

@alexsnaps @didierofrivia @adam-cattermole your feedback much appreciated

[guicassolato]

What the behaviour for limits that target different HTTPRouteRules? E.g.:

limitA: # ===> GET /get
  rates:
   - duration: 10
     limit: 8
     unit: second

limitB: # ===> GET /get Header[x-foo:bar] (different HTTPRouteRule) 
  rates:
   - duration: 3
     limit: 5
     unit: second

[eguzki]

What the behaviour for limits that target different HTTPRouteRules? E.g.:

When the request has x-foo:bar header, both limits apply, but only one will be activated (and only the associated counters incremented, not the others). Which one? Depend on the serialization, on the wasm side will be a list of rules. The first rule that matches will be triggered.

[guicassolato]

The first rule that matches will be triggered.

I was afraid of that. To be consistent with the behaviour of the gateway, we want to make sure the most specific set of conditions wins.

The fact that it is assumed to be always the first one to match implies that whatever is generating the config (i.e. Kuadrant Operator) needs to know exactly what the gateway will do for a given request, to thus order the rules accordingly.

Instead, I was hoping the wasm module would take care of that. My reasoning is that the wasm module literally runs as part of the same process as the gateway. Perhaps it counts on better resources to figure what route is being honoured and avoid somewhat dangerous assumptions?

[eguzki]

Example of use case to help reasoning about this issue.

One simple HTTPRoute named toystore to route traffic about GET /toys. Only relevant section shown

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute                         
metadata:
  name: toystore
spec:
  hostnames:
  - api.toystore.com
  parentRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: kuadrant-ingressgateway
    namespace: gateway-system
  rules:
  - backendRefs:
    - group: ""
      kind: Service
      name: toystore
      port: 80
      weight: 1
    matches:
    - method: GET
      path:
        type: PathPrefix
        value: /toys

Then, we define the following RateLimitPolicy

apiVersion: kuadrant.io/v1beta3
kind: RateLimitPolicy
metadata:
  name: toystore
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  limits:
    "hello-limit":
      rates:
      - limit: 5
        duration: 10
        unit: second
      when:
      - selector: request.headers.X-KUADRANT
        operator: startswith
        value: hello
    "world-limit":
      rates:
      - limit: 2
        duration: 10
        unit: second
      when:
      - selector: request.headers.X-KUADRANT
        operator: endswith
        value: world

It's quite simple:

• All requests with a header X-KUADRANT starting with hello, rate limit to max 5 req / 10 sec.
• All requests with a header X-KUADRANT ending with world, rate limit to max 2 req / 10 sec.

The traffic profile

Three HTTP request types

• Request 1 -> hits hello-limit

GET /toys HTTP/1.1
Host: 172.18.0.16
X-KUADRANT: hello

• Request 2 -> hits world-limit

GET /toys HTTP/1.1
Host: 172.18.0.16
X-KUADRANT: world

• Request 3 -> hits hello-limit AND world-limit

GET /toys HTTP/1.1
Host: 172.18.0.16
X-KUADRANT: helloworld

Today, generated Wasm configuration

Only relevant section shown

     policies:
      - hostnames:
        - api.toystore.com
        name: default/toystore
        rules:
          # RULE 1
        - actions:
          - data:
            - static:
                key: limit.hello_limit__423cedbf
                value: "1"
            extension: limitador
            scope: default/toystore
          conditions:
          - allOf:
            - operator: startswith
              selector: request.url_path
              value: /toys
            - operator: eq
              selector: request.method
              value: GET
            - operator: startswith
              selector: request.headers.X-KUADRANT
              value: hello
          # RULE 2
        - actions:
          - data:
            - static:
                key: limit.world_limit__e503df3e
                value: "1"
            extension: limitador
            scope: default/toystore
          conditions:
          - allOf:
            - operator: startswith
              selector: request.url_path
              value: /toys
            - operator: eq
              selector: request.method
              value: GET
            - operator: endswith
              selector: request.headers.X-KUADRANT
              value: world

Two policy rules are generated. Only one will be activated on each request. The first one for which top level conditions match. This is incorrect configuration and explanation is on the issue description.

Valid Wasm configuration relying on current wasm configuration scheme

        rules:
          # RULE 1 -> only hello-limit
        - actions:
          - data:
            - static:
                key: limit.hello_limit__423cedbf
                value: "1"
            extension: limitador
            scope: default/toystore
          conditions:
          - allOf:
            - operator: startswith
              selector: request.url_path
              value: /toys
            - operator: eq
              selector: request.method
              value: GET
            - operator: startswith
              selector: request.headers.X-KUADRANT
              value: hello
            - operator: DOESNOTendwith
              selector: request.headers.X-KUADRANT
              value: world
          # RULE 2 -> only world-limit
        - actions:
          - data:
            - static:
                key: limit.world_limit__423cedbf
                value: "1"
            extension: limitador
            scope: default/toystore
          conditions:
          - allOf:
            - operator: startswith
              selector: request.url_path
              value: /toys
            - operator: eq
              selector: request.method
              value: GET
            - operator: endswith
              selector: request.headers.X-KUADRANT
              value: world
            - operator: DOESNOTstartwith
              selector: request.headers.X-KUADRANT
              value: hello
          # RULE 3 -> hello-limit and world-limit
        - actions:
          - data:
            - static:
                key: limit.hello_limit__423cedbf
                value: "1"
            - static:
                key: limit.world_limit__423cedbf
                value: "1"
            extension: limitador
            scope: default/toystore
          conditions:
          - allOf:
            - operator: startswith
              selector: request.url_path
              value: /toys
            - operator: eq
              selector: request.method
              value: GET
            - operator: startswith
              selector: request.headers.X-KUADRANT
              value: hello
            - operator: endswith
              selector: request.headers.X-KUADRANT
              value: world

Three rules.

• One rule to activate exclusively hello-limit
• One rule to activate exclusively world-limit
• One rule to activate both hello-limit and world-limit

Valid Wasm configuration relying on the proposed wasm configuration scheme

        rules:
          # RULE 1  -> only one rule
        - conditions:
          - allOf:
            - operator: startswith
              selector: request.url_path
              value: /toys
            - operator: eq
              selector: request.method
              value: GET
          actions:
          - extension: ratelimit-ext
            scope: rlp-ns-A/rlp-name-A
            typed_config:
              "@type": type.kuadrant.io/extensions.ratelimit.v1.Wasm
              config:
                actionRules:
                  - conditions:
                    - operator: startswith
                      selector: request.headers.X-KUADRANT
                      value: hello
                    descriptor:
                    - static:
                        key: limit.hello_limit__423cedbf
                        value: "1"
                  - conditions:
                    - operator: endswith
                      selector: request.headers.X-KUADRANT
                      value: world
                    descriptor:
                    - static:
                        key: limit.world_limit__423cedbf
                        value: "1"

One single rule. Three actionRules. Each actionRule appending one descriptor when conditions match.

[eguzki]

closing in favor of #108