Merge pull request #865 from mattermost/PLT-404

PLT-404 adding basic error checking to config file.

api/admin.go

@@ -82,18 +82,11 @@ func saveConfig(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if len(cfg.ServiceSettings.ListenAddress) == 0 {
+	if err := cfg.IsValid(); err != nil {
-		c.SetInvalidParam("saveConfig", "config")
+		c.Err = err
 		return
 	}
 
-	if cfg.TeamSettings.MaxUsersPerTeam == 0 {
-		c.SetInvalidParam("saveConfig", "config")
-		return
-	}
-
-	// TODO run some cleanup validators
-
 	utils.SaveConfig(utils.CfgFileName, cfg)
 	utils.LoadConfig(utils.CfgFileName)
 	json := utils.Cfg.ToJson()

config/config.json

@@ -86,4 +86,4 @@
         "TokenEndpoint": "",
         "UserApiEndpoint": ""
     }
-}
+}

model/config.go

@@ -16,6 +16,9 @@ const (
 	IMAGE_DRIVER_LOCAL = "local"
 	IMAGE_DRIVER_S3    = "amazons3"
 
+	DATABASE_DRIVER_MYSQL    = "mysql"
+	DATABASE_DRIVER_POSTGRES = "postgres"
+
 	SERVICE_GITLAB = "gitlab"
 )
 
@@ -156,3 +159,92 @@ func ConfigFromJson(data io.Reader) *Config {
 		return nil
 	}
 }
+
+func (o *Config) IsValid() *AppError {
+
+	if o.ServiceSettings.MaximumLoginAttempts <= 0 {
+		return NewAppError("Config.IsValid", "Invalid maximum login attempts for service settings.  Must be a positive number.", "")
+	}
+
+	if len(o.ServiceSettings.ListenAddress) == 0 {
+		return NewAppError("Config.IsValid", "Invalid listen address for service settings Must be set.", "")
+	}
+
+	if o.TeamSettings.MaxUsersPerTeam <= 0 {
+		return NewAppError("Config.IsValid", "Invalid maximum users per team for team settings.  Must be a positive number.", "")
+	}
+
+	if len(o.SqlSettings.AtRestEncryptKey) != 32 {
+		return NewAppError("Config.IsValid", "Invalid at rest encrypt key for SQL settings.  Must be 32 chars.", "")
+	}
+
+	if !(o.SqlSettings.DriverName == DATABASE_DRIVER_MYSQL || o.SqlSettings.DriverName == DATABASE_DRIVER_POSTGRES) {
+		return NewAppError("Config.IsValid", "Invalid driver name for SQL settings.  Must be 'mysql' or 'postgres'", "")
+	}
+
+	if o.SqlSettings.MaxIdleConns <= 0 {
+		return NewAppError("Config.IsValid", "Invalid maximum idle connection for SQL settings.  Must be a positive number.", "")
+	}
+
+	if len(o.SqlSettings.DataSource) == 0 {
+		return NewAppError("Config.IsValid", "Invalid data source for SQL settings.  Must be set.", "")
+	}
+
+	if o.SqlSettings.MaxOpenConns <= 0 {
+		return NewAppError("Config.IsValid", "Invalid maximum open connection for SQL settings.  Must be a positive number.", "")
+	}
+
+	if !(o.FileSettings.DriverName == IMAGE_DRIVER_LOCAL || o.FileSettings.DriverName == IMAGE_DRIVER_S3) {
+		return NewAppError("Config.IsValid", "Invalid driver name for file settings.  Must be 'local' or 'amazons3'", "")
+	}
+
+	if o.FileSettings.PreviewHeight < 0 {
+		return NewAppError("Config.IsValid", "Invalid preview height for file settings.  Must be a zero or positive number.", "")
+	}
+
+	if o.FileSettings.PreviewWidth <= 0 {
+		return NewAppError("Config.IsValid", "Invalid preview width for file settings.  Must be a positive number.", "")
+	}
+
+	if o.FileSettings.ProfileHeight <= 0 {
+		return NewAppError("Config.IsValid", "Invalid profile height for file settings.  Must be a positive number.", "")
+	}
+
+	if o.FileSettings.ProfileWidth <= 0 {
+		return NewAppError("Config.IsValid", "Invalid profile width for file settings.  Must be a positive number.", "")
+	}
+
+	if o.FileSettings.ThumbnailHeight <= 0 {
+		return NewAppError("Config.IsValid", "Invalid thumbnail height for file settings.  Must be a positive number.", "")
+	}
+
+	if o.FileSettings.ThumbnailHeight <= 0 {
+		return NewAppError("Config.IsValid", "Invalid thumbnail width for file settings.  Must be a positive number.", "")
+	}
+
+	if len(o.FileSettings.PublicLinkSalt) != 32 {
+		return NewAppError("Config.IsValid", "Invalid public link salt for file settings.  Must be 32 chars.", "")
+	}
+
+	if !(o.EmailSettings.ConnectionSecurity == CONN_SECURITY_NONE || o.EmailSettings.ConnectionSecurity == CONN_SECURITY_TLS || o.EmailSettings.ConnectionSecurity == CONN_SECURITY_STARTTLS) {
+		return NewAppError("Config.IsValid", "Invalid connection security for email settings.  Must be '', 'TLS', or 'STARTTLS'", "")
+	}
+
+	if len(o.EmailSettings.InviteSalt) != 32 {
+		return NewAppError("Config.IsValid", "Invalid invite salt for email settings.  Must be 32 chars.", "")
+	}
+
+	if len(o.EmailSettings.PasswordResetSalt) != 32 {
+		return NewAppError("Config.IsValid", "Invalid password reset salt for email settings.  Must be 32 chars.", "")
+	}
+
+	if o.RateLimitSettings.MemoryStoreSize <= 0 {
+		return NewAppError("Config.IsValid", "Invalid memory store size for rate limit settings.  Must be a positive number", "")
+	}
+
+	if o.RateLimitSettings.PerSec <= 0 {
+		return NewAppError("Config.IsValid", "Invalid per sec for rate limit settings.  Must be a positive number", "")
+	}
+
+	return nil
+}

