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

feat: accepted policy status condition #347

Merged
merged 13 commits into from
Jan 9, 2024

Conversation

KevFan
Copy link
Contributor

@KevFan KevFan commented Nov 29, 2023

Description

Part of Kuadrant/architecture#38

Implements the Accepted Status Condition to RateLimitPolicy and AuthPolicy

Verfication

Functionality is generally already tested with the integration tests added.

To verify manually instead:

Setup

  • Checkout this branch
  • Deploy using make local-setup
  • Create HTTPRoute
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
EOF

AuthPolicy

  • Create AuthPolicies
kubectl apply -f - <<EOF
# Accepted
---
apiVersion: kuadrant.io/v1beta2
kind: AuthPolicy
metadata:
  name: auth0
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  rules:
    authorization:
      deny-all:
        opa:
          rego: "allow = false"
    response:
      unauthorized:
        headers:
          "content-type":
            value: application/json
        body:
          value: |
            {
              "error": "Forbidden",
              "message": "Access denied by default by the gateway operator. If you are the administrator of the service, create a specific auth policy for the route."
            }
---
# Conflict
apiVersion: kuadrant.io/v1beta2
kind: AuthPolicy
metadata:
  name: auth1
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  rules:
    authorization:
      deny-all:
        opa:
          rego: "allow = false"
    response:
      unauthorized:
        headers:
          "content-type":
            value: application/json
        body:
          value: |
            {
              "error": "Forbidden",
              "message": "Access denied by default by the gateway operator. If you are the administrator of the service, create a specific auth policy for the route."
            }
---
# target not found
apiVersion: kuadrant.io/v1beta2
kind: AuthPolicy
metadata:
  name: auth2
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: istio-ingressgateway-not-found
  rules:
    authorization:
      deny-all:
        opa:
          rego: "allow = false"
    response:
      unauthorized:
        headers:
          "content-type":
            value: application/json
        body:
          value: |
            {
              "error": "Forbidden",
              "message": "Access denied by default by the gateway operator. If you are the administrator of the service, create a specific auth policy for the route."
            }
---
# invalid
apiVersion: kuadrant.io/v1beta2
kind: AuthPolicy
metadata:
  name: auth3
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: istio-ingressgateway
    namespace: istio-system
  rules:
    authorization:
      deny-all:
        opa:
          rego: "allow = false"
    response:
      unauthorized:
        headers:
          "content-type":
            value: application/json
        body:
          value: |
            {
              "error": "Forbidden",
              "message": "Access denied by default by the gateway operator. If you are the administrator of the service, create a specific auth policy for the route."
            }
EOF
  • Verify accepted condition status is as expected
kubectl get authpolicies -n default -o yaml | yq '.items[].status'
Sample Output
conditions:
  - lastTransitionTime: "2023-12-14T17:25:00Z"
    message: AuthPolicy has been accepted
    reason: Accepted
    status: "True"
    type: Accepted
observedGeneration: 2
conditions:
  - lastTransitionTime: "2023-12-14T17:25:00Z"
    message: 'AuthPolicy is conflicted by default/auth0: the gateway.networking.k8s.io/v1, Kind=HTTPRoute target default/toystore is already referenced by policy default/auth0'
    reason: Conflicted
    status: "False"
    type: Accepted
observedGeneration: 2
conditions:
  - lastTransitionTime: "2023-12-14T17:25:00Z"
    message: AuthPolicy target istio-ingressgateway-not-found was not found
    reason: TargetNotFound
    status: "False"
    type: Accepted
observedGeneration: 1
conditions:
  - lastTransitionTime: "2023-12-14T17:25:00Z"
    message: 'AuthPolicy target is invalid: invalid targetRef.Namespace istio-system. Currently only supporting references to the same namespace'
    reason: Invalid
    status: "False"
    type: Accepted
observedGeneration: 2

RateLimitPolicy

  • Create RateLimitPolicies
kubectl apply -f - <<EOF
---
# Accepted
apiVersion: kuadrant.io/v1beta2                                                                                                                          
kind: RateLimitPolicy
metadata:                
  name: rlp0
