operations: Introduce structured errors for UserCreateTagpks-user-create-tag-introduce-structured-errors

The UserCreateTag RPC has several conditions where it fails to create
the tag, but still returns a successful response. That response may
either contain a pre-receive error message indicating that updating the
reference has failed because of one of multiple reasons, or it may
indicate that the tag exists already.

Returning successfully in error cases is a code smell, and we have
started to convert all RPCs that do this to instead return structured
errors. With these structured errors, we are able to keep up the
semantics that failure to perform the action results in an error but
keep the client's ability to special-case some specific errors.

Convert UserCreateTag to do the same and return UserCreateTagErrors.
This both gives us better visibility into what exactly is happening on
the Gitaly side and fixes another case where we needlessly create
replication jobs because of missing transactional votes.

Changelog: fixed

4 files changed, 353 insertions, 75 deletions

diff --git a/internal/gitaly/server/auth_test.go b/internal/gitaly/server/auth_test.go
index 5a128a8f9..464feecc5 100644
--- a/internal/gitaly/server/auth_test.go
+++ b/internal/gitaly/server/auth_test.go
@@ -28,6 +28,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/setup"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
@@ -345,6 +346,10 @@ sleep %vs
 	for i := 0; i < 2; i++ {
 		i := i
 		go func() {
+			// We don't care about the feature flag, but it's required to satisfy our
+			// sanity checks in testing. So let's just explicitly enable it here.
+			ctx := featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.UserCreateTagStructuredErrors, true)
+
 			_, err := client.UserCreateTag(ctx, &gitalypb.UserCreateTagRequest{
 				Repository:     repo,
 				TagName:        []byte(fmt.Sprintf("tag-name-%d", i)),
diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index 6b50cc0c2..f46528a9d 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -12,7 +12,9 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -88,6 +90,7 @@ func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagR
 	}
 
 	targetRevision := git.Revision(req.TargetRevision)
+	referenceName := git.ReferenceName(fmt.Sprintf("refs/tags/%s", req.TagName))
 
 	committerTime, err := dateFromProto(req)
 	if err != nil {
@@ -104,8 +107,107 @@ func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagR
 		return nil, err
 	}
 
-	referenceName := git.ReferenceName(fmt.Sprintf("refs/tags/%s", req.TagName))
+	if featureflag.UserCreateTagStructuredErrors.IsEnabled(ctx) {
+		// We check whether the reference exists up-front so that we can return a proper
+		// detailed error to the caller in case the tag cannot be created. While this is
+		// racy given that the tag can be created afterwards by a concurrent caller, we'd
+		// detect that raciness in `updateReferenceWithHooks()` anyway.
+		if oid, err := quarantineRepo.ResolveRevision(ctx, referenceName.Revision()); err != nil {
+			// If the reference wasn't found we're on the happy path, otherwise
+			// something has gone wrong.
+			if !errors.Is(err, git.ErrReferenceNotFound) {
+				return nil, helper.ErrInternalf("resolving target reference: %w", err)
+			}
+		} else {
+			// When resolving the revision succeeds the tag reference exists already.
+			// Because we don't want to overwrite preexisting references in this RPC we
+			// thus return an error and alert the caller of this condition.
+			detailedErr, err := helper.ErrWithDetails(
+				helper.ErrAlreadyExistsf("tag reference exists already"),
+				&gitalypb.UserCreateTagError{
+					Error: &gitalypb.UserCreateTagError_ReferenceExists{
+						ReferenceExists: &gitalypb.ReferenceExistsError{
+							ReferenceName: []byte(referenceName.String()),
+							Oid:           oid.String(),
+						},
+					},
+				},
+			)
+			if err != nil {
+				return nil, helper.ErrInternalf("creating detailed error: %w", err)
+			}
+
+			return nil, detailedErr
+		}
+	}
+
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, quarantineDir, referenceName, tagID, git.ObjectHashSHA1.ZeroOID); err != nil {
+		if featureflag.UserCreateTagStructuredErrors.IsEnabled(ctx) {
+			var notAllowedError hook.NotAllowedError
+			var customHookErr updateref.CustomHookError
+			var updateRefError updateref.Error
+
+			if errors.As(err, &notAllowedError) {
+				detailedErr, err := helper.ErrWithDetails(
+					helper.ErrPermissionDeniedf("reference update denied by access checks: %w", err),
+					&gitalypb.UserCreateTagError{
+						Error: &gitalypb.UserCreateTagError_AccessCheck{
+							AccessCheck: &gitalypb.AccessCheckError{
+								ErrorMessage: notAllowedError.Message,
+								UserId:       notAllowedError.UserID,
+								Protocol:     notAllowedError.Protocol,
+								Changes:      notAllowedError.Changes,
+							},
+						},
+					},
+				)
+				if err != nil {
+					return nil, helper.ErrInternalf("error details: %w", err)
+				}
+
+				return nil, detailedErr
+			} else if errors.As(err, &customHookErr) {
+				detailedErr, err := helper.ErrWithDetails(
+					// We explicitly don't include the custom hook error itself
+					// in the returned error because that would also contain the
+					// standard output or standard error in the error message.
+					// It's thus needlessly verbose and duplicates information
+					// we have available in the structured error anyway.
+					helper.ErrPermissionDeniedf("reference update denied by custom hooks"),
+					&gitalypb.UserCreateTagError{
+						Error: &gitalypb.UserCreateTagError_CustomHook{
+							CustomHook: customHookErr.Proto(),
+						},
+					},
+				)
+				if err != nil {
+					return nil, helper.ErrInternalf("error details: %w", err)
+				}
+
+				return nil, detailedErr
+			} else if errors.As(err, &updateRefError) {
+				detailedErr, err := helper.ErrWithDetails(
+					helper.ErrFailedPreconditionf("reference update failed: %w", err),
+					&gitalypb.UserCreateTagError{
+						Error: &gitalypb.UserCreateTagError_ReferenceUpdate{
+							ReferenceUpdate: &gitalypb.ReferenceUpdateError{
+								ReferenceName: []byte(updateRefError.Reference.String()),
+								OldOid:        updateRefError.OldOID.String(),
+								NewOid:        updateRefError.NewOID.String(),
+							},
+						},
+					},
+				)
+				if err != nil {
+					return nil, helper.ErrInternalf("error details: %w", err)
+				}
+
+				return nil, detailedErr
+			}
+
+			return nil, helper.ErrInternalf("updating reference: %w", err)
+		}
+
 		var customHookErrr updateref.CustomHookError
 		if errors.As(err, &customHookErrr) {
 			// TODO: this case should return a CustomHookError.
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index c1c53a315..c7b1f6ea7 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -3,6 +3,7 @@
 package operations
 
 import (
+	"context"
 	"fmt"
 	"path/filepath"
 	"testing"
@@ -16,6 +17,7 @@ import (
 	"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/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
@@ -142,7 +144,11 @@ func writeAssertObjectTypeUpdateHook(t *testing.T, repoPath, expectedObjectType
 
 func TestUserCreateTag_successful(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagSuccessful)
+}
+
+func testUserCreateTagSuccessful(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -218,12 +224,15 @@ func TestUserCreateTag_successful(t *testing.T) {
 
 func TestUserCreateTag_transactional(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagTransactional)
+}
+
+func testUserCreateTagTransactional(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runOperationServiceServer(t, cfg, testserver.WithDisablePraefect())
 
-	ctx := testhelper.Context(t)
-
 	transactionServer := &testTransactionServer{}
 
 	// We're using internal gitaly socket to connect to the server.
@@ -335,7 +344,11 @@ func TestUserCreateTag_transactional(t *testing.T) {
 
 func TestUserCreateTag_quarantine(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagQuarantine)
+}
+
+func testUserCreateTagQuarantine(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -368,18 +381,35 @@ func TestUserCreateTag_quarantine(t *testing.T) {
 		Timestamp:      timestamppb.New(time.Unix(1600000000, 0)),
 		Message:        []byte("message"),
 	})
-	require.NoError(t, err)
 
-	// 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: fmt.Sprintf(`object %s
+	expectedObject := fmt.Sprintf(`object %s
 type commit
 tag quarantined-tag
 tagger Jane Doe <janedoe@gitlab.com> 1600000000 +0800
 
-message`, commitID),
-	}, response)
+message`, commitID)
+
+	if featureflag.UserCreateTagStructuredErrors.IsEnabled(ctx) {
+		testhelper.RequireGrpcError(t, errWithDetails(t,
+			helper.ErrPermissionDeniedf("reference update denied by custom hooks"),
+			&gitalypb.UserCreateTagError{
+				Error: &gitalypb.UserCreateTagError_CustomHook{
+					CustomHook: &gitalypb.CustomHookError{
+						HookType: gitalypb.CustomHookError_HOOK_TYPE_PRERECEIVE,
+						Stdout:   []byte(expectedObject),
+					},
+				},
+			},
+		), err)
+		require.Nil(t, response)
+	} else {
+		require.NoError(t, err)
+		// 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: expectedObject,
+		}, response)
+	}
 
 	tagID := text.ChompBytes(testhelper.MustReadFile(t, tagIDOutputPath))
 
@@ -392,7 +422,11 @@ message`, commitID),
 
 func TestUserCreateTag_message(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagMessage)
+}
+
+func testUserCreateTagMessage(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -488,7 +522,11 @@ func TestUserCreateTag_message(t *testing.T) {
 
 func TestUserCreateTag_targetRevision(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagTargetRevision)
+}
+
+func testUserCreateTagTargetRevision(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -588,7 +626,11 @@ func TestUserCreateTag_targetRevision(t *testing.T) {
 
 func TestUserCreateTag_nonCommitTarget(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagNonCommitTarget)
+}
+
+func testUserCreateTagNonCommitTarget(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -681,7 +723,11 @@ func TestUserCreateTag_nonCommitTarget(t *testing.T) {
 
 func TestUserCreateTag_nestedTags(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagNestedTags)
+}
+
+func testUserCreateTagNestedTags(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
@@ -796,7 +842,11 @@ func TestUserCreateTag_nestedTags(t *testing.T) {
 
 func TestUserCreateTag_stableTagIDs(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagStableTagIDs)
+}
+
+func testUserCreateTagStableTagIDs(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -872,7 +922,11 @@ func TestUserDeleteTag_prefixedTag(t *testing.T) {
 
 func TestUserCreateTag_prefixedTag(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagPrefixedTag)
+}
+
+func testUserCreateTagPrefixedTag(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -908,7 +962,11 @@ func TestUserCreateTag_prefixedTag(t *testing.T) {
 
 func TestUserCreateTag_gitHooks(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagGitHooks)
+}
+
+func testUserCreateTagGitHooks(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -1050,32 +1108,71 @@ func TestUserDeleteTag_hookFailure(t *testing.T) {
 
 func TestUserCreateTag_hookFailure(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagHookFailure)
+}
+
+func testUserCreateTagHookFailure(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
-	for _, hookName := range gitlabPreHooks {
-		repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
-		commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
-
-		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,
+	for _, tc := range []struct {
+		hook     string
+		hookType gitalypb.CustomHookError_HookType
+	}{
+		{
+			hook:     "pre-receive",
+			hookType: gitalypb.CustomHookError_HOOK_TYPE_PRERECEIVE,
+		},
+		{
+			hook:     "update",
+			hookType: gitalypb.CustomHookError_HOOK_TYPE_UPDATE,
+		},
+	} {
+		t.Run(tc.hook, func(t *testing.T) {
+			repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+			commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents())
+
+			gittest.WriteCustomHook(t, repoPath, tc.hook, []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,
+			})
+			if featureflag.UserCreateTagStructuredErrors.IsEnabled(ctx) {
+				testhelper.RequireGrpcError(t, errWithDetails(t,
+					helper.ErrPermissionDeniedf("reference update denied by custom hooks"),
+					&gitalypb.UserCreateTagError{
+						Error: &gitalypb.UserCreateTagError_CustomHook{
+							CustomHook: &gitalypb.CustomHookError{
+								HookType: tc.hookType,
+								Stdout: []byte(
+									"GL_ID=" + gittest.TestUser.GlId + "\n",
+								),
+							},
+						},
+					},
+				), err)
+				require.Nil(t, response)
+			} else {
+				require.NoError(t, err)
+				require.Contains(t, response.PreReceiveError, "GL_ID="+gittest.TestUser.GlId)
+			}
 		})
-		require.NoError(t, err)
-		require.Contains(t, response.PreReceiveError, "GL_ID="+gittest.TestUser.GlId)
 	}
 }
 
 func TestUserCreateTag_preexisting(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagPreexisting)
+}
+
+func testUserCreateTagPreexisting(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
@@ -1096,9 +1193,32 @@ func TestUserCreateTag_preexisting(t *testing.T) {
 			tagName:        "v1.1.0",
 			targetRevision: commitID.String(),
 			user:           gittest.TestUser,
-			expectedResponse: &gitalypb.UserCreateTagResponse{
-				Exists: true,
-			},
+			expectedErr: func() error {
+				if featureflag.UserCreateTagStructuredErrors.IsDisabled(ctx) {
+					return nil
+				}
+
+				return errWithDetails(t,
+					helper.ErrAlreadyExistsf("tag reference exists already"),
+					&gitalypb.UserCreateTagError{
+						Error: &gitalypb.UserCreateTagError_ReferenceExists{
+							ReferenceExists: &gitalypb.ReferenceExistsError{
+								ReferenceName: []byte("refs/tags/v1.1.0"),
+								Oid:           commitID.String(),
+							},
+						},
+					},
+				)
+			}(),
+			expectedResponse: func() *gitalypb.UserCreateTagResponse {
+				if featureflag.UserCreateTagStructuredErrors.IsEnabled(ctx) {
+					return nil
+				}
+
+				return &gitalypb.UserCreateTagResponse{
+					Exists: true,
+				}
+			}(),
 		},
 		{
 			desc:           "existing tag nonexisting target revision",
@@ -1123,8 +1243,12 @@ func TestUserCreateTag_preexisting(t *testing.T) {
 
 func TestUserCreateTag_invalidArgument(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testUserCreateTagInvalidArgument)
+}
+
+func testUserCreateTagInvalidArgument(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx)
 
 	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
@@ -1232,52 +1356,76 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
 
 func TestTagHookOutput(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UserCreateTagStructuredErrors).Run(t, testTagHookOutput)
+}
+
+func testTagHookOutput(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
 
-	testCases := []struct {
-		desc        string
-		hookContent string
-		output      func(hookPath string) string
+	for _, tc := range []struct {
+		desc           string
+		hookContent    string
+		expectedStdout string
+		expectedStderr string
+		expectedErr    func(hookPath string) string
 	}{
 		{
 			desc:        "empty stdout and empty stderr",
 			hookContent: "#!/bin/sh\nexit 1",
-			output: func(hookPath string) string {
+			expectedErr: func(hookPath string) string {
 				return fmt.Sprintf("executing custom hooks: error executing %q: exit status 1", hookPath)
 			},
 		},
 		{
-			desc:        "empty stdout and some stderr",
-			hookContent: "#!/bin/sh\necho stderr >&2\nexit 1",
-			output:      func(string) string { return "stderr\n" },
+			desc:           "empty stdout and some stderr",
+			hookContent:    "#!/bin/sh\necho stderr >&2\nexit 1",
+			expectedStderr: "stderr\n",
+			expectedErr:    func(string) string { return "stderr\n" },
 		},
 		{
-			desc:        "some stdout and empty stderr",
-			hookContent: "#!/bin/sh\necho stdout\nexit 1",
-			output:      func(string) string { return "stdout\n" },
+			desc:           "some stdout and empty stderr",
+			hookContent:    "#!/bin/sh\necho stdout\nexit 1",
+			expectedStdout: "stdout\n",
+			expectedErr:    func(string) string { return "stdout\n" },
 		},
 		{
-			desc:        "some stdout and some stderr",
-			hookContent: "#!/bin/sh\necho stdout\necho stderr >&2\nexit 1",
-			output:      func(string) string { return "stderr\n" },
+			desc:           "some stdout and some stderr",
+			hookContent:    "#!/bin/sh\necho stdout\necho stderr >&2\nexit 1",
+			expectedStdout: "stdout\n",
+			expectedStderr: "stderr\n",
+			expectedErr:    func(string) string { return "stderr\n" },
 		},
 		{
-			desc:        "whitespace stdout and some stderr",
-			hookContent: "#!/bin/sh\necho '   '\necho stderr >&2\nexit 1",
-			output:      func(string) string { return "stderr\n" },
+			desc:           "whitespace stdout and some stderr",
+			hookContent:    "#!/bin/sh\necho '   '\necho stderr >&2\nexit 1",
+			expectedStdout: "   \n",
+			expectedStderr: "stderr\n",
+			expectedErr:    func(string) string { return "stderr\n" },
 		},
 		{
-			desc:        "some stdout and whitespace stderr",
-			hookContent: "#!/bin/sh\necho stdout\necho '   ' >&2\nexit 1",
-			output:      func(string) string { return "stdout\n" },
+			desc:           "some stdout and whitespace stderr",
+			hookContent:    "#!/bin/sh\necho stdout\necho '   ' >&2\nexit 1",
+			expectedStdout: "stdout\n",
+			expectedStderr: "   \n",
+			expectedErr:    func(string) string { return "stdout\n" },
 		},
-	}
-
-	for _, hookName := range gitlabPreHooks {
-		for _, testCase := range testCases {
-			t.Run(hookName+"/"+testCase.desc, func(t *testing.T) {
+	} {
+		for _, hookTC := range []struct {
+			hook     string
+			hookType gitalypb.CustomHookError_HookType
+		}{
+			{
+				hook:     "pre-receive",
+				hookType: gitalypb.CustomHookError_HOOK_TYPE_PRERECEIVE,
+			},
+			{
+				hook:     "update",
+				hookType: gitalypb.CustomHookError_HOOK_TYPE_UPDATE,
+			},
+		} {
+			t.Run(hookTC.hook+"/"+tc.desc, func(t *testing.T) {
 				tagNameInput := "some-tag"
 				createRequest := &gitalypb.UserCreateTagRequest{
 					Repository:     repo,
@@ -1291,18 +1439,32 @@ func TestTagHookOutput(t *testing.T) {
 					User:       gittest.TestUser,
 				}
 
-				hookFilename := gittest.WriteCustomHook(t, repoPath, hookName, []byte(testCase.hookContent))
-				expectedError := testCase.output(hookFilename)
+				hookFilename := gittest.WriteCustomHook(t, repoPath, hookTC.hook, []byte(tc.hookContent))
 
 				createResponse, err := client.UserCreateTag(ctx, createRequest)
-				require.NoError(t, err)
+				if featureflag.UserCreateTagStructuredErrors.IsEnabled(ctx) {
+					testhelper.RequireGrpcError(t, errWithDetails(t,
+						helper.ErrPermissionDeniedf("reference update denied by custom hooks"),
+						&gitalypb.UserCreateTagError{
+							Error: &gitalypb.UserCreateTagError_CustomHook{
+								CustomHook: &gitalypb.CustomHookError{
+									HookType: hookTC.hookType,
+									Stdout:   []byte(tc.expectedStdout),
+									Stderr:   []byte(tc.expectedStderr),
+								},
+							},
+						},
+					), err)
+					require.Nil(t, createResponse)
+				} else {
+					require.NoError(t, err)
 
-				createResponseOk := &gitalypb.UserCreateTagResponse{
-					Tag:             createResponse.Tag,
-					Exists:          false,
-					PreReceiveError: expectedError,
+					testhelper.ProtoEqual(t, &gitalypb.UserCreateTagResponse{
+						Tag:             createResponse.Tag,
+						Exists:          false,
+						PreReceiveError: tc.expectedErr(hookFilename),
+					}, createResponse)
 				}
-				testhelper.ProtoEqual(t, createResponseOk, createResponse)
 
 				defer gittest.Exec(t, cfg, "-C", repoPath, "tag", "-d", tagNameInput)
 				gittest.Exec(t, cfg, "-C", repoPath, "tag", tagNameInput)
@@ -1310,7 +1472,7 @@ func TestTagHookOutput(t *testing.T) {
 				deleteResponse, err := client.UserDeleteTag(ctx, deleteRequest)
 				require.NoError(t, err)
 				deleteResponseOk := &gitalypb.UserDeleteTagResponse{
-					PreReceiveError: expectedError,
+					PreReceiveError: tc.expectedErr(hookFilename),
 				}
 				testhelper.ProtoEqual(t, deleteResponseOk, deleteResponse)
 			})
diff --git a/internal/metadata/featureflag/ff_user_create_tag_structured_errors.go b/internal/metadata/featureflag/ff_user_create_tag_structured_errors.go
new file mode 100644
index 000000000..69d1342b5
--- /dev/null
+++ b/internal/metadata/featureflag/ff_user_create_tag_structured_errors.go
@@ -0,0 +1,9 @@
+package featureflag
+
+// UserCreateTagStructuredErrors will enable the use of structured errors in the UserCreateTag RPC.
+var UserCreateTagStructuredErrors = NewFeatureFlag(
+	"user_create_tag_structured_errors",
+	"v15.3.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4372",
+	false,
+)