[MM-16719] Setting the MM_SQLSETTINGS_DATASOURCEREPLICAS environment

variable breaks the server startup (#11504)
mattermost · Jul 5, 2019 · b41421c · b41421c
1 parent 634a2a8
commit b41421c
Show file tree

Hide file tree

Showing 4 changed files with 493 additions and 33 deletions.
diff --git a/config/common.go b/config/common.go
@@ -110,6 +110,7 @@ func (cs *commonStore) load(f io.ReadCloser, needsSave bool, validate func(*mode
 	needsSave = needsSave || loadedCfg.FileSettings.PublicLinkSalt == nil || len(*loadedCfg.FileSettings.PublicLinkSalt) == 0
 
 	loadedCfg.SetDefaults()
+	loadedCfgWithoutEnvOverrides.SetDefaults()
 
 	if validate != nil {
 		if err = validate(loadedCfg); err != nil {

diff --git a/config/database_test.go b/config/database_test.go
@@ -165,25 +165,140 @@ func TestDatabaseStoreGet(t *testing.T) {
 }
 
 func TestDatabaseStoreGetEnivironmentOverrides(t *testing.T) {
-	_, tearDown := setupConfigDatabase(t, testConfig, nil)
-	defer tearDown()
+	t.Run("get override for a string variable", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, testConfig, nil)
+		defer tearDown()
 
-	sqlSettings := mainHelper.GetSqlSettings()
-	ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
-	require.NoError(t, err)
-	defer ds.Close()
+		sqlSettings := mainHelper.GetSqlSettings()
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
 
-	assert.Equal(t, "http://TestStoreNew", *ds.Get().ServiceSettings.SiteURL)
-	assert.Empty(t, ds.GetEnvironmentOverrides())
+		assert.Equal(t, "http://TestStoreNew", *ds.Get().ServiceSettings.SiteURL)
+		assert.Empty(t, ds.GetEnvironmentOverrides())
 
-	os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override")
+		os.Setenv("MM_SERVICESETTINGS_SITEURL", "http://override")
 
-	ds, err = config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
-	require.NoError(t, err)
-	defer ds.Close()
+		ds, err = config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, "http://override", *ds.Get().ServiceSettings.SiteURL)
+		assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, ds.GetEnvironmentOverrides())
+	})
+
+	t.Run("get override for a bool variable", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, testConfig, nil)
+		defer tearDown()
+
+		sqlSettings := mainHelper.GetSqlSettings()
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, false, *ds.Get().PluginSettings.EnableUploads)
+		assert.Empty(t, ds.GetEnvironmentOverrides())
+
+		os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true")
+
+		ds, err = config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, true, *ds.Get().PluginSettings.EnableUploads)
+		assert.Equal(t, map[string]interface{}{"PluginSettings": map[string]interface{}{"EnableUploads": true}}, ds.GetEnvironmentOverrides())
+	})
+
+	t.Run("get override for an int variable", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, testConfig, nil)
+		defer tearDown()
+
+		sqlSettings := mainHelper.GetSqlSettings()
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, model.TEAM_SETTINGS_DEFAULT_MAX_USERS_PER_TEAM, *ds.Get().TeamSettings.MaxUsersPerTeam)
+		assert.Empty(t, ds.GetEnvironmentOverrides())
+
+		os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000")
+
+		ds, err = config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, 3000, *ds.Get().TeamSettings.MaxUsersPerTeam)
+		assert.Equal(t, map[string]interface{}{"TeamSettings": map[string]interface{}{"MaxUsersPerTeam": true}}, ds.GetEnvironmentOverrides())
+	})
+
+	t.Run("get override for an int64 variable", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, testConfig, nil)
+		defer tearDown()
+
+		sqlSettings := mainHelper.GetSqlSettings()
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, int64(63072000), *ds.Get().ServiceSettings.TLSStrictTransportMaxAge)
+		assert.Empty(t, ds.GetEnvironmentOverrides())
+
+		os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456")
+
+		ds, err = config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, int64(123456), *ds.Get().ServiceSettings.TLSStrictTransportMaxAge)
+		assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"TLSStrictTransportMaxAge": true}}, ds.GetEnvironmentOverrides())
+	})
+
+	t.Run("get override for a slice variable - one value", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, testConfig, nil)
+		defer tearDown()
+
+		sqlSettings := mainHelper.GetSqlSettings()
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, []string{}, ds.Get().SqlSettings.DataSourceReplicas)
+		assert.Empty(t, ds.GetEnvironmentOverrides())
+
+		os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db")
+
+		ds, err = config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas)
+		assert.Equal(t, map[string]interface{}{"SqlSettings": map[string]interface{}{"DataSourceReplicas": true}}, ds.GetEnvironmentOverrides())
+	})
+
+	t.Run("get override for a slice variable - three values", func(t *testing.T) {
+		// This should work, but Viper (or we) don't parse environment variables to turn strings with spaces into slices.
+		t.Skip("not implemented yet")
+
+		_, tearDown := setupConfigDatabase(t, testConfig, nil)
+		defer tearDown()
+
+		sqlSettings := mainHelper.GetSqlSettings()
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
 
-	assert.Equal(t, "http://override", *ds.Get().ServiceSettings.SiteURL)
-	assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, ds.GetEnvironmentOverrides())
+		assert.Equal(t, []string{}, ds.Get().SqlSettings.DataSourceReplicas)
+		assert.Empty(t, ds.GetEnvironmentOverrides())
+
+		os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db user:pwd@db2:5433/test-db2 user:pwd@db3:5434/test-db3")
+
+		ds, err = config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, []string{"user:pwd@db:5432/test-db", "user:pwd@db2:5433/test-db2", "user:pwd@db3:5434/test-db3"}, ds.Get().SqlSettings.DataSourceReplicas)
+		assert.Equal(t, map[string]interface{}{"SqlSettings": map[string]interface{}{"DataSourceReplicas": true}}, ds.GetEnvironmentOverrides())
+	})
 }
 
 func TestDatabaseStoreSet(t *testing.T) {
@@ -417,7 +532,7 @@ func TestDatabaseStoreLoad(t *testing.T) {
 		assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"SiteURL": true}}, ds.GetEnvironmentOverrides())
 	})
 
