Inspect changes in Gitaly 88ea77eda336a64b989368104b91cf758acd3e25
Upstream merge commit a855d06d0eff changes Gitaly server version from da0e1ee67e706eb33e22aadc0a87f8b728342303 to 88ea77eda336a64b989368104b91cf758acd3e25, with changes in protocol or one of the source subtrees currently under watch.
To further inspect, go to an up to date Gitaly source clone and run:
git diff da0e1ee67e706eb33e22aadc0a87f8b728342303..88ea77eda336a64b989368104b91cf758acd3e25 -- proto/*.proto internal/backup
Here is a preview, truncated at 500 lines:
diff --git a/internal/backup/backup.go b/internal/backup/backup.go
index 42094222a..0ced2c4c1 100644
--- a/internal/backup/backup.go
+++ b/internal/backup/backup.go
@@ -49,6 +49,8 @@ type Backup struct {
Steps []Step `toml:"steps"`
// ObjectFormat is the name of the object hash used by the repository.
ObjectFormat string `toml:"object_format"`
+ // HeadReference is the reference that HEAD points to.
+ HeadReference string `toml:"head_reference,omitempty"`
}
// Step represents an incremental step that makes up a complete backup for a repository
@@ -98,12 +100,16 @@ type Repository interface {
// repository cannot be found.
Remove(ctx context.Context) error
// Create creates the repository.
- Create(ctx context.Context, hash git.ObjectHash) error
+ Create(ctx context.Context, hash git.ObjectHash, defaultBranch string) error
// FetchBundle fetches references from a bundle. Refs will be mirrored to
// the repository.
- FetchBundle(ctx context.Context, reader io.Reader) error
+ FetchBundle(ctx context.Context, reader io.Reader, updateHead bool) error
// SetCustomHooks updates the custom hooks for the repository.
SetCustomHooks(ctx context.Context, reader io.Reader) error
+ // ObjectHash detects the object hash used by the repository.
+ ObjectHash(ctx context.Context) (git.ObjectHash, error)
+ // HeadReference fetches the reference pointed to by HEAD.
+ HeadReference(ctx context.Context) (git.ReferenceName, error)
}
// ResolveLocator returns a locator implementation based on a locator identifier.
@@ -224,13 +230,25 @@ func (mgr *Manager) Create(ctx context.Context, req *CreateRequest) error {
backup = mgr.locator.BeginFull(ctx, req.VanityRepository, req.BackupID)
}
- refs, err := repo.ListRefs(ctx)
+ hash, err := repo.ObjectHash(ctx)
switch {
case status.Code(err) == codes.NotFound:
return fmt.Errorf("manager: repository not found: %w", ErrSkipped)
case err != nil:
return fmt.Errorf("manager: %w", err)
}
+ backup.ObjectFormat = hash.Format
+
+ headRef, err := repo.HeadReference(ctx)
+ if err != nil {
+ return fmt.Errorf("manager: %w", err)
+ }
+ backup.HeadReference = headRef.String()
+
+ refs, err := repo.ListRefs(ctx)
+ if err != nil {
+ return fmt.Errorf("manager: %w", err)
+ }
step := &backup.Steps[len(backup.Steps)-1]
@@ -285,7 +303,9 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error {
return fmt.Errorf("manager: %w", err)
}
- if err := repo.Create(ctx, hash); err != nil {
+ defaultBranch, defaultBranchKnown := git.ReferenceName(backup.HeadReference).Branch()
+
+ if err := repo.Create(ctx, hash, defaultBranch); err != nil {
return fmt.Errorf("manager: %w", err)
}
@@ -315,7 +335,7 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error {
// Git bundles can not be created for empty repositories. Since empty
// repository backups do not contain a bundle, skip bundle restoration.
if len(refs) > 0 {
- if err := mgr.restoreBundle(ctx, repo, step.BundlePath); err != nil {
+ if err := mgr.restoreBundle(ctx, repo, step.BundlePath, !defaultBranchKnown); err != nil {
return fmt.Errorf("manager: %w", err)
}
}
@@ -462,14 +482,14 @@ func (mgr *Manager) readRefs(ctx context.Context, path string) ([]git.Reference,
return refs, nil
}
-func (mgr *Manager) restoreBundle(ctx context.Context, repo Repository, path string) error {
+func (mgr *Manager) restoreBundle(ctx context.Context, repo Repository, path string, updateHead bool) error {
reader, err := mgr.sink.GetReader(ctx, path)
if err != nil {
return fmt.Errorf("restore bundle: %q: %w", path, err)
}
defer reader.Close()
- if err := repo.FetchBundle(ctx, reader); err != nil {
+ if err := repo.FetchBundle(ctx, reader, updateHead); err != nil {
return fmt.Errorf("restore bundle: %q: %w", path, err)
}
return nil
diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index 13d1cb918..14341ec6a 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -109,9 +109,16 @@ func TestManager_Create(t *testing.T) {
},
},
} {
+
+ type setupData struct {
+ repo *gitalypb.Repository
+ repoPath string
+ expectedBackup *backup.Backup
+ }
+
for _, tc := range []struct {
desc string
- setup func(tb testing.TB) (*gitalypb.Repository, string)
+ setup func(tb testing.TB, vanityRepo storage.Repository) setupData
createsRefList bool
createsBundle bool
createsCustomHooks bool
@@ -119,10 +126,27 @@ func TestManager_Create(t *testing.T) {
}{
{
desc: "no hooks",
- setup: func(tb testing.TB) (*gitalypb.Repository, string) {
+ setup: func(tb testing.TB, vanityRepo storage.Repository) setupData {
repo, repoPath := gittest.CreateRepository(tb, ctx, cfg)
gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch))
- return repo, repoPath
+
+ return setupData{
+ repo: repo,
+ repoPath: repoPath,
+ expectedBackup: &backup.Backup{
+ ID: backupID,
+ Repository: vanityRepo,
+ ObjectFormat: gittest.DefaultObjectHash.Format,
+ HeadReference: git.DefaultRef.String(),
+ Steps: []backup.Step{
+ {
+ BundlePath: joinBackupPath(t, "", vanityRepo, backupID, "001.bundle"),
+ RefPath: joinBackupPath(t, "", vanityRepo, backupID, "001.refs"),
+ CustomHooksPath: joinBackupPath(t, "", vanityRepo, backupID, "001.custom_hooks.tar"),
+ },
+ },
+ },
+ }
},
createsRefList: true,
createsBundle: true,
@@ -130,12 +154,29 @@ func TestManager_Create(t *testing.T) {
},
{
desc: "hooks",
- setup: func(tb testing.TB) (*gitalypb.Repository, string) {
+ setup: func(tb testing.TB, vanityRepo storage.Repository) setupData {
repo, repoPath := gittest.CreateRepository(tb, ctx, cfg)
gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch))
require.NoError(tb, os.Mkdir(filepath.Join(repoPath, "custom_hooks"), perm.PublicDir))
require.NoError(tb, os.WriteFile(filepath.Join(repoPath, "custom_hooks/pre-commit.sample"), []byte("Some hooks"), perm.PublicFile))
- return repo, repoPath
+
+ return setupData{
+ repo: repo,
+ repoPath: repoPath,
+ expectedBackup: &backup.Backup{
+ ID: backupID,
+ Repository: vanityRepo,
+ ObjectFormat: gittest.DefaultObjectHash.Format,
+ HeadReference: git.DefaultRef.String(),
+ Steps: []backup.Step{
+ {
+ BundlePath: joinBackupPath(t, "", vanityRepo, backupID, "001.bundle"),
+ RefPath: joinBackupPath(t, "", vanityRepo, backupID, "001.refs"),
+ CustomHooksPath: joinBackupPath(t, "", vanityRepo, backupID, "001.custom_hooks.tar"),
+ },
+ },
+ },
+ }
},
createsRefList: true,
createsBundle: true,
@@ -143,9 +184,26 @@ func TestManager_Create(t *testing.T) {
},
{
desc: "empty repo",
- setup: func(tb testing.TB) (*gitalypb.Repository, string) {
+ setup: func(tb testing.TB, vanityRepo storage.Repository) setupData {
emptyRepo, repoPath := gittest.CreateRepository(tb, ctx, cfg)
- return emptyRepo, repoPath
+
+ return setupData{
+ repo: emptyRepo,
+ repoPath: repoPath,
+ expectedBackup: &backup.Backup{
+ ID: backupID,
+ Repository: vanityRepo,
+ ObjectFormat: gittest.DefaultObjectHash.Format,
+ HeadReference: git.DefaultRef.String(),
+ Steps: []backup.Step{
+ {
+ BundlePath: joinBackupPath(t, "", vanityRepo, backupID, "001.bundle"),
+ RefPath: joinBackupPath(t, "", vanityRepo, backupID, "001.refs"),
+ CustomHooksPath: joinBackupPath(t, "", vanityRepo, backupID, "001.custom_hooks.tar"),
+ },
+ },
+ },
+ }
},
createsRefList: true,
createsBundle: false,
@@ -153,11 +211,15 @@ func TestManager_Create(t *testing.T) {
},
{
desc: "nonexistent repo",
- setup: func(tb testing.TB) (*gitalypb.Repository, string) {
+ setup: func(tb testing.TB, vanityRepo storage.Repository) setupData {
emptyRepo, repoPath := gittest.CreateRepository(tb, ctx, cfg)
nonexistentRepo := proto.Clone(emptyRepo).(*gitalypb.Repository)
nonexistentRepo.RelativePath = gittest.NewRepositoryName(t)
- return nonexistentRepo, repoPath
+
+ return setupData{
+ repo: nonexistentRepo,
+ repoPath: repoPath,
+ }
},
createsRefList: false,
createsBundle: false,
@@ -166,13 +228,14 @@ func TestManager_Create(t *testing.T) {
},
} {
t.Run(tc.desc, func(t *testing.T) {
- repo, repoPath := tc.setup(t)
backupRoot := testhelper.TempDir(t)
vanityRepo := &gitalypb.Repository{
RelativePath: "some/path.git",
StorageName: "some_storage",
}
+ data := tc.setup(t, vanityRepo)
+
manifestPath := filepath.Join(backupRoot, "manifests", vanityRepo.StorageName, vanityRepo.RelativePath, backupID+".toml")
refsPath := joinBackupPath(t, backupRoot, vanityRepo, backupID, "001.refs")
bundlePath := joinBackupPath(t, backupRoot, vanityRepo, backupID, "001.bundle")
@@ -187,7 +250,7 @@ func TestManager_Create(t *testing.T) {
fsBackup := managerTC.setup(t, sink, locator)
err = fsBackup.Create(ctx, &backup.CreateRequest{
Server: storage.ServerInfo{Address: cfg.SocketPath, Token: cfg.Auth.Token},
- Repository: repo,
+ Repository: data.repo,
VanityRepository: vanityRepo,
BackupID: backupID,
})
@@ -209,10 +272,10 @@ func TestManager_Create(t *testing.T) {
require.NoError(t, err)
require.Equal(t, perm.PrivateFile, bundleInfo.Mode().Perm(), "expecting restricted file permissions")
- output := gittest.Exec(t, cfg, "-C", repoPath, "bundle", "verify", bundlePath)
+ output := gittest.Exec(t, cfg, "-C", data.repoPath, "bundle", "verify", bundlePath)
require.Contains(t, string(output), "The bundle records a complete history")
- expectedRefs := gittest.Exec(t, cfg, "-C", repoPath, "show-ref", "--head")
+ expectedRefs := gittest.Exec(t, cfg, "-C", data.repoPath, "show-ref", "--head")
actualRefs := testhelper.MustReadFile(t, refsPath)
require.Equal(t, string(expectedRefs), string(actualRefs))
} else {
@@ -236,6 +299,15 @@ func TestManager_Create(t *testing.T) {
} else {
require.NoFileExists(t, customHooksPath)
}
+
+ if data.expectedBackup == nil {
+ _, err := locator.Find(ctx, vanityRepo, backupID)
+ require.ErrorIs(t, err, backup.ErrDoesntExist)
+ } else {
+ backup, err := locator.Find(ctx, vanityRepo, backupID)
+ require.NoError(t, err)
+ require.Equal(t, data.expectedBackup, backup)
+ }
})
}
}
@@ -730,8 +802,6 @@ func TestManager_Restore_latest(t *testing.T) {
}
func TestManager_Restore_specific(t *testing.T) {
- gittest.SkipWithSHA256(t)
-
t.Parallel()
const backupID = "abc123"
@@ -796,17 +866,18 @@ func TestManager_Restore_specific(t *testing.T) {
backupRoot := testhelper.TempDir(t)
for _, tc := range []struct {
- desc string
- setup func(tb testing.TB) (*gitalypb.Repository, *git.Checksum)
- alwaysCreate bool
- expectExists bool
- expectedPaths []string
- expectedErrAs error
+ desc string
+ setup func(tb testing.TB) (*gitalypb.Repository, *git.Checksum)
+ alwaysCreate bool
+ expectExists bool
+ expectedPaths []string
+ expectedErrAs error
+ expectedHeadReference git.ReferenceName
}{
{
desc: "missing backup",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
- repo, _ := gittest.CreateRepository(t, ctx, cfg)
+ repo, _ := gittest.CreateRepository(tb, ctx, cfg)
return repo, nil
},
@@ -815,7 +886,9 @@ func TestManager_Restore_specific(t *testing.T) {
{
desc: "single incremental",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
- repo, _ := gittest.CreateRepository(t, ctx, cfg)
+ gittest.SkipWithSHA256(tb) // sha256 only works with manifest files
+
+ repo, _ := gittest.CreateRepository(tb, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
@@ -832,9 +905,11 @@ func TestManager_Restore_specific(t *testing.T) {
{
desc: "many incrementals",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
- _, expectedRepoPath := gittest.CreateRepository(t, ctx, cfg)
+ gittest.SkipWithSHA256(tb) // sha256 only works with manifest files
- repo, _ := gittest.CreateRepository(t, ctx, cfg)
+ _, expectedRepoPath := gittest.CreateRepository(tb, ctx, cfg)
+
+ repo, _ := gittest.CreateRepository(tb, ctx, cfg)
root := gittest.WriteCommit(tb, cfg, expectedRepoPath,
gittest.WithBranch("master"),
@@ -853,7 +928,7 @@ func TestManager_Restore_specific(t *testing.T) {
"refs/heads/master",
"refs/heads/other",
)
- refs1 := gittest.Exec(t, cfg, "-C", expectedRepoPath, "show-ref", "--head")
+ refs1 := gittest.Exec(tb, cfg, "-C", expectedRepoPath, "show-ref", "--head")
master2 := gittest.WriteCommit(tb, cfg, expectedRepoPath,
gittest.WithBranch("master"),
@@ -866,7 +941,7 @@ func TestManager_Restore_specific(t *testing.T) {
"refs/heads/master",
"refs/heads/other",
)
- refs2 := gittest.Exec(t, cfg, "-C", expectedRepoPath, "show-ref", "--head")
+ refs2 := gittest.Exec(tb, cfg, "-C", expectedRepoPath, "show-ref", "--head")
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
@@ -887,6 +962,59 @@ func TestManager_Restore_specific(t *testing.T) {
},
expectExists: true,
},
+ {
+ desc: "manifest, empty backup",
+ setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
+ repo, _ := gittest.CreateRepository(tb, ctx, cfg)
+
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), backupID+".toml"): fmt.Sprintf(
+ `object_format = %q
+head_reference = 'refs/heads/banana'
+
+[[steps]]
+bundle_path = 'repo.bundle'
+ref_path = 'repo.refs'
+custom_hooks_path = 'custom_hooks.tar'
+`, gittest.DefaultObjectHash.Format),
+ "repo.refs": "",
+ })
+
+ return repo, new(git.Checksum)
+ },
+ expectExists: true,
+ expectedHeadReference: "refs/heads/banana",
+ },
+ {
+ desc: "manifest",
+ setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
+ repo, _ := gittest.CreateRepository(tb, ctx, cfg)
+
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), backupID+".toml"): fmt.Sprintf(
+ `object_format = %q
+head_reference = 'refs/heads/banana'
+
+[[steps]]
+bundle_path = 'repo.bundle'
+ref_path = 'repo.refs'
+custom_hooks_path = 'custom_hooks.tar'
+`, gittest.DefaultObjectHash.Format),
+ "repo.bundle": repoBundle,
+ "repo.refs": repoRefs,
+ })
+
+ checksum := gittest.ChecksumRepo(tb, cfg, repoPath)
+ // Negate off the default branch since the manifest is
+ // explicitly setting a different unborn branch that
+ // will not be part of the checksum.
+ checksum.Add(git.NewReference("HEAD", commitID))
+
+ return repo, checksum
+ },
+ expectExists: true,
+ expectedHeadReference: "refs/heads/banana",
+ },
} {
t.Run(tc.desc, func(t *testing.T) {
repo, expectedChecksum := tc.setup(t)
@@ -931,10 +1059,18 @@ func TestManager_Restore_specific(t *testing.T) {
// the repository through Praefect. In order to get to the correct disk paths, we need
// to get the replica path of the rewritten repository.
repoPath := filepath.Join(cfg.Storages[0].Path, gittest.GetReplicaPath(t, ctx, cfg, repo))
+
for _, p := range tc.expectedPaths {
require.FileExists(t, filepath.Join(repoPath, p))
}
}
+
+ if len(tc.expectedHeadReference) > 0 {
+ repoPath := filepath.Join(cfg.Storages[0].Path, gittest.GetReplicaPath(t, ctx, cfg, repo))
+
+ ref := gittest.GetSymbolicRef(t, cfg, repoPath, "HEAD")
+ require.Equal(t, tc.expectedHeadReference, git.ReferenceName(ref.Target))
+ }
})
}
})
@@ -1011,14 +1147,14 @@ func TestResolveLocator(t *testing.T) {
}
}
-func joinBackupPath(tb testing.TB, backupRoot string, repo *gitalypb.Repository, elements ...string) string {
+func joinBackupPath(tb testing.TB, backupRoot string, repo storage.Repository, elements ...string) string {
return filepath.Join(append([]string{
backupRoot,
stripRelativePath(tb, repo),
}, elements...)...)
}
-func stripRelativePath(tb testing.TB, repo *gitalypb.Repository) string {
+func stripRelativePath(tb testing.TB, repo storage.Repository) string {
return strings.TrimSuffix(repo.GetRelativePath(), ".git")
}
diff --git a/internal/backup/repository.go b/internal/backup/repository.go
index ca28a8ef3..beef0c3d8 100644
--- a/internal/backup/repository.go
+++ b/internal/backup/repository.go
@@ -209,11 +209,12 @@ func (rr *remoteRepository) Remove(ctx context.Context) error {
}
// Create creates the repository.
-func (rr *remoteRepository) Create(ctx context.Context, hash git.ObjectHash) error {
+func (rr *remoteRepository) Create(ctx context.Context, hash git.ObjectHash, defaultBranch string) error {
repoClient := rr.newRepoClient()
if _, err := repoClient.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{
- Repository: rr.repo,
- ObjectFormat: hash.ProtoFormat,
+ Repository: rr.repo,
+ ObjectFormat: hash.ProtoFormat,
+ DefaultBranch: []byte(defaultBranch),
}); err != nil {
return fmt.Errorf("remote repository: create: %w", err)
}
@@ -222,13 +223,13 @@ func (rr *remoteRepository) Create(ctx context.Context, hash git.ObjectHash) err
// FetchBundle fetches references from a bundle. Refs will be mirrored to the
// repository.
-func (rr *remoteRepository) FetchBundle(ctx context.Context, reader io.Reader) error {
+func (rr *remoteRepository) FetchBundle(ctx context.Context, reader io.Reader, updateHead bool) error {
repoClient := rr.newRepoClient()
stream, err := repoClient.FetchBundle(ctx)
if err != nil {
return fmt.Errorf("remote repository: fetch bundle: %w", err)
}
- request := &gitalypb.FetchBundleRequest{Repository: rr.repo, UpdateHead: true}
+ request := &gitalypb.FetchBundleRequest{Repository: rr.repo, UpdateHead: updateHead}
bundle := streamio.NewWriter(func(p []byte) error {
request.Data = p
if err := stream.Send(request); err != nil {
@@ -278,6 +279,35 @@ func (rr *remoteRepository) SetCustomHooks(ctx context.Context, reader io.Reader
return nil
}
+// ObjectHash detects the object hash used by the repository.
+func (rr *remoteRepository) ObjectHash(ctx context.Context) (git.ObjectHash, error) {
+ repoClient := rr.newRepoClient()
+
+ response, err := repoClient.ObjectFormat(ctx, &gitalypb.ObjectFormatRequest{
A merge request may have already been submitted, check that on the merge commit, as usually changes in Gitaly are not breaking, compliance becoming mandatory in the following development cycles.