From c363258cc1dbf524c019deb4b38ff6f1637c05dc Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Mon, 14 Sep 2026 16:33:38 +0200 Subject: [PATCH 1/7] Add durable Trino cell catalog lifecycle primitives --- .../000039_add_trino_cell_lifecycle.sql | 21 ++ .../configstore/trino_cell_lifecycle.go | 334 ++++++++++++++++++ tests/configstore/migrations_postgres_test.go | 15 +- .../configstore/trino_cell_lifecycle_test.go | 182 ++++++++++ 4 files changed, 548 insertions(+), 4 deletions(-) create mode 100644 controlplane/configstore/migrations/000039_add_trino_cell_lifecycle.sql create mode 100644 controlplane/configstore/trino_cell_lifecycle.go create mode 100644 tests/configstore/trino_cell_lifecycle_test.go diff --git a/controlplane/configstore/migrations/000039_add_trino_cell_lifecycle.sql b/controlplane/configstore/migrations/000039_add_trino_cell_lifecycle.sql new file mode 100644 index 00000000..7260dce6 --- /dev/null +++ b/controlplane/configstore/migrations/000039_add_trino_cell_lifecycle.sql @@ -0,0 +1,21 @@ +-- +goose Up +-- Ownership has no timeout. Unknown remote mutation outcomes require recovery. +CREATE TABLE duckgres_trino_cell_lifecycle ( + cell_id TEXT PRIMARY KEY, + reconcile_owner TEXT NOT NULL DEFAULT '', + reconcile_epoch BIGINT NOT NULL DEFAULT 0 CHECK (reconcile_epoch >= 0), + intent_sequence BIGINT NOT NULL DEFAULT 0 CHECK (intent_sequence >= 0), + intent JSONB NOT NULL DEFAULT '{}' CHECK (jsonb_typeof(intent) = 'object'), + admission_epoch BIGINT NOT NULL DEFAULT 0 CHECK (admission_epoch >= 0), + freeze_operation_id TEXT NOT NULL DEFAULT '', + freeze_plan_hash TEXT NOT NULL DEFAULT '', + freeze_target TEXT NOT NULL DEFAULT '', + freeze_stable BOOLEAN NOT NULL DEFAULT FALSE, + certificate JSONB NOT NULL DEFAULT '{}' CHECK (jsonb_typeof(certificate) = 'object'), + released_operation_id TEXT NOT NULL DEFAULT '', + released_admission_epoch BIGINT NOT NULL DEFAULT 0, + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- +goose Down +DROP TABLE duckgres_trino_cell_lifecycle; diff --git a/controlplane/configstore/trino_cell_lifecycle.go b/controlplane/configstore/trino_cell_lifecycle.go new file mode 100644 index 00000000..e3a059c3 --- /dev/null +++ b/controlplane/configstore/trino_cell_lifecycle.go @@ -0,0 +1,334 @@ +package configstore + +import ( + "context" + "encoding/json" + "errors" + "regexp" + "strings" + "time" + "unicode" + + "gorm.io/gorm" + "gorm.io/gorm/clause" +) + +var ErrTrinoCellConflict = errors.New("trino cell lifecycle conflict") +var trinoCellHashPattern = regexp.MustCompile(`^[a-f0-9]{64}$`) + +type TrinoCellLease struct { + CellID string + Owner string + ReconcileEpoch int64 + AdmissionEpoch int64 + IntentSequence int64 +} + +type TrinoCatalogIntent struct { + Sequence int64 `json:"sequence"` + ID string `json:"id"` + Backend string `json:"backend"` + Action string `json:"action"` + Catalog string `json:"catalog"` +} + +type TrinoCellCertificate struct { + TargetBackend string `json:"targetBackend"` + NodeID string `json:"nodeId"` + CoordinatorID string `json:"coordinatorId"` + RosterHash string `json:"rosterHash"` + AdmittedCount int `json:"admittedCount"` +} + +type TrinoCellFreeze struct { + OperationID string `json:"operationId"` + PlanHash string `json:"planHash"` + TargetBackend string `json:"targetBackend"` + AdmissionEpoch int64 `json:"admissionEpoch"` + Certificate *TrinoCellCertificate `json:"certificate,omitempty"` + Stable bool `json:"stable"` +} + +type trinoCellLifecycle struct { + CellID string `gorm:"primaryKey"` + ReconcileOwner string + ReconcileEpoch int64 + IntentSequence int64 + Intent string `gorm:"type:jsonb"` + AdmissionEpoch int64 + FreezeOperationID string + FreezePlanHash string + FreezeTarget string + FreezeStable bool + Certificate string `gorm:"type:jsonb"` + ReleasedOperationID string + ReleasedAdmissionEpoch int64 + UpdatedAt time.Time +} + +func (trinoCellLifecycle) TableName() string { return "duckgres_trino_cell_lifecycle" } + +func validTrinoCellValue(s string) bool { + return s != "" && len(s) <= 255 && strings.IndexFunc(s, unicode.IsControl) == -1 +} + +func (cs *ConfigStore) withTrinoCell(ctx context.Context, cell string, fn func(*gorm.DB, *trinoCellLifecycle) error) error { + if !strings.HasPrefix(cell, "registered:") || !validTrinoCellValue(cell) { + return ErrTrinoCellConflict + } + return cs.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + seed := trinoCellLifecycle{CellID: cell, Intent: "{}", Certificate: "{}", UpdatedAt: time.Now().UTC()} + if err := tx.Clauses(clause.OnConflict{DoNothing: true}).Create(&seed).Error; err != nil { + return err + } + var row trinoCellLifecycle + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).First(&row, "cell_id = ?", cell).Error; err != nil { + return err + } + return fn(tx, &row) + }) +} + +func saveTrinoCell(tx *gorm.DB, row *trinoCellLifecycle) error { + row.UpdatedAt = time.Now().UTC() + return tx.Save(row).Error +} + +func ownsTrinoCell(row *trinoCellLifecycle, lease TrinoCellLease) bool { + return row.CellID == lease.CellID && row.ReconcileOwner != "" && row.ReconcileOwner == lease.Owner && row.ReconcileEpoch == lease.ReconcileEpoch +} + +// BeginTrinoCellReconcile must precede the tenant and Gateway snapshots. +// A held owner never expires, including after a controller process disappears. +func (cs *ConfigStore) BeginTrinoCellReconcile(ctx context.Context, cell, owner string) (*TrinoCellLease, bool, error) { + if !validTrinoCellValue(owner) { + return nil, false, ErrTrinoCellConflict + } + var lease *TrinoCellLease + err := cs.withTrinoCell(ctx, cell, func(tx *gorm.DB, row *trinoCellLifecycle) error { + if row.ReconcileOwner != "" { + return nil + } + row.ReconcileOwner = owner + row.ReconcileEpoch++ + if err := saveTrinoCell(tx, row); err != nil { + return err + } + lease = &TrinoCellLease{CellID: cell, Owner: owner, ReconcileEpoch: row.ReconcileEpoch, AdmissionEpoch: row.AdmissionEpoch, IntentSequence: row.IntentSequence} + return nil + }) + return lease, lease != nil && err == nil, err +} + +// SetTrinoCellIntent authorizes one submission only after its durable commit. +// A duplicate call is a conflict even when its payload is identical. +func (cs *ConfigStore) SetTrinoCellIntent(ctx context.Context, lease TrinoCellLease, intent TrinoCatalogIntent) error { + if !validTrinoCellValue(intent.ID) || !validTrinoCellValue(intent.Backend) || !validTrinoCellValue(intent.Catalog) || (intent.Action != "create" && intent.Action != "drop") { + return ErrTrinoCellConflict + } + encoded, err := json.Marshal(intent) + if err != nil { + return err + } + return cs.withTrinoCell(ctx, lease.CellID, func(tx *gorm.DB, row *trinoCellLifecycle) error { + if !ownsTrinoCell(row, lease) || row.Intent != "{}" || row.FreezeOperationID != "" || row.AdmissionEpoch != lease.AdmissionEpoch || intent.Sequence <= 0 || intent.Sequence != row.IntentSequence+1 { + return ErrTrinoCellConflict + } + row.Intent = string(encoded) + row.IntentSequence = intent.Sequence + return saveTrinoCell(tx, row) + }) +} + +// ClearTrinoCellIntent requires a confirmed terminal response for this intent. +func (cs *ConfigStore) ClearTrinoCellIntent(ctx context.Context, lease TrinoCellLease, intentID string) error { + return cs.withTrinoCell(ctx, lease.CellID, func(tx *gorm.DB, row *trinoCellLifecycle) error { + var intent TrinoCatalogIntent + if !ownsTrinoCell(row, lease) || json.Unmarshal([]byte(row.Intent), &intent) != nil || intent.ID == "" || intent.ID != intentID { + return ErrTrinoCellConflict + } + row.Intent = "{}" + return saveTrinoCell(tx, row) + }) +} + +func (cs *ConfigStore) FinishTrinoCellReconcile(ctx context.Context, lease TrinoCellLease) error { + return cs.withTrinoCell(ctx, lease.CellID, func(tx *gorm.DB, row *trinoCellLifecycle) error { + if !ownsTrinoCell(row, lease) || row.Intent != "{}" { + return ErrTrinoCellConflict + } + row.ReconcileOwner = "" + if row.FreezeOperationID != "" { + row.FreezeStable = true + } + return saveTrinoCell(tx, row) + }) +} + +func freezeFromRow(row *trinoCellLifecycle) (*TrinoCellFreeze, error) { + if row.FreezeOperationID == "" { + return nil, nil + } + freeze := &TrinoCellFreeze{OperationID: row.FreezeOperationID, PlanHash: row.FreezePlanHash, TargetBackend: row.FreezeTarget, AdmissionEpoch: row.AdmissionEpoch, Stable: row.FreezeStable} + if row.Certificate != "{}" { + if err := json.Unmarshal([]byte(row.Certificate), &freeze.Certificate); err != nil { + return nil, err + } + } + return freeze, nil +} + +func (cs *ConfigStore) FreezeTrinoCellAdmissions(ctx context.Context, cell, operation, planHash, target string, expectedEpoch int64) (*TrinoCellFreeze, error) { + if !validTrinoCellValue(operation) || !validTrinoCellValue(target) || !trinoCellHashPattern.MatchString(planHash) { + return nil, ErrTrinoCellConflict + } + var result *TrinoCellFreeze + err := cs.withTrinoCell(ctx, cell, func(tx *gorm.DB, row *trinoCellLifecycle) error { + if row.ReleasedOperationID == operation { + return ErrTrinoCellConflict + } + if row.FreezeOperationID != "" { + if row.FreezeOperationID != operation || row.FreezePlanHash != planHash || row.FreezeTarget != target { + return ErrTrinoCellConflict + } + } else { + if row.AdmissionEpoch != expectedEpoch { + return ErrTrinoCellConflict + } + row.AdmissionEpoch++ + row.FreezeOperationID, row.FreezePlanHash, row.FreezeTarget = operation, planHash, target + row.FreezeStable = row.ReconcileOwner == "" + row.Certificate = "{}" + if err := saveTrinoCell(tx, row); err != nil { + return err + } + } + var err error + result, err = freezeFromRow(row) + return err + }) + return result, err +} + +func (cs *ConfigStore) GetTrinoCellFreeze(ctx context.Context, cell string) (*TrinoCellFreeze, error) { + var row trinoCellLifecycle + err := cs.db.WithContext(ctx).First(&row, "cell_id = ?", cell).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, nil + } + if err != nil { + return nil, err + } + return freezeFromRow(&row) +} + +type TrinoCellLifecycleStatus struct { + CellID string + AdmissionEpoch int64 + ReconcileOwner string + ReconcileEpoch int64 + IntentSequence int64 + Intent *TrinoCatalogIntent + Freeze *TrinoCellFreeze + ReleasedOperationID string + ReleasedAdmissionEpoch int64 +} + +// GetTrinoCellLifecycle returns the current epoch even when no freeze is active. +// Reading an uninitialized managed cell does not create a lifecycle row. +func (cs *ConfigStore) GetTrinoCellLifecycle(ctx context.Context, cell string) (*TrinoCellLifecycleStatus, error) { + if !strings.HasPrefix(cell, "registered:") || !validTrinoCellValue(cell) { + return nil, ErrTrinoCellConflict + } + var row trinoCellLifecycle + err := cs.db.WithContext(ctx).First(&row, "cell_id = ?", cell).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return &TrinoCellLifecycleStatus{CellID: cell}, nil + } + if err != nil { + return nil, err + } + freeze, err := freezeFromRow(&row) + if err != nil { + return nil, err + } + result := &TrinoCellLifecycleStatus{CellID: cell, AdmissionEpoch: row.AdmissionEpoch, ReconcileOwner: row.ReconcileOwner, ReconcileEpoch: row.ReconcileEpoch, IntentSequence: row.IntentSequence, Freeze: freeze, ReleasedOperationID: row.ReleasedOperationID, ReleasedAdmissionEpoch: row.ReleasedAdmissionEpoch} + if row.Intent != "{}" { + if err := json.Unmarshal([]byte(row.Intent), &result.Intent); err != nil { + return nil, err + } + } + return result, nil +} + +func (cs *ConfigStore) CertifyTrinoCellTarget(ctx context.Context, lease TrinoCellLease, operation string, certificate TrinoCellCertificate) error { + if !validTrinoCellValue(certificate.TargetBackend) || !validTrinoCellValue(certificate.NodeID) || !validTrinoCellValue(certificate.CoordinatorID) || !trinoCellHashPattern.MatchString(certificate.RosterHash) || certificate.AdmittedCount < 0 || certificate.AdmittedCount > 100000 { + return ErrTrinoCellConflict + } + encoded, err := json.Marshal(certificate) + if err != nil { + return err + } + return cs.withTrinoCell(ctx, lease.CellID, func(tx *gorm.DB, row *trinoCellLifecycle) error { + if !ownsTrinoCell(row, lease) || row.Intent != "{}" || !row.FreezeStable || row.FreezeOperationID != operation || row.FreezeTarget != certificate.TargetBackend || row.AdmissionEpoch != lease.AdmissionEpoch { + return ErrTrinoCellConflict + } + if row.Certificate != "{}" { + var existing TrinoCellCertificate + if json.Unmarshal([]byte(row.Certificate), &existing) != nil || existing != certificate { + return ErrTrinoCellConflict + } + return nil + } + row.Certificate = string(encoded) + return saveTrinoCell(tx, row) + }) +} + +// ReleaseTrinoCellAdmissions follows verification of the exact Gateway target route. +// The caller must validate the active operation and certified coordinator process. +func (cs *ConfigStore) ReleaseTrinoCellAdmissions(ctx context.Context, cell, operation string, epoch int64) error { + return cs.withTrinoCell(ctx, cell, func(tx *gorm.DB, row *trinoCellLifecycle) error { + if row.FreezeOperationID == "" && row.ReleasedOperationID == operation && row.ReleasedAdmissionEpoch == epoch { + return nil + } + if operation == "" || row.FreezeOperationID != operation || row.AdmissionEpoch != epoch || row.Certificate == "{}" { + return ErrTrinoCellConflict + } + row.ReleasedOperationID, row.ReleasedAdmissionEpoch = operation, epoch + row.FreezeOperationID, row.FreezePlanHash, row.FreezeTarget = "", "", "" + row.FreezeStable = false + row.Certificate = "{}" + row.AdmissionEpoch++ + return saveTrinoCell(tx, row) + }) +} + +// UpdateManagedTrinoState fences new admission in the same database transaction. +// A backend health observation cannot implicitly grant new admission after cutover. +func (cs *ConfigStore) UpdateManagedTrinoState(ctx context.Context, lease TrinoCellLease, org string, update TrinoStateUpdate) (bool, error) { + updated := false + err := cs.withTrinoCell(ctx, lease.CellID, func(tx *gorm.DB, row *trinoCellLifecycle) error { + if !ownsTrinoCell(row, lease) { + return ErrTrinoCellConflict + } + var tenant ManagedWarehouseTrino + err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).First(&tenant, "org_id = ? AND enabled = ? AND trino_cell_id = ?", org, true, lease.CellID).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil + } + if err != nil { + return err + } + if update.State == ManagedWarehouseStateReady && tenant.State != ManagedWarehouseStateReady && (row.FreezeOperationID != "" || row.AdmissionEpoch != lease.AdmissionEpoch) { + return nil + } + temporary := &ConfigStore{db: tx} + if err := temporary.UpdateTrinoState(org, update); err != nil { + return err + } + updated = true + return nil + }) + return updated, err +} diff --git a/tests/configstore/migrations_postgres_test.go b/tests/configstore/migrations_postgres_test.go index 090d426b..a2904157 100644 --- a/tests/configstore/migrations_postgres_test.go +++ b/tests/configstore/migrations_postgres_test.go @@ -56,7 +56,12 @@ func TestConfigStoreRunsVersionedSQLMigrations(t *testing.T) { requireGooseMigrationRecorded(t, db, 35) requireGooseMigrationRecorded(t, db, 36) requireGooseMigrationRecorded(t, db, 38) - requireGooseLatestVersion(t, db, 38) + requireGooseMigrationRecorded(t, db, 39) + requireGooseLatestVersion(t, db, 39) + requireTablePresent(t, db, "duckgres_trino_cell_lifecycle") + for _, column := range []string{"reconcile_owner", "reconcile_epoch", "intent_sequence", "intent", "admission_epoch", "freeze_operation_id", "freeze_stable", "certificate"} { + requireColumnPresent(t, db, "duckgres_trino_cell_lifecycle", column) + } requireTableAbsent(t, db, "duckgres_schema_migrations") // Migration 000018 added the reshard operation + verbose log tables. @@ -316,7 +321,8 @@ func TestConfigStoreSQLMigrationsUpgradeVersion8Schema(t *testing.T) { DROP TABLE IF EXISTS duckgres_service_grants; DROP TABLE IF EXISTS duckgres_managed_warehouse_trino; DROP TABLE IF EXISTS duckgres_trino_cluster_bootstrap; - DELETE FROM goose_db_version WHERE version_id IN (9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38); + DROP TABLE IF EXISTS duckgres_trino_cell_lifecycle; + DELETE FROM goose_db_version WHERE version_id IN (9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39); `).Error; err != nil { t.Fatalf("downgrade baseline schema to pre-v9 shape: %v", err) } @@ -367,7 +373,7 @@ func TestConfigStoreSQLMigrationsUpgradeVersion8Schema(t *testing.T) { requireGooseMigrationRecorded(t, upgradedDB, 35) requireGooseMigrationRecorded(t, upgradedDB, 36) requireGooseMigrationRecorded(t, upgradedDB, 38) - requireGooseLatestVersion(t, upgradedDB, 38) + requireGooseLatestVersion(t, upgradedDB, 39) requireColumnPresent(t, upgradedDB, "duckgres_reshard_operations", "password_url") requireTablePresent(t, upgradedDB, "duckgres_worker_spawn_log") requireColumnDefault(t, upgradedDB, "duckgres_orgs", "max_vcpus", "0") @@ -416,7 +422,8 @@ func TestConfigStoreSQLMigration34VersionsExistingAndNewOrgs(t *testing.T) { DROP TABLE IF EXISTS duckgres_service_grants; DROP TABLE IF EXISTS duckgres_managed_warehouse_trino; DROP TABLE IF EXISTS duckgres_trino_cluster_bootstrap; - DELETE FROM goose_db_version WHERE version_id IN (34, 35, 36, 37, 38); + DROP TABLE IF EXISTS duckgres_trino_cell_lifecycle; + DELETE FROM goose_db_version WHERE version_id IN (34, 35, 36, 37, 38, 39); `).Error; err != nil { t.Fatalf("restore pre-migration-34 schema: %v", err) } diff --git a/tests/configstore/trino_cell_lifecycle_test.go b/tests/configstore/trino_cell_lifecycle_test.go new file mode 100644 index 00000000..e7c1ba84 --- /dev/null +++ b/tests/configstore/trino_cell_lifecycle_test.go @@ -0,0 +1,182 @@ +//go:build linux || darwin + +package configstore_test + +import ( + "context" + "sync" + "sync/atomic" + "testing" + + "github.com/google/uuid" + "github.com/posthog/duckgres/controlplane/configstore" +) + +func TestTrinoCellReconcileOwnershipAndUncertainIntentPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + ctx := context.Background() + var successes atomic.Int32 + var winner *configstore.TrinoCellLease + var lock sync.Mutex + var workers sync.WaitGroup + for range 16 { + workers.Go(func() { + lease, acquired, err := store.BeginTrinoCellReconcile(ctx, "registered:cell-test", uuid.NewString()) + if err != nil { + t.Error(err) + return + } + if acquired { + successes.Add(1) + lock.Lock() + winner = lease + lock.Unlock() + } + }) + } + workers.Wait() + if successes.Load() != 1 || winner == nil { + t.Fatal("cell must have exactly one durable owner") + } + intent := configstore.TrinoCatalogIntent{ID: uuid.NewString(), Sequence: winner.IntentSequence + 1, Backend: "group-blue", Action: "create", Catalog: "org_example"} + if err := store.SetTrinoCellIntent(ctx, *winner, intent); err != nil { + t.Fatal(err) + } + if err := store.SetTrinoCellIntent(ctx, *winner, intent); err == nil { + t.Fatal("duplicate intent must not authorize a second remote submission") + } + if err := store.FinishTrinoCellReconcile(ctx, *winner); err == nil { + t.Fatal("unknown remote outcome released cell ownership") + } + if _, acquired, err := store.BeginTrinoCellReconcile(ctx, winner.CellID, uuid.NewString()); err != nil || acquired { + t.Fatal("another replica took an uncertain operation") + } + if err := store.ClearTrinoCellIntent(ctx, *winner, intent.ID); err != nil { + t.Fatal(err) + } + if err := store.SetTrinoCellIntent(ctx, *winner, intent); err == nil { + t.Fatal("cleared intent authorized a duplicate remote submission") + } + nextIntent := intent + nextIntent.ID = uuid.NewString() + nextIntent.Sequence++ + if err := store.SetTrinoCellIntent(ctx, *winner, nextIntent); err != nil { + t.Fatal(err) + } + if err := store.ClearTrinoCellIntent(ctx, *winner, nextIntent.ID); err != nil { + t.Fatal(err) + } + if err := store.SetTrinoCellIntent(ctx, *winner, intent); err == nil { + t.Fatal("intervening intent allowed old submission replay") + } + if err := store.FinishTrinoCellReconcile(ctx, *winner); err != nil { + t.Fatal(err) + } + next, acquired, err := store.BeginTrinoCellReconcile(ctx, winner.CellID, uuid.NewString()) + if err != nil || !acquired || next.ReconcileEpoch <= winner.ReconcileEpoch { + t.Fatal("known completion did not allow a fresh fenced owner") + } + if err := store.SetTrinoCellIntent(ctx, *winner, intent); err == nil { + t.Fatal("old owner submitted after a newer pass acquired the cell") + } + if err := store.ClearTrinoCellIntent(ctx, *winner, intent.ID); err == nil { + t.Fatal("old owner cleared a newer pass") + } + if err := store.FinishTrinoCellReconcile(ctx, *winner); err == nil { + t.Fatal("old owner finished a newer pass") + } + if _, acquired, err := store.BeginTrinoCellReconcile(ctx, "registered:other-cell", uuid.NewString()); err != nil || !acquired { + t.Fatal("a held cell blocked another cell") + } +} + +func TestTrinoCellAdmissionEpochBlocksLateReadyPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + ctx := context.Background() + seedTrinoOrg(t, store, "example") + if err := store.DB().Create(&configstore.ManagedWarehouse{OrgID: "example", DucklingName: "example", State: configstore.ManagedWarehouseStateReady}).Error; err != nil { + t.Fatal(err) + } + cell := "registered:cell-test" + if err := store.SelectTrinoCell("example", cell); err != nil { + t.Fatal(err) + } + if err := store.EnableTrino("example", configstore.TrinoSettings{}); err != nil { + t.Fatal(err) + } + lease, acquired, err := store.BeginTrinoCellReconcile(ctx, cell, uuid.NewString()) + if err != nil || !acquired { + t.Fatal("could not acquire initial pass") + } + freeze, err := store.FreezeTrinoCellAdmissions(ctx, cell, "operation-test", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "group-green", lease.AdmissionEpoch) + if err != nil { + t.Fatal(err) + } + if freeze.Stable { + t.Fatal("freeze acknowledged stable while an earlier owner still runs") + } + retried, err := store.FreezeTrinoCellAdmissions(ctx, cell, freeze.OperationID, freeze.PlanHash, freeze.TargetBackend, lease.AdmissionEpoch) + if err != nil || retried.AdmissionEpoch != freeze.AdmissionEpoch { + t.Fatal("same-operation freeze retry changed epoch") + } + if updated, err := store.UpdateManagedTrinoState(ctx, *lease, "example", configstore.TrinoStateUpdate{State: configstore.ManagedWarehouseStateReady}); err != nil || updated { + t.Fatal("a pre-freeze blue pass admitted a new warehouse") + } + if err := store.FinishTrinoCellReconcile(ctx, *lease); err != nil { + t.Fatal(err) + } + during, acquired, err := store.BeginTrinoCellReconcile(ctx, cell, uuid.NewString()) + if err != nil || !acquired { + t.Fatal("freeze must allow target preparation") + } + if updated, err := store.UpdateManagedTrinoState(ctx, *during, "example", configstore.TrinoStateUpdate{State: configstore.ManagedWarehouseStateReady}); err != nil || updated { + t.Fatal("frozen preparation admitted a new warehouse") + } + if err := store.ReleaseTrinoCellAdmissions(ctx, cell, freeze.OperationID, freeze.AdmissionEpoch); err == nil { + t.Fatal("uncertified target released admissions") + } + certificate := configstore.TrinoCellCertificate{TargetBackend: "group-green", NodeID: "node-test", CoordinatorID: "abcde", RosterHash: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"} + if err := store.CertifyTrinoCellTarget(ctx, *during, freeze.OperationID, certificate); err != nil { + t.Fatal(err) + } + changed := certificate + changed.NodeID = "replacement-node" + if err := store.CertifyTrinoCellTarget(ctx, *during, freeze.OperationID, changed); err == nil { + t.Fatal("certificate process identity was overwritten") + } + if err := store.ReleaseTrinoCellAdmissions(ctx, cell, freeze.OperationID, freeze.AdmissionEpoch); err != nil { + t.Fatal(err) + } + if updated, err := store.UpdateManagedTrinoState(ctx, *during, "example", configstore.TrinoStateUpdate{State: configstore.ManagedWarehouseStateReady}); err != nil || updated { + t.Fatal("during-freeze pass admitted after release") + } + if err := store.SetTrinoCellIntent(ctx, *during, configstore.TrinoCatalogIntent{ID: uuid.NewString(), Sequence: during.IntentSequence + 1, Backend: "group-blue", Action: "create", Catalog: "org_example"}); err == nil { + t.Fatal("old blue pass wrote after cutover release") + } + status, err := store.GetTrinoCellLifecycle(ctx, cell) + if err != nil || status.Freeze != nil || status.AdmissionEpoch != freeze.AdmissionEpoch+1 || status.ReleasedOperationID != freeze.OperationID { + t.Fatal("released lifecycle lost its discoverable admission epoch") + } + if err := store.FinishTrinoCellReconcile(ctx, *during); err != nil { + t.Fatal(err) + } + after, acquired, err := store.BeginTrinoCellReconcile(ctx, cell, uuid.NewString()) + if err != nil || !acquired { + t.Fatal("could not begin post-cutover pass") + } + if updated, err := store.UpdateManagedTrinoState(ctx, *after, "example", configstore.TrinoStateUpdate{State: configstore.ManagedWarehouseStateReady}); err != nil || !updated { + t.Fatal("fresh post-cutover pass did not admit") + } + if updated, err := store.UpdateManagedTrinoState(ctx, *lease, "example", configstore.TrinoStateUpdate{State: configstore.ManagedWarehouseStateReady}); err == nil || updated { + t.Fatal("original owner changed later admission") + } + if err := store.FinishTrinoCellReconcile(ctx, *after); err != nil { + t.Fatal(err) + } + if _, err := store.FreezeTrinoCellAdmissions(ctx, cell, "next-operation", freeze.PlanHash, "group-blue", after.AdmissionEpoch); err != nil { + t.Fatal(err) + } + if _, err := store.FreezeTrinoCellAdmissions(ctx, cell, freeze.OperationID, freeze.PlanHash, freeze.TargetBackend, lease.AdmissionEpoch); err == nil { + t.Fatal("delayed prior operation reopened admissions freeze") + } +} From eff7c259684a90512d825348a2525242e687bba8 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Mon, 14 Sep 2026 16:55:44 +0200 Subject: [PATCH 2/7] Require confirmed terminal catalog DDL outcomes in shared mode --- controlplane/provisioner/opa/policy.rego | 13 + controlplane/provisioner/opa/policy_test.go | 23 +- controlplane/provisioner/trino_nodes.go | 4 + .../trino_shared_catalog_client.go | 217 ++++++++++++++ .../trino_shared_catalog_client_test.go | 271 ++++++++++++++++++ 5 files changed, 526 insertions(+), 2 deletions(-) create mode 100644 controlplane/provisioner/trino_shared_catalog_client.go create mode 100644 controlplane/provisioner/trino_shared_catalog_client_test.go diff --git a/controlplane/provisioner/opa/policy.rego b/controlplane/provisioner/opa/policy.rego index 7bf63011..246be008 100644 --- a/controlplane/provisioner/opa/policy.rego +++ b/controlplane/provisioner/opa/policy.rego @@ -736,6 +736,19 @@ allow if { observer_nodes_table(input.action.resource.table) } +# The provisioner reads catalog startup status without querying tenant data. +allow if { + is_admin + input.action.operation == "SelectFromColumns" + table := input.action.resource.table + table.catalogName == "system" + table.schemaName == "metadata" + table.tableName == "catalogs" + every column in table.columns { + column in {"catalog_name", "state"} + } +} + # --------------------------------------------------------------------------- # Hard denies for customer principals. # diff --git a/controlplane/provisioner/opa/policy_test.go b/controlplane/provisioner/opa/policy_test.go index 26d8edc6..e56ed36e 100644 --- a/controlplane/provisioner/opa/policy_test.go +++ b/controlplane/provisioner/opa/policy_test.go @@ -1817,7 +1817,7 @@ func TestSystemNodesGrantExcludesTenants(t *testing.T) { } } -func TestTrinoProvisionerReadsOnlySystemRuntimeNodes(t *testing.T) { +func TestTrinoProvisionerReadsOnlySystemReadinessInventories(t *testing.T) { q := preparedPolicy(t, twoOrgFixture()) if !evalAllow(t, q, buildInput(AdminPrincipal, "AccessCatalog", catalogResource("system"))) { t.Error("provisioner must reach system for worker readiness inventory") @@ -1829,7 +1829,6 @@ func TestTrinoProvisionerReadsOnlySystemRuntimeNodes(t *testing.T) { {"system", "runtime", "queries"}, {"system", "runtime", "tasks"}, {"system", "runtime", "transactions"}, - {"system", "metadata", "catalogs"}, {"system", "jdbc", "tables"}, {"system", "information_schema", "tables"}, {"system", "other", "nodes"}, @@ -1858,3 +1857,23 @@ func TestTrinoProvisionerReadsOnlySystemRuntimeNodes(t *testing.T) { } } } + +func TestTrinoProvisionerCatalogStatesAreNarrow(t *testing.T) { + q := preparedPolicy(t, twoOrgFixture()) + for _, tc := range []struct { + user string + columns []string + allowed bool + }{ + {AdminPrincipal, []string{"catalog_name", "state"}, true}, + {AdminPrincipal, []string{"connector_name"}, false}, + {ObserverPrincipal, []string{"catalog_name", "state"}, false}, + {"42", []string{"catalog_name", "state"}, false}, + } { + resource := tableResource("system", "metadata", "catalogs") + resource["table"].(map[string]interface{})["columns"] = tc.columns + if actual := evalAllow(t, q, buildInput(tc.user, "SelectFromColumns", resource)); actual != tc.allowed { + t.Fatalf("catalog inventory permission for %s = %v, expected %v", tc.user, actual, tc.allowed) + } + } +} diff --git a/controlplane/provisioner/trino_nodes.go b/controlplane/provisioner/trino_nodes.go index caf13cdf..0add67d2 100644 --- a/controlplane/provisioner/trino_nodes.go +++ b/controlplane/provisioner/trino_nodes.go @@ -25,6 +25,10 @@ func (c *trinoCatalogHTTPClient) ListNodes(ctx context.Context) ([]TrinoNode, er if err != nil { return nil, fmt.Errorf("query Trino node inventory: %w", err) } + return parseTrinoNodes(rows) +} + +func parseTrinoNodes(rows [][]interface{}) ([]TrinoNode, error) { nodes := make([]TrinoNode, 0, len(rows)) ids := make(map[string]bool, len(rows)) uris := make(map[string]bool, len(rows)) diff --git a/controlplane/provisioner/trino_shared_catalog_client.go b/controlplane/provisioner/trino_shared_catalog_client.go new file mode 100644 index 00000000..502d44c8 --- /dev/null +++ b/controlplane/provisioner/trino_shared_catalog_client.go @@ -0,0 +1,217 @@ +//go:build kubernetes + +package provisioner + +import ( + "context" + "crypto/tls" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "regexp" + "strings" + "time" +) + +const ( + sharedTrinoMaxPageBytes = 4 << 20 + sharedTrinoMaxTotalBytes = 32 << 20 + sharedTrinoMaxPages = 256 + sharedTrinoMaxRows = 100000 +) + +var sharedTrinoQueryID = regexp.MustCompile(`^[0-9]{8}_[0-9]{6}_[0-9]{5,}_[a-z0-9]{5}$`) +var sharedTrinoPageToken = regexp.MustCompile(`^[0-9]+$`) +var sharedTrinoSlug = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + +type trinoCatalogTerminalError struct{} + +func (*trinoCatalogTerminalError) Error() string { + return "trino catalog statement failed with confirmed terminal outcome" +} + +// TrinoCatalogOutcomeTerminal permits clearing a durable DDL intent. +// All unclassified failures retain the intent and require explicit recovery. +func TrinoCatalogOutcomeTerminal(err error) bool { + var terminal *trinoCatalogTerminalError + return err == nil || errors.As(err, &terminal) +} + +type trinoSharedCatalogHTTPClient struct { + *trinoCatalogHTTPClient + origin *url.URL +} + +// NewTrinoSharedCatalogHTTPClient requires verified HTTPS and rejects redirects. +// This private coordinator path does not use the environment egress proxy. +func NewTrinoSharedCatalogHTTPClient(baseURL, username, password, tlsServerName string) (*trinoSharedCatalogHTTPClient, error) { + origin, err := url.Parse(baseURL) + if err != nil || origin.Scheme != "https" || origin.Hostname() == "" || origin.User != nil || origin.RawQuery != "" || origin.ForceQuery || origin.Fragment != "" || (origin.Path != "" && origin.Path != "/") { + return nil, errors.New("invalid shared catalog coordinator URL") + } + legacy := NewTrinoCatalogHTTPClient(strings.TrimRight(baseURL, "/"), username, password, tlsServerName).(*trinoCatalogHTTPClient) + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil + transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12, ServerName: tlsServerName} + legacy.hc = &http.Client{ + Transport: transport, + Timeout: 30 * time.Second, + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } + return &trinoSharedCatalogHTTPClient{trinoCatalogHTTPClient: legacy, origin: origin}, nil +} + +func (c *trinoSharedCatalogHTTPClient) validContinuation(raw, queryID string) bool { + next, err := url.Parse(raw) + if err != nil || next.Scheme != c.origin.Scheme || !strings.EqualFold(next.Hostname(), c.origin.Hostname()) || sharedTrinoPort(next) != sharedTrinoPort(c.origin) || next.User != nil || next.RawQuery != "" || next.ForceQuery || next.Fragment != "" || next.RawPath != "" { + return false + } + parts := strings.Split(next.Path, "/") + return len(parts) == 7 && parts[1] == "v1" && parts[2] == "statement" && (parts[3] == "executing" || parts[3] == "queued") && parts[4] == queryID && len(parts[5]) <= 256 && sharedTrinoSlug.MatchString(parts[5]) && len(parts[6]) <= 20 && sharedTrinoPageToken.MatchString(parts[6]) +} + +func sharedTrinoPort(endpoint *url.URL) string { + if port := endpoint.Port(); port != "" { + return port + } + return "443" +} + +func (c *trinoSharedCatalogHTTPClient) runStatement(ctx context.Context, statement string) ([][]interface{}, error) { + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + username, password := c.credentials() + if username == "" || password == "" { + return nil, &trinoCatalogTerminalError{} + } + method, target := http.MethodPost, c.baseURL+"/v1/statement" + var queryID string + var rows [][]interface{} + totalBytes := 0 + for range sharedTrinoMaxPages { + var body io.Reader + if method == http.MethodPost { + body = strings.NewReader(statement) + } + request, err := http.NewRequestWithContext(ctx, method, target, body) + if err != nil { + return nil, errors.New("cannot construct catalog statement request") + } + request.SetBasicAuth(username, password) + request.Header.Set("X-Trino-User", username) + request.Header.Set("X-Trino-Source", TrinoProvisionerSource) + request.Header.Set("Content-Type", "text/plain") + response, err := c.hc.Do(request) + if err != nil { + return nil, errors.New("catalog statement transport outcome unknown") + } + data, readErr := io.ReadAll(io.LimitReader(response.Body, sharedTrinoMaxPageBytes+1)) + closeErr := response.Body.Close() + totalBytes += len(data) + if readErr != nil || closeErr != nil || len(data) > sharedTrinoMaxPageBytes || totalBytes > sharedTrinoMaxTotalBytes || response.StatusCode != http.StatusOK { + return nil, errors.New("catalog statement response outcome unknown") + } + var page trinoStatementResponse + if json.Unmarshal(data, &page) != nil || len(page.ID) > 128 || !sharedTrinoQueryID.MatchString(page.ID) || (queryID != "" && queryID != page.ID) { + return nil, errors.New("catalog statement identity or response invalid") + } + queryID = page.ID + state, _ := page.Stats["state"].(string) + switch state { + case "QUEUED", "WAITING_FOR_RESOURCES", "DISPATCHING", "PLANNING", "STARTING", "RUNNING", "FINISHING", "FINISHED", "FAILED": + default: + return nil, errors.New("catalog statement state invalid") + } + if page.NextURI == "" { + if state == "FAILED" && page.Error != nil && page.Error.ErrorName != "" { + return nil, &trinoCatalogTerminalError{} + } + if state != "FINISHED" || page.Error != nil { + return nil, errors.New("catalog statement terminal outcome unknown") + } + } else if page.Error != nil || state == "FINISHED" || state == "FAILED" || !c.validContinuation(page.NextURI, queryID) { + return nil, errors.New("catalog statement continuation invalid") + } + if len(rows)+len(page.Data) > sharedTrinoMaxRows { + return nil, errors.New("catalog statement row limit exceeded") + } + rows = append(rows, page.Data...) + if page.NextURI == "" { + return rows, nil + } + method, target = http.MethodGet, page.NextURI + } + return nil, errors.New("catalog statement page limit exceeded") +} + +// CatalogStates reads all catalog states with a bounded bulk query. +// A failed startup catalog is not equivalent to a usable catalog name. +func (c *trinoSharedCatalogHTTPClient) CatalogStates(ctx context.Context) (map[string]string, error) { + rows, err := c.runStatement(ctx, "SELECT catalog_name, state FROM system.metadata.catalogs") + if err != nil { + return nil, err + } + states := make(map[string]string, len(rows)) + for _, row := range rows { + if len(row) != 2 { + return nil, errors.New("invalid catalog state inventory") + } + name, nameOK := row[0].(string) + state, stateOK := row[1].(string) + if !nameOK || name == "" || !stateOK || (state != "OPERATIONAL" && state != "FAILING") || states[name] != "" { + return nil, errors.New("invalid catalog state inventory") + } + states[name] = state + } + return states, nil +} + +func (c *trinoSharedCatalogHTTPClient) ListCatalogs(ctx context.Context) ([]string, error) { + states, err := c.CatalogStates(ctx) + if err != nil { + return nil, err + } + names := make([]string, 0, len(states)) + for name, state := range states { + if state != "OPERATIONAL" { + return nil, errors.New("catalog startup failed") + } + names = append(names, name) + } + return names, nil +} + +func (c *trinoSharedCatalogHTTPClient) ListNodes(ctx context.Context) ([]TrinoNode, error) { + rows, err := c.runStatement(ctx, "SELECT node_id, http_uri, coordinator, state FROM system.runtime.nodes") + if err != nil { + return nil, err + } + return parseTrinoNodes(rows) +} + +func (c *trinoSharedCatalogHTTPClient) CreateCatalog(ctx context.Context, name string, props map[string]string) error { + connector := props["connector.name"] + if connector == "" { + return &trinoCatalogTerminalError{} + } + withProps := make(map[string]string, len(props)) + for key, value := range props { + if key != "connector.name" { + withProps[key] = value + } + } + _, err := c.runStatement(ctx, fmt.Sprintf("CREATE CATALOG %s USING %s%s", quoteTrinoIdentifier(name), quoteTrinoIdentifier(connector), renderWithClause(withProps))) + return err +} + +func (c *trinoSharedCatalogHTTPClient) DropCatalog(ctx context.Context, name string) error { + _, err := c.runStatement(ctx, "DROP CATALOG "+quoteTrinoIdentifier(name)) + return err +} + +func (c *trinoSharedCatalogHTTPClient) AlterCatalog(context.Context, string, map[string]string) error { + return &trinoCatalogTerminalError{} +} diff --git a/controlplane/provisioner/trino_shared_catalog_client_test.go b/controlplane/provisioner/trino_shared_catalog_client_test.go new file mode 100644 index 00000000..7e2d39f5 --- /dev/null +++ b/controlplane/provisioner/trino_shared_catalog_client_test.go @@ -0,0 +1,271 @@ +//go:build kubernetes + +package provisioner + +import ( + "context" + "crypto/x509" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" +) + +const sharedTestQueryID = "20260101_000000_00001_abcde" + +func TestSharedCatalogStatementRequiresTerminalProof(t *testing.T) { + for _, tc := range []struct { + name, body string + terminal, success bool + }{ + {"finished", `{"id":"` + sharedTestQueryID + `","stats":{"state":"FINISHED"}}`, true, true}, + {"failed", `{"id":"` + sharedTestQueryID + `","stats":{"state":"FAILED"},"error":{"errorName":"INVALID_CATALOG_PROPERTY","message":"private-value"}}`, true, false}, + {"empty", `{}`, false, false}, + {"truncated", `{"id":`, false, false}, + {"running_without_next", `{"id":"` + sharedTestQueryID + `","stats":{"state":"RUNNING"}}`, false, false}, + {"failed_without_error", `{"id":"` + sharedTestQueryID + `","stats":{"state":"FAILED"}}`, false, false}, + {"error_without_terminal_state", `{"id":"` + sharedTestQueryID + `","error":{"errorName":"UNKNOWN"}}`, false, false}, + {"no_query_identity", `{"stats":{"state":"FINISHED"}}`, false, false}, + {"oversized", strings.Repeat(" ", sharedTrinoMaxPageBytes+1), false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = fmt.Fprint(w, tc.body) })) + defer server.Close() + client := sharedTestClient(t, server) + _, err := client.runStatement(context.Background(), "CREATE CATALOG example USING example") + if (err == nil) != tc.success || TrinoCatalogOutcomeTerminal(err) != tc.terminal { + t.Fatalf("success=%v terminal=%v, expected %v/%v", err == nil, TrinoCatalogOutcomeTerminal(err), tc.success, tc.terminal) + } + if err != nil && strings.Contains(err.Error(), "private-value") { + t.Fatal("response contents leaked into error") + } + }) + } +} + +func TestSharedCatalogContinuationOriginAndIdentity(t *testing.T) { + var foreignCalls atomic.Int32 + foreign := httptest.NewTLSServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { foreignCalls.Add(1) })) + defer foreign.Close() + for _, mode := range []string{"foreign", "redirect", "changed_id", "credentials_rotated", "wrong_path", "unknown_state"} { + t.Run(mode, func(t *testing.T) { + var server *httptest.Server + var client *trinoSharedCatalogHTTPClient + var requests atomic.Int32 + server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + user, password, _ := r.BasicAuth() + if user != "provisioner" || password != "original" { + t.Error("statement credentials changed between pages") + } + if r.Method == http.MethodPost { + if mode == "redirect" { + http.Redirect(w, r, foreign.URL, http.StatusTemporaryRedirect) + return + } + next := server.URL + "/v1/statement/executing/" + sharedTestQueryID + "/token/1" + if mode == "foreign" { + next = foreign.URL + "/v1/statement/executing/" + sharedTestQueryID + "/token/1" + } + if mode == "wrong_path" { + next = server.URL + "/admin" + } + if mode == "credentials_rotated" { + client.SetCredentials("replacement", "replacement") + } + state := "RUNNING" + if mode == "unknown_state" { + state = "UNKNOWN" + } + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":%q},"nextUri":%q}`, sharedTestQueryID, state, next) + return + } + id := sharedTestQueryID + if mode == "changed_id" { + id = "20260101_000000_00002_abcde" + } + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"}}`, id) + })) + defer server.Close() + client = sharedTestClient(t, server) + _, err := client.runStatement(context.Background(), "SELECT 1") + if (err == nil) != (mode == "credentials_rotated") { + t.Fatalf("unexpected success=%v", err == nil) + } + if mode != "credentials_rotated" && TrinoCatalogOutcomeTerminal(err) { + t.Fatal("ambiguous continuation classified terminal") + } + if (mode == "foreign" || mode == "redirect" || mode == "wrong_path") && requests.Load() != 1 { + t.Fatal("unsafe continuation was requested") + } + }) + } + if foreignCalls.Load() != 0 { + t.Fatal("credentials reached foreign server") + } +} + +func TestSharedCatalogProductionTransportTLSAndPaging(t *testing.T) { + var server *httptest.Server + server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Host != strings.TrimPrefix(server.URL, "https://") { + t.Error("TLS name replaced HTTP origin") + } + if r.Method == http.MethodPost { + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"RUNNING"},"nextUri":%q}`, sharedTestQueryID, "https://"+r.Host+"/v1/statement/executing/"+sharedTestQueryID+"/token/1") + return + } + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"}}`, sharedTestQueryID) + })) + defer server.Close() + t.Setenv("HTTPS_PROXY", "http://127.0.0.1:1") + for _, tc := range []struct { + name string + trust, valid bool + }{{"trusted", true, true}, {"untrusted", false, true}, {"wrong_name", true, false}} { + t.Run(tc.name, func(t *testing.T) { + serverName := server.Certificate().DNSNames[0] + if !tc.valid { + serverName = "wrong.example.invalid" + } + client, err := NewTrinoSharedCatalogHTTPClient(server.URL, "provisioner", "password", serverName) + if err != nil { + t.Fatal(err) + } + transport := client.hc.Transport.(*http.Transport) + if transport.Proxy != nil || transport.TLSClientConfig.InsecureSkipVerify { + t.Fatal("unsafe production transport") + } + if tc.trust { + transport.TLSClientConfig.RootCAs = x509.NewCertPool() + transport.TLSClientConfig.RootCAs.AddCert(server.Certificate()) + } + _, err = client.runStatement(context.Background(), "SELECT 1") + if (err == nil) != (tc.trust && tc.valid) { + t.Fatalf("unexpected TLS result: %v", err) + } + }) + } +} + +func TestSharedCatalogContinuationCanonicalOrigin(t *testing.T) { + client, err := NewTrinoSharedCatalogHTTPClient("https://coordinator.example:443", "user", "password", "certificate.example") + if err != nil { + t.Fatal(err) + } + path := "/v1/statement/executing/" + sharedTestQueryID + "/slug/1" + for _, tc := range []struct { + uri string + valid bool + }{ + {"https://coordinator.example" + path, true}, + {"https://coordinator.example:443" + path, true}, + {"https://coordinator.example:8443" + path, false}, + {"https://certificate.example" + path, false}, + {"http://coordinator.example" + path, false}, + {"https://coordinator.example/v1/statement/executing/" + sharedTestQueryID + "/../1", false}, + {"https://coordinator.example/v1/statement/executing/" + sharedTestQueryID + "/slug/..", false}, + } { + if client.validContinuation(tc.uri, sharedTestQueryID) != tc.valid { + t.Fatalf("incorrect origin/path validation for %q", tc.uri) + } + } +} + +func TestSharedCatalogBulkStates(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"},"data":[`, sharedTestQueryID) + for i := range 10000 { + if i > 0 { + _, _ = fmt.Fprint(w, ",") + } + _, _ = fmt.Fprintf(w, `["org_%d","OPERATIONAL"]`, i) + } + _, _ = fmt.Fprint(w, "]}") + })) + defer server.Close() + states, err := sharedTestClient(t, server).CatalogStates(context.Background()) + if err != nil || len(states) != 10000 || requests.Load() != 1 { + t.Fatalf("bulk catalog inventory failed: %v", err) + } +} + +func TestSharedCatalogReadFailuresRetainIntent(t *testing.T) { + for _, mode := range []string{"short_body", "timeout", "non_200", "loop"} { + t.Run(mode, func(t *testing.T) { + var server *httptest.Server + release := make(chan struct{}) + server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if mode == "timeout" { + select { + case <-r.Context().Done(): + case <-release: + } + return + } + if mode == "short_body" { + w.Header().Set("Content-Length", "10000") + } + if mode == "non_200" { + w.WriteHeader(http.StatusServiceUnavailable) + } + if mode == "loop" { + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"RUNNING"},"nextUri":%q}`, sharedTestQueryID, server.URL+"/v1/statement/executing/"+sharedTestQueryID+"/token/1") + return + } + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"}}`, sharedTestQueryID) + })) + defer server.Close() + defer close(release) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _, err := sharedTestClient(t, server).runStatement(ctx, "DROP CATALOG example") + if err == nil || TrinoCatalogOutcomeTerminal(err) { + t.Fatal("incomplete transport released intent") + } + }) + } +} + +func TestSharedCatalogInvalidInventoryFailsAtomically(t *testing.T) { + for _, data := range []string{`[["org_a","OPERATIONAL"],["org_a","OPERATIONAL"]]`, `[["org_a","UNKNOWN"]]`, `[["org_a"]]`, `[[1,"OPERATIONAL"]]`} { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"},"data":%s}`, sharedTestQueryID, data) + })) + states, err := sharedTestClient(t, server).CatalogStates(context.Background()) + server.Close() + if err == nil || states != nil { + t.Fatal("invalid inventory yielded a partial map") + } + } +} + +func TestSharedCatalogFailedStartupIsNotUsable(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"},"data":[["org_a","FAILING"]]}`, sharedTestQueryID) + })) + defer server.Close() + client := sharedTestClient(t, server) + states, err := client.CatalogStates(context.Background()) + if err != nil || states["org_a"] != "FAILING" { + t.Fatal("failed startup placeholder missing") + } + if _, err := client.ListCatalogs(context.Background()); err == nil { + t.Fatal("failed startup reported usable catalog") + } +} + +func sharedTestClient(t *testing.T, server *httptest.Server) *trinoSharedCatalogHTTPClient { + t.Helper() + client, err := NewTrinoSharedCatalogHTTPClient(server.URL, "provisioner", "original", "") + if err != nil { + t.Fatal(err) + } + client.hc.Transport = server.Client().Transport + return client +} From 89d411f90b30bcf1a30e00ffd1939e4be24a6325 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Mon, 14 Sep 2026 17:10:41 +0200 Subject: [PATCH 3/7] Retain catalog intent after remote query failure --- .../trino_shared_catalog_client.go | 5 ++- .../trino_shared_catalog_client_test.go | 34 ++++++++++++++++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/controlplane/provisioner/trino_shared_catalog_client.go b/controlplane/provisioner/trino_shared_catalog_client.go index 502d44c8..799212ed 100644 --- a/controlplane/provisioner/trino_shared_catalog_client.go +++ b/controlplane/provisioner/trino_shared_catalog_client.go @@ -126,9 +126,8 @@ func (c *trinoSharedCatalogHTTPClient) runStatement(ctx context.Context, stateme return nil, errors.New("catalog statement state invalid") } if page.NextURI == "" { - if state == "FAILED" && page.Error != nil && page.Error.ErrorName != "" { - return nil, &trinoCatalogTerminalError{} - } + // FAILED can precede completion of a synchronous catalog mutation. + // Only FINISHED proves that the mutation task returned successfully. if state != "FINISHED" || page.Error != nil { return nil, errors.New("catalog statement terminal outcome unknown") } diff --git a/controlplane/provisioner/trino_shared_catalog_client_test.go b/controlplane/provisioner/trino_shared_catalog_client_test.go index 7e2d39f5..c6b156e6 100644 --- a/controlplane/provisioner/trino_shared_catalog_client_test.go +++ b/controlplane/provisioner/trino_shared_catalog_client_test.go @@ -22,7 +22,7 @@ func TestSharedCatalogStatementRequiresTerminalProof(t *testing.T) { terminal, success bool }{ {"finished", `{"id":"` + sharedTestQueryID + `","stats":{"state":"FINISHED"}}`, true, true}, - {"failed", `{"id":"` + sharedTestQueryID + `","stats":{"state":"FAILED"},"error":{"errorName":"INVALID_CATALOG_PROPERTY","message":"private-value"}}`, true, false}, + {"failed", `{"id":"` + sharedTestQueryID + `","stats":{"state":"FAILED"},"error":{"errorName":"INVALID_CATALOG_PROPERTY","message":"private-value"}}`, false, false}, {"empty", `{}`, false, false}, {"truncated", `{"id":`, false, false}, {"running_without_next", `{"id":"` + sharedTestQueryID + `","stats":{"state":"RUNNING"}}`, false, false}, @@ -232,6 +232,38 @@ func TestSharedCatalogReadFailuresRetainIntent(t *testing.T) { } } +func TestSharedCatalogFailedResponseCanPrecedeMutationCompletion(t *testing.T) { + finishMutation := make(chan struct{}) + mutationDone := make(chan struct{}) + defer func() { + select { + case <-finishMutation: + default: + close(finishMutation) + } + }() + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + go func() { <-finishMutation; close(mutationDone) }() + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FAILED"},"error":{"errorName":"USER_CANCELED"}}`, sharedTestQueryID) + })) + defer server.Close() + _, err := sharedTestClient(t, server).runStatement(context.Background(), "DROP CATALOG example") + select { + case <-mutationDone: + t.Fatal("fixture mutation already completed") + default: + } + if err == nil || TrinoCatalogOutcomeTerminal(err) { + t.Fatal("FAILED response authorized a later writer while mutation still runs") + } + close(finishMutation) + select { + case <-mutationDone: + case <-time.After(time.Second): + t.Fatal("fixture mutation did not finish") + } +} + func TestSharedCatalogInvalidInventoryFailsAtomically(t *testing.T) { for _, data := range []string{`[["org_a","OPERATIONAL"],["org_a","OPERATIONAL"]]`, `[["org_a","UNKNOWN"]]`, `[["org_a"]]`, `[[1,"OPERATIONAL"]]`} { server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { From 6cf7075e042354bb232666dfc992c40211a564ed Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Mon, 14 Sep 2026 16:48:27 +0200 Subject: [PATCH 4/7] Add guarded Trino rollout provisioning endpoints --- controlplane/trino_managed_gateway.go | 196 +++++++++ controlplane/trino_managed_gateway_test.go | 156 +++++++ controlplane/trino_rollout_provisioning.go | 319 ++++++++++++++ .../trino_rollout_provisioning_test.go | 405 ++++++++++++++++++ 4 files changed, 1076 insertions(+) create mode 100644 controlplane/trino_managed_gateway.go create mode 100644 controlplane/trino_managed_gateway_test.go create mode 100644 controlplane/trino_rollout_provisioning.go create mode 100644 controlplane/trino_rollout_provisioning_test.go diff --git a/controlplane/trino_managed_gateway.go b/controlplane/trino_managed_gateway.go new file mode 100644 index 00000000..c1a91389 --- /dev/null +++ b/controlplane/trino_managed_gateway.go @@ -0,0 +1,196 @@ +//go:build kubernetes + +package controlplane + +import ( + "context" + "crypto/tls" + "encoding/json" + "errors" + "io" + "net/http" + "net/url" + "regexp" + "strings" + "time" + "unicode" + + "github.com/google/uuid" +) + +var errTrinoManagedGateway = errors.New("managed Trino Gateway observation unavailable") +var managedGatewayName = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`) +var managedGatewayHash = regexp.MustCompile(`^[a-f0-9]{64}$`) + +type trinoManagedGatewayRoute struct { + RoutingGroup string `json:"routingGroup"` + Generation int64 `json:"generation"` + BackendName string `json:"backendName"` + BackendIncarnation string `json:"backendIncarnation"` +} + +type trinoManagedGatewayPlan struct { + PlanHash string `json:"planHash"` + ExpectedRouteGeneration int64 `json:"expectedRouteGeneration"` + SourceBackend string `json:"sourceBackend"` + SourceIncarnation string `json:"sourceIncarnation"` + TargetBackend string `json:"targetBackend"` + TargetIncarnation string `json:"targetIncarnation"` +} + +type trinoManagedGatewayRollout struct { + RoutingGroup string `json:"routingGroup"` + OperationID string `json:"operationId"` + Plan trinoManagedGatewayPlan `json:"plan"` + Phase string `json:"phase"` + Version int64 `json:"version"` +} + +type trinoManagedGatewayBackend struct { + BackendName string `json:"name"` + Incarnation string `json:"incarnation"` + State string `json:"state"` + NodeID string `json:"nodeId"` + CoordinatorID string `json:"coordinatorId"` +} + +type trinoManagedGatewayObservation struct { + Route trinoManagedGatewayRoute + Rollout *trinoManagedGatewayRollout +} + +type trinoManagedGatewayReader interface { + Observe(context.Context, string) (*trinoManagedGatewayObservation, error) + Backend(context.Context, string) (*trinoManagedGatewayBackend, error) +} + +type trinoManagedGateway struct { + baseURL string + username string + token string + client *http.Client +} + +func newTrinoManagedGateway(endpoint, tlsName, username, token string) (*trinoManagedGateway, error) { + address, err := url.Parse(endpoint) + if err != nil || address.Scheme != "https" || address.Hostname() == "" || address.User != nil || address.RawQuery != "" || address.Fragment != "" || (address.Path != "" && address.Path != "/") || address.RawPath != "" { + return nil, errors.New("managed Gateway requires a credential-free HTTPS origin") + } + if !managedGatewayValue(username, 255) || strings.Contains(username, ":") || len(token) < 32 || !managedGatewayValue(token, 4096) || (tlsName != "" && (!managedGatewayValue(tlsName, 255) || strings.ContainsAny(tlsName, "/:@"))) { + return nil, errors.New("invalid managed Gateway client configuration") + } + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil + transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12, ServerName: tlsName} + transport.MaxIdleConnsPerHost = 4 + transport.ResponseHeaderTimeout = 5 * time.Second + return &trinoManagedGateway{baseURL: strings.TrimSuffix(endpoint, "/"), username: username, token: token, client: &http.Client{ + Transport: transport, Timeout: 10 * time.Second, + CheckRedirect: func(*http.Request, []*http.Request) error { return errTrinoManagedGateway }, + }}, nil +} + +func managedGatewayValue(value string, limit int) bool { + return value != "" && len(value) <= limit && strings.TrimSpace(value) == value && strings.IndexFunc(value, unicode.IsControl) == -1 +} + +func managedGatewayUUID(value string) bool { + parsed, err := uuid.Parse(value) + return err == nil && parsed != uuid.Nil && parsed.String() == value +} + +func managedGatewayPhase(phase string) int { + for index, known := range []string{"CLAIMED", "WARMED", "VERIFIED", "CUTOVER", "DRAINING", "SEALED", "STOPPED", "COMPLETE"} { + if phase == known { + return index + } + } + return -1 +} + +// Observe detects concurrent route or operation changes before returning a view. +// It does not replace the control-plane admission epoch or the Gateway operation guard. +func (g *trinoManagedGateway) Observe(ctx context.Context, group string) (*trinoManagedGatewayObservation, error) { + if !managedGatewayName.MatchString(group) { + return nil, errTrinoManagedGateway + } + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + first, err := g.observeOnce(ctx, group) + if err != nil { + return nil, err + } + second, err := g.observeOnce(ctx, group) + if err != nil || first.Route != second.Route || (first.Rollout == nil) != (second.Rollout == nil) { + return nil, errTrinoManagedGateway + } + if first.Rollout != nil && *first.Rollout != *second.Rollout { + return nil, errTrinoManagedGateway + } + return second, nil +} + +func (g *trinoManagedGateway) observeOnce(ctx context.Context, group string) (*trinoManagedGatewayObservation, error) { + var route *trinoManagedGatewayRoute + if _, err := g.get(ctx, "/routes/"+group, &route, false); err != nil { + return nil, err + } + if route == nil || route.RoutingGroup != group || route.Generation < 1 || !managedGatewayName.MatchString(route.BackendName) || !managedGatewayUUID(route.BackendIncarnation) { + return nil, errTrinoManagedGateway + } + var operation *trinoManagedGatewayRollout + missing, err := g.get(ctx, "/rollouts/"+group, &operation, true) + if err != nil { + return nil, err + } + if !missing { + if operation == nil || operation.RoutingGroup != group || !managedGatewayValue(operation.OperationID, 256) || operation.Version < 0 || managedGatewayPhase(operation.Phase) < 0 { + return nil, errTrinoManagedGateway + } + plan := operation.Plan + if !managedGatewayHash.MatchString(plan.PlanHash) || plan.ExpectedRouteGeneration < 1 || !managedGatewayName.MatchString(plan.SourceBackend) || !managedGatewayName.MatchString(plan.TargetBackend) || plan.SourceBackend == plan.TargetBackend || !managedGatewayUUID(plan.SourceIncarnation) || (plan.TargetIncarnation != "" && !managedGatewayUUID(plan.TargetIncarnation)) { + return nil, errTrinoManagedGateway + } + } + return &trinoManagedGatewayObservation{Route: *route, Rollout: operation}, nil +} + +func (g *trinoManagedGateway) Backend(ctx context.Context, name string) (*trinoManagedGatewayBackend, error) { + if !managedGatewayName.MatchString(name) { + return nil, errTrinoManagedGateway + } + var backend *trinoManagedGatewayBackend + if _, err := g.get(ctx, "/backends/"+name+"/drain", &backend, false); err != nil { + return nil, err + } + if backend == nil || backend.BackendName != name || !managedGatewayUUID(backend.Incarnation) || (backend.State != "ACTIVE" && backend.State != "DRAINING" && backend.State != "SEALED") || (backend.NodeID != "" && !managedGatewayValue(backend.NodeID, 255)) || (backend.CoordinatorID != "" && !managedGatewayValue(backend.CoordinatorID, 255)) { + return nil, errTrinoManagedGateway + } + return backend, nil +} + +func (g *trinoManagedGateway) get(ctx context.Context, path string, target any, allowMissing bool) (bool, error) { + request, err := http.NewRequestWithContext(ctx, http.MethodGet, g.baseURL+"/gateway/transactions"+path, nil) + if err != nil { + return false, errTrinoManagedGateway + } + request.SetBasicAuth(g.username, g.token) + request.Header.Set("X-Gateway-Transaction-Admin-Token", g.token) + request.Header.Set("Accept", "application/json") + response, err := g.client.Do(request) + if err != nil { + return false, errTrinoManagedGateway + } + defer func() { _ = response.Body.Close() }() + if response.StatusCode == http.StatusNotFound && allowMissing { + return true, nil + } + if response.StatusCode != http.StatusOK { + return false, errTrinoManagedGateway + } + body, err := io.ReadAll(io.LimitReader(response.Body, (1<<20)+1)) + if err != nil || len(body) > 1<<20 || json.Unmarshal(body, target) != nil { + return false, errTrinoManagedGateway + } + return false, nil +} diff --git a/controlplane/trino_managed_gateway_test.go b/controlplane/trino_managed_gateway_test.go new file mode 100644 index 00000000..c28bfdcd --- /dev/null +++ b/controlplane/trino_managed_gateway_test.go @@ -0,0 +1,156 @@ +//go:build kubernetes + +package controlplane + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +const managedTestIncarnation = "11111111-1111-4111-8111-111111111111" +const managedTestTargetIncarnation = "22222222-2222-4222-8222-222222222222" + +func managedTestRoute() trinoManagedGatewayRoute { + return trinoManagedGatewayRoute{RoutingGroup: "cell-a", Generation: 3, BackendName: "cell-a-blue", BackendIncarnation: managedTestIncarnation} +} + +func managedTestRollout() trinoManagedGatewayRollout { + return trinoManagedGatewayRollout{RoutingGroup: "cell-a", OperationID: "operation-a", Phase: "CLAIMED", Version: 0, + Plan: trinoManagedGatewayPlan{PlanHash: strings.Repeat("a", 64), ExpectedRouteGeneration: 3, SourceBackend: "cell-a-blue", SourceIncarnation: managedTestIncarnation, TargetBackend: "cell-a-green"}} +} + +func managedTestClient(t *testing.T, handler http.Handler) *trinoManagedGateway { + t.Helper() + server := httptest.NewTLSServer(handler) + t.Cleanup(server.Close) + client, err := newTrinoManagedGateway(server.URL, "", "rollout", strings.Repeat("t", 48)) + if err != nil { + t.Fatal(err) + } + transport := client.client.Transport.(*http.Transport) + transport.TLSClientConfig.RootCAs = server.Client().Transport.(*http.Transport).TLSClientConfig.RootCAs + return client +} + +func TestManagedGatewayReadsStableAuthenticatedSnapshot(t *testing.T) { + calls := 0 + client := managedTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + user, password, ok := r.BasicAuth() + if !ok || user != "rollout" || password != strings.Repeat("t", 48) || r.Header.Get("X-Gateway-Transaction-Admin-Token") != password || r.Method != http.MethodGet { + t.Error("unexpected request authority") + } + if strings.Contains(r.URL.Path, "/routes/") { + _ = json.NewEncoder(w).Encode(managedTestRoute()) + } else { + _ = json.NewEncoder(w).Encode(managedTestRollout()) + } + })) + got, err := client.Observe(context.Background(), "cell-a") + if err != nil || got == nil || got.Route != managedTestRoute() || got.Rollout == nil || *got.Rollout != managedTestRollout() || calls != 4 { + t.Fatalf("stable observation failed: result=%+v calls=%d err=%v", got, calls, err) + } + if client.client.Transport.(*http.Transport).Proxy != nil { + t.Fatal("private Gateway path must explicitly opt out of the proxy") + } +} + +func TestManagedGatewayRejectsUnstableAndInvalidSnapshots(t *testing.T) { + for _, mode := range []string{"route_changed", "operation_changed", "operation_appeared", "negative_version", "foreign_group", "invalid_uuid", "null", "trailing", "oversize", "redirect", "http_error"} { + t.Run(mode, func(t *testing.T) { + routes, operations := 0, 0 + client := managedTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch mode { + case "null": + _, _ = w.Write([]byte("null")) + return + case "trailing": + _, _ = w.Write([]byte(`{} {}`)) + return + case "oversize": + _, _ = w.Write([]byte(strings.Repeat(" ", (1<<20)+1))) + return + case "redirect": + http.Redirect(w, r, "https://untrusted.example.test", http.StatusFound) + return + case "http_error": + http.Error(w, "sensitive upstream body", http.StatusInternalServerError) + return + } + if strings.Contains(r.URL.Path, "/routes/") { + routes++ + route := managedTestRoute() + if mode == "route_changed" && routes > 1 { + route.Generation++ + } + if mode == "foreign_group" { + route.RoutingGroup = "cell-b" + } + if mode == "invalid_uuid" { + route.BackendIncarnation = "invalid" + } + _ = json.NewEncoder(w).Encode(route) + } else { + operations++ + if mode == "operation_appeared" && operations == 1 { + w.WriteHeader(http.StatusNotFound) + return + } + op := managedTestRollout() + if mode == "negative_version" { + op.Version = -1 + } + if mode == "operation_changed" && operations > 1 { + op.Version++ + } + _ = json.NewEncoder(w).Encode(op) + } + })) + if _, err := client.Observe(context.Background(), "cell-a"); err == nil || strings.Contains(err.Error(), "sensitive") { + t.Fatalf("invalid snapshot accepted or error leaked details: %v", err) + } + }) + } +} + +func TestManagedGatewayAllowsNoOperationAndReadsBackendProcess(t *testing.T) { + client := managedTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/routes/"): + _ = json.NewEncoder(w).Encode(managedTestRoute()) + case strings.Contains(r.URL.Path, "/rollouts/"): + w.WriteHeader(http.StatusNotFound) + default: + _, _ = w.Write([]byte(`{"name":"cell-a-blue","incarnation":"` + managedTestIncarnation + `","state":"ACTIVE","nodeId":"node-a","coordinatorId":"process-a","generation":4,"activeQueries":2}`)) + } + })) + got, err := client.Observe(context.Background(), "cell-a") + if err != nil || got.Rollout != nil { + t.Fatalf("missing operation must be allowed for ordinary reads: %v", err) + } + backend, err := client.Backend(context.Background(), "cell-a-blue") + if err != nil || backend.NodeID != "node-a" || backend.CoordinatorID != "process-a" { + t.Fatalf("backend process read failed: %v", err) + } +} + +func TestManagedGatewayRejectsUnsafeConfigurationAndPath(t *testing.T) { + for _, endpoint := range []string{"http://example.test", "https://user:password@example.test", "https://example.test/path", "https://example.test?token=secret", "https://example.test#fragment"} { + if _, err := newTrinoManagedGateway(endpoint, "", "rollout", strings.Repeat("t", 48)); err == nil || strings.Contains(err.Error(), "password") { + t.Fatalf("unsafe URL accepted or leaked: %v", err) + } + } + client := managedTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Error("invalid path reached server") })) + for _, name := range []string{"../cell-a", "cell-a/other", "cell a", ""} { + if _, err := client.Observe(context.Background(), name); err == nil { + t.Error("invalid group accepted") + } + if _, err := client.Backend(context.Background(), name); err == nil { + t.Error("invalid backend accepted") + } + } +} diff --git a/controlplane/trino_rollout_provisioning.go b/controlplane/trino_rollout_provisioning.go new file mode 100644 index 00000000..83a3441d --- /dev/null +++ b/controlplane/trino_rollout_provisioning.go @@ -0,0 +1,319 @@ +//go:build kubernetes + +package controlplane + +import ( + "bytes" + "context" + "crypto/subtle" + "encoding/json" + "errors" + "io" + "math" + "net/http" + "strings" + "time" + + "github.com/posthog/duckgres/controlplane/configstore" +) + +const trinoRolloutProvisioningPrefix = "/internal/trino/rollout-provisioning/" + +type trinoRolloutProvisioningCell struct { + StoredCellID string + BlueBackend string + GreenBackend string +} + +type trinoRolloutLifecycleStore interface { + GetTrinoCellLifecycle(context.Context, string) (*configstore.TrinoCellLifecycleStatus, error) + FreezeTrinoCellAdmissions(context.Context, string, string, string, string, int64) (*configstore.TrinoCellFreeze, error) + ReleaseTrinoCellAdmissions(context.Context, string, string, int64) error +} + +type trinoRolloutProvisioningResponse struct { + OperationID string `json:"operationId"` + AdmissionEpoch int64 `json:"admissionEpoch"` + Frozen bool `json:"frozen"` + Stable bool `json:"stable"` + Prepared bool `json:"prepared"` + TargetBackend string `json:"targetBackend"` + NodeID string `json:"nodeId"` + CoordinatorID string `json:"coordinatorId"` + RosterHash string `json:"rosterHash"` + AdmittedCount int `json:"admittedCount"` +} + +type trinoRolloutProvisioningHandler struct { + token string + cells map[string]trinoRolloutProvisioningCell + store trinoRolloutLifecycleStore + gateway trinoManagedGatewayReader + processProbe func(context.Context, string, string) (string, string, error) + limit chan struct{} + timeout time.Duration +} + +func newTrinoRolloutProvisioningHandler(token string, cells map[string]trinoRolloutProvisioningCell, store trinoRolloutLifecycleStore, gateway trinoManagedGatewayReader, processProbe func(context.Context, string, string) (string, string, error)) (*trinoRolloutProvisioningHandler, error) { + if len(token) < 32 || !managedGatewayValue(token, 4096) || len(cells) == 0 || len(cells) > 16 || store == nil || gateway == nil || processProbe == nil { + return nil, errors.New("invalid Trino rollout provisioning configuration") + } + configured := make(map[string]trinoRolloutProvisioningCell, len(cells)) + owners := make(map[string]bool, len(cells)) + for group, cell := range cells { + logical, registered := strings.CutPrefix(cell.StoredCellID, "registered:") + if !registered || !managedGatewayName.MatchString(logical) || !managedGatewayName.MatchString(group) || cell.BlueBackend != group+"-blue" || cell.GreenBackend != group+"-green" || owners[cell.StoredCellID] { + return nil, errors.New("rollout provisioning requires distinct registered two-slot cells") + } + owners[cell.StoredCellID] = true + configured[group] = cell + } + return &trinoRolloutProvisioningHandler{token: token, cells: configured, store: store, gateway: gateway, processProbe: processProbe, limit: make(chan struct{}, 4), timeout: 10 * time.Second}, nil +} + +func (h *trinoRolloutProvisioningHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Content-Type", "application/json") + tokens := r.Header.Values("X-Gateway-Transaction-Admin-Token") + if h.token == "" || len(tokens) != 1 || subtle.ConstantTimeCompare([]byte(tokens[0]), []byte(h.token)) != 1 { + trinoProvisioningError(w, http.StatusUnauthorized) + return + } + path, ok := strings.CutPrefix(r.URL.Path, trinoRolloutProvisioningPrefix) + parts := strings.Split(path, "/") + if !ok || len(parts) < 1 || len(parts) > 2 || r.URL.RawQuery != "" || r.URL.RawPath != "" { + trinoProvisioningError(w, http.StatusBadRequest) + return + } + cell, ok := h.cells[parts[0]] + if !ok { + trinoProvisioningError(w, http.StatusNotFound) + return + } + if (len(parts) == 1 && r.Method != http.MethodGet) || (len(parts) == 2 && (r.Method != http.MethodPost || (parts[1] != "freeze" && parts[1] != "release"))) { + trinoProvisioningError(w, http.StatusMethodNotAllowed) + return + } + select { + case h.limit <- struct{}{}: + defer func() { <-h.limit }() + default: + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + ctx, cancel := context.WithTimeout(r.Context(), h.timeout) + defer cancel() + var request trinoProvisioningMutation + if len(parts) == 2 { + controller := http.NewResponseController(w) + if controller.SetReadDeadline(time.Now().Add(h.timeout)) != nil { + r.Close = true + w.Header().Set("Connection", "close") + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + var err error + request, err = readTrinoProvisioningMutation(r.Body, parts[1]) + if err != nil { + r.Close = true + w.Header().Set("Connection", "close") + _ = r.Body.Close() + trinoProvisioningError(w, http.StatusBadRequest) + return + } + if controller.SetReadDeadline(time.Time{}) != nil { + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + } + state, err := h.store.GetTrinoCellLifecycle(ctx, cell.StoredCellID) + if err != nil || state == nil || state.CellID != cell.StoredCellID || state.AdmissionEpoch < 0 || ctx.Err() != nil { + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + if len(parts) == 1 { + writeTrinoProvisioning(w, http.StatusOK, trinoProvisioningStatus(state)) + return + } + if parts[1] == "freeze" { + h.freeze(ctx, w, parts[0], cell, state, request) + return + } + h.release(ctx, w, parts[0], cell, state, request) +} + +type trinoProvisioningMutation struct { + operation, hash string + epoch int64 +} + +func readTrinoProvisioningMutation(body io.Reader, action string) (trinoProvisioningMutation, error) { + invalid := errors.New("invalid rollout provisioning request") + data, err := io.ReadAll(io.LimitReader(body, 8193)) + if err != nil || len(data) > 8192 { + return trinoProvisioningMutation{}, invalid + } + decoder := json.NewDecoder(bytes.NewReader(data)) + start, err := decoder.Token() + if err != nil || start != json.Delim('{') { + return trinoProvisioningMutation{}, invalid + } + fields := make(map[string]json.RawMessage) + for decoder.More() { + key, err := decoder.Token() + name, ok := key.(string) + if err != nil || !ok || fields[name] != nil { + return trinoProvisioningMutation{}, invalid + } + var value json.RawMessage + if decoder.Decode(&value) != nil { + return trinoProvisioningMutation{}, invalid + } + fields[name] = value + } + end, err := decoder.Token() + if err != nil || end != json.Delim('}') || decoder.Decode(new(any)) != io.EOF || len(fields) != 3 { + return trinoProvisioningMutation{}, invalid + } + epochField := "admissionEpoch" + if action == "freeze" { + epochField = "expectedAdmissionEpoch" + } + var operation, hash string + var epoch *int64 + if json.Unmarshal(fields["operationId"], &operation) != nil || json.Unmarshal(fields["planHash"], &hash) != nil || json.Unmarshal(fields[epochField], &epoch) != nil || epoch == nil || *epoch < 0 || *epoch == math.MaxInt64 || !managedGatewayValue(operation, 256) || !managedGatewayHash.MatchString(hash) { + return trinoProvisioningMutation{}, invalid + } + return trinoProvisioningMutation{operation: operation, hash: hash, epoch: *epoch}, nil +} + +func (h *trinoRolloutProvisioningHandler) freeze(ctx context.Context, w http.ResponseWriter, group string, cell trinoRolloutProvisioningCell, state *configstore.TrinoCellLifecycleStatus, request trinoProvisioningMutation) { + if (state.Freeze == nil && state.AdmissionEpoch != request.epoch) || (state.Freeze != nil && (state.Freeze.OperationID != request.operation || state.Freeze.PlanHash != request.hash || state.AdmissionEpoch != request.epoch+1)) { + trinoProvisioningError(w, http.StatusConflict) + return + } + observation, err := h.gateway.Observe(ctx, group) + if err != nil { + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + if !trinoProvisioningPlanMatches(observation, group, cell, request) || observation.Rollout.Phase != "CLAIMED" { + trinoProvisioningError(w, http.StatusConflict) + return + } + plan, route := observation.Rollout.Plan, observation.Route + if route.Generation != plan.ExpectedRouteGeneration || route.BackendName != plan.SourceBackend || route.BackendIncarnation != plan.SourceIncarnation { + trinoProvisioningError(w, http.StatusConflict) + return + } + freeze, err := h.store.FreezeTrinoCellAdmissions(ctx, cell.StoredCellID, request.operation, request.hash, plan.TargetBackend, request.epoch) + if err != nil { + trinoProvisioningStoreError(w, err) + return + } + if freeze == nil || freeze.OperationID != request.operation || freeze.PlanHash != request.hash || freeze.AdmissionEpoch != request.epoch+1 || freeze.TargetBackend != plan.TargetBackend { + trinoProvisioningError(w, http.StatusConflict) + return + } + status := http.StatusOK + if !freeze.Stable { + status = http.StatusAccepted + } + writeTrinoProvisioning(w, status, trinoProvisioningStatus(&configstore.TrinoCellLifecycleStatus{AdmissionEpoch: freeze.AdmissionEpoch, Freeze: freeze})) +} + +// Only the guarded rollout workflow may change routes in managed mode. +// An arbitrary manual route flip bypasses the catalog freeze protocol. +func (h *trinoRolloutProvisioningHandler) release(ctx context.Context, w http.ResponseWriter, group string, cell trinoRolloutProvisioningCell, state *configstore.TrinoCellLifecycleStatus, request trinoProvisioningMutation) { + observation, err := h.gateway.Observe(ctx, group) + if err != nil { + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + if !trinoProvisioningPlanMatches(observation, group, cell, request) || managedGatewayPhase(observation.Rollout.Phase) < managedGatewayPhase("CUTOVER") { + trinoProvisioningError(w, http.StatusConflict) + return + } + plan, route := observation.Rollout.Plan, observation.Route + if plan.ExpectedRouteGeneration == math.MaxInt64 || route.Generation != plan.ExpectedRouteGeneration+1 || route.BackendName != plan.TargetBackend { + trinoProvisioningError(w, http.StatusConflict) + return + } + if state.Freeze == nil { + if state.ReleasedOperationID != request.operation || state.ReleasedAdmissionEpoch != request.epoch || state.AdmissionEpoch != request.epoch+1 { + trinoProvisioningError(w, http.StatusConflict) + return + } + writeTrinoProvisioning(w, http.StatusOK, trinoRolloutProvisioningResponse{OperationID: request.operation, AdmissionEpoch: request.epoch + 1}) + return + } + freeze := state.Freeze + if !freeze.Stable || freeze.OperationID != request.operation || freeze.PlanHash != request.hash || freeze.AdmissionEpoch != request.epoch || state.AdmissionEpoch != request.epoch || freeze.TargetBackend != plan.TargetBackend || freeze.Certificate == nil { + trinoProvisioningError(w, http.StatusConflict) + return + } + certificate := freeze.Certificate + backend, err := h.gateway.Backend(ctx, plan.TargetBackend) + if err != nil { + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + if backend == nil || backend.BackendName != plan.TargetBackend || backend.State != "ACTIVE" || backend.Incarnation != route.BackendIncarnation || certificate.TargetBackend != plan.TargetBackend || certificate.NodeID == "" || certificate.CoordinatorID == "" || backend.NodeID != certificate.NodeID || backend.CoordinatorID != certificate.CoordinatorID { + trinoProvisioningError(w, http.StatusConflict) + return + } + nodeID, coordinatorID, err := h.processProbe(ctx, group, plan.TargetBackend) + if err != nil || nodeID != certificate.NodeID || coordinatorID != certificate.CoordinatorID { + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + final, err := h.gateway.Observe(ctx, group) + if err != nil || final == nil || final.Rollout == nil || final.Route != route || *final.Rollout != *observation.Rollout || ctx.Err() != nil { + trinoProvisioningError(w, http.StatusServiceUnavailable) + return + } + if err := h.store.ReleaseTrinoCellAdmissions(ctx, cell.StoredCellID, request.operation, request.epoch); err != nil { + trinoProvisioningStoreError(w, err) + return + } + writeTrinoProvisioning(w, http.StatusOK, trinoRolloutProvisioningResponse{OperationID: request.operation, AdmissionEpoch: request.epoch + 1}) +} + +func trinoProvisioningPlanMatches(observation *trinoManagedGatewayObservation, group string, cell trinoRolloutProvisioningCell, request trinoProvisioningMutation) bool { + if observation == nil || observation.Rollout == nil { + return false + } + op := observation.Rollout + plan := op.Plan + return observation.Route.RoutingGroup == group && op.RoutingGroup == group && op.OperationID == request.operation && plan.PlanHash == request.hash && ((plan.SourceBackend == cell.BlueBackend && plan.TargetBackend == cell.GreenBackend) || (plan.SourceBackend == cell.GreenBackend && plan.TargetBackend == cell.BlueBackend)) +} + +func trinoProvisioningStatus(state *configstore.TrinoCellLifecycleStatus) trinoRolloutProvisioningResponse { + response := trinoRolloutProvisioningResponse{AdmissionEpoch: state.AdmissionEpoch, OperationID: state.ReleasedOperationID} + if freeze := state.Freeze; freeze != nil { + response.OperationID, response.Frozen, response.Stable, response.TargetBackend = freeze.OperationID, true, freeze.Stable, freeze.TargetBackend + if certificate := freeze.Certificate; certificate != nil { + response.Prepared = true + response.NodeID, response.CoordinatorID, response.RosterHash, response.AdmittedCount = certificate.NodeID, certificate.CoordinatorID, certificate.RosterHash, certificate.AdmittedCount + } + } + return response +} + +func writeTrinoProvisioning(w http.ResponseWriter, status int, response trinoRolloutProvisioningResponse) { + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(response) +} + +func trinoProvisioningError(w http.ResponseWriter, status int) { + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "rollout_provisioning_unavailable"}) +} + +func trinoProvisioningStoreError(w http.ResponseWriter, err error) { + status := http.StatusServiceUnavailable + if errors.Is(err, configstore.ErrTrinoCellConflict) { + status = http.StatusConflict + } + trinoProvisioningError(w, status) +} diff --git a/controlplane/trino_rollout_provisioning_test.go b/controlplane/trino_rollout_provisioning_test.go new file mode 100644 index 00000000..f878827d --- /dev/null +++ b/controlplane/trino_rollout_provisioning_test.go @@ -0,0 +1,405 @@ +//go:build kubernetes + +package controlplane + +import ( + "context" + "encoding/json" + "errors" + "github.com/gin-gonic/gin" + "io" + "net/http" + "net/http/httptest" + "net/http/httptrace" + "strings" + "testing" + "time" + + "github.com/posthog/duckgres/controlplane/configstore" +) + +type managedLifecycleFake struct { + status configstore.TrinoCellLifecycleStatus + writes int + pending bool + err error +} + +func (s *managedLifecycleFake) GetTrinoCellLifecycle(context.Context, string) (*configstore.TrinoCellLifecycleStatus, error) { + copy := s.status + return ©, s.err +} + +func (s *managedLifecycleFake) FreezeTrinoCellAdmissions(_ context.Context, cell, operation, hash, target string, expected int64) (*configstore.TrinoCellFreeze, error) { + if s.err != nil { + return nil, s.err + } + if s.status.Freeze != nil { + return s.status.Freeze, nil + } + if expected != s.status.AdmissionEpoch { + return nil, configstore.ErrTrinoCellConflict + } + s.writes++ + s.status.AdmissionEpoch++ + s.status.Freeze = &configstore.TrinoCellFreeze{OperationID: operation, PlanHash: hash, TargetBackend: target, AdmissionEpoch: s.status.AdmissionEpoch, Stable: !s.pending} + return s.status.Freeze, nil +} + +func (s *managedLifecycleFake) ReleaseTrinoCellAdmissions(_ context.Context, cell, operation string, epoch int64) error { + if s.err != nil { + return s.err + } + if s.status.Freeze == nil || s.status.AdmissionEpoch != epoch { + return configstore.ErrTrinoCellConflict + } + s.writes++ + s.status.Freeze = nil + s.status.ReleasedOperationID, s.status.ReleasedAdmissionEpoch = operation, epoch + s.status.AdmissionEpoch++ + return nil +} + +type managedReaderFake struct { + observation trinoManagedGatewayObservation + backend trinoManagedGatewayBackend + err error + reads int + changeOnRead int +} + +func (g *managedReaderFake) Observe(context.Context, string) (*trinoManagedGatewayObservation, error) { + g.reads++ + copy := g.observation + if g.changeOnRead == g.reads { + copy.Route.Generation++ + } + return ©, g.err +} + +func (g *managedReaderFake) Backend(context.Context, string) (*trinoManagedGatewayBackend, error) { + copy := g.backend + return ©, g.err +} + +func provisioningFixture(t *testing.T) (*trinoRolloutProvisioningHandler, *managedLifecycleFake, *managedReaderFake) { + t.Helper() + store := &managedLifecycleFake{status: configstore.TrinoCellLifecycleStatus{CellID: "registered:logical-a", AdmissionEpoch: 7}} + op := managedTestRollout() + reader := &managedReaderFake{observation: trinoManagedGatewayObservation{Route: managedTestRoute(), Rollout: &op}} + handler, err := newTrinoRolloutProvisioningHandler(strings.Repeat("t", 48), map[string]trinoRolloutProvisioningCell{ + "cell-a": {StoredCellID: "registered:logical-a", BlueBackend: "cell-a-blue", GreenBackend: "cell-a-green"}, + }, store, reader, func(context.Context, string, string) (string, string, error) { + return reader.backend.NodeID, reader.backend.CoordinatorID, nil + }) + if err != nil { + t.Fatal(err) + } + return handler, store, reader +} + +func provisioningRequest(h http.Handler, method, suffix, body string) *httptest.ResponseRecorder { + r := httptest.NewRequest(method, "/internal/trino/rollout-provisioning/cell-a"+suffix, strings.NewReader(body)) + r.Header.Set("X-Gateway-Transaction-Admin-Token", strings.Repeat("t", 48)) + w := httptest.NewRecorder() + h.ServeHTTP(provisioningDeadlineRecorder{w}, r) + return w +} + +type provisioningDeadlineRecorder struct{ *httptest.ResponseRecorder } + +func (provisioningDeadlineRecorder) SetReadDeadline(time.Time) error { return nil } + +func freezeRequestBody() string { + return `{"operationId":"operation-a","planHash":"` + strings.Repeat("a", 64) + `","expectedAdmissionEpoch":7}` +} +func releaseRequestBody() string { + return `{"operationId":"operation-a","planHash":"` + strings.Repeat("a", 64) + `","admissionEpoch":8}` +} + +func decodeProvisioning(t *testing.T, w *httptest.ResponseRecorder) trinoRolloutProvisioningResponse { + t.Helper() + var response trinoRolloutProvisioningResponse + if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil { + t.Fatal(err) + } + return response +} + +func TestRolloutProvisioningFreezePollAndReadOnlyStatus(t *testing.T) { + h, store, gateway := provisioningFixture(t) + w := provisioningRequest(h, http.MethodGet, "", "") + response := decodeProvisioning(t, w) + if w.Code != 200 || response.AdmissionEpoch != 7 || response.Frozen || store.writes != 0 || gateway.reads != 0 { + t.Fatal("GET must be a read-only epoch observation") + } + store.pending = true + w = provisioningRequest(h, http.MethodPost, "/freeze", freezeRequestBody()) + response = decodeProvisioning(t, w) + if w.Code != 202 || !response.Frozen || response.Stable || response.AdmissionEpoch != 8 || response.TargetBackend != "cell-a-green" || store.writes != 1 { + t.Fatalf("pending freeze: %d %s", w.Code, w.Body.String()) + } + store.status.Freeze.Stable = true + w = provisioningRequest(h, http.MethodPost, "/freeze", freezeRequestBody()) + if w.Code != 200 || store.writes != 1 || !decodeProvisioning(t, w).Stable { + t.Fatal("same operation must poll without another epoch change") + } +} + +func TestRolloutProvisioningRejectsInvalidAuthorityAndBodies(t *testing.T) { + h, store, gateway := provisioningFixture(t) + for _, headers := range [][]string{nil, {"wrong"}, {strings.Repeat("t", 48), strings.Repeat("t", 48)}} { + r := httptest.NewRequest(http.MethodPost, "/internal/trino/rollout-provisioning/cell-a/freeze", strings.NewReader(freezeRequestBody())) + for _, value := range headers { + r.Header.Add("X-Gateway-Transaction-Admin-Token", value) + } + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != 401 { + t.Errorf("invalid capability accepted: %d", w.Code) + } + } + for _, body := range []string{`{}`, `null`, freezeRequestBody() + `{}`, strings.Replace(freezeRequestBody(), `,"expectedAdmissionEpoch":7`, "", 1), strings.Replace(freezeRequestBody(), `:7`, `:-1`, 1), strings.Replace(freezeRequestBody(), `:7`, `:7.1`, 1), strings.Replace(freezeRequestBody(), `:7`, `:7,"expectedAdmissionEpoch":7`, 1), strings.Replace(freezeRequestBody(), `}`, `,"targetBackend":"cell-b-blue"}`, 1), strings.Repeat(" ", 8193)} { + w := provisioningRequest(h, http.MethodPost, "/freeze", body) + if w.Code != 400 { + t.Errorf("invalid body accepted: %d", w.Code) + } + } + if store.writes != 0 || gateway.reads != 0 { + t.Fatal("invalid authority or syntax reached the data plane") + } +} + +func TestRolloutProvisioningFreezeRejectsStaleOrForeignPlan(t *testing.T) { + for _, mode := range []string{"stale_epoch", "wrong_operation", "wrong_hash", "wrong_phase", "foreign_source", "foreign_target", "route_changed", "no_operation", "gateway_error", "store_error"} { + t.Run(mode, func(t *testing.T) { + h, store, gateway := provisioningFixture(t) + switch mode { + case "stale_epoch": + store.status.AdmissionEpoch++ + case "wrong_operation": + gateway.observation.Rollout.OperationID = "another" + case "wrong_hash": + gateway.observation.Rollout.Plan.PlanHash = strings.Repeat("b", 64) + case "wrong_phase": + gateway.observation.Rollout.Phase = "WARMED" + case "foreign_source": + gateway.observation.Rollout.Plan.SourceBackend = "cell-b-blue" + case "foreign_target": + gateway.observation.Rollout.Plan.TargetBackend = "cell-b-green" + case "route_changed": + gateway.observation.Route.Generation++ + case "no_operation": + gateway.observation.Rollout = nil + case "gateway_error": + gateway.err = errors.New("sensitive error") + case "store_error": + store.err = errors.New("sensitive database") + } + w := provisioningRequest(h, http.MethodPost, "/freeze", freezeRequestBody()) + if w.Code < 400 || store.writes != 0 || strings.Contains(w.Body.String(), "sensitive") { + t.Fatalf("unsafe freeze: %d %s", w.Code, w.Body.String()) + } + }) + } +} + +func preparedFixture(t *testing.T) (*trinoRolloutProvisioningHandler, *managedLifecycleFake, *managedReaderFake) { + t.Helper() + h, store, gateway := provisioningFixture(t) + store.status.AdmissionEpoch = 8 + store.status.Freeze = &configstore.TrinoCellFreeze{OperationID: "operation-a", PlanHash: strings.Repeat("a", 64), TargetBackend: "cell-a-green", AdmissionEpoch: 8, Stable: true, + Certificate: &configstore.TrinoCellCertificate{TargetBackend: "cell-a-green", NodeID: "node-new", CoordinatorID: "process-new", RosterHash: strings.Repeat("c", 64), AdmittedCount: 3}} + gateway.observation.Rollout.Phase = "CUTOVER" + gateway.observation.Route = trinoManagedGatewayRoute{RoutingGroup: "cell-a", Generation: 4, BackendName: "cell-a-green", BackendIncarnation: managedTestTargetIncarnation} + gateway.backend = trinoManagedGatewayBackend{BackendName: "cell-a-green", Incarnation: managedTestTargetIncarnation, State: "ACTIVE", NodeID: "node-new", CoordinatorID: "process-new"} + return h, store, gateway +} + +func TestRolloutProvisioningReleaseReceiptAndReplay(t *testing.T) { + h, store, _ := preparedFixture(t) + before := provisioningRequest(h, http.MethodGet, "", "") + if !decodeProvisioning(t, before).Prepared { + t.Fatal("GET must expose the immutable certificate without changing it") + } + for range 2 { + w := provisioningRequest(h, http.MethodPost, "/release", releaseRequestBody()) + result := decodeProvisioning(t, w) + if w.Code != 200 || result.OperationID != "operation-a" || result.AdmissionEpoch != 9 || result.Frozen || result.Prepared || store.writes != 1 { + t.Fatalf("release receipt: %d %s", w.Code, w.Body.String()) + } + } +} + +func TestRolloutProvisioningReleaseRequiresExactCutoverProcess(t *testing.T) { + for _, mode := range []string{"unprepared", "unstable", "wrong_phase", "old_route", "wrong_incarnation", "restarted", "live_restarted", "live_unavailable", "not_active", "changed_after_backend", "wrong_plan", "wrong_epoch"} { + t.Run(mode, func(t *testing.T) { + h, store, gateway := preparedFixture(t) + switch mode { + case "unprepared": + store.status.Freeze.Certificate = nil + case "unstable": + store.status.Freeze.Stable = false + case "wrong_phase": + gateway.observation.Rollout.Phase = "VERIFIED" + case "old_route": + gateway.observation.Route = managedTestRoute() + case "wrong_incarnation": + gateway.backend.Incarnation = managedTestIncarnation + case "restarted": + gateway.backend.CoordinatorID = "replacement" + case "live_restarted": + h.processProbe = func(context.Context, string, string) (string, string, error) { + return gateway.backend.NodeID, "different-live-process", nil + } + case "live_unavailable": + h.processProbe = func(context.Context, string, string) (string, string, error) { + return "", "", errors.New("sensitive live error") + } + case "not_active": + gateway.backend.State = "SEALED" + case "changed_after_backend": + gateway.changeOnRead = 2 + case "wrong_plan": + gateway.observation.Rollout.Plan.PlanHash = strings.Repeat("d", 64) + case "wrong_epoch": + store.status.AdmissionEpoch++ + } + w := provisioningRequest(h, http.MethodPost, "/release", releaseRequestBody()) + if w.Code < 400 || store.writes != 0 { + t.Fatalf("unsafe release: %d %s", w.Code, w.Body.String()) + } + }) + } +} + +func TestRolloutProvisioningUsesGatewayWireContractAcrossReincarnation(t *testing.T) { + h, store, fixture := provisioningFixture(t) + op := *fixture.observation.Rollout + op.Plan.TargetIncarnation = managedTestTargetIncarnation + route := fixture.observation.Route + backend := trinoManagedGatewayBackend{BackendName: "cell-a-green", Incarnation: "33333333-3333-4333-8333-333333333333", State: "ACTIVE", NodeID: "fresh-node", CoordinatorID: "fresh-process"} + h.processProbe = func(context.Context, string, string) (string, string, error) { + return backend.NodeID, backend.CoordinatorID, nil + } + h.gateway = managedTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/routes/"): + _ = json.NewEncoder(w).Encode(route) + case strings.Contains(r.URL.Path, "/rollouts/"): + _ = json.NewEncoder(w).Encode(op) + case strings.Contains(r.URL.Path, "/backends/"): + _ = json.NewEncoder(w).Encode(backend) + default: + t.Error("unexpected Gateway request") + w.WriteHeader(404) + } + })) + if w := provisioningRequest(h, http.MethodPost, "/freeze", freezeRequestBody()); w.Code != 200 { + t.Fatalf("actual CLAIMED version-zero shape rejected: %d %s", w.Code, w.Body.String()) + } + store.status.Freeze.Certificate = &configstore.TrinoCellCertificate{TargetBackend: backend.BackendName, NodeID: backend.NodeID, CoordinatorID: backend.CoordinatorID, RosterHash: strings.Repeat("c", 64), AdmittedCount: 1} + op.Phase, op.Version = "CUTOVER", 6 + route.Generation, route.BackendName, route.BackendIncarnation = 4, backend.BackendName, backend.Incarnation + if w := provisioningRequest(h, http.MethodPost, "/release", releaseRequestBody()); w.Code != 200 { + t.Fatalf("current reincarnated target must match certificate, not prewarm plan incarnation: %d %s", w.Code, w.Body.String()) + } + if store.writes != 2 { + t.Fatal("expected exactly one freeze and one release") + } +} + +func TestRolloutProvisioningBoundsSlowRequestBody(t *testing.T) { + for name, body := range map[string]string{"incomplete": `{"operationId":`, "oversized": strings.Repeat(" ", 8193)} { + t.Run(name, func(t *testing.T) { testProvisioningSlowBody(t, body) }) + } +} + +func testProvisioningSlowBody(t *testing.T, body string) { + t.Helper() + h, store, _ := provisioningFixture(t) + h.timeout = 50 * time.Millisecond + router := gin.New() + router.Any(trinoRolloutProvisioningPrefix+"*path", gin.WrapH(h)) + server := httptest.NewServer(router) + defer server.Close() + reader, writer := io.Pipe() + defer func() { _ = reader.Close() }() + defer func() { _ = writer.Close() }() + request, err := http.NewRequest(http.MethodPost, server.URL+trinoRolloutProvisioningPrefix+"cell-a/freeze", reader) + if err != nil { + t.Fatal(err) + } + request.Header.Set("X-Gateway-Transaction-Admin-Token", strings.Repeat("t", 48)) + finished := make(chan error, 1) + go func() { + response, err := server.Client().Do(request) + if err == nil { + defer func() { _ = response.Body.Close() }() + if response.StatusCode < 400 { + err = errors.New("slow incomplete body was accepted") + } + } + finished <- err + }() + if _, err := writer.Write([]byte(body)); err != nil { + t.Fatal(err) + } + select { + case err := <-finished: + if err != nil { + t.Fatal(err) + } + case <-time.After(500 * time.Millisecond): + _ = writer.Close() + <-finished + t.Fatal("handler deadline did not interrupt the body read") + } + if store.writes != 0 { + t.Fatal("incomplete body mutated lifecycle state") + } + if len(h.limit) != 0 { + t.Fatal("slow body leaked its request slot") + } +} + +func TestRolloutProvisioningResetsSuccessfulBodyDeadline(t *testing.T) { + h, _, _ := provisioningFixture(t) + h.timeout = 50 * time.Millisecond + router := gin.New() + router.Any(trinoRolloutProvisioningPrefix+"*path", gin.WrapH(h)) + server := httptest.NewServer(router) + defer server.Close() + for attempt := range 2 { + if attempt == 1 { + time.Sleep(75 * time.Millisecond) + } + request, err := http.NewRequest(http.MethodPost, server.URL+trinoRolloutProvisioningPrefix+"cell-a/freeze", strings.NewReader(freezeRequestBody())) + if err != nil { + t.Fatal(err) + } + request.Header.Set("X-Gateway-Transaction-Admin-Token", strings.Repeat("t", 48)) + reused := false + request = request.WithContext(httptrace.WithClientTrace(request.Context(), &httptrace.ClientTrace{GotConn: func(info httptrace.GotConnInfo) { reused = info.Reused }})) + response, err := server.Client().Do(request) + if err != nil { + t.Fatal(err) + } + _, _ = io.Copy(io.Discard, response.Body) + _ = response.Body.Close() + if response.StatusCode != 200 || (attempt == 1 && !reused) { + t.Fatal("successful body deadline prevented connection reuse") + } + } +} + +func TestRolloutProvisioningRejectsUnsupportedDeadlineWriter(t *testing.T) { + h, store, _ := provisioningFixture(t) + r := httptest.NewRequest(http.MethodPost, trinoRolloutProvisioningPrefix+"cell-a/freeze", strings.NewReader(freezeRequestBody())) + r.Header.Set("X-Gateway-Transaction-Admin-Token", strings.Repeat("t", 48)) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != 503 || store.writes != 0 || !r.Close { + t.Fatal("unsupported writer must fail closed before reading or mutating") + } +} From 93e4c9e5758cebf0f79c4fd5eb682b4492be50cb Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Mon, 14 Sep 2026 17:16:31 +0200 Subject: [PATCH 5/7] Reconcile shared Trino catalogs through the active Gateway route --- CLAUDE.md | 15 +- .../configstore/trino_cell_admissions.go | 15 + controlplane/multitenant.go | 22 +- .../provisioner/trino_managed_catalogs.go | 189 +++++++++ .../trino_managed_catalogs_test.go | 362 ++++++++++++++++++ controlplane/provisioner/trino_provisioner.go | 82 +++- controlplane/trino_inputs.go | 24 +- controlplane/trino_managed_wiring.go | 130 +++++++ controlplane/trino_managed_wiring_test.go | 141 +++++++ controlplane/trino_registry.go | 19 +- controlplane/trino_rollout_probe.go | 9 +- docs/runbooks/trino-shared-catalogs.md | 107 ++++++ docs/trino-cells.md | 6 + 13 files changed, 1096 insertions(+), 25 deletions(-) create mode 100644 controlplane/configstore/trino_cell_admissions.go create mode 100644 controlplane/provisioner/trino_managed_catalogs.go create mode 100644 controlplane/provisioner/trino_managed_catalogs_test.go create mode 100644 controlplane/trino_managed_wiring.go create mode 100644 controlplane/trino_managed_wiring_test.go create mode 100644 docs/runbooks/trino-shared-catalogs.md diff --git a/CLAUDE.md b/CLAUDE.md index 251eafc2..44a7e7b6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1604,11 +1604,24 @@ password/tenant/catalog changes never propagate. internal-communication shared secret would split-brain a running cluster. The admin password/hash pair is the deliberate exception (no external consumer ⇒ regenerate-if-missing self-heals). -- **Catalog reconcile is `SHOW CATALOGS` first**: create only what's missing, +- **Static catalog reconcile is `SHOW CATALOGS` first**: create only what's missing, drop only names matching `opa.ManagedCatalogPattern` that aren't wanted, so `system`, `jmx` and hand-made catalogs survive. An org whose password is momentarily unresolvable keeps its existing catalog (never dropped) but is NOT reported ready. +- **Shared catalog mode is explicit and fenced.** Registered cells can use + `catalog_management: paused` for compatibility rollout or `gateway-shared` + for sole-active-backend catalog writes. Freeze new admissions and DDL before + warming the standby; bulk-check catalog states and admitted tenant credentials, + cut over, then release before draining. See + [docs/runbooks/trino-shared-catalogs.md](docs/runbooks/trino-shared-catalogs.md). + Never replay CREATE across both slots. A remote FAILED query is not proof + that synchronous DDL stopped; retain the durable intent until fenced recovery. + Paused and non-owner replicas still refresh auth and OPA projections. +- **Provisioner catalog inventory is narrowly authorized.** The admin can read + `catalog_name` and `state` from `system.metadata.catalogs`, in addition to + the existing node inventory. The observer and customer principals cannot. + `FAILING` startup placeholders are not usable catalogs. - **Catalog naming is a THREE-way contract**: `TrinoCatalogName` (`org_` + sanitized org id, no `_iceberg` suffix — warehouses are DuckLake), `opa.ManagedCatalogPattern`, and the regex literal inside `policy.rego`. diff --git a/controlplane/configstore/trino_cell_admissions.go b/controlplane/configstore/trino_cell_admissions.go new file mode 100644 index 00000000..be20b3b2 --- /dev/null +++ b/controlplane/configstore/trino_cell_admissions.go @@ -0,0 +1,15 @@ +package configstore + +import "context" + +// ListAdmittedTrinoOrgs includes previous admissions after transient state failures. +// Missing credential rows cannot remove a warehouse from the rollout certificate. +func (cs *ConfigStore) ListAdmittedTrinoOrgs(ctx context.Context, cell string) ([]TrinoEnabledOrg, error) { + var orgs []TrinoEnabledOrg + err := cs.db.WithContext(ctx).Table("duckgres_managed_warehouse_trino AS t"). + Select("t.org_id, COALESCE(o.database_name, '') AS database_name, t.trino_cell_id AS cell_id, t.state"). + Joins("LEFT JOIN duckgres_orgs AS o ON o.name = t.org_id"). + Where("t.enabled = ? AND t.trino_cell_id = ? AND (t.ready_at IS NOT NULL OR t.state = ?)", true, cell, ManagedWarehouseStateReady). + Order("t.org_id").Scan(&orgs).Error + return orgs, err +} diff --git a/controlplane/multitenant.go b/controlplane/multitenant.go index 7f141d8e..1429fd27 100644 --- a/controlplane/multitenant.go +++ b/controlplane/multitenant.go @@ -508,6 +508,8 @@ func SetupMultiTenant( // Start provisioning controller (best-effort — K8s API may not be available locally) var trinoCells trinoFleet + var rolloutReadiness *trinoRolloutReadinessHandler + var rolloutProvisioning *trinoRolloutProvisioningHandler provCtrl, err := provisioner.NewController(store, 10*time.Second) if err != nil { // Without the controller, the Trino reconcile loop cannot run. @@ -550,6 +552,14 @@ 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") } + rolloutReadiness, twErr = buildTrinoRolloutReadiness(trinoWire, store) + if twErr != nil { + return nil, nil, nil, nil, nil, nil, twErr + } + rolloutProvisioning, twErr = buildTrinoManagedFleet(trinoWire, store, rolloutReadiness) + if twErr != nil { + return nil, nil, nil, nil, nil, nil, twErr + } provCtrl.WithTrinoReconciler(trinoWire) trinoCells = trinoWire for _, wire := range trinoWire { @@ -792,13 +802,19 @@ func SetupMultiTenant( } } - rolloutReadiness, rolloutErr := buildTrinoRolloutReadiness(trinoCells, store) - if rolloutErr != nil { - return nil, nil, nil, nil, nil, nil, rolloutErr + if rolloutReadiness == nil { + var rolloutErr error + rolloutReadiness, rolloutErr = buildTrinoRolloutReadiness(trinoCells, store) + if rolloutErr != nil { + return nil, nil, nil, nil, nil, nil, rolloutErr + } } if rolloutReadiness != nil { engine.Any(rolloutReadinessPrefix+"*slot", gin.WrapH(rolloutReadiness)) } + if rolloutProvisioning != nil { + engine.Any(trinoRolloutProvisioningPrefix+"*group", gin.WrapH(rolloutProvisioning)) + } // Trino OPA bundle endpoint. Mounted OUTSIDE the /api/v1 admin group on // purpose — it does its own bearer-token auth (the bundle exposes the diff --git a/controlplane/provisioner/trino_managed_catalogs.go b/controlplane/provisioner/trino_managed_catalogs.go new file mode 100644 index 00000000..e55d7c9a --- /dev/null +++ b/controlplane/provisioner/trino_managed_catalogs.go @@ -0,0 +1,189 @@ +//go:build kubernetes + +package provisioner + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "sort" + "strings" + "time" + + "github.com/google/uuid" + "github.com/posthog/duckgres/controlplane/configstore" +) + +type TrinoCellLifecycleStore interface { + BeginTrinoCellReconcile(context.Context, string, string) (*configstore.TrinoCellLease, bool, error) + GetTrinoCellLifecycle(context.Context, string) (*configstore.TrinoCellLifecycleStatus, error) + SetTrinoCellIntent(context.Context, configstore.TrinoCellLease, configstore.TrinoCatalogIntent) error + ClearTrinoCellIntent(context.Context, configstore.TrinoCellLease, string) error + FinishTrinoCellReconcile(context.Context, configstore.TrinoCellLease) error + CertifyTrinoCellTarget(context.Context, configstore.TrinoCellLease, string, configstore.TrinoCellCertificate) error + UpdateManagedTrinoState(context.Context, configstore.TrinoCellLease, string, configstore.TrinoStateUpdate) (bool, error) + ListAdmittedTrinoOrgs(context.Context, string) ([]configstore.TrinoEnabledOrg, error) +} + +type TrinoManagedBackend struct { + Name string + Catalog TrinoCatalogClient +} + +type TrinoManagedCatalogOpts struct { + Paused bool + Store TrinoCellLifecycleStore + CatalogClients []TrinoCatalogClient + Active func(context.Context) (*TrinoManagedBackend, error) + Target func(context.Context, *configstore.TrinoCellFreeze) (*TrinoManagedBackend, error) + TargetProcess func(context.Context, string) (nodeID, coordinatorID string, err error) +} + +// ConfigureManagedCatalogs runs before the provisioner loop starts. +func (p *TrinoProvisioner) ConfigureManagedCatalogs(opts *TrinoManagedCatalogOpts) error { + if opts == nil || !p.explicitAssignmentOnly || !strings.HasPrefix(p.cellID, "registered:") || len(p.additionalCatalogs) != 0 { + return errors.New("managed catalogs require an explicitly assigned registered cell without static extra writers") + } + if !opts.Paused { + if opts.Store == nil || opts.Active == nil || opts.Target == nil || opts.TargetProcess == nil || len(opts.CatalogClients) != 2 { + return errors.New("managed catalogs require lifecycle, Gateway, target readiness and both strict clients") + } + for _, client := range opts.CatalogClients { + if client == nil { + return errors.New("managed catalog client missing") + } + } + } + p.managed = opts + return nil +} + +type trinoManagedCatalogClient struct { + TrinoCatalogClient + store TrinoCellLifecycleStore + lease configstore.TrinoCellLease + backend string + sequence int64 +} + +func (c *trinoManagedCatalogClient) mutate(ctx context.Context, action, name string, submit func() error) error { + intent := configstore.TrinoCatalogIntent{ID: uuid.NewString(), Sequence: c.sequence + 1, Backend: c.backend, Action: action, Catalog: name} + if err := c.store.SetTrinoCellIntent(ctx, c.lease, intent); err != nil { + return err + } + c.sequence = intent.Sequence + err := submit() + if !TrinoCatalogOutcomeTerminal(err) { + return err + } + clearCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 3*time.Second) + defer cancel() + if clearErr := c.store.ClearTrinoCellIntent(clearCtx, c.lease, intent.ID); clearErr != nil { + return errors.Join(err, clearErr) + } + return err +} + +func (c *trinoManagedCatalogClient) CreateCatalog(ctx context.Context, name string, props map[string]string) error { + return c.mutate(ctx, "create", name, func() error { return c.TrinoCatalogClient.CreateCatalog(ctx, name, props) }) +} + +func (c *trinoManagedCatalogClient) DropCatalog(ctx context.Context, name string) error { + return c.mutate(ctx, "drop", name, func() error { return c.TrinoCatalogClient.DropCatalog(ctx, name) }) +} + +func (*trinoManagedCatalogClient) AlterCatalog(context.Context, string, map[string]string) error { + return errors.New("managed catalog alteration requires an explicit migration") +} + +func (p *TrinoProvisioner) managedCatalogs(ctx context.Context, lease configstore.TrinoCellLease, orgs []configstore.TrinoEnabledOrg, tenants tenantSecretProjection) (map[string]catalogOutcome, error) { + state, err := p.managed.Store.GetTrinoCellLifecycle(ctx, p.cellID) + if err != nil { + return nil, err + } + if state.Freeze != nil { + return nil, p.prepareManagedTarget(ctx, lease, state.Freeze, tenants) + } + backend, err := p.managed.Active(ctx) + if err != nil { + return nil, err + } + if backend == nil || backend.Catalog == nil || backend.Name == "" { + return nil, errors.New("managed active backend unavailable") + } + pending, err := p.reconcileBackendReadiness(ctx, backend.Catalog, tenants.data) + if err != nil { + return nil, err + } + if len(pending) != 0 { + outcomes := make(map[string]catalogOutcome, len(orgs)) + for _, org := range orgs { + outcomes[org.OrgID] = catalogOutcome{Pending: true, PendingReason: "waiting for mounted credentials before catalog mutation"} + } + return outcomes, nil + } + client := &trinoManagedCatalogClient{TrinoCatalogClient: backend.Catalog, store: p.managed.Store, lease: lease, backend: backend.Name, sequence: lease.IntentSequence} + return p.reconcileBoundedBackend(ctx, orgs, tenants, client) +} + +func (p *TrinoProvisioner) prepareManagedTarget(ctx context.Context, lease configstore.TrinoCellLease, freeze *configstore.TrinoCellFreeze, tenants tenantSecretProjection) error { + if !freeze.Stable || freeze.Certificate != nil { + return nil + } + orgs, err := p.managed.Store.ListAdmittedTrinoOrgs(ctx, p.cellID) + if err != nil { + return err + } + backend, err := p.managed.Target(ctx, freeze) + if err != nil { + return err + } + if backend == nil { + return nil + } + if backend.Name != freeze.TargetBackend { + return errors.New("managed target differs from frozen target") + } + inventory, ok := backend.Catalog.(interface { + CatalogStates(context.Context) (map[string]string, error) + }) + if !ok { + return errors.New("managed target lacks strict catalog inventory") + } + node, coordinator, err := p.managed.TargetProcess(ctx, backend.Name) + if err != nil { + return err + } + states, err := inventory.CatalogStates(ctx) + if err != nil { + return err + } + var roster []string + expected := make(map[string][]byte) + for _, org := range orgs { + name := TrinoCatalogName(org.TrinoPrincipal()) + if org.TrinoPrincipal() == "" || states[name] != "OPERATIONAL" || !tenants.projected[org.OrgID] || len(tenants.data[org.OrgID]) == 0 { + return errors.New("managed target is missing an admitted catalog or credential") + } + roster = append(roster, org.OrgID+"\x00"+org.TrinoPrincipal()+"\x00"+name) + expected[org.OrgID] = tenants.data[org.OrgID] + } + pending, err := p.reconcileBackendReadiness(ctx, backend.Catalog, expected) + if err != nil || len(pending) != 0 { + return errors.New("managed target credential projection is not ready") + } + afterNode, afterCoordinator, err := p.managed.TargetProcess(ctx, backend.Name) + if err != nil || node != afterNode || coordinator != afterCoordinator { + return errors.New("managed target process changed during certification") + } + sort.Strings(roster) + encoded, err := json.Marshal(roster) + if err != nil { + return fmt.Errorf("encode admitted catalog roster: %w", err) + } + hash := sha256.Sum256(encoded) + return p.managed.Store.CertifyTrinoCellTarget(ctx, lease, freeze.OperationID, configstore.TrinoCellCertificate{TargetBackend: backend.Name, NodeID: node, CoordinatorID: coordinator, RosterHash: hex.EncodeToString(hash[:]), AdmittedCount: len(roster)}) +} diff --git a/controlplane/provisioner/trino_managed_catalogs_test.go b/controlplane/provisioner/trino_managed_catalogs_test.go new file mode 100644 index 00000000..a43f4eaa --- /dev/null +++ b/controlplane/provisioner/trino_managed_catalogs_test.go @@ -0,0 +1,362 @@ +//go:build kubernetes + +package provisioner + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + + "github.com/posthog/duckgres/controlplane/configstore" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type managedIntentFixture struct { + TrinoCellLifecycleStore + intent bool + sequence int64 + claims, clears int +} + +type managedLifecycleFixture struct { + managedIntentFixture + store *fakeTrinoStore + blocked bool + freeze *configstore.TrinoCellFreeze + admitted []configstore.TrinoEnabledOrg + certificate *configstore.TrinoCellCertificate + finishes int +} + +func (s *managedLifecycleFixture) BeginTrinoCellReconcile(_ context.Context, cell, owner string) (*configstore.TrinoCellLease, bool, error) { + return &configstore.TrinoCellLease{CellID: cell, Owner: owner, AdmissionEpoch: 1, IntentSequence: s.sequence}, !s.blocked, nil +} +func (s *managedLifecycleFixture) GetTrinoCellLifecycle(context.Context, string) (*configstore.TrinoCellLifecycleStatus, error) { + return &configstore.TrinoCellLifecycleStatus{AdmissionEpoch: 1, Freeze: s.freeze}, nil +} +func (s *managedLifecycleFixture) FinishTrinoCellReconcile(context.Context, configstore.TrinoCellLease) error { + if s.intent { + return configstore.ErrTrinoCellConflict + } + s.finishes++ + return nil +} +func (s *managedLifecycleFixture) UpdateManagedTrinoState(_ context.Context, _ configstore.TrinoCellLease, org string, update configstore.TrinoStateUpdate) (bool, error) { + if s.freeze != nil && update.State == configstore.ManagedWarehouseStateReady { + return false, nil + } + return true, s.store.UpdateTrinoState(org, update) +} +func (s *managedLifecycleFixture) ListAdmittedTrinoOrgs(context.Context, string) ([]configstore.TrinoEnabledOrg, error) { + return s.admitted, nil +} +func (s *managedLifecycleFixture) CertifyTrinoCellTarget(_ context.Context, _ configstore.TrinoCellLease, _ string, certificate configstore.TrinoCellCertificate) error { + s.certificate = &certificate + return nil +} + +type managedInventoryFixture struct { + *fakeCatalogClient + states map[string]string + reads int +} + +func (c *managedInventoryFixture) CatalogStates(context.Context) (map[string]string, error) { + c.reads++ + return c.states, nil +} + +func managedHarness(t *testing.T) (*testProvisionerHarness, *managedLifecycleFixture, *fakeCatalogClient) { + t.Helper() + h := trinoReadinessHarness(t) + h.provisioner.cellID = "registered:cell-test" + h.provisioner.explicitAssignmentOnly = true + for i := range h.store.orgs { + h.store.orgs[i].CellID = h.provisioner.cellID + } + s := &managedLifecycleFixture{store: h.store} + green := &fakeCatalogClient{} + opts := &TrinoManagedCatalogOpts{Store: s, CatalogClients: []TrinoCatalogClient{h.catalog, green}, Active: func(context.Context) (*TrinoManagedBackend, error) { + return &TrinoManagedBackend{Name: "group-blue", Catalog: h.catalog}, nil + }, Target: func(context.Context, *configstore.TrinoCellFreeze) (*TrinoManagedBackend, error) { return nil, nil }, TargetProcess: func(context.Context, string) (string, string, error) { return "node", "abcde", nil }} + if err := h.provisioner.ConfigureManagedCatalogs(opts); err != nil { + t.Fatal(err) + } + return h, s, green +} + +func TestManagedPausedAndFollowerPreserveOPAWithoutDDL(t *testing.T) { + for _, paused := range []bool{true, false} { + h, s, green := managedHarness(t) + h.provisioner.managed.Paused = paused + s.blocked = !paused + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(h.catalog.created) != 0 || len(green.created) != 0 || len(h.store.states) != 0 || len(h.builder.last) == 0 { + t.Fatal("paused/follower path mutated catalogs or lost local OPA refresh") + } + } +} + +func TestManagedColdStandbyAndRouteChange(t *testing.T) { + h, s, green := managedHarness(t) + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(h.catalog.created) != 2 || len(green.created) != 0 || s.intent { + t.Fatal("active-only initial provisioning failed") + } + green.existing = append([]string(nil), h.catalog.existing...) + h.store.orgs = append(h.store.orgs, configstore.TrinoEnabledOrg{OrgID: "tenant-c", DatabaseName: "tenant-c", CellID: h.provisioner.cellID, RootPasswordHash: "hash-c"}) + h.ducklings["tenant-c"] = readyDuckling("tenant-c") + h.warehouses.rows["tenant-c"] = readyWarehouse("tenant-c") + h.provisioner.managed.Active = func(context.Context) (*TrinoManagedBackend, error) { + return &TrinoManagedBackend{Name: "group-green", Catalog: green}, nil + } + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(green.created) != 1 || green.created["org_tenant_c"] == nil || len(h.catalog.created) != 2 || s.intent { + t.Fatal("cutover replayed existing catalogs or provisioned the old source") + } +} + +func TestManagedProjectionLagDoesNotSubmitDDL(t *testing.T) { + h, s, _ := managedHarness(t) + lag := true + h.provisioner.secretReadiness = &fakeTrinoSecretReadiness{check: func(context.Context, string, string, map[string][]byte, []TrinoNode) (map[string]string, error) { + if lag { + return map[string]string{"tenant-a": "not mounted"}, nil + } + return nil, nil + }} + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if s.claims != 0 || s.intent || len(h.catalog.created) != 0 { + t.Fatal("projection lag claimed or submitted DDL") + } + lag = false + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if s.claims != 2 || s.intent { + t.Fatal("mounted credentials did not allow next reconcile") + } +} + +func TestManagedFrozenCertificateIncludesPreviousAdmissions(t *testing.T) { + h, s, _ := managedHarness(t) + s.freeze = &configstore.TrinoCellFreeze{OperationID: "operation", TargetBackend: "group-green", Stable: true, AdmissionEpoch: 1} + s.admitted = []configstore.TrinoEnabledOrg{{OrgID: "tenant-a", DatabaseName: "tenant-a", State: configstore.ManagedWarehouseStateFailed}} + target := &managedInventoryFixture{fakeCatalogClient: &fakeCatalogClient{}, states: map[string]string{}} + h.provisioner.managed.Target = func(context.Context, *configstore.TrinoCellFreeze) (*TrinoManagedBackend, error) { + return &TrinoManagedBackend{Name: "group-green", Catalog: target}, nil + } + if err := h.provisioner.Reconcile(context.Background()); err == nil || s.certificate != nil { + t.Fatal("previously admitted missing catalog was omitted") + } + target.states["org_tenant_a"] = "OPERATIONAL" + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if s.certificate == nil || s.certificate.AdmittedCount != 1 || s.claims != 0 || len(target.created) != 0 { + t.Fatal("frozen bulk certificate replayed DDL or omitted admission") + } +} + +func TestManagedTenThousandExistingCatalogsIssueNoDDL(t *testing.T) { + h, s, _ := managedHarness(t) + orgs := make([]configstore.TrinoEnabledOrg, 0, 10000) + data := make(map[string][]byte, 10000) + projected := make(map[string]bool, 10000) + for i := range 10000 { + name := fmt.Sprintf("tenant-%d", i) + orgs = append(orgs, configstore.TrinoEnabledOrg{OrgID: name, DatabaseName: name}) + data[name] = []byte("fixture-password") + projected[name] = true + h.catalog.existing = append(h.catalog.existing, TrinoCatalogName(name)) + } + outcomes, err := h.provisioner.managedCatalogs(context.Background(), configstore.TrinoCellLease{}, orgs, tenantSecretProjection{data: data, projected: projected}) + if err != nil || len(outcomes) != 10000 || s.claims != 0 || len(h.catalog.created) != 0 { + t.Fatalf("existing catalogs caused DDL or failed: %v", err) + } +} + +func TestManagedHeldOwnerStillRefreshesPasswordProjection(t *testing.T) { + h, s, _ := managedHarness(t) + s.blocked = true + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + h.store.orgs[0].RootPasswordHash = "replacement-hash" + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + secret, err := h.kube.CoreV1().Secrets(h.provisioner.namespace).Get(context.Background(), TrinoAuthSecretName, metav1.GetOptions{}) + if err != nil || !strings.Contains(string(secret.Data[TrinoAuthSecretKeyPasswordDB]), "replacement-hash") { + t.Fatal("held catalog owner blocked password reset projection") + } + if s.claims != 0 || len(h.catalog.created) != 0 || len(h.store.states) != 0 { + t.Fatal("projection-only replica acquired catalog/admission authority") + } +} + +func TestManagedReadySurvivesBackendFailureButNotSharedInputFailure(t *testing.T) { + h, _, _ := managedHarness(t) + h.store.orgs[0].State = configstore.ManagedWarehouseStateReady + h.catalog.nodesErr = errors.New("coordinator unavailable") + if err := h.provisioner.Reconcile(context.Background()); err == nil { + t.Fatal("backend failure was hidden") + } + if h.store.states["tenant-a"].State != configstore.ManagedWarehouseStateReady { + t.Fatal("backend failure revoked durable admission") + } + h.catalog.nodesErr = nil + h.passwordErr["tenant-a"] = errors.New("credential projection failed") + _ = h.provisioner.Reconcile(context.Background()) + if h.store.states["tenant-a"].State != configstore.ManagedWarehouseStateFailed { + t.Fatal("shared input failure was hidden as backend health") + } +} + +func TestManagedFrozenCertificationRejectsUnusableTarget(t *testing.T) { + for _, mode := range []string{"failed_catalog", "missing_credential", "process_changed"} { + t.Run(mode, func(t *testing.T) { + h, s, _ := managedHarness(t) + s.freeze = &configstore.TrinoCellFreeze{OperationID: "operation", TargetBackend: "group-green", Stable: true, AdmissionEpoch: 1} + s.admitted = []configstore.TrinoEnabledOrg{{OrgID: "tenant-a", DatabaseName: "tenant-a", State: configstore.ManagedWarehouseStateReady}} + target := &managedInventoryFixture{fakeCatalogClient: &fakeCatalogClient{}, states: map[string]string{"org_tenant_a": "OPERATIONAL"}} + h.provisioner.managed.Target = func(context.Context, *configstore.TrinoCellFreeze) (*TrinoManagedBackend, error) { + return &TrinoManagedBackend{Name: "group-green", Catalog: target}, nil + } + if mode == "failed_catalog" { + target.states["org_tenant_a"] = "FAILING" + } + if mode == "missing_credential" { + delete(h.ducklings, "tenant-a") + } + calls := 0 + if mode == "process_changed" { + h.provisioner.managed.TargetProcess = func(context.Context, string) (string, string, error) { + calls++ + return fmt.Sprintf("node-%d", calls), "abcde", nil + } + } + if err := h.provisioner.Reconcile(context.Background()); err == nil || s.certificate != nil || s.claims != 0 { + t.Fatal("unusable target was certified or mutated") + } + }) + } +} + +func TestManagedFrozenNewWarehouseRemainsUnadmitted(t *testing.T) { + h, s, _ := managedHarness(t) + s.freeze = &configstore.TrinoCellFreeze{OperationID: "operation", TargetBackend: "group-green", Stable: true, AdmissionEpoch: 1} + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if len(h.store.states) != 0 || s.claims != 0 || len(h.catalog.created) != 0 { + t.Fatal("frozen pending warehouse gained admission or catalog") + } +} + +type managedCredentialFixture struct { + *fakeCatalogClient + username, password string +} + +func (c *managedCredentialFixture) SetCredentials(username, password string) { + c.username, c.password = username, password +} + +func TestManagedStoppedClientReceivesCredentials(t *testing.T) { + h, _, _ := managedHarness(t) + green := &managedCredentialFixture{fakeCatalogClient: &fakeCatalogClient{}} + h.provisioner.managed.CatalogClients[1] = green + if err := h.provisioner.Reconcile(context.Background()); err != nil { + t.Fatal(err) + } + if green.username == "" || green.password == "" || len(green.created) != 0 { + t.Fatal("stopped client missed credentials or received DDL") + } +} + +func TestManagedConfigurationRejectsUnfencedWriters(t *testing.T) { + h, _, _ := managedHarness(t) + opts := h.provisioner.managed + h.provisioner.explicitAssignmentOnly = false + if err := h.provisioner.ConfigureManagedCatalogs(opts); err == nil { + t.Fatal("unassigned claiming allowed in managed mode") + } + h.provisioner.explicitAssignmentOnly = true + h.provisioner.additionalCatalogs = []TrinoCatalogClient{h.catalog} + if err := h.provisioner.ConfigureManagedCatalogs(opts); err == nil { + t.Fatal("static extra writer allowed in managed mode") + } + h.provisioner.additionalCatalogs = nil + copy := *opts + copy.Store = nil + if err := h.provisioner.ConfigureManagedCatalogs(©); err == nil { + t.Fatal("managed mode silently omitted lifecycle store") + } +} + +func (s *managedIntentFixture) SetTrinoCellIntent(_ context.Context, _ configstore.TrinoCellLease, intent configstore.TrinoCatalogIntent) error { + if s.intent || intent.Sequence != s.sequence+1 { + return configstore.ErrTrinoCellConflict + } + s.intent, s.sequence = true, intent.Sequence + s.claims++ + return nil +} + +func (s *managedIntentFixture) ClearTrinoCellIntent(context.Context, configstore.TrinoCellLease, string) error { + s.intent = false + s.clears++ + return nil +} + +type managedDDLFixture struct { + TrinoCatalogClient + err error + calls int +} + +func (c *managedDDLFixture) CreateCatalog(context.Context, string, map[string]string) error { + c.calls++ + return c.err +} +func (c *managedDDLFixture) DropCatalog(context.Context, string) error { c.calls++; return c.err } + +func TestManagedDDLIntentPrecedesSubmissionAndHoldsUnknown(t *testing.T) { + store := &managedIntentFixture{} + upstream := &managedDDLFixture{err: errors.New("transport disconnected")} + client := &trinoManagedCatalogClient{TrinoCatalogClient: upstream, store: store, backend: "group-blue"} + if err := client.CreateCatalog(context.Background(), "org_example", nil); err == nil { + t.Fatal("unknown outcome reported success") + } + if !store.intent || store.claims != 1 || store.clears != 0 || upstream.calls != 1 { + t.Fatal("uncertain submission did not retain durable intent") + } + _ = client.DropCatalog(context.Background(), "org_example") + if upstream.calls != 1 { + t.Fatal("unknown intent allowed another remote write") + } +} + +func TestManagedDDLTerminalOutcomesClearIntent(t *testing.T) { + for _, outcome := range []error{nil, &trinoCatalogTerminalError{}} { + store := &managedIntentFixture{} + upstream := &managedDDLFixture{err: outcome} + client := &trinoManagedCatalogClient{TrinoCatalogClient: upstream, store: store, backend: "group-blue"} + _ = client.CreateCatalog(context.Background(), "org_example", nil) + _ = client.DropCatalog(context.Background(), "org_example") + if store.intent || store.claims != 2 || store.clears != 2 || upstream.calls != 2 { + t.Fatal("confirmed terminal outcome did not allow next fenced write") + } + } +} diff --git a/controlplane/provisioner/trino_provisioner.go b/controlplane/provisioner/trino_provisioner.go index 4b114b1c..3863278a 100644 --- a/controlplane/provisioner/trino_provisioner.go +++ b/controlplane/provisioner/trino_provisioner.go @@ -20,6 +20,7 @@ import ( "sync" "time" + "github.com/google/uuid" "github.com/posthog/duckgres/controlplane/configstore" "github.com/posthog/duckgres/controlplane/provisioner/opa" "golang.org/x/crypto/bcrypt" @@ -230,6 +231,7 @@ type TrinoCatalogClient interface { // needs. Each is required at construction time — partial wiring would // cause silent reconcile no-ops, which we'd rather surface at startup. type TrinoProvisionerOpts struct { + ManagedCatalogs *TrinoManagedCatalogOpts // Store is the cross-cutting Trino read/write surface. Store TrinoStore @@ -399,6 +401,7 @@ type TrinoDucklingResolver func(ctx context.Context, orgID string) (*DucklingSta // fires on first install; thereafter ensureClusterSecrets adopts the // existing K8s Secrets. type TrinoProvisioner struct { + managed *TrinoManagedCatalogOpts store TrinoStore bootstrapSentinel TrinoBootstrapSentinelStore warehouses TrinoWarehouseStore @@ -521,7 +524,8 @@ func NewTrinoProvisioner(opts TrinoProvisionerOpts) (*TrinoProvisioner, error) { if secretReadiness == nil { secretReadiness = NewKubernetesTrinoSecretReadiness(opts.Kubernetes, nil) } - return &TrinoProvisioner{ + result := &TrinoProvisioner{ + managed: opts.ManagedCatalogs, store: opts.Store, bootstrapSentinel: opts.BootstrapSentinel, warehouses: opts.Warehouses, @@ -542,7 +546,13 @@ func NewTrinoProvisioner(opts TrinoProvisionerOpts) (*TrinoProvisioner, error) { s3MaxConnections: maxConns, filesystemCacheEnabled: opts.FilesystemCacheEnabled, hoglakeURI: opts.HoglakeURI, - }, nil + } + if opts.ManagedCatalogs != nil { + if err := result.ConfigureManagedCatalogs(opts.ManagedCatalogs); err != nil { + return nil, err + } + } + return result, nil } // CellID reports the Trino cell this provisioner owns. Exposed for @@ -573,6 +583,28 @@ func (p *TrinoProvisioner) Reconcile(ctx context.Context) error { if _, err := p.ensureClusterSecrets(ctx); err != nil { return fmt.Errorf("ensure trino cluster secrets: %w", err) } + var managedLease *configstore.TrinoCellLease + managedFollower := false + if p.managed != nil && !p.managed.Paused { + var acquired bool + var err error + managedLease, acquired, err = p.managed.Store.BeginTrinoCellReconcile(ctx, p.cellID, uuid.NewString()) + if err != nil { + return err + } + if !acquired { + managedFollower = true + managedLease = nil + } else { + defer func() { + finishCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 3*time.Second) + defer cancel() + if err := p.managed.Store.FinishTrinoCellReconcile(finishCtx, *managedLease); err != nil { + slog.Error("Could not confirm managed Trino ownership release.", "cell", p.cellID) + } + }() + } + } allOrgs, err := p.store.ListTrinoEnabledOrgs() if err != nil { @@ -643,6 +675,9 @@ func (p *TrinoProvisioner) Reconcile(ctx context.Context) error { if tenantErr != nil { errs = append(errs, fmt.Errorf("reconcile tenant secrets: %w", tenantErr)) } + if p.managed != nil && (p.managed.Paused || managedFollower) { + return errors.Join(errs...) + } // 5. Catalogs (REST). Per-org idempotent CREATE; orgs disabled // since last tick get DROP. Runs last so all the prerequisite @@ -669,7 +704,17 @@ func (p *TrinoProvisioner) Reconcile(ctx context.Context) error { var catalogOutcomes map[string]catalogOutcome if globalErr == nil { var catErr error - catalogOutcomes, catErr = p.reconcileCatalogs(ctx, projectable, tenants) + if managedLease != nil { + catalogOutcomes, catErr = p.managedCatalogs(ctx, *managedLease, projectable, tenants) + if catErr != nil && catalogOutcomes == nil { + catalogOutcomes = make(map[string]catalogOutcome, len(projectable)) + for _, org := range projectable { + catalogOutcomes[org.OrgID] = catalogOutcome{Err: catErr} + } + } + } else { + catalogOutcomes, catErr = p.reconcileCatalogs(ctx, projectable, tenants) + } if catErr != nil { errs = append(errs, fmt.Errorf("reconcile catalogs: %w", catErr)) } @@ -694,7 +739,22 @@ func (p *TrinoProvisioner) Reconcile(ctx context.Context) error { errs = append(errs, fmt.Errorf("org %s: %w", orgID, err)) } } - p.writePerOrgStates(orgs, catalogOutcomes, globalErr) + if managedLease == nil { + p.writePerOrgStates(orgs, catalogOutcomes, globalErr) + } else { + previous := make(map[string]configstore.TrinoEnabledOrg, len(orgs)) + for _, org := range orgs { + previous[org.OrgID] = org + } + p.writePerOrgStates(orgs, catalogOutcomes, globalErr, func(org string, update configstore.TrinoStateUpdate) error { + if previous[org].State == configstore.ManagedWarehouseStateReady && globalErr == nil && collisions[org] == nil && tenants.failed[org] == nil && tenants.projected[org] { + update.State = configstore.ManagedWarehouseStateReady + update.FailedAt = nil + } + _, err := p.managed.Store.UpdateManagedTrinoState(ctx, *managedLease, org, update) + return err + }) + } if len(errs) > 0 { return errors.Join(errs...) @@ -939,6 +999,13 @@ func (p *TrinoProvisioner) ensureClusterSecrets(ctx context.Context) (bundleToke } } + if p.managed != nil { + for _, catalog := range p.managed.CatalogClients { + if updater, ok := catalog.(TrinoCatalogCredentialUpdater); ok { + updater.SetCredentials(opa.AdminPrincipal, adminPlaintext) + } + } + } return bundleToken, nil } @@ -1257,7 +1324,12 @@ func (p *TrinoProvisioner) writePerOrgStates( orgs []configstore.TrinoEnabledOrg, catalogOutcomes map[string]catalogOutcome, globalErr error, + stateWriters ...func(string, configstore.TrinoStateUpdate) error, ) { + writeState := p.store.UpdateTrinoState + if len(stateWriters) != 0 { + writeState = stateWriters[0] + } now := time.Now().UTC() zero := time.Time{} // pointer-to-zero signals "clear failed_at" to UpdateTrinoState for _, o := range orgs { @@ -1315,7 +1387,7 @@ func (p *TrinoProvisioner) writePerOrgStates( upd.FailedAt = &zero } - if err := p.store.UpdateTrinoState(o.OrgID, upd); err != nil { + if err := writeState(o.OrgID, upd); err != nil { slog.Warn("Trino reconcile: failed to write per-org state.", "org", o.OrgID, "error", err) } diff --git a/controlplane/trino_inputs.go b/controlplane/trino_inputs.go index 6039aaf4..56372a68 100644 --- a/controlplane/trino_inputs.go +++ b/controlplane/trino_inputs.go @@ -127,14 +127,15 @@ func trinoProvisionerEnabled() bool { // Registered cells share projections across their independently scheduled backends. // Only the legacy cell claims unassigned tenants. type trinoCell struct { - ID string - PublicID string - RoutingGroup string - Namespace string - Backends []trinoRegisteredBackend - CoordinatorURL string - TLSServerName string - ClientURL string + CatalogManagement string + ID string + PublicID string + RoutingGroup string + Namespace string + Backends []trinoRegisteredBackend + CoordinatorURL string + TLSServerName string + ClientURL string } // consoleCell preserves legacy ownership and exposes each logical identity. @@ -300,10 +301,14 @@ func buildTrinoCellWiring(store trinoWiringStore, kc kubernetes.Interface, duckl // envTrinoCoordinatorServerName). catalogClient := provisioner.NewTrinoCatalogHTTPClient(cell.CoordinatorURL, opa.AdminPrincipal, "", cell.TLSServerName) var additional []provisioner.TrinoCatalogClient + var managed *provisioner.TrinoManagedCatalogOpts + if cell.CatalogManagement != "" { + managed = &provisioner.TrinoManagedCatalogOpts{Paused: true} + } var internalSecrets []string for _, backend := range cell.Backends { internalSecrets = append(internalSecrets, backend.InternalSecretName) - if backend.Running && !backend.RoutingActive { + if cell.CatalogManagement == "" && backend.Running && !backend.RoutingActive { additional = append(additional, provisioner.NewTrinoCatalogHTTPClient(backend.CoordinatorURL, opa.AdminPrincipal, "", backend.TLSServerName)) } } @@ -311,6 +316,7 @@ func buildTrinoCellWiring(store trinoWiringStore, kc kubernetes.Interface, duckl bundleStore := &opa.BundleStore{} trinoProv, err := provisioner.NewTrinoProvisioner(provisioner.TrinoProvisionerOpts{ + ManagedCatalogs: managed, Store: store, BootstrapSentinel: store, Warehouses: store, diff --git a/controlplane/trino_managed_wiring.go b/controlplane/trino_managed_wiring.go new file mode 100644 index 00000000..59bddc8d --- /dev/null +++ b/controlplane/trino_managed_wiring.go @@ -0,0 +1,130 @@ +//go:build kubernetes + +package controlplane + +import ( + "context" + "errors" + "os" + "strings" + "time" + + "github.com/posthog/duckgres/controlplane/configstore" + "github.com/posthog/duckgres/controlplane/provisioner" + "github.com/posthog/duckgres/controlplane/provisioner/opa" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" +) + +func buildTrinoManagedFleet(fleet trinoFleet, store *configstore.ConfigStore, readiness *trinoRolloutReadinessHandler) (*trinoRolloutProvisioningHandler, error) { + managed := make(map[string]trinoRolloutProvisioningCell) + for _, wire := range fleet { + if wire.Cell.CatalogManagement == "gateway-shared" { + managed[wire.Cell.RoutingGroup] = trinoRolloutProvisioningCell{StoredCellID: wire.Cell.ID, BlueBackend: wire.Cell.RoutingGroup + "-blue", GreenBackend: wire.Cell.RoutingGroup + "-green"} + } + } + if len(managed) == 0 { + return nil, nil + } + if readiness == nil { + return nil, errors.New("managed catalog mode requires registered rollout readiness and canaries") + } + gateway, err := newTrinoManagedGateway(strings.TrimSpace(os.Getenv("DUCKGRES_TRINO_MANAGED_GATEWAY_URL")), strings.TrimSpace(os.Getenv("DUCKGRES_TRINO_MANAGED_GATEWAY_SERVER_NAME")), strings.TrimSpace(os.Getenv("DUCKGRES_TRINO_MANAGED_GATEWAY_USERNAME")), readiness.token) + if err != nil { + return nil, err + } + for _, wire := range fleet { + if wire.Cell.CatalogManagement != "gateway-shared" { + continue + } + clients := make(map[string]provisioner.TrinoCatalogClient) + var credentials []provisioner.TrinoCatalogClient + for _, backend := range wire.Cell.Backends { + client, err := provisioner.NewTrinoSharedCatalogHTTPClient(backend.CoordinatorURL, opa.AdminPrincipal, "", backend.TLSServerName) + if err != nil { + return nil, err + } + clients[wire.Cell.RoutingGroup+"-"+backend.ID] = client + credentials = append(credentials, client) + } + opts := managedCatalogOptions(store, gateway, wire.Cell.RoutingGroup, clients, readiness.process) + opts.CatalogClients = credentials + if err := wire.Provisioner.ConfigureManagedCatalogs(opts); err != nil { + return nil, err + } + } + return newTrinoRolloutProvisioningHandler(readiness.token, managed, store, gateway, readiness.process) +} + +func managedCatalogOptions(store provisioner.TrinoCellLifecycleStore, gateway trinoManagedGatewayReader, group string, clients map[string]provisioner.TrinoCatalogClient, process func(context.Context, string, string) (string, string, error)) *provisioner.TrinoManagedCatalogOpts { + return &provisioner.TrinoManagedCatalogOpts{ + Store: store, + Active: func(ctx context.Context) (*provisioner.TrinoManagedBackend, error) { + observation, err := gateway.Observe(ctx, group) + if err != nil { + return nil, err + } + client := clients[observation.Route.BackendName] + if client == nil { + return nil, errors.New("active Gateway route is outside the registered cell") + } + backend, err := gateway.Backend(ctx, observation.Route.BackendName) + if err != nil || backend == nil || backend.State != "ACTIVE" || backend.Incarnation != observation.Route.BackendIncarnation { + return nil, errors.New("active Gateway backend is not available for provisioning") + } + return &provisioner.TrinoManagedBackend{Name: observation.Route.BackendName, Catalog: client}, nil + }, + Target: func(ctx context.Context, freeze *configstore.TrinoCellFreeze) (*provisioner.TrinoManagedBackend, error) { + observation, err := gateway.Observe(ctx, group) + if err != nil { + return nil, err + } + op := observation.Rollout + if op == nil || op.OperationID != freeze.OperationID || op.Plan.PlanHash != freeze.PlanHash || op.Plan.TargetBackend != freeze.TargetBackend { + return nil, errors.New("frozen cell differs from Gateway rollout") + } + if op.Phase == "CLAIMED" { + return nil, nil + } + if op.Phase != "WARMED" && op.Phase != "VERIFIED" { + return nil, errors.New("Gateway rollout cannot prepare a target in this phase") + } + if observation.Route.Generation != op.Plan.ExpectedRouteGeneration || observation.Route.BackendName != op.Plan.SourceBackend || observation.Route.BackendIncarnation != op.Plan.SourceIncarnation || clients[freeze.TargetBackend] == nil { + return nil, errors.New("Gateway source changed before target certification") + } + return &provisioner.TrinoManagedBackend{Name: freeze.TargetBackend, Catalog: clients[freeze.TargetBackend]}, nil + }, + TargetProcess: func(ctx context.Context, backend string) (string, string, error) { return process(ctx, group, backend) }, + } +} + +// process binds live coordinator identity to the registered pod inventory and canary. +func (h *trinoRolloutReadinessHandler) process(ctx context.Context, group, backend string) (string, string, error) { + color, ok := strings.CutPrefix(backend, group+"-") + slot, exists := h.slots[group+"/"+color] + if !ok || !exists || slot.backendName != backend { + return "", "", errors.New("unknown registered rollout target") + } + select { + case h.limit <- struct{}{}: + defer func() { <-h.limit }() + default: + return "", "", errors.New("rollout readiness busy") + } + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + selector := labels.Set{"posthog.com/trino-cell": slot.cell, "posthog.com/trino-color": slot.color}.String() + pods, err := h.kube.CoreV1().Pods(slot.namespace).List(ctx, metav1.ListOptions{LabelSelector: selector, Limit: 1001}) + if err != nil || pods == nil || pods.Continue != "" || len(pods.Items) > 1000 { + return "", "", errors.New("rollout pod inventory unavailable") + } + inventory, err := rolloutPodInventory(pods.Items) + if err != nil || inventory.Terminating != 0 || inventory.Coordinators != 1 || inventory.ReadyCoordinators != 1 || inventory.Workers == 0 || inventory.ReadyWorkers != inventory.Workers { + return "", "", errors.New("rollout target pods not ready") + } + facts, err := h.probe(ctx, slot) + if err != nil || facts == nil || facts.RegisteredWorkers != inventory.Workers || !rolloutMembersMatchPods(facts.members, pods.Items) || ctx.Err() != nil { + return "", "", errors.New("rollout target process not ready") + } + return facts.NodeID, facts.CoordinatorID, nil +} diff --git a/controlplane/trino_managed_wiring_test.go b/controlplane/trino_managed_wiring_test.go new file mode 100644 index 00000000..c50a568b --- /dev/null +++ b/controlplane/trino_managed_wiring_test.go @@ -0,0 +1,141 @@ +//go:build kubernetes + +package controlplane + +import ( + "context" + "io" + "net/http" + "strings" + "testing" + + "github.com/posthog/duckgres/controlplane/configstore" + "github.com/posthog/duckgres/controlplane/provisioner" +) + +type managedProbeTransport func(*http.Request) (*http.Response, error) + +func (f managedProbeTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestManagedReadinessDefaultHTTPSPort(t *testing.T) { + calls := 0 + client := rolloutSQLClient{baseURL: "https://coordinator.example:443", username: "observer", password: "fixture-password", client: &http.Client{Transport: managedProbeTransport(func(*http.Request) (*http.Response, error) { + calls++ + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader("{}")), Header: make(http.Header)}, nil + })}} + for _, tc := range []struct { + endpoint string + valid bool + }{{"https://coordinator.example/v1/info", true}, {"https://coordinator.example:443/v1/info", true}, {"https://coordinator.example:8443/v1/info", false}, {"https://other.example/v1/info", false}} { + _, err := client.read(context.Background(), http.MethodGet, tc.endpoint, "") + if (err == nil) != tc.valid { + t.Fatalf("invalid normalized origin result for %s: %v", tc.endpoint, err) + } + } + if calls != 2 { + t.Fatal("foreign origin reached transport") + } +} + +func TestManagedRegistryModesAreExplicitAndClosed(t *testing.T) { + for _, mode := range []string{"paused", "gateway-shared", "unknown"} { + input := strings.Replace(testTrinoRegistryJSON, `"routing_group":"cell-test"`, `"routing_group":"group-test","catalog_management":"`+mode+`"`, 1) + cells, err := parseTrinoCellRegistry([]byte(input)) + if mode == "unknown" { + if err == nil { + t.Fatal("unknown mode accepted") + } + continue + } + if err != nil || cells[0].CatalogManagement != mode || cells[0].RoutingGroup != "group-test" { + t.Fatalf("explicit mode lost logical/group identity: %v", err) + } + if _, err := parseTrinoCellRegistry([]byte(strings.Replace(input, `"id":"green"`, `"id":"other"`, 1))); err == nil { + t.Fatal("managed mode accepted non-paired slots") + } + } +} + +func TestManagedWiringPausedNeedsNoGatewayOrCanaries(t *testing.T) { + for _, key := range []string{"DUCKGRES_TRINO_MANAGED_GATEWAY_URL", "DUCKGRES_TRINO_MANAGED_GATEWAY_USERNAME", "DUCKGRES_TRINO_ROLLOUT_TOKEN_FILE", "DUCKGRES_TRINO_ROLLOUT_CANARIES_FILE"} { + t.Setenv(key, "") + } + fleet := trinoFleet{&trinoWiring{Cell: trinoCell{ID: "registered:cell-test", PublicID: "cell-test", RoutingGroup: "group-test", CatalogManagement: "paused"}}} + if handler, err := buildTrinoManagedFleet(fleet, nil, nil); err != nil || handler != nil { + t.Fatal("paused bridge required future credentials") + } + fleet[0].Cell.CatalogManagement = "gateway-shared" + if _, err := buildTrinoManagedFleet(fleet, nil, nil); err == nil { + t.Fatal("managed mode silently omitted readiness") + } + if _, err := buildTrinoManagedFleet(fleet, nil, &trinoRolloutReadinessHandler{token: strings.Repeat("t", 48)}); err == nil { + t.Fatal("managed mode silently omitted Gateway credentials") + } +} + +func TestManagedOptionsRejectUnregisteredActiveRoute(t *testing.T) { + reader := &managedReaderFake{observation: trinoManagedGatewayObservation{Route: trinoManagedGatewayRoute{RoutingGroup: "group-test", BackendName: "other-blue"}}} + opts := managedCatalogOptions(nil, reader, "group-test", map[string]provisioner.TrinoCatalogClient{}, nil) + if _, err := opts.Active(context.Background()); err == nil { + t.Fatal("foreign active backend selected") + } +} + +type managedSelectorCatalog struct{ provisioner.TrinoCatalogClient } + +func TestManagedOptionsUseCurrentRouteAndMatchingWarmPlan(t *testing.T) { + blue, green := &managedSelectorCatalog{}, &managedSelectorCatalog{} + reader := &managedReaderFake{observation: trinoManagedGatewayObservation{Route: trinoManagedGatewayRoute{RoutingGroup: "group-test", Generation: 7, BackendName: "group-test-blue", BackendIncarnation: "blue-incarnation"}}, backend: trinoManagedGatewayBackend{BackendName: "group-test-blue", State: "ACTIVE", Incarnation: "blue-incarnation"}} + opts := managedCatalogOptions(nil, reader, "group-test", map[string]provisioner.TrinoCatalogClient{"group-test-blue": blue, "group-test-green": green}, nil) + active, err := opts.Active(context.Background()) + if err != nil || active.Catalog != blue { + t.Fatal("valid blue route was rejected") + } + reader.observation.Route.BackendName = "group-test-green" + reader.observation.Route.BackendIncarnation = "green-incarnation" + reader.backend = trinoManagedGatewayBackend{BackendName: "group-test-green", State: "ACTIVE", Incarnation: "green-incarnation"} + active, err = opts.Active(context.Background()) + if err != nil || active.Catalog != green { + t.Fatal("post-cutover route did not select green") + } + reader.backend.Incarnation = "old-green" + if _, err := opts.Active(context.Background()); err == nil { + t.Fatal("stale backend incarnation accepted") + } + reader.observation.Route = trinoManagedGatewayRoute{RoutingGroup: "group-test", Generation: 7, BackendName: "group-test-blue", BackendIncarnation: "blue-incarnation"} + reader.observation.Rollout = &trinoManagedGatewayRollout{OperationID: "operation", Plan: trinoManagedGatewayPlan{PlanHash: strings.Repeat("a", 64), ExpectedRouteGeneration: 7, SourceBackend: "group-test-blue", SourceIncarnation: "blue-incarnation", TargetBackend: "group-test-green"}} + freeze := &configstore.TrinoCellFreeze{OperationID: "operation", PlanHash: strings.Repeat("a", 64), TargetBackend: "group-test-green"} + for _, phase := range []string{"CLAIMED", "WARMED", "VERIFIED", "CUTOVER"} { + reader.observation.Rollout.Phase = phase + target, err := opts.Target(context.Background(), freeze) + switch phase { + case "CLAIMED": + if err != nil || target != nil { + t.Fatal("claimed operation probed unstarted target") + } + case "CUTOVER": + if err == nil { + t.Fatal("late uncertified target accepted after cutover") + } + default: + if err != nil || target == nil || target.Catalog != green { + t.Fatal("matching warm target rejected") + } + } + } + reader.observation.Rollout.Phase = "WARMED" + reader.observation.Route.Generation++ + if _, err := opts.Target(context.Background(), freeze); err == nil { + t.Fatal("changed source route certified") + } +} + +func TestManagedRolloutFilesWithoutFleetFailStartupValidation(t *testing.T) { + for _, tc := range []struct{ token, canary string }{{"requested", ""}, {"", "requested"}, {"requested", "requested"}} { + t.Setenv("DUCKGRES_TRINO_ROLLOUT_TOKEN_FILE", tc.token) + t.Setenv("DUCKGRES_TRINO_ROLLOUT_CANARIES_FILE", tc.canary) + if _, err := buildTrinoRolloutReadiness(nil, nil); err == nil { + t.Fatal("requested rollout configuration was ignored without a fleet") + } + } +} diff --git a/controlplane/trino_registry.go b/controlplane/trino_registry.go index e38a5346..dde916e7 100644 --- a/controlplane/trino_registry.go +++ b/controlplane/trino_registry.go @@ -80,7 +80,7 @@ func resolveTrinoCells() ([]trinoCell, error) { if !registryOnly && entry.Namespace == legacyNS { return nil, errors.New("registered cell must not share the legacy namespace") } - cell := trinoCell{ID: registeredTrinoCellPrefix + entry.ID, PublicID: entry.ID, RoutingGroup: entry.RoutingGroup, Namespace: entry.Namespace, ClientURL: entry.ClientURL, Backends: entry.Backends} + cell := trinoCell{ID: registeredTrinoCellPrefix + entry.ID, PublicID: entry.ID, RoutingGroup: entry.RoutingGroup, Namespace: entry.Namespace, ClientURL: entry.ClientURL, Backends: entry.Backends, CatalogManagement: entry.CatalogManagement} for _, backend := range entry.Backends { endpoint, _ := trinoEndpointKey(backend.CoordinatorURL) if !registryOnly && endpoint == legacyEndpoint { @@ -96,11 +96,12 @@ func resolveTrinoCells() ([]trinoCell, error) { } 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"` + CatalogManagement string `json:"catalog_management,omitempty"` + 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 { @@ -132,6 +133,12 @@ func parseTrinoCellRegistry(data []byte) ([]trinoRegisteredCell, error) { } identities, namespaces, groups, endpoints := map[string]bool{}, map[string]bool{}, map[string]bool{}, map[string]bool{} for _, cell := range registry.Cells { + if cell.CatalogManagement != "" && cell.CatalogManagement != "paused" && cell.CatalogManagement != "gateway-shared" { + return nil, errors.New("unsupported Trino catalog management mode") + } + if cell.CatalogManagement != "" && (len(cell.Backends) != 2 || cell.Backends[0].ID == cell.Backends[1].ID || (cell.Backends[0].ID != "blue" && cell.Backends[0].ID != "green") || (cell.Backends[1].ID != "blue" && cell.Backends[1].ID != "green")) { + return nil, errors.New("managed Trino cell requires exactly blue and green slots") + } 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") } diff --git a/controlplane/trino_rollout_probe.go b/controlplane/trino_rollout_probe.go index 2cc88a1e..99407808 100644 --- a/controlplane/trino_rollout_probe.go +++ b/controlplane/trino_rollout_probe.go @@ -121,7 +121,7 @@ type rolloutSQLClient struct { func (c rolloutSQLClient) read(ctx context.Context, method, endpoint, sql string) ([]byte, error) { base, baseErr := url.Parse(c.baseURL) parsed, err := url.Parse(endpoint) - if baseErr != nil || err != nil || parsed.Scheme != "https" || parsed.Scheme != base.Scheme || parsed.Host != base.Host || parsed.User != nil || parsed.Fragment != "" || (parsed.Path != "/v1/statement" && !strings.HasPrefix(parsed.Path, "/v1/statement/") && parsed.Path != "/v1/info") { + if baseErr != nil || err != nil || parsed.Scheme != "https" || parsed.Scheme != base.Scheme || !strings.EqualFold(parsed.Hostname(), base.Hostname()) || rolloutHTTPSPort(parsed) != rolloutHTTPSPort(base) || parsed.User != nil || parsed.RawQuery != "" || parsed.ForceQuery || parsed.Fragment != "" || (parsed.Path != "/v1/statement" && !strings.HasPrefix(parsed.Path, "/v1/statement/") && parsed.Path != "/v1/info") { return nil, errors.New("invalid coordinator response endpoint") } if c.username == "" || c.password == "" { @@ -150,6 +150,13 @@ func (c rolloutSQLClient) read(ctx context.Context, method, endpoint, sql string return body, nil } +func rolloutHTTPSPort(endpoint *url.URL) string { + if port := endpoint.Port(); port != "" { + return port + } + return "443" +} + func (c rolloutSQLClient) info(ctx context.Context) (*rolloutCoordinatorFacts, error) { body, err := c.read(ctx, http.MethodGet, c.baseURL+"/v1/info", "") if err != nil { diff --git a/docs/runbooks/trino-shared-catalogs.md b/docs/runbooks/trino-shared-catalogs.md new file mode 100644 index 00000000..8c19fd58 --- /dev/null +++ b/docs/runbooks/trino-shared-catalogs.md @@ -0,0 +1,107 @@ +# Shared catalog cells + +This mode requires the reviewed lifecycle migration, strict catalog client, +Gateway rollout APIs, and rollout-readiness support. It is disabled by default. +Legacy cells retain their existing behavior. + +## Configuration and activation + +Each registered cell accepts `catalog_management`: + +- Omitted: existing static backend reconciliation. +- `paused`: refresh authentication, resource groups, tenant credentials and + each replica's OPA bundle, but submit no catalog DDL or provisioning state updates. +- `gateway-shared`: use the Gateway's authoritative active backend for catalog + mutations. Keep both slot clients credential-ready, regardless of static + `running` flags. Never reconcile catalog DDL against the inactive slot. + +Managed cells must contain exactly `blue` and `green`. They require +`DUCKGRES_TRINO_MANAGED_GATEWAY_URL`, `DUCKGRES_TRINO_MANAGED_GATEWAY_USERNAME`, +and optionally `DUCKGRES_TRINO_MANAGED_GATEWAY_SERVER_NAME`. The capability in +`DUCKGRES_TRINO_ROLLOUT_TOKEN_FILE` supplies both the API-only Basic password +and the transaction-admin header. These are one capability, not two factors. +`DUCKGRES_TRINO_ROLLOUT_CANARIES_FILE` uses the dedicated per-cell canary format +in [rollout readiness](trino-rollout-readiness.md). + +First deploy the compatible image with registered cells `paused`. Let every +older unfenced controller stop, including terminating replicas; verify that +no old catalog mutation remains in flight. Do not kill unrelated client +connections or use a whole-control-plane `Recreate` deployment. The existing +SIGTERM path stops the provisioner independently of client connection drain. +Create dedicated canaries and private credentials while paused, then enable +`gateway-shared`. A pending canary becomes Ready through normal active-backend +provisioning before the first cell rollout. Never reuse customer passwords. + +## Rollout protocol + +1. Acquire the Gateway's durable cell rollout operation. +2. Read the current admission epoch and freeze provisioning **before** starting + the target. Wait until the earlier catalog owner has safely finished. +3. Start the target with the same logical-cell persistent catalog store. + Startup loads the existing definitions; no per-tenant CREATE replay runs. +4. The control plane bulk-checks the target catalogs and verifies mounted + credentials for every enabled previously admitted warehouse. Historical + admissions remain in this check after temporary state or credential errors. + It binds the certificate to the exact live target process and canary. +5. Cut over the Gateway route, then release the provisioning freeze. +6. Provision new catalogs only on the new active backend while the old source drains. + +New warehouse enablement can wait through target startup and checks. Existing +queries continue. Backend health alone does not revoke an existing Ready +admission; genuine shared-input errors still surface. Authentication refresh +retains its existing eventual projection semantics during freezes and holds. + +## Internal API + +All endpoints require the handler-specific transaction-admin capability. +They accept a registered routing group, not a caller-supplied backend URL. + +- `GET /internal/trino/rollout-provisioning/{routingGroup}` returns + `operationId`, `admissionEpoch`, `frozen`, `stable`, `prepared`, + `targetBackend`, `nodeId`, `coordinatorId`, `rosterHash`, and `admittedCount`. +- `POST .../freeze` accepts `operationId`, `planHash`, and + `expectedAdmissionEpoch`. A new freeze increments the epoch exactly once. + `202` means the prior owner has not completed; `200` acknowledges stability. +- `POST .../release` accepts `operationId`, `planHash`, and `admissionEpoch`. + It verifies the Gateway cutover and current target process, then increments + the epoch again. Identical release retries return the durable receipt. + +Never reread a new epoch to force a stale operation through a conflict. +GET is read-only; it does not provision or alter assignments. + +## Failure handling + +Every catalog mutation has a durable, one-shot intent before submission. +Only verified `FINISHED` success or rejection before submission permits +clearing it. A remote `FAILED` query can still have a synchronous catalog +mutation running; cancellation is not a completion fence. Transport failures, +ambiguous responses, controller crashes, and remote query failures retain +ownership. There is no timeout takeover or automatic force-unlock endpoint. + +Before explicit recovery, fence the original controller and any potentially +running coordinator mutation. Inspect the persisted catalog definitions and +current process identities. Reconcile the intended outcome before releasing +ownership through a separately reviewed operator procedure. Do not delete +lifecycle rows, bootstrap sentinels, or regenerate credentials as a shortcut. +An immutable certificate whose target process changed requires recovery; +the control plane must not silently overwrite it. + +## Scale and verification + +Catalog inventory is one bounded bulk SQL query, not one query per warehouse. +Unchanged catalogs require no CREATE or warehouse-property lookup. Mounted +credential checks still scale with warehouse count and live members; they use +bounded batches and deadlines. The 10,000-catalog unit regression proves no +DDL replay, not a production throughput benchmark. Existing secret projection +and metadata resolution costs remain; this change does not eliminate them. + +Only catalog/provisioning selection follows the Gateway in this change. +Existing admin live-query observers and usage collectors still use static +registry selections. Updating those observers after a cell cutover is a +separate follow-up; do not interpret their stale backend health as lost +catalog admission. + +Run `just test-trino Managed`, `just test-trino SharedCatalog`, +`just test-trino-admin`, `just test-controlplane-k8s`, and `just lint`. +The retained isolated end-to-end lane must exercise the actual Gateway and +Trino images before activation. Configuration PRs are not deployment approval. diff --git a/docs/trino-cells.md b/docs/trino-cells.md index b5da376a..93a4d919 100644 --- a/docs/trino-cells.md +++ b/docs/trino-cells.md @@ -129,6 +129,12 @@ budgets are not a hard deadline for stalled database calls. ## Local verification and recovery +For opt-in shared-store blue/green cells, use the +[shared catalog runbook](runbooks/trino-shared-catalogs.md). This mode freezes +new provisioning before target startup and uses the Gateway's active route. +It does not replay catalog CREATE statements on the standby. The static +registry behavior documented above remains the default. + 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 From a59edf67396f9df0fc0a8df5354f69df3f977d9b Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Mon, 14 Sep 2026 17:14:50 +0200 Subject: [PATCH 6/7] test(trino): cover durable admitted roster membership --- .../configstore/trino_cell_admissions_test.go | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 tests/configstore/trino_cell_admissions_test.go diff --git a/tests/configstore/trino_cell_admissions_test.go b/tests/configstore/trino_cell_admissions_test.go new file mode 100644 index 00000000..5941e0ec --- /dev/null +++ b/tests/configstore/trino_cell_admissions_test.go @@ -0,0 +1,128 @@ +//go:build linux || darwin + +package configstore_test + +import ( + "context" + "reflect" + "testing" + "time" + + "github.com/posthog/duckgres/controlplane/configstore" +) + +func seedTrinoAdmission(t *testing.T, store *configstore.ConfigStore, org, cell string, state configstore.ManagedWarehouseProvisioningState, previouslyReady bool) { + t.Helper() + seedTrinoOrg(t, store, org) + if err := store.DB().Create(&configstore.ManagedWarehouse{OrgID: org, DucklingName: org, State: configstore.ManagedWarehouseStateReady}).Error; err != nil { + t.Fatal(err) + } + if err := store.SelectTrinoCell(org, cell); err != nil { + t.Fatal(err) + } + if err := store.EnableTrino(org, configstore.TrinoSettings{}); err != nil { + t.Fatal(err) + } + update := configstore.TrinoStateUpdate{State: state} + if previouslyReady { + now := time.Now().UTC() + update.ReadyAt = &now + } + if err := store.UpdateTrinoState(org, update); err != nil { + t.Fatal(err) + } +} + +func TestTrinoAdmittedRosterHistoryAndMembershipPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + const cell = "registered:cell-test" + seedTrinoAdmission(t, store, "historical", cell, configstore.ManagedWarehouseStateFailed, true) + seedTrinoAdmission(t, store, "current", cell, configstore.ManagedWarehouseStateReady, false) + seedTrinoAdmission(t, store, "pending", cell, configstore.ManagedWarehouseStatePending, false) + seedTrinoAdmission(t, store, "never-ready-failed", cell, configstore.ManagedWarehouseStateFailed, false) + seedTrinoAdmission(t, store, "disabled", cell, configstore.ManagedWarehouseStateReady, true) + seedTrinoAdmission(t, store, "other", "registered:other-cell", configstore.ManagedWarehouseStateReady, true) + if err := store.DisableTrino("disabled"); err != nil { + t.Fatal(err) + } + if err := store.DB().Exec("DELETE FROM duckgres_org_users WHERE org_id = ?", "historical").Error; err != nil { + t.Fatal(err) + } + rows, err := store.ListAdmittedTrinoOrgs(context.Background(), cell) + if err != nil { + t.Fatal(err) + } + var names []string + for _, row := range rows { + names = append(names, row.OrgID) + if row.CellID != cell || row.DatabaseName != row.OrgID+"db" { + t.Fatalf("roster lost assignment or principal: %+v", row) + } + } + if !reflect.DeepEqual(names, []string{"current", "historical"}) || rows[0].State != configstore.ManagedWarehouseStateReady || rows[1].State != configstore.ManagedWarehouseStateFailed { + t.Fatalf("admission history or membership changed: %+v", rows) + } + other, err := store.ListAdmittedTrinoOrgs(context.Background(), "registered:other-cell") + if err != nil || len(other) != 1 || other[0].OrgID != "other" { + t.Fatalf("roster crossed logical cells: %+v %v", other, err) + } +} + +func TestTrinoAdmittedRosterMissingPrincipalPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + const cell = "registered:cell-test" + seedTrinoAdmission(t, store, "admitted", cell, configstore.ManagedWarehouseStateReady, true) + if err := store.DB().Exec("UPDATE duckgres_orgs SET database_name = '' WHERE name = ?", "admitted").Error; err != nil { + t.Fatal(err) + } + rows, err := store.ListAdmittedTrinoOrgs(context.Background(), cell) + if err != nil || len(rows) != 1 || rows[0].OrgID != "admitted" || rows[0].DatabaseName != "" { + t.Fatalf("invalid principal disappeared instead of blocking certification: %+v %v", rows, err) + } +} + +func TestTrinoAdmittedRosterOrphanIsRetainedPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + const cell = "registered:cell-test" + seedTrinoAdmission(t, store, "orphan", cell, configstore.ManagedWarehouseStateReady, true) + // Normal deletion cascades. Remove only this isolated schema's constraint to model an inconsistent restore. + if err := store.DB().Exec("ALTER TABLE duckgres_managed_warehouse_trino DROP CONSTRAINT fk_duckgres_managed_warehouse_trino_org").Error; err != nil { + t.Fatal(err) + } + if err := store.DB().Exec("DELETE FROM duckgres_org_users WHERE org_id = ?", "orphan").Error; err != nil { + t.Fatal(err) + } + if err := store.DB().Exec("DELETE FROM duckgres_orgs WHERE name = ?", "orphan").Error; err != nil { + t.Fatal(err) + } + rows, err := store.ListAdmittedTrinoOrgs(context.Background(), cell) + if err != nil || len(rows) != 1 || rows[0].OrgID != "orphan" || rows[0].DatabaseName != "" { + t.Fatalf("orphaned admission disappeared instead of blocking certification: %+v %v", rows, err) + } +} + +func TestTrinoAdmittedRosterReenablePreservesHistoryNotReadyPostgres(t *testing.T) { + store := newIsolatedConfigStore(t) + const cell = "registered:cell-test" + seedTrinoAdmission(t, store, "returning", cell, configstore.ManagedWarehouseStateReady, true) + if err := store.DisableTrino("returning"); err != nil { + t.Fatal(err) + } + if err := store.EnableTrino("returning", configstore.TrinoSettings{}); err != nil { + t.Fatal(err) + } + rows, err := store.ListAdmittedTrinoOrgs(context.Background(), cell) + if err != nil || len(rows) != 1 || rows[0].OrgID != "returning" || rows[0].State != configstore.ManagedWarehouseStatePending { + t.Fatalf("re-enable lost admission history or granted Ready: %+v %v", rows, err) + } + if err := store.DB().Exec("DELETE FROM duckgres_org_users WHERE org_id = ?", "returning").Error; err != nil { + t.Fatal(err) + } + if err := store.DB().Exec("DELETE FROM duckgres_orgs WHERE name = ?", "returning").Error; err != nil { + t.Fatal(err) + } + rows, err = store.ListAdmittedTrinoOrgs(context.Background(), cell) + if err != nil || len(rows) != 0 { + t.Fatalf("normal org deletion did not cascade its admission: %+v %v", rows, err) + } +} From a43b34dd44fa1ae9ebda402e9c7d53b7fc055d35 Mon Sep 17 00:00:00 2001 From: Benjamin Knofe-Vider Date: Mon, 14 Sep 2026 17:33:06 +0200 Subject: [PATCH 7/7] fix(trino): acknowledge finished result pages --- .../trino_shared_catalog_client.go | 2 +- .../trino_shared_catalog_client_test.go | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/controlplane/provisioner/trino_shared_catalog_client.go b/controlplane/provisioner/trino_shared_catalog_client.go index 799212ed..6e2159b6 100644 --- a/controlplane/provisioner/trino_shared_catalog_client.go +++ b/controlplane/provisioner/trino_shared_catalog_client.go @@ -131,7 +131,7 @@ func (c *trinoSharedCatalogHTTPClient) runStatement(ctx context.Context, stateme if state != "FINISHED" || page.Error != nil { return nil, errors.New("catalog statement terminal outcome unknown") } - } else if page.Error != nil || state == "FINISHED" || state == "FAILED" || !c.validContinuation(page.NextURI, queryID) { + } else if page.Error != nil || state == "FAILED" || !c.validContinuation(page.NextURI, queryID) { return nil, errors.New("catalog statement continuation invalid") } if len(rows)+len(page.Data) > sharedTrinoMaxRows { diff --git a/controlplane/provisioner/trino_shared_catalog_client_test.go b/controlplane/provisioner/trino_shared_catalog_client_test.go index c6b156e6..ff034b0a 100644 --- a/controlplane/provisioner/trino_shared_catalog_client_test.go +++ b/controlplane/provisioner/trino_shared_catalog_client_test.go @@ -108,6 +108,39 @@ func TestSharedCatalogContinuationOriginAndIdentity(t *testing.T) { } } +func TestSharedCatalogFinishedPageStillRequiresAcknowledgement(t *testing.T) { + for _, acknowledge := range []bool{true, false} { + t.Run(fmt.Sprintf("acknowledge_%t", acknowledge), func(t *testing.T) { + var server *httptest.Server + var requests atomic.Int32 + server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + if r.Method == http.MethodPost { + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"},"data":[["example","OPERATIONAL"]],"nextUri":%q}`, sharedTestQueryID, server.URL+"/v1/statement/executing/"+sharedTestQueryID+"/token/1") + return + } + if !acknowledge { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + _, _ = fmt.Fprintf(w, `{"id":%q,"stats":{"state":"FINISHED"}}`, sharedTestQueryID) + })) + defer server.Close() + states, err := sharedTestClient(t, server).CatalogStates(context.Background()) + if requests.Load() != 2 { + t.Fatalf("expected final acknowledgement request, got %d requests", requests.Load()) + } + if acknowledge { + if err != nil || states["example"] != "OPERATIONAL" { + t.Fatalf("finished result page was not retained: states=%v err=%v", states, err) + } + } else if err == nil || TrinoCatalogOutcomeTerminal(err) { + t.Fatal("failed acknowledgement was classified as successful or terminal") + } + }) + } +} + func TestSharedCatalogProductionTransportTLSAndPaging(t *testing.T) { var server *httptest.Server server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {