You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/05/01 20:05:01 UTC

[GitHub] [nifi] jfrazee opened a new pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

jfrazee opened a new pull request #4250:
URL: https://github.com/apache/nifi/pull/4250


   #### Description of PR
   
   This adds built-in support for ZooKeeper client TLS to the `CuratorLeaderElectionManager` so clusters can use TLS-enabled ZooKeepers. This is part of NIFI-7203 and is required for the TLS-enabled embedded ZooKeeper work in #4216.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [X] Is there a JIRA ticket associated with this PR? Is it referenced 
        in the commit message?
   
   - [X] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [X] Has your PR been rebased against the latest commit within the target branch (typically `master`)?
   
   - [X] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
   
   ### For code changes:
   - [X] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
   - [X] Have you written or updated unit tests to verify your changes?
   - [X] Have you verified that the full build is successful on both JDK 8 and JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-712293762


   +1 from me, keen to get this merged so I can fix up my PR so we can get it all merged it.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-685873881


   @YolandaMDavis No it wasn't merged I have time to rebase and retest today actually.
   
   I previously had a PR that included support for the embedded server too, but that got split apart because of some additional work and expanded scope.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-693520289


   #4216 was intended to handle the embedded ZK path and is about forcing TLS for both the embedded server and the client whenever the embedded server is used.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r491145537



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/nifi.properties
##########
@@ -236,6 +236,11 @@ nifi.zookeeper.connect.string=${nifi.zookeeper.connect.string}
 nifi.zookeeper.connect.timeout=${nifi.zookeeper.connect.timeout}
 nifi.zookeeper.session.timeout=${nifi.zookeeper.session.timeout}
 nifi.zookeeper.root.node=${nifi.zookeeper.root.node}
+nifi.zookeeper.client.secure=${nifi.zookeeper.client.secure}

Review comment:
       The properties were aligned to the ZK ones, but I think it makes sense to align them to the other security fields. Will do.
   
   It's not necessary to specify the types (it'll do it by file extension), but you can. I'll add those too.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog closed pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog closed pull request #4250:
