Skip to content

Commit 3627a28

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#95723 from aojea/podcidr
kubeadm: validate podSubnet against node-mask and serviceSubnet max size
2 parents 0c00f7b + 4e14d1b commit 3627a28

File tree

7 files changed

+351
-168
lines changed

7 files changed

+351
-168
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ go_test(
3131
deps = [
3232
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
3333
"//cmd/kubeadm/app/apis/kubeadm/v1beta2:go_default_library",
34+
"//cmd/kubeadm/app/features:go_default_library",
3435
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
3536
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
3637
"//vendor/github.com/spf13/pflag:go_default_library",

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

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,87 @@ func ValidateIPNetFromString(subnetStr string, minAddrs int64, isDualStack bool,
403403
return allErrs
404404
}
405405

406+
// ValidateServiceSubnetSize validates that the maximum subnet size is not exceeded
407+
// Should be a small cidr due to how it is stored in etcd.
408+
// bigger cidr (specially those offered by IPv6) will add no value
409+
// and significantly increase snapshotting time.
410+
// NOTE: This is identical to validation performed in the apiserver.
411+
func ValidateServiceSubnetSize(subnetStr string, fldPath *field.Path) field.ErrorList {
412+
allErrs := field.ErrorList{}
413+
// subnets were already validated
414+
subnets, _ := utilnet.ParseCIDRs(strings.Split(subnetStr, ","))
415+
for _, serviceSubnet := range subnets {
416+
ones, bits := serviceSubnet.Mask.Size()
417+
if bits-ones > constants.MaximumBitsForServiceSubnet {
418+
errMsg := fmt.Sprintf("specified service subnet is too large; for %d-bit addresses, the mask must be >= %d", bits, bits-constants.MaximumBitsForServiceSubnet)
419+
allErrs = append(allErrs, field.Invalid(fldPath, serviceSubnet.String(), errMsg))
420+
}
421+
}
422+
return allErrs
423+
}
424+
425+
// ValidatePodSubnetNodeMask validates that the relation between podSubnet and node-masks is correct
426+
func ValidatePodSubnetNodeMask(subnetStr string, c *kubeadm.ClusterConfiguration, fldPath *field.Path) field.ErrorList {
427+
allErrs := field.ErrorList{}
428+
// subnets were already validated
429+
subnets, _ := utilnet.ParseCIDRs(strings.Split(subnetStr, ","))
430+
for _, podSubnet := range subnets {
431+
// obtain podSubnet mask
432+
mask := podSubnet.Mask
433+
maskSize, _ := mask.Size()
434+
// obtain node-cidr-mask
435+
nodeMask, err := getClusterNodeMask(c, utilnet.IsIPv6(podSubnet.IP))
436+
if err != nil {
437+
allErrs = append(allErrs, field.Invalid(fldPath, podSubnet.String(), err.Error()))
438+
continue
439+
}
440+
// the pod subnet mask needs to allow one or multiple node-masks
441+
// i.e. if it has a /24 the node mask must be between 24 and 32 for ipv4
442+
if maskSize > nodeMask {
443+
allErrs = append(allErrs, field.Invalid(fldPath, podSubnet.String(), "pod subnet size is smaller than the node subnet size"))
444+
} else if (nodeMask - maskSize) > constants.PodSubnetNodeMaskMaxDiff {
445+
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)))
446+
}
447+
}
448+
return allErrs
449+
}
450+
451+
// getClusterNodeMask returns the corresponding node-cidr-mask
452+
// based on the Cluster configuration and the IP family
453+
// Default is 24 for IPv4 and 64 for IPv6
454+
func getClusterNodeMask(c *kubeadm.ClusterConfiguration, isIPv6 bool) (int, error) {
455+
// defaultNodeMaskCIDRIPv4 is default mask size for IPv4 node cidr for use by the controller manager
456+
const defaultNodeMaskCIDRIPv4 = 24
457+
// DefaultNodeMaskCIDRIPv6 is default mask size for IPv6 node cidr for use by the controller manager
458+
const defaultNodeMaskCIDRIPv6 = 64
459+
var maskSize int
460+
var maskArg string
461+
var err error
462+
isDualStack := features.Enabled(c.FeatureGates, features.IPv6DualStack)
463+
464+
if isDualStack && isIPv6 {
465+
maskArg = "node-cidr-mask-size-ipv6"
466+
} else if isDualStack && !isIPv6 {
467+
maskArg = "node-cidr-mask-size-ipv4"
468+
} else {
469+
maskArg = "node-cidr-mask-size"
470+
}
471+
472+
if v, ok := c.ControllerManager.ExtraArgs[maskArg]; ok && v != "" {
473+
// assume it is an integer, if not it will fail later
474+
maskSize, err = strconv.Atoi(v)
475+
if err != nil {
476+
errors.Wrapf(err, "could not parse the value of the kube-controller-manager flag %s as an integer: %v", maskArg, err)
477+
return 0, err
478+
}
479+
} else if isIPv6 {
480+
maskSize = defaultNodeMaskCIDRIPv6
481+
} else {
482+
maskSize = defaultNodeMaskCIDRIPv4
483+
}
484+
return maskSize, nil
485+
}
486+
406487
// ValidateNetworking validates networking configuration
407488
func ValidateNetworking(c *kubeadm.ClusterConfiguration, fldPath *field.Path) field.ErrorList {
408489
allErrs := field.ErrorList{}
@@ -415,9 +496,13 @@ func ValidateNetworking(c *kubeadm.ClusterConfiguration, fldPath *field.Path) fi
415496

416497
if len(c.Networking.ServiceSubnet) != 0 {
417498
allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.ServiceSubnet, constants.MinimumAddressesInServiceSubnet, isDualStack, field.NewPath("serviceSubnet"))...)
499+
// Service subnet was already validated, we need to validate now the subnet size
500+
allErrs = append(allErrs, ValidateServiceSubnetSize(c.Networking.ServiceSubnet, field.NewPath("serviceSubnet"))...)
418501
}
419502
if len(c.Networking.PodSubnet) != 0 {
420-
allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.PodSubnet, constants.MinimumAddressesInServiceSubnet, isDualStack, field.NewPath("podSubnet"))...)
503+
allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.PodSubnet, constants.MinimumAddressesInPodSubnet, isDualStack, field.NewPath("podSubnet"))...)
504+
// Pod subnet was already validated, we need to validate now against the node-mask
505+
allErrs = append(allErrs, ValidatePodSubnetNodeMask(c.Networking.PodSubnet, c, field.NewPath("podSubnet"))...)
421506
}
422507
return allErrs
423508
}

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