-	t.Run("do not persist environment variables", func(t *testing.T) {
+	t.Run("do not persist environment variables - string", func(t *testing.T) {
 		_, tearDown := setupConfigDatabase(t, minimalConfig, nil)
 		defer tearDown()
 
@@ -437,6 +552,118 @@ func TestDatabaseStoreLoad(t *testing.T) {
 		assert.Equal(t, "http://minimal", *actualConfig.ServiceSettings.SiteURL)
 	})
 
+	t.Run("do not persist environment variables - boolean", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, minimalConfig, nil)
+		defer tearDown()
+
+		os.Setenv("MM_PLUGINSETTINGS_ENABLEUPLOADS", "true")
+
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, true, *ds.Get().PluginSettings.EnableUploads)
+
+		_, err = ds.Set(ds.Get())
+		require.NoError(t, err)
+
+		assert.Equal(t, true, *ds.Get().PluginSettings.EnableUploads)
+		assert.Equal(t, map[string]interface{}{"PluginSettings": map[string]interface{}{"EnableUploads": true}}, ds.GetEnvironmentOverrides())
+		// check that in DB config does not include overwritten variable
+		_, actualConfig := getActualDatabaseConfig(t)
+		assert.Equal(t, false, *actualConfig.PluginSettings.EnableUploads)
+	})
+
+	t.Run("do not persist environment variables - int", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, minimalConfig, nil)
+		defer tearDown()
+
+		os.Setenv("MM_TEAMSETTINGS_MAXUSERSPERTEAM", "3000")
+
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, 3000, *ds.Get().TeamSettings.MaxUsersPerTeam)
+
+		_, err = ds.Set(ds.Get())
+		require.NoError(t, err)
+
+		assert.Equal(t, 3000, *ds.Get().TeamSettings.MaxUsersPerTeam)
+		assert.Equal(t, map[string]interface{}{"TeamSettings": map[string]interface{}{"MaxUsersPerTeam": true}}, ds.GetEnvironmentOverrides())
+		// check that in DB config does not include overwritten variable
+		_, actualConfig := getActualDatabaseConfig(t)
+		assert.Equal(t, model.TEAM_SETTINGS_DEFAULT_MAX_USERS_PER_TEAM, *actualConfig.TeamSettings.MaxUsersPerTeam)
+	})
+
+	t.Run("do not persist environment variables - int64", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, minimalConfig, nil)
+		defer tearDown()
+
+		os.Setenv("MM_SERVICESETTINGS_TLSSTRICTTRANSPORTMAXAGE", "123456")
+
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, int64(123456), *ds.Get().ServiceSettings.TLSStrictTransportMaxAge)
+
+		_, err = ds.Set(ds.Get())
+		require.NoError(t, err)
+
+		assert.Equal(t, int64(123456), *ds.Get().ServiceSettings.TLSStrictTransportMaxAge)
+		assert.Equal(t, map[string]interface{}{"ServiceSettings": map[string]interface{}{"TLSStrictTransportMaxAge": true}}, ds.GetEnvironmentOverrides())
+		// check that in DB config does not include overwritten variable
+		_, actualConfig := getActualDatabaseConfig(t)
+		assert.Equal(t, int64(63072000), *actualConfig.ServiceSettings.TLSStrictTransportMaxAge)
+	})
+
+	t.Run("do not persist environment variables - string slice beginning with default", func(t *testing.T) {
+		_, tearDown := setupConfigDatabase(t, minimalConfig, nil)
+		defer tearDown()
+
+		os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db")
+
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas)
+
+		_, err = ds.Set(ds.Get())
+		require.NoError(t, err)
+
+		assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas)
+		assert.Equal(t, map[string]interface{}{"SqlSettings": map[string]interface{}{"DataSourceReplicas": true}}, ds.GetEnvironmentOverrides())
+		// check that in DB config does not include overwritten variable
+		_, actualConfig := getActualDatabaseConfig(t)
+		assert.Equal(t, []string(nil), actualConfig.SqlSettings.DataSourceReplicas)
+	})
+
+	t.Run("do not persist environment variables - string slice beginning with slice of three", func(t *testing.T) {
+		modifiedMinimalConfig := minimalConfig.Clone()
+		modifiedMinimalConfig.SqlSettings.DataSourceReplicas = []string{"user:pwd@db:5432/test-db", "user:pwd@db2:5433/test-db2", "user:pwd@db3:5434/test-db3"}
+		_, tearDown := setupConfigDatabase(t, modifiedMinimalConfig, nil)
+		defer tearDown()
+
+		os.Setenv("MM_SQLSETTINGS_DATASOURCEREPLICAS", "user:pwd@db:5432/test-db")
+
+		ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource))
+		require.NoError(t, err)
+		defer ds.Close()
+
+		assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas)
+
+		_, err = ds.Set(ds.Get())
+		require.NoError(t, err)
+
+		assert.Equal(t, []string{"user:pwd@db:5432/test-db"}, ds.Get().SqlSettings.DataSourceReplicas)
+		assert.Equal(t, map[string]interface{}{"SqlSettings": map[string]interface{}{"DataSourceReplicas": true}}, ds.GetEnvironmentOverrides())
+		// check that in DB config does not include overwritten variable
+		_, actualConfig := getActualDatabaseConfig(t)
+		assert.Equal(t, []string{"user:pwd@db:5432/test-db", "user:pwd@db2:5433/test-db2", "user:pwd@db3:5434/test-db3"}, actualConfig.SqlSettings.DataSourceReplicas)
+	})
+
 	t.Run("invalid", func(t *testing.T) {
 		_, tearDown := setupConfigDatabase(t, emptyConfig, nil)
 		defer tearDown()

diff --git a/config/environment.go b/config/environment.go
@@ -46,16 +46,26 @@ func getPathsRec(src interface{}, curPath []string) [][]string {
 // and returns the reflect.Value of the leaf at the end `path`
 func getVal(src interface{}, path []string) reflect.Value {
 	var val reflect.Value
-	if reflect.ValueOf(src).Kind() == reflect.Ptr {
-		val = reflect.ValueOf(src).Elem().FieldByName(path[0])
+
+	// If we recursed on a Value, we already have it. If we're calling on an interface{}, get the Value.
+	switch v := src.(type) {
+	case reflect.Value:
+		val = v
+	default:
+		val = reflect.ValueOf(src)
+	}
+
+	// Move into the struct
+	if val.Kind() == reflect.Ptr {
+		val = val.Elem().FieldByName(path[0])
 	} else {
-		val = reflect.ValueOf(src).FieldByName(path[0])
+		val = val.FieldByName(path[0])
 	}
 	if val.Kind() == reflect.Ptr {
 		val = val.Elem()
 	}
 	if val.Kind() == reflect.Struct {
-		return getVal(val.Interface(), path[1:])
+		return getVal(val, path[1:])
 	}
 	return val
 }