AuthPolicy top-level conditions based on CEL only

[guicassolato]

• Removes from the AuthPolicy the deprecated selector, operator, value user-defined top-level conditions, in favour of using CEL predicates for top-level conditions only.
• Apply to the AuthPolicy's user-defined top-level conditions the same behaviour as the RateLimitPolicy when merging policies together – i.e. the entire set of conditions treated as one single atomic rule of the policy.
• No longer move AuthPolicy's user-defined top-level conditions to the Authorino AuthConfigs, since these conditions are evaluated in the wasm module.
• Side-effect: drop support for named patterns for user-defined top-level conditions in the AuthPolicy; they can still be used for rule-level conditions though.

Verification steps

make local-setup

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

kubectl apply -f examples/toystore/toystore.yaml

kubectl apply -f - <<EOF
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: toystore
spec:
  parentRefs:
  - name: kuadrant-ingressgateway
    namespace: gateway-system
  rules:
  - backendRefs:
    - name: toystore
      port: 80
EOF

Create a couple policies:

kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta3
kind: RateLimitPolicy
metadata:
  name: toystore
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  when:
    - predicate: request.size > 1024
  limits:
    "create-toy":
      rates:
      - limit: 5
        window: 10s
      when:
      - predicate: "request.url_path != '/admin'"
---
apiVersion: kuadrant.io/v1beta3
kind: AuthPolicy
metadata:
  name: toystore
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  when:
    - predicate: request.size > 1024
  rules:
    authentication:
      "anonymous":
        anonymous: {}
        when:
        - predicate: "request.url_path != '/anonymous'"
EOF

Check the wasm config: (The sets of top-level conditions of each policy go straight into the predicates of the action in the wasm config.)

kubectl get wasmplugin/kuadrant-kuadrant-ingressgateway -n gateway-system -o yaml

Expected output

apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-kuadrant-ingressgateway
  namespace: gateway-system
  …
spec:
  phase: STATS
  pluginConfig:
    actionSets:
    - actions:
      - predicates:
        - request.size > 1024
        scope: e2db39952dd3bc72e152330a2eb15abbd9675c7ac6b54a1a292f07f25f09f138
        service: auth-service
      - data:
        - expression:
            key: limit.create_toy__fb82c51a
            value: "1"
        predicates:
        - request.size > 1024
        - request.url_path != '/admin'
        scope: default/toystore
        service: ratelimit-service
      name: 21cb3adc608c09a360d62a03fd1afd7cc6f8720999a51d7916927fff26a34ef8
      routeRuleConditions:
        hostnames:
        - '*'
        predicates:
        - request.url_path.startsWith('/')
    services:
      auth-service:
        endpoint: kuadrant-auth-service
        failureMode: deny
        type: auth
      ratelimit-service:
        endpoint: kuadrant-ratelimit-service
        failureMode: allow
        type: ratelimit
  targetRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: kuadrant-ingressgateway
  url: oci://quay.io/kuadrant/wasm-shim:latest

Create a default merge policy:

kubectl apply -n gateway-system -f - <<EOF
apiVersion: kuadrant.io/v1beta3
kind: AuthPolicy
metadata:
  name: gw-auth
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: kuadrant-ingressgateway
  defaults:
    strategy: merge
    when:
      - predicate: request.size > 2048
      - predicate: |
          !source.ip.matches("127\.0\.0\.\d+")
    rules:
      authorization:
        "deny-all":
          opa:
            rego: allow = false
EOF

Check the wasm config: (The set of conditions from the most specific policy wins.)

kubectl get wasmplugin/kuadrant-kuadrant-ingressgateway -n gateway-system -o yaml

Expected output

apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-kuadrant-ingressgateway
  namespace: gateway-system
  …
spec:
  phase: STATS
  pluginConfig:
    actionSets:
    - actions:
      - predicates:
        - request.size > 1024
        scope: e2db39952dd3bc72e152330a2eb15abbd9675c7ac6b54a1a292f07f25f09f138
        service: auth-service
      - data:
        - expression:
            key: limit.create_toy__fb82c51a
            value: "1"
        predicates:
        - request.size > 1024
        - request.url_path != '/admin'
        scope: default/toystore
        service: ratelimit-service
      name: 21cb3adc608c09a360d62a03fd1afd7cc6f8720999a51d7916927fff26a34ef8
      routeRuleConditions:
        hostnames:
        - '*'
        predicates:
        - request.url_path.startsWith('/')
    services:
      auth-service:
        endpoint: kuadrant-auth-service
        failureMode: deny
        type: auth
      ratelimit-service:
        endpoint: kuadrant-ratelimit-service
        failureMode: allow
        type: ratelimit
  targetRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: kuadrant-ingressgateway
  url: oci://quay.io/kuadrant/wasm-shim:latest

Create an override merge policy:

