diff --git a/internal/testutils/xds/e2e/bootstrap.go b/internal/testutils/xds/e2e/bootstrap.go index 907a7b396177..b003ae55baf9 100644 --- a/internal/testutils/xds/e2e/bootstrap.go +++ b/internal/testutils/xds/e2e/bootstrap.go @@ -76,7 +76,7 @@ func DefaultBootstrapContents(t *testing.T, nodeID, serverURI string) []byte { "server_uri": "passthrough:///%s", "channel_creds": [{"type": "insecure"}] }`, serverURI))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), CertificateProviders: cpc, ServerListenerResourceNameTemplate: ServerListenerResourceNameTemplate, }) diff --git a/internal/xds/bootstrap/bootstrap.go b/internal/xds/bootstrap/bootstrap.go index f73ac864b49f..af44da183e5e 100644 --- a/internal/xds/bootstrap/bootstrap.go +++ b/internal/xds/bootstrap/bootstrap.go @@ -29,6 +29,7 @@ import ( "os" "slices" "strings" + "sync" "google.golang.org/grpc" "google.golang.org/grpc/credentials/tls/certprovider" @@ -509,54 +510,48 @@ func (c *Config) UnmarshalJSON(data []byte) error { return nil } -// Returns the bootstrap configuration from env vars ${GRPC_XDS_BOOTSTRAP} or -// ${GRPC_XDS_BOOTSTRAP_CONFIG}. If both env vars are set, the former is -// preferred. And if none of the env vars are set, an error is returned. -func bootstrapConfigFromEnvVariable() ([]byte, error) { +// GetConfiguration returns the bootstrap configuration initialized by reading +// the bootstrap file found at ${GRPC_XDS_BOOTSTRAP} or bootstrap contents +// specified at ${GRPC_XDS_BOOTSTRAP_CONFIG}. If both env vars are set, the +// former is preferred. +// +// If none of the env vars are set, this function returns the fallback +// configuration if it is not nil. Else, it returns an error. +// +// 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 GetConfiguration() (*Config, error) { fName := envconfig.XDSBootstrapFileName fContent := envconfig.XDSBootstrapFileContent if fName != "" { if logger.V(2) { - logger.Infof("Using bootstrap file with name %q", fName) + logger.Infof("Using bootstrap file with name %q from GRPC_XDS_BOOTSTRAP environment variable", fName) } - return bootstrapFileReadFunc(fName) + cfg, err := bootstrapFileReadFunc(fName) + if err != nil { + return nil, fmt.Errorf("xds: failed to read bootstrap config from file %q: %v", fName, err) + } + return newConfigFromContents(cfg) } if fContent != "" { - return []byte(fContent), nil + if logger.V(2) { + logger.Infof("Using bootstrap contents from GRPC_XDS_BOOTSTRAP_CONFIG environment variable") + } + return newConfigFromContents([]byte(fContent)) } - return nil, fmt.Errorf("none of the bootstrap environment variables (%q or %q) defined", envconfig.XDSBootstrapFileNameEnv, envconfig.XDSBootstrapFileContentEnv) -} - -// NewConfig returns a new instance of Config initialized by reading the -// bootstrap file found at ${GRPC_XDS_BOOTSTRAP} or bootstrap contents specified -// at ${GRPC_XDS_BOOTSTRAP_CONFIG}. If both env vars are set, the former is -// preferred. -// -// We support a credential registration mechanism and only credentials -// registered through that mechanism will be accepted here. See package -// `xds/bootstrap` for details. -// -// 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, 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) + if cfg := fallbackBootstrapConfig(); cfg != nil { + if logger.V(2) { + logger.Infof("Using bootstrap contents from fallback config") + } + return cfg, nil } - return newConfigFromContents(data) -} -// NewConfigFromContents returns a new Config using the specified -// bootstrap file contents instead of reading the environment variable. -func NewConfigFromContents(data []byte) (*Config, error) { - return newConfigFromContents(data) + return nil, fmt.Errorf("bootstrap environment variables (%q or %q) not defined, and no fallback config set", envconfig.XDSBootstrapFileNameEnv, envconfig.XDSBootstrapFileContentEnv) } func newConfigFromContents(data []byte) (*Config, error) { @@ -596,9 +591,9 @@ type ConfigOptionsForTesting struct { ClientDefaultListenerResourceNameTemplate string // Authorities is a list of non-default authorities. Authorities map[string]json.RawMessage - // NodeID is the node identifier of the gRPC client/server node in the + // Node identifies the gRPC client/server node in the // proxyless service mesh. - NodeID string + Node json.RawMessage } // NewContentsForTesting creates a new bootstrap configuration from the passed in @@ -630,13 +625,17 @@ func NewContentsForTesting(opts ConfigOptionsForTesting) ([]byte, error) { } authorities[k] = a } + node := newNode() + if err := json.Unmarshal(opts.Node, &node); err != nil { + return nil, fmt.Errorf("failed to unmarshal node configuration %s: %v", string(opts.Node), err) + } cfgJSON := configJSON{ XDSServers: servers, CertificateProviders: certProviders, ServerListenerResourceNameTemplate: opts.ServerListenerResourceNameTemplate, ClientDefaultListenerResourceNameTemplate: opts.ClientDefaultListenerResourceNameTemplate, Authorities: authorities, - Node: node{ID: opts.NodeID}, + Node: node, } contents, err := json.MarshalIndent(cfgJSON, " ", " ") if err != nil { @@ -645,6 +644,14 @@ func NewContentsForTesting(opts ConfigOptionsForTesting) ([]byte, error) { return contents, nil } +// NewConfigForTesting creates a new bootstrap configuration from the provided +// contents, for testing purposes. +// +// # Testing-Only +func NewConfigForTesting(contents []byte) (*Config, error) { + return newConfigFromContents(contents) +} + // certproviderNameAndConfig is the internal representation of // the`certificate_providers` field in the bootstrap configuration. type certproviderNameAndConfig struct { @@ -747,3 +754,44 @@ func (n node) toProto() *v3corepb.Node { ClientFeatures: slices.Clone(n.clientFeatures), } } + +// SetFallbackBootstrapConfig sets the fallback bootstrap configuration to be +// used when the bootstrap environment variables are unset. +// +// The provided configuration must be valid JSON. Returns a non-nil error if +// parsing the provided configuration fails. +func SetFallbackBootstrapConfig(cfgJSON []byte) error { + config, err := newConfigFromContents(cfgJSON) + if err != nil { + return err + } + + configMu.Lock() + defer configMu.Unlock() + fallbackBootstrapCfg = config + return nil +} + +// UnsetFallbackBootstrapConfigForTesting unsets the fallback bootstrap +// configuration to be used when the bootstrap environment variables are unset. +// +// # Testing-Only +func UnsetFallbackBootstrapConfigForTesting() { + configMu.Lock() + defer configMu.Unlock() + fallbackBootstrapCfg = nil +} + +// fallbackBootstrapConfig returns the fallback bootstrap configuration +// that will be used by the xDS client when the bootstrap environment +// variables are unset. +func fallbackBootstrapConfig() *Config { + configMu.Lock() + defer configMu.Unlock() + return fallbackBootstrapCfg +} + +var ( + configMu sync.Mutex + fallbackBootstrapCfg *Config +) diff --git a/internal/xds/bootstrap/bootstrap_test.go b/internal/xds/bootstrap/bootstrap_test.go index f0e48e6e3e5c..f7fe89335653 100644 --- a/internal/xds/bootstrap/bootstrap_test.go +++ b/internal/xds/bootstrap/bootstrap_test.go @@ -293,19 +293,16 @@ func setupBootstrapOverride(bootstrapFileMap map[string]string) func() { return func() { bootstrapFileReadFunc = oldFileReadFunc } } -// TODO: enable leak check for this package when -// https://github.com/googleapis/google-cloud-go/issues/2417 is fixed. - // 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) { +func testGetConfigurationWithFileNameEnv(t *testing.T, fileName string, wantError bool, wantConfig *Config) { origBootstrapFileName := envconfig.XDSBootstrapFileName envconfig.XDSBootstrapFileName = fileName defer func() { envconfig.XDSBootstrapFileName = origBootstrapFileName }() - c, err := NewConfig() + c, err := GetConfiguration() if (err != nil) != wantError { - t.Fatalf("NewConfig() returned error %v, wantError: %v", err, wantError) + t.Fatalf("GetConfiguration() returned error %v, wantError: %v", err, wantError) } if wantError { return @@ -317,7 +314,7 @@ 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) { +func testGetConfigurationWithFileContentEnv(t *testing.T, fileName string, wantError bool, wantConfig *Config) { t.Helper() b, err := bootstrapFileReadFunc(fileName) if err != nil { @@ -327,9 +324,9 @@ func testNewConfigWithFileContentEnv(t *testing.T, fileName string, wantError bo envconfig.XDSBootstrapFileContent = string(b) defer func() { envconfig.XDSBootstrapFileContent = origBootstrapContent }() - c, err := NewConfig() + c, err := GetConfiguration() if (err != nil) != wantError { - t.Fatalf("NewConfig() returned error %v, wantError: %v", err, wantError) + t.Fatalf("GetConfiguration() returned error %v, wantError: %v", err, wantError) } if wantError { return @@ -339,8 +336,9 @@ func testNewConfigWithFileContentEnv(t *testing.T, fileName string, wantError bo } } -// Tests NewConfig with bootstrap file contents that are expected to fail. -func (s) TestNewConfig_Failure(t *testing.T) { +// Tests GetConfiguration with bootstrap file contents that are expected to +// fail. +func (s) TestGetConfiguration_Failure(t *testing.T) { bootstrapFileMap := map[string]string{ "empty": "", "badJSON": `["test": 123]`, @@ -387,16 +385,16 @@ func (s) TestNewConfig_Failure(t *testing.T) { for _, name := range []string{"nonExistentBootstrapFile", "empty", "badJSON", "noBalancerName", "emptyXdsServer"} { t.Run(name, func(t *testing.T) { - testNewConfigWithFileNameEnv(t, name, true, nil) - testNewConfigWithFileContentEnv(t, name, true, nil) + testGetConfigurationWithFileNameEnv(t, name, true, nil) + testGetConfigurationWithFileContentEnv(t, name, true, nil) }) } } -// TestNewConfigV3ProtoSuccess exercises the functionality in NewConfig with -// different bootstrap file contents. It overrides the fileReadFunc by returning -// bootstrap file contents defined in this test, instead of reading from a file. -func (s) TestNewConfig_Success(t *testing.T) { +// Tests the functionality in GetConfiguration with different bootstrap file +// contents. It overrides the fileReadFunc by returning bootstrap file contents +// defined in this test, instead of reading from a file. +func (s) TestGetConfiguration_Success(t *testing.T) { cancel := setupBootstrapOverride(v3BootstrapFileMap) defer cancel() @@ -431,19 +429,18 @@ func (s) TestNewConfig_Success(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - testNewConfigWithFileNameEnv(t, test.name, false, test.wantConfig) - testNewConfigWithFileContentEnv(t, test.name, false, test.wantConfig) + testGetConfigurationWithFileNameEnv(t, test.name, false, test.wantConfig) + testGetConfigurationWithFileContentEnv(t, test.name, false, test.wantConfig) }) } } -// TestNewConfigBootstrapEnvPriority tests that the two env variables are read -// in correct priority. +// Tests that the two bootstrap env variables are read in correct priority. // // "GRPC_XDS_BOOTSTRAP" which specifies the file name containing the bootstrap // configuration takes precedence over "GRPC_XDS_BOOTSTRAP_CONFIG", which // directly specifies the bootstrap configuration in itself. -func (s) TestNewConfigBootstrapEnvPriority(t *testing.T) { +func (s) TestGetConfiguration_BootstrapEnvPriority(t *testing.T) { oldFileReadFunc := bootstrapFileReadFunc bootstrapFileReadFunc = func(filename string) ([]byte, error) { return fileReadFromFileMap(v3BootstrapFileMap, filename) @@ -465,17 +462,17 @@ func (s) TestNewConfigBootstrapEnvPriority(t *testing.T) { envconfig.XDSBootstrapFileContent = "" defer func() { envconfig.XDSBootstrapFileContent = origBootstrapContent }() - // When both env variables are empty, NewConfig should fail. - if _, err := NewConfig(); err == nil { - t.Errorf("NewConfig() returned nil error, expected to fail") + // When both env variables are empty, GetConfiguration should fail. + if _, err := GetConfiguration(); err == nil { + t.Errorf("GetConfiguration() returned nil error, expected to fail") } // When one of them is set, it should be used. envconfig.XDSBootstrapFileName = goodFileName1 envconfig.XDSBootstrapFileContent = "" - c, err := NewConfig() + c, err := GetConfiguration() if err != nil { - t.Errorf("NewConfig() failed: %v", err) + t.Errorf("GetConfiguration() failed: %v", err) } if diff := cmp.Diff(goodConfig1, c); diff != "" { t.Errorf("Unexpected diff in bootstrap configuration (-want, +got):\n%s", diff) @@ -483,9 +480,9 @@ func (s) TestNewConfigBootstrapEnvPriority(t *testing.T) { envconfig.XDSBootstrapFileName = "" envconfig.XDSBootstrapFileContent = goodFileContent2 - c, err = NewConfig() + c, err = GetConfiguration() if err != nil { - t.Errorf("NewConfig() failed: %v", err) + t.Errorf("GetConfiguration() failed: %v", err) } if diff := cmp.Diff(goodConfig2, c); diff != "" { t.Errorf("Unexpected diff in bootstrap configuration (-want, +got):\n%s", diff) @@ -494,9 +491,9 @@ func (s) TestNewConfigBootstrapEnvPriority(t *testing.T) { // Set both, file name should be read. envconfig.XDSBootstrapFileName = goodFileName1 envconfig.XDSBootstrapFileContent = goodFileContent2 - c, err = NewConfig() + c, err = GetConfiguration() if err != nil { - t.Errorf("NewConfig() failed: %v", err) + t.Errorf("GetConfiguration() failed: %v", err) } if diff := cmp.Diff(goodConfig1, c); diff != "" { t.Errorf("Unexpected diff in bootstrap configuration (-want, +got):\n%s", diff) @@ -554,7 +551,7 @@ type fakeCertProvider struct { certprovider.Provider } -func (s) TestNewConfigWithCertificateProviders(t *testing.T) { +func (s) TestGetConfiguration_CertificateProviders(t *testing.T) { bootstrapFileMap := map[string]string{ "badJSONCertProviderConfig": ` { @@ -706,13 +703,13 @@ func (s) TestNewConfigWithCertificateProviders(t *testing.T) { 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) + testGetConfigurationWithFileNameEnv(t, test.name, test.wantErr, test.wantConfig) + testGetConfigurationWithFileContentEnv(t, test.name, test.wantErr, test.wantConfig) }) } } -func (s) TestNewConfigWithServerListenerResourceNameTemplate(t *testing.T) { +func (s) TestGetConfiguration_ServerListenerResourceNameTemplate(t *testing.T) { cancel := setupBootstrapOverride(map[string]string{ "badServerListenerResourceNameTemplate:": ` { @@ -775,13 +772,13 @@ func (s) TestNewConfigWithServerListenerResourceNameTemplate(t *testing.T) { 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) + testGetConfigurationWithFileNameEnv(t, test.name, test.wantErr, test.wantConfig) + testGetConfigurationWithFileContentEnv(t, test.name, test.wantErr, test.wantConfig) }) } } -func (s) TestNewConfigWithFederation(t *testing.T) { +func (s) TestGetConfiguration_Federation(t *testing.T) { cancel := setupBootstrapOverride(map[string]string{ "badclientListenerResourceNameTemplate": ` { @@ -984,8 +981,8 @@ func (s) TestNewConfigWithFederation(t *testing.T) { 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) + testGetConfigurationWithFileNameEnv(t, test.name, test.wantErr, test.wantConfig) + testGetConfigurationWithFileContentEnv(t, test.name, test.wantErr, test.wantConfig) }) } } diff --git a/stats/opentelemetry/csm/pluginoption.go b/stats/opentelemetry/csm/pluginoption.go index 6f1da7e472b6..e415f2f53588 100644 --- a/stats/opentelemetry/csm/pluginoption.go +++ b/stats/opentelemetry/csm/pluginoption.go @@ -290,7 +290,7 @@ func initializeLocalAndMetadataLabels(labels map[string]string) (map[string]stri // getNodeID gets the Node ID from the bootstrap data. func getNodeID() string { - cfg, err := bootstrap.NewConfig() + cfg, err := bootstrap.GetConfiguration() if err != nil { return "" // will become "unknown" } diff --git a/test/xds/xds_client_certificate_providers_test.go b/test/xds/xds_client_certificate_providers_test.go index 5dc7dc9a89f8..49bd00feba0c 100644 --- a/test/xds/xds_client_certificate_providers_test.go +++ b/test/xds/xds_client_certificate_providers_test.go @@ -122,7 +122,7 @@ func (s) TestClientSideXDS_WithNoCertificateProvidersInBootstrap_Failure(t *test "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), }) if err != nil { t.Fatalf("Failed to create bootstrap configuration: %v", err) diff --git a/test/xds/xds_client_federation_test.go b/test/xds/xds_client_federation_test.go index 68571fe396f7..4d0346404d06 100644 --- a/test/xds/xds_client_federation_test.go +++ b/test/xds/xds_client_federation_test.go @@ -68,7 +68,7 @@ func (s) TestClientSideFederation(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, serverDefaultAuth.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, // Specify the address of the non-default authority. Authorities: map[string]json.RawMessage{ @@ -162,7 +162,7 @@ func (s) TestClientSideFederationWithOnlyXDSTPStyleLDS(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), ClientDefaultListenerResourceNameTemplate: fmt.Sprintf("xdstp://%s/envoy.config.listener.v3.Listener/%%s", authority), // Specify the address of the non-default authority. Authorities: map[string]json.RawMessage{ diff --git a/test/xds/xds_client_ignore_resource_deletion_test.go b/test/xds/xds_client_ignore_resource_deletion_test.go index ba38f3e05511..8b11d4ad12ad 100644 --- a/test/xds/xds_client_ignore_resource_deletion_test.go +++ b/test/xds/xds_client_ignore_resource_deletion_test.go @@ -276,7 +276,7 @@ func generateBootstrapContents(t *testing.T, serverURI string, ignoreResourceDel } bootstrapContents, err := bootstrap.NewContentsForTesting(bootstrap.ConfigOptionsForTesting{ Servers: []json.RawMessage{serverCfg}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, }) if err != nil { diff --git a/test/xds/xds_server_certificate_providers_test.go b/test/xds/xds_server_certificate_providers_test.go index b4305b3a706d..17748e038f12 100644 --- a/test/xds/xds_server_certificate_providers_test.go +++ b/test/xds/xds_server_certificate_providers_test.go @@ -134,7 +134,7 @@ func (s) TestServerSideXDS_WithNoCertificateProvidersInBootstrap_Failure(t *test "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, }) if err != nil { diff --git a/xds/googledirectpath/googlec2p.go b/xds/googledirectpath/googlec2p.go index c4ed0506726c..936bf2da3274 100644 --- a/xds/googledirectpath/googlec2p.go +++ b/xds/googledirectpath/googlec2p.go @@ -26,6 +26,7 @@ package googledirectpath import ( + "encoding/json" "fmt" "math/rand" "net/url" @@ -37,7 +38,6 @@ import ( internalgrpclog "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/resolver" - "google.golang.org/grpc/xds/internal/xdsclient" _ "google.golang.org/grpc/xds" // To register xds resolvers and balancers. ) @@ -58,15 +58,9 @@ const ( // For overriding in unittests. var ( - onGCE = googlecloud.OnGCE - + onGCE = googlecloud.OnGCE randInt = rand.Int - - newClientWithConfig = func(name string, config *bootstrap.Config) (xdsclient.XDSClient, func(), error) { - return xdsclient.NewWithConfig(name, config) - } - - logger = internalgrpclog.NewPrefixLogger(grpclog.Component("directpath"), logPrefix) + logger = internalgrpclog.NewPrefixLogger(grpclog.Component("directpath"), logPrefix) ) func init() { @@ -105,23 +99,18 @@ func (c2pResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, opts xdsServerCfg := newXdsServerConfig(xdsServerURI) authoritiesCfg := newAuthoritiesConfig(xdsServerCfg) - config, err := bootstrap.NewConfigFromContents([]byte(fmt.Sprintf(` - { - "xds_servers": [%s], - "client_default_listener_resource_name_template": "%%s", - "authorities": %s, - "node": %s - }`, xdsServerCfg, authoritiesCfg, nodeCfg))) - - if err != nil { - return nil, fmt.Errorf("failed to build bootstrap configuration: %v", err) + cfg := map[string]any{ + "xds_servers": []any{xdsServerCfg}, + "client_default_listener_resource_name_template": "%s", + "authorities": authoritiesCfg, + "node": nodeCfg, } - - // Create singleton xds client with this config. The xds client will be - // used by the xds resolver later. - _, close, err := newClientWithConfig(t.String(), config) + cfgJSON, err := json.Marshal(cfg) if err != nil { - return nil, fmt.Errorf("failed to start xDS client: %v", err) + return nil, fmt.Errorf("failed to marshal bootstrap configuration: %v", err) + } + if err := bootstrap.SetFallbackBootstrapConfig(cfgJSON); err != nil { + return nil, fmt.Errorf("failed to set fallback bootstrap configuration: %v", err) } t = resolver.Target{ @@ -131,64 +120,36 @@ func (c2pResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, opts Path: t.URL.Path, }, } - xdsR, err := resolver.Get(xdsName).Build(t, cc, opts) - if err != nil { - close() - return nil, err - } - return &c2pResolver{ - Resolver: xdsR, - clientCloseFunc: close, - }, nil + return resolver.Get(xdsName).Build(t, cc, opts) } func (b c2pResolverBuilder) Scheme() string { return c2pScheme } -type c2pResolver struct { - resolver.Resolver - clientCloseFunc func() -} - -func (r *c2pResolver) Close() { - r.Resolver.Close() - r.clientCloseFunc() -} - -func newNodeConfig(zone string, ipv6Capable bool) string { - metadata := "" +func newNodeConfig(zone string, ipv6Capable bool) map[string]any { + node := map[string]any{ + "id": fmt.Sprintf("C2P-%d", randInt()), + "locality": map[string]any{"zone": zone}, + } if ipv6Capable { - metadata = fmt.Sprintf(`, "metadata": { "%s": true }`, ipv6CapableMetadataName) + node["metadata"] = map[string]any{ipv6CapableMetadataName: true} } - - return fmt.Sprintf(` - { - "id": "%s", - "locality": { - "zone": "%s" - } - %s - }`, fmt.Sprintf("C2P-%d", randInt()), zone, metadata) + return node } -func newAuthoritiesConfig(xdsServer string) string { - return fmt.Sprintf(` - { - "%s": { - "xds_servers": [%s] - } +func newAuthoritiesConfig(serverCfg map[string]any) map[string]any { + return map[string]any{ + c2pAuthority: map[string]any{"xds_servers": []any{serverCfg}}, } - `, c2pAuthority, xdsServer) } -func newXdsServerConfig(xdsServerURI string) string { - return fmt.Sprintf(` - { - "server_uri": "%s", - "channel_creds": [{"type": "google_default"}], - "server_features": ["ignore_resource_deletion"] - }`, xdsServerURI) +func newXdsServerConfig(uri string) map[string]any { + return map[string]any{ + "server_uri": uri, + "channel_creds": []map[string]any{{"type": "google_default"}}, + "server_features": []any{"ignore_resource_deletion"}, + } } // runDirectPath returns whether this resolver should use direct path. diff --git a/xds/googledirectpath/googlec2p_test.go b/xds/googledirectpath/googlec2p_test.go index 96db393729e0..e26993d8a435 100644 --- a/xds/googledirectpath/googlec2p_test.go +++ b/xds/googledirectpath/googlec2p_test.go @@ -19,7 +19,7 @@ package googledirectpath import ( - "context" + "encoding/json" "strconv" "strings" "testing" @@ -32,11 +32,8 @@ import ( "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/resolver" - "google.golang.org/grpc/xds/internal/xdsclient" ) -const defaultTestTimeout = 10 * time.Second - type s struct { grpctest.Tester } @@ -85,28 +82,6 @@ func simulateRunningOnGCE(t *testing.T, gce bool) { t.Cleanup(func() { onGCE = oldOnGCE }) } -type testXDSClient struct { - xdsclient.XDSClient - closed chan struct{} -} - -func (c *testXDSClient) Close() { - c.closed <- struct{}{} -} - -// Overrides the creation of a real xDS client with a test one. -func overrideWithTestXDSClient(t *testing.T) (*testXDSClient, chan *bootstrap.Config) { - xdsC := &testXDSClient{closed: make(chan struct{}, 1)} - configCh := make(chan *bootstrap.Config, 1) - oldNewClient := newClientWithConfig - newClientWithConfig = func(name string, config *bootstrap.Config) (xdsclient.XDSClient, func(), error) { - configCh <- config - return xdsC, func() { xdsC.Close() }, nil - } - t.Cleanup(func() { newClientWithConfig = oldNewClient }) - return xdsC, configCh -} - // Tests the scenario where the bootstrap env vars are set and we're running on // GCE. The test builds a google-c2p resolver and verifies that an xDS resolver // is built and that we don't fallback to DNS (because federation is enabled by @@ -123,8 +98,6 @@ func (s) TestBuildWithBootstrapEnvSet(t *testing.T) { *envP = "does not matter" defer func() { *envP = oldEnv }() - overrideWithTestXDSClient(t) - // Build the google-c2p resolver. r, err := builder.Build(resolver.Target{}, nil, resolver.BuildOptions{}) if err != nil { @@ -133,9 +106,8 @@ func (s) TestBuildWithBootstrapEnvSet(t *testing.T) { defer r.Close() // Build should return xDS, not DNS. - rr := r.(*c2pResolver) - if rrr := rr.Resolver; rrr != testXDSResolver { - t.Fatalf("want xds resolver, got %#v", rrr) + if r != testXDSResolver { + t.Fatalf("Build() returned %#v, want xds resolver", r) } }) } @@ -157,8 +129,22 @@ func (s) TestBuildNotOnGCE(t *testing.T) { // Build should return DNS, not xDS. if r != testDNSResolver { - t.Fatalf("want dns resolver, got %#v", r) + t.Fatalf("Build() returned %#v, want dns resolver", r) + } +} + +func bootstrapConfig(t *testing.T, opts bootstrap.ConfigOptionsForTesting) *bootstrap.Config { + t.Helper() + + contents, err := bootstrap.NewContentsForTesting(opts) + if err != nil { + t.Fatalf("Failed to create bootstrap contents: %v", err) + } + cfg, err := bootstrap.NewConfigForTesting(contents) + if err != nil { + t.Fatalf("Failed to create bootstrap config: %v", err) } + return cfg } // Test that when a google-c2p resolver is built, the xDS client is built with @@ -186,120 +172,87 @@ func (s) TestBuildXDS(t *testing.T) { }{ { desc: "ipv6 false", - wantBootstrapConfig: func() *bootstrap.Config { - cfg, err := bootstrap.NewConfigFromContents([]byte(`{ -"xds_servers": [ - { - "server_uri": "dns:///directpath-pa.googleapis.com", - "channel_creds": [{"type": "google_default"}], - "server_features": ["ignore_resource_deletion"] - } -], -"client_default_listener_resource_name_template": "%s", -"authorities": { - "traffic-director-c2p.xds.googleapis.com": { - "xds_servers": [ - { - "server_uri": "dns:///directpath-pa.googleapis.com", - "channel_creds": [{"type": "google_default"}], - "server_features": ["ignore_resource_deletion"] - } - ] - } -}, -"node": { - "id": "C2P-666", - "locality": { - "zone": "test-zone" - } -} -}`)) - if err != nil { - t.Fatalf("Bootstrap parsing failure: %v", err) - } - return cfg - }(), + wantBootstrapConfig: bootstrapConfig(t, bootstrap.ConfigOptionsForTesting{ + Servers: []json.RawMessage{[]byte(`{ + "server_uri": "dns:///directpath-pa.googleapis.com", + "channel_creds": [{"type": "google_default"}], + "server_features": ["ignore_resource_deletion"] + }`)}, + Authorities: map[string]json.RawMessage{ + "traffic-director-c2p.xds.googleapis.com": []byte(`{ + "xds_servers": [ + { + "server_uri": "dns:///directpath-pa.googleapis.com", + "channel_creds": [{"type": "google_default"}], + "server_features": ["ignore_resource_deletion"] + } + ] + }`), + }, + Node: []byte(`{ + "id": "C2P-666", + "locality": {"zone": "test-zone"} + }`), + }), }, { desc: "ipv6 true", ipv6Capable: true, - wantBootstrapConfig: func() *bootstrap.Config { - cfg, err := bootstrap.NewConfigFromContents([]byte(`{ -"xds_servers": [ - { - "server_uri": "dns:///directpath-pa.googleapis.com", - "channel_creds": [{"type": "google_default"}], - "server_features": ["ignore_resource_deletion"] - } -], -"client_default_listener_resource_name_template": "%s", -"authorities": { - "traffic-director-c2p.xds.googleapis.com": { - "xds_servers": [ - { - "server_uri": "dns:///directpath-pa.googleapis.com", - "channel_creds": [{"type": "google_default"}], - "server_features": ["ignore_resource_deletion"] - } - ] - } -}, -"node": { - "id": "C2P-666", - "locality": { - "zone": "test-zone" - }, - "metadata": { - "TRAFFICDIRECTOR_DIRECTPATH_C2P_IPV6_CAPABLE": true - } -} -}`)) - if err != nil { - t.Fatalf("Bootstrap parsing failure: %v", err) - } - return cfg - }(), + wantBootstrapConfig: bootstrapConfig(t, bootstrap.ConfigOptionsForTesting{ + Servers: []json.RawMessage{[]byte(`{ + "server_uri": "dns:///directpath-pa.googleapis.com", + "channel_creds": [{"type": "google_default"}], + "server_features": ["ignore_resource_deletion"] + }`)}, + Authorities: map[string]json.RawMessage{ + "traffic-director-c2p.xds.googleapis.com": []byte(`{ + "xds_servers": [ + { + "server_uri": "dns:///directpath-pa.googleapis.com", + "channel_creds": [{"type": "google_default"}], + "server_features": ["ignore_resource_deletion"] + } + ] + }`), + }, + Node: []byte(`{ + "id": "C2P-666", + "locality": {"zone": "test-zone"}, + "metadata": { + "TRAFFICDIRECTOR_DIRECTPATH_C2P_IPV6_CAPABLE": true + } + }`), + }), }, { desc: "override TD URI", ipv6Capable: true, tdURIOverride: "test-uri", - wantBootstrapConfig: func() *bootstrap.Config { - cfg, err := bootstrap.NewConfigFromContents([]byte(`{ -"xds_servers": [ - { - "server_uri": "test-uri", - "channel_creds": [{"type": "google_default"}], - "server_features": ["ignore_resource_deletion"] - } -], -"client_default_listener_resource_name_template": "%s", -"authorities": { - "traffic-director-c2p.xds.googleapis.com": { - "xds_servers": [ - { - "server_uri": "test-uri", - "channel_creds": [{"type": "google_default"}], - "server_features": ["ignore_resource_deletion"] - } - ] - } -}, -"node": { - "id": "C2P-666", - "locality": { - "zone": "test-zone" - }, - "metadata": { - "TRAFFICDIRECTOR_DIRECTPATH_C2P_IPV6_CAPABLE": true - } -} -}`)) - if err != nil { - t.Fatalf("Bootstrap parsing failure: %v", err) - } - return cfg - }(), + wantBootstrapConfig: bootstrapConfig(t, bootstrap.ConfigOptionsForTesting{ + Servers: []json.RawMessage{[]byte(`{ + "server_uri": "test-uri", + "channel_creds": [{"type": "google_default"}], + "server_features": ["ignore_resource_deletion"] + }`)}, + Authorities: map[string]json.RawMessage{ + "traffic-director-c2p.xds.googleapis.com": []byte(`{ + "xds_servers": [ + { + "server_uri": "test-uri", + "channel_creds": [{"type": "google_default"}], + "server_features": ["ignore_resource_deletion"] + } + ] + }`), + }, + Node: []byte(`{ + "id": "C2P-666", + "locality": {"zone": "test-zone"}, + "metadata": { + "TRAFFICDIRECTOR_DIRECTPATH_C2P_IPV6_CAPABLE": true + } + }`), + }), }, } { t.Run(tt.desc, func(t *testing.T) { @@ -315,38 +268,24 @@ func (s) TestBuildXDS(t *testing.T) { defer func() { envconfig.C2PResolverTestOnlyTrafficDirectorURI = oldURI }() } - tXDSClient, configCh := overrideWithTestXDSClient(t) - - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - // Build the google-c2p resolver. r, err := builder.Build(resolver.Target{}, nil, resolver.BuildOptions{}) if err != nil { t.Fatalf("failed to build resolver: %v", err) } + defer r.Close() // Build should return xDS, not DNS. - rr := r.(*c2pResolver) - if rrr := rr.Resolver; rrr != testXDSResolver { - t.Fatalf("want xds resolver, got %#v, ", rrr) + if r != testXDSResolver { + t.Fatalf("Build() returned %#v, want xds resolver", r) } - var gotConfig *bootstrap.Config - select { - case gotConfig = <-configCh: - if diff := cmp.Diff(tt.wantBootstrapConfig, gotConfig); diff != "" { - t.Fatalf("Unexpected diff in bootstrap config (-want +got):\n%s", diff) - } - case <-ctx.Done(): - t.Fatalf("Timeout waiting for new xDS client to be built") + gotConfig, err := bootstrap.GetConfiguration() + if err != nil { + t.Fatalf("Failed to get boostrap config: %v", err) } - - r.Close() - select { - case <-tXDSClient.closed: - case <-ctx.Done(): - t.Fatalf("Timeout waiting for xDS client to be closed") + if diff := cmp.Diff(tt.wantBootstrapConfig, gotConfig); diff != "" { + t.Fatalf("Unexpected diff in bootstrap config (-want +got):\n%s", diff) } }) } diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go b/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go index 2dbe59a74111..658afebe7e56 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go @@ -379,7 +379,7 @@ func (s) TestSecurityConfigNotFoundInBootstrap(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, }) if err != nil { @@ -445,7 +445,7 @@ func (s) TestCertproviderStoreError(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, CertificateProviders: map[string]json.RawMessage{e2e.ClientSideCertProviderInstance: providerCfg}, }) diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index 01440f81c8b0..c5e2aa7b7aba 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -166,7 +166,7 @@ func (s) TestResolverResourceName(t *testing.T) { "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, ClientDefaultListenerResourceNameTemplate: tt.listenerResourceNameTemplate, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), } if tt.extraAuthority != "" { // In this test, we really don't care about having multiple diff --git a/xds/internal/xdsclient/authority_test.go b/xds/internal/xdsclient/authority_test.go index 9db4284d57c2..bd6ff27bce6c 100644 --- a/xds/internal/xdsclient/authority_test.go +++ b/xds/internal/xdsclient/authority_test.go @@ -26,12 +26,12 @@ import ( "github.com/google/uuid" "google.golang.org/grpc/internal/grpcsync" - util "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" "google.golang.org/grpc/xds/internal" + "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/xds/bootstrap" - "google.golang.org/grpc/xds/internal/testutils" + xdstestutils "google.golang.org/grpc/xds/internal/testutils" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" @@ -62,16 +62,13 @@ func setupTest(ctx context.Context, t *testing.T, opts e2e.ManagementServerOptio managementServer := e2e.StartManagementServer(t, opts) contents := e2e.DefaultBootstrapContents(t, nodeID, managementServer.Address) - config, err := bootstrap.NewConfigFromContents(contents) + testutils.CreateBootstrapFileForTesting(t, contents) + config, err := bootstrap.GetConfiguration() if err != nil { - t.Fatalf("Failed to build bootstrap configuration: %v", err) - } - serverCfg, err := bootstrap.ServerConfigForTesting(bootstrap.ServerConfigTestingOptions{URI: managementServer.Address}) - if err != nil { - t.Fatalf("Failed to create server config for testing: %v", err) + t.Fatalf("Failed to read bootstrap configuration: %v", err) } a, err := newAuthority(authorityArgs{ - serverCfg: serverCfg, + serverCfg: config.XDSServers()[0], bootstrapCfg: config, serializer: grpcsync.NewCallbackSerializer(ctx), resourceTypeGetter: rtRegistry.get, @@ -94,7 +91,7 @@ func (s) TestTimerAndWatchStateOnSendCallback(t *testing.T) { defer a.close() rn := "xdsclient-test-lds-resource" - w := testutils.NewTestResourceWatcher() + w := xdstestutils.NewTestResourceWatcher() cancelResource := a.watchResource(listenerResourceType, rn, w) defer cancelResource() @@ -141,7 +138,7 @@ func (s) TestTimerAndWatchStateOnErrorCallback(t *testing.T) { defer a.close() rn := "xdsclient-test-lds-resource" - w := testutils.NewTestResourceWatcher() + w := xdstestutils.NewTestResourceWatcher() cancelResource := a.watchResource(listenerResourceType, rn, w) defer cancelResource() @@ -183,11 +180,11 @@ func (s) TestWatchResourceTimerCanRestartOnIgnoredADSRecvError(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() // Create a restartable listener which can close existing connections. - l, err := util.LocalTCPListener() + l, err := testutils.LocalTCPListener() if err != nil { t.Fatalf("testutils.LocalTCPListener() failed: %v", err) } - lis := util.NewRestartableListener(l) + lis := testutils.NewRestartableListener(l) defer lis.Close() streamRestarted := grpcsync.NewEvent() serverOpt := e2e.ManagementServerOptions{ @@ -201,7 +198,7 @@ func (s) TestWatchResourceTimerCanRestartOnIgnoredADSRecvError(t *testing.T) { defer a.close() nameA := "xdsclient-test-lds-resourceA" - watcherA := testutils.NewTestResourceWatcher() + watcherA := xdstestutils.NewTestResourceWatcher() cancelA := a.watchResource(listenerResourceType, nameA, watcherA) if err := updateResourceInServer(ctx, ms, nameA, nodeID); err != nil { @@ -222,7 +219,7 @@ func (s) TestWatchResourceTimerCanRestartOnIgnoredADSRecvError(t *testing.T) { lis.Stop() nameB := "xdsclient-test-lds-resourceB" - watcherB := testutils.NewTestResourceWatcher() + watcherB := xdstestutils.NewTestResourceWatcher() cancelB := a.watchResource(listenerResourceType, nameB, watcherB) defer cancelB() diff --git a/xds/internal/xdsclient/client_new.go b/xds/internal/xdsclient/client_new.go index 2a4f65199ff7..6097e86925e6 100644 --- a/xds/internal/xdsclient/client_new.go +++ b/xds/internal/xdsclient/client_new.go @@ -36,8 +36,11 @@ import ( // value, and is defined in gRFC A71. const NameForServer = "#server" -// New returns a new xDS client configured by the bootstrap file specified in env -// variable GRPC_XDS_BOOTSTRAP or GRPC_XDS_BOOTSTRAP_CONFIG. +// New returns an xDS client configured with bootstrap configuration specified +// by the ordered list: +// - file name containing the configuration specified by GRPC_XDS_BOOTSTRAP +// - actual configuration specified by GRPC_XDS_BOOTSTRAP_CONFIG +// - fallback configuration set using bootstrap.SetFallbackBootstrapConfig // // gRPC client implementations are expected to pass the channel's target URI for // the name field, while server implementations are expected to pass a dedicated @@ -50,19 +53,7 @@ const NameForServer = "#server" // only when all references are released, and it is safe for the caller to // invoke this close function multiple times. func New(name string) (XDSClient, func(), error) { - return NewWithConfig(name, nil) -} - -// NewWithConfig is similar to New, except that it uses the provided bootstrap -// configuration to create the xDS client if and only if the bootstrap -// environment variables are not defined. -// -// # Internal Only -// -// This function should ONLY be used by the internal google-c2p resolver. -// DO NOT use this elsewhere. Use New() instead. -func NewWithConfig(name string, config *bootstrap.Config) (XDSClient, func(), error) { - return newRefCountedWithConfig(name, config, defaultWatchExpiryTimeout, defaultIdleAuthorityDeleteTimeout) + return newRefCounted(name, defaultWatchExpiryTimeout, defaultIdleAuthorityDeleteTimeout) } // newClientImpl returns a new xdsClient with the given config. @@ -121,11 +112,11 @@ func NewForTesting(opts OptionsForTesting) (XDSClient, func(), error) { opts.AuthorityIdleTimeout = defaultIdleAuthorityDeleteTimeout } - cfg, err := bootstrap.NewConfigFromContents(opts.Contents) - if err != nil { + if err := bootstrap.SetFallbackBootstrapConfig(opts.Contents); err != nil { return nil, nil, err } - return newRefCountedWithConfig(opts.Name, cfg, opts.WatchExpiryTimeout, opts.AuthorityIdleTimeout) + client, cancel, err := newRefCounted(opts.Name, opts.WatchExpiryTimeout, opts.AuthorityIdleTimeout) + return client, func() { bootstrap.UnsetFallbackBootstrapConfigForTesting(); cancel() }, err } // GetForTesting returns an xDS client created earlier using the given name. diff --git a/xds/internal/xdsclient/client_refcounted.go b/xds/internal/xdsclient/client_refcounted.go index 72b43faf6be1..fb05eff40b5e 100644 --- a/xds/internal/xdsclient/client_refcounted.go +++ b/xds/internal/xdsclient/client_refcounted.go @@ -23,7 +23,6 @@ import ( "sync/atomic" "time" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/grpcsync" "google.golang.org/grpc/internal/xds/bootstrap" ) @@ -58,13 +57,10 @@ func clientRefCountedClose(name string) { } -// newRefCountedWithConfig creates a new reference counted xDS client -// implementation for name, if one does not exist already. If an xDS client for -// the given name exists, it gets a reference to it and returns it. -// -// The passed in fallback config is used when bootstrap environment variables -// are not defined. -func newRefCountedWithConfig(name string, fallbackConfig *bootstrap.Config, watchExpiryTimeout, idleAuthorityTimeout time.Duration) (XDSClient, func(), error) { +// newRefCounted creates a new reference counted xDS client implementation for +// name, if one does not exist already. If an xDS client for the given name +// exists, it gets a reference to it and returns it. +func newRefCounted(name string, watchExpiryTimeout, idleAuthorityTimeout time.Duration) (XDSClient, func(), error) { clientsMu.Lock() defer clientsMu.Unlock() @@ -73,22 +69,11 @@ func newRefCountedWithConfig(name string, fallbackConfig *bootstrap.Config, watc return c, grpcsync.OnceFunc(func() { clientRefCountedClose(name) }), nil } - // Use fallbackConfig only if bootstrap env vars are unspecified. - var config *bootstrap.Config - if envconfig.XDSBootstrapFileName == "" && envconfig.XDSBootstrapFileContent == "" { - if fallbackConfig == nil { - return nil, nil, fmt.Errorf("xds: bootstrap env vars are unspecified and provided fallback config is nil") - } - config = fallbackConfig - } else { - var err error - config, err = bootstrap.NewConfig() - if err != nil { - return nil, nil, fmt.Errorf("xds: failed to read bootstrap file: %v", err) - } - } - // Create the new client implementation. + config, err := bootstrap.GetConfiguration() + if err != nil { + return nil, nil, fmt.Errorf("xds: failed to get xDS bootstrap config: %v", err) + } c, err := newClientImpl(config, watchExpiryTimeout, idleAuthorityTimeout) if err != nil { return nil, nil, err diff --git a/xds/internal/xdsclient/tests/authority_test.go b/xds/internal/xdsclient/tests/authority_test.go index c2264e58a373..875313e1246d 100644 --- a/xds/internal/xdsclient/tests/authority_test.go +++ b/xds/internal/xdsclient/tests/authority_test.go @@ -87,7 +87,7 @@ func setupForAuthorityTests(ctx context.Context, t *testing.T, idleTimeout time. "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, defaultAuthorityServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ testAuthority1: []byte(`{}`), testAuthority2: []byte(`{}`), diff --git a/xds/internal/xdsclient/tests/cds_watchers_test.go b/xds/internal/xdsclient/tests/cds_watchers_test.go index 4537c33931ac..6ee227f85997 100644 --- a/xds/internal/xdsclient/tests/cds_watchers_test.go +++ b/xds/internal/xdsclient/tests/cds_watchers_test.go @@ -184,7 +184,7 @@ func (s) TestCDSWatch(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -334,7 +334,7 @@ func (s) TestCDSWatch_TwoWatchesForSameResourceName(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -438,7 +438,7 @@ func (s) TestCDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -731,7 +731,7 @@ func (s) TestCDSWatch_ResourceRemoved(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -912,7 +912,7 @@ func (s) TestCDSWatch_PartialValid(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -1004,7 +1004,7 @@ func (s) TestCDSWatch_PartialResponse(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config diff --git a/xds/internal/xdsclient/tests/dump_test.go b/xds/internal/xdsclient/tests/dump_test.go index d046f9c7ca0f..f21069371416 100644 --- a/xds/internal/xdsclient/tests/dump_test.go +++ b/xds/internal/xdsclient/tests/dump_test.go @@ -393,7 +393,7 @@ func (s) TestDumpResources_ManyToMany(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer1.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ authority: []byte(fmt.Sprintf(`{ "xds_servers": [{ diff --git a/xds/internal/xdsclient/tests/eds_watchers_test.go b/xds/internal/xdsclient/tests/eds_watchers_test.go index a4e03007ccb6..7f2d5571efd3 100644 --- a/xds/internal/xdsclient/tests/eds_watchers_test.go +++ b/xds/internal/xdsclient/tests/eds_watchers_test.go @@ -215,7 +215,7 @@ func (s) TestEDSWatch(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -405,7 +405,7 @@ func (s) TestEDSWatch_TwoWatchesForSameResourceName(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -510,7 +510,7 @@ func (s) TestEDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -875,7 +875,7 @@ func (s) TestEDSWatch_PartialValid(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config diff --git a/xds/internal/xdsclient/tests/federation_watchers_test.go b/xds/internal/xdsclient/tests/federation_watchers_test.go index 92135b62d88f..15d72c912f14 100644 --- a/xds/internal/xdsclient/tests/federation_watchers_test.go +++ b/xds/internal/xdsclient/tests/federation_watchers_test.go @@ -56,7 +56,7 @@ func setupForFederationWatchersTest(t *testing.T) (*e2e.ManagementServer, string "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, serverDefaultAuthority.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ testNonDefaultAuthority: []byte(fmt.Sprintf(`{ "xds_servers": [{ diff --git a/xds/internal/xdsclient/tests/lds_watchers_test.go b/xds/internal/xdsclient/tests/lds_watchers_test.go index 1a6b101fc976..23f8f0c8d544 100644 --- a/xds/internal/xdsclient/tests/lds_watchers_test.go +++ b/xds/internal/xdsclient/tests/lds_watchers_test.go @@ -204,7 +204,7 @@ func (s) TestLDSWatch(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -354,7 +354,7 @@ func (s) TestLDSWatch_TwoWatchesForSameResourceName(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -459,7 +459,7 @@ func (s) TestLDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -749,7 +749,7 @@ func (s) TestLDSWatch_ResourceRemoved(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -927,7 +927,7 @@ func (s) TestLDSWatch_PartialValid(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -1020,7 +1020,7 @@ func (s) TestLDSWatch_PartialResponse(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config diff --git a/xds/internal/xdsclient/tests/misc_watchers_test.go b/xds/internal/xdsclient/tests/misc_watchers_test.go index 48f6e04f0720..f51648907182 100644 --- a/xds/internal/xdsclient/tests/misc_watchers_test.go +++ b/xds/internal/xdsclient/tests/misc_watchers_test.go @@ -108,7 +108,7 @@ func (s) TestWatchCallAnotherWatch(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config diff --git a/xds/internal/xdsclient/tests/rds_watchers_test.go b/xds/internal/xdsclient/tests/rds_watchers_test.go index 28a6b20b7168..524bfbe05210 100644 --- a/xds/internal/xdsclient/tests/rds_watchers_test.go +++ b/xds/internal/xdsclient/tests/rds_watchers_test.go @@ -217,7 +217,7 @@ func (s) TestRDSWatch(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -407,7 +407,7 @@ func (s) TestRDSWatch_TwoWatchesForSameResourceName(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp resource names used in this test do not specify an // authority. These will end up looking up an entry with the @@ -512,7 +512,7 @@ func (s) TestRDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config @@ -876,7 +876,7 @@ func (s) TestRDSWatch_PartialValid(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), Authorities: map[string]json.RawMessage{ // Xdstp style resource names used in this test use a slash removed // version of t.Name as their authority, and the empty config diff --git a/xds/server_test.go b/xds/server_test.go index 9d8db8c76d6f..ad092c4bd314 100644 --- a/xds/server_test.go +++ b/xds/server_test.go @@ -178,7 +178,7 @@ func (s) TestNewServer_Failure(t *testing.T) { { desc: "bootstrap env var not set", serverOpts: []grpc.ServerOption{grpc.Creds(xdsCreds)}, - wantErr: "bootstrap env vars are unspecified", + wantErr: "failed to get xDS bootstrap config", }, { desc: "empty bootstrap config", @@ -198,7 +198,7 @@ func (s) TestNewServer_Failure(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, nonExistentManagementServer))}, - NodeID: uuid.New().String(), + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, uuid.New().String())), CertificateProviders: map[string]json.RawMessage{ "cert-provider-instance": json.RawMessage("{}"), }, @@ -500,7 +500,7 @@ func (s) TestHandleListenerUpdate_NoXDSCreds(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), CertificateProviders: map[string]json.RawMessage{ e2e.ServerSideCertProviderInstance: fakeProvider1Config, e2e.ClientSideCertProviderInstance: fakeProvider2Config, @@ -592,7 +592,7 @@ func (s) TestHandleListenerUpdate_ErrorUpdate(t *testing.T) { "server_uri": %q, "channel_creds": [{"type": "insecure"}] }`, mgmtServer.Address))}, - NodeID: nodeID, + Node: []byte(fmt.Sprintf(`{"id": "%s"}`, nodeID)), CertificateProviders: map[string]json.RawMessage{ e2e.ServerSideCertProviderInstance: fakeProvider1Config, e2e.ClientSideCertProviderInstance: fakeProvider2Config,