Merge pull request #51906 from guangxuli/federation_validation

Automatic merge from submit-queue (batch tested with PRs 51929, 52015, 51906, 52069, 51542). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

validate federation cluster spec CIDR

**What this PR does / why we need it**:
Add extra validation for cluster CIDR.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
fix https://github.com/kubernetes/kubernetes/issues/29911

**Special notes for your reviewer**:
none

**Release note**:
none

federation/apis/federation/validation/BUILD

@@ -24,6 +24,7 @@ go_test(
         "//federation/apis/federation:go_default_library",
         "//pkg/api:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],
 )
 

federation/apis/federation/validation/validation.go

@@ -17,6 +17,9 @@ limitations under the License.
 package validation
 
 import (
+	"fmt"
+	"net"
+
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/federation/apis/federation"
 	"k8s.io/kubernetes/pkg/api/validation"
@@ -27,6 +30,15 @@ func ValidateClusterSpec(spec *federation.ClusterSpec, fieldPath *field.Path) fi
 	// address is required.
 	if len(spec.ServerAddressByClientCIDRs) == 0 {
 		allErrs = append(allErrs, field.Required(fieldPath.Child("serverAddressByClientCIDRs"), ""))
+	} else {
+		for i, address := range spec.ServerAddressByClientCIDRs {
+			idxPath := fieldPath.Child("serverAddressByClientCIDRs").Index(i)
+			if len(address.ClientCIDR) > 0 {
+				if _, _, err := net.ParseCIDR(address.ClientCIDR); err != nil {
+					allErrs = append(allErrs, field.Invalid(idxPath.Child("clientCIDR"), address.ClientCIDR, fmt.Sprintf("must be a valid CIDR: %v", err)))
+				}
+			}
+		}
 	}
 	return allErrs
 }

federation/apis/federation/validation/validation_test.go

@@ -20,10 +20,123 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/federation/apis/federation"
 	"k8s.io/kubernetes/pkg/api"
 )
 
+func TestValidateClusterSpec(t *testing.T) {
+	type validateClusterSpecTest struct {
+		testName string
+		spec     *federation.ClusterSpec
+		path     *field.Path
+	}
+
+	successCases := []validateClusterSpecTest{
+		{
+			testName: "normal CIDR",
+			spec: &federation.ClusterSpec{
+				ServerAddressByClientCIDRs: []federation.ServerAddressByClientCIDR{
+					{
+						ClientCIDR:    "0.0.0.0/0",
+						ServerAddress: "localhost:8888",
+					},
+				},
+			},
+			path: field.NewPath("spec"),
+		},
+		{
+			testName: "missing CIDR",
+			spec: &federation.ClusterSpec{
+				ServerAddressByClientCIDRs: []federation.ServerAddressByClientCIDR{
+					{
+						ClientCIDR:    "",
+						ServerAddress: "localhost:8888",
+					},
+				},
+			},
+			path: field.NewPath("spec"),
+		},
+		{
+			testName: "no host in CIDR",
+			spec: &federation.ClusterSpec{
+				ServerAddressByClientCIDRs: []federation.ServerAddressByClientCIDR{
+					{
+						ClientCIDR:    "0.0.0.0/32",
+						ServerAddress: "localhost:8888",
+					},
+				},
+			},
+			path: field.NewPath("spec"),
+		},
+	}
+	for _, successCase := range successCases {
+		errs := ValidateClusterSpec(successCase.spec, successCase.path)
+		if len(errs) != 0 {
+			t.Errorf("expect success for testname: %q  but got: %v", successCase.testName, errs)
+		}
+	}
+
+	errorCases := []validateClusterSpecTest{
+		{
+			testName: "invalid CIDR : network missing",
+			spec: &federation.ClusterSpec{
+				ServerAddressByClientCIDRs: []federation.ServerAddressByClientCIDR{
+					{
+						ClientCIDR:    "0.0.0.0",
+						ServerAddress: "localhost:8888",
+					},
+				},
+			},
+			path: field.NewPath("spec"),
+		},
+		{
+			testName: "invalid CIDR : invalid address value",
+			spec: &federation.ClusterSpec{
+				ServerAddressByClientCIDRs: []federation.ServerAddressByClientCIDR{
+					{
+						ClientCIDR:    "256.0.0.0/16",
+						ServerAddress: "localhost:8888",
+					},
+				},
+			},
+			path: field.NewPath("spec"),
+		},
+		{
+			testName: "invalid CIDR : invalid address formation",
+			spec: &federation.ClusterSpec{
+				ServerAddressByClientCIDRs: []federation.ServerAddressByClientCIDR{
+					{
+						ClientCIDR:    "0.0.0/16",
+						ServerAddress: "localhost:8888",
+					},
+				},
+			},
+			path: field.NewPath("spec"),
+		},
+		{
+			testName: "invalid CIDR : invalid network num",
+			spec: &federation.ClusterSpec{
+				ServerAddressByClientCIDRs: []federation.ServerAddressByClientCIDR{
+					{
+						ClientCIDR:    "0.0.0.0/33",
+						ServerAddress: "localhost:8888",
+					},
+				},
+			},
+			path: field.NewPath("spec"),
+		},
+	}
+
+	for _, errorCase := range errorCases {
+		errs := ValidateClusterSpec(errorCase.spec, errorCase.path)
+		if len(errs) == 0 {
+			t.Errorf("expect failure for testname : %q", errorCase.testName)
+		}
+	}
+
+}
+
 func TestValidateCluster(t *testing.T) {
 	successCases := []federation.Cluster{
 		{