From 764f08cf0aa89953a9bca0fd698060e126e1911c Mon Sep 17 00:00:00 2001 From: Bill Guowei Yang Date: Thu, 4 Jun 2026 14:30:17 -0400 Subject: [PATCH] Fix Trino catalog creation: fs.s3.enabled -> fs.native-s3.enabled buildCatalogProperties emitted `fs.s3.enabled=true`, which Trino 476 rejects at CREATE CATALOG ("Configuration property 'fs.s3.enabled' was not used"), cascading s3.region + s3.iam-role into "unused" and breaking every per-org Iceberg catalog. Trino removed the legacy Hadoop S3 filesystem; since Trino 458 the native S3 filesystem must be enabled per-catalog with `fs.native-s3.enabled=true`. The prop set had drifted from the proven-working maintenance script (charts/trino/files/trino_maintenance.py) the doc comment claims to mirror -- it uses fs.native-s3.enabled + s3.region + s3.iam-role. This restores that match (one property name). Latent until now: no org could be Trino-enabled until the numeric-org-id validator was removed (#672), so CREATE CATALOG had never executed end-to-end. Found live enabling the first Trino org (ben-cnpg-ice) in mw-dev, where the reconcile loops on this error every ~10s. - buildCatalogProperties: fs.s3.enabled -> fs.native-s3.enabled - Correct the doc comment + the unit test that asserted the wrong name Co-Authored-By: Claude Opus 4.8 (1M context) --- controlplane/provisioner/trino_provisioner.go | 13 ++++++++----- controlplane/provisioner/trino_provisioner_test.go | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/controlplane/provisioner/trino_provisioner.go b/controlplane/provisioner/trino_provisioner.go index 3e8de24b..7eef4b64 100644 --- a/controlplane/provisioner/trino_provisioner.go +++ b/controlplane/provisioner/trino_provisioner.go @@ -955,11 +955,14 @@ func (p *TrinoProvisioner) reconcileCatalogs(ctx context.Context, orgs []configs // credentials are embedded here — just the endpoint, the warehouse // name, and the per-org S3 IAM role. // -// Property names follow Trino's current file-system / Iceberg-REST -// surface (matching the working maintenance chart at -// charts/charts/trino/files/trino_maintenance.py): +// Property names MUST stay in sync with the working maintenance chart at +// charts/charts/trino/files/trino_maintenance.py — this set drifted from it +// once (fs.s3.enabled vs fs.native-s3.enabled), which Trino 476 rejects at +// CREATE CATALOG ("Configuration property 'fs.s3.enabled' was not used"): // -// fs.s3.enabled=true enable the unified S3 file system +// fs.native-s3.enabled=true enable Trino's native S3 file system +// (required since Trino 458; the legacy +// Hadoop fs.s3.* surface is rejected by 476) // s3.region= forwarded to the AWS SDK // s3.iam-role= per-org duckling- role // @@ -974,7 +977,7 @@ func (p *TrinoProvisioner) buildCatalogProperties(orgID string, ic *configstore. "iceberg.catalog.type": "rest", "iceberg.rest-catalog.uri": ic.LakekeeperEndpoint, "iceberg.rest-catalog.warehouse": ic.LakekeeperWarehouse, - "fs.s3.enabled": "true", + "fs.native-s3.enabled": "true", } if p.iamAccountID != "" { props["s3.iam-role"] = fmt.Sprintf("arn:aws:iam::%s:role/duckling-%s", p.iamAccountID, orgID) diff --git a/controlplane/provisioner/trino_provisioner_test.go b/controlplane/provisioner/trino_provisioner_test.go index c107b28c..abd86123 100644 --- a/controlplane/provisioner/trino_provisioner_test.go +++ b/controlplane/provisioner/trino_provisioner_test.go @@ -462,8 +462,8 @@ func TestReconcile_CreatesCatalogProjectsSecretAndConfigMap(t *testing.T) { if !strings.Contains(props["s3.iam-role"], "duckling-42") { t.Errorf("expected duckling-42 role ARN under s3.iam-role, got %q", props["s3.iam-role"]) } - if props["fs.s3.enabled"] != "true" { - t.Errorf("expected fs.s3.enabled=true, got %q", props["fs.s3.enabled"]) + if props["fs.native-s3.enabled"] != "true" { + t.Errorf("expected fs.native-s3.enabled=true, got %q", props["fs.native-s3.enabled"]) } // Old property names must NOT be emitted — they're silently ignored // or rejected by current Trino.