diff options
4 files changed, 41 insertions, 33 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java index 836583022ca..66061ad4e9f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java @@ -9,8 +9,7 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.persistence.NameResolver; -import java.net.Inet4Address; -import java.net.Inet6Address; +import java.net.InetAddress; import java.util.Collections; import java.util.Comparator; import java.util.HashSet; @@ -33,9 +32,9 @@ import static com.yahoo.config.provision.NodeType.proxyhost; public class IP { /** Comparator for sorting IP addresses by their natural order */ - public static final Comparator<String> NATURAL_ORDER = (ip1, ip2) -> { - byte[] address1 = InetAddresses.forString(ip1).getAddress(); - byte[] address2 = InetAddresses.forString(ip2).getAddress(); + public static final Comparator<InetAddress> NATURAL_ORDER = (ip1, ip2) -> { + byte[] address1 = ip1.getAddress(); + byte[] address2 = ip2.getAddress(); // IPv4 always sorts before IPv6 if (address1.length < address2.length) return -1; @@ -88,7 +87,7 @@ public class IP { /** Returns a copy of this with pool set to given value */ public Config with(Set<String> primary) { - return new Config(require(primary), pool.asSet()); + return new Config(primary, pool.asSet()); } @Override @@ -110,16 +109,6 @@ public class IP { return String.format("ip config primary=%s pool=%s", primary, pool.asSet()); } - /** Validates and returns the given addresses */ - public static Set<String> require(Set<String> addresses) { - try { - addresses.forEach(InetAddresses::forString); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Found one or more invalid addresses in " + addresses, e); - } - return addresses; - } - /** * Verify IP config of given nodes * @@ -414,14 +403,28 @@ public class IP { } + /** Validate IP address*/ + public static InetAddress parse(String ipAddress) { + try { + return InetAddresses.forString(ipAddress); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid IP address '" + ipAddress + "'", e); + } + } + + /** Convert IP address to string. This uses :: for zero compression in IPv6 addresses. */ + public static String asString(InetAddress inetAddress) { + return InetAddresses.toAddrString(inetAddress); + } + /** Returns whether given string is an IPv4 address */ public static boolean isV4(String ipAddress) { - return InetAddresses.forString(ipAddress) instanceof Inet4Address; + return ipAddress.contains("."); } /** Returns whether given string is an IPv6 address */ public static boolean isV6(String ipAddress) { - return InetAddresses.forString(ipAddress) instanceof Inet6Address; + return ipAddress.contains(":"); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index f7521ba786d..397182e204e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -43,7 +43,6 @@ import java.util.List; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; -import java.util.function.UnaryOperator; /** * Serializes a node to/from JSON. @@ -145,8 +144,8 @@ public class NodeSerializer { private void toSlime(Node node, Cursor object) { object.setString(hostnameKey, node.hostname()); - toSlime(node.ipConfig().primary(), object.setArray(ipAddressesKey), IP.Config::require); - toSlime(node.ipConfig().pool().asSet(), object.setArray(ipAddressPoolKey), UnaryOperator.identity() /* Pool already holds a validated address list */); + toSlime(node.ipConfig().primary(), object.setArray(ipAddressesKey)); + toSlime(node.ipConfig().pool().asSet(), object.setArray(ipAddressPoolKey)); object.setString(idKey, node.id()); node.parentHostname().ifPresent(hostname -> object.setString(parentHostnameKey, hostname)); toSlime(node.flavor(), object); @@ -207,9 +206,10 @@ public class NodeSerializer { object.setString(agentKey, toString(event.agent())); } - private void toSlime(Set<String> ipAddresses, Cursor array, UnaryOperator<Set<String>> validator) { - // Sorting IP addresses is expensive, so we do it at serialization time instead of Node construction time - validator.apply(ipAddresses).stream().sorted(IP.NATURAL_ORDER).forEach(array::addString); + private void toSlime(Set<String> ipAddresses, Cursor array) { + // Validating IP address string literals is expensive, so we do it at serialization time instead of Node + // construction time + ipAddresses.stream().map(IP::parse).sorted(IP.NATURAL_ORDER).map(IP::asString).forEach(array::addString); } // ---------------- Deserialization -------------------------------------------------- diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java index 4602a8f3560..bf83b074387 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.provision.node; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.LockedNodeList; @@ -11,14 +10,15 @@ import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; import org.junit.Test; -import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -59,10 +59,14 @@ public class IPTest { "::1", "::10", "::20", - "2001:db8:0:0:0:0:0:ffff", - "2001:db8:85a3:0:0:8a2e:370:7334", - "2001:db8:95a3:0:0:0:0:7334"), - new ArrayList<>(ImmutableSortedSet.copyOf(IP.NATURAL_ORDER, ipAddresses)) + "2001:db8::ffff", + "2001:db8:85a3::8a2e:370:7334", + "2001:db8:95a3::7334"), + ipAddresses.stream() + .map(IP::parse) + .sorted(IP.NATURAL_ORDER) + .map(IP::asString) + .collect(Collectors.toList()) ); } @@ -99,7 +103,6 @@ public class IPTest { @Test public void test_find_allocation_ipv4_only() { var pool = testPool(false); - assertTrue(pool instanceof IP.Ipv4Pool); var allocation = pool.findAllocation(emptyList, resolver); assertFalse("Found allocation", allocation.isEmpty()); assertEquals("127.0.0.1", allocation.get().primary()); @@ -173,7 +176,9 @@ public class IPTest { .addReverseRecord("::2", "host1"); } - return node.ipConfig().pool(); + IP.Pool pool = node.ipConfig().pool(); + assertNotEquals(dualStack, pool instanceof IP.Ipv4Pool); + return pool; } private static Node createNode(Set<String> ipAddresses) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java index 1a354686ae4..9f6f5043ae8 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java @@ -305,7 +305,7 @@ public class NodesV2ApiTest { ("[" + asNodeJson("host-with-ip.yahoo.com", "default", "foo") + "]"). getBytes(StandardCharsets.UTF_8), Request.Method.POST); - tester.assertResponse(req, 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Found one or more invalid addresses in [foo]: 'foo' is not an IP string literal.\"}"); + tester.assertResponse(req, 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid IP address 'foo': 'foo' is not an IP string literal.\"}"); // Attempt to POST tenant node with already assigned IP tester.assertResponse(new Request("http://localhost:8080/nodes/v2/node", |