[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15656130#comment-15656130
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2014:
-------------------------------------------

Github user fpj commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/96#discussion_r87536973
  
    --- Diff: src/java/test/org/apache/zookeeper/server/DataTreeTest.java ---
    @@ -200,29 +198,34 @@ public void 
testSerializeDoesntLockDataNodeWhileWriting() throws Exception {
             BinaryOutputArchive oa = new BinaryOutputArchive(out) {
                 @Override
                 public void writeRecord(Record r, String tag) throws 
IOException {
    -                DataNode node = (DataNode) r;
    -                if (node.data.length == 1 && node.data[0] == 42) {
    -                    final Semaphore semaphore = new Semaphore(0);
    -                    new Thread(new Runnable() {
    -                        @Override
    -                        public void run() {
    -                            synchronized (markerNode) {
    -                                //When we lock markerNode, allow 
writeRecord to continue
    -                                semaphore.release();
    +                // Need check if the record is a DataNode instance because 
of changes in ZOOKEEPER-2014
    +                // which adds default ACL to config node.
    +                if (r instanceof DataNode) {
    --- End diff --
    
    @hanm hmm, I'm not sure about this. In the changes for `DataTree`, we only 
set the ACL of the `/zookeeper/config` znode, but setting ACLs was something we 
were doing before, so I'm confused about why we can have a mix of znode records 
and ACL records with the changes proposed here. Could you clarify, please?


> Only admin should be allowed to reconfig a cluster
> --------------------------------------------------
>
>                 Key: ZOOKEEPER-2014
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: server
>    Affects Versions: 3.5.0
>            Reporter: Raul Gutierrez Segales
>            Assignee: Michael Han
>            Priority: Blocker
>             Fix For: 3.5.3
>
>         Attachments: ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch
>
>
> ZOOKEEPER-107 introduces reconfiguration support via the reconfig() call. We 
> should, at the very least, ensure that only the Admin can reconfigure a 
> cluster. Perhaps restricting access to /zookeeper/config as well, though this 
> is debatable. Surely one could ensure Admin only access via an ACL, but that 
> would leave everyone who doesn't use ACLs unprotected. We could also force a 
> default ACL to make it a bit more consistent (maybe).
> Finally, making reconfig() only available to Admins means they have to run 
> with zookeeper.DigestAuthenticationProvider.superDigest (which I am not sure 
> if everyone does, or how would it work with other authentication providers). 
> Review board https://reviews.apache.org/r/51546/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to