Rewrite AclMaintainer to converge(NodeAgentContext)

11 files changed, 435 insertions, 460 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
index 27dc4e5237b..cedddc9b0b6 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.hosted.node.admin.docker;
 
 import com.yahoo.vespa.hosted.dockerapi.Container;
-import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
 import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
@@ -31,7 +30,7 @@ public interface DockerOperations {
 
     ProcessResult executeCommandInContainerAsRoot(NodeAgentContext context, Long timeoutSeconds, String... command);
 
-    ProcessResult executeCommandInNetworkNamespace(ContainerName containerName, String... command);
+    ProcessResult executeCommandInNetworkNamespace(NodeAgentContext context, String... command);
 
 
     /** Resume node. Resuming a node means that it is ready to take on traffic. */
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index bdda155b2f3..d65bca89733 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -6,7 +6,6 @@ import com.yahoo.collections.Pair;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.system.ProcessExecuter;
 import com.yahoo.vespa.hosted.dockerapi.Container;
-import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
 import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
@@ -196,12 +195,12 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
-    public ProcessResult executeCommandInNetworkNamespace(ContainerName containerName, String... command) {
-        final Integer containerPid = docker.getContainer(containerName)
+    public ProcessResult executeCommandInNetworkNamespace(NodeAgentContext context, String... command) {
+        final int containerPid = docker.getContainer(context.containerName())
                 .filter(container -> container.state.isRunning())
-                .map(container -> container.pid)
-                .orElseThrow(() -> new RuntimeException("PID not found for container with name: " +
-                        containerName.asString()));
+                .orElseThrow(() -> new RuntimeException(
+                        "Found no running container named " + context.containerName().asString()))
+                .pid;
 
         final String[] wrappedCommand = Stream.concat(
                 Stream.of("nsenter", String.format("--net=/proc/%d/ns/net", containerPid), "--"),
@@ -213,12 +212,12 @@ public class DockerOperationsImpl implements DockerOperations {
             if (result.getFirst() != 0) {
                 throw new RuntimeException(String.format(
                         "Failed to execute %s in network namespace for %s (PID = %d), exit code: %d, output: %s",
-                        Arrays.toString(wrappedCommand), containerName.asString(), containerPid, result.getFirst(), result.getSecond()));
+                        Arrays.toString(wrappedCommand), context.containerName().asString(), containerPid, result.getFirst(), result.getSecond()));
             }
             return new ProcessResult(0, result.getSecond(), "");
         } catch (IOException e) {
             throw new RuntimeException(String.format("IOException while executing %s in network namespace for %s (PID = %d)",
-                    Arrays.toString(wrappedCommand), containerName.asString(), containerPid), e);
+                    Arrays.toString(wrappedCommand), context.containerName().asString(), containerPid), e);
         }
     }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
index a733a52ea10..a68eda7e39e 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
@@ -2,19 +2,29 @@
 package com.yahoo.vespa.hosted.node.admin.maintenance.acl;
 
 import com.google.common.net.InetAddresses;
-import com.yahoo.config.provision.HostName;
-import com.yahoo.vespa.hosted.dockerapi.Container;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
+import com.yahoo.log.LogLevel;
+import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
+import com.yahoo.vespa.hosted.node.admin.task.util.file.Editor;
+import com.yahoo.vespa.hosted.node.admin.task.util.file.LineEditor;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddresses;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
-import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 
+import java.io.IOException;
 import java.net.InetAddress;
