Merge pull request #1543 from yahoo/mpolden/acl-children-filter

Add request property for filtering child ACLs

3 files changed, 33 insertions, 17 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 e310d87aa96..013cf0f45e9 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
@@ -191,16 +191,17 @@ public class NodeRepository extends AbstractComponent {
      * 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) {
+    public List<NodeAcl> getNodeAcls(Node node, boolean children) {
         final List<NodeAcl> nodeAcls = new ArrayList<>();
-        nodeAcls.add(new NodeAcl(node, getTrustedNodes(node)));
 
-        // If a host node requests ACLs, we include ACLs for children of the node (e.g. containers on Docker hosts)
-        if (node.type() == NodeType.host) {
-            final List<Node> children = getNodes(node);
-            children.forEach(child -> nodeAcls.add(new NodeAcl(child, getTrustedNodes(child))));
+        if (children) {
+            final List<Node> childNodes = getNodes(node);
+            childNodes.forEach(childNode -> nodeAcls.add(new NodeAcl(childNode, getTrustedNodes(childNode))));
+        } else {
+            nodeAcls.add(new NodeAcl(node, getTrustedNodes(node)));
         }
 
         return Collections.unmodifiableList(nodeAcls);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeAclResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeAclResponse.java
index d4ab4ddb897..8330b9daa42 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeAclResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeAclResponse.java
@@ -20,13 +20,17 @@ import java.util.List;
  */
 public class NodeAclResponse extends HttpResponse {
 
+    private static final String CHILDREN_REQUEST_PROPERTY = "children";
+
     private final NodeRepository nodeRepository;
     private final Slime slime;
+    private final boolean aclsForChildren;
 
     public NodeAclResponse(HttpRequest request, NodeRepository nodeRepository) {
         super(200);
         this.nodeRepository = nodeRepository;
         this.slime = new Slime();
+        this.aclsForChildren = request.getBooleanProperty(CHILDREN_REQUEST_PROPERTY);
 
         final Cursor root = slime.setObject();
         final String hostname = baseName(request.getUri().getPath());
@@ -41,7 +45,7 @@ public class NodeAclResponse extends HttpResponse {
         Node node = nodeRepository.getNode(hostname)
                 .orElseGet(() -> nodeRepository.getConfigNode(hostname)
                         .orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'")));
-        toSlime(nodeRepository.getNodeAcls(node), object.setArray("trustedNodes"));
+        toSlime(nodeRepository.getNodeAcls(node, aclsForChildren), object.setArray("trustedNodes"));
     }
 
     private void toSlime(List<NodeAcl> nodeAcls, Cursor array) {
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 a70c23321c2..5a280cf3548 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<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node);
+        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node, false);
 
         // Trusted nodes is active nodes in same application, proxy nodes and config servers
         assertAcls(Arrays.asList(activeNodes, proxyNodes, configServers), nodeAcls);
@@ -74,7 +74,7 @@ public class AclProvisioningTest {
 
         // Get trusted nodes for the first ready node
         Node node = readyNodes.get(0);
-        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node);
+        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node, false);
 
         // Trusted nodes is proxy nodes and config servers
         assertAcls(Arrays.asList(proxyNodes, configServers), nodeAcls);
@@ -95,7 +95,7 @@ public class AclProvisioningTest {
         // Get trusted nodes for the first config server
         Node node = tester.nodeRepository().getConfigNode("cfg1")
                 .orElseThrow(() -> new RuntimeException("Failed to find cfg1"));
-        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node);
+        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node, false);
 
         // Trusted nodes is all tenant nodes, all proxy nodes and all config servers
         assertAcls(Arrays.asList(tenantNodes, proxyNodes, configServers), nodeAcls);
@@ -111,7 +111,7 @@ public class AclProvisioningTest {
 
         // Get trusted nodes for first proxy node
         Node node = proxyNodes.get(0);
-        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node);
+        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node, false);
 
         // Trusted nodes is all config servers and all proxy nodes
         assertAcls(Arrays.asList(proxyNodes, configServers), nodeAcls);
@@ -123,17 +123,28 @@ public class AclProvisioningTest {
 
         // Populate repo
         List<Node> dockerHostNodes = tester.makeReadyNodes(2, "default", NodeType.host);
+
+        List<NodeAcl> acls = tester.nodeRepository().getNodeAcls(dockerHostNodes.get(0), false);
+
+        // Trusted nodes is all Docker hosts and all config servers
+        assertAcls(Arrays.asList(dockerHostNodes, configServers), acls.get(0));
+    }
+
+    @Test
+    public void trusted_nodes_for_child_nodes_of_docker_host() {
+        List<Node> configServers = setConfigServers("cfg1:1234,cfg2:1234,cfg3:1234");
+
+        // Populate repo
+        List<Node> dockerHostNodes = tester.makeReadyNodes(2, "default", NodeType.host);
         Node dockerHostNodeUnderTest = dockerHostNodes.get(0);
         List<Node> dockerNodes = tester.makeReadyDockerNodes(5, "docker1",
                 dockerHostNodeUnderTest.hostname());
 
-        List<NodeAcl> acls = tester.nodeRepository().getNodeAcls(dockerHostNodeUnderTest);
-
-        // First ACL is for the Docker host itself
-        assertAcls(Arrays.asList(dockerHostNodes, configServers), acls.get(0));
+        List<NodeAcl> acls = tester.nodeRepository().getNodeAcls(dockerHostNodeUnderTest, true);
 
-        // ... the rest are for each container on the Docker host
+        // ACLs for each container on the Docker host
         assertFalse(dockerNodes.isEmpty());
+        assertEquals(dockerNodes.size(), acls.size());
         for (Node dockerNode : dockerNodes) {
             NodeAcl nodeAcl = acls.stream()
                     .filter(acl -> acl.node().equals(dockerNode))
@@ -153,7 +164,7 @@ public class AclProvisioningTest {
                 .addRecord("cfg3", "127.0.0.3");
 
         List<Node> readyNodes = tester.makeReadyNodes(1, "default", NodeType.tenant);
-        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(readyNodes.get(0));
+        List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(readyNodes.get(0), false);
 
         assertEquals(3, nodeAcls.get(0).trustedNodes().size());
         assertEquals(singleton("127.0.0.1"), nodeAcls.get(0).trustedNodes().get(0).ipAddresses());