From 853be9660c51fa06fa6c4ab1c9611f3910b8a201 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 30 Mar 2022 14:24:47 +0200 Subject: ssh: Clean up output when pre-receive hook fails When the pre-receive hook failed, git-receive-pack(1) exits with code 0. It's arguable whether this is the expected behavior, but anyhow it means cmd.Wait() did not error out. On the other hand, the gitaly-hooks command did stop the transaction upon failure. So this final vote fails. To avoid this error being presented to the end user, ignore it when the transaction was stopped. Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/3636 Changelog: fixed --- internal/gitaly/service/ssh/receive_pack.go | 10 +++++++++- internal/gitaly/service/ssh/receive_pack_test.go | 5 +---- 2 files changed, 10 insertions(+), 5 deletions(-) (limited to 'internal/gitaly/service') diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go index b96d7833f..8501525d7 100644 --- a/internal/gitaly/service/ssh/receive_pack.go +++ b/internal/gitaly/service/ssh/receive_pack.go @@ -101,7 +101,15 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, // git-receive-pack(1) which led to a different outcome across voters, then this final vote // would fail because the sequence of votes would be different. if err := transaction.VoteOnContext(ctx, s.txManager, voting.Vote{}, voting.Committed); err != nil { - return status.Errorf(codes.Aborted, "final transactional vote: %v", err) + // When the pre-receive hook failed, git-receive-pack(1) exits with code 0. + // It's arguable whether this is the expected behavior, but anyhow it means + // cmd.Wait() did not error out. On the other hand, the gitaly-hooks command did + // stop the transaction upon failure. So this final vote fails. + // To avoid this error being presented to the end user, ignore it when the + // transaction was stopped. + if !errors.Is(err, transaction.ErrTransactionStopped) { + return status.Errorf(codes.Aborted, "final transactional vote: %v", err) + } } return nil diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go index 6cdbaa349..6439446bd 100644 --- a/internal/gitaly/service/ssh/receive_pack_test.go +++ b/internal/gitaly/service/ssh/receive_pack_test.go @@ -289,10 +289,7 @@ func TestReceivePackPushHookFailureWithCustomHook(t *testing.T) { require.Contains(t, string(slurpErr), "remote: this is wrong") require.Contains(t, string(slurpErr), "(pre-receive hook declined)") - if testhelper.IsPraefectEnabled() { - // This is a bug tracked in https://gitlab.com/gitlab-org/gitaly/-/issues/3636 - require.Contains(t, string(slurpErr), "final transactional vote: transaction was stopped") - } + require.NotContains(t, string(slurpErr), "final transactional vote: transaction was stopped") } func TestObjectPoolRefAdvertisementHidingSSH(t *testing.T) { -- cgit v1.2.3