Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions controlplane/admin/ducklings_drift.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"context"
"fmt"
"net/http"
"strings"
"time"

"github.com/gin-gonic/gin"
Expand All @@ -17,7 +16,9 @@ import (
// needs. Declared here so the admin package doesn't import provisioner;
// *provisioner.DucklingClient satisfies it.
type DucklingChecker interface {
CRStatus(ctx context.Context, orgID string) (present, ready bool, err error)
// CRStatus takes the Duckling CR name — the warehouse row's duckling_name,
// never a derived value.
CRStatus(ctx context.Context, name string) (present, ready bool, err error)
ListCRNames(ctx context.Context) ([]string, error)
}

Expand Down Expand Up @@ -71,27 +72,23 @@ func (h *driftHandler) findDrift(c *gin.Context) {
}

entries := make([]driftEntry, 0)
// expected holds every CR name a warehouse legitimately maps to, so the
// orphan pass below doesn't flag a CR that simply uses the legacy
// (hyphen-stripped) name.
expected := make(map[string]struct{}, len(warehouses)*2)
// expected holds exactly the stored duckling_name of every warehouse row,
// so the orphan pass below flags any CR not claimed by a row.
expected := make(map[string]struct{}, len(warehouses))

for i := range warehouses {
wh := warehouses[i]
orgID := wh.OrgID
// The stored duckling_name is the authoritative CR name. The column is
// NOT NULL and backfilled; the org-ID fallback only covers legacy rows
// with an empty value (the CR name defaults to the org ID verbatim).
name := wh.DucklingName
if name == "" {
// Fallback for rows the backfill missed; mirrors
// provisioner.ducklingName (lowercased org ID).
name = strings.ToLower(orgID)
name = orgID
}
expected[name] = struct{}{}
// Also expect the canonical and legacy (hyphen-stripped) variants so
// pre-rename CRs aren't misreported as orphans.
expected[strings.ToLower(orgID)] = struct{}{}
expected[strings.ReplaceAll(strings.ToLower(orgID), "-", "")] = struct{}{}

present, ready, cerr := h.checker.CRStatus(ctx, orgID)
present, ready, cerr := h.checker.CRStatus(ctx, name)
if cerr != nil {
entries = append(entries, driftEntry{
Org: orgID,
Expand Down
13 changes: 7 additions & 6 deletions controlplane/admin/ui/src/lib/colors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ describe("hashColor", () => {
});

describe("ducklingEntryFor", () => {
const entries = { "acme-corp": 1, acmecorp: 2, custom: 3 };
const entries = { "acme-corp": 1, custom: 3 };

it("prefers the stored duckling_name, then canonical, then legacy", () => {
it("matches the stored duckling_name or the org name exactly — no derivation", () => {
expect(ducklingEntryFor(entries, "Acme-Corp", "custom")).toBe(3);
expect(ducklingEntryFor(entries, "Acme-Corp")).toBe(1);
expect(ducklingEntryFor({ acmecorp: 2 }, "Acme-Corp")).toBe(2);
expect(ducklingEntryFor(entries, "unknown-org")).toBeUndefined();
expect(ducklingEntryFor(undefined, "Acme-Corp")).toBeUndefined();
expect(ducklingEntryFor(entries, "acme-corp", undefined)).toBe(1);
// No lowercasing or hyphen-stripping of the org name anymore.
expect(ducklingEntryFor(entries, "Acme-Corp", undefined)).toBeUndefined();
expect(ducklingEntryFor(entries, "unknown-org", undefined)).toBeUndefined();
expect(ducklingEntryFor(undefined, "acme-corp", "custom")).toBeUndefined();
});
});
22 changes: 6 additions & 16 deletions controlplane/admin/ui/src/lib/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,27 +145,17 @@ export function isZeroTime(ts: string | null | undefined): boolean {
return !ts || ts.startsWith(ZERO_TIME);
}

// The Duckling (managed warehouse) CR name is the org ID lowercased, hyphens
// preserved.
export function ducklingName(orgName: string): string {
return orgName.toLowerCase();
}

// Resolve an org's entry in a Duckling-CR-name-keyed map: the stored
// duckling_name wins, then the canonical (lowercased) org name, then the
// legacy hyphen-stripped variant pre-rename CRs still use (mirrors the drift
// finder's expected-name set).
// Resolve an org's entry in a Duckling-CR-name-keyed map by exact match only:
// the warehouse's stored duckling_name (authoritative, NOT NULL in the DB)
// wins, then the org name itself. No client-side derivation of CR names.
export function ducklingEntryFor<T>(
entries: Record<string, T> | undefined,
orgName: string,
ducklingNameOverride?: string,
ducklingName: string | undefined,
): T | undefined {
if (!entries) return undefined;
const canonical = ducklingName(orgName);
for (const name of [ducklingNameOverride, canonical, canonical.replaceAll("-", "")]) {
if (name && entries[name] !== undefined) return entries[name];
}
return undefined;
if (ducklingName && entries[ducklingName] !== undefined) return entries[ducklingName];
return entries[orgName];
}

// A Duckling is broken/unhealthy when its warehouse row exists but the overall
Expand Down
10 changes: 5 additions & 5 deletions controlplane/admin/ui/src/pages/OrgDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
DialogTitle,
} from "@/components/ui/dialog";
import { ApiError } from "@/lib/api";
import { ducklingBroken, ducklingEntryFor, ducklingName, fmtTime } from "@/lib/format";
import { ducklingBroken, ducklingEntryFor, fmtTime } from "@/lib/format";
import { ShardBadge } from "@/components/ShardBadge";
import {
useDeleteOrg,
Expand Down Expand Up @@ -308,9 +308,9 @@ function WarehousePanel({
if (data) {
setImage(data.image ?? "");
setVersion(data.ducklake_version ?? "");
setDucklingNameInput(data.duckling_name || ducklingName(orgId));
setDucklingNameInput(data.duckling_name ?? "");
}
}, [data, orgId]);
}, [data]);

const notFound = error instanceof ApiError && error.status === 404;
const missing = notFound || !data;
Expand All @@ -337,7 +337,7 @@ function WarehousePanel({
const body: Partial<ManagedWarehouse> = {};
if (image !== (data?.image ?? "")) body.image = image;
if (version !== (data?.ducklake_version ?? "")) body.ducklake_version = version;
if (ducklingNameInput !== (data?.duckling_name || ducklingName(orgId))) {
if (ducklingNameInput !== (data?.duckling_name ?? "")) {
body.duckling_name = ducklingNameInput;
}
if (Object.keys(body).length === 0) {
Expand All @@ -364,7 +364,7 @@ function WarehousePanel({
<div className="flex items-center justify-between rounded-md border border-border bg-background/40 px-3 py-2">
<div>
<div className="text-[10px] uppercase text-muted-foreground">Duckling</div>
<div className="font-mono text-xs">{data?.duckling_name || ducklingName(orgId)}</div>
<div className="font-mono text-xs">{data?.duckling_name ?? "—"}</div>
</div>
{/* Live metadata-store assignment from the Duckling CR status — the
cnpg shard the tenant's metadata actually lives on (the config
Expand Down
14 changes: 7 additions & 7 deletions controlplane/admin/ui/src/pages/Orgs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { StateBadge } from "@/components/StateBadge";
import { ShardBadge } from "@/components/ShardBadge";
import { EmptyState, ErrorState, TableSkeleton } from "@/components/states";
import { useDucklingDrift, useDucklingsMetadata, useOrgs } from "@/hooks/useApi";
import { ducklingEntryFor, ducklingName, fmtInt } from "@/lib/format";
import { ducklingEntryFor, fmtInt } from "@/lib/format";
import type { DucklingDrift, Org } from "@/types/api";

export function Orgs() {
Expand Down Expand Up @@ -112,12 +112,12 @@ export function Orgs() {
{
id: "duckling",
header: "Duckling",
accessorFn: (o) => (o.warehouse ? o.warehouse.duckling_name || ducklingName(o.name) : ""),
cell: ({ row }) => {
const o = row.original;
const name = o.warehouse ? o.warehouse.duckling_name || ducklingName(o.name) : "";
return <span className="font-mono text-xs text-muted-foreground">{name || "—"}</span>;
},
accessorFn: (o) => o.warehouse?.duckling_name ?? "",
cell: ({ row }) => (
<span className="font-mono text-xs text-muted-foreground">
{row.original.warehouse?.duckling_name || "—"}
</span>
),
},
{
id: "shard",
Expand Down
4 changes: 2 additions & 2 deletions controlplane/admin/ui/src/types/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ export type WarehouseState =

export interface ManagedWarehouse {
org_id: string;
// The explicit Duckling CR name (provisioner-owned). May be "" on legacy rows
// that predate the field — callers fall back to ducklingName(org) in that case.
// The explicit Duckling CR name (provisioner-owned). Authoritative and always
// set — NOT NULL in the DB since migration 000012; no client-side derivation.
duckling_name: string;
image: string;
ducklake_version: string;
Expand Down
7 changes: 4 additions & 3 deletions controlplane/configstore/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,10 @@ type ManagedWarehouse struct {
Image string `gorm:"size:512" json:"image"`
DuckLakeVersion string `gorm:"size:32" json:"ducklake_version"`

// 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 is THE authoritative k8s Duckling CR name — nothing in the
// control plane derives or re-derives it. On warehouse create it defaults
// to the org ID verbatim (org IDs are validated as lowercase DNS-1123
// labels at the provisioning endpoint).
DucklingName string `gorm:"size:255;not null" json:"duckling_name"`

WarehouseDatabase ManagedWarehouseDatabase `gorm:"embedded;embeddedPrefix:warehouse_database_" json:"warehouse_database"`
Expand Down
12 changes: 12 additions & 0 deletions controlplane/configstore/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,18 @@ func (cs *ConfigStore) ListWarehouses() ([]ManagedWarehouse, error) {
return warehouses, nil
}

// GetManagedWarehouse returns the managed-warehouse row for an org. Direct DB
// query (not snapshot-based); returns gorm.ErrRecordNotFound when the org has
// no warehouse row. Callers use it to resolve the authoritative duckling_name
// before talking to the Duckling CR API.
func (cs *ConfigStore) GetManagedWarehouse(orgID string) (*ManagedWarehouse, error) {
var warehouse ManagedWarehouse
if err := cs.db.First(&warehouse, "org_id = ?", orgID).Error; err != nil {
return nil, err
}
return &warehouse, nil
}

func (cs *ConfigStore) ListWarehousesByStates(states []ManagedWarehouseProvisioningState) ([]ManagedWarehouse, error) {
var warehouses []ManagedWarehouse
if err := cs.db.Where("state IN ?", states).Find(&warehouses).Error; err != nil {
Expand Down
13 changes: 12 additions & 1 deletion controlplane/multitenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,18 @@ func SetupMultiTenant(
slog.Warn("Duckling client unavailable, will use config store for infrastructure details.", "error", dcErr)
} else {
resolveDucklingStatus = func(ctx context.Context, orgID string) (*provisioner.DucklingStatus, error) {
return dc.Get(ctx, orgID)
// The CR name is the warehouse row's duckling_name (authoritative,
// never derived). The column is NOT NULL and backfilled; the org-ID
// fallback only covers legacy in-flight rows with an empty value.
warehouse, err := store.GetManagedWarehouse(orgID)
if err != nil {
return nil, fmt.Errorf("resolve duckling name for org %q: %w", orgID, err)
}
name := warehouse.DucklingName
if name == "" {
name = orgID
}
return dc.Get(ctx, name)
}
}

Expand Down
28 changes: 20 additions & 8 deletions controlplane/provisioner/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ func captureProvisionFailed(w *configstore.ManagedWarehouse, reason string, upda
analytics.Default().Capture("warehouse_provision_failed", w.OrgID, props)
}

// ducklingCRName returns the Duckling CR name for a warehouse row: the stored
// duckling_name, which is authoritative (the control plane never derives it).
// The column is NOT NULL and backfilled in the database, so the org-ID
// fallback only covers zero-value test fixtures and legacy in-flight rows
// constructed in memory before the column existed.
func ducklingCRName(w *configstore.ManagedWarehouse) string {
if w.DucklingName != "" {
return w.DucklingName
}
return w.OrgID
}

// WarehouseStore is the subset of configstore.ConfigStore that the controller needs.
type WarehouseStore interface {
ListWarehousesByStates(states []configstore.ManagedWarehouseProvisioningState) ([]configstore.ManagedWarehouse, error)
Expand Down Expand Up @@ -162,7 +174,7 @@ func (c *Controller) reconcilePending(ctx context.Context, w *configstore.Manage
now := time.Now().UTC()

// Check if a Duckling CR already exists (e.g., controller restart)
_, err := c.duckling.Get(ctx, w.OrgID)
_, err := c.duckling.Get(ctx, ducklingCRName(w))
if err == nil {
// CR exists — transition directly to provisioning
log.Info("Duckling CR already exists, transitioning to provisioning.")
Expand All @@ -179,7 +191,7 @@ func (c *Controller) reconcilePending(ctx context.Context, w *configstore.Manage
// Create the Duckling CR
log.Info("Creating Duckling CR.",
"pgbouncer_enabled", w.PgBouncer.Enabled)
if err := c.duckling.Create(ctx, w.OrgID, CreateOptions{
if err := c.duckling.Create(ctx, ducklingCRName(w), CreateOptions{
MetadataStoreType: w.MetadataStore.Kind,
PgBouncerEnabled: w.PgBouncer.Enabled,
ExternalEndpoint: w.MetadataStore.Endpoint,
Expand Down Expand Up @@ -231,7 +243,7 @@ func (c *Controller) reconcileProvisioning(ctx context.Context, w *configstore.M
return
}

status, err := c.duckling.Get(ctx, w.OrgID)
status, err := c.duckling.Get(ctx, ducklingCRName(w))
if err != nil {
log.Warn("Failed to get Duckling CR status.", "error", err)
return
Expand Down Expand Up @@ -352,7 +364,7 @@ func (c *Controller) reconcileProvisioning(ctx context.Context, w *configstore.M
func (c *Controller) reconcileReady(ctx context.Context, w *configstore.ManagedWarehouse) {
log := slog.With("org", w.OrgID, "phase", "ready")

currentPgB, err := c.duckling.GetPgBouncerEnabled(ctx, w.OrgID)
currentPgB, err := c.duckling.GetPgBouncerEnabled(ctx, ducklingCRName(w))
if err != nil {
if apierrors.IsNotFound(err) {
// CR is gone but the warehouse is still marked Ready. Don't try
Expand All @@ -367,7 +379,7 @@ func (c *Controller) reconcileReady(ctx context.Context, w *configstore.ManagedW
if currentPgB != w.PgBouncer.Enabled {
log.Info("PgBouncer drift detected, patching Duckling CR.",
"desired", w.PgBouncer.Enabled, "current", currentPgB)
if err := c.duckling.SetPgBouncerEnabled(ctx, w.OrgID, w.PgBouncer.Enabled); err != nil {
if err := c.duckling.SetPgBouncerEnabled(ctx, ducklingCRName(w), w.PgBouncer.Enabled); err != nil {
log.Warn("Failed to patch Duckling CR pgbouncer.enabled.", "error", err)
}
}
Expand Down Expand Up @@ -401,7 +413,7 @@ func (c *Controller) reconcileBucketName(ctx context.Context, w *configstore.Man
return
}

current, err := c.duckling.GetDataStoreBucketName(ctx, w.OrgID)
current, err := c.duckling.GetDataStoreBucketName(ctx, ducklingCRName(w))
if err != nil {
if apierrors.IsNotFound(err) {
return
Expand All @@ -417,7 +429,7 @@ func (c *Controller) reconcileBucketName(ctx context.Context, w *configstore.Man

if current == "" {
log.Info("Backfilling CP-owned bucket name onto Duckling CR.", "bucket", desired)
if err := c.duckling.SetDataStoreBucketName(ctx, w.OrgID, desired); err != nil {
if err := c.duckling.SetDataStoreBucketName(ctx, ducklingCRName(w), desired); err != nil {
log.Warn("Failed to patch Duckling CR dataStore.bucketName.", "error", err)
return
}
Expand All @@ -438,7 +450,7 @@ func (c *Controller) reconcileDeleting(ctx context.Context, w *configstore.Manag
log := slog.With("org", w.OrgID, "phase", "deleting")

log.Info("Deleting Duckling CR.")
if err := c.duckling.Delete(ctx, w.OrgID); err != nil {
if err := c.duckling.Delete(ctx, ducklingCRName(w)); err != nil {
// Only proceed if the CR is already gone (NotFound). For other errors
// (network, RBAC, etc.) we retry on the next reconcile pass to avoid
// marking as deleted while AWS resources still exist.
Expand Down
Loading
Loading