Skip to content

chore: deprecate experimental host option/parameter #4575

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

Merged
olavloite merged 3 commits into from
Jul 2, 2026
Merged

chore: deprecate experimental host option/parameter #4575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
122e7db
chore: deprecate experimental host option/parameter
sagnghos Jul 1, 2026
47339c5
test: Add TLS and plain-text configuration support for Spanner Omni i…
sagnghos Jul 1, 2026
b096081
fix: use case-insensitive propertyMap for JDBC connection string para…
sagnghos Jul 2, 2026
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
90 changes: 46 additions & 44 deletions src/main/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,13 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.spanner.v1.DatabaseName;
import io.grpc.ExperimentalApi;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
Expand All @@ -65,10 +64,15 @@ public class OptionsMetadata {

static Duration DEFAULT_STARTUP_TIMEOUT = Duration.ofSeconds(30L);

private static final String EXTERNAL_HOST_PROJECT = "default";
private static final String EXTERNAL_HOST_INSTANCE = "default";
private static final String SPANNER_OMNI_PROJECT = "default";
private static final String SPANNER_OMNI_INSTANCE = "default";

private static final String IS_EXPERIMENTAL_HOST_PROPERTY_NAME = "isExperimentalHost";
/**
* @deprecated use {@link #TYPE_PROPERTY_NAME} instead.
*/
private static final String TYPE_PROPERTY_NAME = "type";

private static final String SPANNER_OMNI_TYPE = "omni";

/**
* Builder class for creating an instance of {@link OptionsMetadata}.
Expand Down Expand Up @@ -116,7 +120,8 @@ public static class Builder {
private Duration startupTimeout = DEFAULT_STARTUP_TIMEOUT;
private String clientCertificate;
private String clientKey;
private boolean isExperimentalHost = false;

private String type = null;
private Long describeCacheExpireMinutes;
private Long describeCacheMaxSize;

Expand Down Expand Up @@ -431,27 +436,24 @@ Builder setStartupTimeout(Duration timeout) {

/**
* Configures mTLS authentication using the provided client certificate and key files. mTLS is
* only supported for experimental spanner hosts.
* only supported for Spanner Omni endpoints.
*
* @param clientCertificate Path to the client certificate file.
* @param clientKey Path to the client private key file.
*/
@ExperimentalApi("https://github.com/googleapis/java-spanner/pull/3574")
Builder useClientCert(String clientCertificate, String clientKey) {
this.clientCertificate = clientCertificate;
this.clientKey = clientKey;
return this;
}

/*
* Configures connection to an experimental host endpoint
/**
* Configures the connection instance type.
*
* @params experimentalHost url of the experimental host endpoint
* @param type connection type, e.g., "OMNI"
*/
@ExperimentalApi("https://github.com/googleapis/java-spanner/pull/3574")
Builder setExperimentalHost(String experimentalHost) {
setEndpoint(experimentalHost);
this.isExperimentalHost = true;
Builder setType(String type) {
this.type = type;
return this;
}

Expand Down Expand Up @@ -546,7 +548,7 @@ private String[] toCommandLineArguments() {
|| useVirtualGrpcTransportThreads
|| enableEndToEndTracing
|| (clientKey != null && clientCertificate != null)
|| isExperimentalHost) {
|| SPANNER_OMNI_TYPE.equalsIgnoreCase(this.type)) {
StringBuilder jdbcOptionBuilder = new StringBuilder();
if (usePlainText) {
jdbcOptionBuilder.append("usePlainText=true;");
Expand All @@ -572,8 +574,8 @@ private String[] toCommandLineArguments() {
jdbcOptionBuilder.append("clientCertificate=").append(clientCertificate).append(";");
jdbcOptionBuilder.append("clientKey=").append(clientKey).append(";");
}
if (isExperimentalHost) {
jdbcOptionBuilder.append("isExperimentalHost=true;");
if (SPANNER_OMNI_TYPE.equalsIgnoreCase(this.type)) {
jdbcOptionBuilder.append("type=omni;");
}
addOption(args, OPTION_JDBC_PROPERTIES, jdbcOptionBuilder.toString());
}
Expand Down Expand Up @@ -789,7 +791,7 @@ private OptionsMetadata(Builder builder) {
if (!propertyMap.containsKey("defaultSequenceKind")) {
propertyMap.put("defaultSequenceKind", "bit_reversed_positive");
}
boolean usesExperimentalHost = isExperimentalHost(commandLine, propertyMap);
boolean usesOmniType = hasOmniType(commandLine, propertyMap);

this.environment = environment;
this.osName = osName;
Expand All @@ -807,7 +809,7 @@ private OptionsMetadata(Builder builder) {
+ "OR use -c to set the credentials in PGAdapter and use these credentials for all connections.");
}
if (this.commandLine.hasOption(OPTION_DATABASE_NAME)
&& !usesExperimentalHost
&& !usesOmniType
&& !(this.commandLine.hasOption(OPTION_PROJECT_ID)
&& this.commandLine.hasOption(OPTION_INSTANCE_ID))) {
throw SpannerExceptionFactory.newSpannerException(
Expand All @@ -816,7 +818,7 @@ private OptionsMetadata(Builder builder) {
+ "Use the options -p <project-id> -i <instance-id> -d <database-id> to specify the "
+ "database that all connections to this instance of PGAdapter should use.");
}
if ((usesExperimentalHost
if ((usesOmniType
|| (this.commandLine.hasOption(OPTION_PROJECT_ID)
&& this.commandLine.hasOption(OPTION_INSTANCE_ID)))
&& this.commandLine.hasOption(OPTION_DATABASE_NAME)) {
Expand Down Expand Up @@ -930,7 +932,7 @@ public OptionsMetadata(
this.ddlTransactionMode = DdlTransactionMode.AutocommitImplicitTransaction;
this.replaceJdbcMetadataQueries = replaceJdbcMetadataQueries;
this.commandMetadataJSON = commandMetadata;
this.propertyMap = new HashMap<>();
this.propertyMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
this.disableLocalhostCheck = false;
this.serverVersion = DEFAULT_SERVER_VERSION;
this.debugMode = false;
Expand All @@ -941,7 +943,7 @@ public OptionsMetadata(
}

private Map<String, String> parseProperties(String propertyOptions) {
Map<String, String> properties = new HashMap<>();
Map<String, String> properties = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
if (!propertyOptions.isEmpty()) {
String[] propertyList = propertyOptions.split(";");
for (int i = 0; i < propertyList.length; ++i) {
Expand Down Expand Up @@ -1109,7 +1111,7 @@ public SessionPoolOptions getSessionPoolOptions() {
*/
public String buildCredentialsFile() {
// Skip if a com.google.auth.Credentials instance has been set.
if (isExperimentalHost() || credentials != null) {
if (hasOmniType() || credentials != null) {
return null;
}
if (!commandLine.hasOption(OPTION_CREDENTIALS_FILE)) {
Expand Down Expand Up @@ -1199,23 +1201,23 @@ private static boolean usesEmulator(
|| isAutoConfigEmulator(propertyMap);
}

private boolean isExperimentalHost() {
private boolean hasOmniType() {
if (this.propertyMap == null) {
return isExperimentalHost(
return hasOmniType(
this.commandLine,
parseProperties(this.commandLine.getOptionValue(OPTION_JDBC_PROPERTIES, "")));
}
return isExperimentalHost(this.commandLine, this.propertyMap);
return hasOmniType(this.commandLine, this.propertyMap);
}

private static boolean isExperimentalHost(
CommandLine commandLine, Map<String, String> propertyMap) {
private static boolean hasOmniType(CommandLine commandLine, Map<String, String> propertyMap) {
if (propertyMap == null) {
return false;
}
return commandLine.hasOption(OPTION_SPANNER_ENDPOINT)
&& commandLine.hasOption(OPTION_JDBC_PROPERTIES)
&& propertyMap.containsKey(IS_EXPERIMENTAL_HOST_PROPERTY_NAME);
&& (commandLine.hasOption(OPTION_JDBC_PROPERTIES)
&& (propertyMap.containsKey("isExperimentalHost")
|| SPANNER_OMNI_TYPE.equalsIgnoreCase(propertyMap.get(TYPE_PROPERTY_NAME))));

@olavloite olavloite Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This lookup in propertyMap will fail if someone added Type=omni to the connection string. This is actually a generic problem for multiple other properties here as well. Can we change the type of map that is used for this to this:

Map<String, String> properties = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);

@sagnghos sagnghos Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

}

/** Returns the fully qualified database name based on the given database id or name. */
Expand All @@ -1227,8 +1229,8 @@ public DatabaseName getDatabaseName(String database) {
String projectId;
if (commandLine.hasOption(OPTION_PROJECT_ID)) {
projectId = commandLine.getOptionValue(OPTION_PROJECT_ID);
} else if (isExperimentalHost()) {
projectId = EXTERNAL_HOST_PROJECT;
} else if (hasOmniType()) {
projectId = SPANNER_OMNI_PROJECT;
} else {
projectId = getDefaultProjectId();
}
Expand All @@ -1242,8 +1244,8 @@ public DatabaseName getDatabaseName(String database) {
String instanceId;
if (commandLine.hasOption(OPTION_INSTANCE_ID)) {
instanceId = commandLine.getOptionValue(OPTION_INSTANCE_ID);
} else if (isExperimentalHost()) {
instanceId = EXTERNAL_HOST_INSTANCE;
} else if (hasOmniType()) {
instanceId = SPANNER_OMNI_INSTANCE;
} else {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
Expand Down Expand Up @@ -1682,31 +1684,31 @@ public boolean hasDefaultConnectionUrl() {
public DatabaseId getDefaultDatabaseId() {
return this.hasDefaultConnectionUrl()
? DatabaseId.of(
!commandLine.hasOption(OPTION_PROJECT_ID) && isExperimentalHost()
? EXTERNAL_HOST_PROJECT
!commandLine.hasOption(OPTION_PROJECT_ID) && hasOmniType()
? SPANNER_OMNI_PROJECT
: commandLine.getOptionValue(OPTION_PROJECT_ID),
!commandLine.hasOption(OPTION_INSTANCE_ID) && isExperimentalHost()
? EXTERNAL_HOST_INSTANCE
!commandLine.hasOption(OPTION_INSTANCE_ID) && hasOmniType()
? SPANNER_OMNI_INSTANCE
: commandLine.getOptionValue(OPTION_INSTANCE_ID),
commandLine.getOptionValue(OPTION_DATABASE_NAME))
: null;
}

/** Returns true if these options contain a default instance id. */
public boolean hasDefaultInstanceId() {
return isExperimentalHost()
return hasOmniType()
|| (commandLine.hasOption(OPTION_PROJECT_ID) && commandLine.hasOption(OPTION_INSTANCE_ID));
}

/** Returns the id of the default instance or null if no default has been selected. */
public InstanceId getDefaultInstanceId() {
if (hasDefaultInstanceId()) {
return InstanceId.of(
!commandLine.hasOption(OPTION_PROJECT_ID) && isExperimentalHost()
? EXTERNAL_HOST_PROJECT
!commandLine.hasOption(OPTION_PROJECT_ID) && hasOmniType()
? SPANNER_OMNI_PROJECT
: commandLine.getOptionValue(OPTION_PROJECT_ID),
!commandLine.hasOption(OPTION_INSTANCE_ID) && isExperimentalHost()
? EXTERNAL_HOST_INSTANCE
!commandLine.hasOption(OPTION_INSTANCE_ID) && hasOmniType()
? SPANNER_OMNI_INSTANCE
: commandLine.getOptionValue(OPTION_INSTANCE_ID));
}
return null;
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/google/cloud/spanner/pgadapter/ITAuthTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public String getCredentials() {

@BeforeClass
public static void setup() throws ClassNotFoundException {
IntegrationTest.skipOnExperimentalHost("Cloud auth is not applicable for experimental host");
IntegrationTest.skipOnSpannerOmni("Cloud auth is not applicable for Spanner Omni");
// Make sure the PG JDBC driver is loaded.
Class.forName("org.postgresql.Driver");

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/google/cloud/spanner/pgadapter/ITOpenTelemetryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class ITOpenTelemetryTest implements IntegrationTest {
@BeforeClass
public static void setup() throws IOException {
IntegrationTest.skipOnEmulator("This test requires credentials");
IntegrationTest.skipOnExperimentalHost("Cloud auth is not applicable for experimental host");
IntegrationTest.skipOnSpannerOmni("Cloud auth is not applicable for Spanner Omni");

OptionsMetadata.Builder openTelemetryOptionsBuilder =
OptionsMetadata.newBuilder()
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/com/google/cloud/spanner/pgadapter/IntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ static void skipOnEmulator(String reason) {
assumeFalse(reason, isRunningOnEmulator());
}

static void skipOnExperimentalHost(String reason) {
assumeFalse(reason, isRunningOnExperimentalHost());
static void skipOnSpannerOmni(String reason) {
assumeFalse(reason, isRunningOnSpannerOmni());
}

static boolean isRunningOnEmulator() {
return System.getenv("SPANNER_EMULATOR_HOST") != null;
}

static boolean isRunningOnExperimentalHost() {
return System.getProperty("SPANNER_EXPERIMENTAL_HOST") != null;
static boolean isRunningOnSpannerOmni() {
return System.getProperty("SPANNER_OMNI_HOST") != null;
}
}
Loading
Loading