Skip to content

feat: default authentication support for external hosts #3655

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

feat: default authentication support for external hosts #3655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
180e128
feat: default authentication support for external hosts
sagnghos Feb 22, 2025
61ee038
unit test added
sagnghos Feb 23, 2025
004cea2
unit test typo
sagnghos Feb 23, 2025
e65c555
skip auth_token in emulator host for jdbc
sagnghos Feb 24, 2025
2aa18db
skip auth_token for emulator
sagnghos Feb 24, 2025
c33ae1b
env variable prefix change
sagnghos Feb 24, 2025
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
27 changes: 27 additions & 0 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import com.google.api.gax.tracing.ApiTracerFactory;
import com.google.api.gax.tracing.BaseApiTracerFactory;
import com.google.api.gax.tracing.OpencensusTracerFactory;
import com.google.auth.oauth2.AccessToken;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.NoCredentials;
import com.google.cloud.ServiceDefaults;
import com.google.cloud.ServiceOptions;
Expand Down Expand Up @@ -79,8 +81,11 @@
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Base64;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -92,6 +97,7 @@
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Pattern;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.GuardedBy;
Expand All @@ -110,6 +116,11 @@ public class SpannerOptions extends ServiceOptions<Spanner, SpannerOptions> {

private static final String API_SHORT_NAME = "Spanner";
private static final String DEFAULT_HOST = "https://spanner.googleapis.com";
private static final String CLOUD_SPANNER_HOST_FORMAT = ".*\\.googleapis\\.com.*";

@VisibleForTesting
static final Pattern CLOUD_SPANNER_HOST_PATTERN = Pattern.compile(CLOUD_SPANNER_HOST_FORMAT);

private static final ImmutableSet<String> SCOPES =
ImmutableSet.of(
"https://www.googleapis.com/auth/spanner.admin",
Expand Down Expand Up @@ -799,6 +810,18 @@ protected SpannerOptions(Builder builder) {
enableBuiltInMetrics = builder.enableBuiltInMetrics;
enableEndToEndTracing = builder.enableEndToEndTracing;
monitoringHost = builder.monitoringHost;
String externalHostTokenPath = System.getenv("SPANNER_EXTERNAL_HOST_AUTH_TOKEN");
if (builder.isExternalHost && builder.emulatorHost == null && externalHostTokenPath != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Prefer !Strings.isNullOrEmpty(externalHostTokenPath) over a null check.
  2. This check should also include && builder.credentials == null to prevent the default credentials to override specific credentials that have been set for this builder.

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
#3656

String token;
try {
token =
Base64.getEncoder()
.encodeToString(Files.readAllBytes(Paths.get(externalHostTokenPath)));
} catch (IOException e) {
throw SpannerExceptionFactory.newSpannerException(e);
}
credentials = new GoogleCredentials(new AccessToken(token, null));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This constructor is deprecated. Use the static create method instead.

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
#3656

}
}

/**
Expand Down Expand Up @@ -967,6 +990,7 @@ public static class Builder
private boolean enableBuiltInMetrics = SpannerOptions.environment.isEnableBuiltInMetrics();
private String monitoringHost = SpannerOptions.environment.getMonitoringHost();
private SslContext mTLSContext = null;
private boolean isExternalHost = false;
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 make this a method instead of a field. This now gives the impression of being in sync with the current settings, but it isn't if you do weird stuff (like first setting an emulator host and then overriding that by setting a host). By implementing it as a method, you can just calculate it based on the values at that time, instead of trying to set the correct value when setHost / setEmulatorHost is being set.

Copy link
Contributor Author

@sagnghos sagnghos Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

removed the need to check emulatorHost. But had to keep isExternalHost calculation in setHost. Can't convert it to a method to do a on demand calculation since host is a private member in the parent builder class
#3656


private static String createCustomClientLibToken(String token) {
return token + " " + ServiceOptions.getGoogApiClientLibName();
Expand Down Expand Up @@ -1459,6 +1483,9 @@ public Builder setDecodeMode(DecodeMode decodeMode) {
@Override
public Builder setHost(String host) {
super.setHost(host);
if (this.emulatorHost == null && !CLOUD_SPANNER_HOST_PATTERN.matcher(host).matches()) {
this.isExternalHost = true;
}
// Setting a host should override any SPANNER_EMULATOR_HOST setting.
setEmulatorHost(null);
return this;
Expand Down
3 changes: 3 additions & 0 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,9 @@ Spanner createSpanner(SpannerPoolKey key, ConnectionOptions options) {
if (options.getConfigurator() != null) {
options.getConfigurator().configure(builder);
}
if (options.usesEmulator()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed
#3656

builder.setEmulatorHost(key.host);
}
return builder.build().getService();
}

Expand Down
9 changes: 9 additions & 0 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.spanner;

import static com.google.cloud.spanner.SpannerOptions.CLOUD_SPANNER_HOST_PATTERN;
import static com.google.common.truth.Truth.assertThat;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -1164,4 +1165,12 @@ public void checkGlobalOpenTelemetryWhenNotInjected() {
.build();
assertEquals(GlobalOpenTelemetry.get(), options.getOpenTelemetry());
}

@Test
public void testCloudSpannerHostPattern() {
assertTrue(CLOUD_SPANNER_HOST_PATTERN.matcher("https://spanner.googleapis.com").matches());
assertTrue(
CLOUD_SPANNER_HOST_PATTERN.matcher("https://product-area.googleapis.com:443").matches());
assertFalse(CLOUD_SPANNER_HOST_PATTERN.matcher("https://some-company.com:443").matches());
}
}
Loading