From 0a6b96cb40d9dd5acca7e366df9cd14fa4eff6a7 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 9 Apr 2018 12:16:11 -0400 Subject: MM-9849 Added tracking of which settings are set through environment variables (#8586) * MM-9849 Added tracking of which settings are set through environment variables * Removed old version of viper * Added forked version of viper * Fixed unit tests * Fixed more unit tests * Removed copy from App.GetEnvironmentConfig --- utils/config.go | 90 ++++++++++++++++++++++++++++++++++++++++++---------- utils/config_test.go | 74 +++++++++++++++++++++++++++++++----------- utils/mail_test.go | 6 ++-- 3 files changed, 131 insertions(+), 39 deletions(-) (limited to 'utils') diff --git a/utils/config.go b/utils/config.go index b87f164ee..d0a6a17ed 100644 --- a/utils/config.go +++ b/utils/config.go @@ -188,7 +188,7 @@ func NewConfigWatcher(cfgFileName string, f func()) (*ConfigWatcher, error) { if event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Create == fsnotify.Create { l4g.Info(fmt.Sprintf("Config file watcher detected a change reloading %v", cfgFileName)) - if _, configReadErr := ReadConfigFile(cfgFileName, true); configReadErr == nil { + if _, _, configReadErr := ReadConfigFile(cfgFileName, true); configReadErr == nil { f() } else { l4g.Error(fmt.Sprintf("Failed to read while watching config file at %v with err=%v", cfgFileName, configReadErr.Error())) @@ -212,11 +212,11 @@ func (w *ConfigWatcher) Close() { } // ReadConfig reads and parses the given configuration. -func ReadConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, error) { +func ReadConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, map[string]interface{}, error) { v := newViper(allowEnvironmentOverrides) if err := v.ReadConfig(r); err != nil { - return nil, err + return nil, nil, err } var config model.Config @@ -227,7 +227,15 @@ func ReadConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, err config.PluginSettings = model.PluginSettings{} unmarshalErr = v.UnmarshalKey("pluginsettings", &config.PluginSettings) } - return &config, unmarshalErr + + envConfig := v.EnvSettings() + + var envErr error + if envConfig, envErr = fixEnvSettingsCase(envConfig); envErr != nil { + return nil, nil, envErr + } + + return &config, envConfig, unmarshalErr } func newViper(allowEnvironmentOverrides bool) *viper.Viper { @@ -254,13 +262,19 @@ func newViper(allowEnvironmentOverrides bool) *viper.Viper { // Converts a struct type into a nested map with keys matching the struct's fields and values // matching the zeroed value of the corresponding field. -func structToMap(t reflect.Type) map[string]interface{} { +func structToMap(t reflect.Type) (out map[string]interface{}) { + defer func() { + if r := recover(); r != nil { + l4g.Error("Panicked in structToMap. This should never happen. %v", r) + } + }() + if t.Kind() != reflect.Struct { // Should never hit this, but this will prevent a panic if that does happen somehow return nil } - out := make(map[string]interface{}) + out = map[string]interface{}{} for i := 0; i < t.NumField(); i++ { field := t.Field(i) @@ -279,7 +293,7 @@ func structToMap(t reflect.Type) map[string]interface{} { out[field.Name] = value } - return out + return } // Flattens a nested map so that the result is a single map with keys corresponding to the @@ -313,11 +327,51 @@ func flattenStructToMap(in map[string]interface{}) map[string]interface{} { return out } +// Fixes the case of the environment variables sent back from Viper since Viper stores +// everything as lower case. +func fixEnvSettingsCase(in map[string]interface{}) (out map[string]interface{}, err error) { + defer func() { + if r := recover(); r != nil { + l4g.Error("Panicked in fixEnvSettingsCase. This should never happen. %v", r) + out = in + } + }() + + var fixCase func(map[string]interface{}, reflect.Type) map[string]interface{} + fixCase = func(in map[string]interface{}, t reflect.Type) map[string]interface{} { + if t.Kind() != reflect.Struct { + // Should never hit this, but this will prevent a panic if that does happen somehow + return nil + } + + out := make(map[string]interface{}, len(in)) + + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + + key := field.Name + if value, ok := in[strings.ToLower(key)]; ok { + if valueAsMap, ok := value.(map[string]interface{}); ok { + out[key] = fixCase(valueAsMap, field.Type) + } else { + out[key] = value + } + } + } + + return out + } + + out = fixCase(in, reflect.TypeOf(model.Config{})) + + return +} + // ReadConfigFile reads and parses the configuration at the given file path. -func ReadConfigFile(path string, allowEnvironmentOverrides bool) (*model.Config, error) { +func ReadConfigFile(path string, allowEnvironmentOverrides bool) (*model.Config, map[string]interface{}, error) { f, err := os.Open(path) if err != nil { - return nil, err + return nil, nil, err } defer f.Close() return ReadConfig(f, allowEnvironmentOverrides) @@ -352,22 +406,24 @@ func EnsureConfigFile(fileName string) (string, error) { // LoadConfig will try to search around for the corresponding config file. It will search // /tmp/fileName then attempt ./config/fileName, then ../config/fileName and last it will look at // fileName. -func LoadConfig(fileName string) (config *model.Config, configPath string, appErr *model.AppError) { +func LoadConfig(fileName string) (*model.Config, string, map[string]interface{}, *model.AppError) { + var configPath string + if fileName != filepath.Base(fileName) { configPath = fileName } else { if path, err := EnsureConfigFile(fileName); err != nil { - appErr = model.NewAppError("LoadConfig", "utils.config.load_config.opening.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) - return + appErr := model.NewAppError("LoadConfig", "utils.config.load_config.opening.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) + return nil, "", nil, appErr } else { configPath = path } } - config, err := ReadConfigFile(configPath, true) + config, envConfig, err := ReadConfigFile(configPath, true) if err != nil { - appErr = model.NewAppError("LoadConfig", "utils.config.load_config.decoding.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) - return + appErr := model.NewAppError("LoadConfig", "utils.config.load_config.decoding.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) + return nil, "", nil, appErr } needSave := len(config.SqlSettings.AtRestEncryptKey) == 0 || len(*config.FileSettings.PublicLinkSalt) == 0 || @@ -376,7 +432,7 @@ func LoadConfig(fileName string) (config *model.Config, configPath string, appEr config.SetDefaults() if err := config.IsValid(); err != nil { - return nil, "", err + return nil, "", nil, err } if needSave { @@ -398,7 +454,7 @@ func LoadConfig(fileName string) (config *model.Config, configPath string, appEr } } - return config, configPath, nil + return config, configPath, envConfig, nil } func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.License) map[string]string { diff --git a/utils/config_test.go b/utils/config_test.go index a998bfbc6..fbac577ee 100644 --- a/utils/config_test.go +++ b/utils/config_test.go @@ -18,7 +18,7 @@ import ( func TestConfig(t *testing.T) { TranslationsPreInit() - cfg, _, err := LoadConfig("config.json") + cfg, _, _, err := LoadConfig("config.json") require.Nil(t, err) InitTranslations(cfg.LocalizationSettings) } @@ -67,7 +67,7 @@ func TestConfigFromEnviroVars(t *testing.T) { os.Setenv("MM_TEAMSETTINGS_SITENAME", "From Environment") os.Setenv("MM_TEAMSETTINGS_CUSTOMBRANDTEXT", "Custom Brand") - cfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) require.Nil(t, err) if cfg.TeamSettings.SiteName != "From Environment" { @@ -78,57 +78,105 @@ func TestConfigFromEnviroVars(t *testing.T) { t.Fatal("Couldn't read config from environment var") } + if teamSettings, ok := envCfg["TeamSettings"]; !ok { + t.Fatal("TeamSettings is missing from envConfig") + } else if teamSettingsAsMap, ok := teamSettings.(map[string]interface{}); !ok { + t.Fatal("TeamSettings is not a map in envConfig") + } else { + if siteNameInEnv, ok := teamSettingsAsMap["SiteName"].(bool); !ok || !siteNameInEnv { + t.Fatal("SiteName should be in envConfig") + } + + if customBrandTextInEnv, ok := teamSettingsAsMap["CustomBrandText"].(bool); !ok || !customBrandTextInEnv { + t.Fatal("SiteName should be in envConfig") + } + } + os.Unsetenv("MM_TEAMSETTINGS_SITENAME") os.Unsetenv("MM_TEAMSETTINGS_CUSTOMBRANDTEXT") - cfg, err = ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err = ReadConfig(strings.NewReader(config), true) require.Nil(t, err) if cfg.TeamSettings.SiteName != "Mattermost" { t.Fatal("should have been reset") } + + if _, ok := envCfg["TeamSettings"]; ok { + t.Fatal("TeamSettings should be missing from envConfig") + } }) t.Run("boolean setting", func(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_ENABLECOMMANDS", "false") defer os.Unsetenv("MM_SERVICESETTINGS_ENABLECOMMANDS") - cfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) require.Nil(t, err) if *cfg.ServiceSettings.EnableCommands { t.Fatal("Couldn't read config from environment var") } + + if serviceSettings, ok := envCfg["ServiceSettings"]; !ok { + t.Fatal("ServiceSettings is missing from envConfig") + } else if serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{}); !ok { + t.Fatal("ServiceSettings is not a map in envConfig") + } else { + if enableCommandsInEnv, ok := serviceSettingsAsMap["EnableCommands"].(bool); !ok || !enableCommandsInEnv { + t.Fatal("EnableCommands should be in envConfig") + } + } }) t.Run("integer setting", func(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_READTIMEOUT", "400") defer os.Unsetenv("MM_SERVICESETTINGS_READTIMEOUT") - cfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) require.Nil(t, err) if *cfg.ServiceSettings.ReadTimeout != 400 { t.Fatal("Couldn't read config from environment var") } + + if serviceSettings, ok := envCfg["ServiceSettings"]; !ok { + t.Fatal("ServiceSettings is missing from envConfig") + } else if serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{}); !ok { + t.Fatal("ServiceSettings is not a map in envConfig") + } else { + if readTimeoutInEnv, ok := serviceSettingsAsMap["ReadTimeout"].(bool); !ok || !readTimeoutInEnv { + t.Fatal("ReadTimeout should be in envConfig") + } + } }) t.Run("setting missing from config.json", func(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_SITEURL", "https://example.com") defer os.Unsetenv("MM_SERVICESETTINGS_SITEURL") - cfg, err := ReadConfig(strings.NewReader(config), true) + cfg, envCfg, err := ReadConfig(strings.NewReader(config), true) require.Nil(t, err) if *cfg.ServiceSettings.SiteURL != "https://example.com" { t.Fatal("Couldn't read config from environment var") } + + if serviceSettings, ok := envCfg["ServiceSettings"]; !ok { + t.Fatal("ServiceSettings is missing from envConfig") + } else if serviceSettingsAsMap, ok := serviceSettings.(map[string]interface{}); !ok { + t.Fatal("ServiceSettings is not a map in envConfig") + } else { + if siteURLInEnv, ok := serviceSettingsAsMap["SiteURL"].(bool); !ok || !siteURLInEnv { + t.Fatal("SiteURL should be in envConfig") + } + } }) } func TestValidateLocales(t *testing.T) { TranslationsPreInit() - cfg, _, err := LoadConfig("config.json") + cfg, _, _, err := LoadConfig("config.json") require.Nil(t, err) *cfg.LocalizationSettings.DefaultServerLocale = "en" @@ -326,18 +374,6 @@ func TestGetClientConfig(t *testing.T) { } }) } - -} - -func TestReadConfig(t *testing.T) { - config, err := ReadConfig(strings.NewReader(`{ - "ServiceSettings": { - "SiteURL": "http://foo.bar" - } - }`), false) - require.NoError(t, err) - - assert.Equal(t, "http://foo.bar", *config.ServiceSettings.SiteURL) } func sToP(s string) *string { diff --git a/utils/mail_test.go b/utils/mail_test.go index 65b89c240..6bd8e7044 100644 --- a/utils/mail_test.go +++ b/utils/mail_test.go @@ -13,7 +13,7 @@ import ( ) func TestMailConnectionFromConfig(t *testing.T) { - cfg, _, err := LoadConfig("config.json") + cfg, _, _, err := LoadConfig("config.json") require.Nil(t, err) if conn, err := ConnectToSMTPServer(cfg); err != nil { @@ -36,7 +36,7 @@ func TestMailConnectionFromConfig(t *testing.T) { } func TestMailConnectionAdvanced(t *testing.T) { - cfg, _, err := LoadConfig("config.json") + cfg, _, _, err := LoadConfig("config.json") require.Nil(t, err) if conn, err := ConnectToSMTPServerAdvanced( @@ -86,7 +86,7 @@ func TestMailConnectionAdvanced(t *testing.T) { } func TestSendMailUsingConfig(t *testing.T) { - cfg, _, err := LoadConfig("config.json") + cfg, _, _, err := LoadConfig("config.json") require.Nil(t, err) T = GetUserTranslations("en") -- cgit v1.2.3-1-g7c22