Fix bug on admin subcommand (#17533)

* Fix bug on admin subcommand

* Add signals for all initDB

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

diff --git a/cmd/admin.go b/cmd/admin.go
index 099083ae9100b75acfccbbc7932c5cc6944bc7d1..64106f506019c6cf9ff03ccf7b7e96e7097f2529 100644 (file)
--- a/cmd/admin.go
+++ b/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
        }
 

diff --git a/cmd/admin_auth_ldap.go b/cmd/admin_auth_ldap.go
index 517904957f1da1a8affeb72007c8dc490f92264b..950d515e391652a1640d4b5b551877e9b536fe39 100644 (file)
--- a/cmd/admin_auth_ldap.go
+++ b/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
        }
 

diff --git a/cmd/admin_auth_ldap_test.go b/cmd/admin_auth_ldap_test.go
index db1ba13bcd5b35d0cd42d439a4664d1f72856637..15880639d6e5b5adbc8a550e8971f7a2d1403351 100644 (file)
--- a/cmd/admin_auth_ldap_test.go
+++ b/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 {

diff --git a/cmd/cmd.go b/cmd/cmd.go
index ea025cd98e0f0da0b86b23688f6aeb874d8682f4..b89dd5d8b8acded230b571bd07e1ce518cc9e0af 100644 (file)
--- a/cmd/cmd.go
+++ b/cmd/cmd.go
@@ -56,16 +56,15 @@ func confirm() (bool, error) {
        }
 }
 
-func initDB() error {
-       return initDBDisableConsole(false)
+func initDB(ctx context.Context) error {
+       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

diff --git a/cmd/convert.go b/cmd/convert.go
index 20e4045c3497f2e1ae6ae2fc1fb8cfe433ab6820..0b2e240b32f433019b8322b51666a5ca977a5176 100644 (file)
--- a/cmd/convert.go
+++ b/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
        }
 

diff --git a/cmd/doctor.go b/cmd/doctor.go
index 19b26f09c6617333b964276efa98199b1c82565c..498b859e41842dbf80658b1f5529b8dd578b034c 100644 (file)
--- a/cmd/doctor.go
+++ b/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)
 }

diff --git a/cmd/dump.go b/cmd/dump.go
index 70ed6c2b55c7f0cc469202b6b547de8352fc5388..efb9397208594a1198119ceb660b3dd2e6f5c26a 100644 (file)
--- a/cmd/dump.go
+++ b/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
        }

diff --git a/cmd/dump_repo.go b/cmd/dump_repo.go
index 3ea82f6d1a0cb16be305f4ab465a3ac617391819..6274b4d86543962517ccebc328cf9f7a2a6d9d99 100644 (file)
--- a/cmd/dump_repo.go
+++ b/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
        }
 

diff --git a/cmd/migrate.go b/cmd/migrate.go
index 7bb87d84003108efa3a5182c270af3d8c0818d14..054772d9ec4cc722276ef5a9fd3d0651fb76ebd3 100644 (file)
--- a/cmd/migrate.go
+++ b/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
        }
 

diff --git a/cmd/migrate_storage.go b/cmd/migrate_storage.go
index 7c7629490e13c047db96db97ad60d977144287b1..1829ad00bada6e3d61c6c6878cc8a25b25ec875f 100644 (file)
--- a/cmd/migrate_storage.go
+++ b/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
        }
 

diff --git a/models/db/engine.go b/models/db/engine.go
index 411e39a13bd8966621149154e0d96dbc1890645d..d5f2470a7af674f6ac1426dcc41d1cde9cc02f84 100755 (executable)
--- a/models/db/engine.go
+++ b/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)
        }

diff --git a/modules/doctor/doctor.go b/modules/doctor/doctor.go
index 9a05e828f4beb689a51c77d30e2ae856eff9fb27..6451788f9df4b96622c8b14ea8d16fe4752bddef 100644 (file)
--- a/modules/doctor/doctor.go
+++ b/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

diff --git a/routers/install/install.go b/routers/install/install.go
index e38b51fb7e7a92f02989244d2c1050bc4b9ddd0a..837467056dc139d34c6fde84b593d89cc6561187 100644 (file)
--- a/routers/install/install.go
+++ b/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)