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 ab840a5fef [#7556][followup] fix(authz,iceberg): Fix the issues of
creating tables and renaming tables. (#9200)
ab840a5fef is described below
commit ab840a5fef857898c039b1f3cc827f6acc579aac
Author: roryqi <[email protected]>
AuthorDate: Fri Nov 21 16:21:48 2025 +0800
[#7556][followup] fix(authz,iceberg): Fix the issues of creating tables and
renaming tables. (#9200)
### What changes were proposed in this pull request?
Fix the issues of creating tables and renaming tables.
### Why are the changes needed?
1. The Iceberg connector will verify the existence of the table before
creating the table, so the user needs the privilege to check the
existence of the table. Now , we used load table to check it. I'm
optimizing the Iceberg community code to use the delicated API to check
this.
2. The renaming tables should modify the Gravitino backend data.
Otherwise, the owner will be wrong.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added UT.
---
.../dispatcher/IcebergTableHookDispatcher.java | 56 ++++++++++++++++++++++
.../service/rest/IcebergTableOperations.java | 4 +-
.../test/IcebergTableAuthorizationIT.java | 29 +++++++----
3 files changed, 79 insertions(+), 10 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
index 0d39eeed7e..eae2a19591 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
@@ -18,16 +18,24 @@
*/
package org.apache.gravitino.iceberg.service.dispatcher;
+import java.io.IOException;
+import java.time.Instant;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.TableDispatcher;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.iceberg.common.utils.IcebergIdentifierUtils;
import
org.apache.gravitino.iceberg.service.authorization.IcebergRESTServerContext;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.TableEntity;
import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.rest.requests.CreateTableRequest;
@@ -71,6 +79,20 @@ public class IcebergTableHookDispatcher implements
IcebergTableOperationDispatch
public void dropTable(
IcebergRequestContext context, TableIdentifier tableIdentifier, boolean
purgeRequested) {
dispatcher.dropTable(context, tableIdentifier, purgeRequested);
+ EntityStore store = GravitinoEnv.getInstance().entityStore();
+ try {
+ if (store != null) {
+ // Delete the entity for the dropped table.
+ store.delete(
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ metalake, context.catalogName(), tableIdentifier),
+ Entity.EntityType.TABLE);
+ }
+ } catch (NoSuchEntityException ignore) {
+ // Ignore if the table entity does not exist.
+ } catch (IOException ioe) {
+ throw new RuntimeException("io exception when deleting table entity",
ioe);
+ }
}
@Override
@@ -92,6 +114,40 @@ public class IcebergTableHookDispatcher implements
IcebergTableOperationDispatch
@Override
public void renameTable(IcebergRequestContext context, RenameTableRequest
renameTableRequest) {
dispatcher.renameTable(context, renameTableRequest);
+ NameIdentifier tableSource =
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ metalake, context.catalogName(), renameTableRequest.source());
+ NameIdentifier tableDest =
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ metalake, context.catalogName(), renameTableRequest.destination());
+ EntityStore store = GravitinoEnv.getInstance().entityStore();
+ try {
+ if (store != null) {
+ // Update the entity for the destination table.
+ store.update(
+ tableSource,
+ TableEntity.class,
+ Entity.EntityType.TABLE,
+ tableEntity ->
+ TableEntity.builder()
+ .withId(tableEntity.id())
+ .withName(tableDest.name())
+ .withNamespace(tableDest.namespace())
+ .withColumns(tableEntity.columns())
+ .withAuditInfo(
+ AuditInfo.builder()
+ .withCreator(tableEntity.auditInfo().creator())
+
.withCreateTime(tableEntity.auditInfo().createTime())
+
.withLastModifier(PrincipalUtils.getCurrentPrincipal().getName())
+ .withLastModifiedTime(Instant.now())
+ .build())
+ .build());
+ }
+ } catch (NoSuchEntityException ignore) {
+ // Ignore if the source table entity does not exist.
+ } catch (IOException ioe) {
+ throw new RuntimeException("io exception when renaming table entity",
ioe);
+ }
}
@Override
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
index 8eaa095d0b..98746b5e22 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
@@ -270,7 +270,7 @@ public class IcebergTableOperations {
expression =
"ANY(OWNER, METALAKE, CATALOG) || "
+ "SCHEMA_OWNER_WITH_USE_CATALOG || "
- + "ANY_USE_CATALOG && ANY_USE_SCHEMA && (TABLE::OWNER ||
ANY_SELECT_TABLE|| ANY_MODIFY_TABLE)",
+ + "ANY_USE_CATALOG && ANY_USE_SCHEMA && (TABLE::OWNER ||
ANY_SELECT_TABLE|| ANY_MODIFY_TABLE || ANY_CREATE_TABLE)",
accessMetadataType = MetadataObject.Type.TABLE)
public Response loadTable(
@AuthorizationMetadata(type = Entity.EntityType.CATALOG)
@PathParam("prefix") String prefix,
@@ -316,7 +316,7 @@ public class IcebergTableOperations {
expression =
"ANY(OWNER, METALAKE, CATALOG) || "
+ "SCHEMA_OWNER_WITH_USE_CATALOG || "
- + "ANY_USE_CATALOG && ANY_USE_SCHEMA && (TABLE::OWNER ||
ANY_SELECT_TABLE|| ANY_MODIFY_TABLE)",
+ + "ANY_USE_CATALOG && ANY_USE_SCHEMA && (TABLE::OWNER ||
ANY_SELECT_TABLE || ANY_MODIFY_TABLE || ANY_CREATE_TABLE)",
accessMetadataType = MetadataObject.Type.TABLE)
public Response tableExists(
@AuthorizationMetadata(type = Entity.EntityType.CATALOG)
@PathParam("prefix") String prefix,
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergTableAuthorizationIT.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergTableAuthorizationIT.java
index 46f99702cc..6bf3b425bc 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergTableAuthorizationIT.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergTableAuthorizationIT.java
@@ -95,6 +95,9 @@ public class IcebergTableAuthorizationIT extends
IcebergAuthorizationIT {
.asTableCatalog()
.tableExists(NameIdentifier.of(SCHEMA_NAME, tableName));
Assertions.assertTrue(exists);
+ Assertions.assertDoesNotThrow(
+ () ->
+ Assertions.assertDoesNotThrow(() -> sql("CREATE TABLE %s(a int)",
"anotherTableName")));
Optional<Owner> owner =
metalakeClientWithAllPrivilege.getOwner(
@@ -169,6 +172,23 @@ public class IcebergTableAuthorizationIT extends
IcebergAuthorizationIT {
.asTableCatalog()
.tableExists(NameIdentifier.of(SCHEMA_NAME, tableName));
Assertions.assertFalse(exists);
+
+ // recreate the dropped table
+ createTable(SCHEMA_NAME, tableName);
+ Optional<Owner> owner =
+ metalakeClientWithAllPrivilege.getOwner(
+ MetadataObjects.of(
+ Arrays.asList(GRAVITINO_CATALOG_NAME, SCHEMA_NAME, tableName),
+ MetadataObject.Type.TABLE));
+ Assertions.assertTrue(owner.isPresent());
+ Assertions.assertEquals(SUPER_USER, owner.get().name());
+ setTableOwner(tableName);
+ Assertions.assertDoesNotThrow(() -> sql("DROP TABLE %s", tableName));
+ exists =
+ catalogClientWithAllPrivilege
+ .asTableCatalog()
+ .tableExists(NameIdentifier.of(SCHEMA_NAME, tableName));
+ Assertions.assertFalse(exists);
}
@Test
@@ -243,11 +263,6 @@ public class IcebergTableAuthorizationIT extends
IcebergAuthorizationIT {
setTableOwner(tableName);
Assertions.assertDoesNotThrow(
() -> sql("ALTER TABLE %s RENAME TO %s", tableName, tableName +
"_renamed"));
- Table table =
- catalogClientWithAllPrivilege
- .asTableCatalog()
- .loadTable(NameIdentifier.of(SCHEMA_NAME, tableName + "_renamed"));
- Assertions.assertNotNull(table);
Optional<Owner> owner =
metalakeClientWithAllPrivilege.getOwner(
@@ -283,9 +298,7 @@ public class IcebergTableAuthorizationIT extends
IcebergAuthorizationIT {
securableObjects.add(catalogObject);
SecurableObject schemaObject =
SecurableObjects.ofSchema(
- catalogObject,
- schema,
- ImmutableList.of(Privileges.CreateTable.allow(),
Privileges.SelectTable.allow()));
+ catalogObject, schema,
ImmutableList.of(Privileges.CreateTable.allow()));
securableObjects.add(schemaObject);
metalakeClientWithAllPrivilege.createRole(roleName, new HashMap<>(),
securableObjects);
metalakeClientWithAllPrivilege.grantRolesToUser(ImmutableList.of(roleName),
NORMAL_USER);
