feat(BA-5040): Apply RBAC validator for Image actions by fregataa · Pull Request #10059 · lablup/backend.ai

fregataa · 2026-03-13T03:22:08Z

Summary

Added RBAC base class (ImageSingleEntityAction) for Image service following established patterns
Refactored ForgetImageByIdAction and PurgeImageByIdAction to extend ImageSingleEntityAction and implement required RBAC methods (target_entity_id(), target_element(), entity_type())
Wired SingleEntityActionValidator to Image processors using SingleEntityActionProcessor for entity-level permission checks
Superadmin-only mutations (alias, dealias, modify, untag, scan, etc.) remain on plain ActionProcessor — access is enforced by check_admin_only at the API layer
Internal/system actions remain on plain ActionProcessor without RBAC validators

Test plan

Copilot

Pull request overview

Adds RBAC-aware action base classes for the Image service and wires RBAC validators into selected Image action processors to enforce authorization at the processor layer.

Changes:

Introduced Image RBAC base action/result classes (ImageScopeAction*, ImageSingleEntityAction*).
Refactored scope and single-entity Image actions/results to implement RBAC-required methods and carry scope metadata.
Updated Image processors to use ScopeActionProcessor / SingleEntityActionProcessor with RBAC validators for relevant actions.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/ai/backend/manager/services/image/service.py	Propagates scope metadata into action results for scope-based RBAC actions.
src/ai/backend/manager/services/image/processors.py	Switches certain processors to RBAC-aware processors and attaches RBAC validators.
src/ai/backend/manager/services/image/actions/search_images.py	Converts search action/result to RBAC scope action with scope metadata and target element.
src/ai/backend/manager/services/image/actions/purge_images.py	Converts purge-by-id action/result to single-entity RBAC action with target element.
src/ai/backend/manager/services/image/actions/get_all_images.py	Converts get-all action/result to RBAC scope action with scope metadata and target element.
src/ai/backend/manager/services/image/actions/forget_image.py	Converts forget-by-id action/result to single-entity RBAC action with target element.
src/ai/backend/manager/services/image/actions/base.py	Adds Image-specific RBAC base action/result classes for scope and single-entity patterns.
changes/10059.feature.md	Adds a changelog entry for applying RBAC validators to Image actions.

Comments suppressed due to low confidence (1)

src/ai/backend/manager/services/image/processors.py:1

The PR summary/title indicates applying RBAC validators to Image actions, but get_image_by_id remains on a plain ActionProcessor under the 'without RBAC validation' section. If get_image_by_id is meant to be protected by RBAC (typical for single-entity read), it should likely be moved to a SingleEntityActionProcessor with validators.rbac.single_entity; if it is intentionally exempt, please clarify why in the PR description or via an inline comment to avoid future confusion.

from typing import override

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

This PR introduces RBAC-aware base action types for the Image service and starts wiring RBAC validators into the Image action processing pipeline, updating tests and changelog accordingly.

Changes:

