From d0adba17b134de91703011c900cff8ba82c5853c Mon Sep 17 00:00:00 2001 From: sagnghos Date: Wed, 14 Jan 2026 06:04:13 +0000 Subject: [PATCH 1/6] feat: add driver support for experimental host --- connection_properties.go | 33 ++++++++++++++ driver.go | 80 ++++++++++++++++++++++++++++++++-- driver_test.go | 92 ++++++++++++++++++++++++++++++++++++++++ integration_test.go | 53 +++++++++++++++++++++++ 4 files changed, 255 insertions(+), 3 deletions(-) diff --git a/connection_properties.go b/connection_properties.go index 1baac940..690e5369 100644 --- a/connection_properties.go +++ b/connection_properties.go @@ -557,6 +557,39 @@ var propertyCommitResponse = createReadOnlyConnectionProperty( connectionstate.ContextUser, ) +var propertyCaCertFile = createConnectionProperty( + "ca_cert_file", + "The path to the CA certificate file to use for TLS connections to the server. "+ + "This is only needed when connecting to an experimental host endpoint", + "", + false, + nil, + connectionstate.ContextStartup, + connectionstate.ConvertString, +) + +var propertyClientCertFile = createConnectionProperty( + "client_cert_file", + "The path to the client certificate file to use for mTLS connections to the server. "+ + "This is only needed when connecting to an experimental host endpoint", + "", + false, + nil, + connectionstate.ContextStartup, + connectionstate.ConvertString, +) + +var propertyClientCertKey = createConnectionProperty( + "client_cert_key", + "The path to the client certificate key file to use for mTLS connections to the server. "+ + "This is only needed when connecting to an experimental host endpoint", + "", + false, + nil, + connectionstate.ContextStartup, + connectionstate.ConvertString, +) + func createReadOnlyConnectionProperty[T comparable](name, description string, defaultValue T, hasDefaultValue bool, validValues []T, context connectionstate.Context) *connectionstate.TypedConnectionProperty[T] { converter := connectionstate.CreateReadOnlyConverter[T](name) return createConnectionProperty(name, description, defaultValue, hasDefaultValue, validValues, context, converter) diff --git a/driver.go b/driver.go index 36786199..51ee02f2 100644 --- a/driver.go +++ b/driver.go @@ -16,6 +16,8 @@ package spannerdriver import ( "context" + "crypto/tls" + "crypto/x509" "database/sql" "database/sql/driver" "errors" @@ -46,6 +48,7 @@ import ( "google.golang.org/api/option/internaloption" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" @@ -67,6 +70,9 @@ var defaultStatementCacheSize int // application. const LevelNotice = slog.LevelInfo - 1 +const experimentalHostProject = "default" +const experimentalHostInstance = "default" + // Logger that discards everything and skips (almost) all logs. var noopLogger = slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError + 1})) @@ -96,6 +102,7 @@ var noopLogger = slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{L // // Example: `localhost:9010/projects/test-project/instances/test-instance/databases/test-database;usePlainText=true;disableRouteToLeader=true;enableEndToEndTracing=true` var dsnRegExp = regexp.MustCompile(`((?P[\w.-]+(?:\.[\w\.-]+)*[\w\-\._~:/?#\[\]@!\$&'\(\)\*\+,;=.]+)/)?projects/(?P(([a-z]|[-.:]|[0-9])+|(DEFAULT_PROJECT_ID)))(/instances/(?P([a-z]|[-]|[0-9])+)(/databases/(?P([a-z]|[-]|[_]|[0-9])+))?)?(([\?|;])(?P.*))?`) +var expHostDSNRegExp = regexp.MustCompile(`spanner://(?P[\w.-]+(?::\d+)?)(?:/instances/(?P[a-z0-9-]+))?/databases/(?P[a-z][a-z0-9_-]{0,28}[a-z0-9])(?:[?;](?P.*))?`) var _ driver.DriverContext = &Driver{} var spannerDriver *Driver @@ -481,11 +488,21 @@ func (cc *ConnectorConfig) String() string { // data source name. func ExtractConnectorConfig(dsn string) (ConnectorConfig, error) { match := dsnRegExp.FindStringSubmatch(dsn) + isExpHost := false + + if strings.HasPrefix(dsn, "spanner://") { + isExpHost = true + match = expHostDSNRegExp.FindStringSubmatch(dsn) + } if match == nil { return ConnectorConfig{}, spanner.ToSpannerError(status.Errorf(codes.InvalidArgument, "invalid connection string: %s", dsn)) } matches := make(map[string]string) - for i, name := range dsnRegExp.SubexpNames() { + names := dsnRegExp.SubexpNames() + if isExpHost { + names = expHostDSNRegExp.SubexpNames() + } + for i, name := range names { if i != 0 && name != "" { matches[name] = match[i] } @@ -496,14 +513,24 @@ func ExtractConnectorConfig(dsn string) (ConnectorConfig, error) { return ConnectorConfig{}, err } - return ConnectorConfig{ + c := ConnectorConfig{ Host: matches["HOSTGROUP"], Project: matches["PROJECTGROUP"], Instance: matches["INSTANCEGROUP"], Database: matches["DATABASEGROUP"], Params: params, name: dsn, - }, nil + } + if isExpHost { + c.Configurator = func(config *spanner.ClientConfig, opts *[]option.ClientOption) { + config.IsExperimentalHost = true + } + if matches["INSTANCEGROUP"] == "" { + c.Instance = experimentalHostInstance + } + c.Project = experimentalHostProject + } + return c, nil } func extractConnectorParams(paramsString string) (map[string]string, error) { @@ -671,6 +698,22 @@ func createConnector(d *Driver, connectorConfig ConnectorConfig) (*connector, er if connectorConfig.Configurator != nil { connectorConfig.Configurator(&config, &opts) } + if config.IsExperimentalHost { + var caCertFile string + var clientCertFile string + var clientCertKey string + assignPropertyValueIfExists(state, propertyCaCertFile, &caCertFile) + assignPropertyValueIfExists(state, propertyClientCertFile, &clientCertFile) + assignPropertyValueIfExists(state, propertyClientCertKey, &clientCertKey) + if caCertFile != "" { + credOpts, err := CreateExperimentalHostCredentials(caCertFile, clientCertFile, clientCertKey) + if err != nil { + return nil, err + } + opts = append(opts, credOpts) + opts = append(opts, option.WithoutAuthentication()) + } + } if connectorConfig.AutoConfigEmulator { if connectorConfig.Host == "" { connectorConfig.Host = "localhost:9010" @@ -1656,3 +1699,34 @@ func WithBatchReadOnly(level sql.IsolationLevel) sql.IsolationLevel { func withBatchReadOnly(level driver.IsolationLevel) driver.IsolationLevel { return driver.IsolationLevel(levelBatchReadOnly)<<8 + level } + +// CreateExperimentalHostCredentials is only supported for connecting to experimental +// hosts. It reads the provided CA certificate file and optionally the +// client certificate and key files to set up TLS or mutual TLS credentials, and +// creates gRPC dial options to connect to an experimental host endpoint. The method +// has been kept public to allow integration test to leverage it. +func CreateExperimentalHostCredentials(caCertFile, clientCertificateFile, clientCertificateKey string) (option.ClientOption, error) { + ca, err := os.ReadFile(caCertFile) + if err != nil { + return nil, fmt.Errorf("failed to read CA certificate file: %w", err) + } + capool := x509.NewCertPool() + if !capool.AppendCertsFromPEM(ca) { + return nil, fmt.Errorf("failed to append the CA certificate to CA pool") + } + if clientCertificateFile == "" || clientCertificateKey == "" { + // Setting up TLS with only the CA certificate. + creds := credentials.NewTLS(&tls.Config{RootCAs: capool}) + return option.WithGRPCDialOption(grpc.WithTransportCredentials(creds)), nil + } + // Setting up mutual TLS with both the CA certificate and client certificate. + cert, err := tls.LoadX509KeyPair(clientCertificateFile, clientCertificateKey) + if err != nil { + return nil, fmt.Errorf("failed to load client certificate/key: %w", err) + } + creds := credentials.NewTLS(&tls.Config{ + RootCAs: capool, + Certificates: []tls.Certificate{cert}, + }) + return option.WithGRPCDialOption(grpc.WithTransportCredentials(creds)), nil +} diff --git a/driver_test.go b/driver_test.go index 85853071..352c91ed 100644 --- a/driver_test.go +++ b/driver_test.go @@ -298,6 +298,98 @@ func TestExtractDnsParts(t *testing.T) { } } +func TestExpHostDSNRegExp(t *testing.T) { + tests := []struct { + name string + dsn string + wantMatch bool + want map[string]string + }{ + { + name: "host+instance+db", + dsn: "spanner://localhost:9010/instances/test-instance/databases/test-db", + wantMatch: true, + want: map[string]string{ + "HOSTGROUP": "localhost:9010", + "INSTANCEGROUP": "test-instance", + "DATABASEGROUP": "test-db", + "PARAMSGROUP": "", + }, + }, + { + name: "host+db", + dsn: "spanner://localhost/databases/testdb", + wantMatch: true, + want: map[string]string{ + "HOSTGROUP": "localhost", + "INSTANCEGROUP": "", + "DATABASEGROUP": "testdb", + "PARAMSGROUP": "", + }, + }, + { + name: "params", + dsn: "spanner://example.com/databases/testdb?usePlainText=true;foo=bar", + wantMatch: true, + want: map[string]string{ + "HOSTGROUP": "example.com", + "INSTANCEGROUP": "", + "DATABASEGROUP": "testdb", + "PARAMSGROUP": "usePlainText=true;foo=bar", + }, + }, + { + name: "short-db-fails", + dsn: "spanner://localhost/databases/d", + wantMatch: false, + }, + { + name: "uppercase-db-fails", + dsn: "spanner://localhost/databases/TestDB", + wantMatch: false, + }, + { + name: "ipv4-with-port", + dsn: "spanner://127.0.0.1:1234/databases/ab", + wantMatch: true, + want: map[string]string{ + "HOSTGROUP": "127.0.0.1:1234", + "INSTANCEGROUP": "", + "DATABASEGROUP": "ab", + "PARAMSGROUP": "", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := expHostDSNRegExp.FindStringSubmatch(tc.dsn) + if tc.wantMatch { + if m == nil { + t.Fatalf("expected match for %q, got none", tc.dsn) + } + // collect groups + out := map[string]string{} + for i, name := range expHostDSNRegExp.SubexpNames() { + if name == "" { + continue + } + out[name] = m[i] + } + for k, v := range tc.want { + if out[k] != v { + t.Errorf("for %q expected %s=%q got %q", tc.dsn, k, v, out[k]) + } + } + } else { + if m != nil { + t.Fatalf("expected no match for %q, but got %v", tc.dsn, m) + } + } + }) + } +} + func TestParseBeginTransactionOption(t *testing.T) { tests := []struct { input string diff --git a/integration_test.go b/integration_test.go index 30d21086..d899c42c 100644 --- a/integration_test.go +++ b/integration_test.go @@ -42,13 +42,25 @@ import ( "cloud.google.com/go/spanner/admin/instance/apiv1/instancepb" "github.com/google/go-cmp/cmp" "google.golang.org/api/iterator" + "google.golang.org/api/option" + "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" ) var projectId, instanceId string var skipped bool +var experimentalHost string +var caCertFile string +var clientCertFile string +var clientCertKey string func init() { + flag.StringVar(&experimentalHost, "it.experimental-host", "", "Experimental host integration test flag") + flag.StringVar(&caCertFile, "it.ca-cert-file", "", "CA certificate file for experimental host integration test") + flag.StringVar(&clientCertFile, "it.client-cert-file", "", "Client certificate file for experimental host integration test") + flag.StringVar(&clientCertKey, "it.client-cert-key", "", "Client certificate key file for experimental host integration test") + var ok bool // Get environment variables or set to default. @@ -153,12 +165,32 @@ func initTestInstance(config string) (cleanup func(), err error) { }, nil } +func createExpHostDBAdminClient(ctx context.Context) (*database.DatabaseAdminClient, error) { + opts := []option.ClientOption{ + option.WithEndpoint(experimentalHost), + option.WithoutAuthentication(), + } + if caCertFile != "" { + credOpts, err := CreateExperimentalHostCredentials(caCertFile, clientCertFile, clientCertKey) + if err != nil { + return nil, err + } + opts = append(opts, credOpts) + } else { + opts = append(opts, option.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials()))) + } + return database.NewDatabaseAdminClient(ctx, opts...) +} + func createTestDB(ctx context.Context, statements ...string) (dsn string, cleanup func(), err error) { return createTestDBWithDialect(ctx, databasepb.DatabaseDialect_GOOGLE_STANDARD_SQL, statements...) } func createTestDBWithDialect(ctx context.Context, dialect databasepb.DatabaseDialect, statements ...string) (dsn string, cleanup func(), err error) { databaseAdminClient, err := database.NewDatabaseAdminClient(ctx) + if experimentalHost != "" { + databaseAdminClient, err = createExpHostDBAdminClient(ctx) + } if err != nil { return "", nil, err } @@ -202,8 +234,22 @@ func createTestDBWithDialect(ctx context.Context, dialect databasepb.DatabaseDia } } dsn = "projects/" + projectId + "/instances/" + instanceId + "/databases/" + databaseId + if experimentalHost != "" { + dsn = "spanner://" + experimentalHost + "/databases/" + databaseId + if caCertFile == "" { + dsn += "?use_plain_text=true" + } else { + dsn += "?ca_cert_file=" + caCertFile + if clientCertFile != "" && clientCertKey != "" { + dsn += ";client_cert_file=" + clientCertFile + ";client_cert_key=" + clientCertKey + } + } + } cleanup = func() { databaseAdminClient, err := database.NewDatabaseAdminClient(ctx) + if experimentalHost != "" { + databaseAdminClient, err = createExpHostDBAdminClient(ctx) + } if err != nil { return } @@ -219,6 +265,13 @@ func initIntegrationTests() (cleanup func(), err error) { flag.Parse() // Needed for testing.Short(). noop := func() {} + if experimentalHost != "" { + projectId = "default" + instanceId = "default" + // instance management is not avaialble on experimental host + return noop, nil + } + if testing.Short() { log.Println("Integration tests skipped in -short mode.") return noop, nil From 92e02d5af24954e18a1a75e35c997695bbde8ad0 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Wed, 14 Jan 2026 06:32:08 +0000 Subject: [PATCH 2/6] added changes suggested by gemini review --- driver.go | 36 ++++++++++++++++++++---------------- integration_test.go | 17 +++++++---------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/driver.go b/driver.go index 51ee02f2..6c9eab72 100644 --- a/driver.go +++ b/driver.go @@ -706,7 +706,7 @@ func createConnector(d *Driver, connectorConfig ConnectorConfig) (*connector, er assignPropertyValueIfExists(state, propertyClientCertFile, &clientCertFile) assignPropertyValueIfExists(state, propertyClientCertKey, &clientCertKey) if caCertFile != "" { - credOpts, err := CreateExperimentalHostCredentials(caCertFile, clientCertFile, clientCertKey) + credOpts, err := createExperimentalHostCredentials(caCertFile, clientCertFile, clientCertKey) if err != nil { return nil, err } @@ -1700,12 +1700,11 @@ func withBatchReadOnly(level driver.IsolationLevel) driver.IsolationLevel { return driver.IsolationLevel(levelBatchReadOnly)<<8 + level } -// CreateExperimentalHostCredentials is only supported for connecting to experimental +// createExperimentalHostCredentials is only supported for connecting to experimental // hosts. It reads the provided CA certificate file and optionally the // client certificate and key files to set up TLS or mutual TLS credentials, and -// creates gRPC dial options to connect to an experimental host endpoint. The method -// has been kept public to allow integration test to leverage it. -func CreateExperimentalHostCredentials(caCertFile, clientCertificateFile, clientCertificateKey string) (option.ClientOption, error) { +// creates gRPC dial options to connect to an experimental host endpoint. +func createExperimentalHostCredentials(caCertFile, clientCertificateFile, clientCertificateKey string) (option.ClientOption, error) { ca, err := os.ReadFile(caCertFile) if err != nil { return nil, fmt.Errorf("failed to read CA certificate file: %w", err) @@ -1714,19 +1713,24 @@ func CreateExperimentalHostCredentials(caCertFile, clientCertificateFile, client if !capool.AppendCertsFromPEM(ca) { return nil, fmt.Errorf("failed to append the CA certificate to CA pool") } - if clientCertificateFile == "" || clientCertificateKey == "" { - // Setting up TLS with only the CA certificate. - creds := credentials.NewTLS(&tls.Config{RootCAs: capool}) + + if clientCertificateFile != "" && clientCertificateKey != "" { + // Setting up mutual TLS with both the CA certificate and client certificate. + cert, err := tls.LoadX509KeyPair(clientCertificateFile, clientCertificateKey) + if err != nil { + return nil, fmt.Errorf("failed to load client certificate/key: %w", err) + } + creds := credentials.NewTLS(&tls.Config{ + RootCAs: capool, + Certificates: []tls.Certificate{cert}, + }) return option.WithGRPCDialOption(grpc.WithTransportCredentials(creds)), nil } - // Setting up mutual TLS with both the CA certificate and client certificate. - cert, err := tls.LoadX509KeyPair(clientCertificateFile, clientCertificateKey) - if err != nil { - return nil, fmt.Errorf("failed to load client certificate/key: %w", err) + if clientCertificateFile != "" || clientCertificateKey != "" { + return nil, fmt.Errorf("both client certificate and key must be provided for mTLS, but only one was provided") } - creds := credentials.NewTLS(&tls.Config{ - RootCAs: capool, - Certificates: []tls.Certificate{cert}, - }) + + // Setting up TLS with only the CA certificate. + creds := credentials.NewTLS(&tls.Config{RootCAs: capool}) return option.WithGRPCDialOption(grpc.WithTransportCredentials(creds)), nil } diff --git a/integration_test.go b/integration_test.go index d899c42c..b487c159 100644 --- a/integration_test.go +++ b/integration_test.go @@ -165,13 +165,16 @@ func initTestInstance(config string) (cleanup func(), err error) { }, nil } -func createExpHostDBAdminClient(ctx context.Context) (*database.DatabaseAdminClient, error) { +func createDBAdminClient(ctx context.Context) (*database.DatabaseAdminClient, error) { + if experimentalHost == "" { + return database.NewDatabaseAdminClient(ctx) + } opts := []option.ClientOption{ option.WithEndpoint(experimentalHost), option.WithoutAuthentication(), } if caCertFile != "" { - credOpts, err := CreateExperimentalHostCredentials(caCertFile, clientCertFile, clientCertKey) + credOpts, err := createExperimentalHostCredentials(caCertFile, clientCertFile, clientCertKey) if err != nil { return nil, err } @@ -187,10 +190,7 @@ func createTestDB(ctx context.Context, statements ...string) (dsn string, cleanu } func createTestDBWithDialect(ctx context.Context, dialect databasepb.DatabaseDialect, statements ...string) (dsn string, cleanup func(), err error) { - databaseAdminClient, err := database.NewDatabaseAdminClient(ctx) - if experimentalHost != "" { - databaseAdminClient, err = createExpHostDBAdminClient(ctx) - } + databaseAdminClient, err := createDBAdminClient(ctx) if err != nil { return "", nil, err } @@ -246,10 +246,7 @@ func createTestDBWithDialect(ctx context.Context, dialect databasepb.DatabaseDia } } cleanup = func() { - databaseAdminClient, err := database.NewDatabaseAdminClient(ctx) - if experimentalHost != "" { - databaseAdminClient, err = createExpHostDBAdminClient(ctx) - } + databaseAdminClient, err := createDBAdminClient(ctx) if err != nil { return } From caf2cf476aea95bba2cf403001883a4c73c0b632 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Wed, 14 Jan 2026 06:39:00 +0000 Subject: [PATCH 3/6] used constants for default project and instance ids --- integration_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration_test.go b/integration_test.go index b487c159..87ff3657 100644 --- a/integration_test.go +++ b/integration_test.go @@ -263,8 +263,8 @@ func initIntegrationTests() (cleanup func(), err error) { noop := func() {} if experimentalHost != "" { - projectId = "default" - instanceId = "default" + projectId = experimentalHostProject + instanceId = experimentalHostInstance // instance management is not avaialble on experimental host return noop, nil } From a85d1a9790fcbb334575f31577b990559494f699 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Wed, 14 Jan 2026 06:39:59 +0000 Subject: [PATCH 4/6] spell check --- integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_test.go b/integration_test.go index 87ff3657..ef18da60 100644 --- a/integration_test.go +++ b/integration_test.go @@ -265,7 +265,7 @@ func initIntegrationTests() (cleanup func(), err error) { if experimentalHost != "" { projectId = experimentalHostProject instanceId = experimentalHostInstance - // instance management is not avaialble on experimental host + // instance management is not available on experimental host return noop, nil } From da585ce630f0f5d58d9e6011ea0639a43f864967 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Fri, 16 Jan 2026 11:48:53 +0000 Subject: [PATCH 5/6] reuse regex for all endpoints --- connection_properties.go | 11 ++++ driver.go | 27 ++++---- driver_test.go | 134 ++++++++++++++++++++------------------- integration_test.go | 3 +- 4 files changed, 95 insertions(+), 80 deletions(-) diff --git a/connection_properties.go b/connection_properties.go index 690e5369..10631486 100644 --- a/connection_properties.go +++ b/connection_properties.go @@ -557,6 +557,17 @@ var propertyCommitResponse = createReadOnlyConnectionProperty( connectionstate.ContextUser, ) +var propertyIsExperimentalHost = createConnectionProperty( + "is_experimental_host", + "Indicates whether the connection is to an experimental host endpoint (true/false). "+ + "Set this value to true when connecting to an experimental host endpoint", + false, + false, + nil, + connectionstate.ContextStartup, + connectionstate.ConvertBool, +) + var propertyCaCertFile = createConnectionProperty( "ca_cert_file", "The path to the CA certificate file to use for TLS connections to the server. "+ diff --git a/driver.go b/driver.go index 6c9eab72..e1628749 100644 --- a/driver.go +++ b/driver.go @@ -101,8 +101,7 @@ var noopLogger = slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{L // - rpcPriority: Sets the priority for all RPC invocations from this connection (HIGH/MEDIUM/LOW). The default is HIGH. // // Example: `localhost:9010/projects/test-project/instances/test-instance/databases/test-database;usePlainText=true;disableRouteToLeader=true;enableEndToEndTracing=true` -var dsnRegExp = regexp.MustCompile(`((?P[\w.-]+(?:\.[\w\.-]+)*[\w\-\._~:/?#\[\]@!\$&'\(\)\*\+,;=.]+)/)?projects/(?P(([a-z]|[-.:]|[0-9])+|(DEFAULT_PROJECT_ID)))(/instances/(?P([a-z]|[-]|[0-9])+)(/databases/(?P([a-z]|[-]|[_]|[0-9])+))?)?(([\?|;])(?P.*))?`) -var expHostDSNRegExp = regexp.MustCompile(`spanner://(?P[\w.-]+(?::\d+)?)(?:/instances/(?P[a-z0-9-]+))?/databases/(?P[a-z][a-z0-9_-]{0,28}[a-z0-9])(?:[?;](?P.*))?`) +var dsnRegExp = regexp.MustCompile(`^((?P[\w.-]+(?:\.[\w\.-]+)*[\w\-\._~:#\[\]@!\$&'\(\)\*\+,=.]+)/)?(projects/(?P(([a-z]|[-.:]|[0-9])+|(DEFAULT_PROJECT_ID))))?((?:/)?instances/(?P([a-z]|[-]|[0-9])+))?((?:/)?databases/(?P([a-z]|[-]|[_]|[0-9])+))?(([\?|;])(?P.*))?$`) var _ driver.DriverContext = &Driver{} var spannerDriver *Driver @@ -488,21 +487,11 @@ func (cc *ConnectorConfig) String() string { // data source name. func ExtractConnectorConfig(dsn string) (ConnectorConfig, error) { match := dsnRegExp.FindStringSubmatch(dsn) - isExpHost := false - - if strings.HasPrefix(dsn, "spanner://") { - isExpHost = true - match = expHostDSNRegExp.FindStringSubmatch(dsn) - } if match == nil { return ConnectorConfig{}, spanner.ToSpannerError(status.Errorf(codes.InvalidArgument, "invalid connection string: %s", dsn)) } matches := make(map[string]string) - names := dsnRegExp.SubexpNames() - if isExpHost { - names = expHostDSNRegExp.SubexpNames() - } - for i, name := range names { + for i, name := range dsnRegExp.SubexpNames() { if i != 0 && name != "" { matches[name] = match[i] } @@ -521,7 +510,10 @@ func ExtractConnectorConfig(dsn string) (ConnectorConfig, error) { Params: params, name: dsn, } - if isExpHost { + if params[propertyIsExperimentalHost.Key()] == "true" { + if c.Host == "" { + return ConnectorConfig{}, spanner.ToSpannerError(status.Errorf(codes.InvalidArgument, "host must be specified for experimental host endpoint")) + } c.Configurator = func(config *spanner.ClientConfig, opts *[]option.ClientOption) { config.IsExperimentalHost = true } @@ -529,6 +521,13 @@ func ExtractConnectorConfig(dsn string) (ConnectorConfig, error) { c.Instance = experimentalHostInstance } c.Project = experimentalHostProject + } else { + if c.Project == "" { + return ConnectorConfig{}, spanner.ToSpannerError(status.Errorf(codes.InvalidArgument, "project must be specified in connection string")) + } + if c.Instance == "" { + return ConnectorConfig{}, spanner.ToSpannerError(status.Errorf(codes.InvalidArgument, "instance must be specified in connection string")) + } } return c, nil } diff --git a/driver_test.go b/driver_test.go index 352c91ed..1b2dd10d 100644 --- a/driver_test.go +++ b/driver_test.go @@ -298,92 +298,96 @@ func TestExtractDnsParts(t *testing.T) { } } -func TestExpHostDSNRegExp(t *testing.T) { +func TestExperimentalHostDsn(t *testing.T) { + //goland:noinspection GoDeprecation tests := []struct { - name string - dsn string - wantMatch bool - want map[string]string + name string + dsn string + wantConnectorConfig ConnectorConfig + wantErr bool }{ { - name: "host+instance+db", - dsn: "spanner://localhost:9010/instances/test-instance/databases/test-db", - wantMatch: true, - want: map[string]string{ - "HOSTGROUP": "localhost:9010", - "INSTANCEGROUP": "test-instance", - "DATABASEGROUP": "test-db", - "PARAMSGROUP": "", + name: "no-project", + dsn: "localhost:9010/instances/test-instance/databases/test-db?is_experimental_host=true", + wantConnectorConfig: ConnectorConfig{ + Host: "localhost:9010", + Project: "default", + Instance: "test-instance", + Database: "test-db", + Params: map[string]string{ + "is_experimental_host": "true", + }, }, }, { - name: "host+db", - dsn: "spanner://localhost/databases/testdb", - wantMatch: true, - want: map[string]string{ - "HOSTGROUP": "localhost", - "INSTANCEGROUP": "", - "DATABASEGROUP": "testdb", - "PARAMSGROUP": "", + name: "invalid-project", + dsn: "localhost:9010/projects/test-project/instances/test-instance/databases/test-db?is_experimental_host=true", + wantConnectorConfig: ConnectorConfig{ + Host: "localhost:9010", + Project: "default", + Instance: "test-instance", + Database: "test-db", + Params: map[string]string{ + "is_experimental_host": "true", + }, }, }, { - name: "params", - dsn: "spanner://example.com/databases/testdb?usePlainText=true;foo=bar", - wantMatch: true, - want: map[string]string{ - "HOSTGROUP": "example.com", - "INSTANCEGROUP": "", - "DATABASEGROUP": "testdb", - "PARAMSGROUP": "usePlainText=true;foo=bar", + name: "only-database", + dsn: "localhost:9010/databases/test-db?is_experimental_host=true", + wantConnectorConfig: ConnectorConfig{ + Host: "localhost:9010", + Project: "default", + Instance: "default", + Database: "test-db", + Params: map[string]string{ + "is_experimental_host": "true", + }, }, }, { - name: "short-db-fails", - dsn: "spanner://localhost/databases/d", - wantMatch: false, + name: "only-database", + dsn: "localhost:9010/databases/test-db?is_experimental_host=true", + wantConnectorConfig: ConnectorConfig{ + Host: "localhost:9010", + Project: "default", + Instance: "default", + Database: "test-db", + Params: map[string]string{ + "is_experimental_host": "true", + }, + }, }, { - name: "uppercase-db-fails", - dsn: "spanner://localhost/databases/TestDB", - wantMatch: false, + name: "absent-host", + dsn: "databases/test-db?is_experimental_host=true", + wantErr: true, }, { - name: "ipv4-with-port", - dsn: "spanner://127.0.0.1:1234/databases/ab", - wantMatch: true, - want: map[string]string{ - "HOSTGROUP": "127.0.0.1:1234", - "INSTANCEGROUP": "", - "DATABASEGROUP": "ab", - "PARAMSGROUP": "", - }, + name: "project-mandatory-cloud-spanner", + dsn: "localhost:9010/instances/test-instance/databases/test-db", + wantErr: true, + }, + { + name: "instance-mandatory-cloud-spanner", + dsn: "localhost:9010/projects/test-project/databases/test-db", + wantErr: true, }, } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - m := expHostDSNRegExp.FindStringSubmatch(tc.dsn) - if tc.wantMatch { - if m == nil { - t.Fatalf("expected match for %q, got none", tc.dsn) - } - // collect groups - out := map[string]string{} - for i, name := range expHostDSNRegExp.SubexpNames() { - if name == "" { - continue - } - out[name] = m[i] - } - for k, v := range tc.want { - if out[k] != v { - t.Errorf("for %q expected %s=%q got %q", tc.dsn, k, v, out[k]) - } + t.Run(tc.dsn, func(t *testing.T) { + config, err := ExtractConnectorConfig(tc.dsn) + if err != nil { + if tc.wantErr { + return } + t.Errorf("%q: extract failed for %q: %v", tc.name, tc.dsn, err) } else { - if m != nil { - t.Fatalf("expected no match for %q, but got %v", tc.dsn, m) + if tc.wantErr { + t.Errorf("%q: did not encounter expected error", tc.name) + } + if diff := cmp.Diff(config.Params, tc.wantConnectorConfig.Params); diff != "" { + t.Errorf("%q: connector config mismatch for %q\n%v", tc.name, tc.dsn, diff) } } }) diff --git a/integration_test.go b/integration_test.go index ef18da60..984d450a 100644 --- a/integration_test.go +++ b/integration_test.go @@ -235,7 +235,7 @@ func createTestDBWithDialect(ctx context.Context, dialect databasepb.DatabaseDia } dsn = "projects/" + projectId + "/instances/" + instanceId + "/databases/" + databaseId if experimentalHost != "" { - dsn = "spanner://" + experimentalHost + "/databases/" + databaseId + dsn = experimentalHost + "/databases/" + databaseId if caCertFile == "" { dsn += "?use_plain_text=true" } else { @@ -244,6 +244,7 @@ func createTestDBWithDialect(ctx context.Context, dialect databasepb.DatabaseDia dsn += ";client_cert_file=" + clientCertFile + ";client_cert_key=" + clientCertKey } } + dsn += ";is_experimental_host=true" } cleanup = func() { databaseAdminClient, err := createDBAdminClient(ctx) From 58cdbcc8638490e09e61dcf804fe2345768a15dc Mon Sep 17 00:00:00 2001 From: sagnghos Date: Fri, 16 Jan 2026 13:57:04 +0000 Subject: [PATCH 6/6] fixed typo in test --- connection_properties.go | 47 +++++++++---------- driver.go | 2 +- .../spannerlib-python/tests/system/_helper.py | 2 +- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/connection_properties.go b/connection_properties.go index 10631486..8a6aa461 100644 --- a/connection_properties.go +++ b/connection_properties.go @@ -535,28 +535,6 @@ var propertyConnectTimeout = createConnectionProperty( connectionstate.ConvertDuration, ) -// Generated read-only properties. These cannot be set by the user anywhere. -var propertyCommitTimestamp = createReadOnlyConnectionProperty( - "commit_timestamp", - "The commit timestamp of the last implicit or explicit read/write transaction that "+ - "was executed on the connection, or an error if the connection has not executed a read/write transaction "+ - "that committed successfully. The timestamp is in the local timezone.", - (*time.Time)(nil), - false, - nil, - connectionstate.ContextUser, -) -var propertyCommitResponse = createReadOnlyConnectionProperty( - "commit_response", - "The commit response of the last implicit or explicit read/write transaction that "+ - "was executed on the connection, or an error if the connection has not executed a read/write transaction "+ - "that committed successfully.", - (*spanner.CommitResponse)(nil), - false, - nil, - connectionstate.ContextUser, -) - var propertyIsExperimentalHost = createConnectionProperty( "is_experimental_host", "Indicates whether the connection is to an experimental host endpoint (true/false). "+ @@ -567,7 +545,6 @@ var propertyIsExperimentalHost = createConnectionProperty( connectionstate.ContextStartup, connectionstate.ConvertBool, ) - var propertyCaCertFile = createConnectionProperty( "ca_cert_file", "The path to the CA certificate file to use for TLS connections to the server. "+ @@ -578,7 +555,6 @@ var propertyCaCertFile = createConnectionProperty( connectionstate.ContextStartup, connectionstate.ConvertString, ) - var propertyClientCertFile = createConnectionProperty( "client_cert_file", "The path to the client certificate file to use for mTLS connections to the server. "+ @@ -589,7 +565,6 @@ var propertyClientCertFile = createConnectionProperty( connectionstate.ContextStartup, connectionstate.ConvertString, ) - var propertyClientCertKey = createConnectionProperty( "client_cert_key", "The path to the client certificate key file to use for mTLS connections to the server. "+ @@ -601,6 +576,28 @@ var propertyClientCertKey = createConnectionProperty( connectionstate.ConvertString, ) +// Generated read-only properties. These cannot be set by the user anywhere. +var propertyCommitTimestamp = createReadOnlyConnectionProperty( + "commit_timestamp", + "The commit timestamp of the last implicit or explicit read/write transaction that "+ + "was executed on the connection, or an error if the connection has not executed a read/write transaction "+ + "that committed successfully. The timestamp is in the local timezone.", + (*time.Time)(nil), + false, + nil, + connectionstate.ContextUser, +) +var propertyCommitResponse = createReadOnlyConnectionProperty( + "commit_response", + "The commit response of the last implicit or explicit read/write transaction that "+ + "was executed on the connection, or an error if the connection has not executed a read/write transaction "+ + "that committed successfully.", + (*spanner.CommitResponse)(nil), + false, + nil, + connectionstate.ContextUser, +) + func createReadOnlyConnectionProperty[T comparable](name, description string, defaultValue T, hasDefaultValue bool, validValues []T, context connectionstate.Context) *connectionstate.TypedConnectionProperty[T] { converter := connectionstate.CreateReadOnlyConverter[T](name) return createConnectionProperty(name, description, defaultValue, hasDefaultValue, validValues, context, converter) diff --git a/driver.go b/driver.go index e1628749..1193c67c 100644 --- a/driver.go +++ b/driver.go @@ -510,7 +510,7 @@ func ExtractConnectorConfig(dsn string) (ConnectorConfig, error) { Params: params, name: dsn, } - if params[propertyIsExperimentalHost.Key()] == "true" { + if strings.EqualFold(params[propertyIsExperimentalHost.Key()], "true") { if c.Host == "" { return ConnectorConfig{}, spanner.ToSpannerError(status.Errorf(codes.InvalidArgument, "host must be specified for experimental host endpoint")) } diff --git a/spannerlib/wrappers/spannerlib-python/spannerlib-python/tests/system/_helper.py b/spannerlib/wrappers/spannerlib-python/spannerlib-python/tests/system/_helper.py index 2f0046cb..b762a2f1 100644 --- a/spannerlib/wrappers/spannerlib-python/spannerlib-python/tests/system/_helper.py +++ b/spannerlib/wrappers/spannerlib-python/spannerlib-python/tests/system/_helper.py @@ -40,7 +40,7 @@ ) EMULATOR_TEST_CONNECTION_STRING = ( - f"{SPANNER_EMULATOR_HOST}" + f"{SPANNER_EMULATOR_HOST}/" f"projects/{PROJECT_ID}" f"/instances/{INSTANCE_ID}" f"/databases/{DATABASE_ID}"