roryqi commented on code in PR #12366:
URL: https://github.com/apache/gravitino/pull/12366#discussion_r3756286234
##########
core/src/main/java/org/apache/gravitino/secret/SecretManager.java:
##########
@@ -63,20 +66,108 @@ 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> assembleSecretMaterials(
Review Comment:
This name isn't good enough. We can't know you modify the properties. Could
u give a better name for it?
--
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]
