You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@livy.apache.org by "roczei (via GitHub)" <gi...@apache.org> on 2026/05/27 13:30:08 UTC

[PR] LIVY-1042: Add SSL/TLS support for ZooKeeper connection [incubator-livy]

roczei opened a new pull request, #523:
URL: https://github.com/apache/incubator-livy/pull/523

   ## What changes were proposed in this pull request?
   
   Introduced an optional TLS encryption for the Curator based ZooKeeper client used during session recovery.
   
   Changes by Asif Khatri:
   
   - Added five LivyConf entries (LIVY_ZK_CLIENT_SECURE, LIVY_ZK_CLIENT_SOCKET, LIVY_ZK_KEYSTORE_PASS, LIVY_ZK_TRUSTSTORE_FILE, LIVY_ZK_TRUSTSTORE_PASS) and document them in livy.conf.template.
   - Added ZooKeeperManager.createZKClientConfig that assembles a ZKClientConfig from keystore/truststore settings; applied to the CuratorFramework builder only when livy.server.zk.client.secure=true.
   
   ## How was this patch tested?
   
   Test improvements:
   
   - Extracted mockCurator() helper in ZooKeeperStateStoreSpec to eliminate repeated CuratorFramework mock setup boilerplate.
   - Added an "SSL config" describe block with withSslMock helper (SslTestFixture) and tests covering SSL property mapping and both the secure and non-secure construction paths.
   
   ## Was this patch authored or co-authored using generative AI tooling?
   
   Yes, this was co-authored using Cursor to help generate the new test cases.
   
   Generated-by: Cursor 3.5.17 (claude-4.6-sonnet-medium-thinking)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [livy]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.
ArnavBalyan commented on PR #523:
URL: https://github.com/apache/livy/pull/523#issuecomment-4673504467

   Thank you for the change @roczei 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [incubator-livy]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.
ArnavBalyan commented on PR #523:
URL: https://github.com/apache/incubator-livy/pull/523#issuecomment-4646618459

   > @ArnavBalyan Thanks a lot for looking into this PR!
   > 
   > > (1 minor comment)
   > 
   > What is this comment? Unfortunately, I can't find it.
   
   Opps missed submitting


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [incubator-livy]

Posted by "roczei (via GitHub)" <gi...@apache.org>.
roczei commented on PR #523:
URL: https://github.com/apache/incubator-livy/pull/523#issuecomment-4629523844

   Hi @ArnavBalyan, @askhatri and @gyogal, 
   
   This PR is ready for code review. Could you please take a look at this when you have a moment? Any feedback you provide would be greatly appreciated! Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [incubator-livy]

Posted by "roczei (via GitHub)" <gi...@apache.org>.
roczei commented on PR #523:
URL: https://github.com/apache/incubator-livy/pull/523#issuecomment-4646608542

   @ArnavBalyan Thanks a lot for looking into this PR!
   
   > (1 minor comment)
   
   What is this comment? Unfortunately, I can't find it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [livy]

Posted by "gyogal (via GitHub)" <gi...@apache.org>.
gyogal commented on PR #523:
URL: https://github.com/apache/livy/pull/523#issuecomment-4673606357

   Thank you for the PR and review @roczei and @ArnavBalyan!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [incubator-livy]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.
ArnavBalyan commented on PR #523:
URL: https://github.com/apache/incubator-livy/pull/523#issuecomment-4633271091

   Thanks for the changes, I ran the patch and verified successful TLS connection established from Livy to ZK. The curator session forms and znode operations succeed. CI is green, LGTM (1 minor comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [livy]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.
ArnavBalyan merged PR #523:
URL: https://github.com/apache/livy/pull/523


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [livy]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.
ArnavBalyan commented on PR #523:
URL: https://github.com/apache/livy/pull/523#issuecomment-4662488501

   Thank you @roczei!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [livy]

Posted by "roczei (via GitHub)" <gi...@apache.org>.
roczei commented on code in PR #523:
URL: https://github.com/apache/livy/pull/523#discussion_r3382456851


##########
server/src/main/scala/org/apache/livy/server/recovery/ZooKeeperManager.scala:
##########
@@ -63,6 +64,26 @@ class ZooKeeperManager(
         "Correct format is <max retry count>,<sleep ms between retry>. e.g. 5,100")
   }
 
+  private[recovery] def createZKClientConfig = {
+    val clientConfig = new ZKClientConfig

Review Comment:
   Thanks Arnav for the review! I have implemented your suggestion and updated the code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [LIVY-1042] Add SSL/TLS support for ZooKeeper connection [incubator-livy]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.
ArnavBalyan commented on code in PR #523:
URL: https://github.com/apache/incubator-livy/pull/523#discussion_r3363791888


##########
server/src/main/scala/org/apache/livy/server/recovery/ZooKeeperManager.scala:
##########
@@ -63,6 +64,26 @@ class ZooKeeperManager(
         "Correct format is <max retry count>,<sleep ms between retry>. e.g. 5,100")
   }
 
+  private[recovery] def createZKClientConfig = {
+    val clientConfig = new ZKClientConfig

Review Comment:
   nit: when secure is true, can we strict require() the four SSL fields (truststore location/password, ssl keystore, keystore password) since operator missing out on some configs would cause retry loop to zk connection



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org