RLP v2 controller #199

eguzki · 2023-06-08T09:49:13Z

What

First implementation of the RLP v2 controller

Many issues to address and TODO in the code. But this is a working controller with limited functionality (yet)

No doc, no tests

Verification steps

Setup env

make local-setup

Apply Kuadrant CR

kubectl -n kuadrant-system apply -f - <<EOF
---
apiVersion: kuadrant.io/v1beta1
kind: Kuadrant
metadata:
  name: kuadrant-sample
spec: {}
EOF

Deploy toystore example deployment

kubectl apply -f examples/toystore/toystore.yaml

Deploy HTTPRoute

kubectl apply -f - <<EOF
---
apiVersion: gateway.networking.k8s.io/v1alpha2
kind: HTTPRoute
metadata:
  name: toystore
spec:
  parentRefs:
  - name: istio-ingressgateway
    namespace: istio-system
  hostnames:
  - "*.toystore.acme.com"
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: "/toys"
      method: GET
    - path:
        type: PathPrefix
        value: "/toys"
      method: POST
    backendRefs:
    - name: toystore
      port: 80
  - matches:
    - path:
        type: PathPrefix
        value: "/assets/"
    backendRefs:
    - name: toystore
      port: 80
EOF

Apply RLP v2

kubectl apply -f - <<EOF
---
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: toystore-per-endpoint
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  limits:
    toys:
      rates:
      - limit: 50
        duration: 1
        unit: minute
      counters:
      - auth.identity.username
      routeSelectors:
      - matches: # matches the 1st HTTPRouteRule (i.e. GET or POST to /toys*)
        - path:
            type: PathPrefix
            value: "/toys"
      when:
      - selector: auth.identity.group
        operator: neq
        value: admin
    assets:
      rates:
      - limit: 5
        duration: 1
        unit: minute
      - limit: 100
        duration: 12
        unit: hour
      routeSelectors:
      - matches: # matches the 2nd HTTPRouteRule (i.e. /assets/*)
        - path:
            type: PathPrefix
            value: "/assets/"
EOF

Check Limitador's configuration

❯ k get limitador limitador -n kuadrant-system -o yaml
apiVersion: limitador.kuadrant.io/v1alpha1
kind: Limitador
metadata:
  creationTimestamp: "2023-06-13T15:23:22Z"
  generation: 2
  name: limitador
  namespace: kuadrant-system
  ownerReferences:
  - apiVersion: kuadrant.io/v1beta1
    blockOwnerDeletion: true
    controller: true
    kind: Kuadrant
    name: kuadrant-sample
    uid: 115edcb9-620b-4394-b922-a44ae57f047f
  resourceVersion: "2425"
  uid: b3bf6ccd-faf9-4a77-a502-52bd9d609e3d
spec:
  limits:
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 5
    namespace: istio-system/istio-ingressgateway#*.toystore.acme.com
    seconds: 60
    variables: []
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 100
    namespace: istio-system/istio-ingressgateway#*.toystore.acme.com
    seconds: 43200
    variables: []
  - conditions:
    - default/toystore-per-endpoint/toys == "1"
    max_value: 50
    namespace: istio-system/istio-ingressgateway#*.toystore.acme.com
    seconds: 60
    variables:
    - auth.identity.username
status:
  conditions:
  - lastTransitionTime: "2023-06-13T15:23:32Z"
    message: Limitador is ready
    reason: Ready
    status: "True"
    type: Ready
  observedGeneration: 2
  service:
    host: limitador-limitador.kuadrant-system.svc.cluster.local
    ports:
      grpc: 8081
      http: 8080

Check WASM plugin config

❯ k get wasmplugin kuadrant-istio-ingressgateway -n istio-system  -o yaml
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  creationTimestamp: "2023-06-16T14:47:38Z"
  generation: 1
  name: kuadrant-istio-ingressgateway
  namespace: istio-system
  resourceVersion: "2631"
  uid: a13a3a91-2b15-4cdb-9936-965c01501a55
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/istio-ingressgateway#*.toystore.acme.com
      hostnames:
      - '*.toystore.acme.com'
      name: '*.toystore.acme.com'
      rules:
      - conditions:
        - allOf:
          - operator: startswith
            selector: request.url_path
            value: /assets/
        data:
        - static:
            key: default/toystore-per-endpoint/assets
            value: "1"
      - conditions:
        - allOf:
          - operator: startswith
            selector: request.url_path
            value: /toys
          - operator: neq
            selector: auth.identity.group
            value: admin
        data:
        - static:
            key: default/toystore-per-endpoint/toys
            value: "1"
        - selector:
            selector: auth.identity.username
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      app: istio-ingressgateway
      istio: ingressgateway
  url: oci://quay.io/kuadrant/wasm-shim:latest

