thenatog commented on a change in pull request #4613:
URL: https://github.com/apache/nifi/pull/4613#discussion_r513011699
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/state/manager/StandardStateManagerProvider.java
##########
@@ -217,12 +228,17 @@ private static StateProvider createStateProvider(final
File configFile, final Sc
propertyMap.put(descriptor, new
StandardPropertyValue(entry.getValue(),null, parameterLookup,
variableRegistry));
}
- final SSLContext sslContext;
- try {
- sslContext =
SslContextFactory.createSslContext(StandardTlsConfiguration.fromNiFiProperties(properties));
- } catch (TlsException e) {
- logger.error("Encountered an error configuring TLS for state
manager: ", e);
- throw new IllegalStateException("Error configuring TLS for state
manager", e);
+ // add TLS properties if the state provider is a TLS secured Zookeeper
+ if(properties.isZooKeeperClientSecure() &&
providerConfig.getClassName().equals(ZooKeeperStateProvider.class.getName())) {
+ propertyMap.put(new
PropertyDescriptor.Builder().name(NiFiProperties.ZOOKEEPER_CLIENT_SECURE).build(),
+ new StandardPropertyValue("true", null, parameterLookup,
variableRegistry));
+
+ if(properties.isZooKeeperTlsConfigurationPresent()) {
+ StandardTlsConfiguration zooKeeperTlsConfiguration =
StandardTlsConfiguration.zooKeeperTlsFromNiFiProperties(properties);
+ addTlsPropertiesForProvider(propertyMap,
zooKeeperTlsConfiguration, parameterLookup, variableRegistry);
+ } else {
+ addTlsPropertiesForProvider(propertyMap,
standardTlsConfiguration, parameterLookup, variableRegistry);
+ }
Review comment:
Yeah I agree I was worried about introducing the ZooKeeper specific
implementation into the StateManagerProvider, which is why it would have been
ideal to directly use the SSLContext passed through in the initialization
context. Unfortunately ZK doesn't currently allow for receiving an initialized
context (as far as I could intuit from code, docs and their mailing list), so I
had to figure out some way to pass through properties.
I originally had added PropertyDescriptors to the ZooKeeperStateProvider for
each of the keystore/truststore properties which we could retrieve with
getPropertyDescriptors() and assigned those values, but still we would need
some ZK specific code in the StateManagerProvider.
I think the option to inject the NiFiProperties is better, I wasn't aware of
how to do that.
----------------------------------------------------------------
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