-import java.util.Map;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
+import static com.yahoo.yolean.Exceptions.uncheck;
+
 /**
  * This class maintains the iptables (ipv4 and ipv6) for all running containers.
  * The filter table is synced with ACLs fetched from the Node repository while the nat table
@@ -31,56 +41,100 @@ import java.util.stream.Collectors;
  * @author smorgrav
  */
 public class AclMaintainer {
-
-    private static final PrefixLogger log = PrefixLogger.getNodeAdminLogger(AclMaintainer.class);
+    private static final Logger logger = Logger.getLogger(AclMaintainer.class.getName());
 
     private final DockerOperations dockerOperations;
-    private final NodeRepository nodeRepository;
     private final IPAddresses ipAddresses;
-    private final String hostHostname;
 
-    public AclMaintainer(DockerOperations dockerOperations, NodeRepository nodeRepository,
-                         HostName hostHostname, IPAddresses ipAddresses) {
+    public AclMaintainer(DockerOperations dockerOperations, IPAddresses ipAddresses) {
         this.dockerOperations = dockerOperations;
-        this.nodeRepository = nodeRepository;
         this.ipAddresses = ipAddresses;
-        this.hostHostname = hostHostname.value();
     }
 
-    private void applyRedirect(Container container, InetAddress address) {
+    public void converge(NodeAgentContext context) {
+        // Apply acl to the filter table
+        editFlushOnError(context, IPVersion.IPv4, "filter", FilterTableLineEditor.from(context.acl(), IPVersion.IPv4));
+        editFlushOnError(context, IPVersion.IPv6, "filter", FilterTableLineEditor.from(context.acl(), IPVersion.IPv6));
+
+        ipAddresses.getAddress(context.hostname().value(), IPVersion.IPv4).ifPresent(addr -> applyRedirect(context, addr));
+        ipAddresses.getAddress(context.hostname().value(), IPVersion.IPv6).ifPresent(addr -> applyRedirect(context, addr));
+    }
+
+    private void applyRedirect(NodeAgentContext context, InetAddress address) {
         IPVersion ipVersion = IPVersion.get(address);
         // Necessary to avoid the routing packets destined for the node's own public IP address
         // via the bridge, which is illegal.
         String redirectRule = "-A OUTPUT -d " + InetAddresses.toAddrString(address) + ipVersion.singleHostCidr() + " -j REDIRECT";
-        IPTablesEditor.editLogOnError(dockerOperations, container.name, ipVersion, "nat", NatTableLineEditor.from(redirectRule));
+        editLogOnError(context, ipVersion, "nat", NatTableLineEditor.from(redirectRule));
     }
 
-    private void apply(Container container, Acl acl) {
-        // Apply acl to the filter table
-        IPTablesEditor.editFlushOnError(dockerOperations, container.name, IPVersion.IPv6, "filter", FilterTableLineEditor.from(acl, IPVersion.IPv6));
-        IPTablesEditor.editFlushOnError(dockerOperations, container.name, IPVersion.IPv4, "filter", FilterTableLineEditor.from(acl, IPVersion.IPv4));
+    private boolean editFlushOnError(NodeAgentContext context, IPVersion ipVersion, String table, LineEditor lineEditor) {
+        return edit(context, table, ipVersion, lineEditor, true);
+    }
+
+    private boolean editLogOnError(NodeAgentContext context, IPVersion ipVersion, String table, LineEditor lineEditor) {
+        return edit(context, table, ipVersion, lineEditor, false);
+    }
 
-        ipAddresses.getAddress(container.hostname, IPVersion.IPv4).ifPresent(addr -> applyRedirect(container, addr));
-        ipAddresses.getAddress(container.hostname, IPVersion.IPv6).ifPresent(addr -> applyRedirect(container, addr));
+    private boolean edit(NodeAgentContext context, String table, IPVersion ipVersion, LineEditor lineEditor, boolean flush) {
+        Editor editor = new Editor(
+                ipVersion.iptablesCmd() + "-" + table,
+                listTable(context, table, ipVersion),
+                restoreTable(context, table, ipVersion, flush),
+                lineEditor);
+        return editor.edit(message -> context.log(logger, message));
     }
 
-    private synchronized void configureAcls() {
-        log.info("Configuring ACLs"); // Needed to potentially nail down when ACL maintainer stopped working
-        Map<String, Container> runningContainers = dockerOperations
-                .getAllManagedContainers().stream()
-                .filter(container -> container.state.isRunning())
-                .collect(Collectors.toMap(container -> container.hostname, container -> container));
+    private Supplier<List<String>> listTable(NodeAgentContext context, String table, IPVersion ipVersion) {
+        return () -> {
+            ProcessResult currentRulesResult =
+                    dockerOperations.executeCommandInNetworkNamespace(context, ipVersion.iptablesCmd(), "-S", "-t", table);
+            return Arrays.stream(currentRulesResult.getOutput().split("\n"))
+                    .map(String::trim)
+                    .collect(Collectors.toList());
+        };
+    }
 
-        nodeRepository.getAcls(hostHostname).entrySet().stream()
-                .filter(entry -> runningContainers.containsKey(entry.getKey()))
-                .forEach(entry -> apply(runningContainers.get(entry.getKey()), entry.getValue()));
+    private Consumer<List<String>> restoreTable(NodeAgentContext context, String table, IPVersion ipVersion, boolean flush) {
+        return list -> {
+            try (TemporaryIpTablesFileHandler fileHandler = new TemporaryIpTablesFileHandler(table)) {
+                String rules = String.join("\n", list);
+                String fileContent = "*" + table + "\n" + rules + "\nCOMMIT\n";
+                fileHandler.writeUtf8Content(fileContent);
+                dockerOperations.executeCommandInNetworkNamespace(context, ipVersion.iptablesRestore(), fileHandler.absolutePath());
+            } catch (Exception e) {
+                if (flush) {
+                    context.log(logger, LogLevel.ERROR, "Exception occurred while syncing iptable " + table + ", attempting rollback", e);
+                    try {
+                        dockerOperations.executeCommandInNetworkNamespace(context, ipVersion.iptablesCmd(), "-F", "-t", table);
+                    } catch (Exception ne) {
+                        context.log(logger, LogLevel.ERROR, "Rollback of table " + table + " failed, giving up", ne);
+                    }
+                } else {
+                    context.log(logger, LogLevel.WARNING, "Unable to sync iptables for " + table, e);
+                }
+            }
+        };
     }
 
-    public void converge() {
-        try {
-            configureAcls();
-        } catch (Throwable t) {
-            log.error("Failed to configure ACLs", t);
+    private static class TemporaryIpTablesFileHandler implements AutoCloseable {
+        private final Path path;
+
+        private TemporaryIpTablesFileHandler(String table) {
+            this.path = uncheck(() -> Files.createTempFile("iptables-restore", "." + table));
+        }
+
+        private void writeUtf8Content(String content) throws IOException {
+            Files.write(path, content.getBytes(StandardCharsets.UTF_8));
+        }
+
+        private String absolutePath() {
+            return path.toAbsolutePath().toString();
+        }
+
+        @Override
+        public void close() throws IOException {
+            Files.deleteIfExists(path);
         }
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditor.java
deleted file mode 100644
index d34b41e3ae5..00000000000
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditor.java
+++ /dev/null
@@ -1,107 +0,0 @@
-// Copyright 2018 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.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
-import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
-import com.yahoo.vespa.hosted.node.admin.task.util.file.Editor;
-import com.yahoo.vespa.hosted.node.admin.task.util.file.LineEditor;
-import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
-import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
-
-import java.io.File;
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.List;
-import java.util.function.Consumer;
-import java.util.function.Supplier;
-import java.util.stream.Collectors;
-
-/**
- * Edit the iptables for docker containers.
- */
-class IPTablesEditor {
-
-    private final PrefixLogger log;
-    private final DockerOperations dockerOperations;
-    private final ContainerName containerName;
-    private final Consumer<String> testInterceptor;
-
-    public IPTablesEditor(DockerOperations dockerOperations, ContainerName containerName) {
-        this(dockerOperations, containerName, (result) -> {});
-    }
-
-    IPTablesEditor(DockerOperations dockerOperations, ContainerName containerName, Consumer<String> testInterceptor) {
-        this.dockerOperations = dockerOperations;
-        this.containerName = containerName;
-        this.testInterceptor = testInterceptor;
-        this.log = PrefixLogger.getNodeAgentLogger(AclMaintainer.class, containerName);
-    }
-
-    public static boolean editFlushOnError(DockerOperations dockerOperations, ContainerName containerName, IPVersion ipVersion, String table, LineEditor lineEditor) {
-        return new IPTablesEditor(dockerOperations, containerName).edit(table, ipVersion, lineEditor, true);
-    }
-
-    public static boolean editLogOnError(DockerOperations dockerOperations, ContainerName containerName, IPVersion ipVersion, String table, LineEditor lineEditor) {
-        return new IPTablesEditor(dockerOperations, containerName).edit(table, ipVersion, lineEditor, false);
-    }
-
-    public boolean edit(String table, IPVersion ipVersion, LineEditor lineEditor, boolean flush) {
-        String editId = ipVersion.iptablesCmd() + "-" + table;
-        Editor editor = new Editor(editId, listTable(table, ipVersion), restoreTable(table, ipVersion, flush), lineEditor);
-        return editor.edit(log::info);
-    }
-
-    private Supplier<List<String>> listTable(String table, IPVersion ipVersion) {
-        return () -> {
-            ProcessResult currentRulesResult =
-                    dockerOperations.executeCommandInNetworkNamespace(containerName, ipVersion.iptablesCmd(), "-S", "-t", table);
-            return Arrays.stream(currentRulesResult.getOutput().split("\n"))
-                    .map(String::trim)
-                    .collect(Collectors.toList());
-        };
-    }
-
-    private Consumer<List<String>> restoreTable(String table, IPVersion ipVersion, boolean flush) {
-        return list -> {
-            File file = null;
-            try {
-                String rules = String.join("\n", list);
-                String fileContent = "*" + table + "\n" + rules + "\nCOMMIT\n";
-                file = writeTempFile(table, fileContent);
-                dockerOperations.executeCommandInNetworkNamespace(containerName, ipVersion.iptablesRestore(), file.getAbsolutePath());
-                testInterceptor.accept(fileContent);
-            } catch (Exception e) {
-                if (flush) {
-                    log.error("Exception occurred while syncing iptable " + table + " for " + containerName.asString() + ", attempting rollback", e);
-                    try {
-                        dockerOperations.executeCommandInNetworkNamespace(containerName, ipVersion.iptablesCmd(), "-F", "-t", table);
-                    } catch (Exception ne) {
-                        log.error("Rollback of table " + table + " for " + containerName.asString() + " failed, giving up", ne);
-                    }
-                } else {
-                    log.warning("Unable to sync iptables for " + table, e);
-                }
-            } finally {
-                if (file != null) {
-                    file.delete();
-                }
-            }
-        };
-    }
-
-    private File writeTempFile(String table, String content) {
-        try {
-            Path path = Files.createTempFile("iptables-restore", "." + table);
-            File file = path.toFile();
-            Files.write(path, content.getBytes(StandardCharsets.UTF_8));
-            file.deleteOnExit();
-            return file;
-        } catch (IOException e) {
-            throw new RuntimeException("Unable to write restore file for iptables.", e);
-        }
-    }
-}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index e5e045c6013..d31a888ea44 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -468,11 +468,11 @@ public class NodeAgentImpl implements NodeAgent {
                     containerState = STARTING;
                     startContainer(context);
                     containerState = UNKNOWN;
-                    aclMaintainer.ifPresent(AclMaintainer::converge);
                 } else {
                     updateContainerIfNeeded(context, container.get());
                 }
 
+                aclMaintainer.ifPresent(maintainer -> maintainer.converge(context));
                 startServicesIfNeeded(context);
                 resumeNodeIfNeeded(context);
                 healthChecker.ifPresent(checker -> checker.verifyHealth(context));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
index 99b3fde4eb1..db1d45b9449 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
@@ -75,12 +75,13 @@ public class DockerOperationsImplTest {
 
     @Test
     public void runsCommandInNetworkNamespace() throws IOException {
-        Container container = makeContainer("container-42", Container.State.RUNNING, 42);
+        NodeAgentContext context = new NodeAgentContextImpl.Builder("container-42.domain.tld").build();
+        makeContainer("container-42", Container.State.RUNNING, 42);
 
         when(processExecuter.exec(aryEq(new String[]{"nsenter", "--net=/proc/42/ns/net", "--", "iptables", "-nvL"})))
                 .thenReturn(new Pair<>(0, ""));
 
-        dockerOperations.executeCommandInNetworkNamespace(container.name, "iptables", "-nvL");
+        dockerOperations.executeCommandInNetworkNamespace(context, "iptables", "-nvL");
     }
 
     private Container makeContainer(String name, Container.State state, int pid) {
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 65e8ebf0743..8f455a08860 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,236 +1,263 @@
 // 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.yahoo.config.provision.HostName;
-import com.yahoo.vespa.hosted.dockerapi.Container;
-import com.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl;
+import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesMock;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
+import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.nio.file.FileSystem;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.List;
-import java.util.Map;
+import java.util.function.Function;
 
+import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.endsWith;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 public class AclMaintainerTest {
 
-    private final HostName hostHostname = HostName.from("node-admin.region-1.yahoo.com");
-    private final IPAddressesMock ipAddresses = new IPAddressesMock();
-    private final DockerOperations dockerOperations = mock(DockerOperations.class);
-    private final NodeRepository nodeRepository = mock(NodeRepository.class);
-    private final Map<String, Container> containers = new HashMap<>();
-    private final List<Container> containerList = new ArrayList<>();
-    private final AclMaintainer aclMaintainer =
-            new AclMaintainer(dockerOperations, nodeRepository, hostHostname, ipAddresses);
-
-    @Before
-    public void before() {
-        when(dockerOperations.getAllManagedContainers()).thenReturn(containerList);
-    }
-
-    @Test
-    public void empty_trusted_ports_are_handled() {
-        Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
-        Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
-
-        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
-
-        whenListRules(container.name, "filter", IPVersion.IPv6, "");
-        whenListRules(container.name, "filter", IPVersion.IPv4, "");
-        whenListRules(container.name, "nat", IPVersion.IPv4, "");
-        whenListRules(container.name, "nat", IPVersion.IPv6, "");
-
-        aclMaintainer.converge();
-
-        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), any()); //we don;t have a ip4 address for the container so no redirect either
-        verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), any());
-    }
-
-    @Test
-    public void configures_container_acl_when_iptables_differs() {
-        Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
-        Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
-
-        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
-
-        whenListRules(container.name, "filter", IPVersion.IPv6, "");
-        whenListRules(container.name, "filter", IPVersion.IPv4, "");
-        whenListRules(container.name, "nat", IPVersion.IPv4, "");
-        whenListRules(container.name, "nat", IPVersion.IPv6, "");
-
-        aclMaintainer.converge();
-
-        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), any()); //we don;t have a ip4 address for the container so no redirect either
-        verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), any());
-    }
-
-    @Test
-    public void ignore_containers_not_running() {
-        Container container = addContainer("container1", "container1.host.com", Container.State.EXITED);
-        Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
+    private static final String EMPTY_FILTER_TABLE = "-P INPUT ACCEPT\n-P FORWARD ACCEPT\n-P OUTPUT ACCEPT\n";
+    private static final String EMPTY_NAT_TABLE = "-P PREROUTING ACCEPT\n-P INPUT ACCEPT\n-P OUTPUT ACCEPT\n-P POSTROUTING ACCEPT\n";
 
-        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
+    private final DockerOperations dockerOperations = mock(DockerOperations.class);
+    private final IPAddressesMock ipAddresses = new IPAddressesMock();
+    private final AclMaintainer aclMaintainer = new AclMaintainer(dockerOperations, ipAddresses);
 
-        aclMaintainer.converge();
-
-        verify(dockerOperations, never()).executeCommandInNetworkNamespace(eq(container.name), any());
-    }
+    private final FileSystem fileSystem = TestFileSystem.create();
+    private final Function<Acl, NodeAgentContext> contextGenerator =
+            acl -> new NodeAgentContextImpl.Builder("container1.host.com").fileSystem(fileSystem).acl(acl).build();
+    private final List<String> writtenFileContents = new ArrayList<>();
 
     @Test
-    public void only_configure_iptables_for_ipversion_that_differs() {
-        Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
-        Map<String, Acl> acls = makeAcl(container.hostname, "4321,2345,22", "2001::1", "fd01:1234::4321");
-
-        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
-
-        String IPV6 = "-P INPUT ACCEPT\n" +
+    public void configures_full_container_acl_from_empty() {
+        Acl acl = new Acl.Builder().withTrustedPorts(22, 4443)
+                .withTrustedNode("hostname1", "3001::abcd")
+                .withTrustedNode("hostname2", "3001::1234")
+                .withTrustedNode("hostname1", "192.168.0.5")
+                .withTrustedNode("hostname4", "172.16.5.234").build();
+        NodeAgentContext context = contextGenerator.apply(acl);
+
+        ipAddresses.addAddress(context.hostname().value(), "2001::1");
+        ipAddresses.addAddress(context.hostname().value(), "10.0.0.1");
+
+        whenListRules(context, "filter", IPVersion.IPv4, EMPTY_FILTER_TABLE);
+        whenListRules(context, "filter", IPVersion.IPv6, EMPTY_FILTER_TABLE);
+        whenListRules(context, "nat", IPVersion.IPv4, EMPTY_NAT_TABLE);
+        whenListRules(context, "nat", IPVersion.IPv6, EMPTY_NAT_TABLE);
+
+        aclMaintainer.converge(context);
+
+        verify(dockerOperations, times(4)).executeCommandInNetworkNamespace(eq(context), any(), eq("-S"), eq("-t"), any());
+        verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(context), eq("iptables-restore"), any());
+        verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(context), eq("ip6tables-restore"), any());
+        verifyNoMoreInteractions(dockerOperations);
+
+        List<String> expected = Arrays.asList(
+                // IPv4 filter table restore
+                "*filter\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P FORWARD ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
+                "-A INPUT -i lo -j ACCEPT\n" +
+                "-A INPUT -p icmp -j ACCEPT\n" +
+                "-A INPUT -p tcp -m multiport --dports 22,4443 -j ACCEPT\n" +
+                "-A INPUT -s 172.16.5.234/32 -j ACCEPT\n" +
+                "-A INPUT -s 192.168.0.5/32 -j ACCEPT\n" +
+                "-A INPUT -j REJECT --reject-with icmp-port-unreachable\n" +
+                "COMMIT\n",
+
+                // IPv6 filter table restore
+                "*filter\n" +
+                "-P INPUT ACCEPT\n" +
                 "-P FORWARD ACCEPT\n" +
                 "-P OUTPUT ACCEPT\n" +
                 "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
                 "-A INPUT -i lo -j ACCEPT\n" +
                 "-A INPUT -p ipv6-icmp -j ACCEPT\n" +
-                "-A INPUT -p tcp -m multiport --dports 22,2345,4321 -j ACCEPT\n" +
-                "-A INPUT -s 2001::1/128 -j ACCEPT\n" +
-                "-A INPUT -s fd01:1234::4321/128 -j ACCEPT\n" +
-                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable";
-
-        String NATv6 = "-P PREROUTING ACCEPT\n" +
+                "-A INPUT -p tcp -m multiport --dports 22,4443 -j ACCEPT\n" +
+                "-A INPUT -s 3001::1234/128 -j ACCEPT\n" +
+                "-A INPUT -s 3001::abcd/128 -j ACCEPT\n" +
+                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable\n" +
+                "COMMIT\n",
+
+                // IPv4 nat table restore
+                "*nat\n" +
+                "-P PREROUTING ACCEPT\n" +
                 "-P INPUT ACCEPT\n" +
                 "-P OUTPUT ACCEPT\n" +
                 "-P POSTROUTING ACCEPT\n" +
-                "-A OUTPUT -d 3001::1/128 -j REDIRECT";
+                "-A OUTPUT -d 10.0.0.1/32 -j REDIRECT\n" +
+                "COMMIT\n",
 
-        whenListRules(container.name, "filter", IPVersion.IPv6, IPV6);
-        whenListRules(container.name, "filter", IPVersion.IPv4, ""); //IPv4 will then differ from wanted
-        whenListRules(container.name, "nat", IPVersion.IPv6, NATv6);
-
-        aclMaintainer.converge();
-
-        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), any());
-        verify(dockerOperations, never()).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), any());
+                // IPv6 nat table restore
+                "*nat\n" +
+                "-P PREROUTING ACCEPT\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-P POSTROUTING ACCEPT\n" +
+                "-A OUTPUT -d 2001::1/128 -j REDIRECT\n" +
+                "COMMIT\n");
+        assertEquals(expected, writtenFileContents);
     }
 
     @Test
