MM-11697: Environment overrides do not overwrite config.json on save (#10413)

* MM-11697: Environment overrides do not overwrite config.json on save
#10388

The config store now keeps a copy of the config as loaded from the store
without environment overrides. Whenever persisting, we now check if the
current setting is different from the loaded setting. If it is, then use
the loaded setting instead.

As described in the comments to `removeEnvOverrides` in `common.go`,
this behavior will have to change if we ever let the user change a
setting that has been environmentally overriden.

This was interesting because the `load` function in `common.go` also
persists, so we have to tee the provided `io.ReadCloser` and construct a
config that doesn't have the environment overrides. And then we have to
find the path to the (maybe) changed variable in the config struct
using reflection.

Possible WIP: I had to expose a `GetWithoutEnvOverrides` function in the
Store interface just for the tests -- this is because the `file_test`
and `database_test`s are in the config_test package instead of the
`config` package.

* added function documentation

* fixed a small problem with tests

* MM-11697: big cleanup based on Jesse's PR comments

* MM-11697: edits per PR feedback

* MM-11697: licence header

* MM-11697: now testing that on disk config is not changed by env
overrides

* MM-11697: remove unneeded exports

config/common.go

@@ -4,6 +4,7 @@
 package config
 
 import (
+	"bytes"
 	"io"
 	"sync"
 
@@ -15,9 +16,10 @@ import (
 type commonStore struct {
 	emitter
 
-	configLock           sync.RWMutex
-	config               *model.Config
-	environmentOverrides map[string]interface{}
+	configLock             sync.RWMutex
+	config                 *model.Config
+	configWithoutOverrides *model.Config
+	environmentOverrides   map[string]interface{}
 }
 
 // Get fetches the current, cached configuration.
@@ -67,7 +69,7 @@ func (cs *commonStore) set(newCfg *model.Config, validate func(*model.Config) er
 		}
 	}
 
-	if err := persist(newCfg); err != nil {
+	if err := persist(cs.removeEnvOverrides(newCfg)); err != nil {
 		return nil, errors.Wrap(err, "failed to persist")
 	}
 
@@ -86,10 +88,20 @@ func (cs *commonStore) set(newCfg *model.Config, validate func(*model.Config) er
 //
 // This function assumes no lock has been acquired, as it acquires a write lock itself.
 func (cs *commonStore) load(f io.ReadCloser, needsSave bool, validate func(*model.Config) error, persist func(*model.Config) error) error {
+	// Duplicate f so that we can read a configuration without applying environment overrides
+	f2 := new(bytes.Buffer)
+	tee := io.TeeReader(f, f2)
+
 	allowEnvironmentOverrides := true
-	loadedCfg, environmentOverrides, err := unmarshalConfig(f, allowEnvironmentOverrides)
+	loadedCfg, environmentOverrides, err := unmarshalConfig(tee, allowEnvironmentOverrides)
 	if err != nil {
-		return errors.Wrapf(err, "failed to unmarshal config")
+		return errors.Wrapf(err, "failed to unmarshal config with env overrides")
+	}
+
+	// Keep track of the original values that the Environment settings overrode
+	loadedCfgWithoutEnvOverrides, _, err := unmarshalConfig(f2, false)
+	if err != nil {
+		return errors.Wrapf(err, "failed to unmarshal config without env overrides")
 	}
 
 	// SetDefaults generates various keys and salts if not previously configured. Determine if
@@ -114,13 +126,15 @@ func (cs *commonStore) load(f io.ReadCloser, needsSave bool, validate func(*mode
 	defer unlockOnce.Do(cs.configLock.Unlock)
 
 	if needsSave && persist != nil {
-		if err = persist(loadedCfg); err != nil {
+		cfgWithoutEnvOverrides := removeEnvOverrides(loadedCfg, loadedCfgWithoutEnvOverrides, environmentOverrides)
+		if err = persist(cfgWithoutEnvOverrides); err != nil {
 			return errors.Wrap(err, "failed to persist required changes after load")
 		}
 	}
 
 	oldCfg := cs.config
 	cs.config = loadedCfg
+	cs.configWithoutOverrides = loadedCfgWithoutEnvOverrides
 	cs.environmentOverrides = environmentOverrides
 
 	unlockOnce.Do(cs.configLock.Unlock)
@@ -140,3 +154,8 @@ func (cs *commonStore) validate(cfg *model.Config) error {
 
 	return nil
 }
+
+// removeEnvOverrides returns a new config without the given environment overrides.
+func (cs *commonStore) removeEnvOverrides(cfg *model.Config) *model.Config {
+	return removeEnvOverrides(cfg, cs.configWithoutOverrides, cs.environmentOverrides)
+}