You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by pz...@apache.org on 2020/04/27 18:42:45 UTC

[knox] branch master updated: KNOX-2357 - Descriptor handler should not default discovery type to Ambari unless there is discovery configuration (#326)

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 <pz...@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);