Skip to content

Encryption for REST catalog #13225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
smaheshwar-pltr wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Encryption for REST catalog #13225

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
af672ed
Encryption for REST catalog
smaheshwar-pltr Dec 5, 2025
7ad846d
Nit improvements
smaheshwar-pltr Dec 5, 2025
9ed8d8e
Suppress test method length
smaheshwar-pltr Dec 5, 2025
2d18a6a
Merge branch 'main' into rest-encrypt-on-main-cherry-pick
smaheshwar-pltr Dec 9, 2025
745a959
Spotless
smaheshwar-pltr Dec 9, 2025
1106a26
Merge branch 'main' into rest-encrypt-on-main-cherry-pick
smaheshwar-pltr Jan 21, 2026
8ac5228
Fix conflicts and update test
smaheshwar-pltr Jan 21, 2026
73e0980
Undo 4.0 changes
smaheshwar-pltr Jan 21, 2026
a774a9c
Fix checkstyle
smaheshwar-pltr Jan 21, 2026
86abfab
empty
smaheshwar-pltr Jan 21, 2026
1e42400
Nit: Simplify adding encryption keys
smaheshwar-pltr Jan 21, 2026
3c055ce
Respect `CatalogProperties.ENCRYPTION_KMS_TYPE`
smaheshwar-pltr Mar 12, 2026
b9e5822
Merge
smaheshwar-pltr Mar 12, 2026
cd04290
Fix merge
smaheshwar-pltr Mar 12, 2026
ee27d5d
Nits + spotless
smaheshwar-pltr Mar 12, 2026
2092454
Add TODO for encryption <> scan planning
smaheshwar-pltr May 1, 2026
cfb7345
Fixes
smaheshwar-pltr May 2, 2026
0af4ef4
Merge remote-tracking branch 'origin/main' into rest-encrypt-on-main-…
smaheshwar-pltr May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableCommit;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.encryption.EncryptionUtil;
import org.apache.iceberg.encryption.KeyManagementClient;
import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import org.apache.iceberg.exceptions.NoSuchTableException;
Expand Down Expand Up @@ -167,6 +169,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
private CloseableGroup closeables = null;
private Set<Endpoint> endpoints;
private Supplier<Map<String, String>> mutationHeaders = Map::of;
private KeyManagementClient keyManagementClient = null;
private String namespaceSeparator = null;

private RESTTableCache tableCache;
Expand Down Expand Up @@ -272,6 +275,12 @@ public void initialize(String name, Map<String, String> unresolved) {
mergedProps,
RESTCatalogProperties.METRICS_REPORTING_ENABLED,
RESTCatalogProperties.METRICS_REPORTING_ENABLED_DEFAULT);

if (mergedProps.containsKey(CatalogProperties.ENCRYPTION_KMS_IMPL)) {
this.keyManagementClient = EncryptionUtil.createKmsClient(mergedProps);
this.closeables.addCloseable(this.keyManagementClient);
}

