Fix gitlab event handler (#272)

agent/environment.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"regexp"
 	"strings"
 
 	"github.com/fatih/color"
@@ -204,7 +205,7 @@ func (e *Environment) RunCLI(name string, args ...string) error {
 	cmd.Stderr = io.MultiWriter(e.out, stderr)
 
 	if err := cmd.Start(); err != nil {
-		logger.Error(err, "starting command", "stderr", stderr.String())
+		logger.Error(err, "starting command", "stderr", cleanStderr(stderr.String()))
 		return err
 	}
 	// store process so that it can be canceled
@@ -213,7 +214,7 @@ func (e *Environment) RunCLI(name string, args ...string) error {
 	logger.V(2).Info("running command")
 
 	if err := cmd.Wait(); err != nil {
-		logger.Error(err, "running command", "stderr", stderr.String())
+		logger.Error(err, "running command", "stderr", cleanStderr(stderr.String()))
 		return err
 	}
 
@@ -339,3 +340,14 @@ func writeTerraformVariables(dir string, vars []*otf.Variable) error {
 
 	return nil
 }
+
+var ascii = regexp.MustCompile("[[:^ascii:]]")
+
+// cleanStderr cleans up stderr output to make it suitable for logging:
+// newlines, ansi escape sequences, and non-ascii characters are removed
+func cleanStderr(stderr string) string {
+	stderr = stripAnsi(stderr)
+	stderr = ascii.ReplaceAllLiteralString(stderr, "")
+	stderr = strings.Join(strings.Fields(stderr), " ")
+	return stderr
+}

agent/strip_ansi.go

@@ -0,0 +1,33 @@
+package agent
+
+/* MIT License
+
+Copyright (c) 2018 Andrew Carlson
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+*/
+
+import "regexp"
+
+// ansiEscapes is a regex for ANSI escape codes
+var ansiEscapes = regexp.MustCompile("[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))")
+
+func stripAnsi(str string) string {
+	return ansiEscapes.ReplaceAllString(str, "")
+}

github/client.go

@@ -206,7 +206,7 @@ func (g *Client) GetRepoTarball(ctx context.Context, topts cloud.GetRepoTarballO
 		return nil, err
 	}
 	if len(contents) != 1 {
-		return nil, fmt.Errorf("malformed tarball archive")
+		return nil, fmt.Errorf("expected only one top-level directory; instead got %s", contents)
 	}
 	parentDir := path.Join(untarpath, contents[0].Name())
 	return otf.Pack(parentDir)

github/client_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"net/url"
 	"os"
+	"path"
 	"testing"
 
 	"github.com/leg100/otf"
@@ -58,6 +59,7 @@ func TestGetRepoTarball(t *testing.T) {
 	dst := t.TempDir()
 	err = otf.Unpack(bytes.NewReader(got), dst)
 	require.NoError(t, err)
+	assert.FileExists(t, path.Join(dst, "main.tf"))
 }
 
 func TestCreateWebhook(t *testing.T) {

gitlab/client.go

@@ -1,11 +1,15 @@
 package gitlab
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"net/http"
 	"net/url"
+	"os"
+	"path"
 	"strconv"
+	"strings"
 
 	"github.com/leg100/otf"
 	"github.com/leg100/otf/cloud"
@@ -144,6 +148,11 @@ func (g *Client) ListTags(ctx context.Context, opts cloud.ListTagsOptions) ([]st
 }
 
 func (g *Client) GetRepoTarball(ctx context.Context, opts cloud.GetRepoTarballOptions) ([]byte, error) {
+	owner, name, found := strings.Cut(opts.Identifier, "/")
+	if !found {
+		return nil, fmt.Errorf("malformed identifier: %s", opts.Identifier)
+	}
+
 	tarball, _, err := g.client.Repositories.Archive(opts.Identifier, &gitlab.ArchiveOptions{
 		Format: otf.String("tar.gz"),
 		SHA:    otf.String(opts.Ref),
@@ -152,7 +161,24 @@ func (g *Client) GetRepoTarball(ctx context.Context, opts cloud.GetRepoTarballOp
 		return nil, err
 	}
 
-	return tarball, nil
+	// Gitlab tarball contents are contained within a top-level directory
+	// formatted <repo>-<ref>-<sha>. We want the tarball without this directory,
+	// so we re-tar the contents without the top-level directory.
+	untarpath, err := os.MkdirTemp("", fmt.Sprintf("gitlab-%s-%s-*", owner, name))
+	if err != nil {
+		return nil, err
+	}
+	if err := otf.Unpack(bytes.NewReader(tarball), untarpath); err != nil {
+		return nil, err
+	}
+	contents, err := os.ReadDir(untarpath)
+	if err != nil {
+		return nil, err
+	}
+	if len(contents) != 1 {
+		return nil, fmt.Errorf("expected only one top-level directory; instead got %s", contents)
+	}
+	return otf.Pack(path.Join(untarpath, contents[0].Name()))
 }
 
 func (g *Client) CreateWebhook(ctx context.Context, opts cloud.CreateWebhookOptions) (string, error) {

gitlab/client_test.go

@@ -1,7 +1,10 @@
 package gitlab
 
 import (
+	"bytes"
 	"context"
+	"os"
+	"path"
 	"testing"
 
 	"github.com/leg100/otf"
@@ -62,7 +65,9 @@ func TestClient(t *testing.T) {
 		assert.Equal(t, want, got)
 	})
 	t.Run("GetRepoTarball", func(t *testing.T) {
-		want := otf.NewTestTarball(t, `file1 contents`, `file2 contents`)
+		want, err := os.ReadFile("../testdata/gitlab.tar.gz")
+		require.NoError(t, err)
+
 		client := newTestClient(t,
 			WithGitlabRepo(cloud.Repo{Identifier: "acme/terraform", Branch: "master"}),
 			WithGitlabTarball(want),
@@ -74,7 +79,11 @@ func TestClient(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		assert.Equal(t, want, got)
+		dst := t.TempDir()
+		err = otf.Unpack(bytes.NewReader(got), dst)
+		require.NoError(t, err)
+		assert.FileExists(t, path.Join(dst, "afile"))
+		assert.FileExists(t, path.Join(dst, "bfile"))
 	})
 }
 

gitlab/cloud.go

@@ -14,5 +14,5 @@ func (g *Cloud) NewClient(ctx context.Context, opts cloud.ClientOptions) (cloud.
 }
 
 func (Cloud) HandleEvent(w http.ResponseWriter, r *http.Request, opts cloud.HandleEventOptions) cloud.VCSEvent {
-	return nil
+	return HandleEvent(w, r, opts)
 }

gitlab/event_handler.go

@@ -42,7 +42,7 @@ func handle(r *http.Request, opts cloud.HandleEventOptions) (cloud.VCSEvent, err
 		if len(refParts) != 3 {
 			return nil, fmt.Errorf("malformed ref: %s", event.Ref)
 		}
-		return &cloud.VCSPushEvent{
+		return cloud.VCSPushEvent{
 			WebhookID:  opts.WebhookID,
 			Branch:     refParts[2],
 			Identifier: event.Project.PathWithNamespace,
@@ -53,7 +53,7 @@ func handle(r *http.Request, opts cloud.HandleEventOptions) (cloud.VCSEvent, err
 		if len(refParts) != 3 {
 			return nil, fmt.Errorf("malformed ref: %s", event.Ref)
 		}
-		return &cloud.VCSTagEvent{
+		return cloud.VCSTagEvent{
 			WebhookID: opts.WebhookID,
 			Tag:       refParts[2],
 			// Action:     action,

http/server.go

@@ -53,22 +53,16 @@ func (cfg *ServerConfig) Validate() error {
 
 // Server provides an HTTP/S server
 type Server struct {
-	ServerConfig
-
-	server *http.Server
-
 	logr.Logger
-
-	// provides access to otf services
-	otf.Application
+	otf.Application // provides access to otf services
+	ServerConfig
+	*Router      // http router, exported so that other pkgs can add routes
+	*surl.Signer // sign and validate signed URLs
 
 	CacheService *bigcache.BigCache
 
-	// server-side-events server
-	eventsServer *sse.Server
-	// the http router, exported so that other pkgs can add routes
-	*Router
-	*surl.Signer
+	eventsServer     *sse.Server
+	server           *http.Server
 	vcsEventsHandler *otf.Triggerer
 }
 

http/webhook.go

@@ -31,9 +31,11 @@ func (h *webhookHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	hook, err := h.DB().GetWebhook(r.Context(), opts.ID)
 	if err != nil {
+		h.Error(err, "received vcs event")
 		writeError(w, http.StatusNotFound, err)
 		return
 	}
+	h.V(1).Info("received vcs event", "id", opts.ID, "repo", hook.Identifier, "cloud", hook.CloudName())
 
 	if event := hook.HandleEvent(w, r); event != nil {
 		h.events <- event

sql/queries/registry_session.sql

@@ -18,7 +18,7 @@ AND   expiry > current_timestamp
 
 -- name: DeleteExpiredRegistrySessions :one
 DELETE
-FROM sessions
+FROM registry_sessions
 WHERE expiry < current_timestamp
 RETURNING token
 ;

triggerer.go

@@ -35,7 +35,7 @@ func (h *Triggerer) Start(ctx context.Context) {
 		select {
 		case event := <-h.events:
 			if err := h.handle(ctx, event); err != nil {
-				h.Error(err, "handling event")
+				h.Error(err, "handling vcs event")
 			}
 		case <-ctx.Done():
 			return
@@ -82,6 +82,8 @@ func (h *Triggerer) triggerRun(ctx context.Context, event cloud.VCSEvent) error
 		isPullRequest = true
 	}
 
+	h.Info("triggering run", "id", webhookID)
+
 	workspaces, err := h.ListWorkspacesByWebhookID(ctx, webhookID)
 	if err != nil {
 		return err

workspace.go

@@ -284,6 +284,9 @@ type WorkspaceService interface {
 	GetWorkspace(ctx context.Context, workspaceID string) (*Workspace, error)
 	GetWorkspaceByName(ctx context.Context, organization, workspace string) (*Workspace, error)
 	ListWorkspaces(ctx context.Context, opts WorkspaceListOptions) (*WorkspaceList, error)
+	// ListWorkspacesByWebhookID retrieves workspaces by webhook ID.
+	//
+	// TODO: rename to ListConnectedWorkspaces
 	ListWorkspacesByWebhookID(ctx context.Context, id uuid.UUID) ([]*Workspace, error)
 	UpdateWorkspace(ctx context.Context, workspaceID string, opts UpdateWorkspaceOptions) (*Workspace, error)
 	DeleteWorkspace(ctx context.Context, workspaceID string) (*Workspace, error)