diff --git a/plugins/ipam/static/main.go b/plugins/ipam/static/main.go index d7eaeab11..38cd76161 100644 --- a/plugins/ipam/static/main.go +++ b/plugins/ipam/static/main.go @@ -161,7 +161,7 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) { ip, subnet, err := net.ParseCIDR(ipstr) if err != nil { - return nil, "", fmt.Errorf("invalid CIDR %s: %s", ipstr, err) + return nil, "", fmt.Errorf("the 'ip' field is expected to be in CIDR notation, got: '%s'", ipstr) } addr := Address{ @@ -192,8 +192,13 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) { if n.Args != nil && n.Args.A != nil && len(n.Args.A.IPs) != 0 { // args IP overwrites IP, so clear IPAM Config n.IPAM.Addresses = make([]Address, 0, len(n.Args.A.IPs)) - for _, addr := range n.Args.A.IPs { - n.IPAM.Addresses = append(n.IPAM.Addresses, Address{AddressStr: addr}) + for _, addrStr := range n.Args.A.IPs { + ip, addr, err := net.ParseCIDR(addrStr) + if err != nil { + return nil, "", fmt.Errorf("an entry in the 'ips' field is NOT in CIDR notation, got: '%s'", addrStr) + } + addr.IP = ip + n.IPAM.Addresses = append(n.IPAM.Addresses, Address{AddressStr: addrStr, Address: *addr}) } } @@ -201,8 +206,13 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) { if len(n.RuntimeConfig.IPs) != 0 { // runtimeConfig IP overwrites IP, so clear IPAM Config n.IPAM.Addresses = make([]Address, 0, len(n.RuntimeConfig.IPs)) - for _, addr := range n.RuntimeConfig.IPs { - n.IPAM.Addresses = append(n.IPAM.Addresses, Address{AddressStr: addr}) + for _, addrStr := range n.RuntimeConfig.IPs { + ip, addr, err := net.ParseCIDR(addrStr) + if err != nil { + return nil, "", fmt.Errorf("an entry in the 'ips' field is NOT in CIDR notation, got: '%s'", addrStr) + } + addr.IP = ip + n.IPAM.Addresses = append(n.IPAM.Addresses, Address{AddressStr: addrStr, Address: *addr}) } } @@ -211,12 +221,15 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) { numV6 := 0 for i := range n.IPAM.Addresses { - ip, addr, err := net.ParseCIDR(n.IPAM.Addresses[i].AddressStr) - if err != nil { - return nil, "", fmt.Errorf("invalid CIDR %s: %s", n.IPAM.Addresses[i].AddressStr, err) + if n.IPAM.Addresses[i].Address.IP == nil { + ip, addr, err := net.ParseCIDR(n.IPAM.Addresses[i].AddressStr) + if err != nil { + return nil, "", fmt.Errorf( + "the 'address' field is expected to be in CIDR notation, got: '%s'", n.IPAM.Addresses[i].AddressStr) + } + n.IPAM.Addresses[i].Address = *addr + n.IPAM.Addresses[i].Address.IP = ip } - n.IPAM.Addresses[i].Address = *addr - n.IPAM.Addresses[i].Address.IP = ip if err := canonicalizeIP(&n.IPAM.Addresses[i].Address.IP); err != nil { return nil, "", fmt.Errorf("invalid address %d: %s", i, err) diff --git a/plugins/ipam/static/static_test.go b/plugins/ipam/static/static_test.go index deb1bbadc..f1d4c017a 100644 --- a/plugins/ipam/static/static_test.go +++ b/plugins/ipam/static/static_test.go @@ -546,6 +546,137 @@ var _ = Describe("static Operations", func() { }) Expect(err).Should(MatchError("IPAM config missing 'ipam' key")) }) + + It(fmt.Sprintf("[%s] errors when passed an invalid CIDR via ipam config", ver), func() { + const ifname string = "eth0" + const nspath string = "/some/where" + const ipStr string = "10.10.0.1" + + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "bridge", + "ipam": { + "type": "static", + "addresses": [ { + "address": "%s" + }] + } + }`, ver, ipStr) + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname, + StdinData: []byte(conf), + } + + // Allocate the IP + _, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).Should(MatchError( + fmt.Sprintf("the 'address' field is expected to be in CIDR notation, got: '%s'", ipStr))) + }) + + It(fmt.Sprintf("[%s] errors when passed an invalid CIDR via Args", ver), func() { + const ifname string = "eth0" + const nspath string = "/some/where" + const ipStr string = "10.10.0.1" + + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "bridge", + "ipam": { + "type": "static", + "routes": [{ "dst": "0.0.0.0/0" }] + } + }`, ver) + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname, + StdinData: []byte(conf), + Args: fmt.Sprintf("IP=%s", ipStr), + } + + // Allocate the IP + _, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).Should(MatchError( + fmt.Sprintf("the 'ip' field is expected to be in CIDR notation, got: '%s'", ipStr))) + }) + + It(fmt.Sprintf("[%s] errors when passed an invalid CIDR via CNI_ARGS", ver), func() { + const ifname string = "eth0" + const nspath string = "/some/where" + const ipStr string = "10.10.0.1" + + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "bridge", + "ipam": { + "type": "static", + "routes": [{ "dst": "0.0.0.0/0" }] + }, + "args": { + "cni": { + "ips" : ["%s"] + } + } + }`, ver, ipStr) + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname, + StdinData: []byte(conf), + } + + // Allocate the IP + _, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).Should(MatchError( + fmt.Sprintf("an entry in the 'ips' field is NOT in CIDR notation, got: '%s'", ipStr))) + }) + + It(fmt.Sprintf("[%s] errors when passed an invalid CIDR via RuntimeConfig", ver), func() { + const ifname string = "eth0" + const nspath string = "/some/where" + const ipStr string = "10.10.0.1" + + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "bridge", + "ipam": { + "type": "static", + "routes": [{ "dst": "0.0.0.0/0" }] + }, + "RuntimeConfig": { + "ips" : ["%s"] + } + }`, ver, ipStr) + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname, + StdinData: []byte(conf), + } + + // Allocate the IP + _, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).Should(MatchError( + fmt.Sprintf("an entry in the 'ips' field is NOT in CIDR notation, got: '%s'", ipStr))) + }) } })