diff options
author | Jon Bratseth <bratseth@gmail.com> | 2021-02-08 10:21:44 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2021-02-08 10:21:44 +0100 |
commit | fe0ce8c88845be27203a2ec36ef1ac5e4b30e05a (patch) | |
tree | e740120e3c862f45b244fab6809976f7bd851093 | |
parent | 3bca735697a6fcbf79e51ff452c660d45a68ce5c (diff) |
Move node acl computation under node
6 files changed, 138 insertions, 124 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index 00327dc0002..1ca8b5782b8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -8,19 +8,26 @@ import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancers; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.Allocation; import com.yahoo.vespa.hosted.provision.node.Generation; import com.yahoo.vespa.hosted.provision.node.History; import com.yahoo.vespa.hosted.provision.node.IP; +import com.yahoo.vespa.hosted.provision.node.NodeAcl; import com.yahoo.vespa.hosted.provision.node.Reports; import com.yahoo.vespa.hosted.provision.node.Status; import java.time.Instant; import java.util.Arrays; +import java.util.Comparator; +import java.util.LinkedHashSet; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; /** * A node in the node repository. The identity of a node is given by its id. @@ -429,6 +436,11 @@ public final class Node implements Nodelike { .deviation(); } + /** Returns the ACL for the node (trusted nodes, networks and ports) */ + public NodeAcl acl(NodeList allNodes, LoadBalancers loadBalancers) { + return NodeAcl.from(this, allNodes, loadBalancers); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 4e165fab555..d6a8f107841 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -165,7 +165,7 @@ public class NodeRepository extends AbstractComponent { this.containerImages = new ContainerImages(db, containerImage); this.jobControl = new JobControl(new JobControlFlags(db, flagSource)); this.applications = new Applications(db); - this.loadBalancers = new LoadBalancers(db, this); + this.loadBalancers = new LoadBalancers(db); this.spareCount = spareCount; rewriteNodes(); } @@ -218,6 +218,20 @@ public class NodeRepository extends AbstractComponent { /** The number of nodes we should ensure has free capacity for node failures whenever possible */ public int spareCount() { return spareCount; } + /** + * Returns ACLs for the children of the given host. + * + * @param host node for which to generate ACLs + * @return the list of node ACLs + */ + public List<NodeAcl> getChildAcls(Node host) { + if ( host.type() != NodeType.host) throw new IllegalArgumentException("Only hosts have children"); + NodeList allNodes = list(); + return list().childrenOf(host).asList().stream() + .map(childNode -> childNode.acl(allNodes, loadBalancers)) + .collect(Collectors.toUnmodifiableList()); + } + // ---------------- Query API ---------------------------------------------------------------- /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancers.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancers.java index 370c088ac86..c18403b591c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancers.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancers.java @@ -26,11 +26,9 @@ import java.util.stream.Collectors; public class LoadBalancers { private final CuratorDatabaseClient db; - private final NodeRepository nodeRepository; - public LoadBalancers(CuratorDatabaseClient db, NodeRepository nodeRepository) { + public LoadBalancers(CuratorDatabaseClient db) { this.db = db; - this.nodeRepository = nodeRepository; } /** Returns a filterable list of all load balancers in this repository */ @@ -47,106 +45,4 @@ public class LoadBalancers { return LoadBalancerList.copyOf(db.readLoadBalancers(predicate).values()); } - /** - * Returns the ACL for the node (trusted nodes, networks and ports) - */ - private NodeAcl getNodeAcl(Node node, NodeList candidates) { - Set<Node> trustedNodes = new TreeSet<>(Comparator.comparing(Node::hostname)); - Set<Integer> trustedPorts = new LinkedHashSet<>(); - Set<String> trustedNetworks = new LinkedHashSet<>(); - - // For all cases below, trust: - // - SSH: If the Docker host has one container, and it is using the Docker host's network namespace, - // opening up SSH to the Docker host is done here as a trusted port. For simplicity all nodes have - // SSH opened (which is safe for 2 reasons: SSH daemon is not run inside containers, and NPT networks - // will (should) not forward port 22 traffic to container). - // - parent host (for health checks and metrics) - // - nodes in same application - // - load balancers allocated to application - trustedPorts.add(22); - candidates.parentOf(node).ifPresent(trustedNodes::add); - node.allocation().ifPresent(allocation -> { - trustedNodes.addAll(candidates.owner(allocation.owner()).asList()); - list(allocation.owner()).asList() - .stream() - .map(LoadBalancer::instance) - .map(LoadBalancerInstance::networks) - .forEach(trustedNetworks::addAll); - }); - - switch (node.type()) { - case tenant: - // Tenant nodes in other states than ready, trust: - // - config servers - // - proxy nodes - // - parents of the nodes in the same application: If some of the nodes are on a different IP versions - // or only a subset of them are dual-stacked, the communication between the nodes may be NATed - // with via parent's IP address. - trustedNodes.addAll(candidates.nodeType(NodeType.config).asList()); - trustedNodes.addAll(candidates.nodeType(NodeType.proxy).asList()); - node.allocation().ifPresent(allocation -> - trustedNodes.addAll(candidates.parentsOf(candidates.owner(allocation.owner())).asList())); - - if (node.state() == Node.State.ready) { - // Tenant nodes in state ready, trust: - // - All tenant nodes in zone. When a ready node is allocated to a an application there's a brief - // window where current ACLs have not yet been applied on the node. To avoid service disruption - // during this window, ready tenant nodes trust all other tenant nodes. - trustedNodes.addAll(candidates.nodeType(NodeType.tenant).asList()); - } - break; - - case config: - // Config servers trust: - // - all nodes - // - port 4443 from the world - trustedNodes.addAll(candidates.asList()); - trustedPorts.add(4443); - break; - - case proxy: - // Proxy nodes trust: - // - config servers - // - all connections from the world on 4080 (insecure tb removed), and 4443 - trustedNodes.addAll(candidates.nodeType(NodeType.config).asList()); - trustedPorts.add(443); - trustedPorts.add(4080); - trustedPorts.add(4443); - break; - - case controller: - // Controllers: - // - port 4443 (HTTPS + Athenz) from the world - // - port 443 (HTTPS + Okta) from the world - // - port 80 (HTTP) from the world - for redirect to HTTPS/443 only - trustedPorts.add(4443); - trustedPorts.add(443); - trustedPorts.add(80); - break; - - default: - throw new IllegalArgumentException("Don't know how to create ACL for " + node + - " of type " + node.type()); - } - - return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts); - } - - /** - * Creates a list of node ACLs which identify which nodes the given node should trust - * - * @param node Node for which to generate ACLs - * @param children Return ACLs for the children of the given node (e.g. containers on a Docker host) - * @return List of node ACLs - */ - public List<NodeAcl> getNodeAcls(Node node, boolean children) { - NodeList candidates = nodeRepository.list(); - if (children) { - return candidates.childrenOf(node).asList().stream() - .map(childNode -> getNodeAcl(childNode, candidates)) - .collect(Collectors.toUnmodifiableList()); - } - return List.of(getNodeAcl(node, candidates)); - } - } 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 b24fba83b12..83dba7f9856 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,10 +2,20 @@ package com.yahoo.vespa.hosted.provision.node; import com.google.common.collect.ImmutableSet; +import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancers; +import java.util.Comparator; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; /** * A node ACL. The ACL contains the node which the ACL is valid for, @@ -20,7 +30,7 @@ public class NodeAcl { private final Set<String> trustedNetworks; private final Set<Integer> trustedPorts; - public NodeAcl(Node node, Set<Node> trustedNodes, Set<String> trustedNetworks, Set<Integer> trustedPorts) { + private NodeAcl(Node node, Set<Node> trustedNodes, Set<String> trustedNetworks, Set<Integer> trustedPorts) { this.node = Objects.requireNonNull(node, "node must be non-null"); this.trustedNodes = ImmutableSet.copyOf(Objects.requireNonNull(trustedNodes, "trustedNodes must be non-null")); this.trustedNetworks = ImmutableSet.copyOf(Objects.requireNonNull(trustedNetworks, "trustedNetworks must be non-null")); @@ -43,4 +53,85 @@ public class NodeAcl { return trustedPorts; } + public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers) { + Set<Node> trustedNodes = new TreeSet<>(Comparator.comparing(Node::hostname)); + Set<Integer> trustedPorts = new LinkedHashSet<>(); + Set<String> trustedNetworks = new LinkedHashSet<>(); + + // For all cases below, trust: + // - SSH: If the Docker host has one container, and it is using the Docker host's network namespace, + // opening up SSH to the Docker host is done here as a trusted port. For simplicity all nodes have + // SSH opened (which is safe for 2 reasons: SSH daemon is not run inside containers, and NPT networks + // will (should) not forward port 22 traffic to container). + // - parent host (for health checks and metrics) + // - nodes in same application + // - load balancers allocated to application + trustedPorts.add(22); + allNodes.parentOf(node).ifPresent(trustedNodes::add); + node.allocation().ifPresent(allocation -> { + trustedNodes.addAll(allNodes.owner(allocation.owner()).asList()); + loadBalancers.list(allocation.owner()).asList() + .stream() + .map(LoadBalancer::instance) + .map(LoadBalancerInstance::networks) + .forEach(trustedNetworks::addAll); + }); + + switch (node.type()) { + case tenant: + // Tenant nodes in other states than ready, trust: + // - config servers + // - proxy nodes + // - parents of the nodes in the same application: If some of the nodes are on a different IP versions + // or only a subset of them are dual-stacked, the communication between the nodes may be NATed + // with via parent's IP address. + trustedNodes.addAll(allNodes.nodeType(NodeType.config).asList()); + trustedNodes.addAll(allNodes.nodeType(NodeType.proxy).asList()); + node.allocation().ifPresent(allocation -> + trustedNodes.addAll(allNodes.parentsOf(allNodes.owner(allocation.owner())).asList())); + + if (node.state() == Node.State.ready) { + // Tenant nodes in state ready, trust: + // - All tenant nodes in zone. When a ready node is allocated to a an application there's a brief + // window where current ACLs have not yet been applied on the node. To avoid service disruption + // during this window, ready tenant nodes trust all other tenant nodes. + trustedNodes.addAll(allNodes.nodeType(NodeType.tenant).asList()); + } + break; + + case config: + // Config servers trust: + // - all nodes + // - port 4443 from the world + trustedNodes.addAll(allNodes.asList()); + trustedPorts.add(4443); + break; + + case proxy: + // Proxy nodes trust: + // - config servers + // - all connections from the world on 4080 (insecure tb removed), and 4443 + trustedNodes.addAll(allNodes.nodeType(NodeType.config).asList()); + trustedPorts.add(443); + trustedPorts.add(4080); + trustedPorts.add(4443); + break; + + case controller: + // Controllers: + // - port 4443 (HTTPS + Athenz) from the world + // - port 443 (HTTPS + Okta) from the world + // - port 80 (HTTP) from the world - for redirect to HTTPS/443 only + trustedPorts.add(4443); + trustedPorts.add(443); + trustedPorts.add(80); + break; + + default: + throw new IllegalArgumentException("Don't know how to create ACL for " + node + + " of type " + node.type()); + } + return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts); + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java index 149e2f5d22b..e8002cf41ac 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodeAclResponse.java @@ -42,7 +42,8 @@ public class NodeAclResponse extends HttpResponse { Node node = nodeRepository.getNode(hostname) .orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'")); - List<NodeAcl> acls = nodeRepository.loadBalancers().getNodeAcls(node, aclsForChildren); + List<NodeAcl> acls = aclsForChildren ? nodeRepository.getChildAcls(node) : + List.of(node.acl(nodeRepository.list(), nodeRepository.loadBalancers())); Cursor trustedNodesArray = object.setArray("trustedNodes"); acls.forEach(nodeAcl -> toSlime(nodeAcl, trustedNodesArray)); 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 7075f4dd80b..86366e9a6d1 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 @@ -52,12 +52,12 @@ public class AclProvisioningTest { // Get trusted nodes for the first active node Node node = activeNodes.get(0); List<Node> host = node.parentHostname().flatMap(tester.nodeRepository()::getNode).map(List::of).orElseGet(List::of); - Supplier<List<NodeAcl>> nodeAcls = () -> tester.nodeRepository().loadBalancers().getNodeAcls(node, false); + Supplier<NodeAcl> nodeAcls = () -> node.acl(tester.nodeRepository().list(), tester.nodeRepository().loadBalancers()); // Trusted nodes are active nodes in same application, proxy nodes and config servers assertAcls(List.of(activeNodes, proxyNodes, configServers, host), Set.of("10.2.3.0/24", "10.4.5.0/24"), - nodeAcls.get()); + List.of(nodeAcls.get())); } @Test @@ -73,11 +73,11 @@ public class AclProvisioningTest { // Get trusted nodes for a ready tenant node Node node = tester.nodeRepository().getNodes(NodeType.tenant, Node.State.ready).get(0); - List<NodeAcl> nodeAcls = tester.nodeRepository().loadBalancers().getNodeAcls(node, false); + NodeAcl nodeAcl = node.acl(tester.nodeRepository().list(), tester.nodeRepository().loadBalancers()); List<Node> tenantNodes = tester.nodeRepository().getNodes(NodeType.tenant); // Trusted nodes are all proxy-, config-, and, tenant-nodes - assertAcls(List.of(proxyNodes, configServers, tenantNodes), nodeAcls); + assertAcls(List.of(proxyNodes, configServers, tenantNodes), List.of(nodeAcl)); } @Test @@ -95,10 +95,10 @@ public class AclProvisioningTest { // Get trusted nodes for the first config server Node node = tester.nodeRepository().getNode("cfg1") .orElseThrow(() -> new RuntimeException("Failed to find cfg1")); - List<NodeAcl> nodeAcls = tester.nodeRepository().loadBalancers().getNodeAcls(node, false); + NodeAcl nodeAcl = node.acl(tester.nodeRepository().list(), tester.nodeRepository().loadBalancers()); // Trusted nodes is all tenant nodes, all proxy nodes, all config servers and load balancer subnets - assertAcls(List.of(tenantNodes, proxyNodes, configServers), Set.of("10.2.3.0/24", "10.4.5.0/24"), nodeAcls); + assertAcls(List.of(tenantNodes, proxyNodes, configServers), Set.of("10.2.3.0/24", "10.4.5.0/24"), List.of(nodeAcl)); } @Test @@ -116,10 +116,10 @@ public class AclProvisioningTest { // Get trusted nodes for first proxy node List<Node> proxyNodes = tester.nodeRepository().getNodes(zoneApplication); Node node = proxyNodes.get(0); - List<NodeAcl> nodeAcls = tester.nodeRepository().loadBalancers().getNodeAcls(node, false); + NodeAcl nodeAcl = node.acl(tester.nodeRepository().list(), tester.nodeRepository().loadBalancers()); // Trusted nodes is all config servers and all proxy nodes - assertAcls(List.of(proxyNodes, configServers), nodeAcls); + assertAcls(List.of(proxyNodes, configServers), List.of(nodeAcl)); } @Test @@ -132,7 +132,7 @@ public class AclProvisioningTest { List<Node> dockerNodes = tester.makeReadyVirtualDockerNodes(5, new NodeResources(1, 4, 10, 1), dockerHostNodeUnderTest.hostname()); - List<NodeAcl> acls = tester.nodeRepository().loadBalancers().getNodeAcls(dockerHostNodeUnderTest, true); + List<NodeAcl> acls = tester.nodeRepository().getChildAcls(dockerHostNodeUnderTest); // ACLs for each container on the Docker host assertFalse(dockerNodes.isEmpty()); @@ -156,9 +156,9 @@ public class AclProvisioningTest { List<Node> controllers = tester.deploy(controllerApplication, Capacity.fromRequiredNodeType(NodeType.controller)); // Controllers and hosts all trust each other - List<NodeAcl> controllerAcls = tester.nodeRepository().loadBalancers().getNodeAcls(controllers.get(0), false); - assertAcls(List.of(controllers), controllerAcls); - assertEquals(Set.of(22, 80, 4443, 443), controllerAcls.get(0).trustedPorts()); + NodeAcl controllerAcl = controllers.get(0).acl(tester.nodeRepository().list(), tester.nodeRepository().loadBalancers()); + assertAcls(List.of(controllers), List.of(controllerAcl)); + assertEquals(Set.of(22, 80, 4443, 443), controllerAcl.trustedPorts()); } @Test @@ -184,7 +184,7 @@ public class AclProvisioningTest { // ACL for nodes with allocation trust their respective load balancer networks, if any for (var host : hosts) { - var acls = tester.nodeRepository().loadBalancers().getNodeAcls(host, true); + var acls = tester.nodeRepository().getChildAcls(host); assertEquals(2, acls.size()); assertEquals(Set.of(), acls.get(0).trustedNetworks()); assertEquals(application, acls.get(1).node().allocation().get().owner()); @@ -197,10 +197,10 @@ public class AclProvisioningTest { tester.makeConfigServers(3, "default", Version.fromString("6.123.456")); List<Node> readyNodes = tester.makeReadyNodes(1, "default", NodeType.proxy); - List<NodeAcl> nodeAcls = tester.nodeRepository().loadBalancers().getNodeAcls(readyNodes.get(0), false); + NodeAcl nodeAcl = readyNodes.get(0).acl(tester.nodeRepository().list(), tester.nodeRepository().loadBalancers()); - assertEquals(3, nodeAcls.get(0).trustedNodes().size()); - Iterator<Node> trustedNodes = nodeAcls.get(0).trustedNodes().iterator(); + assertEquals(3, nodeAcl.trustedNodes().size()); + Iterator<Node> trustedNodes = nodeAcl.trustedNodes().iterator(); assertEquals(Set.of("127.0.1.1"), trustedNodes.next().ipConfig().primary()); assertEquals(Set.of("127.0.1.2"), trustedNodes.next().ipConfig().primary()); assertEquals(Set.of("127.0.1.3"), trustedNodes.next().ipConfig().primary()); |