Repository: knox
Updated Branches:
  refs/heads/master fc0e59a9c -> 8a1f03b1c


KNOX-1287 - Invalid federated namenode namespace declaration should not produce 
invalid URLs in topologies.


Project: http://git-wip-us.apache.org/repos/asf/knox/repo
Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/8a1f03b1
Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/8a1f03b1
Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/8a1f03b1

Branch: refs/heads/master
Commit: 8a1f03b1c63d8b445302dc1ed46b033086fcb9be
Parents: fc0e59a
Author: Phil Zampino <pzamp...@apache.org>
Authored: Wed May 2 22:55:16 2018 -0400
Committer: Phil Zampino <pzamp...@apache.org>
Committed: Thu May 3 11:37:59 2018 -0400

----------------------------------------------------------------------
 .../discovery/ambari/HDFSURLCreatorBase.java    |  21 +-
 .../discovery/ambari/NameNodeUrlCreator.java    |  16 +-
 .../AmbariDynamicServiceURLCreatorTest.java     |  64 ++----
 .../discovery/ambari/WebHdfsUrlCreatorTest.java | 216 +++++++++++++++++--
 .../simple/SimpleDescriptorHandler.java         |  20 +-
 5 files changed, 255 insertions(+), 82 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/knox/blob/8a1f03b1/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/HDFSURLCreatorBase.java
----------------------------------------------------------------------
diff --git 
a/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/HDFSURLCreatorBase.java
 
