From c60cb59d276b5e6cc3fc01dbf1ae2b7706c89546 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Wed, 3 Oct 2018 15:19:24 +0200 Subject: 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. --- .../commons/noderepo/IPAddressVerifier.java | 81 +++++++++++++++++----- .../node/verification/spec/SpecVerifier.java | 9 ++- .../commons/noderepo/IPAddressVerifierTest.java | 52 +++++++++++++- 3 files changed, 120 insertions(+), 22 deletions(-) (limited to 'node-maintainer') 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 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 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 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 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; @@ -30,9 +35,44 @@ public class IPAddressVerifierTest { ipv6LookupFormat = "4.3.2.1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.b.a.d.f.ip6.arpa"; } + @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); @@ -49,6 +90,15 @@ public class IPAddressVerifierTest { assertArrayEquals(expectedFaultyIpAddresses, faultyIpAddresses); } + @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); -- cgit v1.2.3