Use SQL for Destination{Create,Update,Delete}

internal/access/destination.go

@@ -24,7 +24,7 @@ func SaveDestination(rCtx RequestContext, destination *models.Destination) error
 		return HandleAuthErr(err, "destination", "update", roles...)
 	}
 
-	return data.SaveDestination(rCtx.DBTxn, destination)
+	return data.UpdateDestination(rCtx.DBTxn, destination)
 }
 
 func GetDestination(c *gin.Context, id uid.ID) (*models.Destination, error) {
@@ -44,5 +44,5 @@ func DeleteDestination(c *gin.Context, id uid.ID) error {
 		return HandleAuthErr(err, "destination", "delete", models.InfraAdminRole)
 	}
 
-	return data.DeleteDestinations(db, data.ByID(id))
+	return data.DeleteDestination(db, id)
 }

internal/server/data/data.go

@@ -439,9 +439,9 @@ func handleError(err error) error {
 				"idx_groups_name":             "name",
 				"idx_providers_name":          "name",
 				"idx_access_keys_name":        "name",
-				"idx_destinations_unique_id":  "uniqueId",
+				"idx_destinations_unique_id":  "uniqueID",
 				"idx_access_keys_key_id":      "keyId",
-				"idx_credentials_identity_id": "identityId",
+				"idx_credentials_identity_id": "identityID",
 				"idx_organizations_domain":    "domain",
 			}
 

internal/server/data/destination.go

@@ -4,30 +4,67 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/infrahq/infra/internal"
 	"github.com/infrahq/infra/internal/server/models"
 	"github.com/infrahq/infra/uid"
 )
 
+type destinationsTable models.Destination
+
+func (d destinationsTable) Table() string {
+	return "destinations"
+}
+
+func (d destinationsTable) Columns() []string {
+	return []string{"connection_ca", "connection_url", "created_at", "deleted_at", "id", "last_seen_at", "name", "organization_id", "resources", "roles", "unique_id", "updated_at", "version"}
+}
+
+func (d destinationsTable) Values() []any {
+	return []any{d.ConnectionCA, d.ConnectionURL, d.CreatedAt, d.DeletedAt, d.ID, d.LastSeenAt, d.Name, d.OrganizationID, d.Resources, d.Roles, d.UniqueID, d.UpdatedAt, d.Version}
+}
+
+func (d *destinationsTable) ScanFields() []any {
+	return []any{&d.ConnectionCA, &d.ConnectionURL, &d.CreatedAt, &d.DeletedAt, &d.ID, &d.LastSeenAt, &d.Name, &d.OrganizationID, &d.Resources, &d.Roles, &d.UniqueID, &d.UpdatedAt, &d.Version}
+}
+
+// destinationsUpdateTable is used to update the destination. It excludes
+// the CreatedAt field, because that field is not part of the input to
+// UpdateDestination.
+type destinationsUpdateTable models.Destination
+
+func (d destinationsUpdateTable) Table() string {
+	return "destinations"
+}
+
+func (d destinationsUpdateTable) Columns() []string {
+	return []string{"connection_ca", "connection_url", "deleted_at", "id", "last_seen_at", "name", "organization_id", "resources", "roles", "unique_id", "updated_at", "version"}
+}
+
+func (d destinationsUpdateTable) Values() []any {
+	return []any{d.ConnectionCA, d.ConnectionURL, d.DeletedAt, d.ID, d.LastSeenAt, d.Name, d.OrganizationID, d.Resources, d.Roles, d.UniqueID, d.UpdatedAt, d.Version}
+}
+
 func validateDestination(dest *models.Destination) error {
 	if dest.Name == "" {
-		return fmt.Errorf("name is required")
+		return fmt.Errorf("Destination.Name is required")
+	}
+	if dest.UniqueID == "" {
+		return fmt.Errorf("Destination.UniqueID is required")
 	}
 	return nil
 }
 
