laserninja commented on code in PR #12867:
URL: https://github.com/apache/gravitino/pull/12867#discussion_r4135837780
##########
api/src/main/java/org/apache/gravitino/authorization/SecurableObjects.java:
##########
@@ -170,6 +170,22 @@ public static SecurableObject ofFunction(
return of(MetadataObject.Type.FUNCTION, names, privileges);
}
+ /**
+ * Create the semantic model {@link SecurableObject} with the given
securable schema object,
+ * semantic model name and privileges.
+ *
+ * @param schema The schema securable object
+ * @param semanticModel The semantic model name
+ * @param privileges The privileges of the semantic model
+ * @return The created semantic model {@link SecurableObject}
+ */
+ public static SecurableObject ofSemanticModel(
+ SecurableObject schema, String semanticModel, List<Privilege>
privileges) {
+ List<String> names =
Lists.newArrayList(DOT_SPLITTER.splitToList(schema.fullName()));
+ names.add(semanticModel);
+ return of(MetadataObject.Type.SEMANTIC_MODEL, names, privileges);
Review Comment:
Fixed in e07c83b99. Added SEMANTIC_MODEL to SecurableObject.type and
metadataObjectTypeOfRole, plus CREATE_SEMANTIC_MODEL, SELECT_SEMANTIC_MODEL,
and MODIFY_SEMANTIC_MODEL to Privilege.name. ./gradlew :docs:build passes,
including both OpenAPI lint tasks.
##########
docs/security/access-control.md:
##########
@@ -276,6 +280,7 @@ return only the entries the caller is entitled to see,
which for a metalake owne
| Fileset | `CREATE_FILESET` | `READ_FILESET` or `WRITE_FILESET` |
`WRITE_FILESET` | Owner |
| Model | `REGISTER_MODEL` | `USE_MODEL` |
Owner | Owner |
| Function | `REGISTER_FUNCTION` | `EXECUTE_FUNCTION` or `MODIFY_FUNCTION` |
`MODIFY_FUNCTION` | Owner |
+| Semantic Model | `CREATE_SEMANTIC_MODEL` | `SELECT_SEMANTIC_MODEL` or
`MODIFY_SEMANTIC_MODEL` | `MODIFY_SEMANTIC_MODEL` | Owner |
Review Comment:
Fixed in e07c83b99. Added Semantic Models to the Ownership section and the
MANAGE_GRANTS Grantable On list.
##########
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##########
@@ -97,7 +97,10 @@ public class AuthorizationUtils {
MetadataObject.Type.JOB_TEMPLATE,
MetadataObject.Type.TAG,
MetadataObject.Type.POLICY,
- MetadataObject.Type.VIEW);
+ MetadataObject.Type.VIEW,
+ // Semantic models live only in Gravitino, underlying connectors
know nothing about
+ // them, so there is no privilege to push down to an authorization
plugin.
+ MetadataObject.Type.SEMANTIC_MODEL);
Review Comment:
You are right, the previous fix missed the privilege-update callbacks. Fixed
in e07c83b99: grant, revoke, and override now compare filtered before/after
states, skip semantic-only changes, and send filtered RoleChange payloads and
role snapshots. The callback becomes add/remove when the first/last connector
privilege changes. Regression tests cover all three operations at metalake,
catalog, and schema scope, including semantic-only and mixed grants. The
focused authorization suite passes.
##########
core/src/main/java/org/apache/gravitino/hook/SemanticModelHookDispatcher.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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.hook;
+
+import java.util.Map;
+import java.util.function.Supplier;
+import javax.annotation.Nullable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.authorization.AuthorizationUtils;
+import org.apache.gravitino.authorization.Owner;
+import org.apache.gravitino.authorization.OwnerDispatcher;
+import org.apache.gravitino.catalog.SemanticModelDispatcher;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+import org.apache.gravitino.exceptions.NoSuchSchemaException;
+import org.apache.gravitino.exceptions.NoSuchSemanticModelException;
+import org.apache.gravitino.exceptions.SemanticModelAlreadyExistsException;
+import org.apache.gravitino.semantic.SemanticModel;
+import org.apache.gravitino.semantic.SemanticModelChange;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
+
+/**
+ * {@code SemanticModelHookDispatcher} is a decorator for {@link
SemanticModelDispatcher} that not
+ * only delegates Semantic Model operations to the underlying dispatcher but
also executes some hook
+ * operations before or after the underlying operations.
+ */
+public class SemanticModelHookDispatcher implements SemanticModelDispatcher {
+
+ private final SemanticModelDispatcher dispatcher;
+ private final Supplier<OwnerDispatcher> ownerDispatcher;
+
+ /**
+ * Creates a Semantic Model hook dispatcher.
+ *
+ * @param dispatcher The underlying dispatcher.
+ * @param ownerDispatcher Supplies the owner dispatcher, or null when
authorization is disabled.
+ */
+ public SemanticModelHookDispatcher(
+ SemanticModelDispatcher dispatcher, Supplier<OwnerDispatcher>
ownerDispatcher) {
+ this.dispatcher = dispatcher;
+ this.ownerDispatcher = ownerDispatcher;
+ }
+
+ @Override
+ public NameIdentifier[] listSemanticModels(Namespace namespace) throws
NoSuchSchemaException {
+ return dispatcher.listSemanticModels(namespace);
+ }
+
+ @Override
+ public SemanticModel loadSemanticModel(NameIdentifier ident) throws
NoSuchSemanticModelException {
+ return dispatcher.loadSemanticModel(ident);
+ }
+
+ @Override
+ public boolean semanticModelExists(NameIdentifier ident) {
+ return dispatcher.semanticModelExists(ident);
+ }
+
+ @Override
+ public SemanticModel createSemanticModel(
+ NameIdentifier ident,
+ @Nullable String comment,
+ SemanticModelDefinition definition,
+ Map<String, String> properties)
+ throws NoSuchSchemaException, SemanticModelAlreadyExistsException,
+ IllegalSemanticModelException {
+ SemanticModel semanticModel =
+ dispatcher.createSemanticModel(ident, comment, definition, properties);
+
+ // Set the creator as the owner of the Semantic Model.
+ OwnerDispatcher ownerManager = ownerDispatcher.get();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.SEMANTIC_MODEL),
+ PrincipalUtils.getCurrentUserName(),
+ Owner.Type.USER);
+ }
+ return semanticModel;
+ }
+
+ @Override
+ public SemanticModel alterSemanticModel(NameIdentifier ident,
SemanticModelChange... changes)
+ throws NoSuchSemanticModelException, SemanticModelAlreadyExistsException,
+ IllegalSemanticModelException {
+ SemanticModel model = dispatcher.alterSemanticModel(ident, changes);
+ for (SemanticModelChange change : changes) {
+ if (change instanceof SemanticModelChange.RenameSemanticModel) {
+ AuthorizationUtils.notifyEntityNameIdMappingChange(ident,
Entity.EntityType.SEMANTIC_MODEL);
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifier.of(ident.namespace(), model.name()),
Entity.EntityType.SEMANTIC_MODEL);
Review Comment:
Fixed in e07c83b99. Semantic Model lookups now canonicalize the parent
namespace before constructing either request-local or shared cache keys,
matching the identifiers used by mutation hooks. The model leaf retains its
case. The regression populates entries through lookups using SCHEMA/ScHeMa,
then checks rename, drop, and name reuse. A separate test verifies that
case-sensitive schema names remain distinct. Both pass.
--
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]
