diff --git a/xds/internal/balancer/edsbalancer/eds_test.go b/xds/internal/balancer/edsbalancer/eds_test.go index 02cb9263e4c..f70a0f2673b 100644 --- a/xds/internal/balancer/edsbalancer/eds_test.go +++ b/xds/internal/balancer/edsbalancer/eds_test.go @@ -25,7 +25,6 @@ import ( "reflect" "testing" - corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" "github.com/golang/protobuf/jsonpb" wrapperspb "github.com/golang/protobuf/ptypes/wrappers" "github.com/google/go-cmp/cmp" @@ -51,7 +50,7 @@ func init() { return &bootstrap.Config{ BalancerName: testBalancerNameFooBar, Creds: grpc.WithInsecure(), - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, }, nil } } diff --git a/xds/internal/balancer/edsbalancer/xds_client_wrapper_test.go b/xds/internal/balancer/edsbalancer/xds_client_wrapper_test.go index f7cc4bda2d8..b87b7463edb 100644 --- a/xds/internal/balancer/edsbalancer/xds_client_wrapper_test.go +++ b/xds/internal/balancer/edsbalancer/xds_client_wrapper_test.go @@ -24,7 +24,6 @@ import ( "time" xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" - corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" "github.com/golang/protobuf/proto" "github.com/google/go-cmp/cmp" "google.golang.org/grpc" @@ -101,7 +100,7 @@ func (s) TestClientWrapperWatchEDS(t *testing.T) { return &bootstrap.Config{ BalancerName: fakeServer.Address, Creds: grpc.WithInsecure(), - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, }, nil } defer func() { bootstrapConfigNew = oldBootstrapConfigNew }() @@ -138,7 +137,7 @@ func (s) TestClientWrapperWatchEDS(t *testing.T) { wantReq := &xdspb.DiscoveryRequest{ TypeUrl: edsType, ResourceNames: []string{test.wantResourceName}, - Node: &corepb.Node{}, + Node: testutils.EmptyNodeProtoV2, } if !proto.Equal(edsReq.Req, wantReq) { t.Fatalf("got EDS request %v, expected: %v, diff: %s", edsReq.Req, wantReq, cmp.Diff(edsReq.Req, wantReq, cmp.Comparer(proto.Equal))) diff --git a/xds/internal/client/bootstrap/bootstrap.go b/xds/internal/client/bootstrap/bootstrap.go index 1ee0f1d47c5..1e2e05e8f9b 100644 --- a/xds/internal/client/bootstrap/bootstrap.go +++ b/xds/internal/client/bootstrap/bootstrap.go @@ -27,15 +27,25 @@ import ( "io/ioutil" "os" - corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" "github.com/golang/protobuf/jsonpb" + "github.com/golang/protobuf/proto" "google.golang.org/grpc" "google.golang.org/grpc/credentials/google" + "google.golang.org/grpc/xds/internal/version" ) const ( // Environment variable which holds the name of the xDS bootstrap file. - fileEnv = "GRPC_XDS_BOOTSTRAP" + bootstrapFileEnv = "GRPC_XDS_BOOTSTRAP" + // Environment variable which controls the use of xDS v3 API. + v3SupportEnv = "GRPC_XDS_EXPERIMENTAL_V3_SUPPORT" + // The "server_features" field in the bootstrap file contains a list of + // features supported by the server. A value of "xds_v3" indicates that the + // server supports the v3 version of the xDS transport protocol. + serverFeaturesV3 = "xds_v3" + // Type name for Google default credentials. googleDefaultCreds = "google_default" gRPCUserAgentName = "gRPC Go" @@ -45,7 +55,7 @@ const ( var gRPCVersion = fmt.Sprintf("%s %s", gRPCUserAgentName, grpc.Version) // For overriding in unit tests. -var fileReadFunc = ioutil.ReadFile +var bootstrapFileReadFunc = ioutil.ReadFile // Config provides the xDS client with several key bits of information that it // requires in its interaction with an xDS server. The Config is initialized @@ -59,8 +69,13 @@ type Config struct { // Creds contains the credentials to be used while talking to the xDS // server, as a grpc.DialOption. Creds grpc.DialOption - // NodeProto contains the node proto to be used in xDS requests. - NodeProto *corepb.Node + // TransportAPI indicates the API version of xDS transport protocol to use. + // This describes the xDS gRPC endpoint and version of + // DiscoveryRequest/Response used on the wire. + TransportAPI version.TransportAPI + // NodeProto contains the Node proto to be used in xDS requests. The actual + // type depends on the transport protocol version used. + NodeProto proto.Message } type channelCreds struct { @@ -85,9 +100,10 @@ type xdsServer struct { // "type": , // "config": // } -// ] +// ], +// "server_features": [ ... ] // }, -// "node": +// "node": // } // // Currently, we support exactly one type of credential, which is @@ -101,13 +117,13 @@ type xdsServer struct { func NewConfig() (*Config, error) { config := &Config{} - fName, ok := os.LookupEnv(fileEnv) + fName, ok := os.LookupEnv(bootstrapFileEnv) if !ok { - return nil, fmt.Errorf("xds: Environment variable %v not defined", fileEnv) + return nil, fmt.Errorf("xds: Environment variable %v not defined", bootstrapFileEnv) } - logger.Infof("Got bootstrap file location from %v environment variable: %v", fileEnv, fName) + logger.Infof("Got bootstrap file location from %v environment variable: %v", bootstrapFileEnv, fName) - data, err := fileReadFunc(fName) + data, err := bootstrapFileReadFunc(fName) if err != nil { return nil, fmt.Errorf("xds: Failed to read bootstrap file %s with error %v", fName, err) } @@ -118,11 +134,18 @@ func NewConfig() (*Config, error) { return nil, fmt.Errorf("xds: Failed to parse file %s (content %v) with error: %v", fName, string(data), err) } + serverSupportsV3 := false m := jsonpb.Unmarshaler{AllowUnknownFields: true} for k, v := range jsonData { switch k { case "node": - n := &corepb.Node{} + // We unconditionally convert the JSON into a v3.Node proto. The v3 + // proto does not contain the deprecated field "build_version" from + // the v2 proto. We do not expect the bootstrap file to contain the + // "build_version" field. In any case, the unmarshal will succeed + // because we have set the `AllowUnknownFields` option on the + // unmarshaler. + n := &v3corepb.Node{} if err := m.Unmarshal(bytes.NewReader(v), n); err != nil { return nil, fmt.Errorf("xds: jsonpb.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } @@ -144,6 +167,17 @@ func NewConfig() (*Config, error) { break } } + case "server_features": + var features []string + if err := json.Unmarshal(v, &features); err != nil { + return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) + } + for _, f := range features { + switch f { + case serverFeaturesV3: + serverSupportsV3 = true + } + } } // Do not fail the xDS bootstrap when an unknown field is seen. This can // happen when an older version client reads a newer version bootstrap @@ -154,20 +188,69 @@ func NewConfig() (*Config, error) { return nil, fmt.Errorf("xds: Required field %q not found in bootstrap", "xds_servers.server_uri") } - // If we don't find a nodeProto in the bootstrap file, we just create an - // empty one here. That way, callers of this function can always expect - // that the NodeProto field is non-nil. - if config.NodeProto == nil { - config.NodeProto = &corepb.Node{} + // We end up using v3 transport protocol version only if the following + // conditions are met: + // 1. Server supports v3, indicated by the presence of "xds_v3" in + // server_features. + // 2. Environment variable "GRPC_XDS_EXPERIMENTAL_V3_SUPPORT" is set to + // true. + // The default value of the enum type "version.TransportAPI" is v2. + if v3Env := os.Getenv(v3SupportEnv); v3Env == "true" { + if serverSupportsV3 { + config.TransportAPI = version.TransportV3 + } } - // BuildVersion is deprecated, and is replaced by user_agent_name and - // user_agent_version. But the management servers are still using the old - // field, so we will keep both set. - config.NodeProto.BuildVersion = gRPCVersion - config.NodeProto.UserAgentName = gRPCUserAgentName - config.NodeProto.UserAgentVersionType = &corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version} - config.NodeProto.ClientFeatures = append(config.NodeProto.ClientFeatures, clientFeatureNoOverprovisioning) + if err := config.updateNodeProto(); err != nil { + return nil, err + } logger.Infof("Bootstrap config for creating xds-client: %+v", config) return config, nil } + +// updateNodeProto updates the node proto read from the bootstrap file. +// +// Node proto in Config contains a v3.Node protobuf message corresponding to the +// JSON contents found in the bootstrap file. This method performs some post +// processing on it: +// 1. If we don't find a nodeProto in the bootstrap file, we create an empty one +// here. That way, callers of this function can always expect that the NodeProto +// field is non-nil. +// 2. If the transport protocol version to be used is not v3, we convert the +// current v3.Node proto in a v2.Node proto. +// 3. Some additional fields which are not expected to be set in the bootstrap +// file are populated here. +func (c *Config) updateNodeProto() error { + if c.TransportAPI == version.TransportV3 { + v3, _ := c.NodeProto.(*v3corepb.Node) + if v3 == nil { + v3 = &v3corepb.Node{} + } + v3.UserAgentName = gRPCUserAgentName + v3.UserAgentVersionType = &v3corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version} + v3.ClientFeatures = append(v3.ClientFeatures, clientFeatureNoOverprovisioning) + c.NodeProto = v3 + return nil + } + + v2 := &v2corepb.Node{} + if c.NodeProto != nil { + v3, err := proto.Marshal(c.NodeProto) + if err != nil { + return fmt.Errorf("xds: proto.Marshal(%v): %v", c.NodeProto, err) + } + if err := proto.Unmarshal(v3, v2); err != nil { + return fmt.Errorf("xds: proto.Unmarshal(%v): %v", v3, err) + } + } + c.NodeProto = v2 + + // BuildVersion is deprecated, and is replaced by user_agent_name and + // user_agent_version. But the management servers are still using the old + // field, so we will keep both set. + v2.BuildVersion = gRPCVersion + v2.UserAgentName = gRPCUserAgentName + v2.UserAgentVersionType = &v2corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version} + v2.ClientFeatures = append(v2.ClientFeatures, clientFeatureNoOverprovisioning) + return nil +} diff --git a/xds/internal/client/bootstrap/bootstrap_test.go b/xds/internal/client/bootstrap/bootstrap_test.go index de0ef3954e7..4a825aa35d8 100644 --- a/xds/internal/client/bootstrap/bootstrap_test.go +++ b/xds/internal/client/bootstrap/bootstrap_test.go @@ -21,70 +21,28 @@ package bootstrap import ( + "fmt" "os" "testing" - corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" "github.com/golang/protobuf/proto" structpb "github.com/golang/protobuf/ptypes/struct" + "github.com/google/go-cmp/cmp" "google.golang.org/grpc" "google.golang.org/grpc/credentials/google" + "google.golang.org/grpc/xds/internal/version" ) var ( - nodeProto = &corepb.Node{ - Id: "ENVOY_NODE_ID", - Metadata: &structpb.Struct{ - Fields: map[string]*structpb.Value{ - "TRAFFICDIRECTOR_GRPC_HOSTNAME": { - Kind: &structpb.Value_StringValue{StringValue: "trafficdirector"}, - }, - }, - }, - BuildVersion: gRPCVersion, - UserAgentName: gRPCUserAgentName, - UserAgentVersionType: &corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version}, - ClientFeatures: []string{clientFeatureNoOverprovisioning}, - } - nilCredsConfig = &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: nil, - NodeProto: nodeProto, - } - nonNilCredsConfig = &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - NodeProto: nodeProto, - } -) - -// TODO: enable leak check for this package when -// https://github.com/googleapis/google-cloud-go/issues/2417 is fixed. - -// TestNewConfig exercises the functionality in NewConfig with different -// bootstrap file contents. It overrides the fileReadFunc by returning -// bootstrap file contents defined in this test, instead of reading from a -// file. -func TestNewConfig(t *testing.T) { - bootstrapFileMap := map[string]string{ - "empty": "", - "badJSON": `["test": 123]`, - "noBalancerName": `{"node": {"id": "ENVOY_NODE_ID"}}`, + v2BootstrapFileMap = map[string]string{ "emptyNodeProto": ` { "xds_servers" : [{ "server_uri": "trafficdirector.googleapis.com:443" }] }`, - "emptyXdsServer": ` - { - "node": { - "id": "ENVOY_NODE_ID", - "metadata": { - "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" - } - } - }`, "unknownTopLevelFieldInFile": ` { "node": { @@ -208,80 +166,286 @@ func TestNewConfig(t *testing.T) { ] }`, } + v3BootstrapFileMap = map[string]string{ + "serverDoesNotSupportsV3": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ + { "type": "google_default" } + ] + }], + "server_features" : ["foo", "bar"] + }`, + "serverSupportsV3": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ + { "type": "google_default" } + ] + }], + "server_features" : ["foo", "bar", "xds_v3"] + }`, + } + metadata = &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "TRAFFICDIRECTOR_GRPC_HOSTNAME": { + Kind: &structpb.Value_StringValue{StringValue: "trafficdirector"}, + }, + }, + } + v2NodeProto = &v2corepb.Node{ + Id: "ENVOY_NODE_ID", + Metadata: metadata, + BuildVersion: gRPCVersion, + UserAgentName: gRPCUserAgentName, + UserAgentVersionType: &v2corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version}, + ClientFeatures: []string{clientFeatureNoOverprovisioning}, + } + v3NodeProto = &v3corepb.Node{ + Id: "ENVOY_NODE_ID", + Metadata: metadata, + UserAgentName: gRPCUserAgentName, + UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version}, + ClientFeatures: []string{clientFeatureNoOverprovisioning}, + } + nilCredsConfigV2 = &Config{ + BalancerName: "trafficdirector.googleapis.com:443", + Creds: nil, + NodeProto: v2NodeProto, + } + nonNilCredsConfigV2 = &Config{ + BalancerName: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + NodeProto: v2NodeProto, + } + nonNilCredsConfigV3 = &Config{ + BalancerName: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + TransportAPI: version.TransportV3, + NodeProto: v3NodeProto, + } +) - oldFileReadFunc := fileReadFunc - fileReadFunc = func(name string) ([]byte, error) { +func (c *Config) compare(want *Config) error { + if c.BalancerName != want.BalancerName { + return fmt.Errorf("config.BalancerName is %s, want %s", c.BalancerName, want.BalancerName) + } + // Since Creds is of type grpc.DialOption interface, where the + // implementation is provided by a function, it is not possible to compare. + if (c.Creds != nil) != (want.Creds != nil) { + return fmt.Errorf("config.Creds is %#v, want %#v", c.Creds, want.Creds) + } + if c.TransportAPI != want.TransportAPI { + return fmt.Errorf("config.TransportAPI is %v, want %v", c.TransportAPI, want.TransportAPI) + + } + if diff := cmp.Diff(want.NodeProto, c.NodeProto, cmp.Comparer(proto.Equal)); diff != "" { + return fmt.Errorf("config.NodeProto diff (-want, +got):\n%s", diff) + } + return nil +} + +func setupBootstrapOverride(bootstrapFileMap map[string]string) func() { + oldFileReadFunc := bootstrapFileReadFunc + bootstrapFileReadFunc = func(name string) ([]byte, error) { if b, ok := bootstrapFileMap[name]; ok { return []byte(b), nil } return nil, os.ErrNotExist } - defer func() { - fileReadFunc = oldFileReadFunc - os.Unsetenv(fileEnv) - }() + return func() { + bootstrapFileReadFunc = oldFileReadFunc + os.Unsetenv(bootstrapFileEnv) + } +} + +// TODO: enable leak check for this package when +// https://github.com/googleapis/google-cloud-go/issues/2417 is fixed. + +// TestNewConfigV2ProtoFailure exercises the functionality in NewConfig with +// different bootstrap file contents which are expected to fail. +func TestNewConfigV2ProtoFailure(t *testing.T) { + bootstrapFileMap := map[string]string{ + "empty": "", + "badJSON": `["test": 123]`, + "noBalancerName": `{"node": {"id": "ENVOY_NODE_ID"}}`, + "emptyXdsServer": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + } + }`, + } + cancel := setupBootstrapOverride(bootstrapFileMap) + defer cancel() + + tests := []struct { + name string + wantError bool + }{ + {"nonExistentBootstrapFile", true}, + {"empty", true}, + {"badJSON", true}, + {"noBalancerName", true}, + {"emptyXdsServer", true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := os.Setenv(bootstrapFileEnv, test.name); err != nil { + t.Fatalf("os.Setenv(%s, %s) failed with error: %v", bootstrapFileEnv, test.name, err) + } + if _, err := NewConfig(); err == nil { + t.Fatalf("NewConfig() returned nil error, expected to fail") + } + }) + } +} + +// TestNewConfigV2ProtoSuccess exercises the functionality in NewConfig with +// different bootstrap file contents. It overrides the fileReadFunc by returning +// bootstrap file contents defined in this test, instead of reading from a file. +func TestNewConfigV2ProtoSuccess(t *testing.T) { + cancel := setupBootstrapOverride(v2BootstrapFileMap) + defer cancel() tests := []struct { name string wantConfig *Config - wantError bool }{ - {"nonExistentBootstrapFile", nil, true}, - {"empty", nil, true}, - {"badJSON", nil, true}, - {"emptyNodeProto", &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - NodeProto: &corepb.Node{ - BuildVersion: gRPCVersion, - UserAgentName: gRPCUserAgentName, - UserAgentVersionType: &corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version}, - ClientFeatures: []string{clientFeatureNoOverprovisioning}, + { + "emptyNodeProto", &Config{ + BalancerName: "trafficdirector.googleapis.com:443", + NodeProto: &v2corepb.Node{ + BuildVersion: gRPCVersion, + UserAgentName: gRPCUserAgentName, + UserAgentVersionType: &v2corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version}, + ClientFeatures: []string{clientFeatureNoOverprovisioning}, + }, }, - }, false}, - {"noBalancerName", nil, true}, - {"emptyXdsServer", nil, true}, - {"unknownTopLevelFieldInFile", nilCredsConfig, false}, - {"unknownFieldInNodeProto", nilCredsConfig, false}, - {"unknownFieldInXdsServer", nilCredsConfig, false}, - {"emptyChannelCreds", nilCredsConfig, false}, - {"nonGoogleDefaultCreds", nilCredsConfig, false}, - {"multipleChannelCreds", nonNilCredsConfig, false}, - {"goodBootstrap", nonNilCredsConfig, false}, - {"multipleXDSServers", nonNilCredsConfig, false}, + }, + {"unknownTopLevelFieldInFile", nilCredsConfigV2}, + {"unknownFieldInNodeProto", nilCredsConfigV2}, + {"unknownFieldInXdsServer", nilCredsConfigV2}, + {"emptyChannelCreds", nilCredsConfigV2}, + {"nonGoogleDefaultCreds", nilCredsConfigV2}, + {"multipleChannelCreds", nonNilCredsConfigV2}, + {"goodBootstrap", nonNilCredsConfigV2}, + {"multipleXDSServers", nonNilCredsConfigV2}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - if err := os.Setenv(fileEnv, test.name); err != nil { - t.Fatalf("os.Setenv(%s, %s) failed with error: %v", fileEnv, test.name, err) + if err := os.Setenv(bootstrapFileEnv, test.name); err != nil { + t.Fatalf("os.Setenv(%s, %s) failed with error: %v", bootstrapFileEnv, test.name, err) } - config, err := NewConfig() + c, err := NewConfig() if err != nil { - if !test.wantError { - t.Fatalf("unexpected error %v", err) - } - return + t.Fatalf("NewConfig() failed: %v", err) } - if test.wantError { - t.Fatalf("wantError: %v, got error %v", test.wantError, err) + if err := c.compare(test.wantConfig); err != nil { + t.Fatal(err) } - if config.BalancerName != test.wantConfig.BalancerName { - t.Errorf("config.BalancerName is %s, want %s", config.BalancerName, test.wantConfig.BalancerName) + }) + } +} + +// TestNewConfigV3SupportNotEnabledOnClient verifies bootstrap functionality +// when the GRPC_XDS_EXPERIMENTAL_V3_SUPPORT environment variable is not enabled +// on the client. In this case, whether the server supports v3 or not, the +// client will end up using v2. +func TestNewConfigV3SupportNotEnabledOnClient(t *testing.T) { + if err := os.Setenv(v3SupportEnv, "false"); err != nil { + t.Fatalf("os.Setenv(%s, %s) failed with error: %v", v3SupportEnv, "true", err) + } + defer os.Unsetenv(v3SupportEnv) + + cancel := setupBootstrapOverride(v3BootstrapFileMap) + defer cancel() + + tests := []struct { + name string + wantConfig *Config + }{ + {"serverDoesNotSupportsV3", nonNilCredsConfigV2}, + {"serverSupportsV3", nonNilCredsConfigV2}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := os.Setenv(bootstrapFileEnv, test.name); err != nil { + t.Fatalf("os.Setenv(%s, %s) failed with error: %v", bootstrapFileEnv, test.name, err) } - if !proto.Equal(config.NodeProto, test.wantConfig.NodeProto) { - t.Errorf("config.NodeProto is %#v, want %#v", config.NodeProto, test.wantConfig.NodeProto) + c, err := NewConfig() + if err != nil { + t.Fatalf("NewConfig() failed: %v", err) + } + if err := c.compare(test.wantConfig); err != nil { + t.Fatal(err) + } + }) + } +} + +// TestNewConfigV3SupportEnabledOnClient verifies bootstrap functionality when +// the GRPC_XDS_EXPERIMENTAL_V3_SUPPORT environment variable is enabled on the +// client. Here the client ends up using v2 or v3 based on what the server +// supports. +func TestNewConfigV3SupportEnabledOnClient(t *testing.T) { + if err := os.Setenv(v3SupportEnv, "true"); err != nil { + t.Fatalf("os.Setenv(%s, %s) failed with error: %v", v3SupportEnv, "true", err) + } + defer os.Unsetenv(v3SupportEnv) + + cancel := setupBootstrapOverride(v3BootstrapFileMap) + defer cancel() + + tests := []struct { + name string + wantConfig *Config + }{ + {"serverDoesNotSupportsV3", nonNilCredsConfigV2}, + {"serverSupportsV3", nonNilCredsConfigV3}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := os.Setenv(bootstrapFileEnv, test.name); err != nil { + t.Fatalf("os.Setenv(%s, %s) failed with error: %v", bootstrapFileEnv, test.name, err) + } + c, err := NewConfig() + if err != nil { + t.Fatalf("NewConfig() failed: %v", err) } - if (config.Creds != nil) != (test.wantConfig.Creds != nil) { - t.Errorf("config.Creds is %#v, want %#v", config.Creds, test.wantConfig.Creds) + if err := c.compare(test.wantConfig); err != nil { + t.Fatal(err) } }) } } -func TestNewConfigEnvNotSet(t *testing.T) { - os.Unsetenv(fileEnv) - config, err := NewConfig() - if err == nil { - t.Errorf("NewConfig() returned: %#v, , wanted non-nil error", config) +// TestNewConfigBootstrapFileEnvNotSet tests the case where the bootstrap file +// environment variable is not set. +func TestNewConfigBootstrapFileEnvNotSet(t *testing.T) { + os.Unsetenv(bootstrapFileEnv) + if _, err := NewConfig(); err == nil { + t.Errorf("NewConfig() returned nil error, expected to fail") } } diff --git a/xds/internal/client/client.go b/xds/internal/client/client.go index 127734fd593..07a0a2637c0 100644 --- a/xds/internal/client/client.go +++ b/xds/internal/client/client.go @@ -34,6 +34,7 @@ import ( "google.golang.org/grpc/internal/grpcsync" "google.golang.org/grpc/keepalive" "google.golang.org/grpc/xds/internal/client/bootstrap" + "google.golang.org/grpc/xds/internal/version" ) // Options provides all parameters required for the creation of an xDS client. @@ -131,7 +132,13 @@ func New(opts Options) (*Client, error) { c.logger = prefixLogger((c)) c.logger.Infof("Created ClientConn to xDS server: %s", opts.Config.BalancerName) - c.v2c = newXDSV2Client(c, cc, opts.Config.NodeProto, backoff.DefaultExponential.Backoff, c.logger) + if opts.Config.TransportAPI == version.TransportV2 { + c.v2c = newXDSV2Client(c, cc, opts.Config.NodeProto.(*corepb.Node), backoff.DefaultExponential.Backoff, c.logger) + } else { + // TODO(easwars): Remove this once v3Client is ready. + return nil, errors.New("xds v3 client is not yet supported") + } + c.logger.Infof("Created") go c.run() return c, nil diff --git a/xds/internal/client/client_test.go b/xds/internal/client/client_test.go index 367e9e02642..043fa4f76de 100644 --- a/xds/internal/client/client_test.go +++ b/xds/internal/client/client_test.go @@ -55,7 +55,7 @@ func clientOpts(balancerName string) Options { Config: bootstrap.Config{ BalancerName: balancerName, Creds: grpc.WithInsecure(), - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, }, } } @@ -78,7 +78,7 @@ func (s) TestNew(t *testing.T) { opts: Options{ Config: bootstrap.Config{ Creds: grpc.WithInsecure(), - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, }, }, wantErr: true, @@ -88,7 +88,7 @@ func (s) TestNew(t *testing.T) { opts: Options{ Config: bootstrap.Config{ BalancerName: "dummy", - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, }, }, wantErr: true, diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index 7e1e49d2d92..dcb5c9e8e94 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -37,8 +37,6 @@ import ( "google.golang.org/grpc/xds/internal/client/bootstrap" "google.golang.org/grpc/xds/internal/testutils" "google.golang.org/grpc/xds/internal/testutils/fakeclient" - - corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" ) const ( @@ -51,7 +49,7 @@ var ( validConfig = bootstrap.Config{ BalancerName: balancerName, Creds: grpc.WithInsecure(), - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, } target = resolver.Target{Endpoint: targetStr} ) @@ -138,7 +136,7 @@ func TestResolverBuilder(t *testing.T) { rbo: resolver.BuildOptions{}, config: bootstrap.Config{ Creds: grpc.WithInsecure(), - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, }, wantErr: true, }, @@ -147,7 +145,7 @@ func TestResolverBuilder(t *testing.T) { rbo: resolver.BuildOptions{}, config: bootstrap.Config{ BalancerName: balancerName, - NodeProto: &corepb.Node{}, + NodeProto: testutils.EmptyNodeProtoV2, }, xdsClientFunc: getXDSClientMakerFunc(xdsclient.Options{Config: validConfig}), wantErr: false, diff --git a/xds/internal/testutils/locality.go b/xds/internal/testutils/protos.go similarity index 75% rename from xds/internal/testutils/locality.go rename to xds/internal/testutils/protos.go index a4e4cc59b86..fb60e91f2e3 100644 --- a/xds/internal/testutils/locality.go +++ b/xds/internal/testutils/protos.go @@ -18,13 +18,16 @@ package testutils import ( - corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" "google.golang.org/grpc/xds/internal" ) +// EmptyNodeProtoV2 is a node proto with no fields set. +var EmptyNodeProtoV2 = &v2corepb.Node{} + // LocalityIDToProto converts a LocalityID to its proto representation. -func LocalityIDToProto(l internal.LocalityID) *corepb.Locality { - return &corepb.Locality{ +func LocalityIDToProto(l internal.LocalityID) *v2corepb.Locality { + return &v2corepb.Locality{ Region: l.Region, Zone: l.Zone, SubZone: l.SubZone, diff --git a/xds/internal/version/version.go b/xds/internal/version/version.go new file mode 100644 index 00000000000..0425fa5e33f --- /dev/null +++ b/xds/internal/version/version.go @@ -0,0 +1,32 @@ +/* + * + * Copyright 2020 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// Package version defines supported xDS API versions. +package version + +// TransportAPI refers to the API version for xDS transport protocol. This +// describes the xDS gRPC endpoint and version of DiscoveryRequest/Response used +// on the wire. +type TransportAPI int + +const ( + // TransportV2 refers to the v2 xDS transport protocol. + TransportV2 TransportAPI = iota + // TransportV3 refers to the v3 xDS transport protocol. + TransportV3 +)