Merge branch 'master' into hakonhall/enforce-cc-timeouts-in-orchestrator-2

5 files changed, 54 insertions, 111 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 b0e1632002b..5060510be20 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
@@ -177,23 +177,22 @@ public class NodeRepository extends AbstractComponent {
      */
     private NodeAcl getNodeAcl(Node node, NodeList candidates) {
         Set<Node> trustedNodes = new TreeSet<>(Comparator.comparing(Node::hostname));
-        Set<String> trustedNetworks = new HashSet<>();
         Set<Integer> trustedPorts = new HashSet<>();
 
         // For all cases below, trust:
         // - nodes in same application
-        // - config servers
         // - ssh
         node.allocation().ifPresent(allocation -> trustedNodes.addAll(candidates.owner(allocation.owner()).asList()));
-        trustedNodes.addAll(candidates.nodeType(NodeType.config).asList());
         trustedPorts.add(22);
 
         switch (node.type()) {
             case tenant:
                 // Tenant nodes in other states than ready, trust:
+                // - config servers
                 // - 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(candidates.nodeType(NodeType.config).asList());
                 trustedNodes.addAll(candidates.parentsOf(trustedNodes).asList()); // TODO: Remove when we no longer have IPv4-only nodes
                 trustedNodes.addAll(candidates.nodeType(NodeType.proxy).asList());
                 if (node.state() == Node.State.ready) {
@@ -206,24 +205,27 @@ public class NodeRepository extends AbstractComponent {
                 break;
 
             case config:
-                // Config servers trust all nodes
+                // Config servers trust:
+                // - all nodes
+                // - port 4443 from the world
                 trustedNodes.addAll(candidates.asList());
-
-                // And all connections on 4443
                 trustedPorts.add(4443);
                 break;
 
             case proxy:
-                // Accept connections from the world on 443 (for dashboard app), 4080 (insecure tb removed), and 4443
+                // Proxy nodes trust:
+                // - config servers
+                // - all connections from the world on 443 (for dashboard app), 4080 (insecure tb removed), and 4443
+                trustedNodes.addAll(candidates.nodeType(NodeType.config).asList());
                 trustedPorts.add(443);
                 trustedPorts.add(4080);
                 trustedPorts.add(4443);
                 break;
 
-            case host:
-                // This is only needed for macvlan networks - for nated networks this is handled elsewhere.
-                // Docker bridge network
-                trustedNetworks.add("172.17.0.0/16");
+            case controller:
+                // Controllers:
+                // - port 4443 (HTTPS) from the world
+                trustedPorts.add(4443);
                 break;
 
             default:
@@ -232,7 +234,7 @@ public class NodeRepository extends AbstractComponent {
                                 node.hostname(), node.type()));
         }
 
-        return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts);
+        return new NodeAcl(node, trustedNodes, Collections.emptySet(), trustedPorts);
     }
 
     /**
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
index fd05eb86667..70750dd6672 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
@@ -72,4 +72,5 @@ public class OrchestratorMock implements Orchestrator {
     public void suspendAll(HostName parentHostname, List<HostName> hostNames) {
         hostNames.forEach(this::suspend);
     }
+
 }
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 c5f33800283..5d8bde960d8 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
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision.provisioning;
 
+import com.google.common.collect.ImmutableSet;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Capacity;
@@ -26,6 +27,7 @@ import static com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester.c
 import static java.util.Collections.singleton;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author mpolden
@@ -34,8 +36,6 @@ public class AclProvisioningTest {
 
     private ProvisioningTester tester;
 
-    private final List<String> dockerBridgeNetwork = Collections.singletonList("172.17.0.0/16");
-
     @Before
     public void before() {
         this.tester = new ProvisioningTester(Zone.defaultZone(), createConfig());
@@ -126,45 +126,7 @@ public class AclProvisioningTest {
     }
 
     @Test
-    public void trusted_nodes_for_docker_host() {
-        List<Node> configServers = tester.makeConfigServers(3, "default", Version.fromString("6.123.456"));
-
-        // Populate repo
-        tester.makeReadyNodes(2, "default", NodeType.host);
-
-        // Deploy zone application
-        ApplicationId zoneApplication = tester.makeApplicationId();
-        allocateNodes(Capacity.fromRequiredNodeType(NodeType.host), zoneApplication);
-
-        List<Node> dockerHostNodes = tester.nodeRepository().getNodes(zoneApplication);
-        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), dockerBridgeNetwork, acls.get(0));
-    }
-
-
-    @Test
-    public void trusted_nodes_for_docker_hosts_nodes_in_zone_application() {
-        List<Node> configServers = tester.makeConfigServers(3, "default", Version.fromString("6.123.456"));
-        ApplicationId applicationId = tester.makeApplicationId(); // use same id for both allocate calls below
-
-        // Populate repo
-        tester.makeReadyNodes(2, "default", NodeType.host);
-
-        // Allocate 2 Docker hosts
-        List<Node> activeDockerHostNodes = allocateNodes(NodeType.host, applicationId);
-        assertEquals(2, activeDockerHostNodes.size());
-
-        // Check trusted nodes for all nodes
-        activeDockerHostNodes.forEach(node -> {
-            List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node, false);
-            assertAcls(Arrays.asList(activeDockerHostNodes, configServers), dockerBridgeNetwork, nodeAcls);
-        });
-    }
-
-    @Test
-    public void trusted_nodes_for_child_nodes_of_docker_host() {
+    public void trusted_nodes_for_children_of_docker_host() {
         List<Node> configServers = tester.makeConfigServers(3, "default", Version.fromString("6.123.456"));
 
         // Populate repo
@@ -189,6 +151,20 @@ public class AclProvisioningTest {
     }
 
     @Test
+    public void trusted_nodes_for_controllers() {
+        tester.makeReadyNodes(3, "default", NodeType.controller);
+
+        // Allocate
+        ApplicationId controllerApplication = tester.makeApplicationId();
+        List<Node> controllers = allocateNodes(Capacity.fromRequiredNodeType(NodeType.controller), controllerApplication);
+
+        // Controllers and hosts all trust each other
+        List<NodeAcl> controllerAcls = tester.nodeRepository().getNodeAcls(controllers.get(0), false);
+        assertAcls(Collections.singletonList(controllers), controllerAcls);
+        assertEquals(ImmutableSet.of(22, 4443), controllerAcls.get(0).trustedPorts());
+    }
+
+    @Test
     public void resolves_hostnames_from_connection_spec() {
         tester.makeConfigServers(3, "default", Version.fromString("6.123.456"));
 
@@ -206,10 +182,6 @@ public class AclProvisioningTest {
         return allocateNodes(Capacity.fromNodeCount(nodeCount), tester.makeApplicationId());
     }
 
-    private List<Node> allocateNodes(NodeType nodeType, ApplicationId applicationId) {
-        return allocateNodes(Capacity.fromRequiredNodeType(nodeType), applicationId);
-    }
-
     private List<Node> allocateNodes(Capacity capacity, ApplicationId applicationId) {
         ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("test"),
                                                   Version.fromString("6.42"), false);
@@ -219,18 +191,10 @@ public class AclProvisioningTest {
     }
 
     private static void assertAcls(List<List<Node>> expected, NodeAcl actual) {
-        assertAcls(expected, Collections.emptyList(), Collections.singletonList(actual));
-    }
-
-    private static void assertAcls(List<List<Node>> expected, List<NodeAcl> actual) {
-        assertAcls(expected, Collections.emptyList(), actual);
-    }
-
-    private static void assertAcls(List<List<Node>> expected, List<String> expectedNetworks, NodeAcl actual) {
-        assertAcls(expected, expectedNetworks, Collections.singletonList(actual));
+        assertAcls(expected, Collections.singletonList(actual));
     }
 
-    private static void assertAcls(List<List<Node>> expectedNodes, List<String> expectedNetworks, List<NodeAcl> actual) {
+    private static void assertAcls(List<List<Node>> expectedNodes, List<NodeAcl> actual) {
         Set<Node> expectedTrustedNodes = expectedNodes.stream()
                 .flatMap(Collection::stream)
                 .collect(Collectors.toSet());
@@ -239,10 +203,9 @@ public class AclProvisioningTest {
                 .collect(Collectors.toSet());
         assertEquals(expectedTrustedNodes, actualTrustedNodes);
 
-        Set<String> expectedTrustedNetworks = new HashSet<>(expectedNetworks);
         Set<String> actualTrustedNetworks = actual.stream()
                 .flatMap(acl -> acl.trustedNetworks().stream())
                 .collect(Collectors.toSet());
-        assertEquals(expectedTrustedNetworks, actualTrustedNetworks);
+        assertTrue("No networks are trusted", actualTrustedNetworks.isEmpty());
     }
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index ebcc46d5661..2ff1e403e35 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -354,7 +354,7 @@ public class RestApiTest {
 
     @Test
     public void acl_request_by_docker_host() throws Exception {
-        assertFile(new Request("http://localhost:8080/nodes/v2/acl/dockerhost1.yahoo.com"), "acl-docker-host.json");
+        assertFile(new Request("http://localhost:8080/nodes/v2/acl/dockerhost1.yahoo.com?children=true"), "acl-docker-host.json");
     }
 
     @Test
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-docker-host.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-docker-host.json
index 4d6607bd1b0..abf3c39001f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-docker-host.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-docker-host.json
@@ -4,85 +4,62 @@
       "hostname": "cfg1.yahoo.com",
       "type": "config",
       "ipAddress": "127.0.1.1",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
       "hostname": "cfg2.yahoo.com",
       "type": "config",
       "ipAddress": "127.0.1.2",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
       "hostname": "dockerhost1.yahoo.com",
       "type": "host",
       "ipAddress": "::1",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
       "hostname": "dockerhost1.yahoo.com",
       "type": "host",
       "ipAddress": "127.0.0.1",
-      "trustedBy": "dockerhost1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost2.yahoo.com",
-      "type": "host",
-      "ipAddress": "::1",
-      "trustedBy": "dockerhost1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost2.yahoo.com",
-      "type": "host",
-      "ipAddress": "127.0.0.1",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
       "hostname": "dockerhost3.yahoo.com",
       "type": "host",
       "ipAddress": "::1",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
       "hostname": "dockerhost3.yahoo.com",
       "type": "host",
       "ipAddress": "127.0.0.1",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
-      "hostname": "dockerhost4.yahoo.com",
-      "type": "host",
+      "hostname": "host4.yahoo.com",
+      "type": "tenant",
       "ipAddress": "::1",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
-      "hostname": "dockerhost4.yahoo.com",
-      "type": "host",
+      "hostname": "host4.yahoo.com",
+      "type": "tenant",
       "ipAddress": "127.0.0.1",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     },
     {
-      "hostname": "dockerhost5.yahoo.com",
-      "type": "host",
-      "ipAddress": "::1",
-      "trustedBy": "dockerhost1.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost5.yahoo.com",
-      "type": "host",
-      "ipAddress": "127.0.0.1",
-      "trustedBy": "dockerhost1.yahoo.com"
-    }
-  ],
-  "trustedNetworks": [
-    {
-      "network": "172.17.0.0/16",
-      "trustedBy": "dockerhost1.yahoo.com"
+      "hostname": "test-container-1",
+      "type": "tenant",
+      "ipAddress": "::2",
+      "trustedBy": "host4.yahoo.com"
     }
   ],
+  "trustedNetworks": [],
   "trustedPorts": [
     {
       "port": 22,
-      "trustedBy": "dockerhost1.yahoo.com"
+      "trustedBy": "host4.yahoo.com"
     }
   ]
 }