use push hooks for branch and tag

corinnekrych · Nov 4, 2018 · ef91064 · ef91064
1 parent 3622865
commit ef91064
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 15 deletions.
diff --git a/scm/driver/gitlab/testdata/webhooks/tag_create.json.golden b/scm/driver/gitlab/testdata/webhooks/tag_create.json.golden
@@ -1,8 +1,5 @@
 {
-    "Ref": {
-        "Name": "v1.0.0",
-        "Sha": "2adc9465c4edfc33834e173fe89436a7cb899a1d"
-    },
+    "Ref": "refs/tags/v1.0.0",
     "Repo": {
         "ID": "4861503",
         "Namespace": "sytses",
@@ -16,7 +13,25 @@
         "Created": "0001-01-01T00:00:00Z",
         "Updated": "0001-01-01T00:00:00Z"
     },
-    "Action": "created",
+    "Commit": {
+        "Sha": "2adc9465c4edfc33834e173fe89436a7cb899a1d",
+        "Message": "added readme\n",
+        "Author": {
+            "Name": "Sid Sijbrandij",
+            "Email": "[email protected]",
+            "Date": "0001-01-01T00:00:00Z",
+            "Login": "sytses",
+            "Avatar": "https://secure.gravatar.com/avatar/8c58a0be77ee441bb8f8595b7f1b4e87?s=80\u0026d=identicon"
+        },
+        "Committer": {
+            "Name": "Sid Sijbrandij",
+            "Email": "[email protected]",
+            "Date": "0001-01-01T00:00:00Z",
+            "Login": "sytses",
+            "Avatar": "https://secure.gravatar.com/avatar/8c58a0be77ee441bb8f8595b7f1b4e87?s=80\u0026d=identicon"
+        },
+        "Link": "https://gitlab.com/gitlab-org/hello-world/commit/2adc9465c4edfc33834e173fe89436a7cb899a1d"
+    },
     "Sender": {
         "Login": "sytses",
         "Name": "Sid Sijbrandij",

diff --git a/scm/driver/gitlab/webhook.go b/scm/driver/gitlab/webhook.go
@@ -29,14 +29,12 @@ func (s *webhookService) Parse(req *http.Request, fn scm.SecretFunc) (scm.Webhoo
 
 	var hook scm.Webhook
 	switch req.Header.Get("X-Gitlab-Event") {
-	case "Push Hook":
+	case "Push Hook", "Tag Push Hook":
 		hook, err = parsePushHook(data)
 	case "Issue Hook":
 		return nil, scm.ErrUnknownEvent
 	case "Merge Request Hook":
 		hook, err = parsePullRequestHook(data)
-	case "Tag Push Hook":
-		return nil, scm.ErrUnknownEvent
 	default:
 		return nil, scm.ErrUnknownEvent
 	}
@@ -75,7 +73,12 @@ func parsePushHook(data []byte) (scm.Webhook, error) {
 		return convertPushHook(src), nil
 	case src.ObjectKind == "push" && src.After == "0000000000000000000000000000000000000000":
 		return converBranchHook(src), nil
-	case src.ObjectKind == "tag_push":
+	case src.ObjectKind == "tag_push" && src.Before == "0000000000000000000000000000000000000000":
+		// TODO we previously considered returning a
+		// tag creation hook, however, the push hook
+		// returns more metadata (commit details).
+		return convertPushHook(src), nil
+	case src.ObjectKind == "tag_push" && src.After == "0000000000000000000000000000000000000000":
 		return convertTagHook(src), nil
 	default:
 		return convertPushHook(src), nil
@@ -101,7 +104,7 @@ func parsePullRequestHook(data []byte) (scm.Webhook, error) {
 }
 
 func convertPushHook(src *pushHook) *scm.PushHook {
-	return &scm.PushHook{
+	dst := &scm.PushHook{
 		Ref: scm.ExpandRef(src.Ref, "refs/heads/"),
 		Repo: scm.Repository{
 			ID:        strconv.Itoa(src.Project.ID),
@@ -115,7 +118,7 @@ func convertPushHook(src *pushHook) *scm.PushHook {
 		},
 		Commit: scm.Commit{
 			Sha:     src.CheckoutSha,
-			Message: src.Commits[0].Message, // TODO need safe option
+			Message: "", // NOTE this is set below
 			Author: scm.Signature{
 				Login:  src.UserUsername,
 				Name:   src.UserName,
@@ -128,7 +131,7 @@ func convertPushHook(src *pushHook) *scm.PushHook {
 				Email:  src.UserEmail,
 				Avatar: src.UserAvatar,
 			},
-			Link: src.Commits[0].URL, // TODO need safe option
+			Link: "", // NOTE this is set below
 		},
 		Sender: scm.User{
 			Login:  src.UserUsername,
@@ -137,6 +140,11 @@ func convertPushHook(src *pushHook) *scm.PushHook {
 			Avatar: src.UserAvatar,
 		},
 	}
+	if len(src.Commits) > 0 {
+		dst.Commit.Message = src.Commits[0].Message
+		dst.Commit.Link = src.Commits[0].URL
+	}
+	return dst
 }
 
 func converBranchHook(src *pushHook) *scm.BranchHook {

diff --git a/scm/driver/gitlab/webhook_test.go b/scm/driver/gitlab/webhook_test.go
@@ -40,10 +40,10 @@ func TestWebhooks(t *testing.T) {
 		},
 		// tag hooks
 		{
-			event:  "Push Hook",
+			event:  "Tag Push Hook",
 			before: "testdata/webhooks/tag_create.json",
 			after:  "testdata/webhooks/tag_create.json.golden",
-			obj:    new(scm.TagHook),
+			obj:    new(scm.PushHook),
 		},
 		{
 			event:  "Push Hook",

diff --git a/scm/util.go b/scm/util.go
@@ -36,7 +36,7 @@ func TrimRef(ref string) string {
 // reference path (e.g refs/heads/master).
 func ExpandRef(name, prefix string) string {
 	prefix = strings.TrimSuffix(prefix, "/")
-	if strings.HasPrefix(name, prefix) {
+	if strings.HasPrefix(name, "refs/") {
 		return name
 	}
 	return prefix + "/" + name

diff --git a/scm/util_test.go b/scm/util_test.go
@@ -87,6 +87,12 @@ func TestExpandRef(t *testing.T) {
 			name:   "master",
 			prefix: "refs/heads/",
 		},
+		// is already a ref
+		{
+			after:  "refs/tags/v1.0.0",
+			name:   "refs/tags/v1.0.0",
+			prefix: "refs/heads/",
+		},
 	}
 	for _, test := range tests {
 		if got, want := ExpandRef(test.name, test.prefix), test.after; got != want {