kubeadm: validate podSubnet against node-cidr-mask

the controller manager should validate the podSubnet against the node-mask
because if they are incorrect can cause the controller-manager to fail.

We don't need to calculate the node-cidr-masks, because those should
be provided by the user, if they are wrong we fail in validation.

Author: Antonio Ojea

cmd/kubeadm/app/apis/kubeadm/validation/BUILD

@@ -31,6 +31,7 @@ go_test(
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/v1beta2:go_default_library",
+        "//cmd/kubeadm/app/features:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",

cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -403,6 +403,59 @@ func ValidateIPNetFromString(subnetStr string, minAddrs int64, isDualStack bool,
 	return allErrs
 }
 
+// ValidatePodSubnetNodeMask validates that the relation between podSubnet and node-masks is correct
+func ValidatePodSubnetNodeMask(subnetStr string, c *kubeadm.ClusterConfiguration, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	// subnets were already validated
+	subnets, _ := utilnet.ParseCIDRs(strings.Split(subnetStr, ","))
+	for _, podSubnet := range subnets {
+		// obtain podSubnet mask
+		mask := podSubnet.Mask
+		maskSize, _ := mask.Size()
+		// obtain node-cidr-mask
+		nodeMask := getClusterNodeMask(c, utilnet.IsIPv6(podSubnet.IP))
+		// the pod subnet mask needs to allow one or multiple node-masks
+		// i.e. if it has a /24 the node mask must be between 24 and 32 for ipv4
+		if maskSize > nodeMask {
+			allErrs = append(allErrs, field.Invalid(fldPath, podSubnet.String(), "pod subnet size is smaller than the node subnet size"))
+		} else if (nodeMask - maskSize) > constants.PodSubnetNodeMaskMaxDiff {
+			allErrs = append(allErrs, field.Invalid(fldPath, podSubnet.String(), fmt.Sprintf("pod subnet mask and node-mask difference can not be greater than %d", constants.PodSubnetNodeMaskMaxDiff)))
+		}
+	}
+	return allErrs
+}
+
+// getClusterNodeMask returns the corresponding node-cidr-mask
+// based on the Cluster configuration and the IP family
+// Default is 24 for IPv4 and 64 for IPv6
+func getClusterNodeMask(c *kubeadm.ClusterConfiguration, isIPv6 bool) int {
+	// defaultNodeMaskCIDRIPv4 is default mask size for IPv4 node cidr for use by the controller manager
+	const defaultNodeMaskCIDRIPv4 = 24
+	// DefaultNodeMaskCIDRIPv6 is default mask size for IPv6 node cidr for use by the controller manager
+	const defaultNodeMaskCIDRIPv6 = 64
+	var maskSize int
+	var maskArg string
+	isDualStack := features.Enabled(c.FeatureGates, features.IPv6DualStack)
+
+	if isDualStack && isIPv6 {
+		maskArg = "node-cidr-mask-size-ipv6"
+	} else if isDualStack && !isIPv6 {
+		maskArg = "node-cidr-mask-size-ipv4"
+	} else {
+		maskArg = "node-cidr-mask-size"
+	}
+
+	if v, ok := c.ControllerManager.ExtraArgs[maskArg]; ok && v != "" {
+		// assume it is an integer, if not it will fail later
+		maskSize, _ = strconv.Atoi(v)
+	} else if isIPv6 {
+		maskSize = defaultNodeMaskCIDRIPv6
+	} else {
+		maskSize = defaultNodeMaskCIDRIPv4
+	}
+	return maskSize
+}
+
 // ValidateNetworking validates networking configuration
 func ValidateNetworking(c *kubeadm.ClusterConfiguration, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
@@ -417,7 +470,9 @@ func ValidateNetworking(c *kubeadm.ClusterConfiguration, fldPath *field.Path) fi
 		allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.ServiceSubnet, constants.MinimumAddressesInServiceSubnet, isDualStack, field.NewPath("serviceSubnet"))...)
 	}
 	if len(c.Networking.PodSubnet) != 0 {
-		allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.PodSubnet, constants.MinimumAddressesInServiceSubnet, isDualStack, field.NewPath("podSubnet"))...)
+		allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.PodSubnet, constants.MinimumAddressesInPodSubnet, isDualStack, field.NewPath("podSubnet"))...)
+		// Pod subnet was already validated, we need to validate now against the node-mask
+		allErrs = append(allErrs, ValidatePodSubnetNodeMask(c.Networking.PodSubnet, c, field.NewPath("podSubnet"))...)
 	}
 	return allErrs
 }

cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

