-
Notifications
You must be signed in to change notification settings - Fork 633
feat: ext auth headers on match #7792
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: ext auth headers on match #7792
Conversation
Signed-off-by: Shreemaan Abhishek <shreemaanabhishek@apache.org>
Signed-off-by: Shreemaan Abhishek <shreemaanabhishek@apache.org>
Signed-off-by: Shreemaan Abhishek <shreemaanabhishek@apache.org>
…-auth-headers-on-match Signed-off-by: Shreemaan Abhishek <shreemaanabhishek@apache.org>
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (43.47%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7792 +/- ##
==========================================
- Coverage 72.83% 72.77% -0.06%
==========================================
Files 236 236
Lines 35190 35228 +38
==========================================
+ Hits 25629 25638 +9
- Misses 7742 7774 +32
+ Partials 1819 1816 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
internal/ir/xds.go
Outdated
| // HeadersToExtAuthOnMatch defines the patterns of the client request headers | ||
| // that will be included in the request to the external authorization service. | ||
| // +optional | ||
| HeadersToExtAuthOnMatch []egv1a1.StringMatch `json:"headersToExtAuthOnMatch,omitempty"` |
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.
Nit: can we use ir.StringMatch here?
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.
those two structs are indeed quite different and using ir.StringMatch by going over the hassle for the data structure conversion does not really serve any real purpose.
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.
Ideally, we should avoid using external APIs directly in the IR layer when possible, since the IR is meant to be an intermediate representation that’s independent of Envoy Gateway and the Gateway API.
That said, this isn’t blocking, as we’re already using some external APIs in the IR today.
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.
Pushed new changes, please review again!
|
/retest |
…ealt/gateway into feat/ext-auth-headers-on-match
…th-headers-on-match Signed-off-by: Shreemaan Abhishek <shreemaanabhishek@apache.org>
Signed-off-by: Shreemaan Abhishek <shreemaanabhishek@apache.org>
…th-headers-on-match
Signed-off-by: Shreemaan Abhishek <shreemaanabhishek@apache.org>
…ealt/gateway into feat/ext-auth-headers-on-match
What type of PR is this?
Enhancement 😎
What this PR does / why we need it:
When using ext auth, it is a common scenario that users might need some specific headers (other then the default) to be sent to the auth service. The envoy go control plane supports matching headers based on
StringMatch, we leverage this feature to assign headers to outgoing ext auth API call based on following matching types:Which issue(s) this PR fixes:
Fixes #7703
Release Notes: Yes/No