From 09535ce2565061c7753a94a798f8697ad9db57db Mon Sep 17 00:00:00 2001 From: Sun Yuhan Date: Fri, 15 May 2026 11:20:00 +0800 Subject: [PATCH 1/6] [#11094] feat(client-python): add Group authorization management Add Group entity support to the Python SDK, including: - Group API interface and GroupDTO with immutable roles - GroupAddRequest, GroupResponse, GroupListResponse, GroupNamesListResponse - GroupErrorHandler for REST error mapping - GravitinoMetalake group CRUD methods (add/remove/get/list/list_names) - GravitinoClient delegate methods - Unit tests and integration tests --- .../gravitino/api/authorization/__init__.py | 2 + .../gravitino/api/authorization/group.py | 47 +++ .../gravitino/client/gravitino_client.py | 69 +++++ .../gravitino/client/gravitino_metalake.py | 119 ++++++++ .../gravitino/dto/authorization/group_dto.py | 93 ++++++ .../dto/requests/group_add_request.py | 38 +++ .../gravitino/dto/responses/group_response.py | 79 ++++++ .../gravitino/exceptions/base.py | 8 + .../handlers/group_error_handler.py | 67 +++++ .../integration/test_group_management.py | 89 ++++++ .../client/test_metalake_group_operations.py | 268 ++++++++++++++++++ .../dto/responses/test_group_response.py | 76 +++++ .../tests/unittests/dto/test_group_dto.py | 130 +++++++++ .../tests/unittests/test_error_handler.py | 59 ++++ 14 files changed, 1144 insertions(+) create mode 100644 clients/client-python/gravitino/api/authorization/group.py create mode 100644 clients/client-python/gravitino/dto/authorization/group_dto.py create mode 100644 clients/client-python/gravitino/dto/requests/group_add_request.py create mode 100644 clients/client-python/gravitino/dto/responses/group_response.py create mode 100644 clients/client-python/gravitino/exceptions/handlers/group_error_handler.py create mode 100644 clients/client-python/tests/integration/test_group_management.py create mode 100644 clients/client-python/tests/unittests/client/test_metalake_group_operations.py create mode 100644 clients/client-python/tests/unittests/dto/responses/test_group_response.py create mode 100644 clients/client-python/tests/unittests/dto/test_group_dto.py diff --git a/clients/client-python/gravitino/api/authorization/__init__.py b/clients/client-python/gravitino/api/authorization/__init__.py index f1b0f5de987..b8420e89153 100644 --- a/clients/client-python/gravitino/api/authorization/__init__.py +++ b/clients/client-python/gravitino/api/authorization/__init__.py @@ -17,12 +17,14 @@ from __future__ import annotations +from gravitino.api.authorization.group import Group from gravitino.api.authorization.privileges import Privileges from gravitino.api.authorization.role import Role from gravitino.api.authorization.securable_objects import SecurableObjects from gravitino.api.authorization.user import User __all__ = [ + "Group", "Role", "SecurableObjects", "Privileges", diff --git a/clients/client-python/gravitino/api/authorization/group.py b/clients/client-python/gravitino/api/authorization/group.py new file mode 100644 index 00000000000..ba5a017d0b6 --- /dev/null +++ b/clients/client-python/gravitino/api/authorization/group.py @@ -0,0 +1,47 @@ +# 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 abstractmethod + +from gravitino.api.auditable import Auditable + + +class Group(Auditable): + """The interface of a group. The group is a collection of users in the authorization system.""" + + @abstractmethod + def name(self) -> str: + """ + The name of the group. + + Returns: + str: The name of the group. + """ + raise NotImplementedError() + + @abstractmethod + def roles(self) -> list[str]: + """ + The roles of the group. A group can have multiple roles. + Every role binds several privileges. + + Returns: + list[str]: The role names of the group. + """ + raise NotImplementedError() diff --git a/clients/client-python/gravitino/client/gravitino_client.py b/clients/client-python/gravitino/client/gravitino_client.py index 1503e265068..f61185d36c2 100644 --- a/clients/client-python/gravitino/client/gravitino_client.py +++ b/clients/client-python/gravitino/client/gravitino_client.py @@ -19,6 +19,7 @@ from typing import Dict, List, Optional +from gravitino.api.authorization.group import Group from gravitino.api.authorization.owner import Owner from gravitino.api.authorization.user import User from gravitino.api.catalog import Catalog @@ -438,3 +439,71 @@ def list_user_names(self) -> list[str]: NoSuchMetalakeException: If the metalake does not exist. """ return self.get_metalake().list_user_names() + + # Group operations + + def add_group(self, group: str) -> Group: + """Add a group to the metalake. + + Args: + group: The name of the group. + + Returns: + The added Group object. + + Raises: + GroupAlreadyExistsException: If a group with the same name already exists. + NoSuchMetalakeException: If the metalake does not exist. + """ + return self.get_metalake().add_group(group) + + def remove_group(self, group: str) -> bool: + """Remove a group from the metalake. + + Args: + group: The name of the group. + + Returns: + True if the group was removed, False if the group did not exist. + + Raises: + NoSuchMetalakeException: If the metalake does not exist. + """ + return self.get_metalake().remove_group(group) + + def get_group(self, group: str) -> Group: + """Get a group by name from the metalake. + + Args: + group: The name of the group. + + Returns: + The Group object. + + Raises: + NoSuchGroupException: If the group does not exist. + NoSuchMetalakeException: If the metalake does not exist. + """ + return self.get_metalake().get_group(group) + + def list_groups(self) -> list[Group]: + """List all groups with details under the metalake. + + Returns: + A list of Group objects. + + Raises: + NoSuchMetalakeException: If the metalake does not exist. + """ + return self.get_metalake().list_groups() + + def list_group_names(self) -> list[str]: + """List all group names under the metalake. + + Returns: + A list of group name strings. + + Raises: + NoSuchMetalakeException: If the metalake does not exist. + """ + return self.get_metalake().list_group_names() diff --git a/clients/client-python/gravitino/client/gravitino_metalake.py b/clients/client-python/gravitino/client/gravitino_metalake.py index 4528aac5914..65c1d4354cb 100644 --- a/clients/client-python/gravitino/client/gravitino_metalake.py +++ b/clients/client-python/gravitino/client/gravitino_metalake.py @@ -18,6 +18,7 @@ import logging from typing import Dict, List, Optional +from gravitino.api.authorization.group import Group from gravitino.api.authorization.owner import Owner from gravitino.api.authorization.user import User from gravitino.api.catalog import Catalog @@ -47,6 +48,7 @@ from gravitino.dto.requests.tag_create_request import TagCreateRequest from gravitino.dto.requests.tag_updates_request import TagUpdatesRequest from gravitino.dto.requests.user_add_request import UserAddRequest +from gravitino.dto.requests.group_add_request import GroupAddRequest from gravitino.dto.responses.catalog_list_response import CatalogListResponse from gravitino.dto.responses.catalog_response import CatalogResponse from gravitino.dto.responses.drop_response import DropResponse @@ -68,7 +70,13 @@ UserNamesListResponse, UserResponse, ) +from gravitino.dto.responses.group_response import ( + GroupListResponse, + GroupNamesListResponse, + GroupResponse, +) from gravitino.exceptions.handlers.catalog_error_handler import CATALOG_ERROR_HANDLER +from gravitino.exceptions.handlers.group_error_handler import GROUP_ERROR_HANDLER from gravitino.exceptions.handlers.job_error_handler import JOB_ERROR_HANDLER from gravitino.exceptions.handlers.owner_error_handler import OWNER_ERROR_HANDLER from gravitino.exceptions.handlers.tag_error_handler import TAG_ERROR_HANDLER @@ -104,6 +112,8 @@ class GravitinoMetalake( # Authorization paths API_METALAKES_USERS_PATH = "api/metalakes/{}/users" API_METALAKES_USER_PATH = "api/metalakes/{}/users/{}" + API_METALAKES_GROUPS_PATH = "api/metalakes/{}/groups" + API_METALAKES_GROUP_PATH = "api/metalakes/{}/groups/{}" def __init__(self, metalake: MetalakeDTO = None, client: HTTPClient = None): super().__init__( @@ -883,3 +893,112 @@ def list_user_names(self) -> list[str]: resp = UserNamesListResponse.from_json(response.body, infer_missing=True) resp.validate() return resp.names() + + ##################### + # Group operations + ##################### + + def add_group(self, group: str) -> Group: + """Add a group to this metalake. + + Args: + group: The name of the group. + + Returns: + The added Group object. + + Raises: + GroupAlreadyExistsException: If a group with the same name already exists. + NoSuchMetalakeException: If the metalake does not exist. + """ + Precondition.check_string_not_empty( + group, "group name must not be null or empty" + ) + req = GroupAddRequest(group) + req.validate() + url = self.API_METALAKES_GROUPS_PATH.format(encode_string(self.name())) + response = self.rest_client.post( + url, json=req, error_handler=GROUP_ERROR_HANDLER + ) + resp = GroupResponse.from_json(response.body, infer_missing=True) + resp.validate() + return resp.group() + + def remove_group(self, group: str) -> bool: + """Remove a group from this metalake. + + Args: + group: The name of the group. + + Returns: + True if the group was removed, False if the group did not exist. + + Raises: + NoSuchMetalakeException: If the metalake does not exist. + """ + Precondition.check_string_not_empty( + group, "group name must not be null or empty" + ) + url = self.API_METALAKES_GROUP_PATH.format( + encode_string(self.name()), encode_string(group) + ) + response = self.rest_client.delete(url, error_handler=GROUP_ERROR_HANDLER) + remove_response = RemoveResponse.from_json(response.body, infer_missing=True) + remove_response.validate() + return remove_response.removed() + + def get_group(self, group: str) -> Group: + """Get a group by name from this metalake. + + Args: + group: The name of the group. + + Returns: + The Group object. + + Raises: + NoSuchGroupException: If the group does not exist. + NoSuchMetalakeException: If the metalake does not exist. + """ + Precondition.check_string_not_empty( + group, "group name must not be null or empty" + ) + url = self.API_METALAKES_GROUP_PATH.format( + encode_string(self.name()), encode_string(group) + ) + response = self.rest_client.get(url, error_handler=GROUP_ERROR_HANDLER) + resp = GroupResponse.from_json(response.body, infer_missing=True) + resp.validate() + return resp.group() + + def list_groups(self) -> list[Group]: + """List all groups with details under this metalake. + + Returns: + A list of Group objects. + + Raises: + NoSuchMetalakeException: If the metalake does not exist. + """ + url = self.API_METALAKES_GROUPS_PATH.format(encode_string(self.name())) + response = self.rest_client.get( + url, params={"details": "true"}, error_handler=GROUP_ERROR_HANDLER + ) + resp = GroupListResponse.from_json(response.body, infer_missing=True) + resp.validate() + return resp.groups() + + def list_group_names(self) -> list[str]: + """List all group names under this metalake. + + Returns: + A list of group name strings. + + Raises: + NoSuchMetalakeException: If the metalake does not exist. + """ + url = self.API_METALAKES_GROUPS_PATH.format(encode_string(self.name())) + response = self.rest_client.get(url, error_handler=GROUP_ERROR_HANDLER) + resp = GroupNamesListResponse.from_json(response.body, infer_missing=True) + resp.validate() + return resp.names() diff --git a/clients/client-python/gravitino/dto/authorization/group_dto.py b/clients/client-python/gravitino/dto/authorization/group_dto.py new file mode 100644 index 00000000000..31e86ea6a1e --- /dev/null +++ b/clients/client-python/gravitino/dto/authorization/group_dto.py @@ -0,0 +1,93 @@ +# 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 dataclasses import dataclass, field +from typing import Optional + +from dataclasses_json import config, dataclass_json + +from gravitino.api.authorization.group import Group +from gravitino.dto.audit_dto import AuditDTO + + +@dataclass_json +@dataclass +class GroupDTO(Group): + """Represents a Group Data Transfer Object (DTO).""" + + _name: str = field(metadata=config(field_name="name")) + _roles: tuple[str, ...] = field( + default_factory=tuple, metadata=config(field_name="roles") + ) + _audit: Optional[AuditDTO] = field( + default=None, metadata=config(field_name="audit") + ) + + def __eq__(self, other: object) -> bool: + if not isinstance(other, GroupDTO): + return False + return ( + self._name == other._name + and self._roles == other._roles + and self._audit == other._audit + ) + + def __hash__(self) -> int: + return hash((self._name, tuple(self._roles), self._audit)) + + @staticmethod + def builder() -> GroupDTO.Builder: + return GroupDTO.Builder() + + def name(self) -> str: + return self._name + + def roles(self) -> list[str]: + return list(self._roles) if self._roles else [] + + def audit_info(self) -> Optional[AuditDTO]: + return self._audit + + class Builder: + """Helper class to build a GroupDTO object.""" + + def __init__(self) -> None: + self._name: str = "" + self._roles: tuple[str, ...] = () + self._audit: Optional[AuditDTO] = None + + def with_name(self, name: str) -> GroupDTO.Builder: + self._name = name + return self + + def with_roles(self, roles: list[str]) -> GroupDTO.Builder: + if roles is not None: + self._roles = tuple(roles) + return self + + def with_audit(self, audit: AuditDTO) -> GroupDTO.Builder: + self._audit = audit + return self + + def build(self) -> GroupDTO: + if not self._name: + raise ValueError("name cannot be null or empty") + if self._audit is None: + raise ValueError("audit cannot be null") + return GroupDTO(self._name, self._roles, self._audit) diff --git a/clients/client-python/gravitino/dto/requests/group_add_request.py b/clients/client-python/gravitino/dto/requests/group_add_request.py new file mode 100644 index 00000000000..f83be9e3f50 --- /dev/null +++ b/clients/client-python/gravitino/dto/requests/group_add_request.py @@ -0,0 +1,38 @@ +# 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 dataclasses import dataclass, field + +from dataclasses_json import config, dataclass_json + +from gravitino.rest.rest_message import RESTRequest +from gravitino.utils.precondition import Precondition + + +@dataclass_json +@dataclass +class GroupAddRequest(RESTRequest): + """Represents a request to add a group.""" + + _name: str = field(metadata=config(field_name="name")) + + def validate(self) -> None: + Precondition.check_string_not_empty( + self._name, "name is required and cannot be empty" + ) diff --git a/clients/client-python/gravitino/dto/responses/group_response.py b/clients/client-python/gravitino/dto/responses/group_response.py new file mode 100644 index 00000000000..cb3baef68b2 --- /dev/null +++ b/clients/client-python/gravitino/dto/responses/group_response.py @@ -0,0 +1,79 @@ +# 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 dataclasses import dataclass, field +from typing import Optional + +from dataclasses_json import config, dataclass_json + +from gravitino.dto.authorization.group_dto import GroupDTO +from gravitino.dto.responses.base_response import BaseResponse +from gravitino.utils.precondition import Precondition + + +@dataclass_json +@dataclass +class GroupResponse(BaseResponse): + """Represents a response for a group.""" + + _group: Optional[GroupDTO] = field( + default=None, metadata=config(field_name="group") + ) + + def group(self) -> Optional[GroupDTO]: + return self._group + + def validate(self) -> None: + Precondition.check_argument( + self._group is not None, "Group response should have a group" + ) + + +@dataclass_json +@dataclass +class GroupListResponse(BaseResponse): + """Represents a response for a list of groups with details.""" + + _groups: list[GroupDTO] = field( + default_factory=list, metadata=config(field_name="groups") + ) + + def groups(self) -> list[GroupDTO]: + return self._groups + + def validate(self) -> None: + Precondition.check_argument( + self._groups is not None, "Group list response should have groups" + ) + + +@dataclass_json +@dataclass +class GroupNamesListResponse(BaseResponse): + """Represents a response for a list of group names.""" + + _names: list[str] = field(default_factory=list, metadata=config(field_name="names")) + + def names(self) -> list[str]: + return self._names + + def validate(self) -> None: + Precondition.check_argument( + self._names is not None, "Group names list response should have names" + ) diff --git a/clients/client-python/gravitino/exceptions/base.py b/clients/client-python/gravitino/exceptions/base.py index 354d234f1a1..e7fe653a4ee 100644 --- a/clients/client-python/gravitino/exceptions/base.py +++ b/clients/client-python/gravitino/exceptions/base.py @@ -263,3 +263,11 @@ class NoSuchUserException(NotFoundException): class UserAlreadyExistsException(AlreadyExistsException): """An exception thrown when a user already exists.""" + + +class NoSuchGroupException(NotFoundException): + """An exception thrown when a group is not found.""" + + +class GroupAlreadyExistsException(AlreadyExistsException): + """An exception thrown when a group already exists.""" diff --git a/clients/client-python/gravitino/exceptions/handlers/group_error_handler.py b/clients/client-python/gravitino/exceptions/handlers/group_error_handler.py new file mode 100644 index 00000000000..5eddb7f3979 --- /dev/null +++ b/clients/client-python/gravitino/exceptions/handlers/group_error_handler.py @@ -0,0 +1,67 @@ +# 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 gravitino.constants.error import ErrorConstants +from gravitino.dto.responses.error_response import ErrorResponse +from gravitino.exceptions.base import ( + GroupAlreadyExistsException, + IllegalArgumentException, + MetalakeNotInUseException, + NoSuchGroupException, + NoSuchMetalakeException, + NoSuchRoleException, + NotFoundException, +) +from gravitino.exceptions.handlers.rest_error_handler import RestErrorHandler + + +class GroupErrorHandler(RestErrorHandler): + """Error handler specific to Group operations.""" + + def handle(self, error_response: ErrorResponse): + error_message = error_response.format_error_message() + code = error_response.code() + exception_type = error_response.type() + + if code == ErrorConstants.ILLEGAL_ARGUMENTS_CODE: + raise IllegalArgumentException(error_message) + + if code == ErrorConstants.NOT_FOUND_CODE: + if exception_type == NoSuchMetalakeException.__name__: + raise NoSuchMetalakeException(error_message) + + if exception_type == NoSuchGroupException.__name__: + raise NoSuchGroupException(error_message) + + if exception_type == NoSuchRoleException.__name__: + raise NoSuchRoleException(error_message) + + raise NotFoundException(error_message) + + if code == ErrorConstants.ALREADY_EXISTS_CODE: + raise GroupAlreadyExistsException(error_message) + + if code == ErrorConstants.NOT_IN_USE_CODE: + raise MetalakeNotInUseException(error_message) + + if code == ErrorConstants.INTERNAL_ERROR_CODE: + raise RuntimeError(error_message) + + super().handle(error_response) + + +GROUP_ERROR_HANDLER = GroupErrorHandler() diff --git a/clients/client-python/tests/integration/test_group_management.py b/clients/client-python/tests/integration/test_group_management.py new file mode 100644 index 00000000000..6568dc7aa18 --- /dev/null +++ b/clients/client-python/tests/integration/test_group_management.py @@ -0,0 +1,89 @@ +# 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. + +import logging +import uuid + +from gravitino import GravitinoAdminClient, GravitinoClient +from gravitino.exceptions.base import ( + GroupAlreadyExistsException, + NoSuchGroupException, +) +from tests.integration.integration_test_env import IntegrationTestEnv + +logger = logging.getLogger(__name__) + + +class TestGroupManagement(IntegrationTestEnv): + _metalake_name: str = f"test_group_metalake_{uuid.uuid4().hex[:8]}" + _gravitino_admin_client: GravitinoAdminClient = GravitinoAdminClient( + uri="http://localhost:8090" + ) + _gravitino_client: GravitinoClient + + def setUp(self): + self._gravitino_admin_client.create_metalake( + self._metalake_name, comment="test group management", properties={} + ) + self._gravitino_client = GravitinoClient( + uri="http://localhost:8090", metalake_name=self._metalake_name + ) + + def tearDown(self): + try: + self._gravitino_admin_client.drop_metalake(self._metalake_name, force=True) + except Exception: # pylint: disable=broad-except + logger.warning("Failed to drop metalake %s", self._metalake_name) + + def test_add_and_get_group(self): + added = self._gravitino_client.add_group("engineers") + self.assertEqual("engineers", added.name()) + + retrieved = self._gravitino_client.get_group("engineers") + self.assertEqual("engineers", retrieved.name()) + + def test_add_duplicate_group(self): + self._gravitino_client.add_group("engineers") + with self.assertRaises(GroupAlreadyExistsException): + self._gravitino_client.add_group("engineers") + + def test_list_group_names(self): + self._gravitino_client.add_group("engineers") + self._gravitino_client.add_group("admins") + + names = self._gravitino_client.list_group_names() + self.assertIn("engineers", names) + self.assertIn("admins", names) + + def test_list_groups(self): + self._gravitino_client.add_group("engineers") + self._gravitino_client.add_group("admins") + + groups = self._gravitino_client.list_groups() + group_names = [g.name() for g in groups] + self.assertIn("engineers", group_names) + self.assertIn("admins", group_names) + + def test_remove_group(self): + self._gravitino_client.add_group("engineers") + self.assertTrue(self._gravitino_client.remove_group("engineers")) + + with self.assertRaises(NoSuchGroupException): + self._gravitino_client.get_group("engineers") + + def test_remove_nonexistent_group(self): + self.assertFalse(self._gravitino_client.remove_group("nonexistent")) diff --git a/clients/client-python/tests/unittests/client/test_metalake_group_operations.py b/clients/client-python/tests/unittests/client/test_metalake_group_operations.py new file mode 100644 index 00000000000..4aa5963a6b8 --- /dev/null +++ b/clients/client-python/tests/unittests/client/test_metalake_group_operations.py @@ -0,0 +1,268 @@ +# 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. + +import unittest +from unittest.mock import patch + +from gravitino.client.gravitino_client import GravitinoClient +from gravitino.dto.authorization.group_dto import GroupDTO +from gravitino.dto.requests.group_add_request import GroupAddRequest +from gravitino.dto.responses.remove_response import RemoveResponse +from gravitino.dto.responses.group_response import ( + GroupListResponse, + GroupNamesListResponse, + GroupResponse, +) +from gravitino.exceptions.base import ( + IllegalArgumentException, + NoSuchMetalakeException, + NoSuchGroupException, + GroupAlreadyExistsException, +) +from gravitino.exceptions.handlers.group_error_handler import GROUP_ERROR_HANDLER +from tests.unittests import mock_base + + +def _build_group_dto(name: str = "engineers", roles: list | None = None) -> GroupDTO: + return ( + GroupDTO.builder() + .with_name(name) + .with_roles(roles if roles is not None else []) + .with_audit(mock_base.build_audit_info()) + .build() + ) + + +class TestMetalakeGroupOperations(unittest.TestCase): + METALAKE_GROUPS_PATH = "api/metalakes/metalake_demo/groups" + METALAKE_GROUP_PATH = "api/metalakes/metalake_demo/groups/engineers" + + def test_add_group(self): + metalake = mock_base.mock_load_metalake() + group = _build_group_dto() + mock_resp = mock_base.mock_http_response(GroupResponse(0, group).to_json()) + + with patch( + "gravitino.utils.http_client.HTTPClient.post", return_value=mock_resp + ) as mock_post: + added = metalake.add_group("engineers") + + self.assertEqual("engineers", added.name()) + self.assertEqual([], added.roles()) + + mock_post.assert_called_once() + call_args = mock_post.call_args + self.assertEqual(self.METALAKE_GROUPS_PATH, call_args.args[0]) + self.assertIsInstance(call_args.kwargs["json"], GroupAddRequest) + self.assertIn('"name": "engineers"', call_args.kwargs["json"].to_json()) + self.assertIs(GROUP_ERROR_HANDLER, call_args.kwargs["error_handler"]) + + def test_add_group_already_exists(self): + metalake = mock_base.mock_load_metalake() + with patch( + "gravitino.utils.http_client.HTTPClient.post", + side_effect=GroupAlreadyExistsException("group engineers already exists"), + ): + with self.assertRaises(GroupAlreadyExistsException): + metalake.add_group("engineers") + + def test_add_group_empty_name_rejected(self): + metalake = mock_base.mock_load_metalake() + with self.assertRaises(IllegalArgumentException): + metalake.add_group("") + + def test_get_group(self): + metalake = mock_base.mock_load_metalake() + group = _build_group_dto(roles=["role_a", "role_b"]) + mock_resp = mock_base.mock_http_response(GroupResponse(0, group).to_json()) + + with patch( + "gravitino.utils.http_client.HTTPClient.get", return_value=mock_resp + ) as mock_get: + retrieved = metalake.get_group("engineers") + + self.assertEqual("engineers", retrieved.name()) + self.assertEqual(["role_a", "role_b"], retrieved.roles()) + + mock_get.assert_called_once() + self.assertEqual(self.METALAKE_GROUP_PATH, mock_get.call_args.args[0]) + self.assertIs( + GROUP_ERROR_HANDLER, mock_get.call_args.kwargs["error_handler"] + ) + + def test_get_group_not_found(self): + metalake = mock_base.mock_load_metalake() + with patch( + "gravitino.utils.http_client.HTTPClient.get", + side_effect=NoSuchGroupException("no such group"), + ): + with self.assertRaises(NoSuchGroupException): + metalake.get_group("engineers") + + def test_remove_group(self): + metalake = mock_base.mock_load_metalake() + mock_resp = mock_base.mock_http_response(RemoveResponse(0, True).to_json()) + + with patch( + "gravitino.utils.http_client.HTTPClient.delete", return_value=mock_resp + ) as mock_delete: + self.assertTrue(metalake.remove_group("engineers")) + + mock_delete.assert_called_once() + self.assertEqual(self.METALAKE_GROUP_PATH, mock_delete.call_args.args[0]) + self.assertIs( + GROUP_ERROR_HANDLER, mock_delete.call_args.kwargs["error_handler"] + ) + + def test_remove_group_returns_false_when_not_removed(self): + metalake = mock_base.mock_load_metalake() + mock_resp = mock_base.mock_http_response(RemoveResponse(0, False).to_json()) + + with patch( + "gravitino.utils.http_client.HTTPClient.delete", return_value=mock_resp + ): + self.assertFalse(metalake.remove_group("engineers")) + + def test_list_groups(self): + metalake = mock_base.mock_load_metalake() + groups = [_build_group_dto("alice"), _build_group_dto("bob")] + mock_resp = mock_base.mock_http_response(GroupListResponse(0, groups).to_json()) + + with patch( + "gravitino.utils.http_client.HTTPClient.get", return_value=mock_resp + ) as mock_get: + result = metalake.list_groups() + + self.assertEqual(["alice", "bob"], [u.name() for u in result]) + + mock_get.assert_called_once_with( + self.METALAKE_GROUPS_PATH, + params={"details": "true"}, + error_handler=GROUP_ERROR_HANDLER, + ) + + def test_list_group_names(self): + metalake = mock_base.mock_load_metalake() + mock_resp = mock_base.mock_http_response( + GroupNamesListResponse(0, ["alice", "bob"]).to_json() + ) + + with patch( + "gravitino.utils.http_client.HTTPClient.get", return_value=mock_resp + ) as mock_get: + names = metalake.list_group_names() + + self.assertEqual(["alice", "bob"], names) + + mock_get.assert_called_once_with( + self.METALAKE_GROUPS_PATH, error_handler=GROUP_ERROR_HANDLER + ) + + def test_list_groups_metalake_not_found(self): + metalake = mock_base.mock_load_metalake() + with patch( + "gravitino.utils.http_client.HTTPClient.get", + side_effect=NoSuchMetalakeException("metalake not found"), + ): + with self.assertRaises(NoSuchMetalakeException): + metalake.list_groups() + + +class TestGravitinoClientGroupDelegates(unittest.TestCase): + """Verify that GravitinoClient correctly delegates Group operations.""" + + def _make_client(self): + client = GravitinoClient.__new__(GravitinoClient) + return client + + def test_client_add_group(self): + client = self._make_client() + group = _build_group_dto() + mock_resp = mock_base.mock_http_response(GroupResponse(0, group).to_json()) + with ( + patch.object( + GravitinoClient, + "get_metalake", + return_value=mock_base.mock_load_metalake(), + ), + patch( + "gravitino.utils.http_client.HTTPClient.post", return_value=mock_resp + ), + ): + result = client.add_group("engineers") + self.assertEqual("engineers", result.name()) + + def test_client_get_group(self): + client = self._make_client() + group = _build_group_dto(roles=["r1"]) + mock_resp = mock_base.mock_http_response(GroupResponse(0, group).to_json()) + with ( + patch.object( + GravitinoClient, + "get_metalake", + return_value=mock_base.mock_load_metalake(), + ), + patch("gravitino.utils.http_client.HTTPClient.get", return_value=mock_resp), + ): + result = client.get_group("engineers") + self.assertEqual(["r1"], result.roles()) + + def test_client_remove_group(self): + client = self._make_client() + mock_resp = mock_base.mock_http_response(RemoveResponse(0, True).to_json()) + with ( + patch.object( + GravitinoClient, + "get_metalake", + return_value=mock_base.mock_load_metalake(), + ), + patch( + "gravitino.utils.http_client.HTTPClient.delete", return_value=mock_resp + ), + ): + self.assertTrue(client.remove_group("engineers")) + + def test_client_list_groups(self): + client = self._make_client() + groups = [_build_group_dto("alice"), _build_group_dto("bob")] + mock_resp = mock_base.mock_http_response(GroupListResponse(0, groups).to_json()) + with ( + patch.object( + GravitinoClient, + "get_metalake", + return_value=mock_base.mock_load_metalake(), + ), + patch("gravitino.utils.http_client.HTTPClient.get", return_value=mock_resp), + ): + result = client.list_groups() + self.assertEqual(["alice", "bob"], [u.name() for u in result]) + + def test_client_list_group_names(self): + client = self._make_client() + mock_resp = mock_base.mock_http_response( + GroupNamesListResponse(0, ["alice", "bob"]).to_json() + ) + with ( + patch.object( + GravitinoClient, + "get_metalake", + return_value=mock_base.mock_load_metalake(), + ), + patch("gravitino.utils.http_client.HTTPClient.get", return_value=mock_resp), + ): + result = client.list_group_names() + self.assertEqual(["alice", "bob"], result) diff --git a/clients/client-python/tests/unittests/dto/responses/test_group_response.py b/clients/client-python/tests/unittests/dto/responses/test_group_response.py new file mode 100644 index 00000000000..62d05a139e8 --- /dev/null +++ b/clients/client-python/tests/unittests/dto/responses/test_group_response.py @@ -0,0 +1,76 @@ +# 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 + +import json as _json +import unittest + +from gravitino.dto.authorization.group_dto import GroupDTO +from gravitino.dto.responses.group_response import ( + GroupListResponse, + GroupNamesListResponse, + GroupResponse, +) + + +class TestGroupResponses(unittest.TestCase): + def test_group_response(self): + group_dto = GroupDTO.builder().with_name("group1").build() + resp = GroupResponse(0, group_dto) + + resp.validate() + + ser_json = _json.dumps(resp.to_dict()) + deser_dict = _json.loads(ser_json) + + self.assertEqual(group_dto, resp.group()) + self.assertEqual(0, deser_dict["code"]) + self.assertIsNotNone(deser_dict.get("group")) + self.assertEqual("group1", deser_dict["group"]["name"]) + + def test_group_response_validate_no_group(self): + resp = GroupResponse(0, None) + with self.assertRaises(ValueError): + resp.validate() + + def test_group_names_list_response(self): + names = ["group1", "group2"] + resp = GroupNamesListResponse(0, names) + + resp.validate() + + ser_json = _json.dumps(resp.to_dict()) + deser_dict = _json.loads(ser_json) + + self.assertEqual(0, deser_dict["code"]) + self.assertEqual(2, len(deser_dict["names"])) + self.assertListEqual(names, deser_dict["names"]) + + def test_group_list_response(self): + group1 = GroupDTO.builder().with_name("group1").with_roles(["r1"]).build() + group2 = GroupDTO.builder().with_name("group2").build() + + resp = GroupListResponse(0, [group1, group2]) + + ser_json = _json.dumps(resp.to_dict()) + deser_dict = _json.loads(ser_json) + + self.assertEqual(0, deser_dict["code"]) + self.assertEqual(2, len(deser_dict["groups"])) + self.assertEqual("group1", deser_dict["groups"][0]["name"]) + self.assertEqual("group2", deser_dict["groups"][1]["name"]) diff --git a/clients/client-python/tests/unittests/dto/test_group_dto.py b/clients/client-python/tests/unittests/dto/test_group_dto.py new file mode 100644 index 00000000000..58182066f83 --- /dev/null +++ b/clients/client-python/tests/unittests/dto/test_group_dto.py @@ -0,0 +1,130 @@ +# 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 + +import json as _json +import unittest + +from gravitino.dto.audit_dto import AuditDTO +from gravitino.dto.authorization.group_dto import GroupDTO + + +class TestGroupDTO(unittest.TestCase): + def test_create_group_dto(self): + audit = AuditDTO( + _creator="admin", + _create_time="2024-01-01T00:00:00Z", + ) + group_dto = ( + GroupDTO.builder() + .with_name("test_group") + .with_roles(["role1", "role2"]) + .with_audit(audit) + .build() + ) + + ser_json = _json.dumps(group_dto.to_dict()).encode("utf-8") + deser_dict = _json.loads(ser_json) + self.assertEqual(deser_dict["name"], "test_group") + self.assertEqual(deser_dict["roles"], ["role1", "role2"]) + self.assertEqual(deser_dict["audit"]["creator"], "admin") + + def test_group_dto_without_roles(self): + group_dto = ( + GroupDTO.builder() + .with_name("test_group_no_roles") + .with_audit(AuditDTO(_creator="admin", _create_time="2024-01-01T00:00:00Z")) + .build() + ) + + ser_json = _json.dumps(group_dto.to_dict()).encode("utf-8") + deser_dict = _json.loads(ser_json) + self.assertEqual(deser_dict["name"], "test_group_no_roles") + self.assertEqual(deser_dict["roles"], []) + + def test_group_dto_methods(self): + group_dto = ( + GroupDTO.builder() + .with_name("method_group") + .with_roles(["admin_role"]) + .with_audit(AuditDTO(_creator="admin", _create_time="2024-01-01T00:00:00Z")) + .build() + ) + self.assertEqual(group_dto.name(), "method_group") + self.assertEqual(group_dto.roles(), ["admin_role"]) + self.assertIsNotNone(group_dto.audit_info()) + + def test_builder_empty_name_raises(self): + with self.assertRaises(ValueError): + GroupDTO.builder().with_audit( + AuditDTO(_creator="test", _create_time="2024-01-01T00:00:00Z") + ).build() + + def test_builder_no_audit_raises(self): + with self.assertRaises(ValueError): + GroupDTO.builder().with_name("test_group").build() + + def test_equality_and_hash(self): + audit = AuditDTO(_creator="test", _create_time="2024-01-01T00:00:00Z") + + group1 = ( + GroupDTO.builder() + .with_name("group1") + .with_roles(["r1"]) + .with_audit(audit) + .build() + ) + group2 = ( + GroupDTO.builder() + .with_name("group1") + .with_roles(["r1"]) + .with_audit(audit) + .build() + ) + group3 = ( + GroupDTO.builder() + .with_name("group2") + .with_roles(["r1"]) + .with_audit(audit) + .build() + ) + + self.assertEqual(group1, group2) + self.assertEqual(hash(group1), hash(group2)) + self.assertNotEqual(group1, group3) + + def test_roles_returns_list_type(self): + group_dto = ( + GroupDTO.builder() + .with_name("test") + .with_roles(["r1"]) + .with_audit(AuditDTO(_creator="admin", _create_time="2024-01-01T00:00:00Z")) + .build() + ) + self.assertIsInstance(group_dto.roles(), list) + + def test_roles_immutability(self): + group_dto = ( + GroupDTO.builder() + .with_name("test") + .with_roles(["r1", "r2"]) + .with_audit(AuditDTO(_creator="admin", _create_time="2024-01-01T00:00:00Z")) + .build() + ) + roles = group_dto.roles() + roles.append("r3") + self.assertEqual(["r1", "r2"], group_dto.roles()) diff --git a/clients/client-python/tests/unittests/test_error_handler.py b/clients/client-python/tests/unittests/test_error_handler.py index ca7ceb00a19..10bd9cb2819 100644 --- a/clients/client-python/tests/unittests/test_error_handler.py +++ b/clients/client-python/tests/unittests/test_error_handler.py @@ -35,6 +35,7 @@ NoSuchMetalakeException, NoSuchPartitionException, NoSuchRoleException, + NoSuchGroupException, NoSuchSchemaException, NoSuchTableException, NoSuchUserException, @@ -47,12 +48,14 @@ TableAlreadyExistsException, UnsupportedOperationException, UserAlreadyExistsException, + GroupAlreadyExistsException, ) from gravitino.exceptions.handlers.catalog_error_handler import CATALOG_ERROR_HANDLER from gravitino.exceptions.handlers.credential_error_handler import ( CREDENTIAL_ERROR_HANDLER, ) from gravitino.exceptions.handlers.fileset_error_handler import FILESET_ERROR_HANDLER +from gravitino.exceptions.handlers.group_error_handler import GROUP_ERROR_HANDLER from gravitino.exceptions.handlers.metalake_error_handler import METALAKE_ERROR_HANDLER from gravitino.exceptions.handlers.partition_error_handler import ( PARTITION_ERROR_HANDLER, @@ -476,3 +479,59 @@ def test_user_error_handler(self): USER_ERROR_HANDLER.handle( ErrorResponse.generate_error_response(Exception, "mock error") ) + + def test_group_error_handler(self): + with self.assertRaises(NoSuchMetalakeException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response( + NoSuchMetalakeException, "mock error" + ) + ) + + with self.assertRaises(NoSuchGroupException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response( + NoSuchGroupException, "mock error" + ) + ) + + with self.assertRaises(NoSuchRoleException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response(NoSuchRoleException, "mock error") + ) + + with self.assertRaises(GroupAlreadyExistsException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response( + GroupAlreadyExistsException, "mock error" + ) + ) + + with self.assertRaises(IllegalArgumentException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response( + IllegalArgumentException, "mock error" + ) + ) + + with self.assertRaises(MetalakeNotInUseException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response( + MetalakeNotInUseException, "mock error" + ) + ) + + with self.assertRaises(NotFoundException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response(NotFoundException, "mock error") + ) + + with self.assertRaises(RuntimeError): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response(InternalError, "mock error") + ) + + with self.assertRaises(RESTException): + GROUP_ERROR_HANDLER.handle( + ErrorResponse.generate_error_response(Exception, "mock error") + ) From ef8fac346dc2d64240cee127ece8e580ea9df31d Mon Sep 17 00:00:00 2001 From: Sun Yuhan Date: Fri, 15 May 2026 12:34:29 +0800 Subject: [PATCH 2/6] [#11094] fix(client-python): Auto-enable authorization in group integration tests --- .../integration/test_group_management.py | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/clients/client-python/tests/integration/test_group_management.py b/clients/client-python/tests/integration/test_group_management.py index 6568dc7aa18..fe8311e608b 100644 --- a/clients/client-python/tests/integration/test_group_management.py +++ b/clients/client-python/tests/integration/test_group_management.py @@ -16,6 +16,7 @@ # under the License. import logging +import os import uuid from gravitino import GravitinoAdminClient, GravitinoClient @@ -30,10 +31,29 @@ class TestGroupManagement(IntegrationTestEnv): _metalake_name: str = f"test_group_metalake_{uuid.uuid4().hex[:8]}" - _gravitino_admin_client: GravitinoAdminClient = GravitinoAdminClient( - uri="http://localhost:8090" - ) - _gravitino_client: GravitinoClient + _gravitino_admin_client: GravitinoAdminClient = None + _gravitino_client: GravitinoClient = None + + @classmethod + def setUpClass(cls): + if not ( + os.environ.get("START_EXTERNAL_GRAVITINO") is not None + and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" + ): + cls._get_gravitino_home() + conf_path = os.path.join( + cls.gravitino_home, "conf", "gravitino.conf" + ) + cls._reset_conf( + {"gravitino.authorization.enable": "true"}, conf_path + ) + cls._append_conf( + {"gravitino.authorization.enable": "true"}, conf_path + ) + super().setUpClass() + cls._gravitino_admin_client = GravitinoAdminClient( + uri="http://localhost:8090" + ) def setUp(self): self._gravitino_admin_client.create_metalake( From 8b22cf9cdd1053ca79af553a046cbf00b0c30e7b Mon Sep 17 00:00:00 2001 From: Sun Yuhan Date: Tue, 19 May 2026 13:26:36 +0800 Subject: [PATCH 3/6] [#11094] fix(client-python): fix group IT config not applied in CI Fix setUpClass to always modify gravitino.conf and restart the server when running under Gradle (START_EXTERNAL_GRAVITINO=true). Add tearDownClass to restore the config after tests. --- .../integration/test_group_management.py | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/clients/client-python/tests/integration/test_group_management.py b/clients/client-python/tests/integration/test_group_management.py index fe8311e608b..a4464f3cdaa 100644 --- a/clients/client-python/tests/integration/test_group_management.py +++ b/clients/client-python/tests/integration/test_group_management.py @@ -36,25 +36,39 @@ class TestGroupManagement(IntegrationTestEnv): @classmethod def setUpClass(cls): - if not ( + cls._get_gravitino_home() + conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") + cls._reset_conf( + {"gravitino.authorization.enable": "true"}, conf_path + ) + cls._append_conf( + {"gravitino.authorization.enable": "true"}, conf_path + ) + if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" ): - cls._get_gravitino_home() - conf_path = os.path.join( - cls.gravitino_home, "conf", "gravitino.conf" - ) - cls._reset_conf( - {"gravitino.authorization.enable": "true"}, conf_path - ) - cls._append_conf( - {"gravitino.authorization.enable": "true"}, conf_path - ) - super().setUpClass() + cls.restart_server() + else: + super().setUpClass() cls._gravitino_admin_client = GravitinoAdminClient( uri="http://localhost:8090" ) + @classmethod + def tearDownClass(cls): + conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") + cls._reset_conf( + {"gravitino.authorization.enable": "false"}, conf_path + ) + if ( + os.environ.get("START_EXTERNAL_GRAVITINO") is not None + and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" + ): + cls.restart_server() + else: + super().tearDownClass() + def setUp(self): self._gravitino_admin_client.create_metalake( self._metalake_name, comment="test group management", properties={} From 8872c191e4200b44637565a3e3547d7208aa7064 Mon Sep 17 00:00:00 2001 From: Sun Yuhan Date: Tue, 19 May 2026 17:16:56 +0800 Subject: [PATCH 4/6] [#11094] fix(client-python): remove unnecessary audit null check in GroupDTO builder --- clients/client-python/gravitino/dto/authorization/group_dto.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/clients/client-python/gravitino/dto/authorization/group_dto.py b/clients/client-python/gravitino/dto/authorization/group_dto.py index 31e86ea6a1e..6d0be5e1ead 100644 --- a/clients/client-python/gravitino/dto/authorization/group_dto.py +++ b/clients/client-python/gravitino/dto/authorization/group_dto.py @@ -88,6 +88,4 @@ def with_audit(self, audit: AuditDTO) -> GroupDTO.Builder: def build(self) -> GroupDTO: if not self._name: raise ValueError("name cannot be null or empty") - if self._audit is None: - raise ValueError("audit cannot be null") return GroupDTO(self._name, self._roles, self._audit) From ce1b20cb230d5469d000a02df8eacdeb05a9aba3 Mon Sep 17 00:00:00 2001 From: Sun Yuhan Date: Wed, 20 May 2026 20:36:55 +0800 Subject: [PATCH 5/6] [#11094] fix(client-python): address review feedback for Group auth - Remove test_builder_no_audit_raises to match relaxed GroupDTO builder - Add unit test for GroupAddRequest - Add serviceAdmins config in group integration test --- .../integration/test_group_management.py | 18 +++++---- .../dto/requests/test_group_add_request.py | 38 +++++++++++++++++++ .../tests/unittests/dto/test_group_dto.py | 4 -- 3 files changed, 49 insertions(+), 11 deletions(-) create mode 100644 clients/client-python/tests/unittests/dto/requests/test_group_add_request.py diff --git a/clients/client-python/tests/integration/test_group_management.py b/clients/client-python/tests/integration/test_group_management.py index a4464f3cdaa..2934d173109 100644 --- a/clients/client-python/tests/integration/test_group_management.py +++ b/clients/client-python/tests/integration/test_group_management.py @@ -38,12 +38,12 @@ class TestGroupManagement(IntegrationTestEnv): def setUpClass(cls): cls._get_gravitino_home() conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") - cls._reset_conf( - {"gravitino.authorization.enable": "true"}, conf_path - ) - cls._append_conf( - {"gravitino.authorization.enable": "true"}, conf_path - ) + auth_confs = { + "gravitino.authorization.enable": "true", + "gravitino.authorization.serviceAdmins": "anonymous", + } + cls._reset_conf(auth_confs, conf_path) + cls._append_conf(auth_confs, conf_path) if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" @@ -59,7 +59,11 @@ def setUpClass(cls): def tearDownClass(cls): conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") cls._reset_conf( - {"gravitino.authorization.enable": "false"}, conf_path + { + "gravitino.authorization.enable": "false", + "gravitino.authorization.serviceAdmins": "", + }, + conf_path, ) if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None diff --git a/clients/client-python/tests/unittests/dto/requests/test_group_add_request.py b/clients/client-python/tests/unittests/dto/requests/test_group_add_request.py new file mode 100644 index 00000000000..a90be72a15b --- /dev/null +++ b/clients/client-python/tests/unittests/dto/requests/test_group_add_request.py @@ -0,0 +1,38 @@ +# 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 + +import json as _json +import unittest + +from gravitino.dto.requests.group_add_request import GroupAddRequest + + +class TestGroupAddRequest(unittest.TestCase): + def test_group_add_request_serde(self) -> None: + group_add_request = GroupAddRequest(_name="group1") + ser_json = _json.dumps(group_add_request.to_dict()) + deser_dict = _json.loads(ser_json) + + self.assertEqual("group1", deser_dict["name"]) + + def test_group_add_request_validate(self) -> None: + group_add_request = GroupAddRequest(_name="group1") + group_add_request.validate() + + with self.assertRaises(ValueError): + GroupAddRequest(_name="").validate() diff --git a/clients/client-python/tests/unittests/dto/test_group_dto.py b/clients/client-python/tests/unittests/dto/test_group_dto.py index 58182066f83..81dd7c0fcbe 100644 --- a/clients/client-python/tests/unittests/dto/test_group_dto.py +++ b/clients/client-python/tests/unittests/dto/test_group_dto.py @@ -74,10 +74,6 @@ def test_builder_empty_name_raises(self): AuditDTO(_creator="test", _create_time="2024-01-01T00:00:00Z") ).build() - def test_builder_no_audit_raises(self): - with self.assertRaises(ValueError): - GroupDTO.builder().with_name("test_group").build() - def test_equality_and_hash(self): audit = AuditDTO(_creator="test", _create_time="2024-01-01T00:00:00Z") From f8906f39a916a3c963c51844d45dbca0beb9eb61 Mon Sep 17 00:00:00 2001 From: Sun Yuhan Date: Fri, 22 May 2026 14:09:02 +0800 Subject: [PATCH 6/6] [#11094] fix(client-python): fix IT tearDown corrupting server config - Fix test_group_management tearDown to properly restore auth config - Fix test_user setUp to include serviceAdmins and tearDown to restore config - Format test_owner with black --- .../integration/test_group_management.py | 17 ++++------ .../tests/integration/test_owner.py | 32 +++++-------------- .../tests/integration/test_user.py | 15 +++++++-- 3 files changed, 27 insertions(+), 37 deletions(-) diff --git a/clients/client-python/tests/integration/test_group_management.py b/clients/client-python/tests/integration/test_group_management.py index 2934d173109..1e0a8c0095f 100644 --- a/clients/client-python/tests/integration/test_group_management.py +++ b/clients/client-python/tests/integration/test_group_management.py @@ -51,20 +51,17 @@ def setUpClass(cls): cls.restart_server() else: super().setUpClass() - cls._gravitino_admin_client = GravitinoAdminClient( - uri="http://localhost:8090" - ) + cls._gravitino_admin_client = GravitinoAdminClient(uri="http://localhost:8090") @classmethod def tearDownClass(cls): conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") - cls._reset_conf( - { - "gravitino.authorization.enable": "false", - "gravitino.authorization.serviceAdmins": "", - }, - conf_path, - ) + reset_confs = { + "gravitino.authorization.enable": "false", + "gravitino.authorization.serviceAdmins": "anonymous", + } + cls._reset_conf(reset_confs, conf_path) + cls._append_conf(reset_confs, conf_path) if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" diff --git a/clients/client-python/tests/integration/test_owner.py b/clients/client-python/tests/integration/test_owner.py index 200724395a7..f292fcb4daa 100644 --- a/clients/client-python/tests/integration/test_owner.py +++ b/clients/client-python/tests/integration/test_owner.py @@ -50,12 +50,8 @@ class TestOwner(IntegrationTestEnv): def setUpClass(cls): cls._get_gravitino_home() conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") - cls._reset_conf( - {"gravitino.authorization.enable": "true"}, conf_path - ) - cls._append_conf( - {"gravitino.authorization.enable": "true"}, conf_path - ) + cls._reset_conf({"gravitino.authorization.enable": "true"}, conf_path) + cls._append_conf({"gravitino.authorization.enable": "true"}, conf_path) if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" @@ -63,16 +59,12 @@ def setUpClass(cls): cls.restart_server() else: super().setUpClass() - cls.gravitino_admin_client = GravitinoAdminClient( - uri="http://localhost:8090" - ) + cls.gravitino_admin_client = GravitinoAdminClient(uri="http://localhost:8090") @classmethod def tearDownClass(cls): conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") - cls._reset_conf( - {"gravitino.authorization.enable": "false"}, conf_path - ) + cls._reset_conf({"gravitino.authorization.enable": "false"}, conf_path) if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" @@ -114,9 +106,7 @@ def clean_test_data(self): logger.warning("Failed to drop catalog %s", self.catalog_name) try: - self.gravitino_admin_client.drop_metalake( - self.metalake_name, force=True - ) + self.gravitino_admin_client.drop_metalake(self.metalake_name, force=True) except GravitinoRuntimeException: logger.warning("Failed to drop metalake %s", self.metalake_name) @@ -135,9 +125,7 @@ def test_set_owner_metalake(self): metalake_obj = MetadataObjects.of( [self.metalake_name], MetadataObject.Type.METALAKE ) - self.gravitino_client.set_owner( - metalake_obj, self.test_user, Owner.Type.USER - ) + self.gravitino_client.set_owner(metalake_obj, self.test_user, Owner.Type.USER) owner = self.gravitino_client.get_owner(metalake_obj) self.assertIsNotNone(owner) @@ -157,9 +145,7 @@ def test_owner_catalog_and_schema(self): self.assertTrue(len(owner.name()) > 0) self.assertEqual(Owner.Type.USER, owner.type()) - self.gravitino_client.set_owner( - catalog_obj, self.test_user, Owner.Type.USER - ) + self.gravitino_client.set_owner(catalog_obj, self.test_user, Owner.Type.USER) owner = self.gravitino_client.get_owner(catalog_obj) self.assertEqual(self.test_user, owner.name()) self.assertEqual(Owner.Type.USER, owner.type()) @@ -171,9 +157,7 @@ def test_owner_catalog_and_schema(self): schema_obj = MetadataObjects.of( [self.catalog_name, schema_name], MetadataObject.Type.SCHEMA ) - self.gravitino_client.set_owner( - schema_obj, self.test_user, Owner.Type.USER - ) + self.gravitino_client.set_owner(schema_obj, self.test_user, Owner.Type.USER) owner = self.gravitino_client.get_owner(schema_obj) self.assertIsNotNone(owner) diff --git a/clients/client-python/tests/integration/test_user.py b/clients/client-python/tests/integration/test_user.py index 95764612142..9178df76009 100644 --- a/clients/client-python/tests/integration/test_user.py +++ b/clients/client-python/tests/integration/test_user.py @@ -41,8 +41,12 @@ class TestUser(IntegrationTestEnv): def setUpClass(cls): cls._get_gravitino_home() conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") - cls._reset_conf({"gravitino.authorization.enable": "true"}, conf_path) - cls._append_conf({"gravitino.authorization.enable": "true"}, conf_path) + auth_confs = { + "gravitino.authorization.enable": "true", + "gravitino.authorization.serviceAdmins": "anonymous", + } + cls._reset_conf(auth_confs, conf_path) + cls._append_conf(auth_confs, conf_path) if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true" @@ -55,7 +59,12 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): conf_path = os.path.join(cls.gravitino_home, "conf", "gravitino.conf") - cls._reset_conf({"gravitino.authorization.enable": "false"}, conf_path) + reset_confs = { + "gravitino.authorization.enable": "false", + "gravitino.authorization.serviceAdmins": "anonymous", + } + cls._reset_conf(reset_confs, conf_path) + cls._append_conf(reset_confs, conf_path) if ( os.environ.get("START_EXTERNAL_GRAVITINO") is not None and os.environ.get("START_EXTERNAL_GRAVITINO").lower() == "true"