dlmarion commented on code in PR #3109:
URL: https://github.com/apache/accumulo/pull/3109#discussion_r1043277307
##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java:
##########
@@ -165,8 +165,10 @@ private void validateACLs(ServerContext context) {
if (((path.equals(Constants.ZROOT) || path.equals(Constants.ZROOT +
Constants.ZINSTANCES))
&& !acls.equals(ZooDefs.Ids.OPEN_ACL_UNSAFE))
- || (!privateWithAuth.equals(acls) &&
!publicWithAuth.equals(acls))) {
- log.error("ZNode at {} has unexpected ACL: {}", path, acls);
+ || (!acls.containsAll(privateWithAuth) &&
!acls.containsAll(publicWithAuth))) {
Review Comment:
Customer ran into an issue testing the 2.1 upgrade. Manual intervention was
performed and ACLs were modified before I got involved. At the point I was
involved user had set acls on the znodes manually using the instructions in the
user guide. The order of the ACLs in the setAcl command appeared "correct", but
this check was still failing. Customer applied this patch and the upgrade got
past this step. So, either the order of the ACLs in the setACL commands was
incorrect, or getAcl could return them in a different order. The objects being
compared are Lists, so equals checks the order. This was an attempt to perform
an order independent comparison without creating new Set objects from the
Lists. If having other ACLs present is an issue, then I can modify to create
Sets and call `equals`.
--
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]
