RLP Defaults

[KevFan]

Description

Closes: #455

• Adds defaults field for full atomic explicit defaults
• The bare rules (i.e. implict defaults "spec.limits") is kept, but is now mutually exclusive with the above via CEL
• RLP controller integrations tests now uses the explicit defaults in the RLP CR which test this functionality

Verification

Functionality is already tested by integration tests but if you want to verify functionality:

• Follow https://github.com/Kuadrant/kuadrant-operator/blob/main/doc/user-guides/simple-rl-for-app-developers.md
• Verify guide complete successfully.
• Now repeat the guide again but at step 3, use defaults field for defining limits instead:

kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: toystore
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  defaults:
    limits:
      "create-toy":
        rates:
        - limit: 5
          duration: 10
          unit: second
        routeSelectors:
        - matches: # selects the 2nd HTTPRouteRule of the targeted route
          - method: POST
            path:
              type: Exact
              value: "/toys"
EOF

• Verify guide completes successfully
• Try creating a RLP with both implicit and explicit defaults:

kubectl apply -f  - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: toystore
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  defaults:
    limits:
      "create-toy":
        rates:
        - limit: 5
          duration: 10
          unit: second
  limits:
    "toy2":
      rates:
      - limit: 10
        duration: 20
        unit: second
EOF

• Verify policy is rejected due to:

The RateLimitPolicy "toystore" is invalid: spec: Invalid value: "object": Implicit and explicit defaults are mutually exclusive

[codecov]

Codecov Report

Merging #456 (e66d50c) into main (ece13e8) will increase coverage by 0.06%.
The diff coverage is 87.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #456      +/-   ##
==========================================
+ Coverage   30.03%   30.10%   +0.06%     
==========================================
  Files          58       58              
  Lines        4132     4136       +4     
==========================================
+ Hits         1241     1245       +4     
  Misses       2822     2822              
  Partials       69       69              

Flag	Coverage Δ
unit	30.10% <87.50%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
api/v1beta1 (u)	∅ <ø> (∅)
api/v1beta2 (u)	52.08% <80.00%> (+1.36%)	⬆️
pkg/common (u)	87.70% <ø> (ø)
pkg/istio (u)	41.61% <ø> (ø)
pkg/log (u)	36.84% <ø> (ø)
pkg/reconcilers (u)	38.20% <ø> (ø)
pkg/rlptools (u)	56.40% <100.00%> (ø)
controllers (i)	8.09% <ø> (ø)

Files	Coverage Δ
pkg/rlptools/utils.go	100.00% <100.00%> (ø)
pkg/rlptools/wasm_utils.go	59.60% <100.00%> (ø)
api/v1beta2/ratelimitpolicy_types.go	22.03% <80.00%> (+5.67%)	⬆️

[KevFan]

Other places such as limitador_cluster_envoyfilter_controller_test.go still use implicit limits to test the implicit path still works as intended

[KevFan]

Not sure what do we want to do with the guides, they still use the implicit default fields for RLP. Not sure do we want to update them to use the explicit default field instead 🤔

[Boomatang]

TL:DR code works as expected.

The function of the code seems to be working as expected, however I do have a local issue with writing the RLP spec into the isisto gateway. Happens on main also. This meant that I could not follow the linked guide.

What I did observe was the limitador CR being update with the implicit or explicit defaults, which was expected. My assumption the kuadrant operator did write these changes to the wasm shim.

[KevFan]

Rebased and resolved conflicts

[KevFan]

If using podman runtime with docker cli also, there was the following warning locally for me:

 => CACHED [stage-1 2/3] COPY --from=builder /workspace/manager .                                                                                  0.0s
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load

This can cause the cluster to use an old build image as the newly built image is kept in cache only