Browse code

Disable ingress ip when cloud provider enabled

Maru Newby authored on 2016/08/17 00:44:03
Showing 5 changed files
... ...
@@ -568,3 +568,15 @@ func HasKubernetesAPIVersion(config KubernetesMasterConfig, groupVersion unversi
568 568
 	enabledVersions := GetEnabledAPIVersionsForGroup(config, groupVersion.Group)
569 569
 	return sets.NewString(enabledVersions...).Has(groupVersion.Version)
570 570
 }
571
+
572
+func CIDRsOverlap(cidr1, cidr2 string) bool {
573
+	_, ipNet1, err := net.ParseCIDR(cidr1)
574
+	if err != nil {
575
+		return false
576
+	}
577
+	_, ipNet2, err := net.ParseCIDR(cidr2)
578
+	if err != nil {
579
+		return false
580
+	}
581
+	return ipNet1.Contains(ipNet2.IP) || ipNet2.Contains(ipNet1.IP)
582
+}
... ...
@@ -157,12 +157,8 @@ func ValidateMasterConfig(config *api.MasterConfig, fldPath *field.Path) Validat
157 157
 			}
158 158
 		}
159 159
 	}
160
-	if len(config.NetworkConfig.IngressIPNetworkCIDR) > 0 {
161
-		cidr := config.NetworkConfig.IngressIPNetworkCIDR
162
-		if _, ipNet, err := net.ParseCIDR(cidr); err != nil || ipNet.IP.IsUnspecified() {
163
-			validationResults.AddErrors(field.Invalid(fldPath.Child("networkConfig", "ingressIPNetworkCIDR").Index(0), cidr, "must be a valid CIDR notation IP range (e.g. 172.30.0.0/16)"))
164
-		}
165
-	}
160
+
161
+	validationResults.AddErrors(ValidateIngressIPNetworkCIDR(config, fldPath.Child("networkConfig", "ingressIPNetworkCIDR").Index(0))...)
166 162
 
167 163
 	validationResults.AddErrors(ValidateKubeConfig(config.MasterClients.OpenShiftLoopbackKubeConfig, fldPath.Child("masterClients", "openShiftLoopbackKubeConfig"))...)
168 164
 
... ...
@@ -641,3 +637,38 @@ func ValidateAdmissionPluginConfigConflicts(masterConfig *api.MasterConfig) Vali
641 641
 
642 642
 	return validationResults
643 643
 }
644
+
645
+func ValidateIngressIPNetworkCIDR(config *api.MasterConfig, fldPath *field.Path) field.ErrorList {
646
+	errors := field.ErrorList{}
647
+
648
+	cidr := config.NetworkConfig.IngressIPNetworkCIDR
649
+
650
+	if len(cidr) == 0 {
651
+		return errors
652
+	}
653
+
654
+	addError := func(errMessage string) {
655
+		errors = append(errors, field.Invalid(fldPath, cidr, errMessage))
656
+	}
657
+
658
+	// TODO Detect cloud provider when not using built-in kubernetes
659
+	kubeConfig := config.KubernetesMasterConfig
660
+	noCloudProvider := kubeConfig != nil && (len(kubeConfig.ControllerArguments["cloud-provider"]) == 0 || kubeConfig.ControllerArguments["cloud-provider"][0] == "")
661
+
662
+	if noCloudProvider {
663
+		if _, ipNet, err := net.ParseCIDR(cidr); err != nil || ipNet.IP.IsUnspecified() {
664
+			addError("must be a valid CIDR notation IP range (e.g. 172.30.0.0/16)")
665
+		} else {
666
+			if api.CIDRsOverlap(cidr, config.NetworkConfig.ClusterNetworkCIDR) {
667
+				addError("conflicts with cluster network CIDR")
668
+			}
669
+			if api.CIDRsOverlap(cidr, config.NetworkConfig.ServiceNetworkCIDR) {
670
+				addError("conflicts with service network CIDR")
671
+			}
672
+		}
673
+	} else {
674
+		addError("should not be provided when a cloud-provider is enabled")
675
+	}
676
+
677
+	return errors
678
+}
... ...
@@ -421,3 +421,66 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
421 421
 		}
