EdColeman commented on code in PR #3109:
URL: https://github.com/apache/accumulo/pull/3109#discussion_r1043409020
##########
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:
I'm looking at adding an option in zoo-info-viewer that can dump the acls in
zookeeper that could be used before running the upgrade.
I was leaning towards checking that our processes have cdrw permissions -
either with the accumulo digest or via wide-open, world-anyone - that ensures
that we have the permissions necessary to run but does not provide any security
assurance.
Otherwise, other than a few nodes like /accumulo and /accumulo/instance I
think all nodes under /accumulo/[IID]/tables could be protected with the
accumulo digest and not allow any other zookeeper user id access - but that
could impact any uses that have external utilities that access ZooKeeper (say
to read FATE status) that might not have the accumulo digest set - they
probably should, but they might have been okay with world-read.
--
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]
