diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go index 6880429254ebca930e6e246478f4c95408551d15..4ce9d0c8a83770d3f26a54ea7318141bf27dfe26 100644 --- a/internal/git/updateref/updateref.go +++ b/internal/git/updateref/updateref.go @@ -77,10 +77,35 @@ func (u *Updater) Create(ref, value string) error { return err } +var mungeMapForTesting = make(map[string]string) + +// MungeMapForTestingAdd is a test-only interface to the private +// mungeMapForTesting. Use it for ad-hoc mapping the Update() +// "oldvalue" to some fake value. The munged value will only be used +// once, it's delete()-ed on retrieval. +func MungeMapForTestingAdd(key, value string) { + mungeMapForTesting[key] = value +} + +func fakeOldValueForTesting(oldValue string) string { + fakeOldValue := mungeMapForTesting[oldValue] + if len(fakeOldValue) != 0 { + // Don't persist the value, it's one-use. If we had a + // Del() interface and the test died and forgot to + // cleanup later tests might die at a distance. + delete(mungeMapForTesting, oldValue) + return fakeOldValue + } + return oldValue +} + // Update commands the reference to be updated to point at the sha specified in // newvalue -func (u *Updater) Update(ref, newvalue, oldvalue string) error { - _, err := fmt.Fprintf(u.cmd, "update %s\x00%s\x00%s\x00", ref, newvalue, oldvalue) +func (u *Updater) Update(ref, newvalue, oldValue string) error { + if len(mungeMapForTesting) != 0 { + oldValue = fakeOldValueForTesting(oldValue) + } + _, err := fmt.Fprintf(u.cmd, "update %s\x00%s\x00%s\x00", ref, newvalue, oldValue) return err } diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index 905b9e47004181fd1f7692816d345d7df5423d21..1586b1ece34bdef17b180cc84a8c97e91041d1a3 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -99,6 +99,60 @@ func TestUpdate(t *testing.T) { require.NotEqual(t, commit.Id, parentCommit.Id, "reference was updated when it shouldn't have been") } +func TestUpdateFake(t *testing.T) { + ctx, testRepo, _, teardown := setup(t) + defer teardown() + + locator := config.NewLocator(config.Config) + masterCommit, err := log.GetCommit(ctx, locator, testRepo, "refs/heads/master") + require.NoError(t, err) + + updater, err := New(ctx, testRepo) + require.NoError(t, err) + + ref := "refs/heads/fix" + + // Sanity check: ensure the ref exists before we start + fixCommit, logErr := log.GetCommit(ctx, locator, testRepo, ref) + require.NoError(t, logErr) + require.NotEqual(t, masterCommit.Id, fixCommit.Id, "%s points to %s in the test repository", ref, fixCommit.Id) + + // Updating it from fix->master with a bad oldvalue fails + updateErr := updater.Update(ref, masterCommit.Id, masterCommit.Id) + require.NoError(t, updateErr) + require.Error(t, updater.Wait()) // the error is deferred + updater, err = New(ctx, testRepo) // .Wait() closes it, create a new one + require.NoError(t, err) + + // check that the ref was not updated + fixCommitTwo, logErr := log.GetCommit(ctx, locator, testRepo, ref) + require.NoError(t, logErr) + require.Equal(t, fixCommit.Id, fixCommitTwo.Id) + + // Now fake up the oldvalue + MungeMapForTestingAdd(masterCommit.Id, fixCommit.Id) + + // Updating it from fix->master with a bad oldvalue works, + // since we're lying via MungeMapForTesting + updateErr = updater.Update(ref, masterCommit.Id, masterCommit.Id) + require.NoError(t, updateErr) + require.NoError(t, updater.Wait()) // the error is deferred + updater, err = New(ctx, testRepo) // .Wait() closes it, create a new one + require.NoError(t, err) + + // check that the ref was updated + fixCommitThree, logErr := log.GetCommit(ctx, locator, testRepo, ref) + require.NoError(t, logErr) + require.Equal(t, masterCommit.Id, fixCommitThree.Id) + + // Updating it with masterCommit.Id does not error, since the + // MungeMapForTesting is one-use (it deleted the key after it + // was used) (it would if Del() wasn't called) + updateErr = updater.Update(ref, fixCommit.Id, masterCommit.Id) + require.NoError(t, updateErr) + require.NoError(t, updater.Wait()) // the error is deferred +} + func TestDelete(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index 9202af6919257303f7657c56c3e20f5d34e0c38f..fe9da56bfa04d2851bb691be2465e8aadea277b2 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -9,7 +9,9 @@ import ( "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" @@ -1253,6 +1255,82 @@ func testFailedUserCreateTagRequestDueToValidation(t *testing.T, ctx context.Con } } +//nolint: errcheck +func TestFailedUserDeleteTagRequestDueToUpdateRef(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() + + ctx = featureflag.OutgoingCtxWithFeatureFlagValue(ctx, featureflag.GoUserDeleteTag, "true") + + serverSocketPath, stop := runOperationServiceServer(t) + defer stop() + + client, conn := newOperationClient(t, serverSocketPath) + defer conn.Close() + + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + tagName := "my-new-tag" + targetRevision := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd" + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", tagName, targetRevision) + + tagID := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", tagName) + require.Equal(t, targetRevision, text.ChompBytes(tagID)) + + updateref.MungeMapForTestingAdd(targetRevision, "612036fac47c5d31c212b17268e2f3ba807bce1e") + request := &gitalypb.UserDeleteTagRequest{ + Repository: testRepo, + TagName: []byte(tagName), + User: testhelper.TestUser, + } + response, err := client.UserDeleteTag(ctx, request) + require.Equal(t, status.Errorf(codes.FailedPrecondition, "Could not update refs/tags/%s. Please refresh and try again.", tagName), err) + require.Nil(t, response) + + tags := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "for-each-ref", "--", "refs/tags/"+tagName) + require.Contains(t, string(tags), targetRevision) +} + +//nolint: errcheck +func TestFailedUserCreateTagRequestDueToUpdateRef(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() + + ctx = featureflag.OutgoingCtxWithFeatureFlagValue(ctx, featureflag.GoUserCreateTag, "true") + + serverSocketPath, stop := runOperationServiceServer(t) + defer stop() + + client, conn := newOperationClient(t, serverSocketPath) + defer conn.Close() + + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + targetRevision := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd" + updateref.MungeMapForTestingAdd(git.NullSHA, "612036fac47c5d31c212b17268e2f3ba807bce1e") + + tagName := "my-new-tag" + request := &gitalypb.UserCreateTagRequest{ + Repository: testRepo, + TagName: []byte(tagName), + TargetRevision: []byte(targetRevision), + User: testhelper.TestUser, + } + response, err := client.UserCreateTag(ctx, request) + require.NoError(t, err) + require.Empty(t, response.PreReceiveError) + responseOk := &gitalypb.UserCreateTagResponse{ + Tag: nil, + Exists: true, + } + require.Equal(t, responseOk, response) + + tags := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "for-each-ref", "--", "refs/tags/"+tagName) + require.Equal(t, string(tags), "") +} + func TestTagHookOutput(t *testing.T) { testhelper.NewFeatureSets([]featureflag.FeatureFlag{ featureflag.GoUserDeleteTag,