-    public void does_not_configure_acl_if_iptables_dualstack_are_ok() {
-        Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
-        Map<String, Acl> acls = makeAcl(container.hostname, "22,4443,2222", "2001::1", "192.64.13.2");
-
-        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
-
-        String IPV4_FILTER = "-P INPUT ACCEPT\n" +
+    public void configures_minimal_container_acl_from_empty() {
+        // The ACL spec is empty and our this node's addresses do not resolve
+        Acl acl = new Acl.Builder().withTrustedPorts().build();
+        NodeAgentContext context = contextGenerator.apply(acl);
+
+        whenListRules(context, "filter", IPVersion.IPv4, EMPTY_FILTER_TABLE);
+        whenListRules(context, "filter", IPVersion.IPv6, EMPTY_FILTER_TABLE);
+        whenListRules(context, "nat", IPVersion.IPv4, EMPTY_NAT_TABLE);
+        whenListRules(context, "nat", IPVersion.IPv6, EMPTY_NAT_TABLE);
+
+        aclMaintainer.converge(context);
+
+        verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(context), any(), eq("-S"), eq("-t"), any());
+        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(context), eq("iptables-restore"), any());
+        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(context), eq("ip6tables-restore"), any());
+        verifyNoMoreInteractions(dockerOperations);
+
+        List<String> expected = Arrays.asList(
+                // IPv4 filter table restore
+                "*filter\n" +
+                "-P INPUT ACCEPT\n" +
                 "-P FORWARD ACCEPT\n" +
                 "-P OUTPUT ACCEPT\n" +
                 "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
                 "-A INPUT -i lo -j ACCEPT\n" +
                 "-A INPUT -p icmp -j ACCEPT\n" +
-                "-A INPUT -p tcp -m multiport --dports 22,2222,4443 -j ACCEPT\n" +
-                "-A INPUT -s 192.64.13.2/32 -j ACCEPT\n" +
-                "-A INPUT -j REJECT --reject-with icmp-port-unreachable";
+                "-A INPUT -j REJECT --reject-with icmp-port-unreachable\n" +
+                "COMMIT\n",
 
-        String IPV6_FILTER = "-P INPUT ACCEPT\n" +
+                // IPv6 filter table restore
+                "*filter\n" +
+                "-P INPUT ACCEPT\n" +
                 "-P FORWARD ACCEPT\n" +
                 "-P OUTPUT ACCEPT\n" +
                 "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
                 "-A INPUT -i lo -j ACCEPT\n" +
                 "-A INPUT -p ipv6-icmp -j ACCEPT\n" +
-                "-A INPUT -p tcp -m multiport --dports 22,2222,4443 -j ACCEPT\n" +
-                "-A INPUT -s 2001::1/128 -j ACCEPT\n" +
-                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable";
+                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable\n" +
+                "COMMIT\n");
+        assertEquals(expected, writtenFileContents);
+    }
 
