Repository: hazelcast/hazelcast-kubernetes (java)

Issue:
Remove 'dnsjava' dependency
Check if possible to remove the `dnsjava` dependency, so that `hazelcast-kubernetes` would not have any dependencies at all.

Pull request description:
Remove 'dnsjava' dependency
fix #97 

The implementation is based on: https://nitschinger.at/Bootstrapping-from-DNS-SRV-records-in-Java/

Tested on PKS and GKE.

Patch:
diff --git a/pom.xml b/pom.xml
index e52c1bd..2af177e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -51,7 +51,6 @@
     <powermock.version>1.7.3</powermock.version>
 
     <hazelcast.version>3.11-BETA-1</hazelcast.version>
-    <dnsjava.version>2.1.8</dnsjava.version>
 
     <maven.checkstyle.plugin.version>2.15</maven.checkstyle.plugin.version>
     <maven.findbugs.plugin.version>3.0.1</maven.findbugs.plugin.version>
@@ -122,11 +121,6 @@
       <artifactId>hazelcast</artifactId>
       <version>${hazelcast.version}</version>
     </dependency>
-    <dependency>
-      <groupId>dnsjava</groupId>
-      <artifactId>dnsjava</artifactId>
-      <version>${dnsjava.version}</version>
-    </dependency>
     <dependency>
       <groupId>com.google.code.findbugs</groupId>
       <artifactId>annotations</artifactId>
@@ -202,7 +196,8 @@
               com.hazelcast.config,com.hazelcast.config.properties,
               com.hazelcast.spi.discovery,com.hazelcast.core,
               com.hazelcast.logging,com.hazelcast.nio, com.hazelcast.util,
-              org.xbill.DNS,com.hazelcast.internal.json,javax.net.ssl,javax.security.auth.x500
+              com.hazelcast.internal.json,javax.net.ssl,javax.security.auth.x500,
+              javax.naming, javax.naming.directory
             </Import-Package>
           </instructions>
         </configuration>
diff --git a/src/main/java/com/hazelcast/kubernetes/DnsEndpointResolver.java b/src/main/java/com/hazelcast/kubernetes/DnsEndpointResolver.java
index 0770a9e..d7ab565 100644
--- a/src/main/java/com/hazelcast/kubernetes/DnsEndpointResolver.java
+++ b/src/main/java/com/hazelcast/kubernetes/DnsEndpointResolver.java
@@ -17,22 +17,25 @@
 package com.hazelcast.kubernetes;
 
 import com.hazelcast.config.NetworkConfig;
+import com.hazelcast.core.HazelcastException;
 import com.hazelcast.logging.ILogger;
 import com.hazelcast.nio.Address;
 import com.hazelcast.spi.discovery.DiscoveryNode;
 import com.hazelcast.spi.discovery.SimpleDiscoveryNode;
-import org.xbill.DNS.ExtendedResolver;
-import org.xbill.DNS.Lookup;
-import org.xbill.DNS.Record;
-import org.xbill.DNS.SRVRecord;
-import org.xbill.DNS.TextParseException;
-import org.xbill.DNS.Type;
 
+import javax.naming.Context;
+import javax.naming.NameNotFoundException;
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.Attributes;
+import javax.naming.directory.DirContext;
+import javax.naming.directory.InitialDirContext;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Hashtable;
 import java.util.List;
 import java.util.Set;
 