b/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/HDFSURLCreatorBase.java
index 54fa87e..5a04e7e 100644
--- 
a/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/HDFSURLCreatorBase.java
+++ 
b/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/HDFSURLCreatorBase.java
@@ -59,22 +59,18 @@ public abstract class HDFSURLCreatorBase implements 
ServiceURLCreator {
                                       
cluster.getServiceConfiguration(CONFIG_SERVICE_HDFS, CONFIG_TYPE_HDFS_SITE);
       if (sc != null) {
         // First, check if it's HA config
-        String nameServices = null;
-        AmbariComponent nameNodeComp = 
cluster.getComponent(CONFIG_SERVICE_NAMENODE);
-        if (nameNodeComp != null) {
-          nameServices = nameNodeComp.getConfigProperty("dfs.nameservices");
-        }
-
+        String nameServices = sc.getProperties().get("dfs.nameservices");
         if (nameServices != null && !nameServices.isEmpty()) {
           String ns = null;
 
           // Parse the nameservices value
           String[] namespaces = nameServices.split(",");
 
-          if (namespaces.length > 1) {
-            String nsParam = (serviceParams != null) ? 
serviceParams.get(NAMESERVICE_PARAM) : null;
+          String nsParam = (serviceParams != null) ? 
serviceParams.get(NAMESERVICE_PARAM) : null;
+
+          if (namespaces.length > 1 || nsParam != null) {
             if (nsParam != null) {
-              if (!validateDeclaredNameService(sc, nsParam)) {
+              if (!validateDeclaredNameService(namespaces, nsParam)) {
                 log.undefinedHDFSNameService(nsParam);
               }
               ns = nsParam;
@@ -136,12 +132,9 @@ public abstract class HDFSURLCreatorBase implements 
ServiceURLCreator {
 
 
   // Verify whether the declared nameservice is among the configured 
nameservices in the cluster
-  private static boolean 
validateDeclaredNameService(AmbariCluster.ServiceConfiguration hdfsSite,
-                                                     String                    
         declaredNameService) {
+  private static boolean validateDeclaredNameService(String[] namespaces, 
String declaredNameService) {
     boolean isValid = false;
-    String nameservices = hdfsSite.getProperties().get("dfs.nameservices");
-    if (nameservices != null) {
-      String[] namespaces = nameservices.split(",");
+    if (namespaces != null) {
       for (String ns : namespaces) {
         if (ns.equals(declaredNameService)) {
           isValid = true;

http://git-wip-us.apache.org/repos/asf/knox/blob/8a1f03b1/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/NameNodeUrlCreator.java
----------------------------------------------------------------------
diff --git 
a/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/NameNodeUrlCreator.java
 
b/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/NameNodeUrlCreator.java
index 478d12f..ff071e8 100644
--- 
a/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/NameNodeUrlCreator.java
+++ 
b/gateway-discovery-ambari/src/main/java/org/apache/knox/gateway/topology/discovery/ambari/NameNodeUrlCreator.java
@@ -53,9 +53,9 @@ public class NameNodeUrlCreator implements ServiceURLCreator {
         // Validate declared nameservice against available nameservices
         if (!validateDeclaredNameService(cluster, declaredNameService)) {
           log.undefinedHDFSNameService(declaredNameService);
+        } else {
+          urls.add("hdfs://" + declaredNameService);
         }
-
-        urls.add("hdfs://" + declaredNameService);
       } else {
         // Add the default nameservice URL to the result
         AmbariCluster.ServiceConfiguration coreSite = 
cluster.getServiceConfiguration("HDFS", "core-site");
@@ -74,11 +74,13 @@ public class NameNodeUrlCreator implements 
ServiceURLCreator {
   // Verify whether the declared nameservice is among the configured 
nameservices in the cluster
   private static boolean validateDeclaredNameService(AmbariCluster cluster, 
String declaredNameService) {
     boolean isValid = false;
-    AmbariCluster.ServiceConfiguration hdfsSite = 
cluster.getServiceConfiguration("HDFS", "hdfs-site");
-    if (hdfsSite != null) {
-      String nameservices = hdfsSite.getProperties().get("dfs.nameservices");
-      if (nameservices != null) {
-        String[] namespaces = nameservices.split(",");
+
+    AmbariComponent nameNodeComp = cluster.getComponent(SERVICE);
+    if (nameNodeComp != null) {
+      String nameServices = nameNodeComp.getConfigProperty("dfs.nameservices");
+      if (nameServices != null && !nameServices.isEmpty()) {
+        // Parse the nameservices value
+        String[] namespaces = nameServices.split(",");
         for (String ns : namespaces) {
           if (ns.equals(declaredNameService)) {
             isValid = true;

http://git-wip-us.apache.org/repos/asf/knox/blob/8a1f03b1/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/AmbariDynamicServiceURLCreatorTest.java
----------------------------------------------------------------------
diff --git 
a/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/AmbariDynamicServiceURLCreatorTest.java
 
b/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/AmbariDynamicServiceURLCreatorTest.java
index 03612c1..2a63535 100644
--- 
a/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/AmbariDynamicServiceURLCreatorTest.java
+++ 
b/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/AmbariDynamicServiceURLCreatorTest.java
@@ -36,6 +36,7 @@ import static junit.framework.TestCase.fail;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 
 
 public class AmbariDynamicServiceURLCreatorTest {
@@ -181,9 +182,7 @@ public class AmbariDynamicServiceURLCreatorTest {
         params.put("discovery-nameservice", delcaredNS);
 
         String url = getTestNameNodeURL("nn1", "ns1,ns2", params);
-        assertEquals("Invalid nameservice declaration should still yield a 
URL.",
-                     "hdfs://" + delcaredNS,
-                     url);
+        assertNull("Invalid nameservice declaration should still yield a 
URL.", url);
     }
 
     private String getTestNameNodeURL(String address) throws Exception {
@@ -208,17 +207,24 @@ public class AmbariDynamicServiceURLCreatorTest {
         }
         
EasyMock.expect(hdfsSC.getProperties()).andReturn(hdfsProps).anyTimes();
         EasyMock.replay(hdfsSC);
+        AmbariComponent nnComp = 
EasyMock.createNiceMock(AmbariComponent.class);
+        
EasyMock.expect(nnComp.getConfigProperty("dfs.nameservices")).andReturn(nameservices).anyTimes();
+        EasyMock.replay(nnComp);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"core-site")).andReturn(coreSC).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
+        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(nnComp).anyTimes();
         EasyMock.replay(cluster);
 
         // Create the URL
         List<String> urls = 
ServiceURLFactory.newInstance(cluster).create("NAMENODE", serviceParams);
         assertNotNull(urls);
-        assertFalse(urls.isEmpty());
-        return urls.get(0);
+        String url = null;
+        if (!urls.isEmpty()) {
+            url = urls.get(0);
+        }
+        return url;
     }
 
 
@@ -399,19 +405,15 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_1 + "/webhdfs";
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_2 + "/webhdfs";
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NAMESERVICES + ".nn1", 
HTTP_ADDRESS_1);
         hdfsProps.put("dfs.namenode.http-address." + NAMESERVICES + ".nn2", 
HTTP_ADDRESS_2);
         
EasyMock.expect(hdfsSC.getProperties()).andReturn(hdfsProps).anyTimes();
         EasyMock.replay(hdfsSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.replay(cluster);
 
@@ -432,19 +434,15 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_1;
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_2;
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NAMESERVICES + ".nn1", 
HTTP_ADDRESS_1);
         hdfsProps.put("dfs.namenode.http-address." + NAMESERVICES + ".nn2", 
HTTP_ADDRESS_2);
         
EasyMock.expect(hdfsSC.getProperties()).andReturn(hdfsProps).anyTimes();
         EasyMock.replay(hdfsSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.replay(cluster);
 
@@ -473,12 +471,9 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_11 + 
"/webhdfs";
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_12 + 
"/webhdfs";
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn1", 
HTTP_ADDRESS_11);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn2", 
HTTP_ADDRESS_12);
         hdfsProps.put("dfs.namenode.http-address." + NS2 + ".nn1", 
HTTP_ADDRESS_21);
@@ -487,7 +482,6 @@ public class AmbariDynamicServiceURLCreatorTest {
         EasyMock.replay(hdfsSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.replay(cluster);
 
@@ -516,12 +510,9 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_11;
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_12;
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn1", 
HTTP_ADDRESS_11);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn2", 
HTTP_ADDRESS_12);
         hdfsProps.put("dfs.namenode.http-address." + NS2 + ".nn1", 
HTTP_ADDRESS_21);
@@ -530,7 +521,6 @@ public class AmbariDynamicServiceURLCreatorTest {
         EasyMock.replay(hdfsSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.replay(cluster);
 
@@ -558,12 +548,9 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_21 + 
"/webhdfs";
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_22 + 
"/webhdfs";
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn1", 
HTTP_ADDRESS_11);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn2", 
HTTP_ADDRESS_12);
         hdfsProps.put("dfs.namenode.http-address." + NS2 + ".nn1", 
HTTP_ADDRESS_21);
@@ -578,7 +565,6 @@ public class AmbariDynamicServiceURLCreatorTest {
         EasyMock.replay(coreSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"core-site")).andReturn(coreSC).anyTimes();
         EasyMock.replay(cluster);
@@ -607,12 +593,9 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_21;
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_22;
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn1", 
HTTP_ADDRESS_11);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn2", 
HTTP_ADDRESS_12);
         hdfsProps.put("dfs.namenode.http-address." + NS2 + ".nn1", 
HTTP_ADDRESS_21);
@@ -627,7 +610,6 @@ public class AmbariDynamicServiceURLCreatorTest {
         EasyMock.replay(coreSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"core-site")).andReturn(coreSC).anyTimes();
         EasyMock.replay(cluster);
@@ -658,12 +640,9 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_21 + 
"/webhdfs";
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_22 + 
"/webhdfs";
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn11", 
HTTP_ADDRESS_11);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn12", 
HTTP_ADDRESS_12);
         hdfsProps.put("dfs.namenode.http-address." + NS2 + ".nn21", 
HTTP_ADDRESS_21);
@@ -680,7 +659,6 @@ public class AmbariDynamicServiceURLCreatorTest {
         EasyMock.replay(coreSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"core-site")).andReturn(coreSC).anyTimes();
         EasyMock.replay(cluster);
@@ -711,12 +689,9 @@ public class AmbariDynamicServiceURLCreatorTest {
         final String EXPECTED_ADDR_1 = "http://"; + HTTP_ADDRESS_21;
         final String EXPECTED_ADDR_2 = "http://"; + HTTP_ADDRESS_22;
 
-        AmbariComponent namenode = 
EasyMock.createNiceMock(AmbariComponent.class);
-        
EasyMock.expect(namenode.getConfigProperty("dfs.nameservices")).andReturn(NAMESERVICES).anyTimes();
-        EasyMock.replay(namenode);
-
         AmbariCluster.ServiceConfiguration hdfsSC = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
         Map<String, String> hdfsProps = new HashMap<>();
+        hdfsProps.put("dfs.nameservices", NAMESERVICES);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn11", 
HTTP_ADDRESS_11);
         hdfsProps.put("dfs.namenode.http-address." + NS1 + ".nn12", 
HTTP_ADDRESS_12);
         hdfsProps.put("dfs.namenode.http-address." + NS2 + ".nn21", 
HTTP_ADDRESS_21);
@@ -733,7 +708,6 @@ public class AmbariDynamicServiceURLCreatorTest {
         EasyMock.replay(coreSC);
 
         AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-        
EasyMock.expect(cluster.getComponent("NAMENODE")).andReturn(namenode).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"hdfs-site")).andReturn(hdfsSC).anyTimes();
         EasyMock.expect(cluster.getServiceConfiguration("HDFS", 
"core-site")).andReturn(coreSC).anyTimes();
         EasyMock.replay(cluster);

http://git-wip-us.apache.org/repos/asf/knox/blob/8a1f03b1/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/WebHdfsUrlCreatorTest.java
----------------------------------------------------------------------
diff --git 
a/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/WebHdfsUrlCreatorTest.java
 
b/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/WebHdfsUrlCreatorTest.java
index 46a2474..e2f8db4 100644
--- 
a/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/WebHdfsUrlCreatorTest.java
+++ 
b/gateway-discovery-ambari/src/test/java/org/apache/knox/gateway/topology/discovery/ambari/WebHdfsUrlCreatorTest.java
@@ -98,10 +98,6 @@ public class WebHdfsUrlCreatorTest {
     final String HTTP_ADDRESS  = "host1:20070";
     final String HTTPS_ADDRESS = "host2:20470";
 
-    AmbariComponent nnComp = EasyMock.createNiceMock(AmbariComponent.class);
-    
EasyMock.expect(nnComp.getConfigProperty("dfs.nameservices")).andReturn("X,Y").anyTimes();
-    EasyMock.replay(nnComp);
-
     AmbariCluster.ServiceConfiguration coreSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
     Map<String, String> corSiteProps = new HashMap<>();
     corSiteProps.put("fs.defaultFS", "hdfs://X");
@@ -113,6 +109,7 @@ public class WebHdfsUrlCreatorTest {
     configProps.put(HDFSURLCreatorBase.HTTP_POLICY_PROPERTY, 
HDFSURLCreatorBase.HTTP_ONLY_POLICY);
     configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY, HTTP_ADDRESS);
     configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY, HTTPS_ADDRESS);
+    configProps.put("dfs.nameservices", "X,Y");
     configProps.put("dfs.ha.namenodes.X", "nn1,nn2");
     configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY + ".X.nn1", 
"host3:20070");
     configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY + ".X.nn2", 
"host4:20070");
@@ -121,9 +118,6 @@ public class WebHdfsUrlCreatorTest {
     EasyMock.replay(hdfsSiteConfig);
 
     AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
-    EasyMock.expect(cluster.getComponent("NAMENODE"))
-            .andReturn(nnComp)
-            .anyTimes();
     EasyMock.expect(cluster.getServiceConfiguration("HDFS", "hdfs-site"))
             .andReturn(hdfsSiteConfig)
             .anyTimes();
@@ -148,6 +142,147 @@ public class WebHdfsUrlCreatorTest {
     final String HTTP_ADDRESS  = "host1:20070";
     final String HTTPS_ADDRESS = "host2:20470";
 
+    AmbariCluster.ServiceConfiguration coreSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> corSiteProps = new HashMap<>();
+    corSiteProps.put("fs.defaultFS", "hdfs://Y");
+    
EasyMock.expect(coreSiteConfig.getProperties()).andReturn(corSiteProps).anyTimes();
+    EasyMock.replay(coreSiteConfig);
+
+    AmbariCluster.ServiceConfiguration hdfsSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> configProps = new HashMap<>();
+    configProps.put(HDFSURLCreatorBase.HTTP_POLICY_PROPERTY, 
HDFSURLCreatorBase.HTTPS_ONLY_POLICY);
+    configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY, HTTP_ADDRESS);
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY, HTTPS_ADDRESS);
+    configProps.put("dfs.nameservices", "X,Y");
+    configProps.put("dfs.ha.namenodes.Y", "nn7,nn8");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn7", 
"host5:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn8", 
"host6:20470");
+
+    
EasyMock.expect(hdfsSiteConfig.getProperties()).andReturn(configProps).anyTimes();
+    EasyMock.replay(hdfsSiteConfig);
+
+    AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "hdfs-site"))
+            .andReturn(hdfsSiteConfig)
+            .anyTimes();
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "core-site"))
+            .andReturn(coreSiteConfig)
+            .anyTimes();
+    EasyMock.replay(cluster);
+
+    WebHdfsUrlCreator c = new WebHdfsUrlCreator();
+    c.init(cluster);
+    List<String> urls = c.create("WEBHDFS", null);
+    assertNotNull(urls);
+    assertFalse(urls.isEmpty());
+    assertEquals(2, urls.size());
+    assertTrue(urls.contains("https://"; + "host5:20470" + "/webhdfs"));
+    assertTrue(urls.contains("https://"; + "host6:20470" + "/webhdfs"));
+  }
+
+
+  @Test
+  public void testFederatedHttpsWebHdfsEndpointAddressWithValidNS() {
+    final String HTTP_ADDRESS  = "host1:20070";
+    final String HTTPS_ADDRESS = "host2:20470";
+
+    AmbariCluster.ServiceConfiguration coreSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> corSiteProps = new HashMap<>();
+    corSiteProps.put("fs.defaultFS", "hdfs://Y");
+    
EasyMock.expect(coreSiteConfig.getProperties()).andReturn(corSiteProps).anyTimes();
+    EasyMock.replay(coreSiteConfig);
+
+    AmbariCluster.ServiceConfiguration hdfsSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> configProps = new HashMap<>();
+    configProps.put(HDFSURLCreatorBase.HTTP_POLICY_PROPERTY, 
HDFSURLCreatorBase.HTTPS_ONLY_POLICY);
+    configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY, HTTP_ADDRESS);
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY, HTTPS_ADDRESS);
+    configProps.put("dfs.nameservices", "X,Y");
+    configProps.put("dfs.ha.namenodes.X", "nn2,nn3");
+    configProps.put("dfs.ha.namenodes.Y", "nn7,nn8");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".X.nn2", 
"host3:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".X.nn3", 
"host4:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn7", 
"host5:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn8", 
"host6:20470");
+
+    
EasyMock.expect(hdfsSiteConfig.getProperties()).andReturn(configProps).anyTimes();
+    EasyMock.replay(hdfsSiteConfig);
+
+    AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "hdfs-site"))
+            .andReturn(hdfsSiteConfig)
+            .anyTimes();
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "core-site"))
+            .andReturn(coreSiteConfig)
+            .anyTimes();
+    EasyMock.replay(cluster);
+
+    Map<String, String> serviceParams = new HashMap<>();
+    serviceParams.put("discovery-nameservice", "X");
+
+    WebHdfsUrlCreator c = new WebHdfsUrlCreator();
+    c.init(cluster);
+    List<String> urls = c.create("WEBHDFS", serviceParams);
+    assertNotNull(urls);
+    assertFalse(urls.isEmpty());
+    assertEquals(2, urls.size());
+    assertTrue(urls.contains("https://"; + "host3:20470" + "/webhdfs"));
+    assertTrue(urls.contains("https://"; + "host4:20470" + "/webhdfs"));
+  }
+
+
+  @Test
+  public void testFederatedHttpsWebHdfsEndpointAddressWithInvalidNS() {
+    final String HTTP_ADDRESS  = "host1:20070";
+    final String HTTPS_ADDRESS = "host2:20470";
+
+    AmbariCluster.ServiceConfiguration coreSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> corSiteProps = new HashMap<>();
+    corSiteProps.put("fs.defaultFS", "hdfs://Y");
+    
EasyMock.expect(coreSiteConfig.getProperties()).andReturn(corSiteProps).anyTimes();
+    EasyMock.replay(coreSiteConfig);
+
+    AmbariCluster.ServiceConfiguration hdfsSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> configProps = new HashMap<>();
+    configProps.put(HDFSURLCreatorBase.HTTP_POLICY_PROPERTY, 
HDFSURLCreatorBase.HTTPS_ONLY_POLICY);
+    configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY, HTTP_ADDRESS);
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY, HTTPS_ADDRESS);
+    configProps.put("dfs.nameservices", "X,Y");
+    configProps.put("dfs.ha.namenodes.X", "nn2,nn3");
+    configProps.put("dfs.ha.namenodes.Y", "nn7,nn8");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".X.nn2", 
"host3:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".X.nn3", 
"host4:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn7", 
"host5:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn8", 
"host6:20470");
+
+    
EasyMock.expect(hdfsSiteConfig.getProperties()).andReturn(configProps).anyTimes();
+    EasyMock.replay(hdfsSiteConfig);
+
+    AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "hdfs-site"))
+            .andReturn(hdfsSiteConfig)
+            .anyTimes();
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "core-site"))
+            .andReturn(coreSiteConfig)
+            .anyTimes();
+    EasyMock.replay(cluster);
+
+    Map<String, String> serviceParams = new HashMap<>();
+    serviceParams.put("discovery-nameservice", "Z");
+
+    WebHdfsUrlCreator c = new WebHdfsUrlCreator();
+    c.init(cluster);
+    List<String> urls = c.create("WEBHDFS", serviceParams);
+    assertNotNull(urls);
+    assertTrue(urls.isEmpty());
+  }
+
+
+  @Test
+  public void testFederatedHttpsNamenodeEndpointAddressWithValidDeclaredNS() {
+    final String HTTP_ADDRESS  = "host1:20070";
+    final String HTTPS_ADDRESS = "host2:20470";
+
     AmbariComponent nnComp = EasyMock.createNiceMock(AmbariComponent.class);
     
EasyMock.expect(nnComp.getConfigProperty("dfs.nameservices")).andReturn("X,Y").anyTimes();
     EasyMock.replay(nnComp);
@@ -163,7 +298,10 @@ public class WebHdfsUrlCreatorTest {
     configProps.put(HDFSURLCreatorBase.HTTP_POLICY_PROPERTY, 
HDFSURLCreatorBase.HTTPS_ONLY_POLICY);
     configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY, HTTP_ADDRESS);
     configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY, HTTPS_ADDRESS);
