From 7ac3bd614dfe065a802d4d2884f879e8ec5c0e7a Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Thu, 20 Sep 2018 16:17:51 -0700 Subject: [PATCH] fix review comments --- balancer/grpclb/grpclb_remote_balancer.go | 2 +- credentials/google/google.go | 6 ++++-- dialoptions.go | 6 +++--- internal/transport/http2_client.go | 12 ++++++------ interop/client/client.go | 2 +- test/creds_test.go | 6 +++--- 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/balancer/grpclb/grpclb_remote_balancer.go b/balancer/grpclb/grpclb_remote_balancer.go index 4195b2c03cb1..7410329046c2 100644 --- a/balancer/grpclb/grpclb_remote_balancer.go +++ b/balancer/grpclb/grpclb_remote_balancer.go @@ -272,7 +272,7 @@ func (lb *lbBalancer) dialRemoteLB(remoteLBName string) { dopts = append(dopts, grpc.WithInsecure()) } } else if bundle := lb.grpclbClientConnCreds; bundle != nil { - dopts = append(dopts, grpc.WithCreds(bundle)) + dopts = append(dopts, grpc.WithCredentialsBundle(bundle)) } else { dopts = append(dopts, grpc.WithInsecure()) } diff --git a/credentials/google/google.go b/credentials/google/google.go index 6e53ab95c921..6197934d2687 100644 --- a/credentials/google/google.go +++ b/credentials/google/google.go @@ -86,11 +86,13 @@ func (c *creds) SwitchMode(mode string) (credentials.Bundle, error) { newCreds.transportCreds = credentials.NewTLS(nil) case internal.CredsBundleModeALTS, internal.CredsBundleModeGRPCLB: if !vmOnGCP { - return nil, errors.New("ALTS, as part of google default credentials, is only supported on GCP") + return nil, errors.New("google default creds: ALTS, as part of google default credentials, is only supported on GCP") } + // Only the clients can use google default credentials, so we only need + // to create new ALTS client creds here. newCreds.transportCreds = alts.NewClientCreds(alts.DefaultClientOptions()) default: - return nil, fmt.Errorf("unsupported mode: %v", mode) + return nil, fmt.Errorf("google default creds: unsupported mode: %v", mode) } // Create per RPC credentials. diff --git a/dialoptions.go b/dialoptions.go index 3d8deb5a2ffb..bc32329b9695 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -302,12 +302,12 @@ func WithPerRPCCredentials(creds credentials.PerRPCCredentials) DialOption { }) } -// WithCreds returns a DialOption to set a credentials bundle for the -// ClientConn.WithCreds. This should not be used together with +// WithCredentialsBundle returns a DialOption to set a credentials bundle for +// the ClientConn.WithCreds. This should not be used together with // WithTransportCredentials. // // This API is experimental. -func WithCreds(b credentials.Bundle) DialOption { +func WithCredentialsBundle(b credentials.Bundle) DialOption { return newFuncDialOption(func(o *dialOptions) { o.copts.CredsBundle = b }) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index 9e2653ede809..15c271a1de78 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -169,21 +169,21 @@ func newHTTP2Client(connectCtx, ctx context.Context, addr TargetInfo, opts Conne isSecure bool authInfo credentials.AuthInfo ) - var creds credentials.TransportCredentials + var transportCreds credentials.TransportCredentials perRPCCreds := opts.PerRPCCredentials if b := opts.CredsBundle; b != nil { - creds = b.TransportCredentials() + transportCreds = b.TransportCredentials() if t := b.PerRPCCredentials(); t != nil { perRPCCreds = append(perRPCCreds, t) } } - if creds == nil { - creds = opts.TransportCredentials + if transportCreds == nil { + transportCreds = opts.TransportCredentials } - if creds != nil { + if transportCreds != nil { scheme = "https" - conn, authInfo, err = creds.ClientHandshake(connectCtx, addr.Authority, conn) + conn, authInfo, err = transportCreds.ClientHandshake(connectCtx, addr.Authority, conn) if err != nil { return nil, connectionErrorf(isTemporary(err), err, "transport: authentication handshake failed: %v", err) } diff --git a/interop/client/client.go b/interop/client/client.go index ae42ad61797e..a6eb2cc31517 100644 --- a/interop/client/client.go +++ b/interop/client/client.go @@ -107,7 +107,7 @@ func main() { altsTC := alts.NewClientCreds(altsOpts) opts = append(opts, grpc.WithTransportCredentials(altsTC)) } else if *useGoogleDefaultCreds { - opts = append(opts, grpc.WithCreds(google.NewDefaultCredentials())) + opts = append(opts, grpc.WithCredentialsBundle(google.NewDefaultCredentials())) } else { opts = append(opts, grpc.WithInsecure()) } diff --git a/test/creds_test.go b/test/creds_test.go index d3c39b31b12c..77c13b7df9b1 100644 --- a/test/creds_test.go +++ b/test/creds_test.go @@ -70,7 +70,7 @@ func TestCredsBundleBoth(t *testing.T) { te := newTest(t, env{name: "creds-bundle", network: "tcp", balancer: "v1", security: "empty"}) te.tapHandle = authHandle te.customDialOptions = []grpc.DialOption{ - grpc.WithCreds(&testCredsBundle{t: t}), + grpc.WithCredentialsBundle(&testCredsBundle{t: t}), } creds, err := credentials.NewServerTLSFromFile(testdata.Path("server1.pem"), testdata.Path("server1.key")) if err != nil { @@ -93,7 +93,7 @@ func TestCredsBundleTransportCredentials(t *testing.T) { defer leakcheck.Check(t) te := newTest(t, env{name: "creds-bundle", network: "tcp", balancer: "v1", security: "empty"}) te.customDialOptions = []grpc.DialOption{ - grpc.WithCreds(&testCredsBundle{t: t, mode: bundleTLSOnly}), + grpc.WithCredentialsBundle(&testCredsBundle{t: t, mode: bundleTLSOnly}), } creds, err := credentials.NewServerTLSFromFile(testdata.Path("server1.pem"), testdata.Path("server1.key")) if err != nil { @@ -117,7 +117,7 @@ func TestCredsBundlePerRPCCredentials(t *testing.T) { te := newTest(t, env{name: "creds-bundle", network: "tcp", balancer: "v1", security: "empty"}) te.tapHandle = authHandle te.customDialOptions = []grpc.DialOption{ - grpc.WithCreds(&testCredsBundle{t: t, mode: bundlePerRPCOnly}), + grpc.WithCredentialsBundle(&testCredsBundle{t: t, mode: bundlePerRPCOnly}), } te.startServer(&testServer{}) defer te.tearDown()