Extract TrustedNode

3 files changed, 61 insertions, 48 deletions

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 26b57677fcf..7f7b1cd1035 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
@@ -11,10 +11,12 @@ 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.Optional;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.stream.StreamSupport;
 
 /**
  * A node ACL declares which nodes, networks and ports a node should trust.
@@ -22,7 +24,7 @@ import java.util.TreeSet;
  * @author mpolden
  */
 public record NodeAcl(Node node,
-                      Set<Node> trustedNodes,
+                      Set<TrustedNode> trustedNodes,
                       Set<String> trustedNetworks,
                       Set<Integer> trustedPorts) {
 
@@ -34,7 +36,7 @@ public record NodeAcl(Node node,
     }
 
     public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers) {
-        Set<Node> trustedNodes = new TreeSet<>(Comparator.comparing(Node::hostname));
+        Set<TrustedNode> trustedNodes = new TreeSet<>(Comparator.comparing(TrustedNode::hostname));
         Set<Integer> trustedPorts = new LinkedHashSet<>();
         Set<String> trustedNetworks = new LinkedHashSet<>();
 
@@ -47,9 +49,9 @@ public record NodeAcl(Node node,
         // - nodes in same application
         // - load balancers allocated to application
         trustedPorts.add(22);
-        allNodes.parentOf(node).ifPresent(trustedNodes::add);
+        allNodes.parentOf(node).map(TrustedNode::of).ifPresent(trustedNodes::add);
         node.allocation().ifPresent(allocation -> {
-            trustedNodes.addAll(allNodes.owner(allocation.owner()).asList());
+            trustedNodes.addAll(TrustedNode.of(allNodes.owner(allocation.owner())));
             loadBalancers.list(allocation.owner()).asList()
                          .stream()
                          .map(LoadBalancer::instance)
@@ -59,57 +61,65 @@ public record NodeAcl(Node node,
         });
 
         switch (node.type()) {
-            case tenant:
+            case tenant -> {
                 // Tenant nodes in other states than ready, trust:
                 // - config servers
                 // - proxy nodes
                 // - parents of the nodes in the same application: If some nodes are on a different IP version
                 //   or only a subset of them are dual-stacked, the communication between the nodes may be NAT-ed
                 //   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()));
-
+                trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config)));
+                trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.proxy)));
+                node.allocation().ifPresent(allocation -> trustedNodes.addAll(TrustedNode.of(allNodes.parentsOf(allNodes.owner(allocation.owner())))));
                 if (node.state() == Node.State.ready) {
                     // Tenant nodes in state ready, trust:
                     // - All tenant nodes in zone. When a ready node is allocated to 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());
+                    trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.tenant)));
                 }
-                break;
-
-            case config:
+            }
+            case config -> {
                 // Config servers trust:
                 // - all nodes
                 // - port 4443 from the world
-                trustedNodes.addAll(allNodes.asList());
+                trustedNodes.addAll(TrustedNode.of(allNodes));
                 trustedPorts.add(4443);
-                break;
-
-            case proxy:
+            }
+            case proxy -> {
                 // Proxy nodes trust:
                 // - config servers
                 // - all connections from the world on 443 (production traffic) and 4443 (health checks)
-                trustedNodes.addAll(allNodes.nodeType(NodeType.config).asList());
+                trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config)));
                 trustedPorts.add(443);
                 trustedPorts.add(4443);
-                break;
-
-            case controller:
+            }
+            case controller -> {
                 // Controllers:
                 // - port 4443 (HTTPS + Athenz) from the world
                 // - port 443 (HTTPS + Okta) from the world
                 trustedPorts.add(4443);
                 trustedPorts.add(443);
-                break;
-
-            default:
-                throw new IllegalArgumentException("Don't know how to create ACL for " + node +
-                                                   " of type " + node.type());
+            }
+            default -> throw new IllegalArgumentException("Don't know how to create ACL for " + node +
+                                                          " of type " + node.type());
         }
         return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts);
     }
 