Added ImageSingleEntityAction / ImageSingleEntityActionResult base classes for RBAC-style single-entity actions.
Refactored ForgetImageByIdAction and PurgeImageByIdAction (+ results) to implement RBAC single-entity requirements (target_entity_id(), target_element()), and wired them through SingleEntityActionProcessor with validators.rbac.single_entity.
Updated unit/component tests to stub RBAC validator calls and added a changelog entry.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/ai/backend/manager/services/image/actions/base.py`	Adds Image single-entity RBAC base action/result types.
`src/ai/backend/manager/services/image/actions/forget_image.py`	Converts forget-by-id action/result to RBAC single-entity style with target element info.
`src/ai/backend/manager/services/image/actions/purge_images.py`	Converts purge-by-id action/result to RBAC single-entity style with target element info.
`src/ai/backend/manager/services/image/processors.py`	Switches forget/purge-by-id processors to `SingleEntityActionProcessor` and attaches RBAC validator.
`tests/unit/manager/services/image/test_image_service.py`	Adjusts processor fixture to provide async RBAC validator mocks.
`tests/component/image/conftest.py`	Adjusts component fixture to provide async RBAC validator mocks.
`changes/10059.feature.md`	Adds changelog note for applying RBAC validators to Image actions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Add ImageScopeAction and ImageSingleEntityAction base classes: - ImageScopeAction: extends BaseScopeAction, returns EntityType.IMAGE - ImageSingleEntityAction: extends BaseSingleEntityAction, returns EntityType.IMAGE, field_data() returns None - Added corresponding result classes: ImageScopeActionResult, ImageSingleEntityActionResult These base classes will be used to apply RBAC validators to image service actions. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Classify and refactor Image service actions to extend RBAC-aware base classes: Scope-based actions (extend ImageScopeAction): - SearchImagesAction: implement scope_type(), scope_id(), target_element() - GetAllImagesAction: add _scope_type/_scope_id fields, implement required methods Single-entity actions (extend ImageSingleEntityAction): - ForgetImageByIdAction: implement target_entity_id(), target_element() - PurgeImageByIdAction: implement target_entity_id(), target_element() Keep as ImageAction (special entity types or deprecated): - Actions with IMAGE_PRELOAD, IMAGE_SCAN, IMAGE_AGENT, IMAGE_TAG entity types - ImageAliasAction subclasses (IMAGE_ALIAS entity type) - ImageResourceLimitAction subclasses (IMAGE_RESOURCE_LIMIT entity type) - Deprecated actions (ForgetImageAction, PurgeImageAction, etc.) This prepares the actions for RBAC validator integration in processors. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

- Use ScopeActionProcessor for scope-based actions (get_all_images, search_images) - Use SingleEntityActionProcessor for single-entity actions (forget_image_by_id, purge_image_by_id) - Pass RBAC validators from ActionValidators to processors - Keep internal/system actions on plain ActionProcessor without RBAC validators Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

- Add _scope_type and _scope_id fields to SearchImagesAction - Update service methods to pass scope info to action results - Fix target_element implementation with proper RBACElementType mapping Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Fix mypy [call-arg] errors by making user_uuid and domain_name optional with default values in SearchImagesAction and GetAllImagesAction. Changes: - Add default empty strings to user_uuid and domain_name fields - Update call sites to pass context where available (REST, GQL legacy) - Internal utilities (data loaders) use defaults for system operations - Service layer updated to use user_uuid instead of _scope_type/_scope_id This allows RBAC validators to work for user-facing APIs while allowing internal batch operations to proceed without user context. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Replace MagicMock(spec=ActionValidators) with plain MagicMock and AsyncMock validators to support nested attribute access and async validate() calls required by ScopeActionProcessor and SingleEntityActionProcessor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…uid in scope actions Remove dead domain_name field from SearchImagesAction and GetAllImagesAction. Make user_uuid required (no default) and update all callers to provide it explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Revert SearchImagesAction to plain ImageAction without scope metadata. Remove user_uuid/domain_name fields, scope methods, and switch processor back to ActionProcessor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Revert GetAllImagesAction to plain ImageAction without scope metadata. Remove ImageScopeAction/ImageScopeActionResult base classes and ScopeActionProcessor usage from Image processors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Use spec'd mocks (ActionValidators/RBACValidators dataclass instances with MagicMock(spec=...)) instead of bare MagicMock() in test fixtures - Add comment explaining superadmin-only mutations skip RBAC validation since access is enforced by check_admin_only at the API layer Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jopemachine · 2026-03-18T02:23:28Z

Overall, it’s unclear whether the actions without permission checks were intentionally left unmodified because they are deprecated, or if they were simply overlooked.

Would it make sense to add deprecation annotations to all actions that have not been updated?

HyeockJinKim · 2026-03-18T06:27:24Z

It seems the admin service should be separated; please review this. Rather than distinguishing between them at the API level, please verify at the service layer whether the respective operations should be restricted to admins only. (For GET requests, check if the user has access permissions; for SEARCH, check based on the scope; for CREATE, UPDATE, DELETE, and PURGE, check based on permissions.) @fregataa

Copilot AI review requested due to automatic review settings March 13, 2026 03:22

github-actions Bot added the size:L 100~500 LoC label Mar 13, 2026

github-actions Bot assigned fregataa Mar 13, 2026

github-actions Bot added the comp:manager Related to Manager component label Mar 13, 2026

fregataa added a commit that referenced this pull request Mar 13, 2026

changelog: add news fragment for PR #10059

1078650

Copilot AI reviewed Mar 13, 2026

View reviewed changes

Comment thread src/ai/backend/manager/services/image/actions/get_all_images.py Outdated

Comment thread src/ai/backend/manager/services/image/actions/search_images.py Outdated

fregataa added a commit that referenced this pull request Mar 13, 2026

changelog: add news fragment for PR #10059

e435910

fregataa force-pushed the BA-5040 branch from 19e412c to 03c62c5 Compare March 13, 2026 09:31

fregataa requested review from a team and Copilot March 13, 2026 10:00

Copilot started reviewing on behalf of fregataa March 13, 2026 10:00 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

Comment thread src/ai/backend/manager/services/image/processors.py

Comment thread src/ai/backend/manager/services/image/actions/base.py

Comment thread tests/unit/manager/services/image/test_image_service.py Outdated

Comment thread tests/component/image/conftest.py Outdated

fregataa added this to the 26.4 milestone Mar 17, 2026

jopemachine reviewed Mar 17, 2026

View reviewed changes

Comment thread src/ai/backend/manager/services/image/processors.py

fregataa and others added 13 commits March 17, 2026 18:14

fix(BA-5040): move import to top-level to resolve linter error

90cf8ae

changelog: add news fragment for PR #10059

38942b0

style(BA-5040): apply ruff format to conftest

ca73d3a

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fregataa force-pushed the BA-5040 branch from 2a02af5 to 0678756 Compare March 17, 2026 09:15

fregataa requested a review from a team March 17, 2026 09:27

HyeockJinKim approved these changes Mar 18, 2026

View reviewed changes

HyeockJinKim merged commit 2573ff8 into main Mar 18, 2026
30 checks passed

HyeockJinKim deleted the BA-5040 branch March 18, 2026 06:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(BA-5040): Apply RBAC validator for Image actions#10059

feat(BA-5040): Apply RBAC validator for Image actions#10059
HyeockJinKim merged 13 commits into
mainfrom
BA-5040

fregataa commented Mar 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jopemachine commented Mar 18, 2026

Uh oh!

HyeockJinKim commented Mar 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

fregataa commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jopemachine commented Mar 18, 2026

Uh oh!

HyeockJinKim commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fregataa commented Mar 13, 2026 •

edited

Loading

HyeockJinKim commented Mar 18, 2026 •

edited

Loading