Merge branch 'pks-user-create-tag-preparatory-refactorings' into 'master'

operations: Preparatory refactorings for UserCreateTag

See merge request gitlab-org/gitaly!4740

6 files changed, 456 insertions, 476 deletions

diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index 5e10f65a7..6b50cc0c2 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -54,32 +54,28 @@ func (s *Server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagR
 }
 
 func validateUserCreateTag(req *gitalypb.UserCreateTagRequest) error {
-	// Emulate validations done by Ruby. A lot of these (e.g. the
-	// upper-case error messages) can be simplified once we're not
-	// doing bug-for-bug Ruby emulation anymore)
 	if len(req.TagName) == 0 {
-		return status.Errorf(codes.InvalidArgument, "empty tag name")
+		return fmt.Errorf("empty tag name")
 	}
 
-	if bytes.Contains(req.TagName, []byte(" ")) {
-		return status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", req.TagName)
+	if err := git.ValidateRevision(req.TagName); err != nil {
+		return fmt.Errorf("invalid tag name: %w", err)
 	}
 
 	if req.User == nil {
-		return status.Errorf(codes.InvalidArgument, "empty user")
+		return fmt.Errorf("empty user")
 	}
 
 	if len(req.TargetRevision) == 0 {
-		return status.Error(codes.InvalidArgument, "empty target revision")
+		return fmt.Errorf("empty target revision")
 	}
 
 	if bytes.Contains(req.Message, []byte("\000")) {
-		return status.Errorf(codes.Unknown, "ArgumentError: string contains null byte")
+		return fmt.Errorf("tag message contains NUL byte")
 	}
 
-	// Our own Go-specific validation
 	if req.GetRepository() == nil {
-		return status.Errorf(codes.Internal, "empty repository")
+		return fmt.Errorf("empty repository")
 	}
 
 	return nil
@@ -87,9 +83,8 @@ func validateUserCreateTag(req *gitalypb.UserCreateTagRequest) error {
 
 //nolint: revive,stylecheck // This is unintentionally missing documentation.
 func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagRequest) (*gitalypb.UserCreateTagResponse, error) {
-	// Validate the request
 	if err := validateUserCreateTag(req); err != nil {
-		return nil, err
+		return nil, helper.ErrInvalidArgumentf("validating request: %w", err)
 	}
 
 	targetRevision := git.Revision(req.TargetRevision)
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index 6782674eb..c1c53a315 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -4,7 +4,6 @@ package operations
 
 import (
 	"fmt"
-	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -14,8 +13,7 @@ import (
 	"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/service"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/hook"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
@@ -29,7 +27,7 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
-func TestSuccessfulUserDeleteTagRequest(t *testing.T) {
+func TestUserDeleteTag_successful(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -53,7 +51,7 @@ func TestSuccessfulUserDeleteTagRequest(t *testing.T) {
 	require.NotContains(t, string(tags), tagNameInput, "tag name still exists in tags list")
 }
 
-func TestSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T) {
+func TestUserDeleteTag_hooks(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -142,131 +140,91 @@ func writeAssertObjectTypeUpdateHook(t *testing.T, repoPath, expectedObjectType
 	`, expectedObjectType)))
 }
 
-func TestSuccessfulUserCreateTagRequest(t *testing.T) {
+func TestUserCreateTag_successful(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
+	repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
 
-	targetRevision := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"
-	targetRevisionCommit, err := repo.ReadCommit(ctx, git.Revision(targetRevision))
+	targetRevisionCommit, err := repo.ReadCommit(ctx, commitID.Revision())
 	require.NoError(t, err)
 
 	inputTagName := "to-be-créated-soon"
 
-	testCases := []struct {
+	for _, tc := range []struct {
 		desc               string
 		tagName            string
 		message            string
-		targetRevision     string
-		expectedTag        *gitalypb.Tag
+		targetRevision     git.Revision
 		expectedObjectType string
+		expectedResponse   *gitalypb.UserCreateTagResponse
 	}{
 		{
 			desc:           "lightweight tag to commit",
 			tagName:        inputTagName,
-			targetRevision: targetRevision,
-			expectedTag: &gitalypb.Tag{
-				Name:         []byte(inputTagName),
-				Id:           targetRevision,
-				TargetCommit: targetRevisionCommit,
+			targetRevision: commitID.Revision(),
+			expectedResponse: &gitalypb.UserCreateTagResponse{
+				Tag: &gitalypb.Tag{
+					Name:         []byte(inputTagName),
+					Id:           commitID.String(),
+					TargetCommit: targetRevisionCommit,
+				},
 			},
 			expectedObjectType: "commit",
 		},
 		{
 			desc:           "annotated tag to commit",
 			tagName:        inputTagName,
-			targetRevision: targetRevision,
+			targetRevision: commitID.Revision(),
 			message:        "This is an annotated tag",
-			expectedTag: &gitalypb.Tag{
-				Name: []byte(inputTagName),
-				// Id: is a new object, filled in below
-				TargetCommit: targetRevisionCommit,
-				Message:      []byte("This is an annotated tag"),
-				MessageSize:  24,
+			expectedResponse: &gitalypb.UserCreateTagResponse{
+				Tag: &gitalypb.Tag{
+					Name:         []byte(inputTagName),
+					Id:           "6c6134431f05e3d22726a3876cc1fecea7df18b5",
+					TargetCommit: targetRevisionCommit,
+					Message:      []byte("This is an annotated tag"),
+					MessageSize:  24,
+				},
 			},
 			expectedObjectType: "tag",
 		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			writeAssertObjectTypePreReceiveHook(t, repoPath, testCase.expectedObjectType)
-			writeAssertObjectTypeUpdateHook(t, repoPath, testCase.expectedObjectType)
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			writeAssertObjectTypePreReceiveHook(t, repoPath, tc.expectedObjectType)
+			writeAssertObjectTypeUpdateHook(t, repoPath, tc.expectedObjectType)
 
-			request := &gitalypb.UserCreateTagRequest{
+			response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
 				Repository:     repoProto,
-				TagName:        []byte(testCase.tagName),
-				TargetRevision: []byte(testCase.targetRevision),
+				TagName:        []byte(tc.tagName),
+				TargetRevision: []byte(tc.targetRevision),
 				User:           gittest.TestUser,
-				Message:        []byte(testCase.message),
-			}
-
-			response, err := client.UserCreateTag(ctx, request)
-			require.NoError(t, err, "error from calling RPC")
-			require.Empty(t, response.PreReceiveError, "PreReceiveError must be empty, signalling the push was accepted")
+				Message:        []byte(tc.message),
+				Timestamp:      timestamppb.New(time.Unix(1600000000, 0)),
+			})
+			require.NoError(t, err)
+			testhelper.ProtoEqual(t, tc.expectedResponse, response)
 
 			defer gittest.Exec(t, cfg, "-C", repoPath, "tag", "-d", inputTagName)
 
-			responseOk := &gitalypb.UserCreateTagResponse{
-				Tag: testCase.expectedTag,
-			}
-			// Fake up *.Id for annotated tags
-			if len(testCase.expectedTag.Id) == 0 {
-				id := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", inputTagName)
-				responseOk.Tag.Id = text.ChompBytes(id)
-			}
-
-			testhelper.ProtoEqual(t, responseOk, response)
-
 			tag := gittest.Exec(t, cfg, "-C", repoPath, "tag")
 			require.Contains(t, string(tag), inputTagName)
 		})
 	}
 }
 
-func TestUserCreateTagWithTransaction(t *testing.T) {
+func TestUserCreateTag_transactional(t *testing.T) {
 	t.Parallel()
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
-
-	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-	hooksOutputDir := testhelper.TempDir(t)
-	hooksOutputPath := filepath.Join(hooksOutputDir, "output")
+	cfg := testcfg.Build(t)
+	cfg.SocketPath = runOperationServiceServer(t, cfg, testserver.WithDisablePraefect())
 
-	// We're creating a set of custom hooks which simply
-	// write to a file. The intention is that we want to
-	// check that the hooks only run on the primary node.
-	hooks := []string{"pre-receive", "update", "post-receive"}
-	for _, hook := range hooks {
-		gittest.WriteCustomHook(t, repoPath, hook,
-			[]byte(fmt.Sprintf("#!/bin/sh\necho %s >>%s\n", hook, hooksOutputPath)),
-		)
-	}
+	ctx := testhelper.Context(t)
 
-	// We're creating a custom server with a fake transaction server which
-	// simply returns success for every call, but tracks the number of
-	// calls. The server is then injected into the client's context to make
-	// it available for transactional voting. We cannot use
-	// runOperationServiceServer as it puts a Praefect server in between if
-	// running Praefect tests, which would break our test setup.
 	transactionServer := &testTransactionServer{}
-	testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
-		gitalypb.RegisterOperationServiceServer(srv, NewServer(
-			deps.GetHookManager(),
-			deps.GetTxManager(),
-			deps.GetLocator(),
-			deps.GetConnsPool(),
-			deps.GetGit2goExecutor(),
-			deps.GetGitCmdFactory(),
-			deps.GetCatfileCache(),
-			deps.GetUpdaterWithHooks(),
-		))
-		gitalypb.RegisterHookServiceServer(srv, hook.NewServer(deps.GetHookManager(), deps.GetGitCmdFactory(), deps.GetPackObjectsCache(), deps.GetPackObjectsConcurrencyTracker()))
-	})
-	ctx := testhelper.Context(t)
 
 	// We're using internal gitaly socket to connect to the server.
 	// This is kind of a hack when running tests with Praefect:
@@ -286,7 +244,7 @@ func TestUserCreateTagWithTransaction(t *testing.T) {
 		),
 	)
 
-	for i, testCase := range []struct {
+	for _, tc := range []struct {
 		desc    string
 		primary bool
 		message string
@@ -310,56 +268,59 @@ func TestUserCreateTagWithTransaction(t *testing.T) {
 			message: "foobar",
 		},
 	} {
-		t.Run(testCase.desc, func(t *testing.T) {
+		t.Run(tc.desc, func(t *testing.T) {
 			*transactionServer = testTransactionServer{}
 
-			if err := os.Remove(hooksOutputPath); err != nil {
-				require.True(t, os.IsNotExist(err), "error when cleaning up work area: %v", err)
-			}
+			repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
+			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-			tagName := fmt.Sprintf("tag-%d", i)
-			targetRevision := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"
-			targetCommit, err := repo.ReadCommit(ctx, git.Revision(targetRevision))
-			require.NoError(t, err)
+			hooksOutputDir := testhelper.TempDir(t)
+			hooksOutputPath := filepath.Join(hooksOutputDir, "output")
 
-			request := &gitalypb.UserCreateTagRequest{
-				Repository:     repoProto,
-				TagName:        []byte(tagName),
-				Message:        []byte(testCase.message),
-				TargetRevision: []byte(targetRevision),
-				User:           gittest.TestUser,
+			// We're creating a set of custom hooks which simply
+			// write to a file. The intention is that we want to
+			// check that the hooks only run on the primary node.
+			hooks := []string{"pre-receive", "update", "post-receive"}
+			for _, hook := range hooks {
+				gittest.WriteCustomHook(t, repoPath, hook,
+					[]byte(fmt.Sprintf("#!/bin/sh\necho %s >>%s\n", hook, hooksOutputPath)),
+				)
 			}
 
+			commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+			targetCommit, err := repo.ReadCommit(ctx, commitID.Revision())
+			require.NoError(t, err)
+
 			// We need to convert to an incoming context first in
 			// order to preserve the feature flag.
-			ctx = metadata.OutgoingToIncoming(ctx)
-			ctx, err = txinfo.InjectTransaction(ctx, 1, "node", testCase.primary)
+			ctx := metadata.OutgoingToIncoming(ctx)
+			ctx, err = txinfo.InjectTransaction(ctx, 1, "node", tc.primary)
 			require.NoError(t, err)
 			ctx = metadata.IncomingToOutgoing(ctx)
 
-			response, err := client.UserCreateTag(ctx, request)
+			response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
+				Repository:     repoProto,
+				TagName:        []byte("v1.0.0"),
+				Message:        []byte(tc.message),
+				TargetRevision: []byte(commitID),
+				User:           gittest.TestUser,
+			})
 			require.NoError(t, err)
-
-			targetOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/tags/"+tagName))
-			peeledOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", targetOID+"^{commit}"))
-			targetOIDOK := targetOID
-			if len(testCase.message) > 0 {
-				targetOIDOK = peeledOID
-			}
-			require.Equal(t, targetOIDOK, targetRevision)
-
 			testhelper.ProtoEqual(t, &gitalypb.UserCreateTagResponse{
 				Tag: &gitalypb.Tag{
-					Name:         []byte(tagName),
-					Message:      []byte(testCase.message),
-					MessageSize:  int64(len(testCase.message)),
-					Id:           targetOID,
+					Name:         []byte("v1.0.0"),
+					Message:      []byte(tc.message),
+					MessageSize:  int64(len(tc.message)),
+					Id:           text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/tags/v1.0.0")),
 					TargetCommit: targetCommit,
 				},
 			}, response)
 
+			peeledTagID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/tags/v1.0.0^{commit}"))
+			require.Equal(t, commitID.String(), peeledTagID)
+
 			// Only the primary node should've executed hooks.
-			if testCase.primary {
+			if tc.primary {
 				contents := testhelper.MustReadFile(t, hooksOutputPath)
 				require.Equal(t, "pre-receive\nupdate\npost-receive\n", string(contents))
 			} else {
@@ -372,30 +333,37 @@ func TestUserCreateTagWithTransaction(t *testing.T) {
 	}
 }
 
-func TestUserCreateTagQuarantine(t *testing.T) {
+func TestUserCreateTag_quarantine(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
+
+	repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+
+	tagIDOutputPath := filepath.Join(testhelper.TempDir(t), "tag-id")
+
 	// We set up a custom "pre-receive" hook which simply prints the new tag to stdout and then
 	// exits with an error. Like this, we can both assert that the hook can see the quarantined
 	// tag, and it allows us to fail the RPC before we migrate quarantined objects. Furthermore,
 	// we also try whether we can print the tag's tagged object to assert that we can see
 	// objects which are not part of the object quarantine.
-	gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(
+	gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(
 		`#!/bin/sh
 		read oldval newval ref &&
 		git cat-file -p $newval^{commit} >/dev/null &&
+		echo "$newval" >%q &&
 		git cat-file -p $newval^{tag} &&
 		exit 1
-	`))
+	`, tagIDOutputPath)))
 
 	response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
 		Repository:     repoProto,
 		TagName:        []byte("quarantined-tag"),
-		TargetRevision: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"),
+		TargetRevision: []byte(commitID),
 		User:           gittest.TestUser,
 		Timestamp:      timestamppb.New(time.Unix(1600000000, 0)),
 		Message:        []byte("message"),
@@ -405,285 +373,313 @@ func TestUserCreateTagQuarantine(t *testing.T) {
 	// Conveniently, the pre-receive error will now contain output from our custom hook and thus
 	// the tag's contents.
 	testhelper.ProtoEqual(t, &gitalypb.UserCreateTagResponse{
-		PreReceiveError: `object c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd
+		PreReceiveError: fmt.Sprintf(`object %s
 type commit
 tag quarantined-tag
 tagger Jane Doe <janedoe@gitlab.com> 1600000000 +0800
 
-message`,
+message`, commitID),
 	}, response)
 
+	tagID := text.ChompBytes(testhelper.MustReadFile(t, tagIDOutputPath))
+
 	// In case we use an object quarantine directory, the tag should not exist in the target
 	// repository because the RPC failed to update the revision.
-	tagExists, err := repo.HasRevision(ctx, "85d279b2cc85df37992e08f84707987321e8ef47^{tag}")
+	tagExists, err := repo.HasRevision(ctx, git.Revision(tagID+"^{tag}"))
 	require.NoError(t, err)
 	require.False(t, tagExists, "tag should not have been migrated")
 }
 
-func TestSuccessfulUserCreateTagRequestAnnotatedLightweightDisambiguation(t *testing.T) {
+func TestUserCreateTag_message(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
-	testCases := []struct {
-		desc    string
-		message string
-		objType string
-		err     error
+	for _, tc := range []struct {
+		desc               string
+		message            string
+		expectedObjectType string
+		expectedErr        error
+		expectedMessage    string
 	}{
 		{
-			desc:    "error: contains null byte",
-			message: "\000",
-			err:     status.Error(codes.Unknown, "ArgumentError: string contains null byte"),
+			desc:        "error: contains null byte",
+			message:     "\000",
+			expectedErr: helper.ErrInvalidArgumentf("validating request: tag message contains NUL byte"),
 		},
 		{
-			desc:    "annotated: some control characters",
-			message: "\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008",
-			objType: "tag",
+			desc:               "annotated: some control characters",
+			message:            "\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008",
+			expectedObjectType: "tag",
+			expectedMessage:    "\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008",
 		},
 		{
-			desc:    "lightweight: empty message",
-			message: "",
-			objType: "commit",
+			desc:               "lightweight: empty message",
+			message:            "",
+			expectedObjectType: "commit",
 		},
 		{
-			desc:    "lightweight: simple whitespace",
-			message: " \t\t",
-			objType: "commit",
+			desc:               "lightweight: simple whitespace",
+			message:            " \t\t",
+			expectedObjectType: "commit",
 		},
 		{
-			desc:    "lightweight: whitespace with newlines",
-			message: "\t\n\f\r ",
-			objType: "commit",
+			desc:               "lightweight: whitespace with newlines",
+			message:            "\t\n\f\r ",
+			expectedObjectType: "commit",
 		},
 		{
-			desc:    "lightweight: simple Unicode whitespace",
-			message: "\u00a0",
-			objType: "tag",
+			desc:               "annotated: simple Unicode whitespace",
+			message:            "\u00a0",
+			expectedObjectType: "tag",
+			expectedMessage:    "\u00a0",
 		},
 		{
-			desc:    "lightweight: lots of Unicode whitespace",
-			message: "\u0020\u00a0\u1680\u180e\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u200b\u202f\u205f\u3000\ufeff",
-			objType: "tag",
+			desc:               "lightweight: lots of Unicode whitespace",
+			message:            "\u0020\u00a0\u1680\u180e\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u200b\u202f\u205f\u3000\ufeff",
+			expectedObjectType: "tag",
+			expectedMessage:    "\u0020\u00a0\u1680\u180e\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u200b\u202f\u205f\u3000\ufeff",
 		},
 		{
-			desc:    "annotated: dot",
-			message: ".",
-			objType: "tag",
+			desc:               "annotated: dot",
+			message:            ".",
+			expectedObjectType: "tag",
+			expectedMessage:    ".",
 		},
-	}
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
+			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			writeAssertObjectTypePreReceiveHook(t, repoPath, testCase.objType)
-			writeAssertObjectTypeUpdateHook(t, repoPath, testCase.objType)
+			commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+			commit, err := repo.ReadCommit(ctx, commitID.Revision())
+			require.NoError(t, err)
+
+			writeAssertObjectTypePreReceiveHook(t, repoPath, tc.expectedObjectType)
+			writeAssertObjectTypeUpdateHook(t, repoPath, tc.expectedObjectType)
 
-			tagName := "what-will-it-be"
 			request := &gitalypb.UserCreateTagRequest{
-				Repository:     repo,
-				TagName:        []byte(tagName),
-				TargetRevision: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"),
+				Repository:     repoProto,
+				TagName:        []byte("what-will-it-be"),
+				TargetRevision: []byte(commitID),
 				User:           gittest.TestUser,
-				Message:        []byte(testCase.message),
+				Message:        []byte(tc.message),
 			}
 
 			response, err := client.UserCreateTag(ctx, request)
-
-			if testCase.err != nil {
-				testhelper.RequireGrpcError(t, testCase.err, err)
+			testhelper.RequireGrpcError(t, tc.expectedErr, err)
+			if tc.expectedErr == nil {
+				response.Tag.Id = ""
+				testhelper.ProtoEqual(t, &gitalypb.UserCreateTagResponse{
+					Tag: &gitalypb.Tag{
+						Name:         []byte("what-will-it-be"),
+						Message:      []byte(tc.expectedMessage),
+						MessageSize:  int64(len(tc.expectedMessage)),
+						TargetCommit: commit,
+					},
+				}, response)
 			} else {
-				defer gittest.Exec(t, cfg, "-C", repoPath, "tag", "-d", tagName)
-				require.NoError(t, err)
-				require.Empty(t, response.PreReceiveError)
+				require.Nil(t, response)
 			}
 		})
 	}
 }
 
-func TestSuccessfulUserCreateTagRequestWithParsedTargetRevision(t *testing.T) {
+func TestUserCreateTag_targetRevision(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
-
-	gittest.Exec(t, cfg, "-C", repoPath, "branch", "heads/master", "master~1")
-	defer gittest.Exec(t, cfg, "-C", repoPath, "branch", "-d", "heads/master")
-	gittest.Exec(t, cfg, "-C", repoPath, "branch", "refs/heads/master", "master~2")
-	defer gittest.Exec(t, cfg, "-C", repoPath, "branch", "-d", "refs/heads/master")
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
-	testCases := []struct {
+	for _, tc := range []struct {
 		desc             string
 		targetRevision   string
 		expectedRevision string
 	}{
 		{
-			desc:             "tag",
+			desc:             "unqualified tag",
 			targetRevision:   "v1.0.0",
 			expectedRevision: "refs/tags/v1.0.0",
 		},
 		{
-			desc:             "tag~",
+			desc:             "parent of unqualified tag",
 			targetRevision:   "v1.0.0~",
 			expectedRevision: "refs/tags/v1.0.0~",
 		},
 		{
-			desc:             "tags/tag~",
+			desc:             "parent of semi-qualified tag",
 			targetRevision:   "tags/v1.0.0~",
 			expectedRevision: "refs/tags/v1.0.0~",
 		},
 		{
-			desc:             "refs/tag~",
+			desc:             "parent of fully-qualified tag",
 			targetRevision:   "refs/tags/v1.0.0~",
 			expectedRevision: "refs/tags/v1.0.0~",
 		},
 		{
-			desc:             "master",
-			targetRevision:   "master",
-			expectedRevision: "master",
+			desc:             "unqualified branch",
+			targetRevision:   "main",
+			expectedRevision: "refs/heads/main",
 		},
 		{
-			desc:             "heads/master",
-			targetRevision:   "heads/master",
-			expectedRevision: "refs/heads/heads/master",
+			desc:             "fully-qualified branch",
+			targetRevision:   "refs/heads/main",
+			expectedRevision: "refs/heads/main",
 		},
 		{
-			desc:             "refs/heads/master",
-			targetRevision:   "refs/heads/master",
-			expectedRevision: "refs/heads/master",
+			desc:             "ambiguous branch starting with heads",
+			targetRevision:   "heads/main",
+			expectedRevision: "refs/heads/main",
 		},
 		{
-			desc:             "heads/refs/heads/master",
-			targetRevision:   "heads/refs/heads/master",
-			expectedRevision: "refs/heads/refs/heads/master",
+			desc:             "ambiguated branch",
+			targetRevision:   "heads/heads/main",
+			expectedRevision: "refs/heads/heads/main",
 		},
-	}
+		{
+			desc:             "deep ambiguous branch",
+			targetRevision:   "heads/refs/heads/main",
+			expectedRevision: "refs/heads/refs/heads/main",
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
+			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			tagName := "what-will-it-be"
-			request := &gitalypb.UserCreateTagRequest{
-				Repository:     repo,
-				TagName:        []byte(tagName),
-				TargetRevision: []byte(testCase.targetRevision),
-				User:           gittest.TestUser,
-			}
+			baseCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(), gittest.WithMessage("1"))
 
-			response, err := client.UserCreateTag(ctx, request)
-			defer gittest.Exec(t, cfg, "-C", repoPath, "tag", "-d", tagName)
+			// We create an ambiguous branching structure that has "refs/heads/main",
+			// "refs/heads/heads/main" and "refs/heads/refs/heads/main" to exercise how
+			// we resolve the tag's target revision.
+			gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(baseCommit), gittest.WithBranch("main"), gittest.WithMessage("2"))
+			gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(baseCommit), gittest.WithBranch("heads/main"), gittest.WithMessage("3"))
+			gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(baseCommit), gittest.WithBranch("refs/heads/main"), gittest.WithMessage("4"))
+
+			taggedCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(baseCommit), gittest.WithMessage("5"))
+			gittest.WriteTag(t, cfg, repoPath, "v1.0.0", taggedCommit.Revision())
+
+			expectedCommit, err := repo.ReadCommit(ctx, git.Revision(tc.expectedRevision))
+			require.NoError(t, err)
+
+			response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
+				Repository:     repoProto,
+				TagName:        []byte("tag"),
+				TargetRevision: []byte(tc.targetRevision),
+				User:           gittest.TestUser,
+			})
 			require.NoError(t, err)
-			require.Empty(t, response.PreReceiveError)
 
-			parsedID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tagName)
-			require.Equal(t, text.ChompBytes(parsedID), response.Tag.TargetCommit.Id)
+			testhelper.ProtoEqual(t, response, &gitalypb.UserCreateTagResponse{
+				Tag: &gitalypb.Tag{
+					Id:           expectedCommit.Id,
+					Name:         []byte("tag"),
+					TargetCommit: expectedCommit,
+				},
+			})
+
+			// Perform another sanity check to verify that the tag really does point to
+			// the commit we expect it to.
+			parsedID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "tag")
+			require.Equal(t, response.Tag.TargetCommit.Id, text.ChompBytes(parsedID))
 		})
 	}
 }
 
