Move validation in own function with tests

Author: johscheuer

cmd/kube-apiserver/app/options/validation.go

@@ -36,8 +36,8 @@ import (
 // validateClusterIPFlags is expected to be called after Complete()
 func validateClusterIPFlags(options *ServerRunOptions) []error {
 	var errs []error
-	// maxCIDRbits is used to define the maximum CIDR size for the cluster ip(s)
-	const maxCIDRbits = 20
+	// maxCIDRBits is used to define the maximum CIDR size for the cluster ip(s)
+	const maxCIDRBits = 20
 
 	// validate that primary has been processed by user provided values or it has been defaulted
 	if options.PrimaryServiceClusterIPRange.IP == nil {
@@ -51,9 +51,8 @@ func validateClusterIPFlags(options *ServerRunOptions) []error {
 
 	// Complete() expected to have set Primary* and Secondary*
 	// primary CIDR validation
-	var ones, bits = options.PrimaryServiceClusterIPRange.Mask.Size()
-	if bits-ones > maxCIDRbits {
-		errs = append(errs, fmt.Errorf("specified --service-cluster-ip-range is too large. Network CIDR should not be bigger than /%d", bits-maxCIDRbits))
+	if err := validateMaxCIDRRange(options.PrimaryServiceClusterIPRange, maxCIDRBits, "--service-cluster-ip-range"); err != nil {
+		errs = append(errs, err)
 	}
 
 	// Secondary IP validation
@@ -77,18 +76,26 @@ func validateClusterIPFlags(options *ServerRunOptions) []error {
 			errs = append(errs, errors.New("--service-cluster-ip-range and --secondary-service-cluster-ip-range must be of different IP family"))
 		}
 
-		// Should be smallish sized cidr, this thing is kept in etcd
-		// bigger cidr (specially those offered by IPv6) will add no value
-		// significantly increase snapshotting time.
-		var ones, bits = options.SecondaryServiceClusterIPRange.Mask.Size()
-		if bits-ones > maxCIDRbits {
-			errs = append(errs, fmt.Errorf("specified --service-cluster-ip-range is too large. Network CIDR should not be bigger than /%d", bits-maxCIDRbits))
+		if err := validateMaxCIDRRange(options.SecondaryServiceClusterIPRange, maxCIDRBits, "--secondary-service-cluster-ip-range"); err != nil {
+			errs = append(errs, err)
 		}
 	}
 
 	return errs
 }
 
+func validateMaxCIDRRange(cidr net.IPNet, maxCIDRBits int, cidrFlag string) error {
+	// Should be smallish sized cidr, this thing is kept in etcd
+	// bigger cidr (specially those offered by IPv6) will add no value
+	// significantly increase snapshotting time.
+	var ones, bits = cidr.Mask.Size()
+	if bits-ones > maxCIDRBits {
+		return fmt.Errorf("specified %s is too large; for %d-bit addresses, the mask must be >= %d", cidrFlag, bits, bits-maxCIDRBits)
+	}
+
+	return nil
+}
+
 func validateServiceNodePort(options *ServerRunOptions) []error {
 	var errs []error
 

cmd/kube-apiserver/app/options/validation_test.go

@@ -142,3 +142,70 @@ func TestClusterSerivceIPRange(t *testing.T) {
 		})
 	}
 }
+
+func getIPnetFromCIDR(cidr string) *net.IPNet {
+	_, ipnet, _ := net.ParseCIDR(cidr)
+	return ipnet
+}
+
+func TestValidateMaxCIDRRange(t *testing.T) {
+	testCases := []struct {
+		// tc.cidr, tc.maxCIDRBits, tc.cidrFlag) tc.expectedErrorMessage
+		name                 string
+		cidr                 net.IPNet
+		maxCIDRBits          int
+		cidrFlag             string
+		expectedErrorMessage string
+		expectErrors         bool
+	}{
+		{
+			name:                 "valid ipv4 cidr",
+			cidr:                 *getIPnetFromCIDR("10.92.0.0/12"),
+			maxCIDRBits:          20,
+			cidrFlag:             "--service-cluster-ip-range",
+			expectedErrorMessage: "",
+			expectErrors:         false,
+		},
+		{
+			name:                 "valid ipv6 cidr",
+			cidr:                 *getIPnetFromCIDR("3000::/108"),
+			maxCIDRBits:          20,
+			cidrFlag:             "--service-cluster-ip-range",
+			expectedErrorMessage: "",
+			expectErrors:         false,
+		},
+		{
+			name:                 "ipv4 cidr to big",
+			cidr:                 *getIPnetFromCIDR("10.92.0.0/8"),
+			maxCIDRBits:          20,
+			cidrFlag:             "--service-cluster-ip-range",
+			expectedErrorMessage: "specified --service-cluster-ip-range is too large; for 32-bit addresses, the mask must be >= 12",
+			expectErrors:         true,
+		},
+		{
+			name:                 "ipv6 cidr to big",
+			cidr:                 *getIPnetFromCIDR("3000::/64"),
+			maxCIDRBits:          20,
+			cidrFlag:             "--service-cluster-ip-range",
+			expectedErrorMessage: "specified --service-cluster-ip-range is too large; for 128-bit addresses, the mask must be >= 108",
+			expectErrors:         true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateMaxCIDRRange(tc.cidr, tc.maxCIDRBits, tc.cidrFlag)
+			if err != nil && !tc.expectErrors {
+				t.Errorf("expected no errors, error found %+v", err)
+			}
+
+			if err == nil && tc.expectErrors {
+				t.Errorf("expected errors, no errors found")
+			}
+
+			if err != nil && tc.expectErrors && err.Error() != tc.expectedErrorMessage {
+				t.Errorf("Expected error message: \"%s\"\nGot: \"%s\"", tc.expectedErrorMessage, err.Error())
+			}
+		})
+	}
+}