Merge pull request #10522 from vespa-engine/revert-10521-revert-10512-freva/simplify-ip

Remove SSH and fix TODO in node-repo ACL rules take 2

6 files changed, 18 insertions, 117 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 fd7397168d5..1fbb83c7718 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
@@ -207,9 +207,10 @@ public class NodeRepository extends AbstractComponent {
         Set<String> trustedNetworks = new LinkedHashSet<>();
 
         // For all cases below, trust:
+        // - parent host (for health checks and metrics)
         // - nodes in same application
         // - load balancers allocated to application
-        // - ssh
+        candidates.parentOf(node).ifPresent(trustedNodes::add);
         node.allocation().ifPresent(allocation -> {
             trustedNodes.addAll(candidates.owner(allocation.owner()).asList());
             loadBalancers.owner(allocation.owner()).asList().stream()
@@ -217,17 +218,13 @@ public class NodeRepository extends AbstractComponent {
                          .map(LoadBalancerInstance::networks)
                          .forEach(trustedNetworks::addAll);
         });
-        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) {
                     // Tenant nodes in state ready, trust:
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 6da72f6ebdd..24b12c4427f 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,7 +1,6 @@
 // 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;
@@ -13,19 +12,14 @@ import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.node.NodeAcl;
 import org.junit.Test;
 
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
-import static java.util.Collections.emptySet;
-import static java.util.Collections.singleton;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 
@@ -57,11 +51,12 @@ public class AclProvisioningTest {
 
         // Get trusted nodes for the first active node
         Node node = activeNodes.get(0);
+        Node host = node.parentHostname().flatMap(tester.nodeRepository()::getNode).get();
         Supplier<List<NodeAcl>> nodeAcls = () -> tester.nodeRepository().getNodeAcls(node, false);
 
         // Trusted nodes are active nodes in same application, proxy nodes and config servers
-        assertAcls(Arrays.asList(activeNodes, proxyNodes, configServers, dockerHost),
-                   ImmutableSet.of("10.2.3.0/24", "10.4.5.0/24"),
+        assertAcls(List.of(activeNodes, proxyNodes, configServers, List.of(host)),
+                   Set.of("10.2.3.0/24", "10.4.5.0/24"),
                    nodeAcls.get());
     }
 
@@ -82,7 +77,7 @@ public class AclProvisioningTest {
         List<Node> tenantNodes = tester.nodeRepository().getNodes(NodeType.tenant);
 
         // Trusted nodes are all proxy-, config-, and, tenant-nodes
-        assertAcls(Arrays.asList(proxyNodes, configServers, tenantNodes), nodeAcls);
+        assertAcls(List.of(proxyNodes, configServers, tenantNodes), nodeAcls);
     }
 
     @Test
@@ -103,7 +98,7 @@ public class AclProvisioningTest {
         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);
+        assertAcls(List.of(tenantNodes, proxyNodes, configServers), nodeAcls);
     }
 
     @Test
@@ -124,7 +119,7 @@ public class AclProvisioningTest {
         List<NodeAcl> nodeAcls = tester.nodeRepository().getNodeAcls(node, false);
 
         // Trusted nodes is all config servers and all proxy nodes
-        assertAcls(Arrays.asList(proxyNodes, configServers), nodeAcls);
+        assertAcls(List.of(proxyNodes, configServers), nodeAcls);
     }
 
     @Test
@@ -148,7 +143,7 @@ public class AclProvisioningTest {
                     .findFirst()
                     .orElseThrow(() -> new RuntimeException("Expected to find ACL for node " + dockerNode.hostname()));
             assertEquals(dockerHostNodeUnderTest.hostname(), dockerNode.parentHostname().get());
-            assertAcls(Arrays.asList(configServers, dockerNodes), nodeAcl);
+            assertAcls(List.of(configServers, dockerNodes, List.of(dockerHostNodeUnderTest)), nodeAcl);
         }
     }
 
@@ -162,8 +157,8 @@ public class AclProvisioningTest {
 
         // 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, 443), controllerAcls.get(0).trustedPorts());
+        assertAcls(List.of(controllers), controllerAcls);
+        assertEquals(Set.of(4443, 443), controllerAcls.get(0).trustedPorts());
     }
 
     @Test
@@ -185,9 +180,9 @@ public class AclProvisioningTest {
 
         assertEquals(3, nodeAcls.get(0).trustedNodes().size());
         Iterator<Node> trustedNodes = nodeAcls.get(0).trustedNodes().iterator();
-        assertEquals(singleton("127.0.1.1"), trustedNodes.next().ipAddresses());
-        assertEquals(singleton("127.0.1.2"), trustedNodes.next().ipAddresses());
-        assertEquals(singleton("127.0.1.3"), trustedNodes.next().ipAddresses());
+        assertEquals(Set.of("127.0.1.1"), trustedNodes.next().ipAddresses());
+        assertEquals(Set.of("127.0.1.2"), trustedNodes.next().ipAddresses());
+        assertEquals(Set.of("127.0.1.3"), trustedNodes.next().ipAddresses());
     }
 
     private List<Node> deploy(int nodeCount) {
@@ -202,7 +197,7 @@ public class AclProvisioningTest {
         ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("test"),
                                                   Version.fromString("6.42"), false);
         List<HostSpec> prepared = tester.prepare(application, cluster, capacity, 1);
-        tester.activate(application, new HashSet<>(prepared));
+        tester.activate(application, Set.copyOf(prepared));
         return tester.getNodes(application, Node.State.active).asList();
     }
 
@@ -211,12 +206,12 @@ public class AclProvisioningTest {
     }
 
     private static void assertAcls(List<List<Node>> expectedNodes, List<NodeAcl> actual) {
-        assertAcls(expectedNodes, emptySet(), actual);
+        assertAcls(expectedNodes, Set.of(), actual);
     }
 
     private static void assertAcls(List<List<Node>> expectedNodes, Set<String> expectedNetworks, List<NodeAcl> actual) {
         List<Node> expectedTrustedNodes = expectedNodes.stream()
-                .flatMap(Collection::stream)
+                .flatMap(List::stream)
                 .distinct()
                 .sorted(Comparator.comparing(Node::hostname))
                 .collect(Collectors.toList());
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 ac2276a95b0..44e7d7659ab 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
@@ -381,11 +381,6 @@ 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?children=true"), "acl-docker-host.json");
-    }
-
-    @Test
     public void test_invalid_requests() throws Exception {
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/node-does-not-exist",
                                    new byte[0], Request.Method.GET),
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-config-server.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-config-server.json
index e489aec2df6..88e8fb44f9f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-config-server.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-config-server.json
@@ -226,10 +226,6 @@
   "trustedNetworks": [],
   "trustedPorts": [
     {
-      "port": 22,
-      "trustedBy": "cfg1.yahoo.com"
-    },
-    {
       "port": 4443,
       "trustedBy": "cfg1.yahoo.com"
     }
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
deleted file mode 100644
index 2acc6d42282..00000000000
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-docker-host.json
+++ /dev/null
@@ -1,77 +0,0 @@
-{
-  "trustedNodes": [
-    {
-      "hostname": "cfg1.yahoo.com",
-      "type": "config",
-      "ipAddress": "127.0.201.1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "cfg1.yahoo.com",
-      "type": "config",
-      "ipAddress": "::201:1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "cfg2.yahoo.com",
-      "type": "config",
-      "ipAddress": "127.0.202.1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "cfg2.yahoo.com",
-      "type": "config",
-      "ipAddress": "::202:1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost1.yahoo.com",
-      "type": "host",
-      "ipAddress": "127.0.100.1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost1.yahoo.com",
-      "type": "host",
-      "ipAddress": "::100:1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost2.yahoo.com",
-      "type": "host",
-      "ipAddress": "127.0.101.1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "dockerhost2.yahoo.com",
-      "type": "host",
-      "ipAddress": "::101:1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "host4.yahoo.com",
-      "type": "tenant",
-      "ipAddress": "127.0.4.1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "host4.yahoo.com",
-      "type": "tenant",
-      "ipAddress": "::4:1",
-      "trustedBy": "host4.yahoo.com"
-    },
-    {
-      "hostname": "test-node-pool-101-2",
-      "type": "tenant",
-      "ipAddress": "::101:2",
-      "trustedBy": "host4.yahoo.com"
-    }
-  ],
-  "trustedNetworks": [],
-  "trustedPorts": [
-    {
-      "port": 22,
-      "trustedBy": "host4.yahoo.com"
-    }
-  ]
-}
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-tenant-node.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-tenant-node.json
index 5fe28550837..040b2ebe167 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-tenant-node.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/acl-tenant-node.json
@@ -170,10 +170,5 @@
     }
   ],
   "trustedNetworks": [],
-  "trustedPorts": [
-    {
-      "port": 22,
-      "trustedBy": "foo.yahoo.com"
-    }
-  ]
+  "trustedPorts": []
 }