HDDS-15982. Fix UnsupportedOperationException and add idempotency to RangerClientMultiTenantAccessController

[MahmoudHassanen99]

What changes were proposed in this pull request?

1. Fixed UnsupportedOperationException when initializing policy items/accesses in RangerClientMultiTenantAccessController by using explicit mutable ArrayList instances.
2. Added idempotency to createRole and createPolicy operations (gracefully catches HTTP 400 duplicate errors and fetches the existing resource).
3. Added tolerant deletion to deleteRole and deletePolicy operations (gracefully catches HTTP 404 missing errors).

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.

[adoroszlai]

Thanks @MahmoudHassanen99 for the patch. Please enable workflows in your fork.

[MahmoudHassanen99]

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

[MahmoudHassanen99]

Hi @adoroszlai @AyushSaxena,

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?
Thank you for your effort

[adoroszlai]

Thanks @MahmoudHassanen99 for updating the patch. @smengcl please take a look.

[MahmoudHassanen99]

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]

Thanks for the patch. The mutable-list change addresses the immediate UnsupportedOperationException. I have two concerns about the new retry and deletion handling:

1. The duplicate handling may not fully guarantee idempotent creation because it can return success without verifying that Ranger reached the requested state.
2. Ranger 2.8 appears to report deletion of an absent role as HTTP 400 rather than 404, so the current check may not make deleteRole idempotent.

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?

[smengcl]

I do not think this fully guarantees idempotency for createPolicy. It could turn a duplicate error into success without confirming that the existing Ranger policy represents the requested resources, ACLs, roles, and labels.

For example, if an earlier tenant-create attempt left a policy for volume A and the retry requests volume B, this branch could return success while Ranger still contains the policy for A. The Ranger background sync appears to match policies by name, so it may not repair that mismatch.

The same concern may apply to createRole, including user membership and nested role/admin membership. Could we either:

• verify that the existing object matches the requested state, then fail or update it when it differs
• remove the generalized idempotency changes from this PR and handle state-aware reconciliation under a separate Jira

[smengcl]

Ranger 2.8 appears to report a missing role from delete-by-name as HTTP 400 rather than 404. Accepting every HTTP 400 could also hide real failures, such as deletion being rejected because the role is still referenced.

Since RangerServiceException does not expose a structured Ranger error code, could we use a narrowly scoped compatibility workaround and document why the response text is inspected?

+  private static boolean isRoleNotFoundException(
+      RangerServiceException e) {
+    if (isNotFoundException(e)) {
+      return true;
+    }
+
+    // Ranger 2.8 returns HTTP 400 instead of 404 when deleting a role
+    // that does not exist. RangerServiceException exposes no structured
+    // Ranger error code, so inspect the response text as a workaround.
+    String message = e.getMessage();
+    return e.getStatus() != null
+        && e.getStatus().getStatusCode() == HTTP_STATUS_CODE_BAD_REQUEST
+        && message != null
+        && message.contains("Role with name")
+        && message.contains("does not exist");
+  }
+
   ...

-      if (isNotFoundException(e)) {
+      if (isRoleNotFoundException(e)) {

Could you also test both the missing-role response and an unrelated HTTP 400 response to confirm that the latter is still propagated?