Skip to content

Commit

Permalink
fix(all): Update hand-written clients to not use WithEndpoint override (
Browse files Browse the repository at this point in the history
#3111)

-logging clients delegate to the gapic generated client with the correct defaults, so no need to override.
-bigtable and spanner clients needs to be manually updated to use DefaultEndpoint and DefaultMTLSEndpoint
  • Loading branch information
andyrzhao authored Nov 6, 2020
1 parent 751afe6 commit f0cfd05
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 11 deletions.
6 changes: 4 additions & 2 deletions bigtable/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
)

const adminAddr = "bigtableadmin.googleapis.com:443"
const mtlsAdminAddr = "bigtableadmin.mtls.googleapis.com:443"

// ErrPartiallyUnavailable is returned when some locations (clusters) are
// unavailable. Both partial results (retrieved from available locations)
Expand All @@ -70,7 +71,7 @@ type AdminClient struct {

// NewAdminClient creates a new AdminClient for a given project and instance.
func NewAdminClient(ctx context.Context, project, instance string, opts ...option.ClientOption) (*AdminClient, error) {
o, err := btopt.DefaultClientOptions(adminAddr, AdminScope, clientUserAgent)
o, err := btopt.DefaultClientOptions(adminAddr, mtlsAdminAddr, AdminScope, clientUserAgent)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -603,6 +604,7 @@ func (ac *AdminClient) TableIAM(tableID string) *iam.Handle {
}

const instanceAdminAddr = "bigtableadmin.googleapis.com:443"
const mtlsInstanceAdminAddr = "bigtableadmin.mtls.googleapis.com:443"

// InstanceAdminClient is a client type for performing admin operations on instances.
// These operations can be substantially more dangerous than those provided by AdminClient.
Expand All @@ -619,7 +621,7 @@ type InstanceAdminClient struct {

// NewInstanceAdminClient creates a new InstanceAdminClient for a given project.
func NewInstanceAdminClient(ctx context.Context, project string, opts ...option.ClientOption) (*InstanceAdminClient, error) {
o, err := btopt.DefaultClientOptions(instanceAdminAddr, InstanceAdminScope, clientUserAgent)
o, err := btopt.DefaultClientOptions(instanceAdminAddr, mtlsInstanceAdminAddr, InstanceAdminScope, clientUserAgent)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion bigtable/bigtable.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
)

const prodAddr = "bigtable.googleapis.com:443"
const mtlsProdAddr = "bigtable.mtls.googleapis.com:443"

// Client is a client for reading and writing data to tables in an instance.
//
Expand All @@ -65,7 +66,7 @@ func NewClient(ctx context.Context, project, instance string, opts ...option.Cli

// NewClientWithConfig creates a new client with the given config.
func NewClientWithConfig(ctx context.Context, project, instance string, config ClientConfig, opts ...option.ClientOption) (*Client, error) {
o, err := btopt.DefaultClientOptions(prodAddr, Scope, clientUserAgent)
o, err := btopt.DefaultClientOptions(prodAddr, mtlsProdAddr, Scope, clientUserAgent)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions bigtable/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ var headersInterceptor = testutil.DefaultHeadersEnforcer()

// NewAdminClient builds a new connected admin client for this environment
func (e *EmulatedEnv) NewAdminClient() (*AdminClient, error) {
o, err := btopt.DefaultClientOptions(e.server.Addr, AdminScope, clientUserAgent)
o, err := btopt.DefaultClientOptions(e.server.Addr, e.server.Addr, AdminScope, clientUserAgent)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -170,7 +170,7 @@ func (e *EmulatedEnv) NewInstanceAdminClient() (*InstanceAdminClient, error) {

// NewClient builds a new connected data client for this environment
func (e *EmulatedEnv) NewClient() (*Client, error) {
o, err := btopt.DefaultClientOptions(e.server.Addr, Scope, clientUserAgent)
o, err := btopt.DefaultClientOptions(e.server.Addr, e.server.Addr, Scope, clientUserAgent)
if err != nil {
return nil, err
}
Expand Down
6 changes: 4 additions & 2 deletions bigtable/internal/option/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"cloud.google.com/go/internal/version"
gax "github.com/googleapis/gax-go/v2"
"google.golang.org/api/option"
"google.golang.org/api/option/internaloption"
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"
)
Expand Down Expand Up @@ -76,7 +77,7 @@ func unaryInterceptor(ctx context.Context, method string, req, reply interface{}

// DefaultClientOptions returns the default client options to use for the
// client's gRPC connection.
func DefaultClientOptions(endpoint, scope, userAgent string) ([]option.ClientOption, error) {
func DefaultClientOptions(endpoint, mtlsEndpoint, scope, userAgent string) ([]option.ClientOption, error) {
var o []option.ClientOption
// Check the environment variables for the bigtable emulator.
// Dial it directly and don't pass any credentials.
Expand All @@ -88,7 +89,8 @@ func DefaultClientOptions(endpoint, scope, userAgent string) ([]option.ClientOpt
o = []option.ClientOption{option.WithGRPCConn(conn)}
} else {
o = []option.ClientOption{
option.WithEndpoint(endpoint),
internaloption.WithDefaultEndpoint(endpoint),
internaloption.WithDefaultMTLSEndpoint(mtlsEndpoint),
option.WithScopes(scope),
option.WithUserAgent(userAgent),
}
Expand Down
1 change: 0 additions & 1 deletion logging/logadmin/logadmin.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func NewClient(ctx context.Context, parent string, opts ...option.ClientOption)
parent = "projects/" + parent
}
opts = append([]option.ClientOption{
option.WithEndpoint(internal.ProdAddr),
option.WithScopes(logging.AdminScope),
}, opts...)
lc, err := vkit.NewClient(ctx, opts...)
Expand Down
1 change: 0 additions & 1 deletion logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ func NewClient(ctx context.Context, parent string, opts ...option.ClientOption)
parent = "projects/" + parent
}
opts = append([]option.ClientOption{
option.WithEndpoint(internal.ProdAddr),
option.WithScopes(WriteScope),
}, opts...)
c, err := vkit.NewClient(ctx, opts...)
Expand Down
6 changes: 4 additions & 2 deletions spanner/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ import (
)

const (
endpoint = "spanner.googleapis.com:443"
endpoint = "spanner.googleapis.com:443"
mtlsEndpoint = "spanner.mtls.googleapis.com:443"

// resourcePrefixHeader is the name of the metadata header used to indicate
// the resource being operated on.
Expand Down Expand Up @@ -166,7 +167,8 @@ func NewClientWithConfig(ctx context.Context, database string, config ClientConf
}
// gRPC options.
allOpts := []option.ClientOption{
option.WithEndpoint(endpoint),
internaloption.WithDefaultEndpoint(endpoint),
internaloption.WithDefaultMTLSEndpoint(mtlsEndpoint),
option.WithScopes(Scope),
option.WithGRPCDialOption(
grpc.WithDefaultCallOptions(
Expand Down

0 comments on commit f0cfd05

Please sign in to comment.