anmolnar commented on code in PR #2270: URL: https://github.com/apache/zookeeper/pull/2270#discussion_r2213528034
########## zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md: ########## @@ -1776,6 +1776,16 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp Specifies whether Online Certificate Status Protocol is enabled in client and quorum TLS protocols. Default: false +* *ssl.tcnative.ocsp* and *ssl.quorum.tcnative.ocsp* : Review Comment: No, the reason for introducing TriState settings is to keep setting Java system properties if needed. I don't want to move away from setting them, because they're required and work fine on the server side. Instead, we should distinguish client and server side code wherever it's necessary. > The ZK TLS config code is currently not structured to handle server-only settings well, adding server-only settings could also confuse users. This is not true. You could add server-only code to this [method](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java#L119). `zoo.cfg` config file is for server settings only, while client config is handled by `ZKClientConfig` class. Both sides could work fine with TriState settings and with proper documentation they won't be confusing at all. ``` if (ocspStapling.isTrue()) { if (provider == JDK) { System.setProperty(...); } else { provider.enableOcsp(...); } } ``` > In any case, we agreed to try and keep the individual changes as small as possible, so this is outside the scope of this particular ticket. You're already introducing a new config setting to resolve the problem and I suggest to generalize for wider use cases, not just for OpenSSL. -- 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: notifications-unsubscr...@zookeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
