Support GOOGLE_OAUTH_ACCESS_TOKEN for Google Cloud Platform

[marensofier]

We are facing a situation where we want to use short lived access tokens with a minimum of permissions for the kms, and not use static long lived credentials ✨

That is why we really want to add support for the use of gcp access tokens in sops using the environment variable GOOGLE_OAUTH_ACCESS_TOKEN (also used by Pulumi/Terrafrom).

Closes #1188.

[devstein]

@marensofier Thank you for your contribution! This has been a frequently requested feature.

I left a few comments, but can you also please add test coverage?

[devstein]

Related to #1358

[christoffer-eide]

Regarding the name of the env var, I would consider naming it GOOGLE_OAUTH_ACCESS_TOKEN.
This is used by Pulumi and Terraform, and has the same GOOGLE_* prefix as the existing GOOGLE_CREDENTIALS env var.

Also, which of the two should take precedence? In Pulumi it's GOOGLE_OAUTH_ACCESS_TOKEN.

[devstein]

Looks good! Thanks for incorporating the feedback. One last minor change

[felixfontein]

The PR now contains a lot of unnecessary files that should be removed. Also --signoff is missing (for DCO). Can you please fix this? Thanks!

[sabre1041]

LGTM

[sabre1041]

There is a fair amount of overlap with #1794 where there does need to be some assessment between the two enhancements

[felixfontein]

While going through all PRs I noticed that #1188 seems to do the same thing (from the general functionality point of view) as this one. So once this is done I guess we can close #1188. (If that's not correct, please tell me :) )

[felixfontein]

@marensofier can you please resolve the conflicts and address the comments? Thanks.

[felixfontein]

@sabre1041 @matheuscscp can you take another look at the latest version?

[matheuscscp]

I'd also change the name of the PR to Add support for gcp access token from environment variable

[felixfontein]

LGTM as far as I can tell.

[felixfontein]

Hmm, the old behavior was to not fail if no credentials have been provided in the default branch. (Whether that makes sense - no idea. Maybe the GCP SDK does something else in that case to get hold of credentials?) If we want to restore this:

Suggested change

	break
	}
	return nil, fmt.Errorf("credentials: none found: missing %q or %q environment variable", SopsGoogleCredentialsEnv, SopsGoogleCredentialsOAuthTokenEnv)
	}

[matheuscscp]

Maybe the GCP SDK does something else in that case to get hold of credentials?

Yes, it does. All the google libraries default to Application Default Credentials, which is essentially searching for credentials in the environment:

• The GOOGLE_APPLICATION_CREDENTIALS standard environment variable (btw none of the other env vars here are this one)
• A file created in a well-known location by the command gcloud auth application-default login
• The GCE/GKE metadata server

However, getGoogleCredentials() (which should probably be called getGoogleCredentialsFromEnv()), only returns an error if the non-standard GOOGLE_CREDENTIALS environment variable is set, and this variable is only set when the user is declaring a clear intention of using it, so erroring out the entire operation makes sense to me. If the code ends up using an unintended authentication method that is coincidentally available, that's pretty bad in my opinion. People need to organize their configuration well, this new version of the code should help them out on that. Disruptive? Yes, probably, but for a greater good in my opinion.

[felixfontein]

Disruptive? Yes, probably, but for a greater good in my opinion.

It is still a breaking change, I'd prefer not to introduce these in a minor release.

[matheuscscp]

Oh nevermind, for some reason I wrapped my head around the error returned by getGoogleCredentials(), and not this one on the bottom. Yes, this comment makes a lot of sense!

All review comments have been addressed by now.

[felixfontein]

@marensofier @warwick-mitchell1 @matheuscscp and everyone else involved in the PRs, thanks a lot for your contributions, reviews, and comments! I'm glad we finally got this resolved and merged :)