-func TestSuccessfulUserCreateTagRequestToNonCommit(t *testing.T) {
+func TestUserCreateTag_nonCommitTarget(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
-	inputTagName := "to-be-créated-soon"
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+	blobID := gittest.WriteBlob(t, cfg, repoPath, []byte("content"))
+	treeID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+		{Path: "file", Mode: "100644", Content: "something"},
+	})
 
-	testCases := []struct {
+	for _, tc := range []struct {
 		desc               string
 		tagName            string
 		message            string
-		targetRevision     string
+		targetRevision     git.Revision
 		expectedTag        *gitalypb.Tag
 		expectedObjectType string
 	}{
 		{
 			desc:           "lightweight tag to tree",
-			tagName:        inputTagName,
-			targetRevision: "612036fac47c5d31c212b17268e2f3ba807bce1e",
+			tagName:        "lightweight-to-tree",
+			targetRevision: treeID.Revision(),
 			expectedTag: &gitalypb.Tag{
-				Name: []byte(inputTagName),
-				Id:   "612036fac47c5d31c212b17268e2f3ba807bce1e",
+				Name: []byte("lightweight-to-tree"),
+				Id:   treeID.String(),
 			},
 			expectedObjectType: "tree",
 		},
 		{
 			desc:           "lightweight tag to blob",
-			tagName:        inputTagName,
-			targetRevision: "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
+			tagName:        "lightweight-to-blob",
+			targetRevision: blobID.Revision(),
 			expectedTag: &gitalypb.Tag{
-				Name: []byte(inputTagName),
-				Id:   "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
+				Name: []byte("lightweight-to-blob"),
+				Id:   blobID.String(),
 			},
 			expectedObjectType: "blob",
 		},
 		{
 			desc:           "annotated tag to tree",
-			tagName:        inputTagName,
-			targetRevision: "612036fac47c5d31c212b17268e2f3ba807bce1e",
+			tagName:        "annotated-to-tree",
+			targetRevision: treeID.Revision(),
 			message:        "This is an annotated tag",
 			expectedTag: &gitalypb.Tag{
-				Name: []byte(inputTagName),
-				// Id: is a new object, filled in below
-				TargetCommit: nil,
-				Message:      []byte("This is an annotated tag"),
-				MessageSize:  24,
+				Name:        []byte("annotated-to-tree"),
+				Message:     []byte("This is an annotated tag"),
+				MessageSize: 24,
 			},
 			expectedObjectType: "tag",
 		},
 		{
 			desc:           "annotated tag to blob",
-			tagName:        inputTagName,
-			targetRevision: "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
+			tagName:        "annotated-to-blob",
+			targetRevision: blobID.Revision(),
 			message:        "This is an annotated tag",
 			expectedTag: &gitalypb.Tag{
-				Name: []byte(inputTagName),
-				// Id: is a new object, filled in below
-				TargetCommit: nil,
-				Message:      []byte("This is an annotated tag"),
-				MessageSize:  24,
+				Name:        []byte("annotated-to-blob"),
+				Message:     []byte("This is an annotated tag"),
+				MessageSize: 24,
 			},
 			expectedObjectType: "tag",
 		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			writeAssertObjectTypePreReceiveHook(t, repoPath, testCase.expectedObjectType)
-			writeAssertObjectTypeUpdateHook(t, repoPath, testCase.expectedObjectType)
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			writeAssertObjectTypePreReceiveHook(t, repoPath, tc.expectedObjectType)
+			writeAssertObjectTypeUpdateHook(t, repoPath, tc.expectedObjectType)
 
-			request := &gitalypb.UserCreateTagRequest{
+			response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
 				Repository:     repo,
-				TagName:        []byte(testCase.tagName),
-				TargetRevision: []byte(testCase.targetRevision),
+				TagName:        []byte(tc.tagName),
+				TargetRevision: []byte(tc.targetRevision),
 				User:           gittest.TestUser,
-				Message:        []byte(testCase.message),
-			}
-
-			responseOk := &gitalypb.UserCreateTagResponse{
-				Tag: testCase.expectedTag,
-			}
-			response, err := client.UserCreateTag(ctx, request)
+				Message:        []byte(tc.message),
+			})
 			require.NoError(t, err)
-			require.Empty(t, response.PreReceiveError)
-			defer gittest.Exec(t, cfg, "-C", repoPath, "tag", "-d", inputTagName)
 
-			// Fake up *.Id for annotated tags
-			if len(testCase.expectedTag.Id) == 0 {
-				tagID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", inputTagName)
-				responseOk.Tag.Id = text.ChompBytes(tagID)
+			// We cannot know the object ID of the annotated tags beforehand, so we just
+			// fill in this detail now.
+			if len(tc.expectedTag.Id) == 0 {
+				tc.expectedTag.Id = text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tc.tagName))
 			}
