From 060bdbee160cbd89683ecb0f2f29457ca99909e5 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 29 Jan 2020 17:18:07 -0300 Subject: [PATCH 1/9] Add doctor check of app.ini paths --- cmd/doctor.go | 118 +++++++++++++++++++++++++++++++++---- modules/options/dynamic.go | 4 ++ modules/options/static.go | 4 ++ 3 files changed, 113 insertions(+), 13 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index d81ead97c72c..6843257afee9 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -8,12 +8,15 @@ import ( "bufio" "errors" "fmt" + "io/ioutil" "os" "os/exec" "path/filepath" "regexp" "strings" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/options" "code.gitea.io/gitea/modules/setting" "github.com/urfave/cli" @@ -22,18 +25,24 @@ import ( // CmdDoctor represents the available doctor sub-command. var CmdDoctor = cli.Command{ Name: "doctor", - Usage: "Diagnose the problems", - Description: "A command to diagnose the problems of current gitea instance according the given configuration.", + Usage: "Diagnose and repair problems", + Description: "A command to diagnose and repair problems of current gitea instance according the given configuration.", Action: runDoctor, } type check struct { title string f func(ctx *cli.Context) ([]string, error) + abort bool } // checklist represents list for all checks var checklist = []check{ + { + title: "Check paths and basic configuration", + f: runDoctorPathInfo, + abort: true, + }, { title: "Check if OpenSSH authorized_keys file id correct", f: runDoctorLocationMoved, @@ -42,21 +51,30 @@ var checklist = []check{ } func runDoctor(ctx *cli.Context) error { - err := initDB() - fmt.Println("Using app.ini at", setting.CustomConf) - if 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 - } + + // Silence the console logger + // TODO: Redirect all logs into `doctor.log` ignoring any other log configuration + log.DelNamedLogger("console") + log.DelNamedLogger(log.DEFAULT) for i, check := range checklist { + if i == 1 { + // Only open database after the most basic configuration check + if err := initDB(); 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 + } + } fmt.Println("[", i+1, "]", check.title) - if messages, err := check.f(ctx); err != nil { + messages, err := check.f(ctx) + for _, message := range messages { + fmt.Println("-", message) + } + if err != nil { fmt.Println("Error:", err) - } else if len(messages) > 0 { - for _, message := range messages { - fmt.Println("-", message) + if check.abort { + return nil } } else { fmt.Println("OK.") @@ -74,6 +92,80 @@ func exePath() (string, error) { return filepath.Abs(file) } +func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { + + res := make([]string, 0, 10) + + if fi, err := os.Stat(setting.CustomConf); err != nil || !fi.Mode().IsRegular() { + res = append(res, fmt.Sprintf("Failed to find configuration file at '%s'.", setting.CustomConf)) + res = append(res, fmt.Sprintf("If you've never ran Gitea yet, this is normal and '%s' will be created for you on first run.", setting.CustomConf)) + res = append(res, "Otherwise check that you are running this command from the correct path and/or provide a `--config` parameter.") + return res, fmt.Errorf("Can't proceed without a configuration file") + } + + setting.NewContext() + + fail := false + + check := func(name, path string, is_dir, is_write bool) { + res = append(res, fmt.Sprintf("%-25s '%s'", name+":", path)) + if fi, err := os.Stat(path); err != nil { + res = append(res, fmt.Sprintf(" ERROR: %v", err)) + fail = true + return + } else if is_dir && !fi.IsDir() { + res = append(res, " ERROR: not a directory") + fail = true + return + } else if !is_dir && !fi.Mode().IsRegular() { + res = append(res, " ERROR: not a regular file") + fail = true + return + } else if is_write { + if err := runDoctorWritableDir(path); err != nil { + res = append(res, fmt.Sprintf(" ERROR: not writable: %v", err)) + fail = true + return + } + } + return + } + + // Note print paths inside quotes to make any leading/trailing spaces evident + check("Configuration File Path", setting.CustomConf, false, false) + check("Repository Root Path", setting.RepoRootPath, true, true) + check("Data Root Path", setting.AppDataPath, true, true) + check("Custom File Root Path", setting.CustomPath, true, false) + check("Work directory", setting.AppWorkPath, true, false) + check("Log Root Path", setting.LogRootPath, true, true) + + if options.IsDynamic() { + // Do not check/report on StaticRootPath if data is embedded in Gitea (-tags bindata) + check("Static File Root Path", setting.StaticRootPath, true, false) + } + + if fail { + return res, fmt.Errorf("Please check your configuration file and try again.") + } + + return res, nil +} + +func runDoctorWritableDir(path string) error { + // There's no platform-independent way of checking if a directory is writable + // https://stackoverflow.com/questions/20026320/how-to-tell-if-folder-exists-and-is-writable + + tmpFile, err := ioutil.TempFile(path, "doctors-order") + if err != nil { + return err + } + if err := os.Remove(tmpFile.Name()); err != nil { + fmt.Printf("Warning: can't remove temporary file: '%s'\n", tmpFile.Name()) + } + tmpFile.Close() + return nil +} + func runDoctorLocationMoved(ctx *cli.Context) ([]string, error) { if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile { return nil, nil diff --git a/modules/options/dynamic.go b/modules/options/dynamic.go index 7de7cfbd61cc..d0b59b56ebd6 100644 --- a/modules/options/dynamic.go +++ b/modules/options/dynamic.go @@ -98,3 +98,7 @@ func fileFromDir(name string) ([]byte, error) { return []byte{}, fmt.Errorf("Asset file does not exist: %s", name) } + +func IsDynamic() bool { + return true +} diff --git a/modules/options/static.go b/modules/options/static.go index 1cf841ebb150..554921dfcf2b 100644 --- a/modules/options/static.go +++ b/modules/options/static.go @@ -112,3 +112,7 @@ func fileFromDir(name string) ([]byte, error) { return ioutil.ReadAll(f) } + +func IsDynamic() bool { + return false +} From 19f67bd227e0b4ea4ac0f9ae4a5b601dd2514327 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 29 Jan 2020 17:39:05 -0300 Subject: [PATCH 2/9] Make /custom dir not mandatory --- cmd/doctor.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 6843257afee9..0ebc69056f9f 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -107,11 +107,15 @@ func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { fail := false - check := func(name, path string, is_dir, is_write bool) { + check := func(name, path string, is_dir, required, is_write bool) { res = append(res, fmt.Sprintf("%-25s '%s'", name+":", path)) if fi, err := os.Stat(path); err != nil { - res = append(res, fmt.Sprintf(" ERROR: %v", err)) - fail = true + if required { + res = append(res, fmt.Sprintf(" ERROR: %v", err)) + fail = true + } else { + res = append(res, fmt.Sprintf(" NOTICE: not accessible (%v)", err)) + } return } else if is_dir && !fi.IsDir() { res = append(res, " ERROR: not a directory") @@ -132,16 +136,16 @@ func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { } // Note print paths inside quotes to make any leading/trailing spaces evident - check("Configuration File Path", setting.CustomConf, false, false) - check("Repository Root Path", setting.RepoRootPath, true, true) - check("Data Root Path", setting.AppDataPath, true, true) - check("Custom File Root Path", setting.CustomPath, true, false) - check("Work directory", setting.AppWorkPath, true, false) - check("Log Root Path", setting.LogRootPath, true, true) + check("Configuration File Path", setting.CustomConf, false, true, false) + check("Repository Root Path", setting.RepoRootPath, true, true, true) + check("Data Root Path", setting.AppDataPath, true, true, true) + check("Custom File Root Path", setting.CustomPath, true, false, false) + check("Work directory", setting.AppWorkPath, true, true, false) + check("Log Root Path", setting.LogRootPath, true, true, true) if options.IsDynamic() { // Do not check/report on StaticRootPath if data is embedded in Gitea (-tags bindata) - check("Static File Root Path", setting.StaticRootPath, true, false) + check("Static File Root Path", setting.StaticRootPath, true, true, false) } if fail { From 63fe37320d161387e3eb8d1e9232b7fba52ced82 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 29 Jan 2020 17:59:01 -0300 Subject: [PATCH 3/9] Fix message and improve interface --- cmd/doctor.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 0ebc69056f9f..6e1b0b75d0ec 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -25,23 +25,26 @@ import ( // CmdDoctor represents the available doctor sub-command. var CmdDoctor = cli.Command{ Name: "doctor", - Usage: "Diagnose and repair problems", - Description: "A command to diagnose and repair problems of current gitea instance according the given configuration.", + Usage: "Diagnose problems", + Description: "A command to diagnose problems of current gitea instance according the given configuration.", Action: runDoctor, } type check struct { - title string - f func(ctx *cli.Context) ([]string, error) - abort bool + title string + f func(ctx *cli.Context) ([]string, error) + abortIfFailed bool + skipDatabaseInit bool } // checklist represents list for all checks var checklist = []check{ { - title: "Check paths and basic configuration", - f: runDoctorPathInfo, - abort: true, + // NOTE: this check should be the first in the list + title: "Check paths and basic configuration", + f: runDoctorPathInfo, + abortIfFailed: true, + skipDatabaseInit: true, }, { title: "Check if OpenSSH authorized_keys file id correct", @@ -57,14 +60,17 @@ func runDoctor(ctx *cli.Context) error { log.DelNamedLogger("console") log.DelNamedLogger(log.DEFAULT) + dbIsInit := false + for i, check := range checklist { - if i == 1 { + if !dbIsInit && !check.skipDatabaseInit { // Only open database after the most basic configuration check if err := initDB(); 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 } + dbIsInit = true } fmt.Println("[", i+1, "]", check.title) messages, err := check.f(ctx) @@ -73,7 +79,7 @@ func runDoctor(ctx *cli.Context) error { } if err != nil { fmt.Println("Error:", err) - if check.abort { + if check.abortIfFailed { return nil } } else { From 292909ccbaa805912971706b8fae278f0bc9731e Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Wed, 29 Jan 2020 18:34:52 -0300 Subject: [PATCH 4/9] Update cmd/doctor.go Co-Authored-By: John Olheiser <42128690+jolheiser@users.noreply.github.com> --- cmd/doctor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 6e1b0b75d0ec..1782046211d2 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -26,7 +26,7 @@ import ( var CmdDoctor = cli.Command{ Name: "doctor", Usage: "Diagnose problems", - Description: "A command to diagnose problems of current gitea instance according the given configuration.", + Description: "A command to diagnose problems with the current Gitea instance according to the given configuration.", Action: runDoctor, } From 3509a7b9dc7150b2050f8bc9ecb4a7ec29451d27 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 29 Jan 2020 21:09:53 -0300 Subject: [PATCH 5/9] Apaise lint --- cmd/doctor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 1782046211d2..9eeb9c68ab31 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -106,7 +106,7 @@ func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { res = append(res, fmt.Sprintf("Failed to find configuration file at '%s'.", setting.CustomConf)) res = append(res, fmt.Sprintf("If you've never ran Gitea yet, this is normal and '%s' will be created for you on first run.", setting.CustomConf)) res = append(res, "Otherwise check that you are running this command from the correct path and/or provide a `--config` parameter.") - return res, fmt.Errorf("Can't proceed without a configuration file") + return res, fmt.Errorf("can't proceed without a configuration file") } setting.NewContext() @@ -155,7 +155,7 @@ func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { } if fail { - return res, fmt.Errorf("Please check your configuration file and try again.") + return res, fmt.Errorf("please check your configuration file and try again") } return res, nil From 3144375a1c0a3110ca095c1b1a406b6c42e59bc3 Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Wed, 29 Jan 2020 21:29:52 -0300 Subject: [PATCH 6/9] Isn't the linter a sweet? (1) --- modules/options/static.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/options/static.go b/modules/options/static.go index 554921dfcf2b..f8811e43ac9e 100644 --- a/modules/options/static.go +++ b/modules/options/static.go @@ -113,6 +113,7 @@ func fileFromDir(name string) ([]byte, error) { return ioutil.ReadAll(f) } +// IsDynamic will return false when using embedded data (-tags bindata) func IsDynamic() bool { return false } From 61298a617a6f5106e086f937ad3fe38c820c9fd1 Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Wed, 29 Jan 2020 21:30:03 -0300 Subject: [PATCH 7/9] Isn't the linter a sweet? (2) --- modules/options/dynamic.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/options/dynamic.go b/modules/options/dynamic.go index d0b59b56ebd6..20dde11dc3e9 100644 --- a/modules/options/dynamic.go +++ b/modules/options/dynamic.go @@ -99,6 +99,7 @@ func fileFromDir(name string) ([]byte, error) { return []byte{}, fmt.Errorf("Asset file does not exist: %s", name) } +// IsDynamic will return false when using embedded data (-tags bindata) func IsDynamic() bool { return true } From f7bf7f75464a08abda2e2d7bb89aa892c457b564 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 29 Jan 2020 21:42:39 -0300 Subject: [PATCH 8/9] Isn't the linter a sweet?? (3) --- cmd/doctor.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 9eeb9c68ab31..a72945779907 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -122,23 +122,18 @@ func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { } else { res = append(res, fmt.Sprintf(" NOTICE: not accessible (%v)", err)) } - return } else if is_dir && !fi.IsDir() { res = append(res, " ERROR: not a directory") fail = true - return } else if !is_dir && !fi.Mode().IsRegular() { res = append(res, " ERROR: not a regular file") fail = true - return } else if is_write { if err := runDoctorWritableDir(path); err != nil { res = append(res, fmt.Sprintf(" ERROR: not writable: %v", err)) fail = true - return } } - return } // Note print paths inside quotes to make any leading/trailing spaces evident From 9b0d6fe4b28bca56834d1efc6d095993ede321d9 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 29 Jan 2020 22:09:39 -0300 Subject: [PATCH 9/9] Restart CI