URL: https://github.com/apache/nifi/pull/4250


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r501273761



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/leader/election/TestSecureClientZooKeeperFactory.java
##########
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.leader.election;
+
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.InstanceSpec;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.properties.StandardNiFiProperties;
+import org.apache.nifi.controller.cluster.ZooKeeperClientConfig;
+import org.apache.nifi.controller.leader.election.CuratorLeaderElectionManager.SecureClientZooKeeperFactory;
+
+import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.testng.Assert;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+
+public class TestSecureClientZooKeeperFactory {
+
+    private static final String NETTY_SERVER_CNXN_FACTORY =
+        "org.apache.zookeeper.server.NettyServerCnxnFactory";
+
+    private static final String CLIENT_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.keystore.p12";
+    private static final String CLIENT_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.truststore.p12";
+    private static final String SERVER_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.keystore.p12";
+    private static final String SERVER_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.truststore.p12";
+    private static final String KEYSTORE_TYPE = "PKCS12";
+    private static final String TEST_PASSWORD = "testpass";
+
+    private static ZooKeeperServer zkServer;
+    private static ServerCnxnFactory serverConnectionFactory;
+    private static Properties serverProperties;
+    private static NiFiProperties clientProperties;
+    private static Path tempDir;
+    private static Path dataDir;
+    private static int clientPort;
+
+    @BeforeClass
+    public static void setup() throws IOException, GeneralSecurityException, InterruptedException {
+        tempDir = Paths.get("target/TestSecureClientZooKeeperFactory");
+        dataDir = tempDir.resolve("state");
+        clientPort = InstanceSpec.getRandomPort();
+
+        Files.createDirectory(tempDir);
+
+        serverProperties = createServerProperties(

Review comment:
       Are the serverProperties ever used?




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r505851651



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/leader/election/TestSecureClientZooKeeperFactory.java
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.leader.election;
+
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.InstanceSpec;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.properties.StandardNiFiProperties;
+import org.apache.nifi.controller.cluster.ZooKeeperClientConfig;
+import org.apache.nifi.controller.leader.election.CuratorLeaderElectionManager.SecureClientZooKeeperFactory;
+
+import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.testng.Assert;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+
+public class TestSecureClientZooKeeperFactory {
+
+    private static final String NETTY_SERVER_CNXN_FACTORY =
+        "org.apache.zookeeper.server.NettyServerCnxnFactory";
+
+    private static final String CLIENT_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.keystore.p12";
+    private static final String CLIENT_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.truststore.p12";
+    private static final String SERVER_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.keystore.p12";
+    private static final String SERVER_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.truststore.p12";

Review comment:
       Decided to revert back to creating the keystrokes in the tests. 




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r505857356



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java
##########
@@ -70,6 +89,38 @@ public String getRootPath() {
         return rootPath;
     }
 
+    public boolean getClientSecure() {

Review comment:
       Done.

##########
File path: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java
##########
@@ -284,6 +291,7 @@
     public static final String DEFAULT_ZOOKEEPER_CONNECT_TIMEOUT = "3 secs";
     public static final String DEFAULT_ZOOKEEPER_SESSION_TIMEOUT = "3 secs";
     public static final String DEFAULT_ZOOKEEPER_ROOT_NODE = "/nifi";
+    public static final Boolean DEFAULT_ZOOKEEPER_CLIENT_SECURE = false;

Review comment:
       Done.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-689024747


   @YolandaMDavis I rebased/retargeted this and ran through the manual testing.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] YolandaMDavis commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
YolandaMDavis commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-685851517


   @jfrazee was doing some research around this issue and found this PR.  Does it need to be targeted to main or was this perhaps merged to main in the context of another PR?


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-695016708


   Right now for this PR I'm thinking we need some consolidation across the TLS + Zookeeper work. It seems like the embedded zookeeper + external zookeeper configuration isn't handled in the most cohesive way, which is fair because they've been developed separately, but it may result in unexpected behavior for users. 
   
   I'd like to get the PR for state management up before we merge this one so that all external-zookeeper interactions are secured, instead of only the cluster election. We may need to break out the SecureClientZooKeeperFactory class into a separate class and put it somewhere so it can be used by both election and state management. 
   
   It also looks like there needs to be more work on the embedded side to fix a few issues Joey raised in his review - I can look at adding some commits there next week.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-694558311


   Tested the cluster leader election over TLS, looks good. I see there's nothing in this PR for state management over TLS. Is there any PRs open that you know of which cover TLS + state management or is that something I should create a ticket for and work on?


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog edited a comment on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog edited a comment on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-695016708


   Right now for this PR I'm thinking we need some consolidation across the TLS + Zookeeper work. It seems like the embedded zookeeper + external zookeeper configuration isn't handled in the most cohesive way, which is fair because they've been developed separately, but it may result in unexpected behavior for users. 
   
   I'd like to get my PR for state management up before we merge this one so that all external-zookeeper interactions are secured, instead of only the cluster election. We may need to break out the SecureClientZooKeeperFactory class into a separate class and put it somewhere so it can be used by both election and state management. 
   
   It also looks like there needs to be more work on the embedded side to fix a few issues Joey raised in his review - I can look at adding some commits there next week.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r501360542



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/leader/election/TestSecureClientZooKeeperFactory.java
##########
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.leader.election;
+
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.InstanceSpec;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.properties.StandardNiFiProperties;
+import org.apache.nifi.controller.cluster.ZooKeeperClientConfig;
+import org.apache.nifi.controller.leader.election.CuratorLeaderElectionManager.SecureClientZooKeeperFactory;
+
+import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.testng.Assert;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+
+public class TestSecureClientZooKeeperFactory {
+
+    private static final String NETTY_SERVER_CNXN_FACTORY =
+        "org.apache.zookeeper.server.NettyServerCnxnFactory";
+
+    private static final String CLIENT_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.keystore.p12";
+    private static final String CLIENT_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.truststore.p12";
+    private static final String SERVER_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.keystore.p12";
+    private static final String SERVER_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.truststore.p12";
+    private static final String KEYSTORE_TYPE = "PKCS12";
+    private static final String TEST_PASSWORD = "testpass";
+
+    private static ZooKeeperServer zkServer;
+    private static ServerCnxnFactory serverConnectionFactory;
+    private static Properties serverProperties;
+    private static NiFiProperties clientProperties;
+    private static Path tempDir;
+    private static Path dataDir;
+    private static int clientPort;
+
+    @BeforeClass
+    public static void setup() throws IOException, GeneralSecurityException, InterruptedException {
+        tempDir = Paths.get("target/TestSecureClientZooKeeperFactory");
+        dataDir = tempDir.resolve("state");
+        clientPort = InstanceSpec.getRandomPort();
+
+        Files.createDirectory(tempDir);
+
+        serverProperties = createServerProperties(

Review comment:
       Thanks. That is indeed dead code from when this included updates to the embedded server. Will remove.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-694981735


   @thenatog No, I don't think anyone has worked on the state management/I don't have any code for that in progress either, so yeah, I think it'd make sense for you to do if you want.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r505857026



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java
##########
@@ -133,6 +207,30 @@ private static int getTimePeriod(final NiFiProperties nifiProperties, final Stri
         }
     }
 
+    private static boolean getClientSecure(final NiFiProperties nifiProperties, final String propertyName, final boolean defaultValue) {

Review comment:
       Done.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] exceptionfactory commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r508111993



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
##########
@@ -589,4 +601,44 @@ public void takeLeadership(final CuratorFramework client) throws Exception {
             }
         }
     }
+
+    public static class SecureClientZooKeeperFactory implements ZookeeperFactory {
+
+        private static final String NETTY_CLIENT_CNXN_SOCKET =
+            "org.apache.zookeeper.ClientCnxnSocketNetty";
+
+        private ZKClientConfig zkSecureClientConfig;
+
+        public SecureClientZooKeeperFactory(final ZooKeeperClientConfig zkConfig) {
+            this.zkSecureClientConfig = new ZKClientConfig();
+
+            // Netty is required for the secure client config.
+            final String cnxnSocket = zkConfig.getConnectionSocket();
+            if (!NETTY_CLIENT_CNXN_SOCKET.equals(cnxnSocket)) {
+                throw new IllegalArgumentException(String.format("connection factory set to '%s', %s required", String.valueOf(cnxnSocket), NETTY_CLIENT_CNXN_SOCKET));
+            }
+            zkSecureClientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, cnxnSocket);
+
+            // This should never happen but won't get checked elsewhere.
+            final boolean clientSecure = zkConfig.getClientSecure();
+            if (!clientSecure) {

Review comment:
       Thanks for the explanation, that makes sense, glad to help move things forward.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r505856782



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java
##########
@@ -133,6 +207,30 @@ private static int getTimePeriod(final NiFiProperties nifiProperties, final Stri
         }
     }
 
+    private static boolean getClientSecure(final NiFiProperties nifiProperties, final String propertyName, final boolean defaultValue) {
+        final String defaultValueStr = String.valueOf(defaultValue);
+
+        String propertyStr = nifiProperties.getProperty(propertyName, defaultValueStr);
+        propertyStr = StringUtils.stripToEmpty(propertyStr);
+        propertyStr = StringUtils.defaultIfBlank(propertyStr, defaultValueStr);
+        propertyStr = StringUtils.lowerCase(propertyStr);
+
+        if (!"true".equalsIgnoreCase(propertyStr) && !"false".equalsIgnoreCase(propertyStr)) {
+            throw new IllegalArgumentException(String.format("%s was '%s', expected true or false", NiFiProperties.ZOOKEEPER_CLIENT_SECURE, propertyStr));
+        }
+
+        return Boolean.parseBoolean(propertyStr);
+    }
+
+    private static String getKeyStoreType(final String keyStore, final NiFiProperties nifiProperties, final String propertyName) {
+        String keyStoreType = StringUtils.stripToNull(nifiProperties.getProperty(propertyName));
+        // ZooKeeper only recognizes the .p12 extension.
+        if (keyStoreType == null && StringUtils.endsWithIgnoreCase(keyStore, ".pkcs12")) {

Review comment:
       Makes sense, removed it. There are some cases such as the tls-toolkit where keystores end up with a `.pkcs12` extension so the goal was to align to that. If it ends up causing any confusion we can revisit whether we need to handle the special case.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] exceptionfactory commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r502724637



##########
File path: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java
##########
@@ -284,6 +291,7 @@
     public static final String DEFAULT_ZOOKEEPER_CONNECT_TIMEOUT = "3 secs";
     public static final String DEFAULT_ZOOKEEPER_SESSION_TIMEOUT = "3 secs";
     public static final String DEFAULT_ZOOKEEPER_ROOT_NODE = "/nifi";
+    public static final Boolean DEFAULT_ZOOKEEPER_CLIENT_SECURE = false;

Review comment:
       This could be a primitive boolean.

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java
##########
@@ -70,6 +89,38 @@ public String getRootPath() {
         return rootPath;
     }
 
+    public boolean getClientSecure() {

Review comment:
       Should this be named isClientSecure() to follow Java Bean conventions?

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
##########
@@ -589,4 +601,44 @@ public void takeLeadership(final CuratorFramework client) throws Exception {
             }
         }
     }
+
+    public static class SecureClientZooKeeperFactory implements ZookeeperFactory {
+
+        private static final String NETTY_CLIENT_CNXN_SOCKET =
+            "org.apache.zookeeper.ClientCnxnSocketNetty";
+
+        private ZKClientConfig zkSecureClientConfig;
+
+        public SecureClientZooKeeperFactory(final ZooKeeperClientConfig zkConfig) {
+            this.zkSecureClientConfig = new ZKClientConfig();
+
+            // Netty is required for the secure client config.
+            final String cnxnSocket = zkConfig.getConnectionSocket();
+            if (!NETTY_CLIENT_CNXN_SOCKET.equals(cnxnSocket)) {

Review comment:
       If the secure client requires the ClientCnxnSocketNetty, it seems more straightforward to set the correct value in zkSecureClientConfig and ignore the ZooKeeperClientConfig.getConnectionSocket() altogether, instead of confirming that it matches the only correct value in this scenario.

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
##########
@@ -589,4 +601,44 @@ public void takeLeadership(final CuratorFramework client) throws Exception {
             }
         }
     }
+
+    public static class SecureClientZooKeeperFactory implements ZookeeperFactory {
+
+        private static final String NETTY_CLIENT_CNXN_SOCKET =
+            "org.apache.zookeeper.ClientCnxnSocketNetty";
+
+        private ZKClientConfig zkSecureClientConfig;
+
+        public SecureClientZooKeeperFactory(final ZooKeeperClientConfig zkConfig) {
+            this.zkSecureClientConfig = new ZKClientConfig();
+
+            // Netty is required for the secure client config.
+            final String cnxnSocket = zkConfig.getConnectionSocket();
+            if (!NETTY_CLIENT_CNXN_SOCKET.equals(cnxnSocket)) {
+                throw new IllegalArgumentException(String.format("connection factory set to '%s', %s required", String.valueOf(cnxnSocket), NETTY_CLIENT_CNXN_SOCKET));
+            }
+            zkSecureClientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, cnxnSocket);
+
+            // This should never happen but won't get checked elsewhere.
+            final boolean clientSecure = zkConfig.getClientSecure();
+            if (!clientSecure) {

Review comment:
       Similar to the previous comment, why not just set ZKClientConfig.SECURE_CLIENT to true regardless of ZooKeeperClientConfig, particularly since the value is already being checked before instantiating the class?

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java
##########
@@ -133,6 +207,30 @@ private static int getTimePeriod(final NiFiProperties nifiProperties, final Stri
         }
     }
 
+    private static boolean getClientSecure(final NiFiProperties nifiProperties, final String propertyName, final boolean defaultValue) {

Review comment:
       It seems like it might be better to encapsulate this property evaluation logic using a new method on NiFiProperties

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java
##########
@@ -133,6 +207,30 @@ private static int getTimePeriod(final NiFiProperties nifiProperties, final Stri
         }
     }
 
+    private static boolean getClientSecure(final NiFiProperties nifiProperties, final String propertyName, final boolean defaultValue) {
+        final String defaultValueStr = String.valueOf(defaultValue);
+
+        String propertyStr = nifiProperties.getProperty(propertyName, defaultValueStr);
+        propertyStr = StringUtils.stripToEmpty(propertyStr);
+        propertyStr = StringUtils.defaultIfBlank(propertyStr, defaultValueStr);
+        propertyStr = StringUtils.lowerCase(propertyStr);
+
+        if (!"true".equalsIgnoreCase(propertyStr) && !"false".equalsIgnoreCase(propertyStr)) {
+            throw new IllegalArgumentException(String.format("%s was '%s', expected true or false", NiFiProperties.ZOOKEEPER_CLIENT_SECURE, propertyStr));
+        }
+
+        return Boolean.parseBoolean(propertyStr);
+    }
+
+    private static String getKeyStoreType(final String keyStore, final NiFiProperties nifiProperties, final String propertyName) {
+        String keyStoreType = StringUtils.stripToNull(nifiProperties.getProperty(propertyName));
+        // ZooKeeper only recognizes the .p12 extension.
+        if (keyStoreType == null && StringUtils.endsWithIgnoreCase(keyStore, ".pkcs12")) {

Review comment:
       This seems like an unnecessarily special case for assisting in key store type determination.  The pkcs12 file extension does not appear to be a common standard, instead, Wikipedia lists [p12 and pfx ](https://en.wikipedia.org/wiki/PKCS_12) as the common file extensions.  Since the ZooKeeper KeyStoreFileType attempts automatic determination based on file extension, it seems better to rely on that capability and otherwise require the administrator to specify the store type explicitly.

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/cluster/ZooKeeperClientConfig.java
##########
@@ -30,24 +30,43 @@
 
 public class ZooKeeperClientConfig {
 
+    public static final String NETTY_CLIENT_CNXN_SOCKET = "org.apache.zookeeper.ClientCnxnSocketNetty";
+    public static final String NIO_CLIENT_CNXN_SOCKET = "org.apache.zookeeper.ClientCnxnSocketNIO";

Review comment:
       For clarity of reference, would it be better to declare these variables using the actual classes and calling class.getName()?

##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/leader/election/TestSecureClientZooKeeperFactory.java
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.leader.election;
+
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.InstanceSpec;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.properties.StandardNiFiProperties;
+import org.apache.nifi.controller.cluster.ZooKeeperClientConfig;
+import org.apache.nifi.controller.leader.election.CuratorLeaderElectionManager.SecureClientZooKeeperFactory;
+
+import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.testng.Assert;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+
+public class TestSecureClientZooKeeperFactory {
+
+    private static final String NETTY_SERVER_CNXN_FACTORY =
+        "org.apache.zookeeper.server.NettyServerCnxnFactory";
+
+    private static final String CLIENT_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.keystore.p12";
+    private static final String CLIENT_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.truststore.p12";
+    private static final String SERVER_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.keystore.p12";
+    private static final String SERVER_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.truststore.p12";

Review comment:
       Although it would take slightly longer to start the test, did you consider using CertificateUtils to generate self-signed certificates and writing temporary key store and trust store files, as opposed to including additional test PKCS12 binaries?




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-693503784


   Reviewing this one, testing it out now. Also checking out https://github.com/apache/nifi/pull/4216, not sure what the differences are there.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r507950335



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
##########
@@ -589,4 +601,44 @@ public void takeLeadership(final CuratorFramework client) throws Exception {
             }
         }
     }
+
+    public static class SecureClientZooKeeperFactory implements ZookeeperFactory {
+
+        private static final String NETTY_CLIENT_CNXN_SOCKET =
+            "org.apache.zookeeper.ClientCnxnSocketNetty";
+
+        private ZKClientConfig zkSecureClientConfig;
+
+        public SecureClientZooKeeperFactory(final ZooKeeperClientConfig zkConfig) {
+            this.zkSecureClientConfig = new ZKClientConfig();
+
+            // Netty is required for the secure client config.
+            final String cnxnSocket = zkConfig.getConnectionSocket();
+            if (!NETTY_CLIENT_CNXN_SOCKET.equals(cnxnSocket)) {
+                throw new IllegalArgumentException(String.format("connection factory set to '%s', %s required", String.valueOf(cnxnSocket), NETTY_CLIENT_CNXN_SOCKET));
+            }
+            zkSecureClientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, cnxnSocket);
+
+            // This should never happen but won't get checked elsewhere.
+            final boolean clientSecure = zkConfig.getClientSecure();
+            if (!clientSecure) {

Review comment:
       @exceptionfactory This is the one change I didn't make based on your suggestions (thanks for them BTW). While, yes, the secure factory knows what it needs, I don't/didn't want to ignore or change the config the user provides since their intent is either different or confused. I want them to know they're using it wrong.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r490622374



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/nifi.properties
##########
@@ -236,6 +236,11 @@ nifi.zookeeper.connect.string=${nifi.zookeeper.connect.string}
 nifi.zookeeper.connect.timeout=${nifi.zookeeper.connect.timeout}
 nifi.zookeeper.session.timeout=${nifi.zookeeper.session.timeout}
 nifi.zookeeper.root.node=${nifi.zookeeper.root.node}
+nifi.zookeeper.client.secure=${nifi.zookeeper.client.secure}

Review comment:
       We might like to change these property names to align with the other nifi security fields:
   - nifi.zookeeper.ssl.keyStore.location -> nifi.zookeeper.security.keystore
   - nifi.zookeeper.ssl.keyStore.password -> nifi.zookeeper.security.keystorePasswd
   - nifi.zookeeper.ssl.trustStore.location -> nifi.zookeeper.security.truststore
   - nifi.zookeeper.ssl.trustStore.password -> nifi.zookeeper.security.truststorePasswd
   
   Also, is there any need to specify the keystore/truststore type as with the other nifi security properties? We currently support P12, JKS, and maybe JKCS




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on a change in pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#discussion_r502731556



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/leader/election/TestSecureClientZooKeeperFactory.java
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.leader.election;
+
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.InstanceSpec;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.properties.StandardNiFiProperties;
+import org.apache.nifi.controller.cluster.ZooKeeperClientConfig;
+import org.apache.nifi.controller.leader.election.CuratorLeaderElectionManager.SecureClientZooKeeperFactory;
+
+import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.testng.Assert;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+
+public class TestSecureClientZooKeeperFactory {
+
+    private static final String NETTY_SERVER_CNXN_FACTORY =
+        "org.apache.zookeeper.server.NettyServerCnxnFactory";
+
+    private static final String CLIENT_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.keystore.p12";
+    private static final String CLIENT_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/client.truststore.p12";
+    private static final String SERVER_KEYSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.keystore.p12";
+    private static final String SERVER_TRUSTSTORE = "src/test/resources/TestSecureClientZooKeeperFactory/server.truststore.p12";

Review comment:
       An earlier version did this but you have to pull nifi-security-utils in for the tests then.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [nifi] jfrazee commented on pull request #4250: NIFI-7401 Add ZooKeeper client TLS to CuratorLeaderElectionManager

Posted by GitBox <gi...@apache.org>.
jfrazee commented on pull request #4250:
URL: https://github.com/apache/nifi/pull/4250#issuecomment-695048582


   @thenatog I think for `SecureClientZooKeeperFactory` to be usable in the state provider, it'd need a small refactor to use Curator, which actually looks more straightforward than I thought. 


----------------------------------------------------------------
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:
users@infra.apache.org