Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds bootstrap: support config content in env variable #4153

Merged
merged 6 commits into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions xds/internal/client/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
menghanl marked this conversation as resolved.
Show resolved Hide resolved
}

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}.
//
Expand Down Expand Up @@ -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
Expand Down
185 changes: 110 additions & 75 deletions xds/internal/client/bootstrap/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,20 +264,67 @@ 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 }
}

// 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) {
Expand Down Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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)
})
}
}
Expand All @@ -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)
})
}
}
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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)
})
}
}
23 changes: 21 additions & 2 deletions xds/internal/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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".
Expand Down