store/sql_channel_store.go

@@ -583,7 +583,7 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelId string, userId string) Sto
 
 		var query string
 
-		if utils.Cfg.SqlSettings.DriverName == "postgres" {
+		if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 			query = `UPDATE
 				ChannelMembers
 			SET
@@ -597,7 +597,7 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelId string, userId string) Sto
 			    Channels.Id = ChannelMembers.ChannelId
 			        AND UserId = :UserId
 			        AND ChannelId = :ChannelId`
-		} else if utils.Cfg.SqlSettings.DriverName == "mysql" {
+		} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
 			query = `UPDATE
 				ChannelMembers, Channels
 			SET

store/sql_post_store.go

@@ -418,7 +418,7 @@ func (s SqlPostStore) Search(teamId string, userId string, terms string, isHasht
 
 		var posts []*model.Post
 
-		if utils.Cfg.SqlSettings.DriverName == "postgres" {
+		if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 
 			// Parse text for wildcards
 			if wildcard, err := regexp.Compile("\\*($| )"); err == nil {
@@ -451,7 +451,7 @@ func (s SqlPostStore) Search(teamId string, userId string, terms string, isHasht
 				result.Err = model.NewAppError("SqlPostStore.Search", "We encounted an error while searching for posts", "teamId="+teamId+", err="+err.Error())
 
 			}
-		} else if utils.Cfg.SqlSettings.DriverName == "mysql" {
+		} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
 			searchQuery := fmt.Sprintf(`SELECT
 				    *
 				FROM

store/sql_post_store_test.go

@@ -530,13 +530,6 @@ func TestPostStoreSearch(t *testing.T) {
 		t.Fatal("returned wrong serach result")
 	}
 
