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: Universe Domain Environment Variable Support #2485

Merged
merged 18 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ jobs:
run: |
mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-Dfmt.skip -DenableTestCoverage
# The `envVarTest` profile runs tests that require an environment variable
- name: Env Var Tests
run: |
mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-Dfmt.skip -DenableTestCoverage -PenvVarTest
# Set the Env Var for this step only
env:
GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com
- run: bazelisk version
- name: Install Maven modules
run: |
Expand Down Expand Up @@ -64,6 +72,14 @@ jobs:
run: |
mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-Dfmt.skip -DenableTestCoverage
# The `envVarTest` profile runs tests that require an environment variable
- name: Env Var Tests
run: |
mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-Dfmt.skip -DenableTestCoverage -PenvVarTest
# Set the Env Var for this step only
env:
GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com
- run: bazelisk version
- name: Install Maven modules
run: |
Expand Down Expand Up @@ -98,6 +114,22 @@ jobs:
mvn verify --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-Dfmt.skip \
-Djvm="${JAVA8_HOME}/bin/java"
# The `envVarTest` profile runs tests that require an environment variable
- name: Compile with Java 17 and run tests with Java 8 (Env Var Tests)
shell: bash
run: |
set -x
export JAVA_HOME=$JAVA_HOME
export PATH=${JAVA_HOME}/bin:$PATH
# Maven surefire plugin lets us to specify the JVM when running tests via
# the "jvm" system property.
export GOOGLE_CLOUD_UNIVERSE_DOMAIN=random.com
mvn test --batch-mode --no-transfer-progress -Dcheckstyle.skip \
-Dfmt.skip -DenableTestCoverage -Dsurefire.failIfNoSpecifiedTests=false \
-PenvVarTest
# Set the Env Var for this step only
env:
GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com

build-java8-gapic-generator-java:
name: "build(8) for gapic-generator-java"
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/sonar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ jobs:
tar -xf showcase-*
./gapic-showcase run &
cd -
# Intentionally do not run the Env Var Tests (no -PenvVarTests) as setting the Env Var
# may alter the results for other tests that use Env Var in the logic. Adding a Sonar
# step for a few tests (env var tests) may be overkill and should be better covered
# when we can upgrade to JUnit 5 (https://github.com/googleapis/sdk-platform-java/issues/1611#issuecomment-1970079325)
- name: Build and analyze for full test coverage
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
Expand All @@ -59,7 +63,6 @@ jobs:
-Dsonar.projectKey=googleapis_gapic-generator-java \
-Dsonar.organization=googleapis \
-Dsonar.host.url=https://sonarcloud.io

