[GitHub] zookeeper issue #106: ZOOKEEPER-1932: Remove deprecated LeaderElection class...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/106 I will review it soon --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #106: ZOOKEEPER-1932: Remove deprecated LeaderElection class...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/106 Jira ZOOKEEPER-1932 title is completely different from what is being done in its MR. It think we should change the title as well as the description --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/106#discussion_r115112169
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int
electionAlgorithm){
//TODO: use a factory rather than a switch
switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not
supported anymore.";
--- End diff --
I think we should log error here and make sure the server jvm exit in this
case
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/106#discussion_r115111921
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
---
@@ -809,28 +809,16 @@ synchronized public void stopLeaderElection() {
responder.interrupt();
--- End diff --
The import java.net.SocketException is never used
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/106#discussion_r115112178
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int
electionAlgorithm){
//TODO: use a factory rather than a switch
switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not
supported anymore.";
--- End diff --
Another option is validate electionAlg in
org.apache.zookeeper.server.quorum.QuorumPeerConfig and exit from there for
wrong input and remove the case from here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/106#discussion_r115111872 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml --- @@ -948,15 +948,14 @@ server.3=zoo3:2888:3888 (No Java system property) - Election implementation to use. A value of "0" corresponds - to the original UDP-based version, "1" corresponds to the + Election implementation to use. A value of "1" corresponds to the non-authenticated UDP-based version of fast leader election, "2" corresponds to the authenticated UDP-based version of fast leader election, and "3" corresponds to TCP-based version of - fast leader election. Currently, algorithm 3 is the default + fast leader election. Currently, algorithm 3 is the default. - The implementations of leader election 0, 1, and 2 are now + The implementations of leader election 1, and 2 are now deprecated . We have the intention of removing them in the next release, at which point only the FastLeaderElection will be available. --- End diff -- There is at least one more reference of election alog type 0. Should be deleted. Reference Text : If electionAlg is 0, then the second port is not necessary --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/106#discussion_r115111914
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int
electionAlgorithm){
//TODO: use a factory rather than a switch
switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not
supported anymore.";
+break;
+case 1:
+le = new AuthFastLeaderElection(this);
+break;
+case 2:
+le = new AuthFastLeaderElection(this, true);
+break;
+case 3:
+qcm = new QuorumCnxManager(this);
+QuorumCnxManager.Listener listener = qcm.listener;
+if(listener != null){
+listener.start();
+FastLeaderElection fle = new FastLeaderElection(this,
qcm);
+fle.start();
+le = fle;
+} else {
+LOG.error("Null listener when initializing cnx
manager");
+}
+break;
+default:
+assert false;
}
return le;
}
@SuppressWarnings("deprecation")
protected Election makeLEStrategy(){
LOG.debug("Initializing leader election protocol...");
-if (getElectionType() == 0) {
-electionAlg = new LeaderElection(this);
-}
return electionAlg;
}
--- End diff --
org.apache.zookeeper.server.quorum.QuorumPeerConfig.parseDynamicConfig(Properties,
int, boolean, boolean)
has a logic related to election algo type 0. This should be removed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #106: ZOOKEEPER-1932: Remove deprecated LeaderElection class...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/106 LGTM +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/254 ZOOKEEPER-2775: ZK Client not able to connect with Xid out of order error Once client enters into Xid out of order issue, It never comes to normal state. It keeps trying to connect and fail with the same error. Recreating/Restarting is the only solution as of now. This happens because of bug in the ZK client code. This MR provides the fix. You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2775-XidOutOfOrder Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/254.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #254 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/254#discussion_r116871274
--- Diff: src/java/test/org/apache/zookeeper/SaslAuthTest.java ---
@@ -0,0 +1,187 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.zookeeper.ClientCnxn.SendThread;
+import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class SaslAuthTest extends ClientBase {
--- End diff --
File was moved to make SendThread accessible in the test case. Now moved
with git mv. This is the right way to move the file
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/254#discussion_r116871380
--- Diff: src/java/test/org/apache/zookeeper/SaslAuthTest.java ---
@@ -0,0 +1,187 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.zookeeper.ClientCnxn.SendThread;
+import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class SaslAuthTest extends ClientBase {
+
+@BeforeClass
+public static void init() {
+System.setProperty("zookeeper.authProvider.1",
"org.apache.zookeeper.server.auth.SASLAuthenticationProvider");
+try {
+File tmpDir = createTmpDir();
+File saslConfFile = new File(tmpDir, "jaas.conf");
+FileWriter fwriter = new FileWriter(saslConfFile);
+
+fwriter.write("" + "Server {\n" + "
org.apache.zookeeper.server.auth.DigestLoginModule required\n"
++ " user_super=\"test\";\n" + "};\n" +
"Client {\n"
++ "
org.apache.zookeeper.server.auth.DigestLoginModule required\n"
++ " username=\"super\"\n" + "
password=\"test\";\n" + "};" + "\n");
+fwriter.close();
+System.setProperty("java.security.auth.login.config",
saslConfFile.getAbsolutePath());
+} catch (IOException e) {
+// could not create tmp directory to hold JAAS conf file :
test will
+// fail now.
+}
+}
+
+@AfterClass
+public static void clean() {
+System.clearProperty("zookeeper.authProvider.1");
+System.clearProperty("java.security.auth.login.config");
+}
+
+private AtomicInteger authFailed = new AtomicInteger(0);
+
+@Override
+protected TestableZooKeeper createClient(String hp) throws
IOException, InterruptedException {
+MyWatcher watcher = new MyWatcher();
+return createClient(watcher, hp);
+}
+
+private class MyWatcher extends CountdownWatcher {
+@Override
+public synchronized void process(WatchedEvent event) {
+if (event.getState() == KeeperState.AuthFailed) {
+authFailed.incrementAndGet();
+} else {
+super.process(event);
+}
+}
+}
+
+@Test
+public void testAuth() throws Exception {
+ZooKeeper zk = createClient();
+try {
+zk.create("/path1", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+Thread.sleep(1000);
+} finally {
+zk.close();
+}
+}
+
+@Test
+public void testValidSaslIds() throws Exception {
+ZooKeeper zk = createClient();
+
+List validIds = new ArrayList();
+validIds.add("user");
+validIds.add("service/host.name.com");
+validIds.add("user@KERB.REALM");
+validIds.add("service/host.name.com@KERB.REALM");
+
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/254#discussion_r116871534
--- Diff: src/java/test/org/apache/zookeeper/SaslAuthTest.java ---
@@ -0,0 +1,187 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.zookeeper.ClientCnxn.SendThread;
+import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class SaslAuthTest extends ClientBase {
+
+@BeforeClass
+public static void init() {
+System.setProperty("zookeeper.authProvider.1",
"org.apache.zookeeper.server.auth.SASLAuthenticationProvider");
+try {
+File tmpDir = createTmpDir();
+File saslConfFile = new File(tmpDir, "jaas.conf");
+FileWriter fwriter = new FileWriter(saslConfFile);
+
+fwriter.write("" + "Server {\n" + "
org.apache.zookeeper.server.auth.DigestLoginModule required\n"
++ " user_super=\"test\";\n" + "};\n" +
"Client {\n"
++ "
org.apache.zookeeper.server.auth.DigestLoginModule required\n"
++ " username=\"super\"\n" + "
password=\"test\";\n" + "};" + "\n");
+fwriter.close();
+System.setProperty("java.security.auth.login.config",
saslConfFile.getAbsolutePath());
+} catch (IOException e) {
+// could not create tmp directory to hold JAAS conf file :
test will
+// fail now.
+}
+}
+
+@AfterClass
+public static void clean() {
+System.clearProperty("zookeeper.authProvider.1");
+System.clearProperty("java.security.auth.login.config");
+}
+
+private AtomicInteger authFailed = new AtomicInteger(0);
+
+@Override
+protected TestableZooKeeper createClient(String hp) throws
IOException, InterruptedException {
+MyWatcher watcher = new MyWatcher();
+return createClient(watcher, hp);
+}
+
+private class MyWatcher extends CountdownWatcher {
+@Override
+public synchronized void process(WatchedEvent event) {
+if (event.getState() == KeeperState.AuthFailed) {
+authFailed.incrementAndGet();
+} else {
+super.process(event);
+}
+}
+}
+
+@Test
+public void testAuth() throws Exception {
+ZooKeeper zk = createClient();
+try {
+zk.create("/path1", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+Thread.sleep(1000);
+} finally {
+zk.close();
+}
+}
+
+@Test
+public void testValidSaslIds() throws Exception {
+ZooKeeper zk = createClient();
+
+List validIds = new ArrayList();
+validIds.add("user");
+validIds.add("service/host.name.com");
+validIds.add("user@KERB.REALM");
+validIds.add("service/host.name.com@KERB.REALM");
+
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/254#discussion_r116872750
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -1080,6 +1080,8 @@ private void startConnect() throws IOException {
zooKeeperSaslClient.shutdown();
}
zooKeeperSaslClient = new
ZooKeeperSaslClient(getServerPrincipal(addr), clientConfig);
+// SASL login succeeded
+saslLoginFailed = false;
--- End diff --
this change has impact on tunnelAuthInProgress. But yes, we should init
the variable on new connection start as this is tunnelAuthInProgress logic
expects.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #254: ZOOKEEPER-2775: ZK Client not able to connect with Xid...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/254 zooKeeperSaslClient == null is already used in tunnelAuthInProgress() which is quite different from saslLoginFailed being false. So I think we can not do above change. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #254: ZOOKEEPER-2775: ZK Client not able to connect with Xid...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/254 Thanks @afine @hanm for the reviews. Addressed the comments, Please have a look. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/254#discussion_r117014954
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -1054,6 +1054,8 @@ private void sendPing() {
private boolean saslLoginFailed = false;
private void startConnect() throws IOException {
+// initializing it for new connection
+saslLoginFailed = false;
--- End diff --
Only one thread involved in new connection creation. So there will not any
chance another thread setting the flag in between.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/254#discussion_r117016986
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -1054,6 +1054,8 @@ private void sendPing() {
private boolean saslLoginFailed = false;
private void startConnect() throws IOException {
+// initializing it for new connection
+saslLoginFailed = false;
--- End diff --
Successfull Login is not necessary to successfully connect to ZK Server.
ZK server may allow connection without client login being successful, based on
configuration. So it is better to set the flag at the start of the new
connection.
This change has impact on tunnelAuthInProgress, in this method also same
thing is expeted. In this there is a sceanrio where zooKeeperSaslClient can be
null
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #254: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/254#discussion_r117021379
--- Diff: src/java/test/org/apache/zookeeper/SaslAuthTest.java ---
@@ -16,54 +16,58 @@
* limitations under the License.
*/
-package org.apache.zookeeper.test;
+package org.apache.zookeeper;
+
+import static org.junit.Assert.assertTrue;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
+import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.zookeeper.CreateMode;
-import org.apache.zookeeper.KeeperException;
-import org.apache.zookeeper.TestableZooKeeper;
-import org.apache.zookeeper.WatchedEvent;
-import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.ClientCnxn.SendThread;
import org.apache.zookeeper.Watcher.Event.KeeperState;
import org.apache.zookeeper.ZooDefs.Ids;
import org.apache.zookeeper.data.ACL;
import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
import org.junit.Assert;
+import org.junit.BeforeClass;
import org.junit.Test;
public class SaslAuthTest extends ClientBase {
-static {
-
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.SASLAuthenticationProvider");
-
+@BeforeClass
+public static void init() {
+System.setProperty("zookeeper.authProvider.1",
+
"org.apache.zookeeper.server.auth.SASLAuthenticationProvider");
try {
File tmpDir = createTmpDir();
File saslConfFile = new File(tmpDir, "jaas.conf");
FileWriter fwriter = new FileWriter(saslConfFile);
-fwriter.write("" +
-"Server {\n" +
-"
org.apache.zookeeper.server.auth.DigestLoginModule required\n" +
-" user_super=\"test\";\n" +
-"};\n" +
-"Client {\n" +
-"
org.apache.zookeeper.server.auth.DigestLoginModule required\n" +
-" username=\"super\"\n" +
-" password=\"test\";\n" +
-"};" + "\n");
+fwriter.write("" + "Server {\n"
--- End diff --
Same content was already present, I tried to format it which could not help
to clean.
Now moved jaas config file content generation to new method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #254: ZOOKEEPER-2775: ZK Client not able to connect with Xid...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/254 This PR can be merged to master and branch-3.5 only. I will raise another pull request for branch-3.4 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #278: ZOOKEEPER-2775: ZK Client not able to connect w...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/278 ZOOKEEPER-2775: ZK Client not able to connect with Xid out of order error Same as PR https://github.com/apache/zookeeper/pull/254 but this is for branch-3.4 You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2775-branch-3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/278.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #278 commit 31580024b18d57eff28acb7099bad0606b51efa3 Author: Mohammad Arshad Date: 2017-06-10T17:36:34Z ZOOKEEPER-2775: ZK Client not able to connect with Xid out of order error --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #278: ZOOKEEPER-2775: ZK Client not able to connect w...
Github user arshadmohammad closed the pull request at: https://github.com/apache/zookeeper/pull/278 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #282: ZOOKEEPER-1782: Let a SASL super user be super
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/282#discussion_r122586367
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java
---
@@ -38,11 +38,6 @@ public String getScheme() {
}
public boolean matches(String id,String aclExpr) {
-if (System.getProperty("zookeeper.superUser") != null) {
-if (id.equals(System.getProperty("zookeeper.superUser")) ||
id.equals(aclExpr)) {
- return true;
-}
-}
if ((id.equals("super") || id.equals(aclExpr))) {
--- End diff --
I think we should remove (id.equals("super") also. Lets have only one way
being super user.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #282: ZOOKEEPER-1782: Let a SASL super user be super
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/282#discussion_r122586398 --- Diff: src/java/test/org/apache/zookeeper/test/SaslSuperUserTest.java --- @@ -0,0 +1,118 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.test; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.TestableZooKeeper; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.Watcher.Event.KeeperState; +import org.apache.zookeeper.ZooDefs.Ids; --- End diff -- The import org.apache.zookeeper.ZooDefs.Ids is never used. There are many other unused imports as well --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #282: ZOOKEEPER-1782: Let a SASL super user be super
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/282#discussion_r122586475
--- Diff: src/java/test/org/apache/zookeeper/test/SaslSuperUserTest.java ---
@@ -0,0 +1,118 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.TestableZooKeeper;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooDefs.Perms;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.server.auth.DigestAuthenticationProvider;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class SaslSuperUserTest extends ClientBase {
+private static Id otherSaslUser = new Id ("sasl", "joe");
+private static Id otherDigestUser;
+
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.SASLAuthenticationProvider");
--- End diff --
Whatever the system properties are set in this class should be cleared at
the end of the class.
Similar test class org.apache.zookeeper.SaslAuthTest could be referred.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #282: ZOOKEEPER-1782: Let a SASL super user be super
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/282#discussion_r123329247
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java
---
@@ -38,11 +38,6 @@ public String getScheme() {
}
public boolean matches(String id,String aclExpr) {
-if (System.getProperty("zookeeper.superUser") != null) {
-if (id.equals(System.getProperty("zookeeper.superUser")) ||
id.equals(aclExpr)) {
- return true;
-}
-}
if ((id.equals("super") || id.equals(aclExpr))) {
--- End diff --
Thanks @revans2 for the details.
On second thought, it would not be appropriate to break the backward
compatibility. So can you please revert changes done for this comment. Rest all
looks good to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #279: ZOOKEEPER-2804:Node creation fails with NPE if ...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/279#discussion_r126481060
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -1436,7 +1436,7 @@ public String create(final String path, byte data[],
List acl,
request.setData(data);
request.setFlags(createMode.toFlag());
request.setPath(serverPath);
-if (acl != null && acl.size() == 0) {
+if (acl == null || acl.isEmpty() || acl.contains(null)) {
--- End diff --
These validations should also be applied on async create APIs
1. org.apache.zookeeper.ZooKeeper.create(String, byte[], List,
CreateMode, Create2Callback, Object, long)
2. org.apache.zookeeper.ZooKeeper.create(String, byte[], List,
CreateMode, StringCallback, Object)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #279: ZOOKEEPER-2804:Node creation fails with NPE if ...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/279#discussion_r126482088
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -1535,7 +1535,7 @@ public String create(final String path, byte data[],
List acl,
RequestHeader h = new RequestHeader();
setCreateHeader(createMode, h);
Create2Response response = new Create2Response();
-if (acl != null && acl.size() == 0) {
+if (acl == null || acl.isEmpty() || acl.contains(null)) {
--- End diff --
As this Validation we are doing multiple places it would be better if this
piece of code is extracted to method. Some thing like
` private void validateACL(List acl) throws InvalidACLException {
if (acl == null || acl.isEmpty() || acl.contains(null)) {
throw new KeeperException.InvalidACLException();
}
}`
and put this method along with other validation methods for example
`
PathUtils.validatePath(clientPath, createMode.isSequential());
EphemeralType.validateTTL(createMode, -1);
validateACL(acl);
`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #279: ZOOKEEPER-2804:Node creation fails with NPE if ...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/279#discussion_r128291070
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -1436,7 +1436,7 @@ public String create(final String path, byte data[],
List acl,
request.setData(data);
request.setFlags(createMode.toFlag());
request.setPath(serverPath);
-if (acl != null && acl.size() == 0) {
+if (acl == null || acl.isEmpty() || acl.contains(null)) {
--- End diff --
Thanks @jainbhupendra24 for the details. We can leave these functions as it
is. It is trivial problem we can ignore over the backward compatibility.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #304: ZOOKEEPER-2355: Ephemeral node is never deleted if fol...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/304 LGTM +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/338 ZOOKEEPER-1260:Audit logging in ZooKeeper servers. Audit logging in ZooKeeper Servers. You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-1260-AuditLog Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/338.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #338 commit 502fbf7b2127cc4cf8284d8ecc181ae47e02b2d6 Author: Mohammad Arshad Date: 2017-07-11T14:42:13Z ZOOKEEPER-1260:Audit logging in ZooKeeper Servers. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r134566109 --- Diff: src/docs/src/documentation/content/xdocs/site.xml --- @@ -52,6 +52,7 @@ See http://forrest.apache.org/docs/linking.html for more info. + --- End diff -- yes it should be
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r134566167 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml --- @@ -931,7 +931,19 @@ server.3=zoo3:2888:3888 feature. Default is "true" - + +audit.enabled + +(Java system property: +zookeeper.audit.enabled) + + +New in 3.5.3: --- End diff -- corrected it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r134566240 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.3. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys --- End diff -- Corrected it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136144283
--- Diff: conf/log4j.properties ---
@@ -63,3 +63,20 @@
log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
### Notice we are including log4j's NDC here (%x)
log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601}
[myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
+#
+# zk audit logging
+#
+zookeeper.auditlog.file=zookeeper_audit.log
+zookeeper.auditlog.threshold=INFO
+audit.logger=INFO, RFAAUDIT
+log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
+log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
+log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
--- End diff --
It is very valid question. By RFAAUDIT i meant Rolling File Audit. Also one
A is extra by mistake. I think it is better to change the name to AUDITFILE.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136146630
--- Diff: conf/log4j.properties ---
@@ -63,3 +63,20 @@
log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
### Notice we are including log4j's NDC here (%x)
log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601}
[myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
+#
+# zk audit logging
+#
+zookeeper.auditlog.file=zookeeper_audit.log
+zookeeper.auditlog.threshold=INFO
+audit.logger=INFO, RFAAUDIT
+log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
+log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
+log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
+log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
+log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
+log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
--- End diff --
Logging is done only from one class, which thread is logging is not
important. so only required parameters are logged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136149088 --- Diff: src/docs/src/documentation/content/xdocs/index.xml --- @@ -66,6 +66,7 @@ Hierarchical quorums Observers - non-voting ensemble members that easily improve ZooKeeper's scalability Dynamic Reconfiguration - a guide on how to use dynamic reconfiguration in ZooKeeper + Audit Logging - a guide on how to configure audit logs in ZooKeeper Server and what contents are logged. --- End diff -- removed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136150114 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml --- @@ -931,7 +931,19 @@ server.3=zoo3:2888:3888 feature. Default is "true" - + +audit.enabled + +(Java system property: +zookeeper.audit.enabled) + + +New in 3.5.4: +By default audit logs are disabled. Set to "true" to enable this feature. Default is "false". See --- End diff -- corrected. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136150155 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit --- End diff -- corrected --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136154472 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. --- End diff -- figure is giving more information. I think it is good to have. May be we can re-frame the sentence, any suggestion? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136154783 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit --- End diff -- corrected --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136155728 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs +refer section + + + + +ip +client IP address --- End diff -- Done --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136157192 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs --- End diff -- In audit log there is information about who has done a zk operation. There are many different authentication mechanisms, some take kerberos principal, some take ip, some take simplly user name as user. Second sentence is directing reader to this detail. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136157257 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs +refer section + + + + +ip +client IP address + + +operation +any one of the selected operations for audit. Possible values are +(serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose) + + + +znode +path of the znode + + +acl +String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged +only for setAcl operation --- End diff -- corrected --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136157705 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs +refer section + + + + +ip +client IP address + + +operation +any one of the selected operations for audit. Possible values are +(serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose) + + + +znode +path of the znode + + +acl +String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged +only for setAcl operation + + +result +result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used --- End diff -- you are right. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136158827 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs +refer section + + + + +ip +client IP address + + +operation +any one of the selected operations for audit. Possible values are +(serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose) + + + +znode +path of the znode + + +acl +String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged +only for setAcl operation + + +result +result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used +for serverStop operation because stop is logged before ensuring that server actually stopped. + + + + + +Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is +zk...@hadoop.com, server principal is zookeeper/192.168@hadoop.com + +user=zookeeper/192.168.1.3 operation=serverStart result=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=success +session=0x1934473 user=192.168.1.2,zk...@h
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136158981 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs +refer section + + + + +ip +client IP address + + +operation +any one of the selected operations for audit. Possible values are +(serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose) + + + +znode +path of the znode + + +acl +String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged +only for setAcl operation + + +result +result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used +for serverStop operation because stop is logged before ensuring that server actually stopped. + + + + + +Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is +zk...@hadoop.com, server principal is zookeeper/192.168@hadoop.com + +user=zookeeper/192.168.1.3 operation=serverStart result=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=success +session=0x1934473 user=192.168.1.2,zk...@h
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136161116
--- Diff: src/java/main/org/apache/zookeeper/audit/ZKAuditLogger.java ---
@@ -0,0 +1,118 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.audit;
+
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZKAuditLogger {
+public static final String SYSPROP_AUDIT_ENABLED =
"zookeeper.audit.enabled";
+private static final Logger LOG =
LoggerFactory.getLogger(ZKAuditLogger.class);
+// By default audit logging is disabled
+public static final boolean isAuditEnabled =
Boolean.getBoolean(SYSPROP_AUDIT_ENABLED);
+public static final boolean isAuditDisabled = !isAuditEnabled;
--- End diff --
not a problem, we can use isAuditEnabled itself, removed isAuditDisabled
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136162453
--- Diff:
src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
@@ -465,6 +490,129 @@ public void processRequest(Request request) {
}
}
+private void addSuccessAudit(Request request,ServerCnxn cnxn, String
op, String path) {
+addSuccessAudit(request, cnxn, op, path, null);
+}
+
+private void addSuccessAudit(Request request,ServerCnxn cnxn, String
op, String path, String acl) {
+if (ZKAuditLogger.isAuditDisabled) {
+return;
+}
+ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl,
+getSessionId(cnxn), getHostAddress(cnxn));
+}
+
+private void addFailureAudit(Request request,ServerCnxn cnxn, String
op, String path) {
+addFailureAudit(request, cnxn, op, path, null);
+}
+
+private void addFailureAudit(Request request,ServerCnxn cnxn, String
op, String path, String acl) {
+if (ZKAuditLogger.isAuditDisabled) {
+return;
+}
+ZKAuditLogger.logFailure(request.getUsers(), op, path, acl,
+getSessionId(cnxn), getHostAddress(cnxn));
+}
+
+private void addAuditLog(Request request, ServerCnxn cnxn, String op,
String path, String acl,
+Code err) {
+if (ZKAuditLogger.isAuditDisabled) {
+return;
+}
+if (err == Code.OK) {
+ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl,
getSessionId(cnxn),
+getHostAddress(cnxn));
+} else {
+ZKAuditLogger.logFailure(request.getUsers(), op, path, acl,
getSessionId(cnxn),
+getHostAddress(cnxn));
+}
+}
+
+private String getACLs(Request request)
+{
+ByteBuffer reqData = request.request.duplicate();
+reqData.rewind();
+SetACLRequest setACLRequest = new SetACLRequest();
+try {
+ByteBufferInputStream.byteBuffer2Record(reqData,
setACLRequest);
+} catch (IOException e) {
+e.printStackTrace();
+}
+return ZKUtil.aclToString(setACLRequest.getAcl());
+}
+
+private void addFailedTxnAduitLog(Request request) {
+if (ZKAuditLogger.isAuditDisabled) {
+return;
+}
+String op = AuditConstants.OP_CREATE;
+if (request.cnxn == null) {
+return;
+}
+String path=null;
+long sessionId = -1;
+String address = null;
+String acls = null;
+boolean exceptionOccured = false;
+ByteBuffer reqData = request.request.duplicate();
+reqData.rewind();
+try {
+sessionId = request.cnxn.getSessionId();
+switch (request.type) {
+case OpCode.create:
+case OpCode.create2:
+case OpCode.createContainer:
+op = AuditConstants.OP_CREATE;
+CreateRequest createRequest = new CreateRequest();
+ByteBufferInputStream.byteBuffer2Record(reqData,
createRequest);
+path=createRequest.getPath();
+break;
+case OpCode.delete:
+case OpCode.deleteContainer:
+op = AuditConstants.OP_DELETE;
+//path = new String(request.request.array());
+DeleteRequest deleteRequest = new DeleteRequest();
+ByteBufferInputStream.byteBuffer2Record(reqData,
deleteRequest);
+path=deleteRequest.getPath();
+break;
+case OpCode.setData:
+op = AuditConstants.OP_SETDATA;
+SetDataRequest setDataRequest = new SetDataRequest();
+ByteBufferInputStream.byteBuffer2Record(reqData,
setDataRequest);
+path=setDataRequest.getPath();
+break;
+case OpCode.setACL:
+op = AuditConstants.OP_SETACL;
+SetACLRequest setACLRequest = new SetACLRequest();
+ByteBufferInputStream.byteBuffer2Record(reqData,
setACLRequest);
+path=setACLRequest.getPath();
+acls = ZKUtil.aclToString(setACLRequest.getAcl());
+break;
+case OpCode.multi:
+op = AuditConstants.OP_MULTI_OP;
+break;
+case OpCode.reconfig:
+op = AuditConstants.OP_RECONFIG;
+break
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136166929
--- Diff:
src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
@@ -476,6 +624,33 @@ private boolean connClosedByClient(Request request) {
return request.cnxn == null;
}
+private String getHostAddress(ServerCnxn cnxn) {
--- End diff --
moved
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136166969
--- Diff:
src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
@@ -476,6 +624,33 @@ private boolean connClosedByClient(Request request) {
return request.cnxn == null;
}
+private String getHostAddress(ServerCnxn cnxn) {
+try {
+if (cnxn == null) {
+return "";
+}
+InetSocketAddress remoteSocketAddress = cnxn
+.getRemoteSocketAddress();
+if (remoteSocketAddress == null) {
+return "";
+}
+InetAddress address = remoteSocketAddress.getAddress();
+if (address == null) {
+return "";
+}
+return address.getHostAddress();
+} catch (Exception e) {
+// ignore
+}
+return "";
+}
+
+private String getSessionId(ServerCnxn cnxn) {
--- End diff --
moved to ServerCnxn
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136168432
--- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
---
@@ -54,6 +54,7 @@
*/
static final ByteBuffer closeConn = ByteBuffer.allocate(0);
+private static String loginUser = System.getProperty("user.name",
"");
--- End diff --
loginUser is the user who started zk server.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136169721
--- Diff: src/java/test/org/apache/zookeeper/server/util/AuthUtilTest.java
---
@@ -0,0 +1,58 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.server.util;
+
+import static org.junit.Assert.*;
+
+import org.apache.zookeeper.data.Id;
+import org.junit.Test;
+
+public class AuthUtilTest {
+
+@Test
+public void testGetUserFromAllAuthenticationScheme() {
+System.setProperty("zookeeper.authProvider.sasl",
--- End diff --
only digest authentication is different, rest all are same. That is why not
needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136172475
--- Diff: src/java/test/org/apache/zookeeper/audit/ZKAuditLoggerTest.java
---
@@ -0,0 +1,377 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.audit;
+
+import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.LineNumberReader;
+import java.io.StringReader;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.log4j.Layout;
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.apache.log4j.SimpleLayout;
+import org.apache.log4j.WriterAppender;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
+import org.apache.zookeeper.Op;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.ZKUtil;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.server.Request;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.quorum.QuorumPeerTestBase.MainThread;
+import org.apache.zookeeper.test.ClientBase;
+import org.apache.zookeeper.test.ClientBase.CountdownWatcher;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ZKAuditLoggerTest extends ZKTestCase {
+private static final Logger LOG =
Logger.getLogger(ZKAuditLoggerTest.class);
+private static int SERVER_COUNT = 3;
+private MainThread[] mt;
+private ZooKeeper zk;
+
+@Test
+public void testAuditLogs() throws Exception {
--- End diff --
Authentication provider make difference only for user attribute, rest all
audit log attributes will remain same.
Test is already running on 3 node cluster. It is validating number of audit
entries but in total. Not on which server. In all the cases expected number of
entry is either 1 or three. In case of 1 entry test case currently not checking
on which specific server entry is expected. But I think it is fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136173824 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs +refer section + + + + +ip +client IP address + + +operation +any one of the selected operations for audit. Possible values are +(serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose) + + + +znode +path of the znode + + +acl +String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged +only for setAcl operation + + +result +result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used +for serverStop operation because stop is logged before ensuring that server actually stopped. + + + + + +Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is +zk...@hadoop.com, server principal is zookeeper/192.168@hadoop.com + +user=zookeeper/192.168.1.3 operation=serverStart result=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=success +session=0x1934473 user=192.168.1.2,zk...@h
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136175120 --- Diff: src/java/test/org/apache/zookeeper/server/util/AuthUtilTest.java --- @@ -0,0 +1,58 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.zookeeper.server.util; + +import static org.junit.Assert.*; --- End diff -- Done --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136176181 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. + + + + + +The audit log captures the detailed information for the operations that are selected to be audited. The audit +information is written as a set of key=value pairs for the following keys. + +Audit Log Content + + + +Key +Value + + + + +session +client session id + + +user + +comma separated list of users who are associate with a client session. To know who is taken as user in audit logs +refer section + + + + +ip +client IP address + + +operation +any one of the selected operations for audit. Possible values are +(serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose) + + + +znode +path of the znode + + +acl +String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged +only for setAcl operation + + +result +result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used +for serverStop operation because stop is logged before ensuring that server actually stopped. + + + + + +Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is +zk...@hadoop.com, server principal is zookeeper/192.168@hadoop.com + +user=zookeeper/192.168.1.3 operation=serverStart result=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=createznode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setData znode=/aresult=success +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=failure +session=0x1934473 user=192.168.1.2,zk...@hadoop.com ip=192.168.1.2operation=setAclznode=/aacl=world:anyone:cdrwa result=success +session=0x1934473 user=192.168.1.2,zk...@h
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136865485
--- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
---
@@ -54,6 +54,7 @@
*/
static final ByteBuffer closeConn = ByteBuffer.allocate(0);
+private static String loginUser = System.getProperty("user.name",
"");
--- End diff --
changed to serverUser
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r136865979
--- Diff: src/java/main/org/apache/zookeeper/audit/AuditConstants.java ---
@@ -0,0 +1,37 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.audit;
+
+public class AuditConstants {
+public static final String SUCCESS = "success";
+public static final String FAILURE = "failure";
+// operation is performed, result is not known yet
+public static final String INVOKED = "invoked";
+public static final String KEY_VAL_SEPARATOR = "=";
+public static final char PAIR_SEPARATOR = '\t';
+
+public static final String OP_START = "serverStart";
--- End diff --
- Operation names are something internal to ZooKeeper server. Shall we
expose this end user through API? I am not sure. I have tried to segregate
operation names though a different class. Please have a look.
- org.apache.zookeeper.ZooDefs.opNames is exposing operation names, this
operation name list is not complete. Also it has operation names which do not
exist like getMaxChildren and setMaxChildren. I have no idea why operation name
should be exposed and how it is used. Any idea on this ?
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/338#discussion_r136866016 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml --- @@ -0,0 +1,205 @@ + + +http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd";> + + ZooKeeper Audit Logging + + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. You may + obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0";>http://www.apache.org/licenses/LICENSE-2.0. + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing permissions + and limitations under the License. + + + +This document contains information about Audit Logs in ZooKeeper. + + + +ZooKeeper Audit Logs +Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit +logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged +only on the servers where client is connected as depicted in bellow figure. --- End diff -- corrected. ---
[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/338 Thanks @hanm @afine for reviewing this feature. I have addressed all the comments, Please have a look. ---
[GitHub] zookeeper pull request #112: ZOOKEEPER-2355:Ephemeral node is never deleted ...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/112 ZOOKEEPER-2355:Ephemeral node is never deleted if follower fails while reading the proposal packet ZOOKEEPER-2355:Ephemeral node is never deleted if follower fails while reading the proposal packet You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2355 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/112.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #112 commit e8b2b59758c755afedfec8d6ab87d3f7ca0a0ffe Author: arshadmohammad Date: 2016-11-21T18:29:33Z Ephemeral node is never deleted if follower fails while reading the proposal packet --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #113: ZOOKEEPER-2517:jute.maxbuffer is ignored
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/113 ZOOKEEPER-2517:jute.maxbuffer is ignored ZOOKEEPER-2517:jute.maxbuffer is ignored You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2517 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/113.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #113 commit d722cacd247cee4352ce4012db92cb604f8fa57b Author: arshadmohammad Date: 2016-11-21T19:09:41Z jute.maxbuffer is ignored --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #119: ZOOKEEPER-2251:Add Client side packet response ...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/119 ZOOKEEPER-2251:Add Client side packet response timeout to avoid infinite wait. Add Client side packet response timeout to avoid infinite wait. You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2251 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/119.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #119 commit b4ac9f9f7f3742ac2bd08e383c9650cbf4962691 Author: arshadmohammad Date: 2016-12-05T18:55:10Z ZOOKEEPER-2251:Add Client side packet response timeout to avoid infinite wait. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request: ZOOKEEPER-2139:Support multiple ZooKeeper ...
Github user arshadmohammad closed the pull request at: https://github.com/apache/zookeeper/pull/59 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #137: ZOOKEEPER-2573: Modify Info.REVISION to adapt g...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/137#discussion_r97247778
--- Diff: src/lastRevision.bat ---
@@ -16,8 +16,9 @@ rem See the License for the specific language governing
permissions and
rem limitations under the License.
rem Find the current revision, store it in a file, for DOS
-svn info | findstr Revision > %1
+git rev-parse HEAD > %1
-For /F "tokens=1,2 delims= " %%a In (%1) Do (
- echo lastRevision=%%b> %1
+For /F "tokens=* delims= " %%a In (%1) Do (
--- End diff --
This is not working. May be you can replace with all the script with
following script:
for /f "delims=" %%i in ('git rev-parse HEAD') do set rev=%%i
echo lastRevision=%rev% > %1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #153: ZOOKEEPER-2671: Fix compilation error in branch...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/153#discussion_r97257715
--- Diff: src/java/test/org/apache/zookeeper/test/ClientBase.java ---
@@ -664,4 +664,32 @@ public static String join(String separator, Object[]
parts) {
}
return sb.toString();
}
+
+public static ZooKeeper createZKClient(String cxnString) throws
Exception {
+return createZKClient(cxnString, CONNECTION_TIMEOUT);
+}
+
+/**
+ * Returns ZooKeeper client after connecting to ZooKeeper Server.
Session
+ * timeout is {@link #CONNECTION_TIMEOUT}
+ *
+ * @param cxnString
+ *connection string in the form of host:port
+ * @param sessionTimeout
+ * @throws IOException
+ * in cases of network failure
+ */
+public static ZooKeeper createZKClient(String cxnString, int
sessionTimeout) throws IOException {
+CountdownWatcher watcher = new CountdownWatcher();
+ZooKeeper zk = new ZooKeeper(cxnString, sessionTimeout, watcher);
+try {
+watcher.waitForConnected(CONNECTION_TIMEOUT);
+} catch (InterruptedException e) {
+Assert.fail("ZooKeeper client can not connect to " +
cxnString);
+}
+catch (TimeoutException e) {
+Assert.fail("ZooKeeper client can not connect to " +
cxnString);
+}
+return zk;
+}
}
--- End diff --
Thanks Rakesh.
Changes LGTM, +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #137: ZOOKEEPER-2573: Modify Info.REVISION to adapt git repo
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/137 I verified on windows. Other than one minor problem the current changes look good to me. commit hash is getting added with one extra space at the end. here is srvr command output --- Zookeeper version: 3.4.10-SNAPSHOT-3271f3d03d75c675ec1e466434eee0834cb9841a , built on 01/23/2017 19:11 GMT Latency min/avg/max: 0/1/31 Received: 52 Sent: 51 Connections: 2 Outstanding: 0 Zxid: 0x1 Mode: standalone Node count: 4 There is space after 3271f3d03d75c675ec1e466434eee0834cb9841a --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #137: ZOOKEEPER-2573: Modify Info.REVISION to adapt git repo
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/137 I think we should handle it in the VerGen. Can we trim and then assign the value to rev variable As: rev=rev.trim() --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #137: ZOOKEEPER-2573: Modify Info.REVISION to adapt git repo
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/137 Thanks @eribeiro for working on this issue. LGTM +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #95: ZOOKEEPER-2622: ZooTrace.logQuorumPacket does nothing
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/95 LGTM, I will commit it. @eribeiro do you have any comment? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #160: ZOOKEEPER-2680:Correct DataNode.getChildren() i...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/160 ZOOKEEPER-2680:Correct DataNode.getChildren() inconsistent behaviour. DataNode.getChildren() API behavior should be changed and it should always return empty set if the node does not have any child. Currently this API returns some times null some times empty set. You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2680-DataNode-getChildren Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/160.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #160 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #160: ZOOKEEPER-2680:Correct DataNode.getChildren() i...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/160#discussion_r98743966
--- Diff: src/java/test/org/apache/zookeeper/server/DataNodeTest.java ---
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.server;
+
+import static org.junit.Assert.*;
+
+import java.util.Set;
+
+import org.junit.Test;
+
+public class DataNodeTest {
+
+@Test
+public void testGetChildrenShouldReturnEmptySetWhenThereAreNoChidren()
{
+// create DataNode and call getChildren
+DataNode dataNode = new DataNode();
+Set children = dataNode.getChildren();
+assertNotNull(children);
+assertEquals(0, children.size());
+
+// add child,remove child and then call getChildren
+String child = "child";
+dataNode.addChild(child);
+dataNode.removeChild(child);
+children = dataNode.getChildren();
+assertNotNull(children);
+assertEquals(0, children.size());
+
+// Returned empty set must not be modifiable
--- End diff --
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #160: ZOOKEEPER-2680:Correct DataNode.getChildren() i...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/160#discussion_r98743925 --- Diff: src/java/test/org/apache/zookeeper/server/DataNodeTest.java --- @@ -0,0 +1,54 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.zookeeper.server; + +import static org.junit.Assert.*; --- End diff -- I think it is OK to have one import instead of multiple imports. Not changing any thing here. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #160: ZOOKEEPER-2680:Correct DataNode.getChildren() inconsis...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/160 1. yes, changes should be applied to branch-3.4 and branch-3.5 also. I will raise merge request for branch-3.4 and branch-3.5 after it is committed to master 2. This is very much needed. Thanks :-). I removed Null check from all references of getChildren and corrected the code as per the need --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #160: ZOOKEEPER-2680:Correct DataNode.getChildren() inconsis...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/160 checking the CI failure. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #161: ZOOKEEPER-2680: Correct DataNode.getChildren() ...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/161 ZOOKEEPER-2680: Correct DataNode.getChildren() inconsistent behaviour. :branch-3.5 You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2680-br-3.5 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/161.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #161 commit 8b9721b69497adf1de9a6a741a44cf11c5d5f986 Author: Mohammad Arshad Date: 2017-02-05T07:27:37Z ZOOKEEPER-2680: Correct DataNode.getChildren() inconsistent behaviour. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #162: ZOOKEEPER-2680: Correct DataNode.getChildren() ...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/162 ZOOKEEPER-2680: Correct DataNode.getChildren() inconsistent behaviour. :branch-3.4 You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2680-br-3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/162.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #162 commit 30c1d1bc89ed142975205ca7d75cd71a077b7f72 Author: Mohammad Arshad Date: 2017-02-05T08:10:03Z ZOOKEEPER-2680: Correct DataNode.getChildren() inconsistent behaviour. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #161: ZOOKEEPER-2680: Correct DataNode.getChildren() inconsi...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/161 CI failure is because of qa-test-pullrequest is not present in branch-3.5 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #162: ZOOKEEPER-2680: Correct DataNode.getChildren() inconsi...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/162 CI failure is because of qa-test-pullrequest is not present in branch-3.4 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #163: ZOOKEEPER-2682:Make it optional to fail build o...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/163 ZOOKEEPER-2682:Make it optional to fail build on test failure You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2682-master-branch-3.5 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/163.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #163 commit 2662f584bd49a68b5917237c0fe3f4eb84d52b39 Author: Mohammad Arshad Date: 2017-02-06T14:56:54Z ZOOKEEPER-2682:Make it optional to fail build on test failure --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #164: ZOOKEEPER-2682: Make it optional to fail build ...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/164 ZOOKEEPER-2682: Make it optional to fail build on test failure You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2682-branch-3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/164.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #164 commit 3ea68e7a99278daf4d7424ce825c6e8315c39198 Author: Mohammad Arshad Date: 2017-02-06T15:13:21Z ZOOKEEPER-2682: Make it optional to fail build on test failure --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #164: ZOOKEEPER-2682: Make it optional to fail build on test...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/164 Closing this PR as it is committed to branch-3.4. Commit id is 3ea68e7a99278daf4d7424ce825c6e8315c39198 Thanks @rakeshr for commiting this PR --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #164: ZOOKEEPER-2682: Make it optional to fail build ...
Github user arshadmohammad closed the pull request at: https://github.com/apache/zookeeper/pull/164 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #170: ZOOKEEPER-2689: Fix Kerberos Authentication rel...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/170#discussion_r100673430 --- Diff: ivy.xml --- @@ -78,30 +78,38 @@ - - - - - - - - - - - - + + + --- End diff -- dependency scope(conf="test->default".) is missing, Because of this when zookeeper is build its lib directory contains lot of extra jars which are not required. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #170: ZOOKEEPER-2689: Fix Kerberos Authentication rel...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/170#discussion_r100673515 --- Diff: src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java --- @@ -18,8 +18,8 @@ package org.apache.zookeeper.server.quorum.auth; -import org.apache.kerby.kerberos.kerb.keytab.Keytab; -import org.apache.kerby.kerberos.kerb.type.base.PrincipalName; +import org.apache.directory.server.kerberos.shared.keytab.Keytab; +import org.apache.directory.server.kerberos.shared.keytab.KeytabEntry; import org.junit.Assert; import org.junit.Test; --- End diff -- import java.util.List; is unused now. delete it. It is not listed here but is part of this class imports --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #170: ZOOKEEPER-2689: Fix Kerberos Authentication related te...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/170 +1, LGTM I will commit this PR today if no more review comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #162: ZOOKEEPER-2680: Correct DataNode.getChildren() ...
Github user arshadmohammad closed the pull request at: https://github.com/apache/zookeeper/pull/162 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #161: ZOOKEEPER-2680: Correct DataNode.getChildren() ...
Github user arshadmohammad closed the pull request at: https://github.com/apache/zookeeper/pull/161 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #170: ZOOKEEPER-2689: Fix Kerberos Authentication related te...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/170 Thanks @eribeiro for the review, Sorry, I saw you comment just now after committing this PR, missed you in the reviewers list. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #175: ZOOKEEPER-2683: RaceConditionTest is flaky
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/175 ZOOKEEPER-2683: RaceConditionTest is flaky RaceConditionTest assertion is wrong, It fails when old leader becomes leader again. You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2683-03 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/175.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #175 commit 797edf7aa9d182df87e91355d6493263e92e11f9 Author: Mohammad Arshad Date: 2017-02-13T19:11:02Z ZOOKEEPER-2683: RaceConditionTest is flaky --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #176: ZOOKEEPER-2687:Deadlock while shutting down the...
GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/176 ZOOKEEPER-2687:Deadlock while shutting down the Leader server. Leader server enters into deadlock while shutting down itself. Shutdown of the leader server is called from the synchronized block which must be called from outside the synchronized block. For detail pls refer ZOOKEEPER-2380 You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2687 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/176.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #176 commit 1e3ed70b281e1afc9adc6a8c8ea72bef5b9c25e8 Author: Mohammad Arshad Date: 2017-02-13T19:48:07Z ZOOKEEPER-2687:Deadlock while shutting down the Leader server. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #176: ZOOKEEPER-2687:Deadlock while shutting down the Leader...
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/176 There is not test case, as the change is very simple and understandable and the test will complicate the overall fix. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/179#discussion_r101818292
--- Diff: src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java ---
@@ -479,7 +479,7 @@ private boolean checkFourLetterWord(final SelectionKey
k, final int len)
// We take advantage of the limited size of the length to look
// for cmds. They are all 4-bytes which fits inside of an int
String cmd = FourLetterCommands.getCmdMapView().get(len);
-if (cmd == null) {
+if (cmd == null ||
!FourLetterCommands.getWhiteListedCmdView().contains(cmd)) {
--- End diff --
if request is for 4lw command, it should be processed in that way only. If
false is returned from here, the request will proceed as the normal request.
This is major issue in the current patch
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/179#discussion_r101818408
--- Diff: src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java ---
@@ -268,7 +268,7 @@ private boolean checkFourLetterWord(final Channel
channel,
// We take advantage of the limited size of the length to look
// for cmds. They are all 4-bytes which fits inside of an int
String cmd = FourLetterCommands.getCmdMapView().get(len);
-if (cmd == null) {
+if (cmd == null ||
!FourLetterCommands.getWhiteListedCmdView().contains(cmd)) {
--- End diff --
same comment as for NIOServerCnxn
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/179#discussion_r101818968 --- Diff: src/java/main/org/apache/zookeeper/server/command/FourLetterCommands.java --- @@ -153,13 +159,50 @@ */ public final static int telnetCloseCmd = 0xfff4fffd; -final static HashMap cmd2String = -new HashMap(); +private static final String ZOOKEEPER_4LW_COMMANDS_WHITELIST = "zookeeper.4lw.commands.whitelist"; + +// A property only used in tests to turn on / off entire set of supported four letter word commands. +private static final String ZOOKEEPER_4LW_TEST = "zookeeper.test.4lw.enabled"; --- End diff -- We should not add new property for test cases, instead use main property for test cases also. may be you can move repetitive test code to utility test class. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/179#discussion_r101819527
--- Diff:
src/java/main/org/apache/zookeeper/server/command/FourLetterCommands.java ---
@@ -153,13 +159,50 @@
*/
public final static int telnetCloseCmd = 0xfff4fffd;
-final static HashMap cmd2String =
-new HashMap();
+private static final String ZOOKEEPER_4LW_COMMANDS_WHITELIST =
"zookeeper.4lw.commands.whitelist";
+
+// A property only used in tests to turn on / off entire set of
supported four letter word commands.
+private static final String ZOOKEEPER_4LW_TEST =
"zookeeper.test.4lw.enabled";
+
+private static final Logger LOG =
LoggerFactory.getLogger(FourLetterCommands.class);
+
+private static final Map cmd2String = new
HashMap();
+
+private static final Set whiteListedCommands = new
HashSet();
public static Map getCmdMapView() {
return Collections.unmodifiableMap(cmd2String);
}
+// ZOOKEEPER-2693: Only allow white listed commands.
+public static Set getWhiteListedCmdView() {
--- End diff --
I think instead of returning all the commands all the time and making
collection object. We can write function isWhiteListedCommand(String command)
and use it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/179#discussion_r101819637
--- Diff:
src/java/main/org/apache/zookeeper/server/command/FourLetterCommands.java ---
@@ -153,13 +159,50 @@
*/
public final static int telnetCloseCmd = 0xfff4fffd;
-final static HashMap cmd2String =
-new HashMap();
+private static final String ZOOKEEPER_4LW_COMMANDS_WHITELIST =
"zookeeper.4lw.commands.whitelist";
+
+// A property only used in tests to turn on / off entire set of
supported four letter word commands.
+private static final String ZOOKEEPER_4LW_TEST =
"zookeeper.test.4lw.enabled";
+
+private static final Logger LOG =
LoggerFactory.getLogger(FourLetterCommands.class);
+
+private static final Map cmd2String = new
HashMap();
+
+private static final Set whiteListedCommands = new
HashSet();
public static Map getCmdMapView() {
return Collections.unmodifiableMap(cmd2String);
}
+// ZOOKEEPER-2693: Only allow white listed commands.
+public static Set getWhiteListedCmdView() {
+if (!whiteListedCommands.isEmpty()) {
+return Collections.unmodifiableSet(whiteListedCommands);
+}
+
+if (System.getProperty(ZOOKEEPER_4LW_TEST,
"false").equals("true")) {
--- End diff --
Should be removed as suggested in previous comment
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/179#discussion_r101821552
--- Diff:
src/java/test/org/apache/zookeeper/server/ZooKeeperServerStartupTest.java ---
@@ -167,6 +167,7 @@ public void
testClientConnectionRequestDuringStartupWithNettyServerCnxn()
*/
@Test(timeout = 3)
public void testFourLetterWords() throws Exception {
+System.setProperty("zookeeper.test.4lw.enabled", "true");
--- End diff --
I is better to use zookeeper.4lw.commands.whitelist. This comment is for
all the test classes where zookeeper.test.4lw.enabled used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/179#discussion_r101824558
--- Diff:
src/java/test/org/apache/zookeeper/test/FourLetterWordsWhiteListTest.java ---
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import java.io.IOException;
+
+
+import org.apache.zookeeper.TestableZooKeeper;
+import org.apache.zookeeper.common.X509Exception.SSLContextException;
+
+import static
org.apache.zookeeper.client.FourLetterWordMain.send4LetterWord;
+
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.Timeout;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FourLetterWordsWhiteListTest extends ClientBase {
--- End diff --
FourLetterWordsWhiteListTest should do testing around the configured value
of zookeeper.4lw.commands.whitelist.
following are some scenairo which can be included
verify whether confiured commands execued properly
verify that the command which is not configured fails
verify that for non-configured commands connection is close
verify default commands executed successfully without any configuration
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #179: ZOOKEEPER-2693: DOS attack on wchp/wchc four le...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/179#discussion_r101824634
--- Diff:
src/java/test/org/apache/zookeeper/test/FourLetterWordsWhiteListTest.java ---
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import java.io.IOException;
+
+
+import org.apache.zookeeper.TestableZooKeeper;
+import org.apache.zookeeper.common.X509Exception.SSLContextException;
+
+import static
org.apache.zookeeper.client.FourLetterWordMain.send4LetterWord;
+
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.Timeout;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FourLetterWordsWhiteListTest extends ClientBase {
+protected static final Logger LOG =
+LoggerFactory.getLogger(FourLetterWordsTest.class);
+
+@Rule
+public Timeout timeout = new Timeout(3);
--- End diff --
The constructor Timeout(int) is deprecated use
org.junit.rules.Timeout.Timeout(long timeout, TimeUnit timeUnit)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
