From 20e139f6fc8a6a803346439fe9b06e9166802dc0 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 3 Jul 2026 16:38:40 +0200 Subject: [PATCH] admin: block org deletion while the warehouse exists; confirm modals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting an org in the admin UI removed only the config rows, silently orphaning the org's Duckling CR and its infrastructure (S3 bucket, metadata database, IAM role) — the drift banner caught exactly this in dev. Enforce the correct order: deprovision the warehouse (provisioner tears down the CR and removes the row), then delete the org. - DeleteOrg checks for a managed-warehouse row inside the delete transaction and refuses with a sentinel; the handler maps it to 409 ("deprovision it and wait for teardown to complete"). Handler tests for both the rejection and the post-teardown success path. - Admin UI gains a Deprovision warehouse action (danger zone in the warehouse panel) with a type-the-org-id confirmation modal spelling out what gets destroyed, wired to POST /orgs/:id/deprovision. - Org deletion is type-to-confirm too; while a warehouse exists the header Delete button is disabled with an immediate tooltip and the dialog hard-disables confirm (server 409 as backstop). --- controlplane/admin/api.go | 16 ++ controlplane/admin/api_test.go | 45 +++++- controlplane/admin/ui/src/hooks/useApi.ts | 14 ++ controlplane/admin/ui/src/lib/api.ts | 4 + controlplane/admin/ui/src/pages/OrgDetail.tsx | 141 +++++++++++++++++- 5 files changed, 212 insertions(+), 8 deletions(-) diff --git a/controlplane/admin/api.go b/controlplane/admin/api.go index 263ee485..cb8cc7ec 100644 --- a/controlplane/admin/api.go +++ b/controlplane/admin/api.go @@ -22,6 +22,8 @@ import ( var errWarehousePayloadNotAllowed = errors.New("warehouse payload must be updated via /orgs/:id/warehouse") +var errWarehouseStillExists = errors.New("managed warehouse still exists for org") + // maxWarehousePutBodyBytes caps the admin PUT body. Warehouse payloads are // under 10 KB in practice; 1 MiB leaves room for future fields while keeping // the handler from loading unbounded input into memory. @@ -225,6 +227,16 @@ func (s *gormAPIStore) UpdateOrg(name string, updates configstore.Org) (*configs func (s *gormAPIStore) DeleteOrg(name string) (bool, error) { returnRows := int64(0) err := s.db().Transaction(func(tx *gorm.DB) error { + // Deleting an org while its managed warehouse row still exists would + // cascade the row away and leak the Duckling CR + infra behind it. The + // warehouse must be deprovisioned (which removes the row) first. + var warehouses int64 + if err := tx.Model(&configstore.ManagedWarehouse{}).Where("org_id = ?", name).Count(&warehouses).Error; err != nil { + return err + } + if warehouses > 0 { + return errWarehouseStillExists + } if err := tx.Where("org_id = ?", name).Delete(&configstore.OrgUser{}).Error; err != nil { return err } @@ -638,6 +650,10 @@ func (h *apiHandler) deleteOrg(c *gin.Context) { name := c.Param("id") ok, err := h.store.DeleteOrg(name) if err != nil { + if errors.Is(err, errWarehouseStillExists) { + c.JSON(http.StatusConflict, gin.H{"error": "warehouse still exists — deprovision it and wait for teardown to complete before deleting the org"}) + return + } c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } diff --git a/controlplane/admin/api_test.go b/controlplane/admin/api_test.go index 9930e0e6..95075ba7 100644 --- a/controlplane/admin/api_test.go +++ b/controlplane/admin/api_test.go @@ -86,8 +86,10 @@ func (s *fakeAPIStore) DeleteOrg(name string) (bool, error) { if _, ok := s.orgs[name]; !ok { return false, nil } + if _, ok := s.warehouses[name]; ok { + return false, errWarehouseStillExists + } delete(s.orgs, name) - delete(s.warehouses, name) return true, nil } @@ -1110,6 +1112,47 @@ func TestUpdateOrgRejectsNestedWarehousePayload(t *testing.T) { } } +func TestDeleteOrgRejectsWhenWarehouseStillExists(t *testing.T) { + store := newFakeAPIStore() + seedOrgWithWarehouse(store, "analytics") + router := newTestAPIRouter(store) + + req := httptest.NewRequest(http.MethodDelete, "/api/v1/orgs/analytics", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusConflict { + t.Fatalf("status = %d, want %d: %s", rec.Code, http.StatusConflict, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "deprovision") { + t.Fatalf("expected error to mention deprovision, got: %s", rec.Body.String()) + } + if _, ok := store.orgs["analytics"]; !ok { + t.Fatal("expected org to survive a rejected delete") + } + if _, ok := store.warehouses["analytics"]; !ok { + t.Fatal("expected warehouse to survive a rejected delete") + } +} + +func TestDeleteOrgSucceedsAfterWarehouseRemoved(t *testing.T) { + store := newFakeAPIStore() + seedOrgWithWarehouse(store, "analytics") + delete(store.warehouses, "analytics") + router := newTestAPIRouter(store) + + req := httptest.NewRequest(http.MethodDelete, "/api/v1/orgs/analytics", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d: %s", rec.Code, http.StatusOK, rec.Body.String()) + } + if _, ok := store.orgs["analytics"]; ok { + t.Fatal("expected org to be deleted once the warehouse is gone") + } +} + func TestGetOrgOmitsMinWorkers(t *testing.T) { store := newFakeAPIStore() store.orgs["analytics"] = &configstore.Org{ diff --git a/controlplane/admin/ui/src/hooks/useApi.ts b/controlplane/admin/ui/src/hooks/useApi.ts index e2b5d2a9..ed5dc279 100644 --- a/controlplane/admin/ui/src/hooks/useApi.ts +++ b/controlplane/admin/ui/src/hooks/useApi.ts @@ -151,6 +151,20 @@ export function useUpdateWarehouse(id: string) { }); } +// POST /orgs/:id/deprovision — asynchronous duckling teardown. Invalidate the +// warehouse (state flips to deleting) and the org queries (warehouse presence +// gates org deletion). +export function useDeprovisionWarehouse(id: string) { + const qc = useQueryClient(); + return useMutation({ + mutationFn: () => api.deprovisionWarehouse(id), + onSuccess: () => { + qc.invalidateQueries({ queryKey: ["orgs", id, "warehouse"] }); + qc.invalidateQueries({ queryKey: ["orgs"] }); + }, + }); +} + // ---- ducklings (admin-only) ---- const EMPTY_DRIFT: DucklingDriftResponse = { available: false, checked: 0, entries: [] }; diff --git a/controlplane/admin/ui/src/lib/api.ts b/controlplane/admin/ui/src/lib/api.ts index 04a9e31a..c590c558 100644 --- a/controlplane/admin/ui/src/lib/api.ts +++ b/controlplane/admin/ui/src/lib/api.ts @@ -129,6 +129,10 @@ export const api = { getWarehouse: (id: string) => get(`/orgs/${enc(id)}/warehouse`), updateWarehouse: (id: string, body: Partial) => put(`/orgs/${enc(id)}/warehouse`, body), + // Kicks off asynchronous teardown of the org's duckling (202). 409s when the + // warehouse state isn't ready/failed/provisioning. + deprovisionWarehouse: (id: string) => + post<{ status: string; org: string }>(`/orgs/${enc(id)}/deprovision`, {}), // ducklings (admin-only) getDucklingDrift: () => get("/ducklings/drift"), diff --git a/controlplane/admin/ui/src/pages/OrgDetail.tsx b/controlplane/admin/ui/src/pages/OrgDetail.tsx index b49aca97..43392f76 100644 --- a/controlplane/admin/ui/src/pages/OrgDetail.tsx +++ b/controlplane/admin/ui/src/pages/OrgDetail.tsx @@ -25,6 +25,7 @@ import { ducklingBroken, ducklingEntryFor, fmtTime } from "@/lib/format"; import { ShardBadge } from "@/components/ShardBadge"; import { useDeleteOrg, + useDeprovisionWarehouse, useDucklingsMetadata, useOrg, useUpdateOrg, @@ -77,6 +78,7 @@ export function OrgDetail() { const [form, setForm] = useState(null); const [msg, setMsg] = useState<{ kind: "ok" | "err"; text: string } | null>(null); const [confirmDelete, setConfirmDelete] = useState(false); + const [deleteConfirmText, setDeleteConfirmText] = useState(""); useEffect(() => { if (org.data) setForm(orgToForm(org.data)); @@ -123,13 +125,23 @@ export function OrgDetail() { } }; + // Org deletion is blocked while a managed warehouse still exists: the correct + // flow is deprovision → provisioner tears down the duckling and removes the + // warehouse row → then delete. The backend 409s too; this is belt-and-suspenders. + const orgHasWarehouse = Boolean(org.data?.warehouse) || Boolean(warehouse.data); + + const closeDelete = () => { + setConfirmDelete(false); + setDeleteConfirmText(""); + }; + const doDelete = async () => { try { await del.mutateAsync(id); navigate("/orgs"); } catch (e) { setMsg({ kind: "err", text: e instanceof Error ? e.message : "Delete failed" }); - setConfirmDelete(false); + closeDelete(); } }; @@ -139,9 +151,26 @@ export function OrgDetail() { id={id} actions={ - + {orgHasWarehouse ? ( + // Disabled buttons swallow pointer events, so the tooltip + // triggers on a wrapping span. delayDuration 0 = immediate. + + + + + + + + Deprovision the warehouse first — org delete is blocked while it exists. + + + ) : ( + + )} } /> @@ -230,7 +259,7 @@ export function OrgDetail() { - + (open ? setConfirmDelete(true) : closeDelete())}> Delete org "{id}"? @@ -238,11 +267,35 @@ export function OrgDetail() { This removes the org and all of its users from the config store. This cannot be undone. +
+ {orgHasWarehouse && ( +

+ + + This org still has a managed warehouse. Deletion is blocked until the warehouse is + deprovisioned and fully gone — deprovision it from the warehouse panel first. + +

+ )} + + setDeleteConfirmText(e.target.value)} + placeholder={id} + className="font-mono text-xs" + /> + +
- - @@ -298,10 +351,13 @@ function WarehousePanel({ error: unknown; }) { const update = useUpdateWarehouse(orgId); + const deprovision = useDeprovisionWarehouse(orgId); const metadata = useDucklingsMetadata(); const [image, setImage] = useState(""); const [version, setVersion] = useState(""); const [ducklingNameInput, setDucklingNameInput] = useState(""); + const [confirmDeprovision, setConfirmDeprovision] = useState(false); + const [deprovisionConfirmText, setDeprovisionConfirmText] = useState(""); const [msg, setMsg] = useState<{ kind: "ok" | "err"; text: string } | null>(null); useEffect(() => { @@ -352,6 +408,26 @@ function WarehousePanel({ } }; + // Already deleting/deleted (or no warehouse at all) → nothing to deprovision. + const canDeprovision = !missing && data != null && data.state !== "deleting" && data.state !== "deleted"; + + const closeDeprovision = () => { + setConfirmDeprovision(false); + setDeprovisionConfirmText(""); + }; + + const doDeprovision = async () => { + setMsg(null); + try { + await deprovision.mutateAsync(); + setMsg({ kind: "ok", text: "Deprovisioning started." }); + } catch (e) { + // A 409 (wrong warehouse state) surfaces its backend message as-is. + setMsg({ kind: "err", text: e instanceof Error ? e.message : "Deprovision failed" }); + } + closeDeprovision(); + }; + return ( @@ -462,9 +538,60 @@ function WarehousePanel({ + + {/* Teardown */} + {canDeprovision && ( +
+

Danger zone

+
+ + + + + Required before the org can be deleted. + +
+
+ )} )} + + (open ? setConfirmDeprovision(true) : closeDeprovision())}> + + + Deprovision warehouse for "{orgId}"? + + This permanently tears down the org's duckling — the Duckling CR, the S3 data bucket, the + metadata database, and the IAM role. Teardown runs asynchronously; the org itself is not + deleted. This cannot be undone. + + + + setDeprovisionConfirmText(e.target.value)} + placeholder={orgId} + className="font-mono text-xs" + /> + + + + + + + ); }