fix: gitlab support (#665)

This PR builds upon #654 to fix further Gitlab issues, and to bring it
generally into line with the Github support.

Fixes #651

go.mod

@@ -46,7 +46,7 @@ require (
 	github.com/spf13/cobra v1.1.3
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.4
-	github.com/xanzy/go-gitlab v0.73.1
+	github.com/xanzy/go-gitlab v0.95.0
 	golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb
 	golang.org/x/mod v0.11.0
 	golang.org/x/net v0.10.0

go.sum

@@ -696,6 +696,8 @@ github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+
 github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI=
 github.com/xanzy/go-gitlab v0.73.1 h1:UMagqUZLJdjss1SovIC+kJCH4k2AZWXl58gJd38Y/hI=
 github.com/xanzy/go-gitlab v0.73.1/go.mod h1:d/a0vswScO7Agg1CZNz15Ic6SSvBG9vfw8egL99t4kA=
+github.com/xanzy/go-gitlab v0.95.0 h1:lnYFPDsZuoSWXSC9xPLMcAWlGgndMn+erexGa+jJsS0=
+github.com/xanzy/go-gitlab v0.95.0/go.mod h1:ETg8tcj4OhrB84UEgeE8dSuV/0h4BBL1uOV/qK0vlyI=
 github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
 github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
 github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=

internal/ghapphandler/ghapphandler.go

@@ -2,6 +2,7 @@
 package ghapphandler
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/gorilla/mux"
@@ -34,12 +35,21 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
+	h.V(2).Info("received vcs event", "github_app", app)
+
 	// use github-specific handler to unmarshal event
-	payload := github.HandleEvent(w, r, app.WebhookSecret)
-	if payload == nil {
+	payload, err := github.HandleEvent(r, app.WebhookSecret)
+	// either ignore the event, return an error, or publish the event onwards
+	var ignore vcs.ErrIgnoreEvent
+	if errors.As(err, &ignore) {
+		h.V(2).Info("ignoring event: "+err.Error(), "github_app", app)
+		w.WriteHeader(http.StatusOK)
+		return
+	} else if err != nil {
+		h.Error(err, "handling vcs event", "github_app", app)
+		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
-	h.V(2).Info("received vcs event", "type", "github-app", "repo", payload.RepoPath)
 	// relay a copy of the event for each vcs provider configured with the
 	// github app install that triggered the event.
 	providers, err := h.VCSProviders.ListVCSProvidersByGithubAppInstall(ctx, *payload.GithubAppInstallID)
@@ -53,4 +63,5 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			EventPayload: *payload,
 		})
 	}
+	w.WriteHeader(http.StatusOK)
 }

internal/git.go

@@ -1,6 +1,9 @@
 package internal
 
-import "strings"
+import (
+	"fmt"
+	"strings"
+)
 
 // ParseBranchRef parses a git ref expecting it to be a reference to a branch. If
 // it is not then false is returned, otherwise the branch name along with
@@ -27,3 +30,12 @@ func ParseRef(ref string) (string, bool) {
 	}
 	return "", false
 }
+
+// ParseTagRef parses the tag from a git reference with the format refs/tags/<tag>
+func ParseTagRef(ref string) (string, error) {
+	tag, found := strings.CutPrefix(ref, "refs/tags/")
+	if !found {
+		return "", fmt.Errorf("invalid tag reference: %s", ref)
+	}
+	return tag, nil
+}

internal/github/repohook_handler.go → internal/github/event_handler.go

@@ -3,86 +3,65 @@ package github
 import (
 	"fmt"
 	"net/http"
+	"slices"
 	"strings"
 
 	"github.com/google/go-github/v55/github"
 	"github.com/leg100/otf/internal/vcs"
 )
 
-func HandleEvent(w http.ResponseWriter, r *http.Request, secret string) *vcs.EventPayload {
-	event, err := handleEventWithError(r, secret)
-	if err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return nil
-	}
-	w.WriteHeader(http.StatusAccepted)
-	return event
-}
-
-func handleEventWithError(r *http.Request, secret string) (*vcs.EventPayload, error) {
+func HandleEvent(r *http.Request, secret string) (*vcs.EventPayload, error) {
 	payload, err := github.ValidatePayload(r, []byte(secret))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("validating payload: %w", err)
 	}
-
 	raw, err := github.ParseWebHook(github.WebHookType(r), payload)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("parsing payload: %w", err)
 	}
 
 	// convert github event to an OTF event
