HADOOP-18087. fix bugs when looking up record from upstream DNS servers.

- add chained CNAME records to answer section
- distinguish between NXDOMAIN and NOERROR + empty answer

Author: 이유비

hadoop-common-project/hadoop-registry/src/main/java/org/apache/hadoop/registry/server/dns/LookupTask.java

@@ -22,7 +22,7 @@
 import org.xbill.DNS.Name;
 import org.xbill.DNS.Record;
 
-public class LookupTask implements Callable<Record[]> {
+public class LookupTask implements Callable<Lookup> {
 
   private Name name;
   private int type;
@@ -33,7 +33,9 @@ public LookupTask(Name name, int type) {
   }
 
   @Override
-  public Record[] call() throws Exception {
-    return new Lookup(name, type).run();
+  public Lookup call() throws Exception {
+    Lookup lookup = new Lookup(name, type);
+    lookup.run();
+    return lookup;
   }
 }

hadoop-common-project/hadoop-registry/src/main/java/org/apache/hadoop/registry/server/dns/RegistryDNS.java

@@ -1125,20 +1125,34 @@ private byte remoteLookup(Message response, Name name, int type,
       type = Type.NS;
     }
 
-    // Always add any CNAMEs to the response first
+    // Support for CNAME chaining
     if (type != Type.CNAME) {
-      Record[] cnameAnswers = getRecords(name, Type.CNAME);
-      if (cnameAnswers != null) {
+      Name targetName = name;
+      while (iterations < 6) {
+        Record[] cnameAnswers = getRecords(targetName, Type.CNAME).answers;
+        if (cnameAnswers == null) {
+          break;
+        }
         for (Record cnameR : cnameAnswers) {
           if (!response.findRecord(cnameR)) {
             response.addRecord(cnameR, Section.ANSWER);
+            targetName = ((CNAMERecord) cnameR).getTarget();
           }
         }
+        iterations++;
+      }
+      if (iterations < 6 && !targetName.equals(name)) {
+        return remoteLookup(response, targetName, type, iterations + 1);
       }
     }
 
     // Forward lookup to primary DNS servers
-    Record[] answers = getRecords(name, type);
+    RemoteAnswer ra = getRecords(name, type);
+    Record[] answers = ra.answers;
+    // no answer
+    if (answers == null) {
+      return (byte)ra.rcode;
+    }
     try {
       for (Record r : answers) {
         if (!response.findRecord(r)) {
@@ -1148,12 +1162,6 @@ private byte remoteLookup(Message response, Name name, int type,
             response.addRecord(r, Section.ANSWER);
           }
         }
-        if (r.getType() == Type.CNAME) {
-          Name cname = r.getName();
-          if (iterations < 6) {
-            remoteLookup(response, cname, type, iterations + 1);
-          }
-        }
       }
     } catch (NullPointerException e) {
       return Rcode.NXDOMAIN;
@@ -1168,20 +1176,32 @@ private byte remoteLookup(Message response, Name name, int type,
    *
    * @param name - query string
    * @param type - type of DNS record to lookup
-   * @return DNS records
+   * @return DNS records and return code from remote DNS server
    */
-  protected Record[] getRecords(Name name, int type) {
-    Record[] result = null;
+  protected RemoteAnswer getRecords(Name name, int type) {
     ExecutorService executor = Executors.newSingleThreadExecutor();
-    Future<Record[]> future = executor.submit(new LookupTask(name, type));
+    Future<Lookup> future = executor.submit(new LookupTask(name, type));
     try {
-      result = future.get(1500, TimeUnit.MILLISECONDS);
-      return result;
+      Lookup lookup = future.get(1500, TimeUnit.MILLISECONDS);
+      int result = lookup.getResult();
+      int rcode = Rcode.NXDOMAIN;
+      switch (result) {
+        case Lookup.SUCCESSFUL:
+        case Lookup.TYPE_NOT_FOUND:
+          rcode = Rcode.NOERROR;
+          break;
+        case Lookup.HOST_NOT_FOUND:
+          break;
+        default:
+          LOG.warn("Unexpected result from lookup: {} type: {} error: {}", name, Type.string(type), lookup.getErrorString());
+          break;
+      }
+      return new RemoteAnswer(lookup.getAnswers(), rcode);
     } catch (InterruptedException | ExecutionException |
         TimeoutException | NullPointerException |
         ExceptionInInitializerError e) {
       LOG.warn("Failed to lookup: {} type: {}", name, Type.string(type), e);
-      return result;
+      return new RemoteAnswer(null, Rcode.NXDOMAIN);
     } finally {
       executor.shutdown();
     }
@@ -1778,4 +1798,14 @@ public void close() {
       lock.unlock();
     }
   }
+
+  public static class RemoteAnswer {
+    public Record[] answers;
+    public int rcode;
+
+    public RemoteAnswer(Record[] answers, int rcode) {
+      this.answers = answers;
+      this.rcode = rcode;
+    }
+  }
 }

hadoop-common-project/hadoop-registry/src/test/java/org/apache/hadoop/registry/server/dns/TestRegistryDNS.java

@@ -642,7 +642,8 @@ public void testSplitReverseZoneNames() throws Exception {
   @Test
   public void testExampleDotCom() throws Exception {
     Name name = Name.fromString("example.com.");
-    Record[] records = getRegistryDNS().getRecords(name, Type.SOA);
+    RegistryDNS.RemoteAnswer ra = getRegistryDNS().getRecords(name, Type.SOA);
+    Record[] records = ra.answers;
     assertNotNull("example.com exists:", records);
   }
 
@@ -703,10 +704,19 @@ public void testMultiARecord() throws Exception {
   @Test(timeout=5000)
   public void testUpstreamFault() throws Exception {
     Name name = Name.fromString("19.0.17.172.in-addr.arpa.");
-    Record[] recs = getRegistryDNS().getRecords(name, Type.CNAME);
+    RegistryDNS.RemoteAnswer ra = getRegistryDNS().getRecords(name, Type.CNAME);
+    Record[] recs = ra.answers;
     assertNull("Record is not null", recs);
   }
 
+  @Test
+  public void testNODATA() throws Exception {
+    Name name = Name.fromString("example.com.");
+    RegistryDNS.RemoteAnswer ra = getRegistryDNS().getRecords(name, Type.CNAME);
+    assertNull("CNAME record for example.com. should be null.", ra.answers);
+    assertEquals("The result of DNS query for example.com. should be NOERROR.", Rcode.NOERROR, ra.rcode);
+  }
+
   public RegistryDNS getRegistryDNS() {
     return registryDNS;
   }