-func CreateDestination(db GormTxn, destination *models.Destination) error {
+func CreateDestination(tx WriteTxn, destination *models.Destination) error {
 	if err := validateDestination(destination); err != nil {
 		return err
 	}
-	return add(db, destination)
+	return insert(tx, (*destinationsTable)(destination))
 }
 
-func SaveDestination(db GormTxn, destination *models.Destination) error {
+func UpdateDestination(tx WriteTxn, destination *models.Destination) error {
 	if err := validateDestination(destination); err != nil {
 		return err
 	}
-	return save(db, destination)
+	return update(tx, (*destinationsUpdateTable)(destination))
 }
 
 func GetDestination(db GormTxn, selectors ...SelectorFunc) (*models.Destination, error) {
@@ -38,51 +75,42 @@ func ListDestinations(db GormTxn, p *Pagination, selectors ...SelectorFunc) ([]m
 	return list[models.Destination](db, p, selectors...)
 }
 
-func DeleteDestinations(db GormTxn, selector SelectorFunc) error {
-	toDelete, err := ListDestinations(db, nil, selector)
-	if err != nil {
-		return err
-	}
-
-	if len(toDelete) > 0 {
-		ids := make([]uid.ID, 0)
-		for _, g := range toDelete {
-			ids = append(ids, g.ID)
-		}
-
-		return deleteAll[models.Destination](db, ByIDs(ids))
-	}
-
-	return internal.ErrNotFound
+func DeleteDestination(tx WriteTxn, id uid.ID) error {
+	stmt := `
+		UPDATE destinations SET deleted_at = ?
+		WHERE id = ? AND organization_id = ? AND deleted_at is null
+	`
+	_, err := tx.Exec(stmt, time.Now(), id, tx.OrganizationID())
+	return handleError(err)
 }
 
-type destinationsCount struct {
+type DestinationsCount struct {
 	Connected bool
 	Version   string
 	Count     float64
 }
 
-func CountDestinationsByConnectedVersion(tx ReadTxn) ([]destinationsCount, error) {
+func CountDestinationsByConnectedVersion(tx ReadTxn) ([]DestinationsCount, error) {
 	timeout := time.Now().Add(-5 * time.Minute)
 
 	stmt := `
-		SELECT *, COUNT(*) AS count
-		FROM (
-			SELECT COALESCE(version, '') AS version, last_seen_at >= ? AS connected
-			FROM destinations
-			WHERE deleted_at IS NULL
-		) AS d
-		GROUP BY version, connected`
+		SELECT COALESCE(version, '') as version,
+			   last_seen_at >= ? as connected,
+			   count(*)
+		FROM destinations
+		WHERE deleted_at IS NULL
+		GROUP BY connected, version
+	`
 	rows, err := tx.Query(stmt, timeout)
 	if err != nil {
 		return nil, err
 
 	}
 	defer rows.Close()
 
-	var result []destinationsCount
+	var result []DestinationsCount
 	for rows.Next() {
-		var item destinationsCount
+		var item DestinationsCount
 		if err := rows.Scan(&item.Version, &item.Connected, &item.Count); err != nil {
 			return nil, err
 		}

internal/server/data/destination_test.go

@@ -1,54 +1,171 @@
 package data
 
 import (
+	"errors"
 	"testing"
 	"time"
 
 	"gotest.tools/v3/assert"
 
+	"github.com/infrahq/infra/internal"
 	"github.com/infrahq/infra/internal/server/models"
 )
 
-func TestDestinationSaveCreatedPersists(t *testing.T) {
+func TestCreateDestination(t *testing.T) {
 	runDBTests(t, func(t *testing.T, db *DB) {
-		destination := &models.Destination{
-			Name: "example-cluster-1",
-		}
+		t.Run("success", func(t *testing.T) {
+			tx := txnForTestCase(t, db, db.DefaultOrg.ID)
 
-		err := CreateDestination(db, destination)
-		assert.NilError(t, err)
-		assert.Assert(t, !destination.CreatedAt.IsZero())
+			destination := &models.Destination{
+				Name:          "kubernetes",
+				UniqueID:      "unique-id",
+				ConnectionURL: "10.0.0.1:1001",
+				ConnectionCA:  "the-pem-encoded-cert",
+				LastSeenAt:    time.Date(2022, 1, 2, 3, 4, 5, 600, time.UTC),
+				Version:       "0.100.1",
+				Resources:     []string{"res1", "res2"},
+				Roles:         []string{"role1", "role2"},
+			}
 
-		destination.Name = "example-cluster-2"
-		destination.CreatedAt = time.Time{}
+			err := CreateDestination(tx, destination)
+			assert.NilError(t, err)
+			assert.Assert(t, destination.ID != 0)
 
-		err = SaveDestination(db, destination)
-		assert.NilError(t, err)
+			expected := &models.Destination{
+				Model: models.Model{
+					ID:        destination.ID,
+					CreatedAt: time.Now(),
+					UpdatedAt: time.Now(),
+				},
+				OrganizationMember: models.OrganizationMember{OrganizationID: defaultOrganizationID},
+				Name:               "kubernetes",
+				UniqueID:           "unique-id",
+				ConnectionURL:      "10.0.0.1:1001",
+				ConnectionCA:       "the-pem-encoded-cert",
+				LastSeenAt:         time.Date(2022, 1, 2, 3, 4, 5, 600, time.UTC),
+				Version:            "0.100.1",
+				Resources:          []string{"res1", "res2"},
+				Roles:              []string{"role1", "role2"},
+			}
+			assert.DeepEqual(t, destination, expected, cmpModel)
+		})
+		t.Run("conflict on uniqueID", func(t *testing.T) {
+			tx := txnForTestCase(t, db, db.DefaultOrg.ID)
+
+			destination := &models.Destination{
+				Name:     "kubernetes",
+				UniqueID: "unique-id",
+			}
+			err := CreateDestination(tx, destination)
+			assert.NilError(t, err)
+			assert.Assert(t, destination.ID != 0)
+
+			next := &models.Destination{
+				Name:     "other",
+				UniqueID: "unique-id",
+			}
+			err = CreateDestination(tx, next)
+			var ucErr UniqueConstraintError
+			assert.Assert(t, errors.As(err, &ucErr))
+			expected := UniqueConstraintError{Table: "destinations", Column: "uniqueID"}
+			assert.DeepEqual(t, ucErr, expected)
+		})
+	})
+}
 
-		destination, err = GetDestination(db, ByID(destination.ID))
+func TestUpdateDestination(t *testing.T) {
+	runDBTests(t, func(t *testing.T, db *DB) {
+		t.Run("success", func(t *testing.T) {
+			tx := txnForTestCase(t, db, db.DefaultOrg.ID)
+
+			created := time.Date(2022, 1, 2, 3, 4, 5, 600, time.UTC)
+			orig := &models.Destination{
+				Model:    models.Model{CreatedAt: created, UpdatedAt: created},
+				Name:     "example-cluster-1",
+				UniqueID: "11111",
+			}
+			createDestinations(t, tx, orig)
+
+			// Unlike other update operations, the passed in destination
+			// may be constructed entirely by the caller and may not have the
+			// created, or updated time set.
+			destination := &models.Destination{
+				Model:         models.Model{ID: orig.ID},
+				Name:          "example-cluster-2",
+				UniqueID:      "22222",
+				ConnectionURL: "dest.internal:10001",
+				ConnectionCA:  "the-pem-encoded-cert",
+				Resources:     []string{"res1", "res3"},
+				Roles:         []string{"role1"},
+				Version:       "0.100.2",
+			}
+			err := UpdateDestination(tx, destination)
+			assert.NilError(t, err)
+
+			actual, err := GetDestination(tx, ByID(destination.ID))
+			assert.NilError(t, err)
+
+			expected := &models.Destination{
+				Model: models.Model{
+					ID:        destination.ID,
+					CreatedAt: created,
+					UpdatedAt: time.Now(),
+				},
+				OrganizationMember: models.OrganizationMember{OrganizationID: defaultOrganizationID},
+				Name:               "example-cluster-2",
+				UniqueID:           "22222",
+				ConnectionURL:      "dest.internal:10001",
+				ConnectionCA:       "the-pem-encoded-cert",
+				Resources:          []string{"res1", "res3"},
+				Roles:              []string{"role1"},
+				Version:            "0.100.2",
+			}
+			assert.DeepEqual(t, actual, expected, cmpModel)
+		})
+	})
+}
+
+func TestDeleteDestination(t *testing.T) {
+	runDBTests(t, func(t *testing.T, db *DB) {
+		tx := txnForTestCase(t, db, db.DefaultOrg.ID)
+
+		dest := &models.Destination{Name: "kube", UniqueID: "1111"}
+		createDestinations(t, tx, dest)
+
+		err := DeleteDestination(tx, dest.ID)
 		assert.NilError(t, err)
-		assert.Assert(t, !destination.CreatedAt.IsZero())
+
+		_, err = GetDestination(tx, ByID(dest.ID))
+		assert.ErrorIs(t, err, internal.ErrNotFound)
 	})
 }
 
 func TestCountDestinationsByConnectedVersion(t *testing.T) {
 	runDBTests(t, func(t *testing.T, db *DB) {
-		assert.NilError(t, CreateDestination(db, &models.Destination{Name: "1", UniqueID: "1", LastSeenAt: time.Now()}))
-		assert.NilError(t, CreateDestination(db, &models.Destination{Name: "2", UniqueID: "2", Version: "", LastSeenAt: time.Now().Add(-10 * time.Minute)}))
-		assert.NilError(t, CreateDestination(db, &models.Destination{Name: "3", UniqueID: "3", Version: "0.1.0", LastSeenAt: time.Now()}))
-		assert.NilError(t, CreateDestination(db, &models.Destination{Name: "4", UniqueID: "4", Version: "0.1.0"}))
-		assert.NilError(t, CreateDestination(db, &models.Destination{Name: "5", UniqueID: "5", Version: "0.1.0"}))
-
+		createDestinations(t, db,
+			&models.Destination{Name: "1", UniqueID: "1", LastSeenAt: time.Now()},
+			&models.Destination{Name: "2", UniqueID: "2", Version: "", LastSeenAt: time.Now().Add(-10 * time.Minute)},
+			&models.Destination{Name: "3", UniqueID: "3", Version: "0.1.0", LastSeenAt: time.Now()},
+			&models.Destination{Name: "4", UniqueID: "4", Version: "0.1.0"},
+			&models.Destination{Name: "5", UniqueID: "5", Version: "0.1.0"},
+		)
 		actual, err := CountDestinationsByConnectedVersion(db)
 		assert.NilError(t, err)
 
-		expected := []destinationsCount{
+		expected := []DestinationsCount{
 			{Connected: false, Version: "", Count: 1},
-			{Connected: true, Version: "", Count: 1},
 			{Connected: false, Version: "0.1.0", Count: 2},
+			{Connected: true, Version: "", Count: 1},
 			{Connected: true, Version: "0.1.0", Count: 1},
 		}
-
 		assert.DeepEqual(t, actual, expected)
 	})
 }
+
+func createDestinations(t *testing.T, tx GormTxn, destinations ...*models.Destination) {
+	t.Helper()
+	for i := range destinations {
+		err := CreateDestination(tx, destinations[i])
+		assert.NilError(t, err, destinations[i].Name)
+	}
+}