Use LinkedList to create rules

3 files changed, 31 insertions, 26 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 ef49822e825..03c4466a3b1 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
@@ -8,6 +8,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 import java.net.InetAddress;
 
 import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
 import java.util.stream.Collectors;
@@ -32,34 +33,38 @@ public class Acl {
         this.trustedPorts = trustedPorts != null ? ImmutableList.copyOf(trustedPorts) : Collections.emptyList();
     }
 
-    public String toRules(IPVersion ipVersion) {
+    public List<String> toRules(IPVersion ipVersion) {
+        List<String> rules = new LinkedList<>();
 
-        String basics = String.join("\n"
-                // We reject with rules instead of using policies
-                , "-P INPUT ACCEPT"
-                , "-P FORWARD ACCEPT"
-                , "-P OUTPUT ACCEPT"
-                // Allow packets belonging to established connections
-                , "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT"
-                // Allow any loopback traffic
-                , "-A INPUT -i lo -j ACCEPT"
-                // Allow ICMP packets. See http://shouldiblockicmp.com/
-                , "-A INPUT -p " + ipVersion.icmpProtocol() + " -j ACCEPT");
+        // We reject with rules instead of using policies
+        rules.add("-P INPUT ACCEPT");
+        rules.add("-P FORWARD ACCEPT");
+        rules.add("-P OUTPUT ACCEPT");
+
+        // Allow packets belonging to established connections
+        rules.add( "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT");
+
+        // Allow any loopback traffic
+        rules.add("-A INPUT -i lo -j ACCEPT");
+
+        // Allow ICMP packets. See http://shouldiblockicmp.com/
+        rules.add("-A INPUT -p " + ipVersion.icmpProtocol() + " -j ACCEPT");
 
         // Allow trusted ports if any
         String commaSeparatedPorts = trustedPorts.stream().map(i -> Integer.toString(i)).collect(Collectors.joining(","));
-        String ports = commaSeparatedPorts.isEmpty() ? "" : "-A INPUT -p tcp -m multiport --dports " + commaSeparatedPorts + " -j ACCEPT\n";
+        if (!commaSeparatedPorts.isEmpty())
+            rules.add("-A INPUT -p tcp -m multiport --dports " + commaSeparatedPorts + " -j ACCEPT");
 
         // Allow traffic from trusted nodes
-        String nodes = trustedNodes.stream()
+        trustedNodes.stream()
                 .filter(ipVersion::match)
                 .map(ipAddress -> "-A INPUT -s " + InetAddresses.toAddrString(ipAddress) + ipVersion.singleHostCidr() + " -j ACCEPT")
-                .collect(Collectors.joining("\n"));
+                .forEach(rules::add);
 
         // We reject instead of dropping to give us an easier time to figure out potential network issues
-        String rejectEverythingElse = "-A INPUT -j REJECT --reject-with " + ipVersion.icmpPortUnreachable();
+        rules.add("-A INPUT -j REJECT --reject-with " + ipVersion.icmpPortUnreachable());
 
-        return basics + "\n" + ports + nodes + "\n" + rejectEverythingElse;
+        return Collections.unmodifiableList(rules);
     }
 
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java
index 4e5906d3c34..163af77a0fe 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java
@@ -5,7 +5,6 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.LineEdit;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.LineEditor;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -16,12 +15,12 @@ class FilterTableLineEditor implements LineEditor {
 
     private final LinkedList<String> wantedRules;
 
-    FilterTableLineEditor(List<String> wantedRules) {
+    private FilterTableLineEditor(List<String> wantedRules) {
         this.wantedRules = new LinkedList<>(wantedRules);
     }
 
     static FilterTableLineEditor from(Acl acl, IPVersion ipVersion) {
-        List<String> rules = Arrays.asList(acl.toRules(ipVersion).split("\n"));
+        List<String> rules = acl.toRules(ipVersion);
         return new FilterTableLineEditor(rules);
     }
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java
index 181928fa438..8bbbd076b49 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java
@@ -1,7 +1,6 @@
-package com.yahoo.vespa.hosted.node.admin.maintenance.acl;
+package com.yahoo.vespa.hosted.node.admin.configserver.noderepository;
 
 import com.google.common.net.InetAddresses;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 import org.junit.Assert;
 import org.junit.Test;
@@ -16,7 +15,7 @@ public class AclTest {
 
     private final Acl aclCommon = new Acl(
             createPortList(1234, 453),
-            createTrustedNodes("192.1.2.2", "fb00::1", "fe80::2"));
+            createTrustedNodes("192.1.2.2", "fb00::1", "fe80::2", "fe80::3"));
 
     private final Acl aclNoPorts = new Acl(
             Collections.emptyList(),
@@ -24,7 +23,7 @@ public class AclTest {
 
     @Test
     public void no_trusted_ports() {
-        String listRulesIpv4 = aclNoPorts.toRules(IPVersion.IPv4);
+        String listRulesIpv4 = String.join("\n", aclNoPorts.toRules(IPVersion.IPv4));
         Assert.assertEquals(
                 "-P INPUT ACCEPT\n" +
                         "-P FORWARD ACCEPT\n" +
@@ -39,7 +38,7 @@ public class AclTest {
 
     @Test
     public void ipv4_list_rules() {
-        String listRulesIpv4 = aclCommon.toRules(IPVersion.IPv4);
+        String listRulesIpv4 = String.join("\n", aclCommon.toRules(IPVersion.IPv4));
         Assert.assertEquals(
                 "-P INPUT ACCEPT\n" +
                         "-P FORWARD ACCEPT\n" +
@@ -55,7 +54,7 @@ public class AclTest {
 
     @Test
     public void ipv6_list_rules() {
-        String listRulesIpv6 = aclCommon.toRules(IPVersion.IPv6);
+        String listRulesIpv6 = String.join("\n", aclCommon.toRules(IPVersion.IPv6));
         Assert.assertEquals(
                 "-P INPUT ACCEPT\n" +
                         "-P FORWARD ACCEPT\n" +
@@ -66,9 +65,11 @@ public class AclTest {
                         "-A INPUT -p tcp -m multiport --dports 1234,453 -j ACCEPT\n" +
                         "-A INPUT -s fb00::1/128 -j ACCEPT\n" +
                         "-A INPUT -s fe80::2/128 -j ACCEPT\n" +
+                        "-A INPUT -s fe80::3/128 -j ACCEPT\n" +
                         "-A INPUT -j REJECT --reject-with icmp6-port-unreachable", listRulesIpv6);
     }
 
+
     private List<Integer> createPortList(Integer... ports) {
         return Arrays.asList(ports);
     }