diff options
Diffstat (limited to 'node-repository')
10 files changed, 107 insertions, 43 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 c34b357b758..0927ffe3ce2 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 @@ -24,14 +24,12 @@ import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; import static com.yahoo.config.provision.NodeType.confighost; import static com.yahoo.config.provision.NodeType.controllerhost; import static com.yahoo.config.provision.NodeType.proxyhost; -import static java.util.function.Predicate.not; /** * This handles IP address configuration and allocation. @@ -113,13 +111,13 @@ public record IP() { * * @throws IllegalArgumentException if there are IP conflicts with existing nodes */ - public static LockedNodeList verify(List<Node> nodes, LockedNodeList allNodes) { + public static LockedNodeList verify(List<Node> nodes, LockedNodeList allNodes, Zone zone) { NodeList sortedNodes = allNodes.sortedBy(Comparator.comparing(Node::hostname)); for (var node : nodes) { + Space ipSpace = Space.of(zone, node.cloudAccount()); for (var other : sortedNodes) { if (node.equals(other)) continue; if (canAssignIpOf(other, node)) continue; - Predicate<String> sharedIpSpace = ip -> inSharedIpSpace(ip, other.cloudAccount(), node.cloudAccount()); var addresses = new HashSet<>(node.ipConfig().primary()); var otherAddresses = new HashSet<>(other.ipConfig().primary()); @@ -127,7 +125,7 @@ public record IP() { addresses.addAll(node.ipConfig().pool().asSet()); otherAddresses.addAll(other.ipConfig().pool().asSet()); } - otherAddresses.removeIf(not(sharedIpSpace)); + otherAddresses.removeIf(otherIp -> !ipSpace.contains(otherIp, other.cloudAccount())); otherAddresses.retainAll(addresses); if (!otherAddresses.isEmpty()) throw new IllegalArgumentException("Cannot assign " + addresses + " to " + node.hostname() + @@ -151,8 +149,8 @@ public record IP() { }; } - public static Node verify(Node node, LockedNodeList allNodes) { - return verify(List.of(node), allNodes).asList().get(0); + public static Node verify(Node node, LockedNodeList allNodes, Zone zone) { + return verify(List.of(node), allNodes, zone).asList().get(0); } } @@ -468,14 +466,33 @@ public record IP() { } /** Returns whether given string is a public IP address */ - public static boolean isPublic(String ip) { + private static boolean isPublic(String ip) { InetAddress address = parse(ip); return ! address.isLoopbackAddress() && ! address.isLinkLocalAddress() && ! address.isSiteLocalAddress(); } - /** Returns true if the IP address is in the IP space of both sourceCloudAccount and targetCloudAccount. */ - public static boolean inSharedIpSpace(String ip, CloudAccount sourceCloudAccount, CloudAccount targetCloudAccount) { - return sourceCloudAccount.equals(targetCloudAccount) || isPublic(ip); + @FunctionalInterface + public interface Space { + static Space of(Zone zone) { return of(zone, zone.cloud().account()); } + + /** Returns the IP space of a cloud account in a zone. */ + static Space of(Zone zone, CloudAccount cloudAccount) { return (ip, account) -> sharedIp(ip, account, cloudAccount, zone); } + + private static boolean sharedIp(String ip, CloudAccount sourceCloudAccount, CloudAccount targetCloudAccount, Zone zone) { + // IPs within the same account and zone are always shared. + if (sourceCloudAccount.equals(targetCloudAccount)) + return true; + + // Only public IPs inside (outside) an exclave account are shared outside (inside). + if (sourceCloudAccount.isExclave(zone) || targetCloudAccount.isExclave(zone)) + return isPublic(ip); + + // IPs in noclave and inclave are always shared. + return true; + } + + /** Returns true if the IP in the given account is in this IP space. */ + boolean contains(String ip, CloudAccount cloudAccount); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java index 14c4a63a500..7df19c97659 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.provision.node; import com.google.common.collect.ImmutableSet; -import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.Node; @@ -51,6 +50,7 @@ public record NodeAcl(Node node, Set<Integer> trustedPorts = new LinkedHashSet<>(); Set<Integer> trustedUdpPorts = new LinkedHashSet<>(); Set<String> trustedNetworks = new LinkedHashSet<>(); + IP.Space ipSpace = simplerAcl ? IP.Space.of(zone, node.cloudAccount()) : (ip, account) -> true; // For all cases below, trust: // - SSH: If the host has one container, and it is using the host's network namespace, @@ -62,9 +62,9 @@ public record NodeAcl(Node node, // and parents if necessary due to NAT. // - load balancers allocated to application trustedPorts.add(22); - allNodes.parentOf(node).map(parent -> TrustedNode.of(parent, node.cloudAccount(), simplerAcl)).ifPresent(trustedNodes::add); + allNodes.parentOf(node).map(parent -> TrustedNode.of(parent, ipSpace)).ifPresent(trustedNodes::add); node.allocation().ifPresent(allocation -> { - trustedNodes.addAll(trustedNodesForChildrenMatching(node, allNodes, n -> n.allocation().map(Allocation::owner).equals(Optional.of(allocation.owner())), Set.of(), simplerAcl)); + trustedNodes.addAll(trustedNodesForChildrenMatching(node, allNodes, n -> n.allocation().map(Allocation::owner).equals(Optional.of(allocation.owner())), Set.of(), ipSpace)); loadBalancers.list(allocation.owner()).asList() .stream() .map(LoadBalancer::instance) @@ -78,8 +78,8 @@ public record NodeAcl(Node node, // Tenant nodes in other states than ready, trust: // - config servers // - proxy nodes - trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config), node.cloudAccount(), simplerAcl)); - trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.proxy), node.cloudAccount(), simplerAcl)); + trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config), ipSpace)); + trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.proxy), ipSpace)); } case config -> { // Config servers trust: @@ -87,7 +87,7 @@ public record NodeAcl(Node node, // - port 19070 (RPC) from all proxy nodes (and their hosts, in case traffic is NAT-ed via parent) // - port 4443 from the world // - udp port 51820 from the world - trustedNodes.addAll(trustedNodesForChildrenMatching(node, allNodes, n -> EnumSet.of(NodeType.tenant, NodeType.proxy).contains(n.type()), RPC_PORTS, simplerAcl)); + trustedNodes.addAll(trustedNodesForChildrenMatching(node, allNodes, n -> EnumSet.of(NodeType.tenant, NodeType.proxy).contains(n.type()), RPC_PORTS, ipSpace)); trustedPorts.add(4443); if (zone.system().isPublic() && zone.cloud().allowEnclave()) { trustedUdpPorts.add(WIREGUARD_PORT); @@ -97,7 +97,7 @@ public record NodeAcl(Node node, // Proxy nodes trust: // - config servers // - all connections from the world on 443 (production traffic) and 4443 (health checks) - trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config), node.cloudAccount(), simplerAcl)); + trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config), ipSpace)); trustedPorts.add(443); trustedPorts.add(4443); } @@ -116,7 +116,7 @@ public record NodeAcl(Node node, /** Returns the set of children matching the selector, and their parent host if traffic from child may be NATed */ private static Set<TrustedNode> trustedNodesForChildrenMatching(Node node, NodeList allNodes, Predicate<Node> childNodeSelector, - Set<Integer> ports, boolean simplerAcl) { + Set<Integer> ports, IP.Space ipSpace) { if (node.type().isHost()) throw new IllegalArgumentException("Host nodes cannot have NAT parents"); @@ -126,12 +126,12 @@ public record NodeAcl(Node node, .filter(n -> !n.type().isHost()) .filter(childNodeSelector) .mapMulti((Node otherNode, Consumer<TrustedNode> consumer) -> { - consumer.accept(TrustedNode.of(otherNode, ports, node.cloudAccount(), simplerAcl)); + consumer.accept(TrustedNode.of(otherNode, ports, ipSpace)); // And parent host if traffic from otherNode may be NATed if (hasIp4 && otherNode.ipConfig().primary().stream().noneMatch(IP::isV4) || hasIp6 && otherNode.ipConfig().primary().stream().noneMatch(IP::isV6)) { - consumer.accept(TrustedNode.of(allNodes.parentOf(otherNode).orElseThrow(), ports, node.cloudAccount(), simplerAcl)); + consumer.accept(TrustedNode.of(allNodes.parentOf(otherNode).orElseThrow(), ports, ipSpace)); } }) .collect(Collectors.toSet()); @@ -140,28 +140,28 @@ public record NodeAcl(Node node, public record TrustedNode(String hostname, NodeType type, Set<String> ipAddresses, Set<Integer> ports) { /** Trust given ports from node, and primary IP addresses shared with given cloud account */ - public static TrustedNode of(Node node, Set<Integer> ports, CloudAccount sourceCloudAccount, boolean simplerAcl) { + public static TrustedNode of(Node node, Set<Integer> ports, IP.Space ipSpace) { Set<String> ipAddresses = node.ipConfig() .primary() .stream() - .filter(ip -> !simplerAcl || IP.inSharedIpSpace(ip, sourceCloudAccount, node.cloudAccount())) + .filter(ip -> ipSpace.contains(ip, node.cloudAccount())) .collect(Collectors.toSet()); return new TrustedNode(node.hostname(), node.type(), ipAddresses, ports); } /** The node in the given sourceCloudAccount should trust all ports from given node */ - public static TrustedNode of(Node node, CloudAccount sourceCloudAccount, boolean simplerAcl) { - return of(node, Set.of(), sourceCloudAccount, simplerAcl); + public static TrustedNode of(Node node, IP.Space ipSpace) { + return of(node, Set.of(), ipSpace); } - public static List<TrustedNode> of(Iterable<Node> nodes, Set<Integer> ports, CloudAccount sourceCloudAccount, boolean simplerAcl) { + public static List<TrustedNode> of(Iterable<Node> nodes, Set<Integer> ports, IP.Space ipSpace) { return StreamSupport.stream(nodes.spliterator(), false) - .map(node -> TrustedNode.of(node, ports, sourceCloudAccount, simplerAcl)) + .map(node -> TrustedNode.of(node, ports, ipSpace)) .toList(); } - public static List<TrustedNode> of(Iterable<Node> nodes, CloudAccount sourceCloudAccount, boolean simplerAcl) { - return of(nodes, Set.of(), sourceCloudAccount, simplerAcl); + public static List<TrustedNode> of(Iterable<Node> nodes, IP.Space ipSpace) { + return of(nodes, Set.of(), ipSpace); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index 0bb045dc6a1..560abb1f0e8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -200,7 +200,7 @@ public class Nodes { } NestedTransaction transaction = new NestedTransaction(); db.removeNodes(nodesToRemove, transaction); - List<Node> resultingNodes = db.addNodesInState(IP.Config.verify(nodesToAdd, list(allocationLock)), Node.State.provisioned, agent, transaction); + List<Node> resultingNodes = db.addNodesInState(IP.Config.verify(nodesToAdd, list(allocationLock), zone), Node.State.provisioned, agent, transaction); transaction.commit(); return resultingNodes; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java index 32bba9336a2..d88cade0022 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java @@ -281,9 +281,9 @@ public class NodePatcher { private Node applyIpconfigField(Node node, String name, Inspector value, LockedNodeList nodes) { return switch (name) { - case "ipAddresses" -> IP.Config.verify(node.with(node.ipConfig().withPrimary(asStringSet(value))), nodes); - case "additionalIpAddresses" -> IP.Config.verify(node.with(node.ipConfig().withPool(node.ipConfig().pool().withIpAddresses(asStringSet(value)))), nodes); - case "additionalHostnames" -> IP.Config.verify(node.with(node.ipConfig().withPool(node.ipConfig().pool().withHostnames(asHostnames(value)))), nodes); + case "ipAddresses" -> IP.Config.verify(node.with(node.ipConfig().withPrimary(asStringSet(value))), nodes, nodeRepository.zone()); + case "additionalIpAddresses" -> IP.Config.verify(node.with(node.ipConfig().withPool(node.ipConfig().pool().withIpAddresses(asStringSet(value)))), nodes, nodeRepository.zone()); + case "additionalHostnames" -> IP.Config.verify(node.with(node.ipConfig().withPool(node.ipConfig().pool().withHostnames(asHostnames(value)))), nodes, nodeRepository.zone()); default -> throw new IllegalArgumentException("Could not apply field '" + name + "' on a node: No such modifiable field"); }; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java index 605bf514f03..5148d2a635c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java @@ -1,8 +1,14 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.History; import com.yahoo.vespa.hosted.provision.node.IP; @@ -53,7 +59,11 @@ public class NodeRepositoryTest { @Test public void test_ip_conflicts() { - NodeRepositoryTester tester = new NodeRepositoryTester(); + var zone = new Zone(Cloud.builder().name(CloudName.AWS).account(CloudAccount.from("aws:123456789012")).allowEnclave(true).build(), + SystemName.Public, + Environment.prod, + RegionName.from("aws-us-east-1a")); + NodeRepositoryTester tester = new NodeRepositoryTester(zone); IP.Config ipConfig = IP.Config.of(Set.of("1.2.3.4", "10.2.3.4"), Set.of("1.2.3.4", "10.2.3.4")); IP.Config publicIpConfig = IP.Config.of(Set.of("1.2.3.4"), Set.of("1.2.3.4")); IP.Config privateIpConfig = IP.Config.of(Set.of("10.2.3.4"), Set.of("10.2.3.4")); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java index 00c4d95b0da..49702a7d4c1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java @@ -33,6 +33,10 @@ public class NodeRepositoryTester { private final MockCurator curator; public NodeRepositoryTester() { + this(Zone.defaultZone()); + } + + public NodeRepositoryTester(Zone zone) { nodeFlavors = new NodeFlavors(createConfig()); clock = new ManualClock(); curator = new MockCurator(); @@ -41,7 +45,7 @@ public class NodeRepositoryTester { new EmptyProvisionServiceProvider(), curator, clock, - Zone.defaultZone(), + zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), Optional.empty(), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java index 2c9da89d8af..26925372b93 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java @@ -15,6 +15,7 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; +import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.node.NodeAcl; import com.yahoo.vespa.hosted.provision.node.NodeAcl.TrustedNode; import org.junit.Test; @@ -112,10 +113,11 @@ public class AclProvisioningTest { // Trusted nodes is all tenant nodes, all proxy nodes, all config servers and load balancer subnets // All tenant hosts because nodes are IPv6 and cfg are IPv4, so traffic is NATed. // NOT proxy hosts because proxies are dual-stacked so no NAT is needed - assertAcls(List.of(TrustedNode.of(tenantHosts, Set.of(19070), node.cloudAccount(), true), - TrustedNode.of(tenantNodes, Set.of(19070), node.cloudAccount(), true), - TrustedNode.of(proxyNodes, Set.of(19070), node.cloudAccount(), true), - TrustedNode.of(configNodes, node.cloudAccount(), true)), + IP.Space ipSpace = IP.Space.of(tester.nodeRepository().zone(), node.cloudAccount()); + assertAcls(List.of(TrustedNode.of(tenantHosts, Set.of(19070), ipSpace), + TrustedNode.of(tenantNodes, Set.of(19070), ipSpace), + TrustedNode.of(proxyNodes, Set.of(19070), ipSpace), + TrustedNode.of(configNodes, ipSpace)), Set.of("10.2.3.0/24", "10.4.5.0/24"), List.of(nodeAcl)); assertEquals(Set.of(22, 4443), nodeAcl.trustedPorts()); @@ -240,11 +242,12 @@ public class AclProvisioningTest { nodeAcl.trustedNodes().stream().map(TrustedNode::ipAddresses).toList()); } - private static List<List<TrustedNode>> trustedNodesOf(List<List<Node>> nodes, Set<Integer> ports, CloudAccount cloudAccount) { - return nodes.stream().map(node -> TrustedNode.of(node, ports, cloudAccount, true)).toList(); + private List<List<TrustedNode>> trustedNodesOf(List<List<Node>> nodes, Set<Integer> ports, CloudAccount cloudAccount) { + IP.Space ipSpace = IP.Space.of(tester.nodeRepository().zone(), cloudAccount); + return nodes.stream().map(node -> TrustedNode.of(node, ports, ipSpace)).toList(); } - private static List<List<TrustedNode>> trustedNodesOf(List<List<Node>> nodes, CloudAccount cloudAccount) { + private List<List<TrustedNode>> trustedNodesOf(List<List<Node>> nodes, CloudAccount cloudAccount) { return trustedNodesOf(nodes, Set.of(), cloudAccount); } 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 e910d562d53..7769523f3d6 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 @@ -459,7 +459,7 @@ public class NodesV2ApiTest { } @Test - public void acls_for_exclave_tenant_host() throws Exception { + public void acls_for_inclave_tenant_host() throws Exception { assertFile(new Request("http://localhost:8080/nodes/v2/acl/host5.yahoo.com"), "acl-tenant-node.json"); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-config-server.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-config-server.json index a4afe470ce9..e3b487325f8 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-config-server.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-config-server.json @@ -135,6 +135,15 @@ { "hostname": "host3.yahoo.com", "type": "tenant", + "ipAddress": "127.0.3.1", + "ports": [ + 19070 + ], + "trustedBy": "cfg1.yahoo.com" + }, + { + "hostname": "host3.yahoo.com", + "type": "tenant", "ipAddress": "::3:1", "ports": [ 19070 @@ -169,6 +178,15 @@ "trustedBy": "cfg1.yahoo.com" }, { + "hostname": "host5.yahoo.com", + "type": "tenant", + "ipAddress": "127.0.5.1", + "ports": [ + 19070 + ], + "trustedBy": "cfg1.yahoo.com" + }, + { "hostname": "host55.yahoo.com", "type": "tenant", "ipAddress": "::55:1", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-tenant-node.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-tenant-node.json index 2ca385a26b6..98a7c5f9036 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-tenant-node.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/acl-tenant-node.json @@ -7,12 +7,24 @@ "trustedBy": "host5.yahoo.com" }, { + "hostname": "cfg1.yahoo.com", + "type": "config", + "ipAddress": "127.0.201.1", + "trustedBy": "host5.yahoo.com" + }, + { "hostname": "cfg2.yahoo.com", "type": "config", "ipAddress": "::202:1", "trustedBy": "host5.yahoo.com" }, { + "hostname": "cfg2.yahoo.com", + "type": "config", + "ipAddress": "127.0.202.1", + "trustedBy": "host5.yahoo.com" + }, + { "hostname": "dockerhost2.yahoo.com", "type": "host", "ipAddress": "::101:1", |