Note that e2e rate limiting does not work. WASM shim work Kuadrant/wasm-shim#35 is needed.

eguzki · 2023-06-13T15:27:09Z

ready for initial review @guicassolato

Still lot of work to be done, but sets a baseline we can merge in rlp-v2-base branch

guicassolato · 2023-06-14T15:44:10Z

api/v1beta2/ratelimitpolicy_types.go

+// ContextSelector defines one item from the well known attributes
+// Attributes: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes
+// Well-known selectors: https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors


It's probably going to be replaced with https://github.com/Kuadrant/architecture/blob/rfc/well-known-attributes/rfcs/0000-well-known-attributes.md (once merged)

guicassolato · 2023-06-15T14:46:42Z

pkg/rlptools/wasm_utils.go

-	if rlp == nil {
-		return flattenActions
-	}
+// TODO implement one of constraint


I see at least a couple ways to implement this. E.g. https://github.com/Kuadrant/authorino/blob/5db6fb9153d5229188b03292249cbf04bfb5fafb/install/crd/kustomization.yaml#L15-L21.

Is that what you had in mind? Or perhaps a validating webhook instead? Or yet, "late" in the process, in the controller, during reconciliation?

This is the OpenAPIv3 validation that prevents objects to be created and/or updated without needing any validation webhook.

guicassolato · 2023-06-15T14:52:52Z

pkg/rlptools/wasm_utils.go

 )

