-
Notifications
You must be signed in to change notification settings - Fork 514
feat: support route rule in SecurityPolicy target #6335
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
base: main
Are you sure you want to change the base?
feat: support route rule in SecurityPolicy target #6335
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6335 +/- ##
==========================================
+ Coverage 71.12% 71.17% +0.05%
==========================================
Files 220 220
Lines 37971 38056 +85
==========================================
+ Hits 27005 27086 +81
- Misses 9393 9396 +3
- Partials 1573 1574 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
76d8ff7
to
e48670a
Compare
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Hi @kkk777-7 thanks for working on this - this PR looks great! Could you please also add an e2e test to verify the route rule level SecurityPolicy? (you could update an existing SP test, such as basic auth or API key auth, to set the SP at route rule level) |
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
/retest |
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
@zhaohuabing |
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
// 1. First translate Policies targeting xRoutes | ||
// 2. Then translate Policies targeting Listeners | ||
// 3. Finally, the policies targeting Gateways | ||
// 1. First translate Policies targeting RouteRules |
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.
if we dont support 4 levels of replacement for other policies like BTP and EEP, can we raise a GH issue to track that work ?
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.
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
thanks @kkk777-7, would be great if we can have 1 testdata showing 4 level replace |
What this PR does / why we need it:
It allows to target only a specific route rule in a Security Policy.
According to the Gateway API specification: https://gateway-api.sigs.k8s.io/geps/gep-2648/#section-names
overriding/merging strategy
overrides are applied in the following order of precedence:
Route Rule > xRoute > Listener > Gateway
(Due to the third rule of the Gateway API specification above, the combination of Route Rule/xRoute and Listener/Gateway can also be interpreted as a merge.)
Related Issue
#4085
remain issue open as all policies have not been addressed.