HW verification of IP

Hosts with HOST_NETWORK fails hardware divergence check of IP addresses because
the reverse lookup returns the node hostname (this is the whole point of the
host network - the container requires reverse lookup to pass, but not the
host). For HOST_NETWORK we'll disable this check.

We may want to add verification of reverse lookup of container IP addresses
later.

To compensate, forward resolution (hostname -> IP addresses) has been added.

3 files changed, 120 insertions, 22 deletions

diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifier.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifier.java
index 496bf47bd7c..69897728739 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifier.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifier.java
@@ -9,7 +9,10 @@ import javax.naming.directory.Attribute;
 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.Arrays;
 import java.util.Enumeration;
 import java.util.Hashtable;
 import java.util.List;
@@ -28,9 +31,11 @@ public class IPAddressVerifier {
     private static final Logger logger = Logger.getLogger(IPAddressVerifier.class.getName());
 
     private final String expectedHostname;
+    private final boolean skipReverseLookup;
 
-    public IPAddressVerifier(String expectedHostname) {
+    public IPAddressVerifier(String expectedHostname, boolean skipReverseLookup) {
         this.expectedHostname = expectedHostname;
+        this.skipReverseLookup = skipReverseLookup;
     }
 
     public void reportFaultyIpAddresses(NodeSpec nodeSpec, SpecVerificationReport specVerificationReport) {
@@ -44,43 +49,80 @@ public class IPAddressVerifier {
         List<String> faultyIpAddresses = new ArrayList<>();
         if (expectedHostname == null || expectedHostname.equals(""))
             return new String[0];
-        if (!isValidIpv4(nodeSpec.getIpv4Address(), expectedHostname)) {
+
+        if (!isValidIpv4(nodeSpec.getIpv4Address())) {
             faultyIpAddresses.add(nodeSpec.getIpv4Address());
         }
-        if (!isValidIpv6(nodeSpec.getIpv6Address(), expectedHostname)) {
+        if (!isValidIpv6(nodeSpec.getIpv6Address())) {
             faultyIpAddresses.add(nodeSpec.getIpv6Address());
         }
         return faultyIpAddresses.toArray(new String[0]);
     }
 
-    private boolean isValidIpv4(String ipv4Address, String expectedHostname) {
-        if (ipv4Address == null) {
-            return true;
+    private boolean hostnameResolvesToIpAddress(String ipAddress) {
+        InetAddress addressFromIpAddress;
+        try {
+            addressFromIpAddress = InetAddress.getByName(ipAddress);
+        } catch (UnknownHostException e) {
+            logger.log(Level.WARNING, "Failed to parse IP address " + ipAddress, e);
+            return false;
         }
-        String ipv4LookupFormat = convertIpv4ToLookupFormat(ipv4Address);
+
+        List<InetAddress> addressesFromHostname;
         try {
-            String ipv4Hostname = reverseLookUp(ipv4LookupFormat);
-            return ipv4Hostname.equals(expectedHostname);
-        } catch (NamingException e) {
-            logger.log(Level.WARNING, "Could not get IPv4 hostname", e);
+            addressesFromHostname = mockableGetAllByName(expectedHostname);
+        } catch (UnknownHostException e) {
+            logger.log(Level.WARNING, "Failed to get IP addresses of hostname " + expectedHostname, e);
+            return false;
         }
-        return false;
+
+        if (addressesFromHostname.stream().noneMatch(addressFromIpAddress::equals)) {
+            logger.log(Level.WARNING, "Hostname " + expectedHostname + " resolved to " + addressesFromHostname +
+                            " which does not contain the IP address " + ipAddress);
+            return false;
+        }
+
+        return true;
     }
 
-    private boolean isValidIpv6(String ipv6Address, String expectedHostname) {
-        if (ipv6Address == null) {
+    private boolean ipAddressResolvesToHostname(String ipAddressLookupFormat) {
+        if (skipReverseLookup) {
             return true;
         }
-        String ipv6LookupFormat = convertIpv6ToLookupFormat(ipv6Address);
+
         try {
-            String ipv6Hostname = reverseLookUp(ipv6LookupFormat);
-            return ipv6Hostname.equals(expectedHostname);
+            String hostnameFromIpAddress = reverseLookUp(ipAddressLookupFormat);
+            if (hostnameFromIpAddress.equals(expectedHostname)) {
+                return true;
+            }
+
+            logger.log(Level.WARNING, "IP address " + ipAddressLookupFormat + " resolved to " +
+                    hostnameFromIpAddress + ", not " + expectedHostname);
         } catch (NamingException e) {
-            logger.log(Level.WARNING, "Could not get IPv6 hostname", e);
+            logger.log(Level.WARNING, "Could not get hostname of IP address " + ipAddressLookupFormat, e);
         }
+
         return false;
     }
 
+    private boolean isValidIpv4(String ipv4Address) {
+        if (ipv4Address == null) {
+            return true;
+        }
+
+        return hostnameResolvesToIpAddress(ipv4Address) &&
+                ipAddressResolvesToHostname(convertIpv4ToLookupFormat(ipv4Address));
+    }
+
+    private boolean isValidIpv6(String ipv6Address) {
+        if (ipv6Address == null) {
+            return true;
+        }
+
+        return hostnameResolvesToIpAddress(ipv6Address) &&
+                ipAddressResolvesToHostname(convertIpv6ToLookupFormat(ipv6Address));
+    }
+
     String reverseLookUp(String ipAddress) throws NamingException {
         Hashtable<String, String> env = new Hashtable<>();
         env.put("java.naming.factory.initial", "com.sun.jndi.dns.DnsContextFactory");
@@ -130,4 +172,7 @@ public class IPAddressVerifier {
         return convertedIpAddress.toString();
     }
 
+    List<InetAddress> mockableGetAllByName(String hostname) throws UnknownHostException {
+        return Arrays.asList(InetAddress.getAllByName(hostname));
+    }
 }
diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/SpecVerifier.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/SpecVerifier.java
index 3a4a1697a75..246321f0b9e 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/SpecVerifier.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/SpecVerifier.java
@@ -45,6 +45,9 @@ public class SpecVerifier extends Main.VerifierCommand {
     @Option(name = {"-i", "--ips"}, description = "Comma separated list of IP addresses assigned to this node")
     private String ipAddresses;
 
+    @Option(name = {"-S", "--skip-reverse-lookup"}, required = false, description = "Skip verification of reverse lookup of IP addresses")
+    private boolean skipReverseLookup = false;
+
     @Override
     public void run(HardwareDivergenceReport hardwareDivergenceReport, CommandExecutor commandExecutor) {
         String[] ips = Optional.ofNullable(ipAddresses)
@@ -61,12 +64,12 @@ public class SpecVerifier extends Main.VerifierCommand {
     private SpecVerificationReport verifySpec(NodeSpec nodeSpec, CommandExecutor commandExecutor) {
         VerifierSettings verifierSettings = new VerifierSettings(false);
         HardwareInfo actualHardware = HardwareInfoRetriever.retrieve(commandExecutor, verifierSettings);
-        return makeVerificationReport(actualHardware, nodeSpec);
+        return makeVerificationReport(actualHardware, nodeSpec, skipReverseLookup);
     }
 
-    private static SpecVerificationReport makeVerificationReport(HardwareInfo actualHardware, NodeSpec nodeSpec) {
+    private static SpecVerificationReport makeVerificationReport(HardwareInfo actualHardware, NodeSpec nodeSpec, boolean skipReverseLookup) {
         SpecVerificationReport specVerificationReport = HardwareNodeComparator.compare(NodeJsonConverter.convertJsonModelToHardwareInfo(nodeSpec), actualHardware);
-        IPAddressVerifier ipAddressVerifier = new IPAddressVerifier(Defaults.getDefaults().vespaHostname());
+        IPAddressVerifier ipAddressVerifier = new IPAddressVerifier(Defaults.getDefaults().vespaHostname(), skipReverseLookup);
         ipAddressVerifier.reportFaultyIpAddresses(nodeSpec, specVerificationReport);
         return specVerificationReport;
     }
diff --git a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifierTest.java b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifierTest.java
index 5216a8563d7..b9ba9417981 100644
--- a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifierTest.java
+++ b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/noderepo/IPAddressVerifierTest.java
@@ -4,9 +4,14 @@ package com.yahoo.vespa.hosted.node.verification.commons.noderepo;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Arrays;
+
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.spy;
 
 /**
@@ -20,7 +25,7 @@ public class IPAddressVerifierTest {
     private final NodeSpec nodeSpec = new NodeSpec(1920, 256, 48, true, 10_000, new String[]{ipv4Address, ipv6Address});
     private final String hostname = "test123.region.domain.tld";
 
-    private IPAddressVerifier ipAddressVerifier = spy(new IPAddressVerifier(hostname));
+    private IPAddressVerifier ipAddressVerifier = spy(new IPAddressVerifier(hostname, false));
     private String ipv4LookupFormat;
     private String ipv6LookupFormat;
 
@@ -31,8 +36,43 @@ public class IPAddressVerifierTest {
     }
 
     @Test
+    public void getFaultyIpAddresses_with_hostname_resolving_to_other_ips() throws Exception {
+        doReturn(Arrays.asList(InetAddress.getByName("1.2.3.4"), InetAddress.getByName("fd00::1"))).when(ipAddressVerifier).mockableGetAllByName(hostname);
+        String[] faultyIpAddresses = ipAddressVerifier.getFaultyIpAddresses(nodeSpec);
+        String[] expectedFaultyIpAddresses = new String[]{ipv4Address, ipv6Address};
+        assertArrayEquals(expectedFaultyIpAddresses, faultyIpAddresses);
+    }
+
+    @Test
+    public void getFaultyIpAddresses_with_hostname_not_resolving_to_ipv4_address() throws Exception {
+        doReturn(Arrays.asList(InetAddress.getByName(ipv6Address))).when(ipAddressVerifier).mockableGetAllByName(hostname);
+        doReturn(hostname).when(ipAddressVerifier).reverseLookUp(ipv6LookupFormat);
+        String[] faultyIpAddresses = ipAddressVerifier.getFaultyIpAddresses(nodeSpec);
+        String[] expectedFaultyIpAddresses = new String[]{ipv4Address};
+        assertArrayEquals(expectedFaultyIpAddresses, faultyIpAddresses);
+    }
+
+    @Test
+    public void getFaultyIpAddresses_with_failing_hostname_resolution() throws Exception {
+        doThrow(new UnknownHostException("bad hostname")).when(ipAddressVerifier).mockableGetAllByName(hostname);
+        String[] faultyIpAddresses = ipAddressVerifier.getFaultyIpAddresses(nodeSpec);
+        String[] expectedFaultyIpAddresses = new String[]{ipv4Address, ipv6Address};
+        assertArrayEquals(expectedFaultyIpAddresses, faultyIpAddresses);
+    }
+
+    @Test
+    public void getFaultyIpAddresses_with_hostname_not_resolving_to_ipv6_address() throws Exception {
+        doReturn(Arrays.asList(InetAddress.getByName(ipv4Address))).when(ipAddressVerifier).mockableGetAllByName(hostname);
+        doReturn(hostname).when(ipAddressVerifier).reverseLookUp(ipv4LookupFormat);
+        String[] faultyIpAddresses = ipAddressVerifier.getFaultyIpAddresses(nodeSpec);
+        String[] expectedFaultyIpAddresses = new String[]{ipv6Address};
+        assertArrayEquals(expectedFaultyIpAddresses, faultyIpAddresses);
+    }
+
+    @Test
     public void getFaultyIpAddresses_should_return_IP_address_when_different_hostname() throws Exception {
         String wrongHostName = "www.yahoo.com";
+        doReturn(Arrays.asList(InetAddress.getByName(ipv4Address), InetAddress.getByName(ipv6Address))).when(ipAddressVerifier).mockableGetAllByName(hostname);
         doReturn(hostname).when(ipAddressVerifier).reverseLookUp(ipv4LookupFormat);
         doReturn(wrongHostName).when(ipAddressVerifier).reverseLookUp(ipv6LookupFormat);
         String[] faultyIpAddresses = ipAddressVerifier.getFaultyIpAddresses(nodeSpec);
@@ -42,6 +82,7 @@ public class IPAddressVerifierTest {
 
     @Test
     public void getFaultyIpAddresses_should_return_empty_array_when_all_addresses_point_to_correct_hostname() throws Exception {
+        doReturn(Arrays.asList(InetAddress.getByName(ipv4Address), InetAddress.getByName(ipv6Address))).when(ipAddressVerifier).mockableGetAllByName(hostname);
         doReturn(hostname).when(ipAddressVerifier).reverseLookUp(ipv4LookupFormat);
         doReturn(hostname).when(ipAddressVerifier).reverseLookUp(ipv6LookupFormat);
         String[] faultyIpAddresses = ipAddressVerifier.getFaultyIpAddresses(nodeSpec);
@@ -50,6 +91,15 @@ public class IPAddressVerifierTest {
     }
 
     @Test
+    public void getFaultyIpAddresses_should_return_empty_array_when_reverse_lookup_is_skipped() throws Exception {
+        ipAddressVerifier = spy(new IPAddressVerifier(hostname, true));
+        doReturn(Arrays.asList(InetAddress.getByName(ipv4Address), InetAddress.getByName(ipv6Address))).when(ipAddressVerifier).mockableGetAllByName(hostname);
+        String[] faultyIpAddresses = ipAddressVerifier.getFaultyIpAddresses(nodeSpec);
+        String[] expectedFaultyIpAddresses = new String[]{};
+        assertArrayEquals(expectedFaultyIpAddresses, faultyIpAddresses);
+    }
+
+    @Test
     public void convertIpv6ToLookupFormat_should_return_properly_converted_ipv6_address() {
         String actualConvertedAddress = ipAddressVerifier.convertIpv6ToLookupFormat(ipv6Address);
         assertEquals(ipv6LookupFormat, actualConvertedAddress);