chore: Refactor x509Provider to create a shared Utils class for Mtls#1907
Open
vverman wants to merge 11 commits intogoogleapis:mainfrom
Open
chore: Refactor x509Provider to create a shared Utils class for Mtls#1907vverman wants to merge 11 commits intogoogleapis:mainfrom
vverman wants to merge 11 commits intogoogleapis:mainfrom
Conversation
vverman
force-pushed
the
refactor-x509-provider
branch
from
1500bea to
f78ed98
Compare
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
| * | ||
| * <p>For internal use only. | ||
| */ | ||
| public class SystemEnvironmentProvider implements EnvironmentProvider, Serializable { |
Member
There was a problem hiding this comment.
qq, what is the difference between SystemEnvironmentProvider and EnvironmentProvider?
Contributor
Author
There was a problem hiding this comment.
An EnvProvider is an interface which can be implemented by test-classes to pass in their own env variables.
The SystemEnvProvider implements this EnvProvider and implements the associated functions such that the env-variables are fetched from the system env.
Member
There was a problem hiding this comment.
Got it. Can you add @internalapi to this class and for SystemPropertyProvider?
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
Member
Since this is a refactor, I'm going to change the title to |
lqiu96
changed the title
|
E
El martes, 24 de marzo de 2026, ***@***.***> escribió:
… ***@***.**** commented on this pull request.
------------------------------
In oauth2_http/java/com/google/auth/mtls/X509Provider.java
<#1907 (comment)>
:
> + public X509Provider(
+ EnvironmentProvider envProvider,
+ PropertyProvider propProvider,
+ String certConfigPathOverride) {
Thanks for the suggestion.
I think we can leave this one out for now because I don't think the
argument list is big enough to justify the Builder's inclusion and we don't
anticipate any changes to X509Provider (in the short-term).
In case this changes, we can implement it then.
—
Reply to this email directly, view it on GitHub
<#1907?email_source=notifications&email_token=B5MKPO3RH26N37TDJ35VCOL4SL3TJA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMBQGIZTKMBWGUYKM4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#discussion_r2984325705>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B5MKPOZWHPYIKAJ5GNPMM5L4SL3TJAVCNFSM6AAAAACW43RKUSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DAMBSGM2TANRVGA>
.
You are receiving this because you are subscribed to this thread.Message
ID: <googleapis/google-auth-library-java/pull/1907/review/4002350650@
github.com>
|
lqiu96
reviewed
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
lqiu96
approved these changes
Mar 24, 2026
lqiu96
reviewed
Mar 24, 2026
nbayati
requested changes
Mar 26, 2026
nbayati
requested changes
Mar 31, 2026
oauth2_http/javatests/com/google/auth/oauth2/TestPropertyProvider.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/SystemPropertyProvider.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java
Outdated
Show resolved
Hide resolved
| private static String getExplicitCertConfigPath(IdentityPoolCredentialSource credentialSource) { | ||
| IdentityPoolCredentialSource.CertificateConfig certConfig = | ||
| credentialSource.getCertificateConfig(); | ||
| return certConfig.useDefaultCertificateConfig() |
Contributor
There was a problem hiding this comment.
Potential null pointer access: The variable certConfig may be null at this location
Contributor
Author
There was a problem hiding this comment.
Thanks for the catch! I added a null pointer check!
product-auto-label
bot
added
size: xl
…ormed cert tests to X509Provider.
vverman
force-pushed
the
refactor-x509-provider
branch
from
b2ff241 to
e8fbea7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes googleapis/google-cloud-java#12582
Addressed a concern raised by Andy refer.
Now X509Provider only exposes the necessary methods needed by the MtlsProvider interface