-			testhelper.ProtoEqual(t, responseOk, response)
-
-			peeledID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", inputTagName+"^{}")
-			require.Equal(t, testCase.targetRevision, text.ChompBytes(peeledID))
+			testhelper.ProtoEqual(t, &gitalypb.UserCreateTagResponse{
+				Tag: tc.expectedTag,
+			}, response)
 
-			objectType := gittest.Exec(t, cfg, "-C", repoPath, "cat-file", "-t", inputTagName)
-			require.Equal(t, testCase.expectedObjectType, text.ChompBytes(objectType))
+			peeledID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tc.tagName+"^{}")
+			require.Equal(t, tc.targetRevision.String(), text.ChompBytes(peeledID))
 		})
 	}
 }
 
-func TestSuccessfulUserCreateTagNestedTags(t *testing.T) {
+func TestUserCreateTag_nestedTags(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -691,7 +687,7 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) {
 
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-	testCases := []struct {
+	for _, tc := range []struct {
 		desc             string
 		targetObject     string
 		targetObjectType string
@@ -712,20 +708,18 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) {
 			targetObject:     "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
 			targetObjectType: "blob",
 		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
 			// We resolve down to commit/tree/blob, but we'll only ever push a "tag"
 			// here.
 			writeAssertObjectTypePreReceiveHook(t, repoPath, "tag")
 			writeAssertObjectTypeUpdateHook(t, repoPath, "tag")
 
-			targetObject := testCase.targetObject
+			targetObject := tc.targetObject
 			nestLevel := 2
 			for i := 0; i <= nestLevel; i++ {
 				tagName := fmt.Sprintf("nested-tag-%v", i)
-				tagMessage := fmt.Sprintf("This is level %v of a nested annotated tag to %v", i, testCase.targetObject)
+				tagMessage := fmt.Sprintf("This is level %v of a nested annotated tag to %v", i, tc.targetObject)
 				request := &gitalypb.UserCreateTagRequest{
 					Repository:     repoProto,
 					TagName:        []byte(tagName),
@@ -751,15 +745,15 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) {
 				}
 				// Fake it up for all levels, except for ^{} == "commit"
 				responseOk.Tag.TargetCommit = response.Tag.TargetCommit
-				if testCase.targetObjectType == "commit" {
-					responseOk.Tag.TargetCommit, err = repo.ReadCommit(ctx, git.Revision(testCase.targetObject))
+				if tc.targetObjectType == "commit" {
+					responseOk.Tag.TargetCommit, err = repo.ReadCommit(ctx, git.Revision(tc.targetObject))
 					require.NoError(t, err)
 				}
 				testhelper.ProtoEqual(t, responseOk, response)
 
 				peeledID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tagName+"^{}")
 				peeledIDStr := text.ChompBytes(peeledID)
-				require.Equal(t, testCase.targetObject, peeledIDStr)
+				require.Equal(t, tc.targetObject, peeledIDStr)
 
 				// Set up the next level of nesting...
 				targetObject = response.Tag.Id
@@ -784,7 +778,7 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) {
 				responseOk = &gitalypb.UserCreateTagResponse{
 					Tag: &gitalypb.Tag{
 						Name:         request.TagName,
-						Id:           testCase.targetObject,
+						Id:           tc.targetObject,
 						TargetCommit: responseOk.Tag.TargetCommit,
 						Message:      nil,
 						MessageSize:  0,
@@ -794,22 +788,29 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) {
 
 				createdIDLight := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tagNameLight)
 				createdIDLightStr := text.ChompBytes(createdIDLight)
-				require.Equal(t, testCase.targetObject, createdIDLightStr)
+				require.Equal(t, tc.targetObject, createdIDLightStr)
 			}
 		})
 	}
 }
 
-func TestUserCreateTagStableTagIDs(t *testing.T) {
+func TestUserCreateTag_stableTagIDs(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, _, repo, _, client := setupOperationsService(t, ctx)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
+
+	repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
+	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+	commit, err := repo.ReadCommit(ctx, commitID.Revision())
+	require.NoError(t, err)
 
 	response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
-		Repository:     repo,
+		Repository:     repoProto,
 		TagName:        []byte("happy-tag"),
-		TargetRevision: []byte("dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82"),
+		TargetRevision: []byte(commitID),
 		Message:        []byte("my message"),
 		User:           gittest.TestUser,
 		Timestamp:      &timestamppb.Timestamp{Seconds: 12345},
@@ -817,14 +818,15 @@ func TestUserCreateTagStableTagIDs(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(t, &gitalypb.Tag{
-		Id:          "123b02f05cc249a7da87aae583babb8e4871cd65",
-		Name:        []byte("happy-tag"),
-		Message:     []byte("my message"),
-		MessageSize: 10,
+		Id:           "d877784c740f492d74e6073de649a6b046ab3656",
+		Name:         []byte("happy-tag"),
+		Message:      []byte("my message"),
+		MessageSize:  10,
+		TargetCommit: commit,
 	}, response.Tag)
 }
 
-func TestUserDeleteTagSuccessfulDeletionOfPrefixedTag(t *testing.T) {
+func TestUserDeleteTag_prefixedTag(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -868,98 +870,82 @@ func TestUserDeleteTagSuccessfulDeletionOfPrefixedTag(t *testing.T) {
 	}
 }
 
-func TestUserCreateTagsuccessfulCreationOfPrefixedTag(t *testing.T) {
+func TestUserCreateTag_prefixedTag(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
+	repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-	testCases := []struct {
-		desc                   string
-		tagNameInput           string
-		tagTargetRevisionInput string
-		user                   *gitalypb.User
-		err                    error
-	}{
-		{
-			desc:                   "possible to create a tag called refs/tags/something",
-			tagNameInput:           "refs/tags/can-create-this",
-			tagTargetRevisionInput: "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863",
-			user:                   gittest.TestUser,
-			err:                    nil,
-		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			defer gittest.Exec(t, cfg, "-C", repoPath, "tag", "-d", testCase.tagNameInput)
-
-			request := &gitalypb.UserCreateTagRequest{
-				Repository:     repoProto,
-				TagName:        []byte(testCase.tagNameInput),
-				TargetRevision: []byte(testCase.tagTargetRevisionInput),
-				User:           testCase.user,
-			}
-
-			response, err := client.UserCreateTag(ctx, request)
-			testhelper.RequireGrpcError(t, testCase.err, err)
-			commitOk, err := repo.ReadCommit(ctx, git.Revision(testCase.tagTargetRevisionInput))
-			require.NoError(t, err)
-
-			responseOk := &gitalypb.UserCreateTagResponse{
-				Tag: &gitalypb.Tag{
-					Name:         []byte(testCase.tagNameInput),
-					Id:           testCase.tagTargetRevisionInput,
-					TargetCommit: commitOk,
-				},
-			}
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+	commit, err := repo.ReadCommit(ctx, commitID.Revision())
+	require.NoError(t, err)
 
-			testhelper.ProtoEqual(t, responseOk, response)
+	// We try to create a tag that has a nested name of "refs/tags/refs/tags/".
+	response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
+		Repository:     repoProto,
+		TagName:        []byte("refs/tags/can-create-this"),
+		TargetRevision: []byte(commitID),
+		User:           gittest.TestUser,
+	})
+	require.NoError(t, err)
+	testhelper.ProtoEqual(t, &gitalypb.UserCreateTagResponse{
+		Tag: &gitalypb.Tag{
+			Name:         []byte("refs/tags/can-create-this"),
+			Id:           commitID.String(),
+			TargetCommit: commit,
+		},
+	}, response)
 
-			refs := gittest.Exec(t, cfg, "-C", repoPath, "for-each-ref", "--", "refs/tags/"+testCase.tagNameInput)
-			require.Contains(t, string(refs), testCase.tagTargetRevisionInput, "tag created, we did not strip off refs/tags/*")
-		})
-	}
+	// Verify that the tag indeed has the awkward but expected name.
+	require.Equal(t,
+		text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/tags/refs/tags/can-create-this")),
+		commitID.String(),
+	)
 }
 
-func TestSuccessfulGitHooksForUserCreateTagRequest(t *testing.T) {
+func TestUserCreateTag_gitHooks(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
-
-	projectPath := "project/path"
-	repo.GlProjectPath = projectPath
-
-	tagName := "new-tag"
-
-	request := &gitalypb.UserCreateTagRequest{
-		Repository:     repo,
-		TagName:        []byte(tagName),
-		TargetRevision: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"),
-		User:           gittest.TestUser,
-	}
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
 	for _, hookName := range GitlabHooks {
 		t.Run(hookName, func(t *testing.T) {
-			defer gittest.Exec(t, cfg, "-C", repoPath, "tag", "-d", tagName)
+			repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg)
+			repo := localrepo.NewTestRepo(t, cfg, repoProto)
+
+			commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+			commit, err := repo.ReadCommit(ctx, commitID.Revision())
+			require.NoError(t, err)
 
 			hookOutputTempPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName)
 
-			response, err := client.UserCreateTag(ctx, request)
+			response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
+				Repository:     repoProto,
+				TagName:        []byte("v1.0.0"),
+				TargetRevision: []byte(commitID),
+				User:           gittest.TestUser,
+			})
 			require.NoError(t, err)
-			require.Empty(t, response.PreReceiveError)
+			testhelper.ProtoEqual(t, &gitalypb.UserCreateTagResponse{
+				Tag: &gitalypb.Tag{
+					Name:         []byte("v1.0.0"),
+					Id:           commitID.String(),
+					TargetCommit: commit,
+				},
+			}, response)
 
 			output := string(testhelper.MustReadFile(t, hookOutputTempPath))
 			require.Contains(t, output, "GL_USERNAME="+gittest.TestUser.GlUsername)
-			require.Contains(t, output, "GL_PROJECT_PATH="+projectPath)
+			require.Contains(t, output, "GL_PROJECT_PATH=gitlab-org/gitlab-test")
 		})
 	}
 }
 
