Merge pull request #7133 from vespa-engine/freva/extend-acl

NodeAdmin: Extend Acl

5 files changed, 138 insertions, 54 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java
index 256b05a74ae..16d1fd28441 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java
@@ -1,16 +1,18 @@
 // 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.node.admin.configserver.noderepository;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.net.InetAddresses;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 
 import java.net.InetAddress;
 
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -21,16 +23,16 @@ import java.util.stream.Collectors;
  */
 public class Acl {
 
-    private final List<InetAddress> trustedNodes;
-    private final List<Integer> trustedPorts;
+    private final Set<Node> trustedNodes;
+    private final Set<Integer> trustedPorts;
 
     /**
      * @param trustedPorts Ports that hostname should trust
-     * @param trustedNodes Other hostnames that this hostname should trust
+     * @param trustedNodes Other nodes that this hostname should trust
      */
-    public Acl(List<Integer> trustedPorts, List<InetAddress> trustedNodes) {
-        this.trustedNodes = trustedNodes != null ? ImmutableList.copyOf(trustedNodes) : Collections.emptyList();
-        this.trustedPorts = trustedPorts != null ? ImmutableList.copyOf(trustedPorts) : Collections.emptyList();
+    public Acl(Set<Integer> trustedPorts, Set<Node> trustedNodes) {
+        this.trustedNodes = trustedNodes != null ? Collections.unmodifiableSet(trustedNodes) : Collections.emptySet();
+        this.trustedPorts = trustedPorts != null ? Collections.unmodifiableSet(trustedPorts) : Collections.emptySet();
     }
 
     public List<String> toRules(IPVersion ipVersion) {
@@ -56,9 +58,8 @@ public class Acl {
             rules.add("-A INPUT -p tcp -m multiport --dports " + commaSeparatedPorts + " -j ACCEPT");
 
         // Allow traffic from trusted nodes
-        trustedNodes.stream()
-                .filter(ipVersion::match)
-                .map(ipAddress -> "-A INPUT -s " + InetAddresses.toAddrString(ipAddress) + ipVersion.singleHostCidr() + " -j ACCEPT")
+        getTrustedNodes(ipVersion).stream()
+                .map(node -> "-A INPUT -s " + node.inetAddressString() + ipVersion.singleHostCidr() + " -j ACCEPT")
                 .sorted()
                 .forEach(rules::add);
 
@@ -68,19 +69,22 @@ public class Acl {
         return Collections.unmodifiableList(rules);
     }
 
-    public List<String> getTrustedNodes(IPVersion ipVersion) {
+    public Set<Node> getTrustedNodes() {
+        return trustedNodes;
+    }
+
+    public Set<Node> getTrustedNodes(IPVersion ipVersion) {
         return trustedNodes.stream()
-                .filter(ipVersion::match)
-                .map(InetAddresses::toAddrString)
-                .sorted()
-                .collect(Collectors.toList());
+                .filter(node -> ipVersion.match(node.inetAddress()))
+                .collect(Collectors.toSet());
     }
 
-    public List<String> getTrustedPorts(IPVersion ipVersion) {
-        return trustedPorts.stream()
-                .map(Object::toString)
-                .sorted()
-                .collect(Collectors.toList());
+    public Set<Integer> getTrustedPorts() {
+        return trustedPorts;
+    }
+
+    public Set<Integer> getTrustedPorts(IPVersion ipVersion) {
+        return trustedPorts;
     }
 
     @Override
@@ -93,7 +97,97 @@ public class Acl {
     }
 
     @Override
+    public String toString() {
+        return "Acl{" +
+                "trustedNodes=" + trustedNodes +
+                ", trustedPorts=" + trustedPorts +
+                '}';
+    }
+
+    @Override
     public int hashCode() {
         return Objects.hash(trustedPorts, trustedNodes);
     }
+
+    public static class Node {
+        private final String hostname;
+        private final InetAddress inetAddress;
+
+        public Node(String hostname, String ipAddress) {
+            this(hostname, InetAddresses.forString(ipAddress));
+        }
+
+        public Node(String hostname, InetAddress inetAddress) {
+            this.hostname = hostname;
+            this.inetAddress = inetAddress;
+        }
+
+        public String hostname() {
+            return hostname;
+        }
+
+        public InetAddress inetAddress() {
+            return inetAddress;
+        }
+
+        public String inetAddressString() {
+            return InetAddresses.toAddrString(inetAddress);
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            Node node = (Node) o;
+            return Objects.equals(hostname, node.hostname) &&
+                    Objects.equals(inetAddress, node.inetAddress);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(hostname, inetAddress);
+        }
+
+        @Override
+        public String toString() {
+            return "Node{" +
+                    "hostname='" + hostname + '\'' +
+                    ", inetAddress=" + inetAddress +
+                    '}';
+        }
+    }
+
+    public static class Builder {
+        private final Set<Node> trustedNodes = new HashSet<>();
+        private final Set<Integer> trustedPorts = new HashSet<>();
+
+        public Builder() { }
+
+        public Builder(Acl acl) {
+            trustedNodes.addAll(acl.trustedNodes);
+            trustedPorts.addAll(acl.trustedPorts);
+        }
+
+        public Builder withTrustedNode(String hostname, String ipAddress) {
+            return withTrustedNode(new Node(hostname, ipAddress));
+        }
+
+        public Builder withTrustedNode(String hostname, InetAddress inetAddress) {
+            return withTrustedNode(new Node(hostname, inetAddress));
+        }
+
+        public Builder withTrustedNode(Node node) {
+            trustedNodes.add(node);
+            return this;
+        }
+
+        public Builder withTrustedPorts(Integer... ports) {
+            trustedPorts.addAll(Arrays.asList(ports));
+            return this;
+        }
+
+        public Acl build() {
+            return new Acl(trustedPorts, trustedNodes);
+        }
+    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
index 7cee1730804..dc3cc6a45de 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.hosted.node.admin.configserver.noderepository;
 
 import com.google.common.base.Strings;
-import com.google.common.net.InetAddresses;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi;
@@ -11,7 +10,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.*;
 import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 import com.yahoo.vespa.hosted.provision.Node;
 
-import java.net.InetAddress;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -81,16 +79,18 @@ public class RealNodeRepository implements NodeRepository {
             final GetAclResponse response = configServerApi.get(path, GetAclResponse.class);
 
             // Group ports by container hostname that trusts them
-            Map<String, List<Integer>> trustedPorts = response.trustedPorts.stream()
+            Map<String, Set<Integer>> trustedPorts = response.trustedPorts.stream()
                     .collect(Collectors.groupingBy(
                             GetAclResponse.Port::getTrustedBy,
-                            Collectors.mapping(port -> port.port, Collectors.toList())));
+                            Collectors.mapping(port -> port.port, Collectors.toSet())));
 
             // Group node ip-addresses by container hostname that trusts them
-            Map<String, List<InetAddress>> trustedNodes = response.trustedNodes.stream()
+            Map<String, Set<Acl.Node>> trustedNodes = response.trustedNodes.stream()
                     .collect(Collectors.groupingBy(
                             GetAclResponse.Node::getTrustedBy,
-                            Collectors.mapping(node -> InetAddresses.forString(node.ipAddress), Collectors.toList())));
+                            Collectors.mapping(
+                                    node -> new Acl.Node(node.hostname, node.ipAddress),
+                                    Collectors.toSet())));
 
 
             // For each hostname create an ACL
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java
index 77bc49ca596..c7aa79cca40 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java
@@ -1,24 +1,23 @@
 package com.yahoo.vespa.hosted.node.admin.configserver.noderepository;
 
-import com.google.common.net.InetAddresses;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 import org.junit.Assert;
 import org.junit.Test;
 
-import java.net.InetAddress;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 public class AclTest {
 
     private final Acl aclCommon = new Acl(
-            createPortList(1234, 453),
+            createPortSet(1234, 453),
             createTrustedNodes("192.1.2.2", "fb00::1", "fe80::2", "fe80::3"));
 
     private final Acl aclNoPorts = new Acl(
-            Collections.emptyList(),
+            Collections.emptySet(),
             createTrustedNodes("192.1.2.2", "fb00::1", "fe80::2"));
 
     @Test
@@ -72,7 +71,7 @@ public class AclTest {
     @Test
     public void ipv6_rules_stable() {
         Acl aclCommonDifferentOrder = new Acl(
-                createPortList(453, 1234),
+                createPortSet(453, 1234),
                 createTrustedNodes("fe80::2", "192.1.2.2", "fb00::1", "fe80::3"));
 
         for (IPVersion ipVersion: IPVersion.values()) {
@@ -80,13 +79,13 @@ public class AclTest {
         }
     }
 
-    private List<Integer> createPortList(Integer... ports) {
-        return Arrays.asList(ports);
+    private Set<Integer> createPortSet(Integer... ports) {
+        return Stream.of(ports).collect(Collectors.toSet());
     }
 
-    private List<InetAddress> createTrustedNodes(String... addresses) {
+    private Set<Acl.Node> createTrustedNodes(String... addresses) {
         return Arrays.stream(addresses)
-                .map(InetAddresses::forString)
-                .collect(Collectors.toList());
+                .map(ipAddress -> new Acl.Node("hostname", ipAddress))
+                .collect(Collectors.toSet());
     }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
index 9d5555e1f92..bc9e5d380a3 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.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.node.admin.maintenance.acl;
 
-import com.google.common.net.InetAddresses;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
@@ -16,13 +15,11 @@ import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
@@ -249,19 +246,17 @@ public class AclMaintainerTest {
     }
 
     private Map<String, Acl> makeAcl(String containerHostname, String portsCommaSeparated, String... addresses) {
-        Map<String, Acl> map = new HashMap<>();
+        Acl.Builder aclBuilder = new Acl.Builder();
 
-        List<Integer> ports = Arrays.stream(portsCommaSeparated.split(","))
+        Arrays.stream(portsCommaSeparated.split(","))
                 .map(Integer::valueOf)
-                .collect(Collectors.toList());
-
-        List<InetAddress> hosts = Arrays.stream(addresses)
-                .map(InetAddresses::forString)
-                .collect(Collectors.toList());
+                .forEach(aclBuilder::withTrustedPorts);
 
-        Acl acl = new Acl(ports, hosts);
-        map.put(containerHostname, acl);
+        Arrays.stream(addresses)
+                .forEach(address -> aclBuilder.withTrustedNode("hostname", address));
 
+        Map<String, Acl> map = new HashMap<>();
+        map.put(containerHostname, aclBuilder.build());
         return map;
     }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditorTest.java
index c32d02a0bda..2cd942c1372 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditorTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditorTest.java
@@ -1,6 +1,5 @@
 package com.yahoo.vespa.hosted.node.admin.maintenance.acl;
 
-import com.google.common.net.InetAddresses;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
@@ -9,8 +8,6 @@ import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 import org.junit.Assert;
 import org.junit.Test;
 
-import java.util.Collections;
-
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -24,8 +21,7 @@ public class IPTablesEditorTest {
 
     @Test
     public void filter_set_wanted_rules() {
-
-        Acl acl = new Acl(Collections.singletonList(22), Collections.singletonList(InetAddresses.forString("3001::1")));
+        Acl acl = new Acl.Builder().withTrustedPorts(22).withTrustedNode("hostname", "3001::1").build();
         FilterTableLineEditor filterLineEditor = FilterTableLineEditor.from(acl, IPVersion.IPv6);
 
         String currentFilterTable = "-P INPUT ACCEPT\n" +