+    public record TrustedNode(String hostname, NodeType type, Set<String> ipAddresses) {
+
+        public static TrustedNode of(Node node) {
+            return new TrustedNode(node.hostname(), node.type(), node.ipConfig().primary());
+
+        }
+
+        public static List<TrustedNode> of(Iterable<Node> nodes) {
+            return StreamSupport.stream(nodes.spliterator(), false)
+                                .map(TrustedNode::of)
+                                .toList();
+        }
+
+    }
+
 }
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 b03d37b4d46..af09278623b 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
@@ -47,7 +47,7 @@ public class NodeAclResponse extends SlimeJsonResponse {
     }
 
     private void toSlime(NodeAcl nodeAcl, Cursor array) {
-        nodeAcl.trustedNodes().forEach(node -> node.ipConfig().primary().forEach(ipAddress -> {
+        nodeAcl.trustedNodes().forEach(node -> node.ipAddresses().forEach(ipAddress -> {
             Cursor object = array.addObject();
             object.setString("hostname", node.hostname());
             object.setString("type", node.type().name());
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 2346b9e2fab..f9f2b6a489a 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
@@ -10,11 +10,10 @@ 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.node.NodeAcl;
+import com.yahoo.vespa.hosted.provision.node.NodeAcl.TrustedNode;
 import org.junit.Test;
 
-import java.util.Collections;
 import java.util.Comparator;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 import java.util.function.Supplier;
@@ -56,7 +55,7 @@ public class AclProvisioningTest {
         Supplier<NodeAcl> nodeAcls = () -> node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers());
 
         // Trusted nodes are active nodes in same application, proxy nodes and config servers
-        assertAcls(List.of(activeNodes, proxyNodes, configServers.asList(), hostOfNode),
+        assertAcls(trustedNodesOf(List.of(activeNodes, proxyNodes, configServers.asList(), hostOfNode)),
                    Set.of("10.2.3.0/24", "10.4.5.0/24"),
                    List.of(nodeAcls.get()));
     }
@@ -78,7 +77,7 @@ public class AclProvisioningTest {
         NodeList tenantNodes = tester.nodeRepository().nodes().list().nodeType(NodeType.tenant);
 
         // Trusted nodes are all proxy-, config-, and, tenant-nodes
-        assertAcls(List.of(proxyNodes, configServers.asList(), tenantNodes.asList()), List.of(nodeAcl));
+        assertAcls(trustedNodesOf(List.of(proxyNodes, configServers.asList(), tenantNodes.asList())), List.of(nodeAcl));
     }
 
     @Test
@@ -99,7 +98,9 @@ public class AclProvisioningTest {
         NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers());
 
         // Trusted nodes is all tenant nodes, all proxy nodes, all config servers and load balancer subnets
-        assertAcls(List.of(tenantNodes.asList(), proxyNodes, configServers.asList()), Set.of("10.2.3.0/24", "10.4.5.0/24"), List.of(nodeAcl));
+        assertAcls(trustedNodesOf(List.of(tenantNodes.asList(), proxyNodes, configServers.asList())),
+                   Set.of("10.2.3.0/24", "10.4.5.0/24"),
+                   List.of(nodeAcl));
         assertEquals(Set.of(22, 4443), nodeAcl.trustedPorts());
     }
 
@@ -121,7 +122,7 @@ public class AclProvisioningTest {
         NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers());
 
         // Trusted nodes is all config servers and all proxy nodes
-        assertAcls(List.of(proxyNodes.asList(), configServers.asList()), List.of(nodeAcl));
+        assertAcls(trustedNodesOf(List.of(proxyNodes.asList(), configServers.asList())), List.of(nodeAcl));
         assertEquals(Set.of(22, 443, 4443), nodeAcl.trustedPorts());
     }
 
@@ -146,7 +147,7 @@ public class AclProvisioningTest {
                     .findFirst()
                     .orElseThrow(() -> new RuntimeException("Expected to find ACL for node " + node.hostname()));
             assertEquals(host.hostname(), node.parentHostname().get());
-            assertAcls(List.of(configServers.asList(), nodes, List.of(host)), nodeAcl);
+            assertAcls(trustedNodesOf(List.of(configServers.asList(), nodes, List.of(host))), nodeAcl);
         }
     }
 
@@ -160,7 +161,7 @@ public class AclProvisioningTest {
 
         // Controllers and hosts all trust each other
         NodeAcl controllerAcl = controllers.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers());
-        assertAcls(List.of(controllers), Set.of("10.2.3.0/24", "10.4.5.0/24"), List.of(controllerAcl));
+        assertAcls(trustedNodesOf(List.of(controllers)), Set.of("10.2.3.0/24", "10.4.5.0/24"), List.of(controllerAcl));
         assertEquals(Set.of(22, 4443, 443), controllerAcl.trustedPorts());
     }
 
@@ -203,10 +204,12 @@ public class AclProvisioningTest {
         NodeAcl nodeAcl = readyNodes.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers());
 
         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());
+        assertEquals(List.of(Set.of("127.0.1.1"), Set.of("127.0.1.2"), Set.of("127.0.1.3")),
+                     nodeAcl.trustedNodes().stream().map(TrustedNode::ipAddresses).toList());
+    }
+
+    private static List<List<TrustedNode>> trustedNodesOf(List<List<Node>> nodes) {
+        return nodes.stream().map(TrustedNode::of).toList();
     }
 
     private List<Node> deploy(int nodeCount) {
@@ -217,24 +220,24 @@ public class AclProvisioningTest {
         return tester.deploy(application, Capacity.from(new ClusterResources(nodeCount, 1, nodeResources)));
     }
 
-    private static void assertAcls(List<List<Node>> expected, NodeAcl actual) {
-        assertAcls(expected, Collections.singletonList(actual));
+    private static void assertAcls(List<List<TrustedNode>> expected, NodeAcl actual) {
+        assertAcls(expected, List.of(actual));
     }
 
-    private static void assertAcls(List<List<Node>> expectedNodes, List<NodeAcl> actual) {
+    private static void assertAcls(List<List<TrustedNode>> expectedNodes, List<NodeAcl> actual) {
         assertAcls(expectedNodes, Set.of(), actual);
     }
 
-    private static void assertAcls(List<List<Node>> expectedNodes, Set<String> expectedNetworks, List<NodeAcl> actual) {
-        List<Node> expectedTrustedNodes = expectedNodes.stream()
+    private static void assertAcls(List<List<TrustedNode>> expectedNodes, Set<String> expectedNetworks, List<NodeAcl> actual) {
+        List<TrustedNode> expectedTrustedNodes = expectedNodes.stream()
                 .flatMap(List::stream)
                 .distinct()
-                .sorted(Comparator.comparing(Node::hostname))
+                .sorted(Comparator.comparing(TrustedNode::hostname))
                 .collect(Collectors.toList());
-        List<Node> actualTrustedNodes = actual.stream()
+        List<TrustedNode> actualTrustedNodes = actual.stream()
                 .flatMap(acl -> acl.trustedNodes().stream())
                 .distinct()
-                .sorted(Comparator.comparing(Node::hostname))
+                .sorted(Comparator.comparing(TrustedNode::hostname))
                 .collect(Collectors.toList());
         assertEquals(expectedTrustedNodes, actualTrustedNodes);