-
Notifications
You must be signed in to change notification settings - Fork 229
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
feat: Support transport and binding-enforcement MDS parameters. #1558
base: main
Are you sure you want to change the base?
Changes from 8 commits
91dba11
ba11e8a
47ec152
9d46bbd
3cb6e98
cb70556
4d0440b
c96df22
a625889
3d46963
f36a19c
8a2d967
ce74e45
8ce3a4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -109,6 +109,16 @@ public class ComputeEngineCredentials extends GoogleCredentials | |
static final int MAX_COMPUTE_PING_TRIES = 3; | ||
static final int COMPUTE_PING_CONNECTION_TIMEOUT_MS = 500; | ||
|
||
public enum Transport { | ||
ALTS, | ||
MTLS | ||
} | ||
|
||
public enum BindingEnforcement { | ||
ON, | ||
IAMPOLICY | ||
} | ||
|
||
private static final String METADATA_FLAVOR = "Metadata-Flavor"; | ||
private static final String GOOGLE = "Google"; | ||
private static final String WINDOWS = "windows"; | ||
|
@@ -122,6 +132,9 @@ public class ComputeEngineCredentials extends GoogleCredentials | |
|
||
private final Collection<String> scopes; | ||
|
||
private final Transport transport; | ||
private final BindingEnforcement bindingEnforcement; | ||
|
||
private transient HttpTransportFactory transportFactory; | ||
private transient String serviceAccountEmail; | ||
|
||
|
@@ -152,6 +165,8 @@ private ComputeEngineCredentials(ComputeEngineCredentials.Builder builder) { | |
scopeList.removeAll(Arrays.asList("", null)); | ||
this.scopes = ImmutableSet.<String>copyOf(scopeList); | ||
} | ||
this.transport = builder.getTransport(); | ||
this.bindingEnforcement = builder.getBindingEnforcement(); | ||
} | ||
|
||
@Override | ||
|
@@ -191,7 +206,10 @@ public final Collection<String> getScopes() { | |
} | ||
|
||
/** | ||
* If scopes is specified, add "?scopes=comma-separated-list-of-scopes" to the token url. | ||
* If scopes is specified, add "?scopes=comma-separated-list-of-scopes" to the token url. If | ||
* transport is specified, add "?transport=xyz" to the token url; xyz is one of "alts" or "mtls". | ||
* If bindingEnforcement is specified, add "?binding-enforcement=xyz" to the token url; xyz is one | ||
* of "iam-policy" or "on". | ||
lqiu96 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
* @return token url with the given scopes | ||
*/ | ||
|
@@ -200,6 +218,16 @@ String createTokenUrlWithScopes() { | |
if (!scopes.isEmpty()) { | ||
tokenUrl.set("scopes", Joiner.on(',').join(scopes)); | ||
} | ||
if (transport == Transport.MTLS) { | ||
tokenUrl.set("transport", "mtls"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fields can be added to the Enum class, so that each Enum can have its own label, then you can just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing this out! Done in latest commit. |
||
} else if (transport == Transport.ALTS) { | ||
tokenUrl.set("transport", "alts"); | ||
} | ||
if (bindingEnforcement == BindingEnforcement.ON) { | ||
tokenUrl.set("binding-enforcement", "on"); | ||
} else if (bindingEnforcement == BindingEnforcement.IAMPOLICY) { | ||
tokenUrl.set("binding-enforcement", "iam-policy"); | ||
} | ||
return tokenUrl.toString(); | ||
} | ||
|
||
|
@@ -643,6 +671,9 @@ public static class Builder extends GoogleCredentials.Builder { | |
private Collection<String> scopes; | ||
private Collection<String> defaultScopes; | ||
|
||
private Transport transport; | ||
private BindingEnforcement bindingEnforcement; | ||
|
||
protected Builder() { | ||
setRefreshMargin(COMPUTE_REFRESH_MARGIN); | ||
setExpirationMargin(COMPUTE_EXPIRATION_MARGIN); | ||
|
@@ -684,6 +715,18 @@ public Builder setQuotaProjectId(String quotaProjectId) { | |
return this; | ||
} | ||
|
||
@CanIgnoreReturnValue | ||
public Builder setTransport(Transport transport) { | ||
this.transport = transport; | ||
return this; | ||
} | ||
|
||
@CanIgnoreReturnValue | ||
public Builder setBindingEnforcement(BindingEnforcement bindingEnforcement) { | ||
this.bindingEnforcement = bindingEnforcement; | ||
return this; | ||
} | ||
|
||
public HttpTransportFactory getHttpTransportFactory() { | ||
return transportFactory; | ||
} | ||
|
@@ -696,6 +739,14 @@ public Collection<String> getDefaultScopes() { | |
return defaultScopes; | ||
} | ||
|
||
public Transport getTransport() { | ||
return transport; | ||
} | ||
|
||
public BindingEnforcement getBindingEnforcement() { | ||
return bindingEnforcement; | ||
} | ||
|
||
@Override | ||
public ComputeEngineCredentials build() { | ||
return new ComputeEngineCredentials(this); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,6 +190,98 @@ public void buildTokenUrlWithScopes_defaultScopes() { | |
assertEquals("bar", scopes.toArray()[1]); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrl_nullTransport() { | ||
Comment on lines
+193
to
+194
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: one additional test where both transport and bindingenforcements are null There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in ce74e45 |
||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setTransport(null) | ||
.setBindingEnforcement(ComputeEngineCredentials.BindingEnforcement.ON) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?binding-enforcement=on", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrl_nullBindingEnforcement() { | ||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setTransport(ComputeEngineCredentials.Transport.MTLS) | ||
.setBindingEnforcement(null) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?transport=mtls", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrlSoftMtlsBound_mtls_transport() { | ||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setTransport(ComputeEngineCredentials.Transport.MTLS) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?transport=mtls", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrlSoftMtlsBound_iam_enforcement() { | ||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setBindingEnforcement(ComputeEngineCredentials.BindingEnforcement.IAMPOLICY) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?binding-enforcement=iam-policy", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrlSoftMtlsBound_mtls_transport_iam_enforcement() { | ||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setTransport(ComputeEngineCredentials.Transport.MTLS) | ||
.setBindingEnforcement(ComputeEngineCredentials.BindingEnforcement.IAMPOLICY) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?transport=mtls&binding-enforcement=iam-policy", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrlHardMtlsBound_always_enforced() { | ||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setBindingEnforcement(ComputeEngineCredentials.BindingEnforcement.ON) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?binding-enforcement=on", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrlHardMtlsBound_mtls_transport_always_enforced() { | ||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setTransport(ComputeEngineCredentials.Transport.MTLS) | ||
.setBindingEnforcement(ComputeEngineCredentials.BindingEnforcement.ON) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?transport=mtls&binding-enforcement=on", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildTokenUrlHardDirectPathBound_alts_transport() { | ||
ComputeEngineCredentials credentials = | ||
ComputeEngineCredentials.newBuilder() | ||
.setTransport(ComputeEngineCredentials.Transport.ALTS) | ||
.build(); | ||
String softBoundTokenUrl = credentials.createTokenUrlWithScopes(); | ||
|
||
assertEquals(TOKEN_URL + "?transport=alts", softBoundTokenUrl); | ||
} | ||
|
||
@Test | ||
public void buildScoped_scopesPresent() throws IOException { | ||
ComputeEngineCredentials credentials = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the it is called
transport
in the token url, but since Transport in GAPIC has different meaning (the value of it is either gRPC or httpJson), can we name this differently? MaybeAuthTransport
? Javadocs for what ALTS and MTLS mean would also be appreciated.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to AuthTransport in latest commit and added some javadocs. Thanks!