@@ -25,7 +25,9 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
+	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 )
 
 func TestValidateToken(t *testing.T) {
@@ -231,6 +233,60 @@ func TestValidateIPNetFromString(t *testing.T) {
 	}
 }
 
+func TestValidatePodSubnetNodeMask(t *testing.T) {
+	var tests = []struct {
+		name           string
+		subnet         string
+		cmExtraArgs    map[string]string
+		checkDualStack bool
+		expected       bool
+	}{
+		{"single IPv4, but mask too small. Default node-mask", "10.0.0.16/29", nil, false, false},
+		{"single IPv4, but mask too small. Configured node-mask", "10.0.0.16/24", map[string]string{"node-cidr-mask-size": "23"}, false, false},
+		{"single IPv6, but mask too small. Default node-mask", "2001:db8::1/112", nil, false, false},
+		{"single IPv6, but mask too small. Configured node-mask", "2001:db8::1/64", map[string]string{"node-cidr-mask-size": "24"}, false, false},
+		{"single IPv6, but mask difference greater than 16. Default node-mask", "2001:db8::1/12", nil, false, false},
+		{"single IPv6, but mask difference greater than 16. Configured node-mask", "2001:db8::1/64", map[string]string{"node-cidr-mask-size": "120"}, false, false},
+		{"single IPv4 CIDR", "10.0.0.16/12", nil, false, true},
+		{"single IPv6 CIDR", "2001:db8::/48", nil, false, true},
+		// dual-stack:
+		{"dual IPv4 only, but mask too small. Default node-mask", "10.0.0.16/29", nil, true, false},
+		{"dual IPv4 only, but mask too small. Configured node-mask", "10.0.0.16/24", map[string]string{"node-cidr-mask-size-ipv4": "23"}, true, false},
+		{"dual IPv6 only, but mask too small. Default node-mask", "2001:db8::1/112", nil, true, false},
+		{"dual IPv6 only, but mask too small. Configured node-mask", "2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "24"}, true, false},
+		{"dual IPv6 only, but mask difference greater than 16. Default node-mask", "2001:db8::1/12", nil, true, false},
+		{"dual IPv6 only, but mask difference greater than 16. Configured node-mask", "2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "120"}, true, false},
+		{"dual IPv4 only CIDR", "10.0.0.16/12", nil, true, true},
+		{"dual IPv6 only CIDR", "2001:db8::/48", nil, true, true},
+		{"dual, but IPv4 mask too small. Default node-mask", "10.0.0.16/29,2001:db8::/48", nil, true, false},
+		{"dual, but IPv4 mask too small. Configured node-mask", "10.0.0.16/24,2001:db8::/48", map[string]string{"node-cidr-mask-size-ipv4": "23"}, true, false},
+		{"dual, but IPv6 mask too small. Default node-mask", "2001:db8::1/112,10.0.0.16/16", nil, true, false},
+		{"dual, but IPv6 mask too small. Configured node-mask", "10.0.0.16/16,2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "24"}, true, false},
+		{"dual, but mask difference greater than 16. Default node-mask", "2001:db8::1/12,10.0.0.16/16", nil, true, false},
+		{"dual, but mask difference greater than 16. Configured node-mask", "10.0.0.16/16,2001:db8::1/64", map[string]string{"node-cidr-mask-size-ipv6": "120"}, true, false},
+		{"dual IPv4 IPv6", "2001:db8::/48,10.0.0.16/12", nil, true, true},
+		{"dual IPv6 IPv4", "2001:db8::/48,10.0.0.16/12", nil, true, true},
+	}
+	for _, rt := range tests {
+		cfg := &kubeadmapi.ClusterConfiguration{
+			FeatureGates: map[string]bool{features.IPv6DualStack: rt.checkDualStack},
+			ControllerManager: kubeadmapi.ControlPlaneComponent{
+				ExtraArgs: rt.cmExtraArgs,
+			},
+		}
+		actual := ValidatePodSubnetNodeMask(rt.subnet, cfg, nil)
+		if (len(actual) == 0) != rt.expected {
+			t.Errorf(
+				"%s test case failed :\n\texpected: %t\n\t  actual: %t\n\t  err(s): %v\n\t",
+				rt.name,
+				rt.expected,
+				(len(actual) == 0),
+				actual,
+			)
+		}
+	}
+}
+
 func TestValidateHostPort(t *testing.T) {
 	var tests = []struct {
 		name     string
@@ -1040,3 +1096,123 @@ func TestValidateEtcd(t *testing.T) {
 		}
 	}
 }
+
+func TestGetClusterNodeMask(t *testing.T) {
+	tests := []struct {
+		name         string
+		cfg          *kubeadmapi.ClusterConfiguration
+		isIPv6       bool
+		expectedMask int
+	}{
+		{
+			name:         "ipv4 default mask",
+			cfg:          &kubeadmapi.ClusterConfiguration{},
+			isIPv6:       false,
+			expectedMask: 24,
+		},
+		{
+			name: "ipv4 custom mask",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				ControllerManager: kubeadmapi.ControlPlaneComponent{
+					ExtraArgs: map[string]string{"node-cidr-mask-size": "23"},
+				},
+			},
+			isIPv6:       false,
+			expectedMask: 23,
+		},
+		{
+			name:         "ipv6 default mask",
+			cfg:          &kubeadmapi.ClusterConfiguration{},
+			isIPv6:       true,
+			expectedMask: 64,
+		},
+		{
+			name: "ipv6 custom mask",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				ControllerManager: kubeadmapi.ControlPlaneComponent{
+					ExtraArgs: map[string]string{"node-cidr-mask-size": "83"},
+				},
+			},
+			isIPv6:       true,
+			expectedMask: 83,
+		},
+		{
+			name: "dual ipv4 default mask",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				FeatureGates: map[string]bool{features.IPv6DualStack: true},
+			},
+			isIPv6:       false,
+			expectedMask: 24,
+		},
+		{
+			name: "dual ipv4 custom mask",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				FeatureGates: map[string]bool{features.IPv6DualStack: true},
+				ControllerManager: kubeadmapi.ControlPlaneComponent{
+					ExtraArgs: map[string]string{"node-cidr-mask-size": "21", "node-cidr-mask-size-ipv4": "23"},
+				},
+			},
+			isIPv6:       false,
+			expectedMask: 23,
+		},
+		{
+			name: "dual ipv6 default mask",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				FeatureGates: map[string]bool{features.IPv6DualStack: true},
+			},
+			isIPv6:       true,
+			expectedMask: 64,
+		},
+		{
+			name: "dual ipv6 custom mask",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				FeatureGates: map[string]bool{features.IPv6DualStack: true},
+				ControllerManager: kubeadmapi.ControlPlaneComponent{
+					ExtraArgs: map[string]string{"node-cidr-mask-size-ipv6": "83"},
+				},
+			},
+			isIPv6:       true,
+			expectedMask: 83,
+		},
+		{
+			name: "dual ipv4 custom mask",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				FeatureGates: map[string]bool{features.IPv6DualStack: true},
+				ControllerManager: kubeadmapi.ControlPlaneComponent{
+					ExtraArgs: map[string]string{"node-cidr-mask-size-ipv4": "23"},
+				},
+			},
+			isIPv6:       false,
+			expectedMask: 23,
+		},
+		{
+			name: "dual ipv6 default mask and legacy flag",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				FeatureGates: map[string]bool{features.IPv6DualStack: true},
+				ControllerManager: kubeadmapi.ControlPlaneComponent{
+					ExtraArgs: map[string]string{"node-cidr-mask-size": "23"},
+				},
+			},
+			isIPv6:       true,
+			expectedMask: 64,
+		},
+		{
+			name: "dual ipv6 custom mask and legacy flag",
+			cfg: &kubeadmapi.ClusterConfiguration{
+				FeatureGates: map[string]bool{features.IPv6DualStack: true},
+				ControllerManager: kubeadmapi.ControlPlaneComponent{
+					ExtraArgs: map[string]string{"node-cidr-mask-size": "23", "node-cidr-mask-size-ipv6": "83"},
+				},
+			},
+			isIPv6:       true,
+			expectedMask: 83,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if mask := getClusterNodeMask(test.cfg, test.isIPv6); mask != test.expectedMask {
+				t.Errorf("expected mask: %d, got %d", test.expectedMask, mask)
+			}
+		})
+	}
+}

