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

[authpolicy-v2] AuthPolicy v1beta2 #249

Merged
merged 35 commits into from
Oct 20, 2023
Merged

[authpolicy-v2] AuthPolicy v1beta2 #249

merged 35 commits into from
Oct 20, 2023

Conversation

guicassolato
Copy link
Contributor

@guicassolato guicassolato commented Sep 4, 2023

Base branch for the first chunk of #207.

Closes:

As well as:

How to review/verify this PR

@guicassolato guicassolato self-assigned this Sep 4, 2023
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #249 (c639e93) into main (4d38913) will increase coverage by 1.39%.
The diff coverage is 69.14%.

@@            Coverage Diff             @@
##             main     #249      +/-   ##
==========================================
+ Coverage   63.42%   64.81%   +1.39%     
==========================================
  Files          33       35       +2     
  Lines        3204     3806     +602     
==========================================
+ Hits         2032     2467     +435     
- Misses        999     1147     +148     
- Partials      173      192      +19     
Flag Coverage Δ
integration 70.58% <69.31%> (+0.97%) ⬆️
unit 58.20% <68.49%> (+0.68%) ⬆️

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

Components Coverage Δ
api/v1beta1 (u) ∅ <ø> (∅)
pkg/common (u) 73.92% <0.00%> (-1.98%) ⬇️
pkg/istio (u) 30.24% <ø> (ø)
pkg/log (u) 31.81% <ø> (ø)
pkg/reconcilers (u) 33.68% <ø> (ø)
pkg/rlptools (u) 56.41% <100.00%> (-1.23%) ⬇️
controllers (i) 70.58% <69.31%> (+0.97%) ⬆️
Files Coverage Δ
api/v1beta2/ratelimitpolicy_types.go 26.86% <100.00%> (+4.64%) ⬆️
api/v1beta2/route_selectors.go 100.00% <100.00%> (ø)
controllers/kuadrant_controller.go 50.00% <100.00%> (-0.71%) ⬇️
pkg/common/common.go 88.39% <ø> (ø)
pkg/rlptools/wasm_utils.go 61.05% <100.00%> (-1.77%) ⬇️
controllers/authpolicy_status.go 94.44% <83.33%> (ø)
controllers/authpolicy_controller.go 74.32% <70.00%> (+4.01%) ⬆️
controllers/httprouteparentrefs_eventmapper.go 53.19% <53.19%> (ø)
pkg/common/gatewayapi_utils.go 62.82% <0.00%> (-3.11%) ⬇️
api/v1beta2/authpolicy_types.go 77.57% <77.57%> (ø)
... and 2 more

... and 2 files with indirect coverage changes

@Boomatang
Copy link
Contributor

So far this is looking good. I am not sure if there are verification steps to be carried out right now.

@guicassolato guicassolato changed the title AuthPolicy v1beta2 [authpolicy-v2] AuthPolicy v1beta2 Sep 19, 2023
@guicassolato guicassolato force-pushed the authpolicy-v2 branch 3 times, most recently from afa63b1 to 6491b20 Compare September 27, 2023 13:39
@guicassolato guicassolato force-pushed the authpolicy-v2 branch 3 times, most recently from 4820025 to 8818edb Compare October 10, 2023 13:33
@guicassolato guicassolato marked this pull request as ready for review October 11, 2023 14:50
@guicassolato guicassolato requested a review from a team as a code owner October 11, 2023 14:50
@maleck13
Copy link
Collaborator

changes all look reasonable. Bit unsure as to why RLP changed in the PR though ( I am sure there is a good reason)

@maleck13
Copy link
Collaborator

/lgtm

@guicassolato guicassolato force-pushed the authpolicy-v2 branch 2 times, most recently from 1ba111e to 856d6d8 Compare October 18, 2023 09:49
@guicassolato guicassolato requested a review from a team October 18, 2023 14:55
Defines new `v1beta2` version of the `AuthPolicy` CRD, based on Authorino's `AuthConfig/v1beta2`.

Closes #247

Depends on Kuadrant/authorino#417, Kuadrant/authorino-operator#137

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version

Update AuthPolicy manifests based on latest AuthConfig v1beta2 changes

Bump Authorino to latest (unreleased) version

Bump Authorino to latest (unreleased) version
Enables [superseding of strict host subsets](https://github.com/Kuadrant/authorino/blob/main/docs/architecture.md#avoiding-host-name-collision) in Authorino – i.e., set `SupersedingHostSubsets` to `true` in the Authorino CR.

Closes #264.
[authpolicy-v2] AuthConfig superseding of strict host subsets
…y) into all Istio AuthorizationPolicy rules that do not include hostnames already built from the route selectors, so we don't send a request to authorino for hosts that are not in the scope of the policy
…entRefs of the route and finds all policies that target one of its parent resources, thus yielding events for those policies.
…or full HTTPRoute only (i.e. ignore config-level conditions)
+ unit tests from Istio AuthorizationPolicy rules from HTTPRouteRules and hostnames
[authpolicy-v2] Well-known attributes in the generated AuthConfigs
…el up

This will pair the level of these policy-wide options to the top-level 'routeSelectors', rather than having two things that have semantics over the same scope defined at different levels in the API.

This change also separates the auth scheme, making it now exclusively about auth rules.
[authpolicy-v2] Move AuthPolicy top-level 'patterns' and 'when' fields one level up
* docs: authpolicy v1beta2

* docs: addressing suggestions of enhancements to the authpolicy docs
* prevent usage of routeSelectors in a gateway AuthPolicy

* AuthPolicy CEL validation rules

- Invalid targetRef.group
- Invalid targetRef.kind
- Route selectors not supported when targeting a Gateway

Note: cannot set a validation rule for !has(spec.targetRef.namespace) || spec.targetRef.namespace == metadata.namespace, because Kubernetes does not allow accessing `metadata.namespace`. See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-rules
Copy link
Collaborator

@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.

Quite an effort put on this one, thanks 🙏🏼

🥇 🥇 🥇

@guicassolato guicassolato merged commit 6bffa0b into main Oct 20, 2023
21 checks passed
@eguzki eguzki deleted the authpolicy-v2 branch December 13, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants