[CLD-8628 #2] Add custom status code for installation name/dns confli…

…cts + bonus (#1091) * Add support for contexts requiring confirmation * Add back * Add custom status code for DNS conflict on creation * Fix tests, add support for passing actor ID through installation deletion webhook events
mattermost · Dec 18, 2024 · 6a79c68 · 6a79c68
1 parent e17c733
commit 6a79c68
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 2 deletions.
diff --git a/Makefile b/Makefile
@@ -294,6 +294,7 @@ endif
 
 .PHONY: unittest
 unittest:
+	@echo $(CLOUD_DATABASE)
 	CLOUD_DATABASE=$(CLOUD_DATABASE) $(GO) test -failfast ./... ${TEST_FLAGS} -covermode=count -coverprofile=coverage.out
 
 .PHONY: verify-mocks

diff --git a/internal/api/installation.go b/internal/api/installation.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/mattermost/mattermost-cloud/internal/common"
+	"github.com/mattermost/mattermost-cloud/internal/events"
 
 	"github.com/pkg/errors"
 
@@ -254,6 +255,13 @@ func handleCreateInstallation(c *Context, w http.ResponseWriter, r *http.Request
 
 	err = c.Store.CreateInstallation(&installation, annotations, dnsRecords)
 	if err != nil {
+		var uniqueErr *store.UniqueConstraintError
+		if errors.As(err, &uniqueErr) {
+			c.Logger.WithError(err).Error("domain name already in use")
+			w.WriteHeader(http.StatusConflict)
+			return
+		}
+
 		c.Logger.WithError(err).Error("failed to create installation")
 		w.WriteHeader(http.StatusInternalServerError)
 		return
@@ -661,7 +669,15 @@ func handleDeleteInstallation(c *Context, w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	err = c.EventProducer.ProduceInstallationStateChangeEvent(installationDTO.Installation, oldState)
+	// Try to parse the user ID from the request context, so it can be passed along with the webhook event
+	actorID := ""
+	if value := r.Context().Value(ContextKeyUserID{}); value != nil {
+		if str, ok := value.(string); ok && str != "" {
+			actorID = str
+		}
+	}
+
+	err = c.EventProducer.ProduceInstallationStateChangeEvent(installationDTO.Installation, oldState, events.DataField{Key: "actor_id", Value: actorID})
 	if err != nil {
 		c.Logger.WithError(err).Error("Failed to create installation state change event")
 	}

diff --git a/internal/api/installation_test.go b/internal/api/installation_test.go
@@ -473,6 +473,46 @@ func TestCreateInstallation(t *testing.T) {
 		require.EqualError(t, err, "failed with status code 400")
 	})
 
+	t.Run("custom status code for conflict in DNS name", func(t *testing.T) {
+		envs := model.EnvVarMap{
+			"MM_TEST2": model.EnvVar{Value: "test2"},
+		}
+		installation, err := client.CreateInstallation(&model.CreateInstallationRequest{
+			OwnerID:     "owner",
+			Version:     "version",
+			DNS:         "useddns.example.com",
+			Affinity:    model.InstallationAffinityIsolated,
+			Annotations: []string{"my-annotation"},
+			PriorityEnv: envs,
+		})
+		require.NoError(t, err)
+		require.Equal(t, "owner", installation.OwnerID)
+		require.Equal(t, "version", installation.Version)
+		require.Equal(t, "mattermost/mattermost-enterprise-edition", installation.Image)
+		require.Equal(t, "useddns.example.com", installation.DNS) //nolint
+		require.Equal(t, "useddns.example.com", installation.DNSRecords[0].DomainName)
+		require.Equal(t, "useddns", installation.Name)
+		require.Equal(t, model.InstallationAffinityIsolated, installation.Affinity)
+		require.Equal(t, model.InstallationStateCreationRequested, installation.State)
+		require.Equal(t, model.DefaultCRVersion, installation.CRVersion)
+		require.Empty(t, installation.LockAcquiredBy)
+		require.EqualValues(t, 0, installation.LockAcquiredAt)
+		require.NotEqual(t, 0, installation.CreateAt)
+		require.EqualValues(t, 0, installation.DeleteAt)
+
+		_, err = client.CreateInstallation(&model.CreateInstallationRequest{
+			OwnerID:     "owner",
+			Version:     "version",
+			DNS:         "useddns.example.com",
+			Affinity:    model.InstallationAffinityIsolated,
+			Annotations: []string{"my-annotation"},
+			PriorityEnv: envs,
+		})
+
+		require.Error(t, err)
+		require.EqualError(t, err, "failed with status code 409")
+	})
+
 	t.Run("valid", func(t *testing.T) {
 		envs := model.EnvVarMap{
 			"MM_TEST2": model.EnvVar{Value: "test2"},

diff --git a/internal/store/installation.go b/internal/store/installation.go
@@ -383,7 +383,7 @@ func (sqlStore *SQLStore) CreateInstallation(installation *model.Installation, a
 
 	err = sqlStore.createInstallation(tx, installation)
 	if err != nil {
-		return errors.Wrap(err, "failed to create installation")
+		return err
 	}
 
 	// We can do bulk insert for better performance, but currently we do not expect more than 1 record.
@@ -480,6 +480,10 @@ func (sqlStore *SQLStore) createInstallation(db execer, installation *model.Inst
 		SetMap(insertsMap),
 	)
 	if err != nil {
+		if isUniqueConstraintViolation(err) {
+			return &UniqueConstraintError{}
+		}
+
 		return errors.Wrap(err, "failed to create installation")
 	}
 

diff --git a/internal/store/installation_dns.go b/internal/store/installation_dns.go
@@ -58,6 +58,10 @@ func (sqlStore *SQLStore) createInstallationDNS(db execer, installationID string
 
 	_, err := sqlStore.execBuilder(db, query)
 	if err != nil {
+		if isUniqueConstraintViolation(err) {
+			return &UniqueConstraintError{}
+		}
+
 		return errors.Wrap(err, "failed to create installation DNS record")
 	}
 

diff --git a/internal/store/util.go b/internal/store/util.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 
 	sq "github.com/Masterminds/squirrel"
+	"github.com/lib/pq"
 	"github.com/mattermost/mattermost-cloud/model"
 )
 
@@ -35,3 +36,18 @@ func applyPagingFilter(builder sq.SelectBuilder, paging model.Paging, deleteAtTa
 
 	return builder
 }
+
+type UniqueConstraintError struct {
+}
+
+func (e *UniqueConstraintError) Error() string {
+	return "unique constraint violation"
+}
+
+// isUniqueConstraintViolation checks if the error is a unique constraint violation.
+func isUniqueConstraintViolation(err error) bool {
+	if pgErr, ok := err.(*pq.Error); ok && pgErr.Code == "23505" {
+		return true
+	}
+	return false
+}