Core: [Demonstration only] handle hybrid mode encryption with REST Catalog by singhpk234 · Pull Request #18081 · apache/iceberg

singhpk234 · 2026-09-12T20:13:40Z

Summary

Adds REST support for encrypted tables using client-side KMS credentials, with an
opt-in hybrid mode for REST-provided storage access.

By default, encrypted REST tables reject server-provided storage access because
the server-side KMS credential spec is not ready. A new catalog property,
rest.encryption.use-client-kms-creds, allows clients to explicitly opt into
using REST-provided storage access while still using client-side KMS credentials.

Behavior

Default: encrypted REST tables use client-side storage access and client-side
KMS credentials.
If an encrypted table receives REST-provided storage access, the client fails
unless rest.encryption.use-client-kms-creds=true.
REST-provided storage access includes:
- storage-credentials
- remote-signing-config
In hybrid mode, REST-provided storage access is allowed, but KMS credentials are
still loaded only from client-side catalog properties.
Existing remote-signing endpoint wiring is unchanged.

Tests

Added REST catalog tests for encrypted tables with vended storage credentials
and remote signing config.
Added REST scan-planning tests for encrypted tables with returned storage
credentials.
Added coverage ensuring KMS client initialization uses client-side properties
instead of server-returned config.

Validation run:

git diff --check
./gradlew :iceberg-core:spotlessCheck
./gradlew :iceberg-core:test --tests org.apache.iceberg.rest.TestRESTCatalog
--tests org.apache.iceberg.rest.TestRESTScanPlanning

———

AI Disclosure

- Model: GPT-5
- Platform/Tool: Codex
- Human Oversight: done
- Prompt Summary: Implemented review feedback for REST encrypted table handling
  with client-side KMS credentials and an opt-in hybrid path for REST-provided
  storage access.

…cherry-pick # Conflicts: # spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestTableEncryption.java # spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/sql/TestTableEncryption.java

Generated-by: Codex

Use client-side storage and KMS credentials by default for encrypted REST tables, with an opt-in for client-side KMS credentials alongside vended storage credentials. Generated-by: Codex

smaheshwar-pltr

Thanks a lot @singhpk234 for putting this up, realise this is a draft but provides a great way to express some high-level thoughts on this approach so I've done so (sorry!)

Mainly resurfaced #13225 (comment), please LMKWYT of the higher-level comments here (cc @szlta too).

smaheshwar-pltr · 2026-09-16T10:27:13Z

