Browse code

Fix: setup user chains even if there are running containers

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 <maldonado@codelutin.com>

Andrés Maldonado authored on 2024/10/04 08:53:46
Showing 3 changed files
... ...
@@ -851,6 +851,10 @@ func (daemon *Daemon) initNetworkController(cfg *config.Config, activeSandboxes
851 851
 		return err
852 852
 	}
853 853
 
854
+	if err := daemon.netController.SetupUserChains(); err != nil {
855
+		log.G(context.TODO()).WithError(err).Warnf("initNetworkController")
856
+	}
857
+
854 858
 	// Set HostGatewayIP to the default bridge's IP if it is empty
855 859
 	setHostGatewayIP(daemon.netController, cfg)
856 860
 	return nil
... ...
@@ -456,6 +456,7 @@ func TestLiveRestore(t *testing.T) {
456 456
 
457 457
 	t.Run("volume references", testLiveRestoreVolumeReferences)
458 458
 	t.Run("autoremove", testLiveRestoreAutoRemove)
459
+	t.Run("user chains", testLiveRestoreUserChainsSetup)
459 460
 }
460 461
 
461 462
 func testLiveRestoreAutoRemove(t *testing.T) {
... ...
@@ -674,6 +675,34 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
674 674
 	})
675 675
 }
676 676
 
677
+func testLiveRestoreUserChainsSetup(t *testing.T) {
678
+	skip.If(t, testEnv.IsRootless(), "rootless daemon uses it's own network namespace")
679
+
680
+	t.Parallel()
681
+	ctx := testutil.StartSpan(baseContext, t)
682
+
683
+	t.Run("user chains should be inserted", func(t *testing.T) {
684
+		d := daemon.New(t)
685
+		d.StartWithBusybox(ctx, t, "--live-restore")
686
+		t.Cleanup(func() {
687
+			d.Stop(t)
688
+			d.Cleanup(t)
689
+		})
690
+
691
+		c := d.NewClientT(t)
692
+
693
+		cID := container.Run(ctx, t, c, container.WithCmd("top"))
694
+		defer c.ContainerRemove(ctx, cID, containertypes.RemoveOptions{Force: true})
695
+
696
+		d.Stop(t)
697
+		icmd.RunCommand("iptables", "--flush", "FORWARD").Assert(t, icmd.Success)
698
+		d.Start(t, "--live-restore")
699
+
700
+		result := icmd.RunCommand("iptables", "-S", "FORWARD", "1")
701
+		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")
702
+	})
703
+}
704
+
677 705
 func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {
678 706
 	skip.If(t, runtime.GOOS == "windows")
679 707
 
... ...
@@ -706,15 +706,22 @@ addToStore:
706 706
 		c.mu.Unlock()
707 707
 	}
708 708
 
709
-	// Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6)
710
-	// that's enabled in the controller's configuration.
709
+	if err := c.SetupUserChains(); err != nil {
710
+		log.G(context.TODO()).WithError(err).Warnf("Controller.NewNetwork %s:", name)
711
+	}
712
+
713
+	return nw, nil
714
+}
715
+
716
+// Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6) that's
717
+// enabled in the controller's configuration.
718
+func (c *Controller) SetupUserChains() error {
711 719
 	for _, ipVersion := range c.enabledIptablesVersions() {
712 720
 		if err := setupUserChain(ipVersion); err != nil {
713
-			log.G(context.TODO()).WithError(err).Warnf("Controller.NewNetwork %s:", name)
721
+			return err
714 722
 		}
715 723
 	}
716
-
717
-	return nw, nil
724
+	return nil
718 725
 }
719 726
 
720 727
 var joinCluster NetworkWalker = func(nw *Network) bool {