cfg trusts host&proxyhost only if NATed

3 files changed, 33 insertions, 88 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 155b6e8b740..14c4a63a500 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
@@ -12,12 +12,15 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancers;
 
 import java.util.Comparator;
+import java.util.EnumSet;
 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.function.Consumer;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
@@ -55,12 +58,13 @@ public record NodeAcl(Node node,
         //   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 (Slobrok for tenant nodes, file distribution and ZK for config servers, etc).
+        // - nodes in same application (Slobrok for tenant nodes, file distribution and ZK for config servers, etc),
+        //   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);
         node.allocation().ifPresent(allocation -> {
-            trustedNodes.addAll(TrustedNode.of(allNodes.owner(allocation.owner()), node.cloudAccount(), simplerAcl));
+            trustedNodes.addAll(trustedNodesForChildrenMatching(node, allNodes, n -> n.allocation().map(Allocation::owner).equals(Optional.of(allocation.owner())), Set.of(), simplerAcl));
             loadBalancers.list(allocation.owner()).asList()
                          .stream()
                          .map(LoadBalancer::instance)
@@ -76,22 +80,6 @@ public record NodeAcl(Node node,
                 // - proxy nodes
                 trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.config), node.cloudAccount(), simplerAcl));
                 trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.proxy), node.cloudAccount(), simplerAcl));
-                // - 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
-                boolean hasIp4 = node.ipConfig().primary().stream().anyMatch(IP::isV4);
-                boolean hasIp6 = node.ipConfig().primary().stream().anyMatch(IP::isV6);
-                node.allocation().ifPresent(allocation -> allNodes
-                        .owner(allocation.owner())
-                        .stream()
-                        .filter(n -> !n.hostname().equals(node.hostname()))
-                        .forEach(otherNode -> {
-                            if (hasIp4 && otherNode.ipConfig().primary().stream().noneMatch(IP::isV4) ||
-                                hasIp6 && otherNode.ipConfig().primary().stream().noneMatch(IP::isV6)) {
-                                // The parent host is assumed to have the required IPv4/IPv6 address for NAT
-                                trustedNodes.add(TrustedNode.of(allNodes.parentOf(otherNode).orElseThrow(), node.cloudAccount(), simplerAcl));
-                            }
-                        }));
             }
             case config -> {
                 // Config servers trust:
@@ -99,11 +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(TrustedNode.of(allNodes.nodeType(NodeType.host, NodeType.tenant,
-                                                                     NodeType.proxyhost, NodeType.proxy),
-                                                   RPC_PORTS,
-                                                   node.cloudAccount(),
-                                                   simplerAcl));
+                trustedNodes.addAll(trustedNodesForChildrenMatching(node, allNodes, n -> EnumSet.of(NodeType.tenant, NodeType.proxy).contains(n.type()), RPC_PORTS, simplerAcl));
                 trustedPorts.add(4443);
                 if (zone.system().isPublic() && zone.cloud().allowEnclave()) {
                     trustedUdpPorts.add(WIREGUARD_PORT);
@@ -130,6 +114,29 @@ public record NodeAcl(Node node,
         return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts, trustedUdpPorts);
     }
 
+    /** 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) {
+        if (node.type().isHost())
+            throw new IllegalArgumentException("Host nodes cannot have NAT parents");
+
+        boolean hasIp4 = node.ipConfig().primary().stream().anyMatch(IP::isV4);
+        boolean hasIp6 = node.ipConfig().primary().stream().anyMatch(IP::isV6);
+        return allNodes.stream()
+                       .filter(n -> !n.type().isHost())
+                       .filter(childNodeSelector)
+                       .mapMulti((Node otherNode, Consumer<TrustedNode> consumer) -> {
+                           consumer.accept(TrustedNode.of(otherNode, ports, node.cloudAccount(), simplerAcl));
+
+                           // 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));
+                           }
+                       })
+                       .collect(Collectors.toSet());
+    }
+
     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 */
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 1fb339e8814..b7dae79ffd7 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
@@ -110,10 +110,11 @@ public class AclProvisioningTest {
                 .orElseThrow(() -> new RuntimeException("Failed to find cfg1"));
         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
+        // 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(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"),
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 de7b4de7fd9..a4afe470ce9 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
@@ -25,33 +25,6 @@
       "trustedBy": "cfg1.yahoo.com"
     },
     {
-      "hostname": "dockerhost1.yahoo.com",
-      "type": "host",
-      "ipAddress": "::100:1",
-      "ports": [
-        19070
-      ],
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost1.yahoo.com",
-      "type": "host",
-      "ipAddress": "127.0.100.1",
-      "ports": [
-        19070
-      ],
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost2.yahoo.com",
-      "type": "host",
-      "ipAddress": "::101:1",
-      "ports": [
-        19070
-      ],
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
       "hostname": "dockerhost3.yahoo.com",
       "type": "host",
       "ipAddress": "::102:1",
@@ -70,42 +43,6 @@
       "trustedBy": "cfg1.yahoo.com"
     },
     {
-      "hostname": "dockerhost4.yahoo.com",
-      "type": "host",
-      "ipAddress": "::103:1",
-      "ports": [
-        19070
-      ],
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost4.yahoo.com",
-      "type": "host",
-      "ipAddress": "127.0.103.1",
-      "ports": [
-        19070
-      ],
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost5.yahoo.com",
-      "type": "host",
-      "ipAddress": "::104:1",
-      "ports": [
-        19070
-      ],
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost5.yahoo.com",
-      "type": "host",
-      "ipAddress": "127.0.104.1",
-      "ports": [
-        19070
-      ],
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
       "hostname": "host1.yahoo.com",
       "type": "tenant",
       "ipAddress": "::1:1",