-
Notifications
You must be signed in to change notification settings - Fork 33
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
sotw: auth #952
sotw: auth #952
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #952 +/- ##
==========================================
- Coverage 81.49% 76.55% -4.94%
==========================================
Files 102 113 +11
Lines 7177 9056 +1879
==========================================
+ Hits 5849 6933 +1084
- Misses 898 1813 +915
+ Partials 430 310 -120
Flags with carried forward coverage won't be shown. Click here to find out more.
|
d3482e6
to
d90b343
Compare
d90b343
to
6236679
Compare
820f8eb
to
b32a441
Compare
e92b91d
to
cba2fbc
Compare
@guicassolato who is the right person to review this? I am happy to take a look to help it move forward, but someone who has worked on auth or sotw might be a better candidate? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some of the things I noticed while reviewing the code.
I was giving this a go following the authenticated rate limiting guide and am hitting an error trying to define an AuthPolicy with kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta3
kind: AuthPolicy
metadata:
name: toystore
spec:
targetRef:
group: gateway.networking.k8s.io
kind: HTTPRoute
name: toystore
rules:
authentication:
"api-key-users":
apiKey:
selector:
matchLabels:
app: toystore
allNamespaces: true
credentials:
authorizationHeader:
prefix: APIKEY
response:
success:
filters:
"identity":
json:
properties:
"userid":
selector: auth.identity.metadata.annotations.secret\.kuadrant\.io/user-id
EOF The operator crash loops panicking but is fine once I remove the The panic:
|
Thanks @adam-cattermole! Silly one this bug. Fixed. |
@guicassolato Is there anything from the guides that should not be working currently from this change? 🤔 I know you've mentioned before that API Key
gives a 403:
|
479d49b
to
3401dbc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets merge move on.
@KevFan, I've added another commit updating the doc. TL;DR – What you experienced when trying the user guide is not an issue of this PR. Rather, it's a combination of 2 things:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to see this merge
Signed-off-by: Guilherme Cassolato <[email protected]>
…teway extension resources Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
… merges Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
Signed-off-by: Guilherme Cassolato <[email protected]>
…h Kuadrant AuthPolicy Signed-off-by: Guilherme Cassolato <[email protected]>
7766d57
to
6138e34
Compare
* Prepare AuthPolicy type for the merge strategy * Structure of named patterns changed from `patterns: map[string][]PatternExpression` to `patterns: map[string]{allOf: []PatternExpression}`. * `spec.response.success.dynamicMetadata` field renamed `spec.response.success.filters`, documented as meant for exporting data to other filters managed by Kuadrant only. Signed-off-by: Guilherme Cassolato <[email protected]> * sotw: auth * AuthPolicies validation * Effective auth policies * Authorino AuthConfigs * Istio/Envoy Gateway cluster patches * Istio/Envoy Gateway wasm extensions * (Most part of) AuthPolicy status update Signed-off-by: Guilherme Cassolato <[email protected]> * activate auth service in the wasm config Signed-off-by: Guilherme Cassolato <[email protected]> * check status of the authconfigs for the authpolicy enforced status condition + refactoring of the ratelimitpolicy staus updater for consistency with auth Signed-off-by: Guilherme Cassolato <[email protected]> * tests: fix unit tests pkg/wasm Signed-off-by: Guilherme Cassolato <[email protected]> * bump policy-machinery to v0.6.2 Signed-off-by: Guilherme Cassolato <[email protected]> * bump policy-machinery to v0.6.3 Signed-off-by: Guilherme Cassolato <[email protected]> * add effective authpolicy count to debug log messages when building gateway extension resources Signed-off-by: Guilherme Cassolato <[email protected]> * fix: equality between envoy gateway extension resources Signed-off-by: Guilherme Cassolato <[email protected]> * De/restructure all objects via JSON Signed-off-by: Guilherme Cassolato <[email protected]> * Remove unused funcs from the reconciliation of AuthConfigs Signed-off-by: Guilherme Cassolato <[email protected]> * fix: equality between envoy gateway cluster patch resources Signed-off-by: Guilherme Cassolato <[email protected]> * bump policy-machinery to v0.6.4 Signed-off-by: Guilherme Cassolato <[email protected]> * remove unnecessary custom json unmarshallers from poliyc types Signed-off-by: Guilherme Cassolato <[email protected]> * tests: activate auth service in the wasm config Signed-off-by: Guilherme Cassolato <[email protected]> * fix: build envoy auth cluster patch with correct name Signed-off-by: Guilherme Cassolato <[email protected]> * fix: cel validations of the authpolicy Signed-off-by: Guilherme Cassolato <[email protected]> * tests: fix authpolicy integration tests Signed-off-by: Guilherme Cassolato <[email protected]> * fix: mark empty authpolicies as enforced Signed-off-by: Guilherme Cassolato <[email protected]> * disable prealloc linter Signed-off-by: Guilherme Cassolato <[email protected]> * refactor: improved tracking of the origin of a policy rule throughout merges Signed-off-by: Guilherme Cassolato <[email protected]> * fix log message Signed-off-by: Guilherme Cassolato <[email protected]> * fix nil custom response unauthenticated/unauthorized configs Signed-off-by: Guilherme Cassolato <[email protected]> * preallocate the modifiedAuthConfigs slice Signed-off-by: Guilherme Cassolato <[email protected]> * docs: updated user guide Enforcing authentication & authorization with Kuadrant AuthPolicy Signed-off-by: Guilherme Cassolato <[email protected]> --------- Signed-off-by: Guilherme Cassolato <[email protected]>
State-of-the-world reconciler – Auth workflow
spec.targetRef.sectionName
spec.(defaults|overrides).strategy
spec.rules.response.success.dynamicMetadata
→spec.rules.response.success.filters
TODOs
Enforced
status conditionDry-run desired AuthConfig to ensure defaults are applied–– probably in a separate PR, so a more holistic approach can be employed, covering all objects that are updated (not only AuthConfigs)Closes #820
Closes #825
Closes #822
Verification steps
Setup the environment:
Enable Envoy Gateway alongside with Istio:
make envoy-gateway-install # Restart the Kuadrant Operator so it can acknowledge the presence of Envoy Gateway kubectl rollout restart deployment/kuadrant-operator-controller-manager -n kuadrant-system
Configure TLS on the Envoy Gateway-provided gateway:
Deploy an application:
(From now on and at anytime) Send requests to the application:
Deploy Kuadrant:
Create a gateway atomic default RateLimitPolicy:
Create a route RateLimitPolicy:
Modify the gateway RateLimitPolicy to atomic override strategy:
Modify the gateway RateLimitPolicy to merge override strategy:
Create a route AuthPolicy:
Create a gateway merge override AuthPolicy:
Try other use cases not covered in the verification steps.
In general, this PR should enable:
spec.targetRef.sectionName
)spec.targetRef.sectionName
)spec.defaults.strategy: merge
,spec.overrides.strategy: merge
)Enforced
condition of the policiesEnforced
condition of the policiesEnforced
condition of the policiesEnforced
condition of the policiesEnforced
condition of the policiesEnforced
condition of the policies