spec:   
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  limits:   
    "create-toy":
      rates:
      - limit: 5
        duration: 10
        unit: second
---
# Conflict
apiVersion: kuadrant.io/v1beta2                                                                                                                          
kind: RateLimitPolicy
metadata:                
  name: rlp1
spec:   
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  limits:   
    "create-toy":
      rates:
      - limit: 5
        duration: 10
        unit: second
---
# Target not found
apiVersion: kuadrant.io/v1beta2                                                                                                                          
kind: RateLimitPolicy
metadata:                
  name: rlp2
spec:   
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: istio-ingressgateway-not-found
  limits:   
    "create-toy":
      rates:
      - limit: 5
        duration: 10
        unit: second
---
# invalid
apiVersion: kuadrant.io/v1beta2                                                                                                                          
kind: RateLimitPolicy
metadata:                
  name: rlp3
spec:   
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: istio-ingressgateway
    namespace: istio-system
  limits:   
    "create-toy":
      rates:
      - limit: 5
        duration: 10
        unit: second
EOF
  • Verifiy RateLimitPolicy accepted condition is as expected
 kubectl get ratelimitpolicies -n default -o yaml | yq '.items[].status'
Sample Output
conditions:
  - lastTransitionTime: "2023-12-14T17:38:13Z"
    message: RateLimitPolicy has been accepted
    reason: Accepted
    status: "True"
    type: Accepted
observedGeneration: 1
conditions:
  - lastTransitionTime: "2023-12-14T17:38:13Z"
    message: 'RateLimitPolicy is conflicted by default/rlp0: the gateway.networking.k8s.io/v1, Kind=HTTPRoute target default/toystore is already referenced by policy default/rlp0'
    reason: Conflicted
    status: "False"
    type: Accepted
observedGeneration: 4
conditions:
  - lastTransitionTime: "2023-12-14T17:38:39Z"
    message: RateLimitPolicy target istio-ingressgateway-not-found was not found
    reason: TargetNotFound
    status: "False"
    type: Accepted
observedGeneration: 1
conditions:
  - lastTransitionTime: "2023-12-14T17:38:39Z"
    message: 'RateLimitPolicy target is invalid: invalid targetRef.Namespace istio-system. Currently only supporting references to the same namespace'
    reason: Invalid
    status: "False"
    type: Accepted

@KevFan KevFan self-assigned this Nov 29, 2023
@KevFan KevFan requested a review from a team as a code owner November 29, 2023 16:29
@KevFan KevFan changed the title wip: rlp todos for new conidtions wip: policy status Nov 29, 2023
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Merging #347 (6874ca6) into main (94683a2) will increase coverage by 0.47%.
The diff coverage is 89.36%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
+ Coverage   65.01%   65.48%   +0.47%     
==========================================
  Files          35       37       +2     
  Lines        3796     3842      +46     
==========================================
+ Hits         2468     2516      +48     
- Misses       1132     1135       +3     
+ Partials      196      191       -5     
Flag Coverage Δ
integration 70.36% <81.48%> (+0.01%) ⬆️
unit 60.14% <92.53%> (+1.23%) ⬆️

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

Components Coverage Δ
api/v1beta1 (u) ∅ <ø> (∅)
pkg/common (u) 78.59% <100.00%> (+1.67%) ⬆️
pkg/istio (u) 37.11% <ø> (ø)
pkg/log (u) 31.81% <ø> (ø)
pkg/reconcilers (u) 33.44% <66.66%> (+0.22%) ⬆️
pkg/rlptools (u) 56.46% <ø> (ø)
controllers (i) 70.36% <81.48%> (+0.01%) ⬆️
Files Coverage Δ
controllers/authpolicy_status.go 93.44% <100.00%> (-1.01%) ⬇️
controllers/ratelimitpolicy_status.go 97.43% <100.00%> (-0.68%) ⬇️
pkg/common/apimachinery_status_conditions.go 100.00% <100.00%> (ø)
pkg/common/common.go 88.39% <ø> (ø)
pkg/common/errors.go 100.00% <100.00%> (ø)
pkg/common/test_utils.go 100.00% <100.00%> (ø)
pkg/reconcilers/targetref_reconciler.go 31.05% <66.66%> (+0.36%) ⬆️
api/v1beta2/authpolicy_types.go 70.83% <0.00%> (-2.03%) ⬇️
api/v1beta2/ratelimitpolicy_types.go 17.24% <0.00%> (-0.62%) ⬇️
controllers/authpolicy_controller.go 76.66% <80.00%> (+2.69%) ⬆️
... and 1 more

... and 3 files with indirect coverage changes

@alexsnaps alexsnaps added this to the v0.6.0 milestone Dec 4, 2023
@KevFan KevFan force-pushed the rlp-policy-status branch 4 times, most recently from 8fdebeb to d5eb4c3 Compare December 5, 2023 16:19
@KevFan KevFan marked this pull request as draft December 5, 2023 18:13
@maleck13
Copy link
Collaborator

maleck13 commented Dec 6, 2023

@sergioifg94 FYI

@KevFan KevFan changed the title wip: policy status feat: accpeted policy status condition Dec 8, 2023
@KevFan KevFan changed the title feat: accpeted policy status condition feat: accepted policy status condition Dec 11, 2023
@KevFan KevFan force-pushed the rlp-policy-status branch 2 times, most recently from 0bb44f2 to b52a06d Compare December 13, 2023 10:45
@KevFan
Copy link
Contributor Author

KevFan commented Dec 13, 2023

Some integration tests based on istioctl is currently broken due to #371

This has been merged to main

@KevFan KevFan marked this pull request as ready for review December 14, 2023 10:45
@KevFan KevFan modified the milestones: v0.6.0, v0.7.0 Dec 14, 2023
Copy link

@sergioifg94 sergioifg94 left a comment

Choose a reason for hiding this comment

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

Code changes look good, haven't verified yet, just had one tiny optional suggestion for the AuthPolicy controller test

controllers/authpolicy_controller_test.go Outdated Show resolved Hide resolved
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.

Tested with make local-setup ISTIO_INSTALL_SAIL=false while #380 is not merged.

❯ kubectl get authpolicies -n default -o wide
NAME    STATUS           TARGETREFKIND   TARGETREFNAME                    AGE
auth0   Accepted         HTTPRoute       toystore                         3m31s
auth1   Conflicted       HTTPRoute       toystore                         3m31s
auth2   TargetNotFound   Gateway         istio-ingressgateway-not-found   3m31s
auth3   Invalid          Gateway         istio-ingressgateway             3m31s

❯  kubectl get ratelimitpolicies -n default -o wide
NAME   STATUS           TARGETREFKIND   TARGETREFNAME                    AGE
rlp0   Accepted         HTTPRoute       toystore                         36s
rlp1   Conflicted       HTTPRoute       toystore                         36s
rlp2   TargetNotFound   Gateway         istio-ingressgateway-not-found   36s
rlp3   Invalid          Gateway         istio-ingressgateway             36s

Good job!

@KevFan
Copy link
Contributor Author

KevFan commented Jan 5, 2024

Just rebased with latest changes from main

@KevFan KevFan added kind/enhancement New feature or request size/medium area/api Changes user facing APIs labels Jan 5, 2024
Copy link
Member

@didierofrivia didierofrivia left a comment

Choose a reason for hiding this comment

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

Most of the current failed tests are failing because of checking the ConfigMap holding the istioctl config, but I don't see any changes here affecting that directly... let me know and I could inspect the tests a bit more thoroughly.

@KevFan
Copy link
Contributor Author

KevFan commented Jan 9, 2024

Rebased with main containing #391

@KevFan KevFan merged commit ed590ec into Kuadrant:main Jan 9, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Changes user facing APIs kind/enhancement New feature or request size/medium
Projects
Status: Done
Status: In Progress
Development

Successfully merging this pull request may close these issues.

6 participants