cmd/kubeadm/app/constants/constants.go

@@ -199,6 +199,15 @@ const (
 	// We need at least ten, because the DNS service is always at the tenth cluster clusterIP
 	MinimumAddressesInServiceSubnet = 10
 
+	// MinimumAddressesInPodSubnet defines minimum amount of pods in the cluster.
+	// We need at least more than services, an IPv4 /28 or IPv6 /128 subnet means 14 util addresses
+	MinimumAddressesInPodSubnet = 14
+
+	// PodSubnetNodeMaskMaxDiff is limited to 16 due to an issue with uncompressed IP bitmap in core:
+	// xref: #44918
+	// The node subnet mask size must be no more than the pod subnet mask size + 16
+	PodSubnetNodeMaskMaxDiff = 16
+
 	// DefaultTokenDuration specifies the default amount of time that a bootstrap token will be valid
 	// Default behaviour is 24 hours
 	DefaultTokenDuration = 24 * time.Hour

cmd/kubeadm/app/phases/addons/proxy/proxy_test.go

@@ -194,12 +194,12 @@ func TestEnsureProxyAddon(t *testing.T) {
 				controlPlaneConfig.LocalAPIEndpoint.AdvertiseAddress = "1.2.3"
 			case IPv6SetBindAddress:
 				controlPlaneConfig.LocalAPIEndpoint.AdvertiseAddress = "1:2::3:4"
-				controlPlaneClusterConfig.Networking.PodSubnet = "2001:101::/96"
+				controlPlaneClusterConfig.Networking.PodSubnet = "2001:101::/48"
 			}
 
 			intControlPlane, err := configutil.DefaultedInitConfiguration(controlPlaneConfig, controlPlaneClusterConfig)
 			if err != nil {
-				t.Errorf("test failed to convert external to internal version")
+				t.Errorf("test failed to convert external to internal version: %v", err)
 				return
 			}
 			err = EnsureProxyAddon(&intControlPlane.ClusterConfiguration, &intControlPlane.LocalAPIEndpoint, client)

cmd/kubeadm/app/phases/controlplane/manifests.go

@@ -270,52 +270,6 @@ func isValidAuthzMode(authzMode string) bool {
 	return false
 }
 
-// calcNodeCidrSize determines the size of the subnets used on each node, based
-// on the pod subnet provided.  For IPv4, we assume that the pod subnet will
-// be /16 and use /24. If the pod subnet cannot be parsed, the IPv4 value will
-// be used (/24).
-//
-// For IPv6, the algorithm will do two three. First, the node CIDR will be set
-// to a multiple of 8, using the available bits for easier readability by user.
-// Second, the number of nodes will be 512 to 64K to attempt to maximize the
-// number of nodes (see NOTE below). Third, pod networks of /113 and larger will
-// be rejected, as the amount of bits available is too small.
-//
-// A special case is when the pod network size is /112, where /120 will be used,
-// only allowing 256 nodes and 256 pods.
-//
-// If the pod network size is /113 or larger, the node CIDR will be set to the same
-// size and this will be rejected later in validation.
-//
-// NOTE: Currently, the design allows a maximum of 64K nodes. This algorithm splits
-// the available bits to maximize the number used for nodes, but still have the node
-// CIDR be a multiple of eight.
-//
-func calcNodeCidrSize(podSubnet string) (string, bool) {
-	maskSize := "24"
-	isIPv6 := false
-	if ip, podCidr, err := net.ParseCIDR(podSubnet); err == nil {
-		if utilsnet.IsIPv6(ip) {
-			var nodeCidrSize int
-			isIPv6 = true
-			podNetSize, totalBits := podCidr.Mask.Size()
-			switch {
-			case podNetSize == 112:
-				// Special case, allows 256 nodes, 256 pods/node
-				nodeCidrSize = 120
-			case podNetSize < 112:
-				// Use multiple of 8 for node CIDR, with 512 to 64K nodes
-				nodeCidrSize = totalBits - ((totalBits-podNetSize-1)/8-1)*8
-			default:
-				// Not enough bits, will fail later, when validate
-				nodeCidrSize = podNetSize
-			}
-			maskSize = strconv.Itoa(nodeCidrSize)
-		}
-	}
-	return maskSize, isIPv6
-}
-
 // getControllerManagerCommand builds the right controller manager command from the given config object and version
 func getControllerManagerCommand(cfg *kubeadmapi.ClusterConfiguration) []string {
 
@@ -367,22 +321,6 @@ func getControllerManagerCommand(cfg *kubeadmapi.ClusterConfiguration) []string
 	if present {
 		defaultArguments["feature-gates"] = fmt.Sprintf("%s=%t", features.IPv6DualStack, enabled)
 	}
-	if cfg.Networking.PodSubnet != "" {
-		if enabled {
-			// any errors will be caught during validation
-			subnets := strings.Split(cfg.Networking.PodSubnet, ",")
-			for _, podSubnet := range subnets {
-				if maskSize, isIPv6 := calcNodeCidrSize(podSubnet); isIPv6 {
-					defaultArguments["node-cidr-mask-size-ipv6"] = maskSize
-				} else {
-					defaultArguments["node-cidr-mask-size-ipv4"] = maskSize
-				}
-			}
-		} else {
-			maskSize, _ := calcNodeCidrSize(cfg.Networking.PodSubnet)
-			defaultArguments["node-cidr-mask-size"] = maskSize
-		}
-	}
 
 	command := []string{"kube-controller-manager"}
 	command = append(command, kubeadmutil.BuildArgumentListFromMap(defaultArguments, cfg.ControllerManager.ExtraArgs)...)