Merge pull request #392 from runatlantis/better-gitlab-warnings

Don't parse gitlab comment events when on commit

scripts/e2e.sh

@@ -26,6 +26,7 @@ echo "Running 'make build'"
 make build
 
 echo "Running e2e test: 'make run'"
+set +e
 make run
 if [[ $? -eq 0 ]]
 then

server/events_controller.go

@@ -324,6 +324,9 @@ func (e *EventsController) handleGitlabPost(w http.ResponseWriter, r *http.Reque
 	case gitlab.MergeEvent:
 		e.Logger.Debug("handling as pull request event")
 		e.HandleGitlabMergeRequestEvent(w, event)
+	case gitlab.CommitCommentEvent:
+		e.Logger.Debug("comments on commits are not supported, only comments on merge requests")
+		e.respond(w, logging.Debug, http.StatusOK, "Ignoring comment on commit event")
 	default:
 		e.respond(w, logging.Debug, http.StatusOK, "Ignoring unsupported event")
 	}

server/events_controller_test.go

@@ -117,6 +117,18 @@ func TestPost_UnsupportedGitlabEvent(t *testing.T) {
 	responseContains(t, w, http.StatusOK, "Ignoring unsupported event")
 }
 
+// Test that if the comment comes from a commit rather than a merge request,
+// we give an error and ignore it.
+func TestPost_GitlabCommentOnCommit(t *testing.T) {
+	e, _, gl, _, _, _, _, _ := setup(t)
+	req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
+	w := httptest.NewRecorder()
+	req.Header.Set(gitlabHeader, "value")
+	When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.CommitCommentEvent{}, nil)
+	e.Post(w, req)
+	responseContains(t, w, http.StatusOK, "Ignoring comment on commit event")
+}
+
 func TestPost_GithubCommentNotCreated(t *testing.T) {
 	t.Log("when the event is a github comment but it's not a created event we ignore it")
 	e, v, _, _, _, _, _, _ := setup(t)

server/gitlab_request_parser_validator.go

@@ -80,11 +80,28 @@ func (d *DefaultGitlabRequestParserValidator) ParseAndValidate(r *http.Request,
 		}
 		return m, nil
 	case noteEventHeader:
-		var m gitlab.MergeCommentEvent
-		if err := json.Unmarshal(bytes, &m); err != nil {
+		// First, parse a small part of the json to determine if this is a
+		// comment on a merge request or a commit.
+		var subset struct {
+			ObjectAttributes struct {
+				NoteableType string `json:"noteable_type"`
+			} `json:"object_attributes"`
+		}
+		if err := json.Unmarshal(bytes, &subset); err != nil {
 			return nil, err
 		}
-		return m, nil
+
+		// We then parse into the correct comment event type.
+		switch subset.ObjectAttributes.NoteableType {
+		case "Commit":
+			var e gitlab.CommitCommentEvent
+			err := json.Unmarshal(bytes, &e)
+			return e, err
+		case "MergeRequest":
+			var e gitlab.MergeCommentEvent
+			err := json.Unmarshal(bytes, &e)
+			return e, err
+		}
 	}
 	return nil, nil
 }

server/gitlab_request_parser_validator_test.go