-	to := vcs.EventPayload{
-		VCSKind: vcs.GithubKind,
-	}
-
+	to := vcs.EventPayload{VCSKind: vcs.GithubKind}
 	switch event := raw.(type) {
 	case *github.PushEvent:
-		// populate event with list of changed file paths
-		for _, c := range event.Commits {
-			to.Paths = c.Added
-			to.Paths = append(to.Paths, c.Modified...)
-			to.Paths = append(to.Paths, c.Removed...)
-		}
 		to.RepoPath = event.GetRepo().GetFullName()
 		to.CommitSHA = event.GetAfter()
 		to.CommitURL = event.GetHeadCommit().GetURL()
 		to.DefaultBranch = event.GetRepo().GetDefaultBranch()
-
 		to.SenderUsername = event.GetSender().GetLogin()
 		to.SenderAvatarURL = event.GetSender().GetAvatarURL()
 		to.SenderHTMLURL = event.GetSender().GetHTMLURL()
-
 		if install := event.GetInstallation(); install != nil {
 			to.GithubAppInstallID = install.ID
 		}
-
-		// a github.PushEvent includes tag events but OTF categorises them as separate
-		// event types
-		parts := strings.Split(event.GetRef(), "/")
-		if len(parts) != 3 || parts[0] != "refs" {
-			return nil, fmt.Errorf("malformed ref: %s", event.GetRef())
+		// populate event with list of changed file paths
+		for _, c := range event.Commits {
+			to.Paths = append(to.Paths, c.Added...)
+			to.Paths = append(to.Paths, c.Modified...)
+			to.Paths = append(to.Paths, c.Removed...)
 		}
-		switch parts[1] {
-		case "tags":
+		// remove duplicate file paths
+		slices.Sort(to.Paths)
+		to.Paths = slices.Compact(to.Paths)
+		// differentiate between tag and branch pushes
+		if tag, found := strings.CutPrefix(event.GetRef(), "refs/tags/"); found {
 			to.Type = vcs.EventTypeTag
-
+			to.Tag = tag
+			// tags can be created or deleted
 			switch {
 			case event.GetCreated():
 				to.Action = vcs.ActionCreated
 			case event.GetDeleted():
 				to.Action = vcs.ActionDeleted
 			default:
-				return nil, fmt.Errorf("no action specified for tag event")
+				return nil, fmt.Errorf("no action found for tag event")
 			}
-
-			to.Tag = parts[2]
-
-		case "heads":
+		} else if branch, found := strings.CutPrefix(event.GetRef(), "refs/heads/"); found {
 			to.Type = vcs.EventTypePush
+			to.Branch = branch
+			// branch pushes are always a create
 			to.Action = vcs.ActionCreated
-			to.Branch = parts[2]
-
-		default:
+		} else {
 			return nil, fmt.Errorf("malformed ref: %s", event.GetRef())
 		}
 	case *github.PullRequestEvent:
@@ -113,7 +92,7 @@ func handleEventWithError(r *http.Request, secret string) (*vcs.EventPayload, er
 			to.Action = vcs.ActionUpdated
 		default:
 			// ignore other pull request events
-			return nil, nil
+			return nil, vcs.NewErrIgnoreEvent("unsupported action: %s", event.GetAction())
 		}
 
 		to.Branch = event.PullRequest.Head.GetRef()
@@ -126,16 +105,16 @@ func handleEventWithError(r *http.Request, secret string) (*vcs.EventPayload, er
 	case *github.InstallationEvent:
 		// ignore events other than uninstallation events
 		if event.GetAction() != "deleted" {
-			return nil, nil
+			return nil, vcs.NewErrIgnoreEvent("unsupported action: %s", event.GetAction())
 		}
 		to.Action = vcs.ActionDeleted
 		to.Type = vcs.EventTypeInstallation
 		to.GithubAppInstallID = event.GetInstallation().ID
 	default:
-		return nil, nil
+		return nil, vcs.NewErrIgnoreEvent("unsupported event: %T", raw)
 	}
 	if err := to.Validate(); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed building OTF event: %w", err)
 	}
 	return &to, nil
 }

internal/github/repohook_handler_test.go → internal/github/event_handler_test.go

@@ -1,6 +1,7 @@
 package github
 
 import (
+	"errors"
 	"net/http/httptest"
 	"os"
 	"testing"
@@ -18,6 +19,7 @@ func TestEventHandler(t *testing.T) {
 		eventType string
 		body      string
 		want      *vcs.EventPayload
+		ignore    bool
 	}{
 		{
 			"push",
@@ -32,11 +34,12 @@ func TestEventHandler(t *testing.T) {
 				CommitSHA:       "42d6fc7dac35cc7945231195e248af2f6256b522",
 				CommitURL:       "https://github.com/leg100/tfc-workspaces/commit/42d6fc7dac35cc7945231195e248af2f6256b522",
 				Action:          vcs.ActionCreated,
-				Paths:           []string{"main.tf"},
+				Paths:           []string{"main.tf", "networks.tf", "servers.tf"},
 				SenderUsername:  "leg100",
 				SenderAvatarURL: "https://avatars.githubusercontent.com/u/75728?v=4",
 				SenderHTMLURL:   "https://github.com/leg100",
 			},
+			false,
 		},
 		{
 			"push from github app install",
@@ -51,12 +54,13 @@ func TestEventHandler(t *testing.T) {
 				CommitSHA:          "0a2d223fa1a3844480e3b7716cf87aacb658b91f",
 				CommitURL:          "https://github.com/leg100/otf-workspaces/commit/0a2d223fa1a3844480e3b7716cf87aacb658b91f",
 				Action:             vcs.ActionCreated,
-				Paths:              []string{},
+				Paths:              nil,
 				SenderUsername:     "leg100",
 				SenderAvatarURL:    "https://avatars.githubusercontent.com/u/75728?v=4",
 				SenderHTMLURL:      "https://github.com/leg100",
 				GithubAppInstallID: internal.Int64(42997659),
 			},
+			false,
 		},
 		{
 			"pull request opened",
@@ -78,6 +82,7 @@ func TestEventHandler(t *testing.T) {
 				SenderAvatarURL:   "https://avatars.githubusercontent.com/u/75728?v=4",
 				SenderHTMLURL:     "https://github.com/leg100",
 			},
+			false,
 		},
 		{
 			"pull request updated",
@@ -99,6 +104,7 @@ func TestEventHandler(t *testing.T) {
 				SenderAvatarURL:   "https://avatars.githubusercontent.com/u/75728?v=4",
 				SenderHTMLURL:     "https://github.com/leg100",
 			},
+			false,
 		},
 		{
 			"tag pushed",
@@ -117,6 +123,21 @@ func TestEventHandler(t *testing.T) {
 				SenderAvatarURL: "https://avatars.githubusercontent.com/u/75728?v=4",
 				SenderHTMLURL:   "https://github.com/leg100",
 			},
+			false,
+		},
+		{
+			"ignore github app install created",
+			"installation",
+			"./testdata/github_app_install_created.json",
+			nil,
+			true,
+		},
+		{
+			"ignore github app pull edited",
+			"pull_request",
+			"./testdata/github_app_pull_edited.json",
+			nil,
+			true,
 		},
 	}
 	for _, tt := range tests {
@@ -129,9 +150,15 @@ func TestEventHandler(t *testing.T) {
 			r.Header.Add("Content-type", "application/json")
 			r.Header.Add(github.EventTypeHeader, tt.eventType)
 			w := httptest.NewRecorder()
-			got := HandleEvent(w, r, "")
-			assert.Equal(t, 202, w.Code, w.Body.String())
-			assert.Equal(t, tt.want, got)
+			got, err := HandleEvent(r, "")
+			if tt.ignore {
+				var ignore vcs.ErrIgnoreEvent
+				assert.True(t, errors.As(err, &ignore))
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, 200, w.Code, w.Body.String())
+				assert.Equal(t, tt.want, got)
+			}
 		})
 	}
 }

internal/github/test_server.go

@@ -402,7 +402,7 @@ func SendEventRequest(t *testing.T, event GithubEvent, url, secret string, paylo
 	res, err := http.DefaultClient.Do(req)
 	require.NoError(t, err)
 
-	if !assert.Equal(t, http.StatusAccepted, res.StatusCode) {
+	if !assert.Equal(t, http.StatusOK, res.StatusCode) {
 		response, err := io.ReadAll(res.Body)
 		require.NoError(t, err)
 		t.Fatal(string(response))