From f14fe9eaf079cba957267fecc8ee0efdc0b4c895 Mon Sep 17 00:00:00 2001 From: Rahul Rane Date: Thu, 30 Jun 2022 17:03:48 -0700 Subject: [PATCH] Fixing updating auth info whenever mapping is updated to TLS connection on unified port. (#84) * Fixing updating auth info whenever mapping is updated to TLS connection on unified port. Co-authored-by: Rahul Rane --- .../ZkClientUriDomainMappingHelper.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java index 75d44933f23..d0490a4abbf 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/znode/groupacl/ZkClientUriDomainMappingHelper.java @@ -144,14 +144,26 @@ public void process(WatchedEvent event) { LOG.info("Processing watched event: {}", event.toString()); parseZNodeMapping(); // Update AuthInfo for all the known connections. + // Note : It is not ideal to iterate over all plaintext connections which are connected over non-TLS but right now + // there is no way to find out if connection on unified port is using SSLHandler or nonSSLHandler. Anyways, we + // should not ideally have any nonSSLHandler connections on unified port after complete rollout. + // TODO Change to read SecureServerCnxnFactory only. The current logic is to support unit test who is not creating // a secured server cnxn factory. It won't cause any problem but is not technically correct. - ServerCnxnFactory factory = - zks.getSecureServerCnxnFactory() == null ? zks.getServerCnxnFactory() : zks.getSecureServerCnxnFactory(); + + // Since port unification is supported, TLS requests could be made on unified as well as secure port. Hence iterate + // over all connections to update auth info. + ServerCnxnFactory factory = zks.getServerCnxnFactory(); + LOG.info("Updating auth info for connections"); + // TODO Evaluate performance impact and potentially use thread pool to parallelize the AuthInfo update. if (factory != null) { - // TODO Evaluate performance impact and potentially use thread pool to parallelize the AuthInfo update. factory.getConnections().forEach(cnxn -> updateDomainBasedAuthInfo(cnxn)); } + ServerCnxnFactory secureFactory = zks.getSecureServerCnxnFactory(); + LOG.info("Updating auth info for TLS connections"); + if (secureFactory != null) { + secureFactory.getConnections().forEach(cnxn -> updateDomainBasedAuthInfo(cnxn)); + } } @Override