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.
This commit is contained in:
Antonio Ojea 2020-10-20 10:59:42 +02:00
parent ededd08ba1
commit 8b52995d32
7 changed files with 246 additions and 167 deletions

View File

@ -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",

View File

@ -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
}

View File

@ -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)
}
})
}
}

View File

@ -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

View File

@ -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)

View File

@ -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)...)

View File

@ -650,7 +650,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--requestheader-client-ca-file=" + testCertsDir + "/front-proxy-ca.crt",
"--allocate-node-cidrs=true",
"--cluster-cidr=10.0.1.15/16",
"--node-cidr-mask-size=24",
},
},
{
@ -680,7 +679,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--requestheader-client-ca-file=" + testCertsDir + "/front-proxy-ca.crt",
"--allocate-node-cidrs=true",
"--cluster-cidr=10.0.1.15/16",
"--node-cidr-mask-size=24",
"--service-cluster-ip-range=172.20.0.0/24",
},
},
@ -743,7 +741,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--requestheader-client-ca-file=" + testCertsDir + "/front-proxy-ca.crt",
"--allocate-node-cidrs=true",
"--cluster-cidr=2001:db8::/64",
"--node-cidr-mask-size=80",
"--service-cluster-ip-range=fd03::/112",
},
},
@ -777,8 +774,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--feature-gates=IPv6DualStack=true",
"--allocate-node-cidrs=true",
"--cluster-cidr=2001:db8::/64,10.1.0.0/16",
"--node-cidr-mask-size-ipv4=24",
"--node-cidr-mask-size-ipv6=80",
"--service-cluster-ip-range=fd03::/112,192.168.0.0/16",
},
},
@ -787,7 +782,7 @@ func TestGetControllerManagerCommand(t *testing.T) {
cfg: &kubeadmapi.ClusterConfiguration{
Networking: kubeadmapi.Networking{PodSubnet: "10.0.1.15/16,2001:db8::/64"},
ControllerManager: kubeadmapi.ControlPlaneComponent{
ExtraArgs: map[string]string{"node-cidr-mask-size-ipv4": "20", "node-cidr-mask-size-ipv6": "96"},
ExtraArgs: map[string]string{"node-cidr-mask-size-ipv4": "20", "node-cidr-mask-size-ipv6": "80"},
},
CertificatesDir: testCertsDir,
KubernetesVersion: cpVersion,
@ -813,7 +808,7 @@ func TestGetControllerManagerCommand(t *testing.T) {
"--allocate-node-cidrs=true",
"--cluster-cidr=10.0.1.15/16,2001:db8::/64",
"--node-cidr-mask-size-ipv4=20",
"--node-cidr-mask-size-ipv6=96",
"--node-cidr-mask-size-ipv6=80",
},
},
}
@ -830,101 +825,6 @@ func TestGetControllerManagerCommand(t *testing.T) {
}
}
func TestCalcNodeCidrSize(t *testing.T) {
tests := []struct {
name string
podSubnet string
expectedPrefix string
expectedIPv6 bool
}{
{
name: "Malformed pod subnet",
podSubnet: "10.10.10/160",
expectedPrefix: "24",
expectedIPv6: false,
},
{
name: "V4: Always uses 24",
podSubnet: "10.10.10.10/16",
expectedPrefix: "24",
expectedIPv6: false,
},
{
name: "V6: Use pod subnet size, when not enough space",
podSubnet: "2001:db8::/128",
expectedPrefix: "128",
expectedIPv6: true,
},
{
name: "V6: Use pod subnet size, when not enough space",
podSubnet: "2001:db8::/113",
expectedPrefix: "113",
expectedIPv6: true,
},
{
name: "V6: Special case with 256 nodes",
podSubnet: "2001:db8::/112",
expectedPrefix: "120",
expectedIPv6: true,
},
{
name: "V6: Using /120 for node CIDR",
podSubnet: "2001:db8::/104",
expectedPrefix: "120",
expectedIPv6: true,
},
{
name: "V6: Using /112 for node CIDR",
podSubnet: "2001:db8::/103",
expectedPrefix: "112",
expectedIPv6: true,
},
{
name: "V6: Using /112 for node CIDR",
podSubnet: "2001:db8::/96",
expectedPrefix: "112",
expectedIPv6: true,
},
{
name: "V6: Using /104 for node CIDR",
podSubnet: "2001:db8::/95",
expectedPrefix: "104",
expectedIPv6: true,
},
{
name: "V6: For /64 pod net, use /80",
podSubnet: "2001:db8::/64",
expectedPrefix: "80",
expectedIPv6: true,
},
{
name: "V6: For /48 pod net, use /64",
podSubnet: "2001:db8::/48",
expectedPrefix: "64",
expectedIPv6: true,
},
{
name: "V6: For /32 pod net, use /48",
podSubnet: "2001:db8::/32",
expectedPrefix: "48",
expectedIPv6: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actualPrefix, actualIPv6 := calcNodeCidrSize(test.podSubnet)
if actualPrefix != test.expectedPrefix {
t.Errorf("Case [%s]\nCalc of node CIDR size for pod subnet %q failed: Expected %q, saw %q",
test.name, test.podSubnet, test.expectedPrefix, actualPrefix)
}
if actualIPv6 != test.expectedIPv6 {
t.Errorf("Case [%s]\nCalc of node CIDR size for pod subnet %q failed: Expected isIPv6=%v, saw isIPv6=%v",
test.name, test.podSubnet, test.expectedIPv6, actualIPv6)
}
})
}
}
func TestGetControllerManagerCommandExternalCA(t *testing.T) {
tests := []struct {
name string