@@ -16,6 +16,7 @@ package server_test
 import (
 	"bytes"
 	"net/http"
+	"reflect"
 	"testing"
 
 	"github.com/lkysow/go-gitlab"
@@ -110,7 +111,19 @@ func TestValidate_ValidMergeEvent(t *testing.T) {
 	b, err := parser.ParseAndValidate(req, nil)
 	Ok(t, err)
 	Equals(t, "atlantis-example", b.(gitlab.MergeEvent).Project.Name)
+}
+
+// If the comment was on a commit instead of a merge request, make sure we
+// return the right object.
+func TestValidate_CommitCommentEvent(t *testing.T) {
 	RegisterMockTestingT(t)
+	buf := bytes.NewBufferString(commitCommentEventJSON)
+	req, err := http.NewRequest("POST", "http://localhost/event", buf)
+	Ok(t, err)
+	req.Header.Set("X-Gitlab-Event", "Note Hook")
+	b, err := parser.ParseAndValidate(req, nil)
+	Ok(t, err)
+	Equals(t, "gitlab.CommitCommentEvent", reflect.TypeOf(b).String())
 }
 
 func TestValidate_ValidMergeCommentEvent(t *testing.T) {
@@ -123,7 +136,6 @@ func TestValidate_ValidMergeCommentEvent(t *testing.T) {
 	b, err := parser.ParseAndValidate(req, nil)
 	Ok(t, err)
 	Equals(t, "Gitlab Test", b.(gitlab.MergeCommentEvent).Project.Name)
-	RegisterMockTestingT(t)
 }
 
 var mergeEventJSON = `{
@@ -417,3 +429,71 @@ var mergeCommentEventJSON = `{
     }
   }
 }`
+
+var commitCommentEventJSON = `{
+  "object_kind": "note",
+  "user": {
+    "name": "Administrator",
+    "username": "root",
+    "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
+  },
+  "project_id": 5,
+  "project":{
+    "id": 5,
+    "name":"Gitlab Test",
+    "description":"Aut reprehenderit ut est.",
+    "web_url":"http://example.com/gitlabhq/gitlab-test",
+    "avatar_url":null,
+    "git_ssh_url":"git@example.com:gitlabhq/gitlab-test.git",
+    "git_http_url":"http://example.com/gitlabhq/gitlab-test.git",
+    "namespace":"GitlabHQ",
+    "visibility_level":20,
+    "path_with_namespace":"gitlabhq/gitlab-test",
+    "default_branch":"master",
+    "homepage":"http://example.com/gitlabhq/gitlab-test",
+    "url":"http://example.com/gitlabhq/gitlab-test.git",
+    "ssh_url":"git@example.com:gitlabhq/gitlab-test.git",
+    "http_url":"http://example.com/gitlabhq/gitlab-test.git"
+  },
+  "repository":{
+    "name": "Gitlab Test",
+    "url": "http://example.com/gitlab-org/gitlab-test.git",
+    "description": "Aut reprehenderit ut est.",
+    "homepage": "http://example.com/gitlab-org/gitlab-test"
+  },
+  "object_attributes": {
+    "id": 1243,
+    "note": "This is a commit comment. How does this work?",
+    "noteable_type": "Commit",
+    "author_id": 1,
+    "created_at": "2015-05-17 18:08:09 UTC",
+    "updated_at": "2015-05-17 18:08:09 UTC",
+    "project_id": 5,
+    "attachment":null,
+    "line_code": "bec9703f7a456cd2b4ab5fb3220ae016e3e394e3_0_1",
+    "commit_id": "cfe32cf61b73a0d5e9f13e774abde7ff789b1660",
+    "noteable_id": null,
+    "system": false,
+    "st_diff": {
+      "diff": "--- /dev/null\n+++ b/six\n@@ -0,0 +1 @@\n+Subproject commit 409f37c4f05865e4fb208c771485f211a22c4c2d\n",
+      "new_path": "six",
+      "old_path": "six",
+      "a_mode": "0",
+      "b_mode": "160000",
+      "new_file": true,
+      "renamed_file": false,
+      "deleted_file": false
+    },
+    "url": "http://example.com/gitlab-org/gitlab-test/commit/cfe32cf61b73a0d5e9f13e774abde7ff789b1660#note_1243"
+  },
+  "commit": {
+    "id": "cfe32cf61b73a0d5e9f13e774abde7ff789b1660",
+    "message": "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
+    "timestamp": "2014-02-27T10:06:20+02:00",
+    "url": "http://example.com/gitlab-org/gitlab-test/commit/cfe32cf61b73a0d5e9f13e774abde7ff789b1660",
+    "author": {
+      "name": "Dmitriy Zaporozhets",
+      "email": "dmitriy.zaporozhets@gmail.com"
+    }
+  }
+}`