xds: incorporate changes to bootstrap file format. (#3065)

* Incorporate changes to bootstrap file format.

The format of the bootstrap file will be as follows:
{
  "xds_server": {
    "server_uri": <string containing URI of xds server>,
    // List of channel creds; client will stop at the first type it
    // supports.
    "channel_creds": [
      {
        "type": <string containing channel cred type>,
        // The "config" field is optional; it may be missing if the
        // credential type does not require config parameters.
        "config": <JSON object containing config for the type>
      }
    ]
  },
  "node": <JSON form of Node proto>
}

- Also, the bootstrap file will be read everytime a new xDS client is created.
- Change NewConfig() to not return error. Instead it just returns with
  certain fields left unspecified if it encounters errors.
- Do not fail the bootstrap process if we see unknown fields in the
  bootstrap file.

xds/internal/balancer/xds_client.go

@@ -272,29 +272,17 @@ func newXDSClient(balancerName string, enableCDS bool, opts balancer.BuildOption
 
 	c.ctx, c.cancel = context.WithCancel(context.Background())
 
-	var err error
+	// It is possible that NewConfig returns a Config object with certain
-	if c.config, err = xdsclient.NewConfig(); err != nil {
+	// fields left unspecified. If so, we need to use some sane defaults here.
-		grpclog.Error(err)
+	c.config = xdsclient.NewConfig()
-		c.config = newConfigFromDefaults(balancerName, &opts)
+	if c.config.BalancerName == "" {
+		c.config.BalancerName = balancerName
 	}
-	return c
+	if c.config.Creds == nil {
+		c.config.Creds = credsFromDefaults(balancerName, &opts)
 	}
-
+	if c.config.NodeProto == nil {
-func newConfigFromDefaults(balancerName string, opts *balancer.BuildOptions) *xdsclient.Config {
+		c.config.NodeProto = &basepb.Node{
-	dopts := grpc.WithInsecure()
-	if opts.DialCreds != nil {
-		if err := opts.DialCreds.OverrideServerName(balancerName); err == nil {
-			dopts = grpc.WithTransportCredentials(opts.DialCreds)
-		} else {
-			grpclog.Warningf("xds: failed to override the server name in credentials: %v, using Insecure", err)
-		}
-	} else {
-		grpclog.Warning("xds: no credentials available, using Insecure")
-	}
-	return &xdsclient.Config{
-		BalancerName: balancerName,
-		Creds:        dopts,
-		NodeProto: &basepb.Node{
 			Metadata: &structpb.Struct{
 				Fields: map[string]*structpb.Value{
 					internal.GrpcHostname: {
@@ -302,6 +290,19 @@ func newConfigFromDefaults(balancerName string, opts *balancer.BuildOptions) *xd
 					},
 				},
 			},
-		},
 		}
 	}
+	return c
+}
+
+func credsFromDefaults(balancerName string, opts *balancer.BuildOptions) grpc.DialOption {
+	if opts.DialCreds == nil {
+		grpclog.Warning("xds: no credentials available, using Insecure")
+		return grpc.WithInsecure()
+	}
+	if err := opts.DialCreds.OverrideServerName(balancerName); err != nil {
+		grpclog.Warningf("xds: failed to override the server name in credentials: %v, using Insecure", err)
+		return grpc.WithInsecure()
+	}
+	return grpc.WithTransportCredentials(opts.DialCreds)
+}

xds/internal/client/helper.go

@@ -23,35 +23,25 @@ package client
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"io/ioutil"
 	"os"
-	"sync"
 
 	"github.com/golang/protobuf/jsonpb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/google"
 	"google.golang.org/grpc/grpclog"
 	basepb "google.golang.org/grpc/xds/internal/proto/envoy/api/v2/core/base"
-	cspb "google.golang.org/grpc/xds/internal/proto/envoy/api/v2/core/config_source"
 )
 
+const (
 	// Environment variable which holds the name of the xDS bootstrap file.
-const bootstrapFileEnv = "GRPC_XDS_BOOTSTRAP"
+	bootstrapFileEnv = "GRPC_XDS_BOOTSTRAP"
-
+	// Type name for Google default credentials.
-var (
+	googleDefaultCreds = "google_default"
-	// Bootstrap file is read once (on the first invocation of NewConfig), and
-	// the results are stored in these package level vars.
-	bsOnce sync.Once
-	bsData *bootstrapData
-	bsErr  error
 )
 
 // For overriding from unit tests.
-var (
+var fileReadFunc = ioutil.ReadFile
-	fileReadFunc = ioutil.ReadFile
-	onceDoerFunc = bsOnce.Do
-)
 
 // 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
@@ -68,95 +58,92 @@ type Config struct {
 	NodeProto *basepb.Node
 }
 
+type channelCreds struct {
+	Type   string          `json:"type"`
+	Config json.RawMessage `json:"config"`
+}
+
+type xdsServer struct {
+	ServerURI    string         `json:"server_uri"`
+	ChannelCreds []channelCreds `json:"channel_creds"`
+}
+
 // NewConfig returns a new instance of Config initialized by reading the
-// bootstrap file found at ${GRPC_XDS_BOOTSTRAP}. Bootstrap file is read only
+// bootstrap file found at ${GRPC_XDS_BOOTSTRAP}.
-// on the first invocation of this function, and further invocations end up
-// using the results from the former.
 //
-// As of today, the bootstrap file only provides the balancer name and the node
+// The format of the bootstrap file will be as follows:
-// proto to be used in calls to the balancer. For transport credentials, the
+// {
-// default TLS config with system certs is used. For call credentials, default
+//    "xds_server": {
-// compute engine credentials are used.
+//      "server_uri": <string containing URI of xds server>,
-func NewConfig() (*Config, error) {
+//      "channel_creds": [
-	onceDoerFunc(func() {
+//        {
+//          "type": <string containing channel cred type>,
+//          "config": <JSON object containing config for the type>
+//        }
+//      ]
+//    },
+//    "node": <JSON form of basepb.Node proto>
+// }
+//
+// Currently, we support exactly one type of credential, which is
+// "google_default", where we use the host's default certs for transport
+// credentials and a Google oauth token for call credentials.
+//
+// This function tries to process as much of the bootstrap file as possible (in
+// the presence of the errors) and may return a Config object with certain
+// fields left unspecified, in which case the caller should use some sane
+// defaults.
+func NewConfig() *Config {
+	config := &Config{}
+
 	fName, ok := os.LookupEnv(bootstrapFileEnv)
 	if !ok {
-			bsData, bsErr = nil, fmt.Errorf("xds: %s environment variable not set", bootstrapFileEnv)
+		grpclog.Errorf("xds: %s environment variable not set", bootstrapFileEnv)
-			return
+		return config
-		}
-		bsData, bsErr = readBootstrapFile(fName)
-	})
-	if bsErr != nil {
-		return nil, bsErr
-	}
-	return &Config{
-		BalancerName: bsData.balancerName(),
-		Creds:        grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()),
-		NodeProto:    bsData.node,
-	}, nil
 	}
 
-// bootstrapData wraps the contents of the bootstrap file.
+	grpclog.Infof("xds: Reading bootstrap file from %s", fName)
-// Today the bootstrap file contains a Node proto and an ApiConfigSource proto
+	data, err := fileReadFunc(fName)
-// in JSON format.
-type bootstrapData struct {
-	node      *basepb.Node
-	xdsServer *cspb.ApiConfigSource
-}
-
-func (bsd *bootstrapData) balancerName() string {
-	// If the bootstrap file was read and parsed successfully, this should be
-	// setup properly. So, we skip checking for the presence of these fields
-	// before accessing and returning it.
-	return bsd.xdsServer.GetGrpcServices()[0].GetGoogleGrpc().GetTargetUri()
-}
-
-func readBootstrapFile(name string) (*bootstrapData, error) {
-	grpclog.Infof("xds: Reading bootstrap file from %s", name)
-	data, err := fileReadFunc(name)
 	if err != nil {
-		return nil, fmt.Errorf("xds: bootstrap file {%v} read failed: %v", name, err)
+		grpclog.Errorf("xds: bootstrap file {%v} read failed: %v", fName, err)
+		return config
 	}
 
 	var jsonData map[string]json.RawMessage
 	if err := json.Unmarshal(data, &jsonData); err != nil {
-		return nil, fmt.Errorf("xds: json.Unmarshal(%v) failed during bootstrap: %v", string(data), err)
+		grpclog.Errorf("xds: json.Unmarshal(%v) failed during bootstrap: %v", string(data), err)
+		return config
 	}
 
-	bsd := &bootstrapData{}
+	m := jsonpb.Unmarshaler{AllowUnknownFields: true}
-	m := jsonpb.Unmarshaler{}
 	for k, v := range jsonData {
 		switch k {
 		case "node":
 			n := &basepb.Node{}
 			if err := m.Unmarshal(bytes.NewReader(v), n); err != nil {
-				return nil, fmt.Errorf("xds: jsonpb.Unmarshal(%v) failed during bootstrap: %v", string(v), err)
+				grpclog.Errorf("xds: jsonpb.Unmarshal(%v) failed during bootstrap: %v", string(v), err)
+				break
 			}
-			bsd.node = n
+			config.NodeProto = n
 		case "xds_server":
-			s := &cspb.ApiConfigSource{}
+			xs := &xdsServer{}
-			if err := m.Unmarshal(bytes.NewReader(v), s); err != nil {
+			if err := json.Unmarshal(v, &xs); err != nil {
-				return nil, fmt.Errorf("xds: jsonpb.Unmarshal(%v) failed during bootstrap: %v", string(v), err)
+				grpclog.Errorf("xds: json.Unmarshal(%v) failed during bootstrap: %v", string(v), err)
+				break
+			}
+			config.BalancerName = xs.ServerURI
+			for _, cc := range xs.ChannelCreds {
+				if cc.Type == googleDefaultCreds {
+					config.Creds = grpc.WithCredentialsBundle(google.NewComputeEngineCredentials())
+					// We stop at the first credential type that we support.
+					break
+				}
 			}
-			bsd.xdsServer = s
 		default:
-			return nil, fmt.Errorf("xds: unexpected data in bootstrap file: {%v, %v}", k, string(v))
+			// Do not fail the xDS bootstrap when an unknown field is seen.
+			grpclog.Warningf("xds: unexpected data in bootstrap file: {%v, %v}", k, string(v))
 		}
 	}
 
-	if bsd.node == nil || bsd.xdsServer == nil {
+	return config
-		return nil, fmt.Errorf("xds: incomplete data in bootstrap file: %v", string(data))
-	}
-
-	if api := bsd.xdsServer.GetApiType(); api != cspb.ApiConfigSource_GRPC {
-		return nil, fmt.Errorf("xds: apiType in bootstrap file is %v, want GRPC", api)
-	}
-	if n := len(bsd.xdsServer.GetGrpcServices()); n != 1 {
-		return nil, fmt.Errorf("xds: %v grpc services listed in bootstrap file, want 1", n)
-	}
-	if bsd.xdsServer.GetGrpcServices()[0].GetGoogleGrpc().GetTargetUri() == "" {
-		return nil, fmt.Errorf("xds: trafficdirector name missing in bootstrap file")
-	}
-
-	return bsd, nil
 }

xds/internal/client/helper_test.go

@@ -20,264 +20,17 @@ package client
 
 import (
 	"os"
-	"strings"
-	"sync"
 	"testing"
 
 	"github.com/golang/protobuf/proto"
 	structpb "github.com/golang/protobuf/ptypes/struct"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/google"
 	basepb "google.golang.org/grpc/xds/internal/proto/envoy/api/v2/core/base"
 )
 
-// TestNewConfig exercises the functionality in NewConfig with different
+var (
-// bootstrap file contents. It overrides the fileReadFunc by returning
+	nodeProto = &basepb.Node{
-// bootstrap file contents defined in this test, instead of reading from a
-// file. It also overrides onceDoerFunc to disable reading the bootstrap file
-// only once.
-func TestNewConfig(t *testing.T) {
-	bootstrapFileMap := map[string]string{
-		"empty":   "",
-		"badJSON": `["test": 123]`,
-		"badNodeProto": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"badField": "foobar",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			}
-		}`,
-		"badApiConfigSourceProto": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			},
-			"xds_server" : {
-			    "api_type": "GRPC",
-				"badField": "foobar",
-			    "grpc_services": [
-					{
-						"google_grpc": {
-							"target_uri": "trafficdirector.googleapis.com:443"
-						}
-					}
-				]
-			}
-		}`,
-		"badTopLevelFieldInFile": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			},
-			"xds_server" : {
-			    "api_type": "GRPC",
-			    "grpc_services": [
-					{
-						"google_grpc": {
-							"target_uri": "trafficdirector.googleapis.com:443"
-						}
-					}
-				]
-			},
-			"badField": "foobar"
-		}`,
-		"emptyNodeProto": `
-		{
-			"xds_server" : {
-			    "api_type": "GRPC",
-			    "grpc_services": [
-					{
-						"google_grpc": {
-							"target_uri": "trafficdirector.googleapis.com:443"
-						}
-					}
-				]
-			}
-		}`,
-		"emptyApiConfigSourceProto": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			}
-		}`,
-		"badApiTypeInFile": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			},
-			"xds_server" : {
-			    "api_type": "REST",
-			    "grpc_services": [
-					{
-						"google_grpc": {
-							"target_uri": "trafficdirector.googleapis.com:443"
-						}
-					}
-				]
-			}
-		}`,
-		"noGrpcServices": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			},
-			"xds_server" : {
-			    "api_type": "GRPC",
-			}
-		}`,
-		"tooManyGrpcServices": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			},
-			"xds_server" : {
-			    "api_type": "GRPC",
-			    "grpc_services": [
-					{
-						"google_grpc": {
-							"target_uri": "trafficdirector.googleapis.com:443"
-						}
-					},
-					{
-						"google_grpc": {
-							"target_uri": "foobar.googleapis.com:443"
-						}
-					}
-				]
-			}
-		}`,
-		"goodBootstrap": `
-		{
-			"node": {
-				"id": "ENVOY_NODE_ID",
-				"metadata": {
-				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
-			    }
-			},
-			"xds_server" : {
-			    "api_type": "GRPC",
-			    "grpc_services": [
-					{
-						"google_grpc": {
-							"target_uri": "trafficdirector.googleapis.com:443"
-						}
-					}
-				]
-			}
-		}`,
-	}
-
-	oldFileReadFunc := fileReadFunc
-	fileReadFunc = func(name string) ([]byte, error) {
-		if b, ok := bootstrapFileMap[name]; ok {
-			return []byte(b), nil
-		}
-		return nil, os.ErrNotExist
-	}
-	oldOnceDoerFunc := onceDoerFunc
-	onceDoerFunc = func(f func()) {
-		// Disable the synce.Once functionality to read the bootstrap file.
-		// Instead, read it everytime NewConfig() is called so that we can test
-		// with different file contents.
-		f()
-	}
-	defer func() {
-		fileReadFunc = oldFileReadFunc
-		onceDoerFunc = oldOnceDoerFunc
-		os.Unsetenv(bootstrapFileEnv)
-	}()
-
-	tests := []struct {
-		name             string
-		fName            string
-		wantErr          bool
-		wantBalancerName string
-		wantNodeProto    *basepb.Node
-		// TODO: It doesn't look like there is an easy way to compare the value
-		// stored in Creds with an expected value. Figure out a way to make it
-		// testable.
-	}{
-		{
-			name:             "non-existent-bootstrap-file",
-			fName:            "dummy",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "bad-json-in-file",
-			fName:            "badJSON",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "bad-nodeProto-in-file",
-			fName:            "badNodeProto",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "bad-ApiConfigSourceProto-in-file",
-			fName:            "badApiConfigSourceProto",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "bad-top-level-field-in-file",
-			fName:            "badTopLevelFieldInFile",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "empty-nodeProto-in-file",
-			fName:            "emptyNodeProto",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "empty-apiConfigSourceProto-in-file",
-			fName:            "emptyApiConfigSourceProto",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "bad-api-type-in-file",
-			fName:            "badApiTypeInFile",
-			wantErr:          true,
-			wantBalancerName: "",
-			wantNodeProto:    nil,
-		},
-		{
-			name:             "good-bootstrap",
-			fName:            "goodBootstrap",
-			wantErr:          false,
-			wantBalancerName: "trafficdirector.googleapis.com:443",
-			wantNodeProto: &basepb.Node{
 		Id: "ENVOY_NODE_ID",
 		Metadata: &structpb.Struct{
 			Fields: map[string]*structpb.Value{
@@ -286,42 +39,127 @@ func TestNewConfig(t *testing.T) {
 				},
 			},
 		},
-			},
-		},
 	}
+	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,
+	}
+)
 
-	for _, test := range tests {
+// TestNewConfig exercises the functionality in NewConfig with different
-		if err := os.Setenv(bootstrapFileEnv, test.fName); err != nil {
+// bootstrap file contents. It overrides the fileReadFunc by returning
-			t.Fatalf("%s: os.Setenv(%s, %s) failed with error: %v", test.name, bootstrapFileEnv, test.fName, err)
+// bootstrap file contents defined in this test, instead of reading from a
-		}
+// file.
-		config, err := NewConfig()
+func TestNewConfig(t *testing.T) {
-		if (err != nil) != test.wantErr {
-			t.Fatalf("%s: NewConfig() returned error: %v, wantErr: %v", test.name, err, test.wantErr)
-		}
-		if !test.wantErr {
-			if got := config.BalancerName; got != test.wantBalancerName {
-				t.Errorf("%s: config.BalancerName is %s, want %s", test.name, got, test.wantBalancerName)
-			}
-			if got := config.NodeProto; !proto.Equal(got, test.wantNodeProto) {
-				t.Errorf("%s: config.NodeProto is %#v, want %#v", test.name, got, test.wantNodeProto)
-			}
-		}
-	}
-}
-
-// TestNewConfigOnce does not override onceDoerFunc, which means that the
-// bootstrap file will be read only once. This test first supplies a bad
-// bootstrap file and makes sure that the error from reading the bootstrap file
-// is stored in package level vars and returned on subsequent calls to
-// NewConfig.
-func TestNewConfigOnce(t *testing.T) {
-	// This test could be executed multiple times as part of the same test
-	// binary (especially in cases where we pass in different values for the
-	// -cpu flag). We want each run to start off with a fresh state.
-	bsOnce = sync.Once{}
-
 	bootstrapFileMap := map[string]string{
+		"empty":   "",
 		"badJSON": `["test": 123]`,
+		"emptyNodeProto": `
+		{
+			"xds_server" : {
+				"server_uri": "trafficdirector.googleapis.com:443"
+			}
+		}`,
+		"emptyXdsServer": `
+		{
+			"node": {
+				"id": "ENVOY_NODE_ID",
+				"metadata": {
+				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
+			    }
+			}
+		}`,
+		"unknownTopLevelFieldInFile": `
+		{
+			"node": {
+				"id": "ENVOY_NODE_ID",
+				"metadata": {
+				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
+			    }
+			},
+			"xds_server" : {
+				"server_uri": "trafficdirector.googleapis.com:443",
+				"channel_creds": [
+					{ "type": "not-google-default" }
+				]
+			},
+			"unknownField": "foobar"
+		}`,
+		"unknownFieldInNodeProto": `
+		{
+			"node": {
+				"id": "ENVOY_NODE_ID",
+				"unknownField": "foobar",
+				"metadata": {
+				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
+			    }
+			}
+		}`,
+		"unknownFieldInXdsServer": `
+		{
+			"node": {
+				"id": "ENVOY_NODE_ID",
+				"metadata": {
+				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
+			    }
+			},
+			"xds_server" : {
+				"server_uri": "trafficdirector.googleapis.com:443",
+				"channel_creds": [
+					{ "type": "not-google-default" }
+				],
+				"unknownField": "foobar"
+			}
+		}`,
+		"emptyChannelCreds": `
+		{
+			"node": {
+				"id": "ENVOY_NODE_ID",
+				"metadata": {
+				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
+			    }
+			},
+			"xds_server" : {
+				"server_uri": "trafficdirector.googleapis.com:443"
+			}
+		}`,
+		"nonGoogleDefaultCreds": `
+		{
+			"node": {
+				"id": "ENVOY_NODE_ID",
+				"metadata": {
+				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
+			    }
+			},
+			"xds_server" : {
+				"server_uri": "trafficdirector.googleapis.com:443",
+				"channel_creds": [
+					{ "type": "not-google-default" }
+				]
+			}
+		}`,
+		"multipleChannelCreds": `
+		{
+			"node": {
+				"id": "ENVOY_NODE_ID",
+				"metadata": {
+				    "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector"
+			    }
+			},
+			"xds_server" : {
+				"server_uri": "trafficdirector.googleapis.com:443",
+				"channel_creds": [
+					{ "type": "not-google-default" },
+					{ "type": "google_default" }
+				]
+			}
+		}`,
 		"goodBootstrap": `
 		{
 			"node": {
@@ -331,13 +169,9 @@ func TestNewConfigOnce(t *testing.T) {
 			    }
 			},
 			"xds_server" : {
-			    "api_type": "GRPC",
+				"server_uri": "trafficdirector.googleapis.com:443",
-			    "grpc_services": [
+				"channel_creds": [
-					{
+					{ "type": "google_default" }
-						"google_grpc": {
-							"target_uri": "trafficdirector.googleapis.com:443"
-						}
-					}
 				]
 			}
 		}`,
@@ -355,21 +189,45 @@ func TestNewConfigOnce(t *testing.T) {
 		os.Unsetenv(bootstrapFileEnv)
 	}()
 
-	// Pass bad JSON in bootstrap file. This should end up being stored in
+	tests := []struct {
-	// package level vars and returned in further calls to NewConfig.
+		name       string
-	if err := os.Setenv(bootstrapFileEnv, "badJSON"); err != nil {
+		wantConfig *Config
-		t.Fatalf("os.Setenv(%s, badJSON) failed with error: %v", bootstrapFileEnv, err)
+	}{
-	}
+		{"nonExistentBootstrapFile", &Config{}},
-	if _, err := NewConfig(); err == nil || !strings.Contains(err.Error(), "json.Unmarshal") {
+		{"empty", &Config{}},
-		t.Fatalf("NewConfig() returned error: %v, want json.Unmarshal error", err)
+		{"badJSON", &Config{}},
+		{"emptyNodeProto", &Config{BalancerName: "trafficdirector.googleapis.com:443"}},
+		{"emptyXdsServer", &Config{NodeProto: nodeProto}},
+		{"unknownTopLevelFieldInFile", nilCredsConfig},
+		{"unknownFieldInNodeProto", &Config{NodeProto: nodeProto}},
+		{"unknownFieldInXdsServer", nilCredsConfig},
+		{"emptyChannelCreds", nilCredsConfig},
+		{"nonGoogleDefaultCreds", nilCredsConfig},
+		{"multipleChannelCreds", nonNilCredsConfig},
+		{"goodBootstrap", nonNilCredsConfig},
 	}
 
-	// Setting the bootstrap file to valid contents should not succeed, as the
+	for _, test := range tests {
-	// file is read only on the first call to NewConfig.
+		if err := os.Setenv(bootstrapFileEnv, test.name); err != nil {
-	if err := os.Setenv(bootstrapFileEnv, "goodBootstrap"); err != nil {
+			t.Fatalf("os.Setenv(%s, %s) failed with error: %v", bootstrapFileEnv, test.name, err)
-		t.Fatalf("os.Setenv(%s, badJSON) failed with error: %v", bootstrapFileEnv, err)
 		}
-	if _, err := NewConfig(); err == nil || !strings.Contains(err.Error(), "json.Unmarshal") {
+		config := NewConfig()
-		t.Fatalf("NewConfig() returned error: %v, want json.Unmarshal error", err)
+		if config.BalancerName != test.wantConfig.BalancerName {
+			t.Errorf("%s: config.BalancerName is %s, want %s", test.name, config.BalancerName, test.wantConfig.BalancerName)
+		}
+		if !proto.Equal(config.NodeProto, test.wantConfig.NodeProto) {
+			t.Errorf("%s: config.NodeProto is %#v, want %#v", test.name, config.NodeProto, test.wantConfig.NodeProto)
+		}
+		if (config.Creds != nil) != (test.wantConfig.Creds != nil) {
+			t.Errorf("%s: config.Creds is %#v, want %#v", test.name, config.Creds, test.wantConfig.Creds)
+		}
+	}
+}
+
+func TestNewConfigEnvNotSet(t *testing.T) {
+	os.Unsetenv(bootstrapFileEnv)
+	wantConfig := Config{}
+	if config := NewConfig(); *config != wantConfig {
+		t.Errorf("NewConfig() returned : %#v, wanted an empty Config object", config)
 	}
 }