ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-896

Improve C client to support dynamic authentication schemes

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 3.3.2
    • Fix Version/s: 3.5.0
    • Component/s: c client
    • Labels:
      None

      Description

      When we started exploring zookeeper for our requirements we found the authentication mechanism is not flexible enough.
      We want to use kerberos for authentication but using the current API we ran into a few problems. The idea is that we get a kerberos token on the client side and than send that token to the server with a kerberos scheme. A server side authentication plugin can use that token to authenticate the client and also use the token for authorization.
      We ran into two problems with this approach:
      1. A different kerberos token is needed for each different server that client can connect to since kerberos uses mutual authentication. That means when the client acquires this kerberos token it has to know which server it connects to and generate the token according to that. The client currently can't generate a token for a specific server. The token stored in the auth_info is used for all the servers.
      2. The kerberos token might have an expiry time so if the client loses the connection to the server and than it tries to reconnect it should acquire a new token. That is not possible currently since the token is stored in auth_info and reused for every connection.

      The problem can be solved if we allow the client to register a callback for authentication instead a static token. This can be a callback with an argument which passes the current host string. The zookeeper client code could call this callback before it sends the authentication info to the server to get a fresh server specific token.

      This would solve our problem with the kerberos authentication and also could be used for other more dynamic authentication schemes.

      The solution could be generalization also for the java client as well.

      1. ZOOKEEPER-896.patch
        8 kB
        Botond Hejj
      2. ZOOKEEPER-896.patch
        8 kB
        Botond Hejj
      3. NIOServerCnxn.patch
        8 kB
        Eugene Koontz
      4. ZOOKEEPER-896.patch
        8 kB
        Mahadev konar

        Issue Links

          Activity

          Hide
          Botond Hejj added a comment -

          Change to the c client which allows registration of callback for authentication token generation before auth_info sending.

          Show
          Botond Hejj added a comment - Change to the c client which allows registration of callback for authentication token generation before auth_info sending.
          Hide
          Patrick Hunt added a comment -

          Ben/Mahadev any comments on this approach?

          Botond, can you add a test for this?

          Show
          Patrick Hunt added a comment - Ben/Mahadev any comments on this approach? Botond, can you add a test for this?
          Hide
          Botond Hejj added a comment -

          added unit test
          fixed initialization bug

          Show
          Botond Hejj added a comment - added unit test fixed initialization bug
          Hide
          Patrick Hunt added a comment -

          Hi Bontond, if this is ready to go (you think it's ready for review/commit) please click the "submit patch" link on the left hand side of this page. That will trigger the necessary workflow. thanks!

          Show
          Patrick Hunt added a comment - Hi Bontond, if this is ready to go (you think it's ready for review/commit) please click the "submit patch" link on the left hand side of this page. That will trigger the necessary workflow. thanks!
          Hide
          Mahadev konar added a comment -

          this is interesting. Botond, can you explain you kerberos setup? Who generates the kerberos tokens? I am very interested in plugging in kerberos with zookeeper.

          Show
          Mahadev konar added a comment - this is interesting. Botond, can you explain you kerberos setup? Who generates the kerberos tokens? I am very interested in plugging in kerberos with zookeeper.
          Hide
          Eugene Koontz added a comment -

          Hi Botond, I am planning on developing a Java version of your patch; are you working on this?

          Show
          Eugene Koontz added a comment - Hi Botond, I am planning on developing a Java version of your patch; are you working on this?
          Hide
          Patrick Hunt added a comment -

          The security docs (both client side and server (plugin arch is totally undoc'd)) is sorely in need of improvement. If you are in the area and would like to help out adding docs would be huge. Thanks.

          Show
          Patrick Hunt added a comment - The security docs (both client side and server (plugin arch is totally undoc'd)) is sorely in need of improvement. If you are in the area and would like to help out adding docs would be huge. Thanks.
          Hide
          Eugene Koontz added a comment -

          Hi Patrick, I would love to help with the documentation and have some notes already for myself that I will use as source material.

          Show
          Eugene Koontz added a comment - Hi Patrick, I would love to help with the documentation and have some notes already for myself that I will use as source material.
          Hide
          Botond Hejj added a comment -

          updated the callback in the patch to pass context

          Show
          Botond Hejj added a comment - updated the callback in the patch to pass context
          Hide
          Botond Hejj added a comment -

          the new patch is for 3.3.2

          Show
          Botond Hejj added a comment - the new patch is for 3.3.2
          Hide
          Patrick Hunt added a comment -

          3.3.2 is out of the gate (released already). Also, as this is an improvement rather than a bug fix it will go into the trunk - 3.4.0. You should be creating your diffs based on trunk code. Thanks!

          Show
          Patrick Hunt added a comment - 3.3.2 is out of the gate (released already). Also, as this is an improvement rather than a bug fix it will go into the trunk - 3.4.0. You should be creating your diffs based on trunk code. Thanks!
          Hide
          Botond Hejj added a comment -

          Sorry I was not aware that patches are made against the trunk. It made more sense to make against the last release because that is final.
          How do you handle situation when the patch is made against an older version of trunk than you have checked out?
          I will update the patch as soon as I can.

          Show
          Botond Hejj added a comment - Sorry I was not aware that patches are made against the trunk. It made more sense to make against the last release because that is final. How do you handle situation when the patch is made against an older version of trunk than you have checked out? I will update the patch as soon as I can.
          Hide
          Botond Hejj added a comment -

          Short description of the Zookeeper kerberos architecture we made for Mahadev Konar

          I will describe our architecture using the workflow of client authentication

          1. Client inits a connection

          2. Client adds a kerberos authentication info using the new function I've added
          zoo_add_auth_cb(zoohandle, "kerberos", authcallback, authcontext, completioncallback, data)

          where authcallback is a function which is able to get an authentication token for the connection.
          The signature of that callback is the following: typedef struct buffer (get_auth_cert_t)(const char *hostname, const void ctx);
          This means it returns the token in the zookeeper buffer structure.
          The callback function gets the hostname where the client will actually connect to and the context which were originally
          passed to zoo_add_auth_cb. The context is not used by the c kerberos code but it might come in handy for other
          protocols and it is needed for our planned python binding.

          3. At this point the auth_info is just stored at the client side. I've made modifications in the client to call the registered callback every time before actually sending the auth_info to the server to ensure the token is not dated. In this callback basically I use the kerberos gss api to initialize a secure context (gss_init_sec_context) based on the host of the server where the client actually connects to. The hostname is required because we do mutual authentication so only the correct host can decode the token that the client sends.

          4. On the server side we made a kerberos authentication plugin using the authentication plugin mechanism of the zookeeper server. That authentication plugin will receive the token and decode the userid of the client from that. Later the plugin can use this userid to make ACL decisions as well. Basically it maintains a map of sessions, users.

          5. To make really use of this authentication the zookeeper nodes should have kerberos ACLs. We actually implemented different algorithms to validate an operation. Basically the plugin have the userid so it can make a decision purely based on that or use this userid and acl of the node to propogate the decision to a service or rule engine or anything else.

          Let me know if you have further questions

          Show
          Botond Hejj added a comment - Short description of the Zookeeper kerberos architecture we made for Mahadev Konar I will describe our architecture using the workflow of client authentication 1. Client inits a connection 2. Client adds a kerberos authentication info using the new function I've added zoo_add_auth_cb(zoohandle, "kerberos", authcallback, authcontext, completioncallback, data) where authcallback is a function which is able to get an authentication token for the connection. The signature of that callback is the following: typedef struct buffer ( get_auth_cert_t)(const char *hostname, const void ctx); This means it returns the token in the zookeeper buffer structure. The callback function gets the hostname where the client will actually connect to and the context which were originally passed to zoo_add_auth_cb. The context is not used by the c kerberos code but it might come in handy for other protocols and it is needed for our planned python binding. 3. At this point the auth_info is just stored at the client side. I've made modifications in the client to call the registered callback every time before actually sending the auth_info to the server to ensure the token is not dated. In this callback basically I use the kerberos gss api to initialize a secure context (gss_init_sec_context) based on the host of the server where the client actually connects to. The hostname is required because we do mutual authentication so only the correct host can decode the token that the client sends. 4. On the server side we made a kerberos authentication plugin using the authentication plugin mechanism of the zookeeper server. That authentication plugin will receive the token and decode the userid of the client from that. Later the plugin can use this userid to make ACL decisions as well. Basically it maintains a map of sessions, users. 5. To make really use of this authentication the zookeeper nodes should have kerberos ACLs. We actually implemented different algorithms to validate an operation. Basically the plugin have the userid so it can make a decision purely based on that or use this userid and acl of the node to propogate the decision to a service or rule engine or anything else. Let me know if you have further questions
          Hide
          Botond Hejj added a comment -

          Eugene:

          We patched also the java code but we used a more hacky faster approach so if you could write a clean patch that would be useful for us as well.

          In that approach the user is not able to register a callback but the user can remove the authentication info and readd it if a new connection made. This approach required less change in the zookeeper code but there might be a chance if the authentication packet stays in the queue for long and token expires fast than the authentication fails.

          Show
          Botond Hejj added a comment - Eugene: We patched also the java code but we used a more hacky faster approach so if you could write a clean patch that would be useful for us as well. In that approach the user is not able to register a callback but the user can remove the authentication info and readd it if a new connection made. This approach required less change in the zookeeper code but there might be a chance if the authentication packet stays in the queue for long and token expires fast than the authentication fails.
          Hide
          Patrick Hunt added a comment -

          @Botond

          I was not aware that patches are made against the trunk

          no worries, that's why I mentioned it. Typically if the patch is for the next major/minor (ie trunk based) release you'd create the patch against trunk. Sometimes the code has diverged btw trunk/branch and you end up doing two patches (not the case here though).

          How do you handle situation when the patch is made against an older version of trunk than you have checked out?

          as long at it applies/compiles/tests cleanly against trunk it's usually not an issue (perhaps this case as well) I wanted to mention it though to make your life easier in future (sux to have to re-merge a conflicting patch)

          Show
          Patrick Hunt added a comment - @Botond I was not aware that patches are made against the trunk no worries, that's why I mentioned it. Typically if the patch is for the next major/minor (ie trunk based) release you'd create the patch against trunk. Sometimes the code has diverged btw trunk/branch and you end up doing two patches (not the case here though). How do you handle situation when the patch is made against an older version of trunk than you have checked out? as long at it applies/compiles/tests cleanly against trunk it's usually not an issue (perhaps this case as well) I wanted to mention it though to make your life easier in future (sux to have to re-merge a conflicting patch)
          Hide
          Botond Hejj added a comment -

          Updated the patch as requested. Now trunk was used

          Show
          Botond Hejj added a comment - Updated the patch as requested. Now trunk was used
          Hide
          Eugene Koontz added a comment -

          Hi Botond, Would you be willing to share the patch that you made to the Zookeeper server (written in Java)? It's ok if it's hacky; I would be interested in seeing it and I'm sure others would too.

          Show
          Eugene Koontz added a comment - Hi Botond, Would you be willing to share the patch that you made to the Zookeeper server (written in Java)? It's ok if it's hacky; I would be interested in seeing it and I'm sure others would too.
          Hide
          Eugene Koontz added a comment -

          Please see NIOServerCnxn.patch, a patch to the NIOServerCnxn() constructor that authenticates a client using Kerberos. Contains a lot of hard-wired stuff that needs to be moved to a configuration file.

          Show
          Eugene Koontz added a comment - Please see NIOServerCnxn.patch, a patch to the NIOServerCnxn() constructor that authenticates a client using Kerberos. Contains a lot of hard-wired stuff that needs to be moved to a configuration file.
          Hide
          Botond Hejj added a comment -

          Hi Eugene,

          In the current state I can't commit our java modifications because it have dependencies on some internal libraries. We plan to clean it up later since we would very much like to see it as part of the core release.

          I've checked your server patch and was wondering why don't you use the authentication plugin mechanism of zookeeper to implement kerberos authentication. I like more the approach to only improve the capabilities of the core server/client code to allow the users plugin more advanced authentication mechanism.

          Show
          Botond Hejj added a comment - Hi Eugene, In the current state I can't commit our java modifications because it have dependencies on some internal libraries. We plan to clean it up later since we would very much like to see it as part of the core release. I've checked your server patch and was wondering why don't you use the authentication plugin mechanism of zookeeper to implement kerberos authentication. I like more the approach to only improve the capabilities of the core server/client code to allow the users plugin more advanced authentication mechanism.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12460214/NIOServerCnxn.patch
          against trunk revision 1036967.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          -1 patch. The patch command could not apply the patch.

          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/50//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12460214/NIOServerCnxn.patch against trunk revision 1036967. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/50//console This message is automatically generated.
          Hide
          Mahadev konar added a comment -

          thanks a lot for the explanation botond. It makes sense to me. I will review the patch shortly after the holidays.

          Show
          Mahadev konar added a comment - thanks a lot for the explanation botond. It makes sense to me. I will review the patch shortly after the holidays.
          Hide
          Eugene Koontz added a comment -

          Hi Botond,

          I agree with you completely; I am planning on working to integrate Kerberos within the authentication plugin architecture on the server side over on https://issues.apache.org/jira/browse/ZOOKEEPER-938.

          Show
          Eugene Koontz added a comment - Hi Botond, I agree with you completely; I am planning on working to integrate Kerberos within the authentication plugin architecture on the server side over on https://issues.apache.org/jira/browse/ZOOKEEPER-938 .
          Hide
          Mahadev konar added a comment -

          attaching botond's patch again to make it run via hudson. Hudson pulls the latest patch from the attachments and is pulling eugene's java patch.

          Show
          Mahadev konar added a comment - attaching botond's patch again to make it run via hudson. Hudson pulls the latest patch from the attachments and is pulling eugene's java patch.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch
          against trunk revision 1049401.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 9 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/69//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/69//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/69//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch against trunk revision 1049401. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 9 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/69//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/69//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/69//console This message is automatically generated.
          Hide
          Mahadev konar added a comment -

          botond/eugene,
          Sorry I have been out for a while and could not rebview the patch. The patch looks good to me. A question for both of you, shouldnt the java client require similar changes?

          Show
          Mahadev konar added a comment - botond/eugene, Sorry I have been out for a while and could not rebview the patch. The patch looks good to me. A question for both of you, shouldnt the java client require similar changes?
          Hide
          Eugene Koontz added a comment -

          Hi Mahadev,
          Good point and thanks for pointing it out. I am adding a comment to ZOOKEEPER-938 regarding the need for the two requirements that Botond mentioned above (server hostname context sensitivity) and ticket expiry.

          Show
          Eugene Koontz added a comment - Hi Mahadev, Good point and thanks for pointing it out. I am adding a comment to ZOOKEEPER-938 regarding the need for the two requirements that Botond mentioned above (server hostname context sensitivity) and ticket expiry.
          Hide
          Botond Hejj added a comment -

          Hi Mahadev,

          Yes it would be very good to have the same in java (and all the other languages).
          We plan to do it if we have time and commit the patch back but we don't have a timeline for it so it would be good if anybody else can patch java code as well.

          Show
          Botond Hejj added a comment - Hi Mahadev, Yes it would be very good to have the same in java (and all the other languages). We plan to do it if we have time and commit the patch back but we don't have a timeline for it so it would be good if anybody else can patch java code as well.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch
          against trunk revision 1055924.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 9 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/93//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/93//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/93//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch against trunk revision 1055924. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 9 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/93//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/93//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/93//console This message is automatically generated.
          Hide
          Michi Mutsuzaki (Inactive) added a comment -

          I'll try making the change for Java client.

          --Michi

          Show
          Michi Mutsuzaki (Inactive) added a comment - I'll try making the change for Java client. --Michi
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch
          against trunk revision 1072085.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/148//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/148//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/148//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch against trunk revision 1072085. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/148//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/148//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/148//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch
          against trunk revision 1072085.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/166//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/166//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/166//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch against trunk revision 1072085. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/166//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/166//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/166//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch
          against trunk revision 1089617.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/224//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/224//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/224//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12466338/ZOOKEEPER-896.patch against trunk revision 1089617. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/224//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/224//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/224//console This message is automatically generated.
          Hide
          Mahadev konar added a comment -

          cancelling this patch for now. If we can add a java api as well as part of this patch itll be great. I am a little hesitant in just having this api in the c part and not in the java client (though adding to the java one should be easy). Any one want to take this up?

          Show
          Mahadev konar added a comment - cancelling this patch for now. If we can add a java api as well as part of this patch itll be great. I am a little hesitant in just having this api in the c part and not in the java client (though adding to the java one should be easy). Any one want to take this up?

            People

            • Assignee:
              Botond Hejj
              Reporter:
              Botond Hejj
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development