Change signature of hook RPCs

In order to use these RPCs with the client/std_stream.go methods, the
success field needs to change to exit_status.

7 files changed, 86 insertions, 90 deletions

diff --git a/internal/service/hooks/post_receive.go b/internal/service/hooks/post_receive.go
index bfe6e628a..1cf2a6a69 100644
--- a/internal/service/hooks/post_receive.go
+++ b/internal/service/hooks/post_receive.go
@@ -40,7 +40,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
 	c := exec.Command(gitlabShellHook("post-receive"))
 	c.Dir = repoPath
 
-	success, err := streamCommandResponse(
+	status, err := streamCommandResponse(
 		stream.Context(),
 		stdin,
 		stdout, stderr,
@@ -53,7 +53,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
 	}
 
 	if err := stream.SendMsg(&gitalypb.PostReceiveHookResponse{
-		Success: success,
+		ExitStatus: &gitalypb.ExitStatus{Value: status},
 	}); err != nil {
 		return helper.ErrInternal(err)
 	}
diff --git a/internal/service/hooks/post_receive_test.go b/internal/service/hooks/post_receive_test.go
index bf9adf0fa..9be1c2099 100644
--- a/internal/service/hooks/post_receive_test.go
+++ b/internal/service/hooks/post_receive_test.go
@@ -58,36 +58,36 @@ func TestPostReceive(t *testing.T) {
 	defer conn.Close()
 
 	testCases := []struct {
-		desc    string
-		stdin   io.Reader
-		req     gitalypb.PostReceiveHookRequest
-		success bool
-		stdout  string
-		stderr  string
+		desc   string
+		stdin  io.Reader
+		req    gitalypb.PostReceiveHookRequest
+		status int32
+		stdout string
+		stderr string
 	}{
 		{
-			desc:    "valid stdin",
-			stdin:   bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
-			req:     gitalypb.PostReceiveHookRequest{Repository: testRepo, KeyId: "key_id"},
-			success: true,
-			stdout:  "OK",
-			stderr:  "",
+			desc:   "valid stdin",
+			stdin:  bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
+			req:    gitalypb.PostReceiveHookRequest{Repository: testRepo, KeyId: "key_id"},
+			status: 0,
+			stdout: "OK",
+			stderr: "",
 		},
 		{
-			desc:    "missing stdin",
-			stdin:   bytes.NewBuffer(nil),
-			req:     gitalypb.PostReceiveHookRequest{Repository: testRepo, KeyId: "key_id"},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			desc:   "missing stdin",
+			stdin:  bytes.NewBuffer(nil),
+			req:    gitalypb.PostReceiveHookRequest{Repository: testRepo, KeyId: "key_id"},
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 		{
-			desc:    "missing key_id",
-			stdin:   bytes.NewBuffer(nil),
-			req:     gitalypb.PostReceiveHookRequest{Repository: testRepo},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			desc:   "missing key_id",
+			stdin:  bytes.NewBuffer(nil),
+			req:    gitalypb.PostReceiveHookRequest{Repository: testRepo},
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 	}
 
@@ -109,7 +109,7 @@ func TestPostReceive(t *testing.T) {
 				require.NoError(t, stream.CloseSend(), "close send")
 			}()
 
-			var success bool
+			var status int32
 			var stdout, stderr bytes.Buffer
 			for {
 				resp, err := stream.Recv()
@@ -120,13 +120,10 @@ func TestPostReceive(t *testing.T) {
 				_, err = stdout.Write(resp.GetStdout())
 				require.NoError(t, err)
 				stderr.Write(resp.GetStderr())
-				require.NoError(t, err)
-
-				success = resp.GetSuccess()
-				require.NoError(t, err)
+				status = resp.GetExitStatus().GetValue()
 			}
 
-			require.Equal(t, tc.success, success)
+			require.Equal(t, tc.status, status)
 			assert.Equal(t, tc.stderr, text.ChompBytes(stderr.Bytes()), "hook stderr")
 			assert.Equal(t, tc.stdout, text.ChompBytes(stdout.Bytes()), "hook stdout")
 		})
diff --git a/internal/service/hooks/pre_receive.go b/internal/service/hooks/pre_receive.go
index 63ff2db90..86eb9877c 100644
--- a/internal/service/hooks/pre_receive.go
+++ b/internal/service/hooks/pre_receive.go
@@ -61,7 +61,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 	c := exec.Command(gitlabShellHook("pre-receive"))
 	c.Dir = repoPath
 
-	success, err := streamCommandResponse(
+	status, err := streamCommandResponse(
 		stream.Context(),
 		stdin,
 		stdout, stderr,
@@ -74,7 +74,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 	}
 
 	if err := stream.SendMsg(&gitalypb.PreReceiveHookResponse{
-		Success: success,
+		ExitStatus: &gitalypb.ExitStatus{Value: status},
 	}); err != nil {
 		return helper.ErrInternal(err)
 	}
diff --git a/internal/service/hooks/pre_receive_test.go b/internal/service/hooks/pre_receive_test.go
index 52ce8ec7b..1b7e52d6d 100644
--- a/internal/service/hooks/pre_receive_test.go
+++ b/internal/service/hooks/pre_receive_test.go
@@ -65,40 +65,40 @@ func TestPreReceive(t *testing.T) {
 		desc           string
 		stdin          io.Reader
 		req            gitalypb.PreReceiveHookRequest
-		success        bool
+		status         int32
 		stdout, stderr string
 	}{
 		{
-			desc:    "valid stdin",
-			stdin:   bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
-			req:     gitalypb.PreReceiveHookRequest{Repository: testRepo, KeyId: "key_id", Protocol: "protocol"},
-			success: true,
-			stdout:  "OK",
-			stderr:  "",
+			desc:   "valid stdin",
+			stdin:  bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
+			req:    gitalypb.PreReceiveHookRequest{Repository: testRepo, KeyId: "key_id", Protocol: "protocol"},
+			status: 0,
+			stdout: "OK",
+			stderr: "",
 		},
 		{
-			desc:    "missing stdin",
-			stdin:   bytes.NewBuffer(nil),
-			req:     gitalypb.PreReceiveHookRequest{Repository: testRepo, KeyId: "key_id", Protocol: "protocol"},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			desc:   "missing stdin",
+			stdin:  bytes.NewBuffer(nil),
+			req:    gitalypb.PreReceiveHookRequest{Repository: testRepo, KeyId: "key_id", Protocol: "protocol"},
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 		{
-			desc:    "missing protocol",
-			stdin:   bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
-			req:     gitalypb.PreReceiveHookRequest{Repository: testRepo, KeyId: "key_id"},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			desc:   "missing protocol",
+			stdin:  bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
+			req:    gitalypb.PreReceiveHookRequest{Repository: testRepo, KeyId: "key_id"},
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 		{
-			desc:    "missing key_id",
-			stdin:   bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
-			req:     gitalypb.PreReceiveHookRequest{Repository: testRepo, Protocol: "protocol"},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			desc:   "missing key_id",
+			stdin:  bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
+			req:    gitalypb.PreReceiveHookRequest{Repository: testRepo, Protocol: "protocol"},
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 	}
 
@@ -117,7 +117,7 @@ func TestPreReceive(t *testing.T) {
 				require.NoError(t, stream.CloseSend(), "close send")
 			}()
 
-			var success bool
+			var status int32
 			var stdout, stderr bytes.Buffer
 			for {
 				resp, err := stream.Recv()
@@ -133,11 +133,11 @@ func TestPreReceive(t *testing.T) {
 				_, err = stderr.Write(resp.GetStderr())
 				require.NoError(t, err)
 
-				success = resp.GetSuccess()
+				status = resp.GetExitStatus().GetValue()
 				require.NoError(t, err)
 			}
 
-			require.Equal(t, tc.success, success)
+			require.Equal(t, tc.status, status)
 			assert.Equal(t, tc.stderr, text.ChompBytes(stderr.Bytes()), "hook stderr")
 			assert.Equal(t, tc.stdout, text.ChompBytes(stdout.Bytes()), "hook stdout")
 		})
diff --git a/internal/service/hooks/stream_command.go b/internal/service/hooks/stream_command.go
index 30d8003bf..2893dd5a8 100644
--- a/internal/service/hooks/stream_command.go
+++ b/internal/service/hooks/stream_command.go
@@ -15,21 +15,20 @@ func streamCommandResponse(
 	stdout, stderr io.Writer,
 	c *exec.Cmd,
 	env []string,
-) (bool, error) {
+) (int32, error) {
 	cmd, err := command.New(ctx, c, stdin, stdout, stderr, env...)
 	if err != nil {
-		return false, helper.ErrInternal(err)
+		return 1, helper.ErrInternal(err)
 	}
 
 	err = cmd.Wait()
 	if err == nil {
-		return true, nil
+		return 0, nil
 	}
 
-	code, ok := command.ExitStatus(err)
-	if ok && code != 0 {
-		return false, nil
+	if code, ok := command.ExitStatus(err); ok {
+		return int32(code), nil
 	}
 
-	return false, err
+	return 1, err
 }
diff --git a/internal/service/hooks/update.go b/internal/service/hooks/update.go
index 6cfad6093..59d5597e6 100644
--- a/internal/service/hooks/update.go
+++ b/internal/service/hooks/update.go
@@ -31,7 +31,7 @@ func (s *server) UpdateHook(in *gitalypb.UpdateHookRequest, stream gitalypb.Hook
 	c := exec.Command(gitlabShellHook("update"), string(in.GetRef()), in.GetOldValue(), in.GetNewValue())
 	c.Dir = repoPath
 
-	success, err := streamCommandResponse(
+	status, err := streamCommandResponse(
 		stream.Context(),
 		nil,
 		stdout, stderr,
@@ -44,7 +44,7 @@ func (s *server) UpdateHook(in *gitalypb.UpdateHookRequest, stream gitalypb.Hook
 	}
 
 	if err := stream.SendMsg(&gitalypb.PreReceiveHookResponse{
-		Success: success,
+		ExitStatus: &gitalypb.ExitStatus{Value: status},
 	}); err != nil {
 		return helper.ErrInternal(err)
 	}
diff --git a/internal/service/hooks/update_test.go b/internal/service/hooks/update_test.go
index 07b64a28c..bfd30821d 100644
--- a/internal/service/hooks/update_test.go
+++ b/internal/service/hooks/update_test.go
@@ -60,7 +60,7 @@ func TestUpdate(t *testing.T) {
 	testCases := []struct {
 		desc           string
 		req            gitalypb.UpdateHookRequest
-		success        bool
+		status         int32
 		stdout, stderr string
 	}{
 		{
@@ -72,9 +72,9 @@ func TestUpdate(t *testing.T) {
 				NewValue:   "b",
 				KeyId:      "key",
 			},
-			success: true,
-			stdout:  "OK",
-			stderr:  "",
+			status: 0,
+			stdout: "OK",
+			stderr: "",
 		},
 		{
 			desc: "missing ref",
@@ -85,9 +85,9 @@ func TestUpdate(t *testing.T) {
 				NewValue:   "b",
 				KeyId:      "key",
 			},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 		{
 			desc: "missing old value",
@@ -98,9 +98,9 @@ func TestUpdate(t *testing.T) {
 				NewValue:   "b",
 				KeyId:      "key",
 			},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 		{
 			desc: "missing new value",
@@ -111,9 +111,9 @@ func TestUpdate(t *testing.T) {
 				NewValue:   "",
 				KeyId:      "key",
 			},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 		{
 			desc: "missing key_id value",
@@ -124,9 +124,9 @@ func TestUpdate(t *testing.T) {
 				NewValue:   "b",
 				KeyId:      "",
 			},
-			success: false,
-			stdout:  "",
-			stderr:  "FAIL",
+			status: 1,
+			stdout: "",
+			stderr: "FAIL",
 		},
 	}
 
@@ -135,7 +135,7 @@ func TestUpdate(t *testing.T) {
 			stream, err := client.UpdateHook(ctx, &tc.req)
 			require.NoError(t, err)
 
-			var success bool
+			var status int32
 			var stderr, stdout bytes.Buffer
 			for {
 				resp, err := stream.Recv()
@@ -150,11 +150,11 @@ func TestUpdate(t *testing.T) {
 					t.Errorf("error when receiving stream: %v", err)
 				}
 
-				success = resp.GetSuccess()
+				status = resp.GetExitStatus().GetValue()
 				require.NoError(t, err)
 			}
 
-			require.Equal(t, tc.success, success)
+			require.Equal(t, tc.status, status)
 			assert.Equal(t, tc.stderr, text.ChompBytes(stderr.Bytes()), "hook stderr")
 			assert.Equal(t, tc.stdout, text.ChompBytes(stdout.Bytes()), "hook stdout")
 		})