refactor: replace the net.ParseCIDR for some validations (istio#41336)

WillardHu · Oct 10, 2022 · fbbaa22 · fbbaa22
1 parent 29ea59b
commit fbbaa22
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 25 deletions.
diff --git a/cni/pkg/plugin/redirect.go b/cni/pkg/plugin/redirect.go
@@ -17,7 +17,7 @@ package plugin
 
 import (
 	"fmt"
-	"net"
+	"net/netip"
 	"strconv"
 	"strings"
 
@@ -114,7 +114,7 @@ func validateInterceptionMode(mode string) error {
 func validateCIDRList(cidrs string) error {
 	if len(cidrs) > 0 {
 		for _, cidr := range strings.Split(cidrs, ",") {
-			if _, _, err := net.ParseCIDR(cidr); err != nil {
+			if _, err := netip.ParsePrefix(cidr); err != nil {
 				return fmt.Errorf("failed parsing cidr '%s': %v", cidr, err)
 			}
 		}

diff --git a/operator/pkg/validate/common.go b/operator/pkg/validate/common.go
@@ -16,7 +16,7 @@ package validate
 
 import (
 	"fmt"
-	"net"
+	"net/netip"
 	"reflect"
 	"regexp"
 	"strconv"
@@ -186,8 +186,7 @@ func validateCIDR(path util.Path, val any) util.Errors {
 	if !util.IsString(val) {
 		err = fmt.Errorf("validateCIDR %s got %T, want string", path, val)
 	} else {
-		_, _, err = net.ParseCIDR(val.(string))
-		if err != nil {
+		if _, err = netip.ParsePrefix(val.(string)); err != nil {
 			err = fmt.Errorf("%s %s", path, err)
 		}
 	}

diff --git a/operator/pkg/validate/validate_test.go b/operator/pkg/validate/validate_test.go
@@ -102,7 +102,7 @@ values:
     proxy:
       includeIPRanges: "1.1.0.300/16,2.2.0.0/16"
 `,
-			wantErrs: makeErrors([]string{`global.proxy.includeIPRanges invalid CIDR address: 1.1.0.300/16`}),
+			wantErrs: makeErrors([]string{`global.proxy.includeIPRanges netip.ParsePrefix("1.1.0.300/16"): ParseAddr("1.1.0.300"): IPv4 field has value >255`}),
 		},
 		{
 			desc: "EmptyValuesIP",

diff --git a/operator/pkg/validate/validate_values_test.go b/operator/pkg/validate/validate_values_test.go
@@ -87,10 +87,10 @@ global:
     excludeIPRanges: "3.3.0.0/33,4.4.0.0/34"
 `,
 			wantErrs: makeErrors([]string{
-				`global.proxy.excludeIPRanges invalid CIDR address: 3.3.0.0/33`,
-				`global.proxy.excludeIPRanges invalid CIDR address: 4.4.0.0/34`,
-				`global.proxy.includeIPRanges invalid CIDR address: 1.1.0.256/16`,
-				`global.proxy.includeIPRanges invalid CIDR address: 2.2.0.257/16`,
+				`global.proxy.excludeIPRanges netip.ParsePrefix("3.3.0.0/33"): prefix length out of range`,
+				`global.proxy.excludeIPRanges netip.ParsePrefix("4.4.0.0/34"): prefix length out of range`,
+				`global.proxy.includeIPRanges netip.ParsePrefix("1.1.0.256/16"): ParseAddr("1.1.0.256"): IPv4 field has value >255`,
+				`global.proxy.includeIPRanges netip.ParsePrefix("2.2.0.257/16"): ParseAddr("2.2.0.257"): IPv4 field has value >255`,
 			}),
 		},
 		{
@@ -101,8 +101,8 @@ global:
     includeIPRanges: "1.2.3/16,1.2.3.x/16"
 `,
 			wantErrs: makeErrors([]string{
-				`global.proxy.includeIPRanges invalid CIDR address: 1.2.3/16`,
-				`global.proxy.includeIPRanges invalid CIDR address: 1.2.3.x/16`,
+				`global.proxy.includeIPRanges netip.ParsePrefix("1.2.3/16"): ParseAddr("1.2.3"): IPv4 address too short`,
+				`global.proxy.includeIPRanges netip.ParsePrefix("1.2.3.x/16"): ParseAddr("1.2.3.x"): unexpected character (at "x")`,
 			}),
 		},
 		{
@@ -112,7 +112,7 @@ global:
   proxy:
     includeIPRanges: "*,1.1.0.0/16,2.2.0.0/16"
 `,
-			wantErrs: makeErrors([]string{`global.proxy.includeIPRanges invalid CIDR address: *`}),
+			wantErrs: makeErrors([]string{`global.proxy.includeIPRanges netip.ParsePrefix("*"): no '/'`}),
 		},
 		{
 			desc: "BadPortRange",

diff --git a/pkg/config/security/security.go b/pkg/config/security/security.go
@@ -16,7 +16,7 @@ package security
 
 import (
 	"fmt"
-	"net"
+	"net/netip"
 	"net/url"
 	"strconv"
 	"strings"
@@ -153,11 +153,11 @@ func ValidateIPs(ips []string) error {
 	var errs *multierror.Error
 	for _, v := range ips {
 		if strings.Contains(v, "/") {
-			if _, _, err := net.ParseCIDR(v); err != nil {
+			if _, err := netip.ParsePrefix(v); err != nil {
 				errs = multierror.Append(errs, fmt.Errorf("bad CIDR range (%s): %v", v, err))
 			}
 		} else {
-			if ip := net.ParseIP(v); ip == nil {
+			if _, err := netip.ParseAddr(v); err != nil {
 				errs = multierror.Append(errs, fmt.Errorf("bad IP address (%s)", v))
 			}
 		}

diff --git a/pkg/config/validation/validation.go b/pkg/config/validation/validation.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/netip"
 	"net/url"
 	"path"
 	"regexp"
@@ -415,22 +416,18 @@ func ValidateIPSubnet(subnet string) error {
 	// E.g., a.b.c.d/xx form or just a.b.c.d or 2001:1::1/64
 	if strings.Count(subnet, "/") == 1 {
 		// We expect a string in "CIDR notation", i.e. a.b.c.d/xx or 2001:1::1/64 form
-		ip, _, err := net.ParseCIDR(subnet)
-		if err != nil {
+		if _, err := netip.ParsePrefix(subnet); err != nil {
 			return fmt.Errorf("%v is not a valid CIDR block", subnet)
 		}
-		if ip.To4() == nil && ip.To16() == nil {
-			return fmt.Errorf("%v is not a valid IPv4 or IPv6 address", subnet)
-		}
+
 		return nil
 	}
 	return ValidateIPAddress(subnet)
 }
 
 // ValidateIPAddress validates that a string in "CIDR notation" or "Dot-decimal notation"
 func ValidateIPAddress(addr string) error {
-	ip := net.ParseIP(addr)
-	if ip == nil {
+	if _, err := netip.ParseAddr(addr); err != nil {
 		return fmt.Errorf("%v is not a valid IP", addr)
 	}
 

diff --git a/pkg/kube/inject/validate.go b/pkg/kube/inject/validate.go
@@ -16,7 +16,7 @@ package inject
 
 import (
 	"fmt"
-	"net"
+	"net/netip"
 	"strconv"
 	"strings"
 
@@ -148,7 +148,7 @@ func validateBool(value string) error {
 func validateCIDRList(cidrs string) error {
 	if len(cidrs) > 0 {
 		for _, cidr := range strings.Split(cidrs, ",") {
-			if _, _, err := net.ParseCIDR(cidr); err != nil {
+			if _, err := netip.ParsePrefix(cidr); err != nil {
 				return fmt.Errorf("failed parsing cidr '%s': %v", cidr, err)
 			}
 		}