-func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) {
+func TestUserDeleteTag_invalidArgument(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -1030,7 +1016,7 @@ func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) {
 	}
 }
 
-func TestFailedUserDeleteTagDueToHooks(t *testing.T) {
+func TestUserDeleteTag_hookFailure(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -1062,195 +1048,184 @@ func TestFailedUserDeleteTagDueToHooks(t *testing.T) {
 	}
 }
 
-func TestFailedUserCreateTagDueToHooks(t *testing.T) {
+func TestUserCreateTag_hookFailure(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, _, repo, repoPath, client := setupOperationsService(t, ctx)
-
-	request := &gitalypb.UserCreateTagRequest{
-		Repository:     repo,
-		TagName:        []byte("new-tag"),
-		TargetRevision: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"),
-		User:           gittest.TestUser,
-	}
-
-	hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID\nexit 1")
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
 	for _, hookName := range gitlabPreHooks {
-		gittest.WriteCustomHook(t, repoPath, hookName, hookContent)
+		repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+		commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
 
-		response, err := client.UserCreateTag(ctx, request)
+		gittest.WriteCustomHook(t, repoPath, hookName, []byte(
+			"#!/bin/sh\necho GL_ID=$GL_ID\nexit 1"),
+		)
+
+		response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
+			Repository:     repo,
+			TagName:        []byte("new-tag"),
+			TargetRevision: []byte(commitID),
+			User:           gittest.TestUser,
+		})
 		require.NoError(t, err)
 		require.Contains(t, response.PreReceiveError, "GL_ID="+gittest.TestUser.GlId)
 	}
 }
 