kubectl apply -n gateway-system -f - <<EOF
apiVersion: kuadrant.io/v1beta3
kind: AuthPolicy
metadata:
  name: gw-auth
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: kuadrant-ingressgateway
  overrides:
    strategy: merge
    when:
      - predicate: request.size > 2048
      - predicate: |
          !source.ip.matches("127\.0\.0\.\d+")
    rules:
      authorization:
        "deny-all":
          opa:
            rego: allow = false
EOF

Check the wasm config: (The set of conditions from the least specific policy wins.)

kubectl get wasmplugin/kuadrant-kuadrant-ingressgateway -n gateway-system -o yaml

Expected output

apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-kuadrant-ingressgateway
  namespace: gateway-system
  …
spec:
  phase: STATS
  pluginConfig:
    actionSets:
    - actions:
      - predicates:
        - request.size > 2048
        - |
          !source.ip.matches("127\.0\.0\.\d+")
        scope: e2db39952dd3bc72e152330a2eb15abbd9675c7ac6b54a1a292f07f25f09f138
        service: auth-service
      - data:
        - expression:
            key: limit.create_toy__fb82c51a
            value: "1"
        predicates:
        - request.size > 1024
        - request.url_path != '/admin'
        scope: default/toystore
        service: ratelimit-service
      name: 21cb3adc608c09a360d62a03fd1afd7cc6f8720999a51d7916927fff26a34ef8
      routeRuleConditions:
        hostnames:
        - '*'
        predicates:
        - request.url_path.startsWith('/')
    services:
      auth-service:
        endpoint: kuadrant-auth-service
        failureMode: deny
        type: auth
      ratelimit-service:
        endpoint: kuadrant-ratelimit-service
        failureMode: allow
        type: ratelimit
  targetRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: kuadrant-ingressgateway
  url: oci://quay.io/kuadrant/wasm-shim:latest

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.33%. Comparing base (cc1b41f) to head (8ecfdbf).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #988      +/-   ##
==========================================
+ Coverage   76.15%   76.33%   +0.17%     
==========================================
  Files         111      112       +1     
  Lines        8986     8995       +9     
==========================================
+ Hits         6843     6866      +23     
+ Misses       1852     1836      -16     
- Partials      291      293       +2     

Flag	Coverage Δ
bare-k8s-integration	10.86% <0.00%> (-0.02%)	⬇️
controllers-integration	58.55% <62.85%> (-0.31%)	⬇️
envoygateway-integration	33.16% <28.57%> (+0.65%)	⬆️
gatewayapi-integration	13.42% <0.00%> (-0.02%)	⬇️
istio-integration	34.68% <82.85%> (+0.34%)	⬆️
unit	25.83% <0.00%> (+0.46%)	⬆️

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

Components	Coverage Δ
api/v1beta1 (u)	92.18% <100.00%> (ø)
api/v1beta2 (u)	∅ <ø> (∅)
pkg/common (u)	87.67% <ø> (ø)
pkg/istio (u)	47.03% <ø> (ø)
pkg/log (u)	93.18% <ø> (ø)
pkg/reconcilers (u)	∅ <ø> (∅)
pkg/rlptools (u)	∅ <ø> (∅)
controllers (i)	84.18% <96.77%> (-0.24%)	⬇️

Files with missing lines	Coverage Δ
api/v1beta3/authpolicy_types.go	81.60% <100.00%> (+2.16%)	⬆️
api/v1beta3/common_types.go	100.00% <100.00%> (ø)
api/v1beta3/ratelimitpolicy_types.go	79.82% <100.00%> (+1.52%)	⬆️
controllers/auth_workflow_helpers.go	95.32% <100.00%> (-0.05%)	⬇️
controllers/authconfigs_reconciler.go	86.31% <ø> (-0.29%)	⬇️
pkg/wasm/types.go	68.93% <ø> (-0.30%)	⬇️
pkg/wasm/utils.go	89.25% <ø> (+6.71%)	⬆️

... and 7 files with indirect coverage changes

[alexsnaps]

i.e. the entire set of conditions treated as one single atomic rule of the policy.

Confused about that bit… Do you mean that they are all now always ANDed together (i.e what all was iirc) and there is no any anymore?

[alexsnaps]

Side-effect: drop support for named patterns for user-defined top-level conditions in the AuthPolicy; they can still be used for rule-level conditions though.

Would love for us to find a way to eventually define these as functions (CEL) and make these usable by the user in their conditions/expression… 🤔

[guicassolato]

i.e. the entire set of conditions treated as one single atomic rule of the policy.

Confused about that bit… Do you mean that they are all now always ANDed together (i.e what all was iirc) and there is no any anymore?

No, that's not what I meant. This is about merging 2 policies with the merge strategy.

Say 2 AuthPolicies specify respectively sets A and B of when predicates. Before this PR, the merge would yield a set of when predicates A ∪ B. Now, it is going to be A or B.