-type GatewayAction struct {
-	Configurations []kuadrantv1beta1.Configuration `json:"configurations"`
+type SelectorSpec struct {


Should we move this or part of it to a common package?

good idea. What do you propose?

github.com/kuadrant/kuadrant-operator/api/v1beta2 ?

github.com/kuadrant/kuadrant-operator/pkg/types ?

done, moved to github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm

guicassolato · 2023-06-15T14:59:17Z

pkg/rlptools/wasm_utils.go

+	// They are named by a dot-separated path (e.g. request.path)
+	// Examples:
+	// "request.path" -> The path portion of the URL
+	Selector string `json:"selector"`


What about defining a type WellknownSelector string?

Suggested change

Selector string `json:"selector"`

Selector WellknownSelector `json:"selector"`

It would also serve as the type for PatternExpression.Selector:

kuadrant-operator/pkg/rlptools/wasm_utils.go

Line 72 in 5c06205

Selector string `json:"selector"`

in

kuadrant-operator/api/v1beta2/ratelimitpolicy_types.go

Line 39 in 3966aa6

type ContextSelector string

we use ContextSelector. Shall we change that as well?

using ContextSelector in WASM plugin Configuration object

guicassolato · 2023-06-15T15:06:45Z

pkg/rlptools/wasm_utils.go

 	// +optional
-	GatewayActions []GatewayAction `json:"gateway_actions,omitempty"`
+	Rules []Rule `json:"rules,omitempty"`


Are we also renaming failure_mode_deny and rate_limit_policies as in Kuadrant/wasm-shim#35 (comment)?

guicassolato

Excellent, @eguzki! 🚀

I've left a few reminders for when continuing the work in the next PRs.

guicassolato · 2023-06-20T11:13:30Z

controllers/ratelimitpolicy_wasm_plugins.go

+	for domain, rules := range wasmRulesByDomain {
+		rateLimitPolicy := wasm.RateLimitPolicy{
+			Name:      domain,
+			Domain:    common.MarshallNamespace(gw.Key(), domain),


guicassolato · 2023-06-20T11:17:09Z

pkg/rlptools/wasm_utils.go

+	conditions := make([]wasm.Condition, 0)
+
+	for routeSelectorIdx := range limit.RouteSelectors {
+		// TODO(eastizle): what if there are only Hostnames (i.e. empty "matches" list)


That means it matches all HTTPRouteRules.

guicassolato · 2023-06-20T11:18:18Z

pkg/rlptools/wasm_utils.go

+		for matchIdx := range limit.RouteSelectors[routeSelectorIdx].Matches {
+			condition := wasm.Condition{
+				AllOf: patternExpresionsFromMatch(&limit.RouteSelectors[routeSelectorIdx].Matches[matchIdx]),
+			}


Actually, the Wasm conditions should not come directly from the limit, but use the limit's routeSelectors to select the rules from within the HTTPRoutes: #200

guicassolato · 2023-06-20T11:26:59Z

pkg/rlptools/wasm_utils.go

+	// merge when expression in the same condition
+	// must be done after adding route level conditions when no route selector are available
+	// prevent conditions only filled with "when" definitions
+	for whenIdx := range limit.When {
+		for idx := range conditions {
+			conditions[idx].AllOf = append(conditions[idx].AllOf, patternExpresionFromWhen(limit.When[whenIdx]))
 		}
+	}


We're drifting a bit from the proposed "soft conditions" implemented in Limitador (see last bullet point at https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#highlights), by going one step ahead to improve performance.

By evaluating the RLP's when condition as a normal condition in the Wasm shim (just like the conditions built from routeSelectors), the RL request will only be sent to Limitador when there's truly at least one limit whose counters to increment.

eguzki · 2023-06-20T11:38:58Z

@guicassolato consider rebasing rlp-v2-base on top of current main to bring all new stuff since it was created. I think no one is currently working in this rlp-v2 so we are safe rebasing it.

eguzki changed the title ~~RLP v2 controller~~ RLP v2 controller (try 01) Jun 13, 2023

eguzki changed the title ~~RLP v2 controller (try 01)~~ RLP v2 controller Jun 13, 2023

eguzki marked this pull request as ready for review June 13, 2023 15:26

eguzki requested a review from a team as a code owner June 13, 2023 15:26

guicassolato reviewed Jun 14, 2023

View reviewed changes

This was referenced Jun 15, 2023

[rlp-v2] Reconciliation of route selectors #200

Closed

[rlp-v2] Do not use gateway name nor host in the RL domain #201

Closed

guicassolato reviewed Jun 15, 2023

View reviewed changes

eguzki added 5 commits June 16, 2023 15:08

rlp v1beta2 controller

2db1af1

WASM plugin configuration object v2

d5d89ba

bundle update

43b4d65

fixup! bundle update

8c45d6d

fix tests

3966aa6

eguzki force-pushed the rlp-v2-controller branch from 5c06205 to 3966aa6 Compare June 16, 2023 14:18

eguzki added 4 commits June 16, 2023 16:35

WASMPlugin update to camel case

f81455a

fix integration tests

188c422

wasm types moved to a common package

12c41d0

different selectors share type: ContextSelector

23ee7b1

guicassolato approved these changes Jun 20, 2023

View reviewed changes

eguzki merged commit 93baa9a into rlp-v2-base Jun 20, 2023

eguzki deleted the rlp-v2-controller branch June 20, 2023 11:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RLP v2 controller #199

RLP v2 controller #199

eguzki commented Jun 8, 2023 •

edited

Loading

eguzki commented Jun 13, 2023

guicassolato Jun 14, 2023

guicassolato Jun 15, 2023

eguzki Jun 16, 2023

guicassolato Jun 15, 2023

eguzki Jun 16, 2023

eguzki Jun 19, 2023

guicassolato Jun 15, 2023

eguzki Jun 16, 2023

eguzki Jun 19, 2023

guicassolato Jun 15, 2023

guicassolato left a comment

guicassolato Jun 20, 2023

guicassolato Jun 20, 2023

guicassolato Jun 20, 2023

guicassolato Jun 20, 2023

eguzki commented Jun 20, 2023 •

edited

Loading

	Selector string `json:"selector"`
	Selector WellknownSelector `json:"selector"`

RLP v2 controller #199

RLP v2 controller #199

Conversation

eguzki commented Jun 8, 2023 • edited Loading

What

Verification steps

eguzki commented Jun 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guicassolato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eguzki commented Jun 20, 2023 • edited Loading

eguzki commented Jun 8, 2023 •

edited

Loading

eguzki commented Jun 20, 2023 •

edited

Loading