From e0442e8627ff267c4b67ab17f324d964132c558a Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 14:22:26 +0200 Subject: [PATCH 1/8] feat: add initial Trino cell selection and fleet admin views --- controlplane/admin/README.md | 23 +++- controlplane/admin/trino.go | 37 +++-- controlplane/admin/trino_fleet.go | 126 ++++++++++++++++++ controlplane/admin/trino_fleet_test.go | 95 +++++++++++++ controlplane/admin/ui/src/hooks/useApi.ts | 40 ++++-- controlplane/admin/ui/src/lib/api.ts | 19 ++- .../admin/ui/src/pages/OrgTrinoCard.test.tsx | 55 +++++++- .../admin/ui/src/pages/OrgTrinoCard.tsx | 40 +++++- .../admin/ui/src/pages/TrinoCluster.tsx | 10 +- .../admin/ui/src/pages/TrinoQueries.test.tsx | 10 +- .../admin/ui/src/pages/TrinoQueries.tsx | 10 +- controlplane/admin/ui/src/types/api.ts | 1 + controlplane/configstore/trino.go | 50 ++++++- justfile | 5 + .../trino_selection_postgres_test.go | 114 ++++++++++++++++ 15 files changed, 585 insertions(+), 50 deletions(-) create mode 100644 controlplane/admin/trino_fleet.go create mode 100644 controlplane/admin/trino_fleet_test.go create mode 100644 tests/configstore/trino_selection_postgres_test.go diff --git a/controlplane/admin/README.md b/controlplane/admin/README.md index 052aa965..43599429 100644 --- a/controlplane/admin/README.md +++ b/controlplane/admin/README.md @@ -194,9 +194,26 @@ 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. +returns the persisted `trino.trino_cell_id`. Registered cells use separate +`registered:` ownership values. Unknown stored owners fail closed. + +`GET /api/v1/trino/cells` lists configured logical cells. Operational Trino +routes accept `?cell=` and default to `legacy`. Org detail resolves +its authoritative stored assignment regardless of a supplied cell parameter. +Each coordinator has separate caches; tenant counts include only its cell. + +Admins can select an initial cell in the org's Trino card or send +`PUT /api/v1/orgs/:id/trino/cell` with `{"cell":"cell-001"}`. This stores a +disabled assignment before the normal enable endpoint is called. The warehouse +must already exist. Selection does not enable Trino or move an existing tenant. +An existing assignment is immutable, including after disable/re-enable; repeat +selection of the same assignment is idempotent. An enabled unassigned row also +rejects selection because its first provisioning tick may already be running. +On a 409, inspect the current assignment rather than editing its database row. +Moving an existing tenant requires a separate maintenance/drain workflow. + +For newly registered cells, configure the shared customer endpoint separately +from the observer coordinator URL. This API does not configure Gateway routing. 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 diff --git a/controlplane/admin/trino.go b/controlplane/admin/trino.go index a1203b29..c53885a8 100644 --- a/controlplane/admin/trino.go +++ b/controlplane/admin/trino.go @@ -225,13 +225,15 @@ func (c *trinoCache[T]) get(ctx context.Context, fetch func(context.Context) (T, // TrinoAPI is the console's Trino surface: a coordinator client, the config // store, and the cell's identity. type TrinoAPI struct { - cell TrinoCell - client TrinoCoordinatorClient - orgs TrinoOrgStore - audit *AuditStore - queries *trinoCache[[]TrinoQuery] - nodes *trinoCache[TrinoNodeInventory] - info *trinoCache[*TrinoServerInfo] + fleet map[string]*TrinoAPI + filterCell bool + cell TrinoCell + client TrinoCoordinatorClient + orgs TrinoOrgStore + audit *AuditStore + queries *trinoCache[[]TrinoQuery] + nodes *trinoCache[TrinoNodeInventory] + info *trinoCache[*TrinoServerInfo] } // NewTrinoAPI builds the console's Trino surface. A nil client or store @@ -259,6 +261,10 @@ func registerTrinoAPI(r *gin.RouterGroup, api *TrinoAPI) { if api == nil { return } + if api.fleet != nil { + registerTrinoFleetAPI(r, api) + return + } r.GET("/trino/status", api.handleStatus) r.GET("/trino/queries", api.handleQueries) r.GET("/trino/queries/:id", api.handleQueryDetail) @@ -281,7 +287,15 @@ func (a *TrinoAPI) index() (principalIndex, error) { return principalIndex{}, err } idx := principalIndex{orgByPrincipal: make(map[string]string, len(rows)), rows: rows} - for _, o := range rows { + if a.filterCell { + idx.rows = nil + for _, row := range rows { + if row.CellID == a.cell.storedID() || (row.CellID == "" && a.cell.ID == "legacy") { + idx.rows = append(idx.rows, row) + } + } + } + for _, o := range idx.rows { if p := o.TrinoPrincipal(); p != "" { idx.orgByPrincipal[p] = o.OrgID } @@ -623,10 +637,14 @@ func (a *TrinoAPI) handleOrgDetail(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } + a.writeOrgDetail(c, orgID, row) +} + +func (a *TrinoAPI) writeOrgDetail(c *gin.Context, orgID string, row *configstore.ManagedWarehouseTrino) { if row == nil || !row.Enabled { // Not an error: most orgs are not Trino-enabled, and the org page // renders a "not enabled" state rather than a failure. - c.JSON(http.StatusOK, gin.H{"cell": a.cell, "enabled": false}) + c.JSON(http.StatusOK, gin.H{"cell": a.cell, "enabled": false, "assigned": row != nil && row.TrinoCellID != ""}) return } @@ -677,6 +695,7 @@ func (a *TrinoAPI) handleOrgDetail(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "cell": a.cell, "enabled": true, + "assigned": row.TrinoCellID != "", "available": available, "status": status, }) diff --git a/controlplane/admin/trino_fleet.go b/controlplane/admin/trino_fleet.go new file mode 100644 index 00000000..2e298ed4 --- /dev/null +++ b/controlplane/admin/trino_fleet.go @@ -0,0 +1,126 @@ +//go:build kubernetes + +package admin + +import ( + "errors" + "net/http" + "sort" + + "github.com/gin-gonic/gin" + "github.com/posthog/duckgres/controlplane/configstore" +) + +// NewTrinoFleetAPI builds isolated coordinator views and preserves the legacy default. +func NewTrinoFleetAPI(cells []TrinoCell, clients []TrinoCoordinatorClient, orgs TrinoOrgStore, audit *AuditStore) *TrinoAPI { + if len(cells) == 0 || len(cells) != len(clients) || orgs == nil { + return nil + } + a := &TrinoAPI{fleet: make(map[string]*TrinoAPI), orgs: orgs, audit: audit} + stored := make(map[string]bool) + for i, cell := range cells { + if cell.ID == "" || a.fleet[cell.ID] != nil || stored[cell.storedID()] { + return nil + } + child := NewTrinoAPI(cell, clients[i], orgs, audit) + if child == nil { + return nil + } + child.filterCell = true + a.fleet[cell.ID] = child + stored[cell.storedID()] = true + } + return a +} + +func registerTrinoFleetAPI(r *gin.RouterGroup, api *TrinoAPI) { + r.GET("/trino/cells", api.handleCells) + r.GET("/trino/status", api.forCell((*TrinoAPI).handleStatus)) + r.GET("/trino/queries", api.forCell((*TrinoAPI).handleQueries)) + r.GET("/trino/queries/:id", api.forCell((*TrinoAPI).handleQueryDetail)) + r.POST("/trino/queries/:id/kill", api.forCell((*TrinoAPI).handleKillQuery)) + r.GET("/trino/nodes", api.forCell((*TrinoAPI).handleNodes)) + r.GET("/trino/orgs", api.forCell((*TrinoAPI).handleOrgs)) + r.GET("/orgs/:id/trino", api.handleFleetOrg) + r.PUT("/orgs/:id/trino/cell", api.handleSelectCell) +} + +func (a *TrinoAPI) forCell(handle func(*TrinoAPI, *gin.Context)) gin.HandlerFunc { + return func(c *gin.Context) { + id := c.DefaultQuery("cell", "legacy") + selected := a.fleet[id] + if selected == nil { + c.JSON(http.StatusNotFound, gin.H{"error": "unknown Trino cell"}) + return + } + handle(selected, c) + } +} + +func (a *TrinoAPI) handleCells(c *gin.Context) { + cells := make([]TrinoCell, 0, len(a.fleet)) + for _, cell := range a.fleet { + cells = append(cells, cell.cell) + } + sort.Slice(cells, func(i, j int) bool { return cells[i].ID < cells[j].ID }) + c.JSON(http.StatusOK, gin.H{"cells": cells}) +} + +func (a *TrinoAPI) handleFleetOrg(c *gin.Context) { + row, err := a.orgs.GetManagedWarehouseTrino(c.Param("id")) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "cannot read Trino assignment"}) + return + } + selected := a.fleet["legacy"] + if row != nil && row.TrinoCellID != "" { + selected = nil + for _, candidate := range a.fleet { + if candidate.cell.storedID() == row.TrinoCellID { + selected = candidate + break + } + } + } + if selected == nil { + c.JSON(http.StatusConflict, gin.H{"error": "the assigned Trino cell is not configured", "assigned": row != nil && row.TrinoCellID != ""}) + return + } + selected.writeOrgDetail(c, c.Param("id"), row) +} + +type trinoCellSelector interface { + SelectTrinoCell(orgID, cellID string) error +} + +func (a *TrinoAPI) handleSelectCell(c *gin.Context) { + identity := IdentityFromContext(c) + if identity == nil || identity.Role != RoleAdmin { + c.JSON(http.StatusForbidden, gin.H{"error": "admin role required"}) + return + } + var req struct { + Cell string `json:"cell" binding:"required"` + } + if err := c.ShouldBindJSON(&req); err != nil || a.fleet[req.Cell] == nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "a configured Trino cell is required"}) + return + } + selector, ok := a.orgs.(trinoCellSelector) + if !ok { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "Trino cell selection is unavailable"}) + return + } + if err := selector.SelectTrinoCell(c.Param("id"), a.fleet[req.Cell].cell.storedID()); err != nil { + switch { + case errors.Is(err, configstore.ErrTrinoCellSelectionConflict): + c.JSON(http.StatusConflict, gin.H{"error": err.Error()}) + case errors.Is(err, configstore.ErrTrinoWarehouseNotFound): + c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) + default: + c.JSON(http.StatusInternalServerError, gin.H{"error": "cannot select Trino cell"}) + } + return + } + c.JSON(http.StatusOK, gin.H{"cell": a.fleet[req.Cell].cell, "assigned": true}) +} diff --git a/controlplane/admin/trino_fleet_test.go b/controlplane/admin/trino_fleet_test.go new file mode 100644 index 00000000..053a353c --- /dev/null +++ b/controlplane/admin/trino_fleet_test.go @@ -0,0 +1,95 @@ +//go:build kubernetes + +package admin + +import ( + "net/http" + "testing" + + "github.com/posthog/duckgres/controlplane/configstore" +) + +func TestTrinoFleetSelectsAuthoritativeOwner(t *testing.T) { + legacy := &fakeTrinoCoordinator{} + current := &fakeTrinoCoordinator{queries: []TrinoQuery{{Principal: "tenant"}}} + store := &fakeTrinoOrgStore{ + orgs: []configstore.TrinoEnabledOrg{{OrgID: "tenant", DatabaseName: "tenant", CellID: "registered:cell-001"}}, + rows: map[string]*configstore.ManagedWarehouseTrino{"tenant": {OrgID: "tenant", Enabled: true, TrinoCellID: "registered:cell-001", State: configstore.ManagedWarehouseStateReady}}, + } + api := NewTrinoFleetAPI([]TrinoCell{{ID: "legacy", StoredID: "cell-001"}, {ID: "cell-001", StoredID: "registered:cell-001", ClientURL: "https://gateway.example.com"}}, []TrinoCoordinatorClient{legacy, current}, store, nil) + r := trinoTestRouter(api, RoleAdmin) + code, data := doTrinoJSON(t, r, http.MethodGet, "/api/v1/trino/queries?cell=cell-001", "") + if code != http.StatusOK || data["cell"].(map[string]any)["id"] != "cell-001" || legacy.queryCalls.Load() != 0 || current.queryCalls.Load() != 1 { + t.Fatalf("wrong coordinator: %d %#v", code, data) + } + code, data = doTrinoJSON(t, r, http.MethodGet, "/api/v1/orgs/tenant/trino?cell=legacy", "") + if code != http.StatusOK || data["cell"].(map[string]any)["id"] != "cell-001" { + t.Fatalf("org must resolve its stored owner: %d %#v", code, data) + } + status := data["status"].(map[string]any) + if status["connection"].(map[string]any)["host"] != "gateway.example.com" { + t.Fatalf("wrong connection: %#v", status) + } + for _, cell := range []string{"legacy", "cell-001"} { + code, data = doTrinoJSON(t, r, http.MethodGet, "/api/v1/trino/orgs?cell="+cell, "") + want := 0 + if cell == "cell-001" { + want = 1 + } + if code != http.StatusOK || len(data["orgs"].([]any)) != want { + t.Fatalf("org listing crossed cell boundary: %d %#v", code, data) + } + } + for _, path := range []string{"/api/v1/trino/queries", "/api/v1/trino/status", "/api/v1/trino/nodes", "/api/v1/trino/orgs", "/api/v1/trino/queries/query"} { + code, _ = doTrinoJSON(t, r, http.MethodGet, path+"?cell=unknown", "") + if code != http.StatusNotFound { + t.Errorf("%s unknown cell returned %d", path, code) + } + } + store.rows["tenant"].TrinoCellID = "unregistered" + code, data = doTrinoJSON(t, r, http.MethodGet, "/api/v1/orgs/tenant/trino", "") + if code != http.StatusConflict || data["status"] != nil { + t.Fatalf("unknown stored owner must fail closed: %d %#v", code, data) + } +} + +func TestTrinoFleetRejectsAmbiguousConfiguration(t *testing.T) { + for _, cells := range [][]TrinoCell{ + {{ID: "legacy", StoredID: "same"}, {ID: "cell-001", StoredID: "same"}}, + {{ID: "legacy", StoredID: "first"}, {ID: "legacy", StoredID: "second"}}, + {{ID: ""}, {ID: "cell-001"}}, + } { + if api := NewTrinoFleetAPI(cells, []TrinoCoordinatorClient{&fakeTrinoCoordinator{}, &fakeTrinoCoordinator{}}, &fakeTrinoOrgStore{}, nil); api != nil { + t.Fatalf("ambiguous fleet accepted: %+v", cells) + } + } +} + +type selectingTrinoStore struct { + fakeTrinoOrgStore + selected string +} + +func (s *selectingTrinoStore) SelectTrinoCell(_, cell string) error { + s.selected = cell + return nil +} + +func TestTrinoFleetSelectionIsAdminOnly(t *testing.T) { + store := &selectingTrinoStore{} + api := NewTrinoFleetAPI([]TrinoCell{{ID: "legacy", StoredID: "cell-001"}, {ID: "cell-001", StoredID: "registered:cell-001"}}, []TrinoCoordinatorClient{&fakeTrinoCoordinator{}, &fakeTrinoCoordinator{}}, store, nil) + for _, role := range []Role{RoleViewer, RoleAdmin} { + code, _ := doTrinoJSON(t, trinoTestRouter(api, role), http.MethodPut, "/api/v1/orgs/tenant/trino/cell", `{"cell":"cell-001"}`) + if role == RoleViewer && (code != http.StatusForbidden || store.selected != "") { + t.Fatalf("viewer changed assignment: %d", code) + } + if role == RoleAdmin && (code != http.StatusOK || store.selected != "registered:cell-001") { + t.Fatalf("admin assignment: %d %q", code, store.selected) + } + } + store.selected = "" + code, _ := doTrinoJSON(t, trinoTestRouter(api, RoleAdmin), http.MethodPut, "/api/v1/orgs/tenant/trino/cell", `{"cell":"unknown"}`) + if code != http.StatusBadRequest || store.selected != "" { + t.Fatalf("unknown cell selected: %d", code) + } +} diff --git a/controlplane/admin/ui/src/hooks/useApi.ts b/controlplane/admin/ui/src/hooks/useApi.ts index 3dccc07f..a34adfd5 100644 --- a/controlplane/admin/ui/src/hooks/useApi.ts +++ b/controlplane/admin/ui/src/hooks/useApi.ts @@ -778,9 +778,9 @@ export function useCancelReshard() { const NO_TRINO_CELL: TrinoCell = { id: "", coordinator_url: "" }; -export function useTrinoStatus() { +export function useTrinoStatus(cell?: string) { return useQuery({ - queryKey: ["trino", "status"], + queryKey: ["trino", "status", cell], queryFn: () => tolerate404({ cell: NO_TRINO_CELL, @@ -792,12 +792,12 @@ export function useTrinoStatus() { failed_nodes: 0, orgs_by_state: {}, total_orgs: 0, - })(api.trinoStatus()), + })(api.trinoStatus(cell)), refetchInterval: POLL.normal, }); } -export function useTrinoQueries(filters: { org?: string; state?: string; active?: boolean }) { +export function useTrinoQueries(filters: { org?: string; state?: string; active?: boolean; cell?: string }) { return useQuery({ queryKey: ["trino", "queries", filters], queryFn: () => @@ -808,23 +808,23 @@ export function useTrinoQueries(filters: { org?: string; state?: string; active? }); } -export function useTrinoNodes() { +export function useTrinoNodes(cell?: string) { return useQuery({ - queryKey: ["trino", "nodes"], + queryKey: ["trino", "nodes", cell], queryFn: () => tolerate404({ cell: NO_TRINO_CELL, available: false, nodes: [] })( - api.trinoNodes(), + api.trinoNodes(cell), ), refetchInterval: POLL.slow, }); } -export function useTrinoOrgs() { +export function useTrinoOrgs(cell?: string) { return useQuery({ - queryKey: ["trino", "orgs"], + queryKey: ["trino", "orgs", cell], queryFn: () => tolerate404({ cell: NO_TRINO_CELL, available: false, orgs: [] })( - api.trinoOrgs(), + api.trinoOrgs(cell), ), refetchInterval: POLL.slow, }); @@ -842,10 +842,26 @@ export function useOrgTrino(org: string) { }); } -export function useKillTrinoQuery() { +export function useTrinoCells() { + return useQuery({ + queryKey: ["trino", "cells"], + queryFn: () => tolerate404<{ cells: TrinoCell[] }>({ cells: [] })(api.trinoCells()), + refetchInterval: POLL.slow, + }); +} + +export function useSelectTrinoCell() { + const qc = useQueryClient(); + return useMutation({ + mutationFn: ({ org, cell }: { org: string; cell: string }) => api.selectTrinoCell(org, cell), + onSuccess: () => qc.invalidateQueries({ queryKey: ["trino"] }), + }); +} + +export function useKillTrinoQuery(cell?: string) { const qc = useQueryClient(); return useMutation({ - mutationFn: ({ id, reason }: { id: string; reason: string }) => api.killTrinoQuery(id, reason), + mutationFn: ({ id, reason }: { id: string; reason: string }) => api.killTrinoQuery(id, reason, cell), // A kill changes what the live list should show, so drop the whole // trino cache rather than waiting out the poll interval. onSuccess: () => qc.invalidateQueries({ queryKey: ["trino"] }), diff --git a/controlplane/admin/ui/src/lib/api.ts b/controlplane/admin/ui/src/lib/api.ts index 48ce859f..e17a968f 100644 --- a/controlplane/admin/ui/src/lib/api.ts +++ b/controlplane/admin/ui/src/lib/api.ts @@ -47,6 +47,7 @@ import type { SessionStatus, StartReshardBody, TrinoKillResult, + TrinoCell, TrinoNodesResponse, TrinoOrgDetail, TrinoOrgsResponse, @@ -294,21 +295,25 @@ export const api = { // trino cell (absent entirely on a deployment with no cell — these 404, // which the *Optional hooks turn into an empty state) - trinoStatus: () => get("/trino/status"), + trinoStatus: (cell?: string) => get("/trino/status", { cell }), // active=1 keeps the live view to the states an operator can still act on; // the server also serves recently-finished queries without it. - trinoQueries: (filters: { org?: string; state?: string; active?: boolean }) => + trinoQueries: (filters: { org?: string; state?: string; active?: boolean; cell?: string }) => get("/trino/queries", { org: filters.org, state: filters.state, active: filters.active ? 1 : undefined, + cell: filters.cell, }), - trinoQuery: (id: string) => get(`/trino/queries/${enc(id)}`), + trinoQuery: (id: string, cell?: string) => get(`/trino/queries/${enc(id)}`, { cell }), // The reason reaches the TENANT as their query's failure message, so they // learn why it died rather than seeing an unexplained cancellation. - killTrinoQuery: (id: string, reason: string) => - post(`/trino/queries/${enc(id)}/kill`, { reason }), - trinoNodes: () => get("/trino/nodes"), - trinoOrgs: () => get("/trino/orgs"), + killTrinoQuery: (id: string, reason: string, cell?: string) => + post(`/trino/queries/${enc(id)}/kill${cell ? `?cell=${enc(cell)}` : ""}`, { reason }), + trinoNodes: (cell?: string) => get("/trino/nodes", { cell }), + trinoOrgs: (cell?: string) => get("/trino/orgs", { cell }), orgTrino: (org: string) => get(`/orgs/${enc(org)}/trino`), + trinoCells: () => get<{ cells: TrinoCell[] }>("/trino/cells"), + selectTrinoCell: (org: string, cell: string) => + put<{ cell: TrinoCell; assigned: boolean }>(`/orgs/${enc(org)}/trino/cell`, { cell }), }; diff --git a/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx b/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx index bb40191e..95e2acfb 100644 --- a/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx +++ b/controlplane/admin/ui/src/pages/OrgTrinoCard.test.tsx @@ -1,10 +1,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { render, screen } from "@testing-library/react"; +import { fireEvent, render, screen } from "@testing-library/react"; import { MemoryRouter } from "react-router-dom"; import type { TrinoOrgDetail, TrinoOrgStatus } from "@/types/api"; -const hooks = vi.hoisted(() => ({ useOrgTrino: vi.fn() })); +const hooks = vi.hoisted(() => ({ useOrgTrino: vi.fn(), useTrinoCells: vi.fn(), useSelectTrinoCell: vi.fn() })); vi.mock("@/hooks/useApi", () => hooks); +const identity = vi.hoisted(() => ({ useIdentity: vi.fn() })); +vi.mock("@/components/IdentityProvider", () => identity); import { OrgTrinoCard } from "./OrgTrinoCard"; @@ -44,7 +46,54 @@ function renderCard() { } describe("OrgTrinoCard", () => { - beforeEach(() => vi.clearAllMocks()); + beforeEach(() => { + vi.clearAllMocks(); + hooks.useTrinoCells.mockReturnValue(ok({ cells: [] })); + hooks.useSelectTrinoCell.mockReturnValue({ mutate: vi.fn(), isPending: false }); + identity.useIdentity.mockReturnValue({ isAdmin: true }); + }); + + it("lets an admin select an initial cell without enabling Trino", () => { + const mutate = vi.fn(); + hooks.useOrgTrino.mockReturnValue(ok(detail({ enabled: false, assigned: false, status: undefined }))); + hooks.useTrinoCells.mockReturnValue(ok({ cells: [{ id: "legacy" }, { id: "cell-001" }] })); + hooks.useSelectTrinoCell.mockReturnValue({ mutate, isPending: false }); + renderCard(); + fireEvent.change(screen.getByLabelText("Initial Trino cell"), { target: { value: "cell-001" } }); + fireEvent.click(screen.getByRole("button", { name: "Select cell" })); + expect(mutate).toHaveBeenCalledWith({ org: "org-a", cell: "cell-001" }); + expect(screen.getByText(/does not enable Trino/)).toBeInTheDocument(); + }); + + it("keeps a disabled warehouse's assignment immutable", () => { + hooks.useOrgTrino.mockReturnValue(ok(detail({ enabled: false, assigned: true, status: undefined }))); + renderCard(); + expect(screen.getByText(/Assigned cell: legacy/)).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Select cell" })).not.toBeInTheDocument(); + }); + + it("never offers selection to viewers", () => { + identity.useIdentity.mockReturnValue({ isAdmin: false }); + hooks.useOrgTrino.mockReturnValue(ok(detail({ enabled: false, assigned: false, status: undefined }))); + hooks.useTrinoCells.mockReturnValue(ok({ cells: [{ id: "legacy" }, { id: "cell-001" }] })); + renderCard(); + expect(screen.queryByRole("button", { name: "Select cell" })).not.toBeInTheDocument(); + }); + + it("shows assignment read failures rather than offering a legacy fallback", () => { + hooks.useOrgTrino.mockReturnValue({ isError: true, error: new Error("Unknown assignment") }); + renderCard(); + expect(screen.getByText(/Unknown assignment/)).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Select cell" })).not.toBeInTheDocument(); + }); + + it("shows a rejected selection without hiding the error", () => { + hooks.useOrgTrino.mockReturnValue(ok(detail({ enabled: false, assigned: false, status: undefined }))); + hooks.useTrinoCells.mockReturnValue(ok({ cells: [{ id: "legacy" }] })); + hooks.useSelectTrinoCell.mockReturnValue({ mutate: vi.fn(), error: new Error("Assignment is immutable") }); + renderCard(); + expect(screen.getByRole("alert")).toHaveTextContent("Assignment is immutable"); + }); it("renders nothing for an org that is not Trino-enabled", () => { // Most orgs have no Trino row, and a control plane with no cell 404s diff --git a/controlplane/admin/ui/src/pages/OrgTrinoCard.tsx b/controlplane/admin/ui/src/pages/OrgTrinoCard.tsx index 3c3e60b3..2950ede6 100644 --- a/controlplane/admin/ui/src/pages/OrgTrinoCard.tsx +++ b/controlplane/admin/ui/src/pages/OrgTrinoCard.tsx @@ -1,3 +1,4 @@ +import { useState } from "react"; import { Link } from "react-router-dom"; import { AlertTriangle, Sparkles } from "lucide-react"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; @@ -5,7 +6,9 @@ import { Badge } from "@/components/ui/badge"; import { StateBadge } from "@/components/StateBadge"; import { LoadingState } from "@/components/states"; import { CopyButton } from "@/components/CopyButton"; -import { useOrgTrino } from "@/hooks/useApi"; +import { useOrgTrino, useTrinoCells, useSelectTrinoCell } from "@/hooks/useApi"; +import { useIdentity } from "@/components/IdentityProvider"; +import { Button } from "@/components/ui/button"; import { fmtInt, fmtTime } from "@/lib/format"; function Field({ label, value, mono }: { label: string; value: React.ReactNode; mono?: boolean }) { @@ -41,7 +44,13 @@ export function OrgTrinoCard({ orgId }: { orgId: string }) { ); } - if (!trino.data?.enabled || !trino.data.status) { + if (trino.isError) { + return Cannot read Trino assignment. {trino.error?.message}; + } + if (!trino.data?.enabled) { + return ; + } + if (!trino.data.status) { return null; } @@ -106,7 +115,7 @@ export function OrgTrinoCard({ orgId }: { orgId: string }) { View this cell's live queries → @@ -115,3 +124,28 @@ export function OrgTrinoCard({ orgId }: { orgId: string }) { ); } + +function InitialCellSelection({ orgId, assigned, cell }: { orgId: string; assigned: boolean; cell: string }) { + const { isAdmin } = useIdentity(); + const cells = useTrinoCells(); + const selection = useSelectTrinoCell(); + const [chosen, setChosen] = useState(""); + if (assigned) { + return Assigned cell: {cell}. Trino is disabled. Existing assignments cannot be changed here.; + } + if (!isAdmin || !cells.data?.cells.length) return null; + return ( + + Trino cell + +

