From 7f9b1844b855d6e25d0df00bf01ce8885a869fc1 Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Fri, 24 May 2019 23:14:31 -0400 Subject: [PATCH 1/2] bridge: add test for ipMasq rules --- plugins/main/bridge/bridge_test.go | 55 ++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 97a93645..e243664b 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -17,12 +17,14 @@ package main import ( "encoding/json" "fmt" - "github.com/vishvananda/netlink/nl" "io/ioutil" "net" "os" "strings" + "github.com/coreos/go-iptables/iptables" + "github.com/vishvananda/netlink/nl" + "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/types/020" @@ -70,6 +72,7 @@ type testCase struct { isLayer2 bool expGWCIDRs []string // Expected gateway addresses in CIDR form vlan int + ipMasq bool } // Range definition for each entry in the ranges list @@ -105,8 +108,7 @@ const ( "vlan": %d` netDefault = `, - "isDefaultGateway": true, - "ipMasq": false` + "isDefaultGateway": true` ipamStartStr = `, "ipam": { @@ -115,6 +117,9 @@ const ( ipamDataDirStr = `, "dataDir": "%s"` + ipMasqConfStr = `, + "ipMasq": %t` + // Single subnet configuration (legacy) subnetConfStr = `, "subnet": "%s"` @@ -147,6 +152,9 @@ func (tc testCase) netConfJSON(dataDir string) string { if tc.vlan != 0 { conf += fmt.Sprintf(vlan, tc.vlan) } + if tc.ipMasq { + conf += tc.ipMasqConfig() + } if !tc.isLayer2 { conf += netDefault @@ -178,6 +186,11 @@ func (tc testCase) subnetConfig() string { return conf } +func (tc testCase) ipMasqConfig() string { + conf := fmt.Sprintf(ipMasqConfStr, tc.ipMasq) + return conf +} + func (tc testCase) rangesConfig() string { conf := rangesStartStr for i, tcRange := range tc.ranges { @@ -1595,4 +1608,40 @@ var _ = Describe("bridge Operations", func() { cmdAddDelCheckTest(originalNS, tc, dataDir) } }) + + FIt("configures a bridge and ipMasq rules for 0.4.0 config", func() { + err := originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + tc := testCase{ + ranges: []rangeInfo{{ + subnet: "10.1.2.0/24", + }}, + ipMasq: true, + cniVersion: "0.4.0", + } + + args := tc.createCmdArgs(originalNS, dataDir) + r, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + result, err := current.GetResult(r) + Expect(err).NotTo(HaveOccurred()) + Expect(result.IPs).Should(HaveLen(1)) + + ipt, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) + Expect(err).NotTo(HaveOccurred()) + + rules, err := ipt.List("nat", "POSTROUTING") + Expect(err).NotTo(HaveOccurred()) + Expect(rules).Should(ContainElement(ContainSubstring(result.IPs[0].Address.IP.String()))) + + err = testutils.CmdDelWithArgs(args, func() error { + return cmdDel(args) + }) + Expect(err).NotTo(HaveOccurred()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) }) From 37d2ee1d5d2b0e7b4da18d7c3e49f9250bbe5ccb Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Sat, 25 May 2019 20:55:47 -0400 Subject: [PATCH 2/2] bridge: don't use canonical CIDR address when setting up IP masquerade --- plugins/main/bridge/bridge.go | 2 +- plugins/main/bridge/bridge_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index 675ffdd2..fb78a5f8 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -517,7 +517,7 @@ func cmdAdd(args *skel.CmdArgs) error { chain := utils.FormatChainName(n.Name, args.ContainerID) comment := utils.FormatComment(n.Name, args.ContainerID) for _, ipc := range result.IPs { - if err = ip.SetupIPMasq(ip.Network(&ipc.Address), chain, comment); err != nil { + if err = ip.SetupIPMasq(&ipc.Address, chain, comment); err != nil { return err } } diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index e243664b..89a05b93 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -1609,7 +1609,7 @@ var _ = Describe("bridge Operations", func() { } }) - FIt("configures a bridge and ipMasq rules for 0.4.0 config", func() { + It("configures a bridge and ipMasq rules for 0.4.0 config", func() { err := originalNS.Do(func(ns.NetNS) error { defer GinkgoRecover() tc := testCase{