-	// if utils.Cfg.SqlSettings.DriverName == "mysql" {
-	// 	r2 := (<-store.Post().Search(teamId, userId, "new york", false)).Data.(*model.PostList)
-	// 	if len(r2.Order) >= 1 && r2.Order[0] != o2.Id {
-	// 		t.Fatal("returned wrong serach result")
-	// 	}
-	// }
-
 	r3 := (<-store.Post().Search(teamId, userId, "new", false)).Data.(*model.PostList)
 	if len(r3.Order) != 2 && r3.Order[0] != o1.Id {
 		t.Fatal("returned wrong serach result")

store/sql_store.go

@@ -160,9 +160,9 @@ func setupConnection(con_type string, driver string, dataSource string, maxIdle
 
 	if driver == "sqlite3" {
 		dbmap = &gorp.DbMap{Db: db, TypeConverter: mattermConverter{}, Dialect: gorp.SqliteDialect{}}
-	} else if driver == "mysql" {
+	} else if driver == model.DATABASE_DRIVER_MYSQL {
 		dbmap = &gorp.DbMap{Db: db, TypeConverter: mattermConverter{}, Dialect: gorp.MySQLDialect{Engine: "InnoDB", Encoding: "UTF8MB4"}}
-	} else if driver == "postgres" {
+	} else if driver == model.DATABASE_DRIVER_POSTGRES {
 		dbmap = &gorp.DbMap{Db: db, TypeConverter: mattermConverter{}, Dialect: gorp.PostgresDialect{}}
 	} else {
 		l4g.Critical("Failed to create dialect specific driver")
@@ -183,7 +183,7 @@ func (ss SqlStore) GetCurrentSchemaVersion() string {
 }
 
 func (ss SqlStore) DoesTableExist(tableName string) bool {
-	if utils.Cfg.SqlSettings.DriverName == "postgres" {
+	if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 		count, err := ss.GetMaster().SelectInt(
 			`SELECT count(relname) FROM pg_class WHERE relname=$1`,
 			strings.ToLower(tableName),
@@ -197,7 +197,7 @@ func (ss SqlStore) DoesTableExist(tableName string) bool {
 
 		return count > 0
 
-	} else if utils.Cfg.SqlSettings.DriverName == "mysql" {
+	} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
 
 		count, err := ss.GetMaster().SelectInt(
 			`SELECT
@@ -228,7 +228,7 @@ func (ss SqlStore) DoesTableExist(tableName string) bool {
 }
 
 func (ss SqlStore) DoesColumnExist(tableName string, columnName string) bool {
-	if utils.Cfg.SqlSettings.DriverName == "postgres" {
+	if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 		count, err := ss.GetMaster().SelectInt(
 			`SELECT COUNT(0)
 			FROM   pg_attribute
@@ -251,7 +251,7 @@ func (ss SqlStore) DoesColumnExist(tableName string, columnName string) bool {
 
 		return count > 0
 
-	} else if utils.Cfg.SqlSettings.DriverName == "mysql" {
+	} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
 
 		count, err := ss.GetMaster().SelectInt(
 			`SELECT
@@ -288,7 +288,7 @@ func (ss SqlStore) CreateColumnIfNotExists(tableName string, columnName string,
 		return false
 	}
 
-	if utils.Cfg.SqlSettings.DriverName == "postgres" {
+	if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 		_, err := ss.GetMaster().Exec("ALTER TABLE " + tableName + " ADD " + columnName + " " + postgresColType + " DEFAULT '" + defaultValue + "'")
 		if err != nil {
 			l4g.Critical("Failed to create column %v", err)
@@ -298,7 +298,7 @@ func (ss SqlStore) CreateColumnIfNotExists(tableName string, columnName string,
 
 		return true
 
-	} else if utils.Cfg.SqlSettings.DriverName == "mysql" {
+	} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
 		_, err := ss.GetMaster().Exec("ALTER TABLE " + tableName + " ADD " + columnName + " " + mySqlColType + " DEFAULT '" + defaultValue + "'")
 		if err != nil {
 			l4g.Critical("Failed to create column %v", err)
@@ -334,7 +334,7 @@ func (ss SqlStore) RemoveColumnIfExists(tableName string, columnName string) boo
 // func (ss SqlStore) RenameColumnIfExists(tableName string, oldColumnName string, newColumnName string, colType string) bool {
 
 // 	// XXX TODO FIXME this should be removed after 0.6.0
-// 	if utils.Cfg.SqlSettings.DriverName == "postgres" {
+// 	if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 // 		return false
 // 	}
 
@@ -363,7 +363,7 @@ func (ss SqlStore) CreateFullTextIndexIfNotExists(indexName string, tableName st
 
 func (ss SqlStore) createIndexIfNotExists(indexName string, tableName string, columnName string, fullText bool) {
 
-	if utils.Cfg.SqlSettings.DriverName == "postgres" {
+	if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
 		_, err := ss.GetMaster().SelectStr("SELECT $1::regclass", indexName)
 		// It should fail if the index does not exist
 		if err == nil {
@@ -383,7 +383,7 @@ func (ss SqlStore) createIndexIfNotExists(indexName string, tableName string, co
 			time.Sleep(time.Second)
 			panic("Failed to create index " + err.Error())
 		}
-	} else if utils.Cfg.SqlSettings.DriverName == "mysql" {
+	} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
 
 		count, err := ss.GetMaster().SelectInt("SELECT COUNT(0) AS index_exists FROM information_schema.statistics WHERE TABLE_SCHEMA = DATABASE() and table_name = ? AND index_name = ?", tableName, indexName)
 		if err != nil {

utils/config.go

@@ -150,6 +150,10 @@ func LoadConfig(fileName string) {
 		CfgFileName = fileName
 	}
 
+	if err := config.IsValid(); err != nil {
+		panic("Error validating config file=" + fileName + ", err=" + err.Message)
+	}
+
 	configureLog(&config.LogSettings)
 	TestConnection(&config)