diff options
| author | Toon Claes <toon@gitlab.com> | 2022-03-30 15:24:47 +0300 |
|---|---|---|
| committer | Toon Claes <toon@gitlab.com> | 2022-04-04 14:37:44 +0300 |
| commit | 853be9660c51fa06fa6c4ab1c9611f3910b8a201 (patch) | |
| tree | 16c0f88e6673fa48c128be47eadab6fc8d052215 | |
| parent | 024b77a89f03912039434abb416121feea740543 (diff) | |
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
| -rw-r--r-- | internal/gitaly/service/ssh/receive_pack.go | 10 | ||||
| -rw-r--r-- | internal/gitaly/service/ssh/receive_pack_test.go | 5 |
2 files changed, 10 insertions, 5 deletions
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) { |