diff options
author | Pavlo Strokov <pstrokov@gitlab.com> | 2022-07-25 11:27:39 +0300 |
---|---|---|
committer | Pavlo Strokov <pstrokov@gitlab.com> | 2022-07-25 11:27:39 +0300 |
commit | 17a3fc6cae47ba8b9c96250bfb38441350c9ee8b (patch) | |
tree | 3f7ad2271861f85024d541ec74e2e5c0a31ebbec | |
parent | 4da75e5814680fe0d657bb734099527c74b76905 (diff) | |
parent | 4a789524c7a786a2c8fb0019c3ac20a66c1f9431 (diff) |
Merge branch 'pks-git-dont-advertise-hidden-refs' into 'master'
git: Don't advertise internal references via git-upload-pack(1)
Closes #4358
See merge request gitlab-org/gitaly!4727
-rw-r--r-- | internal/git/command_description.go | 66 | ||||
-rw-r--r-- | internal/git/gittest/pktline.go | 14 | ||||
-rw-r--r-- | internal/git/reference.go | 43 | ||||
-rw-r--r-- | internal/gitaly/service/cleanup/internalrefs/cleaner.go | 7 | ||||
-rw-r--r-- | internal/gitaly/service/repository/replicate_test.go | 83 | ||||
-rw-r--r-- | internal/gitaly/service/repository/size.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/smarthttp/inforefs_test.go | 194 |
7 files changed, 311 insertions, 100 deletions
diff --git a/internal/git/command_description.go b/internal/git/command_description.go index a56580e0b..a04746913 100644 --- a/internal/git/command_description.go +++ b/internal/git/command_description.go @@ -2,7 +2,6 @@ package git import ( "fmt" - "log" "math" "runtime" "strings" @@ -299,12 +298,12 @@ var commandDescriptions = map[string]commandDescription{ }, "upload-pack": { flags: scNoRefUpdates, - opts: append([]GlobalOption{ + opts: append(append([]GlobalOption{ ConfigPair{Key: "uploadpack.allowFilter", Value: "true"}, // Enables the capability to request individual SHA1's from the // remote repo. ConfigPair{Key: "uploadpack.allowAnySHA1InWant", Value: "true"}, - }, packConfiguration()...), + }, hiddenUploadPackRefPrefixes()...), packConfiguration()...), }, "version": { flags: scNoRefUpdates, @@ -314,34 +313,6 @@ var commandDescriptions = map[string]commandDescription{ }, } -func init() { - // This is the poor-mans static assert that all internal ref prefixes are properly hidden - // from git-receive-pack(1) such that they cannot be written to when the user pushes. - receivePackDesc, ok := commandDescriptions["receive-pack"] - if !ok { - log.Fatal("could not find command description of git-receive-pack(1)") - } - - hiddenRefs := map[string]bool{} - for _, opt := range receivePackDesc.opts { - configPair, ok := opt.(ConfigPair) - if !ok { - continue - } - if configPair.Key != "receive.hideRefs" { - continue - } - - hiddenRefs[configPair.Value] = true - } - - for _, internalRef := range InternalRefPrefixes { - if !hiddenRefs[internalRef] { - log.Fatalf("command description of receive-pack is missing hidden ref %q", internalRef) - } - } -} - // mayUpdateRef indicates if a command is known to update references. // This is useful to determine if a command requires reference hook // configuration. A non-exhaustive list of commands is consulted to determine if @@ -404,13 +375,38 @@ func validatePositionalArg(arg string) error { } func hiddenReceivePackRefPrefixes() []GlobalOption { - var cps []GlobalOption + config := make([]GlobalOption, 0, len(InternalRefPrefixes)) + + for refPrefix, refType := range InternalRefPrefixes { + switch refType { + case InternalReferenceTypeReadonly, InternalReferenceTypeHidden: + // We want to hide both read-only and hidden refs in git-receive-pack(1) so + // that we make neither of them writeable. + config = append(config, ConfigPair{Key: "receive.hideRefs", Value: refPrefix}) + default: + panic(fmt.Sprintf("unhandled internal reference type: %v", refType)) + } + } + + return config +} - for _, ns := range InternalRefPrefixes { - cps = append(cps, ConfigPair{Key: "receive.hideRefs", Value: ns}) +func hiddenUploadPackRefPrefixes() []GlobalOption { + config := make([]GlobalOption, 0, len(InternalRefPrefixes)) + + for refPrefix, refType := range InternalRefPrefixes { + switch refType { + case InternalReferenceTypeHidden: + config = append(config, ConfigPair{Key: "uploadpack.hideRefs", Value: refPrefix}) + case InternalReferenceTypeReadonly: + // git-upload-pack(1) doesn't allow writing references, and we do want to + // announce read-only references that aren't hidden. + default: + panic(fmt.Sprintf("unhandled internal reference type: %v", refType)) + } } - return cps + return config } // fsckConfiguration generates our fsck configuration, including ignored checks. The prefix must diff --git a/internal/git/gittest/pktline.go b/internal/git/gittest/pktline.go index e198f01e0..622719b34 100644 --- a/internal/git/gittest/pktline.go +++ b/internal/git/gittest/pktline.go @@ -3,14 +3,25 @@ package gittest import ( "fmt" "io" + "strings" "testing" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" ) +// Pktlinef formats the given formatting string into a pktline. +func Pktlinef(t *testing.T, format string, a ...interface{}) string { + t.Helper() + var builder strings.Builder + _, err := pktline.WriteString(&builder, fmt.Sprintf(format, a...)) + require.NoError(t, err) + return builder.String() +} + // WritePktlineString writes the pktline-formatted data into the writer. func WritePktlineString(t *testing.T, writer io.Writer, data string) { + t.Helper() _, err := pktline.WriteString(writer, data) require.NoError(t, err) } @@ -18,16 +29,19 @@ func WritePktlineString(t *testing.T, writer io.Writer, data string) { // WritePktlinef formats the given format string and writes the pktline-formatted data into the // writer. func WritePktlinef(t *testing.T, writer io.Writer, format string, args ...interface{}) { + t.Helper() _, err := pktline.WriteString(writer, fmt.Sprintf(format, args...)) require.NoError(t, err) } // WritePktlineFlush writes the pktline-formatted flush into the writer. func WritePktlineFlush(t *testing.T, writer io.Writer) { + t.Helper() require.NoError(t, pktline.WriteFlush(writer)) } // WritePktlineDelim writes the pktline-formatted delimiter into the writer. func WritePktlineDelim(t *testing.T, writer io.Writer) { + t.Helper() require.NoError(t, pktline.WriteDelim(writer)) } diff --git a/internal/git/reference.go b/internal/git/reference.go index c63230b06..225231188 100644 --- a/internal/git/reference.go +++ b/internal/git/reference.go @@ -8,15 +8,44 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" ) +// InternalReferenceType is the type of an internal reference. +type InternalReferenceType int + +const ( + // InternalReferenceTypeHidden indicates that a reference should never be advertised or + // writeable. + InternalReferenceTypeHidden = InternalReferenceType(iota + 1) + // InternalReferenceTypeReadonly indicates that a reference should be advertised, but not + // writeable. + InternalReferenceTypeReadonly +) + // InternalRefPrefixes is an array of all reference prefixes which are used internally by GitLab. // These need special treatment in some cases, e.g. to restrict writing to them. -var InternalRefPrefixes = [...]string{ - "refs/environments/", - "refs/keep-around/", - "refs/merge-requests/", - "refs/pipelines/", - "refs/remotes/", - "refs/tmp/", +var InternalRefPrefixes = map[string]InternalReferenceType{ + // Environments may be interesting to the user in case they want to figure out what exact + // reference an environment has been constructed from. + "refs/environments/": InternalReferenceTypeReadonly, + + // Keep-around references are only used internally to keep alive objects, and thus they + // shouldn't be exposed to the user. + "refs/keep-around/": InternalReferenceTypeHidden, + + // Merge request references should be readable by the user so that they can still fetch the + // changes of specific merge requests. + "refs/merge-requests/": InternalReferenceTypeReadonly, + + // Pipelines may be interesting to the user in case they want to figure out what exact + // reference a specific pipeline has been running with. + "refs/pipelines/": InternalReferenceTypeReadonly, + + // Remote references shouldn't typically exist in repositories nowadays anymore, and there + // is no reason to expose them to the user. + "refs/remotes/": InternalReferenceTypeHidden, + + // Temporary references are used internally by Rails for various operations and should not + // be exposed to the user. + "refs/tmp/": InternalReferenceTypeHidden, } // ObjectPoolRefNamespace is the namespace used for the references of the primary pool member part diff --git a/internal/gitaly/service/cleanup/internalrefs/cleaner.go b/internal/gitaly/service/cleanup/internalrefs/cleaner.go index 1c4094bcc..0ef2986c8 100644 --- a/internal/gitaly/service/cleanup/internalrefs/cleaner.go +++ b/internal/gitaly/service/cleanup/internalrefs/cleaner.go @@ -124,10 +124,15 @@ func (c *Cleaner) processEntry(ctx context.Context, oldSHA, newSHA string) error // action). It is consulted once per line in the object map. Git is optimized // for ref -> SHA lookups, but we want the opposite! func buildLookupTable(ctx context.Context, repo git.RepositoryExecutor) (map[string][]git.ReferenceName, error) { + internalRefPrefixes := make([]string, 0, len(git.InternalRefPrefixes)) + for refPrefix := range git.InternalRefPrefixes { + internalRefPrefixes = append(internalRefPrefixes, refPrefix) + } + cmd, err := repo.Exec(ctx, git.SubCmd{ Name: "for-each-ref", Flags: []git.Option{git.ValueFlag{Name: "--format", Value: "%(objectname) %(refname)"}}, - Args: git.InternalRefPrefixes[:], + Args: internalRefPrefixes, }) if err != nil { return nil, err diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index d11706ee8..1908cbbbb 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/client" "gitlab.com/gitlab-org/gitaly/v15/internal/backchannel" + "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" @@ -107,6 +108,88 @@ func TestReplicateRepository(t *testing.T) { gittest.Exec(t, cfg, "-C", targetRepoPath, "cat-file", "-p", blobID) } +func TestReplicateRepository_hiddenRefs(t *testing.T) { + t.Parallel() + + ctx := testhelper.Context(t) + cfgBuilder := testcfg.NewGitalyCfgBuilder(testcfg.WithStorages("default", "replica")) + cfg := cfgBuilder.Build(t) + + testcfg.BuildGitalyHooks(t, cfg) + testcfg.BuildGitalySSH(t, cfg) + + client, serverSocketPath := runRepositoryService(t, cfg, nil, testserver.WithDisablePraefect()) + cfg.SocketPath = serverSocketPath + + ctx = testhelper.MergeOutgoingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg)) + + t.Run("initial seeding", func(t *testing.T) { + sourceRepo, sourceRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) + + // Create a bunch of internal references, regardless of whether we classify them as hidden + // or read-only. We should be able to replicate all of them. + var expectedRefs []string + for refPrefix := range git.InternalRefPrefixes { + commitID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithParents(), gittest.WithMessage(refPrefix)) + gittest.Exec(t, cfg, "-C", sourceRepoPath, "update-ref", refPrefix+"1", commitID.String()) + expectedRefs = append(expectedRefs, fmt.Sprintf("%s commit\t%s", commitID, refPrefix+"1")) + } + + targetRepo := proto.Clone(sourceRepo).(*gitalypb.Repository) + targetRepo.StorageName = cfg.Storages[1].Name + targetRepoPath := filepath.Join(cfg.Storages[1].Path, targetRepo.GetRelativePath()) + + _, err := client.ReplicateRepository(ctx, &gitalypb.ReplicateRepositoryRequest{ + Repository: targetRepo, + Source: sourceRepo, + }) + require.NoError(t, err) + + require.ElementsMatch(t, expectedRefs, strings.Split(text.ChompBytes(gittest.Exec(t, cfg, "-C", targetRepoPath, "for-each-ref")), "\n")) + + // Perform another sanity-check to verify that source and target repository have the + // same references now. + require.Equal(t, + text.ChompBytes(gittest.Exec(t, cfg, "-C", sourceRepoPath, "for-each-ref")), + text.ChompBytes(gittest.Exec(t, cfg, "-C", targetRepoPath, "for-each-ref")), + ) + }) + + t.Run("incremental replication", func(t *testing.T) { + sourceRepo, sourceRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) + targetRepo, targetRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[1], gittest.InitRepoOpts{ + WithRelativePath: sourceRepo.GetRelativePath(), + }) + + // Create the same commit in both repositories so that they're in a known-good + // state. + sourceCommitID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithParents(), gittest.WithMessage("base"), gittest.WithBranch("main")) + targetCommitID := gittest.WriteCommit(t, cfg, targetRepoPath, gittest.WithParents(), gittest.WithMessage("base"), gittest.WithBranch("main")) + require.Equal(t, sourceCommitID, targetCommitID) + + // Create the internal references now. + for refPrefix := range git.InternalRefPrefixes { + commitID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithParents(), gittest.WithMessage(refPrefix)) + gittest.Exec(t, cfg, "-C", sourceRepoPath, "update-ref", refPrefix+"1", commitID.String()) + } + + // And now replicate the with the new internal references having been created. + // Because the target repository exists already we'll do a fetch instead of + // replicating via an archive. + _, err := client.ReplicateRepository(ctx, &gitalypb.ReplicateRepositoryRequest{ + Repository: targetRepo, + Source: sourceRepo, + }) + require.NoError(t, err) + + // Verify that the references for both repositories match. + require.Equal(t, + text.ChompBytes(gittest.Exec(t, cfg, "-C", sourceRepoPath, "for-each-ref")), + text.ChompBytes(gittest.Exec(t, cfg, "-C", targetRepoPath, "for-each-ref")), + ) + }) +} + func TestReplicateRepositoryTransactional(t *testing.T) { t.Parallel() diff --git a/internal/gitaly/service/repository/size.go b/internal/gitaly/service/repository/size.go index 3eb0f83a5..1879aead9 100644 --- a/internal/gitaly/service/repository/size.go +++ b/internal/gitaly/service/repository/size.go @@ -21,8 +21,8 @@ func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySize var err error var excludes []string - for _, prefix := range git.InternalRefPrefixes { - excludes = append(excludes, prefix+"*") + for refPrefix := range git.InternalRefPrefixes { + excludes = append(excludes, refPrefix+"*") } path, err := repo.Path() diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go index 2ffe65c39..9b371bf7f 100644 --- a/internal/gitaly/service/smarthttp/inforefs_test.go +++ b/internal/gitaly/service/smarthttp/inforefs_test.go @@ -35,27 +35,101 @@ import ( "google.golang.org/protobuf/proto" ) -func TestSuccessfulInfoRefsUploadPack(t *testing.T) { +func TestInfoRefsUploadPack_successful(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) - cfg.SocketPath = runSmartHTTPServer(t, cfg) - ctx := testhelper.Context(t) - repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, + + repo, repoPath := gittest.CreateRepository(ctx, t, cfg) + + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents()) + tagID := gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision(), gittest.WriteTagConfig{ + Message: "annotated tag", }) rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo} response, err := makeInfoRefsUploadPackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, rpcRequest) require.NoError(t, err) - assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response), "001e# service=git-upload-pack", "0000", []string{ - "003ef4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 refs/tags/v1.0.0", - "00416f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 refs/tags/v1.0.0^{}", + requireAdvertisedRefs(t, string(response), "git-upload-pack", []string{ + commitID.String() + " HEAD", + commitID.String() + " refs/heads/main\n", + tagID.String() + " refs/tags/v1.0.0\n", + commitID.String() + " refs/tags/v1.0.0^{}\n", }) } +func TestInfoRefsUploadPack_internalRefs(t *testing.T) { + t.Parallel() + + cfg := testcfg.Build(t) + cfg.SocketPath = runSmartHTTPServer(t, cfg) + ctx := testhelper.Context(t) + + for _, tc := range []struct { + ref string + expectedAdvertisements []string + }{ + { + ref: "refs/merge-requests/1/head", + expectedAdvertisements: []string{ + "HEAD", + "refs/heads/main\n", + "refs/merge-requests/1/head\n", + }, + }, + { + ref: "refs/environments/1", + expectedAdvertisements: []string{ + "HEAD", + "refs/environments/1\n", + "refs/heads/main\n", + }, + }, + { + ref: "refs/pipelines/1", + expectedAdvertisements: []string{ + "HEAD", + "refs/heads/main\n", + "refs/pipelines/1\n", + }, + }, + { + ref: "refs/tmp/1", + expectedAdvertisements: []string{ + "HEAD", + "refs/heads/main\n", + }, + }, + { + ref: "refs/keep-around/1", + expectedAdvertisements: []string{ + "HEAD", + "refs/heads/main\n", + }, + }, + } { + t.Run(tc.ref, func(t *testing.T) { + repo, repoPath := gittest.CreateRepository(ctx, t, cfg) + + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents()) + gittest.Exec(t, cfg, "-C", repoPath, "update-ref", tc.ref, commitID.String()) + + var expectedAdvertisements []string + for _, expectedRef := range tc.expectedAdvertisements { + expectedAdvertisements = append(expectedAdvertisements, commitID.String()+" "+expectedRef) + } + + response, err := makeInfoRefsUploadPackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, &gitalypb.InfoRefsRequest{ + Repository: repo, + }) + require.NoError(t, err) + requireAdvertisedRefs(t, string(response), "git-upload-pack", expectedAdvertisements) + }) + } +} + func TestInfoRefsUploadPack_repositoryDoesntExist(t *testing.T) { t.Parallel() @@ -79,7 +153,7 @@ func TestInfoRefsUploadPack_repositoryDoesntExist(t *testing.T) { testhelper.RequireGrpcError(t, expectedErr, err) } -func TestSuccessfulInfoRefsUploadWithPartialClone(t *testing.T) { +func TestInfoRefsUploadPack_partialClone(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) @@ -106,16 +180,16 @@ func TestSuccessfulInfoRefsUploadWithPartialClone(t *testing.T) { } } -func TestSuccessfulInfoRefsUploadPackWithGitConfigOptions(t *testing.T) { +func TestInfoRefsUploadPack_gitConfigOptions(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) cfg.SocketPath = runSmartHTTPServer(t, cfg) ctx := testhelper.Context(t) - repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) + repo, repoPath := gittest.CreateRepository(ctx, t, cfg) + + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents()) // transfer.hideRefs=refs will hide every ref that info-refs would normally // output, allowing us to test that the custom configuration is respected @@ -125,10 +199,12 @@ func TestSuccessfulInfoRefsUploadPackWithGitConfigOptions(t *testing.T) { } response, err := makeInfoRefsUploadPackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, rpcRequest) require.NoError(t, err) - assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response), "001e# service=git-upload-pack", "0000", []string{}) + requireAdvertisedRefs(t, string(response), "git-upload-pack", []string{ + commitID.String() + " HEAD", + }) } -func TestSuccessfulInfoRefsUploadPackWithGitProtocol(t *testing.T) { +func TestInfoRefsUploadPack_gitProtocol(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) @@ -186,30 +262,32 @@ func makeInfoRefsUploadPackRequest(ctx context.Context, t *testing.T, serverSock return response, err } -func TestSuccessfulInfoRefsReceivePack(t *testing.T) { +func TestInfoRefsReceivePack_successful(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) - cfg.SocketPath = runSmartHTTPServer(t, cfg) - ctx := testhelper.Context(t) - repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) - rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo} + repo, repoPath := gittest.CreateRepository(ctx, t, cfg) - response, err := makeInfoRefsReceivePackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, rpcRequest) + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents()) + tagID := gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision(), gittest.WriteTagConfig{ + Message: "annotated tag", + }) + + response, err := makeInfoRefsReceivePackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, &gitalypb.InfoRefsRequest{ + Repository: repo, + }) require.NoError(t, err) - assertGitRefAdvertisement(t, "InfoRefsReceivePack", string(response), "001f# service=git-receive-pack", "0000", []string{ - "003ef4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 refs/tags/v1.0.0", - "003e8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b refs/tags/v1.1.0", + requireAdvertisedRefs(t, string(response), "git-receive-pack", []string{ + commitID.String() + " refs/heads/main", + tagID.String() + " refs/tags/v1.0.0\n", }) } -func TestObjectPoolRefAdvertisementHiding(t *testing.T) { +func TestInfoRefsReceivePack_hiddenRefs(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) @@ -252,7 +330,7 @@ func TestObjectPoolRefAdvertisementHiding(t *testing.T) { require.NotContains(t, string(response), commitID+" .have") } -func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) { +func TestInfoRefsReceivePack_repoNotFound(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) @@ -272,7 +350,7 @@ func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) { testhelper.RequireGrpcError(t, expectedErr, err) } -func TestFailureRepoNotSetInfoRefsReceivePack(t *testing.T) { +func TestInfoRefsReceivePack_repoNotSet(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) @@ -304,23 +382,28 @@ func makeInfoRefsReceivePackRequest(ctx context.Context, t *testing.T, serverSoc return response, err } -func assertGitRefAdvertisement(t *testing.T, rpc, responseBody string, firstLine, lastLine string, middleLines []string) { - responseLines := strings.Split(responseBody, "\n") +func requireAdvertisedRefs(t *testing.T, responseBody, expectedService string, expectedRefs []string) { + t.Helper() - if responseLines[0] != firstLine { - t.Errorf("%q: expected response first line to be %q, found %q", rpc, firstLine, responseLines[0]) - } + responseLines := strings.SplitAfter(responseBody, "\n") + require.Greater(t, len(responseLines), 2) - lastIndex := len(responseLines) - 1 - if responseLines[lastIndex] != lastLine { - t.Errorf("%q: expected response last line to be %q, found %q", rpc, lastLine, responseLines[lastIndex]) + for i, expectedRef := range expectedRefs { + expectedRefs[i] = gittest.Pktlinef(t, "%s", expectedRef) } - for _, ref := range middleLines { - if !strings.Contains(responseBody, ref) { - t.Errorf("%q: expected response to contain %q, found none", rpc, ref) - } - } + // The first line contains the service announcement. + require.Equal(t, gittest.Pktlinef(t, "# service=%s\n", expectedService), responseLines[0]) + + // The second line contains the first reference as well as the capability announcement. We + // thus split the string at "\x00" and ignore the capability announcement here. + refAndCapabilities := strings.SplitN(responseLines[1], "\x00", 2) + require.Len(t, refAndCapabilities, 2) + // We just replace the first advertised reference to make it easier to compare refs. + responseLines[1] = gittest.Pktlinef(t, "%s", refAndCapabilities[0][8:]) + + require.Equal(t, responseLines[1:len(responseLines)-1], expectedRefs) + require.Equal(t, "0000", responseLines[len(responseLines)-1]) } type mockStreamer struct { @@ -335,7 +418,7 @@ func (ms *mockStreamer) PutStream(ctx context.Context, repo *gitalypb.Repository return ms.Streamer.PutStream(ctx, repo, req, src) } -func TestCacheInfoRefsUploadPack(t *testing.T) { +func TestInfoRefsUploadPack_cache(t *testing.T) { t.Parallel() cfg := testcfg.Build(t) @@ -352,8 +435,12 @@ func TestCacheInfoRefsUploadPack(t *testing.T) { cfg.SocketPath = gitalyServer.Address() ctx := testhelper.Context(t) - repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, + + repo, repoPath := gittest.CreateRepository(ctx, t, cfg) + + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents()) + tagID := gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision(), gittest.WriteTagConfig{ + Message: "annotated tag", }) rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo} @@ -371,13 +458,12 @@ func TestCacheInfoRefsUploadPack(t *testing.T) { response, err := makeInfoRefsUploadPackRequest(ctx, t, addr, cfg.Auth.Token, rpcRequest) require.NoError(t, err) - assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response), - "001e# service=git-upload-pack", "0000", - []string{ - "003ef4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 refs/tags/v1.0.0", - "00416f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 refs/tags/v1.0.0^{}", - }, - ) + requireAdvertisedRefs(t, string(response), "git-upload-pack", []string{ + commitID.String() + " HEAD", + commitID.String() + " refs/heads/main\n", + tagID.String() + " refs/tags/v1.0.0\n", + commitID.String() + " refs/tags/v1.0.0^{}\n", + }) } assertNormalResponse(gitalyServer.Address()) @@ -395,9 +481,7 @@ func TestCacheInfoRefsUploadPack(t *testing.T) { replaceCachedResponse(t, ctx, cache, rewrittenRequest, strings.Join(replacedContents, "\n")) response, err := makeInfoRefsUploadPackRequest(ctx, t, gitalyServer.Address(), cfg.Auth.Token, rpcRequest) require.NoError(t, err) - assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response), - replacedContents[0], replacedContents[3], replacedContents[1:3], - ) + require.Equal(t, strings.Join(replacedContents, "\n"), string(response)) invalidateCacheForRepo := func() { ender, err := cache.StartLease(rewrittenRequest.Repository) |