@@ -40,105 +43,87 @@ final class DnsEndpointResolver
         extends HazelcastKubernetesDiscoveryStrategy.EndpointResolver {
 
     private final String serviceDns;
-    private final int serviceDnsTimeout;
     private final int port;
+    private final DirContext dirContext;
 
-    DnsEndpointResolver(ILogger logger, String serviceDns, int port, int serviceDnsTimeout) {
+    DnsEndpointResolver(ILogger logger, String serviceDns, int port, DirContext dirContext) {
         super(logger);
         this.serviceDns = serviceDns;
         this.port = port;
-        this.serviceDnsTimeout = serviceDnsTimeout;
+        this.dirContext = dirContext;
     }
 
-    List<DiscoveryNode> resolve() {
-        try {
-            Lookup lookup = buildLookup();
-            Record[] records = lookup.run();
-
-            if (lookup.getResult() != Lookup.SUCCESSFUL) {
-                logger.warning("DNS lookup for serviceDns '" + serviceDns + "' failed: " + lookup.getErrorString());
-                return Collections.emptyList();
-            }
-
-            // We collect all records as some instances seem to return multiple dns records
-            Set<Address> addresses = new HashSet<Address>();
-            for (Record record : records) {
-                // Example:
-                // nslookup u219692-hazelcast.u219692-hazelcast.svc.cluster.local 172.30.0.1
-                //      Server:         172.30.0.1
-                //      Address:        172.30.0.1#53
-                //
-                //      Name:   u219692-hazelcast.u219692-hazelcast.svc.cluster.local
-                //      Address: 10.1.2.8
-                //      Name:   u219692-hazelcast.u219692-hazelcast.svc.cluster.local
-                //      Address: 10.1.5.28
-                //      Name:   u219692-hazelcast.u219692-hazelcast.svc.cluster.local
-                //      Address: 10.1.9.33
-                SRVRecord srv = (SRVRecord) record;
-                InetAddress[] inetAddress = getAllAddresses(srv);
-                int port = (this.port > 0) ? this.port : getHazelcastPort(srv.getPort());
-                for (InetAddress i : inetAddress) {
-                    Address address = new Address(i, port);
-
-                    // If address is newly discovered and logger is finest, emit log entry
-                    if (addresses.add(address) && logger.isFinestEnabled()) {
-                        logger.finest("Found node service with address: " + address);
-                    }
-                }
-            }
-
-            if (addresses.size() == 0) {
-                logger.warning("Could not find any service for serviceDns '" + serviceDns + "'");
-                return Collections.emptyList();
-            }
+    DnsEndpointResolver(ILogger logger, String serviceDns, int port, int serviceDnsTimeout) {
+        this(logger, serviceDns, port, createDirContext(serviceDnsTimeout));
 
-            return asDiscoveredNodes(addresses);
+    }
 
-        } catch (TextParseException e) {
-            throw new RuntimeException("Could not resolve services via DNS", e);
-        } catch (UnknownHostException e) {
-            throw new RuntimeException("Could not resolve services via DNS", e);
+    @SuppressWarnings("checkstyle:magicnumber")
+    private static DirContext createDirContext(int serviceDnsTimeout) {
+        Hashtable<String, String> env = new Hashtable<String, String>();
+        env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.dns.DnsContextFactory");
+        env.put(Context.PROVIDER_URL, "dns:");
+        env.put("com.sun.jndi.dns.timeout.initial", String.valueOf(serviceDnsTimeout * 1000L));
+        try {
+            return new InitialDirContext(env);
+        } catch (NamingException e) {
+            throw new HazelcastException("Error while initializing DirContext", e);
         }
     }
 
-    private Lookup buildLookup()
-            throws TextParseException, UnknownHostException {
-
-        ExtendedResolver resolver = new ExtendedResolver();
-        resolver.setTimeout(serviceDnsTimeout);
+    List<DiscoveryNode> resolve() {
+        try {
+            return lookup();
+        } catch (NameNotFoundException e) {
+            logger.warning(String.format("DNS lookup for serviceDns '%s' failed: name not found", serviceDns));
+            return Collections.emptyList();
+        } catch (Exception e) {
+            logger.warning(String.format("DNS lookup for serviceDns '%s' failed", serviceDns), e);
+            return Collections.emptyList();
+        }
+    }
 
-        Lookup lookup = new Lookup(serviceDns, Type.SRV);
-        lookup.setResolver(resolver);
+    private List<DiscoveryNode> lookup()
+            throws NamingException, UnknownHostException {
+        Attributes attributes = dirContext.getAttributes(serviceDns, new String[]{"SRV"});
+        NamingEnumeration<?> servers = attributes.get("srv").getAll();
+
+        Set<String> addresses = new HashSet<String>();
+        while (servers.hasMore()) {
+            String server = (String) servers.next();
+            String serverHost = extractHost(server);
+            InetAddress address = InetAddress.getByName(serverHost);
+            if (addresses.add(address.getHostAddress()) && logger.isFinestEnabled()) {
+                logger.finest("Found node service with address: " + address);
+            }
+        }
 
-        // Avoid caching temporary DNS lookup failures indefinitely in global cache
-        lookup.setCache(null);
+        if (addresses.size() == 0) {
+            logger.warning("Could not find any service for serviceDns '" + serviceDns + "'");
+            return Collections.emptyList();
+        }
 
-        return lookup;
+        List<DiscoveryNode> result = new ArrayList<DiscoveryNode>();
+        for (String address : addresses) {
+            result.add(new SimpleDiscoveryNode(new Address(address, getHazelcastPort(port))));
+        }
+        return result;
     }
 
-    private List<DiscoveryNode> asDiscoveredNodes(Set<Address> addresses) {
-        List<DiscoveryNode> discoveryNodes = new ArrayList<DiscoveryNode>();
-        for (Address address : addresses) {
-            discoveryNodes.add(new SimpleDiscoveryNode(address));
-        }
-        return discoveryNodes;
+    /**
+     * Extracts host from the DNS record.
+     * <p>
+     * Sample record: "10 25 0 6235386366386436.my-release-hazelcast.default.svc.cluster.local".
+     */
+    private static String extractHost(String server) {
+        String host = server.split(" ")[3];
+        return host.replaceAll("\\\\.$", "");
     }
 
-    private int getHazelcastPort(int port) {
+    private static int getHazelcastPort(int port) {
         if (port > 0) {
             return port;
         }
         return NetworkConfig.DEFAULT_PORT;
     }
-
-    private InetAddress[] getAllAddresses(SRVRecord srv)
-            throws UnknownHostException {
-
-        try {
-            return org.xbill.DNS.Address.getAllByName(srv.getTarget().canonicalize().toString(true));
-        } catch (UnknownHostException e) {
-            logger.severe("Parsing DNS records failed", e);
-            throw e;
-        }
-    }
 }


Test patch:
diff --git a/src/test/java/com/hazelcast/kubernetes/DnsEndpointResolverTest.java b/src/test/java/com/hazelcast/kubernetes/DnsEndpointResolverTest.java
index 072f434..0d3b7d6 100644
--- a/src/test/java/com/hazelcast/kubernetes/DnsEndpointResolverTest.java
+++ b/src/test/java/com/hazelcast/kubernetes/DnsEndpointResolverTest.java
@@ -24,98 +24,129 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.powermock.api.mockito.PowerMockito;
-import org.powermock.api.support.membermodification.MemberMatcher;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
-import org.xbill.DNS.Lookup;
-import org.xbill.DNS.Name;
-import org.xbill.DNS.Record;
-import org.xbill.DNS.SRVRecord;
 
+import javax.naming.NameNotFoundException;
+import javax.naming.NamingEnumeration;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.directory.DirContext;
+import java.net.InetAddress;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({DnsEndpointResolver.class, Lookup.class, SRVRecord.class})
+@PrepareForTest(DnsEndpointResolver.class)
 public class DnsEndpointResolverTest {
-
     private static final ILogger LOGGER = new NoLogFactory().getLogger("no");
-    private static final int SERVICE_DNS_TIMEOUT = 5;
 
-    @Mock
-    private DefaultKubernetesClient client;
+    private static final String SERVICE_DNS = "my-release-hazelcast.default.svc.cluster.local";
+    private static final int UNSET_PORT = 0;
+    private static final int DEFAULT_PORT = 5701;
+    private static final int CUSTOM_PORT = 5702;
+    private static final String DNS_SERVER_1 = String.format("12345.%s", SERVICE_DNS);
+    private static final String DNS_SERVER_2 = String.format("6789.%s", SERVICE_DNS);
+    private static final String DNS_ENTRY_SERVER_1 = String.format("10 25 0 %s", DNS_SERVER_1);
+    private static final String DNS_ENTRY_SERVER_2 = String.format("10 25 0 %s", DNS_SERVER_2);
+    private static final String IP_SERVER_1 = "192.168.0.5";
+    private static final String IP_SERVER_2 = "192.168.0.6";
 
     @Mock
-    private Lookup lookup;
-
+    private NamingEnumeration servers;
     @Mock
-    private SRVRecord srvRecord;
+    private DirContext dirContext;
 
     @Before
-    public void setup()
+    public void setUp()
             throws Exception {
-        PowerMockito.whenNew(DefaultKubernetesClient.class).withAnyArguments().thenReturn(client);
-        PowerMockito.whenNew(SRVRecord.class).withAnyArguments().thenReturn(srvRecord);
-        when(srvRecord.getTarget()).thenReturn(Name.fromString("127.0.0.1"));
+        PowerMockito.mockStatic(InetAddress.class);
+
+        Attributes attributes = mock(Attributes.class);
+        when(dirContext.getAttributes(SERVICE_DNS, new String[]{"SRV"})).thenReturn(attributes);
+        Attribute attribute = mock(Attribute.class);
+        when(attributes.get("srv")).thenReturn(attribute);
+        when(attribute.getAll()).thenReturn(servers);
+        when(servers.next()).thenReturn(DNS_ENTRY_SERVER_1, DNS_ENTRY_SERVER_2);
+        when(servers.hasMore()).thenReturn(true, true, false);
+        InetAddress address1 = mock(InetAddress.class);
+        PowerMockito.when(InetAddress.getByName(DNS_SERVER_1)).thenReturn(address1);
+        InetAddress address2 = mock(InetAddress.class);
+        PowerMockito.when(InetAddress.getByName(DNS_SERVER_2)).thenReturn(address2);
+        when(address1.getHostAddress()).thenReturn(IP_SERVER_1);
+        when(address2.getHostAddress()).thenReturn(IP_SERVER_2);
     }
 
     @Test
-    public void testInvalidServiceDns() {
-        DnsEndpointResolver endpointResolver = new DnsEndpointResolver(LOGGER, "http://test", 0, SERVICE_DNS_TIMEOUT);
-        List<DiscoveryNode> nodes = endpointResolver.resolve();
-        assertTrue(nodes.isEmpty());
+    public void resolve() {
+        // given
+        DnsEndpointResolver dnsEndpointResolver = new DnsEndpointResolver(LOGGER, SERVICE_DNS, UNSET_PORT, dirContext);
+
+        // when
+        List<DiscoveryNode> result = dnsEndpointResolver.resolve();
+
+        // then
+
+        Set<?> resultAddresses = setOf(result.get(0).getPrivateAddress().getHost(), result.get(1).getPrivateAddress().getHost());
+        Set<?> resultPorts = setOf(result.get(0).getPrivateAddress().getPort(), result.get(1).getPrivateAddress().getPort());
+        assertEquals(setOf(IP_SERVER_1, IP_SERVER_2), resultAddresses);
+        assertEquals(setOf(DEFAULT_PORT), resultPorts);
     }
 
     @Test
-    public void testValidServiceDns()
-            throws Exception {
-        testValidServiceDns(0, 5701);
+    public void resolveCustomPort() {
+        // given
+        DnsEndpointResolver dnsEndpointResolver = new DnsEndpointResolver(LOGGER, SERVICE_DNS, CUSTOM_PORT, dirContext);
+
+        // when
+        List<DiscoveryNode> result = dnsEndpointResolver.resolve();
+
+        // then
+
+        Set<?> resultAddresses = setOf(result.get(0).getPrivateAddress().getHost(), result.get(1).getPrivateAddress().getHost());
+        Set<?> resultPorts = setOf(result.get(0).getPrivateAddress().getPort(), result.get(1).getPrivateAddress().getPort());
+        assertEquals(setOf(IP_SERVER_1, IP_SERVER_2), resultAddresses);
+        assertEquals(setOf(CUSTOM_PORT), resultPorts);
     }
 
     @Test
-    public void testValidServiceDnsWithCustomPort()
+    public void resolveException()
             throws Exception {
-        testValidServiceDns(333, 333);
-    }
+        // given
+        when(dirContext.getAttributes(SERVICE_DNS, new String[]{"SRV"})).thenThrow(new NameNotFoundException());
+        DnsEndpointResolver dnsEndpointResolver = new DnsEndpointResolver(LOGGER, SERVICE_DNS, UNSET_PORT, dirContext);
 
-    private void testValidServiceDns(final int port, final int expectedPort)
-            throws Exception {
-        DnsEndpointResolver endpointResolver = PowerMockito
-                .spy(new DnsEndpointResolver(LOGGER, "hazelcast.com", port, SERVICE_DNS_TIMEOUT));
-        PowerMockito.when(endpointResolver, MemberMatcher.method(DnsEndpointResolver.class, "buildLookup")).withNoArguments()
-                    .thenReturn(lookup);
-        when(lookup.getResult()).thenReturn(Lookup.SUCCESSFUL);
-        when(lookup.run()).thenReturn(getRecords());
-        List<DiscoveryNode> nodes = endpointResolver.resolve();
-        assertEquals(1, nodes.size());
-        assertEquals("127.0.0.1", nodes.get(0).getPrivateAddress().getHost());
-        assertEquals(expectedPort, nodes.get(0).getPrivateAddress().getPort());
+        // when
+        List<DiscoveryNode> result = dnsEndpointResolver.resolve();
+
+        // then
+        assertEquals(0, result.size());
     }
 
     @Test
-    public void testDnsFailFlow()
+    public void resolveNotFound()
             throws Exception {
-        DnsEndpointResolver endpointResolver = PowerMockito
-                .spy(new DnsEndpointResolver(LOGGER, "hazelcast.com", 0, SERVICE_DNS_TIMEOUT));
-        PowerMockito.when(endpointResolver, MemberMatcher.method(DnsEndpointResolver.class, "buildLookup")).withNoArguments()
-                    .thenReturn(lookup);
-
-        when(lookup.getResult()).thenReturn(Lookup.HOST_NOT_FOUND);
-        when(lookup.run()).thenReturn(getRecords());
+        // given
+        when(servers.hasMore()).thenReturn(false);
+        DnsEndpointResolver dnsEndpointResolver = new DnsEndpointResolver(LOGGER, SERVICE_DNS, UNSET_PORT, dirContext);
 
-        List<DiscoveryNode> nodes = endpointResolver.resolve();
-        assertTrue(nodes.isEmpty());
+        // when
+        List<DiscoveryNode> result = dnsEndpointResolver.resolve();
 
-        verify(lookup).run();
-        verify(lookup).getResult();
-        verify(lookup).getErrorString();
+        // then
+        assertEquals(0, result.size());
     }
 
-    private Record[] getRecords() {
-        return new Record[]{srvRecord};
+    private static Set<?> setOf(Object... objects) {
+        Set<Object> result = new HashSet<Object>();
+        for (Object object : objects) {
+            result.add(object);
+        }
+        return result;
     }
 }
\ No newline at end of file