From 25a49a9b58edae4e9fdf009ff2ae579d14662d0b Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Tue, 12 Oct 2021 11:01:41 -0700 Subject: [PATCH 1/9] [federation_bootstrap] xds/bootstrap: support new fields for xds federation Support new bootstrap fields - client_default_listener_resource_name_template - authorities Changes - new struct for server config (URI, creds, v2/v3, node), and reused for top level and per-authority server - and fix all the usages - node proto message is now for each server, to be consistent with TransportAPI version - config test to use cmp.Diff --- xds/csds/csds.go | 2 +- xds/googledirectpath/googlec2p.go | 10 +- xds/googledirectpath/googlec2p_test.go | 13 +- xds/internal/xdsclient/bootstrap/bootstrap.go | 272 ++++++++++---- .../xdsclient/bootstrap/bootstrap_test.go | 335 ++++++++++++++---- xds/internal/xdsclient/client.go | 30 +- xds/internal/xdsclient/client_test.go | 16 +- xds/internal/xdsclient/dump_test.go | 32 +- xds/internal/xdsclient/loadreport.go | 4 +- xds/internal/xdsclient/loadreport_test.go | 10 +- xds/internal/xdsclient/xdsclient_test.go | 36 +- xds/server_test.go | 32 +- 12 files changed, 585 insertions(+), 207 deletions(-) diff --git a/xds/csds/csds.go b/xds/csds/csds.go index 1d817fbcc865..23f9c760b637 100644 --- a/xds/csds/csds.go +++ b/xds/csds/csds.go @@ -127,7 +127,7 @@ func (s *ClientStatusDiscoveryServer) buildClientStatusRespForReq(req *v3statusp ret := &v3statuspb.ClientStatusResponse{ Config: []*v3statuspb.ClientConfig{ { - Node: nodeProtoToV3(s.xdsClient.BootstrapConfig().NodeProto), + Node: nodeProtoToV3(s.xdsClient.BootstrapConfig().XDSServer.NodeProto), GenericXdsConfigs: configs, }, }, diff --git a/xds/googledirectpath/googlec2p.go b/xds/googledirectpath/googlec2p.go index b9f1c712014e..4f753e49c71a 100644 --- a/xds/googledirectpath/googlec2p.go +++ b/xds/googledirectpath/googlec2p.go @@ -103,10 +103,12 @@ func (c2pResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, opts balancerName = tdURL } config := &bootstrap.Config{ - BalancerName: balancerName, - Creds: grpc.WithCredentialsBundle(google.NewDefaultCredentials()), - TransportAPI: version.TransportV3, - NodeProto: newNode(<-zoneCh, <-ipv6CapableCh), + XDSServer: &bootstrap.ServerConfig{ + ServerURI: balancerName, + Creds: grpc.WithCredentialsBundle(google.NewDefaultCredentials()), + TransportAPI: version.TransportV3, + NodeProto: newNode(<-zoneCh, <-ipv6CapableCh), + }, } // Create singleton xds client with this config. The xds client will be diff --git a/xds/googledirectpath/googlec2p_test.go b/xds/googledirectpath/googlec2p_test.go index a208fad66c58..c8162317cc30 100644 --- a/xds/googledirectpath/googlec2p_test.go +++ b/xds/googledirectpath/googlec2p_test.go @@ -211,15 +211,18 @@ func TestBuildXDS(t *testing.T) { } } wantConfig := &bootstrap.Config{ - BalancerName: tdURL, - TransportAPI: version.TransportV3, - NodeProto: wantNode, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: tdURL, + TransportAPI: version.TransportV3, + NodeProto: wantNode, + }, } if tt.tdURI != "" { - wantConfig.BalancerName = tt.tdURI + wantConfig.XDSServer.ServerURI = tt.tdURI } cmpOpts := cmp.Options{ - cmpopts.IgnoreFields(bootstrap.Config{}, "Creds"), + cmpopts.IgnoreFields(bootstrap.ServerConfig{}, "Creds"), + cmp.AllowUnexported(bootstrap.ServerConfig{}), protocmp.Transform(), } select { diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index fa229d99593e..59ba7089afe8 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -25,6 +25,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "strings" v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -58,18 +59,17 @@ var gRPCVersion = fmt.Sprintf("%s %s", gRPCUserAgentName, grpc.Version) // For overriding in unit tests. var bootstrapFileReadFunc = ioutil.ReadFile -// Config provides the xDS client with several key bits of information that it -// requires in its interaction with the management server. The Config is -// initialized from the bootstrap file. -type Config struct { - // BalancerName is the name of the management server to connect to. +type ServerConfig struct { + // ServerURI is the management server to connect to. // - // The bootstrap file contains a list of servers (with name+creds), but we - // pick the first one. - BalancerName string + // The bootstrap file contains an ordered list of xDS servers to contact for + // this authority. The first one is picked. + ServerURI string // Creds contains the credentials to be used while talking to the xDS // server, as a grpc.DialOption. Creds grpc.DialOption + // credsType is the type of the creds. It will be used to dedup servers. + credsType string // 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. @@ -77,15 +77,137 @@ type Config struct { // NodeProto contains the Node proto to be used in xDS requests. The actual // type depends on the transport protocol version used. NodeProto proto.Message +} + +// UnmarshalJSON takes the json data (a list of servers) and unmarshals the +// first one in the list. +func (sc *ServerConfig) UnmarshalJSON(data []byte) error { + var servers []*xdsServer + if err := json.Unmarshal(data, &servers); err != nil { + return fmt.Errorf("xds: json.Unmarshal(data) for field xds_servers failed during bootstrap: %v", err) + } + if len(servers) < 1 { + return fmt.Errorf("xds: bootstrap file parsing failed during bootstrap: file doesn't contain any management server to connect to") + } + xs := servers[0] + sc.ServerURI = xs.ServerURI + for _, cc := range xs.ChannelCreds { + // We stop at the first credential type that we support. + sc.credsType = cc.Type + if cc.Type == credsGoogleDefault { + sc.Creds = grpc.WithCredentialsBundle(google.NewDefaultCredentials()) + break + } else if cc.Type == credsInsecure { + sc.Creds = grpc.WithTransportCredentials(insecure.NewCredentials()) + break + } + } + for _, f := range xs.ServerFeatures { + if f == serverFeaturesV3 { + sc.TransportAPI = version.TransportV3 + } + } + return nil +} + +type Authority struct { + // ClientListenerResourceNameTemplate is template for the name of the + // Listener resource to subscribe to for a gRPC client channel. Used only + // when the channel is created using an "xds:" URI with this authority name. + // + // The token "%s", if present in this string, will be replaced + // with %-encoded service authority (i.e., the path part of the target + // URI used to create the gRPC channel). + // + // Must start with "xdstp:///". If it does not, + // that is considered a bootstrap file parsing error. + // + // If not present in the bootstrap file, defaults to + // "xdstp:///envoy.config.listener.v3.Listener/%s". + ClientListenerResourceNameTemplate string + // XDSServer is FIXME. + XDSServer *ServerConfig +} + +func (a *Authority) UnmarshalJSON(data []byte) error { + var jsonData map[string]json.RawMessage + if err := json.Unmarshal(data, &jsonData); err != nil { + return fmt.Errorf("xds: failed to parse authority: %v", err) + } + + for k, v := range jsonData { + switch k { + case "xds_servers": + if err := json.Unmarshal(v, &a.XDSServer); err != nil { + return fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) + } + case "client_listener_resource_name_template": + // FIXME: must start with "xdstp://" + if err := json.Unmarshal(v, &a.ClientListenerResourceNameTemplate); err != nil { + return fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) + } + } + } + return nil +} + +// Config provides the xDS client with several key bits of information that it +// requires in its interaction with the management server. The Config is +// initialized from the bootstrap file. +type Config struct { + // XDSServer is the management server to connect to. + // + // The bootstrap file contains a list of servers (with name+creds), but we + // pick the first one. + XDSServer *ServerConfig // CertProviderConfigs contains a mapping from certificate provider plugin // instance names to parsed buildable configs. CertProviderConfigs map[string]*certprovider.BuildableConfig // ServerListenerResourceNameTemplate is a template for the name of the - // Listener resource to subscribe to for a gRPC server. If the token `%s` is - // present in the string, it will be replaced with the server's listening - // "IP:port" (e.g., "0.0.0.0:8080", "[::]:8080"). For example, a value of - // "example/resource/%s" could become "example/resource/0.0.0.0:8080". + // Listener resource to subscribe to for a gRPC server. + // + // If starts with "xdstp:", will be interpreted as a new-style name, + // in which case the authority of the URI will be used to select the + // relevant configuration in the "authorities" map. + // + // The token "%s", if present in this string, will be replaced with the IP + // and port on which the server is listening. (e.g., "0.0.0.0:8080", + // "[::]:8080"). For example, a value of // "example/resource/%s" could + // become "example/resource/0.0.0.0:8080". If the template starts with + // "xdstp:", the replaced string will be %-encoded. + // + // There is no default; if unset, xDS-based server creation fails. ServerListenerResourceNameTemplate string + // A template for the name of the Listener resource to subscribe to + // for a gRPC client channel. Used only when the channel is created + // with an "xds:" URI with no authority. + // + // If starts with "xdstp:", will be interpreted as a new-style name, + // in which case the authority of the URI will be used to select the + // relevant configuration in the "authorities" map. + // + // The token "%s", if present in this string, will be replaced with + // the service authority (i.e., the path part of the target URI + // used to create the gRPC channel). If the template starts with + // "xdstp:", the replaced string will be %-encoded. + // + // Defaults to "%s". + ClientDefaultListenerResourceNameTemplate string + + // Authorities is a map of authority name to corresponding configuration. + // + // This is used in the following cases: + // - A gRPC client channel is created using an "xds:" URI that includes + // an authority. + // - A gRPC client channel is created using an "xds:" URI with no + // authority, but the "client_default_listener_resource_name_template" + // field above turns it into an "xdstp:" URI. + // - A gRPC server is created and the + // "server_listener_resource_name_template" field is an "xdstp:" URI. + // + // In any of those cases, it is an error if the specified authority is + // not present in this map. + Authorities map[string]*Authority } type channelCreds struct { @@ -181,7 +303,7 @@ func NewConfigFromContents(data []byte) (*Config, error) { return nil, fmt.Errorf("xds: Failed to parse bootstrap config: %v", err) } - serverSupportsV3 := false + var node *v3corepb.Node m := jsonpb.Unmarshaler{AllowUnknownFields: true} for k, v := range jsonData { switch k { @@ -192,37 +314,14 @@ func NewConfigFromContents(data []byte) (*Config, error) { // "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 { + node = &v3corepb.Node{} + if err := m.Unmarshal(bytes.NewReader(v), node); err != nil { return nil, fmt.Errorf("xds: jsonpb.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } - config.NodeProto = n case "xds_servers": - var servers []*xdsServer - if err := json.Unmarshal(v, &servers); err != nil { + if err := json.Unmarshal(v, &config.XDSServer); err != nil { return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } - if len(servers) < 1 { - return nil, fmt.Errorf("xds: bootstrap file parsing failed during bootstrap: file doesn't contain any management server to connect to") - } - xs := servers[0] - config.BalancerName = xs.ServerURI - for _, cc := range xs.ChannelCreds { - // We stop at the first credential type that we support. - if cc.Type == credsGoogleDefault { - config.Creds = grpc.WithCredentialsBundle(google.NewDefaultCredentials()) - break - } else if cc.Type == credsInsecure { - config.Creds = grpc.WithTransportCredentials(insecure.NewCredentials()) - break - } - } - for _, f := range xs.ServerFeatures { - switch f { - case serverFeaturesV3: - serverSupportsV3 = true - } - } case "certificate_providers": var providerInstances map[string]json.RawMessage if err := json.Unmarshal(v, &providerInstances); err != nil { @@ -256,27 +355,48 @@ func NewConfigFromContents(data []byte) (*Config, error) { if err := json.Unmarshal(v, &config.ServerListenerResourceNameTemplate); err != nil { return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } + case "client_default_listener_resource_name_template": + if err := json.Unmarshal(v, &config.ClientDefaultListenerResourceNameTemplate); err != nil { + return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) + } + case "authorities": + if err := json.Unmarshal(v, &config.Authorities); err != nil { + return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) + } } // 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 // file with new fields. } - if config.BalancerName == "" { + if config.ClientDefaultListenerResourceNameTemplate == "" { + // Default value of the default client listener name template is "%s". + config.ClientDefaultListenerResourceNameTemplate = "%s" + } + if config.XDSServer == nil { + return nil, fmt.Errorf("xds: Required field %q not found in bootstrap %s", "xds_servers", jsonData["xds_servers"]) + } + if config.XDSServer.ServerURI == "" { return nil, fmt.Errorf("xds: Required field %q not found in bootstrap %s", "xds_servers.server_uri", jsonData["xds_servers"]) } - if config.Creds == nil { + if config.XDSServer.Creds == nil { return nil, fmt.Errorf("xds: Required field %q doesn't contain valid value in bootstrap %s", "xds_servers.channel_creds", jsonData["xds_servers"]) } - - // We end up using v3 transport protocol version only if the server supports - // v3, indicated by the presence of "xds_v3" in server_features. The default - // value of the enum type "version.TransportAPI" is v2. - if serverSupportsV3 { - config.TransportAPI = version.TransportV3 + // Post-process the authorities' client listener resource template field: + // - if set, it must start with "xdstp:///" + // - if not set, it defaults to "xdstp:///envoy.config.listener.v3.Listener/%s" + for n, a := range config.Authorities { + prefix := fmt.Sprintf("xdstp://%s", n) + if a.ClientListenerResourceNameTemplate == "" { + a.ClientListenerResourceNameTemplate = prefix + "/envoy.config.listener.v3.Listener/%s" + continue + } + if !strings.HasPrefix(a.ClientListenerResourceNameTemplate, prefix) { + return nil, fmt.Errorf("xds: field ClientListenerResourceNameTemplate %q of authority %q doesn't start with prefix %q", a.ClientListenerResourceNameTemplate, n, prefix) + } } - if err := config.updateNodeProto(); err != nil { + if err := config.updateNodeProto(node); err != nil { return nil, err } logger.Infof("Bootstrap config for creating xds-client: %v", pretty.ToJSON(config)) @@ -295,37 +415,47 @@ func NewConfigFromContents(data []byte) (*Config, error) { // 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 +func (c *Config) updateNodeProto(node *v3corepb.Node) error { + v3 := node + if v3 == nil { + v3 = &v3corepb.Node{} } + v3.UserAgentName = gRPCUserAgentName + v3.UserAgentVersionType = &v3corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version} + v3.ClientFeatures = append(v3.ClientFeatures, clientFeatureNoOverprovisioning) 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) - } + v3bytes, err := proto.Marshal(v3) + if err != nil { + return fmt.Errorf("xds: proto.Marshal(%v): %v", v3, err) + } + if err := proto.Unmarshal(v3bytes, v2); err != nil { + return fmt.Errorf("xds: proto.Unmarshal(%v): %v", v3bytes, 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) + + switch c.XDSServer.TransportAPI { + case version.TransportV2: + c.XDSServer.NodeProto = v2 + case version.TransportV3: + c.XDSServer.NodeProto = v3 + } + + for _, a := range c.Authorities { + if a.XDSServer == nil { + continue + } + switch a.XDSServer.TransportAPI { + case version.TransportV2: + a.XDSServer.NodeProto = v2 + case version.TransportV3: + a.XDSServer.NodeProto = v3 + } + } + return nil } diff --git a/xds/internal/xdsclient/bootstrap/bootstrap_test.go b/xds/internal/xdsclient/bootstrap/bootstrap_test.go index 501d62102d21..6cab7a7f92cd 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap_test.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap_test.go @@ -30,6 +30,7 @@ import ( "github.com/golang/protobuf/proto" structpb "github.com/golang/protobuf/ptypes/struct" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc" "google.golang.org/grpc/credentials/google" @@ -207,59 +208,44 @@ var ( ClientFeatures: []string{clientFeatureNoOverprovisioning}, } nilCredsConfigV2 = &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: v2NodeProto, + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + credsType: "insecure", + NodeProto: v2NodeProto, + }, + ClientDefaultListenerResourceNameTemplate: "%s", } nonNilCredsConfigV2 = &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - NodeProto: v2NodeProto, + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + NodeProto: v2NodeProto, + }, + ClientDefaultListenerResourceNameTemplate: "%s", } nonNilCredsConfigV3 = &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - TransportAPI: version.TransportV3, - NodeProto: v3NodeProto, + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV3, + NodeProto: v3NodeProto, + }, + ClientDefaultListenerResourceNameTemplate: "%s", } ) 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) - } - if c.ServerListenerResourceNameTemplate != want.ServerListenerResourceNameTemplate { - return fmt.Errorf("config.ServerListenerResourceNameTemplate is %q, want %q", c.ServerListenerResourceNameTemplate, want.ServerListenerResourceNameTemplate) - } - - // A vanilla cmp.Equal or cmp.Diff will not produce useful error message - // here. So, we iterate through the list of configs and compare them one at - // a time. - gotCfgs := c.CertProviderConfigs - wantCfgs := want.CertProviderConfigs - if len(gotCfgs) != len(wantCfgs) { - return fmt.Errorf("config.CertProviderConfigs is %d entries, want %d", len(gotCfgs), len(wantCfgs)) - } - for instance, gotCfg := range gotCfgs { - wantCfg, ok := wantCfgs[instance] - if !ok { - return fmt.Errorf("config.CertProviderConfigs has unexpected plugin instance %q with config %q", instance, gotCfg.String()) - } - if got, want := gotCfg.String(), wantCfg.String(); got != want { - return fmt.Errorf("config.CertProviderConfigs for plugin instance %q has config %q, want %q", instance, got, want) - } + if diff := cmp.Diff(c, want, + cmpopts.EquateEmpty(), + cmp.AllowUnexported(ServerConfig{}), + cmp.Comparer(proto.Equal), + cmp.Comparer(func(a, b grpc.DialOption) bool { return (a != nil) == (b != nil) }), + cmp.Transformer("certproviderconfigstring", func(a *certprovider.BuildableConfig) string { return a.String() }), + ); diff != "" { + return fmt.Errorf("diff: %v", diff) } return nil } @@ -285,6 +271,7 @@ func setupBootstrapOverride(bootstrapFileMap map[string]string) func() { // This function overrides the bootstrap file NAME env variable, to test the // code that reads file with the given fileName. func testNewConfigWithFileNameEnv(t *testing.T, fileName string, wantError bool, wantConfig *Config) { + t.Helper() origBootstrapFileName := env.BootstrapFileName env.BootstrapFileName = fileName defer func() { env.BootstrapFileName = origBootstrapFileName }() @@ -304,10 +291,10 @@ func testNewConfigWithFileNameEnv(t *testing.T, fileName string, wantError bool, // This function overrides the bootstrap file CONTENT env variable, to test the // code that uses the content from env directly. func testNewConfigWithFileContentEnv(t *testing.T, fileName string, wantError bool, wantConfig *Config) { + t.Helper() b, err := bootstrapFileReadFunc(fileName) if err != nil { - // If file reading failed, skip this test. - return + t.Skip(err) } origBootstrapContent := env.BootstrapFileContent env.BootstrapFileContent = string(b) @@ -404,14 +391,18 @@ func TestNewConfigV2ProtoSuccess(t *testing.T) { }{ { "emptyNodeProto", &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: &v2corepb.Node{ - BuildVersion: gRPCVersion, - UserAgentName: gRPCUserAgentName, - UserAgentVersionType: &v2corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version}, - ClientFeatures: []string{clientFeatureNoOverprovisioning}, + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + credsType: "insecure", + NodeProto: &v2corepb.Node{ + BuildVersion: gRPCVersion, + UserAgentName: gRPCUserAgentName, + UserAgentVersionType: &v2corepb.Node_UserAgentVersion{UserAgentVersion: grpc.Version}, + ClientFeatures: []string{clientFeatureNoOverprovisioning}, + }, }, + ClientDefaultListenerResourceNameTemplate: "%s", }, }, {"unknownTopLevelFieldInFile", nilCredsConfigV2}, @@ -670,13 +661,17 @@ func TestNewConfigWithCertificateProviders(t *testing.T) { defer cancel() goodConfig := &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - TransportAPI: version.TransportV3, - NodeProto: v3NodeProto, + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV3, + NodeProto: v3NodeProto, + }, CertProviderConfigs: map[string]*certprovider.BuildableConfig{ "fakeProviderInstance": wantCfg, }, + ClientDefaultListenerResourceNameTemplate: "%s", } tests := []struct { name string @@ -760,11 +755,225 @@ func TestNewConfigWithServerListenerResourceNameTemplate(t *testing.T) { { name: "goodServerListenerResourceNameTemplate", wantConfig: &Config{ - BalancerName: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - TransportAPI: version.TransportV2, - NodeProto: v2NodeProto, - ServerListenerResourceNameTemplate: "grpc/server?xds.resource.listening_address=%s", + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV2, + NodeProto: v2NodeProto, + }, + ServerListenerResourceNameTemplate: "grpc/server?xds.resource.listening_address=%s", + ClientDefaultListenerResourceNameTemplate: "%s", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + testNewConfigWithFileNameEnv(t, test.name, test.wantErr, test.wantConfig) + testNewConfigWithFileContentEnv(t, test.name, test.wantErr, test.wantConfig) + }) + } +} + +func TestNewConfigWithFederation(t *testing.T) { + cancel := setupBootstrapOverride(map[string]string{ + "badClientListenerResourceNameTemplate": ` + { + "node": { "id": "ENVOY_NODE_ID" }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443" + }], + "client_default_listener_resource_name_template": 123456789 + }`, + "badClientListenerResourceNameTemplatePerAuthority": ` + { + "node": { "id": "ENVOY_NODE_ID" }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ { "type": "google_default" } ] + }], + "authorities": { + "xds.td.com": { + "client_listener_resource_name_template": "some/template/%s", + "xds_servers": [{ + "server_uri": "td.com", + "channel_creds": [ { "type": "google_default" } ], + "server_features" : ["foo", "bar", "xds_v3"] + }] + } + } + }`, + "good": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ { "type": "google_default" } ] + }], + "server_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/grpc/server?listening_address=%s", + "client_default_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", + "authorities": { + "xds.td.com": { + "client_listener_resource_name_template": "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", + "xds_servers": [{ + "server_uri": "td.com", + "channel_creds": [ { "type": "google_default" } ], + "server_features" : ["foo", "bar", "xds_v3"] + }] + } + } + }`, + // If client_default_listener_resource_name_template is not set, it + // defaults to "%s". + "goodWithDefaultDefaultClientListenerTemplate": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ { "type": "google_default" } ] + }] + }`, + // If client_listener_resource_name_template in authority is not set, it + // defaults to + // "xdstp:///envoy.config.listener.v3.Listener/%s". + "goodWithDefaultClientListenerTemplatePerAuthority": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ { "type": "google_default" } ] + }], + "client_default_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", + "authorities": { + "xds.td.com": { } + } + }`, + // It's OK for an authority to not have servers. The top-level server + // will be used. + "goodWithNoServerPerAuthority": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ { "type": "google_default" } ] + }], + "client_default_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", + "authorities": { + "xds.td.com": { + "client_listener_resource_name_template": "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s" + } + } + }`, + }) + defer cancel() + + tests := []struct { + name string + wantConfig *Config + wantErr bool + }{ + { + name: "badClientListenerResourceNameTemplate", + wantErr: true, + }, + { + name: "badClientListenerResourceNameTemplatePerAuthority", + wantErr: true, + }, + { + name: "good", + wantConfig: &Config{ + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV2, + NodeProto: v2NodeProto, + }, + ServerListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/grpc/server?listening_address=%s", + ClientDefaultListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", + Authorities: map[string]*Authority{ + "xds.td.com": { + ClientListenerResourceNameTemplate: "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", + XDSServer: &ServerConfig{ + ServerURI: "td.com", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV3, + NodeProto: v3NodeProto, + }, + }, + }, + }, + }, + { + name: "goodWithDefaultDefaultClientListenerTemplate", + wantConfig: &Config{ + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV2, + NodeProto: v2NodeProto, + }, + ClientDefaultListenerResourceNameTemplate: "%s", + }, + }, + { + name: "goodWithDefaultClientListenerTemplatePerAuthority", + wantConfig: &Config{ + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV2, + NodeProto: v2NodeProto, + }, + ClientDefaultListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", + Authorities: map[string]*Authority{ + "xds.td.com": { + ClientListenerResourceNameTemplate: "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", + }, + }, + }, + }, + { + name: "goodWithNoServerPerAuthority", + wantConfig: &Config{ + XDSServer: &ServerConfig{ + ServerURI: "trafficdirector.googleapis.com:443", + Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), + credsType: "google_default", + TransportAPI: version.TransportV2, + NodeProto: v2NodeProto, + }, + ClientDefaultListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", + Authorities: map[string]*Authority{ + "xds.td.com": { + ClientListenerResourceNameTemplate: "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", + }, + }, }, }, } diff --git a/xds/internal/xdsclient/client.go b/xds/internal/xdsclient/client.go index 3230c66c06e3..cf48f446c4d9 100644 --- a/xds/internal/xdsclient/client.go +++ b/xds/internal/xdsclient/client.go @@ -629,27 +629,29 @@ type clientImpl struct { // newWithConfig returns a new xdsClient with the given config. func newWithConfig(config *bootstrap.Config, watchExpiryTimeout time.Duration) (*clientImpl, error) { switch { - case config.BalancerName == "": + case config.XDSServer == nil: + return nil, errors.New("xds: no xds_server provided") + case config.XDSServer.ServerURI == "": return nil, errors.New("xds: no xds_server name provided in options") - case config.Creds == nil: + case config.XDSServer.Creds == nil: return nil, errors.New("xds: no credentials provided in options") - case config.NodeProto == nil: + case config.XDSServer.NodeProto == nil: return nil, errors.New("xds: no node_proto provided in options") } - switch config.TransportAPI { + switch config.XDSServer.TransportAPI { case version.TransportV2: - if _, ok := config.NodeProto.(*v2corepb.Node); !ok { - return nil, fmt.Errorf("xds: Node proto type (%T) does not match API version: %v", config.NodeProto, config.TransportAPI) + if _, ok := config.XDSServer.NodeProto.(*v2corepb.Node); !ok { + return nil, fmt.Errorf("xds: Node proto type (%T) does not match API version: %v", config.XDSServer.NodeProto, config.XDSServer.TransportAPI) } case version.TransportV3: - if _, ok := config.NodeProto.(*v3corepb.Node); !ok { - return nil, fmt.Errorf("xds: Node proto type (%T) does not match API version: %v", config.NodeProto, config.TransportAPI) + if _, ok := config.XDSServer.NodeProto.(*v3corepb.Node); !ok { + return nil, fmt.Errorf("xds: Node proto type (%T) does not match API version: %v", config.XDSServer.NodeProto, config.XDSServer.TransportAPI) } } dopts := []grpc.DialOption{ - config.Creds, + config.XDSServer.Creds, grpc.WithKeepaliveParams(keepalive.ClientParameters{ Time: 5 * time.Minute, Timeout: 20 * time.Second, @@ -677,19 +679,19 @@ func newWithConfig(config *bootstrap.Config, watchExpiryTimeout time.Duration) ( lrsClients: make(map[string]*lrsClient), } - cc, err := grpc.Dial(config.BalancerName, dopts...) + cc, err := grpc.Dial(config.XDSServer.ServerURI, dopts...) if err != nil { // An error from a non-blocking dial indicates something serious. - return nil, fmt.Errorf("xds: failed to dial balancer {%s}: %v", config.BalancerName, err) + return nil, fmt.Errorf("xds: failed to dial balancer {%s}: %v", config.XDSServer.ServerURI, err) } c.cc = cc c.logger = prefixLogger((c)) - c.logger.Infof("Created ClientConn to xDS management server: %s", config.BalancerName) + c.logger.Infof("Created ClientConn to xDS management server: %s", config.XDSServer) - apiClient, err := newAPIClient(config.TransportAPI, cc, BuildOptions{ + apiClient, err := newAPIClient(config.XDSServer.TransportAPI, cc, BuildOptions{ Parent: c, Validator: c.updateValidator, - NodeProto: config.NodeProto, + NodeProto: config.XDSServer.NodeProto, Backoff: backoff.DefaultExponential.Backoff, Logger: c.logger, }) diff --git a/xds/internal/xdsclient/client_test.go b/xds/internal/xdsclient/client_test.go index 7c3423cd5ad7..2a6d6ae2a536 100644 --- a/xds/internal/xdsclient/client_test.go +++ b/xds/internal/xdsclient/client_test.go @@ -82,9 +82,11 @@ func clientOpts(balancerName string, overrideWatchExpiryTimeout bool) (*bootstra watchExpiryTimeout = defaultTestWatchExpiryTimeout } return &bootstrap.Config{ - BalancerName: balancerName, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: balancerName, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV2, + }, }, watchExpiryTimeout } @@ -269,9 +271,11 @@ func (s) TestClientNewSingleton(t *testing.T) { oldBootstrapNewConfig := bootstrapNewConfig bootstrapNewConfig = func() (*bootstrap.Config, error) { return &bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithInsecure(), - NodeProto: xdstestutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithInsecure(), + NodeProto: xdstestutils.EmptyNodeProtoV2, + }, }, nil } defer func() { bootstrapNewConfig = oldBootstrapNewConfig }() diff --git a/xds/internal/xdsclient/dump_test.go b/xds/internal/xdsclient/dump_test.go index d03479ca4ade..c162a9418f23 100644 --- a/xds/internal/xdsclient/dump_test.go +++ b/xds/internal/xdsclient/dump_test.go @@ -75,9 +75,11 @@ func (s) TestLDSConfigDump(t *testing.T) { } client, err := xdsclient.NewWithConfigForTesting(&bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV2, + }, }, defaultTestWatchExpiryTimeout) if err != nil { t.Fatalf("failed to create client: %v", err) @@ -189,9 +191,11 @@ func (s) TestRDSConfigDump(t *testing.T) { } client, err := xdsclient.NewWithConfigForTesting(&bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV2, + }, }, defaultTestWatchExpiryTimeout) if err != nil { t.Fatalf("failed to create client: %v", err) @@ -303,9 +307,11 @@ func (s) TestCDSConfigDump(t *testing.T) { } client, err := xdsclient.NewWithConfigForTesting(&bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV2, + }, }, defaultTestWatchExpiryTimeout) if err != nil { t.Fatalf("failed to create client: %v", err) @@ -403,9 +409,11 @@ func (s) TestEDSConfigDump(t *testing.T) { } client, err := xdsclient.NewWithConfigForTesting(&bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV2, + }, }, defaultTestWatchExpiryTimeout) if err != nil { t.Fatalf("failed to create client: %v", err) diff --git a/xds/internal/xdsclient/loadreport.go b/xds/internal/xdsclient/loadreport.go index 32a71dada7fb..4df9a5c0c3a4 100644 --- a/xds/internal/xdsclient/loadreport.go +++ b/xds/internal/xdsclient/loadreport.go @@ -115,12 +115,12 @@ func (lrsC *lrsClient) startStream() { var cc *grpc.ClientConn lrsC.parent.logger.Infof("Starting load report to server: %s", lrsC.server) - if lrsC.server == "" || lrsC.server == lrsC.parent.config.BalancerName { + if lrsC.server == "" || lrsC.server == lrsC.parent.config.XDSServer.ServerURI { // Reuse the xDS client if server is the same. cc = lrsC.parent.cc } else { lrsC.parent.logger.Infof("LRS server is different from management server, starting a new ClientConn") - ccNew, err := grpc.Dial(lrsC.server, lrsC.parent.config.Creds) + ccNew, err := grpc.Dial(lrsC.server, lrsC.parent.config.XDSServer.Creds) if err != nil { // An error from a non-blocking dial indicates something serious. lrsC.parent.logger.Infof("xds: failed to dial load report server {%s}: %v", lrsC.server, err) diff --git a/xds/internal/xdsclient/loadreport_test.go b/xds/internal/xdsclient/loadreport_test.go index 88a08eb43fda..db31de6cf78b 100644 --- a/xds/internal/xdsclient/loadreport_test.go +++ b/xds/internal/xdsclient/loadreport_test.go @@ -55,10 +55,12 @@ func (s) TestLRSClient(t *testing.T) { defer sCleanup() xdsC, err := xdsclient.NewWithConfigForTesting(&bootstrap.Config{ - BalancerName: fs.Address, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: &v2corepb.Node{}, - TransportAPI: version.TransportV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: fs.Address, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + TransportAPI: version.TransportV2, + NodeProto: &v2corepb.Node{}, + }, }, defaultClientWatchExpiryTimeout) if err != nil { t.Fatalf("failed to create xds client: %v", err) diff --git a/xds/internal/xdsclient/xdsclient_test.go b/xds/internal/xdsclient/xdsclient_test.go index f348df481615..23d6d6f54183 100644 --- a/xds/internal/xdsclient/xdsclient_test.go +++ b/xds/internal/xdsclient/xdsclient_test.go @@ -56,34 +56,42 @@ func (s) TestNew(t *testing.T) { { name: "empty-balancer-name", config: &bootstrap.Config{ - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: testutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: testutils.EmptyNodeProtoV2, + }, }, wantErr: true, }, { name: "empty-dial-creds", config: &bootstrap.Config{ - BalancerName: testXDSServer, - NodeProto: testutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + NodeProto: testutils.EmptyNodeProtoV2, + }, }, wantErr: true, }, { name: "empty-node-proto", config: &bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + }, }, wantErr: true, }, { name: "node-proto-version-mismatch", config: &bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: testutils.EmptyNodeProtoV3, - TransportAPI: version.TransportV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + TransportAPI: version.TransportV2, + NodeProto: testutils.EmptyNodeProtoV3, + }, }, wantErr: true, }, @@ -91,9 +99,11 @@ func (s) TestNew(t *testing.T) { { name: "happy-case", config: &bootstrap.Config{ - BalancerName: testXDSServer, - Creds: grpc.WithInsecure(), - NodeProto: testutils.EmptyNodeProtoV2, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: testXDSServer, + Creds: grpc.WithInsecure(), + NodeProto: testutils.EmptyNodeProtoV2, + }, }, }, } diff --git a/xds/server_test.go b/xds/server_test.go index 0866e0414ae2..4f9a7548c6d8 100644 --- a/xds/server_test.go +++ b/xds/server_test.go @@ -321,9 +321,11 @@ func setupOverrides() (*fakeGRPCServer, *testutils.Channel, func()) { newXDSClient = func() (xdsclient.XDSClient, error) { c := fakeclient.NewClient() c.SetBootstrapConfig(&bootstrap.Config{ - BalancerName: "dummyBalancer", - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV3, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: "dummyBalancer", + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV3, + }, ServerListenerResourceNameTemplate: testServerListenerResourceNameTemplate, CertProviderConfigs: certProviderConfigs, }) @@ -351,9 +353,11 @@ func setupOverridesForXDSCreds(includeCertProviderCfg bool) (*testutils.Channel, newXDSClient = func() (xdsclient.XDSClient, error) { c := fakeclient.NewClient() bc := &bootstrap.Config{ - BalancerName: "dummyBalancer", - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV3, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: "dummyBalancer", + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV3, + }, ServerListenerResourceNameTemplate: testServerListenerResourceNameTemplate, } if includeCertProviderCfg { @@ -598,18 +602,22 @@ func (s) TestServeBootstrapConfigInvalid(t *testing.T) { { desc: "certificate provider config is missing", bootstrapConfig: &bootstrap.Config{ - BalancerName: "dummyBalancer", - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV3, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: "dummyBalancer", + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV3, + }, ServerListenerResourceNameTemplate: testServerListenerResourceNameTemplate, }, }, { desc: "server_listener_resource_name_template is missing", bootstrapConfig: &bootstrap.Config{ - BalancerName: "dummyBalancer", - Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), - NodeProto: xdstestutils.EmptyNodeProtoV3, + XDSServer: &bootstrap.ServerConfig{ + ServerURI: "dummyBalancer", + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + NodeProto: xdstestutils.EmptyNodeProtoV3, + }, CertProviderConfigs: certProviderConfigs, }, }, From 2c34b06b5f4145fe99e6bbbac0fa9300951bf7e3 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Fri, 15 Oct 2021 12:10:44 -0700 Subject: [PATCH 2/9] [federation_bootstrap] c1 --- xds/internal/xdsclient/bootstrap/bootstrap.go | 71 +++++++------------ 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index 59ba7089afe8..d3f35520ef58 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -59,6 +59,8 @@ var gRPCVersion = fmt.Sprintf("%s %s", gRPCUserAgentName, grpc.Version) // For overriding in unit tests. var bootstrapFileReadFunc = ioutil.ReadFile +// ServerConfig contains the configuration to connect to a server, including +// URI, creds, and transport API version (e.g. v2 or v3). type ServerConfig struct { // ServerURI is the management server to connect to. // @@ -76,6 +78,10 @@ type ServerConfig struct { TransportAPI version.TransportAPI // NodeProto contains the Node proto to be used in xDS requests. The actual // type depends on the transport protocol version used. + // + // Note that it's specified in the bootstrap globally for all the servers, + // but we keep it in each server config so that its type (e.g. *v2pb.Node or + // *v3pb.Node) is consistent with the transport API version. NodeProto proto.Message } @@ -125,7 +131,7 @@ type Authority struct { // If not present in the bootstrap file, defaults to // "xdstp:///envoy.config.listener.v3.Listener/%s". ClientListenerResourceNameTemplate string - // XDSServer is FIXME. + // XDSServer is the management server to connect to for this authority. XDSServer *ServerConfig } @@ -142,7 +148,6 @@ func (a *Authority) UnmarshalJSON(data []byte) error { return fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } case "client_listener_resource_name_template": - // FIXME: must start with "xdstp://" if err := json.Unmarshal(v, &a.ClientListenerResourceNameTemplate); err != nil { return fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } @@ -172,9 +177,9 @@ type Config struct { // // The token "%s", if present in this string, will be replaced with the IP // and port on which the server is listening. (e.g., "0.0.0.0:8080", - // "[::]:8080"). For example, a value of // "example/resource/%s" could - // become "example/resource/0.0.0.0:8080". If the template starts with - // "xdstp:", the replaced string will be %-encoded. + // "[::]:8080"). For example, a value of "example/resource/%s" could become + // "example/resource/0.0.0.0:8080". If the template starts with "xdstp:", + // the replaced string will be %-encoded. // // There is no default; if unset, xDS-based server creation fails. ServerListenerResourceNameTemplate string @@ -247,34 +252,6 @@ func bootstrapConfigFromEnvVariable() ([]byte, error) { // NewConfig returns a new instance of Config initialized by reading the // bootstrap file found at ${GRPC_XDS_BOOTSTRAP}. // -// The format of the bootstrap file will be as follows: -// { -// "xds_servers": [ -// { -// "server_uri": , -// "channel_creds": [ -// { -// "type": , -// "config": -// } -// ], -// "server_features": [ ... ], -// } -// ], -// "node": , -// "certificate_providers" : { -// "default": { -// "plugin_name": "default-plugin-name", -// "config": { default plugin config in JSON } -// }, -// "foo": { -// "plugin_name": "foo", -// "config": { foo plugin config in JSON } -// } -// }, -// "server_listener_resource_name_template": "grpc/server?xds.resource.listening_address=%s" -// } -// // 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. @@ -385,14 +362,14 @@ func NewConfigFromContents(data []byte) (*Config, error) { // Post-process the authorities' client listener resource template field: // - if set, it must start with "xdstp:///" // - if not set, it defaults to "xdstp:///envoy.config.listener.v3.Listener/%s" - for n, a := range config.Authorities { - prefix := fmt.Sprintf("xdstp://%s", n) - if a.ClientListenerResourceNameTemplate == "" { - a.ClientListenerResourceNameTemplate = prefix + "/envoy.config.listener.v3.Listener/%s" + for name, authority := range config.Authorities { + prefix := fmt.Sprintf("xdstp://%s", name) + if authority.ClientListenerResourceNameTemplate == "" { + authority.ClientListenerResourceNameTemplate = prefix + "/envoy.config.listener.v3.Listener/%s" continue } - if !strings.HasPrefix(a.ClientListenerResourceNameTemplate, prefix) { - return nil, fmt.Errorf("xds: field ClientListenerResourceNameTemplate %q of authority %q doesn't start with prefix %q", a.ClientListenerResourceNameTemplate, n, prefix) + if !strings.HasPrefix(authority.ClientListenerResourceNameTemplate, prefix) { + return nil, fmt.Errorf("xds: field ClientListenerResourceNameTemplate %q of authority %q doesn't start with prefix %q", authority.ClientListenerResourceNameTemplate, name, prefix) } } @@ -405,16 +382,16 @@ func NewConfigFromContents(data []byte) (*Config, error) { // 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 +// The input node is 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 +// 1. If the node is nil, we create an empty one here. That way, callers of this +// function can always expect that the NodeProto field is non-nil. +// 2. Some additional fields which are not expected to be set in the bootstrap // file are populated here. +// 3. For each server config (both top level and in each authority), we set its +// node field to the v3.Node, or a v2.Node with the same content, depending on +// the server's transprot API version. func (c *Config) updateNodeProto(node *v3corepb.Node) error { v3 := node if v3 == nil { From 392ac365cd04e108d3e89209dc291a446cfc3f97 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Fri, 15 Oct 2021 14:03:44 -0700 Subject: [PATCH 3/9] [federation_bootstrap] c2 --- xds/internal/xdsclient/client.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/xds/internal/xdsclient/client.go b/xds/internal/xdsclient/client.go index cf48f446c4d9..8b7d4bad04f9 100644 --- a/xds/internal/xdsclient/client.go +++ b/xds/internal/xdsclient/client.go @@ -28,8 +28,6 @@ import ( "sync" "time" - 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" "google.golang.org/protobuf/types/known/anypb" @@ -639,17 +637,6 @@ func newWithConfig(config *bootstrap.Config, watchExpiryTimeout time.Duration) ( return nil, errors.New("xds: no node_proto provided in options") } - switch config.XDSServer.TransportAPI { - case version.TransportV2: - if _, ok := config.XDSServer.NodeProto.(*v2corepb.Node); !ok { - return nil, fmt.Errorf("xds: Node proto type (%T) does not match API version: %v", config.XDSServer.NodeProto, config.XDSServer.TransportAPI) - } - case version.TransportV3: - if _, ok := config.XDSServer.NodeProto.(*v3corepb.Node); !ok { - return nil, fmt.Errorf("xds: Node proto type (%T) does not match API version: %v", config.XDSServer.NodeProto, config.XDSServer.TransportAPI) - } - } - dopts := []grpc.DialOption{ config.XDSServer.Creds, grpc.WithKeepaliveParams(keepalive.ClientParameters{ @@ -696,6 +683,7 @@ func newWithConfig(config *bootstrap.Config, watchExpiryTimeout time.Duration) ( Logger: c.logger, }) if err != nil { + cc.Close() return nil, err } c.apiClient = apiClient From b1641940a48d0e5258d72c2a2dc32f5d71dc18a6 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Fri, 15 Oct 2021 14:08:05 -0700 Subject: [PATCH 4/9] [federation_bootstrap] examples in tests??? --- xds/internal/xdsclient/bootstrap/bootstrap.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index d3f35520ef58..d8e9f5e641a7 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -260,6 +260,9 @@ func bootstrapConfigFromEnvVariable() ([]byte, error) { // 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. +// +// Examples of the bootstrap json can be found in the generator tests +// https://github.com/GoogleCloudPlatform/traffic-director-grpc-bootstrap/blob/master/main_test.go. func NewConfig() (*Config, error) { data, err := bootstrapConfigFromEnvVariable() if err != nil { From 39e6ba622e78add3daf16226e1f529e52f5cba84 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Fri, 15 Oct 2021 14:09:15 -0700 Subject: [PATCH 5/9] [federation_bootstrap] out from godoc --- xds/internal/xdsclient/bootstrap/bootstrap.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index d8e9f5e641a7..9e79f5b544f8 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -260,10 +260,9 @@ func bootstrapConfigFromEnvVariable() ([]byte, error) { // 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. -// -// Examples of the bootstrap json can be found in the generator tests -// https://github.com/GoogleCloudPlatform/traffic-director-grpc-bootstrap/blob/master/main_test.go. func NewConfig() (*Config, error) { + // Examples of the bootstrap json can be found in the generator tests + // https://github.com/GoogleCloudPlatform/traffic-director-grpc-bootstrap/blob/master/main_test.go. data, err := bootstrapConfigFromEnvVariable() if err != nil { return nil, fmt.Errorf("xds: Failed to read bootstrap config: %v", err) From d8033eaf5a6042f88f1df8596c885de3da0bda4f Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Wed, 20 Oct 2021 14:16:17 -0700 Subject: [PATCH 6/9] [federation_bootstrap] add Infof for unknown bootstrap fields --- xds/internal/xdsclient/bootstrap/bootstrap.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index 9e79f5b544f8..2fd6419782ab 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -342,6 +342,8 @@ func NewConfigFromContents(data []byte) (*Config, error) { if err := json.Unmarshal(v, &config.Authorities); err != nil { return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } + default: + logger.Infof("Bootstrap content has unknown field: %s", k) } // 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 From cab7f4f7c723a370147c576c0ee3b1108d6f1e20 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Wed, 20 Oct 2021 14:17:08 -0700 Subject: [PATCH 7/9] [federation_bootstrap_refactor] Remove new fields support, so that users will see logging. Will add back after xdsclient and others are updated to make use of this new fields. --- xds/internal/xdsclient/bootstrap/bootstrap.go | 8 - .../xdsclient/bootstrap/bootstrap_test.go | 210 ------------------ 2 files changed, 218 deletions(-) diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index 2fd6419782ab..6745745be787 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -334,14 +334,6 @@ func NewConfigFromContents(data []byte) (*Config, error) { if err := json.Unmarshal(v, &config.ServerListenerResourceNameTemplate); err != nil { return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } - case "client_default_listener_resource_name_template": - if err := json.Unmarshal(v, &config.ClientDefaultListenerResourceNameTemplate); err != nil { - return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) - } - case "authorities": - if err := json.Unmarshal(v, &config.Authorities); err != nil { - return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) - } default: logger.Infof("Bootstrap content has unknown field: %s", k) } diff --git a/xds/internal/xdsclient/bootstrap/bootstrap_test.go b/xds/internal/xdsclient/bootstrap/bootstrap_test.go index 6cab7a7f92cd..9d6ead0ff5b5 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap_test.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap_test.go @@ -775,213 +775,3 @@ func TestNewConfigWithServerListenerResourceNameTemplate(t *testing.T) { }) } } - -func TestNewConfigWithFederation(t *testing.T) { - cancel := setupBootstrapOverride(map[string]string{ - "badClientListenerResourceNameTemplate": ` - { - "node": { "id": "ENVOY_NODE_ID" }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443" - }], - "client_default_listener_resource_name_template": 123456789 - }`, - "badClientListenerResourceNameTemplatePerAuthority": ` - { - "node": { "id": "ENVOY_NODE_ID" }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443", - "channel_creds": [ { "type": "google_default" } ] - }], - "authorities": { - "xds.td.com": { - "client_listener_resource_name_template": "some/template/%s", - "xds_servers": [{ - "server_uri": "td.com", - "channel_creds": [ { "type": "google_default" } ], - "server_features" : ["foo", "bar", "xds_v3"] - }] - } - } - }`, - "good": ` - { - "node": { - "id": "ENVOY_NODE_ID", - "metadata": { - "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" - } - }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443", - "channel_creds": [ { "type": "google_default" } ] - }], - "server_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/grpc/server?listening_address=%s", - "client_default_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", - "authorities": { - "xds.td.com": { - "client_listener_resource_name_template": "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", - "xds_servers": [{ - "server_uri": "td.com", - "channel_creds": [ { "type": "google_default" } ], - "server_features" : ["foo", "bar", "xds_v3"] - }] - } - } - }`, - // If client_default_listener_resource_name_template is not set, it - // defaults to "%s". - "goodWithDefaultDefaultClientListenerTemplate": ` - { - "node": { - "id": "ENVOY_NODE_ID", - "metadata": { - "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" - } - }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443", - "channel_creds": [ { "type": "google_default" } ] - }] - }`, - // If client_listener_resource_name_template in authority is not set, it - // defaults to - // "xdstp:///envoy.config.listener.v3.Listener/%s". - "goodWithDefaultClientListenerTemplatePerAuthority": ` - { - "node": { - "id": "ENVOY_NODE_ID", - "metadata": { - "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" - } - }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443", - "channel_creds": [ { "type": "google_default" } ] - }], - "client_default_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", - "authorities": { - "xds.td.com": { } - } - }`, - // It's OK for an authority to not have servers. The top-level server - // will be used. - "goodWithNoServerPerAuthority": ` - { - "node": { - "id": "ENVOY_NODE_ID", - "metadata": { - "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" - } - }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443", - "channel_creds": [ { "type": "google_default" } ] - }], - "client_default_listener_resource_name_template": "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", - "authorities": { - "xds.td.com": { - "client_listener_resource_name_template": "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s" - } - } - }`, - }) - defer cancel() - - tests := []struct { - name string - wantConfig *Config - wantErr bool - }{ - { - name: "badClientListenerResourceNameTemplate", - wantErr: true, - }, - { - name: "badClientListenerResourceNameTemplatePerAuthority", - wantErr: true, - }, - { - name: "good", - wantConfig: &Config{ - XDSServer: &ServerConfig{ - ServerURI: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - credsType: "google_default", - TransportAPI: version.TransportV2, - NodeProto: v2NodeProto, - }, - ServerListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/grpc/server?listening_address=%s", - ClientDefaultListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", - Authorities: map[string]*Authority{ - "xds.td.com": { - ClientListenerResourceNameTemplate: "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", - XDSServer: &ServerConfig{ - ServerURI: "td.com", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - credsType: "google_default", - TransportAPI: version.TransportV3, - NodeProto: v3NodeProto, - }, - }, - }, - }, - }, - { - name: "goodWithDefaultDefaultClientListenerTemplate", - wantConfig: &Config{ - XDSServer: &ServerConfig{ - ServerURI: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - credsType: "google_default", - TransportAPI: version.TransportV2, - NodeProto: v2NodeProto, - }, - ClientDefaultListenerResourceNameTemplate: "%s", - }, - }, - { - name: "goodWithDefaultClientListenerTemplatePerAuthority", - wantConfig: &Config{ - XDSServer: &ServerConfig{ - ServerURI: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - credsType: "google_default", - TransportAPI: version.TransportV2, - NodeProto: v2NodeProto, - }, - ClientDefaultListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", - Authorities: map[string]*Authority{ - "xds.td.com": { - ClientListenerResourceNameTemplate: "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", - }, - }, - }, - }, - { - name: "goodWithNoServerPerAuthority", - wantConfig: &Config{ - XDSServer: &ServerConfig{ - ServerURI: "trafficdirector.googleapis.com:443", - Creds: grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()), - credsType: "google_default", - TransportAPI: version.TransportV2, - NodeProto: v2NodeProto, - }, - ClientDefaultListenerResourceNameTemplate: "xdstp://xds.example.com/envoy.config.listener.v3.Listener/%s", - Authorities: map[string]*Authority{ - "xds.td.com": { - ClientListenerResourceNameTemplate: "xdstp://xds.td.com/envoy.config.listener.v3.Listener/%s", - }, - }, - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - testNewConfigWithFileNameEnv(t, test.name, test.wantErr, test.wantConfig) - testNewConfigWithFileContentEnv(t, test.name, test.wantErr, test.wantConfig) - }) - } -} From 3cf068b7e1abbeada66d796844a021698c1a86e0 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Wed, 20 Oct 2021 14:45:21 -0700 Subject: [PATCH 8/9] [federation_bootstrap_refactor] fix vet comments --- xds/internal/xdsclient/bootstrap/bootstrap.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index 6745745be787..c9c4faff4205 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -116,6 +116,7 @@ func (sc *ServerConfig) UnmarshalJSON(data []byte) error { return nil } +// Authority represents an Authority for an xDS control plane server. type Authority struct { // ClientListenerResourceNameTemplate is template for the name of the // Listener resource to subscribe to for a gRPC client channel. Used only @@ -135,6 +136,7 @@ type Authority struct { XDSServer *ServerConfig } +// UnmarshalJSON implement json unmarshaller. func (a *Authority) UnmarshalJSON(data []byte) error { var jsonData map[string]json.RawMessage if err := json.Unmarshal(data, &jsonData); err != nil { From 32617359f6040df52e078d14995e98a30e6adb69 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Mon, 1 Nov 2021 14:40:48 -0700 Subject: [PATCH 9/9] [federation_bootstrap_refactor] c --- xds/internal/xdsclient/bootstrap/bootstrap.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index c9c4faff4205..7f75525cc631 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -116,7 +116,8 @@ func (sc *ServerConfig) UnmarshalJSON(data []byte) error { return nil } -// Authority represents an Authority for an xDS control plane server. +// Authority contains configuration for an Authority for an xDS control plane +// server. See the Authorities field in the Config struct for how it's used. type Authority struct { // ClientListenerResourceNameTemplate is template for the name of the // Listener resource to subscribe to for a gRPC client channel. Used only @@ -132,7 +133,8 @@ type Authority struct { // If not present in the bootstrap file, defaults to // "xdstp:///envoy.config.listener.v3.Listener/%s". ClientListenerResourceNameTemplate string - // XDSServer is the management server to connect to for this authority. + // XDSServer contains the management server and config to connect to for + // this authority. XDSServer *ServerConfig } @@ -337,7 +339,7 @@ func NewConfigFromContents(data []byte) (*Config, error) { return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) } default: - logger.Infof("Bootstrap content has unknown field: %s", k) + logger.Warningf("Bootstrap content has unknown field: %s", k) } // 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