fix(e2e): webhook-driven, auto-versioned CMT + fix non-PR dispatch 422

server/cleanup_e2e.go

@@ -0,0 +1,108 @@
+// Copyright (c) 2020-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package server
+
+import (
+	"crypto/subtle"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"strings"
+
+	"github.com/sirupsen/logrus"
+)
+
+// CleanupE2ERequest is the JSON body POSTed by compatibility-matrix-testing.yml
+// to /cleanup_e2e after the CMT matrix tests complete, so matterwick can destroy
+// the provisioned cloud installations.
+type CleanupE2ERequest struct {
+	Repo  string `json:"repo"`
+	RunID int64  `json:"run_id"`
+}
+
+// handleCleanupE2E destroys CMT instances tracked under "{repo}-cmt-{run_id}-*".
+// The run_id passed by the workflow is the CMT provisioner's run_id (cmt_run_id),
+// which matterwick embeds as the middle component of every CMT tracking key.
+//
+// NOTE: this handler does not call CheckLimitRateAndAbortRequest. Cleanup hits
+// the cloud provisioner API, not GitHub, so GitHub rate limits are not the
+// constraint. See handleCMTDispatch for the broader rationale.
+func (s *Server) handleCleanupE2E(w http.ResponseWriter, r *http.Request) {
+	logger := s.Logger.WithField("endpoint", "/cleanup_e2e")
+
+	if s.Config.CleanupSecret == "" {
+		logger.Error("CleanupSecret not configured; rejecting /cleanup_e2e request")
+		w.WriteHeader(http.StatusServiceUnavailable)
+		return
+	}
+
+	provided := r.Header.Get("X-Cleanup-Token")
+	if subtle.ConstantTimeCompare([]byte(provided), []byte(s.Config.CleanupSecret)) != 1 {
+		logger.Warn("Invalid or missing X-Cleanup-Token on /cleanup_e2e")
+		w.WriteHeader(http.StatusForbidden)
+		return
+	}
+
+	body, err := io.ReadAll(r.Body)
+	if err != nil {
+		logger.WithError(err).Error("Failed to read /cleanup_e2e body")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	var req CleanupE2ERequest
+	if err := json.Unmarshal(body, &req); err != nil {
+		logger.WithError(err).Error("Failed to parse /cleanup_e2e JSON body")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	if req.Repo == "" || req.RunID == 0 {
+		logger.WithFields(logrus.Fields{
+			"repo":   req.Repo,
+			"run_id": req.RunID,
+		}).Error("Missing required fields in /cleanup_e2e body")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	cleanupLogger := logger.WithFields(logrus.Fields{
+		"repo":   req.Repo,
+		"run_id": req.RunID,
+		"type":   "cmt_cleanup",
+	})
+
+	// Match the CMT tracking-key format: "{repo}-cmt-{run_id}-{sha}".
+	keyPrefix := fmt.Sprintf("%s-cmt-%d-", req.Repo, req.RunID)
+
+	s.e2eInstancesLock.Lock()
+	var found []*E2EInstance
+	var keysToDelete []string
+	for key, instances := range s.e2eInstances {
+		if strings.HasPrefix(key, keyPrefix) {
+			found = append(found, instances...)
+			keysToDelete = append(keysToDelete, key)
+		}
+	}
+	for _, k := range keysToDelete {
+		delete(s.e2eInstances, k)
+	}
+	s.e2eInstancesLock.Unlock()
+
+	if len(found) == 0 {
+		cleanupLogger.Info("No CMT instances tracked for this run_id; nothing to destroy")
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{"status":"no_instances"}`))
+		return
+	}
+
+	cleanupLogger.WithField("instances", len(found)).Info("Destroying CMT instances for completed run")
+	go s.destroyE2EInstances(found, cleanupLogger)
+
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusOK)
+	_, _ = w.Write([]byte(`{"status":"destroying"}`))
+}

server/cleanup_e2e_test.go

@@ -0,0 +1,157 @@
+// Copyright (c) 2020-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package server
+
+import (
+	"bytes"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+)
+
+// newCleanupE2ETestServer builds a Server for handleCleanupE2E. CloudClient
+// is deliberately nil; callers must avoid paths that trigger the destroy
+// goroutine. The "instances tracked" path is covered indirectly by
+// asserting on the e2eInstances map state and the synchronous response,
+// and by giving the tracked entry an empty []*E2EInstance slice so the
+// goroutine sees no work and exits without touching CloudClient.
+func newCleanupE2ETestServer(t *testing.T, cleanupSecret string) *Server {
+	t.Helper()
+	return &Server{
+		Config: &MatterwickConfig{
+			CleanupSecret: cleanupSecret,
+		},
+		Logger:       logrus.New(),
+		e2eInstances: make(map[string][]*E2EInstance),
+	}
+}
+
+const validCleanupBody = `{"repo": "desktop", "run_id": 26026866029}`
+
+func doCleanupE2ERequest(t *testing.T, s *Server, body, token string) *httptest.ResponseRecorder {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/cleanup_e2e", bytes.NewBufferString(body))
+	req.Header.Set("Content-Type", "application/json")
+	if token != "" {
+		req.Header.Set("X-Cleanup-Token", token)
+	}
+	w := httptest.NewRecorder()
+	s.handleCleanupE2E(w, req)
+	return w
+}
+
+func TestHandleCleanupE2E_RejectsWhenSecretUnconfigured(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "")
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "any-token")
+	assert.Equal(t, http.StatusServiceUnavailable, w.Code)
+}
+
+func TestHandleCleanupE2E_RejectsMissingToken(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "the-secret")
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "")
+	assert.Equal(t, http.StatusForbidden, w.Code)
+}
+
+func TestHandleCleanupE2E_RejectsWrongToken(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "the-secret")
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "not-the-secret")
+	assert.Equal(t, http.StatusForbidden, w.Code)
+}
+
+func TestHandleCleanupE2E_RejectsMalformedJSON(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "the-secret")
+	w := doCleanupE2ERequest(t, s, `{not json`, "the-secret")
+	assert.Equal(t, http.StatusBadRequest, w.Code)
+}
+
+func TestHandleCleanupE2E_RejectsMissingFields(t *testing.T) {
+	cases := []struct {
+		name string
+		body string
+	}{
+		{"missing repo", `{"run_id":12345}`},
+		{"missing run_id", `{"repo":"desktop"}`},
+		{"zero run_id", `{"repo":"desktop","run_id":0}`},
+		{"empty repo", `{"repo":"","run_id":12345}`},
+	}
+	s := newCleanupE2ETestServer(t, "the-secret")
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			w := doCleanupE2ERequest(t, s, tc.body, "the-secret")
+			assert.Equal(t, http.StatusBadRequest, w.Code,
+				"missing field %q must yield 400", tc.name)
+		})
+	}
+}
+
+func TestHandleCleanupE2E_NoMatchingInstances(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "the-secret")
+	// Map is empty. Cleanup should return 200 "no_instances" without panicking
+	// and without launching the destroy goroutine.
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "the-secret")
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Contains(t, w.Body.String(), "no_instances",
+		"empty map must yield no_instances response")
+}
+
+func TestHandleCleanupE2E_NonMatchingKeyIgnored(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "the-secret")
+	// Pre-populate the map with a key that does NOT match the cleanup
+	// request's prefix. The unrelated entry must be left untouched.
+	otherKey := "desktop-pr-9999"
+	s.e2eInstances[otherKey] = []*E2EInstance{{InstallationID: "other-inst"}}
+
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "the-secret")
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Contains(t, w.Body.String(), "no_instances")
+	assert.Contains(t, s.e2eInstances, otherKey,
+		"non-matching tracking key must not be deleted")
+}
+
+func TestHandleCleanupE2E_RemovesMatchingKey(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "the-secret")
+	// Tracking key format from handleCMTWithServerVersions:
+	//   "{repo}-cmt-{runID}-{sha}"
+	// We populate it with an EMPTY instances slice so the goroutine has no
+	// work to do (and therefore does not touch the nil CloudClient).
+	matchingKey := "desktop-cmt-26026866029-abc123def456"
+	s.e2eInstances[matchingKey] = []*E2EInstance{}
+
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "the-secret")
+
+	assert.Equal(t, http.StatusOK, w.Code,
+		"matching key must be removed and 200 returned")
+	assert.NotContains(t, s.e2eInstances, matchingKey,
+		"matching tracking key must be deleted from e2eInstances map")
+}
+
+func TestHandleCleanupE2E_RemovesMultipleMatchingKeys(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "the-secret")
+	// Two CMT entries for the same run_id (e.g. retried provisioning) -- both
+	// share the same {repo}-cmt-{run_id}- prefix. Both must be cleaned.
+	k1 := "desktop-cmt-26026866029-sha-aaa"
+	k2 := "desktop-cmt-26026866029-sha-bbb"
+	kOther := "desktop-cmt-99999999999-sha-ccc" // different run_id, must survive
+	s.e2eInstances[k1] = []*E2EInstance{}
+	s.e2eInstances[k2] = []*E2EInstance{}
+	s.e2eInstances[kOther] = []*E2EInstance{}
+
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "the-secret")
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.NotContains(t, s.e2eInstances, k1)
+	assert.NotContains(t, s.e2eInstances, k2)
+	assert.Contains(t, s.e2eInstances, kOther,
+		"unrelated run_id entry must remain after cleanup")
+}
+
+func TestHandleCleanupE2E_ConstantTimeTokenCompare(t *testing.T) {
+	s := newCleanupE2ETestServer(t, "cleanup-secret-abcdef")
+	w := doCleanupE2ERequest(t, s, validCleanupBody, "cleanup-secret-ZZZZZZ")
+	assert.Equal(t, http.StatusForbidden, w.Code)
+}

server/cmt_dispatch.go

@@ -0,0 +1,123 @@
+// Copyright (c) 2020-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package server
+
+import (
+	"crypto/subtle"
+	"encoding/json"
+	"io"
+	"net/http"
+	"strings"
+
+	"github.com/sirupsen/logrus"
+)
+
+// CMTDispatchRequest is the JSON body sent by cmt-provisioner.yml when a user
+// dispatches CMT against one or more server versions.
+//
+// GitHub's workflow_run webhook payload does not carry workflow_dispatch inputs
+// (verified against the published workflow_run schema), so cmt-provisioner.yml
+// POSTs directly to /cmt_dispatch with everything matterwick needs to provision
+// instances and dispatch compatibility-matrix-testing.yml.
+type CMTDispatchRequest struct {
+	Owner          string `json:"owner"`
+	Repo           string `json:"repo"`
+	SHA            string `json:"sha"`
+	Ref            string `json:"ref"`
+	RunID          int64  `json:"run_id"`
+	ServerVersions string `json:"server_versions"`
+}
+
+// handleCMTDispatch processes a CMT dispatch request from cmt-provisioner.yml.
+// It validates the auth token, parses the JSON body, and starts provisioning
+// asynchronously so the workflow step returns immediately.
+//
+// NOTE: this handler does not call CheckLimitRateAndAbortRequest as the
+// /github_event handler does. Rate-limit checking made sense for synchronous
+// GitHub webhook responses where matterwick might need to make API calls
+// before answering. Here, all GitHub API calls happen later in the goroutine,
+// so checking now would block the request on a network round-trip to GitHub
+// for no benefit, and (more importantly) makes this endpoint untestable in
+// the unit-test environment, which has no live token.
+func (s *Server) handleCMTDispatch(w http.ResponseWriter, r *http.Request) {
+	logger := s.Logger.WithField("endpoint", "/cmt_dispatch")
+
+	// Reject the request if no token is configured rather than running unauthenticated.
+	if s.Config.CMTTriggerSecret == "" {
+		logger.Error("CMTTriggerSecret not configured; rejecting /cmt_dispatch request")
+		w.WriteHeader(http.StatusServiceUnavailable)
+		return
+	}
+
+	provided := r.Header.Get("X-Trigger-Token")
+	if subtle.ConstantTimeCompare([]byte(provided), []byte(s.Config.CMTTriggerSecret)) != 1 {
+		logger.Warn("Invalid or missing X-Trigger-Token on /cmt_dispatch")
+		w.WriteHeader(http.StatusForbidden)
+		return
+	}
+
+	body, err := io.ReadAll(r.Body)
+	if err != nil {
+		logger.WithError(err).Error("Failed to read /cmt_dispatch request body")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	var req CMTDispatchRequest
+	if err := json.Unmarshal(body, &req); err != nil {
+		logger.WithError(err).Error("Failed to parse /cmt_dispatch JSON body")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	if req.Owner == "" || req.Repo == "" || req.SHA == "" || req.Ref == "" || req.RunID == 0 || req.ServerVersions == "" {
+		logger.WithFields(logrus.Fields{
+			"owner":           req.Owner,
+			"repo":            req.Repo,
+			"sha_empty":       req.SHA == "",
+			"ref_empty":       req.Ref == "",
+			"run_id":          req.RunID,
+			"versions_empty":  req.ServerVersions == "",
+		}).Error("Missing required fields in /cmt_dispatch body")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	versions := parseServerVersionsFromString(req.ServerVersions)
+	if len(versions) == 0 {
+		logger.WithField("input", req.ServerVersions).Error("Failed to parse server_versions")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	var instanceType string
+	switch {
+	case strings.Contains(req.Repo, "desktop"):
+		instanceType = "desktop"
+	case strings.Contains(req.Repo, "mobile"):
+		instanceType = "mobile"
+	default:
+		logger.WithField("repo", req.Repo).Warn("Repository is neither desktop nor mobile, refusing /cmt_dispatch")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	dispatchLogger := logger.WithFields(logrus.Fields{
+		"repo":         req.Repo,
+		"owner":        req.Owner,
+		"sha":          req.SHA,
+		"ref":          req.Ref,
+		"run_id":       req.RunID,
+		"versions":     versions,
+		"instanceType": instanceType,
+	})
+	dispatchLogger.Info("Accepted CMT dispatch request, provisioning asynchronously")
+
+	// Provisioning takes ~30 min; respond 202 now and do the work in a goroutine.
+	go s.handleCMTWithServerVersions(req.Owner, req.Repo, instanceType, req.Ref, req.SHA, versions, req.RunID, dispatchLogger)
+
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusAccepted)
+	_, _ = w.Write([]byte(`{"status":"accepted"}`))
+}