+        && tableMetadata.properties().containsKey(TableProperties.ENCRYPTION_TABLE_KEY);
+  }
+
+  private void validateNoServerSideStorageAccess(


As per #13225 (comment), this PR checks storage credentials + signing config, but what about

returned LoadTableResponse config? Those configs configure IO too from the server, from before we had the storage-credentials field

server /config defaults/overrides also still flow through

Also, remote signing can be configured through IO properties too for both these cases. Is it intentional to only cover the storageCredentials and remoteSigningConfig case?

we can handle these as well, though how about if X-Iceberg-Access-delegation : vended-creds is set fail it ... even before making request ?

I'd argue against that - "X-Iceberg-Access-delegation : vended-creds" is only part of the spec change proposal at this time - I'm not sure it makes sense for the code to express uncommitted spec details.

smaheshwar-pltr · 2026-09-16T10:28:53Z

+      return table().io();
+    }
+
+    Preconditions.checkState(


We probably want to clean up plan resources if we throw this. Otherwise with the current implementation, clean up won't happen

we can fail where and make sure where the close is called ?

smaheshwar-pltr · 2026-09-16T10:31:14Z

      List<Credential> storageCredentials,
      RemoteSigningConfig remoteSigningConfig) {
+    if (useClientSideStorageAccessForEncryptedTable(tableMetadata)) {
+      validateNoServerSideStorageAccess(tableIdentifier, storageCredentials, remoteSigningConfig);


As per #13225 (comment), what about when create / register table throws because of this but the creation has succeeded in the catalog given we can only do the checks after? Have we thought about this case?

At the least, I think the client should know about the side-effect. Trying to "undo" the creation sounds questionable to me

smaheshwar-pltr · 2026-09-16T10:37:50Z


+    if (props.containsKey(CatalogProperties.ENCRYPTION_KMS_TYPE)
+        || props.containsKey(CatalogProperties.ENCRYPTION_KMS_IMPL)) {
+      this.keyManagementClient = EncryptionUtil.createKmsClient(props);


This PR now changes to not use the merged props, but just props if I'm understanding right. I think there is something to be discussed here on server-returned configs (defaults + overrides) which are would be ignored with this change e.g.

https://docs.google.com/document/d/1VSewbVmjukU5eTiZruCJVmUZcRZvcAsd4JOeC6Lt3fo/edit?tab=t.0#heading=h.huszjf7jg3fb mentions

"defaults": { "encryption.kms-type": "aws" },

I guess for now that should be fine, that default property announcement only matters with vended credentials mode.

smaheshwar-pltr · 2026-09-16T10:40:28Z

+      TableIdentifier tableIdentifier,
+      List<Credential> storageCredentials,
+      RemoteSigningConfig remoteSigningConfig) {
+    Preconditions.checkState(


(Noting that users will likely run into this given current docs, maybe worth updating the docs)

smaheshwar-pltr · 2026-09-16T10:42:23Z

+                  .addCredential(credential)
+                  .withRemoteSigningConfig(TEST_REMOTE_SIGNING_CONFIG)


Nit for when we do this properly, probably worth testing cred-only + signing-only independently

smaheshwar-pltr · 2026-09-16T10:43:58Z

 | `rest-page-size`                      | null              | The page size to use when listing namespaces, tables, or other paginated resources.                                                                                                              |
 | `namespace-separator`                 | `%1F`             | The separator character used for namespace levels when communicating with the REST server.                                                                                                       |
 | `scan-planning-mode`                  | `client`          | Controls where scan planning is performed. Supported values: `client` (client-side planning), `server` (server-side planning). Can be overridden per-table by the server in LoadTableResponse. |
+| `rest.encryption.use-client-kms-creds` | `false`           | Whether encrypted REST tables may use client-side KMS credentials together with REST-provided storage access, including vended storage credentials and remote signing. When `false`, encrypted tables reject REST-provided storage access. |


For when we do this properly, this name feels misleading. Client KMS is used when set to true and when set to false. Maybe rest.encryption.allow-hybrid-credentials is more accurate

hybrid means nothing unless we explain what its implies ... may always-use-client-side-kms creds ?

To me "static credentials" say more than "client side credentials" - as they're actually not ephemeral in nature / not vended

szlta · 2026-09-18T11:34:06Z

  public static final long REST_SCAN_PLANNING_POLL_TIMEOUT_MS_DEFAULT =
      TimeUnit.MINUTES.toMillis(5);

+  // Allow REST-provided storage access for encrypted tables that use client-side KMS.


IMHO: not "use client-side KMS" but rather "KMS configured with client-side credentials" (or static credentials)

smaheshwar-pltr and others added 21 commits December 5, 2025 19:33

Encryption for REST catalog

af672ed

Nit improvements

7ad846d

Suppress test method length

9ed8d8e

Merge branch 'main' into rest-encrypt-on-main-cherry-pick

2d18a6a

Spotless

745a959

Merge branch 'main' into rest-encrypt-on-main-cherry-pick

1106a26

Fix conflicts and update test

8ac5228

Undo 4.0 changes

73e0980

Fix checkstyle

a774a9c

empty

86abfab

Nit: Simplify adding encryption keys

1e42400

Respect CatalogProperties.ENCRYPTION_KMS_TYPE

3c055ce

Merge

b9e5822

Fix merge

cd04290

Nits + spotless

ee27d5d

Add TODO for encryption <> scan planning

2092454

Fixes

cfb7345

Merge remote-tracking branch 'origin/main' into rest-encrypt-on-main-…

0af4ef4

…cherry-pick # Conflicts: # spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestTableEncryption.java # spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/sql/TestTableEncryption.java

REST: Merge Apache main and preserve table operation hooks

b7fb7bb

REST: Merge Apache main with the MinIO image fix

441d6b5

Generated-by: Codex

REST: Add client KMS creds option

c8116f3

Use client-side storage and KMS credentials by default for encrypted REST tables, with an opt-in for client-side KMS credentials alongside vended storage credentials. Generated-by: Codex

github-actions Bot added spark core docs labels Sep 12, 2026

smaheshwar-pltr reviewed Sep 16, 2026

View reviewed changes

smaheshwar-pltr mentioned this pull request Sep 16, 2026

Core: Support client-configured encryption in RESTCatalog #13225

Open

szlta reviewed Sep 18, 2026

View reviewed changes

		.addCredential(credential)
		.withRemoteSigningConfig(TEST_REMOTE_SIGNING_CONFIG)

Conversation

singhpk234 commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Behavior

Tests

Uh oh!

smaheshwar-pltr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

singhpk234 commented Sep 12, 2026 •

edited

Loading