-func TestFailedUserCreateTagRequestDueToTagExistence(t *testing.T) {
+func TestUserCreateTag_preexisting(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	ctx, _, repo, _, client := setupOperationsService(t, ctx)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
-	testCases := []struct {
-		desc           string
-		tagName        string
-		targetRevision string
-		user           *gitalypb.User
-		response       *gitalypb.UserCreateTagResponse
-		err            error
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+	gittest.WriteTag(t, cfg, repoPath, "v1.1.0", commitID.Revision())
+
+	for _, tc := range []struct {
+		desc             string
+		tagName          string
+		targetRevision   string
+		user             *gitalypb.User
+		expectedResponse *gitalypb.UserCreateTagResponse
+		expectedErr      error
 	}{
 		{
 			desc:           "simple existing tag",
 			tagName:        "v1.1.0",
-			targetRevision: "master",
+			targetRevision: commitID.String(),
 			user:           gittest.TestUser,
-			response: &gitalypb.UserCreateTagResponse{
-				Tag:    nil,
+			expectedResponse: &gitalypb.UserCreateTagResponse{
 				Exists: true,
 			},
-			err: nil,
 		},
 		{
 			desc:           "existing tag nonexisting target revision",
 			tagName:        "v1.1.0",
 			targetRevision: "does-not-exist",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.FailedPrecondition, "revspec '%s' not found", "does-not-exist"),
+			expectedErr:    helper.ErrFailedPreconditionf("revspec 'does-not-exist' not found"),
 		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			request := &gitalypb.UserCreateTagRequest{
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			response, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
 				Repository:     repo,
-				TagName:        []byte(testCase.tagName),
-				TargetRevision: []byte(testCase.targetRevision),
-				User:           testCase.user,
-			}
-
-			response, err := client.UserCreateTag(ctx, request)
-			testhelper.RequireGrpcError(t, testCase.err, err)
-			testhelper.ProtoEqual(t, testCase.response, response)
+				TagName:        []byte(tc.tagName),
+				TargetRevision: []byte(tc.targetRevision),
+				User:           tc.user,
+			})
+			testhelper.RequireGrpcError(t, tc.expectedErr, err)
+			testhelper.ProtoEqual(t, tc.expectedResponse, response)
 		})
 	}
 }
 
