Ready tenant nodes trust all other tenant nodes

2 files changed, 26 insertions, 15 deletions

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 417f0f27bf0..8d850d60cd8 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
@@ -153,23 +153,31 @@ public class NodeRepository extends AbstractComponent {
     private Set<Node> getTrustedNodes(Node node) {
         final Set<Node> trustedNodes = new TreeSet<>(Comparator.comparing(Node::hostname));
 
-        // For all cases below:
-        // - Trust nodes in same application
-        // - All config servers
+        // For all cases below, trust:
+        // - nodes in same application
+        // - config servers
         node.allocation().ifPresent(allocation -> trustedNodes.addAll(getNodes(allocation.owner())));
         trustedNodes.addAll(getConfigNodes());
 
         switch (node.type()) {
             case tenant:
-                // Tenant nodes trust nodes in same application and all infrastructure nodes
-                // They also trust all traffic from Docker hosts of trusted nodes,
-                // as it may be NATed traffic from trusted Docker containers
+                // Tenant nodes in other states than ready, trust:
+                // - proxy nodes
+                // - parent (Docker) hosts of already trusted nodes. This is needed in a transition period, while
+                //   we migrate away from IPv4-only nodes
                 trustedNodes.addAll(getDockerHosts(trustedNodes)); // TODO: Remove when we no longer have IPv4-only nodes
                 trustedNodes.addAll(getNodes(NodeType.proxy));
+                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(getNodes(NodeType.tenant));
+                }
                 break;
 
             case config:
-                // Config servers trust all nodes in the zone
+                // Config servers trust all nodes
                 trustedNodes.addAll(getNodes());
                 break;
 
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 a02a5803f43..a1270f5f821 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
@@ -72,15 +72,19 @@ public class AclProvisioningTest {
         List<Node> configServers = setConfigServers("cfg1:1234,cfg2:1234,cfg3:1234");
 
         // Populate repo
-        List<Node> readyNodes = tester.makeReadyNodes(10, "default");
+        tester.makeReadyNodes(10, "default");
         List<Node> proxyNodes = tester.makeReadyNodes(3, "default", NodeType.proxy);
 
-        // Get trusted nodes for the first ready node
-        Node node = readyNodes.get(0);
+        // Allocate 2 nodes to an application
+        allocateNodes(2);
+
+        // Get trusted nodes for a ready node
+        Node node = tester.nodeRepository().getNodes(Node.State.ready).get(0);
         List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node, false);
+        List<Node> tenantNodes = tester.nodeRepository().getNodes(NodeType.tenant);
 
-        // Trusted nodes is proxy nodes and config servers
-        assertAcls(Arrays.asList(proxyNodes, configServers), nodeAcls);
+        // Trusted nodes are all proxy-, config-, and, tenant-nodes
+        assertAcls(Arrays.asList(proxyNodes, configServers, tenantNodes), nodeAcls);
     }
 
     @Test
@@ -189,8 +193,7 @@ public class AclProvisioningTest {
                     .findFirst()
                     .orElseThrow(() -> new RuntimeException("Expected to find ACL for node " + dockerNode.hostname()));
             assertEquals(dockerHostNodeUnderTest.hostname(), dockerNode.parentHostname().get());
-            // Since the containers are unallocated, they only trust Docker hosts config servers
-            assertAcls(Collections.singletonList(configServers), nodeAcl);
+            assertAcls(Arrays.asList(configServers, dockerNodes), nodeAcl);
         }
     }
 
@@ -201,7 +204,7 @@ public class AclProvisioningTest {
                 .addRecord("cfg2", "127.0.0.2")
                 .addRecord("cfg3", "127.0.0.3");
 
-        List<Node> readyNodes = tester.makeReadyNodes(1, "default", NodeType.tenant);
+        List<Node> readyNodes = tester.makeReadyNodes(1, "default", NodeType.proxy);
         List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(readyNodes.get(0), false);
 
         assertEquals(3, nodeAcls.get(0).trustedNodes().size());