lasdf1234 commented on code in PR #12420:
URL: https://github.com/apache/gravitino/pull/12420#discussion_r3773866013
##########
core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java:
##########
@@ -103,82 +109,108 @@ public NameIdentifier[] listSchemas(Namespace namespace)
throws NoSuchCatalogExc
@Override
public Schema createSchema(NameIdentifier ident, String comment, Map<String,
String> properties)
throws NoSuchCatalogException, SchemaAlreadyExistsException {
+ return createSchema(ident, comment, properties, Collections.emptyMap(),
Collections.emptyMap());
+ }
+
+ @Override
+ public Schema createSchema(
+ NameIdentifier ident,
+ String comment,
+ Map<String, String> properties,
+ Map<String, SecretBinding> secretBindings,
+ Map<String, SecretReference> secretReferences)
+ throws NoSuchCatalogException, SchemaAlreadyExistsException {
NameIdentifier catalogIdent = getCatalogIdentifier(ident);
+ long uid = idGenerator.nextId();
+ Map<String, String> entityProperties =
+ SecretPropertyUtils.copyEntityProperties(properties, secretBindings,
secretReferences);
+ List<SecretMaterial> secretMaterials =
+ secretManager.assembleSecretMaterials(
+ properties, entityProperties, "schema", uid, secretBindings,
secretReferences);
doWithCatalog(
catalogIdent,
c ->
c.doWithPropertiesMeta(
p -> {
- validatePropertyForCreate(p.schemaPropertiesMetadata(),
properties);
+ validatePropertyForCreate(p.schemaPropertiesMetadata(),
entityProperties);
return null;
}),
IllegalArgumentException.class);
- long uid = idGenerator.nextId();
+ secretManager.writeSecrets(secretMaterials);
// Add StringIdentifier to the properties, the specific catalog will
handle this
// StringIdentifier to make sure only when the operation is successful,
the related
// SchemaEntity will be visible.
+ //
+ // Same split as CatalogManager: create/storage properties keep secret
URNs. Connectors that
+ // need plaintext for runtime (e.g. Fileset FS) resolve at the conf
boundary — see
+ // FilesetCatalogOperations.mergeUpLevelConfigurations /
CatalogManager.createBaseCatalog.
StringIdentifier stringId = StringIdentifier.fromId(uid);
Map<String, String> updatedProperties =
- StringIdentifier.newPropertiesWithId(stringId, properties);
-
- return TreeLockUtils.doWithTreeLock(
- catalogIdent,
- LockType.WRITE,
- () -> {
- // we do not retrieve the schema again (to obtain some values
generated by underlying
- // catalog)
- // since some catalogs' API is async and the schema may not be
created immediately
- Schema schema =
- doWithCatalog(
- catalogIdent,
- c -> c.doWithSchemaOps(s -> s.createSchema(ident, comment,
updatedProperties)),
- NoSuchCatalogException.class,
- SchemaAlreadyExistsException.class);
+ StringIdentifier.newPropertiesWithId(stringId, entityProperties);
- // If the Schema is maintained by the Gravitino's store, we don't
have to store again.
- boolean isManagedSchema = isManagedEntity(catalogIdent,
Capability.Scope.SCHEMA);
- if (isManagedSchema) {
- return EntityCombinedSchema.of(schema)
- .withHiddenProperties(
- getHiddenPropertyNames(
- catalogIdent,
- HasPropertyMetadata::schemaPropertiesMetadata,
- schema.properties()));
- }
+ try {
+ return TreeLockUtils.doWithTreeLock(
+ catalogIdent,
+ LockType.WRITE,
+ () -> {
+ // we do not retrieve the schema again (to obtain some values
generated by underlying
+ // catalog)
+ // since some catalogs' API is async and the schema may not be
created immediately
+ Schema schema =
+ doWithCatalog(
+ catalogIdent,
+ c -> c.doWithSchemaOps(s -> s.createSchema(ident, comment,
updatedProperties)),
+ NoSuchCatalogException.class,
+ SchemaAlreadyExistsException.class);
+
+ // If the Schema is maintained by the Gravitino's store, we don't
have to store again.
+ boolean isManagedSchema = isManagedEntity(catalogIdent,
Capability.Scope.SCHEMA);
+ if (isManagedSchema) {
+ return EntityCombinedSchema.of(schema)
+ .withHiddenProperties(
+ getHiddenPropertyNames(
+ catalogIdent,
+ HasPropertyMetadata::schemaPropertiesMetadata,
+ schema.properties()));
+ }
- SchemaEntity schemaEntity =
- SchemaEntity.builder()
- .withId(uid)
- .withName(ident.name())
- .withNamespace(ident.namespace())
- .withAuditInfo(
- AuditInfo.builder()
-
.withCreator(PrincipalUtils.getCurrentPrincipal().getName())
- .withCreateTime(Instant.now())
- .build())
- .build();
+ SchemaEntity schemaEntity =
+ SchemaEntity.builder()
+ .withId(uid)
+ .withName(ident.name())
+ .withNamespace(ident.namespace())
+ .withAuditInfo(
+ AuditInfo.builder()
+
.withCreator(PrincipalUtils.getCurrentPrincipal().getName())
+ .withCreateTime(Instant.now())
+ .build())
+ .build();
Review Comment:
Got resolved.
##########
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java:
##########
@@ -975,9 +1008,12 @@ public boolean dropCatalog(NameIdentifier ident, boolean
force)
// Finally, delete the catalog entity as well as all its
sub-entities from the store.
// Invalidate after store.delete() to prevent a background thread
from repopulating
// the cache with stale data between invalidate and delete.
+ Map<String, String> catalogProperties =
+ catalogWrapper.catalog().entity().getProperties();
boolean deleted = store.delete(ident, EntityType.CATALOG, true);
Review Comment:
Fixed: before cascading `store.delete` / managed `dropSchema`, we now call
`deleteSecretsForSchemaSubtree` for each schema (schema properties + listed
fileset entities under that schema). Catalog-level secrets are still cleaned
after a successful catalog delete.
--
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]
