Skip to content

[#12287] feat(server): Add bulk role access-control APIs - #12732

Open
jarredhj0214 wants to merge 2 commits into
apache:mainfrom
jarredhj0214:codex/bulk-role-apis
Open

[#12287] feat(server): Add bulk role access-control APIs#12732
jarredhj0214 wants to merge 2 commits into
apache:mainfrom
jarredhj0214:codex/bulk-role-apis

Conversation

@jarredhj0214

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR adds best-effort bulk role access-control APIs as the third part of #12287.

Changes include:

  • Add bulk role add request/response DTOs.
  • Add a core RoleAdd bulk item model.
  • Add AccessControlDispatcher#createRoles and #deleteRoles.
  • Implement bulk role create/delete logic in AccessControlManager with item-level results.
  • Dispatch existing per-role create/delete pre, success, and failure events for bulk role operations.
  • Add REST APIs for bulk role add/remove.
  • Add OpenAPI definitions for bulk role APIs.
  • Add tests for best-effort behavior, request validation, role authorization, and core manager behavior.

Why are the changes needed?

#12287 tracks best-effort bulk operations for access-control entities under a metalake. The first PR added the shared bulk foundation and bulk user APIs, and the second PR added bulk group APIs. This PR continues the epic by adding bulk role APIs using the same approach.

Part of #12287.

Does this PR introduce any user-facing change?

Yes.

New REST APIs:

  • POST /api/bulk/metalakes/{metalake}/roles/add
  • POST /api/bulk/metalakes/{metalake}/roles/remove

The APIs use the existing gravitino.server.bulk.maxItems limit introduced by the bulk foundation PR.

How was this patch tested?

  • JAVA_HOME=/opt/homebrew/Cellar/openjdk@17/17.0.18/libexec/openjdk.jdk/Contents/Home ./gradlew :common:compileJava :core:compileTestJava :server:compileTestJava :clients:client-java:compileTestJava
  • JAVA_HOME=/opt/homebrew/Cellar/openjdk@17/17.0.18/libexec/openjdk.jdk/Contents/Home ./gradlew :core:test --tests org.apache.gravitino.authorization.TestAccessControlManager :server:test --tests org.apache.gravitino.server.web.rest.TestBulkOperations :docs:build -PskipITs
  • JAVA_HOME=/opt/homebrew/Cellar/openjdk@17/17.0.18/libexec/openjdk.jdk/Contents/Home ./gradlew :server:test --tests org.apache.gravitino.server.web.rest.TestBulkOperations -PskipITs
  • git diff --check

@jarredhj0214

Copy link
Copy Markdown
Contributor Author

Hi @lasdf1234, this is the third part of #12287, adding bulk role access-control APIs following the same approach as the merged bulk user and group PRs. Could you please help review it when you have time? Thanks!

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

Code Coverage Report

Overall Project 68.89% +0.54% 🟢
Files changed 86.19% 🟢

Module Coverage
aliyun 19.74% 🔴
api 51.74% 🟢
authorization-common 85.96% 🟢
authorization-ranger 4.38% 🔴
aws 53.54% 🟢
azure 32.1% 🔴
catalog-common 19.1% 🔴
catalog-fileset 80.3% 🟢
catalog-glue 69.24% 🟢
catalog-hive 82.96% 🟢
catalog-jdbc-common 45.69% 🟢
catalog-jdbc-doris 82.69% 🟢
catalog-jdbc-mysql 79.33% 🟢
catalog-jdbc-postgresql 83.39% 🟢
catalog-jdbc-starrocks 79.16% 🟢
catalog-kafka 76.99% 🟢
catalog-lakehouse-generic 60.55% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.86% 🟢
catalog-lakehouse-paimon 84.26% 🟢
catalog-model 77.99% 🟢
cli 44.48% 🟢
client-java 77.48% 🟢
common 56.31% +0.16% 🟢
core 83.72% +0.25% 🟢
filesystem-hadoop3 76.45% 🟢
flink 0.0% 🔴
flink-common 52.1% 🟢
flink-runtime 0.0% 🔴
gcp 32.2% 🔴
hadoop-auth 68.0% 🟢
hadoop-common 17.84% 🔴
hive-metastore-common 53.4% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 64.75% 🟢
iceberg-rest-server 75.96% 🟢
idp-basic 85.98% 🟢
integration-test-common 0.0% 🔴
jobs 62.92% 🟢
lance-common 32.63% 🔴
lance-rest-server 65.46% 🟢
lineage 53.02% 🟢
optimizer 83.17% 🟢
optimizer-api 21.95% 🔴
server 87.67% -0.18% 🟢
server-common 80.5% 🟢
spark 28.57% 🔴
spark-common 48.92% 🟢
tencent 81.78% 🟢
trino-connector 51.26% 🟢
Files
Module File Coverage
common BulkRoleAddRequest.java 93.33% 🟢
BulkRoleResponse.java 89.47% 🟢
core RoleAdd.java 100.0% 🟢
RoleMetaBaseSQLProvider.java 100.0% 🟢
RoleMetaPostgreSQLProvider.java 100.0% 🟢
OccWriteSupport.java 100.0% 🟢
AccessControlManager.java 99.11% 🟢
UserMetaService.java 96.23% 🟢
GroupMetaService.java 96.08% 🟢
RoleMetaSQLProviderFactory.java 96.0% 🟢
RoleMetaService.java 94.83% 🟢
FilesetMetaService.java 93.98% 🟢
POConverters.java 88.34% 🟢
AccessControlEventDispatcher.java 82.34% 🟢
ModelMetaService.java 81.82% 🟢
TopicMetaService.java 77.34% 🟢
AccessControlHookDispatcher.java 24.22% 🔴
AccessControlDispatcher.java 0.0% 🔴
RoleMetaMapper.java 0.0% 🔴
server BulkOperations.java 84.86% 🟢

@lasdf1234

Copy link
Copy Markdown
Collaborator
  1. In the "access-control.md" file, add the bulk role and change "authorized once" - the role removal process is now per-item authorization.
  2. Bulk deleteRole and single deleteRole should have the same event.

@jarredhj0214

Copy link
Copy Markdown
Contributor Author
  1. In the "access-control.md" file, add the bulk role and change "authorized once" - the role removal process is now per-item authorization.
  2. Bulk deleteRole and single deleteRole should have the same event.

Thanks for the review. I updated access-control.md to include the bulk role APIs and clarified that bulk role removal is authorized per item. I also aligned bulk delete role events with single delete role events, and added unit tests for the bulk role request/response DTOs.

@roryqi Could you please help review this when you have free time? Thanks!

@roryqi
roryqi requested a review from lasdf1234 September 1, 2026 06:10
@roryqi

roryqi commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@lasdf1234 Could u take a look again?

@lasdf1234

Copy link
Copy Markdown
Collaborator

@lasdf1234 Could u take a look again?

Got. I'll look again.

| `POST /api/bulk/metalakes/{metalake}/groups/remove` | `OWNER` of the metalake or `MANAGE_GROUPS` |
| `POST /api/bulk/metalakes/{metalake}/roles/add` | `OWNER` of the metalake or `CREATE_ROLE` |
| `POST /api/bulk/metalakes/{metalake}/roles/remove` | `OWNER` of the metalake or each role |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OWNER of the metalake, or OWNER of the role May be this better for roles remove.

@lasdf1234

Copy link
Copy Markdown
Collaborator

@jarredhj0214 Only a little issue.

@jarredhj0214

Copy link
Copy Markdown
Contributor Author

@jarredhj0214 Only a little issue.

Fixed. Thanks!

@jarredhj0214

jarredhj0214 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Hi @lasdf1234, @roryqi I have addressed the previous comments. Could you please help take another look when you have time? Thanks!

| `POST /api/bulk/metalakes/{metalake}/groups/remove` | `OWNER` of the metalake or `MANAGE_GROUPS` |
| `POST /api/bulk/metalakes/{metalake}/roles/add` | `OWNER` of the metalake or `CREATE_ROLE` |
| `POST /api/bulk/metalakes/{metalake}/roles/remove` | `OWNER` of the metalake, or `OWNER` of the role |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table alignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants