DOCKER-USER chain not created when IPTableEnable=false.

This fix addresses https://docker.atlassian.net/browse/ENGCORE-1115
Expected behaviors upon docker engine restarts:
1. IPTableEnable=true, DOCKER-USER chain present
   -- no change to DOCKER-USER chain
2. IPTableEnable=true, DOCKER-USER chain not present
   -- DOCKER-USER chain created and inserted top of FORWARD
      chain.
3. IPTableEnable=false, DOCKER-USER chain present
   -- no change to DOCKER-USER chain
      the rational is that DOCKER-USER is populated
      and may be used by end-user for purpose other than
      filtering docker container traffic. Thus even if
      IPTableEnable=false, docker engine does not touch
      pre-existing DOCKER-USER chain.
4. IPTableEnable=false, DOCKER-USER chain not present
   -- DOCKER-USER chain is not created.

Signed-off-by: Su Wang <su.wang@docker.com>

controller.go

@@ -67,6 +67,7 @@ import (
 	"github.com/docker/libnetwork/hostdiscovery"
 	"github.com/docker/libnetwork/ipamapi"
 	"github.com/docker/libnetwork/netlabel"
+	"github.com/docker/libnetwork/options"
 	"github.com/docker/libnetwork/osl"
 	"github.com/docker/libnetwork/types"
 	"github.com/pkg/errors"
@@ -909,8 +910,8 @@ addToStore:
 		arrangeIngressFilterRule()
 		c.Unlock()
 	}
-
-	c.arrangeUserFilterRule()
+	setupArrangeUserFilterRule(c)
+	arrangeUserFilterRule()
 
 	return network, nil
 }
@@ -1363,3 +1364,27 @@ func (c *controller) IsDiagnosticEnabled() bool {
 	defer c.Unlock()
 	return c.DiagnosticServer.IsDiagnosticEnabled()
 }
+
+func (c *controller) iptableEnabled() bool {
+	c.Lock()
+	defer c.Unlock()
+
+	if c.cfg == nil {
+		return false
+	}
+	// parse map cfg["bridge"]["generic"]["EnableIPTable"]
+	cfgBridge, ok := c.cfg.Daemon.DriverCfg["bridge"].(map[string]interface{})
+	if !ok {
+		return false
+	}
+	cfgGeneric, ok := cfgBridge[netlabel.GenericData].(options.Generic)
+	if !ok {
+		return false
+	}
+	enabled, ok := cfgGeneric["EnableIPTables"].(bool)
+	if !ok {
+		// unless user explicitly stated, assume iptable is enabled
+		enabled = true
+	}
+	return enabled
+}

firewall_linux.go

@@ -3,25 +3,38 @@ package libnetwork
 import (
 	"github.com/docker/libnetwork/iptables"
 	"github.com/sirupsen/logrus"
+	"sync"
 )
 
 const userChain = "DOCKER-USER"
 
