Fix bug on admin subcommand (#17533)

* Fix bug on admin subcommand

* Add signals for all initDB

Co-authored-by: Lauris BH <lauris@nix.lv>

cmd/admin.go

@@ -339,7 +339,10 @@ func runChangePassword(c *cli.Context) error {
 		return err
 	}
 
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 	if !pwd.IsComplexEnough(c.String("password")) {
@@ -393,7 +396,10 @@ func runCreateUser(c *cli.Context) error {
 		fmt.Fprintf(os.Stderr, "--name flag is deprecated. Use --username instead.\n")
 	}
 
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 
@@ -456,7 +462,10 @@ func runCreateUser(c *cli.Context) error {
 }
 
 func runListUsers(c *cli.Context) error {
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 
@@ -493,7 +502,10 @@ func runDeleteUser(c *cli.Context) error {
 		return fmt.Errorf("You must provide the id, username or email of a user to delete")
 	}
 
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 
@@ -525,7 +537,10 @@ func runDeleteUser(c *cli.Context) error {
 }
 
 func runRepoSyncReleases(_ *cli.Context) error {
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 
@@ -591,14 +606,20 @@ func getReleaseCount(id int64) (int64, error) {
 }
 
 func runRegenerateHooks(_ *cli.Context) error {
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 	return repo_module.SyncRepositoryHooks(graceful.GetManager().ShutdownContext())
 }
 
 func runRegenerateKeys(_ *cli.Context) error {
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 	return models.RewriteAllPublicKeys()
@@ -628,7 +649,10 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source {
 }
 
 func runAddOauth(c *cli.Context) error {
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 
@@ -645,7 +669,10 @@ func runUpdateOauth(c *cli.Context) error {
 		return fmt.Errorf("--id flag is missing")
 	}
 
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 
@@ -712,7 +739,10 @@ func runUpdateOauth(c *cli.Context) error {
 }
 
 func runListAuth(c *cli.Context) error {
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 
@@ -748,7 +778,10 @@ func runDeleteAuth(c *cli.Context) error {
 		return fmt.Errorf("--id flag is missing")
 	}
 
-	if err := initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(ctx); err != nil {
 		return err
 	}
 

cmd/admin_auth_ldap.go

@@ -5,6 +5,7 @@
 package cmd
 
 import (
+	"context"
 	"fmt"
 	"strings"
 
@@ -16,7 +17,7 @@ import (
 
 type (
 	authService struct {
-		initDB             func() error
+		initDB             func(ctx context.Context) error
 		createLoginSource  func(loginSource *login.Source) error
 		updateLoginSource  func(loginSource *login.Source) error
 		getLoginSourceByID func(id int64) (*login.Source, error)
@@ -299,7 +300,10 @@ func (a *authService) addLdapBindDn(c *cli.Context) error {
 		return err
 	}
 
-	if err := a.initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := a.initDB(ctx); err != nil {
 		return err
 	}
 
@@ -321,7 +325,10 @@ func (a *authService) addLdapBindDn(c *cli.Context) error {
 
 // updateLdapBindDn updates a new LDAP via Bind DN authentication source.
 func (a *authService) updateLdapBindDn(c *cli.Context) error {
-	if err := a.initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := a.initDB(ctx); err != nil {
 		return err
 	}
 
@@ -344,7 +351,10 @@ func (a *authService) addLdapSimpleAuth(c *cli.Context) error {
 		return err
 	}
 
-	if err := a.initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := a.initDB(ctx); err != nil {
 		return err
 	}
 
@@ -366,7 +376,10 @@ func (a *authService) addLdapSimpleAuth(c *cli.Context) error {
 
 // updateLdapBindDn updates a new LDAP (simple auth) authentication source.
 func (a *authService) updateLdapSimpleAuth(c *cli.Context) error {
-	if err := a.initDB(); err != nil {
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	if err := a.initDB(ctx); err != nil {
 		return err
 	}
 

cmd/admin_auth_ldap_test.go

@@ -5,6 +5,7 @@
 package cmd
 
 import (
+	"context"
 	"testing"
 
 	"code.gitea.io/gitea/models/login"
@@ -207,7 +208,7 @@ func TestAddLdapBindDn(t *testing.T) {
 		// Mock functions.
 		var createdLoginSource *login.Source
 		service := &authService{
-			initDB: func() error {
+			initDB: func(context.Context) error {
 				return nil
 			},
 			createLoginSource: func(loginSource *login.Source) error {
@@ -438,7 +439,7 @@ func TestAddLdapSimpleAuth(t *testing.T) {
 		// Mock functions.
 		var createdLoginSource *login.Source
 		service := &authService{
-			initDB: func() error {
+			initDB: func(context.Context) error {
 				return nil
 			},
 			createLoginSource: func(loginSource *login.Source) error {
@@ -863,7 +864,7 @@ func TestUpdateLdapBindDn(t *testing.T) {
 		// Mock functions.
 		var updatedLoginSource *login.Source
 		service := &authService{
-			initDB: func() error {
+			initDB: func(context.Context) error {
 				return nil
 			},
 			createLoginSource: func(loginSource *login.Source) error {
@@ -1227,7 +1228,7 @@ func TestUpdateLdapSimpleAuth(t *testing.T) {
 		// Mock functions.
 		var updatedLoginSource *login.Source
 		service := &authService{
-			initDB: func() error {
+			initDB: func(context.Context) error {
 				return nil
 			},
 			createLoginSource: func(loginSource *login.Source) error {

cmd/cmd.go

@@ -56,16 +56,15 @@ func confirm() (bool, error) {
 	}
 }
 
-func initDB() error {
+func initDB(ctx context.Context) error {
-	return initDBDisableConsole(false)
+	return initDBDisableConsole(ctx, false)
 }
 
-func initDBDisableConsole(disableConsole bool) error {
+func initDBDisableConsole(ctx context.Context, disableConsole bool) error {
 	setting.NewContext()
 	setting.InitDBConfig()
-
 	setting.NewXORMLogService(disableConsole)
-	if err := db.InitEngine(); err != nil {
+	if err := db.InitEngine(ctx); err != nil {
 		return fmt.Errorf("models.SetEngine: %v", err)
 	}
 	return nil

cmd/convert.go

@@ -23,7 +23,10 @@ var CmdConvert = cli.Command{
 }
 
 func runConvert(ctx *cli.Context) error {
-	if err := initDB(); err != nil {
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(stdCtx); err != nil {
 		return err
 	}
 

cmd/doctor.go

@@ -96,7 +96,10 @@ func runRecreateTable(ctx *cli.Context) error {
 	setting.Cfg.Section("log").Key("XORM").SetValue(",")
 
 	setting.NewXORMLogService(!ctx.Bool("debug"))
-	if err := db.InitEngine(); err != nil {
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	if err := db.InitEngine(stdCtx); err != nil {
 		fmt.Println(err)
 		fmt.Println("Check if you are using the right config file. You can use a --config directive to specify one.")
 		return nil
@@ -128,6 +131,9 @@ func runDoctor(ctx *cli.Context) error {
 	log.DelNamedLogger("console")
 	log.DelNamedLogger(log.DEFAULT)
 
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
 	// Now setup our own
 	logFile := ctx.String("log-file")
 	if !ctx.IsSet("log-file") {
@@ -210,5 +216,5 @@ func runDoctor(ctx *cli.Context) error {
 
 	logger := log.GetLogger("doctorouter")
 	defer logger.Close()
-	return doctor.RunChecks(logger, ctx.Bool("fix"), checks)
+	return doctor.RunChecks(stdCtx, logger, ctx.Bool("fix"), checks)
 }

cmd/dump.go

@@ -173,7 +173,10 @@ func runDump(ctx *cli.Context) error {
 	}
 	setting.NewServices() // cannot access session settings otherwise
 
-	err := db.InitEngine()
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	err := db.InitEngine(stdCtx)
 	if err != nil {
 		return err
 	}

cmd/dump_repo.go

@@ -76,7 +76,10 @@ wiki, issues, labels, releases, release_assets, milestones, pull_requests, comme
 }
 
 func runDumpRepository(ctx *cli.Context) error {
-	if err := initDB(); err != nil {
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(stdCtx); err != nil {
 		return err
 	}
 

cmd/migrate.go

@@ -24,7 +24,10 @@ var CmdMigrate = cli.Command{
 }
 
 func runMigrate(ctx *cli.Context) error {
-	if err := initDB(); err != nil {
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(stdCtx); err != nil {
 		return err
 	}
 

cmd/migrate_storage.go

@@ -107,7 +107,10 @@ func migrateRepoAvatars(dstStorage storage.ObjectStorage) error {
 }
 
 func runMigrateStorage(ctx *cli.Context) error {
-	if err := initDB(); err != nil {
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	if err := initDB(stdCtx); err != nil {
 		return err
 	}
 

models/db/engine.go

@@ -128,40 +128,8 @@ func syncTables() error {
 	return x.StoreEngine("InnoDB").Sync2(tables...)
 }
 
-// InitInstallEngineWithMigration creates a new xorm.Engine for testing during install
-//
-// This function will cause the basic database schema to be created
-func InitInstallEngineWithMigration(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err error) {
-	x, err = NewEngine()
-	if err != nil {
-		return fmt.Errorf("failed to connect to database: %w", err)
-	}
-
-	x.SetMapper(names.GonicMapper{})
-	x.SetLogger(NewXORMLogger(!setting.IsProd))
-	x.ShowSQL(!setting.IsProd)
-
-	x.SetDefaultContext(ctx)
-
-	if err = x.Ping(); err != nil {
-		return err
-	}
-
-	// We have to run migrateFunc here in case the user is re-running installation on a previously created DB.
-	// If we do not then table schemas will be changed and there will be conflicts when the migrations run properly.
-	//
-	// Installation should only be being re-run if users want to recover an old database.
-	// However, we should think carefully about should we support re-install on an installed instance,
-	// as there may be other problems due to secret reinitialization.
-	if err = migrateFunc(x); err != nil {
-		return fmt.Errorf("migrate: %v", err)
-	}
-
-	return syncTables()
-}
-
 // InitEngine sets the xorm.Engine
-func InitEngine() (err error) {
+func InitEngine(ctx context.Context) (err error) {
 	x, err = NewEngine()
 	if err != nil {
 		return fmt.Errorf("Failed to connect to database: %v", err)
@@ -175,6 +143,12 @@ func InitEngine() (err error) {
 	x.SetMaxOpenConns(setting.Database.MaxOpenConns)
 	x.SetMaxIdleConns(setting.Database.MaxIdleConns)
 	x.SetConnMaxLifetime(setting.Database.ConnMaxLifetime)
+
+	DefaultContext = &Context{
+		Context: ctx,
+		e:       x,
+	}
+	x.SetDefaultContext(ctx)
 	return nil
 }
 
@@ -184,21 +158,20 @@ func InitEngine() (err error) {
 // that prevents the doctor from fixing anything in the database if the migration level
 // is different from the expected value.
 func InitEngineWithMigration(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err error) {
-	if err = InitEngine(); err != nil {
+	if err = InitEngine(ctx); err != nil {
 		return err
 	}
 
-	DefaultContext = &Context{
-		Context: ctx,
-		e:       x,
-	}
-
-	x.SetDefaultContext(ctx)
-
 	if err = x.Ping(); err != nil {
 		return err
 	}
 
+	// We have to run migrateFunc here in case the user is re-running installation on a previously created DB.
+	// If we do not then table schemas will be changed and there will be conflicts when the migrations run properly.
+	//
+	// Installation should only be being re-run if users want to recover an old database.
+	// However, we should think carefully about should we support re-install on an installed instance,
+	// as there may be other problems due to secret reinitialization.
 	if err = migrateFunc(x); err != nil {
 		return fmt.Errorf("migrate: %v", err)
 	}

modules/doctor/doctor.go

@@ -5,6 +5,7 @@
 package doctor
 
 import (
+	"context"
 	"fmt"
 	"sort"
 	"strings"
@@ -42,12 +43,12 @@ func (w *wrappedLevelLogger) Log(skip int, level log.Level, format string, v ...
 			}, v...)...)
 }
 
-func initDBDisableConsole(disableConsole bool) error {
+func initDBDisableConsole(ctx context.Context, disableConsole bool) error {
 	setting.NewContext()
 	setting.InitDBConfig()
 
 	setting.NewXORMLogService(disableConsole)
-	if err := db.InitEngine(); err != nil {
+	if err := db.InitEngine(ctx); err != nil {
 		return fmt.Errorf("models.SetEngine: %v", err)
 	}
 	return nil
@@ -57,7 +58,7 @@ func initDBDisableConsole(disableConsole bool) error {
 var Checks []*Check
 
 // RunChecks runs the doctor checks for the provided list
-func RunChecks(logger log.Logger, autofix bool, checks []*Check) error {
+func RunChecks(ctx context.Context, logger log.Logger, autofix bool, checks []*Check) error {
 	wrappedLogger := log.LevelLoggerLogger{
 		LevelLogger: &wrappedLevelLogger{logger},
 	}
@@ -67,7 +68,7 @@ func RunChecks(logger log.Logger, autofix bool, checks []*Check) error {
 		if !dbIsInit && !check.SkipDatabaseInitialization {
 			// Only open database after the most basic configuration check
 			setting.EnableXORMLog = false
-			if err := initDBDisableConsole(true); err != nil {
+			if err := initDBDisableConsole(ctx, true); err != nil {
 				logger.Error("Error whilst initializing the database: %v", err)
 				logger.Error("Check if you are using the right config file. You can use a --config directive to specify one.")
 				return nil

routers/install/install.go

@@ -198,7 +198,7 @@ func SubmitInstall(ctx *context.Context) {
 	setting.Database.SSLMode = form.SSLMode
 	setting.Database.Charset = form.Charset
 	setting.Database.Path = form.DbPath
-
+	setting.Database.LogSQL = !setting.IsProd
 	setting.PasswordHashAlgo = form.PasswordAlgorithm
 
 	if (setting.Database.Type == "sqlite3") &&
@@ -209,7 +209,7 @@ func SubmitInstall(ctx *context.Context) {
 	}
 
 	// Set test engine.
-	if err = db.InitInstallEngineWithMigration(ctx, migrations.Migrate); err != nil {
+	if err = db.InitEngineWithMigration(ctx, migrations.Migrate); err != nil {
 		if strings.Contains(err.Error(), `Unknown database type: sqlite3`) {
 			ctx.Data["Err_DbType"] = true
 			ctx.RenderWithErr(ctx.Tr("install.sqlite3_not_available", "https://docs.gitea.io/en-us/install-from-binary/"), tplInstall, &form)