feat!: Apply env overrides only when config loaded from file (#483)

* feat!: Apply env overrides only when configuration loaded from file

BREAKING CHANGE: Overrides are no longer applied to values pulled from Configuration Provider. Configuration Provider is now the system of record for configuration, when used.

closes #482

Signed-off-by: Leonard Goodell <[email protected]>

bootstrap/config/config.go

@@ -140,6 +140,8 @@ func (cp *Processor) Process(
 			return err
 		}
 
+		cp.lc.Info("Common configuration loaded from the Configuration Provider. No overrides applied")
+
 		privateConfigClient, err = CreateProviderClient(cp.lc, serviceKey, configStem, getAccessToken, configProviderInfo.ServiceConfig())
 		if err != nil {
 			return fmt.Errorf("failed to create Configuration Provider client: %s", err.Error())
@@ -157,6 +159,7 @@ func (cp *Processor) Process(
 		if err != nil {
 			return fmt.Errorf("failed check for Configuration Provider has private configiuration: %s", err.Error())
 		}
+
 		if cp.providerHasConfig && !cp.overwriteConfig {
 			privateServiceConfig, err = copyConfigurationStruct(serviceConfig)
 			if err != nil {
@@ -168,6 +171,8 @@ func (cp *Processor) Process(
 			if err := mergeConfigs(serviceConfig, privateServiceConfig); err != nil {
 				return fmt.Errorf("could not merge common and private configurations: %s", err.Error())
 			}
+
+			cp.lc.Info("Private configuration loaded from the Configuration Provider. No overrides applied")
 		}
 	}
 
@@ -178,26 +183,26 @@ func (cp *Processor) Process(
 		if err != nil {
 			return err
 		}
-		cp.lc.Info("Using local private configuration from file")
+
+		// apply overrides - Now only done when loaded from file and values will get pushed into Configuration Provider (if used)
+		overrideCount, err := cp.envVars.OverrideTomlValues(tomlTree)
+		if err != nil {
+			return err
+		}
+		cp.lc.Infof("Configuration loaded from file with %d overrides applied", overrideCount)
+
 		if err := cp.mergeTomlWithConfig(serviceConfig, tomlTree); err != nil {
 			return err
 		}
+
 		if useProvider {
 			if err := privateConfigClient.PutConfigurationToml(tomlTree, cp.overwriteConfig); err != nil {
 				return fmt.Errorf("could not push configuration into Configuration Provider: %s", err.Error())
 			}
 
-			cp.lc.Info("Configuration has been pushed to into Configuration Provider")
+			cp.lc.Info("Configuration has been pushed to into Configuration Provider with overrides applied")
 		}
-
-	}
-
-	// apply overrides
-	overrideCount, err := cp.envVars.OverrideConfiguration(serviceConfig)
-	if err != nil {
-		return err
 	}
-	cp.lc.Infof("Configuration loaded with %d overrides applied", overrideCount)
 
 	// listen for changes on Writable
 	if useProvider {
@@ -319,12 +324,12 @@ func (cp *Processor) getAccessTokenCallback(serviceKey string, secretProvider in
 					err.Error())
 			}
 
-			cp.lc.Infof("Using Configuration Provider access token of length %d", len(accessToken))
+			cp.lc.Debugf("Using Configuration Provider access token of length %d", len(accessToken))
 			return accessToken, nil
 		}
 
 	} else {
-		cp.lc.Info("Not configured to use Config Provider access token")
+		cp.lc.Debug("Not configured to use Config Provider access token")
 	}
 	return getAccessToken, err
 }
@@ -415,7 +420,7 @@ func (cp *Processor) LoadCustomConfigSection(config interfaces.UpdatableConfig,
 }
 
 // ListenForCustomConfigChanges listens for changes to the specified custom configuration section. When changes occur it
-// applies the changes to the custom configuration section and signals the the changes have occurred.
+// applies the changes to the custom configuration section and signals the changes have occurred.
 func (cp *Processor) ListenForCustomConfigChanges(
 	configToWatch interface{},
 	sectionName string,
@@ -920,11 +925,11 @@ func mergeMaps(dest map[string]any, src map[string]any) {
 
 // copyConfigurationStruct returns a copy of the passed in configuration interface
 func copyConfigurationStruct(config interfaces.Configuration) (interfaces.Configuration, error) {
-	copy, err := copystructure.Copy(config)
+	rawCopy, err := copystructure.Copy(config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to load copy the configuration: %s", err.Error())
 	}
-	configCopy, ok := copy.(interfaces.Configuration)
+	configCopy, ok := rawCopy.(interfaces.Configuration)
 	if !ok {
 		return nil, errors.New("failed to cast the copy of the configuration")
 	}

bootstrap/environment/variables.go

@@ -46,9 +46,10 @@ const (
 
 	noConfigProviderValue = "none"
 
-	tomlPathSeparator = "."
-	tomlNameSeparator = "-"
-	envNameSeparator  = "_"
+	tomlPathDotSeparator   = "."
+	tomlPathSlashSeparator = "/"
+	tomlNameSeparator      = "-"
+	envNameSeparator       = "_"
 
 	// insecureSecretsRegexStr is a regex to look for toml keys that are under the Secrets sub-key of values within the
 	// Writable.InsecureSecrets topology.
@@ -64,7 +65,7 @@ var (
 	insecureSecretsRegex = regexp.MustCompile(insecureSecretsRegexStr)
 )
 
-// Variables is a receiver that holds Variables variables and encapsulates toml.Tree-based configuration field
+// Variables is a receiver that holds Variables and encapsulates toml.Tree-based configuration field
 // overrides.  Assumes "_" embedded in Variables variable key separates sub-structs; e.g. foo_bar_baz might refer to
 //
 //			type foo struct {
@@ -114,7 +115,6 @@ func (e *Variables) UseRegistry() (bool, bool) {
 // OverrideConfiguration method replaces values in the configuration for matching Variables variable keys.
 // serviceConfig must be pointer to the service configuration.
 func (e *Variables) OverrideConfiguration(serviceConfig interface{}) (int, error) {
-	var overrideCount = 0
 
 	contents, err := toml.Marshal(reflect.ValueOf(serviceConfig).Elem().Interface())
 	if err != nil {
@@ -126,6 +126,23 @@ func (e *Variables) OverrideConfiguration(serviceConfig interface{}) (int, error
 		return 0, err
 	}
 
+	overrideCount, err := e.OverrideTomlValues(configTree)
+	if err != nil {
+		return 0, err
+	}
+
+	// Put the configuration back into the services configuration struct with the overridden values
+	err = configTree.Unmarshal(serviceConfig)
+	if err != nil {
+		return 0, fmt.Errorf("could not marshal toml configTree to configuration: %s", err.Error())
+	}
+
+	return overrideCount, nil
+}
+
+func (e *Variables) OverrideTomlValues(configTree *toml.Tree) (int, error) {
+	var overrideCount int
+
 	// 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())
@@ -151,12 +168,6 @@ func (e *Variables) OverrideConfiguration(serviceConfig interface{}) (int, error
 		logEnvironmentOverride(e.lc, path, envVar, envValue)
 	}
 
-	// Put the configuration back into the services configuration struct with the overridden values
-	err = configTree.Unmarshal(serviceConfig)
-	if err != nil {
-		return 0, fmt.Errorf("could not marshal toml configTree to configuration: %s", err.Error())
-	}
-
 	return overrideCount, nil
 }
 
@@ -191,8 +202,9 @@ func (e *Variables) buildOverrideNames(paths []string) map[string]string {
 }
 
 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)
+	// ".", "/" & "-" are the only special character allowed in path not allowed in environment variable Name
+	override := strings.ReplaceAll(path, tomlPathDotSeparator, envNameSeparator)
+	override = strings.ReplaceAll(override, tomlPathSlashSeparator, envNameSeparator)
 	override = strings.ReplaceAll(override, tomlNameSeparator, envNameSeparator)
 	override = strings.ToUpper(override)
 	return override
@@ -280,7 +292,7 @@ type StartupInfo struct {
 // GetStartupInfo gets the Service StartupInfo values from an Variables variable value (if it exists)
 // or uses the default values.
 func GetStartupInfo(serviceKey string) StartupInfo {
-	// lc hasn't be created at the time this info is needed so have to create local client.
+	// lc hasn't been created at the time this info is needed so have to create local client.
 	lc := logger.NewClient(serviceKey, models.InfoLog)
 
 	startup := StartupInfo{