Lines changed: 245 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ import (
2525
"k8s.io/apimachinery/pkg/util/sets"
2626
"k8s.io/apimachinery/pkg/util/validation/field"
2727
"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
28+
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
2829
kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
30+
"k8s.io/kubernetes/cmd/kubeadm/app/features"
2931
)
3032

3133
func TestValidateToken(t *testing.T) {
@@ -231,6 +233,91 @@ func TestValidateIPNetFromString(t *testing.T) {
231233
}
232234
}
233235

236+
func TestValidatePodSubnetNodeMask(t *testing.T) {
237+
var tests = []struct {
238+
name string
239+
subnet string
240+
cmExtraArgs map[string]string
241+
checkDualStack bool
242+
expected bool
243+
}{
244+
{"single IPv4, but mask too small. Default node-mask", "10.0.0.16/29", nil, false, false},
245+
{"single IPv4, but mask too small. Configured node-mask", "10.0.0.16/24", map[string]string{"node-cidr-mask-size": "23"}, false, false},
246+
{"single IPv6, but mask too small. Default node-mask", "2001:db8::1/112", nil, false, false},
247+
{"single IPv6, but mask too small. Configured node-mask", "2001:db8::1/64", map[string]string{"node-cidr-mask-size": "24"}, false, false},
248+
{"single IPv6, but mask difference greater than 16. Default node-mask", "2001:db8::1/12", nil, false, false},
249+
{"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},
250+
{"single IPv4 CIDR", "10.0.0.16/12", nil, false, true},
251+
{"single IPv6 CIDR", "2001:db8::/48", nil, false, true},
252+
// dual-stack:
253+
{"dual IPv4 only, but mask too small. Default node-mask", "10.0.0.16/29", nil, true, false},
254+
{"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},
255+
{"dual IPv6 only, but mask too small. Default node-mask", "2001:db8::1/112", nil, true, false},
256+
{"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},
257+
{"dual IPv6 only, but mask difference greater than 16. Default node-mask", "2001:db8::1/12", nil, true, false},
258+
{"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},
259+
{"dual IPv4 only CIDR", "10.0.0.16/12", nil, true, true},
260+
{"dual IPv6 only CIDR", "2001:db8::/48", nil, true, true},
261+
{"dual, but IPv4 mask too small. Default node-mask", "10.0.0.16/29,2001:db8::/48", nil, true, false},
262+
{"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},
263+
{"dual, but IPv6 mask too small. Default node-mask", "2001:db8::1/112,10.0.0.16/16", nil, true, false},
264+
{"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},
265+
{"dual, but mask difference greater than 16. Default node-mask", "2001:db8::1/12,10.0.0.16/16", nil, true, false},
266+
{"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},
267+
{"dual IPv4 IPv6", "2001:db8::/48,10.0.0.16/12", nil, true, true},
268+
{"dual IPv6 IPv4", "2001:db8::/48,10.0.0.16/12", nil, true, true},
269+
}
270+
for _, rt := range tests {
271+
cfg := &kubeadmapi.ClusterConfiguration{
272+
FeatureGates: map[string]bool{features.IPv6DualStack: rt.checkDualStack},
273+
ControllerManager: kubeadmapi.ControlPlaneComponent{
274+
ExtraArgs: rt.cmExtraArgs,
275+
},
276+
}
277+
actual := ValidatePodSubnetNodeMask(rt.subnet, cfg, nil)
278+
if (len(actual) == 0) != rt.expected {
279+
t.Errorf(
280+
"%s test case failed :\n\texpected: %t\n\t actual: %t\n\t err(s): %v\n\t",
281+
rt.name,
282+
rt.expected,
283+
(len(actual) == 0),
284+
actual,
285+
)
286+
}
287+
}
288+
}
289+
290+
func TestValidateServiceSubnetSize(t *testing.T) {
291+
var tests = []struct {
292+
name string
293+
subnet string
294+
expected bool
295+
}{
296+
{"single IPv4, but mask too large.", "10.0.0.16/2", false},
297+
{"single IPv6, but mask too large.", "2001:db8::1/64", false},
298+
{"single IPv4 CIDR", "10.0.0.16/12", true},
299+
{"single IPv6 CIDR", "2001:db8::/112", true},
300+
// dual-stack:
301+
{"dual, but IPv4 mask too large.", "2001:db8::1/112,10.0.0.16/6", false},
302+
{"dual, but IPv6 mask too large.", "2001:db8::1/12,10.0.0.16/16", false},
303+
{"dual IPv4 IPv6", "10.0.0.16/12,2001:db8::/112", true},
304+
{"dual IPv6 IPv4", "2001:db8::/112,10.0.0.16/12", true},
305+
}
306+
for _, rt := range tests {
307+
308+
actual := ValidateServiceSubnetSize(rt.subnet, nil)
309+
if (len(actual) == 0) != rt.expected {
310+
t.Errorf(
311+
"%s test case failed :\n\texpected: %t\n\t actual: %t\n\t err(s): %v\n\t",
312+
rt.name,
313+
rt.expected,
314+
(len(actual) == 0),
315+
actual,
316+
)
317+
}
318+
}
319+
}
320+
234321
func TestValidateHostPort(t *testing.T) {
235322
var tests = []struct {
236323
name string
@@ -465,7 +552,7 @@ func TestValidateInitConfiguration(t *testing.T) {
465552
},
466553
},
467554
Networking: kubeadm.Networking{
468-
ServiceSubnet: "2001:db8::1/98",
555+
ServiceSubnet: "2001:db8::1/112",
469556
DNSDomain: "cluster.local",
470557
},
471558
CertificatesDir: "/some/other/cert/dir",
@@ -1040,3 +1127,160 @@ func TestValidateEtcd(t *testing.T) {
10401127
}
10411128
}
10421129
}
1130+
1131+
func TestGetClusterNodeMask(t *testing.T) {
1132+
tests := []struct {
1133+
name string
1134+
cfg *kubeadmapi.ClusterConfiguration
1135+
isIPv6 bool
1136+
expectedMask int
1137+
expectedError bool
1138+
}{
1139+
{
1140+
name: "ipv4 default mask",
1141+
cfg: &kubeadmapi.ClusterConfiguration{},
1142+
isIPv6: false,
1143+
expectedMask: 24,
1144+
},
1145+
{
1146+
name: "ipv4 custom mask",
1147+
cfg: &kubeadmapi.ClusterConfiguration{
1148+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1149+
ExtraArgs: map[string]string{"node-cidr-mask-size": "23"},
1150+
},
1151+
},
1152+
isIPv6: false,
1153+
expectedMask: 23,
1154+
},
1155+
{
1156+
name: "ipv4 wrong mask",
1157+
cfg: &kubeadmapi.ClusterConfiguration{
1158+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1159+
ExtraArgs: map[string]string{"node-cidr-mask-size": "aa23"},
1160+
},
1161+
},
1162+
isIPv6: false,
1163+
expectedError: true,
1164+
},
1165+
{
1166+
name: "ipv6 default mask",
1167+
cfg: &kubeadmapi.ClusterConfiguration{},
1168+
isIPv6: true,
1169+
expectedMask: 64,
1170+
},
1171+
{
1172+
name: "ipv6 custom mask",
1173+
cfg: &kubeadmapi.ClusterConfiguration{
1174+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1175+
ExtraArgs: map[string]string{"node-cidr-mask-size": "83"},
1176+
},
1177+
},
1178+
isIPv6: true,
1179+
expectedMask: 83,
1180+
},
1181+
{
1182+
name: "dual ipv4 default mask",
1183+
cfg: &kubeadmapi.ClusterConfiguration{
1184+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1185+
},
1186+
isIPv6: false,
1187+
expectedMask: 24,
1188+
},
1189+
{
1190+
name: "dual ipv4 custom mask",
1191+
cfg: &kubeadmapi.ClusterConfiguration{
1192+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1193+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1194+
ExtraArgs: map[string]string{"node-cidr-mask-size": "21", "node-cidr-mask-size-ipv4": "23"},
1195+
},
1196+
},
1197+
isIPv6: false,
1198+
expectedMask: 23,
1199+
},
1200+
{
1201+
name: "dual ipv6 default mask",
1202+
cfg: &kubeadmapi.ClusterConfiguration{
1203+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1204+
},
1205+
isIPv6: true,
1206+
expectedMask: 64,
1207+
},
1208+
{
1209+
name: "dual ipv6 custom mask",
1210+
cfg: &kubeadmapi.ClusterConfiguration{
1211+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1212+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1213+
ExtraArgs: map[string]string{"node-cidr-mask-size-ipv6": "83"},
1214+
},
1215+
},
1216+
isIPv6: true,
1217+
expectedMask: 83,
1218+
},
1219+
{
1220+
name: "dual ipv4 custom mask",
1221+
cfg: &kubeadmapi.ClusterConfiguration{
1222+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1223+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1224+
ExtraArgs: map[string]string{"node-cidr-mask-size-ipv4": "23"},
1225+
},
1226+
},
1227+
isIPv6: false,
1228+
expectedMask: 23,
1229+
},
1230+
{
1231+
name: "dual ipv4 wrong mask",
1232+
cfg: &kubeadmapi.ClusterConfiguration{
1233+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1234+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1235+
ExtraArgs: map[string]string{"node-cidr-mask-size-ipv4": "aa"},
1236+
},
1237+
},
1238+
isIPv6: false,
1239+
expectedError: true,
1240+
},
1241+
{
1242+
name: "dual ipv6 default mask and legacy flag",
1243+
cfg: &kubeadmapi.ClusterConfiguration{
1244+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1245+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1246+
ExtraArgs: map[string]string{"node-cidr-mask-size": "23"},
1247+
},
1248+
},
1249+
isIPv6: true,
1250+
expectedMask: 64,
1251+
},
1252+
{
1253+
name: "dual ipv6 custom mask and legacy flag",
1254+
cfg: &kubeadmapi.ClusterConfiguration{
1255+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1256+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1257+
ExtraArgs: map[string]string{"node-cidr-mask-size": "23", "node-cidr-mask-size-ipv6": "83"},
1258+
},
1259+
},
1260+
isIPv6: true,
1261+
expectedMask: 83,
1262+
},
1263+
{
1264+
name: "dual ipv6 custom mask and wrong flag",
1265+
cfg: &kubeadmapi.ClusterConfiguration{
1266+
FeatureGates: map[string]bool{features.IPv6DualStack: true},
1267+
ControllerManager: kubeadmapi.ControlPlaneComponent{
1268+
ExtraArgs: map[string]string{"node-cidr-mask-size": "23", "node-cidr-mask-size-ipv6": "a83"},
1269+
},
1270+
},
1271+
isIPv6: true,
1272+
expectedError: true,
1273+
},
1274+
}
1275+
for _, test := range tests {
1276+
t.Run(test.name, func(t *testing.T) {
1277+
mask, err := getClusterNodeMask(test.cfg, test.isIPv6)
1278+
if (err == nil) == test.expectedError {
1279+
t.Errorf("expected error: %v, got %v", test.expectedError, err)
1280+
}
1281+
if mask != test.expectedMask {
1282+
t.Errorf("expected mask: %d, got %d", test.expectedMask, mask)
1283+
}
1284+
})
1285+
}
1286+
}

0 commit comments

Comments
 (0)