Cluster: Ensure credentials are deleted after tests

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/api/cluster_nodes_register.go

@@ -198,7 +198,7 @@ func ClusterNodesRegister(router *gin.RouterGroup) {
 			}
 
 			// Ensure that a database for this node exists (rotation optional).
-			creds, _, credsErr := provisioner.GetCredentials(c, conf, n.UUID, name, req.RotateDatabase)
+			creds, _, credsErr := provisioner.EnsureCredentials(c, conf, n.UUID, name, req.RotateDatabase)
 
 			if credsErr != nil {
 				event.AuditWarn([]string{clientIp, string(acl.ResourceCluster), "nodes", "register", "ensure database", event.Failed, "%s"}, clean.Error(credsErr))
@@ -279,7 +279,7 @@ func ClusterNodesRegister(router *gin.RouterGroup) {
 		n.RotatedAt = nowRFC3339()
 
 		// Ensure DB (force rotation at create path to return password).
-		creds, _, err := provisioner.GetCredentials(c, conf, n.UUID, name, true)
+		creds, _, err := provisioner.EnsureCredentials(c, conf, n.UUID, name, true)
 		if err != nil {
 			event.AuditWarn([]string{clientIp, string(acl.ResourceCluster), "nodes", "register", "ensure database", event.Failed, "%s"}, clean.Error(err))
 			c.JSON(http.StatusConflict, gin.H{"error": err.Error()})

internal/commands/cluster_nodes_remove.go

@@ -1,12 +1,15 @@
 package commands
 
 import (
+	"context"
 	"fmt"
+	"time"
 
 	"github.com/manifoldco/promptui"
 	"github.com/urfave/cli/v2"
 
 	"github.com/photoprism/photoprism/internal/config"
+	"github.com/photoprism/photoprism/internal/service/cluster/provisioner"
 	reg "github.com/photoprism/photoprism/internal/service/cluster/registry"
 	"github.com/photoprism/photoprism/pkg/clean"
 )
@@ -19,6 +22,7 @@ var ClusterNodesRemoveCommand = &cli.Command{
 	Flags: []cli.Flag{
 		&cli.BoolFlag{Name: "yes", Aliases: []string{"y"}, Usage: "runs the command non-interactively"},
 		&cli.BoolFlag{Name: "all-ids", Usage: "delete all records that share the same UUID (admin cleanup)"},
+		&cli.BoolFlag{Name: "drop-db", Aliases: []string{"d"}, Usage: "drop the node’s provisioned database and user after registry deletion"},
 	},
 	Hidden: true, // Required for cluster-management only.
 	Action: clusterNodesRemoveAction,
@@ -31,28 +35,37 @@ func clusterNodesRemoveAction(ctx *cli.Context) error {
 		}
 
 		key := ctx.Args().First()
+
 		if key == "" {
 			return cli.Exit(fmt.Errorf("node id or name is required"), 2)
 		}
 
 		r, err := reg.NewClientRegistryWithConfig(conf)
+
 		if err != nil {
 			return cli.Exit(err, 1)
 		}
 
 		// Resolve to id for deletion, but also support name.
 		// Resolve UUID to delete: accept uuid → clientId → name.
-		uuid := key
-		if n, err2 := r.FindByNodeUUID(uuid); err2 == nil && n != nil {
-			uuid = n.UUID
-		} else if n, err2 := r.FindByClientID(uuid); err2 == nil && n != nil {
-			uuid = n.UUID
-		} else if n, err2 := r.FindByName(clean.DNSLabel(key)); err2 == nil && n != nil {
-			uuid = n.UUID
-		} else {
+		var node *reg.Node
+
+		if n, findErr := r.FindByNodeUUID(key); findErr == nil && n != nil {
+			node = n
+		} else if n, findErr = r.FindByClientID(key); findErr == nil && n != nil {
+			node = n
+		} else if name := clean.DNSLabel(key); name != "" {
+			if n, findErr = r.FindByName(name); findErr == nil && n != nil {
+				node = n
+			}
+		}
+
+		if node == nil {
 			return cli.Exit(fmt.Errorf("node not found"), 3)
 		}
 
+		uuid := node.UUID
+
 		confirmed := RunNonInteractively(ctx.Bool("yes"))
 		if !confirmed {
 			prompt := promptui.Prompt{Label: fmt.Sprintf("Delete node %s?", clean.Log(uuid)), IsConfirm: true}
@@ -62,6 +75,13 @@ func clusterNodesRemoveAction(ctx *cli.Context) error {
 			}
 		}
 
+		dropDB := ctx.Bool("drop-db")
+		dbName, dbUser := "", ""
+		if dropDB && node.Database != nil {
+			dbName = node.Database.Name
+			dbUser = node.Database.User
+		}
+
 		if ctx.Bool("all-ids") {
 			if err := r.DeleteAllByUUID(uuid); err != nil {
 				return cli.Exit(err, 1)
@@ -70,7 +90,21 @@ func clusterNodesRemoveAction(ctx *cli.Context) error {
 			return cli.Exit(err, 1)
 		}
 
+		if dropDB {
+			if dbName == "" && dbUser == "" {
+				log.Infof("node %s has been deleted (no database credentials recorded)", clean.Log(uuid))
+			} else {
+				dropCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+				defer cancel()
+				if err := provisioner.DropCredentials(dropCtx, dbName, dbUser); err != nil {
+					return cli.Exit(fmt.Errorf("failed to drop database credentials for node %s: %w", clean.Log(uuid), err), 1)
+				}
+				log.Infof("node %s database %s and user %s have been dropped", clean.Log(uuid), clean.Log(dbName), clean.Log(dbUser))
+			}
+		}
+
 		log.Infof("node %s has been deleted", clean.Log(uuid))
+
 		return nil
 	})
 }

internal/service/cluster/provisioner/credentials.go

@@ -22,11 +22,11 @@ type Credentials struct {
 	RotatedAt string
 }
 
-// GetCredentials ensures a per-node database and user exist with minimal grants.
+// EnsureCredentials ensures a per-node database and user exist with minimal grants.
 // - Requires a MySQL/MariaDB admin connection (this package maintains it).
 // - Returns created=true if the database schema did not exist before.
 // - If rotate is true or created, rotates the user password and includes it (and DSN) in the result.
-func GetCredentials(ctx context.Context, conf *config.Config, nodeUUID, nodeName string, rotate bool) (Credentials, bool, error) {
+func EnsureCredentials(ctx context.Context, conf *config.Config, nodeUUID, nodeName string, rotate bool) (Credentials, bool, error) {
 	out := Credentials{}
 
 	// Normalize the configured admin driver locally so we accept variants like "MySQL"/"MariaDB"
@@ -131,3 +131,45 @@ func GetCredentials(ctx context.Context, conf *config.Config, nodeUUID, nodeName
 
 	return out, created, nil
 }
+
+// DropCredentials removes the database and user created for a node. It is safe to call
+// even if the user or schema no longer exist; errors are aggregated and returned.
+func DropCredentials(ctx context.Context, dbName, user string) error {
+	db, err := GetDB(ctx)
+	if err != nil {
+		return err
+	}
+
+	var errs []string
+
+	if user != "" {
+		acc, accErr := quoteAccount("%", user)
+		if accErr != nil {
+			errs = append(errs, fmt.Sprintf("quote account: %v", accErr))
+		} else {
+			if err := execTimeout(ctx, db, 10*time.Second, "REVOKE ALL PRIVILEGES, GRANT OPTION FROM "+acc); err != nil {
+				errs = append(errs, fmt.Sprintf("revoke privileges: %v", err))
+			}
+			if err := execTimeout(ctx, db, 10*time.Second, "DROP USER IF EXISTS "+acc); err != nil {
+				errs = append(errs, fmt.Sprintf("drop user: %v", err))
+			}
+		}
+	}
+
+	if dbName != "" {
+		qdb, qErr := quoteIdent(dbName)
+		if qErr != nil {
+			errs = append(errs, fmt.Sprintf("quote database: %v", qErr))
+		} else {
+			if err := execTimeout(ctx, db, 15*time.Second, "DROP DATABASE IF EXISTS "+qdb); err != nil {
+				errs = append(errs, fmt.Sprintf("drop database: %v", err))
+			}
+		}
+	}
+
+	if len(errs) > 0 {
+		return fmt.Errorf("drop credentials: %s", strings.Join(errs, "; "))
+	}
+
+	return nil
+}

internal/service/cluster/provisioner/credentials_test.go

@@ -11,10 +11,10 @@ import (
 	"github.com/photoprism/photoprism/internal/config"
 )
 
-// TestGetCredentials_MariaDB exercises the direct mysql driver path using the
+// TestEnsureCredentials_MariaDB exercises the direct mysql driver path using the
 // ProvisionDSN. It skips if MariaDB is not reachable or when not explicitly enabled
 // via environment (PHOTOPRISM_TEST_DRIVER=mysql).
-func TestGetCredentials_MariaDB(t *testing.T) {
+func TestEnsureCredentials_MariaDB(t *testing.T) {
 	ctx := context.Background()
 
 	// Quick liveness probe for AdminDsn; skip fast if not reachable.
@@ -37,10 +37,21 @@ func TestGetCredentials_MariaDB(t *testing.T) {
 	nodeName := "pp-itest-node"
 
 	// 1st call: rotate=true so we receive a password + DSN.
-	creds, created, err := GetCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", nodeName, true)
+	creds, created, err := EnsureCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", nodeName, true)
 	if err != nil {
-		t.Fatalf("GetCredentials(rotate=true) error: %v", err)
+		t.Fatalf("EnsureCredentials(rotate=true) error: %v", err)
 	}
+
+	// Ensure we always drop the temporary DB/user created during this test.
+	t.Cleanup(func() {
+		if creds.Name == "" || creds.User == "" {
+			return
+		}
+		if dropErr := DropCredentials(ctx, creds.Name, creds.User); dropErr != nil {
+			t.Logf("cleanup: %v", dropErr)
+		}
+	})
+
 	if creds.Name == "" || creds.User == "" {
 		t.Fatalf("missing db name/user in creds: %+v", creds)
 	}
@@ -63,36 +74,17 @@ func TestGetCredentials_MariaDB(t *testing.T) {
 	_ = udb.Close()
 
 	// 2nd call: rotate=false should not return a password (idempotent ensure).
-	creds2, _, err := GetCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", nodeName, false)
+	creds2, _, err := EnsureCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", nodeName, false)
 	if err != nil {
-		t.Fatalf("GetCredentials(rotate=false) error: %v", err)
+		t.Fatalf("EnsureCredentials(rotate=false) error: %v", err)
 	}
 	if creds2.Password != "" || creds2.DSN != "" {
 		t.Fatalf("expected no password/DSN without rotation; got: %+v", creds2)
 	}
-
-	// Cleanup: drop user and database to keep the dev DB tidy.
-	adb, err := GetDB(ctx)
-	if err != nil {
-		t.Fatalf("GetDB: %v", err)
-	}
-	qdb, err := quoteIdent(creds.Name)
-	if err != nil {
-		t.Fatalf("quoteIdent: %v", err)
-	}
-	acc, err := quoteAccount("%", creds.User)
-	if err != nil {
-		t.Fatalf("quoteAccount: %v", err)
-	}
-	// Best-effort cleanup; ignore individual errors to avoid masking earlier failures.
-	_ = execTimeout(ctx, adb, 10*time.Second, "REVOKE ALL PRIVILEGES, GRANT OPTION FROM "+acc)
-	_ = execTimeout(ctx, adb, 10*time.Second, "DROP USER IF EXISTS "+acc)
-	_ = execTimeout(ctx, adb, 15*time.Second, "DROP DATABASE IF EXISTS "+qdb)
 }
 
-// Verifies that GetCredentials normalizes DatabaseDriver case and rejects
-// non-MySQL/MariaDB drivers early without attempting a DB connection.
-func TestGetCredentials_DriverNormalization(t *testing.T) {
+// TestEnsureCredentials_DriverNormalization verifies driver normalization and rejections.
+func TestEnsureCredentials_DriverNormalization(t *testing.T) {
 	orig := DatabaseDriver
 	t.Cleanup(func() { DatabaseDriver = orig })
 
@@ -101,13 +93,13 @@ func TestGetCredentials_DriverNormalization(t *testing.T) {
 
 	// Postgres in weird case should hit the explicit rejection path.
 	DatabaseDriver = "PostGreS"
-	_, _, err := GetCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", "pp-node", false)
+	_, _, err := EnsureCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", "pp-node", false)
 	assert.Error(t, err)
 	assert.Equal(t, "PostGreS", DatabaseDriver)
 
 	// Unknown driver should return the unsupported error including normalized name.
 	DatabaseDriver = "TiDB"
-	_, _, err = GetCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", "pp-node", false)
+	_, _, err = EnsureCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", "pp-node", false)
 	if assert.Error(t, err) {
 		assert.Contains(t, err.Error(), "unsupported auto-provisioning database driver: tidb")
 	}

internal/service/cluster/provisioner/naming_test.go

@@ -74,14 +74,14 @@ func TestHmacBase32_LowercaseDeterministic(t *testing.T) {
 	}
 }
 
-func TestGetCredentials_SqliteRejected(t *testing.T) {
+func TestEnsureCredentials_SqliteRejected(t *testing.T) {
 	ctx := context.Background()
 	c := config.NewConfig(config.CliTestContext())
 	origDriver := DatabaseDriver
 	DatabaseDriver = config.SQLite3
 	t.Cleanup(func() { DatabaseDriver = origDriver })
 
-	_, _, err := GetCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", "pp-node-01", false)
+	_, _, err := EnsureCredentials(ctx, c, "11111111-1111-4111-8111-111111111111", "pp-node-01", false)
 	if assert.Error(t, err) {
 		assert.Contains(t, err.Error(), "database must be MySQL/MariaDB")
 	}