diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index 83468d9ce..797409e11 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -22,6 +22,8 @@ import ( "runtime" "syscall" + "io/ioutil" + "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/types/current" @@ -46,6 +48,12 @@ type NetConf struct { HairpinMode bool `json:"hairpinMode"` } +type gwInfo struct { + gws []net.IPNet + family int + defaultRouteFound bool +} + func init() { // this ensures that main runs only on main thread (thread group leader). // since namespace ops (unshare, setns) are done for a single thread, we @@ -63,28 +71,98 @@ func loadNetConf(bytes []byte) (*NetConf, string, error) { return n, n.CNIVersion, nil } -func ensureBridgeAddr(br *netlink.Bridge, ipn *net.IPNet, forceAddress bool) error { - addrs, err := netlink.AddrList(br, syscall.AF_INET) +// calcGateways processes the results from the IPAM plugin and does the +// following for each IP family: +// - Calculates and compiles a list of gateway addresses +// - Adds a default route if needed +func calcGateways(result *current.Result, n *NetConf) (*gwInfo, *gwInfo, error) { + + gwsV4 := &gwInfo{} + gwsV6 := &gwInfo{} + + for _, ipc := range result.IPs { + + // Determine if this config is IPv4 or IPv6 + var gws *gwInfo + defaultNet := &net.IPNet{} + switch { + case ipc.Address.IP.To4() != nil: + gws = gwsV4 + gws.family = netlink.FAMILY_V4 + defaultNet.IP = net.IPv4zero + case len(ipc.Address.IP) == net.IPv6len: + gws = gwsV6 + gws.family = netlink.FAMILY_V6 + defaultNet.IP = net.IPv6zero + default: + return nil, nil, fmt.Errorf("Unknown IP object: %v", ipc) + } + defaultNet.Mask = net.IPMask(defaultNet.IP) + + // All IPs currently refer to the container interface + ipc.Interface = 2 + + // If not provided, calculate the gateway address corresponding + // to the selected IP address + if ipc.Gateway == nil && n.IsGW { + ipc.Gateway = calcGatewayIP(&ipc.Address) + } + + // Add a default route for this family using the current + // gateway address if necessary. + if n.IsDefaultGW && !gws.defaultRouteFound { + for _, route := range result.Routes { + if route.GW != nil && defaultNet.String() == route.Dst.String() { + gws.defaultRouteFound = true + break + } + } + if !gws.defaultRouteFound { + result.Routes = append( + result.Routes, + &types.Route{Dst: *defaultNet, GW: ipc.Gateway}, + ) + gws.defaultRouteFound = true + } + } + + // Append this gateway address to the list of gateways + if n.IsGW { + gw := net.IPNet{ + IP: ipc.Gateway, + Mask: ipc.Address.Mask, + } + gws.gws = append(gws.gws, gw) + } + } + return gwsV4, gwsV6, nil +} + +func ensureBridgeAddr(br *netlink.Bridge, family int, ipn *net.IPNet, forceAddress bool) error { + addrs, err := netlink.AddrList(br, family) if err != nil && err != syscall.ENOENT { return fmt.Errorf("could not get list of IP addresses: %v", err) } - // if there're no addresses on the bridge, it's ok -- we'll add one - if len(addrs) > 0 { - ipnStr := ipn.String() - for _, a := range addrs { - // string comp is actually easiest for doing IPNet comps - if a.IPNet.String() == ipnStr { - return nil - } + ipnStr := ipn.String() + for _, a := range addrs { - // If forceAddress is set to true then reconfigure IP address otherwise throw error + // string comp is actually easiest for doing IPNet comps + if a.IPNet.String() == ipnStr { + return nil + } + + // Multiple IPv6 addresses are allowed on the bridge if the + // corresponding subnets do not overlap. For IPv4 or for + // overlapping IPv6 subnets, reconfigure the IP address if + // forceAddress is true, otherwise throw an error. + if family == netlink.FAMILY_V4 || a.IPNet.Contains(ipn.IP) || ipn.Contains(a.IPNet.IP) { if forceAddress { if err = deleteBridgeAddr(br, a.IPNet); err != nil { return err } } else { - return fmt.Errorf("%q already has an IP address different from %v", br.Name, ipn.String()) + return fmt.Errorf("%q already has an IP address different from %v", br.Name, ipnStr) } } } @@ -99,18 +177,10 @@ func ensureBridgeAddr(br *netlink.Bridge, ipn *net.IPNet, forceAddress bool) err func deleteBridgeAddr(br *netlink.Bridge, ipn *net.IPNet) error { addr := &netlink.Addr{IPNet: ipn, Label: ""} - if err := netlink.LinkSetDown(br); err != nil { - return fmt.Errorf("could not set down bridge %q: %v", br.Name, err) - } - if err := netlink.AddrDel(br, addr); err != nil { return fmt.Errorf("could not remove IP address from %q: %v", br.Name, err) } - if err := netlink.LinkSetUp(br); err != nil { - return fmt.Errorf("could not set up bridge %q: %v", br.Name, err) - } - return nil } @@ -216,6 +286,20 @@ func setupBridge(n *NetConf) (*netlink.Bridge, *current.Interface, error) { }, nil } +// disableIPV6DAD disables IPv6 Duplicate Address Detection (DAD) +// for an interface. +func disableIPV6DAD(ifName string) error { + f := fmt.Sprintf("/proc/sys/net/ipv6/conf/%s/accept_dad", ifName) + return ioutil.WriteFile(f, []byte("0"), 0644) +} + +func enableIPForward(family int) error { + if family == netlink.FAMILY_V4 { + return ip.EnableIP4Forward() + } + return ip.EnableIP6Forward() +} + func cmdAdd(args *skel.CmdArgs) error { n, cniVersion, err := loadNetConf(args.StdinData) if err != nil { @@ -260,54 +344,31 @@ func cmdAdd(args *skel.CmdArgs) error { result.Interfaces = []*current.Interface{brInterface, hostInterface, containerInterface} - for _, ipc := range result.IPs { - // All IPs currently refer to the container interface - ipc.Interface = 2 - if ipc.Gateway == nil && n.IsGW { - ipc.Gateway = calcGatewayIP(&ipc.Address) - } + // Gather gateway information for each IP family + gwsV4, gwsV6, err := calcGateways(result, n) + if err != nil { + return err } + // Configure the container hardware address and IP address(es) if err := netns.Do(func(_ ns.NetNS) error { - // set the default gateway if requested - if n.IsDefaultGW { - for _, ipc := range result.IPs { - defaultNet := &net.IPNet{} - switch { - case ipc.Address.IP.To4() != nil: - defaultNet.IP = net.IPv4zero - defaultNet.Mask = net.IPMask(net.IPv4zero) - case len(ipc.Address.IP) == net.IPv6len && ipc.Address.IP.To4() == nil: - defaultNet.IP = net.IPv6zero - defaultNet.Mask = net.IPMask(net.IPv6zero) - default: - return fmt.Errorf("Unknown IP object: %v", ipc) - } - - for _, route := range result.Routes { - if defaultNet.String() == route.Dst.String() { - if route.GW != nil && !route.GW.Equal(ipc.Gateway) { - return fmt.Errorf( - "isDefaultGateway ineffective because IPAM sets default route via %q", - route.GW, - ) - } - } - } - - result.Routes = append( - result.Routes, - &types.Route{Dst: *defaultNet, GW: ipc.Gateway}, - ) - } + // Disable IPv6 DAD just in case hairpin mode is enabled on the + // bridge. Hairpin mode causes echos of neighbor solicitation + // packets, which causes DAD failures. + // TODO: (short term) Disable DAD conditional on actual hairpin mode + // TODO: (long term) Use enhanced DAD when that becomes available in kernels. + if err := disableIPV6DAD(args.IfName); err != nil { + return err } if err := ipam.ConfigureIface(args.IfName, result); err != nil { return err } - if err := ip.SetHWAddrByIP(args.IfName, result.IPs[0].Address.IP, nil /* TODO IPv6 */); err != nil { - return err + if result.IPs[0].Address.IP.To4() != nil { + if err := ip.SetHWAddrByIP(args.IfName, result.IPs[0].Address.IP, nil /* TODO IPv6 */); err != nil { + return err + } } // Refetch the veth since its MAC address may changed @@ -324,17 +385,23 @@ func cmdAdd(args *skel.CmdArgs) error { if n.IsGW { var firstV4Addr net.IP - for _, ipc := range result.IPs { - gwn := &net.IPNet{ - IP: ipc.Gateway, - Mask: ipc.Address.Mask, - } - if ipc.Gateway.To4() != nil && firstV4Addr == nil { - firstV4Addr = ipc.Gateway + // Set the IP address(es) on the bridge and enable forwarding + for _, gws := range []*gwInfo{gwsV4, gwsV6} { + for _, gw := range gws.gws { + if gw.IP.To4() != nil && firstV4Addr == nil { + firstV4Addr = gw.IP + } + + err = ensureBridgeAddr(br, gws.family, &gw, n.ForceAddress) + if err != nil { + return fmt.Errorf("failed to set bridge addr: %v", err) + } } - if err = ensureBridgeAddr(br, gwn, n.ForceAddress); err != nil { - return err + if gws.gws != nil { + if err = enableIPForward(gws.family); err != nil { + return fmt.Errorf("failed to enable forwarding: %v", err) + } } } @@ -343,10 +410,6 @@ func cmdAdd(args *skel.CmdArgs) error { return err } } - - if err := ip.EnableIP4Forward(); err != nil { - return fmt.Errorf("failed to enable forwarding: %v", err) - } } if n.IPMasq { @@ -392,7 +455,7 @@ func cmdDel(args *skel.CmdArgs) error { var ipn *net.IPNet err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { var err error - ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4) + ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_ALL) if err != nil && err == ip.ErrLinkNotFound { return nil } diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 8fe56fc00..f97111970 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -18,7 +18,6 @@ import ( "fmt" "net" "strings" - "syscall" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -34,43 +33,222 @@ import ( . "github.com/onsi/gomega" ) -func checkBridgeConfig03x(version string, originalNS ns.NetNS) { - const BRNAME = "cni0" - const IFNAME = "eth0" +const ( + BRNAME = "bridge0" + IFNAME = "eth0" +) - gwaddr, subnet, err := net.ParseCIDR("10.1.2.1/24") - Expect(err).NotTo(HaveOccurred()) +// testCase defines the CNI network configuration and the expected +// bridge addresses for a test case. +type testCase struct { + cniVersion string // CNI Version + subnet string // Single subnet config: Subnet CIDR + gateway string // Single subnet config: Gateway + ranges []rangeInfo // Ranges list (multiple subnets config) + isGW bool + expGWCIDRs []string // Expected gateway addresses in CIDR form +} - conf := fmt.Sprintf(`{ - "cniVersion": "%s", - "name": "mynet", - "type": "bridge", - "bridge": "%s", - "isDefaultGateway": true, - "ipMasq": false, +// Range definition for each entry in the ranges list +type rangeInfo struct { + subnet string + gateway string +} + +// netConf() creates a NetConf structure for a test case. +func (tc testCase) netConf() *NetConf { + return &NetConf{ + NetConf: types.NetConf{ + CNIVersion: tc.cniVersion, + Name: "testConfig", + Type: "bridge", + }, + BrName: BRNAME, + IsGW: tc.isGW, + IPMasq: false, + MTU: 5000, + } +} + +// Snippets for generating a JSON network configuration string. +const ( + netConfStr = ` + "cniVersion": "%s", + "name": "testConfig", + "type": "bridge", + "bridge": "%s", + "isDefaultGateway": true, + "ipMasq": false` + + ipamStartStr = `, "ipam": { - "type": "host-local", - "subnet": "%s" - } -}`, version, BRNAME, subnet.String()) + "type": "host-local"` + + // Single subnet configuration (legacy) + subnetConfStr = `, + "subnet": "%s"` + gatewayConfStr = `, + "gateway": "%s"` + + // Ranges (multiple subnets) configuration + rangesStartStr = `, + "ranges": [` + rangeSubnetConfStr = ` + { + "subnet": "%s" + }` + rangeSubnetGWConfStr = ` + { + "subnet": "%s", + "gateway": "%s" + }` + rangesEndStr = ` + ]` + + ipamEndStr = ` + }` +) - targetNs, err := ns.NewNS() - Expect(err).NotTo(HaveOccurred()) - defer targetNs.Close() +// netConfJSON() generates a JSON network configuration string +// for a test case. +func (tc testCase) netConfJSON() string { + conf := fmt.Sprintf(netConfStr, tc.cniVersion, BRNAME) + if tc.subnet != "" || tc.ranges != nil { + conf += ipamStartStr + if tc.subnet != "" { + conf += tc.subnetConfig() + } + if tc.ranges != nil { + conf += tc.rangesConfig() + } + conf += ipamEndStr + } + return "{" + conf + "\n}" +} + +func (tc testCase) subnetConfig() string { + conf := fmt.Sprintf(subnetConfStr, tc.subnet) + if tc.gateway != "" { + conf += fmt.Sprintf(gatewayConfStr, tc.gateway) + } + return conf +} - args := &skel.CmdArgs{ +func (tc testCase) rangesConfig() string { + conf := rangesStartStr + for i, tcRange := range tc.ranges { + if i > 0 { + conf += "," + } + if tcRange.gateway != "" { + conf += fmt.Sprintf(rangeSubnetGWConfStr, tcRange.subnet, tcRange.gateway) + } else { + conf += fmt.Sprintf(rangeSubnetConfStr, tcRange.subnet) + } + } + return conf + rangesEndStr +} + +// createCmdArgs generates network configuration and creates command +// arguments for a test case. +func (tc testCase) createCmdArgs(targetNS ns.NetNS) *skel.CmdArgs { + conf := tc.netConfJSON() + return &skel.CmdArgs{ ContainerID: "dummy", - Netns: targetNs.Path(), + Netns: targetNS.Path(), IfName: IFNAME, StdinData: []byte(conf), } +} + +// expectedCIDRs determines the IPv4 and IPv6 CIDRs in which the resulting +// addresses are expected to be contained. +func (tc testCase) expectedCIDRs() ([]*net.IPNet, []*net.IPNet) { + var cidrsV4, cidrsV6 []*net.IPNet + appendSubnet := func(subnet string) { + ip, cidr, err := net.ParseCIDR(subnet) + Expect(err).NotTo(HaveOccurred()) + if ipVersion(ip) == "4" { + cidrsV4 = append(cidrsV4, cidr) + } else { + cidrsV6 = append(cidrsV6, cidr) + } + } + if tc.subnet != "" { + appendSubnet(tc.subnet) + } + for _, r := range tc.ranges { + appendSubnet(r.subnet) + } + return cidrsV4, cidrsV6 +} + +// delBridgeAddrs() deletes addresses from the bridge +func delBridgeAddrs(testNS ns.NetNS) { + err := testNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + br, err := netlink.LinkByName(BRNAME) + Expect(err).NotTo(HaveOccurred()) + addrs, err := netlink.AddrList(br, netlink.FAMILY_ALL) + Expect(err).NotTo(HaveOccurred()) + for _, addr := range addrs { + if !addr.IP.IsLinkLocalUnicast() { + err = netlink.AddrDel(br, &addr) + Expect(err).NotTo(HaveOccurred()) + } + } + return nil + }) + Expect(err).NotTo(HaveOccurred()) +} + +func ipVersion(ip net.IP) string { + if ip.To4() != nil { + return "4" + } + return "6" +} + +type cmdAddDelTester interface { + setNS(testNS ns.NetNS, targetNS ns.NetNS) + cmdAddTest(tc testCase) + cmdDelTest(tc testCase) +} + +func testerByVersion(version string) cmdAddDelTester { + switch { + case strings.HasPrefix(version, "0.3."): + return &testerV03x{} + default: + return &testerV01xOr02x{} + } +} + +type testerV03x struct { + testNS ns.NetNS + targetNS ns.NetNS + args *skel.CmdArgs + vethName string +} + +func (tester *testerV03x) setNS(testNS ns.NetNS, targetNS ns.NetNS) { + tester.testNS = testNS + tester.targetNS = targetNS +} + +func (tester *testerV03x) cmdAddTest(tc testCase) { + // Generate network config and command arguments + tester.args = tc.createCmdArgs(tester.targetNS) + + // Execute cmdADD on the plugin var result *current.Result - err = originalNS.Do(func(ns.NetNS) error { + err := tester.testNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - r, raw, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error { - return cmdAdd(args) + r, raw, err := testutils.CmdAddWithResult(tester.targetNS.Path(), IFNAME, tester.args.StdinData, func() error { + return cmdAdd(tester.args) }) Expect(err).NotTo(HaveOccurred()) Expect(strings.Index(string(raw), "\"interfaces\":")).Should(BeNumerically(">", 0)) @@ -88,23 +266,30 @@ func checkBridgeConfig03x(version string, originalNS ns.NetNS) { Expect(link.Attrs().Name).To(Equal(BRNAME)) Expect(link).To(BeAssignableToTypeOf(&netlink.Bridge{})) Expect(link.Attrs().HardwareAddr.String()).To(Equal(result.Interfaces[0].Mac)) - hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) - Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) - // Ensure bridge has gateway address - addrs, err := netlink.AddrList(link, syscall.AF_INET) + // Ensure bridge has expected gateway address(es) + addrs, err := netlink.AddrList(link, netlink.FAMILY_ALL) Expect(err).NotTo(HaveOccurred()) Expect(len(addrs)).To(BeNumerically(">", 0)) - found := false - subnetPrefix, subnetBits := subnet.Mask.Size() - for _, a := range addrs { - aPrefix, aBits := a.IPNet.Mask.Size() - if a.IPNet.IP.Equal(gwaddr) && aPrefix == subnetPrefix && aBits == subnetBits { - found = true - break + for _, cidr := range tc.expGWCIDRs { + ip, subnet, err := net.ParseCIDR(cidr) + Expect(err).NotTo(HaveOccurred()) + if ip.To4() != nil { + hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) + Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) + } + + found := false + subnetPrefix, subnetBits := subnet.Mask.Size() + for _, a := range addrs { + aPrefix, aBits := a.IPNet.Mask.Size() + if a.IPNet.IP.Equal(ip) && aPrefix == subnetPrefix && aBits == subnetBits { + found = true + break + } } + Expect(found).To(Equal(true)) } - Expect(found).To(Equal(true)) // Check for the veth link in the main namespace links, err := netlink.LinkList() @@ -114,12 +299,13 @@ func checkBridgeConfig03x(version string, originalNS ns.NetNS) { link, err = netlink.LinkByName(result.Interfaces[1].Name) Expect(err).NotTo(HaveOccurred()) Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{})) + tester.vethName = result.Interfaces[1].Name return nil }) Expect(err).NotTo(HaveOccurred()) // Find the veth peer in the container namespace and the default route - err = targetNs.Do(func(ns.NetNS) error { + err = tester.targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(IFNAME) @@ -127,35 +313,63 @@ func checkBridgeConfig03x(version string, originalNS ns.NetNS) { Expect(link.Attrs().Name).To(Equal(IFNAME)) Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{})) - addrs, err := netlink.AddrList(link, syscall.AF_INET) + expCIDRsV4, expCIDRsV6 := tc.expectedCIDRs() + if expCIDRsV4 != nil { + hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) + Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) + } + addrs, err := netlink.AddrList(link, netlink.FAMILY_V4) Expect(err).NotTo(HaveOccurred()) - Expect(len(addrs)).To(Equal(1)) - - hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) - Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) + Expect(len(addrs)).To(Equal(len(expCIDRsV4))) + addrs, err = netlink.AddrList(link, netlink.FAMILY_V6) + Expect(err).NotTo(HaveOccurred()) + // Ignore link local address which may or may not be + // ready when we read addresses. + var foundAddrs int + for _, addr := range addrs { + if !addr.IP.IsLinkLocalUnicast() { + foundAddrs++ + } + } + Expect(foundAddrs).To(Equal(len(expCIDRsV6))) - // Ensure the default route + // Ensure the default route(s) routes, err := netlink.RouteList(link, 0) Expect(err).NotTo(HaveOccurred()) - var defaultRouteFound bool - for _, route := range routes { - defaultRouteFound = (route.Dst == nil && route.Src == nil && route.Gw.Equal(gwaddr)) - if defaultRouteFound { - break + var defaultRouteFound4, defaultRouteFound6 bool + for _, cidr := range tc.expGWCIDRs { + gwIP, _, err := net.ParseCIDR(cidr) + Expect(err).NotTo(HaveOccurred()) + var found *bool + if ipVersion(gwIP) == "4" { + found = &defaultRouteFound4 + } else { + found = &defaultRouteFound6 + } + if *found == true { + continue + } + for _, route := range routes { + *found = (route.Dst == nil && route.Src == nil && route.Gw.Equal(gwIP)) + if *found { + break + } } + Expect(*found).To(Equal(true)) } - Expect(defaultRouteFound).To(Equal(true)) return nil }) Expect(err).NotTo(HaveOccurred()) +} - err = originalNS.Do(func(ns.NetNS) error { +func (tester *testerV03x) cmdDelTest(tc testCase) { + err := tester.testNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - err := testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error { - return cmdDel(args) + err := testutils.CmdDelWithResult(tester.targetNS.Path(), IFNAME, func() error { + return cmdDel(tester.args) }) Expect(err).NotTo(HaveOccurred()) return nil @@ -163,7 +377,7 @@ func checkBridgeConfig03x(version string, originalNS ns.NetNS) { Expect(err).NotTo(HaveOccurred()) // Make sure the host veth has been deleted - err = targetNs.Do(func(ns.NetNS) error { + err = tester.targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(IFNAME) @@ -174,16 +388,174 @@ func checkBridgeConfig03x(version string, originalNS ns.NetNS) { Expect(err).NotTo(HaveOccurred()) // Make sure the container veth has been deleted - err = originalNS.Do(func(ns.NetNS) error { + err = tester.testNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - link, err := netlink.LinkByName(result.Interfaces[1].Name) + link, err := netlink.LinkByName(tester.vethName) Expect(err).To(HaveOccurred()) Expect(link).To(BeNil()) return nil }) } +type testerV01xOr02x struct { + testNS ns.NetNS + targetNS ns.NetNS + args *skel.CmdArgs + vethName string +} + +func (tester *testerV01xOr02x) setNS(testNS ns.NetNS, targetNS ns.NetNS) { + tester.testNS = testNS + tester.targetNS = targetNS +} + +func (tester *testerV01xOr02x) cmdAddTest(tc testCase) { + // Generate network config and calculate gateway addresses + tester.args = tc.createCmdArgs(tester.targetNS) + + // Execute cmdADD on the plugin + var result *types020.Result + err := tester.testNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + r, raw, err := testutils.CmdAddWithResult(tester.targetNS.Path(), IFNAME, tester.args.StdinData, func() error { + return cmdAdd(tester.args) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(strings.Index(string(raw), "\"ip\":")).Should(BeNumerically(">", 0)) + + // We expect a version 0.1.0 result + result, err = types020.GetResult(r) + Expect(err).NotTo(HaveOccurred()) + + // Make sure bridge link exists + link, err := netlink.LinkByName(BRNAME) + Expect(err).NotTo(HaveOccurred()) + Expect(link.Attrs().Name).To(Equal(BRNAME)) + Expect(link).To(BeAssignableToTypeOf(&netlink.Bridge{})) + + // Ensure bridge has expected gateway address(es) + addrs, err := netlink.AddrList(link, netlink.FAMILY_ALL) + Expect(err).NotTo(HaveOccurred()) + Expect(len(addrs)).To(BeNumerically(">", 0)) + for _, cidr := range tc.expGWCIDRs { + ip, subnet, err := net.ParseCIDR(cidr) + Expect(err).NotTo(HaveOccurred()) + if ip.To4() != nil { + hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) + Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) + } + + found := false + subnetPrefix, subnetBits := subnet.Mask.Size() + for _, a := range addrs { + aPrefix, aBits := a.IPNet.Mask.Size() + if a.IPNet.IP.Equal(ip) && aPrefix == subnetPrefix && aBits == subnetBits { + found = true + break + } + } + Expect(found).To(Equal(true)) + } + + // Check for the veth link in the main namespace; can't + // check the for the specific link since version 0.1.0 + // doesn't report interfaces + links, err := netlink.LinkList() + Expect(err).NotTo(HaveOccurred()) + Expect(len(links)).To(Equal(3)) // Bridge, veth, and loopback + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + // Find the veth peer in the container namespace and the default route + err = tester.targetNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + link, err := netlink.LinkByName(IFNAME) + Expect(err).NotTo(HaveOccurred()) + Expect(link.Attrs().Name).To(Equal(IFNAME)) + Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{})) + + expCIDRsV4, expCIDRsV6 := tc.expectedCIDRs() + if expCIDRsV4 != nil { + hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) + Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) + } + addrs, err := netlink.AddrList(link, netlink.FAMILY_V4) + Expect(err).NotTo(HaveOccurred()) + Expect(len(addrs)).To(Equal(len(expCIDRsV4))) + addrs, err = netlink.AddrList(link, netlink.FAMILY_V6) + Expect(err).NotTo(HaveOccurred()) + Expect(len(addrs)).To(Equal(len(expCIDRsV6) + 1)) // Link local address is automatic + + // Ensure the default route + routes, err := netlink.RouteList(link, 0) + Expect(err).NotTo(HaveOccurred()) + + var defaultRouteFound bool + for _, cidr := range tc.expGWCIDRs { + gwIP, _, err := net.ParseCIDR(cidr) + Expect(err).NotTo(HaveOccurred()) + for _, route := range routes { + defaultRouteFound = (route.Dst == nil && route.Src == nil && route.Gw.Equal(gwIP)) + if defaultRouteFound { + break + } + } + Expect(defaultRouteFound).To(Equal(true)) + } + + return nil + }) + Expect(err).NotTo(HaveOccurred()) +} + +func (tester *testerV01xOr02x) cmdDelTest(tc testCase) { + err := tester.testNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + err := testutils.CmdDelWithResult(tester.targetNS.Path(), IFNAME, func() error { + return cmdDel(tester.args) + }) + Expect(err).NotTo(HaveOccurred()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + // Make sure the container veth has been deleted; cannot check + // host veth as version 0.1.0 can't report its name + err = tester.testNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + link, err := netlink.LinkByName(IFNAME) + Expect(err).To(HaveOccurred()) + Expect(link).To(BeNil()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) +} + +func cmdAddDelTest(testNS ns.NetNS, tc testCase) { + // Get a Add/Del tester based on test case version + tester := testerByVersion(tc.cniVersion) + + targetNS, err := ns.NewNS() + Expect(err).NotTo(HaveOccurred()) + defer targetNS.Close() + tester.setNS(testNS, targetNS) + + // Test IP allocation + tester.cmdAddTest(tc) + + // Test IP Release + tester.cmdDelTest(tc) + + // Clean up bridge addresses for next test case + delBridgeAddrs(testNS) +} + var _ = Describe("bridge Operations", func() { var originalNS ns.NetNS @@ -199,73 +571,50 @@ var _ = Describe("bridge Operations", func() { }) It("creates a bridge", func() { - const IFNAME = "bridge0" - - conf := &NetConf{ - NetConf: types.NetConf{ - CNIVersion: "0.3.1", - Name: "testConfig", - Type: "bridge", - }, - BrName: IFNAME, - IsGW: false, - IPMasq: false, - MTU: 5000, - } - + conf := testCase{cniVersion: "0.3.1"}.netConf() err := originalNS.Do(func(ns.NetNS) error { defer GinkgoRecover() bridge, _, err := setupBridge(conf) Expect(err).NotTo(HaveOccurred()) - Expect(bridge.Attrs().Name).To(Equal(IFNAME)) + Expect(bridge.Attrs().Name).To(Equal(BRNAME)) // Double check that the link was added - link, err := netlink.LinkByName(IFNAME) + link, err := netlink.LinkByName(BRNAME) Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().Name).To(Equal(IFNAME)) + Expect(link.Attrs().Name).To(Equal(BRNAME)) return nil }) Expect(err).NotTo(HaveOccurred()) }) It("handles an existing bridge", func() { - const IFNAME = "bridge0" - err := originalNS.Do(func(ns.NetNS) error { defer GinkgoRecover() err := netlink.LinkAdd(&netlink.Bridge{ LinkAttrs: netlink.LinkAttrs{ - Name: IFNAME, + Name: BRNAME, }, }) Expect(err).NotTo(HaveOccurred()) - link, err := netlink.LinkByName(IFNAME) + link, err := netlink.LinkByName(BRNAME) Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().Name).To(Equal(IFNAME)) + Expect(link.Attrs().Name).To(Equal(BRNAME)) ifindex := link.Attrs().Index - conf := &NetConf{ - NetConf: types.NetConf{ - CNIVersion: "0.3.1", - Name: "testConfig", - Type: "bridge", - }, - BrName: IFNAME, - IsGW: false, - IPMasq: false, - } + tc := testCase{cniVersion: "0.3.1", isGW: false} + conf := tc.netConf() bridge, _, err := setupBridge(conf) Expect(err).NotTo(HaveOccurred()) - Expect(bridge.Attrs().Name).To(Equal(IFNAME)) + Expect(bridge.Attrs().Name).To(Equal(BRNAME)) Expect(bridge.Attrs().Index).To(Equal(ifindex)) // Double check that the link has the same ifindex - link, err = netlink.LinkByName(IFNAME) + link, err = netlink.LinkByName(BRNAME) Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().Name).To(Equal(IFNAME)) + Expect(link.Attrs().Name).To(Equal(BRNAME)) Expect(link.Attrs().Index).To(Equal(ifindex)) return nil }) @@ -273,256 +622,236 @@ var _ = Describe("bridge Operations", func() { }) It("configures and deconfigures a bridge and veth with default route with ADD/DEL for 0.3.0 config", func() { - checkBridgeConfig03x("0.3.0", originalNS) + testCases := []testCase{ + { + // IPv4 only + subnet: "10.1.2.0/24", + expGWCIDRs: []string{"10.1.2.1/24"}, + }, + { + // IPv6 only + subnet: "2001:db8::0/64", + expGWCIDRs: []string{"2001:db8::1/64"}, + }, + { + // Dual-Stack + ranges: []rangeInfo{ + {subnet: "192.168.0.0/24"}, + {subnet: "fd00::0/64"}, + }, + expGWCIDRs: []string{ + "192.168.0.1/24", + "fd00::1/64", + }, + }, + { + // 3 Subnets (1 IPv4 and 2 IPv6 subnets) + ranges: []rangeInfo{ + {subnet: "192.168.0.0/24"}, + {subnet: "fd00::0/64"}, + {subnet: "2001:db8::0/64"}, + }, + expGWCIDRs: []string{ + "192.168.0.1/24", + "fd00::1/64", + "2001:db8::1/64", + }, + }, + } + for _, tc := range testCases { + tc.cniVersion = "0.3.0" + cmdAddDelTest(originalNS, tc) + } }) It("configures and deconfigures a bridge and veth with default route with ADD/DEL for 0.3.1 config", func() { - checkBridgeConfig03x("0.3.1", originalNS) + testCases := []testCase{ + { + // IPv4 only + subnet: "10.1.2.0/24", + expGWCIDRs: []string{"10.1.2.1/24"}, + }, + { + // IPv6 only + subnet: "2001:db8::0/64", + expGWCIDRs: []string{"2001:db8::1/64"}, + }, + { + // Dual-Stack + ranges: []rangeInfo{ + {subnet: "192.168.0.0/24"}, + {subnet: "fd00::0/64"}, + }, + expGWCIDRs: []string{ + "192.168.0.1/24", + "fd00::1/64", + }, + }, + } + for _, tc := range testCases { + tc.cniVersion = "0.3.1" + cmdAddDelTest(originalNS, tc) + } }) It("deconfigures an unconfigured bridge with DEL", func() { - const BRNAME = "cni0" - const IFNAME = "eth0" - - _, subnet, err := net.ParseCIDR("10.1.2.1/24") - Expect(err).NotTo(HaveOccurred()) - - conf := fmt.Sprintf(`{ - "cniVersion": "0.3.0", - "name": "mynet", - "type": "bridge", - "bridge": "%s", - "isDefaultGateway": true, - "ipMasq": false, - "ipam": { - "type": "host-local", - "subnet": "%s" - } -}`, BRNAME, subnet.String()) - - targetNs, err := ns.NewNS() - Expect(err).NotTo(HaveOccurred()) - defer targetNs.Close() - - args := &skel.CmdArgs{ - ContainerID: "dummy", - Netns: targetNs.Path(), - IfName: IFNAME, - StdinData: []byte(conf), + tc := testCase{ + cniVersion: "0.3.0", + subnet: "10.1.2.0/24", + expGWCIDRs: []string{"10.1.2.1/24"}, } - err = originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - err := testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error { - return cmdDel(args) - }) - Expect(err).NotTo(HaveOccurred()) - return nil - }) + tester := testerV03x{} + targetNS, err := ns.NewNS() Expect(err).NotTo(HaveOccurred()) + defer targetNS.Close() + tester.setNS(originalNS, targetNS) + tester.args = tc.createCmdArgs(targetNS) + + // Execute cmdDEL on the plugin, expect no errors + tester.cmdDelTest(tc) }) It("configures and deconfigures a bridge and veth with default route with ADD/DEL for 0.1.0 config", func() { - const BRNAME = "cni0" - const IFNAME = "eth0" - - gwaddr, subnet, err := net.ParseCIDR("10.1.2.1/24") - Expect(err).NotTo(HaveOccurred()) - - conf := fmt.Sprintf(`{ - "cniVersion": "0.1.0", - "name": "mynet", - "type": "bridge", - "bridge": "%s", - "isDefaultGateway": true, - "ipMasq": false, - "ipam": { - "type": "host-local", - "subnet": "%s" - } -}`, BRNAME, subnet.String()) - - targetNs, err := ns.NewNS() - Expect(err).NotTo(HaveOccurred()) - defer targetNs.Close() - - args := &skel.CmdArgs{ - ContainerID: "dummy", - Netns: targetNs.Path(), - IfName: IFNAME, - StdinData: []byte(conf), + testCases := []testCase{ + { + // IPv4 only + subnet: "10.1.2.0/24", + expGWCIDRs: []string{"10.1.2.1/24"}, + }, + { + // IPv6 only + subnet: "2001:db8::0/64", + expGWCIDRs: []string{"2001:db8::1/64"}, + }, + { + // Dual-Stack + ranges: []rangeInfo{ + {subnet: "192.168.0.0/24"}, + {subnet: "fd00::0/64"}, + }, + expGWCIDRs: []string{ + "192.168.0.1/24", + "fd00::1/64", + }, + }, + } + for _, tc := range testCases { + tc.cniVersion = "0.1.0" + cmdAddDelTest(originalNS, tc) } - - var result *types020.Result - err = originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - r, raw, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, []byte(conf), func() error { - return cmdAdd(args) - }) - Expect(err).NotTo(HaveOccurred()) - Expect(strings.Index(string(raw), "\"ip4\":")).Should(BeNumerically(">", 0)) - - // We expect a version 0.1.0 result - result, err = types020.GetResult(r) - Expect(err).NotTo(HaveOccurred()) - - // Make sure bridge link exists - link, err := netlink.LinkByName(BRNAME) - Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().Name).To(Equal(BRNAME)) - Expect(link).To(BeAssignableToTypeOf(&netlink.Bridge{})) - hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) - Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) - - // Ensure bridge has gateway address - addrs, err := netlink.AddrList(link, syscall.AF_INET) - Expect(err).NotTo(HaveOccurred()) - Expect(len(addrs)).To(BeNumerically(">", 0)) - found := false - subnetPrefix, subnetBits := subnet.Mask.Size() - for _, a := range addrs { - aPrefix, aBits := a.IPNet.Mask.Size() - if a.IPNet.IP.Equal(gwaddr) && aPrefix == subnetPrefix && aBits == subnetBits { - found = true - break - } - } - Expect(found).To(Equal(true)) - - // Check for the veth link in the main namespace; can't - // check the for the specific link since version 0.1.0 - // doesn't report interfaces - links, err := netlink.LinkList() - Expect(err).NotTo(HaveOccurred()) - Expect(len(links)).To(Equal(3)) // Bridge, veth, and loopback - return nil - }) - Expect(err).NotTo(HaveOccurred()) - - // Find the veth peer in the container namespace and the default route - err = targetNs.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - link, err := netlink.LinkByName(IFNAME) - Expect(err).NotTo(HaveOccurred()) - Expect(link.Attrs().Name).To(Equal(IFNAME)) - Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{})) - - addrs, err := netlink.AddrList(link, syscall.AF_INET) - Expect(err).NotTo(HaveOccurred()) - Expect(len(addrs)).To(Equal(1)) - - hwAddr := fmt.Sprintf("%s", link.Attrs().HardwareAddr) - Expect(hwAddr).To(HavePrefix(hwaddr.PrivateMACPrefixString)) - - // Ensure the default route - routes, err := netlink.RouteList(link, 0) - Expect(err).NotTo(HaveOccurred()) - - var defaultRouteFound bool - for _, route := range routes { - defaultRouteFound = (route.Dst == nil && route.Src == nil && route.Gw.Equal(gwaddr)) - if defaultRouteFound { - break - } - } - Expect(defaultRouteFound).To(Equal(true)) - - return nil - }) - Expect(err).NotTo(HaveOccurred()) - - err = originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - err := testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error { - return cmdDel(args) - }) - Expect(err).NotTo(HaveOccurred()) - return nil - }) - Expect(err).NotTo(HaveOccurred()) - - // Make sure the container veth has been deleted; cannot check - // host veth as version 0.1.0 can't report its name - err = originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - link, err := netlink.LinkByName(IFNAME) - Expect(err).To(HaveOccurred()) - Expect(link).To(BeNil()) - return nil - }) }) It("ensure bridge address", func() { - const IFNAME = "bridge0" - const EXPECTED_IP = "10.0.0.0/8" - const CHANGED_EXPECTED_IP = "10.1.2.3/16" - - conf := &NetConf{ - NetConf: types.NetConf{ - CNIVersion: "0.3.1", - Name: "testConfig", - Type: "bridge", + conf := testCase{cniVersion: "0.3.1", isGW: true}.netConf() + + testCases := []struct { + gwCIDRFirst string + gwCIDRSecond string + }{ + { + // IPv4 + gwCIDRFirst: "10.0.0.1/8", + gwCIDRSecond: "10.1.2.3/16", + }, + { + // IPv6, overlapping subnets + gwCIDRFirst: "2001:db8:1::1/48", + gwCIDRSecond: "2001:db8:1:2::1/64", + }, + { + // IPv6, non-overlapping subnets + gwCIDRFirst: "2001:db8:1:2::1/64", + gwCIDRSecond: "fd00:1234::1/64", }, - BrName: IFNAME, - IsGW: true, - IPMasq: false, - MTU: 5000, - } - - gwnFirst := &net.IPNet{ - IP: net.IPv4(10, 0, 0, 0), - Mask: net.CIDRMask(8, 32), - } - - gwnSecond := &net.IPNet{ - IP: net.IPv4(10, 1, 2, 3), - Mask: net.CIDRMask(16, 32), } + for _, tc := range testCases { - err := originalNS.Do(func(ns.NetNS) error { - defer GinkgoRecover() - - bridge, _, err := setupBridge(conf) + gwIP, gwSubnet, err := net.ParseCIDR(tc.gwCIDRFirst) Expect(err).NotTo(HaveOccurred()) - // Check if ForceAddress has default value - Expect(conf.ForceAddress).To(Equal(false)) - - err = ensureBridgeAddr(bridge, gwnFirst, conf.ForceAddress) + gwnFirst := net.IPNet{IP: gwIP, Mask: gwSubnet.Mask} + gwIP, gwSubnet, err = net.ParseCIDR(tc.gwCIDRSecond) Expect(err).NotTo(HaveOccurred()) + gwnSecond := net.IPNet{IP: gwIP, Mask: gwSubnet.Mask} + + var family, expNumAddrs int + switch { + case gwIP.To4() != nil: + family = netlink.FAMILY_V4 + expNumAddrs = 1 + default: + family = netlink.FAMILY_V6 + // Expect configured gw address plus link local + expNumAddrs = 2 + } - //Check if IP address is set correctly - addrs, err := netlink.AddrList(bridge, syscall.AF_INET) - Expect(len(addrs)).To(Equal(1)) - addr := addrs[0].IPNet.String() - Expect(addr).To(Equal(EXPECTED_IP)) + subnetsOverlap := gwnFirst.Contains(gwnSecond.IP) || + gwnSecond.Contains(gwnFirst.IP) + + err = originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + // Create the bridge + bridge, _, err := setupBridge(conf) + Expect(err).NotTo(HaveOccurred()) + + // Function to check IP address(es) on bridge + checkBridgeIPs := func(cidr0, cidr1 string) { + addrs, err := netlink.AddrList(bridge, family) + Expect(err).NotTo(HaveOccurred()) + Expect(len(addrs)).To(Equal(expNumAddrs)) + addr := addrs[0].IPNet.String() + Expect(addr).To(Equal(cidr0)) + if cidr1 != "" { + addr = addrs[1].IPNet.String() + Expect(addr).To(Equal(cidr1)) + } + } - //The bridge IP address has been changed. Error expected when ForceAddress is set to false. - err = ensureBridgeAddr(bridge, gwnSecond, false) - Expect(err).To(HaveOccurred()) + // Check if ForceAddress has default value + Expect(conf.ForceAddress).To(Equal(false)) + + // Set first address on bridge + err = ensureBridgeAddr(bridge, family, &gwnFirst, conf.ForceAddress) + Expect(err).NotTo(HaveOccurred()) + checkBridgeIPs(tc.gwCIDRFirst, "") + + // Attempt to set the second address on the bridge + // with ForceAddress set to false. + err = ensureBridgeAddr(bridge, family, &gwnSecond, false) + if family == netlink.FAMILY_V4 || subnetsOverlap { + // IPv4 or overlapping IPv6 subnets: + // Expect an error, and address should remain the same + Expect(err).To(HaveOccurred()) + checkBridgeIPs(tc.gwCIDRFirst, "") + } else { + // Non-overlapping IPv6 subnets: + // There should be 2 addresses (in addition to link local) + Expect(err).NotTo(HaveOccurred()) + expNumAddrs++ + checkBridgeIPs(tc.gwCIDRSecond, tc.gwCIDRFirst) + } - //The IP address should stay the same. - addrs, err = netlink.AddrList(bridge, syscall.AF_INET) - Expect(len(addrs)).To(Equal(1)) - addr = addrs[0].IPNet.String() - Expect(addr).To(Equal(EXPECTED_IP)) + // Set the second address on the bridge + // with ForceAddress set to true. + err = ensureBridgeAddr(bridge, family, &gwnSecond, true) + Expect(err).NotTo(HaveOccurred()) + if family == netlink.FAMILY_V4 || subnetsOverlap { + // IPv4 or overlapping IPv6 subnets: + // IP address should be reconfigured. + checkBridgeIPs(tc.gwCIDRSecond, "") + } else { + // Non-overlapping IPv6 subnets: + // There should be 2 addresses (in addition to link local) + checkBridgeIPs(tc.gwCIDRSecond, tc.gwCIDRFirst) + } - //Reconfigure IP when ForceAddress is set to true and IP address has been changed. - err = ensureBridgeAddr(bridge, gwnSecond, true) + return nil + }) Expect(err).NotTo(HaveOccurred()) - //Retrieve the IP address after reconfiguration - addrs, err = netlink.AddrList(bridge, syscall.AF_INET) - Expect(len(addrs)).To(Equal(1)) - addr = addrs[0].IPNet.String() - Expect(addr).To(Equal(CHANGED_EXPECTED_IP)) - - return nil - }) - Expect(err).NotTo(HaveOccurred()) + // Clean up bridge addresses for next test case + delBridgeAddrs(originalNS) + } }) })