OIDC xds translation #2191

zhaohuabing · 2023-11-15T17:41:20Z

What this PR does:

translate OIDC IR to XDS

docs and e2e test will be in a follow-up PRs.

OIDC will be done per-route. The final xds output will look like this example yaml file: https://github.com/zhaohuabing/playground/blob/main/envoy/per-route-oauth2-oidc/envoy.yaml

codecov · 2023-11-15T19:08:29Z

Codecov Report

Attention: 156 lines in your changes are missing coverage. Please review.

Comparison is base (94fe41c) 64.19% compared to head (d875f39) 64.16%.

❗ Current head d875f39 differs from pull request most recent head d72ed07. Consider uploading reports for the commit d72ed07 to get more accurate results

Files	Patch %	Lines
internal/xds/translator/oidc.go	68.60%	64 Missing and 28 partials ⚠️
internal/gatewayapi/securitypolicy.go	45.45%	13 Missing and 5 partials ⚠️
internal/xds/translator/jwt_authn.go	84.26%	9 Missing and 5 partials ⚠️
internal/xds/translator/translator.go	36.84%	8 Missing and 4 partials ⚠️
internal/xds/translator/shared_types.go	68.96%	6 Missing and 3 partials ⚠️
internal/xds/translator/httpfilters.go	63.63%	4 Missing and 4 partials ⚠️
internal/xds/translator/route.go	50.00%	1 Missing and 1 partial ⚠️
internal/ir/zz_generated.deepcopy.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2191      +/-   ##
==========================================
- Coverage   64.19%   64.16%   -0.03%     
==========================================
  Files         107      109       +2     
  Lines       14924    15266     +342     
==========================================
+ Hits         9580     9796     +216     
- Misses       4768     4855      +87     
- Partials      576      615      +39

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

internal/xds/translator/httpfilters.go

zhaohuabing · 2023-11-16T03:03:41Z

/retest

zhaohuabing · 2023-11-16T03:54:56Z

/retest

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

…o check the oauth2 tokens Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

… have non-native per-route support Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing · 2023-11-16T23:15:16Z

/retest

arkodg · 2023-11-17T01:34:26Z

internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml

+          initialStreamWindowSize: 65536
+          maxConcurrentStreams: 100
+        httpFilters:
+        - name: envoy.filters.http.oauth2_first-route


do we need to do anything here that disables processing these at the listener level ?

// patchRouteCfgWithPerRouteConfig appends per-route filter configurations to the
// route config.
// This is a generic way to add per-route filter configurations for all filters
// that has none-native per-route configuration support.
// - For the filter type that without native per-route configuration support, EG
// adds a filter for each route in the HCM filter chain.
// - patchRouteCfgWithPerRouteConfig disables all the filters in the
// typedFilterConfig of the route config.
// - PatchRouteWithPerRouteConfig enables the corresponding oauth2 filter for each
// route in the typedFilterConfig of the route.
//
// The filter types that have non-native per-route support: oauth2, basic authn
// Note: The filter types that have native per-route configuration support should
// use their own native per-route configuration.

its not clear from the docs https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-msg-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager

http_filters (repeated [extensions.filters.network.http_connection_manager.v3.HttpFilter](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-msg-extensions-filters-network-http-connection-manager-v3-httpfilter)) A list of individual HTTP filters that make up the filter chain for requests made to the connection manager. [Order matters](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/http/http_filters#arch-overview-http-filters-ordering) as the filters are processed sequentially as request events happen.

so although its part of http_filters, it is disabled at per route level, which is interpreted by envoy ? should we also update envoy hcm http_filters docs to explain this ?

so although its part of http_filters, it is disabled at per route level, which is interpreted by envoy ? should we also update envoy hcm http_filters docs to explain this ?

They're disabled in the filter chain and only enabled at the route level by an explicit configuration in that route's typedFilterConfig.

This generic per-route filter config feature is quite new. The Envoy docs actually don't mention it yet.

A workable example can be found here: https://github.com/zhaohuabing/playground/blob/main/envoy/per-route-oauth2-oidc/envoy.yaml

Related Envoy PR: envoyproxy/envoy#30141

internal/xds/translator/translator.go

arkodg · 2023-11-17T01:36:48Z

internal/xds/translator/tracing.go

@@ -132,7 +132,7 @@ func processClusterForTracing(tCtx *types.ResourceVersionTable, tracing *ir.Trac
 		name:         clusterName,
 		settings:     []*ir.DestinationSetting{ds},
 		tSocket:      nil,
-		endpointType: DefaultEndpointType,
+		endpointType: EndpointTypeDNS,


is this always DNS ?

The previous default one was actually DNS, but the name was unclear, so I changed the name.

cc @zirain who may have more context

thanks, it's more clear now.

internal/xds/translator/translator.go

arkodg

LGTM thanks !

zhaohuabing requested a review from a team as a code owner November 15, 2023 17:41

zhaohuabing marked this pull request as draft November 15, 2023 17:41

zhaohuabing force-pushed the oidc-xds branch 2 times, most recently from b581775 to d8780ea Compare November 15, 2023 19:04

zhaohuabing force-pushed the oidc-xds branch 3 times, most recently from 34612dd to c606a93 Compare November 15, 2023 21:41

arkodg reviewed Nov 16, 2023

View reviewed changes

internal/xds/translator/httpfilters.go Outdated Show resolved Hide resolved

zhaohuabing force-pushed the oidc-xds branch from c606a93 to 3f8f557 Compare November 16, 2023 02:20

zhaohuabing marked this pull request as ready for review November 16, 2023 02:40

zhaohuabing added 4 commits November 16, 2023 09:14

OIDC xds translation

ed32dfd

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

refactor: merge shared codes

9c8839f

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

put oauth2 filter before jwtauthFilter, so we can use jwtauthfilter t…

3917f07

…o check the oauth2 tokens Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

add comments to explain per-route configuration for filter types that…

d72ed07

… have non-native per-route support Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing force-pushed the oidc-xds branch from 3c1be45 to 1afd0b7 Compare November 16, 2023 17:15

zhaohuabing requested a review from arkodg November 16, 2023 17:33

zhaohuabing force-pushed the oidc-xds branch from d875f39 to d72ed07 Compare November 16, 2023 22:51

arkodg reviewed Nov 17, 2023

View reviewed changes

internal/xds/translator/translator.go Show resolved Hide resolved

arkodg reviewed Nov 17, 2023

View reviewed changes

internal/xds/translator/translator.go Show resolved Hide resolved

arkodg reviewed Nov 17, 2023

View reviewed changes

internal/xds/translator/translator.go Show resolved Hide resolved

arkodg approved these changes Nov 17, 2023

View reviewed changes

arkodg requested review from a team and removed request for a team November 17, 2023 16:18

arkodg requested review from kflynn, tmsnan and tanujd11 November 17, 2023 16:18

zirain approved these changes Nov 17, 2023

View reviewed changes

zirain merged commit 524f7e3 into envoyproxy:main Nov 17, 2023
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OIDC xds translation #2191

OIDC xds translation #2191

zhaohuabing commented Nov 15, 2023

codecov bot commented Nov 15, 2023 •

edited

Loading

zhaohuabing commented Nov 16, 2023

zhaohuabing commented Nov 16, 2023

zhaohuabing commented Nov 16, 2023

arkodg Nov 17, 2023

zhaohuabing Nov 17, 2023

arkodg Nov 17, 2023

zhaohuabing Nov 17, 2023 •

edited

Loading

arkodg Nov 17, 2023

zhaohuabing Nov 17, 2023 •

edited

Loading

arkodg Nov 17, 2023

zirain Nov 17, 2023

arkodg left a comment

OIDC xds translation #2191

OIDC xds translation #2191

Conversation

zhaohuabing commented Nov 15, 2023

codecov bot commented Nov 15, 2023 • edited Loading

Codecov Report

zhaohuabing commented Nov 16, 2023

zhaohuabing commented Nov 16, 2023

zhaohuabing commented Nov 16, 2023

arkodg Nov 17, 2023

Choose a reason for hiding this comment

zhaohuabing Nov 17, 2023

Choose a reason for hiding this comment

arkodg Nov 17, 2023

Choose a reason for hiding this comment

zhaohuabing Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

arkodg Nov 17, 2023

Choose a reason for hiding this comment

zhaohuabing Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

arkodg Nov 17, 2023

Choose a reason for hiding this comment

zirain Nov 17, 2023

Choose a reason for hiding this comment

arkodg left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 15, 2023 •

edited

Loading

zhaohuabing Nov 17, 2023 •

edited

Loading

zhaohuabing Nov 17, 2023 •

edited

Loading