Support encrypted table operations and server-side purge by hkwi · Pull Request #5060 · apache/polaris

hkwi · 2026-07-15T05:50:51Z

Summary

This draft adds Polaris-side Iceberg table encryption support for table operations and asynchronous server-side purge.

Initialize and own an Iceberg KeyManagementClient from catalog properties.
Build a metadata-aware EncryptionManager and EncryptingFileIO for encrypted tables.
Use uncommitted table metadata while a transaction writes manifests, and persist the wrapped keys generated by the encryption manager in table metadata at commit.
Carry a structured, trusted encryption context into asynchronous cleanup tasks so server-side purge can read encrypted metadata, manifest lists, and manifests before deleting table files.
Close task-owned KMS and FileIO resources on both successful and failed initialization paths.

Related to #2829, which tracks the broader Iceberg encryption support discussion.

Motivation and scope

An Iceberg client can write an encrypted table, but Polaris also executes Iceberg I/O in server-side paths such as purge. Those paths need an encryption manager capable of reading encrypted manifest metadata; otherwise the existing cleanup handlers cannot enumerate and delete all files in an encrypted table.

This change uses Iceberg's existing catalog-property contract (encryption.kms-type / encryption.kms-impl and implementation-specific properties). The Polaris server and an Iceberg client may use the same KMS implementation and settings, but they are configured independently and only need to resolve the same table key ID. An out-of-tree KeyManagementClient can be used when its implementation is present on the Polaris runtime classpath.

This draft does not vend KMS configuration or credentials through the REST Catalog API, define key rotation, or attempt to cover all possible REST semantics for encrypted tables.

Implementation notes

Temporary table operations resolve encryption from the transaction's uncommitted metadata instead of the outer operation's committed metadata cache.
The encryption manager remains owned by the outer table operations instance so wrapped keys registered while writing manifests survive until the final metadata commit.
Cached managers are bound to the table key ID and DEK length. Plaintext results are not cached, and incompatible metadata fails fast rather than reusing a stale manager.
Newly committed metadata is exposed through the local operations cache only after the Polaris catalog create/update succeeds.
Cleanup tasks store KMS configuration and wrapped keys in one structured internal encryption context. That context is not passed to FileIO initialization.

Tests

The targeted tests pass for both NoSQL in-memory and relational catalog implementations, covering:

encrypted create transactions and encrypted manifest/manifest-list output;
encrypted table write, read, refresh, reload, and server-side purge;
metadata-bound encryption manager reuse and mismatch rejection;
structured cleanup-context serialization and isolation from FileIO properties;
encrypted task FileIO round trips and resource closing; and
preservation of the current metadata cache after a failed catalog commit.

Commands run:

./gradlew :polaris-runtime-service:test <targeted encryption, purge, transaction, and commit-failure tests>
./gradlew :polaris-core:spotlessJavaCheck :polaris-runtime-service:spotlessJavaCheck

This PR does not enable or change coverage tooling.

Checklist

🛡️ Don't disclose security issues! (contact security@apache.org)
🔗 Clearly explained why the changes are needed, or linked related issues: Related to Iceberg encryption support #2829
🧪 Added/updated tests with good coverage, or manually tested (and explained how)
💡 Added comments for complex logic
🧾 Updated CHANGELOG.md (if needed; deferred while this is a draft)
📚 Updated documentation in site/content/in-dev/unreleased (if needed; deferred while this is a draft)

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

dimas-b

Thanks for your contribution, @hkwi !

Given that it's a major change, would you mind starting a corresponding discussion on the dev ML for visibility?

This is just a preliminary review from my side, but in general the PR looks pretty solid.

dimas-b · 2026-07-15T22:07:02Z

      }

