tsungchih commented on code in PR #9378: URL: https://github.com/apache/gravitino/pull/9378#discussion_r2606842148
########## clients/client-python/gravitino/api/authorization/privileges.py: ########## @@ -0,0 +1,197 @@ +# 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. +from __future__ import annotations + +from abc import ABC, abstractmethod +from enum import Enum +from typing import TYPE_CHECKING + +from gravitino.api.metadata_object import MetadataObject + +if TYPE_CHECKING: + pass + + +class Privilege(ABC): + """The interface of a privilege. The privilege represents the ability to execute + kinds of operations for kinds of entities + """ + + @abstractmethod + def name(self) -> Privilege.Name: + """ + Return the generic name of the privilege. + + Raises: + NotImplementedError: If the method is not implemented. + + Returns: + Privilege.Name: The generic name of the privilege. + """ + raise NotImplementedError() + + @abstractmethod + def simple_string(self) -> str: + """ + Return a simple string representation of the privilege. + + Raises: + NotImplementedError: If the method is not implemented. + + Returns: + str: A readable string representation for the privilege. + """ + raise NotImplementedError() + + @abstractmethod + def condition(self) -> "Privilege.Condition": + """ + Return the condition of the privilege. + + raises: + NotImplementedError: If the method is not implemented. + + Returns: + Privilege.Condition: The condition of the privilege. + `ALLOW` means that you are allowed to use the privilege, + `DENY` means that you are denied to use the privilege + """ + raise NotImplementedError() + + @abstractmethod + def can_bind_to(self, obj_type: MetadataObject.Type) -> bool: + """ + Check whether this privilege can bind to a securable object type. + + Args: + obj_type: The securable object's metadata type. + + Returns: + True if this privilege can bind to the given type, otherwise False. + """ + raise NotImplementedError() + + class Name(Enum): + """The name of this privilege.""" + + # The privilege to create a catalog. Review Comment: I would recommend to place string literals directly below the variable name to serve as documentation that can be extracted by IDEs or documentation tools. For example: ```python CREATE_CATALOG = (0, 1 << 0) """The privilege to create a catalog.""" ``` ########## clients/client-python/gravitino/api/authorization/securable_objects.py: ########## @@ -0,0 +1,435 @@ +# 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. +from __future__ import annotations + +from abc import ABC, abstractmethod +from collections import Counter +from collections.abc import Collection + +from gravitino.api.authorization.privileges import Privilege +from gravitino.api.metadata_object import MetadataObject +from gravitino.api.metadata_objects import MetadataObjects + + +class SecurableObject(MetadataObject, ABC): + """ + A securable object is an entity on which access control can be granted. + Unless explicitly granted, access is denied. + + Apache Gravitino organizes securable objects in a tree structure. + Each securable object contains three attributes: parent, name, and type. + + Supported types: + - CATALOG + - SCHEMA + - TABLE + - FILESET + - TOPIC + - METALAKE + + Use the helper class `SecurableObjects` to construct the securable object you need. + + In RESTful APIs, you can reference a securable object using its full name and type. + + Examples + -------- + Catalog: + - Python code: + SecurableObjects.catalog("catalog1") + - REST API: + full_name="catalog1", type="CATALOG" + + Schema: + - Python code: + SecurableObjects.schema("catalog1", "schema1") + - REST API: + full_name="catalog1.schema1", type="SCHEMA" + + Table: + - Python code: + SecurableObjects.table("catalog1", "schema1", "table1") + - REST API: + full_name="catalog1.schema1.table1", type="TABLE" + + Topic: + - Python code: + SecurableObjects.topic("catalog1", "schema1", "topic1") + - REST API: + full_name="catalog1.schema1.topic1", type="TOPIC" + + Fileset: + - Python code: + SecurableObjects.fileset("catalog1", "schema1", "fileset1") + - REST API: + full_name="catalog1.schema1.fileset1", type="FILESET" + + Metalake: + - Python code: + SecurableObjects.metalake("metalake1") + - REST API: + full_name="metalake1", type="METALAKE" + + Notes + ----- + - To represent “all catalogs”, you can use the metalake as the root object. + - To grant a privilege on all children, you can assign it to their common parent. + For example, to grant READ TABLE on all tables under `catalog1.schema1`, + simply grant READ TABLE on the schema object itself. + """ + + @abstractmethod + def privileges(self) -> list["Privilege"]: + """The privileges of the securable object. For example: If the securable object is a table, the + privileges could be `READ TABLE`, `WRITE TABLE`, etc. If a schema has the privilege of `LOAD + TABLE`. It means the role can load all tables of the schema. + + returns: + The privileges of the securable object. + """ + raise NotImplementedError() + + +class SecurableObjects: + """The helper class for SecurableObject.""" + + class SecurableObjectImpl(MetadataObjects.MetadataObjectImpl, SecurableObject): + def __init__( + self, + parent: str, + name: str, + type_: MetadataObject.Type, + privileges: list[Privilege], + ) -> None: + super().__init__(name, type_, parent) + self._privileges = list(set(privileges)) + + @staticmethod + def is_equal_collection(c1: Collection, c2: Collection) -> bool: + if c1 is c2: + return True + if c1 is None or c2 is None: + return False + + return Counter(c1) == Counter(c2) + + def __hash__(self) -> int: + return hash((super().__hash__(), self._privileges)) + + def __str__(self) -> str: + privileges_str = ",".join( + f"[{p.simple_string()}]" for p in self._privileges + ) + + return ( + f"SecurableObject: [fullName={self.full_name()}], " + f"[type={self.type()}], [privileges={privileges_str}]" + ) + + def __eq__(self, other: object) -> bool: + if other is self: + return True + if not isinstance(other, SecurableObject): + return False + + return super().__eq__( + other + ) and SecurableObjects.SecurableObjectImpl.is_equal_collection( + self.privileges(), other.privileges() + ) + + def privileges(self) -> list[Privilege]: + return self._privileges + + @staticmethod + def of_metalake( + metalake: str, + privileges: list[Privilege], + ) -> SecurableObjectImpl: + """ + Create the metalake SecurableObject with the given metalake name and privileges. + + Args: + metalake (str): The metalake name. + privileges (list[Privilege]): The privileges of the metalake. + + Returns: + SecurableObjectImpl: The created metalake SecurableObject. + """ + return SecurableObjects.of( + MetadataObject.Type.METALAKE, + [metalake], + privileges, + ) + + @staticmethod + def of_catalog( + catalog: str, + privileges: list[Privilege], + ) -> SecurableObjectImpl: + """ + Create the catalog SecurableObject with the given catalog name and privileges. + + Args: + catalog (str): The catalog name + privileges (list[Privilege]): The privileges of the catalog. + + Returns: + SecurableObjectImpl: The created catalog SecurableObject. + """ + return SecurableObjects.of( + MetadataObject.Type.CATALOG, + [catalog], + privileges, + ) + + @staticmethod + def of_schema( + catalog: SecurableObject, + schema: str, + privileges: list[Privilege], + ) -> SecurableObjectImpl: + """ + Create the schema SecurableObject with the given securable catalog object, schema name and privileges. + + Args: + catalog (SecurableObject): The catalog securable object. + schema (str): The schema name. + privileges (list[Privilege]): The privileges of the schema. + + Returns: + SecurableObjectImpl: The created schema SecurableObject. + """ + names = catalog.full_name().split(".") + names.append(schema) + return SecurableObjects.of( + MetadataObject.Type.SCHEMA, + names, + privileges, + ) + + @staticmethod + def of_table( + schema: SecurableObject, + table: str, + privileges: list[Privilege], + ) -> SecurableObjectImpl: + """ + Create the table SecurableObject with the given securable schema object, table name and privileges. + + Args: + schema (SecurableObject): The schema securable object. + table (str): The table name. + privileges (list[Privilege]): The privileges of the table. + + Returns: + SecurableObjectImpl: The created table SecurableObject. + """ + names = schema.full_name().split(".") + names.append(table) + return SecurableObjects.of( + MetadataObject.Type.TABLE, + names, Review Comment: I would recommend to use unpacking here `[*schema.full_name().split("."), table]`. ########## clients/client-python/gravitino/api/authorization/securable_objects.py: ########## @@ -0,0 +1,435 @@ +# 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. +from __future__ import annotations + +from abc import ABC, abstractmethod +from collections import Counter +from collections.abc import Collection + +from gravitino.api.authorization.privileges import Privilege +from gravitino.api.metadata_object import MetadataObject +from gravitino.api.metadata_objects import MetadataObjects + + +class SecurableObject(MetadataObject, ABC): + """ + A securable object is an entity on which access control can be granted. + Unless explicitly granted, access is denied. + + Apache Gravitino organizes securable objects in a tree structure. + Each securable object contains three attributes: parent, name, and type. + + Supported types: + - CATALOG + - SCHEMA + - TABLE + - FILESET + - TOPIC + - METALAKE + + Use the helper class `SecurableObjects` to construct the securable object you need. + + In RESTful APIs, you can reference a securable object using its full name and type. + + Examples + -------- + Catalog: + - Python code: + SecurableObjects.catalog("catalog1") + - REST API: + full_name="catalog1", type="CATALOG" + + Schema: + - Python code: + SecurableObjects.schema("catalog1", "schema1") + - REST API: + full_name="catalog1.schema1", type="SCHEMA" + + Table: + - Python code: + SecurableObjects.table("catalog1", "schema1", "table1") + - REST API: + full_name="catalog1.schema1.table1", type="TABLE" + + Topic: + - Python code: + SecurableObjects.topic("catalog1", "schema1", "topic1") + - REST API: + full_name="catalog1.schema1.topic1", type="TOPIC" + + Fileset: + - Python code: + SecurableObjects.fileset("catalog1", "schema1", "fileset1") + - REST API: + full_name="catalog1.schema1.fileset1", type="FILESET" + + Metalake: + - Python code: + SecurableObjects.metalake("metalake1") + - REST API: + full_name="metalake1", type="METALAKE" + + Notes + ----- + - To represent “all catalogs”, you can use the metalake as the root object. + - To grant a privilege on all children, you can assign it to their common parent. + For example, to grant READ TABLE on all tables under `catalog1.schema1`, + simply grant READ TABLE on the schema object itself. + """ + + @abstractmethod + def privileges(self) -> list["Privilege"]: + """The privileges of the securable object. For example: If the securable object is a table, the + privileges could be `READ TABLE`, `WRITE TABLE`, etc. If a schema has the privilege of `LOAD + TABLE`. It means the role can load all tables of the schema. + + returns: + The privileges of the securable object. + """ + raise NotImplementedError() + + +class SecurableObjects: + """The helper class for SecurableObject.""" + + class SecurableObjectImpl(MetadataObjects.MetadataObjectImpl, SecurableObject): + def __init__( + self, + parent: str, + name: str, + type_: MetadataObject.Type, + privileges: list[Privilege], + ) -> None: + super().__init__(name, type_, parent) + self._privileges = list(set(privileges)) + + @staticmethod + def is_equal_collection(c1: Collection, c2: Collection) -> bool: + if c1 is c2: + return True + if c1 is None or c2 is None: + return False + + return Counter(c1) == Counter(c2) + + def __hash__(self) -> int: + return hash((super().__hash__(), self._privileges)) + + def __str__(self) -> str: + privileges_str = ",".join( + f"[{p.simple_string()}]" for p in self._privileges + ) + + return ( + f"SecurableObject: [fullName={self.full_name()}], " + f"[type={self.type()}], [privileges={privileges_str}]" + ) + + def __eq__(self, other: object) -> bool: + if other is self: + return True + if not isinstance(other, SecurableObject): + return False + + return super().__eq__( + other + ) and SecurableObjects.SecurableObjectImpl.is_equal_collection( + self.privileges(), other.privileges() + ) + + def privileges(self) -> list[Privilege]: + return self._privileges + + @staticmethod + def of_metalake( + metalake: str, + privileges: list[Privilege], + ) -> SecurableObjectImpl: + """ + Create the metalake SecurableObject with the given metalake name and privileges. + + Args: + metalake (str): The metalake name. + privileges (list[Privilege]): The privileges of the metalake. + + Returns: + SecurableObjectImpl: The created metalake SecurableObject. + """ + return SecurableObjects.of( + MetadataObject.Type.METALAKE, + [metalake], + privileges, + ) + + @staticmethod + def of_catalog( + catalog: str, + privileges: list[Privilege], + ) -> SecurableObjectImpl: + """ + Create the catalog SecurableObject with the given catalog name and privileges. + + Args: + catalog (str): The catalog name + privileges (list[Privilege]): The privileges of the catalog. + + Returns: + SecurableObjectImpl: The created catalog SecurableObject. + """ + return SecurableObjects.of( + MetadataObject.Type.CATALOG, + [catalog], + privileges, + ) + + @staticmethod + def of_schema( + catalog: SecurableObject, + schema: str, + privileges: list[Privilege], + ) -> SecurableObjectImpl: + """ + Create the schema SecurableObject with the given securable catalog object, schema name and privileges. + + Args: + catalog (SecurableObject): The catalog securable object. + schema (str): The schema name. + privileges (list[Privilege]): The privileges of the schema. + + Returns: + SecurableObjectImpl: The created schema SecurableObject. + """ + names = catalog.full_name().split(".") Review Comment: Why do we need to do split here? The behavior is different from the Java client. https://github.com/apache/gravitino/blob/ff14d4b254dbcc873948990b7001a67024e4bab3/api/src/main/java/org/apache/gravitino/authorization/SecurableObjects.java#L74 ########## clients/client-python/gravitino/api/authorization/role_change.py: ########## @@ -0,0 +1,235 @@ +# 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. + +from __future__ import annotations + +from abc import ABC +from dataclasses import dataclass, field + +from dataclasses_json import config + +from gravitino.api.authorization.securable_objects import SecurableObject +from gravitino.utils.precondition import Precondition + + +class RoleChange(ABC): + """The RoleChange interface defines the public API for managing roles in an authorization.""" + + @staticmethod + def add_securable_object( + role_name: str, + securable_object: SecurableObject, + ) -> RoleChange.AddSecurableObject: + """ + Create a RoleChange to add a securable object into a role. + + Args: + role_name (str): The role name. + securable_object (SecurableObject): The securable object. + + Returns: + RoleChange.AddSecurableObject: return a RoleChange for the added securable object. + """ + return RoleChange.AddSecurableObject(role_name, securable_object) + + @staticmethod + def remove_securable_object( + role_name: str, + securable_object: SecurableObject, + ) -> RoleChange.RemoveSecurableObject: + """ + Create a RoleChange to remove a securable object from a role. + + Args: + role_name (str): The role name. + securable_object (SecurableObject): The securable object. + + Returns: + RoleChange.RemoveSecurableObject: return a RoleChange for the added securable object. + """ + return RoleChange.RemoveSecurableObject(role_name, securable_object) + + @staticmethod + def update_securable_object( + role_name: str, + securable_object: SecurableObject, + new_securable_object: SecurableObject, + ) -> RoleChange.UpdateSecurableObject: + """ + Update a securable object RoleChange. + + Args: + role_name (str): The role name. + securable_object (SecurableObject): The securable object. + new_securable_object (SecurableObject): The new securable object. + + Returns: + RoleChange.UpdateSecurableObject: return a RoleChange for the update-securable object. + """ + return RoleChange.UpdateSecurableObject( + role_name, securable_object, new_securable_object + ) + + @dataclass(frozen=True, eq=True) + class AddSecurableObject: + """A AddSecurableObject to add a securable object to a role.""" + + _role_name: str = field(metadata=config(field_name="role_name")) + _securable_object: SecurableObject = field( + metadata=config(field_name="securable_object") + ) + + @property + def role_name(self) -> str: + """ + Returns the role name to be added. + + Returns: + str: The role name to be added. + """ + return self._role_name + + @property + def securable_object(self) -> SecurableObject: + """ + Returns the securable object to be added. + + Returns: + SecurableObject: The securable object to be added. + """ + return self._securable_object + + def __str__(self) -> str: + """ + Returns a string representation of the AddSecurableObject instance. This string format + includes the class name followed by the add securable object operation. + + Returns: + str: A string representation of the AddSecurableObject instance. + """ + return f"ADDSECURABLEOBJECT {self._role_name} + {self._securable_object}" Review Comment: I think there's no `+` in the string representation. It turns out to be `f"ADDSECURABLEOBJECT {self._role_name} {self._securable_object}"`. Similar issue in other `XXXSecurableObject`. -- 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]