-        String IPV6_NAT = "-P PREROUTING ACCEPT\n" +
+    @Test
+    public void only_configure_iptables_for_ipversion_that_differs() {
+        Acl acl = new Acl.Builder().withTrustedPorts(22, 4443).withTrustedNode("hostname1", "3001::abcd").build();
+        NodeAgentContext context = contextGenerator.apply(acl);
+
+        ipAddresses.addAddress(context.hostname().value(), "2001::1");
+
+        whenListRules(context, "filter", IPVersion.IPv4, EMPTY_FILTER_TABLE);
+        whenListRules(context, "filter", IPVersion.IPv6,
+                "-P INPUT ACCEPT\n" +
+                "-P FORWARD ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
+                "-A INPUT -i lo -j ACCEPT\n" +
+                "-A INPUT -p ipv6-icmp -j ACCEPT\n" +
+                "-A INPUT -p tcp -m multiport --dports 22,4443 -j ACCEPT\n" +
+                "-A INPUT -s 3001::abcd/128 -j ACCEPT\n" +
+                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable\n");
+        whenListRules(context, "nat", IPVersion.IPv6,
+                "-P PREROUTING ACCEPT\n" +
                 "-P INPUT ACCEPT\n" +
                 "-P OUTPUT ACCEPT\n" +
                 "-P POSTROUTING ACCEPT\n" +
-                "-A OUTPUT -d 3001::1/128 -j REDIRECT";
+                "-A OUTPUT -d 2001::1/128 -j REDIRECT\n");
 
-        whenListRules(container.name, "filter", IPVersion.IPv6, IPV6_FILTER);
-        whenListRules(container.name, "nat", IPVersion.IPv6, IPV6_NAT);
-        whenListRules(container.name, "filter", IPVersion.IPv4, IPV4_FILTER);
+        aclMaintainer.converge(context);
 
-        aclMaintainer.converge();
+        verify(dockerOperations, times(3)).executeCommandInNetworkNamespace(eq(context), any(), eq("-S"), eq("-t"), any());
+        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(context), eq("iptables-restore"), any());
+        verify(dockerOperations, never()).executeCommandInNetworkNamespace(eq(context), eq("ip6tables-restore"), any()); //we don't have a ip4 address for the container so no redirect
+        verifyNoMoreInteractions(dockerOperations);
 
-        verify(dockerOperations, never()).executeCommandInNetworkNamespace(any(), eq("ip6tables-restore"), any());
-        verify(dockerOperations, never()).executeCommandInNetworkNamespace(any(), eq("iptables-restore"), any());
+        List<String> expected = Collections.singletonList(
+                "*filter\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P FORWARD ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
+                "-A INPUT -i lo -j ACCEPT\n" +
+                "-A INPUT -p icmp -j ACCEPT\n" +
+                "-A INPUT -p tcp -m multiport --dports 22,4443 -j ACCEPT\n" +
+                "-A INPUT -j REJECT --reject-with icmp-port-unreachable\n" +
+                "COMMIT\n");
+        assertEquals(expected, writtenFileContents);
     }
 
