Fix: setup user chains even if there are running containers

am97 · am97 · commit a8bfa83667fb · 2024-10-16T22:41:39.000+02:00
Currently, the DOCKER-USER chains are set up on firewall reload or network
creation. If there are running containers at startup, configureNetworking won't
be called (daemon/daemon_unix.go), so the user chains won't be setup.

This commit puts the setup logic on a separate function, and calls it on the
original place and on initNetworkController.

Signed-off-by: Andrés Maldonado &lt;maldonado@codelutin.com&gt;
diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go
@@ -851,6 +851,10 @@ func (daemon *Daemon) initNetworkController(cfg *config.Config, activeSandboxes
 		return err
 	}
 
+	if err := daemon.netController.SetupUserChains(); err != nil {
+		log.G(context.TODO()).WithError(err).Warnf("initNetworkController")
+	}
+
 	// Set HostGatewayIP to the default bridge's IP if it is empty
 	setHostGatewayIP(daemon.netController, cfg)
 	return nil
diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go
@@ -456,6 +456,7 @@ func TestLiveRestore(t *testing.T) {
 
 	t.Run("volume references", testLiveRestoreVolumeReferences)
 	t.Run("autoremove", testLiveRestoreAutoRemove)
+	t.Run("user chains", testLiveRestoreUserChainsSetup)
 }
 
 func testLiveRestoreAutoRemove(t *testing.T) {
@@ -674,6 +675,34 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
 	})
 }
 
+func testLiveRestoreUserChainsSetup(t *testing.T) {
+	skip.If(t, testEnv.IsRootless(), "rootless daemon uses it's own network namespace")
+
+	t.Parallel()
+	ctx := testutil.StartSpan(baseContext, t)
+
+	t.Run("user chains should be inserted", func(t *testing.T) {
+		d := daemon.New(t)
+		d.StartWithBusybox(ctx, t, "--live-restore")
+		t.Cleanup(func() {
+			d.Stop(t)
+			d.Cleanup(t)
+		})
+
+		c := d.NewClientT(t)
+
+		cID := container.Run(ctx, t, c, container.WithCmd("top"))
+		defer c.ContainerRemove(ctx, cID, containertypes.RemoveOptions{Force: true})
+
+		d.Stop(t)
+		icmd.RunCommand("iptables", "--flush", "FORWARD").Assert(t, icmd.Success)
+		d.Start(t, "--live-restore")
+
+		result := icmd.RunCommand("iptables", "-S", "FORWARD", "1")
+		assert.Check(t, is.Equal(strings.TrimSpace(result.Stdout()), "-A FORWARD -j DOCKER-USER"), "the jump to DOCKER-USER should be the first rule in the FORWARD chain")
+	})
+}
+
 func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows")
 
diff --git a/libnetwork/controller.go b/libnetwork/controller.go
@@ -706,15 +706,22 @@ addToStore:
 		c.mu.Unlock()
 	}
 
-	// Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6)
-	// that's enabled in the controller's configuration.
+	if err := c.SetupUserChains(); err != nil {
+		log.G(context.TODO()).WithError(err).Warnf("Controller.NewNetwork %s:", name)
+	}
+
+	return nw, nil
+}
+
+// Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6) that's
+// enabled in the controller's configuration.
+func (c *Controller) SetupUserChains() error {
 	for _, ipVersion := range c.enabledIptablesVersions() {
 		if err := setupUserChain(ipVersion); err != nil {
-			log.G(context.TODO()).WithError(err).Warnf("Controller.NewNetwork %s:", name)
+			return err
 		}
 	}
-
-	return nw, nil
+	return nil
 }
 
 var joinCluster NetworkWalker = func(nw *Network) bool {
