Guard with simpler-acl flag, enabled by default

5 files changed, 41 insertions, 38 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 b8e237b57cf..81ac4544223 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
@@ -604,8 +604,8 @@ public final class Node implements Nodelike {
     }
 
     /** Returns the ACL for the node (trusted nodes, networks and ports) */
-    public NodeAcl acl(NodeList allNodes, LoadBalancers loadBalancers, Zone zone) {
-        return NodeAcl.from(this, allNodes, loadBalancers, zone);
+    public NodeAcl acl(NodeList allNodes, LoadBalancers loadBalancers, Zone zone, boolean simplerAcl) {
+        return NodeAcl.from(this, allNodes, loadBalancers, zone, simplerAcl);
     }
 
     @Override
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 d6671d41cbd..8be3fbd1bf2 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
@@ -215,11 +215,11 @@ public class NodeRepository extends AbstractComponent {
      * @param host node for which to generate ACLs
      * @return the list of node ACLs
      */
-    public List<NodeAcl> getChildAcls(Node host) {
+    public List<NodeAcl> getChildAcls(Node host, boolean simplerAcl) {
         if ( ! host.type().isHost()) throw new IllegalArgumentException("Only hosts have children");
         NodeList allNodes = nodes().list();
         return allNodes.childrenOf(host)
-                       .mapToList(childNode -> childNode.acl(allNodes, loadBalancers, zone));
+                       .mapToList(childNode -> childNode.acl(allNodes, loadBalancers, zone, simplerAcl));
     }
 
     /** Removes this application: all nodes are set dirty. */
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 e71b2a56676..17a04238ac9 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
@@ -43,7 +43,7 @@ public record NodeAcl(Node node,
         this.trustedUdpPorts = ImmutableSet.copyOf(Objects.requireNonNull(trustedUdpPorts, "trustedUdpPorts must be non-null"));
     }
 
-    public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers, Zone zone) {
+    public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers, Zone zone, boolean simplerAcl) {
         Set<TrustedNode> trustedNodes = new TreeSet<>(Comparator.comparing(TrustedNode::hostname));
         Set<Integer> trustedPorts = new LinkedHashSet<>();
         Set<Integer> trustedUdpPorts = new LinkedHashSet<>();
@@ -58,9 +58,9 @@ public record NodeAcl(Node node,
         // - nodes in same application
         // - load balancers allocated to application
         trustedPorts.add(22);
-        allNodes.parentOf(node).map(parent -> TrustedNode.of(parent, node.cloudAccount())).ifPresent(trustedNodes::add);
+        allNodes.parentOf(node).map(parent -> TrustedNode.of(parent, node.cloudAccount(), simplerAcl)).ifPresent(trustedNodes::add);
         node.allocation().ifPresent(allocation -> {
-            trustedNodes.addAll(TrustedNode.of(allNodes.owner(allocation.owner()), node.cloudAccount()));
+            trustedNodes.addAll(TrustedNode.of(allNodes.owner(allocation.owner()), node.cloudAccount(), simplerAcl));
             loadBalancers.list(allocation.owner()).asList()
                          .stream()
                          .map(LoadBalancer::instance)
@@ -77,15 +77,15 @@ public record NodeAcl(Node node,
                 // - 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(TrustedNode.of(allNodes.nodeType(NodeType.config), node.cloudAccount()));
-                trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.proxy), node.cloudAccount()));
-                node.allocation().ifPresent(allocation -> trustedNodes.addAll(TrustedNode.of(allNodes.parentsOf(allNodes.owner(allocation.owner())), node.cloudAccount())));
+                trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config), node.cloudAccount(), simplerAcl));
+                trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.proxy), node.cloudAccount(), simplerAcl));
+                node.allocation().ifPresent(allocation -> trustedNodes.addAll(TrustedNode.of(allNodes.parentsOf(allNodes.owner(allocation.owner())), node.cloudAccount(), simplerAcl)));
                 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(TrustedNode.of(allNodes.nodeType(NodeType.tenant), node.cloudAccount()));
+                    trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.tenant), node.cloudAccount(), simplerAcl));
                 }
             }
             case config -> {
@@ -97,7 +97,8 @@ public record NodeAcl(Node node,
                 trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.host, NodeType.tenant,
                                                                      NodeType.proxyhost, NodeType.proxy),
                                                    RPC_PORTS,
-                                                   node.cloudAccount()));
+                                                   node.cloudAccount(),
+                                                   simplerAcl));
                 trustedPorts.add(4443);
                 if (zone.system().isPublic() && zone.cloud().allowEnclave()) {
                     trustedUdpPorts.add(WIREGUARD_PORT);
@@ -107,7 +108,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()));
+                trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config), node.cloudAccount(), simplerAcl));
                 trustedPorts.add(443);
                 trustedPorts.add(4443);
             }