-func TestFailedUserCreateTagRequestDueToValidation(t *testing.T) {
+func TestUserCreateTag_invalidArgument(t *testing.T) {
 	t.Parallel()
+
 	ctx := testhelper.Context(t)
+	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
-	ctx, _, repo, _, client := setupOperationsService(t, ctx)
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
 
 	injectedTag := "inject-tag\ntagger . <> 0 +0000\n\nInjected subject\n\n"
-	testCases := []struct {
+
+	for _, tc := range []struct {
 		desc           string
 		tagName        string
 		targetRevision string
 		message        string
 		user           *gitalypb.User
-		response       *gitalypb.UserCreateTagResponse
-		err            error
+		expectedErr    error
 	}{
 		{
 			desc:           "empty target revision",
 			tagName:        "shiny-new-tag",
 			targetRevision: "",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Error(codes.InvalidArgument, "empty target revision"),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: empty target revision"),
 		},
 		{
 			desc:           "empty user",
 			tagName:        "shiny-new-tag",
-			targetRevision: "master",
+			targetRevision: "main",
 			user:           nil,
-			response:       nil,
-			err:            status.Error(codes.InvalidArgument, "empty user"),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: empty user"),
 		},
 		{
 			desc:           "empty starting point",
 			tagName:        "new-tag",
 			targetRevision: "",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Error(codes.InvalidArgument, "empty target revision"),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: empty target revision"),
 		},
 		{
 			desc:           "non-existing starting point",
 			tagName:        "new-tag",
 			targetRevision: "i-dont-exist",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.FailedPrecondition, "revspec '%s' not found", "i-dont-exist"),
+			expectedErr:    helper.ErrFailedPreconditionf("revspec '%s' not found", "i-dont-exist"),
 		},
 		{
 			desc:           "space in lightweight tag name",
 			tagName:        "a tag",
-			targetRevision: "master",
+			targetRevision: "main",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", "a tag"),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "space in annotated tag name",
 			tagName:        "a tag",
-			targetRevision: "master",
+			targetRevision: "main",
 			message:        "a message",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", "a tag"),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "newline in lightweight tag name",
 			tagName:        "a\ntag",
-			targetRevision: "master",
+			targetRevision: "main",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", "a\ntag"),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "newline in annotated tag name",
 			tagName:        "a\ntag",
-			targetRevision: "master",
+			targetRevision: "main",
 			message:        "a message",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Error(codes.Unknown, "Rugged::InvalidError: failed to parse signature - expected prefix doesn't match actual"),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "injection in lightweight tag name",
 			tagName:        injectedTag,
-			targetRevision: "master",
+			targetRevision: "main",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", injectedTag),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "injection in annotated tag name",
 			tagName:        injectedTag,
-			targetRevision: "master",
+			targetRevision: "main",
 			message:        "a message",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", injectedTag),
+			expectedErr:    helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
 			request := &gitalypb.UserCreateTagRequest{
 				Repository:     repo,
-				TagName:        []byte(testCase.tagName),
-				TargetRevision: []byte(testCase.targetRevision),
-				User:           testCase.user,
-				Message:        []byte(testCase.message),
+				TagName:        []byte(tc.tagName),
+				TargetRevision: []byte(tc.targetRevision),
+				User:           tc.user,
+				Message:        []byte(tc.message),
 			}
 
 			response, err := client.UserCreateTag(ctx, request)
-			testhelper.RequireGrpcError(t, testCase.err, err)
-			testhelper.ProtoEqual(t, testCase.response, response)
+			testhelper.RequireGrpcError(t, tc.expectedErr, err)
+			require.Nil(t, response)
 		})
 	}
 }