-
     @Test
     public void rollback_is_attempted_when_applying_acl_fail() {
-        Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
-        Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
-        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
+        Acl acl = new Acl.Builder().withTrustedPorts(22, 4443).withTrustedNode("hostname1", "3001::abcd").build();
+        NodeAgentContext context = contextGenerator.apply(acl);
+
+        ipAddresses.addAddress(context.hostname().value(), "2001::1");
 
-        String IPV6_NAT = "-P PREROUTING ACCEPT\n" +
+        whenListRules(context, "filter", IPVersion.IPv4, EMPTY_FILTER_TABLE);
+        whenListRules(context, "filter", IPVersion.IPv6,
+                "-P INPUT ACCEPT\n" +
+                "-P FORWARD ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
+                "-A INPUT -i lo -j ACCEPT\n" +
+                "-A INPUT -p ipv6-icmp -j ACCEPT\n" +
+                "-A INPUT -p tcp -m multiport --dports 22,4443 -j ACCEPT\n" +
+                "-A INPUT -s 3001::abcd/128 -j ACCEPT\n" +
+                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable\n");
+        whenListRules(context, "nat", IPVersion.IPv6,
+                "-P PREROUTING ACCEPT\n" +
                 "-P INPUT ACCEPT\n" +
                 "-P OUTPUT ACCEPT\n" +
                 "-P POSTROUTING ACCEPT\n" +
-                "-A OUTPUT -d 3001::1/128 -j REDIRECT";
+                "-A OUTPUT -d 2001::1/128 -j REDIRECT\n");
 
-        whenListRules(container.name, "filter", IPVersion.IPv6, "");
-        whenListRules(container.name, "filter", IPVersion.IPv4, "");
-        whenListRules(container.name, "nat", IPVersion.IPv6, IPV6_NAT);
+        when(dockerOperations.executeCommandInNetworkNamespace(eq(context), eq("iptables-restore"), any()))
+                .thenThrow(new RuntimeException("iptables restore failed"));
 
-        when(dockerOperations.executeCommandInNetworkNamespace(
-                eq(container.name),
-                eq("ip6tables-restore"), any())).thenThrow(new RuntimeException("iptables restore failed"));
+        aclMaintainer.converge(context);
 
-        when(dockerOperations.executeCommandInNetworkNamespace(
-                eq(container.name),
-                eq("iptables-restore"), any())).thenThrow(new RuntimeException("iptables restore failed"));
+        verify(dockerOperations, times(3)).executeCommandInNetworkNamespace(eq(context), any(), eq("-S"), eq("-t"), any());
+        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(context), eq("iptables-restore"), any());
+        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(context), eq("iptables"), eq("-F"), eq("-t"), eq("filter"));
+        verifyNoMoreInteractions(dockerOperations);
 
