diff options
author | Håkon Hallingstad <hakon@oath.com> | 2018-10-03 15:19:24 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2018-10-03 15:19:24 +0200 |
commit | c60cb59d276b5e6cc3fc01dbf1ae2b7706c89546 (patch) | |
tree | 7a75b58a4208fefbdedb1ec5e61bce4cc74d0a94 | |
parent | b234a0daf5076d4ece4b373240883073d2b932ee (diff) |
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.
5 files changed, 126 insertions, 22 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index cbb05ace0d1..236415d1bcd 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions; import com.yahoo.vespa.hosted.dockerapi.metrics.GaugeWrapper; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; +import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.logging.FilebeatConfigProvider; import com.yahoo.vespa.hosted.node.admin.component.Environment; @@ -424,6 +425,10 @@ public class StorageMaintainer { "--bandwidth", Double.toString(node.getBandwidth()), "--ips", String.join(",", node.getIpAddresses()))); + if (environment.getDockerNetworking() == DockerNetworking.HOST_NETWORK) { + arguments.add("--skip-reverse-lookup"); + } + node.getHardwareDivergence().ifPresent(hardwareDivergence -> { arguments.add("--divergence"); arguments.add(hardwareDivergence); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java index 90e4b2e6c8b..8be4f3b416a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java @@ -137,6 +137,7 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { // Only update hardware divergence if there is a change. if (!node.getHardwareDivergence().orElse("null").equals(hardwareDivergence)) { NodeAttributes nodeAttributes = new NodeAttributes().withHardwareDivergence(hardwareDivergence); + log.info("Updating hardware divergence to " + hardwareDivergence); nodeRepository.updateNodeAttributes(dockerHostHostName, nodeAttributes); } } catch (RuntimeException e) { 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); |