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 d4316e127c [#12297] feat(secret): Support
setSecretBinding/setSecretReference on catalog/schema alter (#12674)
d4316e127c is described below
commit d4316e127ceaf2e2bf82f504f8e06b22fa76d97f
Author: MaSai <[email protected]>
AuthorDate: Fri Aug 28 09:45:27 2026 +0800
[#12297] feat(secret): Support setSecretBinding/setSecretReference on
catalog/schema alter (#12674)
### What changes were proposed in this pull request?
Follow-up to #12646 (fileset alter). Adds catalog / schema alter support
for `setSecretBinding` / `setSecretReference`:
- API / DTO / OpenAPI / Java & Python clients: new `CatalogChange` and
`SchemaChange` types
- `CatalogManager` / `SchemaOperationDispatcher`: local
`prepare*SecretChanges` loops calling existing `SecretManager` alter
helpers, rewriting secret ops to `setProperty` / `removeProperty` (same
pattern as `FilesetOperationDispatcher`)
- `OperationDispatcher`: validate alter upserts for schema secret
changes
- Unit tests for alter removeProperty write-through cleanup and JSON
serde
### Why are the changes needed?
Create-time `secretBindings` / `secretReferences` for catalog and schema
are already on `main` (#12420). Catalog and schema alter still need the
typed secret update contract from the entity-secrets design (ยง5.9.4).
Fix: #12297
### Does this PR introduce _any_ user-facing change?
Yes. Catalog and schema alter APIs / REST update requests gain
`setSecretBinding` and `setSecretReference`.
### How was this patch tested?
```bash
./gradlew spotlessApply
./gradlew :core:test \
--tests 'org.apache.gravitino.secret.TestSecretManagerAlter' \
--tests
'org.apache.gravitino.catalog.TestCatalogManager.testAlterRemovePropertyDeletesWriteThroughSecret'
\
--tests
'org.apache.gravitino.catalog.TestCatalogManager.testSecretRollback' \
--tests
'org.apache.gravitino.catalog.TestSchemaOperationDispatcher.testAlterRemovePropertyDeletesWriteThroughSecret'
\
--tests
'org.apache.gravitino.catalog.TestSchemaOperationDispatcher.testCreateAndAlterSchemaRejectMaskedPlaceholder'
\
:common:test --tests 'org.apache.gravitino.json.TestRequestJsonSerDe' \
:docs:build \
-PskipITs
```
---------
Co-authored-by: Cursor <[email protected]>
---
.../java/org/apache/gravitino/CatalogChange.java | 118 ++++++++++++
.../java/org/apache/gravitino/SchemaChange.java | 118 ++++++++++++
.../catalog/fileset/FilesetCatalogOperations.java | 24 ++-
.../org/apache/gravitino/client/DTOConverters.java | 29 +++
.../client-python/gravitino/api/catalog_change.py | 86 +++++++++
.../client-python/gravitino/api/schema_change.py | 65 +++++++
.../gravitino/client/base_schema_catalog.py | 10 ++
.../gravitino/client/dto_converters.py | 10 ++
.../dto/requests/catalog_update_request.py | 64 ++++++-
.../dto/requests/schema_update_request.py | 63 +++++++
.../dto/requests/CatalogUpdateRequest.java | 125 ++++++++++++-
.../dto/requests/SchemaUpdateRequest.java | 125 ++++++++++++-
.../gravitino/json/TestRequestJsonSerDe.java | 41 +++--
.../apache/gravitino/catalog/CatalogManager.java | 80 ++++++---
.../catalog/FilesetOperationDispatcher.java | 83 +++------
.../gravitino/catalog/ManagedSchemaOperations.java | 34 +---
.../gravitino/catalog/OperationDispatcher.java | 6 +
.../gravitino/catalog/SchemaEntityChanges.java | 70 ++++++++
.../catalog/SchemaOperationDispatcher.java | 139 ++++++++++++++-
.../gravitino/secret/SecretAlterChanges.java | 198 +++++++++++++++++++++
.../gravitino/secret/SecretMaterialsHolder.java | 42 +++++
.../gravitino/catalog/TestCatalogManager.java | 10 ++
.../catalog/TestFilesetOperationDispatcher.java | 53 +-----
.../catalog/TestSchemaOperationDispatcher.java | 4 +
docs/open-api/catalogs.yaml | 65 +++++++
docs/open-api/schemas.yaml | 65 +++++++
26 files changed, 1545 insertions(+), 182 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/CatalogChange.java
b/api/src/main/java/org/apache/gravitino/CatalogChange.java
index d1b82cf29d..d1af43eee1 100644
--- a/api/src/main/java/org/apache/gravitino/CatalogChange.java
+++ b/api/src/main/java/org/apache/gravitino/CatalogChange.java
@@ -20,6 +20,8 @@ package org.apache.gravitino;
import java.util.Objects;
import org.apache.gravitino.annotation.Evolving;
+import org.apache.gravitino.secret.SecretBinding;
+import org.apache.gravitino.secret.SecretReference;
/**
* A catalog change is a change to a catalog. It can be used to rename a
catalog, update the comment
@@ -69,6 +71,28 @@ public interface CatalogChange {
return new RemoveProperty(property);
}
+ /**
+ * Creates a new catalog change to bind a write-through secret for a
property.
+ *
+ * @param property The property name to bind.
+ * @param binding The write-through binding ({@code provider} + {@code
plaintext}).
+ * @return The catalog change.
+ */
+ static CatalogChange setSecretBinding(String property, SecretBinding
binding) {
+ return new SetSecretBinding(property, binding);
+ }
+
+ /**
+ * Creates a new catalog change to bind an external secret reference for a
property.
+ *
+ * @param property The property name to bind.
+ * @param reference The external secret locator ({@code provider} + {@code
attributes}).
+ * @return The catalog change.
+ */
+ static CatalogChange setSecretReference(String property, SecretReference
reference) {
+ return new SetSecretReference(property, reference);
+ }
+
/** A catalog change to rename the catalog. */
final class RenameCatalog implements CatalogChange {
private final String newName;
@@ -300,4 +324,98 @@ public interface CatalogChange {
return "REMOVEPROPERTY " + property;
}
}
+
+ /** A catalog change to bind a write-through secret for a property. */
+ final class SetSecretBinding implements CatalogChange {
+ private final String property;
+ private final SecretBinding binding;
+
+ private SetSecretBinding(String property, SecretBinding binding) {
+ this.property = property;
+ this.binding = binding;
+ }
+
+ /**
+ * Retrieves the property name being bound.
+ *
+ * @return The property name.
+ */
+ public String getProperty() {
+ return property;
+ }
+
+ /**
+ * Retrieves the write-through binding.
+ *
+ * @return The secret binding.
+ */
+ public SecretBinding getBinding() {
+ return binding;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ SetSecretBinding that = (SetSecretBinding) o;
+ return Objects.equals(property, that.property) &&
Objects.equals(binding, that.binding);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(property, binding);
+ }
+
+ @Override
+ public String toString() {
+ return "SETSECRETBINDING " + property + " " + binding;
+ }
+ }
+
+ /** A catalog change to bind an external secret reference for a property. */
+ final class SetSecretReference implements CatalogChange {
+ private final String property;
+ private final SecretReference reference;
+
+ private SetSecretReference(String property, SecretReference reference) {
+ this.property = property;
+ this.reference = reference;
+ }
+
+ /**
+ * Retrieves the property name being bound.
+ *
+ * @return The property name.
+ */
+ public String getProperty() {
+ return property;
+ }
+
+ /**
+ * Retrieves the external secret reference.
+ *
+ * @return The secret reference.
+ */
+ public SecretReference getReference() {
+ return reference;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ SetSecretReference that = (SetSecretReference) o;
+ return Objects.equals(property, that.property) &&
Objects.equals(reference, that.reference);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(property, reference);
+ }
+
+ @Override
+ public String toString() {
+ return "SETSECRETREFERENCE " + property + " " + reference;
+ }
+ }
}
diff --git a/api/src/main/java/org/apache/gravitino/SchemaChange.java
b/api/src/main/java/org/apache/gravitino/SchemaChange.java
index 45fc7c04ca..c8e2291160 100644
--- a/api/src/main/java/org/apache/gravitino/SchemaChange.java
+++ b/api/src/main/java/org/apache/gravitino/SchemaChange.java
@@ -22,6 +22,8 @@ package org.apache.gravitino;
import java.util.Objects;
import org.apache.gravitino.annotation.Evolving;
+import org.apache.gravitino.secret.SecretBinding;
+import org.apache.gravitino.secret.SecretReference;
/** NamespaceChange class to set the property and value pairs for the
namespace. */
@Evolving
@@ -48,6 +50,28 @@ public interface SchemaChange {
return new RemoveProperty(property);
}
+ /**
+ * Creates a schema change to bind a write-through secret for a property.
+ *
+ * @param property The property name to bind.
+ * @param binding The write-through binding ({@code provider} + {@code
plaintext}).
+ * @return The SchemaChange object.
+ */
+ static SchemaChange setSecretBinding(String property, SecretBinding binding)
{
+ return new SetSecretBinding(property, binding);
+ }
+
+ /**
+ * Creates a schema change to bind an external secret reference for a
property.
+ *
+ * @param property The property name to bind.
+ * @param reference The external secret locator ({@code provider} + {@code
attributes}).
+ * @return The SchemaChange object.
+ */
+ static SchemaChange setSecretReference(String property, SecretReference
reference) {
+ return new SetSecretReference(property, reference);
+ }
+
/** SchemaChange class to set the property and value pairs for the schema. */
final class SetProperty implements SchemaChange {
private final String property;
@@ -168,4 +192,98 @@ public interface SchemaChange {
return "REMOVEPROPERTY " + property;
}
}
+
+ /** SchemaChange to bind a write-through secret for a property. */
+ final class SetSecretBinding implements SchemaChange {
+ private final String property;
+ private final SecretBinding binding;
+
+ private SetSecretBinding(String property, SecretBinding binding) {
+ this.property = property;
+ this.binding = binding;
+ }
+
+ /**
+ * Retrieves the property name being bound.
+ *
+ * @return The property name.
+ */
+ public String getProperty() {
+ return property;
+ }
+
+ /**
+ * Retrieves the write-through binding.
+ *
+ * @return The secret binding.
+ */
+ public SecretBinding getBinding() {
+ return binding;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ SetSecretBinding that = (SetSecretBinding) o;
+ return Objects.equals(property, that.property) &&
Objects.equals(binding, that.binding);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(property, binding);
+ }
+
+ @Override
+ public String toString() {
+ return "SETSECRETBINDING " + property + " " + binding;
+ }
+ }
+
+ /** SchemaChange to bind an external secret reference for a property. */
+ final class SetSecretReference implements SchemaChange {
+ private final String property;
+ private final SecretReference reference;
+
+ private SetSecretReference(String property, SecretReference reference) {
+ this.property = property;
+ this.reference = reference;
+ }
+
+ /**
+ * Retrieves the property name being bound.
+ *
+ * @return The property name.
+ */
+ public String getProperty() {
+ return property;
+ }
+
+ /**
+ * Retrieves the external secret reference.
+ *
+ * @return The secret reference.
+ */
+ public SecretReference getReference() {
+ return reference;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ SetSecretReference that = (SetSecretReference) o;
+ return Objects.equals(property, that.property) &&
Objects.equals(reference, that.reference);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(property, reference);
+ }
+
+ @Override
+ public String toString() {
+ return "SETSECRETREFERENCE " + property + " " + reference;
+ }
+ }
}
diff --git
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
index f08c416d7d..24670074f6 100644
---
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
+++
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
@@ -66,6 +66,7 @@ import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.tuple.Pair;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
@@ -115,7 +116,10 @@ import org.apache.gravitino.meta.FilesetEntity;
import org.apache.gravitino.meta.SchemaEntity;
import org.apache.gravitino.metrics.MetricsSystem;
import org.apache.gravitino.metrics.source.FilesetCatalogMetricsSource;
+import org.apache.gravitino.secret.SecretAlterChanges;
import org.apache.gravitino.secret.SecretManager;
+import org.apache.gravitino.secret.SecretMaterial;
+import org.apache.gravitino.secret.SecretMaterialsHolder;
import org.apache.gravitino.utils.FilesetUtil;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
@@ -679,14 +683,26 @@ public class FilesetCatalogOperations extends
ManagedSchemaOperations
throw new RuntimeException("Failed to load fileset " + ident, ioe);
}
+ SecretMaterialsHolder writtenSecretMaterials = new SecretMaterialsHolder();
+ boolean alterCommitted = false;
try {
FilesetEntity updatedFilesetEntity =
store.update(
ident,
FilesetEntity.class,
Entity.EntityType.FILESET,
- e -> updateFilesetEntity(ident, e, changes));
-
+ existing -> {
+ Map<String, String> currentProperties =
+ existing.properties() == null
+ ? new HashMap<>()
+ : new HashMap<>(existing.properties());
+ Pair<FilesetChange[], List<SecretMaterial>> secretResult =
+ SecretAlterChanges.prepareFilesetChanges(
+ secretManager, currentProperties, existing.id(),
changes);
+ writtenSecretMaterials.set(secretResult.getRight());
+ return updateFilesetEntity(ident, existing,
secretResult.getLeft());
+ });
+ alterCommitted = true;
return FilesetImpl.builder()
.withName(updatedFilesetEntity.name())
.withComment(updatedFilesetEntity.comment())
@@ -703,6 +719,10 @@ public class FilesetCatalogOperations extends
ManagedSchemaOperations
// This is happened when renaming a fileset to an existing fileset name.
throw new RuntimeException(
"Fileset with the same name " + ident.name() + " already exists",
aee);
+ } finally {
+ if (!alterCommitted) {
+ secretManager.rollbackSecrets(writtenSecretMaterials.get());
+ }
}
}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
index 4c5408dfe7..84c6f2bbf2 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
@@ -187,6 +187,21 @@ class DTOConverters {
return new CatalogUpdateRequest.RemoveCatalogPropertyRequest(
((CatalogChange.RemoveProperty) change).getProperty());
+ } else if (change instanceof CatalogChange.SetSecretBinding) {
+ CatalogChange.SetSecretBinding setSecretBinding =
(CatalogChange.SetSecretBinding) change;
+ return new CatalogUpdateRequest.SetCatalogSecretBindingRequest(
+ setSecretBinding.getProperty(),
+ setSecretBinding.getBinding().provider(),
+ setSecretBinding.getBinding().plaintext());
+
+ } else if (change instanceof CatalogChange.SetSecretReference) {
+ CatalogChange.SetSecretReference setSecretReference =
+ (CatalogChange.SetSecretReference) change;
+ return new CatalogUpdateRequest.SetCatalogSecretReferenceRequest(
+ setSecretReference.getProperty(),
+ setSecretReference.getReference().provider(),
+ setSecretReference.getReference().attributes());
+
} else {
throw new IllegalArgumentException(
"Unknown change type: " + change.getClass().getSimpleName());
@@ -203,6 +218,20 @@ class DTOConverters {
return new SchemaUpdateRequest.RemoveSchemaPropertyRequest(
((SchemaChange.RemoveProperty) change).getProperty());
+ } else if (change instanceof SchemaChange.SetSecretBinding) {
+ SchemaChange.SetSecretBinding setSecretBinding =
(SchemaChange.SetSecretBinding) change;
+ return new SchemaUpdateRequest.SetSchemaSecretBindingRequest(
+ setSecretBinding.getProperty(),
+ setSecretBinding.getBinding().provider(),
+ setSecretBinding.getBinding().plaintext());
+
+ } else if (change instanceof SchemaChange.SetSecretReference) {
+ SchemaChange.SetSecretReference setSecretReference =
(SchemaChange.SetSecretReference) change;
+ return new SchemaUpdateRequest.SetSchemaSecretReferenceRequest(
+ setSecretReference.getProperty(),
+ setSecretReference.getReference().provider(),
+ setSecretReference.getReference().attributes());
+
} else {
throw new IllegalArgumentException(
"Unknown change type: " + change.getClass().getSimpleName());
diff --git a/clients/client-python/gravitino/api/catalog_change.py
b/clients/client-python/gravitino/api/catalog_change.py
index 0a66963dcc..c80cd9675c 100644
--- a/clients/client-python/gravitino/api/catalog_change.py
+++ b/clients/client-python/gravitino/api/catalog_change.py
@@ -17,6 +17,8 @@
from abc import ABC
+from gravitino.api.secret import SecretBinding, SecretReference
+
class CatalogChange(ABC):
"""
@@ -73,6 +75,32 @@ class CatalogChange(ABC):
"""
return CatalogChange.RemoveProperty(catalog_property)
+ @staticmethod
+ def set_secret_binding(catalog_property, binding: SecretBinding):
+ """Creates a catalog change to bind a write-through secret for a
property.
+
+ Args:
+ catalog_property: The property name to bind.
+ binding: The write-through secret binding.
+
+ Returns:
+ The catalog change.
+ """
+ return CatalogChange.SetSecretBinding(catalog_property, binding)
+
+ @staticmethod
+ def set_secret_reference(catalog_property, reference: SecretReference):
+ """Creates a catalog change to bind an external secret reference for a
property.
+
+ Args:
+ catalog_property: The property name to bind.
+ reference: The external secret reference.
+
+ Returns:
+ The catalog change.
+ """
+ return CatalogChange.SetSecretReference(catalog_property, reference)
+
class RenameCatalog:
"""A catalog change to rename the catalog."""
@@ -265,3 +293,61 @@ class CatalogChange(ABC):
A string summary of the property removal operation.
"""
return f"REMOVEPROPERTY {self._property}"
+
+ class SetSecretBinding:
+ """A catalog change to bind a write-through secret for a property."""
+
+ def __init__(self, catalog_property, binding: SecretBinding):
+ self._property = catalog_property
+ self._binding = binding
+
+ def property(self):
+ """Retrieves the property name being bound."""
+ return self._property
+
+ def binding(self):
+ """Retrieves the write-through secret binding."""
+ return self._binding
+
+ def __eq__(self, other) -> bool:
+ if not isinstance(other, CatalogChange.SetSecretBinding):
+ return False
+ return (
+ self._property == other.property()
+ and self._binding == other.binding()
+ )
+
+ def __hash__(self):
+ return hash((self._property, self._binding))
+
+ def __str__(self):
+ return f"SETSECRETBINDING {self._property} {self._binding}"
+
+ class SetSecretReference:
+ """A catalog change to bind an external secret reference for a
property."""
+
+ def __init__(self, catalog_property, reference: SecretReference):
+ self._property = catalog_property
+ self._reference = reference
+
+ def property(self):
+ """Retrieves the property name being bound."""
+ return self._property
+
+ def reference(self):
+ """Retrieves the external secret reference."""
+ return self._reference
+
+ def __eq__(self, other) -> bool:
+ if not isinstance(other, CatalogChange.SetSecretReference):
+ return False
+ return (
+ self._property == other.property()
+ and self._reference == other.reference()
+ )
+
+ def __hash__(self):
+ return hash((self._property, self._reference))
+
+ def __str__(self):
+ return f"SETSECRETREFERENCE {self._property} {self._reference}"
diff --git a/clients/client-python/gravitino/api/schema_change.py
b/clients/client-python/gravitino/api/schema_change.py
index c1cd70a9fc..2d6c8a3947 100644
--- a/clients/client-python/gravitino/api/schema_change.py
+++ b/clients/client-python/gravitino/api/schema_change.py
@@ -20,6 +20,8 @@ from dataclasses import dataclass, field
from dataclasses_json import config
+from gravitino.api.secret import SecretBinding, SecretReference
+
class SchemaChange(ABC):
"""NamespaceChange class to set the property and value pairs for the
namespace."""
@@ -49,6 +51,16 @@ class SchemaChange(ABC):
"""
return SchemaChange.RemoveProperty(schema_property)
+ @staticmethod
+ def set_secret_binding(schema_property: str, binding: SecretBinding):
+ """Creates a schema change to bind a write-through secret for a
property."""
+ return SchemaChange.SetSecretBinding(schema_property, binding)
+
+ @staticmethod
+ def set_secret_reference(schema_property: str, reference: SecretReference):
+ """Creates a schema change to bind an external secret reference for a
property."""
+ return SchemaChange.SetSecretReference(schema_property, reference)
+
@dataclass
class SetProperty:
"""SchemaChange class to set the property and value pairs for the
schema."""
@@ -149,3 +161,56 @@ class SchemaChange(ABC):
A string summary of the property removal operation.
"""
return f"REMOVEPROPERTY {self._property}"
+
+ @dataclass
+ class SetSecretBinding:
+ """SchemaChange class to bind a write-through secret for a property."""
+
+ _property: str = field(metadata=config(field_name="property"))
+ _binding: SecretBinding = field(metadata=config(field_name="binding"))
+
+ def property(self):
+ return self._property
+
+ def binding(self):
+ return self._binding
+
+ def __eq__(self, other):
+ if not isinstance(other, SchemaChange.SetSecretBinding):
+ return False
+ return (
+ self._property == other.property() and self._binding ==
other.binding()
+ )
+
+ def __hash__(self):
+ return hash((self._property, self._binding))
+
+ def __str__(self):
+ return f"SETSECRETBINDING {self._property} {self._binding}"
+
+ @dataclass
+ class SetSecretReference:
+ """SchemaChange class to bind an external secret reference for a
property."""
+
+ _property: str = field(metadata=config(field_name="property"))
+ _reference: SecretReference =
field(metadata=config(field_name="reference"))
+
+ def property(self):
+ return self._property
+
+ def reference(self):
+ return self._reference
+
+ def __eq__(self, other):
+ if not isinstance(other, SchemaChange.SetSecretReference):
+ return False
+ return (
+ self._property == other.property()
+ and self._reference == other.reference()
+ )
+
+ def __hash__(self):
+ return hash((self._property, self._reference))
+
+ def __str__(self):
+ return f"SETSECRETREFERENCE {self._property} {self._reference}"
diff --git a/clients/client-python/gravitino/client/base_schema_catalog.py
b/clients/client-python/gravitino/client/base_schema_catalog.py
index 16d9884a5a..66483a3dac 100644
--- a/clients/client-python/gravitino/client/base_schema_catalog.py
+++ b/clients/client-python/gravitino/client/base_schema_catalog.py
@@ -369,6 +369,16 @@ class BaseSchemaCatalog(
)
if isinstance(change, SchemaChange.RemoveProperty):
return
SchemaUpdateRequest.RemoveSchemaPropertyRequest(change.property())
+ if isinstance(change, SchemaChange.SetSecretBinding):
+ binding = change.binding()
+ return SchemaUpdateRequest.SetSchemaSecretBindingRequest(
+ change.property(), binding.provider, binding.plaintext
+ )
+ if isinstance(change, SchemaChange.SetSecretReference):
+ reference = change.reference()
+ return SchemaUpdateRequest.SetSchemaSecretReferenceRequest(
+ change.property(), reference.provider, reference.attributes
+ )
raise ValueError(f"Unknown change type: {type(change).__name__}")
def validate(self):
diff --git a/clients/client-python/gravitino/client/dto_converters.py
b/clients/client-python/gravitino/client/dto_converters.py
index c61ded134b..801e91b584 100644
--- a/clients/client-python/gravitino/client/dto_converters.py
+++ b/clients/client-python/gravitino/client/dto_converters.py
@@ -145,6 +145,16 @@ class DTOConverters:
return CatalogUpdateRequest.RemoveCatalogPropertyRequest(
change.get_property()
)
+ if isinstance(change, CatalogChange.SetSecretBinding):
+ binding = change.binding()
+ return CatalogUpdateRequest.SetCatalogSecretBindingRequest(
+ change.property(), binding.provider, binding.plaintext
+ )
+ if isinstance(change, CatalogChange.SetSecretReference):
+ reference = change.reference()
+ return CatalogUpdateRequest.SetCatalogSecretReferenceRequest(
+ change.property(), reference.provider, reference.attributes
+ )
raise ValueError(f"Unknown change type: {type(change).__name__}")
diff --git
a/clients/client-python/gravitino/dto/requests/catalog_update_request.py
b/clients/client-python/gravitino/dto/requests/catalog_update_request.py
index b4ba71c546..e39982f633 100644
--- a/clients/client-python/gravitino/dto/requests/catalog_update_request.py
+++ b/clients/client-python/gravitino/dto/requests/catalog_update_request.py
@@ -17,11 +17,12 @@
from abc import abstractmethod
from dataclasses import field, dataclass
-from typing import Optional
+from typing import Dict, Optional
from dataclasses_json import config
from gravitino.api.catalog_change import CatalogChange
+from gravitino.api.secret import SecretBinding, SecretReference
from gravitino.rest.rest_message import RESTRequest
@@ -124,3 +125,64 @@ class CatalogUpdateRequest:
def validate(self):
if not self._property:
raise ValueError('"property" field is required and cannot be
empty')
+
+ @dataclass
+ class SetCatalogSecretBindingRequest(CatalogUpdateRequestBase):
+ """Request to bind a write-through secret for a catalog property."""
+
+ _property: Optional[str] =
field(metadata=config(field_name="property"))
+ _provider: Optional[str] =
field(metadata=config(field_name="provider"))
+ _plaintext: Optional[str] =
field(metadata=config(field_name="plaintext"))
+
+ def __init__(self, catalog_property: str, provider: str, plaintext:
str):
+ super().__init__("setSecretBinding")
+ self._property = catalog_property
+ self._provider = provider
+ self._plaintext = plaintext
+
+ def catalog_change(self):
+ return CatalogChange.set_secret_binding(
+ self._property,
+ SecretBinding(self._provider, self._plaintext),
+ )
+
+ def validate(self):
+ if not self._property:
+ raise ValueError('"property" field is required and cannot be
empty')
+ if not self._provider:
+ raise ValueError('"provider" field is required and cannot be
empty')
+ if self._plaintext is None:
+ raise ValueError('"plaintext" field is required and cannot be
null')
+
+ @dataclass
+ class SetCatalogSecretReferenceRequest(CatalogUpdateRequestBase):
+ """Request to bind an external secret reference for a catalog
property."""
+
+ _property: Optional[str] =
field(metadata=config(field_name="property"))
+ _provider: Optional[str] =
field(metadata=config(field_name="provider"))
+ _attributes: Optional[Dict[str, str]] = field(
+ metadata=config(field_name="attributes")
+ )
+
+ def __init__(
+ self,
+ catalog_property: str,
+ provider: str,
+ attributes: Optional[Dict[str, str]] = None,
+ ):
+ super().__init__("setSecretReference")
+ self._property = catalog_property
+ self._provider = provider
+ self._attributes = attributes
+
+ def catalog_change(self):
+ return CatalogChange.set_secret_reference(
+ self._property,
+ SecretReference(self._provider, self._attributes or {}),
+ )
+
+ def validate(self):
+ if not self._property:
+ raise ValueError('"property" field is required and cannot be
empty')
+ if not self._provider:
+ raise ValueError('"provider" field is required and cannot be
empty')
diff --git
a/clients/client-python/gravitino/dto/requests/schema_update_request.py
b/clients/client-python/gravitino/dto/requests/schema_update_request.py
index 7475f5304e..7872d6e838 100644
--- a/clients/client-python/gravitino/dto/requests/schema_update_request.py
+++ b/clients/client-python/gravitino/dto/requests/schema_update_request.py
@@ -17,10 +17,12 @@
from abc import abstractmethod
from dataclasses import dataclass, field
+from typing import Dict, Optional
from dataclasses_json import config
from gravitino.api.schema_change import SchemaChange
+from gravitino.api.secret import SecretBinding, SecretReference
from gravitino.rest.rest_message import RESTRequest
@@ -95,3 +97,64 @@ class SchemaUpdateRequest:
def schema_change(self):
return SchemaChange.remove_property(self._property)
+
+ @dataclass
+ class SetSchemaSecretBindingRequest(SchemaUpdateRequestBase):
+ """Represents a request to bind a write-through secret for a schema
property."""
+
+ _property: Optional[str] =
field(metadata=config(field_name="property"))
+ _provider: Optional[str] =
field(metadata=config(field_name="provider"))
+ _plaintext: Optional[str] =
field(metadata=config(field_name="plaintext"))
+
+ def __init__(self, schema_property: str, provider: str, plaintext:
str):
+ super().__init__("setSecretBinding")
+ self._property = schema_property
+ self._provider = provider
+ self._plaintext = plaintext
+
+ def validate(self):
+ if not self._property:
+ raise ValueError('"property" field is required and cannot be
empty')
+ if not self._provider:
+ raise ValueError('"provider" field is required and cannot be
empty')
+ if self._plaintext is None:
+ raise ValueError('"plaintext" field is required and cannot be
null')
+
+ def schema_change(self):
+ return SchemaChange.set_secret_binding(
+ self._property,
+ SecretBinding(self._provider, self._plaintext),
+ )
+
+ @dataclass
+ class SetSchemaSecretReferenceRequest(SchemaUpdateRequestBase):
+ """Represents a request to bind an external secret reference for a
schema property."""
+
+ _property: Optional[str] =
field(metadata=config(field_name="property"))
+ _provider: Optional[str] =
field(metadata=config(field_name="provider"))
+ _attributes: Optional[Dict[str, str]] = field(
+ metadata=config(field_name="attributes")
+ )
+
+ def __init__(
+ self,
+ schema_property: str,
+ provider: str,
+ attributes: Optional[Dict[str, str]] = None,
+ ):
+ super().__init__("setSecretReference")
+ self._property = schema_property
+ self._provider = provider
+ self._attributes = attributes
+
+ def validate(self):
+ if not self._property:
+ raise ValueError('"property" field is required and cannot be
empty')
+ if not self._provider:
+ raise ValueError('"provider" field is required and cannot be
empty')
+
+ def schema_change(self):
+ return SchemaChange.set_secret_reference(
+ self._property,
+ SecretReference(self._provider, self._attributes or {}),
+ )
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
index aabdf2901f..107f95c149 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
@@ -23,12 +23,16 @@ import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.ToString;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.CatalogChange;
import org.apache.gravitino.rest.RESTRequest;
+import org.apache.gravitino.secret.SecretBinding;
+import org.apache.gravitino.secret.SecretReference;
/** Represents an interface for catalog update requests. */
@JsonIgnoreProperties(ignoreUnknown = true)
@@ -43,7 +47,13 @@ import org.apache.gravitino.rest.RESTRequest;
name = "setProperty"),
@JsonSubTypes.Type(
value = CatalogUpdateRequest.RemoveCatalogPropertyRequest.class,
- name = "removeProperty")
+ name = "removeProperty"),
+ @JsonSubTypes.Type(
+ value = CatalogUpdateRequest.SetCatalogSecretBindingRequest.class,
+ name = "setSecretBinding"),
+ @JsonSubTypes.Type(
+ value = CatalogUpdateRequest.SetCatalogSecretReferenceRequest.class,
+ name = "setSecretReference")
})
public interface CatalogUpdateRequest extends RESTRequest {
@@ -214,4 +224,117 @@ public interface CatalogUpdateRequest extends RESTRequest
{
return CatalogChange.removeProperty(property);
}
}
+
+ /** Request to bind a write-through secret for a catalog property. */
+ @EqualsAndHashCode
+ @ToString(exclude = "plaintext")
+ class SetCatalogSecretBindingRequest implements CatalogUpdateRequest {
+
+ @Getter
+ @JsonProperty("property")
+ private final String property;
+
+ @Getter
+ @JsonProperty("provider")
+ private final String provider;
+
+ @Getter
+ @JsonProperty("plaintext")
+ private final String plaintext;
+
+ /**
+ * Constructor for SetCatalogSecretBindingRequest.
+ *
+ * @param property The property to bind.
+ * @param provider The registered secrets-provider instance name.
+ * @param plaintext The plaintext secret to write through.
+ */
+ public SetCatalogSecretBindingRequest(String property, String provider,
String plaintext) {
+ this.property = property;
+ this.provider = provider;
+ this.plaintext = plaintext;
+ }
+
+ /** Default constructor for SetCatalogSecretBindingRequest. */
+ public SetCatalogSecretBindingRequest() {
+ this(null, null, null);
+ }
+
+ /**
+ * Validates the fields of the request.
+ *
+ * @throws IllegalArgumentException if required fields are not set.
+ */
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(property), "\"property\" field is required
and cannot be empty");
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(provider), "\"provider\" field is required
and cannot be empty");
+ Preconditions.checkArgument(
+ plaintext != null, "\"plaintext\" field is required and cannot be
null");
+ }
+
+ @Override
+ public CatalogChange catalogChange() {
+ return CatalogChange.setSecretBinding(property, new
SecretBinding(provider, plaintext));
+ }
+ }
+
+ /** Request to bind an external secret reference for a catalog property. */
+ @EqualsAndHashCode
+ @ToString
+ class SetCatalogSecretReferenceRequest implements CatalogUpdateRequest {
+
+ @Getter
+ @JsonProperty("property")
+ private final String property;
+
+ @Getter
+ @JsonProperty("provider")
+ private final String provider;
+
+ @Getter
+ @JsonProperty("attributes")
+ private final Map<String, String> attributes;
+
+ /**
+ * Constructor for SetCatalogSecretReferenceRequest.
+ *
+ * @param property The property to bind.
+ * @param provider The registered secrets-provider instance name.
+ * @param attributes Provider-specific locator attributes.
+ */
+ public SetCatalogSecretReferenceRequest(
+ String property, String provider, Map<String, String> attributes) {
+ this.property = property;
+ this.provider = provider;
+ this.attributes = attributes;
+ }
+
+ /** Default constructor for SetCatalogSecretReferenceRequest. */
+ public SetCatalogSecretReferenceRequest() {
+ this(null, null, null);
+ }
+
+ /**
+ * Validates the fields of the request.
+ *
+ * @throws IllegalArgumentException if required fields are not set.
+ */
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(property), "\"property\" field is required
and cannot be empty");
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(provider), "\"provider\" field is required
and cannot be empty");
+ }
+
+ @Override
+ public CatalogChange catalogChange() {
+ return CatalogChange.setSecretReference(
+ property,
+ new SecretReference(provider, attributes == null ? ImmutableMap.of()
: attributes));
+ }
+ }
}
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/SchemaUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/SchemaUpdateRequest.java
index 7a2e650309..99f57abfa6 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/SchemaUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/SchemaUpdateRequest.java
@@ -23,12 +23,16 @@ import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.ToString;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.SchemaChange;
import org.apache.gravitino.rest.RESTRequest;
+import org.apache.gravitino.secret.SecretBinding;
+import org.apache.gravitino.secret.SecretReference;
/** Represents a request to update a schema. */
@JsonIgnoreProperties(ignoreUnknown = true)
@@ -39,7 +43,13 @@ import org.apache.gravitino.rest.RESTRequest;
name = "setProperty"),
@JsonSubTypes.Type(
value = SchemaUpdateRequest.RemoveSchemaPropertyRequest.class,
- name = "removeProperty")
+ name = "removeProperty"),
+ @JsonSubTypes.Type(
+ value = SchemaUpdateRequest.SetSchemaSecretBindingRequest.class,
+ name = "setSecretBinding"),
+ @JsonSubTypes.Type(
+ value = SchemaUpdateRequest.SetSchemaSecretReferenceRequest.class,
+ name = "setSecretReference")
})
public interface SchemaUpdateRequest extends RESTRequest {
@@ -147,4 +157,117 @@ public interface SchemaUpdateRequest extends RESTRequest {
return SchemaChange.removeProperty(property);
}
}
+
+ /** Represents a request to bind a write-through secret for a schema
property. */
+ @EqualsAndHashCode
+ @ToString(exclude = "plaintext")
+ class SetSchemaSecretBindingRequest implements SchemaUpdateRequest {
+
+ @Getter
+ @JsonProperty("property")
+ private final String property;
+
+ @Getter
+ @JsonProperty("provider")
+ private final String provider;
+
+ @Getter
+ @JsonProperty("plaintext")
+ private final String plaintext;
+
+ /**
+ * Creates a new SetSchemaSecretBindingRequest.
+ *
+ * @param property The property to bind.
+ * @param provider The registered secrets-provider instance name.
+ * @param plaintext The plaintext secret to write through.
+ */
+ public SetSchemaSecretBindingRequest(String property, String provider,
String plaintext) {
+ this.property = property;
+ this.provider = provider;
+ this.plaintext = plaintext;
+ }
+
+ /** Default constructor for Jackson deserialization. */
+ public SetSchemaSecretBindingRequest() {
+ this(null, null, null);
+ }
+
+ /**
+ * Validates the request.
+ *
+ * @throws IllegalArgumentException If the request is invalid, this
exception is thrown.
+ */
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(property), "\"property\" field is required
and cannot be empty");
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(provider), "\"provider\" field is required
and cannot be empty");
+ Preconditions.checkArgument(
+ plaintext != null, "\"plaintext\" field is required and cannot be
null");
+ }
+
+ @Override
+ public SchemaChange schemaChange() {
+ return SchemaChange.setSecretBinding(property, new
SecretBinding(provider, plaintext));
+ }
+ }
+
+ /** Represents a request to bind an external secret reference for a schema
property. */
+ @EqualsAndHashCode
+ @ToString
+ class SetSchemaSecretReferenceRequest implements SchemaUpdateRequest {
+
+ @Getter
+ @JsonProperty("property")
+ private final String property;
+
+ @Getter
+ @JsonProperty("provider")
+ private final String provider;
+
+ @Getter
+ @JsonProperty("attributes")
+ private final Map<String, String> attributes;
+
+ /**
+ * Creates a new SetSchemaSecretReferenceRequest.
+ *
+ * @param property The property to bind.
+ * @param provider The registered secrets-provider instance name.
+ * @param attributes Provider-specific locator attributes.
+ */
+ public SetSchemaSecretReferenceRequest(
+ String property, String provider, Map<String, String> attributes) {
+ this.property = property;
+ this.provider = provider;
+ this.attributes = attributes;
+ }
+
+ /** Default constructor for Jackson deserialization. */
+ public SetSchemaSecretReferenceRequest() {
+ this(null, null, null);
+ }
+
+ /**
+ * Validates the request.
+ *
+ * @throws IllegalArgumentException If the request is invalid, this
exception is thrown.
+ */
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(property), "\"property\" field is required
and cannot be empty");
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(provider), "\"provider\" field is required
and cannot be empty");
+ }
+
+ @Override
+ public SchemaChange schemaChange() {
+ return SchemaChange.setSecretReference(
+ property,
+ new SecretReference(provider, attributes == null ? ImmutableMap.of()
: attributes));
+ }
+ }
}
diff --git
a/common/src/test/java/org/apache/gravitino/json/TestRequestJsonSerDe.java
b/common/src/test/java/org/apache/gravitino/json/TestRequestJsonSerDe.java
index 5ccd0a1cf1..9887e1df86 100644
--- a/common/src/test/java/org/apache/gravitino/json/TestRequestJsonSerDe.java
+++ b/common/src/test/java/org/apache/gravitino/json/TestRequestJsonSerDe.java
@@ -28,6 +28,7 @@ import org.apache.gravitino.dto.requests.FilesetUpdateRequest;
import org.apache.gravitino.dto.requests.MetalakeCreateRequest;
import org.apache.gravitino.dto.requests.MetalakeUpdateRequest;
import org.apache.gravitino.dto.requests.MetalakeUpdatesRequest;
+import org.apache.gravitino.dto.requests.SchemaUpdateRequest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -159,20 +160,32 @@ public class TestRequestJsonSerDe {
}
@Test
- public void testFilesetUpdateRequestSerDe() throws JsonProcessingException {
- FilesetUpdateRequest req =
- new FilesetUpdateRequest.SetFilesetSecretBindingRequest("password",
"env", "secret");
- String serJson = JsonUtils.objectMapper().writeValueAsString(req);
- FilesetUpdateRequest deserReq =
- JsonUtils.objectMapper().readValue(serJson,
FilesetUpdateRequest.class);
- Assertions.assertEquals(req, deserReq);
-
- FilesetUpdateRequest req1 =
+ public void testSecretUpdateRequestSerDe() throws JsonProcessingException {
+ roundTrip(
+ new CatalogUpdateRequest.SetCatalogSecretBindingRequest("password",
"env", "secret"),
+ CatalogUpdateRequest.class);
+ roundTrip(
+ new CatalogUpdateRequest.SetCatalogSecretReferenceRequest(
+ "password", "vault", ImmutableMap.of("path",
"secret/data/my-password")),
+ CatalogUpdateRequest.class);
+ roundTrip(
+ new SchemaUpdateRequest.SetSchemaSecretBindingRequest("password",
"env", "secret"),
+ SchemaUpdateRequest.class);
+ roundTrip(
+ new SchemaUpdateRequest.SetSchemaSecretReferenceRequest(
+ "password", "vault", ImmutableMap.of("path",
"secret/data/my-password")),
+ SchemaUpdateRequest.class);
+ roundTrip(
+ new FilesetUpdateRequest.SetFilesetSecretBindingRequest("password",
"env", "secret"),
+ FilesetUpdateRequest.class);
+ roundTrip(
new FilesetUpdateRequest.SetFilesetSecretReferenceRequest(
- "password", "vault", ImmutableMap.of("path",
"secret/data/my-password"));
- String serJson1 = JsonUtils.objectMapper().writeValueAsString(req1);
- FilesetUpdateRequest deserReq1 =
- JsonUtils.objectMapper().readValue(serJson1,
FilesetUpdateRequest.class);
- Assertions.assertEquals(req1, deserReq1);
+ "password", "vault", ImmutableMap.of("path",
"secret/data/my-password")),
+ FilesetUpdateRequest.class);
+ }
+
+ private static <T> void roundTrip(T request, Class<T> type) throws
JsonProcessingException {
+ String json = JsonUtils.objectMapper().writeValueAsString(request);
+ Assertions.assertEquals(request, JsonUtils.objectMapper().readValue(json,
type));
}
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
index bd15b808bd..8cc0625f6a 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -108,9 +108,11 @@ import org.apache.gravitino.rel.SupportsPartitions;
import org.apache.gravitino.rel.Table;
import org.apache.gravitino.rel.TableCatalog;
import org.apache.gravitino.rel.ViewCatalog;
+import org.apache.gravitino.secret.SecretAlterChanges;
import org.apache.gravitino.secret.SecretBinding;
import org.apache.gravitino.secret.SecretManager;
import org.apache.gravitino.secret.SecretMaterial;
+import org.apache.gravitino.secret.SecretMaterialsHolder;
import org.apache.gravitino.secret.SecretPropertyUtils;
import org.apache.gravitino.secret.SecretReference;
import org.apache.gravitino.storage.IdGenerator;
@@ -926,23 +928,7 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
LockType.WRITE,
() -> {
try {
- CatalogEntity updatedCatalog =
- store.update(
- ident,
- CatalogEntity.class,
- EntityType.CATALOG,
- catalog -> {
- CatalogEntity.Builder newCatalogBuilder =
- newCatalogBuilder(ident.namespace(), catalog);
-
- Map<String, String> newProps =
- catalog.getProperties() == null
- ? new HashMap<>()
- : new HashMap<>(catalog.getProperties());
- newCatalogBuilder = updateEntity(newCatalogBuilder,
newProps, changes);
-
- return newCatalogBuilder.build();
- });
+ CatalogEntity updatedCatalog = alterCatalogUnderLock(ident,
changes);
// Invalidate after store.update() so that any background thread
that tries to reload
// the old catalog identifier from the store (after the
invalidate) will get
// NoSuchCatalogException instead of stale data. Invalidating
before the update creates
@@ -959,14 +945,17 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
catalogCache.put(convertedCatalog.nameIdentifier(), newWrapper);
return newWrapper.catalog();
- } catch (NoSuchEntityException ne) {
- LOG.warn("Catalog {} does not exist", ident, ne);
- throw new NoSuchCatalogException(CATALOG_DOES_NOT_EXIST_MSG,
ident);
+ } catch (NoSuchCatalogException e) {
+ throw e;
} catch (IllegalArgumentException iae) {
LOG.warn("Failed to alter catalog {} with unknown change", ident,
iae);
throw iae;
+ } catch (NoSuchEntityException ne) {
+ LOG.warn("Catalog {} does not exist", ident, ne);
+ throw new NoSuchCatalogException(CATALOG_DOES_NOT_EXIST_MSG,
ident);
+
} catch (IOException ioe) {
LOG.error("Failed to alter catalog {}", ident, ioe);
throw new RuntimeException(ioe);
@@ -974,6 +963,48 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
});
}
+ private CatalogEntity alterCatalogUnderLock(NameIdentifier ident,
CatalogChange... changes)
+ throws NoSuchCatalogException, IllegalArgumentException, IOException {
+ SecretMaterialsHolder writtenSecretMaterials = new SecretMaterialsHolder();
+ boolean alterCommitted = false;
+ try {
+ CatalogEntity updatedCatalog =
+ store.update(
+ ident,
+ CatalogEntity.class,
+ EntityType.CATALOG,
+ existing -> {
+ Map<String, String> currentProperties =
+ existing.getProperties() == null
+ ? new HashMap<>()
+ : new HashMap<>(existing.getProperties());
+
+ Pair<CatalogChange[], List<SecretMaterial>> secretResult =
+ SecretAlterChanges.prepareCatalogChanges(
+ secretManager, currentProperties, existing.id(),
changes);
+ writtenSecretMaterials.set(secretResult.getRight());
+ CatalogChange[] effectiveChanges = secretResult.getLeft();
+
+ CatalogEntity.Builder newCatalogBuilder =
+ newCatalogBuilder(ident.namespace(), existing);
+
+ Map<String, String> newProps =
+ existing.getProperties() == null
+ ? new HashMap<>()
+ : new HashMap<>(existing.getProperties());
+ return updateEntity(newCatalogBuilder, newProps,
effectiveChanges).build();
+ });
+ alterCommitted = true;
+ return updatedCatalog;
+ } catch (NoSuchEntityException e) {
+ throw new NoSuchCatalogException(CATALOG_DOES_NOT_EXIST_MSG, ident);
+ } finally {
+ if (!alterCommitted) {
+ secretManager.rollbackSecrets(writtenSecretMaterials.get());
+ }
+ }
+ }
+
@Override
public boolean dropCatalog(NameIdentifier ident, boolean force)
throws NonEmptyEntityException, CatalogInUseException {
@@ -1238,6 +1269,15 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
if (catalogChange instanceof SetProperty) {
SetProperty setProperty = (SetProperty) catalogChange;
upserts.put(setProperty.getProperty(), setProperty.getValue());
+ } else if (catalogChange instanceof
CatalogChange.SetSecretBinding) {
+ CatalogChange.SetSecretBinding setSecretBinding =
+ (CatalogChange.SetSecretBinding) catalogChange;
+ upserts.put(
+ setSecretBinding.getProperty(),
setSecretBinding.getBinding().plaintext());
+ } else if (catalogChange instanceof
CatalogChange.SetSecretReference) {
+ CatalogChange.SetSecretReference setSecretReference =
+ (CatalogChange.SetSecretReference) catalogChange;
+ upserts.put(setSecretReference.getProperty(),
setSecretReference.getProperty());
} else if (catalogChange instanceof RemoveProperty) {
RemoveProperty removeProperty = (RemoveProperty) catalogChange;
deletes.put(removeProperty.getProperty(),
removeProperty.getProperty());
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java
index 00f7bbe907..4dd30a42e9 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java
@@ -22,12 +22,10 @@ import static
org.apache.gravitino.Entity.EntityType.FILESET;
import static
org.apache.gravitino.catalog.PropertiesMetadataHelpers.validatePropertyForCreate;
import static
org.apache.gravitino.utils.NameIdentifierUtil.getCatalogIdentifier;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import javax.annotation.Nullable;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.NameIdentifier;
@@ -45,9 +43,11 @@ import org.apache.gravitino.file.FilesetChange;
import org.apache.gravitino.lock.LockType;
import org.apache.gravitino.lock.TreeLockUtils;
import org.apache.gravitino.meta.FilesetEntity;
+import org.apache.gravitino.secret.SecretAlterChanges;
import org.apache.gravitino.secret.SecretBinding;
import org.apache.gravitino.secret.SecretManager;
import org.apache.gravitino.secret.SecretMaterial;
+import org.apache.gravitino.secret.SecretMaterialsHolder;
import org.apache.gravitino.secret.SecretPropertyUtils;
import org.apache.gravitino.secret.SecretReference;
import org.apache.gravitino.storage.IdGenerator;
@@ -256,6 +256,24 @@ public class FilesetOperationDispatcher extends
OperationDispatcher implements F
private Fileset alterFilesetUnderLock(
NameIdentifier ident, NameIdentifier catalogIdent, FilesetChange...
changes) {
+ validateAlterProperties(ident,
HasPropertyMetadata::filesetPropertiesMetadata, changes);
+ if (usesFilesetCatalogEntityStore(catalogIdent)) {
+ return doWithCatalog(
+ catalogIdent,
+ c -> c.doWithFilesetOps(f -> f.alterFileset(ident, changes)),
+ NoSuchFilesetException.class,
+ IllegalArgumentException.class);
+ }
+ return alterFilesetWithPreparedSecrets(ident, catalogIdent, changes);
+ }
+
+ private boolean usesFilesetCatalogEntityStore(NameIdentifier catalogIdent) {
+ return doWithCatalog(
+ catalogIdent, c -> "fileset".equals(c.catalog().provider()),
NoSuchFilesetException.class);
+ }
+
+ private Fileset alterFilesetWithPreparedSecrets(
+ NameIdentifier ident, NameIdentifier catalogIdent, FilesetChange...
changes) {
Fileset currentFileset =
doWithCatalog(
catalogIdent,
@@ -274,8 +292,6 @@ public class FilesetOperationDispatcher extends
OperationDispatcher implements F
currentProperties = new HashMap<>();
}
- validateAlterProperties(ident,
HasPropertyMetadata::filesetPropertiesMetadata, changes);
-
StringIdentifier currentStringId =
getStringIdFromProperties(currentProperties);
long filesetId;
if (currentStringId != null) {
@@ -286,12 +302,13 @@ public class FilesetOperationDispatcher extends
OperationDispatcher implements F
filesetId = 0L;
}
- List<SecretMaterial> writtenSecretMaterials = List.of();
+ SecretMaterialsHolder writtenSecretMaterials = new SecretMaterialsHolder();
boolean alterCommitted = false;
try {
Pair<FilesetChange[], List<SecretMaterial>> secretResult =
- prepareFilesetSecretChanges(currentProperties, filesetId, changes);
- writtenSecretMaterials = secretResult.getRight();
+ SecretAlterChanges.prepareFilesetChanges(
+ secretManager, currentProperties, filesetId, changes);
+ writtenSecretMaterials.set(secretResult.getRight());
FilesetChange[] effectiveChanges = secretResult.getLeft();
Fileset altered =
@@ -304,7 +321,7 @@ public class FilesetOperationDispatcher extends
OperationDispatcher implements F
return altered;
} finally {
if (!alterCommitted) {
- secretManager.rollbackSecrets(writtenSecretMaterials);
+ secretManager.rollbackSecrets(writtenSecretMaterials.get());
}
}
}
@@ -373,54 +390,4 @@ public class FilesetOperationDispatcher extends
OperationDispatcher implements F
c -> c.doWithFilesetOps(f -> f.getFileLocation(ident, subPath,
locationName)),
NonEmptyEntityException.class));
}
-
- /**
- * Rewrites fileset changes that involve secrets into plain setProperty /
removeProperty, writing
- * secrets as needed. Rolls back any written materials if preparation fails.
- *
- * @param currentProperties current fileset properties (may be null)
- * @param entityId fileset entity id
- * @param changes fileset changes
- * @return effective changes and written write-through materials
- */
- private Pair<FilesetChange[], List<SecretMaterial>>
prepareFilesetSecretChanges(
- @Nullable Map<String, String> currentProperties, long entityId,
FilesetChange... changes) {
- Map<String, String> properties =
- currentProperties == null ? new HashMap<>() : new
HashMap<>(currentProperties);
- List<FilesetChange> out = new ArrayList<>(changes.length);
- List<SecretMaterial> written = new ArrayList<>();
- try {
- for (FilesetChange change : changes) {
- if (change instanceof FilesetChange.SetSecretBinding) {
- FilesetChange.SetSecretBinding c = (FilesetChange.SetSecretBinding)
change;
- String urn =
- secretManager.alterSetSecretBinding(
- properties, "fileset", entityId, c.getProperty(),
c.getBinding(), written);
- out.add(FilesetChange.setProperty(c.getProperty(), urn));
- } else if (change instanceof FilesetChange.SetSecretReference) {
- FilesetChange.SetSecretReference c =
(FilesetChange.SetSecretReference) change;
- String urn =
- secretManager.alterSetSecretReference(
- properties, "fileset", entityId, c.getProperty(),
c.getReference());
- out.add(FilesetChange.setProperty(c.getProperty(), urn));
- } else if (change instanceof FilesetChange.SetProperty) {
- FilesetChange.SetProperty c = (FilesetChange.SetProperty) change;
- String value =
- secretManager.alterSetProperty(
- properties, "fileset", entityId, c.getProperty(),
c.getValue());
- out.add(FilesetChange.setProperty(c.getProperty(), value));
- } else if (change instanceof FilesetChange.RemoveProperty) {
- FilesetChange.RemoveProperty c = (FilesetChange.RemoveProperty)
change;
- secretManager.alterRemoveProperty(properties, "fileset", entityId,
c.getProperty());
- out.add(change);
- } else {
- out.add(change);
- }
- }
- return Pair.of(out.toArray(new FilesetChange[0]), List.copyOf(written));
- } catch (RuntimeException e) {
- secretManager.rollbackSecrets(written);
- throw e;
- }
- }
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java
b/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java
index 1c2dbade53..e4a78a9db8 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java
@@ -19,7 +19,6 @@
package org.apache.gravitino.catalog;
import com.google.common.base.Preconditions;
-import com.google.common.collect.Maps;
import java.io.IOException;
import java.time.Instant;
import java.util.List;
@@ -203,37 +202,6 @@ public abstract class ManagedSchemaOperations implements
SupportsSchemas {
private SchemaEntity updateSchemaEntity(
NameIdentifier ident, SchemaEntity schemaEntity, SchemaChange...
changes) {
- Map<String, String> props =
- schemaEntity.properties() == null
- ? Maps.newHashMap()
- : Maps.newHashMap(schemaEntity.properties());
-
- for (SchemaChange change : changes) {
- if (change instanceof SchemaChange.SetProperty) {
- SchemaChange.SetProperty setProperty = (SchemaChange.SetProperty)
change;
- props.put(setProperty.getProperty(), setProperty.getValue());
- } else if (change instanceof SchemaChange.RemoveProperty) {
- SchemaChange.RemoveProperty removeProperty =
(SchemaChange.RemoveProperty) change;
- props.remove(removeProperty.getProperty());
- } else {
- throw new IllegalArgumentException(
- "Unsupported schema change: " + change.getClass().getSimpleName());
- }
- }
-
- return SchemaEntity.builder()
- .withName(schemaEntity.name())
- .withNamespace(ident.namespace())
- .withId(schemaEntity.id())
- .withComment(schemaEntity.comment())
- .withProperties(props)
- .withAuditInfo(
- AuditInfo.builder()
- .withCreator(schemaEntity.auditInfo().creator())
- .withCreateTime(schemaEntity.auditInfo().createTime())
-
.withLastModifier(PrincipalUtils.getCurrentPrincipal().getName())
- .withLastModifiedTime(Instant.now())
- .build())
- .build();
+ return SchemaEntityChanges.apply(ident, schemaEntity, changes);
}
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
index 31fb86b76e..2b07cc8e99 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/OperationDispatcher.java
@@ -289,6 +289,12 @@ public abstract class OperationDispatcher {
} else if (item instanceof SchemaChange.SetProperty) {
SchemaChange.SetProperty setProperty = (SchemaChange.SetProperty) item;
properties.put(setProperty.getProperty(), setProperty.getValue());
+ } else if (item instanceof SchemaChange.SetSecretBinding) {
+ SchemaChange.SetSecretBinding setSecretBinding =
(SchemaChange.SetSecretBinding) item;
+ properties.put(setSecretBinding.getProperty(),
setSecretBinding.getBinding().plaintext());
+ } else if (item instanceof SchemaChange.SetSecretReference) {
+ SchemaChange.SetSecretReference setSecretReference =
(SchemaChange.SetSecretReference) item;
+ properties.put(setSecretReference.getProperty(),
setSecretReference.getProperty());
} else if (item instanceof FilesetChange.SetProperty) {
FilesetChange.SetProperty setProperty = (FilesetChange.SetProperty)
item;
properties.put(setProperty.getProperty(), setProperty.getValue());
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/SchemaEntityChanges.java
b/core/src/main/java/org/apache/gravitino/catalog/SchemaEntityChanges.java
new file mode 100644
index 0000000000..79f0dff91d
--- /dev/null
+++ b/core/src/main/java/org/apache/gravitino/catalog/SchemaEntityChanges.java
@@ -0,0 +1,70 @@
+/*
+ * 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.catalog;
+
+import com.google.common.collect.Maps;
+import java.time.Instant;
+import java.util.Map;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.SchemaChange;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.SchemaEntity;
+import org.apache.gravitino.utils.PrincipalUtils;
+
+/** Applies {@link SchemaChange} updates to {@link SchemaEntity}. */
+final class SchemaEntityChanges {
+
+ private SchemaEntityChanges() {}
+
+ static SchemaEntity apply(
+ NameIdentifier ident, SchemaEntity schemaEntity, SchemaChange...
changes) {
+ Map<String, String> props =
+ schemaEntity.properties() == null
+ ? Maps.newHashMap()
+ : Maps.newHashMap(schemaEntity.properties());
+
+ for (SchemaChange change : changes) {
+ if (change instanceof SchemaChange.SetProperty) {
+ SchemaChange.SetProperty setProperty = (SchemaChange.SetProperty)
change;
+ props.put(setProperty.getProperty(), setProperty.getValue());
+ } else if (change instanceof SchemaChange.RemoveProperty) {
+ SchemaChange.RemoveProperty removeProperty =
(SchemaChange.RemoveProperty) change;
+ props.remove(removeProperty.getProperty());
+ } else {
+ throw new IllegalArgumentException(
+ "Unsupported schema change: " + change.getClass().getSimpleName());
+ }
+ }
+
+ return SchemaEntity.builder()
+ .withName(schemaEntity.name())
+ .withNamespace(ident.namespace())
+ .withId(schemaEntity.id())
+ .withComment(schemaEntity.comment())
+ .withProperties(props)
+ .withAuditInfo(
+ AuditInfo.builder()
+ .withCreator(schemaEntity.auditInfo().creator())
+ .withCreateTime(schemaEntity.auditInfo().createTime())
+
.withLastModifier(PrincipalUtils.getCurrentPrincipal().getName())
+ .withLastModifiedTime(Instant.now())
+ .build())
+ .build();
+ }
+}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
index c98dfe5add..46da516d60 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
@@ -31,6 +31,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
+import org.apache.commons.lang3.tuple.Pair;
import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.NameIdentifier;
@@ -50,9 +51,11 @@ import org.apache.gravitino.lock.TreeLockUtils;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.FilesetEntity;
import org.apache.gravitino.meta.SchemaEntity;
+import org.apache.gravitino.secret.SecretAlterChanges;
import org.apache.gravitino.secret.SecretBinding;
import org.apache.gravitino.secret.SecretManager;
import org.apache.gravitino.secret.SecretMaterial;
+import org.apache.gravitino.secret.SecretMaterialsHolder;
import org.apache.gravitino.secret.SecretPropertyUtils;
import org.apache.gravitino.secret.SecretReference;
import org.apache.gravitino.storage.IdGenerator;
@@ -307,12 +310,10 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
ident,
LockType.WRITE,
() -> {
- validateAlterProperties(ident,
HasPropertyMetadata::schemaPropertiesMetadata, changes);
- Schema alteredSchema =
- doWithCatalog(
- catalogIdent,
- c -> c.doWithSchemaOps(s -> s.alterSchema(ident, changes)),
- NoSuchSchemaException.class);
+ Pair<Schema, SchemaChange[]> alterResult =
+ alterSchemaUnderLock(ident, catalogIdent, changes);
+ Schema alteredSchema = alterResult.getLeft();
+ SchemaChange[] effectiveChanges = alterResult.getRight();
// If the Schema is maintained by the Gravitino's store, we don't
have to alter again.
boolean isManagedSchema = isManagedEntity(catalogIdent,
Capability.Scope.SCHEMA);
@@ -361,7 +362,8 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
.withName(schemaEntity.name())
.withNamespace(ident.namespace())
.withProperties(
-
propertiesForSchemaEntityAlter(schemaEntity, changes))
+ propertiesForSchemaEntityAlter(
+ schemaEntity, effectiveChanges))
.withAuditInfo(
AuditInfo.builder()
.withCreator(schemaEntity.auditInfo().creator())
@@ -383,6 +385,129 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
});
}
+ private Pair<Schema, SchemaChange[]> alterSchemaUnderLock(
+ NameIdentifier ident, NameIdentifier catalogIdent, SchemaChange...
changes)
+ throws NoSuchSchemaException {
+ if (isManagedEntity(catalogIdent, Capability.Scope.SCHEMA)
+ && usesManagedSchemaOperations(catalogIdent)) {
+ return alterManagedSchemaUnderLock(ident, changes);
+ }
+ return alterExternalSchemaUnderLock(ident, catalogIdent, changes);
+ }
+
+ private boolean usesManagedSchemaOperations(NameIdentifier catalogIdent) {
+ return doWithCatalog(
+ catalogIdent,
+ c -> c.doWithSchemaOps(s -> s instanceof ManagedSchemaOperations),
+ NoSuchSchemaException.class);
+ }
+
+ private Pair<Schema, SchemaChange[]> alterManagedSchemaUnderLock(
+ NameIdentifier ident, SchemaChange... changes) throws
NoSuchSchemaException {
+ validateAlterProperties(ident,
HasPropertyMetadata::schemaPropertiesMetadata, changes);
+
+ SecretMaterialsHolder writtenSecretMaterials = new SecretMaterialsHolder();
+ SchemaChange[][] effectiveChangesHolder = new SchemaChange[1][];
+ boolean alterCommitted = false;
+ try {
+ SchemaEntity updatedEntity =
+ store.update(
+ ident,
+ SchemaEntity.class,
+ SCHEMA,
+ existing -> {
+ Map<String, String> currentProperties =
+ existing.properties() == null
+ ? new HashMap<>()
+ : new HashMap<>(existing.properties());
+ Pair<SchemaChange[], List<SecretMaterial>> secretResult =
+ SecretAlterChanges.prepareSchemaChanges(
+ secretManager, currentProperties, existing.id(),
changes);
+ writtenSecretMaterials.set(secretResult.getRight());
+ effectiveChangesHolder[0] = secretResult.getLeft();
+ return SchemaEntityChanges.apply(ident, existing,
secretResult.getLeft());
+ });
+ alterCommitted = true;
+ Schema alteredSchema =
+ ManagedSchemaOperations.ManagedSchema.builder()
+ .withName(ident.name())
+ .withComment(updatedEntity.comment())
+ .withProperties(updatedEntity.properties())
+ .withAuditInfo(updatedEntity.auditInfo())
+ .build();
+ return Pair.of(alteredSchema, effectiveChangesHolder[0]);
+ } catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist", ident);
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to alter schema " + ident, e);
+ } finally {
+ if (!alterCommitted) {
+ secretManager.rollbackSecrets(writtenSecretMaterials.get());
+ }
+ }
+ }
+
+ private Pair<Schema, SchemaChange[]> alterExternalSchemaUnderLock(
+ NameIdentifier ident, NameIdentifier catalogIdent, SchemaChange...
changes)
+ throws NoSuchSchemaException {
+ Schema currentSchema = null;
+ try {
+ currentSchema =
+ doWithCatalog(
+ catalogIdent,
+ c -> c.doWithSchemaOps(s -> s.loadSchema(ident)),
+ NoSuchSchemaException.class);
+ } catch (NoSuchSchemaException e) {
+ // Defer missing-schema handling to catalog alterSchema to preserve
catalog semantics.
+ }
+ // Prefer SchemaEntity properties for secret URNs (catalog loadSchema may
omit them).
+ SchemaEntity schemaEntityForSecrets = getEntity(ident, SCHEMA,
SchemaEntity.class);
+ Map<String, String> currentProperties;
+ if (schemaEntityForSecrets != null
+ && schemaEntityForSecrets.properties() != null
+ && !schemaEntityForSecrets.properties().isEmpty()) {
+ currentProperties = new HashMap<>(schemaEntityForSecrets.properties());
+ } else if (currentSchema != null && currentSchema.properties() != null) {
+ currentProperties = new HashMap<>(currentSchema.properties());
+ } else {
+ currentProperties = new HashMap<>();
+ }
+
+ validateAlterProperties(ident,
HasPropertyMetadata::schemaPropertiesMetadata, changes);
+
+ StringIdentifier currentStringId =
getStringIdFromProperties(currentProperties);
+ long entityIdForSecrets;
+ if (currentStringId != null) {
+ entityIdForSecrets = currentStringId.id();
+ } else if (schemaEntityForSecrets != null) {
+ entityIdForSecrets = schemaEntityForSecrets.id();
+ } else {
+ entityIdForSecrets = 0L;
+ }
+
+ SecretMaterialsHolder writtenSecretMaterials = new SecretMaterialsHolder();
+ boolean alterCommitted = false;
+ try {
+ Pair<SchemaChange[], List<SecretMaterial>> secretResult =
+ SecretAlterChanges.prepareSchemaChanges(
+ secretManager, currentProperties, entityIdForSecrets, changes);
+ writtenSecretMaterials.set(secretResult.getRight());
+ SchemaChange[] effectiveChanges = secretResult.getLeft();
+
+ Schema alteredSchema =
+ doWithCatalog(
+ catalogIdent,
+ c -> c.doWithSchemaOps(s -> s.alterSchema(ident,
effectiveChanges)),
+ NoSuchSchemaException.class);
+ alterCommitted = true;
+ return Pair.of(alteredSchema, effectiveChanges);
+ } finally {
+ if (!alterCommitted) {
+ secretManager.rollbackSecrets(writtenSecretMaterials.get());
+ }
+ }
+ }
+
/**
* Drops a schema.
*
diff --git
a/core/src/main/java/org/apache/gravitino/secret/SecretAlterChanges.java
b/core/src/main/java/org/apache/gravitino/secret/SecretAlterChanges.java
new file mode 100644
index 0000000000..1615847a0a
--- /dev/null
+++ b/core/src/main/java/org/apache/gravitino/secret/SecretAlterChanges.java
@@ -0,0 +1,198 @@
+/*
+ * 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.secret;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.gravitino.CatalogChange;
+import org.apache.gravitino.SchemaChange;
+import org.apache.gravitino.file.FilesetChange;
+
+/**
+ * Rewrites entity alter changes that involve secrets into plain setProperty /
removeProperty.
+ *
+ * <p>Rolls back any written materials if preparation fails.
+ */
+public final class SecretAlterChanges {
+
+ private SecretAlterChanges() {}
+
+ /**
+ * Prepares catalog alter changes that involve secrets.
+ *
+ * @param secretManager secret manager
+ * @param currentProperties current catalog properties (may be null)
+ * @param entityId catalog entity id
+ * @param changes catalog changes
+ * @return effective changes and written write-through materials
+ */
+ public static Pair<CatalogChange[], List<SecretMaterial>>
prepareCatalogChanges(
+ SecretManager secretManager,
+ @Nullable Map<String, String> currentProperties,
+ long entityId,
+ CatalogChange... changes) {
+ Map<String, String> properties =
+ currentProperties == null ? new HashMap<>() : new
HashMap<>(currentProperties);
+ List<CatalogChange> out = new ArrayList<>(changes.length);
+ List<SecretMaterial> written = new ArrayList<>();
+ try {
+ for (CatalogChange change : changes) {
+ if (change instanceof CatalogChange.SetSecretBinding) {
+ CatalogChange.SetSecretBinding c = (CatalogChange.SetSecretBinding)
change;
+ String urn =
+ secretManager.alterSetSecretBinding(
+ properties, "catalog", entityId, c.getProperty(),
c.getBinding(), written);
+ out.add(CatalogChange.setProperty(c.getProperty(), urn));
+ } else if (change instanceof CatalogChange.SetSecretReference) {
+ CatalogChange.SetSecretReference c =
(CatalogChange.SetSecretReference) change;
+ String urn =
+ secretManager.alterSetSecretReference(
+ properties, "catalog", entityId, c.getProperty(),
c.getReference());
+ out.add(CatalogChange.setProperty(c.getProperty(), urn));
+ } else if (change instanceof CatalogChange.SetProperty) {
+ CatalogChange.SetProperty c = (CatalogChange.SetProperty) change;
+ String value =
+ secretManager.alterSetProperty(
+ properties, "catalog", entityId, c.getProperty(),
c.getValue());
+ out.add(CatalogChange.setProperty(c.getProperty(), value));
+ } else if (change instanceof CatalogChange.RemoveProperty) {
+ CatalogChange.RemoveProperty c = (CatalogChange.RemoveProperty)
change;
+ secretManager.alterRemoveProperty(properties, "catalog", entityId,
c.getProperty());
+ out.add(change);
+ } else {
+ out.add(change);
+ }
+ }
+ return Pair.of(out.toArray(new CatalogChange[0]), List.copyOf(written));
+ } catch (RuntimeException e) {
+ secretManager.rollbackSecrets(written);
+ throw e;
+ }
+ }
+
+ /**
+ * Prepares schema alter changes that involve secrets.
+ *
+ * @param secretManager secret manager
+ * @param currentProperties current schema properties (may be null)
+ * @param entityId schema entity id
+ * @param changes schema changes
+ * @return effective changes and written write-through materials
+ */
+ public static Pair<SchemaChange[], List<SecretMaterial>>
prepareSchemaChanges(
+ SecretManager secretManager,
+ @Nullable Map<String, String> currentProperties,
+ long entityId,
+ SchemaChange... changes) {
+ Map<String, String> properties =
+ currentProperties == null ? new HashMap<>() : new
HashMap<>(currentProperties);
+ List<SchemaChange> out = new ArrayList<>(changes.length);
+ List<SecretMaterial> written = new ArrayList<>();
+ try {
+ for (SchemaChange change : changes) {
+ if (change instanceof SchemaChange.SetSecretBinding) {
+ SchemaChange.SetSecretBinding c = (SchemaChange.SetSecretBinding)
change;
+ String urn =
+ secretManager.alterSetSecretBinding(
+ properties, "schema", entityId, c.getProperty(),
c.getBinding(), written);
+ out.add(SchemaChange.setProperty(c.getProperty(), urn));
+ } else if (change instanceof SchemaChange.SetSecretReference) {
+ SchemaChange.SetSecretReference c =
(SchemaChange.SetSecretReference) change;
+ String urn =
+ secretManager.alterSetSecretReference(
+ properties, "schema", entityId, c.getProperty(),
c.getReference());
+ out.add(SchemaChange.setProperty(c.getProperty(), urn));
+ } else if (change instanceof SchemaChange.SetProperty) {
+ SchemaChange.SetProperty c = (SchemaChange.SetProperty) change;
+ String value =
+ secretManager.alterSetProperty(
+ properties, "schema", entityId, c.getProperty(),
c.getValue());
+ out.add(SchemaChange.setProperty(c.getProperty(), value));
+ } else if (change instanceof SchemaChange.RemoveProperty) {
+ SchemaChange.RemoveProperty c = (SchemaChange.RemoveProperty) change;
+ secretManager.alterRemoveProperty(properties, "schema", entityId,
c.getProperty());
+ out.add(change);
+ } else {
+ out.add(change);
+ }
+ }
+ return Pair.of(out.toArray(new SchemaChange[0]), List.copyOf(written));
+ } catch (RuntimeException e) {
+ secretManager.rollbackSecrets(written);
+ throw e;
+ }
+ }
+
+ /**
+ * Prepares fileset alter changes that involve secrets.
+ *
+ * @param secretManager secret manager
+ * @param currentProperties current fileset properties (may be null)
+ * @param entityId fileset entity id
+ * @param changes fileset changes
+ * @return effective changes and written write-through materials
+ */
+ public static Pair<FilesetChange[], List<SecretMaterial>>
prepareFilesetChanges(
+ SecretManager secretManager,
+ @Nullable Map<String, String> currentProperties,
+ long entityId,
+ FilesetChange... changes) {
+ Map<String, String> properties =
+ currentProperties == null ? new HashMap<>() : new
HashMap<>(currentProperties);
+ List<FilesetChange> out = new ArrayList<>(changes.length);
+ List<SecretMaterial> written = new ArrayList<>();
+ try {
+ for (FilesetChange change : changes) {
+ if (change instanceof FilesetChange.SetSecretBinding) {
+ FilesetChange.SetSecretBinding c = (FilesetChange.SetSecretBinding)
change;
+ String urn =
+ secretManager.alterSetSecretBinding(
+ properties, "fileset", entityId, c.getProperty(),
c.getBinding(), written);
+ out.add(FilesetChange.setProperty(c.getProperty(), urn));
+ } else if (change instanceof FilesetChange.SetSecretReference) {
+ FilesetChange.SetSecretReference c =
(FilesetChange.SetSecretReference) change;
+ String urn =
+ secretManager.alterSetSecretReference(
+ properties, "fileset", entityId, c.getProperty(),
c.getReference());
+ out.add(FilesetChange.setProperty(c.getProperty(), urn));
+ } else if (change instanceof FilesetChange.SetProperty) {
+ FilesetChange.SetProperty c = (FilesetChange.SetProperty) change;
+ String value =
+ secretManager.alterSetProperty(
+ properties, "fileset", entityId, c.getProperty(),
c.getValue());
+ out.add(FilesetChange.setProperty(c.getProperty(), value));
+ } else if (change instanceof FilesetChange.RemoveProperty) {
+ FilesetChange.RemoveProperty c = (FilesetChange.RemoveProperty)
change;
+ secretManager.alterRemoveProperty(properties, "fileset", entityId,
c.getProperty());
+ out.add(change);
+ } else {
+ out.add(change);
+ }
+ }
+ return Pair.of(out.toArray(new FilesetChange[0]), List.copyOf(written));
+ } catch (RuntimeException e) {
+ secretManager.rollbackSecrets(written);
+ throw e;
+ }
+ }
+}
diff --git
a/core/src/main/java/org/apache/gravitino/secret/SecretMaterialsHolder.java
b/core/src/main/java/org/apache/gravitino/secret/SecretMaterialsHolder.java
new file mode 100644
index 0000000000..b94ce938bc
--- /dev/null
+++ b/core/src/main/java/org/apache/gravitino/secret/SecretMaterialsHolder.java
@@ -0,0 +1,42 @@
+/*
+ * 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.secret;
+
+import java.util.List;
+
+/**
+ * Mutable holder so {@code store.update} lambdas can record written secrets
for rollback.
+ *
+ * <p>Used by catalog, schema, and fileset alter paths that prepare secrets
inside {@code
+ * store.update} and roll back on failure.
+ */
+public final class SecretMaterialsHolder {
+
+ private List<SecretMaterial> materials = List.of();
+
+ /** Returns written secret materials. */
+ public List<SecretMaterial> get() {
+ return materials;
+ }
+
+ /** Records written secret materials. */
+ public void set(List<SecretMaterial> materials) {
+ this.materials = materials;
+ }
+}
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
index e140e9fc25..d890c95394 100644
--- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
+++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
@@ -1585,6 +1585,16 @@ public class TestCatalogManager {
.get(PROPERTY_KEY4);
Assertions.assertTrue(SecretPropertyUtils.isSecretProperty(PROPERTY_KEY4, urn));
Assertions.assertEquals("s3cr3t",
secrets.readSecret(SecretUrn.parse(urn)));
+
+ manager.alterCatalog(ident, CatalogChange.removeProperty(PROPERTY_KEY4));
+ Assertions.assertFalse(
+ entityStore
+ .get(ident, EntityType.CATALOG, CatalogEntity.class)
+ .getProperties()
+ .containsKey(PROPERTY_KEY4));
+ Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
secrets.readSecret(SecretUrn.parse(urn)));
+
Assertions.assertTrue(manager.dropCatalog(ident, true));
Assertions.assertThrows(
IllegalArgumentException.class, () ->
secrets.readSecret(SecretUrn.parse(urn)));
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
index bee22f6ecc..33cd97a175 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
@@ -365,6 +365,14 @@ public class TestFilesetOperationDispatcher extends
TestOperationDispatcher {
SecretConstants.ATTR_ENTITY_ID, String.valueOf(entityId),
SecretConstants.ATTR_PROPERTY_KEY, "k2"));
Assertions.assertEquals("s3cr3t", secrets.readSecret(urn));
+ filesets.alterFileset(ident, FilesetChange.removeProperty("k2"));
+ Assertions.assertFalse(
+ catalogManager
+ .loadCatalogAndWrap(NameIdentifier.of(metalake, catalog))
+ .doWithFilesetOps(ops -> ops.loadFileset(ident))
+ .properties()
+ .containsKey("k2"));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
secrets.readSecret(urn));
Assertions.assertThrows(
FilesetAlreadyExistsException.class,
() ->
@@ -401,51 +409,6 @@ public class TestFilesetOperationDispatcher extends
TestOperationDispatcher {
"k3");
}
- @Test
- public void testAlterRemovePropertyDeletesWriteThroughSecret() throws
Exception {
- try (SecretManager secrets = memorySecretManager()) {
- AtomicLong nextId = new AtomicLong(9100L);
- IdGenerator ids = nextId::getAndIncrement;
- FilesetOperationDispatcher filesets =
- new FilesetOperationDispatcher(catalogManager, entityStore, ids,
secrets);
- new SchemaOperationDispatcher(catalogManager, entityStore, ids, secrets,
filesets)
- .createSchema(
- NameIdentifier.of(metalake, catalog,
"schema_secret_fileset_remove"),
- "comment",
- ImmutableMap.of("k1", "v1"));
-
- NameIdentifier ident =
- NameIdentifier.of(
- metalake, catalog, "schema_secret_fileset_remove",
"fileset_secret_remove");
- Map<String, SecretBinding> bindings = Map.of("k2", new
SecretBinding("memory", "s3cr3t"));
- Map<String, String> locations = Map.of(Fileset.LOCATION_NAME_UNKNOWN,
"loc");
- Map<String, String> props = ImmutableMap.of("k1", "v1");
- long entityId = nextId.get();
- filesets.createMultipleLocationFileset(
- ident, "comment", Fileset.Type.MANAGED, locations, props, bindings,
Map.of());
-
- SecretUrn urn =
- SecretUrn.buildWriteThrough(
- "memory",
- Map.of(
- SecretConstants.ATTR_ENTITY_TYPE, "fileset",
- SecretConstants.ATTR_ENTITY_ID, String.valueOf(entityId),
- SecretConstants.ATTR_PROPERTY_KEY, "k2"));
- Assertions.assertEquals("s3cr3t", secrets.readSecret(urn));
-
- filesets.alterFileset(ident, FilesetChange.removeProperty("k2"));
-
- Fileset stored =
- catalogManager
- .loadCatalogAndWrap(NameIdentifier.of(metalake, catalog))
- .doWithFilesetOps(ops -> ops.loadFileset(ident));
- Assertions.assertFalse(stored.properties().containsKey("k2"));
- Assertions.assertThrows(IllegalArgumentException.class, () ->
secrets.readSecret(urn));
-
- Assertions.assertTrue(filesets.dropFileset(ident));
- }
- }
-
private static SecretManager memorySecretManager() {
Config c = new Config(false) {};
Properties p = new Properties();
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
index e31802593d..05ca381d1c 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
@@ -467,6 +467,10 @@ public class TestSchemaOperationDispatcher extends
TestOperationDispatcher {
SecretConstants.ATTR_ENTITY_ID, String.valueOf(entity.id()),
SecretConstants.ATTR_PROPERTY_KEY, "k2"));
Assertions.assertEquals("s3cr3t", secrets.readSecret(urn));
+ d.alterSchema(ident, SchemaChange.removeProperty("k2"));
+ Assertions.assertFalse(
+ entityStore.get(ident, SCHEMA,
SchemaEntity.class).properties().containsKey("k2"));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
secrets.readSecret(urn));
Assertions.assertThrows(
SchemaAlreadyExistsException.class,
() -> d.createSchema(ident, "comment", ImmutableMap.of("k1", "v1"),
bindings, Map.of()));
diff --git a/docs/open-api/catalogs.yaml b/docs/open-api/catalogs.yaml
index ae82be9ce8..1340ecea74 100644
--- a/docs/open-api/catalogs.yaml
+++ b/docs/open-api/catalogs.yaml
@@ -509,6 +509,8 @@ components:
- $ref: "#/components/schemas/UpdateCatalogCommentRequest"
- $ref: "#/components/schemas/SetCatalogPropertyRequest"
- $ref: "#/components/schemas/RemoveCatalogPropertyRequest"
+ - $ref: "#/components/schemas/SetCatalogSecretBindingRequest"
+ - $ref: "#/components/schemas/SetCatalogSecretReferenceRequest"
discriminator:
propertyName: "@type"
mapping:
@@ -516,6 +518,8 @@ components:
updateComment: "#/components/schemas/UpdateCatalogCommentRequest"
setProperty: "#/components/schemas/SetCatalogPropertyRequest"
removeProperty: "#/components/schemas/RemoveCatalogPropertyRequest"
+ setSecretBinding:
"#/components/schemas/SetCatalogSecretBindingRequest"
+ setSecretReference:
"#/components/schemas/SetCatalogSecretReferenceRequest"
RenameCatalogRequest:
type: object
@@ -594,6 +598,67 @@ components:
"property": "key2"
}
+ SetCatalogSecretBindingRequest:
+ type: object
+ required:
+ - "@type"
+ - property
+ - provider
+ - plaintext
+ properties:
+ "@type":
+ type: string
+ enum:
+ - setSecretBinding
+ property:
+ type: string
+ description: The property to bind
+ provider:
+ type: string
+ description: Registered secrets-provider instance name
+ plaintext:
+ type: string
+ description: Plaintext secret to write through
+ format: password
+ example: {
+ "@type": "setSecretBinding",
+ "property": "password",
+ "provider": "env",
+ "plaintext": "secret-value"
+ }
+
+ SetCatalogSecretReferenceRequest:
+ type: object
+ required:
+ - "@type"
+ - property
+ - provider
+ properties:
+ "@type":
+ type: string
+ enum:
+ - setSecretReference
+ property:
+ type: string
+ description: The property to bind
+ provider:
+ type: string
+ description: Registered secrets-provider instance name
+ attributes:
+ type: object
+ description: Provider-specific locator keys (empty object if none)
+ additionalProperties:
+ type: string
+ default: {}
+ example: {
+ "@type": "setSecretReference",
+ "property": "password",
+ "provider": "vault",
+ "attributes": {
+ "path": "secret/data/my-password"
+ }
+ }
+
responses:
diff --git a/docs/open-api/schemas.yaml b/docs/open-api/schemas.yaml
index df03ce4c52..3539e76da6 100644
--- a/docs/open-api/schemas.yaml
+++ b/docs/open-api/schemas.yaml
@@ -249,11 +249,15 @@ components:
oneOf:
- $ref: "#/components/schemas/SetSchemaPropertyRequest"
- $ref: "#/components/schemas/RemoveSchemaPropertyRequest"
+ - $ref: "#/components/schemas/SetSchemaSecretBindingRequest"
+ - $ref: "#/components/schemas/SetSchemaSecretReferenceRequest"
discriminator:
propertyName: "@type"
mapping:
setProperty: "#/components/schemas/SetSchemaPropertyRequest"
removeProperty: "#/components/schemas/RemoveSchemaPropertyRequest"
+ setSecretBinding:
"#/components/schemas/SetSchemaSecretBindingRequest"
+ setSecretReference:
"#/components/schemas/SetSchemaSecretReferenceRequest"
SetSchemaPropertyRequest:
type: object
@@ -295,6 +299,67 @@ components:
"property": "key2"
}
+ SetSchemaSecretBindingRequest:
+ type: object
+ required:
+ - "@type"
+ - property
+ - provider
+ - plaintext
+ properties:
+ "@type":
+ type: string
+ enum:
+ - setSecretBinding
+ property:
+ type: string
+ description: The property to bind
+ provider:
+ type: string
+ description: Registered secrets-provider instance name
+ plaintext:
+ type: string
+ description: Plaintext secret to write through
+ format: password
+ example: {
+ "@type": "setSecretBinding",
+ "property": "password",
+ "provider": "env",
+ "plaintext": "secret-value"
+ }
+
+ SetSchemaSecretReferenceRequest:
+ type: object
+ required:
+ - "@type"
+ - property
+ - provider
+ properties:
+ "@type":
+ type: string
+ enum:
+ - setSecretReference
+ property:
+ type: string
+ description: The property to bind
+ provider:
+ type: string
+ description: Registered secrets-provider instance name
+ attributes:
+ type: object
+ description: Provider-specific locator keys (empty object if none)
+ additionalProperties:
+ type: string
+ default: {}
+ example: {
+ "@type": "setSecretReference",
+ "property": "password",
+ "provider": "vault",
+ "attributes": {
+ "path": "secret/data/my-password"
+ }
+ }
+
responses:
SchemaResponse:
description: Returns include the schema object