-        aclMaintainer.converge();
+        aclMaintainer.converge(context);
+    }
 
-        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name),
-                eq("ip6tables"), eq("-F"), eq("-t"), eq("filter"));
-        verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name),
-                eq("iptables"), eq("-F"),  eq("-t"), eq("filter"));
+    @Before
+    public void setup() {
+        doAnswer(invoc -> {
+            String path = invoc.getArgument(2);
+            writtenFileContents.add(new UnixPath(path).readUtf8File());
+            return new ProcessResult(0, "", "");
+        }).when(dockerOperations).executeCommandInNetworkNamespace(any(), endsWith("-restore"), any());
     }
 
-    private void whenListRules(ContainerName name, String table, IPVersion ipVersion, String result) {
+    private void whenListRules(NodeAgentContext context, String table, IPVersion ipVersion, String result) {
         when(dockerOperations.executeCommandInNetworkNamespace(
-                eq(name),
-                eq(ipVersion.iptablesCmd()), eq("-S"), eq("-t"), eq(table)))
+                eq(context), eq(ipVersion.iptablesCmd()), eq("-S"), eq("-t"), eq(table)))
                 .thenReturn(new ProcessResult(0, result, ""));
     }
-
-    private Container addContainer(String name, String hostname, Container.State state) {
-        final ContainerName containerName = new ContainerName(name);
-        final Container container = new Container(hostname, new DockerImage("mock"), null,
-                containerName, state, 2);
-        containers.put(name, container);
-        containerList.add(container);
-        ipAddresses.addAddress(hostname, "3001::" + containers.size());
-        return container;
-    }
-
-    private Map<String, Acl> makeAcl(String containerHostname, String portsCommaSeparated, String... addresses) {
-        Acl.Builder aclBuilder = new Acl.Builder();
-
-        Arrays.stream(portsCommaSeparated.split(","))
-                .map(Integer::valueOf)
-                .forEach(aclBuilder::withTrustedPorts);
-
-        Arrays.stream(addresses)
-                .forEach(address -> aclBuilder.withTrustedNode(new Acl.Node("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/FilterTableLineEditorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditorTest.java
new file mode 100644
index 00000000000..f72cde92839
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditorTest.java
@@ -0,0 +1,50 @@
+// Copyright 2019 Oath Inc. 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.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
+import com.yahoo.vespa.hosted.node.admin.task.util.file.Editor;
+import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author freva
+ */
+public class FilterTableLineEditorTest {
+
+    @Test
+    public void filter_set_wanted_rules() {
+        Acl acl = new Acl.Builder().withTrustedPorts(22).withTrustedNode("hostname", "3001::1").build();
+
+        assertFilterTableLineEditorResult(
+                acl, IPVersion.IPv6,
+
+                "-P INPUT ACCEPT\n" +
+                "-P FORWARD ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n",
+
+                "-P INPUT ACCEPT\n" +
+                "-P FORWARD ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
+                "-A INPUT -i lo -j ACCEPT\n" +
+                "-A INPUT -p ipv6-icmp -j ACCEPT\n" +
+                "-A INPUT -p tcp -m multiport --dports 22 -j ACCEPT\n" +
+                "-A INPUT -s 3001::1/128 -j ACCEPT\n" +
+                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable");
+    }
+
+    private static void assertFilterTableLineEditorResult(
+            Acl acl, IPVersion ipVersion, String currentFilterTable, String expectedRestoreFileContent) {
+        FilterTableLineEditor filterLineEditor = FilterTableLineEditor.from(acl, ipVersion);
+        Editor editor = new Editor(
+                "nat-table",
+                () -> Arrays.asList(currentFilterTable.split("\n")),
+                result -> assertEquals(expectedRestoreFileContent, String.join("\n", result)),
+                filterLineEditor);
+        editor.edit(m -> {});
+    }
+}
\ No newline at end of file
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
deleted file mode 100644
index abcba44bb7a..00000000000
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditorTest.java
+++ /dev/null
@@ -1,144 +0,0 @@
-// Copyright 2018 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.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
-import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
-import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
-import org.junit.Assert;
-import org.junit.Test;
-
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-public class IPTablesEditorTest {
-
-    private final DockerOperations dockerOperations = mock(DockerOperations.class);
-    private final ContainerName containerName = ContainerName.fromHostname("myhostname");
-    private String actualRestoreContent = null;
-    private final IPTablesEditor editor = new IPTablesEditor(dockerOperations, containerName, (fileContent) -> actualRestoreContent = fileContent);
-
-    @Test
-    public void filter_set_wanted_rules() {
-        Acl acl = new Acl.Builder().withTrustedPorts(22).withTrustedNode(new Acl.Node("hostname", "3001::1")).build();
-        FilterTableLineEditor filterLineEditor = FilterTableLineEditor.from(acl, IPVersion.IPv6);
-
-        String currentFilterTable = "-P INPUT ACCEPT\n" +
-                "-P FORWARD ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n";
-
-        String expectedRestoreFileContent = "*filter\n" +
-                "-P INPUT ACCEPT\n" +
-                "-P FORWARD ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n" +
-                "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" +
-                "-A INPUT -i lo -j ACCEPT\n" +
-                "-A INPUT -p ipv6-icmp -j ACCEPT\n" +
-                "-A INPUT -p tcp -m multiport --dports 22 -j ACCEPT\n" +
-                "-A INPUT -s 3001::1/128 -j ACCEPT\n" +
-                "-A INPUT -j REJECT --reject-with icmp6-port-unreachable\n" +
-                "COMMIT\n";
-
-        whenListRules(containerName, "filter", IPVersion.IPv6, currentFilterTable);
-
-        editor.edit("filter", IPVersion.IPv6, filterLineEditor, false);
-
-        Assert.assertEquals(expectedRestoreFileContent, actualRestoreContent);
-    }
-
-    @Test
-    public void nat_set_redirect_rule_without_touching_docker_rules() {
-        NatTableLineEditor natLineEditor = NatTableLineEditor.from("-A OUTPUT -d 3001::1/128 -j REDIRECT");
-
-        String currentNatTable = "-P PREROUTING ACCEPT\n" +
-                "-P INPUT ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n" +
-                "-P POSTROUTING ACCEPT\n" +
-                "-N DOCKER_OUTPUT\n" +
-                "-N DOCKER_POSTROUTING\n" +
-                "-A OUTPUT -d 127.0.0.11/32 -j DOCKER_OUTPUT\n" +
-                "-A POSTROUTING -d 127.0.0.11/32 -j DOCKER_POSTROUTING\n" +
-                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p tcp -m tcp --dport 53 -j DNAT --to-destination 127.0.0.11:43500\n" +
-                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p udp -m udp --dport 53 -j DNAT --to-destination 127.0.0.11:57392\n" +
-                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p tcp -m tcp --sport 43500 -j SNAT --to-source :53\n" +
-                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p udp -m udp --sport 57392 -j SNAT --to-source :53\n";
-
-        String expectedRestoreFileContent = "*nat\n-P PREROUTING ACCEPT\n" +
-                "-P INPUT ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n" +
-                "-P POSTROUTING ACCEPT\n" +
-                "-N DOCKER_OUTPUT\n" +
-                "-N DOCKER_POSTROUTING\n" +
-                "-A OUTPUT -d 127.0.0.11/32 -j DOCKER_OUTPUT\n" +
-                "-A POSTROUTING -d 127.0.0.11/32 -j DOCKER_POSTROUTING\n" +
-                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p tcp -m tcp --dport 53 -j DNAT --to-destination 127.0.0.11:43500\n" +
-                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p udp -m udp --dport 53 -j DNAT --to-destination 127.0.0.11:57392\n" +
-                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p tcp -m tcp --sport 43500 -j SNAT --to-source :53\n" +
-                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p udp -m udp --sport 57392 -j SNAT --to-source :53\n" +
-                "-A OUTPUT -d 3001::1/128 -j REDIRECT\nCOMMIT\n";
-
-        whenListRules(containerName, "nat", IPVersion.IPv6, currentNatTable);
-
-        editor.edit("nat", IPVersion.IPv6, natLineEditor, false);
-
-        Assert.assertEquals(expectedRestoreFileContent, actualRestoreContent);
-    }
-
-    @Test
-    public void nat_cleanup_wrong_redirect_rules() {
-        NatTableLineEditor natLineEditor = NatTableLineEditor.from("-A OUTPUT -d 3001::1/128 -j REDIRECT");
-
-        String currentNatTable = "-P PREROUTING ACCEPT\n" +
-                "-P INPUT ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n" +
-                "-P POSTROUTING ACCEPT\n" +
-                "-A OUTPUT -d 3001::2/128 -j REDIRECT\n";
-
-        String expectedRestoreFileContent = "-P PREROUTING ACCEPT\n" +
-                "-P INPUT ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n" +
-                "-P POSTROUTING ACCEPT\n" +
-                "-A OUTPUT -d 3001::1/128 -j REDIRECT\n";
-
-        whenListRules(containerName, "nat", IPVersion.IPv6, currentNatTable);
-
-        editor.edit("nat", IPVersion.IPv6, natLineEditor, false);
-
-        Assert.assertEquals(expectedRestoreFileContent, expectedRestoreFileContent);
-    }
-
-    @Test
-    public void nat_delete_duplicate_rules() {
-        NatTableLineEditor natLineEditor = NatTableLineEditor.from("-A OUTPUT -d 3001::1/128 -j REDIRECT");
-
-        String currentNatTable = "-P PREROUTING ACCEPT\n" +
-                "-P INPUT ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n" +
-                "-P POSTROUTING ACCEPT\n" +
-                "-A OUTPUT -d 3001::2/128 -j REDIRECT\n" +
-                "-A OUTPUT -d 3001::1/128 -j REDIRECT\n" +
-                "-A OUTPUT -d 3001::4/128 -j REDIRECT\n";
-
-        String expectedRestoreFileContent = "-P PREROUTING ACCEPT\n" +
-                "-P INPUT ACCEPT\n" +
-                "-P OUTPUT ACCEPT\n" +
-                "-P POSTROUTING ACCEPT\n" +
-                "-A OUTPUT -d 3001::1/128 -j REDIRECT\n";
-
-        whenListRules(containerName, "nat", IPVersion.IPv6, currentNatTable);
-
-        editor.edit("nat", IPVersion.IPv6, natLineEditor, false);
-
-        Assert.assertEquals(expectedRestoreFileContent, expectedRestoreFileContent);
-    }
-
-    private void whenListRules(ContainerName name, String table, IPVersion ipVersion, String result) {
-        when(dockerOperations.executeCommandInNetworkNamespace(
-                eq(name),
-                eq(ipVersion.iptablesCmd()), eq("-S"), eq("-t"), eq(table)))
-                .thenReturn(new ProcessResult(0, result, ""));
-    }
-
-}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/NatTableLineEditorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/NatTableLineEditorTest.java
new file mode 100644
index 00000000000..63dc69a180c
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/NatTableLineEditorTest.java
@@ -0,0 +1,96 @@
+// Copyright 2019 Oath Inc. 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.yahoo.vespa.hosted.node.admin.task.util.file.Editor;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author freva
+ */
+public class NatTableLineEditorTest {
+
+    @Test
+    public void nat_set_redirect_rule_without_touching_docker_rules() {
+        assertNatTableLineEditorResult(
+                "-A OUTPUT -d 3001::1/128 -j REDIRECT",
+
+                "-P PREROUTING ACCEPT\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-P POSTROUTING ACCEPT\n" +
+                "-N DOCKER_OUTPUT\n" +
+                "-N DOCKER_POSTROUTING\n" +
+                "-A OUTPUT -d 127.0.0.11/32 -j DOCKER_OUTPUT\n" +
+                "-A POSTROUTING -d 127.0.0.11/32 -j DOCKER_POSTROUTING\n" +
+                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p tcp -m tcp --dport 53 -j DNAT --to-destination 127.0.0.11:43500\n" +
+                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p udp -m udp --dport 53 -j DNAT --to-destination 127.0.0.11:57392\n" +
+                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p tcp -m tcp --sport 43500 -j SNAT --to-source :53\n" +
+                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p udp -m udp --sport 57392 -j SNAT --to-source :53\n",
+
+                "-P PREROUTING ACCEPT\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-P POSTROUTING ACCEPT\n" +
+                "-N DOCKER_OUTPUT\n" +
+                "-N DOCKER_POSTROUTING\n" +
+                "-A OUTPUT -d 127.0.0.11/32 -j DOCKER_OUTPUT\n" +
+                "-A POSTROUTING -d 127.0.0.11/32 -j DOCKER_POSTROUTING\n" +
+                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p tcp -m tcp --dport 53 -j DNAT --to-destination 127.0.0.11:43500\n" +
+                "-A DOCKER_OUTPUT -d 127.0.0.11/32 -p udp -m udp --dport 53 -j DNAT --to-destination 127.0.0.11:57392\n" +
+                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p tcp -m tcp --sport 43500 -j SNAT --to-source :53\n" +
+                "-A DOCKER_POSTROUTING -s 127.0.0.11/32 -p udp -m udp --sport 57392 -j SNAT --to-source :53\n" +
+                "-A OUTPUT -d 3001::1/128 -j REDIRECT");
+    }
+
+    @Test
+    public void nat_cleanup_wrong_redirect_rules() {
+        assertNatTableLineEditorResult(
+                "-A OUTPUT -d 3001::1/128 -j REDIRECT",
+
+                "-P PREROUTING ACCEPT\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-P POSTROUTING ACCEPT\n" +
+                "-A OUTPUT -d 3001::2/128 -j REDIRECT\n",
+
+                "-P PREROUTING ACCEPT\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-P POSTROUTING ACCEPT\n" +
+                "-A OUTPUT -d 3001::1/128 -j REDIRECT");
+    }
+
+    @Test
+    public void nat_delete_duplicate_rules() {
+        assertNatTableLineEditorResult(
+                "-A OUTPUT -d 3001::1/128 -j REDIRECT",
+
+                "-P PREROUTING ACCEPT\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-P POSTROUTING ACCEPT\n" +
+                "-A OUTPUT -d 3001::2/128 -j REDIRECT\n" +
+                "-A OUTPUT -d 3001::1/128 -j REDIRECT\n" +
+                "-A OUTPUT -d 3001::4/128 -j REDIRECT\n",
+
+                "-P PREROUTING ACCEPT\n" +
+                "-P INPUT ACCEPT\n" +
+                "-P OUTPUT ACCEPT\n" +
+                "-P POSTROUTING ACCEPT\n" +
+                "-A OUTPUT -d 3001::1/128 -j REDIRECT");
+    }
+
+    private static void assertNatTableLineEditorResult(String redirectRule, String currentNatTable, String expectedNatTable) {
+        NatTableLineEditor natLineEditor = NatTableLineEditor.from(redirectRule);
+        Editor editor = new Editor(
+                "nat-table",
+                () -> Arrays.asList(currentNatTable.split("\n")),
+                result -> assertEquals(expectedNatTable, String.join("\n", result)),
+                natLineEditor);
+        editor.edit(m -> {});
+    }
+}
\ No newline at end of file
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index 60c36ad4412..35494b375f6 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -199,7 +199,7 @@ public class NodeAgentImplTest {
         inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
         inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any());
         inOrder.verify(dockerOperations, times(1)).startContainer(eq(context));
-        inOrder.verify(aclMaintainer, times(1)).converge();
+        inOrder.verify(aclMaintainer, times(1)).converge(eq(context));
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context));
         inOrder.verify(healthChecker, times(1)).verifyHealth(eq(context));
         inOrder.verify(nodeRepository).updateNodeAttributes(
@@ -742,7 +742,7 @@ public class NodeAgentImplTest {
         inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
         inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any());
         inOrder.verify(dockerOperations, times(1)).startContainer(eq(context));
-        inOrder.verify(aclMaintainer, times(1)).converge();
+        inOrder.verify(aclMaintainer, times(1)).converge(eq(context));
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context));
         inOrder.verify(nodeRepository).updateNodeAttributes(
                 hostName, new NodeAttributes().withDockerImage(dockerImage));