ZOOKEEPER-4276. Serving only with secureClientPort fails #2117
Conversation
abhilash1in
force-pushed
the
tls-only-cluster
branch
4 times, most recently
from
e863a9c
to
81c5b52
Compare
abhilash1in
force-pushed
the
tls-only-cluster
branch
2 times, most recently
from
69e70b2
to
d0846b2
Compare
abhilash1in
force-pushed
the
tls-only-cluster
branch
from
d0846b2
to
5f42c74
Compare
| becomes as follows: | ||
|
|
||
| server.<positive id> = <address1>:<port1>:<port2>[:role];[<client port address>:]<client port>** | ||
| server.<positive id> = <address1>:<port1>:<port2>[:role];[[<client port address>:]<client port>][;[<secure client port address>:]<secure client port>] |
There was a problem hiding this comment.
OT - maybe describe what "port1" and "port2" for. Not a blocker for this patch.
| Starting with 3.5.0 the | ||
| _clientPort_ and _clientPortAddress_ configuration parameters should no longer be used in zoo.cfg. | ||
|
|
||
| Starting with 3.9.2 the |
anmolnar
changed the title
| try (X509Util x509Util = new ClientX509Util()) { | ||
| sslContext = x509Util.getDefaultSSLContext(); | ||
| } |
There was a problem hiding this comment.
What's the point of this try block without catch clause?
It's a bit strange to me.
There was a problem hiding this comment.
The "try-with-resources" construct ensures that each resource declared in the try is closed at the end of the statement, which can help prevent resource leaks. In this case, it's ensuring that the X509Util instance is properly closed after use (X509Util implements a Closeable interface). It doesn't necessarily need a catch clause because it's about managing resources rather than catching and handling exceptions.
There was a problem hiding this comment.
Yeah. It's for closing the resource X509Util.
There was a problem hiding this comment.
oh, sure of course.
thanks
|
Merging... Thanks for your contribution @abhilash1in! |
| } | ||
| } | ||
|
|
||
| if (serverClientParts.length == 3 && !serverClientParts[2].isEmpty()) { |
There was a problem hiding this comment.
Should we make this forward compatible to tolerate possible new section from future and backport it to old releases ?
I guess it might be safe in cluster upgrade as we probably don't allow downgrade. But at least, I would expect Curator, which use QuorumMaj(Properties props) to parse ensemble config, will fail here. Not sure other downstream third-party projects.
org.apache.zookeeper.server.quorum.QuorumPeerConfig$ConfigException: 10.1.2.3:2888:3888:participant;10.2.3.4:2181;10.2.3.4:2188 does not have the form server_config or server_config;client_config where server_config is the pipe separated list of host:port:port or host:port:port:type and client_config is port or host:port
at org.apache.zookeeper.server.quorum.QuorumPeer$QuorumServer.initializeWithAddressString(QuorumPeer.java:340)
at org.apache.zookeeper.server.quorum.QuorumPeer$QuorumServer.<init>(QuorumPeer.java:279)
at org.apache.zookeeper.server.quorum.QuorumPeer$QuorumServer.<init>(QuorumPeer.java:274)
at org.apache.zookeeper.server.quorum.flexible.QuorumMaj.<init>(QuorumMaj.java:92)
Or it is third-party libraries' responsibility to accommodate this ?
There was a problem hiding this comment.
This is only available from 3.10.0 which is a major upgrade and as far as I know the code is able to read the old config, so backward compatibility is ok. I guess Curator will support ZK 3.10.0 from a certain release where they'll add the new config.
There was a problem hiding this comment.
Yeh. I think, at the very least, users of Curator could pin their ZooKeeper to 3.10.0 before a Curator release.
* Support TLS-only ZK server * Cleanup * Update documentation according to review comments
This PR addresses ZOOKEEPER-4276, which enables a cluster to run as a TLS-only cluster on the secureClientPort.
Previously, this was not possible due to reasons mentioned in ZOOKEEPER-4276.
In this change, we introduce a new section to
sever.Xentries of dynamic config to indicatesecureClientPort. We also makeclientPortsection of the dynamic config as optional (previously, this was mandatory).For example:
does not have a
clientPortand has 1237 as thesecureClientPort.Dev mailing list thread - https://lists.apache.org/thread/1lmqj2wonldxmk3wxbc5lvj3vsz05l7c