Skip to content

feat: Add Credential Information to GoogleCredential classes#1791

Merged
lqiu96 merged 22 commits intomainfrom
actionable-errors
Aug 29, 2025
Merged

feat: Add Credential Information to GoogleCredential classes#1791
lqiu96 merged 22 commits intomainfrom
actionable-errors

Conversation

@lqiu96
Copy link
Copy Markdown
Member

@lqiu96 lqiu96 commented Aug 11, 2025 •
edited
Loading

See b/360939866 for more info.

Returns a mapping of how the Credential classes were initialized:

  • Source: Where the initialization information came from (Well known file, Env var)
  • Name: User-friendly Credential class name
  • Principal: Credential identity

@product-auto-label product-auto-label Bot added the size: l Pull request size is large. label Aug 11, 2025
@lqiu96 lqiu96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2025
@lqiu96 lqiu96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2025
@lqiu96 lqiu96 requested a review from zhumin8 August 18, 2025 16:40
@lqiu96 lqiu96 marked this pull request as ready for review August 18, 2025 16:40
@lqiu96 lqiu96 requested review from a team August 18, 2025 16:40
zhumin8
zhumin8 reviewed Aug 27, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@zhumin8 zhumin8 left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few nit comments.
A question on UserCredentials: the design says to use account from credential json if present. I don't see relevant logic in user cred. What is the reason for this?

Comment thread oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java Outdated
Comment thread oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Comment thread oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java Outdated
Comment thread oauth2_http/java/com/google/auth/oauth2/DefaultCredentialsProvider.java Outdated
Comment thread oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java Outdated
Comment thread oauth2_http/java/com/google/auth/oauth2/ServiceAccountJwtAccessCredentials.java Outdated
@lqiu96 lqiu96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 27, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 27, 2025
@lqiu96
Copy link
Copy Markdown
Member Author

lqiu96 commented Aug 27, 2025

LGTM. Just a few nit comments. A question on UserCredentials: the design says to use account from credential json if present. I don't see relevant logic in user cred. What is the reason for this?

I missed that part. The account logic should be added now.

zhumin8
zhumin8 reviewed Aug 28, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@zhumin8 zhumin8 left a comment

Choose a reason for hiding this comment

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

A few nits, non-blocking

Comment thread oauth2_http/java/com/google/auth/oauth2/DefaultCredentialsProvider.java Outdated
Comment thread oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java
Comment thread oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java Outdated
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Aug 28, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

zhumin8
zhumin8 approved these changes Aug 29, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@zhumin8 zhumin8 left a comment

Choose a reason for hiding this comment

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

LGTM

@lqiu96 lqiu96 merged commit 5511913 into main Aug 29, 2025
25 of 26 checks passed
@lqiu96 lqiu96 deleted the actionable-errors branch August 29, 2025 15:17
@release-please release-please Bot mentioned this pull request Aug 29, 2025
Merged
@lqiu96 lqiu96 mentioned this pull request Nov 6, 2025
Merged
@release-please release-please Bot mentioned this pull request Dec 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhumin8 zhumin8 zhumin8 approved these changes

Assignees

No one assigned

Labels

size: l Pull request size is large.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lqiu96 @zhumin8 @yoshi-kokoro