-
Notifications
You must be signed in to change notification settings - Fork 360
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: authorization implementation #3399
Conversation
53aa011
to
9224aea
Compare
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
9224aea
to
d53caf5
Compare
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
01f8408
to
ac682d9
Compare
ac682d9
to
f657664
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3399 +/- ##
==========================================
+ Coverage 67.25% 67.37% +0.11%
==========================================
Files 166 167 +1
Lines 19722 19914 +192
==========================================
+ Hits 13265 13418 +153
- Misses 5503 5522 +19
- Partials 954 974 +20 ☔ View full report in Codecov by Sentry. |
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
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.
LGTM thanks !
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Good point. I added a regex validation for CIDR. It should prevent most invalid input such as lacking of mask, but still need a second check in gateway API translation for some corner cases.
I actually proposed the same during the API review. :-) Most gateway implementations have a standalone IP Allow/Deny list. However, some, like the Istio Authorization API, integrate this feature into their Authorization API. Incorporating the client IP range in the Authorization enables use cases that require a combination of the client IP and other conditions. For instance, " allows admin users access an HTTPRoute from use-west-region, but not from other regions", which is either impossible or significantly challenging to implement with two independent filters. |
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
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.
LGTM, please fix the conflict.
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
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.
one nit, otherwise lgtm
|
||
// CIDR defines a CIDR Address range. | ||
// A CIDR can be an IPv4 address range such as "192.168.1.0/24" or an IPv6 address range such as "2001:0db8:11a3:09d7::/64". | ||
// +kubebuilder:validation:Pattern=`((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\/([0-9]+))|((([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))\/([0-9]+))` |
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.
perhaps prettier would be use golang in https://github.com/envoyproxy/gateway/blob/main/api/v1alpha1/validation/securitypolicy_validate.go to check that its valid?
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.
It's two-level validation, first level is k8s API server, the second level is in gateway API translation in parseCIDR
method.
This PR adds gateway API translator, XDS translator, and e2e tests for the
Authorization
setting of theSecurityPolicy
.Only source IP is supported for now. Other principals such as JWT claims, basic auth, etc. will be supported later.
Fix: #2462
Related: #2250
Example config:
Test results: