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

feat: Support transport and binding-enforcement MDS parameters. #1558

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ public class ComputeEngineCredentials extends GoogleCredentials

private final Collection<String> scopes;

private final String transport;
private final String bindingEnforcement;

private transient HttpTransportFactory transportFactory;
private transient String serviceAccountEmail;

Expand Down Expand Up @@ -152,6 +155,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
Expand Down Expand Up @@ -191,7 +196,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
*/
Expand All @@ -200,6 +208,12 @@ String createTokenUrlWithScopes() {
if (!scopes.isEmpty()) {
tokenUrl.set("scopes", Joiner.on(',').join(scopes));
}
if (!transport.isEmpty()) {
tokenUrl.set("transport", transport);
}
if (!bindingEnforcement.isEmpty()) {
tokenUrl.set("binding-enforcement", bindingEnforcement);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be null and empty check. Setter can set a null value unless we want to add a check there

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out, done in 9d46bbd

return tokenUrl.toString();
}

Expand Down Expand Up @@ -643,6 +657,9 @@ public static class Builder extends GoogleCredentials.Builder {
private Collection<String> scopes;
private Collection<String> defaultScopes;

private String transport = "";
private String bindingEnforcement = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
private String transport = "";
private String bindingEnforcement = "";
private String transport;
private String bindingEnforcement;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 3cb6e98


protected Builder() {
setRefreshMargin(COMPUTE_REFRESH_MARGIN);
setExpirationMargin(COMPUTE_EXPIRATION_MARGIN);
Expand Down Expand Up @@ -684,6 +701,18 @@ public Builder setQuotaProjectId(String quotaProjectId) {
return this;
}

@CanIgnoreReturnValue
public Builder setTransport(String transport) {
this.transport = transport;
return this;
}

@CanIgnoreReturnValue
public Builder setBindingEnforcement(String bindingEnforcement) {
this.bindingEnforcement = bindingEnforcement;
return this;
}

public HttpTransportFactory getHttpTransportFactory() {
return transportFactory;
}
Expand All @@ -696,6 +725,14 @@ public Collection<String> getDefaultScopes() {
return defaultScopes;
}

public String getTransport() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please provide javadoc on these, pointing to publicly documented Metadata Service parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline: We will update public MDS docs when the feature is non-experimental. Since the usage of the S2A + MTLS + bound tokens flow is guarded by an environment variable (EXPERIMENTAL_GOOGLE_API_USE_S2A). Until the public docs are updated, we can link to AIP here and #1400 (AIP already linked in this merged PR).

return transport;
}

public String getBindingEnforcement() {
return bindingEnforcement;
}

@Override
public ComputeEngineCredentials build() {
return new ComputeEngineCredentials(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,66 @@ public void buildTokenUrlWithScopes_defaultScopes() {
assertEquals("bar", scopes.toArray()[1]);
}

@Test
public void buildTokenUrlSoftMtlsBound_mtls_transport() {
ComputeEngineCredentials credentials =
ComputeEngineCredentials.newBuilder().setTransport("mtls").build();
String softBoundTokenUrl = credentials.createTokenUrlWithScopes();

assertEquals(TOKEN_URL + "?transport=mtls", softBoundTokenUrl);
}

@Test
public void buildTokenUrlSoftMtlsBound_iam_enforcement() {
ComputeEngineCredentials credentials =
ComputeEngineCredentials.newBuilder().setBindingEnforcement("iam-policy").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("mtls")
.setBindingEnforcement("iam-policy")
.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("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("mtls")
.setBindingEnforcement("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("alts").build();
String softBoundTokenUrl = credentials.createTokenUrlWithScopes();

assertEquals(TOKEN_URL + "?transport=alts", softBoundTokenUrl);
}

@Test
public void buildScoped_scopesPresent() throws IOException {
ComputeEngineCredentials credentials =
Expand Down