s4ravanan opened a new pull request, #13507:
URL: https://github.com/apache/gravitino/pull/13507
### What changes were proposed in this pull request?
- In `RoleMetaService` (`insertRole` and `toSecurableObjectPOs`), catch
`NoSuchEntityException` from `EntityIdService.getEntityId(...)` when resolving
securable object identifiers and throw `NoSuchMetadataObjectException` with the
original exception preserved as the cause.
- In `PermissionManager` (`grantPrivilegesToRole`,
`revokePrivilegesFromRole`, `overridePrivilegesInRole`), check if
`NoSuchEntityException` is for a missing securable metadata object instead of a
role entity. If so, throw `NoSuchMetadataObjectException(nse, ...)`. When the
role itself is missing, preserve `nse` as the cause in
`NoSuchRoleException(nse, ...)`.
- Update `AccessControlDispatcher` and its implementations/dispatchers
(`AccessControlManager`, `AccessControlHookDispatcher`,
`AccessControlEventDispatcher`) to declare `NoSuchMetadataObjectException` in
`grantPrivilegeToRole`, `revokePrivilegesFromRole`, and
`overridePrivilegesInRole`.
- Added unit tests in `TestRoleMetaService` and
`TestAccessControlManagerForPermissions`.
### Why are the changes needed?
Previously, when granting, revoking, or overriding privileges on a role for
a non-existent securable metadata object (such as a catalog, schema, or table),
`EntityIdService.getEntityId(...)` threw a `NoSuchEntityException`.
`PermissionManager` unconditionally caught all `NoSuchEntityException`
instances and rethrew `NoSuchRoleException`, misreporting that the role was
missing instead of the securable object.
Fix: #13504
### Does this PR introduce _any_ user-facing change?
Yes. Operating privileges on non-existent securable objects will now
correctly raise `NoSuchMetadataObjectException` (reporting that the metadata
object does not exist) rather than `NoSuchRoleException` (which falsely
reported that the role did not exist).
### How was this patch tested?
- Added `testInsertAndUpdateRoleWithNonExistentMetadataObject` in
`TestRoleMetaService`.
- Added `testMissingSecurableObjectThrowsNoSuchMetadataObjectException` and
verified cause preservation in `TestAccessControlManagerForPermissions`.
- Ran:
- `./gradlew :core:test --tests "*TestRoleMetaService*" --tests
"*TestAccessControlManagerForPermissions*"`
- `./gradlew :core:test --tests "*TestRoleEvent*" :server:test --tests
"*TestPermissionOperations*"`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
