[#12131] feat(kms): Define key inspection API by nevzheng · Pull Request #12132 · apache/gravitino

nevzheng · 2026-07-22T00:30:42Z

What changes were proposed in this pull request?

Add provider-neutral Java contracts for server-private KMS key inspection:

KmsReference identifies a key by canonical KMS API, configured source, and provider-native key ID;
KmsClient and KmsClientFactory define the provider extension contract in common;
KmsKeyProperties exposes normalized, point-in-time state for a located key;
an empty result is reserved for authoritative key-not-found, while indeterminate failures remain exceptions;
configuration and authentication failures remain distinct; and
reusable fake implementations and contract tests support independent providers.

The user-facing reference value remains in api. The developer-facing SPI, exceptions, and reusable provider test contracts live in common.

Why are the changes needed?

KMS integrations need a common contract that does not expose credentials or key material. Applications can identify and inspect keys without depending on provider SDK types, while providers remain independently loadable through canonical string identifiers.

This PR defines only the provider-neutral API/SPI. Named-source routing follows in #12133. Concrete providers, provider integration tests, server configuration, and cryptographic operations are outside this PR.

Part of #12131.

Does this PR introduce any user-facing change?

Yes. It adds new @DeveloperApi Java contracts. It does not add REST APIs, server configuration, concrete providers, or cryptographic operations.

How was this patch tested?

./gradlew :api:test :common:test
./gradlew :api:spotlessCheck :common:spotlessCheck
./gradlew :common:testFixturesJar
./gradlew :api:javadoc :common:javadoc

Series

Epic: [EPIC] Add provider-neutral KMS interfaces #12131
Position: 1 of 2
Previous: none
Next: [#12131] feat(kms): Configure and route named sources #12133
Full stack: [#12131] feat(kms): Define key inspection API #12132 → [#12131] feat(kms): Configure and route named sources #12133

nevzheng · 2026-07-22T00:32:17Z

@lasdf1234, could you also review this KMS API series? The full series is tracked in #12131.

nevzheng · 2026-07-22T00:33:08Z

Hi @roryqi and @lasdf1234 — please review this KMS API change. The full series and review scope are tracked in #12131.

github-actions · 2026-07-22T01:49:31Z

Code Coverage Report

Overall Project	67.78% `+0.01%`	🟢
Files changed	84.0%	🟢

Module	Coverage
aliyun	1.72%	🔴
api	48.25% `+0.43%`	🟢
authorization-common	85.96%	🟢
aws	42.04%	🟢
azure	2.47%	🔴
catalog-common	9.92%	🔴
catalog-fileset	79.74%	🟢
catalog-glue	66.91%	🟢
catalog-hive	79.4%	🟢
catalog-jdbc-common	45.7%	🟢
catalog-jdbc-doris	81.72%	🟢
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.93%	🟢
catalog-lakehouse-paimon	84.23%	🟢
catalog-model	77.72%	🟢
cli	44.5%	🟢
client-java	78.27%	🟢
common	51.95%	🟢
core	82.54%	🟢
filesystem-hadoop3	77.28%	🟢
flink	0.0%	🔴
flink-common	47.09%	🟢
flink-runtime	0.0%	🔴
gcp	14.12%	🔴
hadoop-auth	68.0%	🟢
hadoop-common	12.7%	🔴
hive-metastore-common	53.4%	🟢
iceberg-common	64.65%	🟢
iceberg-rest-server	74.91%	🟢
idp-basic	86.02%	🟢
integration-test-common	0.0%	🔴
jobs	66.17%	🟢
lance-common	20.67%	🔴
lance-rest-server	64.84%	🟢
lineage	53.02%	🟢
optimizer	83.24%	🟢
optimizer-api	21.95%	🔴
server	85.92%	🟢
server-common	76.12%	🟢
spark	28.57%	🔴
spark-common	46.01%	🟢
tencent	69.84%	🟢
trino-connector	40.29%	🟢

Files

Module	File	Coverage
api	KmsApi.java	100.0%	🟢
	KmsClient.java	100.0%	🟢
	KmsReference.java	100.0%	🟢
	KmsAuthenticationException.java	0.0%	🔴
	KmsClientFactory.java	0.0%	🔴
	KmsConfigurationException.java	0.0%	🔴
	KmsKeyProperties.java	0.0%	🔴

roryqi · 2026-07-22T06:11:52Z

+
+/** The KMS product API that defines key identifier and operation semantics. */
+@DeveloperApi
+public enum KmsApi {


If we want to use enum here and add a new KMS service, it will be difficult.
I prefer using SPI patten. Maybe you can refer to catalog provider and credential provider.
I feel that it would be better to use String.

Done in 10fb53a. The closed enum is removed: KmsReference.api() and KmsClientFactory.api() now use stable string identifiers, and references reject null/blank values and canonicalize by trimming and lowercasing. The identifier is only a registry key; it is never used as a class name or reflection input. API/common tests, JSON round-trip/canonicalization coverage, Spotless, test-fixture packaging, and Javadocs all pass.

roryqi · 2026-07-22T06:25:42Z

+ * <p>Capabilities describe downstream use; this API does not perform cryptographic operations.
+ */
+@DeveloperApi
+public interface KmsKeyProperties {


I don't know the concrete scenario? I feel a weird. Because it will common to use a capabilities class instead of properties class. You can refer to our CatalogCapabiities class.

The proposed CatalogCapability model is not applicable here. KmsKeyProperties describes a specific key’s presence, enabled state, and supported operations. These values vary by key; they are not capabilities of KmsClient.

Client- or provider-level capability discovery is a separate API. It is out of scope and can be added later when there is a concrete requirement.

Copilot

Pull request overview

This PR introduces provider-neutral Java contracts in the api module for server-side KMS key inspection/validation (API identification, key references, client/factory SPI, and normalized key properties), along with reusable test fixtures and baseline unit tests to support future provider implementations.

Changes:

Added KmsApi, KmsReference, KmsClient, KmsClientFactory, and KmsKeyProperties as @DeveloperApi contracts for key inspection.
Added distinct exception types for configuration vs authentication failures (KmsConfigurationException, KmsAuthenticationException).
Introduced test fixtures (contract tests + FakeKmsClient) and enabled Gradle test fixtures for the api module.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
api/src/main/java/org/apache/gravitino/encryption/kms/KmsApi.java	Defines supported KMS APIs and stable wire-value parsing.
api/src/main/java/org/apache/gravitino/encryption/kms/KmsAuthenticationException.java	Adds a KMS-specific authentication failure type layered on `ConnectionFailedException`.
api/src/main/java/org/apache/gravitino/encryption/kms/KmsClient.java	Introduces the provider-neutral key inspection client contract.
api/src/main/java/org/apache/gravitino/encryption/kms/KmsClientFactory.java	Defines the provider factory SPI for creating source-bound `KmsClient`s.
api/src/main/java/org/apache/gravitino/encryption/kms/KmsConfigurationException.java	Adds a KMS-specific configuration validation exception.
api/src/main/java/org/apache/gravitino/encryption/kms/KmsKeyProperties.java	Defines the normalized key property/capability reporting interface.
api/src/main/java/org/apache/gravitino/encryption/kms/KmsReference.java	Adds the typed key reference model (api/source/keyId) with value semantics.
api/src/test/java/org/apache/gravitino/encryption/kms/TestFakeKmsClient.java	Applies the shared client contract fixture to `FakeKmsClient`.
api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsApi.java	Unit tests for `KmsApi` wire parsing and error messaging.
api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClient.java	Unit tests for `KmsClient` behavior and default `close()`.
api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsReference.java	Unit tests for `KmsReference` validation and value semantics.
api/src/testFixtures/java/org/apache/gravitino/encryption/kms/FakeKmsClient.java	Provides an in-memory test fixture implementation of `KmsClient`.
api/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientContract.java	Shared contract tests for provider `KmsClient` implementations.
api/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientFactoryContract.java	Shared contract test for provider `KmsClientFactory` implementations.
api/build.gradle.kts	Enables `java-test-fixtures` and exposes JUnit to fixture consumers.

nevzheng · 2026-07-22T20:33:50Z

+/**
+ * Common properties reported for a KMS key.
+ *
+ * <p>Capabilities describe downstream use; this API does not perform cryptographic operations.
+ */


Addressed in cf4a8c9 by making presence structural: KmsClient.getKeyProperties now returns Optional<KmsKeyProperties>, and present() is removed. Optional.empty() is reserved for authoritative provider not-found, while authentication, authorization, timeout, availability, and other indeterminate failures remain exceptions. KmsKeyProperties therefore only describes a located key, and its Javadocs now define the lifecycle and structural-capability semantics.

nevzheng · 2026-07-22T20:33:56Z

+    Assertions.assertEquals(reference, properties.reference());
+    Assertions.assertFalse(properties.present());
+  }


Addressed in cf4a8c9. The reusable provider contract now requires a non-null Optional, asserts Optional.of for a found key, and asserts Optional.empty for authoritative not-found. The fake-client tests also cover a disabled-but-present key, so lifecycle state is independent from presence. API/common tests, Spotless, test-fixture packaging, and Javadocs pass.

jerryshao · 2026-07-22T06:52:27Z

+  public KmsAuthenticationException(@FormatString String message, Object... args) {
+    super(message, args);
+  }
+}


The API module is mainly the user-facing APIs. These interfaces are more developer-facing APIs. We should not put it here.

We can either put them in connector module. Or, we can put them in the place where we use it.

KMS should be used for server and client. Maybe we could consider putting it into common module, too.

Done in 391ec3d. api now retains only the user-facing KmsReference and KmsApi data contracts. KmsClient, KmsClientFactory, key properties, exceptions, the fake client, and shared contract-test fixtures now live in common. Validation passed with :api:test, :common:test, both modules’ Spotless checks, the common test-fixtures JAR, and API/common Javadocs.

lasdf1234 · 2026-07-22T10:39:20Z

@nevzheng All the java.class files are located in a single package path. Can different functions of java.class be placed in different packages?

nevzheng · 2026-07-22T18:13:19Z

Added canonical JSON persistence for KmsReference and KmsApi plus round-trip coverage. Persisted key references now retain their canonical API, source, and key ID. #12133 has been restacked on this head.

nevzheng · 2026-07-22T20:12:14Z

@lasdf1234 Done in 391ec3d. I separated the functions by Gradle module rather than creating several small Java subpackages: the user-facing reference contract remains in api, while the provider SPI, exceptions, and reusable test contracts moved to common. The Java package remains cohesive, following the existing credential precedent where Credential is in api and CredentialProvider is in common under the same org.apache.gravitino.credential package. Concrete providers can use provider-specific subpackages.

nevzheng · 2026-07-22T20:46:15Z

Thanks for the review feedback. I’ve completed the author-side revisions on the current PR:

Moved the provider SPI, exceptions, and reusable test contracts from api to common; api retains the public KmsReference value.
Replaced the closed KmsApi enum with canonical string identifiers so independently loaded providers can extend the SPI safely.
Clarified that KmsKeyProperties describes one inspected key, not client- or provider-level capabilities.
Changed key lookup to return Optional<KmsKeyProperties>: empty means authoritative not-found, while authentication, authorization, availability, and indeterminate failures remain exceptions.
Documented that lifecycle state is point-in-time and may apply to the selected/current key version; operation support does not guarantee authorization or future success.
Added contract coverage for canonical identifiers, found/missing/disabled keys, invalid provider results, and distinct failure types.

Local API/common tests, Spotless, test-fixture packaging, and Javadocs pass. GitHub CI currently has no failures; the remaining integration jobs are still running.

Current head: cf4a8c976

@roryqi @lasdf1234 — please take another look and either approve or identify any remaining concrete concerns.

feat(kms): define key inspection API

b675a5c

nevzheng requested a review from roryqi July 22, 2026 00:30

nevzheng mentioned this pull request Jul 22, 2026

[EPIC] Add provider-neutral KMS interfaces #12131

Open

2 tasks

nevzheng self-assigned this Jul 22, 2026

roryqi reviewed Jul 22, 2026

View reviewed changes

jerryshao requested a review from Copilot July 22, 2026 06:47

Copilot started reviewing on behalf of jerryshao July 22, 2026 06:47 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

jerryshao reviewed Jul 22, 2026

View reviewed changes

nevzheng mentioned this pull request Jul 22, 2026

[MOVED] fix(kms): Package providers in the server distribution #12144

Closed

fix(kms): persist key references with canonical API values

6f6424d

nevzheng requested review from lasdf1234 and roryqi July 22, 2026 18:56

refactor(kms): move provider SPI to common

391ec3d

nevzheng added 2 commits July 22, 2026 13:17

refactor(kms): use extensible API identifiers

10fb53a

refactor(kms): represent missing keys with Optional

cf4a8c9

nevzheng requested review from jerryshao, lasdf1234 and roryqi and removed request for lasdf1234 and roryqi July 22, 2026 20:46

Uh oh!

Conversation

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

Uh oh!

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?

Series

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Code Coverage Report

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

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!

roryqi Jul 22, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

lasdf1234 commented Jul 22, 2026

Uh oh!

nevzheng commented Jul 22, 2026

Uh oh!

nevzheng commented Jul 22, 2026

Uh oh!

nevzheng commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nevzheng commented Jul 22, 2026 •

edited

Loading

nevzheng commented Jul 22, 2026 •

edited

Loading

nevzheng commented Jul 22, 2026 •

edited

Loading

github-actions Bot commented Jul 22, 2026 •

edited

Loading

roryqi Jul 22, 2026 •

edited

Loading