jfrazee commented on a change in pull request #4216:
URL: https://github.com/apache/nifi/pull/4216#discussion_r413039974



##########
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:
       I think here are the scenarios:
   1. Port is set in connect string in nifi.properties, same port used in state 
server => success
   2. Port is not set in connect string in nifi.properties, same randomly 
chosen port used for it and state server => success
   3. Port is set in connect string in nifi.properties, differently randomly 
chosen port in state server =>  error
   
   So maybe the behavior should be that if the connect string is set in 
nifi.properties, then use that port in the state server, else chose a random 
port and use it for both the client and the state server.
   
   I don't think it'd involve updating the nifi props so much as checking to 
see if they're defined.
   
   That said, I think this (scenario 2) is only possible when running the 
embedded server with a single node since you'd really have to have a connect 
string set for a cluster.




----------------------------------------------------------------
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:
[email protected]


Reply via email to