Merge branch 'backup_restore_manifest' into 'master'

Use manifests on restore when available

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

Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>

2 files changed, 148 insertions, 7 deletions

diff --git a/internal/backup/locator.go b/internal/backup/locator.go
index da789011d..1e1c42550 100644
--- a/internal/backup/locator.go
+++ b/internal/backup/locator.go
@@ -294,7 +294,7 @@ func (l ManifestLocator) Commit(ctx context.Context, backup *Backup) (returnErr
 		return err
 	}
 
-	f, err := l.Sink.GetWriter(ctx, manifestPath(backup))
+	f, err := l.Sink.GetWriter(ctx, manifestPath(backup.Repository, backup.ID))
 	if err != nil {
 		return fmt.Errorf("manifest: commit: %w", err)
 	}
@@ -316,16 +316,35 @@ func (l ManifestLocator) FindLatest(ctx context.Context, repo storage.Repository
 	return l.Fallback.FindLatest(ctx, repo)
 }
 
-// Find passes through to Fallback
+// Find loads the manifest for the provided repo and backupID. If this manifest
+// does not exist, the fallback is used.
 func (l ManifestLocator) Find(ctx context.Context, repo storage.Repository, backupID string) (*Backup, error) {
-	return l.Fallback.Find(ctx, repo, backupID)
+	f, err := l.Sink.GetReader(ctx, manifestPath(repo, backupID))
+	switch {
+	case errors.Is(err, ErrDoesntExist):
+		return l.Fallback.Find(ctx, repo, backupID)
+	case err != nil:
+		return nil, fmt.Errorf("manifest: find: %w", err)
+	}
+	defer f.Close()
+
+	var backup Backup
+
+	if err := toml.NewDecoder(f).Decode(&backup); err != nil {
+		return nil, fmt.Errorf("manifest: find: %w", err)
+	}
+
+	backup.ID = backupID
+	backup.Repository = repo
+
+	return &backup, nil
 }
 
-func manifestPath(backup *Backup) string {
-	storageName := backup.Repository.GetStorageName()
+func manifestPath(repo storage.Repository, backupID string) string {
+	storageName := repo.GetStorageName()
 	// Other locators strip the .git suffix off of relative paths. This suffix
 	// is determined by gitlab-rails not gitaly. So here we leave the relative
 	// path as-is so that new backups can be more independent.
-	relativePath := backup.Repository.GetRelativePath()
-	return path.Join("manifests", storageName, relativePath, backup.ID+".toml")
+	relativePath := repo.GetRelativePath()
+	return path.Join("manifests", storageName, relativePath, backupID+".toml")
 }
diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go
index 7aed83608..90cac3664 100644
--- a/internal/backup/locator_test.go
+++ b/internal/backup/locator_test.go
@@ -11,9 +11,11 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
+	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 )
 
 func TestLegacyLocator(t *testing.T) {
@@ -483,3 +485,123 @@ custom_hooks_path = '%[1]s/%[2]s/002.custom_hooks.tar'
 `, repo.RelativePath, backupID), string(manifest))
 	})
 }
+
+func TestManifestLocator_Find(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		desc           string
+		repo           storage.Repository
+		backupID       string
+		setup          func(t *testing.T, ctx context.Context, backupPath string)
+		expectedBackup *Backup
+	}{
+		{
+			desc: "finds manifest",
+			repo: &gitalypb.Repository{
+				StorageName:  "default",
+				RelativePath: "vanity/repo.git",
+			},
+			backupID: "abc123",
+			setup: func(t *testing.T, ctx context.Context, backupPath string) {
+				testhelper.WriteFiles(t, backupPath, map[string]any{
+					"vanity/repo/LATEST":        "abc123",
+					"vanity/repo/abc123/LATEST": "002",
+					"manifests/default/vanity/repo.git/abc123.toml": `object_format = 'sha1'
+
+[[steps]]
+bundle_path = 'path/to/001.bundle'
+ref_path = 'path/to/001.refs'
+custom_hooks_path = 'path/to/001.custom_hooks.tar'
+
+[[steps]]
+bundle_path = 'path/to/002.bundle'
+ref_path = 'path/to/002.refs'
+previous_ref_path = 'path/to/001.refs'
+custom_hooks_path = 'path/to/002.custom_hooks.tar'
+`,
+				})
+			},
+			expectedBackup: &Backup{
+				ID: "abc123",
+				Repository: &gitalypb.Repository{
+					StorageName:  "default",
+					RelativePath: "vanity/repo.git",
+				},
+				ObjectFormat: "sha1",
+				Steps: []Step{
+					{
+						BundlePath:      "path/to/001.bundle",
+						RefPath:         "path/to/001.refs",
+						CustomHooksPath: "path/to/001.custom_hooks.tar",
+					},
+					{
+						BundlePath:      "path/to/002.bundle",
+						RefPath:         "path/to/002.refs",
+						PreviousRefPath: "path/to/001.refs",
+						CustomHooksPath: "path/to/002.custom_hooks.tar",
+					},
+				},
+			},
+		},
+		{
+			desc: "fallback",
+			repo: &gitalypb.Repository{
+				StorageName:  "default",
+				RelativePath: "vanity/repo.git",
+			},
+			backupID: "abc123",
+			setup: func(t *testing.T, ctx context.Context, backupPath string) {
+				testhelper.WriteFiles(t, backupPath, map[string]any{
+					"vanity/repo/LATEST":        "abc123",
+					"vanity/repo/abc123/LATEST": "002",
+				})
+			},
+			expectedBackup: &Backup{
+				ID: "abc123",
+				Repository: &gitalypb.Repository{
+					StorageName:  "default",
+					RelativePath: "vanity/repo.git",
+				},
+				ObjectFormat: "sha1",
+				Steps: []Step{
+					{
+						BundlePath:      "vanity/repo/abc123/001.bundle",
+						RefPath:         "vanity/repo/abc123/001.refs",
+						CustomHooksPath: "vanity/repo/abc123/001.custom_hooks.tar",
+					},
+					{
+						BundlePath:      "vanity/repo/abc123/002.bundle",
+						RefPath:         "vanity/repo/abc123/002.refs",
+						PreviousRefPath: "vanity/repo/abc123/001.refs",
+						CustomHooksPath: "vanity/repo/abc123/002.custom_hooks.tar",
+					},
+				},
+			},
+		},
+	} {
+		tc := tc
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
+			ctx := testhelper.Context(t)
+			backupPath := testhelper.TempDir(t)
+
+			tc.setup(t, ctx, backupPath)
+
+			sink := NewFilesystemSink(backupPath)
+			var l Locator = PointerLocator{
+				Sink: sink,
+			}
+			l = ManifestLocator{
+				Sink:     sink,
+				Fallback: l,
+			}
+
+			backup, err := l.Find(ctx, tc.repo, tc.backupID)
+			require.NoError(t, err)
+
+			require.Equal(t, tc.expectedBackup, backup)
+		})
+	}
+}