- name: Build and analyze Showcase Integration Tests Coverage
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
Expand Down
17 changes: 17 additions & 0 deletions gapic-generator-java-pom-parent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,23 @@
</plugins>
</build>
</profile>
<profile>
<id>envVarTest</id>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<!-- Unless overriden in the submodules, this profile run no tests in a submodule -->
<excludes>
<exclude>**/*.java</exclude>
</excludes>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>
<repositories>
<repository>
Expand Down
19 changes: 19 additions & 0 deletions gax-java/gax/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,27 @@
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>-Djava.util.logging.SimpleFormatter.format="%1$tY %1$tl:%1$tM:%1$tS.%1$tL %2$s %4$s: %5$s%6$s%n"</argLine>
<!-- These tests require an Env Var to be set. Use -PenvVarTest to ONLY run these tests -->
<test>!EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority</test>
</configuration>
</plugin>
</plugins>
</build>

<profiles>
<profile>
<id>envVarTest</id>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<test>EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority</test>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
@InternalApi
@AutoValue
public abstract class EndpointContext {
private static final String GOOGLE_CLOUD_UNIVERSE_DOMAIN = "GOOGLE_CLOUD_UNIVERSE_DOMAIN";
private static final String INVALID_UNIVERSE_DOMAIN_ERROR_TEMPLATE =
"The configured universe domain (%s) does not match the universe domain found in the credentials (%s). If you haven't configured the universe domain explicitly, `googleapis.com` is the default.";
public static final String UNABLE_TO_RETRIEVE_CREDENTIALS_ERROR_MESSAGE =
Expand Down Expand Up @@ -201,20 +202,27 @@ public abstract static class Builder {
abstract EndpointContext autoBuild();

private String determineUniverseDomain() {
String universeDomain = universeDomain();
if (usingGDCH()) {
// GDC-H has no concept of Universe Domain. User should not set a custom value
if (universeDomain() != null) {
if (universeDomain != null) {
throw new IllegalArgumentException(
"Universe domain configuration is incompatible with GDC-H");
}
return Credentials.GOOGLE_DEFAULT_UNIVERSE;
}
// Check for "" (empty string)
if (universeDomain() != null && universeDomain().isEmpty()) {
if (universeDomain != null && universeDomain.isEmpty()) {
throw new IllegalArgumentException("The universe domain value cannot be empty.");
}
// Override with user set universe domain if provided
return universeDomain() != null ? universeDomain() : Credentials.GOOGLE_DEFAULT_UNIVERSE;
// If the universe domain wasn't configured explicitly in the settings, check the
// environment variable for the value
if (universeDomain == null) {
universeDomain = System.getenv(GOOGLE_CLOUD_UNIVERSE_DOMAIN);
}
// If the universe domain is configured by the user, the universe domain will either be
// from the settings or from the env var. The value from ClientSettings has priority.
return universeDomain != null ? universeDomain : Credentials.GOOGLE_DEFAULT_UNIVERSE;
}

/** Determines the fully resolved endpoint and universe domain values */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,41 @@ public void endpointContextBuild_gdchFlow_noUniverseDomain_customEndpoint() thro
.isEqualTo(Credentials.GOOGLE_DEFAULT_UNIVERSE);
}

// This Universe Domain should match the `GOOGLE_CLOUD_UNIVERSE_DOMAIN` Env Var
// For this test running locally or in CI, check that the Env Var is set properly.
// This test should only run when the maven profile `EnvVarTest` is enabled.
@Test
public void endpointContextBuild_universeDomainEnvVarSet() throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add one more test case that if universe domain is set on both ClientSettings and Env Var, then ClientSettings should take precedence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, added a test case that will be run with -PenvVarTest

String envVarUniverseDomain = "random.com";
EndpointContext endpointContext =
defaultEndpointContextBuilder
.setUniverseDomain(null)
.setClientSettingsEndpoint(null)
.build();
Truth.assertThat(endpointContext.resolvedEndpoint()).isEqualTo("test.random.com:443");
Truth.assertThat(endpointContext.resolvedUniverseDomain()).isEqualTo(envVarUniverseDomain);
}

// This Universe Domain should match the `GOOGLE_CLOUD_UNIVERSE_DOMAIN` Env Var
// For this test running locally or in CI, check that the Env Var is set properly.
// This test should only run when the maven profile `EnvVarTest` is enabled.
@Test
public void endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority()
throws IOException {
// This test has `GOOGLE_CLOUD_UNIVERSE_DOMAIN` = `random.com`
String clientSettingsUniverseDomain = "clientSettingsUniverseDomain.com";
EndpointContext endpointContext =
defaultEndpointContextBuilder
.setUniverseDomain(clientSettingsUniverseDomain)
.setClientSettingsEndpoint(null)
.build();
Truth.assertThat(endpointContext.resolvedEndpoint())
.isEqualTo("test.clientSettingsUniverseDomain.com:443");
// Client Settings Universe Domain (if set) takes priority
Truth.assertThat(endpointContext.resolvedUniverseDomain())
.isEqualTo(clientSettingsUniverseDomain);
}

@Test
public void hasValidUniverseDomain_gdchFlow_anyCredentials() throws IOException {
Credentials noCredentials = NoCredentialsProvider.create().getCredentials();
Expand Down
Loading