diff --git a/CLAUDE.md b/CLAUDE.md index 92a697143..b0ecb7c35 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1680,6 +1680,13 @@ password/tenant/catalog changes never propagate. and deliberately no assignment policy, capacity model, rebalancer or cell drain; `resolveTrinoCell` becoming `resolveTrinoCells` is the whole shape of adding a second. +- **The existing deployment's API identity is `legacy`.** The Trino console + exposes this name in `cell.id` and in owned orgs' `status.cell` / `orgs[].cell`. + `TrinoCell.StoredID` keeps the configured ownership ID private to the adapter. + Match connection readiness against the raw persisted ID, never the alias. + Do not rename org assignments, catalog-store keys or environment settings. + The general org endpoint still exposes the original `trino.trino_cell_id`. + Unknown and unassigned ownership values are not relabeled. - **The bundle endpoint is mounted OUTSIDE `/api/v1`** (`/bundles/trino`) with its own bearer auth, and `buildTrinoWiring` bootstraps SYNCHRONOUSLY so the handler is constructed with the real token — there is no window where it diff --git a/README.md b/README.md index c443016c1..741c4040a 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,7 @@ A PostgreSQL wire protocol compatible server backed by DuckDB. Connect with any ## Table of Contents - [Features](#features) +- [Trino API identity](#trino-api-identity) - [Metrics](#metrics) - [Runbooks](#runbooks) - [Perf Runbook](docs/perf-harness-runbook.md) @@ -59,6 +60,16 @@ A PostgreSQL wire protocol compatible server backed by DuckDB. Connect with any - **Flexible Configuration**: YAML config files, environment variables, and CLI flags - **Prometheus Metrics**: Built-in metrics endpoint for monitoring +## Trino API identity + +The existing Trino deployment appears as `legacy` in the Trino console API. +This name does not change its stored org assignments or catalog-store key. +`DUCKGRES_TRINO_CELL_ID` remains the ownership setting, with the existing default +`cell-001`; do not change it to `legacy` to match the API display name. +Connection details remain readiness-gated and use the existing endpoint. +See the [Trino admin API documentation](controlplane/admin/README.md#trino-cell-views-trinogo--trino_clientgo) +for local verification, compatibility details, and recovery instructions. + ## Metrics Duckgres exposes Prometheus metrics on `:9090/metrics`. The metrics port is currently fixed at 9090 and cannot be changed via configuration. diff --git a/controlplane/admin/README.md b/controlplane/admin/README.md index 92451c665..052aa965c 100644 --- a/controlplane/admin/README.md +++ b/controlplane/admin/README.md @@ -190,6 +190,21 @@ API, as a dedicated **observer principal** (`opa.ObserverPrincipal` = `__duckgres_observer`) that the provisioner mints alongside the admin pair and projects into `password.db` / `group.db`. +The existing deployment appears as `legacy` in the Trino API's `cell.id` and +its owned orgs' `status.cell` / `orgs[].cell`. This is an API alias, not a storage +migration: `DUCKGRES_TRINO_CELL_ID`, persisted org assignments, and Trino's +catalog-store key retain their existing values. The general org endpoint still +returns the persisted `trino.trino_cell_id`. Unassigned and foreign-cell rows +retain their original IDs and receive no connection details. This change adds +no cell selection or tenant migration endpoint. + +For local verification, run `just test-controlplane-k8s` and `just ui-test`. +The isolated Trino end-to-end suite checks both the API alias and unchanged +persisted ownership. If connection details disappear after an upgrade, check +the org's persisted ID against the configured `DUCKGRES_TRINO_CELL_ID`; do not +rename stored IDs to match the API alias. Roll back the application version if +an API consumer requires the previous displayed ID. + - **Why a second principal.** Trino routes operator reads through the same access-control SPI as everything else — `GET /v1/query` filters through `FilterViewQueryOwnedBy`, `/v1/query/{id}` is gated on `ViewQueryOwnedBy`, diff --git a/controlplane/admin/trino.go b/controlplane/admin/trino.go index 9024b6e56..a1203b294 100644 --- a/controlplane/admin/trino.go +++ b/controlplane/admin/trino.go @@ -46,12 +46,28 @@ type TrinoOrgStore interface { // (see controlplane/trino_inputs.go); every payload carries the id so the // SPA and its consumers are already cell-aware when a second one lands. type TrinoCell struct { - ID string `json:"id"` + ID string `json:"id"` + // StoredID separates persisted ownership from the API identity. Empty uses ID. + StoredID string `json:"-"` CoordinatorURL string `json:"coordinator_url"` TLSServerName string `json:"-"` ClientURL string `json:"-"` } +func (c TrinoCell) storedID() string { + if c.StoredID != "" { + return c.StoredID + } + return c.ID +} + +func (c TrinoCell) publicID(storedID string) string { + if storedID != "" && storedID == c.storedID() { + return c.ID + } + return storedID +} + type TrinoConnection struct { Host string `json:"host"` Port int `json:"port"` @@ -591,7 +607,9 @@ func (a *TrinoAPI) handleOrgs(c *gin.Context) { out := make([]TrinoOrgStatus, 0, len(idx.rows)) for _, o := range idx.rows { - out = append(out, trinoOrgStatus(o, running[o.OrgID], queued[o.OrgID])) + status := trinoOrgStatus(o, running[o.OrgID], queued[o.OrgID]) + status.Cell = a.cell.publicID(o.CellID) + out = append(out, status) } sort.Slice(out, func(i, j int) bool { return out[i].Org < out[j].Org }) @@ -651,8 +669,8 @@ func (a *TrinoAPI) handleOrgDetail(c *gin.Context) { status.ReadyAt = row.ReadyAt status.FailedAt = row.FailedAt status.Tier = row.Tier - status.Cell = row.TrinoCellID - if available && status.State == string(configstore.ManagedWarehouseStateReady) && status.Cell == a.cell.ID { + status.Cell = a.cell.publicID(row.TrinoCellID) + if available && status.State == string(configstore.ManagedWarehouseStateReady) && row.TrinoCellID != "" && row.TrinoCellID == a.cell.storedID() { status.Connection = a.cell.connectionFor(status.Principal) } diff --git a/controlplane/admin/trino_test.go b/controlplane/admin/trino_test.go index 493ca931a..eb7b25c66 100644 --- a/controlplane/admin/trino_test.go +++ b/controlplane/admin/trino_test.go @@ -596,6 +596,54 @@ func TestReadyOrgDetailReturnsTenantClientConnection(t *testing.T) { } } +func TestTrinoCellAPIAliasPreservesStoredOwnership(t *testing.T) { + for _, storedID := range []string{"stored-cell", "another-cell", "legacy", ""} { + t.Run("stored="+storedID, func(t *testing.T) { + store := &fakeTrinoOrgStore{ + orgs: []configstore.TrinoEnabledOrg{{OrgID: "org-a", DatabaseName: "db_a", CellID: storedID, State: configstore.ManagedWarehouseStateReady}}, + rows: map[string]*configstore.ManagedWarehouseTrino{ + "org-a": {OrgID: "org-a", Enabled: true, TrinoCellID: storedID, State: configstore.ManagedWarehouseStateReady}, + }, + } + cell := TrinoCell{ID: "legacy", StoredID: "stored-cell", CoordinatorURL: "https://coordinator.example.test"} + api := NewTrinoAPI(cell, &fakeTrinoCoordinator{}, store, nil) + router := trinoTestRouter(api, RoleViewer) + wantID := storedID + if storedID == "stored-cell" { + wantID = "legacy" + } + for _, endpoint := range []string{"/api/v1/orgs/org-a/trino", "/api/v1/trino/orgs"} { + code, body := doTrinoJSON(t, router, http.MethodGet, endpoint, "") + if code != http.StatusOK { + t.Fatalf("%s: status %d", endpoint, code) + } + if body["cell"].(map[string]any)["id"] != "legacy" { + t.Fatalf("API cell identity: %+v", body) + } + var status map[string]any + if endpoint == "/api/v1/trino/orgs" { + status = body["orgs"].([]any)[0].(map[string]any) + } else { + status = body["status"].(map[string]any) + _, connected := status["connection"] + if connected != (storedID == "stored-cell") { + t.Fatalf("connection must match persisted ownership: %+v", status) + } + } + if status["cell"] != wantID { + t.Fatalf("status cell = %v, want %s", status["cell"], wantID) + } + if identity := body["cell"].(map[string]any); len(identity) != 2 || identity["coordinator_url"] != cell.CoordinatorURL { + t.Fatalf("unexpected public identity fields: %+v", identity) + } + } + if store.rows["org-a"].TrinoCellID != storedID || store.orgs[0].CellID != storedID { + t.Fatal("API alias mutated persisted ownership") + } + }) + } +} + // TestOrgDetailNotEnabledIsNotAnError: most orgs have no Trino row, and // the org page renders a "not enabled" state rather than a failure. func TestOrgDetailNotEnabledIsNotAnError(t *testing.T) { diff --git a/controlplane/admin/ui/src/lib/trino.test.ts b/controlplane/admin/ui/src/lib/trino.test.ts index 4e2176f70..f6f34c948 100644 --- a/controlplane/admin/ui/src/lib/trino.test.ts +++ b/controlplane/admin/ui/src/lib/trino.test.ts @@ -60,7 +60,7 @@ function node(over: Partial = {}): TrinoNode { function status(over: Partial = {}): TrinoStatus { return { - cell: { id: "cell-001", coordinator_url: "https://coordinator" }, + cell: { id: "legacy", coordinator_url: "https://coordinator" }, available: true, queries_by_state: {}, blocked_queries: 0, @@ -79,7 +79,7 @@ function org(over: Partial = {}): TrinoOrgStatus { principal: "db_a", catalog: "org_db_a", tier: "free", - cell: "cell-001", + cell: "legacy", state: "ready", running_queries: 0, queued_queries: 0, diff --git a/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx b/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx index 5a255bf32..bb40191ef 100644 --- a/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx +++ b/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx @@ -16,7 +16,7 @@ function orgStatus(over: Partial = {}): TrinoOrgStatus { principal: "product_analytics", catalog: "org_product_analytics", tier: "free", - cell: "cell-001", + cell: "legacy", state: "ready", ready_at: "2026-08-01T10:00:00Z", running_queries: 2, @@ -27,7 +27,7 @@ function orgStatus(over: Partial = {}): TrinoOrgStatus { function detail(over: Partial = {}): TrinoOrgDetail { return { - cell: { id: "cell-001", coordinator_url: "https://coordinator" }, + cell: { id: "legacy", coordinator_url: "https://coordinator" }, enabled: true, available: true, status: orgStatus(), @@ -61,6 +61,7 @@ describe("OrgTrinoCard", () => { expect(screen.getByText("product_analytics")).toBeInTheDocument(); expect(screen.getByText("org_product_analytics")).toBeInTheDocument(); expect(screen.getByText("ready")).toBeInTheDocument(); + expect(screen.getByText("legacy")).toBeInTheDocument(); }); it("surfaces the reconcile failure message, which is the actionable part", () => { diff --git a/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx b/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx index e3bb9a590..09fe40695 100644 --- a/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx +++ b/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx @@ -54,7 +54,7 @@ function query(over: Partial = {}): TrinoQuery { function status(over: Partial = {}): TrinoStatus { return { - cell: { id: "cell-001", coordinator_url: "https://coordinator" }, + cell: { id: "legacy", coordinator_url: "https://coordinator" }, available: true, queries_by_state: {}, blocked_queries: 0, diff --git a/controlplane/trino_identity_test.go b/controlplane/trino_identity_test.go new file mode 100644 index 000000000..b3789d726 --- /dev/null +++ b/controlplane/trino_identity_test.go @@ -0,0 +1,16 @@ +//go:build kubernetes + +package controlplane + +import "testing" + +func TestTrinoLegacyConsoleIdentityKeepsProvisionerID(t *testing.T) { + cell := trinoCell{ID: "stored-cell", CoordinatorURL: "https://coordinator.example.test", TLSServerName: "tls.example.test", ClientURL: "https://client.example.test"} + console := cell.consoleCell() + if console.ID != "legacy" || console.StoredID != "stored-cell" { + t.Fatalf("console identity: %+v", console) + } + if cell.ID != "stored-cell" || console.CoordinatorURL != cell.CoordinatorURL || console.TLSServerName != cell.TLSServerName || console.ClientURL != cell.ClientURL { + t.Fatal("alias changed storage identity or connection configuration") + } +} diff --git a/controlplane/trino_inputs.go b/controlplane/trino_inputs.go index bdfeab3e6..8ada793d5 100644 --- a/controlplane/trino_inputs.go +++ b/controlplane/trino_inputs.go @@ -144,6 +144,17 @@ type trinoCell struct { ClientURL string } +// consoleCell names the existing deployment without changing its persisted ownership. +func (c trinoCell) consoleCell() admin.TrinoCell { + return admin.TrinoCell{ + ID: "legacy", + StoredID: c.ID, + CoordinatorURL: c.CoordinatorURL, + TLSServerName: c.TLSServerName, + ClientURL: c.ClientURL, + } +} + // resolveTrinoCell reads the single cell's configuration from the // environment. Returns an error when the coordinator URL is missing, which // is fatal for an operator who asked for Trino. @@ -312,12 +323,7 @@ func buildTrinoWiring( BundleHandler: bundleHandler, Cell: cell, Console: &trinoConsoleWiring{ - Cell: admin.TrinoCell{ - ID: cell.ID, - CoordinatorURL: cell.CoordinatorURL, - TLSServerName: cell.TLSServerName, - ClientURL: cell.ClientURL, - }, + Cell: cell.consoleCell(), // Read the credential through the provisioner on every call // rather than capturing it here: the pair is regenerated if it // ever goes missing, and a captured copy would 401 forever diff --git a/tests/mw-dev/e2e/trino.sh b/tests/mw-dev/e2e/trino.sh index ac0155f70..7424094f2 100644 --- a/tests/mw-dev/e2e/trino.sh +++ b/tests/mw-dev/e2e/trino.sh @@ -94,9 +94,12 @@ wait_trino() { # org expected-principal expected-catalog body="$(api "$API/api/v1/orgs/$1/trino" 2>/dev/null || true)" state="$(printf %s "$body" | jq -r '.status.state // empty' 2>/dev/null || true)" if [ "$state" = ready ]; then - printf %s "$body" | jq -e --arg p "$2" --arg c "$3" --arg cell "ci-pr-$PR" --arg host "duckgres-trino.$NS.svc" \ + printf %s "$body" | jq -e --arg p "$2" --arg c "$3" --arg cell legacy --arg host "duckgres-trino.$NS.svc" \ '.enabled == true and .available == true and .status.principal == $p and .status.catalog == $c and .status.cell == $cell and .status.tier == "free" and .status.connection.host == $host and .status.connection.port == 8443 and .status.connection.username == $p and (.status.connection | has("password") | not)' >/dev/null \ || fail "$1 Trino status identity mismatch: $body" + api "$API/api/v1/orgs/$1" | jq -e --arg stored_cell "ci-pr-$PR" \ + '.trino.trino_cell_id == $stored_cell' >/dev/null \ + || fail "$1 legacy API identity changed persisted Trino ownership" TRINO="https://$(printf %s "$body" | jq -r '.status.connection.host'):$(printf %s "$body" | jq -r '.status.connection.port')" return 0 fi @@ -214,10 +217,10 @@ must_fail "$DB_B" "$pw_b" "DROP TABLE $CAT_A.$schema.$table" 'denied|access|cata [ "$(scalar "$DB_A" "$pw_a" "SELECT count(*) FROM $CAT_A.$schema.$table")" = 1 ] || fail "cross-tenant attempts changed tenant A data" log "admin Trino fleet/org/query surfaces" -api "$API/api/v1/trino/status" | jq -e --arg cell "ci-pr-$PR" '.available == true and .cell.id == $cell' >/dev/null +api "$API/api/v1/trino/status" | jq -e --arg cell legacy '.available == true and .cell.id == $cell' >/dev/null api "$API/api/v1/trino/nodes" | jq -e '.available == true and (.nodes | length) >= 2' >/dev/null api "$API/api/v1/trino/orgs" | jq -e --arg a "$ORG_A" --arg b "$ORG_B" \ - 'any(.orgs[]; .org == $a and .state == "ready") and any(.orgs[]; .org == $b and .state == "ready")' >/dev/null + 'any(.orgs[]; .org == $a and .state == "ready" and .cell == "legacy") and any(.orgs[]; .org == $b and .state == "ready" and .cell == "legacy")' >/dev/null queries="$(api "$API/api/v1/trino/queries?org=$ORG_A")" printf %s "$queries" | jq -e --arg a "$ORG_A" --arg table "$table" \ 'all(.queries[]; .org == $a) and any(.queries[]; .query | contains($table))' >/dev/null \