Merge branch 'sh-fix-push-options-gitaly-hook' into 'master'

Fix push options not working with Gitaly post-receive hook

Closes #2970

See merge request gitlab-org/gitaly!2394

3 files changed, 98 insertions, 1 deletions

diff --git a/changelogs/unreleased/sh-fix-push-options-gitaly-hook.yml b/changelogs/unreleased/sh-fix-push-options-gitaly-hook.yml
new file mode 100644
index 000000000..4dd6ea3da
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-push-options-gitaly-hook.yml
@@ -0,0 +1,5 @@
+---
+title: Fix push options not working with Gitaly post-receive hook
+merge_request: 2394
+author:
+type: fixed
diff --git a/internal/service/hook/access.go b/internal/service/hook/access.go
index 5ba74dafd..4b1793696 100644
--- a/internal/service/hook/access.go
+++ b/internal/service/hook/access.go
@@ -212,7 +212,7 @@ type PostReceiveMessage struct {
 
 // PostReceive decreases the reference counter for a push for a given gl_repository through the gitlab internal API /post_receive endpoint
 func (a *gitlabAPI) PostReceive(glRepository, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
-	resp, err := a.client.Post("/post_receive", map[string]interface{}{"gl_repository": glRepository, "identifier": glID, "changes": changes, "push_options[]": pushOptions})
+	resp, err := a.client.Post("/post_receive", map[string]interface{}{"gl_repository": glRepository, "identifier": glID, "changes": changes, "push_options": pushOptions})
 	if err != nil {
 		return false, nil, fmt.Errorf("http post to gitlab api /post_receive endpoint: %w", err)
 	}
diff --git a/internal/service/hook/access_test.go b/internal/service/hook/access_test.go
index 8566420c0..3768273c7 100644
--- a/internal/service/hook/access_test.go
+++ b/internal/service/hook/access_test.go
@@ -1,6 +1,7 @@
 package hook_test
 
 import (
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -14,6 +15,13 @@ import (
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
+type postReceiveRequest struct {
+	GLRepository string   `json:"gl_repository,omitempty"`
+	Identifier   string   `json:"identifier,omitempty"`
+	Changes      string   `json:"changes,omitempty"`
+	PushOptions  []string `json:"push_options,omitempty"`
+}
+
 func TestAllowedVerifyParams(t *testing.T) {
 	user, password := "user", "password"
 	secretToken := "topsecret"
@@ -398,3 +406,87 @@ func TestPrereceive(t *testing.T) {
 		})
 	}
 }
+
+func TestPostReceive(t *testing.T) {
+	tempDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
+
+	testhelper.WriteShellSecretFile(t, tempDir, "secret_token")
+
+	secretFilePath := filepath.Join(tempDir, ".gitlab_shell_secret")
+	var receivedRequest postReceiveRequest
+
+	testCases := []struct {
+		desc               string
+		postReceiveHandler func(w http.ResponseWriter, r *http.Request)
+		pushOptions        []string
+		success            bool
+		errMsg             string
+	}{
+		{
+			desc: "everything ok",
+			postReceiveHandler: func(w http.ResponseWriter, r *http.Request) {
+				err := json.NewDecoder(r.Body).Decode(&receivedRequest)
+				require.NoError(t, err)
+
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusOK)
+				w.Write([]byte(`{"reference_counter_decreased": true}`))
+			},
+			pushOptions: []string{"mr.create", "mr.label=test"},
+			success:     true,
+			errMsg:      "",
+		},
+		{
+			desc: "reference counter not decreased",
+			postReceiveHandler: func(w http.ResponseWriter, r *http.Request) {
+				err := json.NewDecoder(r.Body).Decode(&receivedRequest)
+				require.NoError(t, err)
+
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusOK)
+				w.Write([]byte(`{"reference_counter_increased": false}`))
+			},
+			success: false,
+			errMsg:  "",
+		},
+		{
+			desc: "server unavailable",
+			postReceiveHandler: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusServiceUnavailable)
+				w.Write([]byte(`{"message": "server is down!"}`))
+			},
+			success: false,
+			errMsg:  "server is down!",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			receivedRequest = postReceiveRequest{}
+			server := httptest.NewServer(http.HandlerFunc(tc.postReceiveHandler))
+			defer server.Close()
+
+			c, err := hook.NewGitlabAPI(config.Gitlab{
+				URL:        server.URL,
+				SecretFile: secretFilePath,
+			})
+			require.NoError(t, err)
+
+			repositoryID := "project-123"
+			identifier := "key-123"
+			changes := "000 000 refs/heads/master"
+			success, _, err := c.PostReceive(repositoryID, identifier, changes, tc.pushOptions...)
+			require.Equal(t, tc.success, success)
+			if err != nil {
+				require.Contains(t, err.Error(), tc.errMsg)
+			} else {
+				require.Equal(t, repositoryID, receivedRequest.GLRepository)
+				require.Equal(t, identifier, receivedRequest.Identifier)
+				require.Equal(t, changes, receivedRequest.Changes)
+				require.Equal(t, tc.pushOptions, receivedRequest.PushOptions)
+			}
+		})
+	}
+}