From 1fea3ce6592afca7f8d16c147effd343244746fa Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Fri, 1 Dec 2023 13:56:03 +0000 Subject: [PATCH] [GITEA] new doctor check: fix-push-mirrors-without-git-remote (#1853) Same as https://codeberg.org/forgejo/forgejo/pulls/1853, backported to v1.21/forgejo Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1864 Co-authored-by: Gergely Nagy Co-committed-by: Gergely Nagy --- models/migrations/v1_21/v276.go | 29 +------- models/repo/pushmirror.go | 24 ++++++ modules/doctor/push_mirror_consistency.go | 91 +++++++++++++++++++++++ tests/integration/mirror_push_test.go | 28 ++++++- 4 files changed, 145 insertions(+), 27 deletions(-) create mode 100644 modules/doctor/push_mirror_consistency.go diff --git a/models/migrations/v1_21/v276.go b/models/migrations/v1_21/v276.go index ed1bc3bda5..67e950177d 100644 --- a/models/migrations/v1_21/v276.go +++ b/models/migrations/v1_21/v276.go @@ -4,13 +4,7 @@ package v1_21 //nolint import ( - "context" - "fmt" - "path/filepath" - "strings" - - "code.gitea.io/gitea/modules/git" - giturl "code.gitea.io/gitea/modules/git/url" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/setting" "xorm.io/xorm" @@ -73,7 +67,7 @@ func migratePullMirrors(x *xorm.Engine) error { start += len(mirrors) for _, m := range mirrors { - remoteAddress, err := getRemoteAddress(m.RepoOwner, m.RepoName, "origin") + remoteAddress, err := repo_model.GetPushMirrorRemoteAddress(m.RepoOwner, m.RepoName, "origin") if err != nil { return err } @@ -136,7 +130,7 @@ func migratePushMirrors(x *xorm.Engine) error { start += len(mirrors) for _, m := range mirrors { - remoteAddress, err := getRemoteAddress(m.RepoOwner, m.RepoName, m.RemoteName) + remoteAddress, err := repo_model.GetPushMirrorRemoteAddress(m.RepoOwner, m.RepoName, m.RemoteName) if err != nil { return err } @@ -160,20 +154,3 @@ func migratePushMirrors(x *xorm.Engine) error { return sess.Commit() } - -func getRemoteAddress(ownerName, repoName, remoteName string) (string, error) { - repoPath := filepath.Join(setting.RepoRootPath, strings.ToLower(ownerName), strings.ToLower(repoName)+".git") - - remoteURL, err := git.GetRemoteAddress(context.Background(), repoPath, remoteName) - if err != nil { - return "", fmt.Errorf("get remote %s's address of %s/%s failed: %v", remoteName, ownerName, repoName, err) - } - - u, err := giturl.Parse(remoteURL) - if err != nil { - return "", err - } - u.User = nil - - return u.String(), nil -} diff --git a/models/repo/pushmirror.go b/models/repo/pushmirror.go index 73c1384444..e199d887ef 100644 --- a/models/repo/pushmirror.go +++ b/models/repo/pushmirror.go @@ -5,10 +5,16 @@ package repo import ( "context" + "fmt" + "path/filepath" + "strings" "time" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/git" + giturl "code.gitea.io/gitea/modules/git/url" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -145,3 +151,21 @@ func PushMirrorsIterate(ctx context.Context, limit int, f func(idx int, bean any } return sess.Iterate(new(PushMirror), f) } + +// GetPushMirrorRemoteAddress returns the address of associated with a repository's given remote. +func GetPushMirrorRemoteAddress(ownerName, repoName, remoteName string) (string, error) { + repoPath := filepath.Join(setting.RepoRootPath, strings.ToLower(ownerName), strings.ToLower(repoName)+".git") + + remoteURL, err := git.GetRemoteAddress(context.Background(), repoPath, remoteName) + if err != nil { + return "", fmt.Errorf("get remote %s's address of %s/%s failed: %v", remoteName, ownerName, repoName, err) + } + + u, err := giturl.Parse(remoteURL) + if err != nil { + return "", err + } + u.User = nil + + return u.String(), nil +} diff --git a/modules/doctor/push_mirror_consistency.go b/modules/doctor/push_mirror_consistency.go new file mode 100644 index 0000000000..f469ab47ff --- /dev/null +++ b/modules/doctor/push_mirror_consistency.go @@ -0,0 +1,91 @@ +// Copyright 2023 The Forgejo Authors c/o Codeberg e.V.. All rights reserved. +// SPDX-License-Identifier: MIT + +package doctor + +import ( + "context" + "strings" + + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/log" + + "xorm.io/builder" +) + +func FixPushMirrorsWithoutGitRemote(ctx context.Context, logger log.Logger, autofix bool) error { + var missingMirrors []*repo_model.PushMirror + + err := db.Iterate(ctx, builder.Gt{"id": 0}, func(ctx context.Context, repo *repo_model.Repository) error { + pushMirrors, _, err := repo_model.GetPushMirrorsByRepoID(ctx, repo.ID, db.ListOptions{}) + if err != nil { + return err + } + + for i := 0; i < len(pushMirrors); i++ { + _, err = repo_model.GetPushMirrorRemoteAddress(repo.OwnerName, repo.Name, pushMirrors[i].RemoteName) + if err != nil { + if strings.Contains(err.Error(), "No such remote") { + missingMirrors = append(missingMirrors, pushMirrors[i]) + } else if logger != nil { + logger.Warn("Unable to retrieve the remote address of a mirror: %s", err) + } + } + } + + return nil + }) + if err != nil { + if logger != nil { + logger.Critical("Unable to iterate across repounits to fix push mirrors without a git remote: Error %v", err) + } + return err + } + + count := len(missingMirrors) + if !autofix { + if logger != nil { + if count == 0 { + logger.Info("Found no push mirrors with missing git remotes") + } else { + logger.Warn("Found %d push mirrors with missing git remotes", count) + } + } + return nil + } + + for i := 0; i < len(missingMirrors); i++ { + if logger != nil { + logger.Info("Removing push mirror #%d (remote: %s), for repo: %s/%s", + missingMirrors[i].ID, + missingMirrors[i].RemoteName, + missingMirrors[i].GetRepository().OwnerName, + missingMirrors[i].GetRepository().Name) + } + + err = repo_model.DeletePushMirrors(ctx, repo_model.PushMirrorOptions{ + ID: missingMirrors[i].ID, + RepoID: missingMirrors[i].RepoID, + RemoteName: missingMirrors[i].RemoteName, + }) + if err != nil { + if logger != nil { + logger.Critical("Error removing a push mirror (repo_id: %d, push_mirror: %d): %s", missingMirrors[i].Repo.ID, missingMirrors[i].ID, err) + } + return err + } + } + + return nil +} + +func init() { + Register(&Check{ + Title: "Check for push mirrors without a git remote configured", + Name: "fix-push-mirrors-without-git-remote", + IsDefault: false, + Run: FixPushMirrorsWithoutGitRemote, + Priority: 7, + }) +} diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index c6f0c85616..8df2567c4e 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" gitea_context "code.gitea.io/gitea/modules/context" + doctor "code.gitea.io/gitea/modules/doctor" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/services/migrations" @@ -47,10 +48,11 @@ func testMirrorPush(t *testing.T, u *url.URL) { ctx := NewAPITestContext(t, user.LowerName, srcRepo.Name) doCreatePushMirror(ctx, fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape(mirrorRepo.Name)), user.LowerName, userPassword)(t) + doCreatePushMirror(ctx, fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape("does-not-matter")), user.LowerName, userPassword)(t) mirrors, _, err := repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{}) assert.NoError(t, err) - assert.Len(t, mirrors, 1) + assert.Len(t, mirrors, 2) ok := mirror_service.SyncPushMirror(context.Background(), mirrors[0].ID) assert.True(t, ok) @@ -71,6 +73,30 @@ func testMirrorPush(t *testing.T, u *url.URL) { assert.Equal(t, srcCommit.ID, mirrorCommit.ID) + // Test that we can "repair" push mirrors where the remote doesn't exist in git's state. + // To do that, we artificially remove the remote... + cmd := git.NewCommand(db.DefaultContext, "remote", "rm").AddDynamicArguments(mirrors[0].RemoteName) + _, _, err = cmd.RunStdString(&git.RunOpts{Dir: srcRepo.RepoPath()}) + assert.NoError(t, err) + + // ...then ensure that trying to get its remote address fails + _, err = repo_model.GetPushMirrorRemoteAddress(srcRepo.OwnerName, srcRepo.Name, mirrors[0].RemoteName) + assert.Error(t, err) + + // ...and that we can fix it. + err = doctor.FixPushMirrorsWithoutGitRemote(db.DefaultContext, nil, true) + assert.NoError(t, err) + + // ...and after fixing, we only have one remote + mirrors, _, err = repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, mirrors, 1) + + // ...one we can get the address of, and it's not the one we removed + remoteAddress, err := repo_model.GetPushMirrorRemoteAddress(srcRepo.OwnerName, srcRepo.Name, mirrors[0].RemoteName) + assert.NoError(t, err) + assert.Contains(t, remoteAddress, "does-not-matter") + // Cleanup doRemovePushMirror(ctx, fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape(mirrorRepo.Name)), user.LowerName, userPassword, int(mirrors[0].ID))(t) mirrors, _, err = repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{})