diff --git a/xds/internal/client/bootstrap/bootstrap.go b/xds/internal/client/bootstrap/bootstrap.go index e833a4c91f4f..385ba87cd7df 100644 --- a/xds/internal/client/bootstrap/bootstrap.go +++ b/xds/internal/client/bootstrap/bootstrap.go @@ -96,6 +96,29 @@ type xdsServer struct { ServerFeatures []string `json:"server_features"` } +func bootstrapConfigFromEnvVariable() ([]byte, error) { + fName := env.BootstrapFileName + fContent := env.BootstrapFileContent + + // Bootstrap file name has higher priority than bootstrap content. + if fName != "" { + // If file name is set + // - If file not found (or other errors), fail + // - Otherwise, use the content. + // + // Note that even if the content is invalid, we don't failover to the + // file content env variable. + logger.Debugf("xds: using bootstrap file with name %q", fName) + return bootstrapFileReadFunc(fName) + } + + if fContent != "" { + return []byte(fContent), nil + } + + return nil, fmt.Errorf("none of the bootstrap environment variables (%q or %q) defined", env.BootstrapFileNameEnv, env.BootstrapFileContentEnv) +} + // NewConfig returns a new instance of Config initialized by reading the // bootstrap file found at ${GRPC_XDS_BOOTSTRAP}. // @@ -136,21 +159,15 @@ type xdsServer struct { func NewConfig() (*Config, error) { config := &Config{} - fName := env.BootstrapFileName - if fName == "" { - return nil, fmt.Errorf("xds: Environment variable %q not defined", "GRPC_XDS_BOOTSTRAP") - } - logger.Infof("Got bootstrap file location %q", fName) - - data, err := bootstrapFileReadFunc(fName) + data, err := bootstrapConfigFromEnvVariable() if err != nil { - return nil, fmt.Errorf("xds: Failed to read bootstrap file %s with error %v", fName, err) + return nil, fmt.Errorf("xds: Failed to read bootstrap config: %v", err) } logger.Debugf("Bootstrap content: %s", data) var jsonData map[string]json.RawMessage if err := json.Unmarshal(data, &jsonData); err != nil { - return nil, fmt.Errorf("xds: Failed to parse file %s (content %v) with error: %v", fName, string(data), err) + return nil, fmt.Errorf("xds: Failed to parse bootstrap config: %v", err) } serverSupportsV3 := false diff --git a/xds/internal/client/bootstrap/bootstrap_test.go b/xds/internal/client/bootstrap/bootstrap_test.go index 9d5aec26df71..3368af1f6a58 100644 --- a/xds/internal/client/bootstrap/bootstrap_test.go +++ b/xds/internal/client/bootstrap/bootstrap_test.go @@ -264,13 +264,17 @@ func (c *Config) compare(want *Config) error { return nil } +func fileReadFromFileMap(bootstrapFileMap map[string]string, name string) ([]byte, error) { + if b, ok := bootstrapFileMap[name]; ok { + return []byte(b), nil + } + return nil, os.ErrNotExist +} + func setupBootstrapOverride(bootstrapFileMap map[string]string) func() { oldFileReadFunc := bootstrapFileReadFunc - bootstrapFileReadFunc = func(name string) ([]byte, error) { - if b, ok := bootstrapFileMap[name]; ok { - return []byte(b), nil - } - return nil, os.ErrNotExist + bootstrapFileReadFunc = func(filename string) ([]byte, error) { + return fileReadFromFileMap(bootstrapFileMap, filename) } return func() { bootstrapFileReadFunc = oldFileReadFunc } } @@ -278,6 +282,49 @@ func setupBootstrapOverride(bootstrapFileMap map[string]string) func() { // 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) { + origBootstrapFileName := env.BootstrapFileName + env.BootstrapFileName = fileName + defer func() { env.BootstrapFileName = origBootstrapFileName }() + + c, err := NewConfig() + if (err != nil) != wantError { + t.Fatalf("NewConfig() returned error %v, wantError: %v", err, wantError) + } + if wantError { + return + } + if err := c.compare(wantConfig); err != nil { + t.Fatal(err) + } +} + +// 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) { + b, err := bootstrapFileReadFunc(fileName) + if err != nil { + // If file reading failed, skip this test. + return + } + origBootstrapContent := env.BootstrapFileContent + env.BootstrapFileContent = string(b) + defer func() { env.BootstrapFileContent = origBootstrapContent }() + + c, err := NewConfig() + if (err != nil) != wantError { + t.Fatalf("NewConfig() returned error %v, wantError: %v", err, wantError) + } + if wantError { + return + } + if err := c.compare(wantConfig); err != nil { + t.Fatal(err) + } +} + // TestNewConfigV2ProtoFailure exercises the functionality in NewConfig with // different bootstrap file contents which are expected to fail. func TestNewConfigV2ProtoFailure(t *testing.T) { @@ -338,13 +385,8 @@ func TestNewConfigV2ProtoFailure(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - origBootstrapFileName := env.BootstrapFileName - env.BootstrapFileName = test.name - defer func() { env.BootstrapFileName = origBootstrapFileName }() - - if _, err := NewConfig(); err == nil { - t.Fatalf("NewConfig() returned nil error, expected to fail") - } + testNewConfigWithFileNameEnv(t, test.name, true, nil) + testNewConfigWithFileContentEnv(t, test.name, true, nil) }) } } @@ -382,17 +424,8 @@ func TestNewConfigV2ProtoSuccess(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - origBootstrapFileName := env.BootstrapFileName - env.BootstrapFileName = test.name - defer func() { env.BootstrapFileName = origBootstrapFileName }() - - c, err := NewConfig() - if err != nil { - t.Fatalf("NewConfig() failed: %v", err) - } - if err := c.compare(test.wantConfig); err != nil { - t.Fatal(err) - } + testNewConfigWithFileNameEnv(t, test.name, false, test.wantConfig) + testNewConfigWithFileContentEnv(t, test.name, false, test.wantConfig) }) } } @@ -419,17 +452,8 @@ func TestNewConfigV3SupportNotEnabledOnClient(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - origBootstrapFileName := env.BootstrapFileName - env.BootstrapFileName = test.name - defer func() { env.BootstrapFileName = origBootstrapFileName }() - - c, err := NewConfig() - if err != nil { - t.Fatalf("NewConfig() failed: %v", err) - } - if err := c.compare(test.wantConfig); err != nil { - t.Fatal(err) - } + testNewConfigWithFileNameEnv(t, test.name, false, test.wantConfig) + testNewConfigWithFileContentEnv(t, test.name, false, test.wantConfig) }) } } @@ -456,31 +480,66 @@ func TestNewConfigV3SupportEnabledOnClient(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - origBootstrapFileName := env.BootstrapFileName - env.BootstrapFileName = test.name - defer func() { env.BootstrapFileName = origBootstrapFileName }() - - c, err := NewConfig() - if err != nil { - t.Fatalf("NewConfig() failed: %v", err) - } - if err := c.compare(test.wantConfig); err != nil { - t.Fatal(err) - } + testNewConfigWithFileNameEnv(t, test.name, false, test.wantConfig) + testNewConfigWithFileContentEnv(t, test.name, false, test.wantConfig) }) } } -// TestNewConfigBootstrapFileEnvNotSet tests the case where the bootstrap file +// TestNewConfigBootstrapEnvPriority tests that the two env variables are read in correct priority +// +// the case where the bootstrap file // environment variable is not set. -func TestNewConfigBootstrapFileEnvNotSet(t *testing.T) { +func TestNewConfigBootstrapEnvPriority(t *testing.T) { + origV3Support := env.V3Support + env.V3Support = true + defer func() { env.V3Support = origV3Support }() + + oldFileReadFunc := bootstrapFileReadFunc + bootstrapFileReadFunc = func(filename string) ([]byte, error) { + return fileReadFromFileMap(v2BootstrapFileMap, filename) + } + defer func() { bootstrapFileReadFunc = oldFileReadFunc }() + + goodFileName1 := "goodBootstrap" + goodConfig1 := nonNilCredsConfigV2 + + goodFileName2 := "serverSupportsV3" + goodFileContent2 := v3BootstrapFileMap[goodFileName2] + goodConfig2 := nonNilCredsConfigV3 + origBootstrapFileName := env.BootstrapFileName env.BootstrapFileName = "" defer func() { env.BootstrapFileName = origBootstrapFileName }() + origBootstrapContent := env.BootstrapFileContent + env.BootstrapFileContent = "" + defer func() { env.BootstrapFileContent = 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 one of them is set, it should be used. + env.BootstrapFileName = goodFileName1 + env.BootstrapFileContent = "" + if c, err := NewConfig(); err != nil || c.compare(goodConfig1) != nil { + t.Errorf("NewConfig() = %v, %v, want: %v, %v", c, err, goodConfig1, nil) + } + + env.BootstrapFileName = "" + env.BootstrapFileContent = goodFileContent2 + if c, err := NewConfig(); err != nil || c.compare(goodConfig2) != nil { + t.Errorf("NewConfig() = %v, %v, want: %v, %v", c, err, goodConfig1, nil) + } + + // Set both, file name should be read. + env.BootstrapFileName = goodFileName1 + env.BootstrapFileContent = goodFileContent2 + if c, err := NewConfig(); err != nil || c.compare(goodConfig1) != nil { + t.Errorf("NewConfig() = %v, %v, want: %v, %v", c, err, goodConfig1, nil) + } } func init() { @@ -686,20 +745,8 @@ func TestNewConfigWithCertificateProviders(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - origBootstrapFileName := env.BootstrapFileName - env.BootstrapFileName = test.name - defer func() { env.BootstrapFileName = origBootstrapFileName }() - - c, err := NewConfig() - if (err != nil) != test.wantErr { - t.Fatalf("NewConfig() returned: %v, wantErr: %v", err, test.wantErr) - } - if test.wantErr { - return - } - if err := c.compare(test.wantConfig); err != nil { - t.Fatal(err) - } + testNewConfigWithFileNameEnv(t, test.name, test.wantErr, test.wantConfig) + testNewConfigWithFileContentEnv(t, test.name, test.wantErr, test.wantConfig) }) } } @@ -764,20 +811,8 @@ func TestNewConfigWithServerResourceNameID(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - origBootstrapFileName := env.BootstrapFileName - env.BootstrapFileName = test.name - defer func() { env.BootstrapFileName = origBootstrapFileName }() - - c, err := NewConfig() - if (err != nil) != test.wantErr { - t.Fatalf("NewConfig() returned (%+v, %v), wantErr: %v", c, err, test.wantErr) - } - if test.wantErr { - return - } - if err := c.compare(test.wantConfig); err != nil { - t.Fatal(err) - } + testNewConfigWithFileNameEnv(t, test.name, test.wantErr, test.wantConfig) + testNewConfigWithFileContentEnv(t, test.name, test.wantErr, test.wantConfig) }) } } diff --git a/xds/internal/env/env.go b/xds/internal/env/env.go index c4b46bae171b..38b62808fa39 100644 --- a/xds/internal/env/env.go +++ b/xds/internal/env/env.go @@ -26,7 +26,18 @@ import ( ) const ( - bootstrapFileNameEnv = "GRPC_XDS_BOOTSTRAP" + // BootstrapFileNameEnv is the env variable to set bootstrap file name. + // Do not use this and read from env directly. Its value is read and kept in + // variable BootstrapFileName. + // + // When both bootstrap FileName and FileContent are set, FileName is used. + BootstrapFileNameEnv = "GRPC_XDS_BOOTSTRAP" + // BootstrapFileContentEnv is the env variable to set bootstrapp file + // content. Do not use this and read from env directly. Its value is read + // and kept in variable BootstrapFileName. + // + // When both bootstrap FileName and FileContent are set, FileName is used. + BootstrapFileContentEnv = "GRPC_XDS_BOOTSTRAP_CONFIG" xdsV3SupportEnv = "GRPC_XDS_EXPERIMENTAL_V3_SUPPORT" circuitBreakingSupportEnv = "GRPC_XDS_EXPERIMENTAL_CIRCUIT_BREAKING" timeoutSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_TIMEOUT" @@ -36,7 +47,15 @@ var ( // BootstrapFileName holds the name of the file which contains xDS bootstrap // configuration. Users can specify the location of the bootstrap file by // setting the environment variable "GRPC_XDS_BOOSTRAP". - BootstrapFileName = os.Getenv(bootstrapFileNameEnv) + // + // When both bootstrap FileName and FileContent are set, FileName is used. + BootstrapFileName = os.Getenv(BootstrapFileNameEnv) + // BootstrapFileContent holds the content of the xDS bootstrap + // configuration. Users can specify the bootstrap config by + // setting the environment variable "GRPC_XDS_BOOSTRAP_CONFIG". + // + // When both bootstrap FileName and FileContent are set, FileName is used. + BootstrapFileContent = os.Getenv(BootstrapFileContentEnv) // V3Support indicates whether xDS v3 API support is enabled, which can be // done by setting the environment variable // "GRPC_XDS_EXPERIMENTAL_V3_SUPPORT" to "true".