Implement basic app.ini and path checks to doctor cmd (#10064)

* Add doctor check of app.ini paths

* Make /custom dir not mandatory

* Fix message and improve interface

* Update cmd/doctor.go

Co-Authored-By: John Olheiser <42128690+jolheiser@users.noreply.github.com>

* Apaise lint

* Isn't the linter a sweet? (1)

* Isn't the linter a sweet? (2)

* Isn't the linter a sweet?? (3)

* Restart CI

Co-authored-by: John Olheiser <42128690+jolheiser@users.noreply.github.com>
Co-authored-by: zeripath <art27@cantab.net>

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,27 @@ import (
 // CmdDoctor represents the available doctor sub-command.
 var CmdDoctor = cli.Command{
 	Name:        "doctor",
-	Usage:       "Diagnose the problems",
+	Usage:       "Diagnose problems",
-	Description: "A command to diagnose the 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,
 }
 
 type check struct {
 	title            string
 	f                func(ctx *cli.Context) ([]string, error)
+	abortIfFailed    bool
+	skipDatabaseInit bool
 }
 
 // checklist represents list for all checks
 var checklist = []check{
+	{
+		// 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",
 		f:     runDoctorLocationMoved,
@@ -42,22 +54,34 @@ var checklist = []check{
 }
 
 func runDoctor(ctx *cli.Context) error {
-	err := initDB()
+
-	fmt.Println("Using app.ini at", setting.CustomConf)
+	// Silence the console logger
-	if err != nil {
+	// TODO: Redirect all logs into `doctor.log` ignoring any other log configuration
+	log.DelNamedLogger("console")
+	log.DelNamedLogger(log.DEFAULT)
+
+	dbIsInit := false
+
+	for i, check := range checklist {
+		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
-	for i, check := range checklist {
+		}
 		fmt.Println("[", i+1, "]", check.title)
-		if messages, err := check.f(ctx); err != nil {
+		messages, err := check.f(ctx)
-			fmt.Println("Error:", err)
-		} else if len(messages) > 0 {
 		for _, message := range messages {
 			fmt.Println("-", message)
 		}
+		if err != nil {
+			fmt.Println("Error:", err)
+			if check.abortIfFailed {
+				return nil
+			}
 		} else {
 			fmt.Println("OK.")
 		}
@@ -74,6 +98,79 @@ 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, required, is_write bool) {
+		res = append(res, fmt.Sprintf("%-25s  '%s'", name+":", path))
+		if fi, err := os.Stat(path); err != nil {
+			if required {
+				res = append(res, fmt.Sprintf("    ERROR: %v", err))
+				fail = true
+			} else {
+				res = append(res, fmt.Sprintf("    NOTICE: not accessible (%v)", err))
+			}
+		} else if is_dir && !fi.IsDir() {
+			res = append(res, "    ERROR: not a directory")
+			fail = true
+		} else if !is_dir && !fi.Mode().IsRegular() {
+			res = append(res, "    ERROR: not a regular file")
+			fail = true
+		} else if is_write {
+			if err := runDoctorWritableDir(path); err != nil {
+				res = append(res, fmt.Sprintf("    ERROR: not writable: %v", err))
+				fail = true
+			}
+		}
+	}
+
+	// Note print paths inside quotes to make any leading/trailing spaces evident
+	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, 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

modules/options/dynamic.go

@@ -98,3 +98,8 @@ 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
+}

modules/options/static.go

@@ -112,3 +112,8 @@ 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
+}