A83: xDS GCP Authentication Filter by markdroth · Pull Request #438 · grpc/proposal

markdroth · 2024-05-21T16:45:15Z

No description provided.

A83-xds-gcp-authn-filter.md

This is a step toward gRFC A83 (grpc/proposal#438). Closes #37468 COPYBARA_INTEGRATE_REVIEW=#37468 from markdroth:cds_metadata 2d17c46 PiperOrigin-RevId: 664910293

markdroth · 2024-08-21T17:27:35Z

I made one more small change here related to how we plumb the CDS data into the filters. I think that can just reuse the attribute added in A74 (which was already a prereq for this design) rather than adding a new attribute.

#37510) Previously, `grpc_oauth2_token_fetcher_credentials` provided functionality for on-demand token-fetching, but it was integrated into the oauth2 code, so it was not possible to use that same code for on-demand fetching of (e.g.) JWT tokens. This PR splits that class into two parts: 1. A base `TokenFetcherCredentials` class that provides a framework for on-demand fetching of any arbitrary type of auth token. 2. An `Oauth2TokenFetcherCredentials` subclass that derives from `TokenFetcherCredentials` and provides handling for oauth2 tokens. The `grpc_compute_engine_token_fetcher_credentials`, `StsTokenFetcherCredentials`, and `grpc_google_refresh_token_credentials` classes that previously derived from `grpc_oauth2_token_fetcher_credentials` now derive from `Oauth2TokenFetcherCredentials` instead, so there's not much change to those classes (other than a cleaner interface with the base class functionality). The `ExternalAccountCredentials` class and its subclasses got more extensive changes here. Previously, this class inheritted from `grpc_oauth2_token_fetcher_credentials` and fooled the base class into thinking that it directly fetched the oauth2 token, when in fact it actually performed a number of steps to gather data and then constructed a synthetic HTTP response to pass back to the base class. I have changed this to instead derive directly from `TokenFetcherCredentials` to provide a much cleaner interface with the parent class. In addition, I have changed `grpc_call_credentials` from `RefCounted<>` to `DualRefCounted<>` to provide a clean way to shut down any in-flight token fetch when the credentials are unreffed. This PR paves the way for subsequent work that will allow implementing an on-demand JWT token fetcher call credential, as part of gRFC A83 (grpc/proposal#438). Closes #37510 COPYBARA_INTEGRATE_REVIEW=#37510 from markdroth:token_fetcher_call_creds_refactor 3bd398a PiperOrigin-RevId: 666547985

This adds functionality that is intended to be used for the new GcpServiceAccountIdentityCallCredentials implementation, as per gRFC A83 (grpc/proposal#438). However, it is also a useful improvement for all token-fetching call credentials types, so I am adding it to the base class. Closes #37531 COPYBARA_INTEGRATE_REVIEW=#37531 from markdroth:token_fetcher_call_creds_prefetch_and_backoff 0fcdb48 PiperOrigin-RevId: 666809903

Add validation of the `Audience` cluster metadata type, as per gRFC A83 (grpc/proposal#438). I had previously changed the metadata to be represented as JSON in grpc#37468. However, while working on the GCP Authentication filter implementation, I realized that that's not an ideal representation, because it would have required us to validate the JSON on a per-RPC basis, which would be bad for performance. So I've changed the representation of metadata to be an abstract type, and we now store the `Audience` metadata as a simple string. I've also moved metadata into its own type with its own validation code, so that in the future we can use it in places other than CDS (many xDS resource types have metadata fields). While I was at it, I also add some helper functions for validating the `UInt32Value` and `UInt64Value` wrapper protos. Closes grpc#37566 PiperOrigin-RevId: 668281729

Final piece of gRFC A83 (grpc/proposal#438): the GCP authentication filter itself. Infrastructure changes include: - Added a general-purpose LRU cache library that can be reused elsewhere. - Fixed the client channel code to use the channel args returned by the resolver for the dynamic filters. This was necessary so that the GCP auth filter could access the `XdsConfig` object, which is passed via a channel arg. - Unlike the other xDS HTTP filters we support, the GCP auth filter does not support config overrides, and its configuration includes a cache size parameter that we always need at the channel level, not per-call. As a result, I had to change the xDS HTTP filter API to give it the ability to set top-level fields in the service config, not just per-method fields. (We use the service config as a way of passing configuration down into xDS HTTP filters.) Note that for now, this works only on the client side, because we don't have machinery for a top-level service config on the server side. - The GCP auth filter is also the first case where the filter needs to know its instance name from the xDS config, so I changed the xDS HTTP filter API to plumb that through. - Fixed a bug in the HTTP client library that prevented the override functions from declining to override a particular request. Closes #37550 COPYBARA_INTEGRATE_REVIEW=#37550 from markdroth:xds_gcp_auth_filter 19eaefb PiperOrigin-RevId: 669371249

…te, and use it for cache in GCP auth filter (#37646) This is the last piece of gRFC A83 (grpc/proposal#438). Note that although this is the first use-case for this "blackboard" mechanism, we will also use it in the future for the xDS rate-limiting filter on the gRPC server side. Closes #37646 COPYBARA_INTEGRATE_REVIEW=#37646 from markdroth:gcp_auth_filter_state 72d0d96 PiperOrigin-RevId: 679707134

ejona86

I'm not approving yet to resolve the "RPC during backoff delay" behavior. The rest looks fine/easy.

A83-xds-gcp-authn-filter.md

dfawley

LGTM

As per discussion in grpc/proposal#438 (comment). Closes #38005 COPYBARA_INTEGRATE_REVIEW=#38005 from markdroth:gcp_auth_cache_size_validation_fix ac8565b PiperOrigin-RevId: 689930399

As per discussion in grpc/proposal#438 (comment). Closes #38004 COPYBARA_INTEGRATE_REVIEW=#38004 from markdroth:token_fetcher_creds_backoff_fix 8de2fec PiperOrigin-RevId: 690678927

As per discussion in grpc/proposal#438 (comment). Closes grpc#38005 COPYBARA_INTEGRATE_REVIEW=grpc#38005 from markdroth:gcp_auth_cache_size_validation_fix ac8565b PiperOrigin-RevId: 689930399

As per discussion in grpc/proposal#438 (comment). Closes grpc#38004 COPYBARA_INTEGRATE_REVIEW=grpc#38004 from markdroth:token_fetcher_creds_backoff_fix 8de2fec PiperOrigin-RevId: 690678927

This is a step toward gRFC A83 (grpc/proposal#438). Closes grpc#37468 COPYBARA_INTEGRATE_REVIEW=grpc#37468 from markdroth:cds_metadata 2d17c46 PiperOrigin-RevId: 664910293

grpc#37510) Previously, `grpc_oauth2_token_fetcher_credentials` provided functionality for on-demand token-fetching, but it was integrated into the oauth2 code, so it was not possible to use that same code for on-demand fetching of (e.g.) JWT tokens. This PR splits that class into two parts: 1. A base `TokenFetcherCredentials` class that provides a framework for on-demand fetching of any arbitrary type of auth token. 2. An `Oauth2TokenFetcherCredentials` subclass that derives from `TokenFetcherCredentials` and provides handling for oauth2 tokens. The `grpc_compute_engine_token_fetcher_credentials`, `StsTokenFetcherCredentials`, and `grpc_google_refresh_token_credentials` classes that previously derived from `grpc_oauth2_token_fetcher_credentials` now derive from `Oauth2TokenFetcherCredentials` instead, so there's not much change to those classes (other than a cleaner interface with the base class functionality). The `ExternalAccountCredentials` class and its subclasses got more extensive changes here. Previously, this class inheritted from `grpc_oauth2_token_fetcher_credentials` and fooled the base class into thinking that it directly fetched the oauth2 token, when in fact it actually performed a number of steps to gather data and then constructed a synthetic HTTP response to pass back to the base class. I have changed this to instead derive directly from `TokenFetcherCredentials` to provide a much cleaner interface with the parent class. In addition, I have changed `grpc_call_credentials` from `RefCounted<>` to `DualRefCounted<>` to provide a clean way to shut down any in-flight token fetch when the credentials are unreffed. This PR paves the way for subsequent work that will allow implementing an on-demand JWT token fetcher call credential, as part of gRFC A83 (grpc/proposal#438). Closes grpc#37510 COPYBARA_INTEGRATE_REVIEW=grpc#37510 from markdroth:token_fetcher_call_creds_refactor 3bd398a PiperOrigin-RevId: 666547985

This adds functionality that is intended to be used for the new GcpServiceAccountIdentityCallCredentials implementation, as per gRFC A83 (grpc/proposal#438). However, it is also a useful improvement for all token-fetching call credentials types, so I am adding it to the base class. Closes grpc#37531 COPYBARA_INTEGRATE_REVIEW=grpc#37531 from markdroth:token_fetcher_call_creds_prefetch_and_backoff 0fcdb48 PiperOrigin-RevId: 666809903

As per gRFC A83 (grpc/proposal#438). For now, I am not exposing this new call creds type via the C-core API or in any C++ or wrapped language public APIs, so there's no way to use it externally. We can easily add that in the future if someone asks, but for now the intent is to use it only internally via the xDS GCP authentication filter, which I'll implement in a subsequent PR. As part of this, I changed the test framework in credentials_test to check the status code in addition to the message on failure. This exposed several places where existing credential types are returnign the wrong status code (unsurprisingly, because of all of the tech debt surrounding grpc_error). I have not fixed this behavior, but I have added TODOs in the test showing which ones I think need to be fixed. Closes grpc#37544 COPYBARA_INTEGRATE_REVIEW=grpc#37544 from markdroth:gcp_service_account_identity_call_creds 97e0efc PiperOrigin-RevId: 666869692

This is needed to use `XdsConfig` in an xDS HTTP filter without causing a circular dependency, which will be needed as part of gRFC A83 (grpc/proposal#438). Closes grpc#37564 COPYBARA_INTEGRATE_REVIEW=grpc#37564 from markdroth:xds_config_dependency_refactor 877d9af PiperOrigin-RevId: 666950471

Add validation of the `Audience` cluster metadata type, as per gRFC A83 (grpc/proposal#438). I had previously changed the metadata to be represented as JSON in grpc#37468. However, while working on the GCP Authentication filter implementation, I realized that that's not an ideal representation, because it would have required us to validate the JSON on a per-RPC basis, which would be bad for performance. So I've changed the representation of metadata to be an abstract type, and we now store the `Audience` metadata as a simple string. I've also moved metadata into its own type with its own validation code, so that in the future we can use it in places other than CDS (many xDS resource types have metadata fields). While I was at it, I also add some helper functions for validating the `UInt32Value` and `UInt64Value` wrapper protos. Closes grpc#37566 PiperOrigin-RevId: 668281729

Final piece of gRFC A83 (grpc/proposal#438): the GCP authentication filter itself. Infrastructure changes include: - Added a general-purpose LRU cache library that can be reused elsewhere. - Fixed the client channel code to use the channel args returned by the resolver for the dynamic filters. This was necessary so that the GCP auth filter could access the `XdsConfig` object, which is passed via a channel arg. - Unlike the other xDS HTTP filters we support, the GCP auth filter does not support config overrides, and its configuration includes a cache size parameter that we always need at the channel level, not per-call. As a result, I had to change the xDS HTTP filter API to give it the ability to set top-level fields in the service config, not just per-method fields. (We use the service config as a way of passing configuration down into xDS HTTP filters.) Note that for now, this works only on the client side, because we don't have machinery for a top-level service config on the server side. - The GCP auth filter is also the first case where the filter needs to know its instance name from the xDS config, so I changed the xDS HTTP filter API to plumb that through. - Fixed a bug in the HTTP client library that prevented the override functions from declining to override a particular request. Closes grpc#37550 COPYBARA_INTEGRATE_REVIEW=grpc#37550 from markdroth:xds_gcp_auth_filter 19eaefb PiperOrigin-RevId: 669371249

…te, and use it for cache in GCP auth filter (grpc#37646) This is the last piece of gRFC A83 (grpc/proposal#438). Note that although this is the first use-case for this "blackboard" mechanism, we will also use it in the future for the xDS rate-limiting filter on the gRPC server side. Closes grpc#37646 COPYBARA_INTEGRATE_REVIEW=grpc#37646 from markdroth:gcp_auth_filter_state 72d0d96 PiperOrigin-RevId: 679707134

As per discussion in grpc/proposal#438 (comment). Closes grpc#38005 COPYBARA_INTEGRATE_REVIEW=grpc#38005 from markdroth:gcp_auth_cache_size_validation_fix ac8565b PiperOrigin-RevId: 689930399

As per discussion in grpc/proposal#438 (comment). Closes grpc#38004 COPYBARA_INTEGRATE_REVIEW=grpc#38004 from markdroth:token_fetcher_creds_backoff_fix 8de2fec PiperOrigin-RevId: 690678927

A83: xDS GCP Authentication Filter

0d58169

markdroth requested review from dfawley and ejona86 May 21, 2024 16:45

markdroth mentioned this pull request Jun 5, 2024

xDS metadata keys should be filter instance names, not filter implementation names envoyproxy/envoy#34251

Open

markdroth added 4 commits June 6, 2024 19:42

add mailing list link

68a57a5

use standalone call credential type

f4d7da4

note that A74 is a prereq

67cfc91

various updates

51e2079

This was referenced Aug 14, 2024

[xDS] generalize CDS metadata handling grpc/grpc#37468

Closed

[call creds] refactor token-fetching call creds to allow cleaner reuse grpc/grpc#37510

Closed

ejona86 reviewed Aug 16, 2024

View reviewed changes

markdroth added 6 commits August 16, 2024 20:03

review comments

0cae3c5

clarify wording

fcf80f6

refresh interval can vary a bit

ff059e2

include both early expiration and pre-emptive refresh

cc436ff

fix status mapping

70069e0

clarify cluster metadata handling

3e90a27

markdroth mentioned this pull request Aug 21, 2024

[call creds] implement GcpServiceAccountIdentityCredentials grpc/grpc#37544

Closed

reuse existing attribute for A74 instead of adding a new one

39c624b

markdroth added 3 commits August 21, 2024 18:32

cache size defaults to 10 and cannot be set to 0

691ecc6

note that the filter does not support config overrides

3d93988

clarify wording on CDS metadata access

87c3030

markdroth mentioned this pull request Aug 21, 2024

[xDS] implement GCP Auth filter grpc/grpc#37550

Closed

cluster metadata map must contain proto type of value

6a71bb6

markdroth mentioned this pull request Aug 23, 2024

[TokenFetcherCredentials] add backoff and pre-fetching grpc/grpc#37531

Closed

markdroth mentioned this pull request Aug 23, 2024

[xDS] move XdsConfig out of XdsDependencyManager grpc/grpc#37564

Closed

markdroth mentioned this pull request Sep 6, 2024

[GCP auth filter] add "blackboard" mechanism for retaining global state, and use it for cache in GCP auth filter grpc/grpc#37646

Closed

more info on cache behavior, and add list of PRs in C-core

042ae6f

ejona86 reviewed Oct 23, 2024

View reviewed changes

markdroth added 3 commits October 25, 2024 00:41

code review comments

67d2da0

fix handling of RPCs while in backoff

f15987d

generalize wording about cache size

829e95b

ejona86 approved these changes Oct 25, 2024

View reviewed changes

This was referenced Oct 25, 2024

[TokenFetcherCredentials] fix backoff behavior grpc/grpc#38004

Closed

[xDS gcp_authn filter] remove upper bound for cache size grpc/grpc#38005

Closed

dfawley approved these changes Oct 25, 2024

View reviewed changes

markdroth merged commit b638bb0 into grpc:master Oct 25, 2024

markdroth deleted the xds_gcp_authn_filter branch October 25, 2024 19:37

shivaspeaks mentioned this pull request Oct 25, 2024

xds: Implement GcpAuthenticationFilter grpc/grpc-java#11638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A83: xDS GCP Authentication Filter#438

A83: xDS GCP Authentication Filter#438
markdroth merged 21 commits intogrpc:masterfrom
markdroth:xds_gcp_authn_filter

markdroth commented May 21, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

markdroth commented Aug 21, 2024

Uh oh!

ejona86 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dfawley left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

markdroth commented May 21, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

markdroth commented Aug 21, 2024

Uh oh!

ejona86 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dfawley left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants