This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 6b818355b1 [#12488] feat(kms): Identify keys by provider and keyId
(#12489)
6b818355b1 is described below
commit 6b818355b14be4ea67889a79a4062f0c70f36ea9
Author: Nevin Zheng <[email protected]>
AuthorDate: Wed Aug 19 11:48:20 2026 +0800
[#12488] feat(kms): Identify keys by provider and keyId (#12489)
### 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](https://cursor.com)
---------
Co-authored-by: Cursor <[email protected]>
---
.../encryption/kms/KmsApiIdentifiers.java | 57 ----
.../gravitino/encryption/kms/KmsReference.java | 50 +--
.../encryption/kms/TestKmsApiIdentifiers.java | 62 ----
.../gravitino/encryption/kms/TestKmsReference.java | 55 +---
common/build.gradle.kts | 1 +
.../dto/encryption/kms/KmsReferenceDTO.java | 14 +-
.../gravitino/encryption/kms/KmsClientFactory.java | 25 +-
.../dto/encryption/kms/TestKmsReferenceDTO.java | 48 +--
.../encryption/kms/TestFakeKmsClient.java | 55 +---
.../gravitino/encryption/kms/TestKmsClient.java | 2 +-
.../gravitino/encryption/kms/FakeKmsClient.java | 26 +-
.../encryption/kms/TestKmsClientContract.java | 15 +-
.../kms/TestKmsClientFactoryContract.java | 45 ---
.../encryption/kms/KmsClientRegistry.java | 132 ++++----
.../apache/gravitino/encryption/kms/KmsConfig.java | 108 +++----
.../TestGravitinoEnvKmsClientRegistry.java | 5 +-
.../encryption/kms/TestKmsClientRegistry.java | 355 +++++++++------------
.../gravitino/encryption/kms/TestKmsConfig.java | 110 ++++---
docs/gravitino-server-config.md | 39 +++
19 files changed, 440 insertions(+), 764 deletions(-)
diff --git
a/api/src/main/java/org/apache/gravitino/encryption/kms/KmsApiIdentifiers.java
b/api/src/main/java/org/apache/gravitino/encryption/kms/KmsApiIdentifiers.java
deleted file mode 100644
index 7b2f9b74b6..0000000000
---
a/api/src/main/java/org/apache/gravitino/encryption/kms/KmsApiIdentifiers.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.gravitino.encryption.kms;
-
-import com.google.common.base.Preconditions;
-import java.util.regex.Pattern;
-import org.apache.commons.lang3.StringUtils;
-import org.apache.gravitino.annotation.DeveloperApi;
-
-/**
- * Validation helpers for KMS API identifiers.
- *
- * <p>Identifiers are matched exactly. They must be lowercase kebab-case with
no surrounding
- * whitespace (for example {@code aws-kms}). Values are never normalized.
- */
-@DeveloperApi
-public final class KmsApiIdentifiers {
-
- private static final Pattern LOWERCASE_KEBAB_CASE =
Pattern.compile("^[a-z0-9]+(-[a-z0-9]+)*$");
-
- private KmsApiIdentifiers() {}
-
- /**
- * Validates a KMS API identifier.
- *
- * @param api the identifier to validate
- * @return the same identifier when valid
- * @throws IllegalArgumentException if {@code api} is null, blank, padded,
or not lowercase
- * kebab-case
- */
- public static String requireValid(String api) {
- Preconditions.checkArgument(StringUtils.isNotBlank(api), "KMS API cannot
be blank");
- Preconditions.checkArgument(
- api.equals(api.trim()), "KMS API cannot have leading or trailing
whitespace");
- Preconditions.checkArgument(
- LOWERCASE_KEBAB_CASE.matcher(api).matches(),
- "KMS API must be lowercase kebab-case: '%s'",
- api);
- return api;
- }
-}
diff --git
a/api/src/main/java/org/apache/gravitino/encryption/kms/KmsReference.java
b/api/src/main/java/org/apache/gravitino/encryption/kms/KmsReference.java
index e68c0c7e00..8adae1d1ad 100644
--- a/api/src/main/java/org/apache/gravitino/encryption/kms/KmsReference.java
+++ b/api/src/main/java/org/apache/gravitino/encryption/kms/KmsReference.java
@@ -25,56 +25,40 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.annotation.DeveloperApi;
/**
- * Identifies a key owned by a configured KMS source.
+ * Identifies a key owned by a named KMS provider.
*
- * <p>Contains no credentials or key material.
- *
- * <p>KMS API identifiers use exact string matching. Callers must supply
lowercase kebab-case values
- * with no leading or trailing whitespace (for example {@code aws-kms}). This
type does not
- * normalize the identifier.
+ * <p>Contains no credentials or key material. The provider name is the
configured instance handle
+ * ({@code gravitino.kms.providers}). The server loads that name's factory
from {@code
+ * gravitino.kms.provider.<name>.className} and resolves a client from {@code
KmsClientRegistry}.
*/
@DeveloperApi
public final class KmsReference {
- private final String api;
- private final String source;
+ private final String provider;
private final String keyId;
/**
* Creates a structurally valid key reference without contacting the
provider.
*
- * @param api explicitly selected KMS API identifier; must be lowercase
kebab-case with no
- * surrounding whitespace
- * @param source configured KMS client-instance name
+ * @param provider configured KMS provider name
* @param keyId provider-native key identifier
- * @throws IllegalArgumentException if any argument is null or blank, or if
{@code api} is not a
- * valid KMS API identifier
+ * @throws IllegalArgumentException if either argument is null or blank
*/
- public KmsReference(String api, String source, String keyId) {
- this.api = KmsApiIdentifiers.requireValid(api);
- Preconditions.checkArgument(StringUtils.isNotBlank(source), "KMS source
cannot be blank");
+ public KmsReference(String provider, String keyId) {
+ Preconditions.checkArgument(StringUtils.isNotBlank(provider), "KMS
provider cannot be blank");
Preconditions.checkArgument(StringUtils.isNotBlank(keyId), "KMS key ID
cannot be blank");
- this.source = source.trim();
+ this.provider = provider.trim();
this.keyId = keyId;
}
/**
- * Returns the explicitly selected KMS API identifier.
- *
- * @return the exact KMS API identifier
- */
- public String api() {
- return api;
- }
-
- /**
- * Returns the configured KMS client-instance name.
+ * Returns the configured KMS provider name.
*
- * @return the source name
+ * @return the provider name
*/
- public String source() {
- return source;
+ public String provider() {
+ return provider;
}
/**
@@ -95,16 +79,16 @@ public final class KmsReference {
return false;
}
KmsReference that = (KmsReference) other;
- return api.equals(that.api) && source.equals(that.source) &&
keyId.equals(that.keyId);
+ return provider.equals(that.provider) && keyId.equals(that.keyId);
}
@Override
public int hashCode() {
- return Objects.hash(api, source, keyId);
+ return Objects.hash(provider, keyId);
}
@Override
public String toString() {
- return String.format("KmsReference{api='%s', source='%s', keyId='%s'}",
api, source, keyId);
+ return String.format("KmsReference{provider='%s', keyId='%s'}", provider,
keyId);
}
}
diff --git
a/api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsApiIdentifiers.java
b/api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsApiIdentifiers.java
deleted file mode 100644
index 2743451393..0000000000
---
a/api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsApiIdentifiers.java
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.gravitino.encryption.kms;
-
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-
-public class TestKmsApiIdentifiers {
-
- @Test
- void testAcceptsLowercaseKebabCase() {
- Assertions.assertEquals("aws-kms",
KmsApiIdentifiers.requireValid("aws-kms"));
- Assertions.assertEquals("test", KmsApiIdentifiers.requireValid("test"));
- Assertions.assertEquals("acme-kms-v2",
KmsApiIdentifiers.requireValid("acme-kms-v2"));
- }
-
- @Test
- void testRejectsBlankAndPaddedValues() {
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid(null));
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid(""));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> KmsApiIdentifiers.requireValid("
"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> KmsApiIdentifiers.requireValid("
aws-kms"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid("aws-kms "));
- }
-
- @Test
- void testRejectsNonKebabCaseValues() {
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid("AWS-KMS"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid("aws_kms"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid("aws kms"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid("aws--kms"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid("-aws-kms"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
KmsApiIdentifiers.requireValid("aws-kms-"));
- }
-}
diff --git
a/api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsReference.java
b/api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsReference.java
index 9c658b9c9b..071144fce1 100644
---
a/api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsReference.java
+++
b/api/src/test/java/org/apache/gravitino/encryption/kms/TestKmsReference.java
@@ -24,64 +24,39 @@ import org.junit.jupiter.api.Test;
public class TestKmsReference {
@Test
- void testStoresExactApiAndPreservesProviderKey() {
- KmsReference reference = new KmsReference("aws-kms", " production ", "
alias/Customer-Key ");
+ void testTrimsProviderAndPreservesKeyId() {
+ KmsReference reference = new KmsReference(" production ", "
alias/Customer-Key ");
- Assertions.assertEquals("aws-kms", reference.api());
- Assertions.assertEquals("production", reference.source());
+ Assertions.assertEquals("production", reference.provider());
Assertions.assertEquals(" alias/Customer-Key ", reference.keyId());
}
@Test
void testRejectsMissingFields() {
+ Assertions.assertThrows(IllegalArgumentException.class, () -> new
KmsReference(null, "key"));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> new
KmsReference("", "key"));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> new
KmsReference(" ", "key"));
Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference(null,
"production", "key"));
+ IllegalArgumentException.class, () -> new KmsReference("production",
null));
Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("",
"production", "key"));
+ IllegalArgumentException.class, () -> new KmsReference("production",
""));
Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference(" ",
"production", "key"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws-kms",
null, "key"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws-kms",
"production", null));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws-kms", "",
"key"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws-kms", " ",
"key"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws-kms",
"production", ""));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws-kms",
"production", " "));
- }
-
- @Test
- void testRejectsInvalidApiIdentifiers() {
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference(" aws-kms",
"production", "key"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws-kms ",
"production", "key"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("AWS-KMS",
"production", "key"));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsReference("aws_kms",
"production", "key"));
+ IllegalArgumentException.class, () -> new KmsReference("production", "
"));
}
@Test
void testValueSemantics() {
- KmsReference first = new KmsReference("aws-kms", "production", "key");
- KmsReference same = new KmsReference("aws-kms", "production", "key");
- KmsReference differentApi = new KmsReference("google-cloud-kms",
"production", "key");
- KmsReference differentSource = new KmsReference("aws-kms", "recovery",
"key");
- KmsReference differentKey = new KmsReference("aws-kms", "production",
"another-key");
+ KmsReference first = new KmsReference("production", "key");
+ KmsReference same = new KmsReference("production", "key");
+ KmsReference differentProvider = new KmsReference("recovery", "key");
+ KmsReference differentKey = new KmsReference("production", "another-key");
Assertions.assertEquals(first, same);
Assertions.assertEquals(first.hashCode(), same.hashCode());
- Assertions.assertNotEquals(first, differentApi);
- Assertions.assertNotEquals(first, differentSource);
+ Assertions.assertNotEquals(first, differentProvider);
Assertions.assertNotEquals(first, differentKey);
Assertions.assertNotEquals(first, null);
Assertions.assertNotEquals(first, "key");
- Assertions.assertEquals(
- "KmsReference{api='aws-kms', source='production', keyId='key'}",
first.toString());
+ Assertions.assertEquals("KmsReference{provider='production',
keyId='key'}", first.toString());
}
}
diff --git a/common/build.gradle.kts b/common/build.gradle.kts
index 3f20efd17a..09d549183b 100644
--- a/common/build.gradle.kts
+++ b/common/build.gradle.kts
@@ -54,6 +54,7 @@ dependencies {
testFixturesApi(project(":api"))
testFixturesApi(libs.junit.jupiter.api)
+ testFixturesImplementation(libs.commons.lang3)
}
fun getGitCommitId(): String {
diff --git
a/common/src/main/java/org/apache/gravitino/dto/encryption/kms/KmsReferenceDTO.java
b/common/src/main/java/org/apache/gravitino/dto/encryption/kms/KmsReferenceDTO.java
index 3e9f501a23..ac4fb88e6d 100644
---
a/common/src/main/java/org/apache/gravitino/dto/encryption/kms/KmsReferenceDTO.java
+++
b/common/src/main/java/org/apache/gravitino/dto/encryption/kms/KmsReferenceDTO.java
@@ -35,11 +35,8 @@ import org.apache.gravitino.encryption.kms.KmsReference;
@Builder(setterPrefix = "with")
public class KmsReferenceDTO {
- @JsonProperty("api")
- private String api;
-
- @JsonProperty("source")
- private String source;
+ @JsonProperty("provider")
+ private String provider;
@JsonProperty("keyId")
private String keyId;
@@ -47,13 +44,10 @@ public class KmsReferenceDTO {
/**
* Converts this DTO to a {@link KmsReference}.
*
- * <p>{@code api} is validated by {@link KmsReference}: it must already be
lowercase kebab-case
- * with no surrounding whitespace and is matched exactly.
- *
* @return the KMS key reference
*/
public KmsReference toKmsReference() {
- return new KmsReference(api, source, keyId);
+ return new KmsReference(provider, keyId);
}
/**
@@ -63,6 +57,6 @@ public class KmsReferenceDTO {
* @return the KMS key reference DTO
*/
public static KmsReferenceDTO fromKmsReference(KmsReference reference) {
- return new KmsReferenceDTO(reference.api(), reference.source(),
reference.keyId());
+ return new KmsReferenceDTO(reference.provider(), reference.keyId());
}
}
diff --git
a/common/src/main/java/org/apache/gravitino/encryption/kms/KmsClientFactory.java
b/common/src/main/java/org/apache/gravitino/encryption/kms/KmsClientFactory.java
index c72ea2a89a..c290edfe87 100644
---
a/common/src/main/java/org/apache/gravitino/encryption/kms/KmsClientFactory.java
+++
b/common/src/main/java/org/apache/gravitino/encryption/kms/KmsClientFactory.java
@@ -21,22 +21,17 @@ package org.apache.gravitino.encryption.kms;
import java.util.Map;
import org.apache.gravitino.annotation.DeveloperApi;
-/** Creates server-side KMS clients for one KMS API. */
+/**
+ * Creates a server-side KMS client for one configured provider.
+ *
+ * <p>The server loads an implementation from {@code
gravitino.kms.provider.<name>.className}. The
+ * class must have a public no-arg constructor.
+ */
@DeveloperApi
public interface KmsClientFactory {
/**
- * Returns the exact KMS API identifier implemented by this factory.
- *
- * <p>Identifiers use lowercase kebab-case with no surrounding whitespace
({@link
- * KmsApiIdentifiers}) and are matched exactly against {@link
KmsReference#api()}.
- *
- * @return the KMS API identifier
- */
- String api();
-
- /**
- * Creates a client bound to a configured KMS source.
+ * Creates a client bound to a configured KMS provider.
*
* <p>Provider credentials are private implementation details of the
returned client. They must
* not be exposed as Gravitino credentials or key properties. The caller
owns the returned client
@@ -44,10 +39,10 @@ public interface KmsClientFactory {
* contacting the configured KMS; network and authentication failures are
reported by client
* operations.
*
- * @param source logical name of the configured KMS instance
+ * @param provider logical name of the configured KMS instance
* @param properties provider-specific configuration
* @return the configured client
- * @throws IllegalArgumentException if the source or configuration is invalid
+ * @throws IllegalArgumentException if the provider or configuration is
invalid
*/
- KmsClient create(String source, Map<String, String> properties);
+ KmsClient create(String provider, Map<String, String> properties);
}
diff --git
a/common/src/test/java/org/apache/gravitino/dto/encryption/kms/TestKmsReferenceDTO.java
b/common/src/test/java/org/apache/gravitino/dto/encryption/kms/TestKmsReferenceDTO.java
index a7353f45ed..1b0f10eeee 100644
---
a/common/src/test/java/org/apache/gravitino/dto/encryption/kms/TestKmsReferenceDTO.java
+++
b/common/src/test/java/org/apache/gravitino/dto/encryption/kms/TestKmsReferenceDTO.java
@@ -30,9 +30,8 @@ public class TestKmsReferenceDTO {
private final ObjectMapper objectMapper = JsonUtils.objectMapper();
@Test
- void testRoundTripPreservesExactApi() throws JsonProcessingException {
- KmsReference reference =
- new KmsReference("google-cloud-kms", "analytics-prod",
"projects/p/keys/k");
+ void testRoundTripPreservesProviderAndKeyId() throws JsonProcessingException
{
+ KmsReference reference = new KmsReference("analytics-prod",
"projects/p/keys/k");
KmsReferenceDTO dto = KmsReferenceDTO.fromKmsReference(reference);
String json = objectMapper.writeValueAsString(dto);
@@ -40,52 +39,23 @@ public class TestKmsReferenceDTO {
Assertions.assertEquals(reference, restored);
Assertions.assertEquals(
- objectMapper.readTree(
- "{\"api\":\"google-cloud-kms\",\"source\":\"analytics-prod\","
- + "\"keyId\":\"projects/p/keys/k\"}"),
+
objectMapper.readTree("{\"provider\":\"analytics-prod\",\"keyId\":\"projects/p/keys/k\"}"),
objectMapper.readTree(json));
}
@Test
- void testAcceptsCustomApiIdentifier() throws JsonProcessingException {
- KmsReference reference = new KmsReference("acme-kms-v2", "team-a",
"keys/primary");
- KmsReferenceDTO dto = KmsReferenceDTO.fromKmsReference(reference);
-
- String json = objectMapper.writeValueAsString(dto);
- KmsReference restored = objectMapper.readValue(json,
KmsReferenceDTO.class).toKmsReference();
-
- Assertions.assertEquals("acme-kms-v2", restored.api());
- Assertions.assertEquals(reference, restored);
- }
-
- @Test
- void testToKmsReferenceRejectsPaddedApi() {
+ void testToKmsReferenceRejectsBlankProvider() {
KmsReferenceDTO dto =
- KmsReferenceDTO.builder()
- .withApi(" google-cloud-kms ")
- .withSource("analytics-prod")
- .withKeyId("projects/p/keys/k")
- .build();
+ KmsReferenceDTO.builder().withProvider("
").withKeyId("projects/p/keys/k").build();
Assertions.assertThrows(IllegalArgumentException.class,
dto::toKmsReference);
}
@Test
- void testToKmsReferenceRejectsInvalidApiFormat() {
- KmsReferenceDTO uppercase =
- KmsReferenceDTO.builder()
- .withApi("AWS-KMS")
- .withSource("production")
- .withKeyId("key")
- .build();
- KmsReferenceDTO snakeCase =
- KmsReferenceDTO.builder()
- .withApi("aws_kms")
- .withSource("production")
- .withKeyId("key")
- .build();
+ void testToKmsReferenceRejectsBlankKeyId() {
+ KmsReferenceDTO dto =
+ KmsReferenceDTO.builder().withProvider("analytics-prod").withKeyId("
").build();
- Assertions.assertThrows(IllegalArgumentException.class,
uppercase::toKmsReference);
- Assertions.assertThrows(IllegalArgumentException.class,
snakeCase::toKmsReference);
+ Assertions.assertThrows(IllegalArgumentException.class,
dto::toKmsReference);
}
}
diff --git
a/common/src/test/java/org/apache/gravitino/encryption/kms/TestFakeKmsClient.java
b/common/src/test/java/org/apache/gravitino/encryption/kms/TestFakeKmsClient.java
index 99683410ac..224e4e7bf4 100644
---
a/common/src/test/java/org/apache/gravitino/encryption/kms/TestFakeKmsClient.java
+++
b/common/src/test/java/org/apache/gravitino/encryption/kms/TestFakeKmsClient.java
@@ -23,66 +23,35 @@ import org.junit.jupiter.api.Test;
public class TestFakeKmsClient extends TestKmsClientContract {
- private static final String API = "test-kms";
- private static final String SOURCE = "test";
+ private static final String PROVIDER = "test";
private static final String USABLE_KEY = "usable";
private static final String DISABLED_KEY = "disabled";
private static final String MISSING_KEY = "missing";
private final FakeKmsClient client =
- new FakeKmsClient(API, SOURCE)
+ new FakeKmsClient(PROVIDER)
.putKey(USABLE_KEY, true, true, true)
.putKey(DISABLED_KEY, false, true, true);
@Test
- void testRejectsBlankApi() {
- Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(null, SOURCE));
- Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient("", SOURCE));
- Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(" ", SOURCE));
+ void testRejectsBlankProvider() {
+ Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(null));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(""));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(" "));
}
@Test
- void testRejectsPaddedApi() {
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new FakeKmsClient(" " + API,
SOURCE));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new FakeKmsClient(API + " ",
SOURCE));
- }
-
- @Test
- void testRejectsInvalidApiFormat() {
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new FakeKmsClient("TEST-KMS",
SOURCE));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new FakeKmsClient("test_kms",
SOURCE));
- }
-
- @Test
- void testMatchesApiExactly() {
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> client.getKeyProperties(new KmsReference("other-kms", SOURCE,
USABLE_KEY)));
- }
-
- @Test
- void testRejectsBlankSource() {
- Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(API, null));
- Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(API, ""));
- Assertions.assertThrows(IllegalArgumentException.class, () -> new
FakeKmsClient(API, " "));
- }
-
- @Test
- void testNormalizesSource() {
- FakeKmsClient paddedSourceClient = new FakeKmsClient(API, " " + SOURCE + "
");
+ void testNormalizesProvider() {
+ FakeKmsClient paddedProviderClient = new FakeKmsClient(" " + PROVIDER + "
");
Assertions.assertDoesNotThrow(
- () -> paddedSourceClient.getKeyProperties(new KmsReference(API,
SOURCE, MISSING_KEY)));
+ () -> paddedProviderClient.getKeyProperties(new KmsReference(PROVIDER,
MISSING_KEY)));
}
@Test
void testReportsDisabledKeyAsPresent() {
KmsKeyProperties properties =
- client.getKeyProperties(new KmsReference(API, SOURCE,
DISABLED_KEY)).orElseThrow();
+ client.getKeyProperties(new KmsReference(PROVIDER,
DISABLED_KEY)).orElseThrow();
Assertions.assertFalse(properties.enabled());
}
@@ -94,11 +63,11 @@ public class TestFakeKmsClient extends
TestKmsClientContract {
@Override
protected KmsReference usableKey() {
- return new KmsReference(API, SOURCE, USABLE_KEY);
+ return new KmsReference(PROVIDER, USABLE_KEY);
}
@Override
protected KmsReference missingKey() {
- return new KmsReference(API, SOURCE, MISSING_KEY);
+ return new KmsReference(PROVIDER, MISSING_KEY);
}
}
diff --git
a/common/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClient.java
b/common/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClient.java
index 9c2b6b4c08..65f1c6cefa 100644
---
a/common/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClient.java
+++
b/common/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClient.java
@@ -25,7 +25,7 @@ import org.junit.jupiter.api.Test;
public class TestKmsClient {
- private static final KmsReference REFERENCE = new KmsReference("test-kms",
"production", "key");
+ private static final KmsReference REFERENCE = new KmsReference("production",
"key");
@Test
void testReturnsProviderProperties() {
diff --git
a/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/FakeKmsClient.java
b/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/FakeKmsClient.java
index 591c13f203..eb1f31ff6d 100644
---
a/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/FakeKmsClient.java
+++
b/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/FakeKmsClient.java
@@ -21,27 +21,24 @@ package org.apache.gravitino.encryption.kms;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
+import org.apache.commons.lang3.StringUtils;
/** In-memory KMS client for contract and consumer tests. */
public final class FakeKmsClient implements KmsClient {
- private final String api;
- private final String source;
+ private final String provider;
private final Map<String, KeyState> keys = new HashMap<>();
/**
* Creates an empty fake client.
*
- * @param api exact KMS API identifier accepted by the client; lowercase
kebab-case with no
- * surrounding whitespace
- * @param source configured source accepted by the client
+ * @param provider configured provider accepted by the client
*/
- public FakeKmsClient(String api, String source) {
- this.api = KmsApiIdentifiers.requireValid(api);
- if (source == null || source.trim().isEmpty()) {
- throw new IllegalArgumentException("KMS source cannot be blank");
+ public FakeKmsClient(String provider) {
+ if (StringUtils.isBlank(provider)) {
+ throw new IllegalArgumentException("KMS provider cannot be blank");
}
- this.source = source.trim();
+ this.provider = provider.trim();
}
/**
@@ -75,14 +72,11 @@ public final class FakeKmsClient implements KmsClient {
if (reference == null) {
throw new IllegalArgumentException("KMS reference cannot be null");
}
- if (!reference.api().equals(api)) {
- throw new IllegalArgumentException(
- String.format("Expected KMS API '%s' but received '%s'", api,
reference.api()));
- }
- if (!source.equals(reference.source())) {
+ if (!provider.equals(reference.provider())) {
throw new IllegalArgumentException(
String.format(
- "KMS source %s does not match configured source %s",
reference.source(), source));
+ "KMS provider %s does not match configured provider %s",
+ reference.provider(), provider));
}
}
diff --git
a/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientContract.java
b/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientContract.java
index 868d64bd62..9b4b55ec67 100644
---
a/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientContract.java
+++
b/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientContract.java
@@ -74,20 +74,9 @@ public abstract class TestKmsClientContract {
}
@Test
- void testRejectsMismatchedSource() {
+ void testRejectsMismatchedProvider() {
KmsReference reference = usableKey();
- KmsReference mismatched =
- new KmsReference(reference.api(), reference.source() + "-other",
reference.keyId());
-
- Assertions.assertThrows(
- IllegalArgumentException.class, () ->
client().getKeyProperties(mismatched));
- }
-
- @Test
- void testRejectsMismatchedApi() {
- KmsReference reference = usableKey();
- KmsReference mismatched =
- new KmsReference(reference.api() + "-other", reference.source(),
reference.keyId());
+ KmsReference mismatched = new KmsReference(reference.provider() +
"-other", reference.keyId());
Assertions.assertThrows(
IllegalArgumentException.class, () ->
client().getKeyProperties(mismatched));
diff --git
a/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientFactoryContract.java
b/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientFactoryContract.java
deleted file mode 100644
index 52eec8f5dc..0000000000
---
a/common/src/testFixtures/java/org/apache/gravitino/encryption/kms/TestKmsClientFactoryContract.java
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.gravitino.encryption.kms;
-
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-
-/** Common contract for KMS client factories. */
-public abstract class TestKmsClientFactoryContract {
-
- /**
- * Returns the factory under test.
- *
- * @return the factory
- */
- protected abstract KmsClientFactory factory();
-
- /**
- * Returns the API expected from the factory.
- *
- * @return the expected API identifier
- */
- protected abstract String expectedApi();
-
- @Test
- void testReportsExpectedApi() {
- Assertions.assertEquals(expectedApi(), factory().api());
- }
-}
diff --git
a/core/src/main/java/org/apache/gravitino/encryption/kms/KmsClientRegistry.java
b/core/src/main/java/org/apache/gravitino/encryption/kms/KmsClientRegistry.java
index c7a687fe53..c8f9a585c8 100644
---
a/core/src/main/java/org/apache/gravitino/encryption/kms/KmsClientRegistry.java
+++
b/core/src/main/java/org/apache/gravitino/encryption/kms/KmsClientRegistry.java
@@ -23,55 +23,54 @@ import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.ServiceLoader;
import org.apache.gravitino.Config;
-/** Creates, resolves, and owns server-private KMS clients by configured
source. */
+/** Creates, resolves, and owns server-private KMS clients by configured
provider. */
public final class KmsClientRegistry implements AutoCloseable {
- private final Map<String, ConfiguredClient> clients;
+ private final Map<String, KmsClient> clients;
private volatile boolean closed;
/**
- * Loads configuration and available {@link KmsClientFactory}
implementations, then creates one
- * client for each configured source.
+ * Loads configuration and creates one client for each configured provider
by instantiating {@code
+ * gravitino.kms.provider.<name>.className}.
*
* @param config Gravitino server configuration
- * @throws IllegalArgumentException if configuration or factory discovery is
invalid
+ * @throws IllegalArgumentException if configuration or factory construction
is invalid
*/
public KmsClientRegistry(Config config) {
- this(config, loadFactories());
+ this(config, KmsClientRegistry::loadFactory);
}
- KmsClientRegistry(Config config, Iterable<KmsClientFactory> factories) {
+ KmsClientRegistry(Config config, FactoryLoader loader) {
KmsConfig kmsConfig = new KmsConfig(config);
- if (kmsConfig.sources().isEmpty()) {
+ if (kmsConfig.providers().isEmpty()) {
this.clients = Collections.emptyMap();
return;
}
- if (factories == null) {
- throw new IllegalArgumentException("KMS client factories cannot be
null");
+ if (loader == null) {
+ throw new IllegalArgumentException("KMS client factory loader cannot be
null");
}
- Map<String, KmsClientFactory> factoriesByApi = indexFactories(factories);
- this.clients = createClients(kmsConfig.sources(), factoriesByApi);
+ this.clients = createClients(kmsConfig.providers(), loader);
}
/**
* Resolves the client configured for a key reference.
*
* <p>The registry owns the returned client. Callers must not close it or
use it after the
- * registry is closed.
+ * registry is closed. Lookup is by {@link KmsReference#provider()} only;
the provider's factory
+ * was loaded at startup from {@code
gravitino.kms.provider.<name>.className}.
*
- * @param reference key whose source and API select the client
+ * @param reference key whose provider name selects the client
* @return client configured for the reference
- * @throws IllegalArgumentException if the source is unknown or configured
for another API
+ * @throws IllegalArgumentException if the provider is unknown
* @throws IllegalStateException if the registry is closed
*/
public KmsClient getClient(KmsReference reference) {
checkOpen();
- return resolveClient(reference).client;
+ return resolveClient(reference);
}
/** Closes all configured clients. This operation is idempotent. */
@@ -87,52 +86,55 @@ public final class KmsClientRegistry implements
AutoCloseable {
}
}
- private static Map<String, KmsClientFactory> indexFactories(
- Iterable<KmsClientFactory> factories) {
- Map<String, KmsClientFactory> factoriesByApi = new LinkedHashMap<>();
- for (KmsClientFactory factory : factories) {
- if (factory == null) {
- throw new IllegalArgumentException("KMS client factory cannot be
null");
- }
- String api = KmsApiIdentifiers.requireValid(factory.api());
- KmsClientFactory existing = factoriesByApi.putIfAbsent(api, factory);
- if (existing != null) {
- throw new IllegalArgumentException(
- String.format("Multiple KMS client factories support API '%s'",
api));
- }
- }
- return factoriesByApi;
+ /** Loads a {@link KmsClientFactory} from a configured class name. */
+ @FunctionalInterface
+ interface FactoryLoader {
+ /**
+ * Instantiates the factory named by {@code className}.
+ *
+ * @param className factory class name
+ * @return the factory
+ */
+ KmsClientFactory load(String className);
}
- private static Iterable<KmsClientFactory> loadFactories() {
- ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
- if (classLoader == null) {
- classLoader = KmsClientRegistry.class.getClassLoader();
+ private static KmsClientFactory loadFactory(String className) {
+ try {
+ Object instance =
Class.forName(className).getDeclaredConstructor().newInstance();
+ if (!(instance instanceof KmsClientFactory)) {
+ throw new IllegalArgumentException(
+ String.format("KMS factory class '%s' does not implement
KmsClientFactory", className));
+ }
+ return (KmsClientFactory) instance;
+ } catch (ClassNotFoundException e) {
+ throw new IllegalArgumentException(
+ String.format("No KMS client factory class '%s'", className), e);
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalArgumentException(
+ String.format("Failed to create KMS client factory '%s'",
className), e);
}
- return ServiceLoader.load(KmsClientFactory.class, classLoader);
}
- private static Map<String, ConfiguredClient> createClients(
- Map<String, KmsConfig.SourceConfig> sourceConfigs,
- Map<String, KmsClientFactory> factoriesByApi) {
- Map<String, ConfiguredClient> clients = new LinkedHashMap<>();
+ private static Map<String, KmsClient> createClients(
+ Map<String, KmsConfig.ProviderConfig> providerConfigs, FactoryLoader
loader) {
+ Map<String, KmsClient> clients = new LinkedHashMap<>();
try {
- sourceConfigs.forEach(
- (source, sourceConfig) -> {
- KmsClientFactory factory = factoriesByApi.get(sourceConfig.api());
+ providerConfigs.forEach(
+ (provider, providerConfig) -> {
+ KmsClientFactory factory = loader.load(providerConfig.className());
if (factory == null) {
- throw new IllegalArgumentException(
+ throw new IllegalStateException(
String.format(
- "No KMS client factory supports API '%s' for source
'%s'",
- sourceConfig.api(), source));
+ "KMS client factory '%s' returned null",
providerConfig.className()));
}
- KmsClient client = factory.create(source,
sourceConfig.properties());
+ KmsClient client = factory.create(provider,
providerConfig.properties());
if (client == null) {
throw new IllegalStateException(
String.format(
- "KMS client factory for API '%s' returned null",
sourceConfig.api()));
+ "KMS client factory '%s' returned a null client",
+ providerConfig.className()));
}
- clients.put(source, new ConfiguredClient(sourceConfig.api(),
client));
+ clients.put(provider, client);
});
return Collections.unmodifiableMap(clients);
} catch (RuntimeException | Error e) {
@@ -144,11 +146,11 @@ public final class KmsClientRegistry implements
AutoCloseable {
}
}
- private static RuntimeException closeClients(List<ConfiguredClient> clients)
{
+ private static RuntimeException closeClients(List<KmsClient> clients) {
RuntimeException failure = null;
for (int index = clients.size() - 1; index >= 0; index--) {
try {
- clients.get(index).client.close();
+ clients.get(index).close();
} catch (RuntimeException e) {
if (failure == null) {
failure = e;
@@ -160,23 +162,17 @@ public final class KmsClientRegistry implements
AutoCloseable {
return failure;
}
- private ConfiguredClient resolveClient(KmsReference reference) {
+ private KmsClient resolveClient(KmsReference reference) {
if (reference == null) {
throw new IllegalArgumentException("KMS reference cannot be null");
}
- ConfiguredClient configuredClient = clients.get(reference.source());
- if (configuredClient == null) {
- throw new IllegalArgumentException(
- String.format("No KMS client is configured for source '%s'",
reference.source()));
- }
- if (!configuredClient.api.equals(reference.api())) {
+ KmsClient client = clients.get(reference.provider());
+ if (client == null) {
throw new IllegalArgumentException(
- String.format(
- "KMS source '%s' uses API '%s', not '%s'",
- reference.source(), configuredClient.api, reference.api()));
+ String.format("No KMS client is configured for provider '%s'",
reference.provider()));
}
- return configuredClient;
+ return client;
}
private void checkOpen() {
@@ -184,14 +180,4 @@ public final class KmsClientRegistry implements
AutoCloseable {
throw new IllegalStateException("KMS client registry is closed");
}
}
-
- private static final class ConfiguredClient {
- private final String api;
- private final KmsClient client;
-
- private ConfiguredClient(String api, KmsClient client) {
- this.api = api;
- this.client = client;
- }
- }
}
diff --git
a/core/src/main/java/org/apache/gravitino/encryption/kms/KmsConfig.java
b/core/src/main/java/org/apache/gravitino/encryption/kms/KmsConfig.java
index 24b46f26d7..63e65da4fb 100644
--- a/core/src/main/java/org/apache/gravitino/encryption/kms/KmsConfig.java
+++ b/core/src/main/java/org/apache/gravitino/encryption/kms/KmsConfig.java
@@ -31,14 +31,14 @@ import org.apache.gravitino.Config;
final class KmsConfig {
static final String KMS_CONFIG_PREFIX = "gravitino.kms.";
- static final String KMS_SOURCES = KMS_CONFIG_PREFIX + "sources";
+ static final String KMS_PROVIDERS = KMS_CONFIG_PREFIX + "providers";
- private static final String SOURCES = "sources";
- private static final String SOURCE_PREFIX = "source.";
- private static final String API = "api";
- private static final Pattern SOURCE_NAME_PATTERN =
Pattern.compile("[A-Za-z0-9][A-Za-z0-9_-]*");
+ private static final String PROVIDERS = "providers";
+ private static final String PROVIDER_PREFIX = "provider.";
+ private static final String CLASS_NAME = "className";
+ private static final Pattern PROVIDER_NAME_PATTERN =
Pattern.compile("[A-Za-z0-9][A-Za-z0-9_-]*");
- private final Map<String, SourceConfig> sources;
+ private final Map<String, ProviderConfig> providers;
KmsConfig(Config config) {
if (config == null) {
@@ -46,94 +46,88 @@ final class KmsConfig {
}
Map<String, String> values =
config.getConfigsWithPrefix(KMS_CONFIG_PREFIX);
- List<String> configuredSources = parseSources(values.get(SOURCES));
- this.sources = parseSourceConfigs(values, configuredSources);
+ List<String> configuredProviders = parseProviders(values.get(PROVIDERS));
+ this.providers = parseProviderConfigs(values, configuredProviders);
}
- Map<String, SourceConfig> sources() {
- return sources;
+ Map<String, ProviderConfig> providers() {
+ return providers;
}
- private static List<String> parseSources(String value) {
+ private static List<String> parseProviders(String value) {
if (value == null || value.trim().isEmpty()) {
return Collections.emptyList();
}
- List<String> sources = new ArrayList<>();
- Set<String> uniqueSources = new LinkedHashSet<>();
+ List<String> providers = new ArrayList<>();
+ Set<String> uniqueProviders = new LinkedHashSet<>();
for (String item : value.split(",", -1)) {
- String source = item.trim();
- if (!SOURCE_NAME_PATTERN.matcher(source).matches()) {
+ String provider = item.trim();
+ if (!PROVIDER_NAME_PATTERN.matcher(provider).matches()) {
throw new KmsConfigurationException(
- "Invalid KMS source name '%s' in %s", source, KMS_SOURCES);
+ "Invalid KMS provider name '%s' in %s", provider, KMS_PROVIDERS);
}
- if (!uniqueSources.add(source)) {
- throw new KmsConfigurationException("Duplicate KMS source '%s' in %s",
source, KMS_SOURCES);
+ if (!uniqueProviders.add(provider)) {
+ throw new KmsConfigurationException(
+ "Duplicate KMS provider '%s' in %s", provider, KMS_PROVIDERS);
}
- sources.add(source);
+ providers.add(provider);
}
- return Collections.unmodifiableList(sources);
+ return Collections.unmodifiableList(providers);
}
- private static Map<String, SourceConfig> parseSourceConfigs(
- Map<String, String> values, List<String> configuredSources) {
- Map<String, Map<String, String>> propertiesBySource = new
LinkedHashMap<>();
- for (String source : configuredSources) {
- propertiesBySource.put(source, new LinkedHashMap<>());
+ private static Map<String, ProviderConfig> parseProviderConfigs(
+ Map<String, String> values, List<String> configuredProviders) {
+ Map<String, Map<String, String>> propertiesByProvider = new
LinkedHashMap<>();
+ for (String provider : configuredProviders) {
+ propertiesByProvider.put(provider, new LinkedHashMap<>());
}
for (Map.Entry<String, String> entry : values.entrySet()) {
String key = entry.getKey();
- if (SOURCES.equals(key)) {
+ if (PROVIDERS.equals(key)) {
continue;
}
- if (!key.startsWith(SOURCE_PREFIX)) {
+ if (!key.startsWith(PROVIDER_PREFIX)) {
throw invalidConfigurationKey(key);
}
- String sourceAndProperty = key.substring(SOURCE_PREFIX.length());
- int separator = sourceAndProperty.indexOf('.');
- if (separator <= 0 || separator == sourceAndProperty.length() - 1) {
+ String providerAndProperty = key.substring(PROVIDER_PREFIX.length());
+ int separator = providerAndProperty.indexOf('.');
+ if (separator <= 0 || separator == providerAndProperty.length() - 1) {
throw invalidConfigurationKey(key);
}
- String source = sourceAndProperty.substring(0, separator);
- if (!SOURCE_NAME_PATTERN.matcher(source).matches()) {
+ String provider = providerAndProperty.substring(0, separator);
+ if (!PROVIDER_NAME_PATTERN.matcher(provider).matches()) {
throw invalidConfigurationKey(key);
}
- Map<String, String> properties = propertiesBySource.get(source);
+ Map<String, String> properties = propertiesByProvider.get(provider);
if (properties == null) {
throw new KmsConfigurationException(
- "KMS configuration references unlisted source '%s'", source);
+ "KMS configuration references unlisted provider '%s'", provider);
}
- String property = sourceAndProperty.substring(separator + 1);
+ String property = providerAndProperty.substring(separator + 1);
properties.put(property, entry.getValue());
}
- Map<String, SourceConfig> sourceConfigs = new LinkedHashMap<>();
+ Map<String, ProviderConfig> providerConfigs = new LinkedHashMap<>();
- for (String source : configuredSources) {
- String apiKey = SOURCE_PREFIX + source + "." + API;
- Map<String, String> properties = propertiesBySource.get(source);
- String apiValue = properties.remove(API);
- if (apiValue == null || apiValue.trim().isEmpty()) {
- throw new KmsConfigurationException(
- "KMS API property '%s%s' cannot be blank", KMS_CONFIG_PREFIX,
apiKey);
- }
- String api;
- try {
- api = KmsApiIdentifiers.requireValid(apiValue);
- } catch (IllegalArgumentException e) {
+ for (String provider : configuredProviders) {
+ String classNameKey = PROVIDER_PREFIX + provider + "." + CLASS_NAME;
+ Map<String, String> properties = propertiesByProvider.get(provider);
+ String className = properties.remove(CLASS_NAME);
+ if (className == null || className.trim().isEmpty()) {
throw new KmsConfigurationException(
- e, "Invalid KMS API property '%s%s': %s", KMS_CONFIG_PREFIX,
apiKey, e.getMessage());
+ "KMS className property '%s%s' cannot be blank",
KMS_CONFIG_PREFIX, classNameKey);
}
- sourceConfigs.put(source, new SourceConfig(api, properties));
+ providerConfigs.put(provider, new ProviderConfig(className.trim(),
properties));
}
- return Collections.unmodifiableMap(sourceConfigs);
+ return Collections.unmodifiableMap(providerConfigs);
}
private static KmsConfigurationException invalidConfigurationKey(String key)
{
@@ -141,17 +135,17 @@ final class KmsConfig {
"Invalid KMS configuration key '%s%s'", KMS_CONFIG_PREFIX, key);
}
- static final class SourceConfig {
- private final String api;
+ static final class ProviderConfig {
+ private final String className;
private final Map<String, String> properties;
- private SourceConfig(String api, Map<String, String> properties) {
- this.api = api;
+ private ProviderConfig(String className, Map<String, String> properties) {
+ this.className = className;
this.properties = Collections.unmodifiableMap(new
LinkedHashMap<>(properties));
}
- String api() {
- return api;
+ String className() {
+ return className;
}
Map<String, String> properties() {
diff --git
a/core/src/test/java/org/apache/gravitino/TestGravitinoEnvKmsClientRegistry.java
b/core/src/test/java/org/apache/gravitino/TestGravitinoEnvKmsClientRegistry.java
index 5f08061a78..142aaa500a 100644
---
a/core/src/test/java/org/apache/gravitino/TestGravitinoEnvKmsClientRegistry.java
+++
b/core/src/test/java/org/apache/gravitino/TestGravitinoEnvKmsClientRegistry.java
@@ -35,7 +35,7 @@ public class TestGravitinoEnvKmsClientRegistry {
FieldUtils.writeField(env, "kmsClientRegistry", registry, true);
Assertions.assertSame(registry, env.kmsClientRegistry());
- KmsReference reference = new KmsReference("aws-kms", "missing", "key");
+ KmsReference reference = new KmsReference("missing", "key");
Assertions.assertThrows(IllegalArgumentException.class, () ->
registry.getClient(reference));
env.shutdown();
@@ -54,8 +54,7 @@ public class TestGravitinoEnvKmsClientRegistry {
Assertions.assertSame(registry, env.kmsClientRegistry());
Assertions.assertThrows(
- IllegalStateException.class,
- () -> registry.getClient(new KmsReference("aws-kms", "missing",
"key")));
+ IllegalStateException.class, () -> registry.getClient(new
KmsReference("missing", "key")));
}
private static final class TestGravitinoEnv extends GravitinoEnv {}
diff --git
a/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClientRegistry.java
b/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClientRegistry.java
index aebdcfcfb5..ea9d559a6a 100644
---
a/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClientRegistry.java
+++
b/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsClientRegistry.java
@@ -18,11 +18,6 @@
*/
package org.apache.gravitino.encryption.kms;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -32,47 +27,51 @@ import java.util.concurrent.atomic.AtomicInteger;
import org.apache.gravitino.Config;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.io.TempDir;
public class TestKmsClientRegistry {
- private static final String AWS_API = "aws-kms";
- private static final String GCP_API = "google-cloud-kms";
- private static final String AZURE_API = "azure-key-vault";
+ private static final String AWS_FACTORY = "test.AwsKmsClientFactory";
+ private static final String GCP_FACTORY = "test.GcpKmsClientFactory";
+ private static final String AZURE_FACTORY = "test.AzureKmsClientFactory";
@Test
- void testEmptyRegistryDoesNotEnumerateFactories() {
- Iterable<KmsClientFactory> factories =
- () -> {
- throw new AssertionError("Factories must not be enumerated without
configured sources");
+ void testEmptyRegistryDoesNotLoadFactories() {
+ KmsClientRegistry.FactoryLoader loader =
+ className -> {
+ throw new AssertionError("Factories must not be loaded without
configured providers");
};
- KmsClientRegistry registry = new KmsClientRegistry(config(), factories);
- KmsReference reference = new KmsReference(AWS_API, "primary",
"alias/orders");
+ KmsClientRegistry registry = new KmsClientRegistry(config(), loader);
+ KmsReference reference = new KmsReference("primary", "alias/orders");
IllegalArgumentException exception =
Assertions.assertThrows(
IllegalArgumentException.class, () ->
registry.getClient(reference));
Assertions.assertEquals(
- "No KMS client is configured for source 'primary'",
exception.getMessage());
+ "No KMS client is configured for provider 'primary'",
exception.getMessage());
}
@Test
void testCreatesAndDispatchesConfiguredClients() {
- RecordingFactory awsFactory = new RecordingFactory(AWS_API);
- RecordingFactory gcpFactory = new RecordingFactory(GCP_API);
+ RecordingFactory awsFactory = new RecordingFactory();
+ RecordingFactory gcpFactory = new RecordingFactory();
KmsClientRegistry registry =
new KmsClientRegistry(
config(
- "gravitino.kms.sources", "primary,analytics",
- "gravitino.kms.source.primary.api", AWS_API,
- "gravitino.kms.source.primary.endpoint.region", "us-west-2",
- "gravitino.kms.source.analytics.api", "google-cloud-kms",
- "gravitino.kms.source.analytics.endpoint.project",
"data-project"),
- List.of(awsFactory, gcpFactory));
-
- KmsReference awsReference = new KmsReference(AWS_API, "primary",
"alias/orders");
+ "gravitino.kms.providers",
+ "primary,analytics",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.primary.endpoint.region",
+ "us-west-2",
+ "gravitino.kms.provider.analytics.className",
+ GCP_FACTORY,
+ "gravitino.kms.provider.analytics.endpoint.project",
+ "data-project"),
+ loader(Map.of(AWS_FACTORY, awsFactory, GCP_FACTORY, gcpFactory)));
+
+ KmsReference awsReference = new KmsReference("primary", "alias/orders");
KmsReference gcpReference =
- new KmsReference(GCP_API, "analytics",
"projects/p/locations/l/keyRings/r/cryptoKeys/k");
+ new KmsReference("analytics",
"projects/p/locations/l/keyRings/r/cryptoKeys/k");
KmsClient awsClient = registry.getClient(awsReference);
KmsClient gcpClient = registry.getClient(gcpReference);
@@ -81,43 +80,45 @@ public class TestKmsClientRegistry {
Assertions.assertSame(gcpClient, registry.getClient(gcpReference));
Assertions.assertEquals(Map.of("endpoint.region", "us-west-2"),
awsFactory.properties);
Assertions.assertEquals(Map.of("endpoint.project", "data-project"),
gcpFactory.properties);
- Assertions.assertEquals("primary", awsFactory.createdSource);
- Assertions.assertEquals("analytics", gcpFactory.createdSource);
+ Assertions.assertEquals("primary", awsFactory.createdProvider);
+ Assertions.assertEquals("analytics", gcpFactory.createdProvider);
Assertions.assertEquals(1, awsFactory.createCount.get());
Assertions.assertEquals(1, gcpFactory.createCount.get());
}
@Test
- void testRejectsUnknownSourceAndApiMismatch() {
+ void testRejectsUnknownProvider() {
KmsClientRegistry registry =
new KmsClientRegistry(
config(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.primary.api", "aws-kms"),
- List.of(new RecordingFactory(AWS_API)));
+ "gravitino.kms.providers",
+ "primary",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY),
+ loader(Map.of(AWS_FACTORY, new RecordingFactory())));
Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> registry.getClient(new KmsReference(AWS_API, "other", "key")));
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> registry.getClient(new KmsReference(GCP_API, "primary", "key")));
+ IllegalArgumentException.class, () -> registry.getClient(new
KmsReference("other", "key")));
+ Assertions.assertDoesNotThrow(() -> registry.getClient(new
KmsReference("primary", "key")));
Assertions.assertThrows(IllegalArgumentException.class, () ->
registry.getClient(null));
}
@Test
- void testCreatesMultipleSourcesForSameApi() {
- RecordingFactory factory = new RecordingFactory(AZURE_API);
+ void testCreatesMultipleProvidersForSameClass() {
+ RecordingFactory factory = new RecordingFactory();
KmsClientRegistry registry =
new KmsClientRegistry(
config(
- "gravitino.kms.sources", "azure-eu,azure-us",
- "gravitino.kms.source.azure-eu.api", "azure-key-vault",
- "gravitino.kms.source.azure-us.api", "azure-key-vault"),
- List.of(factory));
+ "gravitino.kms.providers",
+ "azure-eu,azure-us",
+ "gravitino.kms.provider.azure-eu.className",
+ AZURE_FACTORY,
+ "gravitino.kms.provider.azure-us.className",
+ AZURE_FACTORY),
+ loader(Map.of(AZURE_FACTORY, factory)));
- KmsReference euReference = new KmsReference(AZURE_API, "azure-eu",
"primary");
- KmsReference usReference = new KmsReference(AZURE_API, "azure-us",
"primary");
+ KmsReference euReference = new KmsReference("azure-eu", "primary");
+ KmsReference usReference = new KmsReference("azure-us", "primary");
KmsClient euClient = registry.getClient(euReference);
KmsClient usClient = registry.getClient(usReference);
@@ -128,136 +129,89 @@ public class TestKmsClientRegistry {
}
@Test
- void testRoutesCustomApi() {
- String customApi = "custom-kms";
- KmsClientRegistry registry =
- new KmsClientRegistry(
- config("gravitino.kms.sources", "custom",
"gravitino.kms.source.custom.api", customApi),
- List.of(new RecordingFactory(customApi)));
- KmsReference reference = new KmsReference(customApi, "custom", "key");
-
- Assertions.assertNotNull(registry.getClient(reference));
- }
-
- @Test
- void testMatchesApiIdentifiersByValue() {
- KmsClientRegistry registry =
- new KmsClientRegistry(
- config(
- "gravitino.kms.sources",
- "primary",
- "gravitino.kms.source.primary.api",
- new String(AWS_API)),
- List.of(new RecordingFactory(new String(AWS_API))));
- KmsReference reference = new KmsReference(new String(AWS_API), "primary",
"key");
-
- Assertions.assertNotNull(registry.getClient(reference));
- }
-
- @Test
- void testRejectsMissingDuplicateAndInvalidFactories() {
+ void testRejectsMissingFactoryClass() {
Config awsConfig =
config(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.primary.api", "aws-kms");
+ "gravitino.kms.providers",
+ "primary",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY);
Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsClientRegistry(awsConfig,
List.of()));
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () ->
- new KmsClientRegistry(
- awsConfig, List.of(new RecordingFactory(AWS_API), new
RecordingFactory(AWS_API))));
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> new KmsClientRegistry(awsConfig, List.of(new
RecordingFactory(null))));
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> new KmsClientRegistry(awsConfig, List.of(new RecordingFactory("
"))));
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> new KmsClientRegistry(awsConfig, List.of(new RecordingFactory("
aws-kms"))));
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> new KmsClientRegistry(awsConfig, List.of(new
RecordingFactory("AWS-KMS"))));
+ IllegalArgumentException.class, () -> new KmsClientRegistry(awsConfig,
loader(Map.of())));
Assertions.assertThrows(
IllegalArgumentException.class,
- () -> new KmsClientRegistry(awsConfig,
java.util.Arrays.asList((KmsClientFactory) null)));
- Assertions.assertThrows(
- IllegalArgumentException.class, () -> new KmsClientRegistry(awsConfig,
null));
+ () -> new KmsClientRegistry(awsConfig,
(KmsClientRegistry.FactoryLoader) null));
}
@Test
- void testRejectsConfiguredApiWithoutFactory() {
- Config customConfig =
- config(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.primary.api", "custom-kms");
+ void testPublicConstructorLoadsFactoryByClassName() {
+ try (KmsClientRegistry registry =
+ new KmsClientRegistry(
+ config(
+ "gravitino.kms.providers",
+ "primary",
+ "gravitino.kms.provider.primary.className",
+ ClassLoadedFactory.class.getName()))) {
+ KmsReference reference = new KmsReference("primary", "key");
+ Assertions.assertNotNull(registry.getClient(reference));
+ }
+ }
+ @Test
+ void testPublicConstructorRejectsUnknownClass() {
IllegalArgumentException exception =
Assertions.assertThrows(
IllegalArgumentException.class,
- () -> new KmsClientRegistry(customConfig, List.of(new
RecordingFactory(AWS_API))));
- Assertions.assertTrue(
- exception.getMessage().contains("No KMS client factory supports API
'custom-kms'"));
- }
-
- @Test
- void testPublicConstructorUsesContextClassLoader(@TempDir Path
tempDirectory) throws Exception {
- Path serviceFile =
- tempDirectory.resolve(
-
"META-INF/services/org.apache.gravitino.encryption.kms.KmsClientFactory");
- Files.createDirectories(serviceFile.getParent());
- Files.write(serviceFile,
ServiceLoadedFactory.class.getName().getBytes(StandardCharsets.UTF_8));
-
- ClassLoader originalClassLoader =
Thread.currentThread().getContextClassLoader();
- try (URLClassLoader serviceClassLoader =
- new URLClassLoader(new URL[] {tempDirectory.toUri().toURL()},
originalClassLoader)) {
- Thread.currentThread().setContextClassLoader(serviceClassLoader);
- try (KmsClientRegistry registry =
- new KmsClientRegistry(
- config(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.primary.api", "aws-kms"))) {
- KmsReference reference = new KmsReference(AWS_API, "primary", "key");
- Assertions.assertNotNull(registry.getClient(reference));
- }
- } finally {
- Thread.currentThread().setContextClassLoader(originalClassLoader);
- }
+ () ->
+ new KmsClientRegistry(
+ config(
+ "gravitino.kms.providers",
+ "primary",
+ "gravitino.kms.provider.primary.className",
+ "test.MissingKmsClientFactory")));
+ Assertions.assertTrue(exception.getMessage().contains("No KMS client
factory class"));
}
@Test
void testRejectsNullClientAndClosesPreviouslyCreatedClient() {
- CloseTrackingFactory awsFactory =
- new CloseTrackingFactory(AWS_API, "aws", new ArrayList<>(), null);
+ CloseTrackingFactory awsFactory = new CloseTrackingFactory("aws", new
ArrayList<>(), null);
Config awsConfig =
config(
- "gravitino.kms.sources", "primary,analytics",
- "gravitino.kms.source.primary.api", "aws-kms",
- "gravitino.kms.source.analytics.api", "google-cloud-kms");
+ "gravitino.kms.providers",
+ "primary,analytics",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.analytics.className",
+ GCP_FACTORY);
- KmsClientFactory nullClientFactory = factory(GCP_API, (source, properties)
-> null);
+ KmsClientFactory nullClientFactory = factory((provider, properties) ->
null);
Assertions.assertThrows(
IllegalStateException.class,
- () -> new KmsClientRegistry(awsConfig, List.of(awsFactory,
nullClientFactory)));
+ () ->
+ new KmsClientRegistry(
+ awsConfig,
+ loader(Map.of(AWS_FACTORY, awsFactory, GCP_FACTORY,
nullClientFactory))));
Assertions.assertEquals(1, awsFactory.closeCount.get());
}
@Test
void testClosesClientsInReverseOrderAndIsIdempotent() {
List<String> closeOrder = new ArrayList<>();
- CloseTrackingFactory awsFactory = new CloseTrackingFactory(AWS_API, "aws",
closeOrder, null);
- CloseTrackingFactory gcpFactory = new CloseTrackingFactory(GCP_API, "gcp",
closeOrder, null);
+ CloseTrackingFactory awsFactory = new CloseTrackingFactory("aws",
closeOrder, null);
+ CloseTrackingFactory gcpFactory = new CloseTrackingFactory("gcp",
closeOrder, null);
KmsClientRegistry registry =
new KmsClientRegistry(
config(
- "gravitino.kms.sources", "primary,analytics",
- "gravitino.kms.source.primary.api", "aws-kms",
- "gravitino.kms.source.analytics.api", "google-cloud-kms"),
- List.of(awsFactory, gcpFactory));
- KmsReference awsReference = new KmsReference(AWS_API, "primary", "key");
+ "gravitino.kms.providers",
+ "primary,analytics",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.analytics.className",
+ GCP_FACTORY),
+ loader(Map.of(AWS_FACTORY, awsFactory, GCP_FACTORY, gcpFactory)));
+ KmsReference awsReference = new KmsReference("primary", "key");
registry.close();
registry.close();
@@ -271,11 +225,10 @@ public class TestKmsClientRegistry {
@Test
void testClosesCreatedClientsAfterPartialInitializationFailure() {
List<String> closeOrder = new ArrayList<>();
- CloseTrackingFactory awsFactory = new CloseTrackingFactory(AWS_API, "aws",
closeOrder, null);
+ CloseTrackingFactory awsFactory = new CloseTrackingFactory("aws",
closeOrder, null);
KmsClientFactory failingFactory =
factory(
- GCP_API,
- (source, properties) -> {
+ (provider, properties) -> {
throw new IllegalArgumentException("invalid GCP configuration");
});
@@ -284,10 +237,13 @@ public class TestKmsClientRegistry {
() ->
new KmsClientRegistry(
config(
- "gravitino.kms.sources", "primary,analytics",
- "gravitino.kms.source.primary.api", "aws-kms",
- "gravitino.kms.source.analytics.api", "google-cloud-kms"),
- List.of(awsFactory, failingFactory)));
+ "gravitino.kms.providers",
+ "primary,analytics",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.analytics.className",
+ GCP_FACTORY),
+ loader(Map.of(AWS_FACTORY, awsFactory, GCP_FACTORY,
failingFactory))));
Assertions.assertEquals(1, awsFactory.closeCount.get());
Assertions.assertEquals(List.of("aws"), closeOrder);
@@ -297,13 +253,12 @@ public class TestKmsClientRegistry {
void testPreservesInitializationFailureWhenCleanupFails() {
RuntimeException closeFailure = new IllegalStateException("close failed");
CloseTrackingFactory awsFactory =
- new CloseTrackingFactory(AWS_API, "aws", new ArrayList<>(),
closeFailure);
+ new CloseTrackingFactory("aws", new ArrayList<>(), closeFailure);
IllegalArgumentException creationFailure =
new IllegalArgumentException("invalid GCP configuration");
KmsClientFactory failingFactory =
factory(
- GCP_API,
- (source, properties) -> {
+ (provider, properties) -> {
throw creationFailure;
});
@@ -313,10 +268,13 @@ public class TestKmsClientRegistry {
() ->
new KmsClientRegistry(
config(
- "gravitino.kms.sources", "primary,analytics",
- "gravitino.kms.source.primary.api", "aws-kms",
- "gravitino.kms.source.analytics.api",
"google-cloud-kms"),
- List.of(awsFactory, failingFactory)));
+ "gravitino.kms.providers",
+ "primary,analytics",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.analytics.className",
+ GCP_FACTORY),
+ loader(Map.of(AWS_FACTORY, awsFactory, GCP_FACTORY,
failingFactory))));
Assertions.assertSame(creationFailure, exception);
Assertions.assertArrayEquals(new Throwable[] {closeFailure},
exception.getSuppressed());
@@ -327,16 +285,19 @@ public class TestKmsClientRegistry {
RuntimeException awsFailure = new IllegalStateException("aws close
failed");
RuntimeException gcpFailure = new IllegalStateException("gcp close
failed");
CloseTrackingFactory awsFactory =
- new CloseTrackingFactory(AWS_API, "aws", new ArrayList<>(),
awsFailure);
+ new CloseTrackingFactory("aws", new ArrayList<>(), awsFailure);
CloseTrackingFactory gcpFactory =
- new CloseTrackingFactory(GCP_API, "gcp", new ArrayList<>(),
gcpFailure);
+ new CloseTrackingFactory("gcp", new ArrayList<>(), gcpFailure);
KmsClientRegistry registry =
new KmsClientRegistry(
config(
- "gravitino.kms.sources", "primary,analytics",
- "gravitino.kms.source.primary.api", "aws-kms",
- "gravitino.kms.source.analytics.api", "google-cloud-kms"),
- List.of(awsFactory, gcpFactory));
+ "gravitino.kms.providers",
+ "primary,analytics",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.analytics.className",
+ GCP_FACTORY),
+ loader(Map.of(AWS_FACTORY, awsFactory, GCP_FACTORY, gcpFactory)));
RuntimeException exception =
Assertions.assertThrows(RuntimeException.class, registry::close);
Assertions.assertSame(gcpFailure, exception);
@@ -351,70 +312,54 @@ public class TestKmsClientRegistry {
return new MapConfig(properties);
}
- private static KmsClientFactory factory(String api, ClientCreator creator) {
- return new KmsClientFactory() {
- @Override
- public String api() {
- return api;
- }
-
- @Override
- public KmsClient create(String source, Map<String, String> properties) {
- return creator.create(source, properties);
+ private static KmsClientRegistry.FactoryLoader loader(Map<String,
KmsClientFactory> factories) {
+ return className -> {
+ KmsClientFactory factory = factories.get(className);
+ if (factory == null) {
+ throw new IllegalArgumentException(
+ String.format("No KMS client factory class '%s'", className));
}
+ return factory;
};
}
+ private static KmsClientFactory factory(ClientCreator creator) {
+ return creator::create;
+ }
+
private interface ClientCreator {
- KmsClient create(String source, Map<String, String> properties);
+ KmsClient create(String provider, Map<String, String> properties);
}
private static final class RecordingFactory implements KmsClientFactory {
- private final String api;
- private String createdSource;
+ private String createdProvider;
private Map<String, String> properties;
private final AtomicInteger createCount = new AtomicInteger();
- private RecordingFactory(String api) {
- this.api = api;
- }
-
- @Override
- public String api() {
- return api;
- }
-
@Override
- public KmsClient create(String source, Map<String, String> properties) {
+ public KmsClient create(String provider, Map<String, String> properties) {
createCount.incrementAndGet();
- this.createdSource = source;
+ this.createdProvider = provider;
this.properties = properties;
return reference -> Optional.of(new Properties(reference));
}
}
private static final class CloseTrackingFactory implements KmsClientFactory {
- private final String api;
private final String name;
private final List<String> closeOrder;
private final RuntimeException closeFailure;
private final AtomicInteger closeCount = new AtomicInteger();
private CloseTrackingFactory(
- String api, String name, List<String> closeOrder, RuntimeException
closeFailure) {
- this.api = api;
+ String name, List<String> closeOrder, RuntimeException closeFailure) {
this.name = name;
this.closeOrder = closeOrder;
this.closeFailure = closeFailure;
}
@Override
- public String api() {
- return api;
- }
-
- @Override
- public KmsClient create(String source, Map<String, String> properties) {
+ public KmsClient create(String provider, Map<String, String> properties) {
return new KmsClient() {
@Override
public Optional<KmsKeyProperties> getKeyProperties(KmsReference
reference) {
@@ -468,21 +413,15 @@ public class TestKmsClientRegistry {
}
}
- /** Factory exposed for the context-classloader ServiceLoader test. */
- public static final class ServiceLoadedFactory implements KmsClientFactory {
+ /** Factory loaded by {@link Class#forName(String)} in the
public-constructor test. */
+ public static final class ClassLoadedFactory implements KmsClientFactory {
- /** Creates a test service-loaded factory. */
- public ServiceLoadedFactory() {}
-
- /** {@inheritDoc} */
- @Override
- public String api() {
- return AWS_API;
- }
+ /** Creates a test factory. */
+ public ClassLoadedFactory() {}
/** {@inheritDoc} */
@Override
- public KmsClient create(String source, Map<String, String> properties) {
+ public KmsClient create(String provider, Map<String, String> properties) {
return reference -> Optional.of(new Properties(reference));
}
}
diff --git
a/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsConfig.java
b/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsConfig.java
index 5a47052cb2..8cdbde87c4 100644
--- a/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsConfig.java
+++ b/core/src/test/java/org/apache/gravitino/encryption/kms/TestKmsConfig.java
@@ -26,97 +26,109 @@ import org.junit.jupiter.api.Test;
public class TestKmsConfig {
- private static final String AWS_API = "aws-kms";
- private static final String GCP_API = "google-cloud-kms";
+ private static final String AWS_FACTORY =
+ "org.apache.gravitino.encryption.kms.aws.AwsKmsClientFactory";
+ private static final String GCP_FACTORY =
+ "org.apache.gravitino.encryption.kms.gcp.GcpKmsClientFactory";
@Test
- void testParsesSourcesAndProviderProperties() {
+ void testParsesProvidersAndProperties() {
KmsConfig config =
parse(
Map.of(
- "gravitino.kms.sources", "primary, disaster-recovery",
- "gravitino.kms.source.primary.api", AWS_API,
- "gravitino.kms.source.primary.endpoint.region", "us-west-2",
- "gravitino.kms.source.primary.credential.method", "default",
- "gravitino.kms.source.disaster-recovery.api",
"google-cloud-kms",
- "gravitino.kms.source.disaster-recovery.endpoint.projectId",
"backup-project",
- "gravitino.kms.source.disaster-recovery.credential.method",
"default"));
+ "gravitino.kms.providers",
+ "primary,disaster-recovery",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.primary.endpoint.region",
+ "us-west-2",
+ "gravitino.kms.provider.primary.credential.method",
+ "default",
+ "gravitino.kms.provider.disaster-recovery.className",
+ GCP_FACTORY,
+ "gravitino.kms.provider.disaster-recovery.endpoint.projectId",
+ "backup-project",
+ "gravitino.kms.provider.disaster-recovery.credential.method",
+ "default"));
- Assertions.assertEquals(2, config.sources().size());
- Assertions.assertEquals(AWS_API, config.sources().get("primary").api());
+ Assertions.assertEquals(2, config.providers().size());
+ Assertions.assertEquals(AWS_FACTORY,
config.providers().get("primary").className());
Assertions.assertEquals(
Map.of("endpoint.region", "us-west-2", "credential.method", "default"),
- config.sources().get("primary").properties());
- Assertions.assertEquals(GCP_API,
config.sources().get("disaster-recovery").api());
+ config.providers().get("primary").properties());
+ Assertions.assertEquals(GCP_FACTORY,
config.providers().get("disaster-recovery").className());
Assertions.assertEquals(
Map.of("endpoint.projectId", "backup-project", "credential.method",
"default"),
- config.sources().get("disaster-recovery").properties());
+ config.providers().get("disaster-recovery").properties());
Assertions.assertThrows(
UnsupportedOperationException.class,
- () ->
config.sources().get("primary").properties().put("endpoint.region", "other"));
+ () ->
config.providers().get("primary").properties().put("endpoint.region", "other"));
}
@Test
- void testAllowsNoConfiguredSources() {
- Assertions.assertTrue(parse(Map.of()).sources().isEmpty());
- Assertions.assertTrue(parse(Map.of("gravitino.kms.sources", "
")).sources().isEmpty());
+ void testAllowsNoConfiguredProviders() {
+ Assertions.assertTrue(parse(Map.of()).providers().isEmpty());
+ Assertions.assertTrue(parse(Map.of("gravitino.kms.providers", "
")).providers().isEmpty());
}
@Test
- void testRejectsInvalidOrDuplicateSourceNames() {
- assertInvalid(Map.of("gravitino.kms.sources", "primary,"), "Invalid KMS
source name");
- assertInvalid(Map.of("gravitino.kms.sources", "bad.name"), "Invalid KMS
source name");
+ void testRejectsInvalidOrDuplicateProviderNames() {
+ assertInvalid(Map.of("gravitino.kms.providers", "primary,"), "Invalid KMS
provider name");
+ assertInvalid(Map.of("gravitino.kms.providers", "bad.name"), "Invalid KMS
provider name");
assertInvalid(
- Map.of("gravitino.kms.sources", "primary,primary"), "Duplicate KMS
source 'primary'");
+ Map.of("gravitino.kms.providers", "primary,primary"), "Duplicate KMS
provider 'primary'");
}
@Test
- void testRejectsMalformedOrUnlistedSourceProperties() {
+ void testRejectsMalformedOrUnlistedProviderProperties() {
assertInvalid(Map.of("gravitino.kms.unexpected", "value"), "Invalid KMS
configuration key");
- assertInvalid(Map.of("gravitino.kms.source.primary", "value"), "Invalid
KMS configuration key");
- assertInvalid(Map.of("gravitino.kms.source..api", AWS_API), "Invalid KMS
configuration key");
assertInvalid(
- Map.of("gravitino.kms.source.bad$name.api", AWS_API), "Invalid KMS
configuration key");
+ Map.of("gravitino.kms.provider.primary", "value"), "Invalid KMS
configuration key");
+ assertInvalid(
+ Map.of("gravitino.kms.provider..className", AWS_FACTORY), "Invalid KMS
configuration key");
+ assertInvalid(
+ Map.of("gravitino.kms.provider.bad$name.className", AWS_FACTORY),
+ "Invalid KMS configuration key");
assertInvalid(
Map.of(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.other.api", "aws-kms"),
- "unlisted source 'other'");
+ "gravitino.kms.providers",
+ "primary",
+ "gravitino.kms.provider.other.className",
+ AWS_FACTORY),
+ "unlisted provider 'other'");
assertInvalid(
Map.of(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.primary.", "value"),
+ "gravitino.kms.providers", "primary",
+ "gravitino.kms.provider.primary.", "value"),
"Invalid KMS configuration key");
}
@Test
- void testRequiresApi() {
+ void testRequiresClassName() {
assertInvalid(
- Map.of("gravitino.kms.sources", "primary"),
- "gravitino.kms.source.primary.api' cannot be blank");
- assertInvalid(
- Map.of(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.primary.api", " "),
- "gravitino.kms.source.primary.api' cannot be blank");
+ Map.of("gravitino.kms.providers", "primary"),
+ "gravitino.kms.provider.primary.className' cannot be blank");
assertInvalid(
Map.of(
- "gravitino.kms.sources", "primary",
- "gravitino.kms.source.primary.api", "Custom-KMS"),
- "must be lowercase kebab-case");
+ "gravitino.kms.providers", "primary",
+ "gravitino.kms.provider.primary.className", " "),
+ "gravitino.kms.provider.primary.className' cannot be blank");
}
@Test
- void testAllowsMoreThanOneSourceForAnApi() {
+ void testAllowsMoreThanOneProviderForAClass() {
KmsConfig config =
parse(
Map.of(
- "gravitino.kms.sources", "primary,secondary",
- "gravitino.kms.source.primary.api", "aws-kms",
- "gravitino.kms.source.secondary.api", "aws-kms"));
+ "gravitino.kms.providers",
+ "primary,secondary",
+ "gravitino.kms.provider.primary.className",
+ AWS_FACTORY,
+ "gravitino.kms.provider.secondary.className",
+ AWS_FACTORY));
- Assertions.assertEquals(AWS_API, config.sources().get("primary").api());
- Assertions.assertEquals(AWS_API, config.sources().get("secondary").api());
+ Assertions.assertEquals(AWS_FACTORY,
config.providers().get("primary").className());
+ Assertions.assertEquals(AWS_FACTORY,
config.providers().get("secondary").className());
}
@Test
diff --git a/docs/gravitino-server-config.md b/docs/gravitino-server-config.md
index c2ad57ba33..963774a51b 100644
--- a/docs/gravitino-server-config.md
+++ b/docs/gravitino-server-config.md
@@ -486,6 +486,45 @@ server, are documented with those services. See
| `gravitino.job.stagingDirKeepTimeInMs` | How long in milliseconds a finished
job's staging files are kept. Use at least 10 minutes outside testing. |
`604800000` (7 days) |
| `gravitino.job.statusPullIntervalInMs` | Interval in milliseconds between
job status polls. Use at least 1 minute outside testing. |
`300000` (5 minutes) |
+### Key Management
+
+The server talks to KMS instances you name in `gravitino.conf`. Each name is
one configured
+instance. To add one, implement `KmsClientFactory` with a public no-arg
constructor, put the jar on
+the server classpath, and set `gravitino.kms.provider.<name>.className` to
that class.
+`create(provider, properties)` builds the `KmsClient` for that name. Gravitino
does not ship AWS or
+Azure factories. Two names may share one class, which is how you run more than
one vault of the
+same kind.
+
+The list is empty by default, and then the server has no KMS clients. Naming a
provider without a
+`className`, or with a class the server cannot construct as a
`KmsClientFactory`, fails startup.
+Client construction validates local configuration only; the first call to the
provider is a later
+key inspection, not startup.
+
+| Configuration Item | Description
| Default Value |
+|--------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|
+| `gravitino.kms.providers` | Comma-separated KMS instance
names, with no spaces after commas. Each name must match
`[A-Za-z0-9][A-Za-z0-9_-]*` and cannot contain `.`. Duplicates fail startup. |
(empty) |
+| `gravitino.kms.provider.<name>.className` | Required factory class name for
that instance. The class must have a no-arg constructor and implement
`KmsClientFactory`. | (none) |
+| `gravitino.kms.provider.<name>.<key>` | Any other property under that
name is passed to that factory. Nested dots in `<key>` are allowed, as in
`endpoint.region`. | (none) |
+
+Every name in `providers` needs a matching `.className`. A
`gravitino.kms.provider.<name>.*` key
+for a name that is not in the list, or any other `gravitino.kms.*` key, fails
startup.
+
+Callers name the instance and the key. They do not send `className`. The
server already constructed
+the factory for `aws-prod` at startup.
+
+```text
+# conf/gravitino.conf
+gravitino.kms.providers = aws-prod,aws-dr,azure-eu
+
+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
+```
+
+That configuration builds three clients: two instances of one custom AWS
factory and one custom
+Azure factory. Further `gravitino.kms.provider.<name>.*` keys are factory
properties, not a closed
+schema; each factory documents the keys it accepts.
+
## Catalog Properties
Catalog properties configure one catalog rather than the server. They come
from two places: a
