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: Add Universe Domain to Java-Core #2329

Merged
merged 24 commits into from Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion gapic-generator-java-pom-parent/pom.xml
Expand Up @@ -27,7 +27,7 @@
consistent across modules in this repository -->
<javax.annotation-api.version>1.3.2</javax.annotation-api.version>
<grpc.version>1.60.0</grpc.version>
<google.auth.version>1.20.0</google.auth.version>
<google.auth.version>1.21.0</google.auth.version>
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
<google.http-client.version>1.43.3</google.http-client.version>
<gson.version>2.10.1</gson.version>
<guava.version>32.1.3-jre</guava.version>
Expand Down
Expand Up @@ -81,7 +81,6 @@ public abstract class ServiceOptions<
implements Serializable {

public static final String CREDENTIAL_ENV_NAME = "GOOGLE_APPLICATION_CREDENTIALS";

private static final String DEFAULT_HOST = "https://www.googleapis.com";
private static final String LEGACY_PROJECT_ENV_NAME = "GCLOUD_PROJECT";
private static final String PROJECT_ENV_NAME = "GOOGLE_CLOUD_PROJECT";
Expand All @@ -95,6 +94,7 @@ public abstract class ServiceOptions<
protected final String clientLibToken;

private final String projectId;
private final String universeDomain;
private final String host;
private final RetrySettings retrySettings;
private final String serviceRpcFactoryClassName;
Expand Down Expand Up @@ -125,6 +125,7 @@ public abstract static class Builder<
private final ImmutableSet<String> allowedClientLibTokens =
ImmutableSet.of(ServiceOptions.getGoogApiClientLibName());
private String projectId;
private String universeDomain;
private String host;
protected Credentials credentials;
private RetrySettings retrySettings;
Expand All @@ -142,6 +143,7 @@ protected Builder() {}
@InternalApi("This class should only be extended within google-cloud-java")
protected Builder(ServiceOptions<ServiceT, OptionsT> options) {
projectId = options.projectId;
universeDomain = options.universeDomain;
host = options.host;
credentials = options.credentials;
retrySettings = options.retrySettings;
Expand Down Expand Up @@ -199,6 +201,17 @@ public B setHost(String host) {
return self();
}

/**
* Universe Domain is the domain for Google Cloud Services. It follows the format of
suztomo marked this conversation as resolved.
Show resolved Hide resolved
* `{ServiceName}.{UniverseDomain}`. For example, speech.googleapis.com would have a Universe
* Domain value of `googleapis.com` and cloudasset.test.com would have a Universe Domain of
* `test.com`. If this value is not set, this will default to `googleapis.com`.
Copy link
Member

Choose a reason for hiding this comment

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

Would you add a paragraph that explains the validity of the argument in terms of isValidUniverseDomain?

*/
public B setUniverseDomain(String universeDomain) {
this.universeDomain = universeDomain;
suztomo marked this conversation as resolved.
Show resolved Hide resolved
return self();
}

/**
* Sets the service authentication credentials. If no credentials are set, {@link
* GoogleCredentials#getApplicationDefault()} will be used to attempt getting credentials from
Expand Down Expand Up @@ -306,6 +319,7 @@ protected ServiceOptions(
"A project ID is required for this service but could not be determined from the builder "
+ "or the environment. Please set a project ID using the builder.");
}
universeDomain = builder.universeDomain;
host = firstNonNull(builder.host, getDefaultHost());
credentials = builder.credentials != null ? builder.credentials : defaultCredentials();
retrySettings = firstNonNull(builder.retrySettings, getDefaultRetrySettings());
Expand Down Expand Up @@ -582,6 +596,16 @@ public String getProjectId() {
return projectId;
}

/**
* Universe Domain is the domain for Google Cloud Services. It follows the format of
* `{ServiceName}.{UniverseDomain}`. For example, speech.googleapis.com would have a Universe
* Domain value of `googleapis.com` and cloudasset.test.com would have a Universe Domain of
* `test.com`. If this value is not set, this will default to `googleapis.com`.
suztomo marked this conversation as resolved.
Show resolved Hide resolved
*/
public String getUniverseDomain() {
return universeDomain;
}

/** Returns the service host. */
public String getHost() {
return host;
Expand Down Expand Up @@ -767,4 +791,48 @@ public String getClientLibToken() {
public String getQuotaProjectId() {
return quotaProjectId;
}

/**
* Returns the resolved endpoint for the Service to connect to Google Cloud
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
suztomo marked this conversation as resolved.
Show resolved Hide resolved
*
* <p>The resolved endpoint is always in `host:port` format
suztomo marked this conversation as resolved.
Show resolved Hide resolved
*/
@InternalApi
public String getResolvedHost(String serviceName) {
Copy link
Member

Choose a reason for hiding this comment

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

Calling it as host is itchy. Fine but ensure to explain this to library owners.
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URL.html#getHost()

if (universeDomain != null && universeDomain.isEmpty()) {
throw new IllegalArgumentException("Universe Domain cannot be empty");
Copy link
Member

Choose a reason for hiding this comment

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

I felt it's wrong a method that "Returns the resolved endpoint for the Service to connect to Google Cloud" works only when universeDomain is set.

Copy link
Contributor Author

@lqiu96 lqiu96 Jan 4, 2024

Choose a reason for hiding this comment

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

This is a required check. If universedomain is set as "", we must error out. If universeDomain is null, the resolvedUniverseDomain will be googleapis.com. Otherwise, the resolvedUniverseDomain will be whatever is inputted.

Copy link
Member

Choose a reason for hiding this comment

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

I misunderstood the meaning of the code. Added suggested edits.

Copy link
Member

Choose a reason for hiding this comment

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

Then how about setting the requirement in setUniverseDomain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that makes sense. Guidance for temporary Apiary is to use whatever is passed in as the universe domain. The empty check is for handwritten and gapics. I'll ask about the empty string check, but it shouldn't be blocking for this.

lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
}
String resolvedUniverseDomain =
universeDomain != null ? universeDomain : Credentials.GOOGLE_DEFAULT_UNIVERSE;
// The host value set to DEFAULT_HOST if the user didn't configure a host. If the
// user set a host the library uses that value, otherwise, construct the host for the user.
// The DEFAULT_HOST value is not a valid host for handwritten libraries.
if (!host.equals(DEFAULT_HOST)) {
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
return host;
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
}
return "https://www." + serviceName + "." + resolvedUniverseDomain + "/";
}

/**
* Temporarily used for BigQuery and Storage Apiary Wrapped Libraries. To be removed in the
* future. Returns the host to be used for the rootUrl and output is in the format of:
suztomo marked this conversation as resolved.
Show resolved Hide resolved
* "https://serviceName.universeDomain/"
*/
@InternalApi
public String getResolvedApiaryHost(String serviceName) {
String resolvedUniverseDomain =
getUniverseDomain() != null ? getUniverseDomain() : Credentials.GOOGLE_DEFAULT_UNIVERSE;
return "https://www." + serviceName + "." + resolvedUniverseDomain + "/";
}

/**
* Validates that Credentials' Universe Domain matches the user configured Universe Domain.
* Currently, this is intended for BigQuery and Storage Apiary Wrapped Libraries
suztomo marked this conversation as resolved.
Show resolved Hide resolved
*/
@InternalApi
public boolean isValidUniverseDomain() throws IOException {
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
suztomo marked this conversation as resolved.
Show resolved Hide resolved
String resolvedUniverseDomain =
getUniverseDomain() != null ? getUniverseDomain() : Credentials.GOOGLE_DEFAULT_UNIVERSE;
suztomo marked this conversation as resolved.
Show resolved Hide resolved
return resolvedUniverseDomain.equals(getCredentials().getUniverseDomain());
}
}
Expand Up @@ -22,6 +22,7 @@
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -55,6 +56,7 @@ public class ServiceOptionsTest {
private static GoogleCredentials credentials;
private static GoogleCredentials credentialsWithProjectId;
private static GoogleCredentials credentialsWithQuotaProject;
private static GoogleCredentials credentialsNotInGDU;

private static final String JSON_KEY =
"{\n"
Expand All @@ -81,7 +83,8 @@ public class ServiceOptionsTest {
+ "XyRDW4IG1Oa2p\\nrALStNBx5Y9t0/LQnFI4w3aG\\n-----END PRIVATE KEY-----\\n\",\n"
+ " \"client_email\": \"someclientid@developer.gserviceaccount.com\",\n"
+ " \"client_id\": \"someclientid.apps.googleusercontent.com\",\n"
+ " \"type\": \"service_account\"\n"
+ " \"type\": \"service_account\",\n"
+ " \"universe_domain\": \"googleapis.com\"\n"
+ "}";

private static final String JSON_KEY_PROJECT_ID =
Expand Down Expand Up @@ -110,7 +113,8 @@ public class ServiceOptionsTest {
+ " \"project_id\": \"someprojectid\",\n"
+ " \"client_email\": \"someclientid@developer.gserviceaccount.com\",\n"
+ " \"client_id\": \"someclientid.apps.googleusercontent.com\",\n"
+ " \"type\": \"service_account\"\n"
+ " \"type\": \"service_account\",\n"
+ " \"universe_domain\": \"googleapis.com\"\n"
+ "}";

private static final String JSON_KEY_QUOTA_PROJECT_ID =
Expand Down Expand Up @@ -140,13 +144,44 @@ public class ServiceOptionsTest {
+ " \"client_email\": \"someclientid@developer.gserviceaccount.com\",\n"
+ " \"client_id\": \"someclientid.apps.googleusercontent.com\",\n"
+ " \"type\": \"service_account\",\n"
+ " \"quota_project_id\": \"some-quota-project-id\"\n"
+ " \"quota_project_id\": \"some-quota-project-id\",\n"
+ " \"universe_domain\": \"googleapis.com\"\n"
+ "}";

private static final String JSON_KEY_NON_GDU =
Copy link
Member

Choose a reason for hiding this comment

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

Would you add source code comment how you generated this value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't generate this value. I copied it from above and added a new json entry for universe_domain. This universe_domain addition to Credentials was part of Auth v1.21.0

Copy link
Member

Choose a reason for hiding this comment

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

Can you add that "copied it from above and added a new json entry for universe_domain"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

"{\n"
+ " \"private_key_id\": \"somekeyid\",\n"
+ " \"private_key\": \"-----BEGIN PRIVATE KEY-----\\nMIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggS"
+ "kAgEAAoIBAQC+K2hSuFpAdrJI\\nnCgcDz2M7t7bjdlsadsasad+fvRSW6TjNQZ3p5LLQY1kSZRqBqylRkzteMOyHg"
+ "aR\\n0Pmxh3ILCND5men43j3h4eDbrhQBuxfEMalkG92sL+PNQSETY2tnvXryOvmBRwa/\\nQP/9dJfIkIDJ9Fw9N4"
+ "Bhhhp6mCcRpdQjV38H7JsyJ7lih/oNjECgYAt\\nknddadwkwewcVxHFhcZJO+XWf6ofLUXpRwiTZakGMn8EE1uVa2"
+ "LgczOjwWHGi99MFjxSer5m9\\n1tCa3/KEGKiS/YL71JvjwX3mb+cewlkcmweBKZHM2JPTk0ZednFSpVZMtycjkbLa"
+ "\\ndYOS8V85AgMBewECggEBAKksaldajfDZDV6nGqbFjMiizAKJolr/M3OQw16K6o3/\\n0S31xIe3sSlgW0+UbYlF"
+ "4U8KifhManD1apVSC3csafaspP4RZUHFhtBywLO9pR5c\\nr6S5aLp+gPWFyIp1pfXbWGvc5VY/v9x7ya1VEa6rXvL"
+ "sKupSeWAW4tMj3eo/64ge\\nsdaceaLYw52KeBYiT6+vpsnYrEkAHO1fF/LavbLLOFJmFTMxmsNaG0tuiJHgjshB\\"
+ "n82DpMCbXG9YcCgI/DbzuIjsdj2JC1cascSP//3PmefWysucBQe7Jryb6NQtASmnv\\nCdDw/0jmZTEjpe4S1lxfHp"
+ "lAhHFtdgYTvyYtaLZiVVkCgYEA8eVpof2rceecw/I6\\n5ng1q3Hl2usdWV/4mZMvR0fOemacLLfocX6IYxT1zA1FF"
+ "JlbXSRsJMf/Qq39mOR2\\nSpW+hr4jCoHeRVYLgsbggtrevGmILAlNoqCMpGZ6vDmJpq6ECV9olliDvpPgWOP+\\nm"
+ "YPDreFBGxWvQrADNbRt2dmGsrsCgYEAyUHqB2wvJHFqdmeBsaacewzV8x9WgmeX\\ngUIi9REwXlGDW0Mz50dxpxcK"
+ "CAYn65+7TCnY5O/jmL0VRxU1J2mSWyWTo1C+17L0\\n3fUqjxL1pkefwecxwecvC+gFFYdJ4CQ/MHHXU81Lwl1iWdF"
+ "Cd2UoGddYaOF+KNeM\\nHC7cmqra+JsCgYEAlUNywzq8nUg7282E+uICfCB0LfwejuymR93CtsFgb7cRd6ak\\nECR"
+ "8FGfCpH8ruWJINllbQfcHVCX47ndLZwqv3oVFKh6pAS/vVI4dpOepP8++7y1u\\ncoOvtreXCX6XqfrWDtKIvv0vjl"
+ "HBhhhp6mCcRpdQjV38H7JsyJ7lih/oNjECgYAt\\nkndj5uNl5SiuVxHFhcZJO+XWf6ofLUregtevZakGMn8EE1uVa"
+ "2AY7eafmoU/nZPT\\n00YB0TBATdCbn/nBSuKDESkhSg9s2GEKQZG5hBmL5uCMfo09z3SfxZIhJdlerreP\\nJ7gSi"
+ "dI12N+EZxYd4xIJh/HFDgp7RRO87f+WJkofMQKBgGTnClK1VMaCRbJZPriw\\nEfeFCoOX75MxKwXs6xgrw4W//AYG"
+ "GUjDt83lD6AZP6tws7gJ2IwY/qP7+lyhjEqN\\nHtfPZRGFkGZsdaksdlaksd323423d+15/UvrlRSFPNj1tWQmNKk"
+ "XyRDW4IG1Oa2p\\nrALStNBx5Y9t0/LQnFI4w3aG\\n-----END PRIVATE KEY-----\\n\",\n"
+ " \"client_email\": \"someclientid@developer.gserviceaccount.com\",\n"
+ " \"client_id\": \"someclientid.apps.googleusercontent.com\",\n"
+ " \"type\": \"service_account\",\n"
+ " \"universe_domain\": \"random.com\"\n"
+ "}";

static {
credentials = loadCredentials(JSON_KEY);
credentialsWithProjectId = loadCredentials(JSON_KEY_PROJECT_ID);
credentialsWithQuotaProject = loadCredentials(JSON_KEY_QUOTA_PROJECT_ID);
credentialsNotInGDU = loadCredentials(JSON_KEY_NON_GDU);
}

static GoogleCredentials loadCredentials(String credentialFile) {
Expand Down Expand Up @@ -471,6 +506,129 @@ public void testResponseHeaderDoesNotContainMetaDataFlavor() throws Exception {
assertThat(ServiceOptions.headerContainsMetadataFlavor(httpResponse)).isFalse();
}

@Test
public void testGetResolvedEndpoint_noUniverseDomain() {
TestServiceOptions options = TestServiceOptions.newBuilder().setProjectId("project-id").build();
assertThat(options.getResolvedHost("service")).isEqualTo("https://www.service.googleapis.com/");
}

@Test
public void testGetResolvedEndpoint_emptyUniverseDomain() {
TestServiceOptions options =
TestServiceOptions.newBuilder().setUniverseDomain("").setProjectId("project-id").build();
IllegalArgumentException exception =
assertThrows(IllegalArgumentException.class, () -> options.getResolvedHost("service"));
assertThat(exception.getMessage()).isEqualTo("Universe Domain cannot be empty");
}

@Test
public void testGetResolvedEndpoint_customUniverseDomain() {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setUniverseDomain("test.com")
.setProjectId("project-id")
.build();
assertThat(options.getResolvedHost("service")).isEqualTo("https://www.service.test.com/");
}

@Test
public void testGetResolvedEndpoint_customUniverseDomain_customHost() {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setUniverseDomain("test.com")
.setHost("https://www.service.random.com/")
.setProjectId("project-id")
.build();
assertThat(options.getResolvedHost("service")).isEqualTo("https://www.service.random.com/");
}

@Test
public void testGetResolvedApiaryHost_noUniverseDomain() {
TestServiceOptions options = TestServiceOptions.newBuilder().setProjectId("project-id").build();
assertThat(options.getResolvedApiaryHost("service"))
.isEqualTo("https://www.service.googleapis.com/");
}

@Test
public void testGetResolvedApiaryHost_customUniverseDomain_noHost() {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setUniverseDomain("test.com")
.setHost(null)
.setProjectId("project-id")
.build();
assertThat(options.getResolvedApiaryHost("service")).isEqualTo("https://www.service.test.com/");
}

@Test
public void testGetResolvedApiaryHost_customUniverseDomain_customHost() {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setUniverseDomain("test.com")
.setHost("https://service.random.com")
.setProjectId("project-id")
.build();
assertThat(options.getResolvedApiaryHost("service")).isEqualTo("https://www.service.test.com/");
}

// No User Configuration = GDU, Default Credentials = GDU
@Test
public void testIsValidUniverseDomain_noUserUniverseDomainConfig_defaultCredentials()
throws IOException {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setProjectId("project-id")
.setHost("https://test.random.com")
.setCredentials(credentials)
.build();
assertThat(options.isValidUniverseDomain()).isTrue();
}

// No User Configuration = GDU, non Default Credentials = random.com
// non-GDU Credentials could be any domain, the tests use random.com
@Test
public void testIsValidUniverseDomain_noUserUniverseDomainConfig_nonGDUCredentials()
throws IOException {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setProjectId("project-id")
.setHost("https://test.random.com")
.setCredentials(credentialsNotInGDU)
.build();
assertThat(options.isValidUniverseDomain()).isFalse();
}

// User Configuration = random.com, Default Credentials = GDU
// User Credentials could be set to any domain, the tests use random.com
@Test
public void testIsValidUniverseDomain_userUniverseDomainConfig_defaultCredentials()
throws IOException {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setProjectId("project-id")
.setHost("https://test.random.com")
.setUniverseDomain("random.com")
.setCredentials(credentials)
.build();
assertThat(options.isValidUniverseDomain()).isFalse();
}

// User Configuration = random.com, non Default Credentials = random.com
// User Credentials and non GDU Credentials could be set to any domain,
// the tests use random.com
@Test
public void testIsValidUniverseDomain_userUniverseDomainConfig_nonGDUCredentials()
throws IOException {
TestServiceOptions options =
TestServiceOptions.newBuilder()
.setProjectId("project-id")
.setHost("https://test.random.com")
.setUniverseDomain("random.com")
.setCredentials(credentialsNotInGDU)
.build();
assertThat(options.isValidUniverseDomain()).isTrue();
}

private HttpResponse createHttpResponseWithHeader(final Multimap<String, String> headers)
throws Exception {
HttpTransport mockHttpTransport =
Expand Down