lasdf1234 commented on code in PR #12366:
URL: https://github.com/apache/gravitino/pull/12366#discussion_r3756020730
##########
core/src/main/java/org/apache/gravitino/secret/SecretManager.java:
##########
@@ -144,75 +234,154 @@ public List<SecretUrn> getSecretBindingUrns(
* Writes plaintext secrets from {@code secretBindings} values into the
write-through providers
* for {@code secretUrns} (e.g. Vault).
*
- * <p>{@code secretUrns} must come from {@link #getSecretBindingUrns}. On
failure, already-written
- * URNs are rolled back. Callers must put URN strings into properties
themselves (e.g. via {@link
- * SecretPropertyUtils#applySecretUrns}).
+ * <p>{@code secretUrns} must come from {@link #assembleSecretUrns} or {@link
+ * #getSecretBindingUrns}. On failure, already-written URNs are rolled back.
When using {@link
+ * #assembleSecretUrns}, URN strings are already in properties; otherwise
callers must put them
+ * themselves (e.g. via {@link SecretPropertyUtils#putSecretUrns}).
*
- * @param secretBindings property key → write-through binding
- * @param secretUrns write-through URNs from {@link #getSecretBindingUrns}
+ * <p>Callers must pass non-null maps/lists (use empty collections when
there are no secrets).
+ * Null checking belongs at the system entrance before invoking this method.
+ *
+ * @param secretBindings property key → write-through binding (empty is a
no-op)
+ * @param secretUrns write-through URNs from {@link #assembleSecretUrns} or
{@link
+ * #getSecretBindingUrns}
*/
public void writeSecrets(Map<String, SecretBinding> secretBindings,
List<SecretUrn> secretUrns) {
- Preconditions.checkArgument(
- secretBindings != null && !secretBindings.isEmpty(),
- "secretBindings must not be null or empty");
- Preconditions.checkArgument(
- secretUrns != null && !secretUrns.isEmpty(), "secretUrns must not be
null or empty");
+ Preconditions.checkArgument(secretBindings != null, "secretBindings must
not be null");
+ Preconditions.checkArgument(secretUrns != null, "secretUrns must not be
null");
+ if (secretBindings.isEmpty()) {
+ Preconditions.checkArgument(
+ secretUrns.isEmpty(), "secretUrns must be empty when bindings are
empty");
+ return;
+ }
+ Preconditions.checkArgument(!secretUrns.isEmpty(), "secretUrns must not be
empty");
validateSecretBindings(secretBindings);
List<SecretUrn> written = new ArrayList<>(secretUrns.size());
try {
for (SecretUrn urn : secretUrns) {
List<String> segments = urn.identifierSegments();
- Preconditions.checkArgument(
- segments.size() == 3,
- "Write-through secret URN must have entityType, entityId,
propertyKey segments: %s",
- urn);
+ if (segments.size() != 3) {
+ throw new IllegalArgumentException(
+ "Write-through secret URN must have entityType, entityId,
propertyKey segments: "
+ + urn);
+ }
String entityType = segments.get(0);
String entityId = segments.get(1);
- String propertyKey = segments.get(2);
+ String propertyKey = urn.propertyKey();
SecretBinding binding = secretBindings.get(propertyKey);
- Preconditions.checkArgument(
- binding != null, "No secretBindings entry for property key
\"%s\"", propertyKey);
- String plaintext = binding.plaintext();
+ if (binding == null) {
Review Comment:
Got resolved.
##########
core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java:
##########
@@ -143,45 +148,67 @@ public Fileset createMultipleLocationFileset(
String comment,
Fileset.Type type,
Map<String, String> storageLocations,
- Map<String, String> properties)
+ Map<String, String> properties,
+ Map<String, SecretBinding> secretBindings,
+ Map<String, SecretReference> secretReferences)
throws NoSuchSchemaException, FilesetAlreadyExistsException {
+ // System entrance: normalize null secret maps to empty so
SecretManager.writeSecrets can
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]
