Skip to content

Commit

Permalink
fix: Replace hyphen with underscore in override names
Browse files Browse the repository at this point in the history
Hyphen is not a standard character for environment variable names

closes #212

BREAKING CHANGE: Overrides that have hyphens will not longer work and must be updated replace hyphens with underscores.

Signed-off-by: lenny <[email protected]>
  • Loading branch information
lenny committed Apr 20, 2021
1 parent eb96085 commit 49d5a6f
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 26 deletions.
58 changes: 36 additions & 22 deletions bootstrap/environment/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ const (
envConfDir = "EDGEX_CONF_DIR"
envProfile = "EDGEX_PROFILE"
envFile = "EDGEX_CONFIG_FILE"

tomlPathSeparator = "."
tomlNameSeparator = "-"
envNameSeparator = "_"
)

// Variables is receiver that holds Variables variables and encapsulates toml.Tree-based configuration field
Expand All @@ -57,6 +61,11 @@ type Variables struct {
lc logger.LoggingClient
}

type overrideSpec struct {
tomlPath string
overrideName string
}

// NewEnvironment constructor reads/stores os.Environ() for use by Variables receiver methods.
func NewVariables(lc logger.LoggingClient) *Variables {
osEnv := os.Environ()
Expand Down Expand Up @@ -109,28 +118,26 @@ func (e *Variables) OverrideConfiguration(serviceConfig interface{}) (int, error
// The toml.Tree API keys() only return to top level keys, rather that paths.
// It is also missing a GetPaths so have to spin our own
paths := e.buildPaths(configTree.ToMap())
// Now that we have all the paths in the config tree, we need to create a map that has the uppercase versions as
// the map keys and the original versions as the map values so we can match against uppercase names but use the
// originals to set values.
pathMap := e.buildUppercasePathMap(paths)
// Now that we have all the paths in the config tree, we need to create a override specs that have
// the matching override names for each path.
overrideSpecs := e.buildOverrideSpecs(paths)

for envVar, envValue := range e.variables {
envKey := strings.Replace(envVar, "_", ".", -1)
key, found := e.getKeyForMatchedPath(pathMap, envKey)
path, found := e.getPathForMatchedOverride(overrideSpecs, envVar)
if !found {
continue
}

oldValue := configTree.Get(key)
oldValue := configTree.Get(path)

newValue, err := e.convertToType(oldValue, envValue)
if err != nil {
return 0, fmt.Errorf("environment value override failed for %s=%s: %s", envVar, envValue, err.Error())
}

configTree.Set(key, newValue)
configTree.Set(path, newValue)
overrideCount++
logEnvironmentOverride(e.lc, key, envVar, envValue)
logEnvironmentOverride(e.lc, path, envVar, envValue)
}

// Put the configuration back into the services configuration struct with the overridden values
Expand Down Expand Up @@ -163,24 +170,31 @@ func (e *Variables) buildPaths(keyMap map[string]interface{}) []string {
return paths
}

// buildUppercasePathMap builds a map where the key is the uppercase version of the path
// and the value is original version of the path
func (e *Variables) buildUppercasePathMap(paths []string) map[string]string {
ucMap := make(map[string]string)
func (e *Variables) buildOverrideSpecs(paths []string) []overrideSpec {
var specs []overrideSpec
for _, path := range paths {
ucMap[strings.ToUpper(path)] = path
spec := overrideSpec{
tomlPath: path,
overrideName: e.getOverrideNameFor(path),
}
specs = append(specs, spec)
}

return ucMap
return specs
}

func (_ *Variables) getOverrideNameFor(path string) string {
// "." & "-" are the only special character allowed in TOML path not allowed in environment variable Name
override := strings.ReplaceAll(path, tomlPathSeparator, envNameSeparator)
override = strings.ReplaceAll(override, tomlNameSeparator, envNameSeparator)
override = strings.ToUpper(override)
return override
}

// getKeyForMatchedPath searches for match of the environment variable name with the uppercase path (pathMap keys)
// If matched found to original path (pathMap values) is returned as the "key"
// For backward compatibility a case insensitive comparison is currently used.
func (e *Variables) getKeyForMatchedPath(pathMap map[string]string, envVarName string) (string, bool) {
for ucKey, lcKey := range pathMap {
if ucKey == envVarName {
return lcKey, true
func (e *Variables) getPathForMatchedOverride(specs []overrideSpec, envVarName string) (string, bool) {
for _, spec := range specs {
if spec.overrideName == envVarName {
return spec.tomlPath, true
}
}

Expand Down
21 changes: 17 additions & 4 deletions bootstrap/environment/variables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,18 +324,22 @@ func TestOverrideConfigurationExactCase(t *testing.T) {
func TestOverrideConfigurationUppercase(t *testing.T) {
_, lc := initializeTest()

expectedOverrideCount := 4
expectedHost := "edgex-core-consul"
expectedOverrideCount := 5
expectedRegistryHost := "edgex-core-consul"
expectedCoreDataHost := "edgex-core-data"
expectedList := []string{"joe", "mary", "bob"}
expectedFloatVal := float32(24.234)
expectedAuthType := "secure"
expectedAuthToken := "token"

coreDataClientKey := "edgex-core-data"

serviceConfig := struct {
Registry config.RegistryInfo
List []string
FloatVal float32
SecretStore config.SecretStoreInfo
Clients map[string]config.ClientInfo
}{
Registry: config.RegistryInfo{
Host: "localhost",
Expand All @@ -350,9 +354,17 @@ func TestOverrideConfigurationUppercase(t *testing.T) {
AuthToken: expectedAuthToken,
},
},
Clients: map[string]config.ClientInfo{
coreDataClientKey: {
Host: "localhost",
Port: 49080,
Protocol: "http",
},
},
}

_ = os.Setenv("REGISTRY_HOST", expectedHost)
_ = os.Setenv("REGISTRY_HOST", expectedRegistryHost)
_ = os.Setenv("CLIENTS_EDGEX_CORE_DATA_HOST", expectedCoreDataHost)
_ = os.Setenv("LIST", " joe,mary , bob ")
strVal := fmt.Sprintf("%v", expectedFloatVal)
_ = os.Setenv("FLOATVAL", strVal)
Expand All @@ -365,7 +377,8 @@ func TestOverrideConfigurationUppercase(t *testing.T) {

require.NoError(t, err)
assert.Equal(t, expectedOverrideCount, actualCount)
assert.Equal(t, expectedHost, serviceConfig.Registry.Host)
assert.Equal(t, expectedRegistryHost, serviceConfig.Registry.Host)
assert.Equal(t, expectedCoreDataHost, serviceConfig.Clients[coreDataClientKey].Host)
assert.Equal(t, expectedList, serviceConfig.List)
assert.Equal(t, expectedFloatVal, serviceConfig.FloatVal)
assert.Equal(t, expectedAuthType, serviceConfig.SecretStore.Authentication.AuthType)
Expand Down

0 comments on commit 49d5a6f

Please sign in to comment.