Skip to content

SS-470 Add support for vended credentials for GCS backed Iceberg catalogs - #38733

Merged
patrickwwbutler merged 5 commits into
patrick/iceberg-storage-providerfrom
patrick/unity-gcs
Sep 10, 2026
Merged

SS-470 Add support for vended credentials for GCS backed Iceberg catalogs#38733
patrickwwbutler merged 5 commits into
patrick/iceberg-storage-providerfrom
patrick/unity-gcs

Conversation

@patrickwwbutler

Copy link
Copy Markdown
Contributor

Updates cargo.toml to point at the new revision of our iceberg fork, containing the custom credential loader support for GCS. Adds logic to opendal storage interface construction to build custom credential loaders for GCS when the access delegation option is specified, and removes the restriction on using ACCESS DELEGATION when creating a GCP biglake iceberg catalog connection.

@patrickwwbutler
patrickwwbutler added this pull request to stack #38734 September 9, 2026 20:40
@linear-code

linear-code Bot commented Sep 9, 2026

Copy link
Copy Markdown

SS-470

@patrickwwbutler
patrickwwbutler force-pushed the patrick/unity-gcs branch 2 times, most recently from 0900966 to b096a5f Compare September 10, 2026 15:02
@patrickwwbutler
patrickwwbutler marked this pull request as ready for review September 10, 2026 17:01
@patrickwwbutler
patrickwwbutler requested review from a team as code owners September 10, 2026 17:01

@peterdukelarsen peterdukelarsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread Cargo.lock Outdated
name = "iceberg"
version = "0.10.1"
source = "git+https://github.com/MaterializeInc/iceberg-rust.git?rev=958ac5b3c318026aeb362485c593e4956aeabe95#958ac5b3c318026aeb362485c593e4956aeabe95"
source = "git+https://github.com/MaterializeInc/iceberg-rust.git?rev=f789d90ff4b516331ba07afc2b9790b20fb3db2a#f789d90ff4b516331ba07afc2b9790b20fb3db2a"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

caught up with patrick offline and he's going to update this to point to the release branch before merging -- I noticed after the fact (thanks to claude) that this was pointing to revision that wasn't merged yet.

@patrickwwbutler
patrickwwbutler merged commit 8ceeee9 into main Sep 10, 2026
82 checks passed
@patrickwwbutler
patrickwwbutler deleted the patrick/unity-gcs branch September 10, 2026 21:12
@def-

def- commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review (Post Merge)

@patrickwwbutler — an automated review of commit 8ceeee9a3a found the following potential MEDIUM+ issue(s) after this PR was merged.

1. MEDIUM -- CREATE CONNECTION reference still documents the restriction this change removes, and misstates GCP storage credentials

doc/user/content/sql/create-connection.md:1003

The ACCESS DELEGATION reference still says the option is "not supported ... for REST catalogs using GCP CONNECTION", and the storage-credential table below it still says a rest catalog with a GCP CONNECTION uses "only the GCP connection's service account". Both are wrong as of this change, and the second one is what operators read to decide which identity to grant bucket access to.

Details

Line 1003: "Valid with | CATALOG TYPE = 'rest' using CREDENTIAL. Not supported for CATALOG TYPE = 's3tablesrest' ... or for REST catalogs using GCP CONNECTION." The sql_bail! that enforced this is exactly what the diff deletes from src/sql/src/plan/statement/ddl/connection.rs.

Line 1012: "| CATALOG TYPE = 'rest' with GCP CONNECTION | Only the GCP connection's service account. |" With ACCESS DELEGATION set, gcs_storage_factory (src/storage-types/src/connections.rs:1079) installs a CustomGcsCredentialLoader, and the fork's gcs_config_build then clears token, credential, credential_path and service_account from the GCS config, so the service-account key authenticates only the catalog. Your own NOTE at src/storage-types/src/connections.rs:1275 says this; the docs table contradicts it. An operator following the table would grant the warehouse-bucket role to the service account and never grant BigLake the ability to vend, then see the sink fail on storage.

That row needs to split into with-delegation and without-delegation. The BigLake syntax block (doc/user/data/examples/create_connection.yml:753) also omits ACCESS DELEGATION, so the new option is undiscoverable for BigLake users.

2. MEDIUM -- a rest + CREDENTIAL catalog with STORAGE PROVIDER = 'gcs' and no delegation falls back to the environment's ambient GCP credentials

src/storage-types/src/connections.rs:1224

When no vended-credential loader is installed, nothing suppresses OpenDAL's GCS Application Default Credentials chain, so a REST catalog that returns no gcs.* storage props leaves the sink signing requests with GOOGLE_APPLICATION_CREDENTIALS, the well-known ADC file, or the GKE metadata server. On a GCP-hosted environment that means a user-named gs:// location can be reached with the environment's own identity rather than the connection's. This predates the commit (it arrives with IcebergStorageProvider::Gcs in bdb2187), but gcs_storage_factory is now the single place GCS storage is built and is where the fix belongs.

Details

Evidence: opendal-service-gcs-0.57.0/src/backend.rs:307 seeds the chain with DefaultCredentialProvider::builder(), whose default slots are env ADC, well-known ADC and VmMetadataCredentialProvider (reqsign-google-3.1.0/src/provide_credential/default.rs:164); no_env()/no_well_known()/no_vm_metadata() are applied only when disable_config_load/disable_vm_metadata are set. The GCP-auth branch sets both props (connections.rs:1253-1254); the OAuth branch sets neither. With a loader installed the fork's suppress_default_credential_sources sets them, which is why the delegated path is safe and the non-delegated one is not.

Reachable path: CREATE CONNECTION ... TO ICEBERG CATALOG (CATALOG TYPE = 'rest', URL = <catalog I control>, CREDENTIAL = SECRET s, STORAGE PROVIDER = 'gcs') with no ACCESS DELEGATION, backed by a catalog whose loadTable returns a gs:// location and no gcs.* credentials.

Fix: insert GCS_DISABLE_VM_METADATA and GCS_DISABLE_CONFIG_LOAD into props whenever the resolved storage provider is GCS, not just in the GCP-auth branch. STORAGE PROVIDER = 'adls' is worth the same check against opendal's Azure credential chain, which I did not verify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants