[#12488] feat(kms): Identify keys by provider and keyId by nevzheng · Pull Request #12489 · apache/gravitino

nevzheng · 2026-08-18T10:58:56Z

What changes were proposed in this pull request?

KmsReference now identifies a key by configured provider name and keyId only. The KMS protocol (api) stays in server config, and the previous source name is replaced by provider.

Callers resolve a client from KmsClientRegistry by provider. The registry binds each named provider to its API at startup from gravitino.kms.provider.<name>.api. The public reference no longer carries api() or source().

Server config becomes provider-named:

gravitino.kms.providers=aws-prod,aws-dr,azure-eu
gravitino.kms.provider.aws-prod.api=aws-kms
gravitino.kms.provider.aws-dr.api=aws-kms
gravitino.kms.provider.azure-eu.api=azure-key-vault

JSON DTO: {"provider":"aws-prod","keyId":"..."}.

This implements #12488 on the unreleased Developer API from #12132 and #12133. Enterprise stack, design-pack adapter prose, demo conf, and gravitino-server-config.md remain follow-up.

Why are the changes needed?

Putting api and source on the public key identity forced callers to know the protocol and invented a second name for a configured instance. Reviewers asked to change upstream KmsReference instead of adding a parallel identity type, and to use provider rather than source.

Keeping the protocol in server config lets many named providers share one API without leaking that binding onto stored or exchanged key identity.

Fix: #12488

Does this PR introduce any user-facing change?

Yes, for the unreleased @DeveloperApi KMS contracts:

KmsReference is {provider, keyId} instead of {api, source, keyId}
DTO JSON uses provider instead of api and source
Server config keys are gravitino.kms.providers and gravitino.kms.provider.<name>.* instead of sources / source.<name>.*
KmsClientFactory.create and registry lookup take a provider name; registry lookup no longer checks reference.api()

There are no REST API changes. Docs for gravitino-server-config.md are out of scope here.

How was this patch tested?

./gradlew :api:test :common:test :core:test for KMS tests passed

Related work

Closes: #12488
Related to: #12131, #12132, #12133

Nevin
Sent from my 🤖 (Cursor)

Made with Cursor

KmsReference should name a configured provider and keyId so the protocol stays in server config instead of the public key identity. Co-authored-by: Cursor <cursoragent@cursor.com>

Operators need the server-config keys that bind each instance to an API. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi · 2026-08-18T11:21:05Z

@lasdf1234 Could you help me review this pull request?

lasdf1234 · 2026-08-18T12:01:02Z

One last small issue: Could the spaces between the commas be removed?
change 'gravitino.kms.providers = aws-prod, aws-dr, azure-eu' to 'gravitino.kms.providers = aws-prod,aws-dr,azure-eu',

Everything else is fine.

Follow the existing listener convention and drop spaces after commas in the providers list. Co-authored-by: Cursor <cursoragent@cursor.com>

nevzheng · 2026-08-18T12:17:59Z

@lasdf1234 Removed the spaces after commas. The example is now gravitino.kms.providers = aws-prod,aws-dr,azure-eu.

fbf7694

Nevin
Sent from my 🤖 (Cursor)

Co-authored-by: Cursor <cursoragent@cursor.com>

…Fixtures classpath StringUtils.isBlank does not compile from common testFixtures without an explicit dependency. Co-authored-by: Cursor <cursoragent@cursor.com>

…ntifiers The server loads factories by className. A protocol identifier is not part of that contract. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 · 2026-08-18T12:57:43Z

@lasdf1234 Removed the spaces after commas. The example is now gravitino.kms.providers = aws-prod,aws-dr,azure-eu.

fbf7694

Nevin Sent from my 🤖 (Cursor)

I have no more comments.I'll approve. LGTM @roryqi Colud take a look?

Modify the document.

roryqi · 2026-08-18T15:37:48Z

+# conf/gravitino.conf
+gravitino.kms.providers = aws-prod,aws-dr,azure-eu
+
+gravitino.kms.provider.aws-prod.className = org.apache.gravitino.encryption.kms.aws.AwsKmsClientFactory


Could u give a better class name?
This class name will make the user feel that the code contains the AwsKmsClientFactory.
Actually, user need to implement his kms client factory.
From my side, it may be better to use AwsCustomKmsClientFactory.
It would be better to add some words tell users how to implement a custom kms client factory briefly.

The example FQCNs were the problem. They sat under org.apache.gravitino... so they looked shipped.

Key Management now matches Event Listeners: implement KmsClientFactory, public no-arg constructor, put the jar on the classpath, set className. Gravitino does not ship AWS or Azure factories. The example uses your Custom names in com.example.kms:

gravitino.kms.provider.aws-prod.className = com.example.kms.AwsCustomKmsClientFactory gravitino.kms.provider.aws-dr.className = com.example.kms.AwsCustomKmsClientFactory gravitino.kms.provider.azure-eu.className = com.example.kms.AzureCustomKmsClientFactory

d06fa4a

Nevin
Sent from my 🤖 (Cursor)

Looks better.

github-actions · 2026-08-18T15:51:42Z

Code Coverage Report

Overall Project	68.79% `+0.09%`	🟢
Files changed	90.5%	🟢

Module	Coverage
aliyun	1.72%	🔴
api	49.2% `+0.16%`	🟢
authorization-common	85.96%	🟢
aws	42.04%	🟢
azure	2.47%	🔴
catalog-common	9.92%	🔴
catalog-fileset	79.77%	🟢
catalog-glue	68.95%	🟢
catalog-hive	82.96%	🟢
catalog-jdbc-common	45.7%	🟢
catalog-jdbc-doris	81.8%	🟢
catalog-jdbc-mysql	79.33%	🟢
catalog-jdbc-postgresql	83.39%	🟢
catalog-jdbc-starrocks	79.16%	🟢
catalog-kafka	77.01%	🟢
catalog-lakehouse-generic	59.18%	🟢
catalog-lakehouse-hudi	79.1%	🟢
catalog-lakehouse-iceberg	85.86%	🟢
catalog-lakehouse-paimon	84.23%	🟢
catalog-model	77.72%	🟢
cli	44.48%	🟢
client-java	78.56%	🟢
common	52.81% `+0.03%`	🟢
core	83.61% `+0.05%`	🟢
filesystem-hadoop3	77.28%	🟢
flink	0.0%	🔴
flink-common	48.68%	🟢
flink-runtime	0.0%	🔴
gcp	14.12%	🔴
hadoop-auth	68.0%	🟢
hadoop-common	12.7%	🔴
hive-metastore-common	53.4%	🟢
iceberg-aliyun-bundle	0.0%	🔴
iceberg-common	64.75%	🟢
iceberg-rest-server	75.0%	🟢
idp-basic	86.02% `+0.58%`	🟢
integration-test-common	0.0%	🔴
jobs	62.92%	🟢
lance-common	31.63%	🔴
lance-rest-server	63.47%	🟢
lineage	53.02%	🟢
optimizer	83.24%	🟢
optimizer-api	21.95%	🔴
server	87.25%	🟢
server-common	79.57%	🟢
spark	28.57%	🔴
spark-common	45.89%	🟢
tencent	69.84%	🟢
trino-connector	41.56%	🟢

Files

Module	File	Coverage
api	KmsReference.java	93.75%	🟢
common	KmsReferenceDTO.java	85.71%	🟢
	KmsClientFactory.java	0.0%	🔴
core	KmsConfig.java	100.0%	🟢
	KmsClientRegistry.java	88.57%	🟢
idp-basic	IdpCredentialValidator.java	100.0%	🟢
	IdpGroupDTO.java	100.0%	🟢
	IdpGroupMetaBaseSQLProvider.java	100.0%	🟢
	IdpGroupMetaH2Provider.java	100.0%	🟢
	IdpGroupMetaPostgreSQLProvider.java	100.0%	🟢
	IdpGroupOperations.java	100.0%	🟢
	IdpUserGroupManager.java	92.59%	🟢
	AddGroupRequest.java	90.91%	🟢
	IdpGroupPO.java	88.89%	🟢
	IdpPOConverters.java	69.23%	🟢
	IdpGroup.java	46.67%	🔴
	IdpGroupWithUsersPO.java	33.33%	🔴

…tions Example FQCNs looked like shipped AWS and Azure factories. Document the EventListener-style hook instead. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi

LGTM.

[apache#12488] feat(kms): Identify keys by provider and keyId

a5ef5fe

KmsReference should name a configured provider and keyId so the protocol stays in server config instead of the public key identity. Co-authored-by: Cursor <cursoragent@cursor.com>

nevzheng requested review from lasdf1234 and roryqi August 18, 2026 10:58

nevzheng self-assigned this Aug 18, 2026

nevzheng added branch-1.3 Automatically cherry-pick commit to branch-1.3 and removed branch-1.3 Automatically cherry-pick commit to branch-1.3 labels Aug 18, 2026

[apache#12488] docs(kms): Document named KMS provider configuration

4b94604

Operators need the server-config keys that bind each instance to an API. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 reviewed Aug 18, 2026

View reviewed changes

Comment thread common/src/testFixtures/java/org/apache/gravitino/encryption/kms/FakeKmsClient.java

lasdf1234 reviewed Aug 18, 2026

View reviewed changes

Comment thread docs/gravitino-server-config.md Outdated

nevzheng requested a review from lasdf1234 August 18, 2026 12:06

[apache#12488] feat(kms): Load provider factories by className

fbf7694

Follow the existing listener convention and drop spaces after commas in the providers list. Co-authored-by: Cursor <cursoragent@cursor.com>

nevzheng and others added 3 commits August 18, 2026 20:19

[apache#12488] style(kms): Apply Spotless to className registry changes

ec34881

Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#12488] fix(kms): Give FakeKmsClient commons-lang3 on the test…

8e8bf9c

…Fixtures classpath StringUtils.isBlank does not compile from common testFixtures without an explicit dependency. Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#12488] refactor(kms): Drop unused factory api() and KmsApiIde…

280c36e

…ntifiers The server loads factories by className. A protocol identifier is not part of that contract. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 approved these changes Aug 18, 2026

View reviewed changes

This comment was marked as spam.

Sign in to view

roryqi reviewed Aug 18, 2026

View reviewed changes

[apache#12488] docs(kms): Treat factory classNames as user implementa…

d06fa4a

…tions Example FQCNs looked like shipped AWS and Azure factories. Document the EventListener-style hook instead. Co-authored-by: Cursor <cursoragent@cursor.com>

nevzheng requested a review from roryqi August 19, 2026 01:44

roryqi approved these changes Aug 19, 2026

View reviewed changes

roryqi merged commit 6b81835 into apache:main Aug 19, 2026
38 checks passed

Navigation Menu

Uh oh!

[#12488] feat(kms): Identify keys by provider and keyId - #12489

[#12488] feat(kms): Identify keys by provider and keyId#12489
roryqi merged 7 commits into
apache:mainfrom
nevzheng:feat/kms-reference-provider-keyid

nevzheng commented Aug 18, 2026

Uh oh!

roryqi commented Aug 18, 2026

Uh oh!

Uh oh!

Uh oh!

lasdf1234 commented Aug 18, 2026

Uh oh!

nevzheng commented Aug 18, 2026 •

edited

Loading

Uh oh!

lasdf1234 commented Aug 18, 2026

Uh oh!

This comment was marked as spam.

Uh oh!

roryqi Aug 18, 2026

Uh oh!

nevzheng Aug 19, 2026

Uh oh!

roryqi Aug 19, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

roryqi left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

nevzheng commented Aug 18, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Related work

Uh oh!

roryqi commented Aug 18, 2026

Uh oh!

Uh oh!

Uh oh!

lasdf1234 commented Aug 18, 2026

Uh oh!

nevzheng commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lasdf1234 commented Aug 18, 2026

Uh oh!

This comment was marked as spam.

Uh oh!

roryqi Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

nevzheng Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

roryqi Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Report

Uh oh!

roryqi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nevzheng commented Aug 18, 2026 •

edited

Loading

github-actions Bot commented Aug 18, 2026 •

edited

Loading