fapifta commented on code in PR #6263:
URL: https://github.com/apache/hadoop/pull/6263#discussion_r1394197568
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSZKFailoverController.java:
##########
@@ -297,8 +298,8 @@ public List<HAServiceTarget> getAllOtherNodes() {
@Override
protected boolean isSSLEnabled() {
- return conf.getBoolean(
- DFSConfigKeys.ZK_CLIENT_SSL_ENABLED,
- DFSConfigKeys.DEFAULT_ZK_CLIENT_SSL_ENABLED);
+ return conf.getBoolean(CommonConfigurationKeys.ZK_CLIENT_SSL_ENABLED,
+ conf.getBoolean(DFSConfigKeys.ZK_CLIENT_SSL_ENABLED,
+ DFSConfigKeys.DEFAULT_ZK_CLIENT_SSL_ENABLED));
Review Comment:
The idea is to use the `hadoop.zk.ssl.enabled` as a central configuration
option, if that is set to false or true, then that effectively forces all
Hadoop services to use SSL, while if `hadoop.zk.ssl.enabled` is not set, then
different components can still set it on their own without affecting other
components.
I am not sure I understand what do you mean, as I see Yarn, HDFS and Common
components does this the same way in the patch. Do I miss something? Also if
the idea is bad, or goes against other similar things, I am open to adjust the
behaviour of these configs it just feels straightforward this way.
For the keystore and truststore data I think we need it the other way
around, and we need to use the component specific setting first, and fall back
to the service specific setting.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
