From ff00749191f93c947739bb40565ce3175068ac1f Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Fri, 1 Dec 2023 13:07:36 +0000 Subject: [PATCH] [GITEA] new doctor check: fix-push-mirrors-without-git-remote (#1853) This adds a new `doctor` check: `fix-push-mirrors-without-git-remote`. The new check looks for push mirrors that do not have their remotes configured in git. If automatic fixing is enabled, it will remove these push mirrors from the database. The check is not run by default, and thus, must be invoked manually. It should be usable in a half-migrated state, too, and as such, fixes #1800. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1853 Co-authored-by: Gergely Nagy Co-committed-by: Gergely Nagy (cherry picked from commit 9038e07ef35978336612588d68c1315179a45c73) (cherry picked from commit b15bafcbc7d9033b0cc7b0fd888915b117e08d42) (cherry picked from commit 93ba05a2dd9fdec46f337542cd5f22c8960ac55f) (cherry picked from commit e418ea80822361e387b460c583592bbd83d4a39e) (cherry picked from commit 321790a91ec8553d1b3668f606ebec762865dd17) (cherry picked from commit f4e19d332392cb455b3b4e32e271f3e42302bbc8) (cherry picked from commit 4d9923dee851a4046050761d3dd352f2f343f4fc) (cherry picked from commit 049df69eda1ceb47f6e74c9a67e9ce5041e65c3b) Conflicts: services/doctor/push_mirror_consistency.go https://codeberg.org/forgejo/forgejo/pulls/2214 (cherry picked from commit c79cba8d556320be0da7ca8324b39cd8930465bf) (cherry picked from commit f3a3969c02cade7261a5f25c9e342800ccdf9111) --- models/migrations/v1_21/v276.go | 29 +------ models/repo/pushmirror.go | 24 ++++++ services/doctor/push_mirror_consistency.go | 91 ++++++++++++++++++++++ tests/integration/mirror_push_test.go | 28 ++++++- 4 files changed, 145 insertions(+), 27 deletions(-) create mode 100644 services/doctor/push_mirror_consistency.go diff --git a/models/migrations/v1_21/v276.go b/models/migrations/v1_21/v276.go index ed1bc3bda..67e950177 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 bf134abfb..e08333511 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" @@ -132,3 +138,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/services/doctor/push_mirror_consistency.go b/services/doctor/push_mirror_consistency.go new file mode 100644 index 000000000..68b96d641 --- /dev/null +++ b/services/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(ctx).OwnerName, + missingMirrors[i].GetRepository(ctx).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 3dc719593..4263f12f9 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" + doctor "code.gitea.io/gitea/services/doctor" "code.gitea.io/gitea/services/migrations" mirror_service "code.gitea.io/gitea/services/mirror" repo_service "code.gitea.io/gitea/services/repository" @@ -48,10 +49,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) @@ -72,6 +74,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{})