This is an automated email from the ASF dual-hosted git repository.
pzampino pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new ce0f2a9 KNOX-2371 - DefaultTopologyService may skip cluster config
change processing of valid descriptors (#336)
ce0f2a9 is described below
commit ce0f2a9d6a757610132343cb6133cc2bda9388e2
Author: Phil Zampino <[email protected]>
AuthorDate: Tue May 12 18:25:39 2020 -0400
KNOX-2371 - DefaultTopologyService may skip cluster config change
processing of valid descriptors (#336)
---
.../org/apache/knox/gateway/GatewayMessages.java | 3 +-
.../topology/impl/DefaultTopologyService.java | 32 +++---
.../topology/DefaultTopologyServiceTest.java | 112 ++++++++++++++++++++-
3 files changed, 126 insertions(+), 21 deletions(-)
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
index e37f40a..d47583b 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
@@ -606,9 +606,10 @@ public interface GatewayMessages {
@Message(level = MessageLevel.ERROR,
- text = "Encountered an error while responding to {1} @ {0}
configuration change: {2}")
+ text = "Encountered an error processing {2} in response to a {1} @
{0} configuration change: {3}")
void errorRespondingToConfigChange(String source,
String clusterName,
+ String descriptor,
@StackTrace(level = MessageLevel.DEBUG)
Exception e);
@Message(level = MessageLevel.INFO,
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
b/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
index 66874f9..b9a24cc 100644
---
a/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
+++
b/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
@@ -89,14 +89,14 @@ public class DefaultTopologyService extends
FileAlterationListenerAdaptor implem
private static final JAXBContext jaxbContext = getJAXBContext();
- private static Auditor auditor =
AuditServiceFactory.getAuditService().getAuditor(
+ private static final Auditor auditor =
AuditServiceFactory.getAuditService().getAuditor(
AuditConstants.DEFAULT_AUDITOR_NAME, AuditConstants.KNOX_SERVICE_NAME,
AuditConstants.KNOX_COMPONENT_NAME);
public static final List<String> SUPPORTED_TOPOLOGY_FILE_EXTENSIONS =
Collections.unmodifiableList(Arrays.asList("xml", "conf"));
- private static GatewayMessages log =
MessagesFactory.get(GatewayMessages.class);
- private Map<String, FileAlterationMonitor> monitors = new
ConcurrentHashMap<>();
+ private static final GatewayMessages log =
MessagesFactory.get(GatewayMessages.class);
+ private final Map<String, FileAlterationMonitor> monitors = new
ConcurrentHashMap<>();
private File topologiesDirectory;
private File sharedProvidersDirectory;
private File descriptorsDirectory;
@@ -193,7 +193,6 @@ public class DefaultTopologyService extends
FileAlterationListenerAdaptor implem
} else {
Thread.sleep(10);
elapsed = System.currentTimeMillis() - start;
- continue;
}
} else {
auditor.audit(Action.REDEPLOY, topology.getName(),
ResourceType.TOPOLOGY,
@@ -759,8 +758,8 @@ public class DefaultTopologyService extends
FileAlterationListenerAdaptor implem
*/
private static class TopologyDiscoveryTrigger implements
ClusterConfigurationMonitor.ConfigurationChangeListener {
- private TopologyService topologyService;
- private ClusterConfigurationMonitorService ccms;
+ private final TopologyService topologyService;
+ private final ClusterConfigurationMonitorService ccms;
TopologyDiscoveryTrigger(TopologyService topologyService,
ClusterConfigurationMonitorService ccms) {
this.topologyService = topologyService;
@@ -770,10 +769,11 @@ public class DefaultTopologyService extends
FileAlterationListenerAdaptor implem
@Override
public void onConfigurationChange(final String source, final String
clusterName) {
log.noticedClusterConfigurationChange(source, clusterName);
- try {
- boolean affectedDescriptors = false;
- // Identify any descriptors associated with the cluster configuration
change
- for (File descriptor : topologyService.getDescriptors()) {
+ boolean affectedDescriptors = false;
+
+ // Identify any descriptors associated with the cluster configuration
change
+ for (File descriptor : topologyService.getDescriptors()) {
+ try {
SimpleDescriptor sd =
SimpleDescriptorFactory.parse(descriptor.getAbsolutePath());
if (source.equals(sd.getDiscoveryAddress()) &&
clusterName.equals(sd.getCluster())) {
affectedDescriptors = true;
@@ -781,14 +781,14 @@ public class DefaultTopologyService extends
FileAlterationListenerAdaptor implem
// 'Touch' the descriptor to trigger re-generation of the
associated topology
descriptor.setLastModified(System.currentTimeMillis());
}
+ } catch (IOException e) {
+ log.errorRespondingToConfigChange(source, clusterName,
descriptor.getName(), e);
}
+ }
- if (!affectedDescriptors) {
- // If no descriptors are affected by this configuration, then clear
the cache to prevent future notifications
- ccms.clearCache(source, clusterName);
- }
- } catch (Exception e) {
- log.errorRespondingToConfigChange(source, clusterName, e);
+ if (!affectedDescriptors) {
+ // If no descriptors are affected by this configuration, then clear
the cache to prevent future notifications
+ ccms.clearCache(source, clusterName);
}
}
}
diff --git
a/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java
b/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java
index b71ce27..4b0890d 100644
---
a/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java
+++
b/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java
@@ -25,9 +25,14 @@ import org.apache.knox.gateway.GatewayServer;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.services.GatewayServices;
import org.apache.knox.gateway.services.ServiceType;
+import
org.apache.knox.gateway.services.topology.impl.DefaultClusterConfigurationMonitorService;
import org.apache.knox.gateway.services.topology.impl.DefaultTopologyService;
import org.apache.knox.gateway.services.topology.monitor.DescriptorsMonitor;
import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.topology.ClusterConfigurationMonitorService;
+import org.apache.knox.gateway.topology.discovery.ClusterConfigurationMonitor;
+import org.apache.knox.gateway.topology.simple.SimpleDescriptor;
+import org.apache.knox.gateway.topology.simple.SimpleDescriptorFactory;
import org.apache.knox.test.TestUtils;
import org.apache.knox.gateway.topology.Param;
import org.apache.knox.gateway.topology.Provider;
@@ -37,21 +42,25 @@ import org.apache.knox.gateway.topology.TopologyListener;
import org.easymock.EasyMock;
import org.junit.Test;
+import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.TimeUnit;
import static org.easymock.EasyMock.anyObject;
import static org.hamcrest.CoreMatchers.is;
@@ -70,14 +79,19 @@ public class DefaultTopologyServiceTest {
}
private File createFile(File parent, String name, String resource, long
timestamp) throws IOException {
+ try(InputStream input = ClassLoader.getSystemResourceAsStream(resource)) {
+ return createFile(parent, name, input, timestamp);
+ }
+ }
+
+ private File createFile(File parent, String name, InputStream content, long
timestamp) throws IOException {
File file = new File(parent, name);
if (!file.exists()) {
FileUtils.touch(file);
}
- try(InputStream input = ClassLoader.getSystemResourceAsStream(resource);
- OutputStream output = FileUtils.openOutputStream(file)) {
- assertNotNull(input);
- IOUtils.copy(input, output);
+ try(OutputStream output = FileUtils.openOutputStream(file)) {
+ assertNotNull(content);
+ IOUtils.copy(content, output);
}
file.setLastModified(timestamp);
assertTrue("Failed to create test file " + file.getAbsolutePath(),
file.exists());
@@ -602,6 +616,96 @@ public class DefaultTopologyServiceTest {
}
}
+ /**
+ * KNOX-2371
+ */
+ @Test
+ public void testTopologyDiscoveryTriggerHandlesInvalidDescriptorContent()
throws Exception {
+ File dir = createDir();
+ File topologyDir = new File(dir, "topologies");
+ topologyDir.mkdirs();
+
+ File descriptorsDir = new File(dir, "descriptors");
+ descriptorsDir.mkdirs();
+
+ File sharedProvidersDir = new File(dir, "shared-providers");
+ sharedProvidersDir.mkdirs();
+
+ try {
+ GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+
EasyMock.expect(config.getGatewayTopologyDir()).andReturn(topologyDir.getAbsolutePath()).anyTimes();
+
EasyMock.expect(config.getGatewayConfDir()).andReturn(descriptorsDir.getParentFile().getAbsolutePath()).anyTimes();
+ EasyMock.replay(config);
+
+ TopologyService ts = new DefaultTopologyService();
+ ts.init(config, Collections.emptyMap());
+
+ ClusterConfigurationMonitorService ccms = new
DefaultClusterConfigurationMonitorService();
+ ccms.init(config, Collections.emptyMap());
+
+ // GatewayServices mock
+ GatewayServices gws = EasyMock.createNiceMock(GatewayServices.class);
+
EasyMock.expect(gws.getService(ServiceType.TOPOLOGY_SERVICE)).andReturn(ts).anyTimes();
+
EasyMock.expect(gws.getService(ServiceType.CLUSTER_CONFIGURATION_MONITOR_SERVICE)).andReturn(ccms).anyTimes();
+ EasyMock.replay(gws);
+ setGatewayServices(gws);
+
+ // Write out the referenced provider config first
+ createFile(sharedProvidersDir,
+ "provider-config-one.xml",
+
"org/apache/knox/gateway/topology/file/provider-config-one.xml",
+ System.currentTimeMillis());
+
+ // Create a valid simple descriptor, which depends on
provider-config-one.xml
+ File validDescriptorFile = createFile(descriptorsDir,
+ "valid-descriptor.json",
+
"org/apache/knox/gateway/topology/file/simple-descriptor-six.json",
+ System.currentTimeMillis() -
TimeUnit.HOURS.toMillis(1)); // One hour ago
+ long initialValidTimestamp = validDescriptorFile.lastModified();
+
+ // Parse the valid test descriptor
+ final SimpleDescriptor validDescriptor =
SimpleDescriptorFactory.parse(validDescriptorFile.getAbsolutePath());
+
+ // Create an invalid simple descriptor
+ final String invalidDescriptorContent = "{\"utter\" = \"nonsense\"}";
+ File invalidDescriptorFile =
+ createFile(descriptorsDir,
+ "invalid-descriptor.json",
+ new
ByteArrayInputStream(invalidDescriptorContent.getBytes(StandardCharsets.UTF_8)),
+ System.currentTimeMillis()-
TimeUnit.MINUTES.toMillis(45)); // 45 minutes ago
+ long initialInvalidTimestamp = invalidDescriptorFile.lastModified();
+
+ // Hack the DefaultTopologyService class to access the internal
TopologyDiscoveryTrigger,
+ // which is what we're actually trying to test
+ ClusterConfigurationMonitor.ConfigurationChangeListener
topologyDiscoveryTrigger = null;
+ Class[] classes = DefaultTopologyService.class.getDeclaredClasses();
+ for (Class clazz : classes) {
+ if ("TopologyDiscoveryTrigger".equals(clazz.getSimpleName())) {
+ Constructor ctor =
+ clazz.getDeclaredConstructor(TopologyService.class,
ClusterConfigurationMonitorService.class);
+ ctor.setAccessible(true);
+ topologyDiscoveryTrigger =
+ (ClusterConfigurationMonitor.ConfigurationChangeListener)
ctor.newInstance(ts, ccms);
+ break;
+ }
+ }
+ assertNotNull("Failed to access the cluster configuration change
listener under test.", topologyDiscoveryTrigger);
+
+ // Invoke the TopologyDiscoveryTrigger
+
topologyDiscoveryTrigger.onConfigurationChange(validDescriptor.getDiscoveryAddress(),
validDescriptor.getCluster());
+
+ assertEquals("Expected the invalid descriptor file's timestamp to have
remained unchanged.",
+ initialInvalidTimestamp,
+ invalidDescriptorFile.lastModified());
+ assertTrue("Expected the timestamp of the valid descriptor to have been
updated.",
+ (initialValidTimestamp < validDescriptorFile.lastModified()));
+
+ } finally {
+ FileUtils.deleteQuietly(dir);
+ setGatewayServices(null);
+ }
+ }
+
private class TestTopologyListener implements TopologyListener {
List<List<TopologyEvent>> events = new ArrayList<>();