+    configProps.put("dfs.ha.namenodes.X", "nn2,nn3");
     configProps.put("dfs.ha.namenodes.Y", "nn7,nn8");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".X.nn2", 
"host3:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".X.nn3", 
"host4:20470");
     configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn7", 
"host5:20470");
     configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn8", 
"host6:20470");
 
@@ -182,14 +320,68 @@ public class WebHdfsUrlCreatorTest {
             .anyTimes();
     EasyMock.replay(cluster);
 
-    WebHdfsUrlCreator c = new WebHdfsUrlCreator();
+    NameNodeUrlCreator c = new NameNodeUrlCreator();
     c.init(cluster);
-    List<String> urls = c.create("WEBHDFS", null);
+
+    Map<String, String> serviceParams = new HashMap<>();
+    serviceParams.put("discovery-nameservice", "X");
+
+    List<String> urls = c.create("NAMENODE", serviceParams);
     assertNotNull(urls);
     assertFalse(urls.isEmpty());
-    assertEquals(2, urls.size());
-    assertTrue(urls.contains("https://"; + "host5:20470" + "/webhdfs"));
-    assertTrue(urls.contains("https://"; + "host6:20470" + "/webhdfs"));
+    assertEquals(1, urls.size());
+    assertTrue(urls.contains("hdfs://X"));
+  }
+
+
+  @Test
+  public void testFederatedHttpsNamenodeEndpointAddressWithInvalidDeclaredNS() 
{
+    final String HTTP_ADDRESS  = "host1:20070";
+    final String HTTPS_ADDRESS = "host2:20470";
+
+    AmbariComponent nnComp = EasyMock.createNiceMock(AmbariComponent.class);
+    
EasyMock.expect(nnComp.getConfigProperty("dfs.nameservices")).andReturn("X,Y").anyTimes();
+    EasyMock.replay(nnComp);
+
+    AmbariCluster.ServiceConfiguration coreSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> corSiteProps = new HashMap<>();
+    corSiteProps.put("fs.defaultFS", "hdfs://Y");
+    
EasyMock.expect(coreSiteConfig.getProperties()).andReturn(corSiteProps).anyTimes();
+    EasyMock.replay(coreSiteConfig);
+
+    AmbariCluster.ServiceConfiguration hdfsSiteConfig = 
EasyMock.createNiceMock(AmbariCluster.ServiceConfiguration.class);
+    Map<String, String> configProps = new HashMap<>();
+    configProps.put(HDFSURLCreatorBase.HTTP_POLICY_PROPERTY, 
HDFSURLCreatorBase.HTTPS_ONLY_POLICY);
+    configProps.put(HDFSURLCreatorBase.HTTP_ADDRESS_PROPERTY, HTTP_ADDRESS);
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY, HTTPS_ADDRESS);
+    configProps.put("dfs.ha.namenodes.Y", "nn7,nn8");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn7", 
"host5:20470");
+    configProps.put(HDFSURLCreatorBase.HTTPS_ADDRESS_PROPERTY + ".Y.nn8", 
"host6:20470");
+
+    
EasyMock.expect(hdfsSiteConfig.getProperties()).andReturn(configProps).anyTimes();
+    EasyMock.replay(hdfsSiteConfig);
+
+    AmbariCluster cluster = EasyMock.createNiceMock(AmbariCluster.class);
+    EasyMock.expect(cluster.getComponent("NAMENODE"))
+            .andReturn(nnComp)
+            .anyTimes();
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "hdfs-site"))
+            .andReturn(hdfsSiteConfig)
+            .anyTimes();
+    EasyMock.expect(cluster.getServiceConfiguration("HDFS", "core-site"))
+            .andReturn(coreSiteConfig)
+            .anyTimes();
+    EasyMock.replay(cluster);
+
+    NameNodeUrlCreator c = new NameNodeUrlCreator();
+    c.init(cluster);
+
+    Map<String, String> serviceParams = new HashMap<>();
+    serviceParams.put("discovery-nameservice", "Z");
+
+    List<String> urls = c.create("NAMENODE", serviceParams);
+    assertNotNull(urls);
+    assertTrue(urls.isEmpty());
   }
 
 }

