Skip to content

Conversation

@harkamaljot
Copy link
Collaborator

@harkamaljot harkamaljot commented Jun 27, 2025

As part of allowing the user to provide their own subject token provider, this PR makes following changes:

Make SubjectTokenProvider trait public.
A new builder ProgrammaticSourcedCredentials is created to utilize this trait and a corresponding test case has been added.

@harkamaljot harkamaljot requested a review from a team as a code owner June 27, 2025 05:41
@harkamaljot harkamaljot marked this pull request as draft June 27, 2025 05:41
@harkamaljot harkamaljot changed the title draft(auth): draft change feat(auth): Add ProgrammaticSourcedCredentials Jun 27, 2025
@codecov
Copy link

codecov bot commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 98.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.47%. Comparing base (b5c298d) to head (e648a7d).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/auth/src/credentials/external_account.rs 97.36% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2541      +/-   ##
==========================================
+ Coverage   95.34%   95.47%   +0.12%     
==========================================
  Files          79       81       +2     
  Lines        3224     3314      +90     
==========================================
+ Hits         3074     3164      +90     
  Misses        150      150              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@harkamaljot harkamaljot reopened this Jun 27, 2025
Comment on lines 466 to 467
/// Sets the required token URL for the STS token exchange.
pub fn with_token_url<S: Into<String>>(mut self, token_url: S) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we can default to sts.googleapis.com. We do not have to force user into providing this. @leosiracusa thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah having a default here is better, I added a default here for now.

@harkamaljot harkamaljot reopened this Jun 27, 2025
@harkamaljot harkamaljot marked this pull request as ready for review June 27, 2025 17:18
@harkamaljot harkamaljot requested a review from sai-sunder-s June 27, 2025 17:18
@harkamaljot harkamaljot requested a review from alvarowolfx June 27, 2025 17:23
@harkamaljot harkamaljot requested a review from sai-sunder-s June 28, 2025 00:20
@harkamaljot harkamaljot merged commit dcbb948 into googleapis:main Jun 30, 2025
23 checks passed
@harkamaljot harkamaljot deleted the custom-2 branch June 30, 2025 18:57
Copy link
Member

@dbolduc dbolduc left a comment

Choose a reason for hiding this comment

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

Also it looks like only the credentials::Builder heeds the GOOGLE_CLOUD_QUOTA_PROJECT env var. Is that env var only for ADC?

@harkamaljot
Copy link
Collaborator Author

GOOGLE_CLOUD_QUOTA_PROJECT

According to the guidance in AIP-4110, this variable is intended to specify a quota project for Application Default Credentials (ADC). I've also reviewed the Java implementation of this, and it confirms that GOOGLE_CLOUD_QUOTA_PROJECT is only used in the ADC workflow.

Currently, our api_key_credentials builder also reads this environment variable. So it shouldn't be reading this env variable value.

@dbolduc
Copy link
Member

dbolduc commented Jul 7, 2025

Ack, thanks for checking

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.

5 participants