diff --git a/internal/xds/bootstrap/bootstrap.go b/internal/xds/bootstrap/bootstrap.go index c725bc1ea..35aeea701 100644 --- a/internal/xds/bootstrap/bootstrap.go +++ b/internal/xds/bootstrap/bootstrap.go @@ -220,20 +220,14 @@ func (sc *ServerConfig) ServerFeaturesIgnoreResourceDeletion() bool { return false } -// CredsDialOption returns the first supported transport credentials from the -// configuration, as a dial option. -func (sc *ServerConfig) CredsDialOption() grpc.DialOption { - return sc.credsDialOption -} - -// DialerOption returns the Dialer function that specifies how to dial the xDS -// server determined by the first supported credentials from the configuration, -// as a dial option. -// -// TODO(https://github.com/grpc/grpc-go/issues/7661): change ServerConfig type -// to have a single method that returns all configured dial options. -func (sc *ServerConfig) DialerOption() grpc.DialOption { - return sc.dialerOption +// DialOptions returns a slice of all the configured dial options for this +// server. +func (sc *ServerConfig) DialOptions() []grpc.DialOption { + dopts := []grpc.DialOption{sc.credsDialOption} + if sc.dialerOption != nil { + dopts = append(dopts, sc.dialerOption) + } + return dopts } // Cleanups returns a collection of functions to be called when the xDS client diff --git a/xds/internal/xdsclient/transport/transport.go b/xds/internal/xdsclient/transport/transport.go index 134a9519f..59b221727 100644 --- a/xds/internal/xdsclient/transport/transport.go +++ b/xds/internal/xdsclient/transport/transport.go @@ -192,19 +192,14 @@ func New(opts Options) (*Transport, error) { return nil, errors.New("missing OnSend callback handler when creating a new transport") } - // Dial the xDS management with the passed in credentials. - dopts := []grpc.DialOption{ - opts.ServerCfg.CredsDialOption(), - grpc.WithKeepaliveParams(keepalive.ClientParameters{ - // We decided to use these sane defaults in all languages, and - // kicked the can down the road as far making these configurable. - Time: 5 * time.Minute, - Timeout: 20 * time.Second, - }), - } - if dialerOpts := opts.ServerCfg.DialerOption(); dialerOpts != nil { - dopts = append(dopts, dialerOpts) - } + // Dial the xDS management server with dial options specified by the server + // configuration and a static keepalive configuration that is common across + // gRPC language implementations. + kpCfg := grpc.WithKeepaliveParams(keepalive.ClientParameters{ + Time: 5 * time.Minute, + Timeout: 20 * time.Second, + }) + dopts := append([]grpc.DialOption{kpCfg}, opts.ServerCfg.DialOptions()...) grpcNewClient := transportinternal.GRPCNewClient.(func(string, ...grpc.DialOption) (*grpc.ClientConn, error)) cc, err := grpcNewClient(opts.ServerCfg.ServerURI(), dopts...) if err != nil { diff --git a/xds/internal/xdsclient/transport/transport_test.go b/xds/internal/xdsclient/transport/transport_test.go index 7aac0ccdb..b51f58b74 100644 --- a/xds/internal/xdsclient/transport/transport_test.go +++ b/xds/internal/xdsclient/transport/transport_test.go @@ -18,17 +18,11 @@ package transport_test import ( - "context" - "encoding/json" - "net" "testing" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" - "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/grpctest" internalbootstrap "google.golang.org/grpc/internal/xds/bootstrap" - "google.golang.org/grpc/xds/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient/transport" "google.golang.org/grpc/xds/internal/xdsclient/transport/internal" @@ -102,80 +96,3 @@ func (s) TestNewWithGRPCDial(t *testing.T) { t.Fatalf("transport.New(%+v) custom dialer called = true, want false", opts) } } - -const testDialerCredsBuilderName = "test_dialer_creds" - -// testDialerCredsBuilder implements the `Credentials` interface defined in -// package `xds/bootstrap` and encapsulates an insecure credential with a -// custom Dialer that specifies how to dial the xDS server. -type testDialerCredsBuilder struct{} - -func (t *testDialerCredsBuilder) Build(json.RawMessage) (credentials.Bundle, func(), error) { - return &testDialerCredsBundle{insecure.NewBundle()}, func() {}, nil -} - -func (t *testDialerCredsBuilder) Name() string { - return testDialerCredsBuilderName -} - -// testDialerCredsBundle implements the `Bundle` interface defined in package -// `credentials` and encapsulates an insecure credential with a custom Dialer -// that specifies how to dial the xDS server. -type testDialerCredsBundle struct { - credentials.Bundle -} - -func (t *testDialerCredsBundle) Dialer(context.Context, string) (net.Conn, error) { - return nil, nil -} - -func (s) TestNewWithDialerFromCredentialsBundle(t *testing.T) { - // Override grpc.NewClient with a custom one. - doptsLen := 0 - customGRPCNewClient := func(target string, opts ...grpc.DialOption) (*grpc.ClientConn, error) { - doptsLen = len(opts) - return grpc.NewClient(target, opts...) - } - oldGRPCNewClient := internal.GRPCNewClient - internal.GRPCNewClient = customGRPCNewClient - defer func() { internal.GRPCNewClient = oldGRPCNewClient }() - - bootstrap.RegisterCredentials(&testDialerCredsBuilder{}) - serverCfg, err := internalbootstrap.ServerConfigForTesting(internalbootstrap.ServerConfigTestingOptions{ - URI: "trafficdirector.googleapis.com:443", - ChannelCreds: []internalbootstrap.ChannelCreds{{Type: testDialerCredsBuilderName}}, - }) - if err != nil { - t.Fatalf("Failed to create server config for testing: %v", err) - } - if serverCfg.DialerOption() == nil { - t.Fatalf("Dialer for xDS transport in server config for testing is nil, want non-nil") - } - // Create a new transport. - opts := transport.Options{ - ServerCfg: serverCfg, - NodeProto: &v3corepb.Node{}, - OnRecvHandler: func(update transport.ResourceUpdate, onDone func()) error { - onDone() - return nil - }, - OnErrorHandler: func(error) {}, - OnSendHandler: func(*transport.ResourceSendInfo) {}, - } - c, err := transport.New(opts) - defer func() { - if c != nil { - c.Close() - } - }() - if err != nil { - t.Fatalf("transport.New(%v) failed: %v", opts, err) - } - // Verify there are three dial options passed to the custom grpc.NewClient. - // The first is opts.ServerCfg.CredsDialOption(), the second is - // grpc.WithKeepaliveParams(), and the third is opts.ServerCfg.DialerOption() - // from the credentials bundle. - if doptsLen != 3 { - t.Fatalf("transport.New(%v) custom grpc.NewClient called with %d dial options, want 3", opts, doptsLen) - } -}