Select the initial cell before enabling Trino. This does not enable Trino. The assignment cannot be changed here afterward.

+ + + {selection.error &&

{selection.error.message}

} + + + ); +} diff --git a/controlplane/admin/ui/src/pages/TrinoCluster.tsx b/controlplane/admin/ui/src/pages/TrinoCluster.tsx index 1e37f373..169d15bb 100644 --- a/controlplane/admin/ui/src/pages/TrinoCluster.tsx +++ b/controlplane/admin/ui/src/pages/TrinoCluster.tsx @@ -1,5 +1,5 @@ import { useMemo } from "react"; -import { Link } from "react-router-dom"; +import { Link, useSearchParams } from "react-router-dom"; import { AlertTriangle, Boxes, Cpu, Network, ServerCog } from "lucide-react"; import { PageBody, PageHeader } from "@/components/AppShell"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; @@ -20,9 +20,11 @@ import { } from "@/lib/trino"; export function TrinoCluster() { - const status = useTrinoStatus(); - const nodes = useTrinoNodes(); - const orgs = useTrinoOrgs(); + const [params] = useSearchParams(); + const cell = params.get("cell") ?? undefined; + const status = useTrinoStatus(cell); + const nodes = useTrinoNodes(cell); + const orgs = useTrinoOrgs(cell); const orgLabels = useOrgLabels(); // The nodes payload names its own inventory; status carries it too for diff --git a/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx b/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx index 09fe4069..9aed9417 100644 --- a/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx +++ b/controlplane/admin/ui/src/pages/TrinoQueries.test.tsx @@ -67,9 +67,9 @@ function status(over: Partial = {}): TrinoStatus { }; } -function renderPage() { +function renderPage(path = "/trino/queries") { return render( - + @@ -113,6 +113,12 @@ describe("TrinoQueries page", () => { expect(statValue("Blocked")).toBe("1"); }); + it("keeps a selected cell in both query and status requests", () => { + renderPage("/trino/queries?cell=cell-001"); + expect(hooks.useTrinoQueries).toHaveBeenCalledWith({ active: true, cell: "cell-001" }); + expect(hooks.useTrinoStatus).toHaveBeenCalledWith("cell-001"); + }); + it("flags a blocked query rather than calling it merely slow", () => { hooks.useTrinoQueries.mockReturnValue( ok({ diff --git a/controlplane/admin/ui/src/pages/TrinoQueries.tsx b/controlplane/admin/ui/src/pages/TrinoQueries.tsx index 110054ac..6eb10cb4 100644 --- a/controlplane/admin/ui/src/pages/TrinoQueries.tsx +++ b/controlplane/admin/ui/src/pages/TrinoQueries.tsx @@ -1,4 +1,5 @@ import { useMemo, useState } from "react"; +import { useSearchParams } from "react-router-dom"; import { AlertTriangle, Ban, Database, Gauge, Hourglass, Timer } from "lucide-react"; import { PageBody, PageHeader } from "@/components/AppShell"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; @@ -59,7 +60,8 @@ function KillDialog({ onClose: () => void; }) { const [reason, setReason] = useState(""); - const kill = useKillTrinoQuery(); + const [params] = useSearchParams(); + const kill = useKillTrinoQuery(params.get("cell") ?? undefined); return ( !open && onClose()}> @@ -115,8 +117,10 @@ export function TrinoQueries() { const [orgFilter, setOrgFilter] = useState(""); const [killing, setKilling] = useState(null); - const status = useTrinoStatus(); - const queries = useTrinoQueries({ active: activeOnly }); + const [params] = useSearchParams(); + const cell = params.get("cell") ?? undefined; + const status = useTrinoStatus(cell); + const queries = useTrinoQueries({ active: activeOnly, ...(cell ? { cell } : {}) }); const orgLabels = useOrgLabels(); const rows = useMemo(() => { diff --git a/controlplane/admin/ui/src/types/api.ts b/controlplane/admin/ui/src/types/api.ts index c186a561..d1d8f3de 100644 --- a/controlplane/admin/ui/src/types/api.ts +++ b/controlplane/admin/ui/src/types/api.ts @@ -902,6 +902,7 @@ export interface TrinoOrgsResponse { export interface TrinoOrgDetail { cell: TrinoCell; enabled: boolean; + assigned?: boolean; available?: boolean; status?: TrinoOrgStatus; } diff --git a/controlplane/configstore/trino.go b/controlplane/configstore/trino.go index 94213506..70ce3faa 100644 --- a/controlplane/configstore/trino.go +++ b/controlplane/configstore/trino.go @@ -131,11 +131,17 @@ func (cs *ConfigStore) UpdateTrinoState(orgID string, upd TrinoStateUpdate) erro // else (or disabled) between the list and the write. The next tick re-reads // and skips the org because its cell no longer matches. func (cs *ConfigStore) AssignTrinoCell(orgID, cellID string) error { + _, err := cs.ClaimTrinoCell(orgID, cellID) + return err +} + +// ClaimTrinoCell reports whether this call acquired the previously unassigned row. +func (cs *ConfigStore) ClaimTrinoCell(orgID, cellID string) (bool, error) { if orgID == "" { - return errors.New("AssignTrinoCell: orgID is required") + return false, errors.New("ClaimTrinoCell: orgID is required") } if cellID == "" { - return errors.New("AssignTrinoCell: cellID is required") + return false, errors.New("ClaimTrinoCell: cellID is required") } result := cs.db.Model(&ManagedWarehouseTrino{}). Where("org_id = ? AND enabled = ? AND (trino_cell_id IS NULL OR trino_cell_id = ?)", orgID, true, ""). @@ -144,9 +150,45 @@ func (cs *ConfigStore) AssignTrinoCell(orgID, cellID string) error { "updated_at": time.Now().UTC(), }) if result.Error != nil { - return fmt.Errorf("assign trino cell for %q: %w", orgID, result.Error) + return false, fmt.Errorf("assign trino cell for %q: %w", orgID, result.Error) } - return nil + return result.RowsAffected == 1, nil +} + +var ( + ErrTrinoCellSelectionConflict = errors.New("trino cell selection is only available before initial enablement; existing assignments cannot change") + ErrTrinoWarehouseNotFound = errors.New("managed warehouse not found") +) + +// SelectTrinoCell assigns an initial cell without enabling Trino or moving an existing tenant. +func (cs *ConfigStore) SelectTrinoCell(orgID, cellID string) error { + if orgID == "" || cellID == "" { + return errors.New("SelectTrinoCell: orgID and cellID are required") + } + return cs.db.Transaction(func(tx *gorm.DB) error { + var warehouse ManagedWarehouse + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).First(&warehouse, "org_id = ?", orgID).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return ErrTrinoWarehouseNotFound + } + return err + } + row := ManagedWarehouseTrino{OrgID: orgID, State: ManagedWarehouseStatePending} + if err := tx.Clauses(clause.OnConflict{DoNothing: true}).Create(&row).Error; err != nil { + return err + } + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).First(&row, "org_id = ?", orgID).Error; err != nil { + return err + } + if row.TrinoCellID == cellID { + return nil + } + if row.TrinoCellID != "" || row.Enabled || row.State != ManagedWarehouseStatePending || row.ReadyAt != nil || row.FailedAt != nil { + return ErrTrinoCellSelectionConflict + } + return tx.Model(&ManagedWarehouseTrino{}).Where("org_id = ?", orgID). + Updates(map[string]any{"trino_cell_id": cellID, "updated_at": time.Now().UTC()}).Error + }) } // DisableTrino marks the org as no longer Trino-enabled. The row is diff --git a/justfile b/justfile index ba08dc73..946caa56 100644 --- a/justfile +++ b/justfile @@ -333,6 +333,11 @@ test-configstore-integration: test-controlplane-k8s: go test -v -count=1 -tags kubernetes . ./controlplane ./controlplane/admin ./controlplane/provisioner +# Test Trino cell selection and the admin API. +[group('test')] +test-trino-admin: + go test -v -count=1 -tags kubernetes -run Trino ./controlplane/admin ./tests/configstore + # Print the test impact plan for the current branch [group('test')] test-impact-plan base="origin/main" head="HEAD": diff --git a/tests/configstore/trino_selection_postgres_test.go b/tests/configstore/trino_selection_postgres_test.go new file mode 100644 index 00000000..2452b0c3 --- /dev/null +++ b/tests/configstore/trino_selection_postgres_test.go @@ -0,0 +1,114 @@ +//go:build linux || darwin + +package configstore_test + +import ( + "errors" + "fmt" + "sync" + "testing" + + "github.com/posthog/duckgres/controlplane/configstore" +) + +func TestTrinoInitialSelectionPreservesOwnershipPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + seedTrinoOrg(t, store, "tenant") + if err := store.DB().Create(&configstore.ManagedWarehouse{OrgID: "tenant", DucklingName: "tenant"}).Error; err != nil { + t.Fatal(err) + } + if err := store.SelectTrinoCell("tenant", "registered:cell-001"); err != nil { + t.Fatal(err) + } + if row := trinoRow(t, store, "tenant"); row.Enabled || row.TrinoCellID != "registered:cell-001" { + t.Fatalf("selection enabled or misassigned tenant: %+v", row) + } + if err := store.EnableTrino("tenant", configstore.TrinoSettings{}); err != nil { + t.Fatal(err) + } + if claimed, err := store.ClaimTrinoCell("tenant", "cell-001"); err != nil || claimed { + t.Fatalf("legacy stole selected tenant: %v %v", claimed, err) + } + if err := store.DisableTrino("tenant"); err != nil { + t.Fatal(err) + } + if err := store.SelectTrinoCell("tenant", "cell-001"); !errors.Is(err, configstore.ErrTrinoCellSelectionConflict) { + t.Fatalf("disabled ownership must be immutable: %v", err) + } + if err := store.SelectTrinoCell("tenant", "registered:cell-001"); err != nil { + t.Fatalf("same selection must be idempotent: %v", err) + } + if err := store.EnableTrino("tenant", configstore.TrinoSettings{}); err != nil { + t.Fatal(err) + } + if row := trinoRow(t, store, "tenant"); !row.Enabled || row.TrinoCellID != "registered:cell-001" { + t.Fatalf("reenable lost selection: %+v", row) + } +} + +func TestTrinoSelectionRacesFirstLegacyClaimPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + for i := 0; i < 20; i++ { + org := fmt.Sprintf("tenant-%d", i) + seedTrinoOrg(t, store, org) + if err := store.DB().Create(&configstore.ManagedWarehouse{OrgID: org, DucklingName: org}).Error; err != nil { + t.Fatal(err) + } + start := make(chan struct{}) + var wg sync.WaitGroup + var selectionErr, claimErr error + var claimed bool + wg.Add(2) + go func() { + defer wg.Done() + <-start + selectionErr = store.SelectTrinoCell(org, "registered:cell-001") + }() + go func() { + defer wg.Done() + <-start + if claimErr = store.EnableTrino(org, configstore.TrinoSettings{}); claimErr == nil { + claimed, claimErr = store.ClaimTrinoCell(org, "cell-001") + } + }() + close(start) + wg.Wait() + if claimErr != nil || (selectionErr != nil && !errors.Is(selectionErr, configstore.ErrTrinoCellSelectionConflict)) { + t.Fatalf("race failed: claim=%v selection=%v", claimErr, selectionErr) + } + row := trinoRow(t, store, org) + if selectionErr == nil { + if claimed || row.TrinoCellID != "registered:cell-001" { + t.Fatalf("successful selection lost ownership: claim=%v row=%+v", claimed, row) + } + } else if !claimed || row.TrinoCellID != "cell-001" { + t.Fatalf("legacy winner not authoritative: claim=%v row=%+v", claimed, row) + } + } +} + +func TestTrinoSelectionRejectsUnknownWarehouseAndEnabledRowPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + if err := store.SelectTrinoCell("missing", "cell-001"); !errors.Is(err, configstore.ErrTrinoWarehouseNotFound) { + t.Fatalf("unknown warehouse: %v", err) + } + seedTrinoOrg(t, store, "tenant") + if err := store.SelectTrinoCell("tenant", "cell-001"); !errors.Is(err, configstore.ErrTrinoWarehouseNotFound) { + t.Fatalf("org without warehouse: %v", err) + } + if err := store.DB().Create(&configstore.ManagedWarehouse{OrgID: "tenant", DucklingName: "tenant"}).Error; err != nil { + t.Fatal(err) + } + if err := store.EnableTrino("tenant", configstore.TrinoSettings{}); err != nil { + t.Fatal(err) + } + if err := store.SelectTrinoCell("tenant", "registered:cell-001"); !errors.Is(err, configstore.ErrTrinoCellSelectionConflict) { + t.Fatalf("selection after enable must fail: %v", err) + } + if claimed, err := store.ClaimTrinoCell("tenant", "cell-001"); err != nil || !claimed { + t.Fatalf("first claim: %v %v", claimed, err) + } + if claimed, err := store.ClaimTrinoCell("tenant", "registered:cell-001"); err != nil || claimed { + t.Fatalf("second claim: %v %v", claimed, err) + } +} From 4f3280936d58100dcfd97fc21fe6c29aeda8e8b9 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 14:27:59 +0200 Subject: [PATCH 2/8] feat: reconcile registered Trino cells and running backends --- CLAUDE.md | 29 ++- README.md | 3 + controlplane/multitenant.go | 41 ++-- controlplane/provisioner/controller.go | 4 +- .../provisioner/trino_cell_backends_test.go | 203 ++++++++++++++++++ controlplane/provisioner/trino_provisioner.go | 151 +++++++++---- .../provisioner/trino_provisioner_test.go | 6 +- controlplane/trino_fleet.go | 75 +++++++ controlplane/trino_fleet_test.go | 106 +++++++++ controlplane/trino_inputs.go | 108 ++++++---- controlplane/trino_registry.go | 180 ++++++++++++++++ controlplane/trino_registry_test.go | 125 +++++++++++ docs/trino-cells.md | 128 +++++++++++ justfile | 4 + 14 files changed, 1042 insertions(+), 121 deletions(-) create mode 100644 controlplane/provisioner/trino_cell_backends_test.go create mode 100644 controlplane/trino_fleet.go create mode 100644 controlplane/trino_fleet_test.go create mode 100644 controlplane/trino_registry.go create mode 100644 controlplane/trino_registry_test.go create mode 100644 docs/trino-cells.md diff --git a/CLAUDE.md b/CLAUDE.md index b0ecb7c3..4cab25d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1551,8 +1551,10 @@ limits on a shared multi-tenant Trino cluster called a **cell**. The control plane is the only writer of that state: `provisioner/trino_provisioner.go` projects it every controller tick from `duckgres_managed_warehouse_trino` + the org's warehouse row + its Duckling CR. Enablement is env-inferred — -`DUCKGRES_TRINO_COORDINATOR_URL` set means on (`controlplane/trino_inputs.go`); -unset means the branch never wires and nothing changes. **Trino is binary: if +`DUCKGRES_TRINO_COORDINATOR_URL` identifies legacy (`controlplane/trino_inputs.go`). +The optional `DUCKGRES_TRINO_CELLS_FILE` adds namespace-isolated logical cells +with blue/green backends; it still requires legacy configuration. With neither +setting, the branch never wires and nothing changes. **Trino is binary: if you asked for it, a wiring failure is fatal at startup**, because silently skipping leaves the cell's OPA sidecar serving a last-good bundle while password/tenant/catalog changes never propagate. @@ -1674,12 +1676,22 @@ password/tenant/catalog changes never propagate. breaks every reconcile tick's own DDL. - **Cells, minimally**: `trino_cell_id` on the row names the owning cell (`DUCKGRES_TRINO_CELL_ID`, default `configstore.DefaultTrinoCellID`). A - provisioner claims unassigned orgs (`AssignTrinoCell`, conditional in SQL so + provisioner claims unassigned orgs (`ClaimTrinoCell`, conditional in SQL so no cell can steal another's tenant), reconciles its own, and ignores the - rest — including writing NO state for them. There is exactly ONE cell today - and deliberately no assignment policy, capacity model, rebalancer or cell - drain; `resolveTrinoCell` becoming `resolveTrinoCells` is the whole shape of - adding a second. + rest — including writing NO state for them. Claims return an authoritative + boolean: a lost claim must never project the losing cell's tenant. Only + legacy claims unassigned warehouses. Registered logical IDs have the reserved + storage prefix `registered:`; the old stored `cell-001` remains legacy. + Admin-only initial selection runs before first enablement and refuses changes + to any already owned warehouse, including a disabled one. No maintenance move, + capacity model, rebalancer, drain, or Gateway routing controller is included. + See [docs/trino-cells.md](docs/trino-cells.md) for configuration and recovery. +- **Blue/green projections share one logical cell's namespace**, but internal + communication Secrets remain distinct, chart-owned read-only references. + Stopped backends do not receive catalog calls or observer polling. Every + running backend must reconcile its independent catalog set before the org is + ready. Cell failures do not block sibling-cell projections. Configuration is + startup-loaded and must agree across control-plane replicas. - **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. @@ -1690,7 +1702,8 @@ password/tenant/catalog changes never propagate. - **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 - serves under a placeholder. + serves under a placeholder. Registered cells use `/bundles/trino/`; + each cell's token can read only its own bundle. Legacy retains the old path. - Touching any of this → update `provisioner/trino_provisioner_test.go`, `provisioner/trino_cluster_secrets_test.go`, `provisioner/opa/*_test.go`, `provisioning/api_test.go`, `tests/configstore/trino_postgres_test.go` + diff --git a/README.md b/README.md index 623e9d2b..a9ad2b2b 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,8 @@ # Duckgres +Trino operators: see [cell registration and placement](docs/trino-cells.md) for +the optional registry, unchanged legacy defaults, and migration limitations. +

Duckgres Mascot

diff --git a/controlplane/multitenant.go b/controlplane/multitenant.go index 3ef9b2d0..a5b76508 100644 --- a/controlplane/multitenant.go +++ b/controlplane/multitenant.go @@ -21,7 +21,6 @@ import ( "github.com/posthog/duckgres/controlplane/admin" "github.com/posthog/duckgres/controlplane/configstore" "github.com/posthog/duckgres/controlplane/provisioner" - "github.com/posthog/duckgres/controlplane/provisioner/opa" "github.com/posthog/duckgres/controlplane/provisioning" "github.com/posthog/duckgres/server" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -508,13 +507,7 @@ func SetupMultiTenant( } // Start provisioning controller (best-effort — K8s API may not be available locally) - var trinoBundleHandler *opa.Handler - // trinoConsole carries what the admin console needs from the Trino - // branch (cell identity + an observer-credentialed coordinator client). - // Nil unless the branch wires, so a deployment without a cell simply - // has no /trino routes. Held rather than turned into a TrinoAPI here - // because that also needs the audit store, which is built later. - var trinoConsole *trinoConsoleWiring + var trinoCells trinoFleet provCtrl, err := provisioner.NewController(store, 10*time.Second) if err != nil { // Without the controller, the Trino reconcile loop cannot run. @@ -549,7 +542,7 @@ func SetupMultiTenant( // Same Duckling CR read the worker activation path uses; nil // when the Duckling client couldn't be built, which // buildTrinoWiring rejects rather than half-wiring. - trinoWire, twErr := buildTrinoWiring(store, kc, resolveDucklingStatus) + trinoWire, twErr := buildTrinoFleetWiring(store, kc, resolveDucklingStatus) if twErr != nil { return nil, nil, nil, nil, nil, nil, fmt.Errorf("trino provisioner wiring failed: %w", twErr) } @@ -559,10 +552,11 @@ func SetupMultiTenant( // that. So a nil here is a wiring bug. return nil, nil, nil, nil, nil, nil, fmt.Errorf("trino provisioner enabled but buildTrinoWiring returned no wiring; this should be unreachable") } - provCtrl.WithTrinoProvisioner(trinoWire.Provisioner) - trinoBundleHandler = trinoWire.BundleHandler - trinoConsole = trinoWire.Console - slog.Info("Trino provisioner enabled.", "cell", trinoWire.Cell.ID, "coordinator", trinoWire.Cell.CoordinatorURL) + provCtrl.WithTrinoProvisioner(trinoWire) + trinoCells = trinoWire + for _, wire := range trinoWire { + slog.Info("Trino provisioner enabled.", "cell", wire.Cell.consoleCell().ID, "coordinator", wire.Cell.CoordinatorURL) + } } // SIGTERM stops the reconcile loop immediately, rather than letting a // replaced replica ride out the drain. @@ -787,26 +781,25 @@ func SetupMultiTenant( Audit: auditStore, Metrics: metricsProxy, ClusterClient: clusterClient, - Trino: newTrinoAdminAPI(trinoConsole, store, auditStore), + Trino: trinoCells.adminAPI(store, auditStore), }) - if janitorLeader != nil && trinoConsole != nil { + if janitorLeader != nil { // The coordinator owns the authoritative runtime view of Trino query // usage. Keep its collector under the existing leader lease so one CP // emits each terminal query, independent of admin-console traffic. - janitorLeader.AttachLeaderLoop(newTrinoUsageCollector( - trinoConsole.Observer, - store, - store.OrgUsageTeamID, - ).Run) + for _, wire := range trinoCells { + for _, observer := range wire.Observers { + janitorLeader.AttachLeaderLoop(newTrinoUsageCollector(observer, store, store.OrgUsageTeamID).Run) + } + } } // Trino OPA bundle endpoint. Mounted OUTSIDE the /api/v1 admin group on // purpose — it does its own bearer-token auth (the bundle exposes the // customer roster; a separate shared secret between provisioner and the - // OPA sidecar). When the Trino provisioner is disabled (the default), - // trinoBundleHandler is nil and the route isn't registered. - if trinoBundleHandler != nil { - engine.Any("/bundles/trino", gin.WrapH(trinoBundleHandler)) + // OPA sidecar). The fleet is empty when provisioning is disabled. + for _, wire := range trinoCells { + engine.Any(wire.bundlePath(), gin.WrapH(wire.BundleHandler)) } // Live Duckling drift finder. Reuse the in-cluster Duckling client built diff --git a/controlplane/provisioner/controller.go b/controlplane/provisioner/controller.go index 701fda94..8cec97fa 100644 --- a/controlplane/provisioner/controller.go +++ b/controlplane/provisioner/controller.go @@ -80,7 +80,7 @@ type Controller struct { // deployments without a Trino cell, or in tests that don't exercise // the Trino path), the Trino reconcile step is skipped silently. // See WithTrinoProvisioner. - trinoProvisioner *TrinoProvisioner + trinoProvisioner interface{ Reconcile(context.Context) error } // cnpgShardFieldUnsupported latches when a cnpg-shard backfill read-back // shows the API server pruned spec.metadataStore.cnpgShard — i.e. the @@ -144,7 +144,7 @@ func (c *Controller) WithBucketSuffix(suffix string) *Controller { // // Skipped entirely if p is nil so deployments without a Trino cell don't // need to know about it. -func (c *Controller) WithTrinoProvisioner(p *TrinoProvisioner) *Controller { +func (c *Controller) WithTrinoProvisioner(p interface{ Reconcile(context.Context) error }) *Controller { if p == nil { panic("WithTrinoProvisioner: provisioner is nil; call NewTrinoProvisioner first") } diff --git a/controlplane/provisioner/trino_cell_backends_test.go b/controlplane/provisioner/trino_cell_backends_test.go new file mode 100644 index 00000000..0203be5d --- /dev/null +++ b/controlplane/provisioner/trino_cell_backends_test.go @@ -0,0 +1,203 @@ +//go:build kubernetes + +package provisioner + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/posthog/duckgres/controlplane/configstore" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestTrinoCellsKeepTenantProjectionsIsolatedAndHydrateStartedGreen(t *testing.T) { + orgs := []configstore.TrinoEnabledOrg{ + {OrgID: "tenant-a", DatabaseName: "tenant-a", CellID: testCellID, RootPasswordHash: "hash-a"}, + {OrgID: "tenant-b", DatabaseName: "tenant-b", CellID: "registered:cell-test", RootPasswordHash: "hash-b"}, + } + warehouses := map[string]*configstore.ManagedWarehouse{"tenant-a": readyWarehouse("tenant-a"), "tenant-b": readyWarehouse("tenant-b")} + legacy := newTestTrinoProvisioner(t, orgs, warehouses) + cell := newTestTrinoProvisioner(t, orgs, warehouses) + cell.provisioner.namespace = "trino-second" + cell.provisioner.cellID = "registered:cell-test" + cell.provisioner.explicitAssignmentOnly = true + cell.provisioner.store = legacy.store + cell.provisioner.kubernetes = legacy.kube + for i, h := range []*testProvisionerHarness{legacy, cell} { + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + own, other := orgs[i], orgs[1-i] + auth, err := legacy.kube.CoreV1().Secrets(h.provisioner.namespace).Get(context.Background(), TrinoAuthSecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + passwords := string(auth.Data[TrinoAuthSecretKeyPasswordDB]) + if !strings.Contains(passwords, own.DatabaseName+":") || strings.Contains(passwords, other.DatabaseName+":") { + t.Fatal("cell authentication contains the wrong tenant") + } + tenantSecret, err := legacy.kube.CoreV1().Secrets(h.provisioner.namespace).Get(context.Background(), TrinoTenantSecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if len(tenantSecret.Data) != 1 || len(tenantSecret.Data[own.OrgID]) == 0 { + t.Fatal("metadata credentials crossed cells") + } + if h.builder.last[TrinoGroupName(own.DatabaseName)] == nil || h.builder.last[TrinoGroupName(other.DatabaseName)] != nil { + t.Fatal("OPA authorization crossed cells") + } + if len(h.catalog.created) != 1 || h.catalog.created[TrinoCatalogName(own.DatabaseName)] == nil { + t.Fatal("catalog ownership crossed cells") + } + } + green := &fakeCatalogClient{createErr: trinoSecretMountLagError("tenant-b")} + cell.provisioner.additionalCatalogs = []TrinoCatalogClient{green} + if err := cell.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if state, _ := legacy.store.lastState("tenant-b"); state.State != configstore.ManagedWarehouseStateProvisioning { + t.Fatal("blue success concealed starting green's pending catalog") + } + green.createErr = nil + if err := cell.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if state, _ := legacy.store.lastState("tenant-b"); state.State != configstore.ManagedWarehouseStateReady { + t.Fatal("green did not hydrate to ready") + } + if state, _ := legacy.store.lastState("tenant-a"); state.State != configstore.ManagedWarehouseStateReady { + t.Fatal("starting green changed legacy readiness") + } +} + +type blockedCatalog struct{ fakeCatalogClient } + +func (c *blockedCatalog) ListCatalogs(ctx context.Context) ([]string, error) { + <-ctx.Done() + return nil, ctx.Err() +} + +func TestTrinoBackendTimeoutDoesNotStarveSibling(t *testing.T) { + org := configstore.TrinoEnabledOrg{OrgID: "tenant", DatabaseName: "tenant", CellID: testCellID, RootPasswordHash: "hash"} + h := newTestTrinoProvisioner(t, []configstore.TrinoEnabledOrg{org}, map[string]*configstore.ManagedWarehouse{"tenant": readyWarehouse("tenant")}) + h.provisioner.catalog = &blockedCatalog{} + h.provisioner.catalogTimeout = time.Millisecond + h.provisioner.additionalCatalogs = []TrinoCatalogClient{h.catalog} + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + if err := h.provisioner.Reconcile(ctx); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("wanted backend timeout, got %v", err) + } + if len(h.catalog.created) != 1 { + t.Fatal("timed-out backend starved healthy sibling") + } + if state, _ := h.store.lastState("tenant"); state.State != configstore.ManagedWarehouseStateFailed { + t.Fatal("healthy sibling hid failed backend") + } +} + +func TestTrinoRegisteredCellNeverClaimsUnassignedTenants(t *testing.T) { + org := configstore.TrinoEnabledOrg{OrgID: "tenant", DatabaseName: "tenant", RootPasswordHash: "hash"} + h := newTestTrinoProvisioner(t, []configstore.TrinoEnabledOrg{org}, map[string]*configstore.ManagedWarehouse{"tenant": readyWarehouse("tenant")}) + h.provisioner.explicitAssignmentOnly = true + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(h.store.claimLog) != 0 || len(h.catalog.created) != 0 { + t.Fatal("registered cell stole default placement") + } +} + +type lostClaimStore struct{ *fakeTrinoStore } + +func (s lostClaimStore) AssignTrinoCell(string, string) error { return nil } +func (s lostClaimStore) ClaimTrinoCell(string, string) (bool, error) { return false, nil } + +func TestTrinoLostClaimCannotProjectTenant(t *testing.T) { + org := configstore.TrinoEnabledOrg{OrgID: "tenant", DatabaseName: "tenant", RootPasswordHash: "hash"} + h := newTestTrinoProvisioner(t, []configstore.TrinoEnabledOrg{org}, map[string]*configstore.ManagedWarehouse{"tenant": readyWarehouse("tenant")}) + h.provisioner.store = lostClaimStore{h.store} + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(h.catalog.created) != 0 { + t.Fatal("a lost assignment race projected a tenant into the losing cell") + } + if _, ok := h.store.lastState("tenant"); ok { + t.Fatal("a lost assignment race changed tenant status") + } +} + +func TestTrinoCellBackendReadinessAndIndependentCatalogSets(t *testing.T) { + org := configstore.TrinoEnabledOrg{OrgID: "tenant", DatabaseName: "tenant", CellID: testCellID, RootPasswordHash: "hash"} + h := newTestTrinoProvisioner(t, []configstore.TrinoEnabledOrg{org}, map[string]*configstore.ManagedWarehouse{"tenant": readyWarehouse("tenant")}) + second := &fakeCatalogClient{} + h.provisioner.additionalCatalogs = []TrinoCatalogClient{second} + h.catalog.existing = []string{"org_tenant", "org_old_blue"} + second.existing = []string{"org_old_green"} + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(h.catalog.created) != 0 || len(second.created) != 1 { + t.Fatal("a catalog present on blue must still be created on running green") + } + if len(h.catalog.dropped) != 1 || len(second.dropped) != 1 { + t.Fatal("each running backend must clean its own stale catalog set") + } + second.listErr = errors.New("backend unavailable") + if err := h.provisioner.Reconcile(context.Background()); err == nil { + t.Fatal("a failed running backend must fail reconciliation") + } + state, ok := h.store.lastState("tenant") + if !ok || state.State != configstore.ManagedWarehouseStateFailed { + t.Fatalf("blue success hid green failure: %+v", state) + } +} + +func TestTrinoCellInternalSecretsAreReadOnlyReferences(t *testing.T) { + p, kc, _ := newClusterSecretsTestProvisioner(t) + p.existingInternalSecrets = []string{"blue-internal", "green-internal"} + for _, name := range p.existingInternalSecrets { + _, err := kc.CoreV1().Secrets(TrinoCustomerNamespace).Create(context.Background(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: TrinoCustomerNamespace}, + Data: map[string][]byte{TrinoInternalCommunicationSecretKey: []byte("existing-" + name)}, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + } + kc.ClearActions() + if _, err := p.Bootstrap(context.Background()); err != nil { + t.Fatal(err) + } + for _, action := range kc.Actions() { + if action.GetVerb() == "update" || action.GetVerb() == "patch" || action.GetVerb() == "delete" { + if named, ok := action.(interface{ GetName() string }); ok && (named.GetName() == "blue-internal" || named.GetName() == "green-internal") { + t.Fatal("bootstrap modified a chart-owned internal secret") + } + } + } + for _, name := range p.existingInternalSecrets { + secret := getSecret(t, kc, name) + if secret.Immutable != nil || string(secret.Data[TrinoInternalCommunicationSecretKey]) != "existing-"+name { + t.Fatal("bootstrap changed the existing secret") + } + } + if _, err := kc.CoreV1().Secrets(TrinoCustomerNamespace).Get(context.Background(), TrinoInternalCommunicationSecretName, metav1.GetOptions{}); !apierrors.IsNotFound(err) { + t.Fatal("new cell unexpectedly created the legacy internal secret") + } + if err := kc.CoreV1().Secrets(TrinoCustomerNamespace).Delete(context.Background(), "green-internal", metav1.DeleteOptions{}); err != nil { + t.Fatal(err) + } + if _, err := p.Bootstrap(context.Background()); err == nil { + t.Fatal("missing stopped-backend internal secret must fail without regeneration") + } + if _, err := kc.CoreV1().Secrets(TrinoCustomerNamespace).Get(context.Background(), "green-internal", metav1.GetOptions{}); !apierrors.IsNotFound(err) { + t.Fatal("bootstrap regenerated a missing chart-owned secret") + } +} diff --git a/controlplane/provisioner/trino_provisioner.go b/controlplane/provisioner/trino_provisioner.go index df7d12aa..cb4384bb 100644 --- a/controlplane/provisioner/trino_provisioner.go +++ b/controlplane/provisioner/trino_provisioner.go @@ -295,6 +295,17 @@ type TrinoProvisionerOpts struct { // Empty == configstore.DefaultTrinoCellID. CellID string + // ExplicitAssignmentOnly prevents this cell from claiming unassigned tenants. + ExplicitAssignmentOnly bool + + // AdditionalCatalogs contains other running backends in this logical cell. + // Stopped backends must not be included. All running backends gate readiness. + AdditionalCatalogs []TrinoCatalogClient + + // ExistingInternalSecrets names chart-owned internal-communication secrets. + // When set, the provisioner validates these references without modifying them. + ExistingInternalSecrets []string + // TenantSecretMountPath is the in-pod path the chart mounts // TrinoTenantSecretName at; each org's catalog points its // connection-password-file at /. Empty == @@ -367,9 +378,8 @@ type TrinoCatalogCredentialUpdater interface { type TrinoStore interface { ListTrinoEnabledOrgs() ([]configstore.TrinoEnabledOrg, error) UpdateTrinoState(orgID string, upd configstore.TrinoStateUpdate) error - // AssignTrinoCell claims an org with no cell into this provisioner's - // cell. Only ever writes rows whose cell is still unset. - AssignTrinoCell(orgID, cellID string) error + // ClaimTrinoCell returns true only when this call acquires ownership. + ClaimTrinoCell(orgID, cellID string) (bool, error) } // TrinoWarehouseStore reads a single org's warehouse row to populate the @@ -402,20 +412,24 @@ type TrinoDucklingResolver func(ctx context.Context, orgID string) (*DucklingSta // fires on first install; thereafter ensureClusterSecrets adopts the // existing K8s Secrets. type TrinoProvisioner struct { - store TrinoStore - bootstrapSentinel TrinoBootstrapSentinelStore - warehouses TrinoWarehouseStore - ducklings TrinoDucklingResolver - kubernetes kubernetes.Interface - namespace string - cellID string - catalog TrinoCatalogClient - bundleStore *opa.BundleStore - bundleBuilder opa.BundleBuilder - tenantSecretMountPath string - awsRegion string - s3MaxConnections int - filesystemCacheEnabled bool + store TrinoStore + bootstrapSentinel TrinoBootstrapSentinelStore + warehouses TrinoWarehouseStore + ducklings TrinoDucklingResolver + kubernetes kubernetes.Interface + namespace string + cellID string + explicitAssignmentOnly bool + catalog TrinoCatalogClient + additionalCatalogs []TrinoCatalogClient + catalogTimeout time.Duration + existingInternalSecrets []string + bundleStore *opa.BundleStore + bundleBuilder opa.BundleBuilder + tenantSecretMountPath string + awsRegion string + s3MaxConnections int + filesystemCacheEnabled bool // adminPasswordHash is cached on each Reconcile from the // trino-auth K8s Secret and prepended to password.db on projection. @@ -487,6 +501,11 @@ func NewTrinoProvisioner(opts TrinoProvisionerOpts) (*TrinoProvisioner, error) { if opts.Catalog == nil { return nil, errors.New("TrinoProvisioner: Catalog client is required") } + for _, catalog := range opts.AdditionalCatalogs { + if catalog == nil { + return nil, errors.New("TrinoProvisioner: additional catalog client must not be nil") + } + } if opts.BundleStore == nil { return nil, errors.New("TrinoProvisioner: BundleStore is required") } @@ -510,20 +529,24 @@ func NewTrinoProvisioner(opts TrinoProvisionerOpts) (*TrinoProvisioner, error) { maxConns = defaultTrinoS3MaxConnections } return &TrinoProvisioner{ - store: opts.Store, - bootstrapSentinel: opts.BootstrapSentinel, - warehouses: opts.Warehouses, - ducklings: opts.Ducklings, - kubernetes: opts.Kubernetes, - namespace: ns, - cellID: cell, - catalog: opts.Catalog, - bundleStore: opts.BundleStore, - bundleBuilder: opts.BundleBuilder, - tenantSecretMountPath: strings.TrimRight(mountPath, "/"), - awsRegion: opts.AWSRegion, - s3MaxConnections: maxConns, - filesystemCacheEnabled: opts.FilesystemCacheEnabled, + store: opts.Store, + bootstrapSentinel: opts.BootstrapSentinel, + warehouses: opts.Warehouses, + ducklings: opts.Ducklings, + kubernetes: opts.Kubernetes, + namespace: ns, + cellID: cell, + explicitAssignmentOnly: opts.ExplicitAssignmentOnly, + catalog: opts.Catalog, + additionalCatalogs: append([]TrinoCatalogClient(nil), opts.AdditionalCatalogs...), + catalogTimeout: 30 * time.Second, + existingInternalSecrets: append([]string(nil), opts.ExistingInternalSecrets...), + bundleStore: opts.BundleStore, + bundleBuilder: opts.BundleBuilder, + tenantSecretMountPath: strings.TrimRight(mountPath, "/"), + awsRegion: opts.AWSRegion, + s3MaxConnections: maxConns, + filesystemCacheEnabled: opts.FilesystemCacheEnabled, }, nil } @@ -768,8 +791,7 @@ func rejectPrincipalCollisions(orgs []configstore.TrinoEnabledOrg) (projectable // error and not a state write: writing state for an org we don't own // would fight the owning cell's writer every tick. // -// There is exactly one cell today. This function is the whole of "cell -// awareness" — no assignment policy, no rebalancing, no capacity model. +// Registered cells require explicit assignment and never claim the default fleet. func (p *TrinoProvisioner) claimCellOrgs(orgs []configstore.TrinoEnabledOrg) []configstore.TrinoEnabledOrg { mine := make([]configstore.TrinoEnabledOrg, 0, len(orgs)) for _, o := range orgs { @@ -777,7 +799,11 @@ func (p *TrinoProvisioner) claimCellOrgs(orgs []configstore.TrinoEnabledOrg) []c case p.cellID: mine = append(mine, o) case "": - if err := p.store.AssignTrinoCell(o.OrgID, p.cellID); err != nil { + if p.explicitAssignmentOnly { + continue + } + claimed, err := p.store.ClaimTrinoCell(o.OrgID, p.cellID) + if err != nil { // Transient write failure — leave the org unassigned and // let the next tick claim it. Projecting it now would // mean serving a tenant whose ownership is unrecorded. @@ -785,6 +811,9 @@ func (p *TrinoProvisioner) claimCellOrgs(orgs []configstore.TrinoEnabledOrg) []c "org", o.OrgID, "cell", p.cellID, "error", err) continue } + if !claimed { + continue + } slog.Info("Trino reconcile: org claimed into cell.", "org", o.OrgID, "cell", p.cellID) o.CellID = p.cellID mine = append(mine, o) @@ -851,8 +880,16 @@ func (p *TrinoProvisioner) ensureClusterSecrets(ctx context.Context) (bundleToke // internal-communication shared secret: write-once + immutable. The // provisioner doesn't consume its value at runtime (it's env- // projected to Trino pods by the chart), but it must exist. - if _, err := p.ensureWriteOnceSecret(ctx, TrinoInternalCommunicationSecretName, TrinoInternalCommunicationSecretKey, bootstrapped); err != nil { - return "", err + if len(p.existingInternalSecrets) == 0 { + if _, err := p.ensureWriteOnceSecret(ctx, TrinoInternalCommunicationSecretName, TrinoInternalCommunicationSecretKey, bootstrapped); err != nil { + return "", err + } + } else { + for _, name := range p.existingInternalSecrets { + if _, err := p.readSecretKey(ctx, name, TrinoInternalCommunicationSecretKey); err != nil { + return "", fmt.Errorf("read chart-owned internal secret %s: %w", name, err) + } + } } // OPA bundle bearer token: write-once + immutable. Returned so the @@ -901,6 +938,11 @@ func (p *TrinoProvisioner) ensureClusterSecrets(ctx context.Context) (bundleToke if updater, ok := p.catalog.(TrinoCatalogCredentialUpdater); ok { updater.SetCredentials(opa.AdminPrincipal, adminPlaintext) } + for _, catalog := range p.additionalCatalogs { + if updater, ok := catalog.(TrinoCatalogCredentialUpdater); ok { + updater.SetCredentials(opa.AdminPrincipal, adminPlaintext) + } + } return bundleToken, nil } @@ -1370,10 +1412,41 @@ func (p *TrinoProvisioner) reconcileCatalogs( ctx context.Context, orgs []configstore.TrinoEnabledOrg, tenants tenantSecretProjection, +) (map[string]catalogOutcome, error) { + outcomes, firstErr := p.reconcileBoundedBackend(ctx, orgs, tenants, p.catalog) + errs := []error{firstErr} + for i, catalog := range p.additionalCatalogs { + backendOutcomes, err := p.reconcileBoundedBackend(ctx, orgs, tenants, catalog) + if err != nil { + errs = append(errs, fmt.Errorf("additional running backend %d: %w", i, err)) + } + for orgID, next := range backendOutcomes { + previous := outcomes[orgID] + if next.Err != nil { + outcomes[orgID] = catalogOutcome{Err: errors.Join(previous.Err, next.Err)} + } else if previous.Err == nil && next.Pending { + outcomes[orgID] = next + } + } + } + return outcomes, errors.Join(errs...) +} + +func (p *TrinoProvisioner) reconcileBoundedBackend(ctx context.Context, orgs []configstore.TrinoEnabledOrg, tenants tenantSecretProjection, catalog TrinoCatalogClient) (map[string]catalogOutcome, error) { + backendCtx, cancel := context.WithTimeout(ctx, p.catalogTimeout) + defer cancel() + return p.reconcileBackendCatalogs(backendCtx, orgs, tenants, catalog) +} + +func (p *TrinoProvisioner) reconcileBackendCatalogs( + ctx context.Context, + orgs []configstore.TrinoEnabledOrg, + tenants tenantSecretProjection, + catalog TrinoCatalogClient, ) (map[string]catalogOutcome, error) { outcomes := make(map[string]catalogOutcome, len(orgs)) - existing, err := p.catalog.ListCatalogs(ctx) + existing, err := catalog.ListCatalogs(ctx) if err != nil { // Listing failed — we can't safely attribute per-org outcomes, // so flag every org as failed-with-this-error so they don't @@ -1470,7 +1543,7 @@ func (p *TrinoProvisioner) reconcileCatalogs( continue } props := p.buildCatalogProperties(o.OrgID, warehouse, duckling) - if err := p.catalog.CreateCatalog(ctx, name, props); err != nil { + if err := catalog.CreateCatalog(ctx, name, props); err != nil { if p.tenantSecretNotMountedYet(err, o.OrgID) { // Not a failure: the Secret key is projected (checked // above) and the pods just have not seen it yet. @@ -1498,7 +1571,7 @@ func (p *TrinoProvisioner) reconcileCatalogs( if wanted[c] { continue } - if err := p.catalog.DropCatalog(ctx, c); err != nil { + if err := catalog.DropCatalog(ctx, c); err != nil { errs = append(errs, fmt.Errorf("drop stale catalog %s: %w", c, err)) continue } diff --git a/controlplane/provisioner/trino_provisioner_test.go b/controlplane/provisioner/trino_provisioner_test.go index e5fa2cb1..685d9791 100644 --- a/controlplane/provisioner/trino_provisioner_test.go +++ b/controlplane/provisioner/trino_provisioner_test.go @@ -56,12 +56,12 @@ func (s *fakeTrinoStore) UpdateTrinoState(orgID string, upd configstore.TrinoSta return nil } -func (s *fakeTrinoStore) AssignTrinoCell(orgID, cellID string) error { +func (s *fakeTrinoStore) ClaimTrinoCell(orgID, cellID string) (bool, error) { s.mu.Lock() defer s.mu.Unlock() s.claimLog = append(s.claimLog, orgID+"->"+cellID) if s.cellErr != nil { - return s.cellErr + return false, s.cellErr } if s.cells == nil { s.cells = make(map[string]string) @@ -74,7 +74,7 @@ func (s *fakeTrinoStore) AssignTrinoCell(orgID, cellID string) error { s.orgs[i].CellID = cellID } } - return nil + return true, nil } func (s *fakeTrinoStore) lastState(orgID string) (configstore.TrinoStateUpdate, bool) { diff --git a/controlplane/trino_fleet.go b/controlplane/trino_fleet.go new file mode 100644 index 00000000..fcdf9bb9 --- /dev/null +++ b/controlplane/trino_fleet.go @@ -0,0 +1,75 @@ +//go:build kubernetes + +package controlplane + +import ( + "context" + "errors" + "fmt" + "sync" + "time" + + "github.com/posthog/duckgres/controlplane/admin" + "github.com/posthog/duckgres/controlplane/configstore" + "github.com/posthog/duckgres/controlplane/provisioner" + "k8s.io/client-go/kubernetes" +) + +type trinoFleet []*trinoWiring + +func buildTrinoFleetWiring(store *configstore.ConfigStore, kc kubernetes.Interface, ducklings provisioner.TrinoDucklingResolver) (trinoFleet, error) { + if !trinoProvisionerEnabled() { + return nil, nil + } + cells, err := resolveTrinoCells() + if err != nil { + return nil, err + } + fleet := make(trinoFleet, 0, len(cells)) + for _, cell := range cells { + wire, err := buildTrinoCellWiring(store, kc, ducklings, cell) + if err != nil { + return nil, fmt.Errorf("wire Trino cell %s: %w", cell.consoleCell().ID, err) + } + fleet = append(fleet, wire) + } + return fleet, nil +} + +// Reconcile isolates cell failures and bounds each cell's external API work. +// Config-store methods retain their existing database timeout behavior. +func (f trinoFleet) Reconcile(ctx context.Context) error { + errs := make([]error, len(f)) + var wg sync.WaitGroup + for i, wire := range f { + wg.Go(func() { + cellCtx, cancel := context.WithTimeout(ctx, 90*time.Second) + defer cancel() + if err := wire.Provisioner.Reconcile(cellCtx); err != nil { + errs[i] = fmt.Errorf("cell %s: %w", wire.Cell.consoleCell().ID, err) + } + }) + } + wg.Wait() + return errors.Join(errs...) +} + +func (f trinoFleet) adminAPI(orgs admin.TrinoOrgStore, audit *admin.AuditStore) *admin.TrinoAPI { + if len(f) == 0 { + return nil + } + cells := make([]admin.TrinoCell, 0, len(f)) + clients := make([]admin.TrinoCoordinatorClient, 0, len(f)) + for _, wire := range f { + cells = append(cells, wire.Console.Cell) + clients = append(clients, wire.Console.Observer) + } + return admin.NewTrinoFleetAPI(cells, clients, orgs, audit) +} + +func (w *trinoWiring) bundlePath() string { + if w.Cell.PublicID == "" { + return "/bundles/trino" + } + return "/bundles/trino/" + w.Cell.PublicID +} diff --git a/controlplane/trino_fleet_test.go b/controlplane/trino_fleet_test.go new file mode 100644 index 00000000..d89f0efd --- /dev/null +++ b/controlplane/trino_fleet_test.go @@ -0,0 +1,106 @@ +//go:build kubernetes + +package controlplane + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/posthog/duckgres/controlplane/configstore" + "github.com/posthog/duckgres/controlplane/provisioner" + "github.com/posthog/duckgres/controlplane/provisioner/opa" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kubefake "k8s.io/client-go/kubernetes/fake" +) + +type fleetBootstrapStore struct{ initialized map[string]bool } + +func (s *fleetBootstrapStore) ListTrinoEnabledOrgs() ([]configstore.TrinoEnabledOrg, error) { + return nil, nil +} +func (s *fleetBootstrapStore) UpdateTrinoState(string, configstore.TrinoStateUpdate) error { + return nil +} +func (s *fleetBootstrapStore) ClaimTrinoCell(string, string) (bool, error) { return false, nil } +func (s *fleetBootstrapStore) GetManagedWarehouseForTrino(string) (*configstore.ManagedWarehouse, error) { + return nil, nil +} +func (s *fleetBootstrapStore) IsTrinoClusterBootstrapped(_ context.Context, namespace string) (bool, error) { + return s.initialized[namespace], nil +} +func (s *fleetBootstrapStore) MarkTrinoClusterBootstrapped(_ context.Context, namespace string) error { + s.initialized[namespace] = true + return nil +} + +func TestTrinoFleetBootstrapSeparatesBundleTokensAndLegacyPath(t *testing.T) { + t.Setenv(envTrinoFilesystemCacheEnabled, "false") + store := &fleetBootstrapStore{initialized: map[string]bool{}} + kc := kubefake.NewClientset() + registered, err := parseTrinoCellRegistry([]byte(testTrinoRegistryJSON)) + if err != nil { + t.Fatal(err) + } + entry := registered[0] + for _, backend := range entry.Backends { + _, err := kc.CoreV1().Secrets(entry.Namespace).Create(context.Background(), &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: backend.InternalSecretName}, Data: map[string][]byte{"shared-secret": []byte("chart-managed-" + backend.ID)}}, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + } + cells := []trinoCell{ + {ID: "cell-001", Namespace: "legacy", CoordinatorURL: "https://legacy.example.test"}, + {ID: "registered:cell-test", PublicID: "cell-test", Namespace: entry.Namespace, ClientURL: entry.ClientURL, CoordinatorURL: entry.Backends[0].CoordinatorURL, Backends: entry.Backends}, + } + engine := gin.New() + var wires []*trinoWiring + var tokens []string + for _, cell := range cells { + wire, err := buildTrinoCellWiring(store, kc, func(context.Context, string) (*provisioner.DucklingStatus, error) { return nil, nil }, cell) + if err != nil { + t.Fatal(err) + } + wire.BundleStore.Set(opa.NewBundle([]byte(cell.ID))) + engine.Any(wire.bundlePath(), gin.WrapH(wire.BundleHandler)) + secret, err := kc.CoreV1().Secrets(cell.Namespace).Get(context.Background(), provisioner.TrinoOPABundleTokenSecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + tokens = append(tokens, string(secret.Data["token"])) + wires = append(wires, wire) + if wire.Provisioner.CellID() != cell.ID { + t.Fatal("bootstrap changed ownership") + } + if len(wire.Observers) != 1 { + t.Fatal("stopped backend acquired a live observer") + } + } + if wires[0].bundlePath() != "/bundles/trino" || wires[1].bundlePath() != "/bundles/trino/cell-test" { + t.Fatal("bundle paths changed") + } + if tokens[0] == tokens[1] { + t.Fatal("cells share a bundle token") + } + for target, wire := range wires { + for owner, token := range tokens { + request := httptest.NewRequest(http.MethodGet, wire.bundlePath(), nil) + request.Header.Set("Authorization", "Bearer "+token) + response := httptest.NewRecorder() + engine.ServeHTTP(response, request) + want := http.StatusUnauthorized + if owner == target { + want = http.StatusOK + } + if response.Code != want { + t.Fatalf("bundle %d token %d: status %d, want %d", target, owner, response.Code, want) + } + } + } + if len(store.initialized) != 2 { + t.Fatal("bootstrap sentinels share state") + } +} diff --git a/controlplane/trino_inputs.go b/controlplane/trino_inputs.go index d195360b..4e2da974 100644 --- a/controlplane/trino_inputs.go +++ b/controlplane/trino_inputs.go @@ -59,8 +59,7 @@ const ( // this to a non-empty value enables the Trino provisioner branch; // leaving it empty disables it. Required configuration when enabled. // - // One cell today, so one URL. See resolveTrinoCell for the shape a - // second cell takes. + // This URL continues to identify the legacy cell when a registry is mounted. envTrinoCoordinatorURL = "DUCKGRES_TRINO_COORDINATOR_URL" // envTrinoCoordinatorServerName is the TLS server name (the coordinator @@ -114,44 +113,33 @@ const ( envTrinoFilesystemCacheEnabled = "DUCKGRES_TRINO_FILESYSTEM_CACHE_ENABLED" ) -// trinoProvisionerEnabled reports whether the control plane should -// wire the Trino provisioner branch. True iff -// DUCKGRES_TRINO_COORDINATOR_URL is non-empty; the URL doubles as the -// enable signal because operators can't meaningfully use the -// provisioner without it (there'd be nothing to talk to). +// trinoProvisionerEnabled recognizes legacy or registry configuration. +// A registry still requires the legacy URL to preserve existing ownership. func trinoProvisionerEnabled() bool { - return strings.TrimSpace(os.Getenv(envTrinoCoordinatorURL)) != "" + return strings.TrimSpace(os.Getenv(envTrinoCoordinatorURL)) != "" || strings.TrimSpace(os.Getenv(envTrinoCellsFile)) != "" } -// trinoCell is one Trino cell: an id plus the coordinator that serves it. -// A cell is the unit a shared Trino deployment scales in — one coordinator -// and worker fleet, one OPA sidecar, one set of projected Secrets, and the -// orgs stamped with its id. -// -// There is exactly ONE cell today, described by the two env vars below. -// The shape a second cell takes is deliberate and small: resolveTrinoCell -// becomes resolveTrinoCells returning a map[cellID]trinoCell (parsed from a -// JSON env var or a mounted config), SetupMultiTenant builds one -// TrinoProvisioner per entry, and each provisioner keeps reconciling only -// the orgs its own claim/skip filter admits. Nothing else in the reconcile -// path has to change — which is the entire reason the cell id is on the row -// rather than implied by the deployment. -// -// What is explicitly NOT here and NOT planned in this change: assignment -// policy (which cell should a new org land on?), capacity accounting, -// rebalancing, or draining a cell. An org lands on whichever cell claims it -// first, and today only one does. +// trinoCell separates durable ownership from the operator-visible identity. +// Registered cells share projections across their independently scheduled backends. +// Only the legacy cell claims unassigned tenants. type trinoCell struct { ID string + PublicID string + Namespace string + Backends []trinoRegisteredBackend CoordinatorURL string TLSServerName string ClientURL string } -// consoleCell names the existing deployment without changing its persisted ownership. +// consoleCell preserves legacy ownership and exposes each logical identity. func (c trinoCell) consoleCell() admin.TrinoCell { + id := c.PublicID + if id == "" { + id = "legacy" + } return admin.TrinoCell{ - ID: "legacy", + ID: id, StoredID: c.ID, CoordinatorURL: c.CoordinatorURL, TLSServerName: c.TLSServerName, @@ -173,6 +161,7 @@ func resolveTrinoCell() (trinoCell, error) { } return trinoCell{ ID: cellID, + Namespace: strings.TrimSpace(os.Getenv(envTrinoNamespace)), CoordinatorURL: coordinatorURL, TLSServerName: strings.TrimSpace(os.Getenv(envTrinoCoordinatorServerName)), ClientURL: strings.TrimSpace(os.Getenv(envTrinoClientURL)), @@ -199,7 +188,8 @@ type trinoWiring struct { // here rather than in multitenant.go so the observer credential is read // through the provisioner that owns it and nothing else has to know how // the coordinator is authenticated. - Console *trinoConsoleWiring + Console *trinoConsoleWiring + Observers []admin.TrinoCoordinatorClient } // trinoConsoleWiring is the admin console's half of the Trino branch: the @@ -267,6 +257,16 @@ func buildTrinoWiring( if err != nil { return nil, err } + return buildTrinoCellWiring(store, kc, ducklings, cell) +} + +type trinoWiringStore interface { + provisioner.TrinoStore + provisioner.TrinoBootstrapSentinelStore + provisioner.TrinoWarehouseStore +} + +func buildTrinoCellWiring(store trinoWiringStore, kc kubernetes.Interface, ducklings provisioner.TrinoDucklingResolver, cell trinoCell) (*trinoWiring, error) { filesystemCacheEnabled, err := trinoFilesystemCacheEnabled() if err != nil { @@ -287,24 +287,35 @@ func buildTrinoWiring( // is https but dials the in-cluster Service address (see // envTrinoCoordinatorServerName). catalogClient := provisioner.NewTrinoCatalogHTTPClient(cell.CoordinatorURL, opa.AdminPrincipal, "", cell.TLSServerName) + var additional []provisioner.TrinoCatalogClient + var internalSecrets []string + for _, backend := range cell.Backends { + internalSecrets = append(internalSecrets, backend.InternalSecretName) + if backend.Running && !backend.RoutingActive { + additional = append(additional, provisioner.NewTrinoCatalogHTTPClient(backend.CoordinatorURL, opa.AdminPrincipal, "", backend.TLSServerName)) + } + } bundleStore := &opa.BundleStore{} trinoProv, err := provisioner.NewTrinoProvisioner(provisioner.TrinoProvisionerOpts{ - Store: store, - BootstrapSentinel: store, - Warehouses: store, - Ducklings: ducklings, - Kubernetes: kc, - Namespace: strings.TrimSpace(os.Getenv(envTrinoNamespace)), - CellID: cell.ID, - TenantSecretMountPath: strings.TrimSpace(os.Getenv(envTrinoTenantSecretMountPath)), - Catalog: catalogClient, - BundleStore: bundleStore, - BundleBuilder: opa.NewBuilder(), - AWSRegion: strings.TrimSpace(os.Getenv(envTrinoAWSRegion)), - S3MaxConnections: envInt(envTrinoS3MaxConnections), - FilesystemCacheEnabled: filesystemCacheEnabled, + Store: store, + BootstrapSentinel: store, + Warehouses: store, + Ducklings: ducklings, + Kubernetes: kc, + Namespace: cell.Namespace, + CellID: cell.ID, + ExplicitAssignmentOnly: cell.PublicID != "", + AdditionalCatalogs: additional, + ExistingInternalSecrets: internalSecrets, + TenantSecretMountPath: strings.TrimSpace(os.Getenv(envTrinoTenantSecretMountPath)), + Catalog: catalogClient, + BundleStore: bundleStore, + BundleBuilder: opa.NewBuilder(), + AWSRegion: strings.TrimSpace(os.Getenv(envTrinoAWSRegion)), + S3MaxConnections: envInt(envTrinoS3MaxConnections), + FilesystemCacheEnabled: filesystemCacheEnabled, }) if err != nil { return nil, fmt.Errorf("construct Trino provisioner: %w", err) @@ -326,20 +337,27 @@ func buildTrinoWiring( // no post-construction swap, so no window where the endpoint serves // with a token a real client could match by accident. bundleHandler := opa.NewHandler(bundleStore, opa.BearerTokenAuth(bundleToken)) + observer := admin.NewTrinoCoordinatorClient(cell.CoordinatorURL, cell.TLSServerName, trinoProv.ObserverCredential) + observers := []admin.TrinoCoordinatorClient{observer} + for _, backend := range cell.Backends { + if backend.Running && !backend.RoutingActive { + observers = append(observers, admin.NewTrinoCoordinatorClient(backend.CoordinatorURL, backend.TLSServerName, trinoProv.ObserverCredential)) + } + } return &trinoWiring{ Provisioner: trinoProv, BundleStore: bundleStore, BundleHandler: bundleHandler, Cell: cell, + Observers: observers, Console: &trinoConsoleWiring{ 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 // after that self-heal. - Observer: admin.NewTrinoCoordinatorClient( - cell.CoordinatorURL, cell.TLSServerName, trinoProv.ObserverCredential), + Observer: observer, }, }, nil } diff --git a/controlplane/trino_registry.go b/controlplane/trino_registry.go new file mode 100644 index 00000000..225b6449 --- /dev/null +++ b/controlplane/trino_registry.go @@ -0,0 +1,180 @@ +//go:build kubernetes + +package controlplane + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "net" + "net/url" + "os" + "strconv" + "strings" + + "github.com/posthog/duckgres/controlplane/provisioner" + "k8s.io/apimachinery/pkg/util/validation" +) + +const envTrinoCellsFile = "DUCKGRES_TRINO_CELLS_FILE" + +const registeredTrinoCellPrefix = "registered:" + +// resolveTrinoCells extends the legacy configuration without changing its ownership. +func resolveTrinoCells() ([]trinoCell, error) { + legacy, err := resolveTrinoCell() + if err != nil { + return nil, err + } + if strings.HasPrefix(legacy.ID, registeredTrinoCellPrefix) { + return nil, errors.New("legacy Trino cell ID uses the reserved registered prefix") + } + cells := []trinoCell{legacy} + path := strings.TrimSpace(os.Getenv(envTrinoCellsFile)) + if path == "" { + return cells, nil + } + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read Trino registry: %w", err) + } + registered, err := parseTrinoCellRegistry(data) + if err != nil { + return nil, err + } + legacyNS := legacy.Namespace + if legacyNS == "" { + legacyNS = provisioner.TrinoCustomerNamespace + } + legacyEndpoint, err := trinoEndpointKey(legacy.CoordinatorURL) + if err != nil { + return nil, fmt.Errorf("legacy coordinator URL: %w", err) + } + for _, entry := range registered { + if entry.Namespace == legacyNS { + return nil, errors.New("registered cell must not share the legacy namespace") + } + cell := trinoCell{ID: registeredTrinoCellPrefix + entry.ID, PublicID: entry.ID, Namespace: entry.Namespace, ClientURL: entry.ClientURL, Backends: entry.Backends} + for _, backend := range entry.Backends { + endpoint, _ := trinoEndpointKey(backend.CoordinatorURL) + if endpoint == legacyEndpoint { + return nil, errors.New("registered cell must not share a legacy coordinator") + } + if backend.RoutingActive { + cell.CoordinatorURL, cell.TLSServerName = backend.CoordinatorURL, backend.TLSServerName + } + } + cells = append(cells, cell) + } + return cells, nil +} + +type trinoRegisteredCell struct { + ID string `json:"id"` + Namespace string `json:"namespace"` + ClientURL string `json:"client_url"` + RoutingGroup string `json:"routing_group"` + Backends []trinoRegisteredBackend `json:"backends"` +} + +type trinoRegisteredBackend struct { + ID string `json:"id"` + CoordinatorURL string `json:"coordinator_url"` + TLSServerName string `json:"tls_server_name,omitempty"` + Running bool `json:"running"` + RoutingActive bool `json:"routing_active"` + InternalSecretName string `json:"internal_secret_name"` +} + +func parseTrinoCellRegistry(data []byte) ([]trinoRegisteredCell, error) { + var registry struct { + Cells []trinoRegisteredCell `json:"cells"` + } + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(®istry); err != nil { + return nil, fmt.Errorf("decode Trino cell registry: %w", err) + } + if err := decoder.Decode(new(any)); !errors.Is(err, io.EOF) { + return nil, errors.New("Trino cell registry must contain one JSON document") + } + if len(registry.Cells) == 0 { + return nil, errors.New("Trino cell registry must contain at least one cell") + } + if len(registry.Cells) > 16 { + return nil, errors.New("Trino cell registry supports at most 16 additional cells") + } + identities, namespaces, groups, endpoints := map[string]bool{}, map[string]bool{}, map[string]bool{}, map[string]bool{} + for _, cell := range registry.Cells { + if cell.ID == "legacy" || len(validation.IsDNS1123Label(cell.ID)) != 0 { + return nil, errors.New("Trino cell identity must be a DNS label other than legacy") + } + if len(validation.IsDNS1123Label(cell.Namespace)) != 0 { + return nil, fmt.Errorf("Trino cell %s has an invalid namespace", cell.ID) + } + if len(validation.IsDNS1123Label(cell.RoutingGroup)) != 0 { + return nil, fmt.Errorf("Trino cell %s has an invalid routing group", cell.ID) + } + if identities[cell.ID] || namespaces[cell.Namespace] || groups[cell.RoutingGroup] { + return nil, errors.New("Trino cells must have distinct identities, namespaces and routing groups") + } + identities[cell.ID], namespaces[cell.Namespace], groups[cell.RoutingGroup] = true, true, true + if _, err := trinoEndpointKey(cell.ClientURL); err != nil { + return nil, fmt.Errorf("Trino cell %s client URL: %w", cell.ID, err) + } + backendIDs, secrets := map[string]bool{}, map[string]bool{} + if len(cell.Backends) > 2 { + return nil, fmt.Errorf("Trino cell %s supports at most two backends", cell.ID) + } + active := 0 + for _, backend := range cell.Backends { + if len(validation.IsDNS1123Label(backend.ID)) != 0 || backendIDs[backend.ID] { + return nil, fmt.Errorf("Trino cell %s has an invalid or duplicate backend identity", cell.ID) + } + backendIDs[backend.ID] = true + if len(validation.IsDNS1123Subdomain(backend.InternalSecretName)) != 0 || secrets[backend.InternalSecretName] { + return nil, fmt.Errorf("Trino cell %s has an invalid or shared internal secret reference", cell.ID) + } + secrets[backend.InternalSecretName] = true + endpoint, err := trinoEndpointKey(backend.CoordinatorURL) + if err != nil { + return nil, fmt.Errorf("Trino cell %s backend %s URL: %w", cell.ID, backend.ID, err) + } + if endpoints[endpoint] { + return nil, errors.New("Trino backends must have distinct coordinator endpoints") + } + endpoints[endpoint] = true + if backend.TLSServerName != "" && len(validation.IsDNS1123Subdomain(backend.TLSServerName)) != 0 { + return nil, fmt.Errorf("Trino cell %s backend %s has an invalid TLS server name", cell.ID, backend.ID) + } + if backend.RoutingActive { + if !backend.Running { + return nil, fmt.Errorf("Trino cell %s routes to a stopped backend", cell.ID) + } + active++ + } + } + if active != 1 { + return nil, fmt.Errorf("Trino cell %s must have exactly one routing-active backend", cell.ID) + } + } + return registry.Cells, nil +} + +func trinoEndpointKey(raw string) (string, error) { + endpoint, err := url.Parse(raw) + if err != nil || endpoint.Scheme != "https" || endpoint.Hostname() == "" || endpoint.User != nil || endpoint.RawQuery != "" || endpoint.ForceQuery || endpoint.Fragment != "" || (endpoint.Path != "" && endpoint.Path != "/") { + return "", errors.New("expected an HTTPS endpoint without credentials, path, query or fragment") + } + port := endpoint.Port() + if port == "" { + port = "443" + } + number, err := strconv.Atoi(port) + if err != nil || number < 1 || number > 65535 { + return "", errors.New("invalid HTTPS endpoint port") + } + return net.JoinHostPort(strings.TrimSuffix(strings.ToLower(endpoint.Hostname()), "."), strconv.Itoa(number)), nil +} diff --git a/controlplane/trino_registry_test.go b/controlplane/trino_registry_test.go new file mode 100644 index 00000000..87f3cbe5 --- /dev/null +++ b/controlplane/trino_registry_test.go @@ -0,0 +1,125 @@ +//go:build kubernetes + +package controlplane + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestTrinoRegistryRuntimePreservesLegacyAndSkipsStoppedBackend(t *testing.T) { + path := filepath.Join(t.TempDir(), "cells.json") + if err := os.WriteFile(path, []byte(testTrinoRegistryJSON), 0600); err != nil { + t.Fatal(err) + } + t.Setenv(envTrinoCellsFile, path) + t.Setenv(envTrinoCoordinatorURL, "https://legacy.example.test") + t.Setenv(envTrinoCellID, "cell-001") + t.Setenv(envTrinoNamespace, "trino-legacy") + cells, err := resolveTrinoCells() + if err != nil { + t.Fatal(err) + } + if len(cells) != 2 || cells[0].ID != "cell-001" || cells[0].consoleCell().ID != "legacy" { + t.Fatalf("legacy ownership changed: %+v", cells) + } + if cells[1].ID != "registered:cell-test" || cells[1].consoleCell().ID != "cell-test" { + t.Fatalf("registry storage identity collision: %+v", cells[1]) + } + if cells[1].CoordinatorURL != "https://blue.example.test" || len(cells[1].Backends) != 2 { + t.Fatal("runtime discarded blue or stopped green") + } + t.Setenv(envTrinoCoordinatorURL, "") + if _, err := resolveTrinoCells(); err == nil { + t.Fatal("registry silently removed legacy") + } +} + +const testTrinoRegistryJSON = `{"cells":[{"id":"cell-test","namespace":"trino-test","client_url":"https://gateway.example.test","routing_group":"cell-test","backends":[{"id":"blue","coordinator_url":"https://blue.example.test","running":true,"routing_active":true,"internal_secret_name":"blue-internal"},{"id":"green","coordinator_url":"https://green.example.test","running":false,"routing_active":false,"internal_secret_name":"green-internal"}]}]}` + +func TestTrinoRegistryPreservesStoppedBackend(t *testing.T) { + cells, err := parseTrinoCellRegistry([]byte(testTrinoRegistryJSON)) + if err != nil { + t.Fatal(err) + } + if len(cells) != 1 || cells[0].ID != "cell-test" { + t.Fatalf("unexpected cell identities: %+v", cells) + } + if len(cells[0].Backends) != 2 || !cells[0].Backends[0].Running || cells[0].Backends[1].Running || cells[0].Backends[1].RoutingActive { + t.Fatalf("stopped backend configuration changed: %+v", cells[0].Backends) + } +} + +func TestTrinoRegistryRejectsUnsafeConfiguration(t *testing.T) { + tests := map[string]string{ + "unknown field": strings.Replace(testTrinoRegistryJSON, `"cells":`, `"typo":`, 1), + "reserved legacy identity": strings.Replace(testTrinoRegistryJSON, `"id":"cell-test"`, `"id":"legacy"`, 1), + "unsafe namespace": strings.Replace(testTrinoRegistryJSON, `"namespace":"trino-test"`, `"namespace":"../other"`, 1), + "credentials field": strings.Replace(testTrinoRegistryJSON, `"cells":`, `"password":"secret","cells":`, 1), + "empty registry": `{"cells":[]}`, + "no backends": `{"cells":[{"id":"cell-test","namespace":"trino-test","client_url":"https://gateway.example.test","routing_group":"cell-test","backends":[]}]}`, + "plain HTTP": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `http://blue.example.test`, 1), + "embedded credentials": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://user:secret@blue.example.test`, 1), + "URL query": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://blue.example.test?token=value`, 1), + "URL path": strings.Replace(testTrinoRegistryJSON, `https://blue.example.test`, `https://blue.example.test/catalogs`, 1), + "active backend stopped": strings.Replace(testTrinoRegistryJSON, `"running":true`, `"running":false`, 1), + "no active backend": strings.Replace(testTrinoRegistryJSON, `"routing_active":true`, `"routing_active":false`, 1), + "two active backends": strings.Replace(testTrinoRegistryJSON, `"running":false,"routing_active":false`, `"running":true,"routing_active":true`, 1), + "duplicate backend identity": strings.Replace(testTrinoRegistryJSON, `"id":"green"`, `"id":"blue"`, 1), + "duplicate endpoint": strings.Replace(testTrinoRegistryJSON, `https://green.example.test`, `https://blue.example.test`, 1), + "duplicate canonical endpoint": strings.Replace(testTrinoRegistryJSON, `https://green.example.test`, `https://BLUE.example.test.:0443/`, 1), + "shared internal secret": strings.Replace(testTrinoRegistryJSON, `green-internal`, `blue-internal`, 1), + "invalid internal secret": strings.Replace(testTrinoRegistryJSON, `green-internal`, `../other`, 1), + "header injection": strings.Replace(testTrinoRegistryJSON, `"routing_group":"cell-test"`, `"routing_group":"cell-test\r\nHost: other"`, 1), + "trailing document": testTrinoRegistryJSON + `{}`, + } + for name, data := range tests { + t.Run(name, func(t *testing.T) { + if _, err := parseTrinoCellRegistry([]byte(data)); err == nil { + t.Fatal("unsafe registry accepted") + } + }) + } +} + +func TestTrinoRegistryRuntimeRejectsLegacyCollisions(t *testing.T) { + path := filepath.Join(t.TempDir(), "cells.json") + if err := os.WriteFile(path, []byte(testTrinoRegistryJSON), 0600); err != nil { + t.Fatal(err) + } + t.Setenv(envTrinoCellsFile, path) + t.Setenv(envTrinoCoordinatorURL, "https://legacy.example.test") + t.Setenv(envTrinoNamespace, "legacy") + t.Setenv(envTrinoCellID, "legacy-owned") + for _, tc := range []struct{ name, key, value string }{ + {"namespace", envTrinoNamespace, "trino-test"}, + {"endpoint", envTrinoCoordinatorURL, "https://blue.example.test.:0443/"}, + {"storage prefix", envTrinoCellID, "registered:cell-test"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(tc.key, tc.value) + if _, err := resolveTrinoCells(); err == nil { + t.Fatal("unsafe registry accepted") + } + }) + } +} + +func TestTrinoRegistryRejectsDuplicateCellOwnership(t *testing.T) { + cell := strings.TrimSuffix(strings.TrimPrefix(testTrinoRegistryJSON, `{"cells":[`), `]}`) + other := strings.NewReplacer("cell-test", "cell-other", "trino-test", "trino-other", "blue.example", "other-blue.example", "green.example", "other-green.example").Replace(cell) + for name, second := range map[string]string{ + "identity": strings.Replace(other, `"id":"cell-other"`, `"id":"cell-test"`, 1), + "namespace": strings.Replace(other, "trino-other", "trino-test", 1), + "routing group": strings.Replace(other, `"routing_group":"cell-other"`, `"routing_group":"cell-test"`, 1), + "endpoint": strings.Replace(other, "other-blue.example", "blue.example", 1), + } { + t.Run(name, func(t *testing.T) { + if _, err := parseTrinoCellRegistry([]byte(`{"cells":[` + cell + `,` + second + `]}`)); err == nil { + t.Fatal("duplicate cell ownership accepted") + } + }) + } +} diff --git a/docs/trino-cells.md b/docs/trino-cells.md new file mode 100644 index 00000000..e21cdced --- /dev/null +++ b/docs/trino-cells.md @@ -0,0 +1,128 @@ +# Trino cell registration and initial placement + +The control plane can reconcile the existing `legacy` cell alongside additional +logical cells. A logical cell has one namespace and up to two independent Trino +backends, usually blue and green. Catalogs and credentials are projected only +for warehouses assigned to that cell. This is compute placement: DuckLake's +metadata database, S3 objects, and customer password do not move. + +## Configuration + +`DUCKGRES_TRINO_CELLS_FILE` defaults to unset. Set it to a mounted JSON file to +register up to 16 additional cells. Configuration is loaded at startup, not +hot-reloaded. All control-plane replicas must receive the same configuration. +Keep the existing `DUCKGRES_TRINO_COORDINATOR_URL`, namespace, TLS name, and cell +ID unchanged; they continue to describe legacy. A registry without a legacy +coordinator is rejected, rather than silently abandoning existing warehouses. + +```json +{ + "cells": [ + { + "id": "cell-001", + "namespace": "trino-cell-001", + "client_url": "https://warehouse.example.test", + "routing_group": "cell-001", + "backends": [ + { + "id": "blue", + "coordinator_url": "https://blue.example.test", + "running": true, + "routing_active": true, + "internal_secret_name": "trino-blue-internal" + }, + { + "id": "green", + "coordinator_url": "https://green.example.test", + "running": false, + "routing_active": false, + "internal_secret_name": "trino-green-internal" + } + ] + } + ] +} +``` + +URLs must use HTTPS without embedded credentials, paths, queries, or fragments. +An optional backend `tls_server_name` pins certificate verification when the +coordinator URL uses a different name. Cells must have distinct namespaces, +routing groups, and coordinator endpoints, including relative to legacy. +Each cell must have exactly one routing-active backend, and that backend must +be marked running. Registry fields describe operator intent; they do not scale +workloads or change Gateway routing. + +The existing per-namespace projection names remain `trino-auth`, +`trino-tenant-secrets`, `trino-resource-groups`, and `trino-opa-bundle-token`. +Blue and green share these cell-local projections, but have distinct, +deployment-managed internal-communication Secrets. Duckgres reads those +references and refuses missing keys; it never creates or modifies them. +They must exist even for a stopped backend. Grant the control-plane service +account the corresponding namespace-scoped projection permissions first. + +New-cell OPA sidecars poll `/bundles/trino/` with that namespace's +bundle token. Legacy keeps `/bundles/trino`. Tokens cannot read another cell's +bundle. The observer credential remains separate from the catalog administrator. + +## Initial assignment + +Existing stored ownership is unchanged. In particular, a legacy row whose +stored ID is `cell-001` still belongs to legacy. New registry cells use the +reserved storage prefix `registered:`, so logical `cell-001` stores +`registered:cell-001`. Do not edit these values directly. + +Use the authenticated operator console to select a cell **before first enabling +Trino**, or use its admin-only `PUT /api/v1/orgs//trino/cell` endpoint with +`{"cell":"cell-001"}`. Selection itself does not enable Trino. Unselected new +warehouses retain the existing default: legacy claims them when enabled. +Assignments survive disable/re-enable. Already owned warehouses cannot change +cells through this endpoint, even when disabled. + +`client_url` is the opaque client endpoint, not a cell-selection instruction to +customers. This PR does not implement authenticated Gateway assignment lookup. +Do not advertise a shared Gateway URL as usable for a new cell until server-side +routing has been separately wired and tested with an authenticated query. +Coordinator/catalog readiness alone does not prove Gateway reachability. + +## Blue running, green stopped + +Both backends receive shared authentication, tenant-password, and OPA +projections. Only backends configured `running: true` receive catalog API calls +or live observer polls. A stopped green does not make blue's tenants unhealthy. +When green starts, update the registry and restart the control plane. Every +running backend must reconcile successfully before a tenant is reported ready; +one successful coordinator cannot conceal another's missing catalog or failure. +The console observes the configured routing-active backend. Usage collection +polls each running backend independently under the existing leader lease. + +Each backend's catalog reconciliation has a 30-second context budget. Cell +reconciliation runs independently with a 90-second external-API budget. +Existing config-store methods retain their database timeout behavior; these +budgets are not a hard deadline for stalled database calls. + +## Local verification and recovery + +Run `just test-trino`, `just test-trino-admin`, `just ui-test`, and `just lint`. +The PostgreSQL-backed tests exercise initial-selection races against legacy +claiming and enablement. The isolated Trino CI lane exercises the real query +and projection path; never redirect it to a shared coordinator. + +For a failed rollout, correct invalid JSON, duplicate identities, missing +namespace permissions, missing chart-managed internal Secrets, or TLS errors +and restart the control plane. Do not delete bootstrap sentinels or regenerate +internal credentials as a recovery shortcut. Preserve the registry while any +warehouse remains assigned to it. Removing a configured cell does not migrate +its warehouses: unknown ownership fails closed in the console. + +## Existing-warehouse migration follow-up + +Initial assignment is deliberately not a live-migration API. Disabling Trino +does not prove that cached credentials, open transactions, or direct coordinator +clients can no longer submit work. A maintenance move needs an enforced source +admission barrier, verified drain, destination provisioning, an explicit +assignment/routing switch, and source cleanup. Until that barrier exists, use a +previously non-Trino-enabled test warehouse for new-cell testing. Do not change +an existing warehouse's row to simulate a completed move. + +Gateway public exposure is also a separate gate: authenticate every externally +reachable API and UI before publishing it; keep unauthenticated probes internal. diff --git a/justfile b/justfile index 946caa56..1bcdb06f 100644 --- a/justfile +++ b/justfile @@ -329,6 +329,10 @@ test-configstore-integration: go test -v -count=1 ./tests/configstore/... # Run Kubernetes-only control plane package tests +[group('test')] +test-trino pattern="Trino": + go test -v -count=1 -tags kubernetes -run '{{pattern}}' ./controlplane ./controlplane/admin ./controlplane/provisioner + [group('test')] test-controlplane-k8s: go test -v -count=1 -tags kubernetes . ./controlplane ./controlplane/admin ./controlplane/provisioner From adb4f849a22f2e6e41477dcdb2eb7905f4719339 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 14:32:43 +0200 Subject: [PATCH 3/8] test: verify slow Trino cells cannot starve siblings --- controlplane/trino_fleet_test.go | 58 ++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/controlplane/trino_fleet_test.go b/controlplane/trino_fleet_test.go index d89f0efd..6c190ceb 100644 --- a/controlplane/trino_fleet_test.go +++ b/controlplane/trino_fleet_test.go @@ -4,9 +4,11 @@ package controlplane import ( "context" + "errors" "net/http" "net/http/httptest" "testing" + "time" "github.com/gin-gonic/gin" "github.com/posthog/duckgres/controlplane/configstore" @@ -17,6 +19,62 @@ import ( kubefake "k8s.io/client-go/kubernetes/fake" ) +type fleetCatalog struct { + called chan struct{} + block bool +} + +func (c *fleetCatalog) ListCatalogs(ctx context.Context) ([]string, error) { + close(c.called) + if c.block { + <-ctx.Done() + return nil, ctx.Err() + } + return nil, nil +} +func (c *fleetCatalog) CreateCatalog(context.Context, string, map[string]string) error { return nil } +func (c *fleetCatalog) AlterCatalog(context.Context, string, map[string]string) error { return nil } +func (c *fleetCatalog) DropCatalog(context.Context, string) error { return nil } + +func TestTrinoFleetSlowCellDoesNotBlockSibling(t *testing.T) { + store := &fleetBootstrapStore{initialized: map[string]bool{}} + kc := kubefake.NewClientset() + catalogs := []*fleetCatalog{{called: make(chan struct{}), block: true}, {called: make(chan struct{})}} + var fleet trinoFleet + for i, namespace := range []string{"legacy", "registered"} { + p, err := provisioner.NewTrinoProvisioner(provisioner.TrinoProvisionerOpts{ + Store: store, BootstrapSentinel: store, Warehouses: store, Kubernetes: kc, + Ducklings: func(context.Context, string) (*provisioner.DucklingStatus, error) { return nil, nil }, + Namespace: namespace, CellID: namespace, Catalog: catalogs[i], BundleStore: &opa.BundleStore{}, BundleBuilder: opa.NewBuilder(), + }) + if err != nil { + t.Fatal(err) + } + if _, err := p.Bootstrap(context.Background()); err != nil { + t.Fatal(err) + } + fleet = append(fleet, &trinoWiring{Provisioner: p, Cell: trinoCell{ID: namespace}}) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + result := make(chan error, 1) + go func() { result <- fleet.Reconcile(ctx) }() + select { + case <-catalogs[1].called: + case <-time.After(10 * time.Second): + t.Fatal("blocked cell prevented its sibling from reconciling") + } + cancel() + select { + case err := <-result: + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected cancellation, got %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("fleet did not cancel") + } +} + type fleetBootstrapStore struct{ initialized map[string]bool } func (s *fleetBootstrapStore) ListTrinoEnabledOrgs() ([]configstore.TrinoEnabledOrg, error) { From 6abda15c672e6ce04221d5eded1c96da1cfbb306 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 14:35:46 +0200 Subject: [PATCH 4/8] fix: preserve typed Trino provisioner validation --- controlplane/multitenant.go | 2 +- controlplane/provisioner/controller.go | 13 ++++++++++--- controlplane/provisioner/trino_controller_test.go | 15 +++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 controlplane/provisioner/trino_controller_test.go diff --git a/controlplane/multitenant.go b/controlplane/multitenant.go index a5b76508..61c3910c 100644 --- a/controlplane/multitenant.go +++ b/controlplane/multitenant.go @@ -552,7 +552,7 @@ func SetupMultiTenant( // that. So a nil here is a wiring bug. return nil, nil, nil, nil, nil, nil, fmt.Errorf("trino provisioner enabled but buildTrinoWiring returned no wiring; this should be unreachable") } - provCtrl.WithTrinoProvisioner(trinoWire) + provCtrl.WithTrinoReconciler(trinoWire) trinoCells = trinoWire for _, wire := range trinoWire { slog.Info("Trino provisioner enabled.", "cell", wire.Cell.consoleCell().ID, "coordinator", wire.Cell.CoordinatorURL) diff --git a/controlplane/provisioner/controller.go b/controlplane/provisioner/controller.go index 8cec97fa..622039cf 100644 --- a/controlplane/provisioner/controller.go +++ b/controlplane/provisioner/controller.go @@ -142,12 +142,19 @@ func (c *Controller) WithBucketSuffix(suffix string) *Controller { // batched output (a handful of Secrets + one ConfigMap + one OPA bundle for // the whole cell). // -// Skipped entirely if p is nil so deployments without a Trino cell don't -// need to know about it. -func (c *Controller) WithTrinoProvisioner(p interface{ Reconcile(context.Context) error }) *Controller { +// A nil provisioner is rejected before the controller starts. +func (c *Controller) WithTrinoProvisioner(p *TrinoProvisioner) *Controller { if p == nil { panic("WithTrinoProvisioner: provisioner is nil; call NewTrinoProvisioner first") } + return c.WithTrinoReconciler(p) +} + +// WithTrinoReconciler installs the configured fleet's reconciliation entry point. +func (c *Controller) WithTrinoReconciler(p interface{ Reconcile(context.Context) error }) *Controller { + if p == nil { + panic("WithTrinoReconciler: reconciler is nil") + } c.trinoProvisioner = p return c } diff --git a/controlplane/provisioner/trino_controller_test.go b/controlplane/provisioner/trino_controller_test.go new file mode 100644 index 00000000..628d7606 --- /dev/null +++ b/controlplane/provisioner/trino_controller_test.go @@ -0,0 +1,15 @@ +//go:build kubernetes + +package provisioner + +import "testing" + +func TestTrinoControllerRejectsTypedNilProvisioner(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("typed nil provisioner must be rejected before reconciliation") + } + }() + var p *TrinoProvisioner + new(Controller).WithTrinoProvisioner(p) +} From d830cfdbc72bbe0e59949a4054df6545b33a0618 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 14:38:29 +0200 Subject: [PATCH 5/8] test: exercise isolated Trino multicell placement and green hydration --- justfile | 5 + tests/mw-dev/README.md | 37 ++++- tests/mw-dev/e2e/trino-multicell.sh | 124 +++++++++++++++ tests/mw-dev/e2e/trino.sh | 11 +- tests/mw-dev/manifests.tmpl.yaml | 1 + tests/mw-dev/run.sh | 120 ++++++++++++++- tests/mw-dev/run_sh_test.go | 24 ++- tests/mw-dev/trino-multicell.tmpl.yaml | 126 ++++++++++++++++ tests/mw-dev/trino_multicell_test.go | 200 +++++++++++++++++++++++++ 9 files changed, 630 insertions(+), 18 deletions(-) create mode 100644 tests/mw-dev/e2e/trino-multicell.sh create mode 100644 tests/mw-dev/trino-multicell.tmpl.yaml create mode 100644 tests/mw-dev/trino_multicell_test.go diff --git a/justfile b/justfile index 1bcdb06f..ba0cd9ce 100644 --- a/justfile +++ b/justfile @@ -342,6 +342,11 @@ test-controlplane-k8s: test-trino-admin: go test -v -count=1 -tags kubernetes -run Trino ./controlplane/admin ./tests/configstore +# Test isolated deployment fixtures without contacting a cluster. +[group('test')] +test-mw-fixtures: + go test -v -count=1 ./tests/mw-dev + # Print the test impact plan for the current branch [group('test')] test-impact-plan base="origin/main" head="HEAD": diff --git a/tests/mw-dev/README.md b/tests/mw-dev/README.md index fc110b03..814e7913 100644 --- a/tests/mw-dev/README.md +++ b/tests/mw-dev/README.md @@ -518,6 +518,41 @@ normal `go test ./...` lane. ## Isolation model +### Trino multicell lane + +The full Trino E2E lane adds a second disposable namespace, +`duckgres-ci-pr-0`, alongside the canonical `duckgres-ci-pr-` identity. +Canonical lane identities must be positive numbers without a leading zero. +The secondary namespace carries the original lane label and a `trino-cell` +component label. It owns no control plane, config-store, or Duckling resources. +Performance scenarios keep the existing single-cell fixture and resource budget. + +The additional `cell-test` starts with one blue coordinator and one worker; +green remains at zero replicas. Both colors use distinct internal credentials, +node environments, discovery Services, and catalog-store keys. Their shared +cell-local auth, tenant-password, OPA, and resource-group projections are +managed by the primary control plane through a scoped RoleBinding. + +`e2e/trino-multicell.sh` provisions a new warehouse without Trino, selects its +initial cell, enables Trino, and verifies real DuckLake writes and reads. It +asserts that legacy remains queryable, credentials and OPA tokens cannot cross +cells, and an existing legacy assignment cannot be changed. It then starts +green, updates the startup-loaded registry, restarts the control plane, and +queries the same data through green's independently hydrated catalog. Direct +coordinator URLs are fixture-only; this does not test Gateway routing or a +maintenance move of an existing warehouse. + +Run `just test-mw-fixtures` for local rendering and cleanup guard tests. The +real acceptance gate is the PR's Trino E2E workflow. A rendered fixture is not +proof that CI has the required cross-namespace RBAC and Pod Identity grants. +On failure, keep the PR in draft and inspect its isolated job/deployment logs. +Do not redirect the suite to an existing shared cell. + +Normal reset/teardown and stale cleanup delete the secondary namespace only +after its name, original lane label, component label, and UID match. Namespace +deletion uses a UID precondition. Secondary cleanup removes its Pod Identity +association but never independently deletes the primary lane's warehouses. + Dedicated CP + throwaway config-store **per e2e lane**, provisioning three **real** CNPG-backed ducklings (org IDs `ci-pr--cnpg` plus the ducklake-only resilience-lane orgs `ci-pr--res1`/`-res2`) through the **shared** @@ -672,7 +707,7 @@ pod-identity calls. Both images point at the same all-in-one ref. ```sh IMG=/duckgres: AWS_PROFILE=mw-dev \ -NAMESPACE=duckgres-ci-pr-0 PR_NUMBER=0 KUBE_CONTEXT=posthog-mw-dev \ +NAMESPACE="duckgres-ci-pr-${LANE_ID:?}" PR_NUMBER="$LANE_ID" KUBE_CONTEXT=posthog-mw-dev \ WORKER_IMAGE=$IMG CONTROLPLANE_IMAGE=$IMG \ CP_POD_IDENTITY_ROLE=arn:aws:iam:::role/duckgres-control-plane-dev \ bash tests/mw-dev/run.sh deploy diff --git a/tests/mw-dev/e2e/trino-multicell.sh b/tests/mw-dev/e2e/trino-multicell.sh new file mode 100644 index 00000000..bc36870e --- /dev/null +++ b/tests/mw-dev/e2e/trino-multicell.sh @@ -0,0 +1,124 @@ +#!/bin/sh +# The main Trino harness supplies authenticated API and SQL helpers. +CELL_NS="${TRINO_CELL_NAMESPACE:?}" +ORG_C="ci-pr-${PR}-trinoc" +DB_C="trino-c-${PR}" +CAT_C="org_$(printf %s "$DB_C" | tr '-' '_')" +LEGACY_TRINO="$TRINO" +BLUE_TRINO="https://duckgres-trino-blue.$CELL_NS.svc:8443" +GREEN_TRINO="https://duckgres-trino-green.$CELL_NS.svc:8443" + +log "multicell initial placement with green stopped" +api "$API/api/v1/trino/cells" | jq -e '.cells | map(.id) | sort == ["cell-test","legacy"]' >/dev/null \ + || fail "both cells must be registered" +for target in coordinator worker; do + "$KUBECTL" -n "$CELL_NS" get deployment "duckgres-trino-green-$target" -o json \ + | jq -e '.spec.replicas == 0 and (.status.replicas // 0) == 0' >/dev/null \ + || fail "green must start stopped" +done +blue_internal="$("$KUBECTL" -n "$CELL_NS" get secret trino-blue-internal -o json | jq -r '.data["shared-secret"]')" +green_internal="$("$KUBECTL" -n "$CELL_NS" get secret trino-green-internal -o json | jq -r '.data["shared-secret"]')" +[ -n "$blue_internal" ] && [ "$blue_internal" != "$green_internal" ] || fail "backend internal secrets must differ" + +pw_c="$(api -X POST -H 'Content-Type: application/json' \ + -d '{"database_name":"'"$DB_C"'","team_id":93003,"metadata_store":{"type":"cnpg-shard"},"data_store":{"type":"s3bucket"},"ducklake":{"enabled":true},"trino":{"enabled":false}}' \ + "$API/api/v1/orgs/$ORG_C/provision" | jq -r .password)" +[ -n "$pw_c" ] && [ "$pw_c" != null ] || fail "new warehouse returned no password" +wait_warehouse "$ORG_C" +bootstrap_ducklake "$ORG_C" "$pw_c" +api -X PUT -H 'Content-Type: application/json' -d '{"cell":"cell-test"}' \ + "$API/api/v1/orgs/$ORG_C/trino/cell" | jq -e '.assigned == true and .cell.id == "cell-test"' >/dev/null \ + || fail "initial cell selection failed" +api "$API/api/v1/orgs/$ORG_C/trino" | jq -e '.enabled == false and .assigned == true and .cell.id == "cell-test"' >/dev/null \ + || fail "selection must not enable Trino" +api -X POST -H 'Content-Type: application/json' -d '{"enabled":true,"tier":"free"}' "$API/api/v1/orgs/$ORG_C/trino" >/dev/null + +wait_cell_ready() { + attempt=0 + while [ "$attempt" -lt 120 ]; do + result="$(api "$API/api/v1/orgs/$ORG_C/trino" 2>/dev/null || true)" + if printf %s "$result" | jq -e --arg principal "$DB_C" --arg catalog "$CAT_C" \ + '.enabled == true and .available == true and .cell.id == "cell-test" and .status.cell == "cell-test" and .status.state == "ready" and .status.principal == $principal and .status.catalog == $catalog' >/dev/null 2>&1; then + return 0 + fi + sleep 5 + attempt=$((attempt + 1)) + done + fail "registered cell did not reconcile tenant readiness" +} +wait_cell_ready +wait_cell_auth() { + attempt=0 + while [ "$attempt" -lt 36 ]; do + if catalogs="$(trino_query "$DB_C" "$pw_c" 'SHOW CATALOGS' 2>/dev/null)" && \ + printf %s "$catalogs" | jq -e --arg catalog "$CAT_C" 'any(.[]; .[0] == $catalog)' >/dev/null; then + return 0 + fi + sleep 5 + attempt=$((attempt + 1)) + done + fail "tenant auth and OPA catalog visibility did not converge" +} +api "$API/api/v1/orgs/$ORG_C" | jq -e '.trino.trino_cell_id == "registered:cell-test"' >/dev/null \ + || fail "new logical cell did not preserve its distinct stored ownership" +code="$(curl --connect-timeout 5 --max-time 30 -sS -o /tmp/trino-cell-selection-error -w '%{http_code}' -H "$H" -H 'Content-Type: application/json' \ + -X PUT -d '{"cell":"cell-test"}' "$API/api/v1/orgs/$ORG_A/trino/cell")" +[ "$code" = 409 ] || fail "existing legacy ownership must reject a cell move" + +TRINO="$BLUE_TRINO" +wait_cell_auth +trino_query "$DB_C" "$pw_c" "CREATE SCHEMA $CAT_C.cell_test" >/dev/null +trino_query "$DB_C" "$pw_c" "CREATE TABLE $CAT_C.cell_test.values_test (value BIGINT)" >/dev/null +trino_query "$DB_C" "$pw_c" "INSERT INTO $CAT_C.cell_test.values_test VALUES (7),(11)" >/dev/null +result="$(trino_query "$DB_C" "$pw_c" "SELECT COUNT(*), SUM(value) FROM $CAT_C.cell_test.values_test")" +[ "$result" = '[[2,18]]' ] || fail "new cell did not query real DuckLake data" +must_fail "$DB_A" "$pw_a" 'SELECT 1' '401|Unauthorized|Authentication|credentials' +TRINO="$LEGACY_TRINO" +log "legacy remains queryable" +[ "$(trino_query "$DB_A" "$pw_a" 'SELECT 1')" = '[[1]]' ] || fail "legacy query regressed" +must_fail "$DB_C" "$pw_c" 'SELECT 1' '401|Unauthorized|Authentication|credentials' + +legacy_token="$("$KUBECTL" -n "$NS" get secret trino-opa-bundle-token -o json | jq -r '.data.token' | base64 -d)" +cell_token="$("$KUBECTL" -n "$CELL_NS" get secret trino-opa-bundle-token -o json | jq -r '.data.token' | base64 -d)" +for pair in "legacy-to-cell" "cell-to-legacy"; do + token="$legacy_token" path=/bundles/trino/cell-test + if [ "$pair" = cell-to-legacy ]; then token="$cell_token"; path=/bundles/trino; fi + code="$(curl --connect-timeout 5 --max-time 30 -sS -o /dev/null -w '%{http_code}' -H "Authorization: Bearer $token" "$API$path")" + [ "$code" = 401 ] || [ "$code" = 403 ] || fail "OPA token crossed cell boundary" +done + +log "green catalog hydration" +registry="$("$KUBECTL" -n "$NS" get configmap trino-cell-registry -o json \ + | jq -r '.data["cells.json"]' | jq '.cells[0].backends |= map(if .id == "green" then .running=true else . end)')" +patch="$(printf %s "$registry" | jq -Rs '{data:{"cells.json":.}}')" +"$KUBECTL" -n "$NS" patch configmap trino-cell-registry --type=merge -p "$patch" >/dev/null +"$KUBECTL" -n "$NS" rollout restart deployment/duckgres-control-plane >/dev/null +"$KUBECTL" -n "$NS" rollout status deployment/duckgres-control-plane --timeout=180s >/dev/null +for target in coordinator worker; do + "$KUBECTL" -n "$CELL_NS" patch deployment "duckgres-trino-green-$target" --type=merge -p '{"spec":{"replicas":1}}' >/dev/null +done +# Named reads avoid namespace-wide Deployment list permissions. +attempt=0 +while [ "$attempt" -lt 90 ]; do + ready=1 + for target in coordinator worker; do + "$KUBECTL" -n "$CELL_NS" get deployment "duckgres-trino-green-$target" -o json \ + | jq -e '.status.observedGeneration == .metadata.generation and .status.readyReplicas == 1 and .status.updatedReplicas == 1' >/dev/null \ + || ready=0 + done + [ "$ready" = 1 ] && break + sleep 5 + attempt=$((attempt + 1)) +done +[ "$ready" = 1 ] || fail "green did not become ready" +wait_cell_ready +TRINO="$GREEN_TRINO" +wait_cell_auth +result="$(trino_query "$DB_C" "$pw_c" "SELECT COUNT(*), SUM(value) FROM $CAT_C.cell_test.values_test")" +[ "$result" = '[[2,18]]' ] || fail "green failed to hydrate its independent catalog from the same DuckLake warehouse" +must_fail "$DB_A" "$pw_a" 'SELECT 1' '401|Unauthorized|Authentication|credentials' +TRINO="$BLUE_TRINO" +[ "$(trino_query "$DB_C" "$pw_c" "SELECT COUNT(*) FROM $CAT_C.cell_test.values_test")" = '[[2]]' ] || fail "blue stopped serving during green hydration" +TRINO="$LEGACY_TRINO" +[ "$(trino_query "$DB_A" "$pw_a" 'SELECT 1')" = '[[1]]' ] || fail "legacy failed during green hydration" +log "PASS: initial placement + stopped green + isolated credentials/OPA + real DuckLake queries + green hydration" diff --git a/tests/mw-dev/e2e/trino.sh b/tests/mw-dev/e2e/trino.sh index 7424094f..5140167f 100644 --- a/tests/mw-dev/e2e/trino.sh +++ b/tests/mw-dev/e2e/trino.sh @@ -41,7 +41,7 @@ curl -fsSLo "$KUBECTL" "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/a chmod +x "$KUBECTL" "$KUBECTL" version --client >/dev/null || fail "pinned kubectl bootstrap failed" -api() { curl -fsS -H "$H" "$@"; } +api() { curl --connect-timeout 5 --max-time 60 -fsS -H "$H" "$@"; } # A fresh managed warehouse has an empty metadata database. DuckLake's Trino # connector consumes an existing DuckLake catalog; it does not create the @@ -113,7 +113,7 @@ wait_trino() { # org expected-principal expected-catalog # nextUri; every follow-up keeps both Basic auth and the tenant identity. trino_query() { # principal password sql principal="$1" password="$2" sql="$3" - response="$(curl --cacert "$CA" -fsS --user "$principal:$password" \ + response="$(curl --connect-timeout 5 --max-time 60 --cacert "$CA" -fsS --user "$principal:$password" \ -H "X-Trino-User: $principal" -H 'X-Trino-Time-Zone: UTC' \ --data-binary "$sql" "$TRINO/v1/statement")" || return 1 rows='[]' @@ -123,7 +123,7 @@ trino_query() { # principal password sql rows="$(printf %s "$response" | jq -c --argjson rows "$rows" '$rows + (.data // [])')" next="$(printf %s "$response" | jq -r '.nextUri // empty')" [ -n "$next" ] || break - response="$(curl --cacert "$CA" -fsS --user "$principal:$password" \ + response="$(curl --connect-timeout 5 --max-time 60 --cacert "$CA" -fsS --user "$principal:$password" \ -H "X-Trino-User: $principal" -H 'X-Trino-Time-Zone: UTC' "$next")" || return 1 done printf '%s\n' "$rows" @@ -311,4 +311,7 @@ trino_query "$DB_A" "$pw_a" "DROP TABLE $CAT_A.$schema.$table" >/dev/null trino_query "$DB_A" "$pw_a" "DROP TABLE $CAT_A.$schema.$scratch" >/dev/null trino_query "$DB_A" "$pw_a" "DROP TABLE $CAT_A.$schema.$writes" >/dev/null trino_query "$DB_A" "$pw_a" "DROP SCHEMA $CAT_A.$schema" >/dev/null -log "PASS: isolated Trino provisioning + verified auth + DDL/DML + OPA isolation/batching + hot-add + admin + rotation + restart + disable" +if [ "${TRINO_MULTICELL_ENABLED:-false}" = true ]; then + . /harness/trino-multicell.sh +fi +log "PASS: isolated Trino provisioning + verified auth + DDL/DML + OPA isolation/batching + hot-add + admin + rotation + restart + disable + multicell" diff --git a/tests/mw-dev/manifests.tmpl.yaml b/tests/mw-dev/manifests.tmpl.yaml index bef9ac38..cac67224 100644 --- a/tests/mw-dev/manifests.tmpl.yaml +++ b/tests/mw-dev/manifests.tmpl.yaml @@ -231,6 +231,7 @@ rules: - cnpg-tenant-ci-pr-${PR_NUMBER}-cnpg-password - cnpg-tenant-ci-pr-${PR_NUMBER}-trinoa-password - cnpg-tenant-ci-pr-${PR_NUMBER}-trinob-password + - cnpg-tenant-ci-pr-${PR_NUMBER}-trinoc-password verbs: ["get"] --- apiVersion: rbac.authorization.k8s.io/v1 diff --git a/tests/mw-dev/run.sh b/tests/mw-dev/run.sh index 4f1ed73b..14ca02c0 100755 --- a/tests/mw-dev/run.sh +++ b/tests/mw-dev/run.sh @@ -16,6 +16,7 @@ CTX="${KUBE_CONTEXT:?}" # NAMESPACE is required by deploy|test|diagnostics|teardown but NOT by # e2e-cleanup (which discovers stale namespaces itself). Don't require it here. NS="${NAMESPACE:-}" +TRINO_CELL_NS="duckgres-ci-pr-0${PR_NUMBER:-}" KUBECTL=(kubectl --context "$CTX") EKS_CLUSTER_NAME="${EKS_CLUSTER_NAME:-posthog-mw-dev}" AWS_REGION="${AWS_REGION:-us-east-1}" @@ -67,7 +68,7 @@ trino_server_p12_file="$secret_dir/duckgres-ci-trino-server.p12" require_pr_identity() { : "${PR_NUMBER:?PR_NUMBER is required}" case "$PR_NUMBER" in - *[!0-9]*) + 0*|*[!0-9]*) echo "PR_NUMBER must be numeric, got '$PR_NUMBER'." >&2 return 2 ;; @@ -80,6 +81,65 @@ require_pr_identity() { echo "NAMESPACE '$NS' does not match PR_NUMBER '$PR_NUMBER'." >&2 return 2 fi + TRINO_CELL_NS="duckgres-ci-pr-0${PR_NUMBER}" +} + +trino_multicell_enabled() { + [ "$E2E_SUITE" = trino ] && [ "$SCENARIO_NAME" = full-suite ] +} + +render_trino_backend() { + local color="$1" + TRINO_CA_CERT_B64="$(base64 < "$trino_ca_cert_file" | tr -d '\n')" \ + TRINO_SERVER_P12_B64="$(base64 < "$trino_server_p12_file" | tr -d '\n')" \ + TRINO_IMAGE="$TRINO_IMAGE" TRINO_TLS_PASSWORD="$TRINO_TLS_PASSWORD" \ + NAMESPACE="$TRINO_CELL_NS" PR_NUMBER="$PR_NUMBER" \ + envsubst '$NAMESPACE $PR_NUMBER $TRINO_IMAGE $TRINO_TLS_PASSWORD $TRINO_CA_CERT_B64 $TRINO_SERVER_P12_B64' \ + < "$HERE/manifests.trino.tmpl.yaml" \ + | sed -e "s/duckgres-trino-coordinator/duckgres-trino-$color-coordinator/g" \ + -e "s/duckgres-trino-worker/duckgres-trino-$color-worker/g" \ + -e "s/app: duckgres-trino/app: duckgres-trino-$color/g" \ + -e "s/name: duckgres-trino$/name: duckgres-trino-$color/" \ + -e "s/duckgres-trino\.$TRINO_CELL_NS\.svc/duckgres-trino-$color.$TRINO_CELL_NS.svc/g" \ + -e "s/trino-internal-communication/trino-$color-internal/g" \ + -e "s/cell-id=ci-pr-$PR_NUMBER/cell-id=ci-pr-$PR_NUMBER-$color/" \ + -e "s/node.environment=ci_pr_$PR_NUMBER/node.environment=ci_pr_${PR_NUMBER}_$color/" \ + -e "s/duckgres-config-store\.$TRINO_CELL_NS\.svc/duckgres-config-store.$NS.svc/g" \ + -e "s/duckgres-control-plane\.$TRINO_CELL_NS\.svc/duckgres-control-plane.$NS.svc/g" \ + -e 's@resource: /bundles/trino$@resource: /bundles/trino/cell-test@' +} + +render_trino_multicell() { + local color + for color in blue green; do + [ -f "$secret_dir/trino-$color-internal" ] || (umask 077; openssl rand -base64 32 > "$secret_dir/trino-$color-internal") + done + TRINO_BLUE_INTERNAL_SECRET="$(cat "$secret_dir/trino-blue-internal")" \ + TRINO_GREEN_INTERNAL_SECRET="$(cat "$secret_dir/trino-green-internal")" \ + TRINO_CELL_NAMESPACE="$TRINO_CELL_NS" NAMESPACE="$NS" PR_NUMBER="$PR_NUMBER" \ + envsubst '$TRINO_CELL_NAMESPACE $NAMESPACE $PR_NUMBER $TRINO_BLUE_INTERNAL_SECRET $TRINO_GREEN_INTERNAL_SECRET' \ + < "$HERE/trino-multicell.tmpl.yaml" + render_trino_backend blue + render_trino_backend green +} + +trino_cell_namespace_uid() { + local target="duckgres-ci-pr-0${PR_NUMBER}" namespace_json + namespace_json="$("${KUBECTL[@]}" get namespace "$target" --ignore-not-found -o json)" || return 1 + [ -n "$namespace_json" ] || return 0 + printf %s "$namespace_json" | jq -er --arg pr "$PR_NUMBER" --arg name "$target" \ + 'select(.metadata.name == $name and .metadata.labels["app.kubernetes.io/managed-by"] == "e2e-mw-dev" and .metadata.labels["duckgres.posthog.com/ci-pr"] == $pr and .metadata.labels["duckgres.posthog.com/ci-component"] == "trino-cell") | .metadata.uid | select(type == "string" and length > 0)' +} + +delete_trino_cell_stack() { + local target="duckgres-ci-pr-0${PR_NUMBER}" uid + uid="$(trino_cell_namespace_uid)" \ + || { echo "Refusing to delete a namespace without matching Trino fixture ownership." >&2; return 1; } + [ -n "$uid" ] || return 0 + NS="$target" delete_pod_identity + jq -n --arg uid "$uid" '{apiVersion:"v1",kind:"DeleteOptions",preconditions:{uid:$uid}}' \ + | "${KUBECTL[@]}" delete --raw "/api/v1/namespaces/$target" -f - >/dev/null + "${KUBECTL[@]}" wait --for=delete namespace/"$target" --timeout=720s } render() { @@ -106,11 +166,16 @@ render() { NAMESPACE="$NS" PR_NUMBER="$PR_NUMBER" \ envsubst '$NAMESPACE $PR_NUMBER $TRINO_IMAGE $TRINO_TLS_PASSWORD $TRINO_CA_CERT_B64 $TRINO_SERVER_P12_B64' \ < "$HERE/manifests.trino.tmpl.yaml" + if trino_multicell_enabled; then render_trino_multicell; fi fi } ensure_trino_tls() { local san="duckgres-trino.$NS.svc" + local sans="DNS:$san" + if trino_multicell_enabled; then + sans="$sans,DNS:duckgres-trino-blue.$TRINO_CELL_NS.svc,DNS:duckgres-trino-green.$TRINO_CELL_NS.svc" + fi if [ -s "$trino_ca_cert_file" ] && [ -s "$trino_server_p12_file" ]; then return fi @@ -121,7 +186,7 @@ ensure_trino_tls() { -subj "/CN=duckgres-e2e-trino-ca" \ -keyout "$trino_ca_key_file" -out "$trino_ca_cert_file" >/dev/null 2>&1 openssl req -newkey rsa:2048 -nodes -subj "/CN=$san" \ - -addext "subjectAltName=DNS:$san,DNS:$san.cluster.local" \ + -addext "subjectAltName=$sans,DNS:$san.cluster.local" \ -keyout "$trino_server_key_file" -out "$trino_server_csr_file" >/dev/null 2>&1 openssl x509 -req -days 2 -sha256 \ -in "$trino_server_csr_file" -CA "$trino_ca_cert_file" -CAkey "$trino_ca_key_file" \ @@ -274,7 +339,7 @@ drop_cnpg_role() { # org-id # (harness.sh main()). Keep in sync with harness.sh. ci_orgs() { # pr-number local pr="$1" - echo "ci-pr-${pr}-cnpg ci-pr-${pr}-res1 ci-pr-${pr}-res2 ci-pr-${pr}-trinoa ci-pr-${pr}-trinob" + echo "ci-pr-${pr}-cnpg ci-pr-${pr}-res1 ci-pr-${pr}-res2 ci-pr-${pr}-trinoa ci-pr-${pr}-trinob ci-pr-${pr}-trinoc" } delete_ci_ducklings() { # pr-number @@ -325,6 +390,7 @@ reset_pr_stack() { wait_ci_ducklings_deleted "$PR_NUMBER" 300s for org in $(ci_orgs "$PR_NUMBER"); do drop_cnpg_role "$org"; done delete_pod_identity + delete_trino_cell_stack delete_ci_bindings "$PR_NUMBER" # Reshard runners intentionally get 600s to roll back safely on termination. # A cancelled workflow can leave one in that grace period, so the next run's @@ -354,6 +420,11 @@ cmd_deploy() { # Associate before admitting Trino pods: the Pod Identity agent injects # credentials only at admission and never retrofits an existing pod. ensure_trino_pod_identity + if trino_multicell_enabled; then + NS="$TRINO_CELL_NS" ensure_trino_pod_identity + "${KUBECTL[@]}" -n "$NS" patch deployment duckgres-control-plane --type=strategic -p \ + '{"spec":{"template":{"spec":{"containers":[{"name":"controlplane","env":[{"name":"DUCKGRES_TRINO_CELLS_FILE","value":"/etc/duckgres/trino-cells/cells.json"}],"volumeMounts":[{"name":"trino-cell-registry","mountPath":"/etc/duckgres/trino-cells","readOnly":true}]}],"volumes":[{"name":"trino-cell-registry","configMap":{"name":"trino-cell-registry"}}]}}}}' + fi TRINO_FILESYSTEM_CACHE_ENABLED="$TRINO_FILESYSTEM_CACHE_ENABLED" \ envsubst '$NAMESPACE $PR_NUMBER $TRINO_FILESYSTEM_CACHE_ENABLED' < "$HERE/trino-controlplane-patch.tmpl.json" \ | "${KUBECTL[@]}" -n "$NS" patch deployment duckgres-control-plane \ @@ -378,6 +449,14 @@ cmd_deploy() { --type=merge -p '{"spec":{"replicas":3}}' "${KUBECTL[@]}" -n "$NS" rollout status deploy/duckgres-trino-coordinator --timeout=300s "${KUBECTL[@]}" -n "$NS" rollout status deploy/duckgres-trino-worker --timeout=300s + if trino_multicell_enabled; then + "${KUBECTL[@]}" -n "$TRINO_CELL_NS" wait --for=create secret/trino-auth --timeout=120s + "${KUBECTL[@]}" -n "$TRINO_CELL_NS" wait --for=create configmap/trino-resource-groups --timeout=120s + "${KUBECTL[@]}" -n "$TRINO_CELL_NS" patch deployment duckgres-trino-blue-coordinator --type=merge -p '{"spec":{"replicas":1}}' + "${KUBECTL[@]}" -n "$TRINO_CELL_NS" patch deployment duckgres-trino-blue-worker --type=merge -p '{"spec":{"replicas":1}}' + "${KUBECTL[@]}" -n "$TRINO_CELL_NS" rollout status deploy/duckgres-trino-blue-coordinator --timeout=300s + "${KUBECTL[@]}" -n "$TRINO_CELL_NS" rollout status deploy/duckgres-trino-blue-worker --timeout=300s + fi fi } @@ -395,10 +474,13 @@ cmd_test_e2e() { fi "${KUBECTL[@]}" -n "$NS" create configmap duckgres-harness \ --from-file=harness.sh="$harness_file" \ + --from-file=trino-multicell.sh="$HERE/e2e/trino-multicell.sh" \ --dry-run=client -o yaml | "${KUBECTL[@]}" apply --server-side --force-conflicts -f - INTERNAL_SECRET="$(cat "$internal_secret_file")" INTERNAL_SECRET_FALLBACK="$(cat "$internal_secret_fallback_file")" + TRINO_MULTICELL_ENABLED=false + if trino_multicell_enabled; then TRINO_MULTICELL_ENABLED=true; fi "${KUBECTL[@]}" -n "$NS" delete job duckgres-harness --ignore-not-found cat </dev/null \ - | while read -r ns created; do + -o jsonpath='{range .items[*]}{.metadata.name}{" "}{.metadata.creationTimestamp}{" "}{.metadata.labels.duckgres\.posthog\.com/ci-pr}{" "}{.metadata.labels.duckgres\.posthog\.com/ci-component}{"\n"}{end}' 2>/dev/null \ + | while read -r ns created pr component; do [ -n "$ns" ] || continue + case "$pr" in ''|0*|*[!0-9]*) echo "e2e-cleanup: skip namespace without a canonical PR label: $ns"; continue ;; esac + if [ "$ns" != "duckgres-ci-pr-$pr" ] && { [ "$ns" != "duckgres-ci-pr-0$pr" ] || [ "$component" != trino-cell ]; }; then + echo "e2e-cleanup: skip unexpected namespace: $ns"; continue + fi age=$(( (now - $(date -d "$created" +%s)) / 3600 )) if [ "$age" -lt "$max_age_h" ]; then echo "e2e-cleanup: keep $ns (age ${age}h < ${max_age_h}h)"; continue fi - pr="${ns#duckgres-ci-pr-}" + if [ "$component" = trino-cell ]; then + PR_NUMBER="$pr" delete_trino_cell_stack + continue + fi echo "e2e-cleanup: reaping $ns (age ${age}h, PR $pr)" delete_ci_ducklings "$pr" wait_ci_ducklings_deleted "$pr" 300s || true for org in $(ci_orgs "$pr"); do drop_cnpg_role "$org"; done NS="$ns" delete_pod_identity + PR_NUMBER="$pr" delete_trino_cell_stack delete_ci_bindings "$pr" "${KUBECTL[@]}" delete namespace "$ns" --ignore-not-found --wait=false done diff --git a/tests/mw-dev/run_sh_test.go b/tests/mw-dev/run_sh_test.go index fed7ef8e..46d5dc6b 100644 --- a/tests/mw-dev/run_sh_test.go +++ b/tests/mw-dev/run_sh_test.go @@ -347,10 +347,10 @@ func TestTeardownFailsWhenCNPGCleanupCannotReachAPrimary(t *testing.T) { } calls := fakes.calls(t) - if got := strings.Count(calls, "get pod -l cnpg.io/cluster=shard-001,cnpg.io/instanceRole=primary"); got != 15 { - t.Fatalf("primary discovery calls = %d, want 15 (three retries for each CI org); calls:\n%s", got, calls) + if got := strings.Count(calls, "get pod -l cnpg.io/cluster=shard-001,cnpg.io/instanceRole=primary"); got != 18 { + t.Fatalf("primary discovery calls = %d, want 18 (three retries for each CI org); calls:\n%s", got, calls) } - if tt.name == "all psql executions fail" && strings.Count(calls, "exec shard-001-2 -c postgres -- psql") != 15 { + if tt.name == "all psql executions fail" && strings.Count(calls, "exec shard-001-2 -c postgres -- psql") != 18 { t.Fatalf("psql attempts were not bounded to three per CI org; calls:\n%s", calls) } if !strings.Contains(calls, "delete namespace duckgres-ci-pr-123 --ignore-not-found --wait=false") { @@ -1297,7 +1297,7 @@ func TestE2ELanesCanReadOnlyRequiredDucklingsSecrets(t *testing.T) { } wantResourceNames := "cnpg-shard-001-provisioner,cnpg-shard-002-provisioner," + "cnpg-tenant-ci-pr-123-cnpg-password," + - "cnpg-tenant-ci-pr-123-trinoa-password,cnpg-tenant-ci-pr-123-trinob-password" + "cnpg-tenant-ci-pr-123-trinoa-password,cnpg-tenant-ci-pr-123-trinob-password,cnpg-tenant-ci-pr-123-trinoc-password" if got := strings.Join(stringSlice(rule["resourceNames"]), ","); got != wantResourceNames { t.Fatalf("resourceNames = %q", got) } @@ -1853,13 +1853,21 @@ if [[ "$*" == *" -n cnpg-shards exec "* && "$*" == *" psql -U postgres -c "* ]]; exit 0 fi if [[ "$*" == *" apply -f -"* ]]; then - tee -a "$RUN_SH_TEST_CALLS" >/dev/null + if [[ -n "${RUN_SH_TEST_RENDERED:-}" ]]; then + tee -a "$RUN_SH_TEST_CALLS" "$RUN_SH_TEST_RENDERED" >/dev/null + else + tee -a "$RUN_SH_TEST_CALLS" >/dev/null + fi exit 0 fi if [[ "$*" == *" --patch-file=/dev/stdin"* ]]; then tee -a "$RUN_SH_TEST_CALLS" >/dev/null exit 0 fi +if [[ "$*" == *" delete --raw /api/v1/namespaces/"* ]]; then + tee -a "$RUN_SH_TEST_CALLS" >/dev/null + exit 0 +fi if [[ -n "${SCENARIO_DEV_FAIL_COPY:-}" && "$*" == *" cp "* ]]; then exit 1 fi @@ -1945,7 +1953,11 @@ YAML exit 0 fi if [[ "$*" == *" get ns -l app.kubernetes.io/managed-by=e2e-mw-dev "* ]]; then - printf 'duckgres-ci-pr-123 2026-01-01T00:00:00Z\n' + printf '%s\n' "${RUN_SH_TEST_NAMESPACE_INVENTORY:-duckgres-ci-pr-123 2026-01-01T00:00:00Z 123}" + exit 0 +fi +if [[ "$*" == *" get namespace duckgres-ci-pr-0123 "* ]]; then + printf '%s' "${RUN_SH_TEST_SECONDARY_NAMESPACE:-}" exit 0 fi if [[ "$*" == *" get pod -l app=duckgres-control-plane "* ]]; then diff --git a/tests/mw-dev/trino-multicell.tmpl.yaml b/tests/mw-dev/trino-multicell.tmpl.yaml new file mode 100644 index 00000000..a6ba3325 --- /dev/null +++ b/tests/mw-dev/trino-multicell.tmpl.yaml @@ -0,0 +1,126 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: ${TRINO_CELL_NAMESPACE} + labels: + app.kubernetes.io/managed-by: e2e-mw-dev + duckgres.posthog.com/ci-pr: "${PR_NUMBER}" + duckgres.posthog.com/ci-component: trino-cell +--- +apiVersion: v1 +kind: Secret +metadata: + name: trino-blue-internal + namespace: ${TRINO_CELL_NAMESPACE} +stringData: + shared-secret: "${TRINO_BLUE_INTERNAL_SECRET}" +--- +apiVersion: v1 +kind: Secret +metadata: + name: trino-green-internal + namespace: ${TRINO_CELL_NAMESPACE} +stringData: + shared-secret: "${TRINO_GREEN_INTERNAL_SECRET}" +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: trino-cell-registry + namespace: ${NAMESPACE} +data: + cells.json: | + {"cells":[{"id":"cell-test","namespace":"${TRINO_CELL_NAMESPACE}","routing_group":"cell-test","client_url":"https://duckgres-trino-blue.${TRINO_CELL_NAMESPACE}.svc:8443","backends":[{"id":"blue","coordinator_url":"https://duckgres-trino-blue.${TRINO_CELL_NAMESPACE}.svc:8443","running":true,"routing_active":true,"internal_secret_name":"trino-blue-internal"},{"id":"green","coordinator_url":"https://duckgres-trino-green.${TRINO_CELL_NAMESPACE}.svc:8443","running":false,"routing_active":false,"internal_secret_name":"trino-green-internal"}]}]} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: trino-cell-projection + namespace: ${TRINO_CELL_NAMESPACE} +rules: + - apiGroups: [""] + resources: ["secrets", "configmaps"] + verbs: ["create"] + - apiGroups: [""] + resources: ["secrets"] + resourceNames: ["trino-auth", "trino-tenant-secrets", "trino-opa-bundle-token"] + verbs: ["get", "update", "patch"] + - apiGroups: [""] + resources: ["secrets"] + resourceNames: ["trino-blue-internal", "trino-green-internal"] + verbs: ["get"] + - apiGroups: [""] + resources: ["configmaps"] + resourceNames: ["trino-resource-groups"] + verbs: ["get", "update", "patch"] + - apiGroups: ["apps"] + resources: ["deployments"] + resourceNames: ["duckgres-trino-blue-coordinator", "duckgres-trino-blue-worker", "duckgres-trino-green-coordinator", "duckgres-trino-green-worker"] + verbs: ["get", "patch", "watch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: trino-cell-projection + namespace: ${TRINO_CELL_NAMESPACE} +subjects: + - kind: ServiceAccount + name: duckgres + namespace: ${NAMESPACE} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: trino-cell-projection +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: trino-cell-harness + namespace: ${NAMESPACE} +rules: + - apiGroups: ["apps"] + resources: ["deployments"] + resourceNames: ["duckgres-control-plane"] + verbs: ["patch"] + - apiGroups: [""] + resources: ["configmaps"] + resourceNames: ["trino-cell-registry"] + verbs: ["get", "update", "patch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: trino-cell-harness + namespace: ${NAMESPACE} +subjects: + - kind: ServiceAccount + name: duckgres + namespace: ${NAMESPACE} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: trino-cell-harness +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: trino-cell-to-control-plane + namespace: ${NAMESPACE} +spec: + podSelector: + matchExpressions: + - key: app + operator: In + values: [duckgres-control-plane, duckgres-config-store] + policyTypes: [Ingress] + ingress: + - from: + - podSelector: {} + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: ${TRINO_CELL_NAMESPACE} + ports: + - { protocol: TCP, port: 5432 } + - { protocol: TCP, port: 8080 } diff --git a/tests/mw-dev/trino_multicell_test.go b/tests/mw-dev/trino_multicell_test.go new file mode 100644 index 00000000..923cfedb --- /dev/null +++ b/tests/mw-dev/trino_multicell_test.go @@ -0,0 +1,200 @@ +package e2emwdev_test + +import ( + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + utilyaml "k8s.io/apimachinery/pkg/util/yaml" +) + +func TestTrinoMulticellFixtureKeepsIndependentBackendState(t *testing.T) { + raw, err := os.ReadFile("trino-multicell.tmpl.yaml") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + `name: ${TRINO_CELL_NAMESPACE}`, `namespace: ${NAMESPACE}`, + `"id":"cell-test"`, `"running":false`, `"routing_active":false`, + `trino-blue-internal`, `trino-green-internal`, + `name: trino-cell-projection`, `resourceNames: ["trino-auth", "trino-tenant-secrets", "trino-opa-bundle-token"]`, + } { + if !strings.Contains(string(raw), want) { + t.Errorf("missing isolated fleet fixture contract %q", want) + } + } + raw, err = os.ReadFile("run.sh") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{`TRINO_CELL_NS="duckgres-ci-pr-0${PR_NUMBER}"`, `render_trino_backend`, `delete_trino_cell_stack`, `DUCKGRES_TRINO_CELLS_FILE`} { + if !strings.Contains(string(raw), want) { + t.Errorf("missing fleet lifecycle contract %q", want) + } + } +} + +func TestTrinoMulticellRenderedBackendsAreIsolated(t *testing.T) { + envsubst, err := exec.LookPath("envsubst") + if err != nil { + t.Fatal("envsubst is required to verify the real renderer") + } + fakes := newRunSHFakes(t) + writeFake(t, fakes.binDir, "envsubst", "#!/usr/bin/env bash\nexec "+envsubst+" \"$@\"\n") + secretDir := filepath.Join(filepath.Dir(fakes.binDir), "secrets") + for _, name := range []string{"duckgres-ci-trino-ca.crt", "duckgres-ci-trino-server.p12"} { + if err := os.WriteFile(filepath.Join(secretDir, name), []byte("test-tls-material"), 0o600); err != nil { + t.Fatal(err) + } + } + renderedFile := filepath.Join(t.TempDir(), "rendered.yaml") + cmd := runSHCommand(t, fakes.binDir, "deploy", "SCENARIO_DEV_ALLOW_DUCKLING_DELETE=1", "SCENARIO_NAME=full-suite", "E2E_SUITE=trino", "TRINO_POD_IDENTITY_ROLE=arn:aws:iam::123456789012:role/test-trino", "RUN_SH_TEST_RENDERED="+renderedFile) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("render/deploy: %v\n%s", err, out) + } + raw, err := os.ReadFile(renderedFile) + if err != nil { + t.Fatal(err) + } + decoder := utilyaml.NewYAMLOrJSONDecoder(strings.NewReader(string(raw)), 4096) + configs := map[string]map[string]any{} + deployments := map[string]map[string]any{} + for { + var manifest map[string]any + if err := decoder.Decode(&manifest); err == io.EOF { + break + } else if err != nil { + t.Fatalf("decode real manifests: %v", err) + } + if manifest["kind"] == "ConfigMap" { + configs[manifestName(manifest)] = manifest["data"].(map[string]any) + } + if manifest["kind"] == "Deployment" { + deployments[manifestName(manifest)] = manifest + } + } + for _, color := range []string{"blue", "green"} { + name := "duckgres-trino-" + color + config := configs[name+"-coordinator"] + if config == nil { + t.Fatalf("missing %s config", color) + } + for key, want := range map[string]string{ + "config.properties": "discovery.uri=http://" + name + ".duckgres-ci-pr-0123.svc:8080", + "node.properties": "node.environment=ci_pr_123_" + color, + "catalog-store.properties": "catalog-store.cell-id=ci-pr-123-" + color, + } { + if !strings.Contains(config[key].(string), want) { + t.Fatalf("%s %s missing %q", color, key, want) + } + } + if !strings.Contains(config["catalog-store.properties"].(string), "duckgres-config-store.duckgres-ci-pr-123.svc") { + t.Fatal("catalog store must remain in primary namespace") + } + for _, role := range []string{"coordinator", "worker"} { + deployment := deployments[name+"-"+role] + if deployment == nil { + t.Fatalf("missing %s %s", color, role) + } + spec := deployment["spec"].(map[string]any) + if spec["replicas"] != float64(0) { + t.Fatal("all Trino pods must await projection bootstrap") + } + labels := spec["selector"].(map[string]any)["matchLabels"].(map[string]any) + if labels["app"] != name { + t.Fatalf("backend selector overlaps: %+v", labels) + } + } + } + if !strings.Contains(configs["duckgres-trino-opa"]["config.yaml"].(string), "/bundles/trino/cell-test") { + t.Fatal("new cell must use its scoped OPA bundle") + } + calls := fakes.calls(t) + if strings.Contains(calls, "patch deployment duckgres-trino-green-") { + t.Fatal("deploy must not start stopped green") + } + if !strings.Contains(calls, "--namespace duckgres-ci-pr-0123 --service-account trino") { + t.Fatal("new cell must receive its own Pod Identity association") + } +} + +func TestTrinoSecondaryNamespaceCleanupFailsClosed(t *testing.T) { + for _, inventory := range []string{ + "duckgres-ci-pr-0123 2026-01-01T00:00:00Z 999 trino-cell", + "duckgres-ci-pr-123 2026-01-01T00:00:00Z", + "unrelated 2026-01-01T00:00:00Z 123", + } { + fakes := newRunSHFakes(t) + cmd := runSHCommand(t, fakes.binDir, "e2e-cleanup", "RUN_SH_TEST_NAMESPACE_INVENTORY="+inventory) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("cleanup: %v %s", err, out) + } + calls := fakes.calls(t) + if strings.Contains(calls, " delete ") || strings.Contains(calls, "aws eks") { + t.Fatalf("unowned namespace triggered cleanup: %s", calls) + } + } +} + +func TestTrinoSecondaryNamespaceCleanupRequiresOwnershipAndUID(t *testing.T) { + for _, tc := range []struct { + name, object string + allowed bool + }{ + {"owned", `{"metadata":{"name":"duckgres-ci-pr-0123","uid":"test-uid","labels":{"app.kubernetes.io/managed-by":"e2e-mw-dev","duckgres.posthog.com/ci-pr":"123","duckgres.posthog.com/ci-component":"trino-cell"}}}`, true}, + {"unowned", `{"metadata":{"name":"duckgres-ci-pr-0123","uid":"test-uid","labels":{"app.kubernetes.io/managed-by":"e2e-mw-dev","duckgres.posthog.com/ci-pr":"999","duckgres.posthog.com/ci-component":"trino-cell"}}}`, false}, + {"missing-uid", `{"metadata":{"name":"duckgres-ci-pr-0123","labels":{"app.kubernetes.io/managed-by":"e2e-mw-dev","duckgres.posthog.com/ci-pr":"123","duckgres.posthog.com/ci-component":"trino-cell"}}}`, false}, + } { + t.Run(tc.name, func(t *testing.T) { + fakes := newRunSHFakes(t) + cmd := runSHCommand(t, fakes.binDir, "e2e-cleanup", "RUN_SH_TEST_NAMESPACE_INVENTORY=duckgres-ci-pr-0123 2026-01-01T00:00:00Z 123 trino-cell", "RUN_SH_TEST_SECONDARY_NAMESPACE="+tc.object) + out, err := cmd.CombinedOutput() + if tc.allowed && err != nil { + t.Fatalf("owned cleanup failed: %v %s", err, out) + } + if !tc.allowed && err == nil { + t.Fatal("unowned cleanup must fail") + } + calls := fakes.calls(t) + if strings.Contains(calls, "delete --raw /api/v1/namespaces/duckgres-ci-pr-0123") != tc.allowed { + t.Fatalf("wrong deletion decision: %s", calls) + } + if tc.allowed && !strings.Contains(calls, `"uid": "test-uid"`) { + t.Fatal("namespace delete lost UID precondition") + } + if strings.Contains(calls, "ducklings") || strings.Contains(calls, "cnpg-shards") { + t.Fatal("secondary cleanup touched primary warehouse resources") + } + }) + } +} + +func TestTrinoNamespaceRejectsNoncanonicalPRNumbers(t *testing.T) { + for _, pr := range []string{"0", "0123", "-1", "123x"} { + fakes := newRunSHFakes(t) + cmd := runSHCommand(t, fakes.binDir, "deploy", "PR_NUMBER="+pr, "NAMESPACE=duckgres-ci-pr-"+pr) + if out, err := cmd.CombinedOutput(); err == nil { + t.Fatalf("noncanonical PR accepted: %s %s", pr, out) + } + } +} + +func TestTrinoMulticellHarnessExercisesRealPlacementAndHydration(t *testing.T) { + raw, err := os.ReadFile("e2e/trino-multicell.sh") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + `/trino/cell`, `registered:cell-test`, `green catalog hydration`, + `/bundles/trino/cell-test`, `trino-blue-internal`, `trino-green-internal`, + `legacy remains queryable`, `SELECT COUNT(*)`, + `'{"enabled":true,"tier":"free"}'`, + } { + if !strings.Contains(string(raw), want) { + t.Errorf("missing real fleet assertion %q", want) + } + } +} From 5a020e67033516e59af2c4b5e91cdfc008b9e4c6 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 14:40:19 +0200 Subject: [PATCH 6/8] test: report multicell success only when exercised --- tests/mw-dev/e2e/trino.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mw-dev/e2e/trino.sh b/tests/mw-dev/e2e/trino.sh index 5140167f..cbdf44ef 100644 --- a/tests/mw-dev/e2e/trino.sh +++ b/tests/mw-dev/e2e/trino.sh @@ -314,4 +314,4 @@ trino_query "$DB_A" "$pw_a" "DROP SCHEMA $CAT_A.$schema" >/dev/null if [ "${TRINO_MULTICELL_ENABLED:-false}" = true ]; then . /harness/trino-multicell.sh fi -log "PASS: isolated Trino provisioning + verified auth + DDL/DML + OPA isolation/batching + hot-add + admin + rotation + restart + disable + multicell" +log "PASS: isolated Trino provisioning + verified auth + DDL/DML + OPA isolation/batching + hot-add + admin + rotation + restart + disable" From 3dfd16169306bfa2d8b357be15fb5c08f6fa7cb5 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 14:57:31 +0200 Subject: [PATCH 7/8] test: preserve baseline multicell network policy permissions --- tests/mw-dev/README.md | 6 ++++++ tests/mw-dev/trino-multicell.tmpl.yaml | 23 ----------------------- tests/mw-dev/trino_multicell_test.go | 8 ++++++++ 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/tests/mw-dev/README.md b/tests/mw-dev/README.md index 814e7913..b7494633 100644 --- a/tests/mw-dev/README.md +++ b/tests/mw-dev/README.md @@ -542,6 +542,12 @@ queries the same data through green's independently hydrated catalog. Direct coordinator URLs are fixture-only; this does not test Gateway routing or a maintenance move of an existing warehouse. +The fixture preserves the existing CI network-policy posture. It does not +create network policies or add cluster-wide RBAC grants. Isolation assertions +cover application authentication and OPA authorization, not network isolation. +If an existing cluster policy blocks the fixture, investigate that policy; +do not weaken it to make the test pass. + Run `just test-mw-fixtures` for local rendering and cleanup guard tests. The real acceptance gate is the PR's Trino E2E workflow. A rendered fixture is not proof that CI has the required cross-namespace RBAC and Pod Identity grants. diff --git a/tests/mw-dev/trino-multicell.tmpl.yaml b/tests/mw-dev/trino-multicell.tmpl.yaml index a6ba3325..6e62ba31 100644 --- a/tests/mw-dev/trino-multicell.tmpl.yaml +++ b/tests/mw-dev/trino-multicell.tmpl.yaml @@ -101,26 +101,3 @@ roleRef: apiGroup: rbac.authorization.k8s.io kind: Role name: trino-cell-harness ---- -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: trino-cell-to-control-plane - namespace: ${NAMESPACE} -spec: - podSelector: - matchExpressions: - - key: app - operator: In - values: [duckgres-control-plane, duckgres-config-store] - policyTypes: [Ingress] - ingress: - - from: - - podSelector: {} - - from: - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: ${TRINO_CELL_NAMESPACE} - ports: - - { protocol: TCP, port: 5432 } - - { protocol: TCP, port: 8080 } diff --git a/tests/mw-dev/trino_multicell_test.go b/tests/mw-dev/trino_multicell_test.go index 923cfedb..eaf19a5b 100644 --- a/tests/mw-dev/trino_multicell_test.go +++ b/tests/mw-dev/trino_multicell_test.go @@ -69,6 +69,14 @@ func TestTrinoMulticellRenderedBackendsAreIsolated(t *testing.T) { } else if err != nil { t.Fatalf("decode real manifests: %v", err) } + switch manifest["kind"] { + case "ClusterRoleBinding": + if manifestName(manifest) != "duckgres-ci-pr-123-duckling-reader" { + t.Errorf("multicell renderer added cluster privileges: %s", manifestName(manifest)) + } + case "NetworkPolicy", "CiliumNetworkPolicy", "CiliumClusterwideNetworkPolicy", "ClusterRole": + t.Errorf("multicell renderer must preserve baseline network policy and cluster privileges, got %s %s", manifest["kind"], manifestName(manifest)) + } if manifest["kind"] == "ConfigMap" { configs[manifestName(manifest)] = manifest["data"].(map[string]any) } From 054d64fa92b9cedb917a5d0bdad6787dd5d709a7 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Fri, 11 Sep 2026 15:01:23 +0200 Subject: [PATCH 8/8] test: randomize isolated config-store credentials --- tests/mw-dev/README.md | 6 ++ tests/mw-dev/manifests.tmpl.yaml | 14 +++- tests/mw-dev/manifests.trino.tmpl.yaml | 2 +- tests/mw-dev/run.sh | 17 +++- tests/mw-dev/run_sh_test.go | 1 + tests/mw-dev/trino_multicell_test.go | 107 ++++++++++++++++++++++++- 6 files changed, 137 insertions(+), 10 deletions(-) diff --git a/tests/mw-dev/README.md b/tests/mw-dev/README.md index b7494633..caa6451d 100644 --- a/tests/mw-dev/README.md +++ b/tests/mw-dev/README.md @@ -548,6 +548,12 @@ cover application authentication and OPA authorization, not network isolation. If an existing cluster policy blocks the fixture, investigate that policy; do not weaken it to make the test pass. +All lanes generate a random config-store password in `DUCKGRES_CI_SECRET_DIR` +and reuse it for that run. PostgreSQL, the control plane, and benchmark Jobs +read Kubernetes Secret references; Trino receives the same password through +its catalog-store Secret. Credentials never appear as literal pod environment +values. GitHub Actions masks the generated password before deployment. + Run `just test-mw-fixtures` for local rendering and cleanup guard tests. The real acceptance gate is the PR's Trino E2E workflow. A rendered fixture is not proof that CI has the required cross-namespace RBAC and Pod Identity grants. diff --git a/tests/mw-dev/manifests.tmpl.yaml b/tests/mw-dev/manifests.tmpl.yaml index cac67224..e8ead92b 100644 --- a/tests/mw-dev/manifests.tmpl.yaml +++ b/tests/mw-dev/manifests.tmpl.yaml @@ -18,6 +18,15 @@ metadata: app.kubernetes.io/managed-by: e2e-mw-dev duckgres.posthog.com/ci-pr: "${PR_NUMBER}" --- +apiVersion: v1 +kind: Secret +metadata: + name: duckgres-config-store-credentials + namespace: ${NAMESPACE} +stringData: + password: "${CONFIG_STORE_PASSWORD}" + dsn: "postgres://duckgres:${CONFIG_STORE_PASSWORD}@duckgres-config-store.${NAMESPACE}.svc:5432/duckgres?sslmode=disable" +--- # Throwaway config-store. The namespace-scoped PVC is deleted on teardown, but # survives a config-store pod recreation during the e2e run; losing these rows # mid-harness makes the control plane forget already-provisioned orgs. @@ -55,7 +64,8 @@ spec: image: public.ecr.aws/docker/library/postgres:16-alpine env: - { name: POSTGRES_USER, value: duckgres } - - { name: POSTGRES_PASSWORD, value: duckgres } + - name: POSTGRES_PASSWORD + valueFrom: { secretKeyRef: { name: duckgres-config-store-credentials, key: password } } - { name: POSTGRES_DB, value: duckgres } - { name: PGDATA, value: /var/lib/postgresql/data/pgdata } ports: [{ containerPort: 5432 }] @@ -285,7 +295,7 @@ spec: - name: NODE_NAME valueFrom: { fieldRef: { fieldPath: spec.nodeName } } - name: DUCKGRES_CONFIG_STORE - value: "postgres://duckgres:duckgres@duckgres-config-store.${NAMESPACE}.svc:5432/duckgres?sslmode=disable" + valueFrom: { secretKeyRef: { name: duckgres-config-store-credentials, key: dsn } } - { name: DUCKGRES_K8S_WORKER_IMAGE, value: "${WORKER_IMAGE}" } - { name: DUCKGRES_K8S_WORKER_IMAGE_PULL_POLICY, value: "IfNotPresent" } # Spawn workers in THIS namespace so teardown is a single ns delete. diff --git a/tests/mw-dev/manifests.trino.tmpl.yaml b/tests/mw-dev/manifests.trino.tmpl.yaml index 5ae00abc..5657dbf0 100644 --- a/tests/mw-dev/manifests.trino.tmpl.yaml +++ b/tests/mw-dev/manifests.trino.tmpl.yaml @@ -26,7 +26,7 @@ metadata: name: duckgres-trino-catalog-store namespace: ${NAMESPACE} stringData: - password: duckgres + password: "${CONFIG_STORE_PASSWORD}" --- apiVersion: v1 kind: ConfigMap diff --git a/tests/mw-dev/run.sh b/tests/mw-dev/run.sh index 14ca02c0..ed46e5ef 100755 --- a/tests/mw-dev/run.sh +++ b/tests/mw-dev/run.sh @@ -58,6 +58,7 @@ internal_secret_fallback_file="$secret_dir/duckgres-ci-internal-secret-fallback" # AES key for user persistent secrets (DUCKGRES_USER_SECRET_KEY). Random per # run: stored user secrets only need to outlive the run's sessions. user_secret_key_file="$secret_dir/duckgres-ci-user-secret-key" +config_store_password_file="$secret_dir/duckgres-ci-config-store-password" trino_ca_key_file="$secret_dir/duckgres-ci-trino-ca.key" trino_ca_cert_file="$secret_dir/duckgres-ci-trino-ca.crt" trino_server_key_file="$secret_dir/duckgres-ci-trino-server.key" @@ -93,8 +94,9 @@ render_trino_backend() { TRINO_CA_CERT_B64="$(base64 < "$trino_ca_cert_file" | tr -d '\n')" \ TRINO_SERVER_P12_B64="$(base64 < "$trino_server_p12_file" | tr -d '\n')" \ TRINO_IMAGE="$TRINO_IMAGE" TRINO_TLS_PASSWORD="$TRINO_TLS_PASSWORD" \ + CONFIG_STORE_PASSWORD="$(cat "$config_store_password_file")" \ NAMESPACE="$TRINO_CELL_NS" PR_NUMBER="$PR_NUMBER" \ - envsubst '$NAMESPACE $PR_NUMBER $TRINO_IMAGE $TRINO_TLS_PASSWORD $TRINO_CA_CERT_B64 $TRINO_SERVER_P12_B64' \ + envsubst '$NAMESPACE $PR_NUMBER $TRINO_IMAGE $TRINO_TLS_PASSWORD $TRINO_CA_CERT_B64 $TRINO_SERVER_P12_B64 $CONFIG_STORE_PASSWORD' \ < "$HERE/manifests.trino.tmpl.yaml" \ | sed -e "s/duckgres-trino-coordinator/duckgres-trino-$color-coordinator/g" \ -e "s/duckgres-trino-worker/duckgres-trino-$color-worker/g" \ @@ -148,6 +150,11 @@ render() { [ -f "$internal_secret_file" ] || (umask 077; openssl rand -hex 16 > "$internal_secret_file") [ -f "$internal_secret_fallback_file" ] || (umask 077; openssl rand -hex 16 > "$internal_secret_fallback_file") [ -f "$user_secret_key_file" ] || (umask 077; openssl rand -base64 32 > "$user_secret_key_file") + [ -s "$config_store_password_file" ] || (umask 077; openssl rand -hex 32 > "$config_store_password_file") + if [ "${GITHUB_ACTIONS:-}" = true ]; then + printf '::add-mask::%s\n' "$(cat "$config_store_password_file")" >&2 + fi + CONFIG_STORE_PASSWORD="$(cat "$config_store_password_file")" \ INTERNAL_SECRET="$(cat "$internal_secret_file")" \ INTERNAL_SECRET_FALLBACK="$(cat "$internal_secret_fallback_file")" \ USER_SECRET_KEY="$(cat "$user_secret_key_file")" \ @@ -155,7 +162,7 @@ render() { WORKER_IMAGE="$WORKER_IMAGE" CONTROLPLANE_IMAGE="$CONTROLPLANE_IMAGE" \ DUCKGRES_K8S_WORKER_CPU_REQUEST="$DUCKGRES_K8S_WORKER_CPU_REQUEST" \ DUCKGRES_K8S_WORKER_MEMORY_REQUEST="$DUCKGRES_K8S_WORKER_MEMORY_REQUEST" \ - envsubst '$NAMESPACE $PR_NUMBER $WORKER_IMAGE $CONTROLPLANE_IMAGE $INTERNAL_SECRET $INTERNAL_SECRET_FALLBACK $USER_SECRET_KEY $DUCKGRES_K8S_WORKER_CPU_REQUEST $DUCKGRES_K8S_WORKER_MEMORY_REQUEST' \ + envsubst '$NAMESPACE $PR_NUMBER $WORKER_IMAGE $CONTROLPLANE_IMAGE $INTERNAL_SECRET $INTERNAL_SECRET_FALLBACK $USER_SECRET_KEY $DUCKGRES_K8S_WORKER_CPU_REQUEST $DUCKGRES_K8S_WORKER_MEMORY_REQUEST $CONFIG_STORE_PASSWORD' \ < "$HERE/manifests.tmpl.yaml" if [ "$E2E_SUITE" = "trino" ]; then @@ -163,8 +170,9 @@ render() { TRINO_CA_CERT_B64="$(base64 < "$trino_ca_cert_file" | tr -d '\n')" \ TRINO_SERVER_P12_B64="$(base64 < "$trino_server_p12_file" | tr -d '\n')" \ TRINO_IMAGE="$TRINO_IMAGE" TRINO_TLS_PASSWORD="$TRINO_TLS_PASSWORD" \ + CONFIG_STORE_PASSWORD="$(cat "$config_store_password_file")" \ NAMESPACE="$NS" PR_NUMBER="$PR_NUMBER" \ - envsubst '$NAMESPACE $PR_NUMBER $TRINO_IMAGE $TRINO_TLS_PASSWORD $TRINO_CA_CERT_B64 $TRINO_SERVER_P12_B64' \ + envsubst '$NAMESPACE $PR_NUMBER $TRINO_IMAGE $TRINO_TLS_PASSWORD $TRINO_CA_CERT_B64 $TRINO_SERVER_P12_B64 $CONFIG_STORE_PASSWORD' \ < "$HERE/manifests.trino.tmpl.yaml" if trino_multicell_enabled; then render_trino_multicell; fi fi @@ -644,7 +652,8 @@ spec: - { name: DUCKGRES_SCENARIO_FROZEN_S3_URI, value: "$FROZEN_S3_URI" } - { name: DUCKGRES_SCENARIO_TRINO_CA_CERT, value: "/trino-ca/ca.crt" } # Only the throwaway benchmark config store; never a shared dev/prod store. - - { name: DUCKGRES_SCENARIO_TRINO_CATALOG_STORE_DSN, value: "postgres://duckgres:duckgres@duckgres-config-store.$NS.svc:5432/duckgres?sslmode=disable" } + - name: DUCKGRES_SCENARIO_TRINO_CATALOG_STORE_DSN + valueFrom: { secretKeyRef: { name: duckgres-config-store-credentials, key: dsn } } - { name: DUCKGRES_SCENARIO_ATHENA_REGION, value: "$AWS_REGION" } - { name: DUCKGRES_SCENARIO_ATHENA_WORKGROUP, value: "${DUCKGRES_SCENARIO_ATHENA_WORKGROUP:-}" } - { name: DUCKGRES_SCENARIO_ATHENA_DATABASE, value: "${DUCKGRES_SCENARIO_ATHENA_DATABASE:-}" } diff --git a/tests/mw-dev/run_sh_test.go b/tests/mw-dev/run_sh_test.go index 46d5dc6b..6dfcc896 100644 --- a/tests/mw-dev/run_sh_test.go +++ b/tests/mw-dev/run_sh_test.go @@ -444,6 +444,7 @@ func TestScenarioRunsSelectedScenarioAgainstIsolatedStack(t *testing.T) { "value: \"isolated-test-secret\"", "name: DUCKGRES_SCENARIO_ORG_ID, value: \"ci-pr-123-cnpg\"", "name: DUCKGRES_SCENARIO_TRINO_CA_CERT, value: \"/trino-ca/ca.crt\"", + "name: DUCKGRES_SCENARIO_TRINO_CATALOG_STORE_DSN\n valueFrom: { secretKeyRef: { name: duckgres-config-store-credentials, key: dsn } }", "name: DUCKGRES_SCENARIO_ATHENA_REGION, value: \"us-east-1\"", "name: DUCKGRES_SCENARIO_ATHENA_WORKGROUP, value: \"benchmark\"", "name: DUCKGRES_SCENARIO_ATHENA_DATABASE, value: \"benchmark_frozen\"", diff --git a/tests/mw-dev/trino_multicell_test.go b/tests/mw-dev/trino_multicell_test.go index eaf19a5b..3c57b24f 100644 --- a/tests/mw-dev/trino_multicell_test.go +++ b/tests/mw-dev/trino_multicell_test.go @@ -1,6 +1,7 @@ package e2emwdev_test import ( + "encoding/json" "io" "os" "os/exec" @@ -44,6 +45,11 @@ func TestTrinoMulticellRenderedBackendsAreIsolated(t *testing.T) { } fakes := newRunSHFakes(t) writeFake(t, fakes.binDir, "envsubst", "#!/usr/bin/env bash\nexec "+envsubst+" \"$@\"\n") + openssl, err := exec.LookPath("openssl") + if err != nil { + t.Fatal(err) + } + writeFake(t, fakes.binDir, "openssl", "#!/usr/bin/env bash\nexec "+openssl+" \"$@\"\n") secretDir := filepath.Join(filepath.Dir(fakes.binDir), "secrets") for _, name := range []string{"duckgres-ci-trino-ca.crt", "duckgres-ci-trino-server.p12"} { if err := os.WriteFile(filepath.Join(secretDir, name), []byte("test-tls-material"), 0o600); err != nil { @@ -51,9 +57,10 @@ func TestTrinoMulticellRenderedBackendsAreIsolated(t *testing.T) { } } renderedFile := filepath.Join(t.TempDir(), "rendered.yaml") - cmd := runSHCommand(t, fakes.binDir, "deploy", "SCENARIO_DEV_ALLOW_DUCKLING_DELETE=1", "SCENARIO_NAME=full-suite", "E2E_SUITE=trino", "TRINO_POD_IDENTITY_ROLE=arn:aws:iam::123456789012:role/test-trino", "RUN_SH_TEST_RENDERED="+renderedFile) - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("render/deploy: %v\n%s", err, out) + cmd := runSHCommand(t, fakes.binDir, "deploy", "SCENARIO_DEV_ALLOW_DUCKLING_DELETE=1", "SCENARIO_NAME=full-suite", "E2E_SUITE=trino", "TRINO_POD_IDENTITY_ROLE=arn:aws:iam::123456789012:role/test-trino", "RUN_SH_TEST_RENDERED="+renderedFile, "GITHUB_ACTIONS=true") + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("render/deploy: %v\n%s", err, output) } raw, err := os.ReadFile(renderedFile) if err != nil { @@ -62,6 +69,8 @@ func TestTrinoMulticellRenderedBackendsAreIsolated(t *testing.T) { decoder := utilyaml.NewYAMLOrJSONDecoder(strings.NewReader(string(raw)), 4096) configs := map[string]map[string]any{} deployments := map[string]map[string]any{} + secrets := []map[string]any{} + publicManifests := []map[string]any{} for { var manifest map[string]any if err := decoder.Decode(&manifest); err == io.EOF { @@ -83,6 +92,98 @@ func TestTrinoMulticellRenderedBackendsAreIsolated(t *testing.T) { if manifest["kind"] == "Deployment" { deployments[manifestName(manifest)] = manifest } + if manifest["kind"] == "Secret" { + secrets = append(secrets, manifest) + } else { + publicManifests = append(publicManifests, manifest) + } + } + passwordBytes, err := os.ReadFile(filepath.Join(secretDir, "duckgres-ci-config-store-password")) + if err != nil { + t.Fatal("renderer must generate a per-run config-store credential:", err) + } + password := strings.TrimSpace(string(passwordBytes)) + if len(password) != 64 || strings.Trim(password, "0123456789abcdef") != "" { + t.Fatal("config store must use a random, URL-safe 32-byte password") + } + mask := "::add-mask::" + password + "\n" + if !strings.Contains(string(output), mask) || strings.Contains(strings.ReplaceAll(string(output), mask, ""), password) { + t.Fatal("password must only appear in the GitHub masking directive") + } + info, err := os.Stat(filepath.Join(secretDir, "duckgres-ci-config-store-password")) + if err != nil || info.Mode().Perm() != 0o600 { + t.Fatal("local credential must be owner-readable only") + } + credentialCount := 0 + for _, secret := range secrets { + switch manifestName(secret) { + case "duckgres-config-store-credentials", "duckgres-trino-catalog-store": + credentialCount++ + data := secret["stringData"].(map[string]any) + if data["password"] != password { + t.Fatal("config-store credentials differ between consumers") + } + if manifestName(secret) == "duckgres-config-store-credentials" && data["dsn"] != "postgres://duckgres:"+password+"@duckgres-config-store.duckgres-ci-pr-123.svc:5432/duckgres?sslmode=disable" { + t.Fatal("control-plane DSN does not match config-store password") + } + } + } + if credentialCount != 4 { + t.Fatalf("expected primary credential and three catalog-store Secrets, got %d", credentialCount) + } + for _, manifest := range publicManifests { + encoded, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(encoded), password) { + t.Fatalf("credential leaked into non-Secret %s %s", manifest["kind"], manifestName(manifest)) + } + } + for deploymentName, envName := range map[string]string{ + "duckgres-config-store": "POSTGRES_PASSWORD", "duckgres-control-plane": "DUCKGRES_CONFIG_STORE", + } { + deployment := deployments[deploymentName] + containers := deployment["spec"].(map[string]any)["template"].(map[string]any)["spec"].(map[string]any)["containers"].([]any) + found := false + for _, container := range containers { + for _, value := range container.(map[string]any)["env"].([]any) { + env := value.(map[string]any) + if env["name"] != envName { + continue + } + found = true + ref := env["valueFrom"].(map[string]any)["secretKeyRef"].(map[string]any) + key := "dsn" + if envName == "POSTGRES_PASSWORD" { + key = "password" + } + if ref["name"] != "duckgres-config-store-credentials" || ref["key"] != key { + t.Fatalf("incorrect credential reference for %s", envName) + } + } + } + if !found { + t.Fatalf("missing credential reference for %s", envName) + } + } + for _, sameRun := range []bool{true, false} { + repeatFakes := fakes + if !sameRun { + repeatFakes = newRunSHFakes(t) + writeFake(t, repeatFakes.binDir, "openssl", "#!/usr/bin/env bash\nexec "+openssl+" \"$@\"\n") + } + cmd := runSHCommand(t, repeatFakes.binDir, "deploy", "SCENARIO_DEV_ALLOW_DUCKLING_DELETE=1", "GITHUB_ACTIONS=false") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("repeat render: %v %s", err, out) + } + repeated, err := os.ReadFile(filepath.Join(filepath.Dir(repeatFakes.binDir), "secrets", "duckgres-ci-config-store-password")) + if err != nil { + t.Fatal(err) + } + if (strings.TrimSpace(string(repeated)) == password) != sameRun { + t.Fatal("credential must be stable within a run and different across fresh runs") + } } for _, color := range []string{"blue", "green"} { name := "duckgres-trino-" + color