From 744774a5dfe2089ad3b96b0f97ccbf00f034a9d5 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 3 Apr 2020 21:29:22 +0100 Subject: [PATCH 01/12] Add recalculate_merge_bases check to doctor Add recalculate merge bases check/fixer to doctor Add functionality for doctor only run certain checks Signed-off-by: Andrew Thornton --- cmd/doctor.go | 184 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 180 insertions(+), 4 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index a72945779907..d9cded246711 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -14,10 +14,14 @@ import ( "path/filepath" "regexp" "strings" + "text/tabwriter" + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/options" "code.gitea.io/gitea/modules/setting" + "xorm.io/builder" "github.com/urfave/cli" ) @@ -28,10 +32,30 @@ var CmdDoctor = cli.Command{ Usage: "Diagnose problems", Description: "A command to diagnose problems with the current Gitea instance according to the given configuration.", Action: runDoctor, + Flags: []cli.Flag{ + cli.BoolFlag{ + Name: "list", + Usage: "List the available checks", + }, + cli.BoolFlag{ + Name: "default", + Usage: "Run the default checks (if neither run or all is set this is the default behaviour)", + }, + cli.StringSliceFlag{ + Name: "run", + Usage: "Run the provided checks - if default is set the default checks will also run", + }, + cli.BoolFlag{ + Name: "all", + Usage: "Run all the available checks", + }, + }, } type check struct { title string + name string + isDefault bool f func(ctx *cli.Context) ([]string, error) abortIfFailed bool skipDatabaseInit bool @@ -42,13 +66,23 @@ var checklist = []check{ { // NOTE: this check should be the first in the list title: "Check paths and basic configuration", + name: "paths", + isDefault: true, f: runDoctorPathInfo, abortIfFailed: true, skipDatabaseInit: true, }, { - title: "Check if OpenSSH authorized_keys file id correct", - f: runDoctorLocationMoved, + title: "Check if OpenSSH authorized_keys file id correct", + name: "authorized_keys", + isDefault: true, + f: runDoctorLocationMoved, + }, + { + title: "Recalculate merge bases", + name: "recalculate_merge_bases", + isDefault: false, + f: runDoctorPRMergeBase, }, // more checks please append here } @@ -60,9 +94,55 @@ func runDoctor(ctx *cli.Context) error { log.DelNamedLogger("console") log.DelNamedLogger(log.DEFAULT) + if ctx.IsSet("list") { + w := tabwriter.NewWriter(os.Stdout, 0, 8, 0, '\t', 0) + _, _ = w.Write([]byte("Default\tName\tTitle\n")) + for _, check := range checklist { + if check.isDefault { + _, _ = w.Write([]byte{'*'}) + } + _, _ = w.Write([]byte{'\t'}) + _, _ = w.Write([]byte(check.name)) + _, _ = w.Write([]byte{'\t'}) + _, _ = w.Write([]byte(check.title)) + _, _ = w.Write([]byte{'\n'}) + } + return w.Flush() + } + + var checks []check + if ctx.IsSet("all") && ctx.Bool("all") { + checks = checklist + } else if ctx.IsSet("run") { + addDefault := ctx.IsSet("default") && ctx.Bool("default") + names := ctx.StringSlice("run") + for i, name := range names { + names[i] = strings.ToLower(strings.TrimSpace(name)) + } + + for _, check := range checklist { + if addDefault && check.isDefault { + checks = append(checks, check) + continue + } + for _, name := range names { + if name == check.name { + checks = append(checks, check) + break + } + } + } + } else { + for _, check := range checklist { + if check.isDefault { + checks = append(checks, check) + } + } + } + dbIsInit := false - for i, check := range checklist { + for i, check := range checks { if !dbIsInit && !check.skipDatabaseInit { // Only open database after the most basic configuration check if err := initDB(); err != nil { @@ -200,7 +280,7 @@ func runDoctorLocationMoved(ctx *cli.Context) ([]string, error) { // command="/home/user/gitea --config='/home/user/etc/app.ini' serv key-999",option-1,option-2,option-n ssh-rsa public-key-value key-name res := exp.FindStringSubmatch(firstline) if res == nil { - return nil, errors.New("Unknow authorized_keys format") + return nil, errors.New("Unknown authorized_keys format") } giteaPath := res[1] // => /home/user/gitea @@ -225,3 +305,99 @@ func runDoctorLocationMoved(ctx *cli.Context) ([]string, error) { return nil, nil } + +func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { + opts := &models.SearchRepoOptions{ + ListOptions: models.ListOptions{ + Page: 1, + PageSize: 50, + }, + } + results := make([]string, 0, 10) + numRepos := 0 + numPRs := 0 + numPRsUpdated := 0 + for { + repos, _, err := models.SearchRepositoryByCondition(opts, builder.NewCond(), false) + if err != nil { + return nil, err + } + + for _, repo := range repos { + prOpts := &models.PullRequestsOptions{ + ListOptions: models.ListOptions{ + Page: 1, + PageSize: 50, + }, + } + for { + prs, _, err := models.PullRequests(repo.ID, prOpts) + if err != nil { + return nil, err + } + for _, pr := range prs { + pr.BaseRepo = repo + repoPath := repo.RepoPath() + + oldMergeBase := pr.MergeBase + + if !pr.HasMerged { + var err error + pr.MergeBase, err = git.NewCommand("merge-base", "--", pr.BaseBranch, pr.GetGitRefName()).RunInDir(repoPath) + if err != nil { + var err2 error + pr.MergeBase, err2 = git.NewCommand("rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath) + if err2 != nil { + results = append(results, fmt.Sprintf("WARN: Unable to get merge base for PR ID %d, #%d in %s/%s\n Error: %v\n Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2)) + log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2) + continue + } + } + } else { + parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath) + if err != nil { + results = append(results, fmt.Sprintf("WARN: Unable to get parents for merged PR ID %d, #%d in %s/%s\n Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err)) + log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) + continue + } + parents := strings.Split(strings.TrimSpace(parentsString), " ") + if len(parents) < 2 { + continue + } + + args := append([]string{"merge-base", "--"}, parents[1:]...) + args = append(args, pr.GetGitRefName()) + + pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath) + if err != nil { + results = append(results, fmt.Sprintf("WARN: Unable to get merge base for merged PR ID %d, #%d in %s/%s\n Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err)) + log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) + continue + } + } + pr.MergeBase = strings.TrimSpace(pr.MergeBase) + if pr.MergeBase != oldMergeBase { + if err := pr.UpdateCols("merge_base"); err != nil { + return results, err + } + numPRsUpdated++ + } + } + numPRs += len(prs) + if len(prs) < 50 { + break + } + } + prOpts.Page++ + } + + numRepos += len(repos) + if len(repos) < 50 { + break + } + opts.Page++ + } + results = append(results, fmt.Sprintf("%d PRs updated of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) + + return results, nil +} From 67378c557657ecb2d748991c704ee37202a928c5 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 3 Apr 2020 23:00:53 +0100 Subject: [PATCH 02/12] Add fix command Signed-off-by: Andrew Thornton --- cmd/doctor.go | 144 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 102 insertions(+), 42 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index d9cded246711..7fd998fe1ebc 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -49,6 +49,10 @@ var CmdDoctor = cli.Command{ Name: "all", Usage: "Run all the available checks", }, + cli.BoolFlag{ + Name: "fix", + Usage: "Automatically fix if we can", + }, }, } @@ -111,10 +115,10 @@ func runDoctor(ctx *cli.Context) error { } var checks []check - if ctx.IsSet("all") && ctx.Bool("all") { + if ctx.Bool("all") { checks = checklist } else if ctx.IsSet("run") { - addDefault := ctx.IsSet("default") && ctx.Bool("default") + addDefault := ctx.Bool("default") names := ctx.StringSlice("run") for i, name := range names { names[i] = strings.ToLower(strings.TrimSpace(name)) @@ -195,16 +199,31 @@ func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { 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 { + fi, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) && ctx.Bool("fix") && is_dir { + if err := os.MkdirAll(path, 0777); err != nil { + res = append(res, fmt.Sprintf(" ERROR: %v", err)) + fail = true + return + } + fi, err = os.Stat(path) + } + } + if 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)) + return } - } else if is_dir && !fi.IsDir() { + res = append(res, fmt.Sprintf(" NOTICE: not accessible (%v)", err)) + return + } + + 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 @@ -251,6 +270,10 @@ func runDoctorWritableDir(path string) error { return nil } +const tplCommentPrefix = `# gitea public key` + +var giteaExpected = regexp.MustCompile(`^[ \t]*(?:command=")([^ ]+) --config='([^']+)' serv key-([^"]+)",(?:[^ ]+) ssh-rsa ([^ ]+) ([^ ]+)[ \t]*$`) + func runDoctorLocationMoved(ctx *cli.Context) ([]string, error) { if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile { return nil, nil @@ -263,47 +286,76 @@ func runDoctorLocationMoved(ctx *cli.Context) ([]string, error) { } defer f.Close() - var firstline string + runFix := 0 + results := make([]string, 0, 10) + scanner := bufio.NewScanner(f) for scanner.Scan() { - firstline = strings.TrimSpace(scanner.Text()) - if len(firstline) == 0 || firstline[0] == '#' { - continue + line := scanner.Text() + if strings.HasPrefix(line, tplCommentPrefix) { + res, err := checkGiteaLine(strings.TrimSpace(scanner.Text())) + if err != nil { + if ctx.Bool("fix") { + results = append(results, err.Error()) + runFix++ + } else { + return nil, err + } + } + if len(res) > 0 { + if ctx.Bool("fix") { + runFix++ + } else { + results = append(results, res) + } + } } - break } - // command="/Volumes/data/Projects/gitea/gitea/gitea --config - if len(firstline) > 0 { - exp := regexp.MustCompile(`^[ \t]*(?:command=")([^ ]+) --config='([^']+)' serv key-([^"]+)",(?:[^ ]+) ssh-rsa ([^ ]+) ([^ ]+)[ \t]*$`) - - // command="/home/user/gitea --config='/home/user/etc/app.ini' serv key-999",option-1,option-2,option-n ssh-rsa public-key-value key-name - res := exp.FindStringSubmatch(firstline) - if res == nil { - return nil, errors.New("Unknown authorized_keys format") - } - - giteaPath := res[1] // => /home/user/gitea - iniPath := res[2] // => /home/user/etc/app.ini - - p, err := exePath() - if err != nil { - return nil, err - } - p, err = filepath.Abs(p) + if runFix > 0 { + err := models.RewriteAllPublicKeys() if err != nil { return nil, err } + results = append(results, fmt.Sprintf("%d keys needed to be rewritten", runFix)) + } - if len(giteaPath) > 0 && giteaPath != p { - return []string{fmt.Sprintf("Gitea exe path wants %s but %s on %s", p, giteaPath, fPath)}, nil - } - if len(iniPath) > 0 && iniPath != setting.CustomConf { - return []string{fmt.Sprintf("Gitea config path wants %s but %s on %s", setting.CustomConf, iniPath, fPath)}, nil - } + return results, nil +} + +func checkGiteaLine(line string) (string, error) { + if len(line) == 0 { + return "", nil + } + + fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys") + + // command="/home/user/gitea --config='/home/user/etc/app.ini' serv key-999",option-1,option-2,option-n ssh-rsa public-key-value key-name + res := giteaExpected.FindStringSubmatch(line) + if res == nil { + return "", errors.New("Unknown authorized_keys format") + } + + giteaPath := res[1] // => /home/user/gitea + iniPath := res[2] // => /home/user/etc/app.ini + + p, err := exePath() + if err != nil { + return "", err + } + p, err = filepath.Abs(p) + if err != nil { + return "", err } - return nil, nil + if len(giteaPath) > 0 && giteaPath != p { + return fmt.Sprintf("Gitea exe path wants %s but %s on %s", p, giteaPath, fPath), nil + } + if len(iniPath) > 0 && iniPath != setting.CustomConf { + return fmt.Sprintf("Gitea config path wants %s but %s on %s", setting.CustomConf, iniPath, fPath), nil + } + + return "", nil } func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { @@ -348,7 +400,7 @@ func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { var err2 error pr.MergeBase, err2 = git.NewCommand("rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath) if err2 != nil { - results = append(results, fmt.Sprintf("WARN: Unable to get merge base for PR ID %d, #%d in %s/%s\n Error: %v\n Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2)) + results = append(results, fmt.Sprintf("WARN: Unable to get merge base for PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2) continue } @@ -356,7 +408,7 @@ func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { } else { parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath) if err != nil { - results = append(results, fmt.Sprintf("WARN: Unable to get parents for merged PR ID %d, #%d in %s/%s\n Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err)) + results = append(results, fmt.Sprintf("WARN: Unable to get parents for merged PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) continue } @@ -370,15 +422,19 @@ func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath) if err != nil { - results = append(results, fmt.Sprintf("WARN: Unable to get merge base for merged PR ID %d, #%d in %s/%s\n Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err)) + results = append(results, fmt.Sprintf("WARN: Unable to get merge base for merged PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) continue } } pr.MergeBase = strings.TrimSpace(pr.MergeBase) if pr.MergeBase != oldMergeBase { - if err := pr.UpdateCols("merge_base"); err != nil { - return results, err + if ctx.Bool("fix") { + if err := pr.UpdateCols("merge_base"); err != nil { + return results, err + } + } else { + results = append(results, fmt.Sprintf("#%d onto %s in %s/%s: MergeBase should be %s but is %s", pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, oldMergeBase, pr.MergeBase)) } numPRsUpdated++ } @@ -397,7 +453,11 @@ func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { } opts.Page++ } - results = append(results, fmt.Sprintf("%d PRs updated of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) + if ctx.Bool("fix") { + results = append(results, fmt.Sprintf("%d PRs with incorrect mergebases of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) + } else { + results = append(results, fmt.Sprintf("%d PRs updated of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) + } return results, nil } From 26caeb7ccd9631c6f217631f93f1028b19e09246 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 4 Apr 2020 15:53:14 +0100 Subject: [PATCH 03/12] Apply suggestions from code review Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com> --- cmd/doctor.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 7fd998fe1ebc..308624af6ef2 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -39,11 +39,11 @@ var CmdDoctor = cli.Command{ }, cli.BoolFlag{ Name: "default", - Usage: "Run the default checks (if neither run or all is set this is the default behaviour)", + Usage: "Run the default checks (if neither --run or --all is set, this is the default behaviour)", }, cli.StringSliceFlag{ Name: "run", - Usage: "Run the provided checks - if default is set the default checks will also run", + Usage: "Run the provided checks - (if --default is set, the default checks will also run)", }, cli.BoolFlag{ Name: "all", @@ -51,7 +51,7 @@ var CmdDoctor = cli.Command{ }, cli.BoolFlag{ Name: "fix", - Usage: "Automatically fix if we can", + Usage: "Automatically fix what we can", }, }, } @@ -77,7 +77,7 @@ var checklist = []check{ skipDatabaseInit: true, }, { - title: "Check if OpenSSH authorized_keys file id correct", + title: "Check if OpenSSH authorized_keys file is correct", name: "authorized_keys", isDefault: true, f: runDoctorLocationMoved, From 589c6918360be56dfb6770f1d59014090ce02f9d Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Apr 2020 10:11:27 +0100 Subject: [PATCH 04/12] Add doctor step for checking current version Signed-off-by: Andrew Thornton --- cmd/doctor.go | 19 ++++++++++++++ models/migrations/migrations.go | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/cmd/doctor.go b/cmd/doctor.go index 308624af6ef2..9ce597108012 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -6,6 +6,7 @@ package cmd import ( "bufio" + "context" "errors" "fmt" "io/ioutil" @@ -17,6 +18,7 @@ import ( "text/tabwriter" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/migrations" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/options" @@ -82,6 +84,13 @@ var checklist = []check{ isDefault: true, f: runDoctorLocationMoved, }, + { + title: "Check Database Version", + name: "check-db", + isDefault: true, + f: runDoctorCheckDBVersion, + abortIfFailed: true, + }, { title: "Recalculate merge bases", name: "recalculate_merge_bases", @@ -358,6 +367,16 @@ func checkGiteaLine(line string) (string, error) { return "", nil } +func runDoctorCheckDBVersion(ctx *cli.Context) ([]string, error) { + if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { + if ctx.Bool("fix") { + return nil, models.NewEngine(context.Background(), migrations.Migrate) + } + return nil, err + } + return nil, nil +} + func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { opts := &models.SearchRepoOptions{ ListOptions: models.ListOptions{ diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 847cd75d521f..946d82289f9f 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -206,6 +206,52 @@ var migrations = []Migration{ NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), } +// GetCurrentDBVersion returns the current db version +func GetCurrentDBVersion(x *xorm.Engine) (int64, error) { + if err := x.Sync(new(Version)); err != nil { + return -1, fmt.Errorf("sync: %v", err) + } + + currentVersion := &Version{ID: 1} + has, err := x.Get(currentVersion) + if err != nil { + return -1, fmt.Errorf("get: %v", err) + } + if !has { + return -1, nil + } + return currentVersion.Version, nil +} + +// ExpectedVersion returns the expected db version +func ExpectedVersion() int64 { + return int64(minDBVersion + len(migrations)) +} + +// EnsureUpToDate will check if the db is at the correct version +func EnsureUpToDate(x *xorm.Engine) error { + currentDB, err := GetCurrentDBVersion(x) + if err != nil { + return err + } + + if currentDB < 0 { + return fmt.Errorf("Database has not been initialised") + } + + if minDBVersion > currentDB { + return fmt.Errorf("DB version %d (<= %d) is too old for auto-migration. Upgrade to Gitea 1.6.4 first then upgrade to this version", currentDB, minDBVersion) + } + + expected := ExpectedVersion() + + if currentDB != expected { + return fmt.Errorf("Current database version %d is not equal to the expected version %d. Please run migrate", currentDB, expected) + } + + return nil +} + // Migrate database to current version func Migrate(x *xorm.Engine) error { if err := x.Sync(new(Version)); err != nil { From ca34873ac6cb04a0f672770d847b52458a2d5902 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Apr 2020 11:28:01 +0100 Subject: [PATCH 05/12] Make gitea doctor authorized_keys check work again Signed-off-by: Andrew Thornton --- cmd/doctor.go | 103 +++++++++++++--------------------------------- models/ssh_key.go | 22 ++++++++-- 2 files changed, 46 insertions(+), 79 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 9ce597108012..f8734e5b7c3b 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -6,14 +6,12 @@ package cmd import ( "bufio" + "bytes" "context" - "errors" "fmt" "io/ioutil" "os" - "os/exec" "path/filepath" - "regexp" "strings" "text/tabwriter" @@ -82,7 +80,7 @@ var checklist = []check{ title: "Check if OpenSSH authorized_keys file is correct", name: "authorized_keys", isDefault: true, - f: runDoctorLocationMoved, + f: runDoctorAuthorizedKeys, }, { title: "Check Database Version", @@ -183,14 +181,6 @@ func runDoctor(ctx *cli.Context) error { return nil } -func exePath() (string, error) { - file, err := exec.LookPath(os.Args[0]) - if err != nil { - return "", err - } - return filepath.Abs(file) -} - func runDoctorPathInfo(ctx *cli.Context) ([]string, error) { res := make([]string, 0, 10) @@ -281,9 +271,7 @@ func runDoctorWritableDir(path string) error { const tplCommentPrefix = `# gitea public key` -var giteaExpected = regexp.MustCompile(`^[ \t]*(?:command=")([^ ]+) --config='([^']+)' serv key-([^"]+)",(?:[^ ]+) ssh-rsa ([^ ]+) ([^ ]+)[ \t]*$`) - -func runDoctorLocationMoved(ctx *cli.Context) ([]string, error) { +func runDoctorAuthorizedKeys(ctx *cli.Context) ([]string, error) { if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile { return nil, nil } @@ -291,80 +279,45 @@ func runDoctorLocationMoved(ctx *cli.Context) ([]string, error) { fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys") f, err := os.Open(fPath) if err != nil { + if ctx.Bool("fix") { + return []string{fmt.Sprintf("Error whilst opening authorized_keys: %v. Attempting regeneration", err)}, models.RewriteAllPublicKeys() + } return nil, err } defer f.Close() - runFix := 0 - results := make([]string, 0, 10) + linesInAuthorizedKeys := map[string]bool{} scanner := bufio.NewScanner(f) for scanner.Scan() { line := scanner.Text() if strings.HasPrefix(line, tplCommentPrefix) { - res, err := checkGiteaLine(strings.TrimSpace(scanner.Text())) - if err != nil { - if ctx.Bool("fix") { - results = append(results, err.Error()) - runFix++ - } else { - return nil, err - } - } - if len(res) > 0 { - if ctx.Bool("fix") { - runFix++ - } else { - results = append(results, res) - } - } + continue } + linesInAuthorizedKeys[line] = true } + f.Close() - if runFix > 0 { - err := models.RewriteAllPublicKeys() - if err != nil { - return nil, err - } - results = append(results, fmt.Sprintf("%d keys needed to be rewritten", runFix)) - } - - return results, nil -} - -func checkGiteaLine(line string) (string, error) { - if len(line) == 0 { - return "", nil - } - - fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys") - - // command="/home/user/gitea --config='/home/user/etc/app.ini' serv key-999",option-1,option-2,option-n ssh-rsa public-key-value key-name - res := giteaExpected.FindStringSubmatch(line) - if res == nil { - return "", errors.New("Unknown authorized_keys format") - } - - giteaPath := res[1] // => /home/user/gitea - iniPath := res[2] // => /home/user/etc/app.ini - - p, err := exePath() - if err != nil { - return "", err - } - p, err = filepath.Abs(p) - if err != nil { - return "", err - } - - if len(giteaPath) > 0 && giteaPath != p { - return fmt.Sprintf("Gitea exe path wants %s but %s on %s", p, giteaPath, fPath), nil + // now we regenerate and check if there are any lines missing + regenerated := &bytes.Buffer{} + if err := models.RegeneratePublicKeys(regenerated); err != nil { + return nil, err } - if len(iniPath) > 0 && iniPath != setting.CustomConf { - return fmt.Sprintf("Gitea config path wants %s but %s on %s", setting.CustomConf, iniPath, fPath), nil + scanner = bufio.NewScanner(regenerated) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, tplCommentPrefix) { + continue + } + if ok := linesInAuthorizedKeys[line]; ok { + continue + } + if ctx.Bool("fix") { + return []string{"authorized_keys is out of date, attempting regeneration"}, models.RewriteAllPublicKeys() + } + return []string{"authorized_keys is out of date and should be regenerated with gitea admin regenerate keys"}, nil } - - return "", nil + return nil, nil } func runDoctorCheckDBVersion(ctx *cli.Context) ([]string, error) { diff --git a/models/ssh_key.go b/models/ssh_key.go index 33ba86c5bf5d..64b859fa0241 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -15,6 +15,7 @@ import ( "encoding/pem" "errors" "fmt" + "io" "io/ioutil" "math/big" "os" @@ -701,7 +702,21 @@ func rewriteAllPublicKeys(e Engine) error { } } - err = e.Iterate(new(PublicKey), func(idx int, bean interface{}) (err error) { + if err := regeneratePublicKeys(e, t); err != nil { + return err + } + + t.Close() + return os.Rename(tmpPath, fPath) +} + +// RegeneratePublicKeys regenerates the authorized_keys file +func RegeneratePublicKeys(t io.StringWriter) error { + return regeneratePublicKeys(x, t) +} + +func regeneratePublicKeys(e Engine, t io.StringWriter) error { + err := e.Iterate(new(PublicKey), func(idx int, bean interface{}) (err error) { _, err = t.WriteString((bean.(*PublicKey)).AuthorizedString()) return err }) @@ -709,6 +724,7 @@ func rewriteAllPublicKeys(e Engine) error { return err } + fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys") if com.IsExist(fPath) { f, err := os.Open(fPath) if err != nil { @@ -729,9 +745,7 @@ func rewriteAllPublicKeys(e Engine) error { } f.Close() } - - t.Close() - return os.Rename(tmpPath, fPath) + return nil } // ________ .__ ____ __. From 5c19b9cbe212f6dbee70616475a343f7f909b716 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Apr 2020 11:30:54 +0100 Subject: [PATCH 06/12] Ensure db checker is higher on the list Signed-off-by: Andrew Thornton --- cmd/doctor.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index f8734e5b7c3b..e44a5f3d6c0e 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -76,12 +76,6 @@ var checklist = []check{ abortIfFailed: true, skipDatabaseInit: true, }, - { - title: "Check if OpenSSH authorized_keys file is correct", - name: "authorized_keys", - isDefault: true, - f: runDoctorAuthorizedKeys, - }, { title: "Check Database Version", name: "check-db", @@ -89,6 +83,12 @@ var checklist = []check{ f: runDoctorCheckDBVersion, abortIfFailed: true, }, + { + title: "Check if OpenSSH authorized_keys file is correct", + name: "authorized_keys", + isDefault: true, + f: runDoctorAuthorizedKeys, + }, { title: "Recalculate merge bases", name: "recalculate_merge_bases", From fe9fc5c79ce142cd9194d855598648b9360d7528 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Apr 2020 18:31:06 +0100 Subject: [PATCH 07/12] Add doctor for hooks and ensure hooks are executable Signed-off-by: Andrew Thornton --- cmd/doctor.go | 196 ++++++++++++++++++++---------------- modules/repository/hooks.go | 109 +++++++++++++++++--- 2 files changed, 204 insertions(+), 101 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index e44a5f3d6c0e..b9cb159afff9 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/options" + "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "xorm.io/builder" @@ -84,11 +85,17 @@ var checklist = []check{ abortIfFailed: true, }, { - title: "Check if OpenSSH authorized_keys file is correct", + title: "Check if OpenSSH authorized_keys file is up-to-date", name: "authorized_keys", isDefault: true, f: runDoctorAuthorizedKeys, }, + { + title: "Check if hook files are up-to-date and executable", + name: "hooks", + isDefault: false, + f: runDoctorHooks, + }, { title: "Recalculate merge bases", name: "recalculate_merge_bases", @@ -330,106 +337,119 @@ func runDoctorCheckDBVersion(ctx *cli.Context) ([]string, error) { return nil, nil } -func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { - opts := &models.SearchRepoOptions{ - ListOptions: models.ListOptions{ - Page: 1, - PageSize: 50, +func iterateRepositories(each func(*models.Repository) ([]string, error)) ([]string, error) { + results := []string{} + err := models.Iterate( + models.DefaultDBContext(), + new(models.Repository), + builder.Gt{"id": 0}, + func(idx int, bean interface{}) error { + res, err := each(bean.(*models.Repository)) + results = append(results, res...) + return err }, - } - results := make([]string, 0, 10) - numRepos := 0 - numPRs := 0 - numPRsUpdated := 0 - for { - repos, _, err := models.SearchRepositoryByCondition(opts, builder.NewCond(), false) + ) + return results, err +} + +func iteratePRs(repo *models.Repository, each func(*models.Repository, *models.PullRequest) ([]string, error)) ([]string, error) { + results := []string{} + err := models.Iterate( + models.DefaultDBContext(), + new(models.PullRequest), + builder.Eq{"base_repo_id": repo.ID}, + func(idx int, bean interface{}) error { + res, err := each(repo, bean.(*models.PullRequest)) + results = append(results, res...) + return err + }, + ) + return results, err +} + +func runDoctorHooks(ctx *cli.Context) ([]string, error) { + // Need to iterate across all of the repositories + return iterateRepositories(func(repo *models.Repository) ([]string, error) { + results, err := repository.CheckDelegateHooks(repo.RepoPath()) if err != nil { return nil, err } + if len(results) > 0 && ctx.Bool("fix") { + return []string{fmt.Sprintf("regenerated hooks for %s", repo.FullName())}, repository.CreateDelegateHooks(repo.RepoPath()) + } - for _, repo := range repos { - prOpts := &models.PullRequestsOptions{ - ListOptions: models.ListOptions{ - Page: 1, - PageSize: 50, - }, - } - for { - prs, _, err := models.PullRequests(repo.ID, prOpts) + return results, nil + }) +} + +func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { + numRepos := 0 + numPRs := 0 + numPRsUpdated := 0 + results, err := iterateRepositories(func(repo *models.Repository) ([]string, error) { + numRepos++ + return iteratePRs(repo, func(repo *models.Repository, pr *models.PullRequest) ([]string, error) { + numPRs++ + results := []string{} + pr.BaseRepo = repo + repoPath := repo.RepoPath() + + oldMergeBase := pr.MergeBase + + if !pr.HasMerged { + var err error + pr.MergeBase, err = git.NewCommand("merge-base", "--", pr.BaseBranch, pr.GetGitRefName()).RunInDir(repoPath) if err != nil { - return nil, err - } - for _, pr := range prs { - pr.BaseRepo = repo - repoPath := repo.RepoPath() - - oldMergeBase := pr.MergeBase - - if !pr.HasMerged { - var err error - pr.MergeBase, err = git.NewCommand("merge-base", "--", pr.BaseBranch, pr.GetGitRefName()).RunInDir(repoPath) - if err != nil { - var err2 error - pr.MergeBase, err2 = git.NewCommand("rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath) - if err2 != nil { - results = append(results, fmt.Sprintf("WARN: Unable to get merge base for PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) - log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2) - continue - } - } - } else { - parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath) - if err != nil { - results = append(results, fmt.Sprintf("WARN: Unable to get parents for merged PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) - log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) - continue - } - parents := strings.Split(strings.TrimSpace(parentsString), " ") - if len(parents) < 2 { - continue - } - - args := append([]string{"merge-base", "--"}, parents[1:]...) - args = append(args, pr.GetGitRefName()) - - pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath) - if err != nil { - results = append(results, fmt.Sprintf("WARN: Unable to get merge base for merged PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) - log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) - continue - } - } - pr.MergeBase = strings.TrimSpace(pr.MergeBase) - if pr.MergeBase != oldMergeBase { - if ctx.Bool("fix") { - if err := pr.UpdateCols("merge_base"); err != nil { - return results, err - } - } else { - results = append(results, fmt.Sprintf("#%d onto %s in %s/%s: MergeBase should be %s but is %s", pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, oldMergeBase, pr.MergeBase)) - } - numPRsUpdated++ + var err2 error + pr.MergeBase, err2 = git.NewCommand("rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath) + if err2 != nil { + results = append(results, fmt.Sprintf("WARN: Unable to get merge base for PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) + log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2) + return results, nil } } - numPRs += len(prs) - if len(prs) < 50 { - break + } else { + parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath) + if err != nil { + results = append(results, fmt.Sprintf("WARN: Unable to get parents for merged PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) + log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) + return results, nil + } + parents := strings.Split(strings.TrimSpace(parentsString), " ") + if len(parents) < 2 { + return results, nil + } + + args := append([]string{"merge-base", "--"}, parents[1:]...) + args = append(args, pr.GetGitRefName()) + + pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath) + if err != nil { + results = append(results, fmt.Sprintf("WARN: Unable to get merge base for merged PR ID %d, #%d onto %s in %s/%s", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name)) + log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) + return results, nil } } - prOpts.Page++ - } + pr.MergeBase = strings.TrimSpace(pr.MergeBase) + if pr.MergeBase != oldMergeBase { + if ctx.Bool("fix") { + if err := pr.UpdateCols("merge_base"); err != nil { + return results, err + } + } else { + results = append(results, fmt.Sprintf("#%d onto %s in %s/%s: MergeBase should be %s but is %s", pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, oldMergeBase, pr.MergeBase)) + } + numPRsUpdated++ + } + return results, nil + }) + }) - numRepos += len(repos) - if len(repos) < 50 { - break - } - opts.Page++ - } if ctx.Bool("fix") { - results = append(results, fmt.Sprintf("%d PRs with incorrect mergebases of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) - } else { results = append(results, fmt.Sprintf("%d PRs updated of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) + } else { + results = append(results, fmt.Sprintf("%d PRs with incorrect mergebases of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) } - return results, nil + return results, err } diff --git a/modules/repository/hooks.go b/modules/repository/hooks.go index 60e3418571a0..142782a5c9c7 100644 --- a/modules/repository/hooks.go +++ b/modules/repository/hooks.go @@ -14,10 +14,26 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "github.com/unknwon/com" "xorm.io/builder" ) +func getHookTemplates() (hookNames, hookTpls, giteaHookTpls []string) { + hookNames = []string{"pre-receive", "update", "post-receive"} + hookTpls = []string{ + fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType), + fmt.Sprintf("#!/usr/bin/env %s\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\n\"${hook}\" $1 $2 $3\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType), + fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType), + } + giteaHookTpls = []string{ + fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' pre-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf), + fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' update $1 $2 $3\n", setting.ScriptType, setting.AppPath, setting.CustomConf), + fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' post-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf), + } + return +} + // CreateDelegateHooks creates all the hooks scripts for the repo func CreateDelegateHooks(repoPath string) error { return createDelegateHooks(repoPath) @@ -26,19 +42,7 @@ func CreateDelegateHooks(repoPath string) error { // createDelegateHooks creates all the hooks scripts for the repo func createDelegateHooks(repoPath string) (err error) { - var ( - hookNames = []string{"pre-receive", "update", "post-receive"} - hookTpls = []string{ - fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType), - fmt.Sprintf("#!/usr/bin/env %s\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\n\"${hook}\" $1 $2 $3\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType), - fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType), - } - giteaHookTpls = []string{ - fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' pre-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf), - fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' update $1 $2 $3\n", setting.ScriptType, setting.AppPath, setting.CustomConf), - fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' post-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf), - } - ) + hookNames, hookTpls, giteaHookTpls := getHookTemplates() hookDir := filepath.Join(repoPath, "hooks") @@ -58,17 +62,96 @@ func createDelegateHooks(repoPath string) (err error) { return fmt.Errorf("write old hook file '%s': %v", oldHookPath, err) } + if err = ensureExecutable(oldHookPath); err != nil { + return fmt.Errorf("Unable to set %s executable. Error %v", oldHookPath, err) + } + if err = os.Remove(newHookPath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("unable to pre-remove new hook file '%s' prior to rewriting: %v", newHookPath, err) } if err = ioutil.WriteFile(newHookPath, []byte(giteaHookTpls[i]), 0777); err != nil { return fmt.Errorf("write new hook file '%s': %v", newHookPath, err) } + + if err = ensureExecutable(newHookPath); err != nil { + return fmt.Errorf("Unable to set %s executable. Error %v", oldHookPath, err) + } } return nil } +func checkExecutable(filename string) bool { + fileInfo, err := os.Stat(filename) + if err != nil { + return false + } + return (fileInfo.Mode() & 0100) > 0 +} + +func ensureExecutable(filename string) error { + fileInfo, err := os.Stat(filename) + if err != nil { + return err + } + if (fileInfo.Mode() & 0100) > 0 { + return nil + } + mode := fileInfo.Mode() | 0100 + return os.Chmod(filename, mode) +} + +// CheckDelegateHooks checks the hooks scripts for the repo +func CheckDelegateHooks(repoPath string) ([]string, error) { + hookNames, hookTpls, giteaHookTpls := getHookTemplates() + + hookDir := filepath.Join(repoPath, "hooks") + results := make([]string, 0, 10) + + for i, hookName := range hookNames { + oldHookPath := filepath.Join(hookDir, hookName) + newHookPath := filepath.Join(hookDir, hookName+".d", "gitea") + + cont := false + if !com.IsExist(oldHookPath) { + results = append(results, fmt.Sprintf("old hook file %s does not exist", oldHookPath)) + cont = true + } + if !com.IsExist(oldHookPath + ".d") { + results = append(results, fmt.Sprintf("hooks directory %s does not exist", oldHookPath+".d")) + cont = true + } + if !com.IsExist(newHookPath) { + results = append(results, fmt.Sprintf("new hook file %s does not exist", newHookPath)) + cont = true + } + if cont { + continue + } + contents, err := ioutil.ReadFile(oldHookPath) + if err != nil { + return results, err + } + if string(contents) != hookTpls[i] { + results = append(results, fmt.Sprintf("old hook file %s is out of date", oldHookPath)) + } + if !checkExecutable(oldHookPath) { + results = append(results, fmt.Sprintf("old hook file %s is not executable", oldHookPath)) + } + contents, err = ioutil.ReadFile(newHookPath) + if err != nil { + return results, err + } + if string(contents) != giteaHookTpls[i] { + results = append(results, fmt.Sprintf("new hook file %s is out of date", newHookPath)) + } + if !checkExecutable(newHookPath) { + results = append(results, fmt.Sprintf("new hook file %s is not executable", newHookPath)) + } + } + return results, nil +} + // SyncRepositoryHooks rewrites all repositories' pre-receive, update and post-receive hooks // to make sure the binary and custom conf path are up-to-date. func SyncRepositoryHooks(ctx context.Context) error { From c869b02ab0fd9c441bc9fceeaef7c7248ca0ffd4 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Apr 2020 21:03:28 +0100 Subject: [PATCH 08/12] Fix #10977 Signed-off-by: Andrew Thornton --- cmd/doctor.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cmd/doctor.go b/cmd/doctor.go index b9cb159afff9..310727cd8342 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -11,6 +11,7 @@ import ( "fmt" "io/ioutil" "os" + "os/exec" "path/filepath" "strings" "text/tabwriter" @@ -90,6 +91,12 @@ var checklist = []check{ isDefault: true, f: runDoctorAuthorizedKeys, }, + { + title: "Check if SCRIPT_TYPE is available", + name: "script-type", + isDefault: false, + f: runDoctorScriptType, + }, { title: "Check if hook files are up-to-date and executable", name: "hooks", @@ -453,3 +460,11 @@ func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { return results, err } + +func runDoctorScriptType(ctx *cli.Context) ([]string, error) { + path, err := exec.LookPath(setting.ScriptType) + if err != nil { + return []string{fmt.Sprintf("ScriptType %s is not on the current PATH", setting.ScriptType)}, err + } + return []string{fmt.Sprintf("ScriptType %s is on the current PATH at %s", setting.ScriptType, path)}, nil +} From c2256dc4714432000155b3452ffbe6148b2db4d8 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Apr 2020 21:41:51 +0100 Subject: [PATCH 09/12] adjust messages as per @guillep2k Signed-off-by: Andrew Thornton --- cmd/doctor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 310727cd8342..213e7b214e2b 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -337,7 +337,7 @@ func runDoctorAuthorizedKeys(ctx *cli.Context) ([]string, error) { func runDoctorCheckDBVersion(ctx *cli.Context) ([]string, error) { if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { if ctx.Bool("fix") { - return nil, models.NewEngine(context.Background(), migrations.Migrate) + return []string{fmt.Sprintf("WARN: Got Error %v during ensure up to date", err), "Attempting to migrate to the latest DB version to fix this."}, models.NewEngine(context.Background(), migrations.Migrate) } return nil, err } From 600517ec730c1b45f519a18efa9056c8ff452abc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Apr 2020 21:50:39 +0100 Subject: [PATCH 10/12] as per @guillep2k Signed-off-by: Andrew Thornton --- cmd/doctor.go | 2 +- models/migrations/migrations.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 213e7b214e2b..4afd53140210 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -453,7 +453,7 @@ func runDoctorPRMergeBase(ctx *cli.Context) ([]string, error) { }) if ctx.Bool("fix") { - results = append(results, fmt.Sprintf("%d PRs updated of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) + results = append(results, fmt.Sprintf("%d PR mergebases updated of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) } else { results = append(results, fmt.Sprintf("%d PRs with incorrect mergebases of %d PRs total in %d repos", numPRsUpdated, numPRs, numRepos)) } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 946d82289f9f..cad7f05f1598 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -246,7 +246,7 @@ func EnsureUpToDate(x *xorm.Engine) error { expected := ExpectedVersion() if currentDB != expected { - return fmt.Errorf("Current database version %d is not equal to the expected version %d. Please run migrate", currentDB, expected) + return fmt.Errorf(`Current database version %d is not equal to the expected version %d. Please run "gitea [--config /path/to/app.ini] migrate" to update the database version`, currentDB, expected) } return nil From 060253ae5aca2f715d63f820d54eeb7025b415cc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 6 Apr 2020 08:50:12 +0100 Subject: [PATCH 11/12] And so it continues - now we have logging configuration Signed-off-by: Andrew Thornton --- cmd/doctor.go | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 4afd53140210..0e26bfc3fca0 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "io/ioutil" + golog "log" "os" "os/exec" "path/filepath" @@ -55,6 +56,10 @@ var CmdDoctor = cli.Command{ Name: "fix", Usage: "Automatically fix what we can", }, + cli.StringFlag{ + Name: "log-file", + Usage: `Name of the log file (default: "doctor.log"). Set to "-" to output to stdout, set to "" to disable.`, + }, }, } @@ -114,11 +119,29 @@ var checklist = []check{ func runDoctor(ctx *cli.Context) error { - // Silence the console logger - // TODO: Redirect all logs into `doctor.log` ignoring any other log configuration + // Silence the default loggers log.DelNamedLogger("console") log.DelNamedLogger(log.DEFAULT) + // Now setup our own + logFile := ctx.String("log-file") + if !ctx.IsSet("log-file") { + logFile = "doctor.log" + } + + if len(logFile) == 0 { + log.NewLogger(1000, "doctor", "console", `{"level":"NONE","stacktracelevel":"NONE","colorize":"%t"}`) + } else if logFile == "-" { + log.NewLogger(1000, "doctor", "console", `{"level":"trace","stacktracelevel":"NONE"}`) + } else { + log.NewLogger(1000, "doctor", "file", fmt.Sprintf(`{"filename":%q,"level":"trace","stacktracelevel":"NONE"}`, logFile)) + } + + // Finally redirect the default golog to here + golog.SetFlags(0) + golog.SetPrefix("") + golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT))) + if ctx.IsSet("list") { w := tabwriter.NewWriter(os.Stdout, 0, 8, 0, '\t', 0) _, _ = w.Write([]byte("Default\tName\tTitle\n")) @@ -170,7 +193,8 @@ func runDoctor(ctx *cli.Context) error { for i, check := range checks { if !dbIsInit && !check.skipDatabaseInit { // Only open database after the most basic configuration check - if err := initDB(); err != nil { + setting.EnableXORMLog = false + if err := initDBDisableConsole(true); 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 From ec83174456d1a9fa44624b6da34049c46029f8d2 Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 6 Apr 2020 09:02:17 +0100 Subject: [PATCH 12/12] Update cmd/doctor.go --- cmd/doctor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 0e26bfc3fca0..3b0b5c9e9f53 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -58,7 +58,7 @@ var CmdDoctor = cli.Command{ }, cli.StringFlag{ Name: "log-file", - Usage: `Name of the log file (default: "doctor.log"). Set to "-" to output to stdout, set to "" to disable.`, + Usage: `Name of the log file (default: "doctor.log"). Set to "-" to output to stdout, set to "" to disable`, }, }, }