-func (c *controller) arrangeUserFilterRule() {
-	c.Lock()
-	arrangeUserFilterRule()
-	c.Unlock()
-	iptables.OnReloaded(func() {
+var (
+	ctrl          *controller = nil
+	userChainOnce sync.Once
+)
+
+func setupArrangeUserFilterRule(c *controller) {
+	userChainOnce.Do(func() {
 		c.Lock()
-		arrangeUserFilterRule()
-		c.Unlock()
+		if ctrl != nil {
+			logrus.Fatalf("Multiple controllers assigned")
+		}
+		ctrl = c
+		c.Unlock() // defer executes at end of function, so dont use
+		iptables.OnReloaded(arrangeUserFilterRule)
 	})
 }
 
 // This chain allow users to configure firewall policies in a way that persists
 // docker operations/restarts. Docker will not delete or modify any pre-existing
 // rules from the DOCKER-USER filter chain.
+// Note once DOCKER-USER chain is created, docker engine does not remove it when
+// IPTableForwarding is disabled, because it contains rules configure by user that
+// are beyond docker engine's control.
 func arrangeUserFilterRule() {
+	if ctrl == nil || !ctrl.iptableEnabled() {
+		return
+	}
 	_, err := iptables.NewChain(userChain, iptables.Filter, false)
 	if err != nil {
 		logrus.Warnf("Failed to create %s chain: %v", userChain, err)

firewall_others.go

@@ -2,5 +2,5 @@
 
 package libnetwork
 
-func (c *controller) arrangeUserFilterRule() {
-}
+func setupArrangeUserFilterRule(c *controller) {}
+func arrangeUserFilterRule()                   {}

firewall_test.go

@@ -0,0 +1,98 @@
+package libnetwork
+
+import (
+	"fmt"
+	"gotest.tools/assert"
+	"strings"
+	"testing"
+
+	"github.com/docker/libnetwork/iptables"
+	"github.com/docker/libnetwork/netlabel"
+	"github.com/docker/libnetwork/options"
+)
+
+const (
+	fwdChainName = "FORWARD"
+	usrChainName = userChain
+)
+
+func TestUserChain(t *testing.T) {
+	nc, err := New()
+	assert.NilError(t, err)
+
+	tests := []struct {
+		iptables  bool
+		insert    bool // insert other rules to FORWARD
+		fwdChain  []string
+		userChain []string
+	}{
+		{
+			iptables: false,
+			insert:   false,
+			fwdChain: []string{"-P FORWARD ACCEPT"},
+		},
+		{
+			iptables:  true,
+			insert:    false,
+			fwdChain:  []string{"-P FORWARD ACCEPT", "-A FORWARD -j DOCKER-USER"},
+			userChain: []string{"-N DOCKER-USER", "-A DOCKER-USER -j RETURN"},
+		},
+		{
+			iptables:  true,
+			insert:    true,
+			fwdChain:  []string{"-P FORWARD ACCEPT", "-A FORWARD -j DOCKER-USER", "-A FORWARD -j DROP"},
+			userChain: []string{"-N DOCKER-USER", "-A DOCKER-USER -j RETURN"},
+		},
+	}
+
+	resetIptables(t)
+	for _, tc := range tests {
+		tc := tc
+		t.Run(fmt.Sprintf("iptables=%v,insert=%v", tc.iptables, tc.insert), func(t *testing.T) {
+			c := nc.(*controller)
+			c.cfg.Daemon.DriverCfg["bridge"] = map[string]interface{}{
+				netlabel.GenericData: options.Generic{
+					"EnableIPTables": tc.iptables,
+				},
+			}
+
+			// init. condition, FORWARD chain empty DOCKER-USER not exist
+			assert.DeepEqual(t, getRules(t, fwdChainName), []string{"-P FORWARD ACCEPT"})
+
+			if tc.insert {
+				_, err = iptables.Raw("-A", fwdChainName, "-j", "DROP")
+				assert.NilError(t, err)
+			}
+			setupArrangeUserFilterRule(c)
+			arrangeUserFilterRule()
+
+			assert.DeepEqual(t, getRules(t, fwdChainName), tc.fwdChain)
+			if tc.userChain != nil {
+				assert.DeepEqual(t, getRules(t, usrChainName), tc.userChain)
+			} else {
+				_, err := iptables.Raw("-S", usrChainName)
+				assert.Assert(t, err != nil, "chain %v: created unexpectedly", usrChainName)
+			}
+		})
+		resetIptables(t)
+	}
+}
+
+func getRules(t *testing.T, chain string) []string {
+	t.Helper()
+	output, err := iptables.Raw("-S", chain)
+	assert.NilError(t, err, "chain %s: failed to get rules", chain)
+
+	rules := strings.Split(string(output), "\n")
+	if len(rules) > 0 {
+		rules = rules[:len(rules)-1]
+	}
+	return rules
+}
+
+func resetIptables(t *testing.T) {
+	t.Helper()
+	_, err := iptables.Raw("-F", fwdChainName)
+	assert.NilError(t, err)
+	_ = iptables.RemoveExistingChain(usrChainName, "")
+}