diff --git a/proto/go/gitalypb/operations.pb.go b/proto/go/gitalypb/operations.pb.go
index bfff67ddc..539abbc8c 100644
--- a/proto/go/gitalypb/operations.pb.go
+++ b/proto/go/gitalypb/operations.pb.go
@@ -806,26 +806,29 @@ func (x *UserDeleteTagResponse) GetPreReceiveError() string {
 	return ""
 }
 
-// This comment is left unintentionally blank.
+// UserCreateTagRequest is a request for the UserCreateTag RPC.
 type UserCreateTagRequest struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	// repository is the repository in which the tag shall be created.
+	// Repository is the repository in which the tag shall be created.
 	Repository *Repository `protobuf:"bytes,1,opt,name=repository,proto3" json:"repository,omitempty"`
-	// tag_name is the name of the tag that shall be created.
+	// TagName is the name of the tag that shall be created. Note that this should be set to the name
+	// only: if you want to create a tag `refs/heads/v1.0`, you need to pass `v1.0` as TagName.
 	TagName []byte `protobuf:"bytes,2,opt,name=tag_name,json=tagName,proto3" json:"tag_name,omitempty"`
-	// user is the user as which the tag shall be created.
+	// User is the user as which the tag shall be created. This user is used to perform access checks
+	// against Rails' `/internal/allowed` endpoint.
 	User *User `protobuf:"bytes,3,opt,name=user,proto3" json:"user,omitempty"`
-	// target_revision is the revision which the tag should point to.
+	// TargetRevision is the revision that the newly created tag should be pointing to. Note that if
+	// the revision points to a tag, that tag will be peeled to the commit it is pointing to. If the
+	// TargetRevision does not point to a commit then the RPC will return an error.
 	TargetRevision []byte `protobuf:"bytes,4,opt,name=target_revision,json=targetRevision,proto3" json:"target_revision,omitempty"`
