Skip to content
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

Use native per-route config for basic auth #3182

Merged
merged 6 commits into from
Apr 19, 2024

Conversation

zhaohuabing
Copy link
Member

@zhaohuabing zhaohuabing commented Apr 12, 2024

Switching to native per-route config for basic auth to shrink HCM filter chain size.

Per-route config for basic auth has been supported in Envoy.

@zhaohuabing zhaohuabing requested a review from a team as a code owner April 12, 2024 05:48
@zhaohuabing zhaohuabing force-pushed the basic-auth-per-route branch 4 times, most recently from 5fab569 to 0a83056 Compare April 12, 2024 06:03
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing force-pushed the basic-auth-per-route branch from 0d82082 to 71aab5f Compare April 12, 2024 06:30
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Copy link
Contributor

@guydc guydc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big +1 for reducing FC size, one small question on the top-level filter config

basicAuthProto = &basicauthv3.BasicAuth{
Users: &corev3.DataSource{
Specifier: &corev3.DataSource_InlineBytes{
InlineBytes: basicAuth.Users,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using the first route's security policy basic config to define the top-level HCM "global" config? It's overriden on each route, but makes XDS look a bit strange... Maybe just leave it blank?

Copy link
Member Author

@zhaohuabing zhaohuabing Apr 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'd like to leave it blank as well, but the Users field is mandatory.

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing requested a review from guydc April 17, 2024 00:56
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks !

@arkodg arkodg merged commit a428eb7 into envoyproxy:main Apr 19, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants