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 dbdfe7f KNOX-2357 - Descriptor handler should not default discovery
type to Ambari unless there is discovery configuration (#326)
dbdfe7f is described below
commit dbdfe7fc23c3e2f05deb0867913cea74c0b0032c
Author: Phil Zampino <pzamp...@apache.org>
AuthorDate: Mon Apr 27 14:42:35 2020 -0400
KNOX-2357 - Descriptor handler should not default discovery type to Ambari
unless there is discovery configuration (#326)
---
.../simple/SimpleDescriptorHandlerTest.java | 64 ++++++++++++++++++++++
.../topology/simple/SimpleDescriptorHandler.java | 43 ++++++++++++---
.../topology/simple/SimpleDescriptorMessages.java | 10 +++-
3 files changed, 108 insertions(+), 9 deletions(-)
diff --git
a/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java
b/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java
index d6ecc81..58304a2 100644
---
a/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java
+++
b/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java
@@ -129,6 +129,70 @@ public class SimpleDescriptorHandlerTest {
" </provider>\n" +
" </gateway>\n";
+
+ @Test
+ public void testSkipDiscovery_NoDiscoveryConfig() throws Exception {
+ // There should be no exception because in this case, discovery should
be skipped altogether
+ doTestDiscoveryConfig(null, null, null, null, null);
+ }
+
+ private void doTestDiscoveryConfig(final String discoveryType,
+ final String address,
+ final String clusterName,
+ final String user,
+ final String pwdAlias) throws Exception
{
+ GatewayConfig gc = EasyMock.createNiceMock(GatewayConfig.class);
+ EasyMock.replay(gc);
+
+ // Write the externalized provider config to a temp file
+ File providerConfig = new File(System.getProperty("java.io.tmpdir"),
"test-providers.xml");
+ FileUtils.write(providerConfig, TEST_PROVIDER_CONFIG,
StandardCharsets.UTF_8);
+
+ // Mock out the simple descriptor
+ SimpleDescriptor testDescriptor =
EasyMock.createNiceMock(SimpleDescriptor.class);
+
EasyMock.expect(testDescriptor.getName()).andReturn("mysimpledescriptor").anyTimes();
+
EasyMock.expect(testDescriptor.getProviderConfig()).andReturn(providerConfig.getAbsolutePath()).anyTimes();
+
EasyMock.expect(testDescriptor.getDiscoveryAddress()).andReturn(address).anyTimes();
+
EasyMock.expect(testDescriptor.getDiscoveryType()).andReturn(discoveryType).anyTimes();
+
EasyMock.expect(testDescriptor.getDiscoveryUser()).andReturn(user).anyTimes();
+
EasyMock.expect(testDescriptor.getDiscoveryPasswordAlias()).andReturn(pwdAlias).anyTimes();
+
EasyMock.expect(testDescriptor.getCluster()).andReturn(clusterName).anyTimes();
+ List<SimpleDescriptor.Service> serviceMocks;
+ SimpleDescriptor.Service svc =
EasyMock.createNiceMock(SimpleDescriptor.Service.class);
+ EasyMock.expect(svc.getName()).andReturn("KNOXTOKEN").anyTimes();
+ EasyMock.expect(svc.getVersion()).andReturn(null).anyTimes();
+
EasyMock.expect(svc.getURLs()).andReturn(Collections.emptyList()).anyTimes();
+
+ Map<String, String> serviceParams = new HashMap<>();
+ serviceParams.put("knox.token.ttl", "120000");
+ EasyMock.expect(svc.getParams()).andReturn(serviceParams).anyTimes();
+
+ EasyMock.replay(svc);
+ serviceMocks = Collections.singletonList(svc);
+
+
EasyMock.expect(testDescriptor.getServices()).andReturn(serviceMocks).anyTimes();
+ EasyMock.replay(testDescriptor);
+
+ File destDir = new
File(System.getProperty("java.io.tmpdir")).getCanonicalFile();
+ File topologyFile = null;
+
+ try {
+ // Invoke the simple descriptor handler
+ Map<String, File> files =
+ SimpleDescriptorHandler.handle(gc,
+ testDescriptor,
+ providerConfig.getParentFile(), // simple desc
co-located with provider config
+ destDir);
+ topologyFile = files.get("topology");
+ assertTrue(topologyFile.exists());
+ } finally {
+ providerConfig.delete();
+ if (topologyFile != null) {
+ topologyFile.delete();
+ }
+ }
+ }
+
/*
* KNOX-1006
*
diff --git
a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
index f94fe3e..f31912a 100644
---
a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
+++
b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
@@ -106,9 +106,14 @@ public class SimpleDescriptorHandler {
Map<String, Map<String, String>> serviceParams = new HashMap<>();
Map<String, List<String>> serviceURLs = new HashMap<>();
- ServiceDiscovery.Cluster cluster = performDiscovery(config, desc,
gatewayServices);
- if (cluster == null) {
- log.failedToDiscoverClusterServices(desc.getName());
+ ServiceDiscovery.Cluster cluster = null;
+ if (shouldPerformDiscovery(desc)) {
+ cluster = performDiscovery(config, desc, gatewayServices);
+ if (cluster == null) {
+ log.failedToDiscoverClusterServices(desc.getName());
+ }
+ } else {
+ log.discoveryNotConfiguredForDescriptor(desc.getName());
}
for (SimpleDescriptor.Service descService : desc.getServices()) {
@@ -195,6 +200,22 @@ public class SimpleDescriptorHandler {
gws);
}
+ /**
+ * Determine whether discovery should be performed for the specified
descriptor.
+ *
+ * @param desc A SimpleDescriptor
+ * @return true, if discovery should be performed for the descriptor;
Otherwise, false.
+ */
+ private static boolean shouldPerformDiscovery(final SimpleDescriptor desc)
{
+ // If there is a discovery type specified, then discovery should be
performed
+ final String discoveryType = desc.getDiscoveryType();
+ if (discoveryType != null && !discoveryType.isEmpty()) {
+ return true;
+ }
+ log.missingDiscoveryTypeInDescriptor(desc.getName());
+ return false;
+ }
+
private static GatewayServices getGatewayServices(Service... services) {
for (Service service : services) {
if (service instanceof GatewayServices) {
@@ -220,6 +241,9 @@ public class SimpleDescriptorHandler {
ServiceDiscovery sd = discoveryInstances.get(discoveryType);
if (sd == null) {
sd = ServiceDiscoveryFactory.get(discoveryType, gatewayServices);
+ if (sd == null) {
+ throw new IllegalArgumentException("Unsupported service
discovery type: " + discoveryType);
+ }
discoveryInstances.put(discoveryType, sd);
}
@@ -624,11 +648,14 @@ public class SimpleDescriptorHandler {
// If it does exist, only overwrite it if it has changed
(KNOX-2302)
// Compare the generated topology with the in-memory topology
Topology existing = null;
- TopologyService topologyService =
gwServices.getService(ServiceType.TOPOLOGY_SERVICE);
- for (Topology t : topologyService.getTopologies()) {
- if (topologyName.equals(t.getName())) {
- existing = t;
- break;
+ TopologyService topologyService = null;
+ if (gwServices != null) {
+ topologyService =
gwServices.getService(ServiceType.TOPOLOGY_SERVICE);
+ for (Topology t : topologyService.getTopologies()) {
+ if (topologyName.equals(t.getName())) {
+ existing = t;
+ break;
+ }
}
}
diff --git
a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorMessages.java
b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorMessages.java
index 6beceab..e2642ae 100644
---
a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorMessages.java
+++
b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorMessages.java
@@ -21,9 +21,17 @@ import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;
import org.apache.knox.gateway.i18n.messages.StackTrace;
-@Messages(logger="org.apache.gateway.topology.simple")
+@Messages(logger="org.apache.knox.gateway.topology.simple")
public interface SimpleDescriptorMessages {
+ @Message(level = MessageLevel.INFO,
+ text = "Skipping service discovery for the \"{0}\" descriptor
because its contents do not indicate it is intended.")
+ void discoveryNotConfiguredForDescriptor(String descriptorName);
+
+ @Message(level = MessageLevel.INFO,
+ text = "The \"{0}\" descriptor does not include discovery-type.")
+ void missingDiscoveryTypeInDescriptor(String descriptorName);
+
@Message(level = MessageLevel.ERROR,
text = "Unable to complete service discovery for cluster {0}.")
void failedToDiscoverClusterServices(String descriptorName);
