RFC: Make DefaultCredentialsProvider public.

[tjgq]

Motivation: Bazel uses GoogleCredentials.getApplicationDefault() to locate and load Application Default Credentials. Because this method calls through to a DefaultCredentialsProvider singleton instance which caches the credentials, they can never be reloaded; the only way to force a reload is to restart the Bazel server (see bazelbuild/bazel#23368). With this change, Bazel could instantiate a fresh DefaultCredentialsProvider and call getDefaultCredentials() on it directly.

I'd also accept any alternative that has the effect of forcing a reload (e.g., a GoogleCredentials.getApplicationDefaultUncached() method; or a reload argument to the existing method; or a cache reset method). All I want is to avoid duplicating the DefaultCredentialsProvider logic in Bazel.

[lqiu96]

DefaultCredentialsProvider singleton instance which caches the credentials, they can never be reloaded

@tjgq Can you provide a bit more information about this regarding how Bazel uses this? From the logs on the other ticket, it seems like this is trying to call ADC multiple times (or a testing workflow that calls it multiple times?).

Does the underlying credential change or do you need to just get a new access token?

Happy to have a discussion here, but we are slowing migrating over to https://github.com/googleapis/google-cloud-java. Future PRs and Issues should be raised against the monorepo. Thanks!

[tjgq]

DefaultCredentialsProvider singleton instance which caches the credentials, they can never be reloaded

@tjgq Can you provide a bit more information about this regarding how Bazel uses this? From the logs on the other ticket, it seems like this is trying to call ADC multiple times (or a testing workflow that calls it multiple times?).

Does the underlying credential change or do you need to just get a new access token?

I'm not sure, but I think it's the former; this is about the contents of the file pointed to by the GOOGLE_APPLICATION_CREDENTIALS environment variable (or the well-known location in the user's home directory, if the environment variable isn't set).

The main use case is the credentials expiring or getting revoked in between builds. However, looking at what DefaultCredentialsProvider does internally, I suppose the issue would also occur if one were to run a subsequent build with a different value for the GOOGLE_APPLICATION_CREDENTIALS environment variable.

(In case it's not clear, note that Bazel keeps a persistent server process running in the background between build commands; that's where the state is preserved. One can force a server restart to work around this problem, but that destroys build incrementality, and is a poor/confusing user experience.)

Happy to have a discussion here, but we are slowing migrating over to https://github.com/googleapis/google-cloud-java. Future PRs and Issues should be raised against the monorepo. Thanks!

Sure, I'm happy to recreate the PR there once we have agreement on what to do (but let's please continue the discussion on this thread).

[lqiu96]

The main use case is the credentials expiring or getting revoked in between builds.

If the credential is expiring, then the auth library has a method to refresh the access token (

google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/OAuth2Credentials.java

Line 205 in 77c85a9

public void refreshIfExpired() throws IOException {

). I'm not familiar enough with Bazel to know why the credential should to be revoked in between builds.

I see from the other ticket that there is the call to gcloud auth application-default revoke. This looks like it deletes the existing credential file that ADC is connected to. My understanding of the original ticket is that it looks like want to refresh for a new token. If this is correct, then I think using the existing refresh() or refreshIfExpired() calls would be better and maybe that would be something that can be implemented within bazel itself to force a refresh.

I suppose the issue would also occur if one were to run a subsequent build with a different value for the GOOGLE_APPLICATION_CREDENTIALS environment variable.

I think I'll need more information about this use case. Our client libraries don't support this behavior and we've never really considered this use case (e.g. swapping the ADC value without a restart).

note that Bazel keeps a persistent server process running in the background between build commands

I think I understand a use case where there may be an expiring cred (e.g. SA cred is valid for ~3 months and needs to be rotated). Even for this use case, I think it would fine to do a restart as it's not resetting every build.

[tjgq]

I see from the other ticket that there is the call to gcloud auth application-default revoke. This looks like it deletes the existing credential file that ADC is connected to. My understanding of the original ticket is that it looks like want to refresh for a new token. If this is correct, then I think using the existing refresh() or refreshIfExpired() calls would be better and maybe that would be something that can be implemented within bazel itself to force a refresh.

We already wrap all of our calls in a retry-after-refresh (see https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/remote/util/Utils.java;l=482-496) so I'm pretty sure that this is about a change to the credential file, not just an expired token. (Tne stack traces in the Bazel issue also corroborate this.)

I think the explicit gcloud auth application-default revoke is just being used in the Bazel issue to simulate what happens if the credentials expire and you're required to run gcloud auth application-default login again; you may assume that explicitly calling revoke is not a realistic scenario.

I think I understand a use case where there may be an expiring cred (e.g. SA cred is valid for ~3 months and needs to be rotated).

Yes, you may assume that this is is the only use case I care about; the other scenarios I described are of marginal importance.

Even for this use case, I think it would fine to do a restart as it's not resetting every build.

It might be infrequent, but when it does happen, it's still annoying and confusing. I'd rather not expose my users to it.

I'm curious - why do you feel it's important for the auth library to impose a requirement that the only way to reload the credentials from disk is to restart the process? Is there a subtle security implication that I'm missing here? Outside of possible security concerns, I'd argue that persistent un-resettable global state is just a bad implementation choice for a general-purpose library.