this.namespaceSeparator =
PropertyUtil.propertyAsString(
mergedProps,
Expand Down Expand Up @@ -575,6 +584,7 @@ private Supplier<BaseTable> createTableSupplier(
Map::of,
mutationHeaders,
tableFileIO(context, tableConf, credentials),
keyManagementClient,
Copy link
Copy Markdown
Contributor

@singhpk234 singhpk234 Jan 29, 2026

Choose a reason for hiding this comment

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

have you all thought about the IRC remote scan planning and it integration with encryption ?

do we have test for the same.

Copy link
Copy Markdown
Contributor Author

@smaheshwar-pltr smaheshwar-pltr Mar 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for bringing this up!

Don't have all the context here, but some initial thoughts:

  1. key-metadata is part of the ContentFile schema in the REST spec

    iceberg/open-api/rest-catalog-open-api.yaml

    Line 4493 in f865bac

    key-metadata:
    so FileScanTasks returned by server-side scan planning can carry the
    encryption metadata needed for clients to decrypt data files. See debugging from [POC] JDBC encryption to test encrypted scan planning #15603:
image

  1. This does require that REST implementations obtain the key metadata for data files that clients have written, which due to the nature of enveloping encryption may limit custom server-side optimisations

  2. fileIOForPlanId catches my eye

    iceberg/core/src/main/java/org/apache/iceberg/rest/RESTTableScan.java

    Line 215 in f865bac

    private FileIO fileIOForPlanId(List<Credential> storageCredentials) {
    but thinking more I actually suspect that when executors read data files, they don't use per-plan file IO but instead the table's IO via SerializableTable which maybe feels odd if so. WDYT?

  3. I think testing is a bit involved as it requires the server-side catalog to support encryption (or some custom interception to return key metadata in scan tasks). I think it's doable though and tests should pass without changes - see [POC] JDBC encryption to test encrypted scan planning #15603 (TestRemoteScanPlanningWithEncryption) for a proof of concept passing test suite (I threw together JDBC encryption for the POC).

Copy link
Copy Markdown
Contributor Author

@smaheshwar-pltr smaheshwar-pltr Mar 12, 2026

Choose a reason for hiding this comment

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

Would suggest us moving forwards with this PR as is for now to keep it concise, and it feels no regret to do so? Curious for thoughts on that and the above 🙏

Copy link
Copy Markdown
Contributor Author

@smaheshwar-pltr smaheshwar-pltr Mar 18, 2026

Choose a reason for hiding this comment

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

Ah sorry on (3) I see maybe #15448 addresses this and https://github.com/apache/iceberg/pull/15448/changes#diff-3e1df3e550b23bbdfd41f0cdbfe1923fe26755e8954aab0f78cfb288fbb6e08fR94 looks good

tableMetadata,
endpoints);

Expand Down Expand Up @@ -674,6 +684,7 @@ public Table registerTable(
Map::of,
mutationHeaders,
tableFileIO(context, tableConf, response.credentials()),
keyManagementClient,
response.tableMetadata(),
endpoints);

Expand Down Expand Up @@ -943,6 +954,7 @@ public Table create() {
Map::of,
mutationHeaders,
tableFileIO(context, tableConf, response.credentials()),
keyManagementClient,
response.tableMetadata(),
endpoints);

Expand Down Expand Up @@ -976,6 +988,7 @@ public Transaction createTransaction() {
Map::of,
mutationHeaders,
tableFileIO(context, tableConf, response.credentials()),
keyManagementClient,
RESTTableOperations.UpdateType.CREATE,
createChanges(meta),
meta,
Expand Down Expand Up @@ -1041,6 +1054,7 @@ public Transaction replaceTransaction() {
Map::of,
mutationHeaders,
tableFileIO(context, tableConf, response.credentials()),
keyManagementClient,
RESTTableOperations.UpdateType.REPLACE,
changes.build(),
base,
Expand Down Expand Up @@ -1181,6 +1195,7 @@ private FileIO tableFileIO(
* @param mutationHeaderSupplier a supplier for additional HTTP headers to include in mutation
* requests (POST/DELETE)
* @param fileIO the FileIO implementation for reading and writing table metadata and data files
* @param kmsClient the {@link KeyManagementClient} for encrypted tables
Copy link
Copy Markdown
Contributor Author

@smaheshwar-pltr smaheshwar-pltr Jan 21, 2026

Choose a reason for hiding this comment

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

Pointing out that I'm changing the signature of this method. #14740 did the same recently, and I think #14465 that hasn't released yet so there shouldn't be problems with this

* @param current the current table metadata
* @param supportedEndpoints the set of supported REST endpoints
* @return a new RESTTableOperations instance
Expand All @@ -1191,10 +1206,18 @@ protected RESTTableOperations newTableOps(
Supplier<Map<String, String>> readHeaders,
Supplier<Map<String, String>> mutationHeaderSupplier,
FileIO fileIO,
KeyManagementClient kmsClient,
TableMetadata current,
Set<Endpoint> supportedEndpoints) {
return new RESTTableOperations(
restClient, path, readHeaders, mutationHeaderSupplier, fileIO, current, supportedEndpoints);
restClient,
path,
readHeaders,
mutationHeaderSupplier,
fileIO,
kmsClient,
current,
supportedEndpoints);
}

/**
Expand All @@ -1211,6 +1234,7 @@ protected RESTTableOperations newTableOps(
* @param mutationHeaderSupplier a supplier for additional HTTP headers to include in mutation
* requests (POST/DELETE)
* @param fileIO the FileIO implementation for reading and writing table metadata and data files
* @param kmsClient the {@link KeyManagementClient} for encrypted tables
* @param updateType the {@link RESTTableOperations.UpdateType} being performed
* @param createChanges the list of metadata updates to apply during table creation or replacement
* @param current the current table metadata (may be null for CREATE operations)
Expand All @@ -1223,6 +1247,7 @@ protected RESTTableOperations newTableOps(
Supplier<Map<String, String>> readHeaders,
Supplier<Map<String, String>> mutationHeaderSupplier,
FileIO fileIO,
KeyManagementClient kmsClient,
RESTTableOperations.UpdateType updateType,
List<MetadataUpdate> createChanges,
TableMetadata current,
Expand All @@ -1233,6 +1258,7 @@ protected RESTTableOperations newTableOps(
readHeaders,
mutationHeaderSupplier,
fileIO,
kmsClient,
updateType,
createChanges,
current,
Expand Down
Loading