jfrazee commented on a change in pull request #4216: NIFI-7356 Enable TLS for embedded Zookeeper when NiFi has TLS enabled URL: https://github.com/apache/nifi/pull/4216#discussion_r410224609
########## File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/state/server/ZooKeeperStateServer.java ########## @@ -198,6 +216,64 @@ public static ZooKeeperStateServer create(final NiFiProperties properties) throw zkProperties.load(bis); } - return new ZooKeeperStateServer(zkProperties); + return new ZooKeeperStateServer(reconcileProperties(properties, zkProperties)); + } + + private static QuorumPeerConfig reconcileProperties(NiFiProperties niFiProperties, Properties zkProperties) throws IOException, ConfigException { + QuorumPeerConfig peerConfig = new QuorumPeerConfig(); + peerConfig.parseProperties(zkProperties); + + // If this is an insecure NiFi or if the ZooKeeper is distributed, no changes are needed: + if (!niFiProperties.isHTTPSConfigured() || peerConfig.isDistributed()) { + logger.info("NiFi properties not mapped to ZooKeeper properties because NiFi is insecure or ZooKeeper is distributed or both."); + return peerConfig; + } + + // Remove HTTP client ports and addresses and warn if set, see NIFI-7203: + InetSocketAddress clientPort = peerConfig.getClientPortAddress(); + if (clientPort != null) { + zkProperties.remove("clientPort"); + zkProperties.remove("clientPortAddress"); + logger.warn("Invalid configuration detected: secure NiFi with embedded ZooKeeper configured for unsecured HTTP connections."); + logger.warn("Removed HTTP port from embedded ZooKeeper configuration to deactivate insecure HTTP connections."); + } + + // Disallow partial TLS configurations for ZK, it's either all or nothing to avoid inconsistent setups, see NIFI-7203: + final Set<String> zkPropKeys = ZOOKEEPER_TO_NIFI_PROPERTIES.keySet(); + final int zkConfiguredPropCount = zkPropKeys.stream().mapToInt(key -> zkProperties.containsKey(key) ? 1 : 0).sum(); + if (zkConfiguredPropCount != 0 && zkConfiguredPropCount != zkPropKeys.size()) { + throw new ConfigException("Embedded ZooKeeper configuration incomplete. Either all TLS properties must be set or none must be set to avoid inconsistent or partial configurations."); + } + + // The secure port/address was not checked above, so add one now if missing: + if (peerConfig.getSecureClientPortAddress() == null) { + final String port = String.valueOf(SocketUtils.findAvailableTcpPort(MIN_AVAILABLE_PORT)); Review comment: Eventually (once we get the election manager, client-side of this merged) this will have to agree with the ZooKeeper connect string in nifi.properties. So, picking a random port isn't going to work unless we also rewrite the port specified in the connect string. Is this just a placeholder, or do you think we should rewrite the port in the scenario where we automatically enabled ZK TLS? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services