Repository: knox Updated Branches: refs/heads/master a60062273 -> 3999c9234
KNOX-1505 - Knox should close CuratorFramework clients when finished Signed-off-by: Kevin Risden <kris...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/3999c923 Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/3999c923 Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/3999c923 Branch: refs/heads/master Commit: 3999c9234dca285900617926b9a32ef989d9852f Parents: a600622 Author: Kevin Risden <kris...@apache.org> Authored: Wed Oct 3 12:14:25 2018 -0400 Committer: Kevin Risden <kris...@apache.org> Committed: Thu Oct 4 15:12:27 2018 -0400 ---------------------------------------------------------------------- .../provider/impl/AtlasZookeeperURLManager.java | 19 +-- .../provider/impl/HBaseZookeeperURLManager.java | 16 +-- .../provider/impl/HS2ZookeeperURLManager.java | 17 +-- .../provider/impl/KafkaZookeeperURLManager.java | 13 +- .../provider/impl/SOLRZookeeperURLManager.java | 12 +- .../impl/AtlasZookeeperURLManagerTest.java | 18 ++- .../impl/HBaseZookeeperURLManagerTest.java | 15 ++- .../impl/HS2ZookeeperURLManagerTest.java | 5 +- .../impl/KafkaZookeeperURLManagerTest.java | 18 ++- .../impl/SOLRZookeeperURLManagerTest.java | 2 + .../security/impl/RemoteAliasMonitorTest.java | 3 + ...emoteConfigurationRegistryClientService.java | 12 ++ .../ZooKeeperConfigurationMonitorTest.java | 10 +- .../config/remote/zk/CuratorClientService.java | 12 ++ ...eConfigurationRegistryClientServiceTest.java | 3 + .../RemoteConfigurationRegistryClient.java | 2 +- .../monitor/RemoteConfigurationMonitorTest.java | 131 ++++++++++--------- 17 files changed, 170 insertions(+), 138 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManager.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManager.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManager.java index 8d3ce38..0d5627e 100644 --- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManager.java +++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManager.java @@ -28,6 +28,7 @@ import org.apache.knox.gateway.i18n.messages.MessagesFactory; import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeUnit; public class AtlasZookeeperURLManager extends DefaultURLManager { private static final String DEFAULT_ZOOKEEPER_NAMESPACE = "/apache_atlas"; @@ -67,28 +68,22 @@ public class AtlasZookeeperURLManager extends DefaultURLManager { public List<String> lookupURLs() { List<String> serverHosts = new ArrayList<>(); - CuratorFramework zooKeeperClient = - CuratorFrameworkFactory.builder().connectString(zooKeeperEnsemble) - .retryPolicy(new ExponentialBackoffRetry(1000, 3)) - .build(); - try { + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder() + .connectString(zooKeeperEnsemble) + .retryPolicy(new ExponentialBackoffRetry(1000, 3)) + .build()) { + zooKeeperClient.start(); + zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS); byte[] bytes = zooKeeperClient.getData().forPath(zooKeeperNamespace + APACHE_ATLAS_ACTIVE_SERVER_INFO); String activeURL = new String(bytes, Charset.forName("UTF-8")); serverHosts.add(activeURL); - } catch (Exception e) { - LOG.failedToGetZookeeperUrls(e); throw new RuntimeException(e); - } finally { - // Close the client connection with ZooKeeper - if (zooKeeperClient != null) { - zooKeeperClient.close(); - } } return serverHosts; } http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManager.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManager.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManager.java index 5e3a762..7f7522e 100644 --- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManager.java +++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManager.java @@ -24,6 +24,7 @@ import org.apache.curator.retry.ExponentialBackoffRetry; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.TimeUnit; /** * Implementation of URLManager intended for query of Zookeeper for active HBase RegionServer hosts. @@ -94,13 +95,11 @@ public class HBaseZookeeperURLManager extends BaseZookeeperURLManager { { List<String> serverHosts = new ArrayList<>(); - CuratorFramework zooKeeperClient = - CuratorFrameworkFactory.builder().connectString(getZookeeperEnsemble()) - .retryPolicy(new ExponentialBackoffRetry(1000, 3)) - .build(); - - try { + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder().connectString(getZookeeperEnsemble()) + .retryPolicy(new ExponentialBackoffRetry(1000, 3)) + .build()) { zooKeeperClient.start(); + zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS); List<String> serverNodes = null; @@ -133,11 +132,6 @@ public class HBaseZookeeperURLManager extends BaseZookeeperURLManager { } catch (Exception e) { LOG.failedToGetZookeeperUrls(e); throw new RuntimeException(e); - } finally { - // Close the client connection with ZooKeeper - if (zooKeeperClient != null) { - zooKeeperClient.close(); - } } return serverHosts; http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManager.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManager.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManager.java index 4356465..4f495b6 100644 --- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManager.java +++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManager.java @@ -29,6 +29,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -70,13 +71,12 @@ public class HS2ZookeeperURLManager extends DefaultURLManager { public List<String> lookupURLs() { List<String> serverHosts = new ArrayList<>(); - CuratorFramework zooKeeperClient = - CuratorFrameworkFactory.builder().connectString(zooKeeperEnsemble) - .retryPolicy(new ExponentialBackoffRetry(1000, 3)).build(); - try { + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder().connectString(zooKeeperEnsemble) + .retryPolicy(new ExponentialBackoffRetry(1000, 3)).build()) { zooKeeperClient.start(); + zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS); List<String> serverNodes = zooKeeperClient.getChildren().forPath("/" + zooKeeperNamespace); - for ( String serverNode : serverNodes ) { + for (String serverNode : serverNodes) { String serverInfo = new String( zooKeeperClient.getData().forPath("/" + zooKeeperNamespace + "/" + serverNode), @@ -84,14 +84,9 @@ public class HS2ZookeeperURLManager extends DefaultURLManager { String serverURL = constructURL(serverInfo); serverHosts.add(serverURL); } - } catch ( Exception e ) { + } catch (Exception e) { LOG.failedToGetZookeeperUrls(e); throw new RuntimeException(e); - } finally { - // Close the client connection with ZooKeeper - if ( zooKeeperClient != null ) { - zooKeeperClient.close(); - } } return serverHosts; } http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManager.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManager.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManager.java index 0e72d16..534291b 100644 --- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManager.java +++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManager.java @@ -29,6 +29,7 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.TimeUnit; /** * Implementation of URLManager intended for query of Zookeeper for active Kafka hosts. @@ -93,13 +94,12 @@ public class KafkaZookeeperURLManager extends BaseZookeeperURLManager { { List<String> serverHosts = new ArrayList<>(); - CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder() + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder() .connectString(getZookeeperEnsemble()) .retryPolicy(new ExponentialBackoffRetry(1000, 3)) - .build(); - - try { + .build()) { zooKeeperClient.start(); + zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS); // Retrieve list of host URLs from ZooKeeper List<String> brokers = zooKeeperClient.getChildren().forPath(BASE_PATH); @@ -113,11 +113,6 @@ public class KafkaZookeeperURLManager extends BaseZookeeperURLManager { } catch (Exception e) { LOG.failedToGetZookeeperUrls(e); throw new RuntimeException(e); - } finally { - // Close the client connection with ZooKeeper - if (zooKeeperClient != null) { - zooKeeperClient.close(); - } } return serverHosts; http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManager.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManager.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManager.java index 112fce7..830ef66 100644 --- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManager.java +++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManager.java @@ -24,6 +24,7 @@ import org.apache.curator.retry.ExponentialBackoffRetry; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.TimeUnit; /** * Implementation of URLManager intended for query of Zookeeper for active SOLR Cloud hosts. @@ -73,13 +74,13 @@ public class SOLRZookeeperURLManager extends BaseZookeeperURLManager { { List<String> serverHosts = new ArrayList<>(); - CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder() + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder() .connectString(getZookeeperEnsemble()) .retryPolicy(new ExponentialBackoffRetry(1000, 3)) - .build(); + .build()) { - try { zooKeeperClient.start(); + zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS); List<String> serverNodes = zooKeeperClient.getChildren().forPath("/live_nodes"); for (String serverNode : serverNodes) { String serverURL = constructURL(serverNode); @@ -88,11 +89,6 @@ public class SOLRZookeeperURLManager extends BaseZookeeperURLManager { } catch (Exception e) { LOG.failedToGetZookeeperUrls(e); throw new RuntimeException(e); - } finally { - // Close the client connection with ZooKeeper - if (zooKeeperClient != null) { - zooKeeperClient.close(); - } } return serverHosts; http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManagerTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManagerTest.java b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManagerTest.java index 2e4a1d6..4c2c8a8 100644 --- a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManagerTest.java +++ b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManagerTest.java @@ -32,9 +32,11 @@ import org.junit.Test; import java.io.IOException; import java.nio.charset.Charset; import java.util.List; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; public class AtlasZookeeperURLManagerTest { @@ -54,6 +56,8 @@ public class AtlasZookeeperURLManagerTest { .build(); zooKeeperClient.start(); + assertTrue(zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS)); + zooKeeperClient.create().forPath("/apache_atlas"); zooKeeperClient.create().forPath("/apache_atlas/active_server_info"); zooKeeperClient.setData().forPath("/apache_atlas/active_server_info", @@ -143,16 +147,16 @@ public class AtlasZookeeperURLManagerTest { void setAtlasActiveHostURLInZookeeper(String activeURL) throws Exception { - - CuratorFramework zooKeeperClient = + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder().connectString(cluster.getConnectString()) .retryPolicy(new ExponentialBackoffRetry(1000, 3)) - .build(); + .build()) { + zooKeeperClient.start(); + zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS); - zooKeeperClient.start(); - zooKeeperClient.setData().forPath("/apache_atlas/active_server_info", - activeURL.getBytes(Charset.forName("UTF-8"))); - zooKeeperClient.close(); + zooKeeperClient.setData().forPath("/apache_atlas/active_server_info", + activeURL.getBytes(Charset.forName("UTF-8"))); + } } } http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManagerTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManagerTest.java b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManagerTest.java index 7d57b49..dfda14f 100644 --- a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManagerTest.java +++ b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManagerTest.java @@ -18,6 +18,7 @@ package org.apache.knox.gateway.ha.provider.impl; import java.io.IOException; +import java.util.concurrent.TimeUnit; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; @@ -31,6 +32,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** @@ -113,13 +115,14 @@ public class HBaseZookeeperURLManagerTest { private void createZNodes(String namespace) throws Exception { - CuratorFramework zooKeeperClient = + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder().connectString(cluster.getConnectString()) - .retryPolicy(new ExponentialBackoffRetry(1000, 3)).build(); - zooKeeperClient.start(); - zooKeeperClient.create().forPath(namespace); - zooKeeperClient.create().forPath(namespace + "/rs"); - zooKeeperClient.close(); + .retryPolicy(new ExponentialBackoffRetry(1000, 3)).build()) { + zooKeeperClient.start(); + assertTrue(zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS)); + zooKeeperClient.create().forPath(namespace); + zooKeeperClient.create().forPath(namespace + "/rs"); + } } } http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManagerTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManagerTest.java b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManagerTest.java index 9c5e25c..dcdb926 100644 --- a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManagerTest.java +++ b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManagerTest.java @@ -32,8 +32,10 @@ import org.junit.Test; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class HS2ZookeeperURLManagerTest { @@ -58,6 +60,7 @@ public class HS2ZookeeperURLManagerTest { String host4 = "hive.server2.authentication=NONE;hive.server2.transport.mode=http;hive.server2.thrift.http.path=cliservice;" + "hive.server2.thrift.http.port=10004;hive.server2.thrift.bind.host=host4;hive.server2.use.SSL=true"; zooKeeperClient.start(); + assertTrue(zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS)); zooKeeperClient.create().forPath("/hiveServer2"); zooKeeperClient.create().forPath("/hiveServer2/host1", host1.getBytes(StandardCharsets.UTF_8)); zooKeeperClient.create().forPath("/hiveServer2/host2", host2.getBytes(StandardCharsets.UTF_8)); @@ -128,7 +131,7 @@ public class HS2ZookeeperURLManagerTest { config.setZookeeperNamespace("hiveServer2"); URLManager manager = URLManagerLoader.loadURLManager(config); Assert.assertNotNull(manager); - Assert.assertTrue(manager instanceof HS2ZookeeperURLManager); + assertTrue(manager instanceof HS2ZookeeperURLManager); } http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManagerTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManagerTest.java b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManagerTest.java index fb02517..4c2fa45 100644 --- a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManagerTest.java +++ b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManagerTest.java @@ -18,6 +18,7 @@ package org.apache.knox.gateway.ha.provider.impl; import java.io.IOException; +import java.util.concurrent.TimeUnit; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; @@ -31,6 +32,8 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertTrue; + /** * Simple unit tests for KafkaZookeeperURLManager. * @@ -44,14 +47,15 @@ public class KafkaZookeeperURLManagerTest { cluster = new TestingCluster(3); cluster.start(); - CuratorFramework zooKeeperClient = - CuratorFrameworkFactory.builder().connectString(cluster.getConnectString()) - .retryPolicy(new ExponentialBackoffRetry(1000, 3)).build(); + try (CuratorFramework zooKeeperClient = CuratorFrameworkFactory.builder() + .connectString(cluster.getConnectString()) + .retryPolicy(new ExponentialBackoffRetry(1000, 3)).build()) { - zooKeeperClient.start(); - zooKeeperClient.create().forPath("/brokers"); - zooKeeperClient.create().forPath("/brokers/ids"); - zooKeeperClient.close(); + zooKeeperClient.start(); + assertTrue(zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS)); + zooKeeperClient.create().forPath("/brokers"); + zooKeeperClient.create().forPath("/brokers/ids"); + } } @After http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManagerTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManagerTest.java b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManagerTest.java index c327131..a37a138 100644 --- a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManagerTest.java +++ b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManagerTest.java @@ -32,6 +32,7 @@ import org.junit.Test; import java.io.IOException; import java.util.List; import java.util.TreeSet; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -56,6 +57,7 @@ public class SOLRZookeeperURLManagerTest { .retryPolicy(new ExponentialBackoffRetry(1000, 3)).build(); zooKeeperClient.start(); + assertTrue(zooKeeperClient.blockUntilConnected(10, TimeUnit.SECONDS)); zooKeeperClient.create().forPath("/live_nodes"); zooKeeperClient.create().forPath("/live_nodes/host1:8983_solr"); zooKeeperClient.create().forPath("/live_nodes/host2:8983_solr"); http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasMonitorTest.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasMonitorTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasMonitorTest.java index 000ca7d..5e97f4b 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasMonitorTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasMonitorTest.java @@ -45,9 +45,11 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import static org.easymock.EasyMock.capture; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; /** * Test the listener/monitor service for @@ -96,6 +98,7 @@ public class RemoteAliasMonitorTest { .retryPolicy(new ExponentialBackoffRetry(100, 3)).build(); assertNotNull(client); client.start(); + assertTrue(client.blockUntilConnected(10, TimeUnit.SECONDS)); // Create the knox config paths with an ACL for the sasl user configured for the client List<ACL> acls = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java b/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java index 5810e51..96e390b 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java @@ -77,12 +77,24 @@ public class LocalFileSystemRemoteConfigurationRegistryClientService implements @Override public void stop() throws ServiceLifecycleException { + for(RemoteConfigurationRegistryClient client : clients.values()) { + try { + client.close(); + } catch (Exception e) { + throw new ServiceLifecycleException("failed to close client", e); + } + } } private RemoteConfigurationRegistryClient createClient(RemoteConfigurationRegistryConfig config) { String rootDir = config.getConnectionString(); return new RemoteConfigurationRegistryClient() { + @Override + public void close() throws Exception { + + } + private File root = new File(rootDir); @Override http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-server/src/test/java/org/apache/knox/gateway/topology/monitor/ZooKeeperConfigurationMonitorTest.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/topology/monitor/ZooKeeperConfigurationMonitorTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/topology/monitor/ZooKeeperConfigurationMonitorTest.java index 136f0b8..100e2e6 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/topology/monitor/ZooKeeperConfigurationMonitorTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/topology/monitor/ZooKeeperConfigurationMonitorTest.java @@ -43,6 +43,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -108,6 +109,10 @@ public class ZooKeeperConfigurationMonitorTest { assertNotNull(client); client.start(); + boolean connected = client.blockUntilConnected(10, TimeUnit.SECONDS); + assertTrue(connected); + assertTrue(client.isZk34CompatibilityMode()); + // Create the knox config paths with an ACL for the sasl user configured for the client List<ACL> acls = new ArrayList<>(); acls.add(new ACL(ZooDefs.Perms.ALL, ZooDefs.Ids.ANYONE_ID_UNSAFE)); @@ -124,7 +129,9 @@ public class ZooKeeperConfigurationMonitorTest { public static void tearDownSuite() throws Exception { // Clean up the ZK nodes, and close the client if (client != null) { - client.delete().deletingChildrenIfNeeded().forPath(PATH_KNOX); + if (client.checkExists().forPath(PATH_KNOX) != null) { + client.delete().deletingChildrenIfNeeded().forPath(PATH_KNOX); + } client.close(); } @@ -252,6 +259,7 @@ public class ZooKeeperConfigurationMonitorTest { Thread.sleep(100); assertFalse(desc_one.exists()); } finally { + clientService.stop(); cm.stop(); } } http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java ---------------------------------------------------------------------- diff --git a/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java b/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java index 21791fe..da10e93 100644 --- a/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java +++ b/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java @@ -102,6 +102,13 @@ class CuratorClientService implements ZooKeeperClientService { @Override public void stop() throws ServiceLifecycleException { + for(RemoteConfigurationRegistryClient client : clients.values()) { + try { + client.close(); + } catch (Exception e) { + throw new ServiceLifecycleException("failed to close client", e); + } + } } @Override @@ -324,6 +331,11 @@ class CuratorClientService implements ZooKeeperClientService { log.errorInteractingWithRemoteConfigRegistry(e); } } + + @Override + public void close() throws Exception { + delegate.close(); + } } /** http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryClientServiceTest.java ---------------------------------------------------------------------- diff --git a/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryClientServiceTest.java b/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryClientServiceTest.java index ba172d1..981a037 100644 --- a/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryClientServiceTest.java +++ b/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryClientServiceTest.java @@ -44,6 +44,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -272,6 +273,8 @@ public class RemoteConfigurationRegistryClientServiceTest { assertNotNull(setupClient); setupClient.start(); + assertTrue(setupClient.blockUntilConnected(10, TimeUnit.SECONDS)); + List<ACL> acls = new ArrayList<>(); if (principal != null) { acls.add(new ACL(ZooDefs.Perms.ALL, new Id("sasl", principal))); http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-spi/src/main/java/org/apache/knox/gateway/services/config/client/RemoteConfigurationRegistryClient.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/services/config/client/RemoteConfigurationRegistryClient.java b/gateway-spi/src/main/java/org/apache/knox/gateway/services/config/client/RemoteConfigurationRegistryClient.java index 443764e..94be07b 100644 --- a/gateway-spi/src/main/java/org/apache/knox/gateway/services/config/client/RemoteConfigurationRegistryClient.java +++ b/gateway-spi/src/main/java/org/apache/knox/gateway/services/config/client/RemoteConfigurationRegistryClient.java @@ -18,7 +18,7 @@ package org.apache.knox.gateway.services.config.client; import java.util.List; -public interface RemoteConfigurationRegistryClient { +public interface RemoteConfigurationRegistryClient extends AutoCloseable { String getAddress(); http://git-wip-us.apache.org/repos/asf/knox/blob/3999c923/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java ---------------------------------------------------------------------- diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java b/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java index 0a9cd9b..60f1fd4 100644 --- a/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java +++ b/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java @@ -50,13 +50,13 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; /** * Test the RemoteConfigurationMonitor functionality with SASL configured, and znode ACLs applied. @@ -185,6 +185,9 @@ public class RemoteConfigurationMonitorTest { assertNotNull(client); client.start(); + assertTrue(client.blockUntilConnected(10, TimeUnit.SECONDS)); + assertTrue(client.isZk34CompatibilityMode()); + // Create test config nodes with an ACL for a sasl user that is NOT configured for the test client List<ACL> acls = Arrays.asList(new ACL(ZooDefs.Perms.ALL, new Id("sasl", ALT_USERNAME)), new ACL(ZooDefs.Perms.READ, ZooDefs.Ids.ANYONE_ID_UNSAFE)); @@ -222,23 +225,23 @@ public class RemoteConfigurationMonitorTest { EasyMock.expect(gc.getGatewayProvidersConfigDir()).andReturn(providersDir.getAbsolutePath()).anyTimes(); EasyMock.expect(gc.getGatewayDescriptorsDir()).andReturn(descriptorsDir.getAbsolutePath()).anyTimes(); EasyMock.expect(gc.getRemoteRegistryConfigurationNames()) - .andReturn(Collections.singletonList(configMonitorName)) - .anyTimes(); + .andReturn(Collections.singletonList(configMonitorName)) + .anyTimes(); final String registryConfig = - GatewayConfig.REMOTE_CONFIG_REGISTRY_TYPE + "=" + ZooKeeperClientService.TYPE + ";" + - GatewayConfig.REMOTE_CONFIG_REGISTRY_ADDRESS + "=" + zkCluster.getConnectString() + ";" + - GatewayConfig.REMOTE_CONFIG_REGISTRY_PRINCIPAL + "=" + ZK_USERNAME + ";" + - GatewayConfig.REMOTE_CONFIG_REGISTRY_AUTH_TYPE + "=Digest;" + - GatewayConfig.REMOTE_CONFIG_REGISTRY_CREDENTIAL_ALIAS + "=" + alias; + GatewayConfig.REMOTE_CONFIG_REGISTRY_TYPE + "=" + ZooKeeperClientService.TYPE + ";" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_ADDRESS + "=" + zkCluster.getConnectString() + ";" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_PRINCIPAL + "=" + ZK_USERNAME + ";" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_AUTH_TYPE + "=Digest;" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_CREDENTIAL_ALIAS + "=" + alias; EasyMock.expect(gc.getRemoteRegistryConfiguration(configMonitorName)) - .andReturn(registryConfig).anyTimes(); + .andReturn(registryConfig).anyTimes(); EasyMock.expect(gc.getRemoteConfigurationMonitorClientName()).andReturn(configMonitorName).anyTimes(); EasyMock.replay(gc); AliasService aliasService = EasyMock.createNiceMock(AliasService.class); EasyMock.expect(aliasService.getPasswordFromAliasForGateway(alias)) - .andReturn(ZK_PASSWORD.toCharArray()) - .anyTimes(); + .andReturn(ZK_PASSWORD.toCharArray()) + .anyTimes(); EasyMock.replay(aliasService); RemoteConfigurationRegistryClientService clientService = (new ZooKeeperClientServiceProvider()).newInstance(); @@ -270,16 +273,17 @@ public class RemoteConfigurationMonitorTest { try { cm.start(); - } catch (Exception e) { - fail("Failed to start monitor: " + e.getMessage()); - } - // Validate the expected ACLs on the Knox config znodes (make sure the monitor removed the world:anyone ACL) - List<ACL> expectedACLs = Collections.singletonList(SASL_TESTUSER_ALL); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + // Validate the expected ACLs on the Knox config znodes (make sure the monitor removed the world:anyone ACL) + List<ACL> expectedACLs = Collections.singletonList(SASL_TESTUSER_ALL); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + } finally { + clientService.stop(); + cm.stop(); + } } @@ -345,18 +349,19 @@ public class RemoteConfigurationMonitorTest { try { cm.start(); - } catch (Exception e) { - fail("Failed to start monitor: " + e.getMessage()); - } - // Validate the expected ACLs on the Knox config znodes (make sure the monitor removed the world:anyone ACL) - List<ACL> expectedACLs = new ArrayList<>(); - expectedACLs.add(SASL_TESTUSER_ALL); - expectedACLs.add(WORLD_ANYONE_READ); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + // Validate the expected ACLs on the Knox config znodes (make sure the monitor removed the world:anyone ACL) + List<ACL> expectedACLs = new ArrayList<>(); + expectedACLs.add(SASL_TESTUSER_ALL); + expectedACLs.add(WORLD_ANYONE_READ); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + } finally { + clientService.stop(); + cm.stop(); + } } @@ -413,23 +418,24 @@ public class RemoteConfigurationMonitorTest { try { cm.start(); - } catch (Exception e) { - fail("Failed to start monitor: " + e.getMessage()); - } - // Test auth violation - clientService.get(configMonitorName).createEntry("/auth_test/child_node/test1"); - assertNull("Creation should have been prevented since write access is not granted to the test client.", + // Test auth violation + clientService.get(configMonitorName).createEntry("/auth_test/child_node/test1"); + assertNull("Creation should have been prevented since write access is not granted to the test client.", client.checkExists().forPath("/auth_test/child_node/test1")); - assertTrue("Creation should have been prevented since write access is not granted to the test client.", + assertTrue("Creation should have been prevented since write access is not granted to the test client.", client.getChildren().forPath("/auth_test/child_node").isEmpty()); - // Validate the expected ACLs on the Knox config znodes (make sure the monitor didn't change them) - List<ACL> expectedACLs = Collections.singletonList(SASL_TESTUSER_ALL); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + // Validate the expected ACLs on the Knox config znodes (make sure the monitor didn't change them) + List<ACL> expectedACLs = Collections.singletonList(SASL_TESTUSER_ALL); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + } finally { + clientService.stop(); + cm.stop(); + } } @@ -480,26 +486,22 @@ public class RemoteConfigurationMonitorTest { try { cm.start(); - } catch (Exception e) { - fail("Failed to start monitor: " + e.getMessage()); - } - - // Test auth violation - clientService.get(configMonitorName).createEntry("/auth_test/child_node/test1"); - assertNull("Creation should have been prevented since write access is not granted to the test client.", - client.checkExists().forPath("/auth_test/child_node/test1")); - assertTrue("Creation should have been prevented since write access is not granted to the test client.", - client.getChildren().forPath("/auth_test/child_node").isEmpty()); - - // Validate the expected ACLs on the Knox config znodes (make sure the monitor created them correctly) - List<ACL> expectedACLs = Collections.singletonList(SASL_TESTUSER_ALL); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); - validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); - - // Test the Knox config nodes, for which authentication should be sufficient for access - try { + + // Test auth violation + clientService.get(configMonitorName).createEntry("/auth_test/child_node/test1"); + assertNull("Creation should have been prevented since write access is not granted to the test client.", + client.checkExists().forPath("/auth_test/child_node/test1")); + assertTrue("Creation should have been prevented since write access is not granted to the test client.", + client.getChildren().forPath("/auth_test/child_node").isEmpty()); + + // Validate the expected ACLs on the Knox config znodes (make sure the monitor created them correctly) + List<ACL> expectedACLs = Collections.singletonList(SASL_TESTUSER_ALL); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + + // Test the Knox config nodes, for which authentication should be sufficient for access final String pc_one_znode = getProviderPath("providers-config1.xml"); final File pc_one = new File(providersDir, "providers-config1.xml"); final String pc_two_znode = getProviderPath("providers-config2.xml"); @@ -567,6 +569,7 @@ public class RemoteConfigurationMonitorTest { Thread.sleep(100); assertFalse(desc_one.exists()); } finally { + clientService.stop(); cm.stop(); } }
