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 13 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>
<!-- This test requires an Env Var to be set. Use -PenvVarTest to ONLY run this test -->
<test>!EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet</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</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 @@ -213,7 +214,8 @@ private String determineUniverseDomain() {
if (universeDomain() != null && universeDomain().isEmpty()) {
throw new IllegalArgumentException("The universe domain value cannot be empty.");
}
// Override with user set universe domain if provided
// 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;
}

Expand Down Expand Up @@ -283,6 +285,11 @@ String mtlsEndpointResolver(
}

public EndpointContext build() throws IOException {
// If the universe domain wasn't configured explicitly in the settings, check the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move the logic to determineUniverseDomain()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, moved it to determineUniverseDomain()

// environment variable for the value
if (universeDomain() == null) {
setUniverseDomain(System.getenv(GOOGLE_CLOUD_UNIVERSE_DOMAIN));
}
// The Universe Domain is used to resolve the Endpoint. It should be resolved first
setResolvedUniverseDomain(determineUniverseDomain());
setResolvedEndpoint(determineEndpoint());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,21 @@ 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);
}

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