http://git-wip-us.apache.org/repos/asf/knox/blob/8a1f03b1/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
----------------------------------------------------------------------
diff --git 
a/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
 
b/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
index 28a8094..89020d8 100644
--- 
a/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
+++ 
b/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
@@ -158,10 +158,22 @@ public class SimpleDescriptorHandler {
             }
 
             // Service params
-            if (descService.getParams() != null) {
-                serviceParams.put(serviceName, descService.getParams());
-                if (!validServiceNames.contains(serviceName)) {
-                    validServiceNames.add(serviceName);
+            Map<String, String> descriptorServiceParams = 
descService.getParams();
+            if (descriptorServiceParams != null && 
!descriptorServiceParams.isEmpty()) {
+                boolean hasNonDiscoveryParams = false;
+                // Determine if there are any params which are not 
discovery-only
+                for (String paramName : descriptorServiceParams.keySet()) {
+                    if (!paramName.startsWith(DISCOVERY_PARAM_PREFIX)) {
+                        hasNonDiscoveryParams = true;
+                        break;
+                    }
+                }
+                // Don't add the service if the only params are discovery-only 
params
+                if (hasNonDiscoveryParams) {
+                    serviceParams.put(serviceName, descService.getParams());
+                    if (!validServiceNames.contains(serviceName)) {
+                        validServiceNames.add(serviceName);
+                    }
                 }
             }
         }

Reply via email to