[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 <forgejo@gergo.csillger.hu>
Co-committed-by: Gergely Nagy <forgejo@gergo.csillger.hu>

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
-}

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
+}

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,
+	})
+}

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{})