-	// message is the message of the tag. If it is empty, a lightweight tag is
-	// created. Otherwise, an annotated tag is created.
+	// Message is the message of the tag. If it is empty, a lightweight tag is created. Otherwise, an
+	// annotated tag is created.
 	Message []byte `protobuf:"bytes,5,opt,name=message,proto3" json:"message,omitempty"`
-	// timestamp is the optional timestamp to use for the created tag tags. If
-	// it's not set, the current time will be used. It's only used if an
-	// annotated tag is being created.
+	// Timestamp is the optional timestamp to use for the created tag tags. If it's not set, the
+	// current time will be used. It's only used if an annotated tag is being created.
 	Timestamp *timestamppb.Timestamp `protobuf:"bytes,7,opt,name=timestamp,proto3" json:"timestamp,omitempty"`
 }
 
@@ -903,7 +906,7 @@ func (x *UserCreateTagRequest) GetTimestamp() *timestamppb.Timestamp {
 	return nil
 }
 
-// This comment is left unintentionally blank.
+// UserCreateTagResponse is a response for the UserCreateTag RPC.
 type UserCreateTagResponse struct {
 	state         protoimpl.MessageState
 	sizeCache     protoimpl.SizeCache
diff --git a/proto/go/gitalypb/operations_grpc.pb.go b/proto/go/gitalypb/operations_grpc.pb.go
index e2538ab0e..f472462dc 100644
--- a/proto/go/gitalypb/operations_grpc.pb.go
+++ b/proto/go/gitalypb/operations_grpc.pb.go
@@ -41,7 +41,8 @@ type OperationServiceClient interface {
 	//   `gitaly_user_delete_branch_structured_errors` feature flag is set this error case will
 	//   instead return `FailedPrecondition` with a `ReferenceUpdate` structured error.
 	UserDeleteBranch(ctx context.Context, in *UserDeleteBranchRequest, opts ...grpc.CallOption) (*UserDeleteBranchResponse, error)
-	// UserCreateTag creates a new tag.
+	// UserCreateTag creates a new tag. This RPC knows to create both lightweight and annotated tags
+	// depending on whether a message is set.
 	UserCreateTag(ctx context.Context, in *UserCreateTagRequest, opts ...grpc.CallOption) (*UserCreateTagResponse, error)
 	// This comment is left unintentionally blank.
 	UserDeleteTag(ctx context.Context, in *UserDeleteTagRequest, opts ...grpc.CallOption) (*UserDeleteTagResponse, error)
@@ -356,7 +357,8 @@ type OperationServiceServer interface {
 	//   `gitaly_user_delete_branch_structured_errors` feature flag is set this error case will
 	//   instead return `FailedPrecondition` with a `ReferenceUpdate` structured error.
 	UserDeleteBranch(context.Context, *UserDeleteBranchRequest) (*UserDeleteBranchResponse, error)
-	// UserCreateTag creates a new tag.
+	// UserCreateTag creates a new tag. This RPC knows to create both lightweight and annotated tags
+	// depending on whether a message is set.
 	UserCreateTag(context.Context, *UserCreateTagRequest) (*UserCreateTagResponse, error)
 	// This comment is left unintentionally blank.
 	UserDeleteTag(context.Context, *UserDeleteTagRequest) (*UserDeleteTagResponse, error)
diff --git a/proto/operations.proto b/proto/operations.proto
index 17ab4b052..1e74b914b 100644
--- a/proto/operations.proto
+++ b/proto/operations.proto
@@ -49,7 +49,8 @@ service OperationService {
     };
   }
 
-  // UserCreateTag creates a new tag.
+  // UserCreateTag creates a new tag. This RPC knows to create both lightweight and annotated tags
+  // depending on whether a message is set.
   rpc UserCreateTag(UserCreateTagRequest) returns (UserCreateTagResponse) {
     option (op_type) = {
       op: MUTATOR
@@ -257,26 +258,29 @@ message UserDeleteTagResponse {
   string pre_receive_error = 1;
 }
 
-// This comment is left unintentionally blank.
+// UserCreateTagRequest is a request for the UserCreateTag RPC.
 message UserCreateTagRequest {
-  // repository is the repository in which the tag shall be created.
+  // Repository is the repository in which the tag shall be created.
   Repository repository = 1 [(target_repository)=true];
-  // tag_name is the name of the tag that shall be created.
+  // TagName is the name of the tag that shall be created. Note that this should be set to the name
+  // only: if you want to create a tag `refs/heads/v1.0`, you need to pass `v1.0` as TagName.
   bytes tag_name = 2;
-  // user is the user as which the tag shall be created.
+  // User is the user as which the tag shall be created. This user is used to perform access checks
+  // against Rails' `/internal/allowed` endpoint.
   User user = 3;
-  // target_revision is the revision which the tag should point to.
+  // TargetRevision is the revision that the newly created tag should be pointing to. Note that if
+  // the revision points to a tag, that tag will be peeled to the commit it is pointing to. If the
+  // TargetRevision does not point to a commit then the RPC will return an error.
   bytes target_revision = 4;
-  // message is the message of the tag. If it is empty, a lightweight tag is
-  // created. Otherwise, an annotated tag is created.
+  // Message is the message of the tag. If it is empty, a lightweight tag is created. Otherwise, an
+  // annotated tag is created.
   bytes message = 5;
-  // timestamp is the optional timestamp to use for the created tag tags. If
-  // it's not set, the current time will be used. It's only used if an
-  // annotated tag is being created.
+  // Timestamp is the optional timestamp to use for the created tag tags. If it's not set, the
+  // current time will be used. It's only used if an annotated tag is being created.
   google.protobuf.Timestamp timestamp = 7;
 }
 
-// This comment is left unintentionally blank.
+// UserCreateTagResponse is a response for the UserCreateTag RPC.
 message UserCreateTagResponse {
   // tag is the newly created tag.
   Tag tag = 1;
diff --git a/ruby/proto/gitaly/operations_services_pb.rb b/ruby/proto/gitaly/operations_services_pb.rb
index c7e22ac03..f2a427687 100644
--- a/ruby/proto/gitaly/operations_services_pb.rb
+++ b/ruby/proto/gitaly/operations_services_pb.rb
@@ -37,7 +37,8 @@ module Gitaly
       #   `gitaly_user_delete_branch_structured_errors` feature flag is set this error case will
       #   instead return `FailedPrecondition` with a `ReferenceUpdate` structured error.
       rpc :UserDeleteBranch, ::Gitaly::UserDeleteBranchRequest, ::Gitaly::UserDeleteBranchResponse
-      # UserCreateTag creates a new tag.
+      # UserCreateTag creates a new tag. This RPC knows to create both lightweight and annotated tags
+      # depending on whether a message is set.
       rpc :UserCreateTag, ::Gitaly::UserCreateTagRequest, ::Gitaly::UserCreateTagResponse
       # This comment is left unintentionally blank.
       rpc :UserDeleteTag, ::Gitaly::UserDeleteTagRequest, ::Gitaly::UserDeleteTagResponse