warehouse: make duckling_name mandatory (NOT NULL) + editable in admin UI (#858)

benben · web-flow · commit 63aec482f1f5 · 2026-07-01T16:17:47.000+02:00
* warehouse: make duckling_name mandatory (NOT NULL) and editable in admin UI

The duckling_name column (warehouse → Duckling CR link) was nullable and only
populated by best-effort backfill/set-on-create. Make it authoritative:

- Migration 000012: re-backfill any NULL/empty to lower(org_id), then
  ALTER COLUMN duckling_name SET NOT NULL. GORM tag gains not null; migration
  parity/version test bumped 11→12.
- App-level non-empty guard (NOT NULL still allows ''): provisionWarehouse
  rejects an empty duckling_name on create; putManagedWarehouse rejects
  blanking it on edit.
- Editable via the admin UI: duckling_name added to the warehouse PUT
  allow-list and surfaced as a required field in the org warehouse editor, so
  corrections flow through the audited PUT path.

* tests/seeds: set duckling_name for NOT NULL; fix v8-upgrade baseline

configstore-integration-tests was red on the new NOT NULL constraint:
- The dev/kind/tenant-isolation config-store seed SQL INSERTed warehouses
  without duckling_name → not-null violation. Set it to the org_id (= the
  canonical CR name) in all four seed files.
- TestConfigStoreSQLMigrationsUpgradeVersion8Schema pinned its pre-upgrade
  baseline at goose v8 but only deleted versions 9-11, leaving 12 recorded
  (latest read 12, want 8). Drop the NOT NULL and delete version 12 too so
  the baseline is a true v8 and migration 000012 re-applies on upgrade.
- Set duckling_name in the two Postgres warehouse test fixtures for clarity.
diff --git a/controlplane/admin/api.go b/controlplane/admin/api.go
@@ -456,6 +456,7 @@ type apiHandler struct {
 type managedWarehouseRequest struct {
 	Image                        string                                        `json:"image"`
 	DuckLakeVersion              string                                        `json:"ducklake_version"`
+	DucklingName                 string                                        `json:"duckling_name"`
 	WarehouseDatabase            configstore.ManagedWarehouseDatabase          `json:"warehouse_database"`
 	MetadataStore                configstore.ManagedWarehouseMetadataStore     `json:"metadata_store"`
 	PgBouncer                    configstore.ManagedWarehousePgBouncer         `json:"pgbouncer"`
@@ -818,6 +819,9 @@ func (h *apiHandler) putManagedWarehouse(c *gin.Context) {
 		if err := json.Unmarshal(body, w); err != nil {
 			return warehouseBadRequestError{err}
 		}
+		if w.DucklingName == "" {
+			return warehouseBadRequestError{errors.New("duckling_name cannot be empty")}
+		}
 		cfgView := &configstore.ManagedWarehouseConfig{
 			OrgID:                        orgID,
 			WarehouseDatabase:            w.WarehouseDatabase,
diff --git a/controlplane/admin/api_test.go b/controlplane/admin/api_test.go
@@ -222,7 +222,8 @@ func newTestAPIRouter(store apiStore) *gin.Engine {
 
 func seedOrgWithWarehouse(store *fakeAPIStore, name string) {
 	warehouse := &configstore.ManagedWarehouse{
-		OrgID: name,
+		OrgID:        name,
+		DucklingName: name,
 		WarehouseDatabase: configstore.ManagedWarehouseDatabase{
 			Endpoint: fmt.Sprintf("%s.cluster.example", name),
 			Port:     5432,
@@ -611,6 +612,7 @@ func TestPutWarehouseUpsertsForExistingOrg(t *testing.T) {
 	router := newTestAPIRouter(store)
 
 	body := []byte(`{
+		"duckling_name": "analytics",
 		"warehouse_database": {
 			"endpoint": "analytics.cluster.example",
 			"port": 5432
@@ -864,6 +866,7 @@ func TestPutWarehouseRejectsSecretRefsWithoutWorkerNamespace(t *testing.T) {
 	router := newTestAPIRouter(store)
 
 	body := []byte(`{
+		"duckling_name": "analytics",
 		"worker_identity": {
 			"iam_role_arn": "arn:aws:iam::123456789012:role/analytics-worker"
 		},
@@ -930,6 +933,7 @@ func TestPutWarehouseAllowsCustomProvisioningStates(t *testing.T) {
 	router := newTestAPIRouter(store)
 
 	body := []byte(`{
+		"duckling_name": "analytics",
 		"state": "awaiting-human-approval",
 		"metadata_store_state": "vendor-pending",
 		"s3_state": "bucket-handshake",
@@ -965,6 +969,7 @@ func TestPutWarehouseRejectsCrossTenantSecretRefs(t *testing.T) {
 	router := newTestAPIRouter(store)
 
 	body := []byte(`{
+		"duckling_name": "analytics",
 		"worker_identity": {
 			"namespace": "tenant-analytics"
 		},
@@ -994,6 +999,7 @@ func TestPutWarehouseRejectsSecretRefWithoutExplicitNamespace(t *testing.T) {
 	router := newTestAPIRouter(store)
 
 	body := []byte(`{
+		"duckling_name": "analytics",
 		"worker_identity": {
 			"namespace": "tenant-analytics"
 		},
@@ -1080,6 +1086,7 @@ func TestPutWarehouseRejectsSecretReferenceWithoutOrgPrefix(t *testing.T) {
 	router := newTestAPIRouter(store)
 
 	body := []byte(`{
+		"duckling_name": "analytics",
 		"worker_identity": {
 			"namespace": "tenant-a"
 		},
diff --git a/controlplane/admin/ui/src/pages/OrgDetail.tsx b/controlplane/admin/ui/src/pages/OrgDetail.tsx
@@ -288,14 +288,16 @@ function WarehousePanel({
   const update = useUpdateWarehouse(orgId);
   const [image, setImage] = useState("");
   const [version, setVersion] = useState("");
+  const [ducklingNameInput, setDucklingNameInput] = useState("");
   const [msg, setMsg] = useState<{ kind: "ok" | "err"; text: string } | null>(null);
 
   useEffect(() => {
     if (data) {
       setImage(data.image ?? "");
       setVersion(data.ducklake_version ?? "");
+      setDucklingNameInput(data.duckling_name || ducklingName(orgId));
     }
-  }, [data]);
+  }, [data, orgId]);
 
   const notFound = error instanceof ApiError && error.status === 404;
   const missing = notFound || !data;
@@ -308,10 +310,20 @@ function WarehousePanel({
         ? `Duckling unhealthy (state: ${data?.state})`
         : null;
 
+  const ducklingNameEmpty = ducklingNameInput.trim() === "";
+
   const save = async () => {
     setMsg(null);
+    if (ducklingNameEmpty) {
+      setMsg({ kind: "err", text: "Duckling name is required." });
+      return;
+    }
     try {
-      await update.mutateAsync({ image, ducklake_version: version });
+      await update.mutateAsync({
+        image,
+        ducklake_version: version,
+        duckling_name: ducklingNameInput,
+      });
       setMsg({ kind: "ok", text: "Saved." });
     } catch (e) {
       setMsg({ kind: "err", text: e instanceof Error ? e.message : "Save failed" });
@@ -374,6 +386,13 @@ function WarehousePanel({
             {/* Editable pinning */}
             <div className="space-y-3 border-t border-border pt-3">
               <p className="text-xs font-medium uppercase tracking-wide text-muted-foreground">Pinning</p>
+              <Field label="Duckling name">
+                <Input
+                  value={ducklingNameInput}
+                  onChange={(e) => setDucklingNameInput(e.target.value)}
+                  className="font-mono text-xs"
+                />
+              </Field>
               <Field label="Worker image">
                 <Input value={image} onChange={(e) => setImage(e.target.value)} className="font-mono text-xs" />
               </Field>
@@ -387,7 +406,7 @@ function WarehousePanel({
               </Field>
               <div className="flex items-center gap-3">
                 <AdminGate>
-                  <Button size="sm" onClick={save} disabled={update.isPending}>
+                  <Button size="sm" onClick={save} disabled={update.isPending || ducklingNameEmpty}>
                     <Save className="h-4 w-4" /> {update.isPending ? "Saving…" : "Save warehouse"}
                   </Button>
                 </AdminGate>
diff --git a/controlplane/configstore/migrations/000012_duckling_name_not_null.sql b/controlplane/configstore/migrations/000012_duckling_name_not_null.sql
@@ -0,0 +1,14 @@
+-- +goose Up
+
+-- Make the Duckling CR name mandatory. Re-run the lower(org_id) backfill first
+-- as a safety net (mirrors migration 000008) so any row left NULL/empty is
+-- filled before the NOT NULL constraint is applied, then enforce NOT NULL.
+UPDATE duckgres_managed_warehouses
+SET duckling_name = lower(org_id)
+WHERE duckling_name IS NULL OR duckling_name = '';
+
+ALTER TABLE duckgres_managed_warehouses ALTER COLUMN duckling_name SET NOT NULL;
+
+-- +goose Down
+
+ALTER TABLE duckgres_managed_warehouses ALTER COLUMN duckling_name DROP NOT NULL;
diff --git a/controlplane/configstore/models.go b/controlplane/configstore/models.go
@@ -290,7 +290,7 @@ type ManagedWarehouse struct {
 	// DucklingName is the k8s Duckling CR name; canonical = lowercased org ID.
 	// Authoritative, not derived — stored explicitly so lookups stop guessing
 	// it from the org ID (see provisioner.ducklingName for the canonical form).
-	DucklingName string `gorm:"size:255" json:"duckling_name"`
+	DucklingName string `gorm:"size:255;not null" json:"duckling_name"`
 
 	WarehouseDatabase ManagedWarehouseDatabase       `gorm:"embedded;embeddedPrefix:warehouse_database_" json:"warehouse_database"`
 	MetadataStore     ManagedWarehouseMetadataStore  `gorm:"embedded;embeddedPrefix:metadata_store_" json:"metadata_store"`
diff --git a/controlplane/provisioning/api.go b/controlplane/provisioning/api.go
@@ -274,6 +274,10 @@ func (h *handler) provisionWarehouse(c *gin.Context) {
 		// we inline it rather than import provisioner into this package.
 		DucklingName: strings.ToLower(orgID),
 	}
+	if warehouse.DucklingName == "" {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "duckling_name is required"})
+		return
+	}
 	if icebergEnabled {
 		warehouse.Iceberg = configstore.ManagedWarehouseIceberg{
 			Enabled:   true,
diff --git a/k8s/kind/config-store-seed.sql b/k8s/kind/config-store-seed.sql
@@ -5,6 +5,7 @@ SET updated_at = NOW();
 
 INSERT INTO duckgres_managed_warehouses (
     org_id,
+    duckling_name,
     warehouse_database_endpoint,
     warehouse_database_port,
     metadata_store_kind,
@@ -45,6 +46,7 @@ INSERT INTO duckgres_managed_warehouses (
     updated_at
 )
 VALUES (
+    'local',
     'local',
     'local-warehouse-db',
     5432,
diff --git a/k8s/kind/config-store.seed.sql b/k8s/kind/config-store.seed.sql
@@ -5,6 +5,7 @@ SET updated_at = NOW();
 
 INSERT INTO duckgres_managed_warehouses (
     org_id,
+    duckling_name,
     image,
     warehouse_database_endpoint,
     warehouse_database_port,
@@ -46,6 +47,7 @@ INSERT INTO duckgres_managed_warehouses (
     updated_at
 )
 VALUES (
+    'local',
     'local',
     '',
     'local-warehouse-db',
diff --git a/k8s/local-config-store.seed.sql b/k8s/local-config-store.seed.sql
@@ -5,6 +5,7 @@ SET updated_at = NOW();
 
 INSERT INTO duckgres_managed_warehouses (
     org_id,
+    duckling_name,
     warehouse_database_endpoint,
     warehouse_database_port,
     metadata_store_kind,
@@ -45,6 +46,7 @@ INSERT INTO duckgres_managed_warehouses (
     updated_at
 )
 VALUES (
+    'local',
     'local',
     'host.docker.internal',
     35434,
diff --git a/tests/configstore/managed_warehouse_postgres_test.go b/tests/configstore/managed_warehouse_postgres_test.go
@@ -31,7 +31,8 @@ func TestManagedWarehouseConfigStorePostgres(t *testing.T) {
 		t.Fatalf("create user: %v", err)
 	}
 	if err := store.DB().Create(&configstore.ManagedWarehouse{
-		OrgID: "analytics",
+		OrgID:        "analytics",
+		DucklingName: "analytics",
 		WarehouseDatabase: configstore.ManagedWarehouseDatabase{
 			Endpoint: "analytics.cluster.example",
 			Port:     5432,
@@ -84,8 +85,9 @@ func TestManagedWarehouseConfigStorePostgres(t *testing.T) {
 		t.Fatalf("create cleanup org: %v", err)
 	}
 	if err := store.DB().Create(&configstore.ManagedWarehouse{
-		OrgID: "cleanup",
-		State: configstore.ManagedWarehouseStateReady,
+		OrgID:        "cleanup",
+		DucklingName: "cleanup",
+		State:        configstore.ManagedWarehouseStateReady,
 	}).Error; err != nil {
 		t.Fatalf("create cleanup warehouse: %v", err)
 	}
diff --git a/tests/configstore/migrations_postgres_test.go b/tests/configstore/migrations_postgres_test.go
@@ -29,7 +29,8 @@ func TestConfigStoreRunsVersionedSQLMigrations(t *testing.T) {
 	requireGooseMigrationRecorded(t, db, 9)
 	requireGooseMigrationRecorded(t, db, 10)
 	requireGooseMigrationRecorded(t, db, 11)
-	requireGooseLatestVersion(t, db, 11)
+	requireGooseMigrationRecorded(t, db, 12)
+	requireGooseLatestVersion(t, db, 12)
 	requireTableAbsent(t, db, "duckgres_schema_migrations")
 
 	// Migration 000007 added the compute-usage billing buffer + drain state.
@@ -82,7 +83,8 @@ func TestConfigStoreSQLMigrationsUpgradeVersion8Schema(t *testing.T) {
 			ALTER TABLE duckgres_org_users DROP COLUMN max_vcpus;
 			ALTER TABLE duckgres_org_users DROP COLUMN disabled;
 			ALTER TABLE duckgres_orgs ADD COLUMN IF NOT EXISTS max_connections BIGINT DEFAULT 0;
-			DELETE FROM goose_db_version WHERE version_id IN (9, 10, 11);
+			ALTER TABLE duckgres_managed_warehouses ALTER COLUMN duckling_name DROP NOT NULL;
+			DELETE FROM goose_db_version WHERE version_id IN (9, 10, 11, 12);
 		`).Error; err != nil {
 		t.Fatalf("downgrade baseline schema to pre-v9 shape: %v", err)
 	}
@@ -104,7 +106,8 @@ func TestConfigStoreSQLMigrationsUpgradeVersion8Schema(t *testing.T) {
 	requireGooseMigrationRecorded(t, upgradedDB, 9)
 	requireGooseMigrationRecorded(t, upgradedDB, 10)
 	requireGooseMigrationRecorded(t, upgradedDB, 11)
-	requireGooseLatestVersion(t, upgradedDB, 11)
+	requireGooseMigrationRecorded(t, upgradedDB, 12)
+	requireGooseLatestVersion(t, upgradedDB, 12)
 	requireColumnDefault(t, upgradedDB, "duckgres_orgs", "max_vcpus", "0")
 	requireColumnDefault(t, upgradedDB, "duckgres_org_users", "max_vcpus", "0")
 	requireColumnDefault(t, upgradedDB, "duckgres_org_users", "disabled", "false")
diff --git a/tests/configstore/testdata/tenant-isolation.seed.sql b/tests/configstore/testdata/tenant-isolation.seed.sql
@@ -8,6 +8,7 @@ SET database_name = EXCLUDED.database_name,
 
 INSERT INTO duckgres_managed_warehouses (
     org_id,
+    duckling_name,
     image,
     warehouse_database_endpoint,
     warehouse_database_port,
@@ -50,6 +51,7 @@ INSERT INTO duckgres_managed_warehouses (
 )
 VALUES
 (
+    'analytics',
     'analytics',
     '',
     'duckgres-local-warehouse-db',
@@ -92,6 +94,7 @@ VALUES
     NOW()
 ),
 (
+    'billing',
     'billing',
     '',
     'duckgres-local-warehouse-db',
