-
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
Disallow empty AuthPolicies #1034
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1034 +/- ##
==========================================
+ Coverage 76.15% 83.77% +7.62%
==========================================
Files 111 81 -30
Lines 8986 6694 -2292
==========================================
- Hits 6843 5608 -1235
+ Misses 1852 868 -984
+ Partials 291 218 -73
Flags with carried forward coverage won't be shown. Click here to find out more.
|
50bb948
to
8f19232
Compare
@guicassolato what is the priority of this in your mind? Is it a must for v1? |
2784176
to
821e9cd
Compare
I think I replied about this on Slack, but for the record... This is mainly for consistency with the RateLimitPolicy. Until then, an AuthPolicy without any rules is reported as For Kuadrant, we want policies to be more explicit and contain at least one rule. |
65b39dd
to
9e0430b
Compare
Signed-off-by: Guilherme Cassolato <[email protected]>
9e0430b
to
3c86f02
Compare
Verified. Changes look good. Shame the validation has to be so complex takes a while when reading it but it is what we have to work with. /lgtm |
Verification steps
Must all return:
The AuthPolicy "gateway-auth" is invalid: spec: Invalid value: "object": At least one spec.rules must be defined
Must return:
The AuthPolicy "gateway-auth" is invalid: spec: Invalid value: "object": At least one spec.defaults.rules must be defined
Must return:
The AuthPolicy "gateway-auth" is invalid: spec: Invalid value: "object": At least one spec.overrides.rules must be defined
Must all succeed: