Aliyun: Add AliyunKeyManagementClient for client-side encryption by mengna-lin · Pull Request #18210 · apache/iceberg

mengna-lin · 2026-09-22T21:53:53Z

What

Adds AliyunKeyManagementClient, an Aliyun KMS implementation of the KeyManagementClient SPI, so tables using Iceberg's envelope encryption can wrap/unwrap data keys with Alibaba Cloud KMS..

Why

The aliyun module previously supported only OSS; there was no way to use Iceberg client-side (envelope) encryption on Alibaba Cloud. This closes that gap.

Scope

This PR adds a wrap-only KMS client (wrapKey/unwrapKey). Server-side key generation (generateKey/GenerateDataKey) is intentionally out of scope here — Iceberg's StandardEncryptionManager always creates key encryption keys via wrapKey and never calls generateKey, so wiring that path is a separate, core-level change tracked in #18300.

Changes

AliyunKeyManagementClient — wrapKey (KMS Encrypt) and unwrapKey (KMS Decrypt). On unwrap, the wrapping key is verified against the Decrypt response for immutable references (bare key id / key ARN); aliases are skipped (a mutable pointer may differ from the key that wrapped older data). The client is built lazily from serializable properties so it survives Java/Kryo serialization to Spark/Flink executors.
AliyunClientFactories#newKmsClient — builds the KMS client from client.region (the SDK resolves kms.<region>.aliyuncs.com, the same region-based model AWS uses), with an optional client.kms-endpoint override for a KMS Instance (DKMS). Credential resolution follows the same order as newOSSClient:
1. RRSA/OIDC — used when its environment variables are present (ALIBABA_CLOUD_OIDC_PROVIDER_ARN, ALIBABA_CLOUD_ROLE_ARN, ALIBABA_CLOUD_OIDC_TOKEN_FILE). Takes precedence over the static access-key properties.
2. Otherwise the static client.access-key-id / client.access-key-secret (+ optional client.security-token).
AliyunProperties — adds client.region, client.kms-endpoint, and KMS client tuning (kms.client.aliyun.max.attempts, kms.client.aliyun.connect.timeout.ms, kms.client.aliyun.read.timeout.ms).
Core — registers aliyun as a pre-defined encryption.kms-type (alongside aws/azure/gcp).
Build — adds com.aliyun:kms20160120 (aliyun-sdk-kms 2.3.0) as compileOnly; bumps credentials-java to 1.0.3 and tea to match the KMS SDK's transitive requirement; adds mockito-core and kryo to the aliyun test scope.
Docs — encryption.md lists aliyun as a supported encryption.kms-type.
Tests — TestAliyunKeyManagementClient covers wrap/unwrap mapping, selection via encryption.kms-type=aliyun, wrapping-key verification (bare id / ARN / alias), and Java+Kryo serialization; TestAliyunClientFactories covers region-based endpoint resolution and the missing-region error.

Configuration

# Select the Aliyun KMS client (catalog property)
encryption.kms-type = aliyun
#   (or) encryption.kms-impl = org.apache.iceberg.aliyun.AliyunKeyManagementClient
client.region       = cn-hangzhou          # resolves kms.<region>.aliyuncs.com
client.kms-endpoint = <dkms-endpoint>       # optional: KMS Instance (DKMS) endpoint override

# Credential resolution order (same as OSS):
#   1. RRSA/OIDC — used if its env vars are set (ALIBABA_CLOUD_OIDC_PROVIDER_ARN,
#      ALIBABA_CLOUD_ROLE_ARN, ALIBABA_CLOUD_OIDC_TOKEN_FILE). TAKES PRECEDENCE over the keys below.
#   2. otherwise the static access key / secret (+ optional security token)
client.access-key-id     = <access-key-id>
client.access-key-secret = <access-key-secret>
client.security-token    = <sts-token>      # optional

# optional KMS client tuning:
kms.client.aliyun.max.attempts       = 3       # default
kms.client.aliyun.connect.timeout.ms = 2000    # default
kms.client.aliyun.read.timeout.ms    = 30000   # default

Activate encryption on a table with the master key id (stored in Aliyun KMS):

CREATE TABLE db.t (id bigint, data string) USING iceberg
TBLPROPERTIES ('encryption.key-id' = '<kms-key-id | key-arn | alias>');

Testing

./gradlew :iceberg-aliyun:test --tests "org.apache.iceberg.aliyun.TestAliyunKeyManagementClient" --tests "org.apache.iceberg.aliyun.TestAliyunClientFactories" — all pass.

End-to-end validation against real Alibaba Cloud KMS

Validated AliyunKeyManagementClient end-to-end against a real Alibaba Cloud KMS Instance (DKMS) key, in-cluster, with keyless RRSA credentials. Every operation runs through the Iceberg table (StandardEncryptionManager + EncryptingFileIO), format-version=3, HadoopFileIO with a file-based warehouse. (Encryption was injected via a small HadoopTableOperations subclass, since upstream currently wires client-side encryption only in HiveCatalog.)

Create → table.encryption() resolves to a StandardEncryptionManager backed by AliyunKeyManagementClient.
Write (OutputFileFactory + appendFile().commit()) → the table key is wrapped via KMS Encrypt; on disk, data files are PARE (Parquet modular encryption, encrypted footer), manifest + manifest-list .avro are AGS1 (Iceberg AES-GCM stream), and root metadata.json is plaintext (expected — it holds only the KMS-wrapped key + table structure and bootstraps decryption).
Read (IcebergGenerics.read(table) scan) → decrypts manifest-list → manifests → data via KMS unwrapKey; decrypted rows exactly match what was written.
Cross-process cold read → a separate JVM opens the table from the persisted metadata.json, rebuilds the StandardEncryptionManager, and scan-reads the decrypted rows — proving decryption bootstraps from table metadata with no in-process key cache.
Negative: a tampered wrapped key is rejected by unwrapKey (KMS Decrypt fails — no silent garbage).
KMS Instance (DKMS) endpoint: client.kms-endpoint is required for an instance (key-…) key — with the override the client reaches the instance crypto endpoint and succeeds; without it the request falls back to the shared regional KMS and fails This action is not supported.

Add an Alibaba Cloud KMS implementation of the KeyManagementClient SPI so tables using Iceberg envelope encryption can wrap/unwrap data keys with Aliyun KMS, at parity with AwsKeyManagementClient. The aliyun module previously supported only OSS. - AliyunKeyManagementClient: wrapKey, unwrapKey, and server-side generateKey; supportsKeyGeneration() returns true. - AliyunClientFactory#newKmsClient: build the KMS client from client.region, resolving credentials from access key/secret/token and falling back to the default chain (env vars, RRSA/OIDC, ECS RAM role). - AliyunProperties: add client.region and kms.data-key-spec (default AES_256). - Build: add com.aliyun:kms20160120 (compileOnly) and mockito-core (test). - Tests: cover wrap/unwrap mapping, key generation, and region-based endpoint resolution.

- Add kms.client.aliyun.key.generation.enabled (default true) to drive supportsKeyGeneration(); reject wrapKey when generation is enabled. - unwrapKey verifies the wrapping key id (ids/ARNs checked, aliases skipped). - Issue KMS calls via *WithOptions with configurable retry/timeouts; cache client + options in ClientState. - Resolve KMS credentials like newOSSClient (RRSA/OIDC, then static keys). - Rename kms.data-key-spec to kms.client.aliyun.generation.data_key_spec. - Tests: key-id verification, wrapKey rejection, retry wiring, serialization.

Add a client.kms-endpoint property that overrides the region-derived kms.<region>.aliyuncs.com endpoint, so keys stored in a KMS Instance (DKMS) endpoint can be used. Defaults to the region-based endpoint when unset.

ggershinsky · 2026-09-24T07:00:35Z

Per #15272 , do you want to add the new KMS type?

dramaticlly · 2026-09-24T21:36:05Z

+
+  @Override
+  public ByteBuffer wrapKey(ByteBuffer key, String wrappingKeyId) {
+    if (enableKeyGeneration) {


today the generateKey is not exercised in the iceberg code base, if enableKeyGeneration is default to true , the wrapKey called by StandardEncryptionManager.keyEncryptionKeyID() will always throw exception

Issue is fixed with 29567a5.

dramaticlly · 2026-09-24T21:37:56Z

+
+  /**
+   * Enables server-side data key generation. When enabled (the default), Iceberg calls {@link
+   * #generateKey(String)}; set to {@code false} to have Iceberg generate keys locally and {@link


iceberg does not seem to call generateKey in existing code base today

With 29567a5 now it calls generateKey.

StandardEncryptionManager always wrapped new key encryption keys via KeyManagementClient.wrapKey, so supportsKeyGeneration()/generateKey() were never consulted. This breaks KMS clients whose wrapKey rejects calls when key generation is enabled. Branch key encryption key creation on kmsClient.supportsKeyGeneration(): generate via KMS when supported, otherwise generate locally and wrap. AwsKeyManagementClient now reports supportsKeyGeneration() = false so it keeps using wrapKey; its behavior is unchanged. Aliyun: - Register "aliyun" as a pre-defined encryption.kms-type. - Default key generation to false so the client uses wrapKey by default, consistent with AWS/Azure/GCP; generateKey is opt-in via kms.client.aliyun.key.generation.enabled=true. - Move the KMS client property keys and defaults into AliyunProperties and read them through it, matching AwsProperties; hyphenate the data key spec property (kms.client.aliyun.generation.data-key-spec) and document that it sizes the key encryption key on the generateKey path. - Bump credentials-java to 1.0.3 to match the KMS SDK's requirement.

mengna-lin · 2026-09-28T04:16:39Z

@ggershinsky @dramaticlly Comments addressed. Please take another look. Thanks.

dramaticlly · 2026-09-28T17:58:42Z

  @Override
  public boolean supportsKeyGeneration() {
-    return true;
+    return false;


This is changed probably because the encryption manager now use generate key instead of wrapping key if keyGeneration is supported. Capability wise AWS KMS does support the key generation, so we just need a graceful way to rollout the behavior change, maybe a devlist email if we want to bundle this together in this PR, @ggershinsky what do you think?

The reason why I made the default value false is that I want to keep the behavior consistent after the change. Shall we make this configurable in this PR or a separate PR?

Yep, it was my suggestion to add leveraging key generation here, but I missed the existing support in AwsKms, which makes this a behavior change. The generation leverage probably should be implemented in a separate PR.

Create an issue to track it: #18300.

In that case @mengna-lin let's just focus on AliyunKeyManagementClient change for now, I would suggest to leave core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java and aws/src/main/java/org/apache/iceberg/aws/AwsKeyManagementClient.java unchanged for now and handle it in a separate PR

@dramaticlly I have made this PR that Aliyun always goes with wrapKey. AliyunKeyManagementClient supportsKeyGeneration is hardcoded to be false. The same as AwsKeyManagementClient.
Also update the scope of #18300.

Following review, scope this PR to the Aliyun KMS client only: - Revert the StandardEncryptionManager key-generation branch and the AwsKeyManagementClient supportsKeyGeneration change; wiring StandardEncryptionManager to use generateKey is deferred to a separate PR. - Hardcode AliyunKeyManagementClient.supportsKeyGeneration() to false and remove the enable/disable flag (kms.client.aliyun.key.generation.enabled) and the wrapKey guard, so the client always wraps keys via KMS. generateKey and the data key spec property are kept as a dormant capability, mirroring AwsKeyManagementClient.

Set kms.client.aliyun.generation.data-key-spec to AES_128 (non-default) in testKmsClientSerialization and assert the round-tripped client sends it, so the test verifies a non-default spec survives serialization.

StandardEncryptionManager always wraps key encryption keys via wrapKey and never calls generateKey, so the Aliyun client's generateKey() / supportsKeyGeneration() override and the kms.client.aliyun.generation.data-key-spec property were dead code. Remove them, leaving a wrap-only client consistent with the Azure and GCP KMS clients.

ggershinsky · 2026-10-06T06:13:21Z

An obvious observation - out of the platform modules in Iceberg, AWS, GCP and Azure already have a built-in KMS client. One should be added for Aliyun too, of course.

ggershinsky · 2026-10-06T06:16:07Z

I'd suggest tracking #18337, since it will affect this one.

mengna-lin · 2026-10-06T18:48:20Z

I'd suggest tracking #18337, since it will affect this one.

The scope of this PR does not cover key generation. I will tackle it in a separate PR. Thanks for the reminder.

mengna-lin · 2026-10-06T22:51:21Z

@ggershinsky @dramaticlly I have addressed all comments and applied real env tests. Real env tests details are in PR description. Please take another look when you get a chance. Thanks!

ggershinsky · 2026-10-07T07:45:45Z

The scope of this PR does not cover key generation. I will tackle it in a separate PR. Thanks for the reminder.

Makes sense, the PR split will make it easier for the community to review this.

ggershinsky

Thanks @mengna-lin ! Encryption-wise, LGTM.

ggershinsky · 2026-10-07T07:54:45Z

Tagging committers @RussellSpitzer @singhpk234 @huaxingao

dramaticlly

Thanks Mengna for the Aliyun KMS work and E2E verification in a real aliyun environment. I think this complement our encryption support on all major cloud provider.

@nastra, I saw you helped a few aliyun change in the past, do you want to take a look as well?

singhpk234

LGTM too, seems like a straight forward impl for the CSP

huaxingao

LGTM

Make the new SPI method a default throwing UnsupportedOperationException so existing custom factories wired via client.factory-impl keep compiling. Generated-by: Claude Code

dramaticlly · 2026-10-09T19:08:05Z

@huaxingao @singhpk234 thanks for the review, wondering if this one is ready to merge?

singhpk234 · 2026-10-10T00:21:39Z

Thanks for the contribution @mengna-lin and thanks for review @ggershinsky @dramaticlly @mengna-lin

github-actions Bot added build ALIYUN labels Sep 22, 2026

mengna-lin marked this pull request as draft September 22, 2026 22:58

mengna-lin marked this pull request as ready for review September 23, 2026 23:54

Aliyun: Support KMS endpoint override for KMS Instance (DKMS)

29516a0

Add a client.kms-endpoint property that overrides the region-derived kms.<region>.aliyuncs.com endpoint, so keys stored in a KMS Instance (DKMS) endpoint can be used. Defaults to the region-based endpoint when unset.

ggershinsky reviewed Sep 24, 2026

View reviewed changes

Comment thread aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java Outdated

ggershinsky reviewed Sep 24, 2026

View reviewed changes

Comment thread aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunKeyManagementClient.java Outdated

dramaticlly reviewed Sep 24, 2026

View reviewed changes

github-actions Bot added core docs AWS labels Sep 28, 2026

dramaticlly reviewed Sep 28, 2026

View reviewed changes

github-actions Bot removed the AWS label Sep 29, 2026

dramaticlly reviewed Sep 29, 2026

View reviewed changes

Comment thread aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunKeyManagementClient.java Outdated

abhishekmauryaKsolves mentioned this pull request Sep 30, 2026

Make KMS key generation configurable for AwsKeyManagementClient and AliyunKeyManagementClient (currently always wraps) #18300

Open

1 of 3 tasks

mengna-lin added 3 commits September 30, 2026 14:47

Aliyun: Use non-default data key spec in KMS client serialization test

19d7abe

Set kms.client.aliyun.generation.data-key-spec to AES_128 (non-default) in testKmsClientSerialization and assert the round-tripped client sends it, so the test verifies a non-default spec survives serialization.

Merge remote-tracking branch 'upstream/main' into aliyun-kms

e692f21

abhishekmauryaKsolves mentioned this pull request Oct 6, 2026

Core: Make KMS key generation configurable via encryption.kms-key-generation-enabled #18337

Open

ggershinsky approved these changes Oct 7, 2026

View reviewed changes

dramaticlly approved these changes Oct 7, 2026

View reviewed changes

singhpk234 approved these changes Oct 7, 2026

View reviewed changes

huaxingao reviewed Oct 7, 2026

View reviewed changes

Comment thread aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactory.java Outdated

huaxingao approved these changes Oct 7, 2026

View reviewed changes

Aliyun: Add default newKmsClient() to AliyunClientFactory

ab4804a

Make the new SPI method a default throwing UnsupportedOperationException so existing custom factories wired via client.factory-impl keep compiling. Generated-by: Claude Code

singhpk234 merged commit 0f3131f into apache:main Oct 10, 2026
44 checks passed

smaheshwar-pltr mentioned this pull request Oct 13, 2026

Spec: REST KMS credential vending for table encryption #17155

Open

Conversation

mengna-lin commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Scope

Changes

Configuration

Testing

End-to-end validation against real Alibaba Cloud KMS

Uh oh!

Uh oh!

Uh oh!

ggershinsky commented Sep 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mengna-lin commented Sep 28, 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!

Uh oh!

ggershinsky commented Oct 6, 2026

Uh oh!

ggershinsky commented Oct 6, 2026

Uh oh!

mengna-lin commented Oct 6, 2026

Uh oh!

mengna-lin commented Oct 6, 2026

Uh oh!

ggershinsky commented Oct 7, 2026

Uh oh!

ggershinsky left a comment

Choose a reason for hiding this comment

Uh oh!

ggershinsky commented Oct 7, 2026

Uh oh!

dramaticlly left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

singhpk234 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huaxingao left a comment

Choose a reason for hiding this comment

Uh oh!

dramaticlly commented Oct 9, 2026

Uh oh!

Uh oh!

singhpk234 commented Oct 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mengna-lin commented Sep 22, 2026 •

edited

Loading

dramaticlly left a comment •

edited

Loading