HDDS-15982. Fix UnsupportedOperationException and add idempotent deletion in RangerClientMultiTenantAccessController - #10872
Conversation
…RangerClientMultiTenantAccessController Co-authored-by: sallmayasser <sallmayasser512@gmail.com>
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @MahmoudHassanen99 for the patch. Please enable workflows in your fork.
|
Thanks @adoroszlai and @ayushtkn for the review I have pushed a new commit addressing all the feedback (cleaned up Javadoc/license, removed internal tags, and kept single-line logging). Ready for another look |
Co-authored-by: Amr ElBoridy <amrelboridy7@gmail.com>
|
Fixed the issues, all CI tests are now passing 100% green on my fork Could you please approve the workflow run here when you get a chance? |
|
Thanks @MahmoudHassanen99 for updating the patch. @smengcl please take a look. |
|
Hi @smengcl, just a gentle check-in on this PR when you have a moment. All CI checks are passing 100% green (43/43), Thank you |
smengcl
left a comment
smengcl
left a comment
There was a problem hiding this comment.
Thanks for the patch. The mutable-list change addresses the immediate UnsupportedOperationException. I have two concerns about the new retry and deletion handling:
- The duplicate handling may not fully guarantee idempotent creation because it can return success without verifying that Ranger reached the requested state.
- Ranger 2.8 appears to report deletion of an absent role as HTTP 400 rather than 404, so the current check may not make
deleteRoleidempotent.
I left inline comments with more detail. Could you also add coverage for matching and conflicting duplicate state, deletion of an absent role, and an unrelated HTTP 400 deletion failure that should still be propagated?
…t creation and Ranger 2.8 role deletion compatibility. Co-authored-by: Salma<sallmayasser512@gmail.com>
Co-authored-by: Amr ElBoridy <amrelboridy7@gmail.com> Co-authored-by: Salma Yasser <sallmayaser512@gmail.com>
…RangerClientMultiTenantAccessController Removed commented-out code and unnecessary comments to clean up the RangerClientMultiTenantAccessController class. Co-authored-by: Amr ElBoridy <amrelboridy7@gmail.com> Co-authored-by: Mahmoud Hassanen <mahmoudhassanen99@gmail.com>
This class contains unit tests for the RangerClientMultiTenantAccessController using a mock RangerClient. It tests various scenarios including role deletion and policy creation. Co-authored-by: Amr ElBoridy <amrelboridy7@gmail.com> Co-authored-by: Mahmoud Hassanen <mahmoudhassanen99@gmail.com>
Initialize Kerberos name rules in the test setup. Co-authored-by: Amr ElBoridy <amrelboridy7@gmail.com> Co-authored-by: Mahmoud Hassanen <mahmoudhassanen99@gmail.com>
smengcl
changed the title
|
CI passed on fork: https://github.com/MahmoudHassanen99/ozone/actions/runs/31516939523 |
|
Thanks @MahmoudHassanen99 for the patch. Thanks @adoroszlai @ayushtkn @sallmayasser for the reviews. |
|
Thanks @smengcl for merging, and thanks @adoroszlai @ayushtkn @sallmayasser for the thorough reviews I will open a follow-up JIRA ticket to track state-aware validation and reconciliation for Ranger roles and policies (handling duplicate creation retries and drift visibility) so we can explore that as a dedicated improvement. |
5 participants
What changes were proposed in this pull request?
UnsupportedOperationExceptionwhen initializing policy items/accesses inRangerClientMultiTenantAccessControllerby using explicit mutableArrayListinstances.createRoleandcreatePolicyoperations fail-fast. Ranger creation errors, including duplicate errors, are propagated.deleteRoleanddeletePolicyoperations. Missing resources reported through HTTP 404 are ignored. The Ranger 2.8 HTTP 400 response for a missing role is also ignored.What is the link to the Apache JIRA issue?
https://issues.apache.org/jira/browse/HDDS-15982
How was this patch tested?
Verified via code inspection and integration flow testing for multi-tenant Ranger client access management.