Merge pull request #16428 from vespa-engine/bratseth/node-repo-refactor

Bratseth/node repo refactor

9 files changed, 206 insertions, 158 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 8e14b61db9a..2642983dd2a 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
@@ -20,10 +20,7 @@ import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.hosted.provision.Node.State;
 import com.yahoo.vespa.hosted.provision.applications.Applications;
-import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
-import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId;
-import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance;
-import com.yahoo.vespa.hosted.provision.lb.LoadBalancerList;
+import com.yahoo.vespa.hosted.provision.lb.LoadBalancers;
 import com.yahoo.vespa.hosted.provision.maintenance.InfrastructureVersions;
 import com.yahoo.vespa.hosted.provision.maintenance.NodeFailer;
 import com.yahoo.vespa.hosted.provision.maintenance.PeriodicApplicationMaintainer;
@@ -50,17 +47,13 @@ import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Comparator;
 import java.util.EnumSet;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
-import java.util.TreeSet;
 import java.util.function.BiFunction;
-import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -111,6 +104,7 @@ public class NodeRepository extends AbstractComponent {
     private final ContainerImages containerImages;
     private final JobControl jobControl;
     private final Applications applications;
+    private final LoadBalancers loadBalancers;
     private final int spareCount;
 
     /**
@@ -171,6 +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.spareCount = spareCount;
         rewriteNodes();
     }
@@ -213,15 +208,30 @@ public class NodeRepository extends AbstractComponent {
     /** Returns this node repo's view of the applications deployed to it */
     public Applications applications() { return applications; }
 
-    public NodeFlavors flavors() {
-        return flavors;
-    }
+    /** Returns the load balancers available in this node repo */
+    public LoadBalancers loadBalancers() { return loadBalancers; }
+
+    public NodeFlavors flavors() { return flavors; }
 
     public HostResourcesCalculator resourcesCalculator() { return resourcesCalculator; }
 
     /** 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().isHost()) 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 ----------------------------------------------------------------
 
     /**
@@ -274,125 +284,11 @@ public class NodeRepository extends AbstractComponent {
         return new LockedNodeList(getNodes(), lock);
     }
 
-    /** Returns a filterable list of all load balancers in this repository */
-    public LoadBalancerList loadBalancers() {
-        return loadBalancers((ignored) -> true);
-    }
-
-    /** Returns a filterable list of load balancers belonging to given application */
-    public LoadBalancerList loadBalancers(ApplicationId application) {
-        return loadBalancers((id) -> id.application().equals(application));
-    }
-
-    private LoadBalancerList loadBalancers(Predicate<LoadBalancerId> predicate) {
-        return LoadBalancerList.copyOf(db.readLoadBalancers(predicate).values());
-    }
-
     public List<Node> getNodes(ApplicationId id, State ... inState) { return db.readNodes(id, inState); }
     public List<Node> getInactive() { return db.readNodes(State.inactive); }
     public List<Node> getFailed() { return db.readNodes(State.failed); }
 
     /**
-     * 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());
-            loadBalancers(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() == 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:
-                illegal("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 = list();
-        if (children) {
-            return candidates.childrenOf(node).asList().stream()
-                             .map(childNode -> getNodeAcl(childNode, candidates))
-                             .collect(Collectors.toUnmodifiableList());
-        }
-        return List.of(getNodeAcl(node, candidates));
-    }
-
-    /**
      * Returns whether the zone managed by this node repository seems to be working.
      * If too many nodes are not responding, there is probably some zone-wide issue
      * and we should probably refrain from making changes to it.
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
new file mode 100644
index 00000000000..9a2652e884c
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancers.java
@@ -0,0 +1,48 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.lb;
+
+import com.yahoo.config.provision.ApplicationId;
+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.NodeRepository;
+import com.yahoo.vespa.hosted.provision.node.NodeAcl;
+import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient;
+
+import java.util.Comparator;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+/**
+ * The load balancers of this node repo.
+ *
+ * @author bratseth
+ * @author mpolden
+ */
+public class LoadBalancers {
+
+    private final CuratorDatabaseClient db;
+
+    public LoadBalancers(CuratorDatabaseClient db) {
+        this.db = db;
+    }
+
+    /** Returns a filterable list of all load balancers in this repository */
+    public LoadBalancerList list() {
+        return list((ignored) -> true);
+    }
+
+    /** Returns a filterable list of load balancers belonging to given application */
+    public LoadBalancerList list(ApplicationId application) {
+        return list((id) -> id.application().equals(application));
+    }
+
+    private LoadBalancerList list(Predicate<LoadBalancerId> predicate) {
+        return LoadBalancerList.copyOf(db.readLoadBalancers(predicate).values());
+    }
+
+}
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/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
index a048f8bb8d2..cfcaa1f83c0 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
@@ -115,12 +115,12 @@ public class LoadBalancerProvisioner {
      * load balancer(s).
      */
     public void deactivate(ApplicationTransaction transaction) {
-        deactivate(nodeRepository.loadBalancers(transaction.application()).asList(), transaction.nested());
+        deactivate(nodeRepository.loadBalancers().list(transaction.application()).asList(), transaction.nested());
     }
 
     /** Returns load balancers of given application that are no longer referenced by given clusters */
     private List<LoadBalancer> surplusLoadBalancersOf(ApplicationId application, Set<ClusterSpec.Id> activeClusters) {
-        var activeLoadBalancersByCluster = nodeRepository.loadBalancers(application)
+        var activeLoadBalancersByCluster = nodeRepository.loadBalancers().list(application)
                                                          .in(LoadBalancer.State.active)
                                                          .asList()
                                                          .stream()
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
index 092426c75d4..1ef449555d9 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
@@ -40,9 +40,9 @@ public class LoadBalancersResponse extends HttpResponse {
         LoadBalancerList loadBalancers;
         var application = application();
         if (application.isPresent()) {
-            loadBalancers = nodeRepository.loadBalancers(application.get());
+            loadBalancers = nodeRepository.loadBalancers().list(application.get());
         } else {
-            loadBalancers = nodeRepository.loadBalancers();
+            loadBalancers = nodeRepository.loadBalancers().list();
         }
         return loadBalancers.asList();
     }
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 07e93111b6f..2d2feccc114 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
@@ -31,7 +31,7 @@ public class NodeAclResponse extends HttpResponse {
         super(200);
         this.nodeRepository = nodeRepository;
         this.slime = new Slime();
-        this.aclsForChildren = request.getBooleanProperty(CHILDREN_REQUEST_PROPERTY);
+        this.aclsForChildren = request.getBooleanProperty(CHILDREN_REQUEST_PROPERTY); // This is always true?
 
         Cursor root = slime.setObject();
         String hostname = baseName(request.getUri().getPath());
@@ -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.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 fd54a44c3df..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().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().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().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().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().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().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
@@ -177,14 +177,14 @@ public class AclProvisioningTest {
         assertEquals(2, activeNodes.size());
 
         // Load balancer is allocated to application
-        var loadBalancers = tester.nodeRepository().loadBalancers(application);
+        var loadBalancers = tester.nodeRepository().loadBalancers().list(application);
         assertEquals(1, loadBalancers.asList().size());
         var lbNetworks = loadBalancers.asList().get(0).instance().networks();
         assertEquals(2, lbNetworks.size());
 
         // ACL for nodes with allocation trust their respective load balancer networks, if any
         for (var host : hosts) {
-            var acls = tester.nodeRepository().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().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());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
index 105f2122e0c..ec08ed15ab4 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
@@ -49,8 +49,8 @@ public class LoadBalancerProvisionerTest {
 
     @Test
     public void provision_load_balancer() {
-        Supplier<List<LoadBalancer>> lbApp1 = () -> tester.nodeRepository().loadBalancers(app1).asList();
-        Supplier<List<LoadBalancer>> lbApp2 = () -> tester.nodeRepository().loadBalancers(app2).asList();
+        Supplier<List<LoadBalancer>> lbApp1 = () -> tester.nodeRepository().loadBalancers().list(app1).asList();
+        Supplier<List<LoadBalancer>> lbApp2 = () -> tester.nodeRepository().loadBalancers().list(app2).asList();
         ClusterSpec.Id containerCluster1 = ClusterSpec.Id.from("qrs1");
         ClusterSpec.Id contentCluster = ClusterSpec.Id.from("content");
 
@@ -82,7 +82,7 @@ public class LoadBalancerProvisionerTest {
         tester.activate(app1, prepare(app1,
                                       clusterRequest(ClusterSpec.Type.container, containerCluster1),
                                       clusterRequest(ClusterSpec.Type.content, contentCluster)));
-        LoadBalancer loadBalancer = tester.nodeRepository().loadBalancers(app1).asList().get(0);
+        LoadBalancer loadBalancer = tester.nodeRepository().loadBalancers().list(app1).asList().get(0);
         assertEquals(2, loadBalancer.instance().reals().size());
         assertTrue("Failed node is removed", loadBalancer.instance().reals().stream()
                                                          .map(Real::hostname)
@@ -159,7 +159,7 @@ public class LoadBalancerProvisionerTest {
         tester.makeReadyHosts(2, resources);
         tester.activateTenantHosts();
         var nodes = tester.prepare(app1, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs")), 2 , 1, resources);
-        Supplier<LoadBalancer> lb = () -> tester.nodeRepository().loadBalancers(app1).asList().get(0);
+        Supplier<LoadBalancer> lb = () -> tester.nodeRepository().loadBalancers().list(app1).asList().get(0);
         assertTrue("Load balancer provisioned with empty reals", tester.loadBalancerService().instances().get(lb.get().id()).reals().isEmpty());
         assignIps(tester.nodeRepository().getNodes(app1));
         tester.activate(app1, nodes);
@@ -185,7 +185,7 @@ public class LoadBalancerProvisionerTest {
                                            clusterRequest(ClusterSpec.Type.container,
                                                           ClusterSpec.Id.from("tenant-host"))));
         assertTrue("No load balancer provisioned", tester.loadBalancerService().instances().isEmpty());
-        assertEquals(List.of(), tester.nodeRepository().loadBalancers(infraApp1).asList());
+        assertEquals(List.of(), tester.nodeRepository().loadBalancers().list(infraApp1).asList());
     }
 
     @Test
@@ -193,12 +193,12 @@ public class LoadBalancerProvisionerTest {
         tester.activate(app1, prepare(app1, clusterRequest(ClusterSpec.Type.content,
                                                            ClusterSpec.Id.from("tenant-host"))));
         assertTrue("No load balancer provisioned", tester.loadBalancerService().instances().isEmpty());
-        assertEquals(List.of(), tester.nodeRepository().loadBalancers(app1).asList());
+        assertEquals(List.of(), tester.nodeRepository().loadBalancers().list(app1).asList());
     }
 
     @Test
     public void provision_load_balancer_combined_cluster() {
-        Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers(app1).asList();
+        Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers().list(app1).asList();
         var combinedId = ClusterSpec.Id.from("container1");
         var nodes = prepare(app1, clusterRequest(ClusterSpec.Type.combined, ClusterSpec.Id.from("content1"), Optional.of(combinedId)));
         assertEquals(1, lbs.get().size());
@@ -211,7 +211,7 @@ public class LoadBalancerProvisionerTest {
     @Test
     public void provision_load_balancer_config_server_cluster() {
         ApplicationId configServerApp = ApplicationId.from("hosted-vespa", "zone-config-servers", "default");
-        Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers(configServerApp).asList();
+        Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers().list(configServerApp).asList();
         var cluster = ClusterSpec.Id.from("zone-config-servers");
         var nodes = prepare(configServerApp, Capacity.fromRequiredNodeType(NodeType.config),
                             clusterRequest(ClusterSpec.Type.admin, cluster));
@@ -226,7 +226,7 @@ public class LoadBalancerProvisionerTest {
     @Test
     public void provision_load_balancer_controller_cluster() {
         ApplicationId controllerApp = ApplicationId.from("hosted-vespa", "controller", "default");
-        Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers(controllerApp).asList();
+        Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers().list(controllerApp).asList();
         var cluster = ClusterSpec.Id.from("zone-config-servers");
         var nodes = prepare(controllerApp, Capacity.fromRequiredNodeType(NodeType.controller),
                             clusterRequest(ClusterSpec.Type.container, cluster));