xds: make fallback bootstrap configuration per-process (#7401)

Author: easwars

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,
 	})

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
+)

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,27 +462,27 @@ 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)
 	}
 
 	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)
 		})
 	}
 }