-      // We diverge from `BaseMetastoreTableOperations` in the below code block
+      if (null == existingLocation) {


Will this conflict with #5057? 🤔

Thanks for flagging this. I merged the current main, including #5057, without force-pushing. The conflict was in doCommit. I kept the MetadataWriteResult/writeSucceeded cleanup flow from #5057 unchanged and removed the writer-side encryption changes from that path. The remaining LocalIcebergCatalog change only preserves encryption context for the server-side purge task, so the two changes no longer overlap semantically.

flyrain · 2026-07-17T17:34:34Z

Thanks @hkwi for working on it. The scope sounds good to me.

flyrain · 2026-07-17T17:40:16Z

    closeableGroup.addCloseable(metricsReporter());
+    if (properties.containsKey(CatalogProperties.ENCRYPTION_KMS_TYPE)
+        || properties.containsKey(CatalogProperties.ENCRYPTION_KMS_IMPL)) {
+      keyManagementClient = EncryptionUtil.createKmsClient(properties);


I think we should document how the KMS client obtains credentials for the KMS store, especially when multiple KMS stores are used. Not a blocker, though. Either a follow up or documenting it in this PR would be nice.

Thanks. I added a code comment at the dynamic KMS load site explaining that this follows the same structural pattern as CatalogUtil.loadFileIO: the configured KeyManagementClient is instantiated from trusted catalog properties, and credential resolution is delegated to that implementation. The cleanup task keeps those catalog KMS properties and the wrapped table keys in a task-only envelope rather than merging them into FileIO/storage properties. If a custom client selects among multiple KMS stores, that selection and credential handling remain the responsibility of that KeyManagementClient. Broader deployment documentation can follow separately.

dimas-b · 2026-07-18T20:13:51Z


-      String newLocation = writeNewMetadataIfRequired(base == null, metadata);
+      // SnapshotProducer may already have used this manager to generate and register encryption
+      // keys while writing encrypted manifests. Recreating it here would discard those keys


Polaris Servers never write manifests, IIRC... or did I miss it?

You are right: Polaris should not write manifests. I removed the writer-side EncryptionManager/FileIO initialization and the doCommit key propagation from LocalIcebergCatalog. The only server-side KMS path left is in cleanup tasks, where Polaris reads and decrypts manifest lists and manifests previously written by the engine so that purge can enumerate files to delete. I also added a comment at task creation to make this ownership boundary explicit.

dimas-b · 2026-07-18T20:18:04Z

+
+    @Override
+    public EncryptionManager encryption() {
+      return encryption(currentMetadata);


Why is this override necessary? IIRC, Polaris only writes metadata files, which are not encrypted? 🤔

Agreed. That override was only supporting writer-side encryption and is no longer necessary, so I removed it along with the writer-side encryption manager. Polaris continues to write the plaintext metadata JSON using the normal FileIO. A separate encryption-aware FileIO is reconstructed only by cleanup tasks to read encrypted manifest lists and manifests during server-side purge.

dimas-b · 2026-07-18T20:23:29Z

+              .withProperty(TableProperties.FORMAT_VERSION, "3")
+              .withProperty(TableProperties.ENCRYPTION_TABLE_KEY, PolarisTestKms.MASTER_KEY_NAME)
+              .createTransaction();
+      transaction.table().newFastAppend().appendFile(FILE_A).commit();


This test appears to require more from LocalIcebergCatalog than what is necessary for usual Polaris operation.

Would it be possible to convert this to an integration test similar to RestCatalogRustFSSpecialIT?

Encrypted writes would normally be performed by the "engine" (represented by test) code, which Polaris should only handle purges. WDYT?

Agreed. I replaced the LocalIcebergCatalog test with a REST integration test in RestCatalogFileIntegrationTest. The test-side client/engine writes encrypted data, manifest lists, and manifests, verifies their raw AGS1 headers, commits through REST, requests server-side purge, and verifies deletion. The Polaris production path only handles the purge. Because the corresponding Iceberg REST client encryption support in apache/iceberg#13225 is still pending, the test uses a test-only TableOperations adapter to model the engine-side behavior.

…server-purge-squashed # Conflicts: # runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java

hkwi · 2026-07-21T04:57:03Z

Thanks for the preliminary review. I pushed an update that merges the current main and addresses the inline feedback: writer-side encryption was removed from LocalIcebergCatalog, the remaining KMS use is scoped to reading encrypted manifest lists/manifests during server-side purge, and the test was moved to a REST integration test where the test-side engine performs encrypted writes. Validation completed: format and compileAll passed; PolarisEncryptionUtilTest, TaskFileIOSupplierTest, and RestCatalogFileIntegrationTest.encryptedTableWrittenByClientCanBePurgedByServer passed. I will handle the requested dev mailing-list discussion separately; no ML post has been made yet.

Support encrypted table operations and server-side purge

999271b

github-project-automation Bot added this to Basic Kanban Board Jul 15, 2026

github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board Jul 15, 2026

hkwi marked this pull request as ready for review July 15, 2026 08:39

Copilot AI review requested due to automatic review settings July 15, 2026 08:39

Copilot AI reviewed Jul 15, 2026

dimas-b reviewed Jul 15, 2026

View reviewed changes

flyrain reviewed Jul 17, 2026

View reviewed changes

dimas-b reviewed Jul 18, 2026

View reviewed changes

hkwi mentioned this pull request Jul 21, 2026

Support server-side purge for encrypted tables hkwi/polaris#2

Draft

6 tasks

hkwi added 2 commits July 21, 2026 13:27

Merge remote-tracking branch 'origin/main' into feat/encrypted-table-…

66cc661

…server-purge-squashed # Conflicts: # runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java

Address review feedback for encrypted purge

c00fdf9

hkwi mentioned this pull request Jul 21, 2026

Iceberg encryption support #2829

Open

Uh oh!

Conversation

hkwi commented Jul 15, 2026

Summary

Motivation and scope

Implementation notes

Tests

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b 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!

flyrain commented Jul 17, 2026

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!

hkwi commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants