feat: ComputeEngineCredential supports specifying scopes #457
Conversation
google-cla
bot
added
the
cla: yes
|
@chingor13 This is the patch we discussed. |
elharo
left a comment
elharo
left a comment
There was a problem hiding this comment.
It really helps if changes like this are associated with a detailed issue.
| */ | ||
| public static ComputeEngineCredentials create() { | ||
| return new ComputeEngineCredentials(null); | ||
| return new ComputeEngineCredentials(ImmutableSet.<String>of(), null); |
There was a problem hiding this comment.
Should this default to null instead?
An empty collection of scopes might make the url something like https://some-url/?scopes= which may be correct if you ask for no scopes, but if you pass null, it should exclude the scopes param.
There was a problem hiding this comment.
Yes, you're right. I originally wrote this so that an empty collection would result in not sending the ?scopes param, but then changed it to null.
chingor13
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
The existing behavior of ComputeEngineCredentials is to not provide the `?scopes` parameter in its access token request to the metadata server. This is workable on GCE, which has the concept of a scope lock / default scopes applied to the VM. However, neither new AppEngine nor GKE Workload Identity have this concept; they just return a hardcoded set of default scopes if no specific scopes are requested. This makes usage of some Google APIs impossible using the Java client libraries. This change adds scope support to ComputeEngineCredentials. It's optional, so the current behavior will stay the default. However, if the user specifies scopes on the builder or using `createScoped`, then those scopes will be passed to the metadata server.
chingor13
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
chingor13
left a comment
chingor13
left a comment
There was a problem hiding this comment.
Also please run mvn com.coveo:fmt-maven-plugin:format to appease the linter.
| HttpResponse response = getMetadataResponse(getTokenServerEncodedUrl()); | ||
| GenericUrl url = new GenericUrl(getTokenServerEncodedUrl()); | ||
| if (scopes != null) { | ||
| url.set("scopes", String.join(",", scopes)); |
There was a problem hiding this comment.
Looks like String.join was added in Java 8 and this library still currently support Java 7.
There was a problem hiding this comment.
You can use Guava's Joiner in this class.
| HttpResponse response = getMetadataResponse(getTokenServerEncodedUrl()); | ||
| GenericUrl url = new GenericUrl(getTokenServerEncodedUrl()); | ||
| if (scopes != null) { | ||
| url.set("scopes", String.join(",", scopes)); |
There was a problem hiding this comment.
You can use Guava's Joiner in this class.
| } | ||
|
|
||
| public Builder setScopes(Collection<String> scopes) { | ||
| this.scopes = scopes; |
There was a problem hiding this comment.
You probably want a defensive copy to avoid exposing mutable internal state.
| return transportFactory; | ||
| } | ||
|
|
||
| public Collection<String> getScopes() { |
There was a problem hiding this comment.
You probably want a defensive copy to avoid exposing mutable internal state.
| GenericUrl parsedURL = new GenericUrl(url); | ||
|
|
||
| if ("/computeMetadata/v1/instance/service-accounts/default/token".equals(parsedURL.getRawPath())) { | ||
| String scopes = (String)parsedURL.getFirst("scopes"); |
|
Addressed in #514 |
4 participants
The existing behavior of ComputeEngineCredentials is to not provide the
?scopesparameter in its access token request to the metadata server. This is workable on GCE, which has the concept of a scope lock / default scopes applied to the VM. However, neither new AppEngine nor GKE Workload Identity have this concept; they just return a hardcoded set of default scopes if nospecific scopes are requested. This makes usage of some Google APIs impossible using the Java client libraries.
This change adds scope support to ComputeEngineCredentials. It's optional, so the current behavior will stay the default.
However, if the user specifies scopes on the builder or using
createScoped, then those scopes will be passed to the metadata server.