Merge branch 'pks-user-create-tag-introduce-structured-errors' into 'master'

operations: Introduce structured errors for UserCreateTag

Closes #4137

See merge request gitlab-org/gitaly!4743

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,
+)