From 17723691e585581f4a90ac9b98e55d9a0785b52d Mon Sep 17 00:00:00 2001 From: Bjorn Neergaard Date: Sat, 14 Jan 2023 15:27:52 -0700 Subject: [PATCH] Revert "libnetwork: Support IPv6 in arrangeUserFilterRule()" This reverts commit 2d397beb0068ebf42f7196e243f33842544693d5. moby#44706 and moby#44805 were both merged, and both refactored the same file. The combination broke the build, and was not detected in CI as only the combination of the two, applied to the same parent commit, caused the failure. moby#44706 should be carried forward, based on the current master, in order to resolve this conflict. Signed-off-by: Bjorn Neergaard --- libnetwork/controller.go | 20 -------------- libnetwork/firewall_linux.go | 44 ++++++++++--------------------- libnetwork/firewall_linux_test.go | 42 +++++++++++------------------ 3 files changed, 29 insertions(+), 77 deletions(-) diff --git a/libnetwork/controller.go b/libnetwork/controller.go index 87315c1291..a01e4fa3d1 100644 --- a/libnetwork/controller.go +++ b/libnetwork/controller.go @@ -1247,23 +1247,3 @@ func (c *Controller) iptablesEnabled() bool { } return enabled } - -func (c *controller) ip6tablesEnabled() bool { - c.Lock() - defer c.Unlock() - - if c.cfg == nil { - return false - } - // parse map cfg["bridge"]["generic"]["EnableIP6Table"] - cfgBridge, ok := c.cfg.DriverCfg["bridge"].(map[string]interface{}) - if !ok { - return false - } - cfgGeneric, ok := cfgBridge[netlabel.GenericData].(options.Generic) - if !ok { - return false - } - enabled, _ := cfgGeneric["EnableIP6Tables"].(bool) - return enabled -} diff --git a/libnetwork/firewall_linux.go b/libnetwork/firewall_linux.go index 4eef752d19..0379744c11 100644 --- a/libnetwork/firewall_linux.go +++ b/libnetwork/firewall_linux.go @@ -21,40 +21,24 @@ func setupArrangeUserFilterRule(c *Controller) { // IPTableForwarding is disabled, because it contains rules configured by user that // are beyond docker engine's control. func arrangeUserFilterRule() { - if ctrl == nil { + if ctrl == nil || !ctrl.iptablesEnabled() { + return + } + // TODO IPv6 support + iptable := iptables.GetIptable(iptables.IPv4) + _, err := iptable.NewChain(userChain, iptables.Filter, false) + if err != nil { + logrus.Warnf("Failed to create %s chain: %v", userChain, err) return } - conds := []struct { - ipVer iptables.IPVersion - cond bool - }{ - {ipVer: iptables.IPv4, cond: ctrl.iptablesEnabled()}, - {ipVer: iptables.IPv6, cond: ctrl.ip6tablesEnabled()}, + if err = iptable.AddReturnRule(userChain); err != nil { + logrus.Warnf("Failed to add the RETURN rule for %s: %v", userChain, err) + return } - for _, ipVerCond := range conds { - cond := ipVerCond.cond - if !cond { - continue - } - - ipVer := ipVerCond.ipVer - iptable := iptables.GetIptable(ipVer) - _, err := iptable.NewChain(userChain, iptables.Filter, false) - if err != nil { - logrus.WithError(err).Warnf("Failed to create %s %v chain", userChain, ipVer) - return - } - - if err = iptable.AddReturnRule(userChain); err != nil { - logrus.WithError(err).Warnf("Failed to add the RETURN rule for %s %v", userChain, ipVer) - return - } - - err = iptable.EnsureJumpRule("FORWARD", userChain) - if err != nil { - logrus.WithError(err).Warnf("Failed to ensure the jump rule for %s %v", userChain, ipVer) - } + err = iptable.EnsureJumpRule("FORWARD", userChain) + if err != nil { + logrus.Warnf("Failed to ensure the jump rule for %s: %v", userChain, err) } } diff --git a/libnetwork/firewall_linux_test.go b/libnetwork/firewall_linux_test.go index 091a91c6b7..226bf4dfa5 100644 --- a/libnetwork/firewall_linux_test.go +++ b/libnetwork/firewall_linux_test.go @@ -18,8 +18,7 @@ const ( ) func TestUserChain(t *testing.T) { - iptable4 := iptables.GetIptable(iptables.IPv4) - iptable6 := iptables.GetIptable(iptables.IPv6) + iptable := iptables.GetIptable(iptables.IPv4) tests := []struct { iptables bool @@ -57,40 +56,32 @@ func TestUserChain(t *testing.T) { defer c.Stop() c.cfg.DriverCfg["bridge"] = map[string]interface{}{ netlabel.GenericData: options.Generic{ - "EnableIPTables": tc.iptables, - "EnableIP6Tables": tc.iptables, + "EnableIPTables": tc.iptables, }, } // init. condition, FORWARD chain empty DOCKER-USER not exist - assert.DeepEqual(t, getRules(t, iptables.IPv4, fwdChainName), []string{"-P FORWARD ACCEPT"}) - assert.DeepEqual(t, getRules(t, iptables.IPv6, fwdChainName), []string{"-P FORWARD ACCEPT"}) + assert.DeepEqual(t, getRules(t, fwdChainName), []string{"-P FORWARD ACCEPT"}) if tc.insert { - _, err = iptable4.Raw("-A", fwdChainName, "-j", "DROP") - assert.NilError(t, err) - _, err = iptable6.Raw("-A", fwdChainName, "-j", "DROP") + _, err = iptable.Raw("-A", fwdChainName, "-j", "DROP") assert.NilError(t, err) } arrangeUserFilterRule() - assert.DeepEqual(t, getRules(t, iptables.IPv4, fwdChainName), tc.fwdChain) - assert.DeepEqual(t, getRules(t, iptables.IPv6, fwdChainName), tc.fwdChain) + assert.DeepEqual(t, getRules(t, fwdChainName), tc.fwdChain) if tc.userChain != nil { - assert.DeepEqual(t, getRules(t, iptables.IPv4, usrChainName), tc.userChain) - assert.DeepEqual(t, getRules(t, iptables.IPv6, usrChainName), tc.userChain) + assert.DeepEqual(t, getRules(t, usrChainName), tc.userChain) } else { - _, err := iptable4.Raw("-S", usrChainName) - assert.Assert(t, err != nil, "ipv4 chain %v: created unexpectedly", usrChainName) - _, err = iptable6.Raw("-S", usrChainName) - assert.Assert(t, err != nil, "ipv6 chain %v: created unexpectedly", usrChainName) + _, err := iptable.Raw("-S", usrChainName) + assert.Assert(t, err != nil, "chain %v: created unexpectedly", usrChainName) } }) } } -func getRules(t *testing.T, ipVer iptables.IPVersion, chain string) []string { - iptable := iptables.GetIptable(ipVer) +func getRules(t *testing.T, chain string) []string { + iptable := iptables.GetIptable(iptables.IPv4) t.Helper() output, err := iptable.Raw("-S", chain) @@ -104,13 +95,10 @@ func getRules(t *testing.T, ipVer iptables.IPVersion, chain string) []string { } func resetIptables(t *testing.T) { + iptable := iptables.GetIptable(iptables.IPv4) + t.Helper() - - for _, ipVer := range []iptables.IPVersion{iptables.IPv4, iptables.IPv6} { - iptable := iptables.GetIptable(ipVer) - - _, err := iptable.Raw("-F", fwdChainName) - assert.Check(t, err) - _ = iptable.RemoveExistingChain(usrChainName, "") - } + _, err := iptable.Raw("-F", fwdChainName) + assert.NilError(t, err) + _ = iptable.RemoveExistingChain(usrChainName, "") }