Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for reading GOOGLE_APPLICATION_CREDENTIALS as a property #1234

Open
roque86 opened this issue Jun 30, 2023 · 1 comment
Open
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@roque86
Copy link

roque86 commented Jun 30, 2023

In Java, we can have "system properties" as well as "environment variables" that keep our map between keys and string values.
We access system properties using the method System.getProperty while to access the environment variables we use System.getenv.
Different systems may use different approaches to pass configurations. For instance, Amazon Elastibewnsatck passes our variables to Tomcat as properties that will have to be accessed using System.getProperty (see link here).

Currently, DefaultCredentialsProvider reads GOOGLE_APPLICATION_CREDENTIALS only using System.getenv (apart from using getWellKnownCredentialsFile).
It would be nice if it could also try to read the credentials from properties.

I was unable to send a pull request (permission denied), so here goes the draft code I think would easily fix the issue:
New method for DefaultCredentialsProvider:

  String getEnvOrProperty(String name) {
	  String envValue = getEnv(name);
	  if (envValue == null || envValue.length() == 0) {
		  envValue = getProperty(name, "");
	  }		  
	  return envValue;
  }

Change line 135 of DefaultCredentialsProvider#getDefaultCredentialsUnsynchronized from:
String credentialsPath = getEnv(CREDENTIAL_ENV_VAR);
to:
String credentialsPath = getEnvOrProperty(CREDENTIAL_ENV_VAR);

Another approach would be to modify GoogleAuthUtils#getWellKnownCredentialsFile to add a new clause based on getProperty and the default CREDENTIAL_ENV_VAR.

@TimurSadykov
Copy link
Member

Hi Rafael, thanks for suggestion and explaining the use case. It makes sense and we can consider it as a feature request. I will update sometime later if we pick it up.

BTW, you can submit PRs using fork-PR strategy. It does not require any extra access. But for this particular thing I would suggest to wait for an update. This is because ADC logic follow the AIP: https://google.aip.dev/auth/4110 and an update would be a cross-language change.

Also, the are more variables involved in authentication, like quota project, mtls, etc. The change would have to cover all of those variables as well.

@TimurSadykov TimurSadykov self-assigned this Aug 10, 2023
@TimurSadykov TimurSadykov added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants
@TimurSadykov @roque86 and others