422 422
 	}
423 423
 }
424
+
425
+func TestValidateIngressIPNetworkCIDR(t *testing.T) {
426
+	testCases := []struct {
427
+		testName      string
428
+		cidr          string
429
+		serviceCIDR   string
430
+		clusterCIDR   string
431
+		cloudProvider string
432
+		errorCount    int
433
+	}{
434
+		{
435
+			testName: "No CIDR",
436
+		},
437
+		{
438
+			testName:   "No cloud provider and invalid cidr",
439
+			cidr:       "foo",
440
+			errorCount: 1,
441
+		},
442
+		{
443
+			testName:   "No cloud provider and unspecified cidr",
444
+			cidr:       "0.0.0.0/32",
445
+			errorCount: 1,
446
+		},
447
+		{
448
+			testName:    "No cloud provider and conflicting cidrs",
449
+			cidr:        "172.16.0.0/16",
450
+			serviceCIDR: "172.16.0.0/16",
451
+			clusterCIDR: "172.16.0.0/16",
452
+			errorCount:  2,
453
+		},
454
+		{
455
+			testName:      "CIDR specified but cloud provider enabled",
456
+			cidr:          "172.16.0.0/16",
457
+			cloudProvider: "foo",
458
+			errorCount:    1,
459
+		},
460
+		{
461
+			testName:    "No cloud provider and valid, non-conflicting cidr",
462
+			cidr:        "172.16.0.0/16",
463
+			serviceCIDR: "172.17.0.0/16",
464
+			clusterCIDR: "172.18.0.0/16",
465
+		},
466
+	}
467
+	for _, test := range testCases {
468
+		config := &configapi.MasterConfig{
469
+			KubernetesMasterConfig: &configapi.KubernetesMasterConfig{
470
+				ControllerArguments: configapi.ExtendedArguments{
471
+					"cloud-provider": []string{test.cloudProvider},
472
+				},
473
+			},
474
+			NetworkConfig: configapi.MasterNetworkConfig{
475
+				IngressIPNetworkCIDR: test.cidr,
476
+				ServiceNetworkCIDR:   test.serviceCIDR,
477
+				ClusterNetworkCIDR:   test.clusterCIDR,
478
+			},
479
+		}
480
+		errors := ValidateIngressIPNetworkCIDR(config, nil)
481
+		errorCount := len(errors)
482
+		if test.errorCount != errorCount {
483
+			t.Errorf("%s: expected %d errors, got %d", test.testName, test.errorCount, errorCount)
484
+		}
485
+	}
486
+}
... ...
@@ -482,7 +482,6 @@ func newAdmissionChain(pluginNames []string, admissionConfigFilename string, plu
482 482
 				// should have been caught with validation
483 483
 				return nil, err
484 484
 			}
485
-			// TODO need to disallow if a cloud provider is configured
486 485
 			allowIngressIP := len(options.NetworkConfig.IngressIPNetworkCIDR) > 0
487 486
 			plugins = append(plugins, serviceadmit.NewExternalIPRanger(reject, admit, allowIngressIP))
488 487
 
... ...
@@ -530,7 +530,6 @@ func (c *MasterConfig) RunClusterQuotaReconciliationController() {
530 530
 
531 531
 // RunIngressIPController starts the ingress ip controller if IngressIPNetworkCIDR is configured.
532 532
 func (c *MasterConfig) RunIngressIPController(client *kclient.Client) {
533
-	// TODO need to disallow if a cloud provider is configured
534 533
 	if len(c.Options.NetworkConfig.IngressIPNetworkCIDR) == 0 {
535 534
 		return
536 535
 	}