From 49983cb3b49655488487e13da40191a73ff6589a Mon Sep 17 00:00:00 2001
From: Attila Magyar <m.magyar3@gmail.com>
Date: Fri, 3 Feb 2023 12:40:21 +0100
Subject: [PATCH] KNOX-2871 - Refine should perform discovery check (#721)

---
 .../knox/gateway/SimpleDescriptorHandlerFuncTest.java  | 10 +++++++---
 .../topology/simple/SimpleDescriptorHandler.java       |  4 ++--
 .../topology/simple/SimpleDescriptorMessages.java      |  2 +-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/SimpleDescriptorHandlerFuncTest.java b/gateway-test/src/test/java/org/apache/knox/gateway/SimpleDescriptorHandlerFuncTest.java
index b414b19cb5..33185e33de 100644
--- a/gateway-test/src/test/java/org/apache/knox/gateway/SimpleDescriptorHandlerFuncTest.java
+++ b/gateway-test/src/test/java/org/apache/knox/gateway/SimpleDescriptorHandlerFuncTest.java
@@ -108,6 +108,7 @@ public class SimpleDescriptorHandlerFuncTest {
           "            <param><name>localhost</name><value>sandbox,sandbox.hortonworks.com</value></param>\n" +
           "        </provider>\n" +
           "    </gateway>\n";
+  public static final String DISCOVERY_ADDRESS = "http://dummy_address";
 
 
   /*
@@ -147,7 +148,7 @@ public void testSimpleDescriptorHandlerQueryStringCredentialAliasCreation() thro
       // Mock out the simple descriptor
       SimpleDescriptor testDescriptor = EasyMock.createNiceMock(SimpleDescriptor.class);
       EasyMock.expect(testDescriptor.getName()).andReturn("mysimpledescriptor").anyTimes();
-      EasyMock.expect(testDescriptor.getDiscoveryAddress()).andReturn(null).anyTimes();
+      EasyMock.expect(testDescriptor.getDiscoveryAddress()).andReturn(DISCOVERY_ADDRESS).anyTimes();
       EasyMock.expect(testDescriptor.getDiscoveryType()).andReturn(discoveryType).anyTimes();
       EasyMock.expect(testDescriptor.getDiscoveryUser()).andReturn(null).anyTimes();
       EasyMock.expect(testDescriptor.getProviderConfig()).andReturn(providerConfig.getAbsolutePath()).anyTimes();
@@ -245,6 +246,7 @@ public void testSimpleDescriptorHandlerQueryStringCredentialAliasCreation() thro
       assertEquals("Unexpected alias value (should be master secret + topology name.",
                    testMasterSecret + testDescriptor.getName(), capturedPwd.getValue());
 
+      assertEquals(1, NoOpServiceDiscovery.discoveryCalled);
     } catch (Exception e) {
       e.printStackTrace();
       fail(e.getMessage());
@@ -274,6 +276,7 @@ public ServiceDiscovery newInstance(GatewayConfig gatewayConfig) {
 
   private static final class NoOpServiceDiscovery implements ServiceDiscovery {
     static final String TYPE = "NO_OP";
+    static int discoveryCalled;
 
     @Override
     public String getType() {
@@ -290,12 +293,12 @@ public String getName() {
 
         @Override
         public List<String> getServiceURLs(String serviceName) {
-          return null;
+          return Collections.emptyList();
         }
 
         @Override
         public List<String> getServiceURLs(String serviceName, Map<String, String> serviceParams) {
-          return null;
+          return Collections.emptyList();
         }
 
         @Override
@@ -307,6 +310,7 @@ public ZooKeeperConfig getZooKeeperConfiguration(String serviceName) {
 
     @Override
     public Cluster discover(GatewayConfig gwConfig, ServiceDiscoveryConfig config, String clusterName, Collection<String> includedServices) {
+      discoveryCalled++;
       return discover(gwConfig, config, clusterName);
     }
   }
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 7ef0c6adcf..cb343fbcee 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
@@ -16,6 +16,7 @@
  */
 package org.apache.knox.gateway.topology.simple;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.text.StringEscapeUtils;
 import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.i18n.messages.MessagesFactory;
@@ -210,8 +211,7 @@ public static Map<String, File> handle(GatewayConfig config, SimpleDescriptor de
      */
     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()) {
+        if (StringUtils.isNotBlank(desc.getDiscoveryType()) && StringUtils.isNotBlank(desc.getDiscoveryAddress())) {
             return true;
         }
         log.missingDiscoveryTypeInDescriptor(desc.getName());
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 c3d2e927f0..881c5d5bd7 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
@@ -29,7 +29,7 @@ public interface SimpleDescriptorMessages {
     void discoveryNotConfiguredForDescriptor(String descriptorName);
 
     @Message(level = MessageLevel.INFO,
-            text = "The \"{0}\" descriptor does not include discovery-type.")
+            text = "The \"{0}\" descriptor does not include discovery-type or discovery-address.")
     void missingDiscoveryTypeInDescriptor(String descriptorName);
 
     @Message(level = MessageLevel.WARN,