static ipam: improve error msgs when provisioning invalid CIDR

This commit addresses the scenarios when the invalid CIDR is provisioned via: - CNI_ARGS - RuntimeConfig Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
containernetworking · Jun 25, 2021 · b370fa0 · b370fa0
1 parent a786b12
commit b370fa0
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 1 deletion.
diff --git a/plugins/ipam/static/main.go b/plugins/ipam/static/main.go
@@ -193,6 +193,10 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 		// 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 {
+			_, _, err := net.ParseCIDR(addr)
+			if err != nil {
+				return nil, "", fmt.Errorf("an entry in the 'ips' field is NOT in CIDR notation, got: '%s'", addr)
+			}
 			n.IPAM.Addresses = append(n.IPAM.Addresses, Address{AddressStr: addr})
 		}
 	}
@@ -202,6 +206,10 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 		// runtimeConfig IP overwrites IP, so clear IPAM Config
 		n.IPAM.Addresses = make([]Address, 0, len(n.RuntimeConfig.IPs))
 		for _, addr := range n.RuntimeConfig.IPs {
+			_, _, err := net.ParseCIDR(addr)
+			if err != nil {
+				return nil, "", fmt.Errorf("an entry in the 'ips' field is NOT in CIDR notation, got: '%s'", addr)
+			}
 			n.IPAM.Addresses = append(n.IPAM.Addresses, Address{AddressStr: addr})
 		}
 	}

diff --git a/plugins/ipam/static/static_test.go b/plugins/ipam/static/static_test.go
@@ -547,7 +547,7 @@ var _ = Describe("static Operations", func() {
 			Expect(err).Should(MatchError("IPAM config missing 'ipam' key"))
 		})
 
-		It(fmt.Sprintf("[%s] errors when passed an invalid CIDR", ver), func() {
+		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"
@@ -578,6 +578,105 @@ var _ = Describe("static Operations", func() {
 			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)))
+		})
 	}
 })