[LIVY-1042] Add SSL/TLS support for ZooKeeper connection#523
Conversation
roczei
changed the title
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. 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 Co-authored-by: Asif Khatri <123077165+askhatri@users.noreply.github.com>
|
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! |
|
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) |
|
@ArnavBalyan Thanks a lot for looking into this PR!
What is this comment? Unfortunately, I can't find it. |
| } | ||
|
|
||
| private[recovery] def createZKClientConfig = { | ||
| val clientConfig = new ZKClientConfig |
There was a problem hiding this 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
There was a problem hiding this comment.
Thanks Arnav for the review! I have implemented your suggestion and updated the code.
Oops missed submitting |
When livy.server.zk.client.secure=true, the following are required: livy.keystore, livy.server.zk.ssl.keyStore.password livy.server.zk.ssl.truststore.location, livy.server.zk.ssl.truststore.password.
|
Thank you @roczei! |
2 participants
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:
How was this patch tested?
Test improvements:
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