This is an automated email from the ASF dual-hosted git repository.
liuxun 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 d37426426f [#8182] fix(authz): Fix list model version bug (#8223)
d37426426f is described below
commit d37426426f9228adc417315c9cd1ad2f98781496
Author: yangyang zhong <[email protected]>
AuthorDate: Thu Aug 21 19:11:31 2025 +0800
[#8182] fix(authz): Fix list model version bug (#8223)
### What changes were proposed in this pull request?
fix list model version bug
### Why are the changes needed?
Fix: #8182
### Does this PR introduce _any_ user-facing change?
None
### How was this patch tested?
org.apache.gravitino.client.integration.test.authorization.ModelAuthorizationIT
---
.../test/authorization/ModelAuthorizationIT.java | 40 +++++++++++++++-------
.../apache/gravitino/utils/NameIdentifierUtil.java | 29 ++++++++++++++++
.../server/authorization/MetadataFilterHelper.java | 19 +++++++++-
3 files changed, 74 insertions(+), 14 deletions(-)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
index e75e4f514c..43c490e039 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
@@ -239,29 +239,32 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
@Test
@Order(7)
public void testListModelVersion() {
- ModelCatalog modelCatalog =
client.loadMetalake(METALAKE).loadCatalog(CATALOG).asModelCatalog();
+ GravitinoMetalake gravitinoMetalake = client.loadMetalake(METALAKE);
+ ModelCatalog modelCatalog =
gravitinoMetalake.loadCatalog(CATALOG).asModelCatalog();
Catalog catalogEntityLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG);
ModelCatalog modelCatalogLoadByNormalUser =
catalogEntityLoadByNormalUser.asModelCatalog();
int[] versions = modelCatalog.listModelVersions(NameIdentifier.of(SCHEMA,
"model1"));
assertEquals(2, versions.length);
- assertThrows(
- "Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA
+ "model1" + "}.",
- ForbiddenException.class,
- () -> {
- modelCatalogLoadByNormalUser.linkModelVersion(
- NameIdentifier.of(SCHEMA, "model1"),
- "uri1",
- new String[] {"alias2"},
- "comment2",
- null);
- });
+ versions =
modelCatalogLoadByNormalUser.listModelVersions(NameIdentifier.of(SCHEMA,
"model1"));
+ assertEquals(0, versions.length);
+ gravitinoMetalake.grantPrivilegesToRole(
+ role,
+ MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"),
MetadataObject.Type.MODEL),
+ ImmutableSet.of(Privileges.UseModel.allow()));
+ versions =
modelCatalogLoadByNormalUser.listModelVersions(NameIdentifier.of(SCHEMA,
"model1"));
+ assertEquals(2, versions.length);
+ gravitinoMetalake.revokePrivilegesFromRole(
+ role,
+ MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"),
MetadataObject.Type.MODEL),
+ ImmutableSet.of(Privileges.UseModel.allow()));
}
@Test
@Order(8)
public void testLoadModelVersion() {
- ModelCatalog modelCatalog =
client.loadMetalake(METALAKE).loadCatalog(CATALOG).asModelCatalog();
+ GravitinoMetalake gravitinoMetalake = client.loadMetalake(METALAKE);
+ ModelCatalog modelCatalog =
gravitinoMetalake.loadCatalog(CATALOG).asModelCatalog();
Catalog catalogEntityLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG);
ModelCatalog modelCatalogLoadByNormalUser =
catalogEntityLoadByNormalUser.asModelCatalog();
@@ -273,6 +276,17 @@ public class ModelAuthorizationIT extends
BaseRestApiAuthorizationIT {
() -> {
modelCatalogLoadByNormalUser.getModelVersion(NameIdentifier.of(SCHEMA,
"model1"), 1);
});
+
+ gravitinoMetalake.grantPrivilegesToRole(
+ role,
+ MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"),
MetadataObject.Type.MODEL),
+ ImmutableSet.of(Privileges.UseModel.allow()));
+ version =
modelCatalogLoadByNormalUser.getModelVersion(NameIdentifier.of(SCHEMA,
"model1"), 1);
+ assertEquals(1, version.version());
+ gravitinoMetalake.revokePrivilegesFromRole(
+ role,
+ MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"),
MetadataObject.Type.MODEL),
+ ImmutableSet.of(Privileges.UseModel.allow()));
}
@Test
diff --git
a/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
b/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
index c1fd511807..095fe6259b 100644
--- a/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
+++ b/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
@@ -560,4 +560,33 @@ public class NameIdentifierUtil {
public static NameIdentifier ofStatistic(NameIdentifier entityIdent, String
name) {
return NameIdentifier.of(Namespace.fromString(entityIdent.toString()),
name);
}
+
+ /**
+ * Try to get the model {@link NameIdentifier} from the given {@link
NameIdentifier}.
+ *
+ * @param ident The {@link NameIdentifier} to check.
+ * @return The model {@link NameIdentifier}
+ * @throws IllegalNameIdentifierException If the given {@link
NameIdentifier} does not include
+ * model name
+ */
+ public static NameIdentifier getModelIdentifier(NameIdentifier ident) {
+ NameIdentifier.check(
+ ident.name() != null && !ident.name().isEmpty(),
+ "The name variable in the NameIdentifier must have value.");
+ Namespace.check(
+ ident.namespace() != null
+ && !ident.namespace().isEmpty()
+ && ident.namespace().length() >= 4,
+ "ModelVersion namespace must be non-null and at least 4 level, the
input namespace is %s",
+ ident.namespace());
+
+ List<String> allElems =
+ Stream.concat(Arrays.stream(ident.namespace().levels()),
Stream.of(ident.name()))
+ .collect(Collectors.toList());
+ if (allElems.size() < 4) {
+ throw new IllegalNameIdentifierException(
+ "Cannot create a model NameIdentifier less than four elements.");
+ }
+ return NameIdentifier.of(allElems.get(0), allElems.get(1),
allElems.get(2), allElems.get(3));
+ }
}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
index 3f7ade56a6..1a002daafc 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
@@ -172,6 +172,15 @@ public class MetadataFilterHelper {
nameIdentifierMap.put(
Entity.EntityType.CATALOG,
NameIdentifierUtil.getCatalogIdentifier(nameIdentifier));
break;
+ case MODEL_VERSION:
+ nameIdentifierMap.put(Entity.EntityType.MODEL_VERSION, nameIdentifier);
+ nameIdentifierMap.put(
+ Entity.EntityType.MODEL,
NameIdentifierUtil.getModelIdentifier(nameIdentifier));
+ nameIdentifierMap.put(
+ Entity.EntityType.SCHEMA,
NameIdentifierUtil.getSchemaIdentifier(nameIdentifier));
+ nameIdentifierMap.put(
+ Entity.EntityType.CATALOG,
NameIdentifierUtil.getCatalogIdentifier(nameIdentifier));
+ break;
case TOPIC:
nameIdentifierMap.put(Entity.EntityType.TOPIC, nameIdentifier);
nameIdentifierMap.put(
@@ -186,9 +195,17 @@ public class MetadataFilterHelper {
nameIdentifierMap.put(
Entity.EntityType.CATALOG,
NameIdentifierUtil.getCatalogIdentifier(nameIdentifier));
break;
- default:
+ case METALAKE:
+ nameIdentifierMap.put(entityType, nameIdentifier);
+ break;
+ case ROLE:
nameIdentifierMap.put(entityType, nameIdentifier);
break;
+ case USER:
+ nameIdentifierMap.put(entityType, nameIdentifier);
+ break;
+ default:
+ throw new IllegalArgumentException("Unsupported entity type: " +
entityType);
}
return nameIdentifierMap;
}