From 112e36c59f94038c033a8404551737437416681f Mon Sep 17 00:00:00 2001 From: Gyubong Lee Date: Fri, 14 Feb 2025 15:00:04 +0900 Subject: [PATCH] fix(BA-754): ContainerRegistry per-project API misc bugs (#3701) Co-authored-by: octodog --- changes/3701.fix.md | 1 + docs/manager/rest-reference/openapi.json | 449 ++++++++++++++++++ .../backend/manager/api/container_registry.py | 4 +- src/ai/backend/manager/api/exceptions.py | 4 + .../client/container_registry/harbor.py | 6 +- .../models/gql_models/container_registry.py | 6 +- src/ai/backend/manager/models/image.py | 16 +- src/ai/backend/manager/server.py | 1 + .../service/container_registry/harbor.py | 5 +- 9 files changed, 480 insertions(+), 12 deletions(-) create mode 100644 changes/3701.fix.md diff --git a/changes/3701.fix.md b/changes/3701.fix.md new file mode 100644 index 0000000000..cefdccbde4 --- /dev/null +++ b/changes/3701.fix.md @@ -0,0 +1 @@ +Fix ContainerRegistry per-project API misc bugs. diff --git a/docs/manager/rest-reference/openapi.json b/docs/manager/rest-reference/openapi.json index 020c65e4a8..8c2999a60d 100644 --- a/docs/manager/rest-reference/openapi.json +++ b/docs/manager/rest-reference/openapi.json @@ -20,6 +20,386 @@ } }, "schemas": { + "AllowedGroupsModel": { + "properties": { + "add": { + "default": [], + "items": { + "type": "string" + }, + "title": "Add", + "type": "array" + }, + "remove": { + "default": [], + "items": { + "type": "string" + }, + "title": "Remove", + "type": "array" + } + }, + "title": "AllowedGroupsModel", + "type": "object" + }, + "ContainerRegistryType": { + "enum": [ + "docker", + "harbor", + "harbor2", + "github", + "gitlab", + "ecr", + "ecr-public", + "local" + ], + "title": "ContainerRegistryType", + "type": "string" + }, + "PatchContainerRegistryRequestModel": { + "properties": { + "id": { + "anyOf": [ + { + "format": "uuid", + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Id" + }, + "url": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Url" + }, + "registry_name": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Registry Name" + }, + "type": { + "anyOf": [ + { + "$ref": "#/components/schemas/ContainerRegistryType" + }, + { + "type": "null" + } + ], + "default": null + }, + "project": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Project" + }, + "username": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Username" + }, + "password": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Password" + }, + "ssl_verify": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Ssl Verify" + }, + "is_global": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Is Global" + }, + "extra": { + "anyOf": [ + { + "type": "object" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Extra" + }, + "allowed_groups": { + "anyOf": [ + { + "$ref": "#/components/schemas/AllowedGroupsModel" + }, + { + "type": "null" + } + ], + "default": null + } + }, + "title": "PatchContainerRegistryRequestModel", + "type": "object" + }, + "PatchContainerRegistryResponseModel": { + "properties": { + "id": { + "anyOf": [ + { + "format": "uuid", + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Id" + }, + "url": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Url" + }, + "registry_name": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Registry Name" + }, + "type": { + "anyOf": [ + { + "$ref": "#/components/schemas/ContainerRegistryType" + }, + { + "type": "null" + } + ], + "default": null + }, + "project": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Project" + }, + "username": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Username" + }, + "password": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Password" + }, + "ssl_verify": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Ssl Verify" + }, + "is_global": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Is Global" + }, + "extra": { + "anyOf": [ + { + "type": "object" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Extra" + } + }, + "title": "PatchContainerRegistryResponseModel", + "type": "object" + }, + "EventData": { + "properties": { + "resources": { + "description": "List of related artifacts involved in the event", + "items": { + "$ref": "#/components/schemas/Resource" + }, + "title": "Resources", + "type": "array" + }, + "repository": { + "$ref": "#/components/schemas/Repository", + "description": "Repository details" + } + }, + "required": [ + "resources", + "repository" + ], + "title": "EventData", + "type": "object" + }, + "Repository": { + "properties": { + "namespace": { + "description": "Harbor project (namespace)", + "title": "Namespace", + "type": "string" + }, + "name": { + "description": "Name of the repository", + "title": "Name", + "type": "string" + } + }, + "required": [ + "namespace", + "name" + ], + "title": "Repository", + "type": "object" + }, + "Resource": { + "properties": { + "resource_url": { + "description": "URL of the artifact", + "title": "Resource Url", + "type": "string" + }, + "tag": { + "description": "Tag of the artifact", + "title": "Tag", + "type": "string" + } + }, + "required": [ + "resource_url", + "tag" + ], + "title": "Resource", + "type": "object" + }, + "HarborWebhookRequestModel": { + "properties": { + "type": { + "description": "Type of the webhook event triggered by Harbor. See Harbor documentation for details.", + "title": "Type", + "type": "string" + }, + "event_data": { + "$ref": "#/components/schemas/EventData", + "description": "Event details" + } + }, + "required": [ + "type", + "event_data" + ], + "title": "HarborWebhookRequestModel", + "type": "object" + }, "VFolderPermission": { "description": "Permissions for a virtual folder given to a specific access key.\nRW_DELETE includes READ_WRITE and READ_WRITE includes READ_ONLY.", "enum": [ @@ -1218,6 +1598,75 @@ "description": "\n**Preconditions:**\n* User privilege required.\n* Manager status required: RUNNING\n" } }, + "/container-registries/{registry_id}": { + "patch": { + "operationId": "container-registries.patch_container_registry", + "tags": [ + "container-registries" + ], + "responses": { + "200": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PatchContainerRegistryResponseModel" + } + } + } + } + }, + "security": [ + { + "TokenAuth": [] + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PatchContainerRegistryRequestModel" + } + } + } + }, + "parameters": [ + { + "name": "registry_id", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "description": "\n**Preconditions:**\n* Superadmin privilege required.\n* Manager status required: one of FROZEN, RUNNING\n" + } + }, + "/container-registries/webhook/harbor": { + "post": { + "operationId": "container-registries.harbor_webhook_handler", + "tags": [ + "container-registries" + ], + "responses": { + "200": { + "description": "Successful response" + } + }, + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HarborWebhookRequestModel" + } + } + } + }, + "parameters": [], + "description": "\n**Preconditions:**\n* Manager status required: RUNNING\n" + } + }, "/config/resource-slots": { "get": { "operationId": "config.get_resource_slots", diff --git a/src/ai/backend/manager/api/container_registry.py b/src/ai/backend/manager/api/container_registry.py index 8193e27c85..c96881072b 100644 --- a/src/ai/backend/manager/api/container_registry.py +++ b/src/ai/backend/manager/api/container_registry.py @@ -27,10 +27,10 @@ from ai.backend.manager.models.gql_models.container_registry_v2 import handle_allowed_groups_update from .exceptions import ( - ContainerRegistryNotFound, GenericBadRequest, HarborWebhookContainerRegistryRowNotFound, InternalServerError, + ObjectNotFound, ) if TYPE_CHECKING: @@ -72,7 +72,7 @@ async def patch_container_registry( if params.allowed_groups: await handle_allowed_groups_update(root_ctx.db, registry_id, params.allowed_groups) - except ContainerRegistryNotFound as e: + except ObjectNotFound as e: raise e except IntegrityError as e: raise GenericBadRequest(f"Failed to update allowed groups! Details: {str(e)}") diff --git a/src/ai/backend/manager/api/exceptions.py b/src/ai/backend/manager/api/exceptions.py index 00608392fe..ab2847c575 100644 --- a/src/ai/backend/manager/api/exceptions.py +++ b/src/ai/backend/manager/api/exceptions.py @@ -260,6 +260,10 @@ class ContainerRegistryNotFound(ObjectNotFound): object_name = "container_registry" +class ContainerRegistryGroupsAssociationNotFound(ObjectNotFound): + object_name = "association of container_registry and group" + + class TooManySessionsMatched(BackendError, web.HTTPNotFound): error_type = "https://api.backend.ai/probs/too-many-sessions-matched" error_title = "Too many sessions matched." diff --git a/src/ai/backend/manager/client/container_registry/harbor.py b/src/ai/backend/manager/client/container_registry/harbor.py index 5fc520a958..fd72c7276e 100644 --- a/src/ai/backend/manager/client/container_registry/harbor.py +++ b/src/ai/backend/manager/client/container_registry/harbor.py @@ -40,7 +40,7 @@ async def delete_quota( ) -> None: raise NotImplementedError - async def read_quota(self, project_info: HarborProjectInfo) -> int: + async def read_quota(self, project_info: HarborProjectInfo, auth_args: HarborAuthArgs) -> int: raise NotImplementedError @@ -94,10 +94,10 @@ async def _get_quota_info( return HarborProjectQuotaInfo(previous_quota=previous_quota, quota_id=quota_id) @override - async def read_quota(self, project_info: HarborProjectInfo) -> int: + async def read_quota(self, project_info: HarborProjectInfo, auth_args: HarborAuthArgs) -> int: connector = aiohttp.TCPConnector(ssl=project_info.ssl_verify) async with aiohttp.ClientSession(connector=connector) as sess: - rqst_args: dict[str, Any] = {} + rqst_args = _get_harbor_auth_args(auth_args) quota_info = await self._get_quota_info(sess, project_info, rqst_args) previous_quota = quota_info["previous_quota"] if previous_quota == -1: diff --git a/src/ai/backend/manager/models/gql_models/container_registry.py b/src/ai/backend/manager/models/gql_models/container_registry.py index fabda6098d..f924114932 100644 --- a/src/ai/backend/manager/models/gql_models/container_registry.py +++ b/src/ai/backend/manager/models/gql_models/container_registry.py @@ -13,7 +13,7 @@ from ai.backend.common.container_registry import AllowedGroupsModel, ContainerRegistryType from ai.backend.logging import BraceStyleAdapter from ai.backend.manager.api.exceptions import ( - ContainerRegistryNotFound, + ContainerRegistryGroupsAssociationNotFound, ) from ai.backend.manager.models.container_registry import ContainerRegistryRow from ai.backend.manager.models.gql_models.fields import ScopeField @@ -295,7 +295,9 @@ async def handle_allowed_groups_update( ) result = await db_sess.execute(delete_query) if result.rowcount == 0: - raise ContainerRegistryNotFound() + raise ContainerRegistryGroupsAssociationNotFound( + f"Tried to remove non-existing associations for registry_id: {registry_id}, group_ids: {allowed_group_updates.remove}" + ) class CreateContainerRegistryNode(graphene.Mutation): diff --git a/src/ai/backend/manager/models/image.py b/src/ai/backend/manager/models/image.py index dcb24a94e8..07fc2b4cd8 100644 --- a/src/ai/backend/manager/models/image.py +++ b/src/ai/backend/manager/models/image.py @@ -885,7 +885,7 @@ async def build_ctx_in_domain_scope( user_scope_perm_ctx = await self._in_user_scope(ctx, UserScope(ctx.user_id)) perm_ctx.merge(user_scope_perm_ctx) - project_scopes = await self._get_user_accessible_project_scopes(ctx, UserScope(ctx.user_id)) + project_scopes = await self._get_domain_accessible_project_scopes(ctx, scope) non_global_container_registries_perm_ctx = await self._in_project_scopes_non_global( ctx, project_scopes ) @@ -1022,6 +1022,17 @@ async def _get_user_accessible_project_scopes( return [ProjectScope(project_id=group_id) for group_id in group_ids] + async def _get_domain_accessible_project_scopes( + self, + ctx: ClientContext, + scope: DomainScope, + ) -> list[ProjectScope]: + from .group import GroupRow + + stmt = sa.select(GroupRow.id).where(GroupRow.domain_name == scope.domain_name) + project_ids = await self.db_session.scalars(stmt) + return [ProjectScope(project_id=proj_id) for proj_id in project_ids] + async def _verify_project_scope_and_calculate_permission( self, ctx: ClientContext, scope: ProjectScope ) -> frozenset[ImagePermission]: @@ -1044,9 +1055,6 @@ async def _in_project_scopes_by_registry_condition( ) -> ImagePermissionContext: from .container_registry import ContainerRegistryRow - if not scopes: - raise InvalidScope("No project scopes provided") - project_ids = [scope.project_id for scope in scopes] project_id_to_permission_map: dict[str, frozenset[ImagePermission]] = {} diff --git a/src/ai/backend/manager/server.py b/src/ai/backend/manager/server.py index 18d2031b23..e81645ed94 100644 --- a/src/ai/backend/manager/server.py +++ b/src/ai/backend/manager/server.py @@ -182,6 +182,7 @@ global_subapp_pkgs: Final[list[str]] = [ ".acl", + ".container_registry", ".etcd", ".events", ".auth", diff --git a/src/ai/backend/manager/service/container_registry/harbor.py b/src/ai/backend/manager/service/container_registry/harbor.py index c666ff2e9c..5d748d534e 100644 --- a/src/ai/backend/manager/service/container_registry/harbor.py +++ b/src/ai/backend/manager/service/container_registry/harbor.py @@ -144,4 +144,7 @@ async def read_quota(self, scope_id: ProjectScope) -> int: registry_info = await self._repository.fetch_container_registry_row(scope_id) client = self._client_pool.make_client(registry_info.type) project_info = self._registry_row_to_harbor_project_info(registry_info) - return await client.read_quota(project_info) + credential = HarborAuthArgs( + username=registry_info.username, password=registry_info.password + ) + return await client.read_quota(project_info, credential)