lasdf1234 commented on code in PR #12366:
URL: https://github.com/apache/gravitino/pull/12366#discussion_r3756255145
##########
core/src/main/java/org/apache/gravitino/secret/SecretManager.java:
##########
@@ -63,20 +66,116 @@ public SecretProviderRegistry getRegistry() {
return registry;
}
+ /**
+ * Ensures each property key appears at most once across {@code properties},
{@code
+ * secretBindings}, and {@code secretReferences}, and that {@code
properties} do not contain raw
+ * secret URN values.
+ *
+ * <p>{@code null} maps are treated as empty. Callers must bind/reference
secrets via the typed
+ * maps rather than embedding {@code urn:gravitino-secret:} values in
properties.
+ *
+ * @param properties entity properties from the create request (may be null)
+ * @param secretBindings property key → write-through binding (may be null)
+ * @param secretReferences property key → secret locator (may be null)
+ */
+ public void checkSecretKeys(
+ @Nullable Map<String, String> properties,
+ @Nullable Map<String, SecretBinding> secretBindings,
+ @Nullable Map<String, SecretReference> secretReferences) {
+ // Reject raw URNs in request properties: they bypass typed
secretBindings/secretReferences
+ // and can be used to resolve another entity's secret.
+ if (properties != null && !properties.isEmpty()) {
+ for (Map.Entry<String, String> entry : properties.entrySet()) {
+ String key = entry.getKey();
+ String value = entry.getValue();
+ if (SecretPropertyUtils.isSecretProperty(key, value)) {
+ throw new IllegalArgumentException(
+ "Property \""
+ + key
+ + "\" must not contain a raw gravitino secret URN; use
secretBindings or"
+ + " secretReferences instead");
+ }
+ }
+ }
+ Set<String> keys = new HashSet<>();
+ int count = 0;
+ if (properties != null) {
+ keys.addAll(properties.keySet());
+ count += properties.size();
+ }
+ if (secretBindings != null) {
+ keys.addAll(secretBindings.keySet());
+ count += secretBindings.size();
+ }
+ if (secretReferences != null) {
+ keys.addAll(secretReferences.keySet());
+ count += secretReferences.size();
+ }
+ if (keys.size() != count) {
+ throw new IllegalArgumentException(
+ "Duplicate property key across properties, secretBindings and
secretReferences");
+ }
+ }
+
+ /**
+ * Checks secret keys and puts reference / write-through URN strings into
{@code
+ * targetProperties}.
+ *
+ * <p>Callers typically validate {@code targetProperties}, then call {@link
#writeSecrets} with
+ * the returned secret materials, and {@link #rollbackSecrets} on failure.
{@code properties} is
+ * used only for key uniqueness checks (e.g. the original request map);
{@code targetProperties}
+ * is the mutable map that will be stored (may already contain merged
catalog conf).
+ *
+ * @param properties properties used for key uniqueness checks (may be null)
+ * @param targetProperties mutable properties that receive URN values
+ * @param entityType {@code catalog}, {@code schema}, or {@code fileset}
+ * @param entityId stable numeric entity id
+ * @param secretBindings property key → write-through binding (may be null)
+ * @param secretReferences property key → secret locator (may be null)
+ * @return write-through secret materials for {@link #writeSecrets} / {@link
#rollbackSecrets}
+ * (empty when there are no bindings)
+ */
+ public List<SecretMaterial> assembleSecretUrns(
Review Comment:
Got rename to assembleSecretMaterials.
##########
core/src/main/java/org/apache/gravitino/secret/SecretManager.java:
##########
@@ -63,20 +66,116 @@ public SecretProviderRegistry getRegistry() {
return registry;
}
+ /**
+ * Ensures each property key appears at most once across {@code properties},
{@code
+ * secretBindings}, and {@code secretReferences}, and that {@code
properties} do not contain raw
+ * secret URN values.
+ *
+ * <p>{@code null} maps are treated as empty. Callers must bind/reference
secrets via the typed
+ * maps rather than embedding {@code urn:gravitino-secret:} values in
properties.
+ *
+ * @param properties entity properties from the create request (may be null)
+ * @param secretBindings property key → write-through binding (may be null)
+ * @param secretReferences property key → secret locator (may be null)
+ */
+ public void checkSecretKeys(
+ @Nullable Map<String, String> properties,
+ @Nullable Map<String, SecretBinding> secretBindings,
+ @Nullable Map<String, SecretReference> secretReferences) {
+ // Reject raw URNs in request properties: they bypass typed
secretBindings/secretReferences
+ // and can be used to resolve another entity's secret.
+ if (properties != null && !properties.isEmpty()) {
+ for (Map.Entry<String, String> entry : properties.entrySet()) {
+ String key = entry.getKey();
+ String value = entry.getValue();
+ if (SecretPropertyUtils.isSecretProperty(key, value)) {
+ throw new IllegalArgumentException(
+ "Property \""
+ + key
+ + "\" must not contain a raw gravitino secret URN; use
secretBindings or"
+ + " secretReferences instead");
+ }
+ }
+ }
+ Set<String> keys = new HashSet<>();
+ int count = 0;
+ if (properties != null) {
+ keys.addAll(properties.keySet());
+ count += properties.size();
+ }
+ if (secretBindings != null) {
+ keys.addAll(secretBindings.keySet());
+ count += secretBindings.size();
+ }
+ if (secretReferences != null) {
+ keys.addAll(secretReferences.keySet());
+ count += secretReferences.size();
+ }
+ if (keys.size() != count) {
+ throw new IllegalArgumentException(
+ "Duplicate property key across properties, secretBindings and
secretReferences");
+ }
+ }
+
+ /**
+ * Checks secret keys and puts reference / write-through URN strings into
{@code
+ * targetProperties}.
+ *
+ * <p>Callers typically validate {@code targetProperties}, then call {@link
#writeSecrets} with
+ * the returned secret materials, and {@link #rollbackSecrets} on failure.
{@code properties} is
+ * used only for key uniqueness checks (e.g. the original request map);
{@code targetProperties}
+ * is the mutable map that will be stored (may already contain merged
catalog conf).
+ *
+ * @param properties properties used for key uniqueness checks (may be null)
+ * @param targetProperties mutable properties that receive URN values
+ * @param entityType {@code catalog}, {@code schema}, or {@code fileset}
+ * @param entityId stable numeric entity id
+ * @param secretBindings property key → write-through binding (may be null)
+ * @param secretReferences property key → secret locator (may be null)
+ * @return write-through secret materials for {@link #writeSecrets} / {@link
#rollbackSecrets}
+ * (empty when there are no bindings)
+ */
+ public List<SecretMaterial> assembleSecretUrns(
+ @Nullable Map<String, String> properties,
+ Map<String, String> targetProperties,
+ String entityType,
+ long entityId,
+ @Nullable Map<String, SecretBinding> secretBindings,
+ @Nullable Map<String, SecretReference> secretReferences) {
+ checkSecretKeys(properties, secretBindings, secretReferences);
+ Map<String, SecretBinding> bindings = secretBindings == null ? Map.of() :
secretBindings;
+ Map<String, SecretReference> references =
+ secretReferences == null ? Map.of() : secretReferences;
+ SecretPropertyUtils.putSecretUrns(targetProperties,
getSecretReferenceUrns(references));
+ List<SecretUrn> bindingUrns = getSecretBindingUrns(entityType, entityId,
bindings);
+ List<SecretMaterial> secretMaterials = new ArrayList<>(bindingUrns.size());
+ for (SecretUrn urn : bindingUrns) {
+ String propertyKey = urn.propertyKey();
+ SecretBinding binding = bindings.get(propertyKey);
+ Preconditions.checkArgument(
+ binding != null, "No secretBindings entry for property key \"%s\"",
propertyKey);
+ secretMaterials.add(new SecretMaterial(urn, binding.plaintext()));
+ }
+ SecretPropertyUtils.putSecretUrns(targetProperties, bindingUrns);
+ return List.copyOf(secretMaterials);
+ }
+
/**
* Builds external-reference URNs from {@code secretReferences} without
writing secret material.
*
* <p>Callers must put the returned URN strings into properties themselves
(e.g. via {@link
- * SecretPropertyUtils#applySecretUrns}). External-ref URNs are owned
outside Gravitino and must
- * not be passed to {@link #rollbackWritten}.
+ * SecretPropertyUtils#putSecretUrns}). External-ref URNs are owned outside
Gravitino and must not
+ * be passed to {@link #rollbackSecrets}.
*
- * @param secretReferences property key → secret locator
+ * @param secretReferences property key → secret locator (empty returns an
empty list; must not be
+ * null)
* @return external-reference URNs (insertion order)
*/
public List<SecretUrn> getSecretReferenceUrns(Map<String, SecretReference>
secretReferences) {
Review Comment:
Got resolved.
##########
core/src/main/java/org/apache/gravitino/secret/SecretManager.java:
##########
@@ -105,21 +204,23 @@ public List<SecretUrn> getSecretReferenceUrns(Map<String,
SecretReference> secre
/**
* Builds write-through URNs from {@code secretBindings} without writing
secret material.
*
- * <p>Callers should pass the returned URNs to {@link #writeSecrets} to
persist plaintext from
- * each binding's value, then put URNs into properties (e.g. via {@link
- * SecretPropertyUtils#applySecretUrns}).
+ * <p>Callers typically pass the returned URNs to {@link #writeSecrets} via
{@link
+ * #assembleSecretUrns}, or put URN strings into properties themselves (e.g.
via {@link
+ * SecretPropertyUtils#putSecretUrns}).
*
* @param entityType {@code catalog}, {@code schema}, or {@code fileset}
* @param entityId stable numeric entity id
- * @param secretBindings property key → write-through binding
+ * @param secretBindings property key → write-through binding (empty returns
an empty list; must
+ * not be null)
* @return write-through URNs (insertion order)
*/
public List<SecretUrn> getSecretBindingUrns(
Review Comment:
Got resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
