Inspect changes in Gitaly 5b408a9dd8a93cc47a0bd561067991880f984c1e
Upstream merge commit 4c8609dcd440 changes Gitaly server version from 89d7991b2b72d79526bbf5abb6b05c4da4bd5d22 to 5b408a9dd8a93cc47a0bd561067991880f984c1e, with changes in protocol or one of the source subtrees currently under watch.
The topic upstream-merge
is at 177 non-merge commits.
To further inspect, go to an up to date Gitaly source clone and run:
git diff 89d7991b2b72d79526bbf5abb6b05c4da4bd5d22..5b408a9dd8a93cc47a0bd561067991880f984c1e -- proto/*.proto internal/backup
Here is a preview, truncated at 500 lines:
diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index 96a06fba3..42e2d3c39 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -556,8 +556,6 @@ func TestManager_Create_incremental(t *testing.T) {
}
func TestManager_Restore_latest(t *testing.T) {
- gittest.SkipWithSHA256(t)
-
t.Parallel()
cfg := testcfg.Build(t)
@@ -621,7 +619,6 @@ func TestManager_Restore_latest(t *testing.T) {
for _, tc := range []struct {
desc string
- locators []string
setup func(tb testing.TB) (*gitalypb.Repository, *git.Checksum)
alwaysCreate bool
expectExists bool
@@ -629,13 +626,20 @@ func TestManager_Restore_latest(t *testing.T) {
expectedErrAs error
}{
{
- desc: "existing repo, without hooks",
- locators: []string{"legacy", "pointer"},
+ desc: "existing repo, without hooks",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), "+latest.toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+bundle_path = '%[2]s.bundle'
+ref_path = '%[2]s.refs'
+custom_hooks_path = '%[2]s/custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath),
relativePath + ".bundle": repoBundle,
relativePath + ".refs": repoRefs,
})
@@ -645,14 +649,21 @@ func TestManager_Restore_latest(t *testing.T) {
expectExists: true,
},
{
- desc: "existing repo, with hooks",
- locators: []string{"legacy", "pointer"},
+ desc: "existing repo, with hooks",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
customHooksPath := filepath.Join(backupRoot, relativePath, "custom_hooks.tar")
testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), "+latest.toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+bundle_path = '%[2]s.bundle'
+ref_path = '%[2]s.refs'
+custom_hooks_path = '%[2]s/custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath),
relativePath + ".bundle": repoBundle,
relativePath + ".refs": repoRefs,
})
@@ -670,8 +681,7 @@ func TestManager_Restore_latest(t *testing.T) {
expectExists: true,
},
{
- desc: "missing backup",
- locators: []string{"legacy", "pointer"},
+ desc: "missing backup",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
return repo, nil
@@ -679,8 +689,7 @@ func TestManager_Restore_latest(t *testing.T) {
expectedErrAs: backup.ErrSkipped,
},
{
- desc: "missing backup, always create",
- locators: []string{"legacy", "pointer"},
+ desc: "missing backup, always create",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
return repo, new(git.Checksum)
@@ -689,13 +698,19 @@ func TestManager_Restore_latest(t *testing.T) {
expectExists: true,
},
{
- desc: "empty backup",
- locators: []string{"legacy", "pointer"},
+ desc: "empty backup",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), "+latest.toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+ref_path = '%[2]s.refs'
+custom_hooks_path = '%[2]s/custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath),
relativePath + ".refs": "",
})
@@ -704,13 +719,19 @@ func TestManager_Restore_latest(t *testing.T) {
expectExists: true,
},
{
- desc: "empty backup, always create",
- locators: []string{"legacy", "pointer"},
+ desc: "empty backup, always create",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), "+latest.toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+ref_path = '%[2]s.refs'
+custom_hooks_path = '%[2]s/custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath),
relativePath + ".refs": "",
})
@@ -720,8 +741,7 @@ func TestManager_Restore_latest(t *testing.T) {
expectExists: true,
},
{
- desc: "nonexistent repo",
- locators: []string{"legacy", "pointer"},
+ desc: "nonexistent repo",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo := &gitalypb.Repository{
StorageName: "default",
@@ -730,6 +750,14 @@ func TestManager_Restore_latest(t *testing.T) {
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), "+latest.toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+bundle_path = '%[2]s.bundle'
+ref_path = '%[2]s.refs'
+custom_hooks_path = '%[2]s/custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath),
relativePath + ".bundle": repoBundle,
relativePath + ".refs": repoRefs,
})
@@ -739,45 +767,7 @@ func TestManager_Restore_latest(t *testing.T) {
expectExists: true,
},
{
- desc: "single incremental",
- locators: []string{"pointer"},
- setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
- const backupID = "abc123"
- repo, _ := gittest.CreateRepository(t, ctx, cfg)
-
- relativePath := stripRelativePath(tb, repo)
- testhelper.WriteFiles(tb, backupRoot, map[string]any{
- filepath.Join(relativePath, "LATEST"): backupID,
- filepath.Join(relativePath, backupID, "LATEST"): "001",
- filepath.Join(relativePath, backupID, "001.bundle"): repoBundle,
- filepath.Join(relativePath, backupID, "001.refs"): repoRefs,
- })
-
- return repo, repoChecksum
- },
- expectExists: true,
- },
- {
- desc: "single incremental, empty backup",
- locators: []string{"pointer"},
- setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
- const backupID = "abc123"
- repo, _ := gittest.CreateRepository(t, ctx, cfg)
-
- relativePath := stripRelativePath(tb, repo)
- testhelper.WriteFiles(tb, backupRoot, map[string]any{
- filepath.Join(relativePath, "LATEST"): backupID,
- filepath.Join(relativePath, backupID, "LATEST"): "001",
- filepath.Join(relativePath, backupID, "001.refs"): "",
- })
-
- return repo, new(git.Checksum)
- },
- expectExists: true,
- },
- {
- desc: "many incrementals",
- locators: []string{"pointer"},
+ desc: "many incrementals",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
const backupID = "abc123"
@@ -819,8 +809,19 @@ func TestManager_Restore_latest(t *testing.T) {
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
- filepath.Join(relativePath, "LATEST"): backupID,
- filepath.Join(relativePath, backupID, "LATEST"): "002",
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), "+latest.toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+bundle_path = '%[2]s/%[3]s/001.bundle'
+ref_path = '%[2]s/%[3]s/001.refs'
+custom_hooks_path = '%[2]s/%[3]s/001.custom_hooks.tar'
+
+[[steps]]
+bundle_path = '%[2]s/%[3]s/002.bundle'
+ref_path = '%[2]s/%[3]s/002.refs'
+custom_hooks_path = '%[2]s/%[3]s/002.custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath, backupID),
filepath.Join(relativePath, backupID, "001.bundle"): bundle1,
filepath.Join(relativePath, backupID, "002.bundle"): bundle2,
filepath.Join(relativePath, backupID, "001.refs"): refs1,
@@ -838,57 +839,51 @@ func TestManager_Restore_latest(t *testing.T) {
},
} {
t.Run(tc.desc, func(t *testing.T) {
- require.GreaterOrEqual(t, len(tc.locators), 1, "each test case must specify a locator")
-
- for _, locatorName := range tc.locators {
- t.Run(locatorName, func(t *testing.T) {
- repo, expectedChecksum := tc.setup(t)
-
- sink := backup.NewFilesystemSink(backupRoot)
- defer testhelper.MustClose(t, sink)
-
- locator, err := backup.ResolveLocator(locatorName, sink)
- require.NoError(t, err)
-
- fsBackup := managerTC.setup(t, sink, locator)
- err = fsBackup.Restore(ctx, &backup.RestoreRequest{
- Server: storage.ServerInfo{Address: cfg.SocketPath, Token: cfg.Auth.Token},
- Repository: repo,
- VanityRepository: repo,
- AlwaysCreate: tc.alwaysCreate,
- BackupID: "",
- })
- if tc.expectedErrAs != nil {
- require.ErrorAs(t, err, &tc.expectedErrAs)
- } else {
- require.NoError(t, err)
- }
-
- exists, err := repoClient.RepositoryExists(ctx, &gitalypb.RepositoryExistsRequest{
- Repository: repo,
- })
- require.NoError(t, err)
- require.Equal(t, tc.expectExists, exists.Exists, "repository exists")
-
- if expectedChecksum != nil {
- checksum, err := repoClient.CalculateChecksum(ctx, &gitalypb.CalculateChecksumRequest{
- Repository: repo,
- })
- require.NoError(t, err)
-
- require.Equal(t, expectedChecksum.String(), checksum.GetChecksum())
- }
-
- if len(tc.expectedPaths) > 0 {
- // Restore has to use the rewritten path as the relative path due to the test creating
- // 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))
- }
- }
+ repo, expectedChecksum := tc.setup(t)
+
+ sink := backup.NewFilesystemSink(backupRoot)
+ defer testhelper.MustClose(t, sink)
+
+ locator, err := backup.ResolveLocator("pointer", sink)
+ require.NoError(t, err)
+
+ fsBackup := managerTC.setup(t, sink, locator)
+ err = fsBackup.Restore(ctx, &backup.RestoreRequest{
+ Server: storage.ServerInfo{Address: cfg.SocketPath, Token: cfg.Auth.Token},
+ Repository: repo,
+ VanityRepository: repo,
+ AlwaysCreate: tc.alwaysCreate,
+ BackupID: "",
+ })
+ if tc.expectedErrAs != nil {
+ require.ErrorAs(t, err, &tc.expectedErrAs)
+ } else {
+ require.NoError(t, err)
+ }
+
+ exists, err := repoClient.RepositoryExists(ctx, &gitalypb.RepositoryExistsRequest{
+ Repository: repo,
+ })
+ require.NoError(t, err)
+ require.Equal(t, tc.expectExists, exists.Exists, "repository exists")
+
+ if expectedChecksum != nil {
+ checksum, err := repoClient.CalculateChecksum(ctx, &gitalypb.CalculateChecksumRequest{
+ Repository: repo,
})
+ require.NoError(t, err)
+
+ require.Equal(t, expectedChecksum.String(), checksum.GetChecksum())
+ }
+
+ if len(tc.expectedPaths) > 0 {
+ // Restore has to use the rewritten path as the relative path due to the test creating
+ // 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))
+ }
}
})
}
@@ -982,14 +977,18 @@ func TestManager_Restore_specific(t *testing.T) {
{
desc: "single incremental",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
- 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{
- filepath.Join(relativePath, "LATEST"): backupID,
- filepath.Join(relativePath, backupID, "LATEST"): "001",
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), backupID+".toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+bundle_path = '%[2]s/%[3]s/001.bundle'
+ref_path = '%[2]s/%[3]s/001.refs'
+custom_hooks_path = '%[2]s/%[3]s/001.custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath, backupID),
filepath.Join(relativePath, backupID, "001.bundle"): repoBundle,
filepath.Join(relativePath, backupID, "001.refs"): repoRefs,
})
@@ -1001,8 +1000,6 @@ func TestManager_Restore_specific(t *testing.T) {
{
desc: "many incrementals",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
- gittest.SkipWithSHA256(tb) // sha256 only works with manifest files
-
_, expectedRepoPath := gittest.CreateRepository(tb, ctx, cfg)
repo, _ := gittest.CreateRepository(tb, ctx, cfg)
@@ -1041,8 +1038,19 @@ func TestManager_Restore_specific(t *testing.T) {
relativePath := stripRelativePath(tb, repo)
testhelper.WriteFiles(tb, backupRoot, map[string]any{
- filepath.Join(relativePath, "LATEST"): backupID,
- filepath.Join(relativePath, backupID, "LATEST"): "002",
+ filepath.Join("manifests", repo.GetStorageName(), repo.GetRelativePath(), backupID+".toml"): fmt.Sprintf(`
+object_format = '%[1]s'
+
+[[steps]]
+bundle_path = '%[2]s/%[3]s/001.bundle'
+ref_path = '%[2]s/%[3]s/001.refs'
+custom_hooks_path = '%[2]s/%[3]s/001.custom_hooks.tar'
+
+[[steps]]
+bundle_path = '%[2]s/%[3]s/002.bundle'
+ref_path = '%[2]s/%[3]s/002.refs'
+custom_hooks_path = '%[2]s/%[3]s/002.custom_hooks.tar'
+ `, gittest.DefaultObjectHash.Format, relativePath, backupID),
filepath.Join(relativePath, backupID, "001.bundle"): bundle1,
filepath.Join(relativePath, backupID, "002.bundle"): bundle2,
filepath.Join(relativePath, backupID, "001.refs"): refs1,
@@ -1059,7 +1067,7 @@ func TestManager_Restore_specific(t *testing.T) {
expectExists: true,
},
{
- desc: "manifest, empty backup",
+ desc: "empty backup",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(tb, ctx, cfg)
@@ -1082,7 +1090,7 @@ custom_hooks_path = 'custom_hooks.tar'
expectedHeadReference: "refs/heads/banana",
},
{
- desc: "manifest",
+ desc: "head reference",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(tb, ctx, cfg)
@@ -1174,8 +1182,6 @@ custom_hooks_path = 'custom_hooks.tar'
}
func TestManager_CreateRestore_contextServerInfo(t *testing.T) {
- gittest.SkipWithSHA256(t)
-
t.Parallel()
cfg := testcfg.Build(t)
@@ -1213,8 +1219,6 @@ func TestManager_CreateRestore_contextServerInfo(t *testing.T) {
}
func TestResolveLocator(t *testing.T) {
- gittest.SkipWithSHA256(t)
-
t.Parallel()
for _, tc := range []struct {
diff --git a/internal/backup/server_side_test.go b/internal/backup/server_side_test.go
index 69669df60..eedbed3cd 100644
--- a/internal/backup/server_side_test.go
+++ b/internal/backup/server_side_test.go
@@ -23,8 +23,6 @@ import (
)
func TestServerSideAdapter_Create(t *testing.T) {
- gittest.SkipWithSHA256(t)
-
t.Parallel()
ctx := testhelper.Context(t)
@@ -148,8 +146,6 @@ func TestServerSideAdapter_Create(t *testing.T) {
}
func TestServerSideAdapter_Restore(t *testing.T) {
- gittest.SkipWithSHA256(t)
-
t.Parallel()
ctx := testhelper.Context(t)
@@ -191,6 +187,8 @@ func TestServerSideAdapter_Restore(t *testing.T) {
require.NoError(t, err)
require.NoError(t, w.Close())
+ backup.ObjectFormat = gittest.DefaultObjectHash.Format
+
require.NoError(t, backupLocator.Commit(ctx, backup))
return setupData{
diff --git a/proto/cleanup.proto b/proto/cleanup.proto
index 77cf6c6a3..94fff3ae0 100644
--- a/proto/cleanup.proto
+++ b/proto/cleanup.proto
@@ -27,10 +27,11 @@ service CleanupService {
// - `InvalidArgument` in the following situations:
// - The provided repository can't be validated.
// - The repository field is set on any request other than the initial one.
- // - Any request, including the initial one, does not contain either blobs to
- // remove or redaction patterns to redact.
+ // - All of the client requests do not contain either blobs to remove or
+ // redaction patterns to redact.
// - A blob object ID is invalid.
// - A redaction pattern contains a newline character.
+ // - `Aborted` if the repository is mutated while this RPC is executing.
rpc RewriteHistory(stream RewriteHistoryRequest) returns (RewriteHistoryResponse) {
option (op_type) = {
op : MUTATOR
@@ -69,7 +70,6 @@ message ApplyBfgObjectMapStreamResponse {
}
// RewriteHistoryRequest is a request for the RewriteHistory RPC.
-// Each request must contain blobs, redactions, or both.
message RewriteHistoryRequest {
// repository is the repository that shall be rewritten.
// Must be sent on the first request only.
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.