@@ -127,28 +128,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 cloudAccount) {
+        public static TrustedNode of(Node node, Set<Integer> ports, CloudAccount cloudAccount, boolean simplerAcl) {
             Set<String> ipAddresses = node.ipConfig()
                                           .primary()
                                           .stream()
-                                          .filter(ip -> IP.inSharedIpSpace(ip, node.cloudAccount(), cloudAccount))
+                                          .filter(ip -> !simplerAcl || IP.inSharedIpSpace(ip, node.cloudAccount(), cloudAccount))
                                           .collect(Collectors.toSet());
             return new TrustedNode(node.hostname(), node.type(), ipAddresses, ports);
         }
 
         /** Trust all ports from given node */
-        public static TrustedNode of(Node node, CloudAccount cloudAccount) {
-            return of(node, Set.of(), cloudAccount);
+        public static TrustedNode of(Node node, CloudAccount cloudAccount, boolean simplerAcl) {
+            return of(node, Set.of(), cloudAccount, simplerAcl);
         }
 
-        public static List<TrustedNode> of(Iterable<Node> nodes, Set<Integer> ports, CloudAccount cloudAccount) {
+        public static List<TrustedNode> of(Iterable<Node> nodes, Set<Integer> ports, CloudAccount cloudAccount, boolean simplerAcl) {
             return StreamSupport.stream(nodes.spliterator(), false)
-                                .map(node -> TrustedNode.of(node, ports, cloudAccount))
+                                .map(node -> TrustedNode.of(node, ports, cloudAccount, simplerAcl))
                                 .toList();
         }
 
-        public static List<TrustedNode> of(Iterable<Node> nodes, CloudAccount cloudAccount) {
-            return of(nodes, Set.of(), cloudAccount);
+        public static List<TrustedNode> of(Iterable<Node> nodes, CloudAccount cloudAccount, boolean simplerAcl) {
+            return of(nodes, Set.of(), cloudAccount, simplerAcl);
         }
 
     }
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 6fe14715355..784f8f82d14 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
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.restapi;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.restapi.SlimeJsonResponse;
 import com.yahoo.slime.Cursor;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.NodeAcl;
@@ -33,8 +34,9 @@ public class NodeAclResponse extends SlimeJsonResponse {
         Node node = nodeRepository.nodes().node(hostname)
                 .orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'"));
 
-        List<NodeAcl> acls = aclsForChildren ? nodeRepository.getChildAcls(node) :
-                                               List.of(node.acl(nodeRepository.nodes().list(), nodeRepository.loadBalancers(), nodeRepository.zone()));
+        boolean simplerAcl = Flags.SIMPLER_ACL.bindTo(nodeRepository.flagSource()).value();
+        List<NodeAcl> acls = aclsForChildren ? nodeRepository.getChildAcls(node, simplerAcl) :
+                                               List.of(node.acl(nodeRepository.nodes().list(), nodeRepository.loadBalancers(), nodeRepository.zone(), simplerAcl));
 
         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 87b9a85edcd..8d487dea38b 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
@@ -58,7 +58,7 @@ public class AclProvisioningTest {
         // Get trusted nodes for the first active node
         Node node = activeNodes.get(0);
         List<Node> hostOfNode = node.parentHostname().flatMap(tester.nodeRepository().nodes()::node).map(List::of).orElseGet(List::of);
-        Supplier<NodeAcl> nodeAcls = () -> node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone());
+        Supplier<NodeAcl> nodeAcls = () -> node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true);
 
         // Trusted nodes are active nodes in same application, proxy nodes and config servers
         assertAcls(trustedNodesOf(List.of(activeNodes, proxyNodes, configServers.asList(), hostOfNode), node.cloudAccount()),
@@ -79,7 +79,7 @@ public class AclProvisioningTest {
 
         // Get trusted nodes for a ready tenant node
         Node node = tester.nodeRepository().nodes().list(Node.State.ready).nodeType(NodeType.tenant).first().get();
-        NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone());
+        NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true);
         NodeList tenantNodes = tester.nodeRepository().nodes().list().nodeType(NodeType.tenant);
 
         // Trusted nodes are all proxy-, config-, and, tenant-nodes
@@ -105,14 +105,14 @@ public class AclProvisioningTest {
         // Get trusted nodes for the first config server
         Node node = tester.nodeRepository().nodes().node("cfg1")
                 .orElseThrow(() -> new RuntimeException("Failed to find cfg1"));
-        NodeAcl nodeAcl = node.acl(nodes, tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone());
+        NodeAcl nodeAcl = node.acl(nodes, tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true);
 
         // Trusted nodes is all tenant nodes+hosts, all proxy nodes+hosts, all config servers and load balancer subnets
-        assertAcls(List.of(TrustedNode.of(tenantHosts, Set.of(19070), node.cloudAccount()),
-                           TrustedNode.of(tenantNodes, Set.of(19070), node.cloudAccount()),
-                           TrustedNode.of(proxyHosts, Set.of(19070), node.cloudAccount()),
-                           TrustedNode.of(proxyNodes, Set.of(19070), node.cloudAccount()),
-                           TrustedNode.of(configNodes, node.cloudAccount())),
+        assertAcls(List.of(TrustedNode.of(tenantHosts, Set.of(19070), node.cloudAccount(), true),
+                           TrustedNode.of(tenantNodes, Set.of(19070), node.cloudAccount(), true),
+                           TrustedNode.of(proxyHosts, Set.of(19070), node.cloudAccount(), true),
+                           TrustedNode.of(proxyNodes, Set.of(19070), node.cloudAccount(), true),
+                           TrustedNode.of(configNodes, node.cloudAccount(), true)),
                    Set.of("10.2.3.0/24", "10.4.5.0/24"),
                    List.of(nodeAcl));
         assertEquals(Set.of(22, 4443), nodeAcl.trustedPorts());
@@ -123,7 +123,7 @@ public class AclProvisioningTest {
         publicTester.makeConfigServers(3, "default", Version.fromString("6.123.456"));
         Node publicCfgNode = publicTester.nodeRepository().nodes().node("cfg1")
                 .orElseThrow(() -> new RuntimeException("Failed to find cfg1"));
-        NodeAcl publicNodeAcl = publicCfgNode.acl(nodes, publicTester.nodeRepository().loadBalancers(), publicTester.nodeRepository().zone());
+        NodeAcl publicNodeAcl = publicCfgNode.acl(nodes, publicTester.nodeRepository().loadBalancers(), publicTester.nodeRepository().zone(), true);
         assertEquals(Set.of(51820), publicNodeAcl.trustedUdpPorts());
     }
 
@@ -142,7 +142,7 @@ public class AclProvisioningTest {
         // Get trusted nodes for first proxy node
         NodeList proxyNodes = tester.nodeRepository().nodes().list().owner(zoneApplication);
         Node node = proxyNodes.first().get();
-        NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone());
+        NodeAcl nodeAcl = node.acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true);
 
         // Trusted nodes is all config servers and all proxy nodes
         assertAcls(trustedNodesOf(List.of(proxyNodes.asList(), configServers.asList()), node.cloudAccount()), List.of(nodeAcl));
@@ -160,7 +160,7 @@ public class AclProvisioningTest {
         List<Node> nodes = tester.makeReadyChildren(5, new NodeResources(1, 4, 10, 1),
                                                           host.hostname());
 
-        List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host);
+        List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host, true);
 
         // ACLs for each container on the host
         assertFalse(nodes.isEmpty());
@@ -184,7 +184,7 @@ public class AclProvisioningTest {
         List<Node> controllers = tester.deploy(controllerApplication, Capacity.fromRequiredNodeType(NodeType.controller));
 
         // Controllers and hosts all trust each other
-        NodeAcl controllerAcl = controllers.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone());
+        NodeAcl controllerAcl = controllers.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true);
         assertAcls(trustedNodesOf(List.of(controllers), controllers.get(0).cloudAccount()), Set.of("10.2.3.0/24", "10.4.5.0/24"), List.of(controllerAcl));
         assertEquals(Set.of(22, 4443, 443), controllerAcl.trustedPorts());
         assertEquals(Set.of(), controllerAcl.trustedUdpPorts());
@@ -213,7 +213,7 @@ public class AclProvisioningTest {
 
         // ACL for nodes with allocation trust their respective load balancer networks, if any
         for (var host : hosts) {
-            List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host);
+            List<NodeAcl> acls = tester.nodeRepository().getChildAcls(host, true);
             assertEquals(2, acls.size());
             for (var acl : acls) {
                 if (acl.node().allocation().isPresent()) {
@@ -231,7 +231,7 @@ public class AclProvisioningTest {
         tester.makeConfigServers(3, "default", Version.fromString("6.123.456"));
 
         List<Node> readyNodes = tester.makeReadyNodes(1, "default", NodeType.proxy);
-        NodeAcl nodeAcl = readyNodes.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone());
+        NodeAcl nodeAcl = readyNodes.get(0).acl(tester.nodeRepository().nodes().list(), tester.nodeRepository().loadBalancers(), tester.nodeRepository().zone(), true);
 
         assertEquals(3, nodeAcl.trustedNodes().size());
         assertEquals(List.of(Set.of("127.0.1.1"), Set.of("127.0.1.2"), Set.of("127.0.1.3")),
@@ -239,7 +239,7 @@ public class AclProvisioningTest {
     }
 
     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)).toList();
+        return nodes.stream().map(node -> TrustedNode.of(node, ports, cloudAccount, true)).toList();
     }
 
     private static List<List<TrustedNode>> trustedNodesOf(List<List<Node>> nodes, CloudAccount cloudAccount) {