Merge branch 'smh-recursive-quarantine' into 'master'

Migrate objects to the repository's configured object directory

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6048

Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Reviewed-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Sami Hiltunen <shiltunen@gitlab.com>

3 files changed, 48 insertions, 3 deletions

diff --git a/internal/git/quarantine/quarantine.go b/internal/git/quarantine/quarantine.go
index 0adbeab58..14811a93a 100644
--- a/internal/git/quarantine/quarantine.go
+++ b/internal/git/quarantine/quarantine.go
@@ -92,15 +92,22 @@ func (d *Dir) QuarantinedRepo() *gitalypb.Repository {
 	return d.quarantinedRepo
 }
 
-// Migrate migrates all objects part of the quarantine directory into the main repository and thus
-// makes them generally available. This implementation follows the git.git's `tmp_objdir_migrate()`.
+// Migrate migrates all objects part of the quarantine directory into the repository and thus makes
+// them generally available. This implementation follows the git.git's `tmp_objdir_migrate()`.
 func (d *Dir) Migrate() error {
 	repoPath, err := d.locator.GetRepoPath(d.repo, storage.WithRepositoryVerificationSkipped())
 	if err != nil {
 		return fmt.Errorf("migrating quarantine: %w", err)
 	}
 
-	return migrate(d.dir.Path(), filepath.Join(repoPath, "objects"))
+	objectDir := d.repo.GitObjectDirectory
+	if objectDir == "" {
+		// Migrate the objects to the default object directory if the repository
+		// didn't have an object directory explicitly configured.
+		objectDir = "objects"
+	}
+
+	return migrate(d.dir.Path(), filepath.Join(repoPath, objectDir))
 }
 
 func migrate(sourcePath, targetPath string) error {
diff --git a/internal/git/quarantine/quarantine_test.go b/internal/git/quarantine/quarantine_test.go
index 9f74a9b71..fff85d10c 100644
--- a/internal/git/quarantine/quarantine_test.go
+++ b/internal/git/quarantine/quarantine_test.go
@@ -130,6 +130,43 @@ func TestQuarantine_Migrate(t *testing.T) {
 		oldContents["objects/file"] = "foobar"
 		require.Equal(t, oldContents, newContents)
 	})
+
+	t.Run("simple change into existing quarantine", func(t *testing.T) {
+		ctx := testhelper.Context(t)
+
+		repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+			SkipCreationViaService: true,
+		})
+
+		repoContents := listEntries(t, repoPath)
+		require.NotContains(t, repoContents, "objects/file")
+
+		quarantine, err := New(ctx, repo, locator)
+		require.NoError(t, err)
+
+		require.Empty(t, listEntries(t, quarantine.dir.Path()))
+
+		// Quarantine the already quarantined repository and write the object there. We expect the
+		// object to be migrated from the second level quarantine to the first level quarantine. The
+		// main repository should stay untouched.
+		recursiveQuarantine, err := New(ctx, quarantine.QuarantinedRepo(), locator)
+		require.NoError(t, err)
+
+		require.NoError(t, os.WriteFile(filepath.Join(recursiveQuarantine.dir.Path(), "file"), []byte("foobar"), perm.PublicFile))
+		require.NoError(t, recursiveQuarantine.Migrate())
+
+		// The main repo should be untouched and still not contain the object.
+		require.Equal(t, repoContents, listEntries(t, repoPath))
+
+		// The quarantine should contain the object.
+		require.Equal(t,
+			map[string]string{"file": "foobar"},
+			listEntries(t, quarantine.dir.Path()),
+		)
+
+		// Recursive quarantine should no longer exist.
+		require.NoDirExists(t, recursiveQuarantine.dir.Path())
+	})
 }
 
 func TestApply(t *testing.T) {
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index d46b02199..362c67bef 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -974,6 +974,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 					continue
 				}
 
+				require.NoError(t, err)
 				require.Equal(t, step.branchCreated, resp.BranchUpdate.BranchCreated, "step %d", i+1)
 				require.Equal(t, step.repoCreated, resp.BranchUpdate.RepoCreated, "step %d", i+1)
 				gittest.RequireTree(t, cfg, repoPath, branch, step.treeEntries)