This is an automated email from the ASF dual-hosted git repository. diqiu50 pushed a commit to branch cherry-pick/10659-to-branch-1.2 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit fc3f120dffca0004e6a11cd218cee853728a9817 Author: Chisom Uma <[email protected]> AuthorDate: Thu Apr 23 13:36:23 2026 +0100 [Cherry-pick to branch-1.2] [#10659] fix(core): Correctly handle tag rename conflict in TagManager (#10661) --- .../org/apache/gravitino/tag/TagOperations.java | 1 + .../apache/gravitino/client/GravitinoClient.java | 2 +- .../apache/gravitino/client/GravitinoMetalake.java | 2 +- .../gravitino/api/tag/tag_operations.py | 2 + .../gravitino/client/gravitino_client.py | 2 + .../gravitino/client/gravitino_metalake.py | 2 + .../apache/gravitino/hook/TagHookDispatcher.java | 4 +- .../gravitino/listener/TagEventDispatcher.java | 4 +- .../org/apache/gravitino/tag/TagDispatcher.java | 3 + .../java/org/apache/gravitino/tag/TagManager.java | 12 +- .../org/apache/gravitino/tag/TestTagManager.java | 18 +++ design-docs/gravitino-function-privilege.md | 163 +++++++++++++++++++++ 12 files changed, 207 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/apache/gravitino/tag/TagOperations.java b/api/src/main/java/org/apache/gravitino/tag/TagOperations.java index 1745efb236..c58ceeda8e 100644 --- a/api/src/main/java/org/apache/gravitino/tag/TagOperations.java +++ b/api/src/main/java/org/apache/gravitino/tag/TagOperations.java @@ -78,6 +78,7 @@ public interface TagOperations { * @return The altered tag. * @throws NoSuchTagException If the tag does not exist. * @throws IllegalArgumentException If the changes cannot be applied to the tag. + * @throws TagAlreadyExistsException If a tag with the new name already exists. */ Tag alterTag(String name, TagChange... changes) throws NoSuchTagException, IllegalArgumentException; diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java index d47c88882a..c7e19ba73c 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java @@ -565,7 +565,7 @@ public class GravitinoClient extends GravitinoClientBase @Override public Tag alterTag(String name, TagChange... changes) - throws NoSuchTagException, IllegalArgumentException { + throws NoSuchTagException, IllegalArgumentException, TagAlreadyExistsException { return getMetalake().alterTag(name, changes); } diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java index 1d3c9c2a23..2be2b88d92 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java @@ -537,7 +537,7 @@ public class GravitinoMetalake extends MetalakeDTO */ @Override public Tag alterTag(String name, TagChange... changes) - throws NoSuchTagException, IllegalArgumentException { + throws NoSuchTagException, IllegalArgumentException, TagAlreadyExistsException { Preconditions.checkArgument(StringUtils.isNotBlank(name), "tag name must not be null or empty"); List<TagUpdateRequest> updates = Arrays.stream(changes).map(DTOConverters::toTagUpdateRequest).collect(Collectors.toList()); diff --git a/clients/client-python/gravitino/api/tag/tag_operations.py b/clients/client-python/gravitino/api/tag/tag_operations.py index 58bbce2657..7089564705 100644 --- a/clients/client-python/gravitino/api/tag/tag_operations.py +++ b/clients/client-python/gravitino/api/tag/tag_operations.py @@ -114,6 +114,8 @@ class TagOperations(ABC): Raises: NoSuchTagException: If the tag does not exist. NoSuchMetalakeException: If the metalake does not exist. + IllegalArgumentException: If the changes cannot be applied to the tag. + TagAlreadyExistsException: If a tag with the new name already exists. """ pass diff --git a/clients/client-python/gravitino/client/gravitino_client.py b/clients/client-python/gravitino/client/gravitino_client.py index 862b4bce33..29c36a8937 100644 --- a/clients/client-python/gravitino/client/gravitino_client.py +++ b/clients/client-python/gravitino/client/gravitino_client.py @@ -312,6 +312,8 @@ class GravitinoClient(GravitinoClientBase, SupportsJobs, TagOperations): Raises: NoSuchTagException: If the tag does not exist. NoSuchMetalakeException: If the metalake does not exist. + IllegalArgumentException: If the changes cannot be applied to the tag. + TagAlreadyExistsException: If a tag with the new name already exists. """ return self.get_metalake().alter_tag(tag_name, *changes) diff --git a/clients/client-python/gravitino/client/gravitino_metalake.py b/clients/client-python/gravitino/client/gravitino_metalake.py index 00b7ae3694..abb2f0cc2f 100644 --- a/clients/client-python/gravitino/client/gravitino_metalake.py +++ b/clients/client-python/gravitino/client/gravitino_metalake.py @@ -622,6 +622,8 @@ class GravitinoMetalake( Raises: NoSuchTagException: If the tag does not exist. NoSuchMetalakeException: If the metalake does not exist. + IllegalArgumentException: If the changes cannot be applied to the tag. + TagAlreadyExistsException: If a tag with the new name already exists. """ # TODO implement alter_tag raise NotImplementedError() diff --git a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java index 1db84fdff8..444f597fe4 100644 --- a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java @@ -24,6 +24,7 @@ import org.apache.gravitino.MetadataObject; import org.apache.gravitino.authorization.Owner; import org.apache.gravitino.authorization.OwnerDispatcher; import org.apache.gravitino.exceptions.NoSuchTagException; +import org.apache.gravitino.exceptions.TagAlreadyExistsException; import org.apache.gravitino.tag.Tag; import org.apache.gravitino.tag.TagChange; import org.apache.gravitino.tag.TagDispatcher; @@ -73,7 +74,8 @@ public class TagHookDispatcher implements TagDispatcher { } @Override - public Tag alterTag(String metalake, String name, TagChange... changes) { + public Tag alterTag(String metalake, String name, TagChange... changes) + throws IllegalArgumentException, TagAlreadyExistsException { return dispatcher.alterTag(metalake, name, changes); } diff --git a/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java b/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java index 2dd81cde3f..8b06f10ddb 100644 --- a/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java @@ -21,6 +21,7 @@ package org.apache.gravitino.listener; import java.util.Map; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.exceptions.NoSuchTagException; +import org.apache.gravitino.exceptions.TagAlreadyExistsException; import org.apache.gravitino.listener.api.event.AlterTagEvent; import org.apache.gravitino.listener.api.event.AlterTagFailureEvent; import org.apache.gravitino.listener.api.event.AlterTagPreEvent; @@ -141,7 +142,8 @@ public class TagEventDispatcher implements TagDispatcher { } @Override - public Tag alterTag(String metalake, String name, TagChange... changes) { + public Tag alterTag(String metalake, String name, TagChange... changes) + throws IllegalArgumentException, TagAlreadyExistsException { AlterTagPreEvent preEvent = new AlterTagPreEvent(PrincipalUtils.getCurrentUserName(), metalake, name, changes); diff --git a/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java b/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java index 6e0276703d..e48113dfdf 100644 --- a/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java @@ -21,6 +21,7 @@ package org.apache.gravitino.tag; import java.util.Map; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.exceptions.NoSuchTagException; +import org.apache.gravitino.exceptions.TagAlreadyExistsException; /** * {@code TagDispatcher} interface provides functionalities for managing tags within a metalake. It @@ -72,6 +73,8 @@ public interface TagDispatcher { * @param name The name of the tag. * @param changes The changes to apply to the tag. * @return The updated tag. + * @throws IllegalArgumentException If the changes cannot be applied to the tag. + * @throws TagAlreadyExistsException If a tag with the new name already exists. */ Tag alterTag(String metalake, String name, TagChange... changes); diff --git a/core/src/main/java/org/apache/gravitino/tag/TagManager.java b/core/src/main/java/org/apache/gravitino/tag/TagManager.java index 38c3f5bab7..eb26cf37e9 100644 --- a/core/src/main/java/org/apache/gravitino/tag/TagManager.java +++ b/core/src/main/java/org/apache/gravitino/tag/TagManager.java @@ -173,10 +173,14 @@ public class TagManager implements TagDispatcher { throw new NoSuchTagException( "Tag with name %s under metalake %s does not exist", name, metalake); } catch (EntityAlreadyExistsException e) { - throw new RuntimeException( - String.format( - "Trying to alter tag %s under metalake %s, but the new name already exists", - name, metalake)); + String newName = + Arrays.stream(changes) + .filter(c -> c instanceof TagChange.RenameTag) + .map(c -> ((TagChange.RenameTag) c).getNewName()) + .findFirst() + .orElse(name); + throw new TagAlreadyExistsException( + e, "Tag with name %s under metalake %s already exists", newName, metalake); } catch (IOException ioe) { LOG.error("Failed to alter tag {} under metalake {}", name, metalake, ioe); throw new RuntimeException(ioe); diff --git a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java index c1ca78e8cf..d7e86abba3 100644 --- a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java +++ b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java @@ -356,6 +356,24 @@ public class TestTagManager { Assertions.assertEquals(expectedProp2, removedPropTag.properties()); } + @Test + public void testAlterTagRenameToExistingTag() { + tagManager.createTag(METALAKE, "tag1", null, null); + tagManager.createTag(METALAKE, "tag2", null, null); + + TagAlreadyExistsException exception = + Assertions.assertThrows( + TagAlreadyExistsException.class, + () -> tagManager.alterTag(METALAKE, "tag1", TagChange.rename("tag2"))); + + Assertions.assertEquals( + "Tag with name tag2 under metalake metalake_for_tag_test already exists", + exception.getMessage()); + + Assertions.assertEquals("tag1", tagManager.getTag(METALAKE, "tag1").name()); + Assertions.assertEquals("tag2", tagManager.getTag(METALAKE, "tag2").name()); + } + @Test public void testDeleteTag() { tagManager.createTag(METALAKE, "tag1", null, null); diff --git a/design-docs/gravitino-function-privilege.md b/design-docs/gravitino-function-privilege.md new file mode 100644 index 0000000000..8ad3c01932 --- /dev/null +++ b/design-docs/gravitino-function-privilege.md @@ -0,0 +1,163 @@ +<!-- + 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. +--> + +# Design of Function Privilege Control in Gravitino + +## Background + +Apache Gravitino provides a unified function registry that allows users to define custom functions (UDFs) once and share them across multiple compute engines (Trino, Spark, Flink). Gravitino currently supports full function metadata management — register, list, get, alter, drop — but **does not yet have privilege control for functions**. + +The existing Gravitino access control framework covers catalogs, schemas, tables, views, topics, filesets, models, tags, policies, and jobs. Each object type has well-defined privilege types (e.g., `CREATE_TABLE`, `SELECT_TABLE`, `MODIFY_TABLE`) with privilege inheritance through the metalake → catalog → schema hierarchy. Functions are the notable gap in this model: + +- **No function privilege types** are defined in `Privilege.Name` — any authenticated user can register, execute, alter, or drop any function. +- **No function visibility control** — `listFunctions` returns all functions regardless of the user's permissions. +- **No ownership tracking** — `FunctionHookDispatcher` is missing, so function ownership is not set on creation. + +--- + +## Goals + +1. **Integrate with Existing Access Control Framework**: Define function privilege types that follow established Gravitino naming conventions and privilege inheritance patterns. + +2. **Function Visibility Control**: Users should only see functions they have privileges on. `listFunctions` and `getFunction` should filter results based on user permissions, consistent with how tables and filesets handle visibility. + +3. **Ownership Tracking**: Functions should have owners, set automatically on registration and manageable through Gravitino's existing ownership mechanism. + +4. **Backward Compatibility**: Existing function management APIs remain unchanged. Privilege enforcement is additive — when authorization is disabled, behavior is identical to current functionality. + +--- + +## Non-Goals + +1. **DEFINER/INVOKER Security Mode**: Gravitino's function metadata model does not currently include a `securityType` field. Adding security mode support to functions is a separate metadata model concern and is outside the scope of this privilege design. + +2. **New Function Management Capabilities**: This design adds privilege control on top of the existing function management API. No new CRUD operations or metadata model changes are introduced. + +3. **Per-Definition Privilege Control**: Privileges are defined at the function level, not at the individual function definition (overload) level. A user who can execute a function can execute any of its definitions. + +4. **Built-in Function Privilege Control**: Built-in functions provided by compute engines are not managed by Gravitino and are outside the scope of this design. + +--- + +## Proposal + +### Privilege Types + +Three privilege types are defined for functions, following established Gravitino privilege naming conventions: + +| Privilege | Securable Object Levels | Description | +|---------------------|-------------------------------------|---------------------------------------------------------------| +| `REGISTER_FUNCTION` | Metalake, Catalog, Schema | Permission to register new functions | +| `EXECUTE_FUNCTION` | Metalake, Catalog, Schema, Function | Permission to execute/invoke a function and view its metadata | +| `MODIFY_FUNCTION` | Metalake, Catalog, Schema, Function | Permission to alter a function's metadata | + +**Naming rationale:** + +- `REGISTER_FUNCTION` — Consistent with `REGISTER_MODEL` and the `registerFunction` API method. Gravitino uses "register" for managed metadata objects (functions, models) to distinguish from "create" used for delegated objects (tables, views, schemas). +- `EXECUTE_FUNCTION` — The universally adopted privilege name across all surveyed systems (Databricks UC, Trino, MySQL, PostgreSQL, OceanBase, Hologres). Analogous to `SELECT_TABLE` / `SELECT_VIEW` in semantics (the fundamental "use" privilege), but `EXECUTE` is the standard term for functions. +- `MODIFY_FUNCTION` — Consistent with `MODIFY_TABLE`. Covers alter operations on function metadata (comment, definitions, properties). Drop operations require function ownership, following the same pattern as table and fileset drop (see `TableOperations.java` and `FilesetOperations.java` where DROP uses owner-only expressions). + +**Privilege inheritance:** Privileges granted at metalake, catalog, or schema level cascade to all functions within that scope, consistent with existing behavior for tables and filesets. + +**Deny privileges:** Each privilege has a corresponding deny form (`DENY_REGISTER_FUNCTION`, `DENY_EXECUTE_FUNCTION`, `DENY_MODIFY_FUNCTION`) following the existing deny privilege pattern. + +--- + +### Securable Object Hierarchy + +Functions follow the existing three-level namespace hierarchy: + +``` +metalake + └── catalog + └── schema + └── function +``` + +This is consistent with tables, filesets, and other schema-scoped objects. Functions can be registered under any catalog type that supports schemas (relational, Hive, Iceberg, etc.). A new `MetadataObject.Type.FUNCTION` type and `SecurableObjects.ofFunction()` factory method are added. + +**Privilege applicability by level:** + +| Securable Object | REGISTER_FUNCTION | EXECUTE_FUNCTION | MODIFY_FUNCTION | +|------------------|-------------------|------------------|-----------------| +| Metalake | ✅ | ✅ | ✅ | +| Catalog | ✅ | ✅ | ✅ | +| Schema | ✅ | ✅ | ✅ | +| Function | — | ✅ | ✅ | + +> `REGISTER_FUNCTION` is not applicable at the function level because creation happens at the schema level (a function must be created within a schema). + +--- + +### Visibility Control + +Function visibility follows the same pattern as tables and filesets — users can only see functions they have at least one operational privilege on: + +1. **`listFunctions`** + - Requires `USE_CATALOG` + `USE_SCHEMA` at the endpoint level to access the schema (consistent with `listTables`). + - Applies a filter expression to the result set — only functions the user has at least one privilege on (`EXECUTE_FUNCTION`, `MODIFY_FUNCTION`, or ownership) are returned. + - `REGISTER_FUNCTION` alone does not grant visibility (consistent with `CREATE_TABLE` not granting table visibility). + +2. **`getFunction`** + - Requires `USE_CATALOG` + `USE_SCHEMA` at the endpoint level, plus `EXECUTE_FUNCTION`, `MODIFY_FUNCTION`, or ownership (consistent with `loadTable`). + - If the user lacks privileges, the authorization framework denies access. + +--- + +### Authorization Pushdown — Not Applicable + +Unlike tables, which are delegated to underlying data sources (Hive, MySQL, Iceberg, etc.) that have their own privilege systems, **function management is fully managed by Gravitino** — all function metadata is stored in Gravitino's own database via `ManagedFunctionOperations`. There is no delegation to external catalogs. + +Therefore, **authorization pushdown is not needed for functions**. Gravitino's own authorization layer is the single enforcement point for all function privilege checks. This is simpler than the table model and eliminates the complexity of privilege mapping to heterogeneous data source privilege systems. + +--- + +### GRANT / REVOKE Syntax + +Function privileges are managed through Gravitino's existing GRANT/REVOKE REST API. No new API endpoints are needed. + +**REST API:** + +``` +PUT /api/metalakes/{metalake}/roles/{role}/grant +``` + +```json +{ + "securableObjects": [ + { + "fullName": "catalog.schema.my_function", + "type": "FUNCTION", + "privileges": [ + {"name": "EXECUTE_FUNCTION", "condition": "ALLOW"} + ] + } + ] +} +``` + +> Note: Trino's `GRANT` SQL syntax only supports table and schema privileges (see [Trino GRANT docs](https://trino.io/docs/current/sql/grant.html)); there is no `GRANT ... ON FUNCTION` syntax. Spark and Flink also do not support SQL `GRANT`/`REVOKE`. The Gravitino CLI (`gcli`) does not yet support function privilege types either (the CLI's privilege validation list currently covers catalog, schema, table, fileset, and topic privileges only). Users should manage function privileges throug [...] + +--- + +### Engine Connector Integration + +All function operations (register, get, list, alter, drop) go through the Gravitino server's REST API, where the `@AuthorizationExpression` annotations on `FunctionOperations.java` automatically enforce privilege checks when authorization is enabled on the Gravitino server. + +This is the same pattern used for tables and other objects — the server-side authorization layer is the single enforcement point, and the engine connectors (Trino, Spark, Flink) act as pure REST clients.
