update ServiceCIDR API: replace IPv4,IPv6 fields by an array of strings

https: //docs.google.com/document/d/1-CKDPRHsG6XfnGs-Bb7ptWgDtTq9-hj8aJQpGz1CSh8/edit

Change-Id: If7134a5553b4ecc12b1d58ffc52ebb9f8594778a

api/api-rules/violation_exceptions.list

@@ -179,6 +179,7 @@ API rule violation: list_type_missing,k8s.io/api/networking/v1,NetworkPolicyIngr
 API rule violation: list_type_missing,k8s.io/api/networking/v1,NetworkPolicySpec,Egress
 API rule violation: list_type_missing,k8s.io/api/networking/v1,NetworkPolicySpec,Ingress
 API rule violation: list_type_missing,k8s.io/api/networking/v1,NetworkPolicySpec,PolicyTypes
+API rule violation: list_type_missing,k8s.io/api/networking/v1alpha1,ServiceCIDRSpec,CIDRs
 API rule violation: list_type_missing,k8s.io/api/networking/v1beta1,HTTPIngressRuleValue,Paths
 API rule violation: list_type_missing,k8s.io/api/networking/v1beta1,IngressLoadBalancerStatus,Ingress
 API rule violation: list_type_missing,k8s.io/api/networking/v1beta1,IngressSpec,Rules
@@ -355,8 +356,7 @@ API rule violation: names_match,k8s.io/api/core/v1,RBDVolumeSource,RBDPool
 API rule violation: names_match,k8s.io/api/core/v1,RBDVolumeSource,RadosUser
 API rule violation: names_match,k8s.io/api/core/v1,VolumeSource,CephFS
 API rule violation: names_match,k8s.io/api/core/v1,VolumeSource,StorageOS
-API rule violation: names_match,k8s.io/api/networking/v1alpha1,ServiceCIDRSpec,IPv4
+API rule violation: names_match,k8s.io/api/networking/v1alpha1,ServiceCIDRSpec,CIDRs
-API rule violation: names_match,k8s.io/api/networking/v1alpha1,ServiceCIDRSpec,IPv6
 API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaProps,Ref
 API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaProps,Schema
 API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaProps,XEmbeddedResource

pkg/apis/networking/fuzzer/fuzzer.go

@@ -87,10 +87,15 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 		},
 		func(obj *networking.ServiceCIDR, c fuzz.Continue) {
 			c.FuzzNoCustom(obj) // fuzz self without calling this function again
-			cidrv4 := generateRandomCIDR(false, c)
+			boolean := []bool{false, true}
-			obj.Spec.IPv4 = cidrv4
+
-			cidrv6 := generateRandomCIDR(true, c)
+			is6 := boolean[c.Rand.Intn(2)]
-			obj.Spec.IPv6 = cidrv6
+			primary := generateRandomCIDR(is6, c)
+			obj.Spec.CIDRs = []string{primary}
+
+			if boolean[c.Rand.Intn(2)] {
+				obj.Spec.CIDRs = append(obj.Spec.CIDRs, generateRandomCIDR(!is6, c))
+			}
 		},
 	}
 }

pkg/apis/networking/types.go

@@ -647,8 +647,8 @@ type IPAddressList struct {
 // +genclient:nonNamespaced
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
-// ServiceCIDR defines a range of IPs using CIDR format (192.168.0.0/24 or 2001:db2::/64).
+// ServiceCIDR defines a range of IP addresses using CIDR format (e.g. 192.168.0.0/24 or 2001:db2::/64).
-// This range is used by the cluster to allocate the ClusterIPs associated to the Services object.
+// This range is used to allocate ClusterIPs to Service objects.
 type ServiceCIDR struct {
 	metav1.TypeMeta
 	// Standard object's metadata.
@@ -665,16 +665,12 @@ type ServiceCIDR struct {
 	Status ServiceCIDRStatus
 }
 
-// ServiceCIDRSpec describe how the ServiceCIDR's specification looks like.
 type ServiceCIDRSpec struct {
-	// IPv4 defines an IPv4 IP block in CIDR notation (e.g. "192.168.0.0/24").
+	// CIDRs defines the IP blocks in CIDR notation (e.g. "192.168.0.0/24" or "2001:db8::/64")
+	// from which to assign service cluster IPs. Max of two CIDRs is allowed, one of each IP family.
 	// This field is immutable.
 	// +optional
-	IPv4 string
+	CIDRs []string
-	// IPv6 defines an IPv6 IP block in CIDR notation (e.g. "2001:db8::/64").
-	// This field is immutable.
-	// +optional
-	IPv6 string
 }
 
 // ServiceCIDRStatus describes the current state of the ServiceCIDR.

pkg/apis/networking/validation/validation.go

@@ -726,55 +726,54 @@ var ValidateServiceCIDRName = apimachineryvalidation.NameIsDNSSubdomain
 
 func ValidateServiceCIDR(cidrConfig *networking.ServiceCIDR) field.ErrorList {
 	allErrs := apivalidation.ValidateObjectMeta(&cidrConfig.ObjectMeta, false, ValidateServiceCIDRName, field.NewPath("metadata"))
+	fieldPath := field.NewPath("spec", "cidrs")
 
-	if cidrConfig.Spec.IPv4 == "" && cidrConfig.Spec.IPv6 == "" {
+	if len(cidrConfig.Spec.CIDRs) == 0 {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("Spec"), cidrConfig.Spec, "at least one CIDR required"))
+		allErrs = append(allErrs, field.Required(fieldPath, "at least one CIDR required"))
 		return allErrs
 	}
 
-	if cidrConfig.Spec.IPv4 != "" {
+	if len(cidrConfig.Spec.CIDRs) > 2 {
-		prefix, err := netip.ParsePrefix(cidrConfig.Spec.IPv4)
+		allErrs = append(allErrs, field.Invalid(fieldPath, cidrConfig.Spec, "may only hold up to 2 values"))
-		if err != nil {
+		return allErrs
-			allErrs = append(allErrs, field.Invalid(field.NewPath("IPv4"), cidrConfig.Spec.IPv4, err.Error()))
+	}
-		} else {
+	// validate cidrs are dual stack, one of each IP family
-			if prefix.Addr() != prefix.Masked().Addr() {
+	if len(cidrConfig.Spec.CIDRs) == 2 {
-				allErrs = append(allErrs, field.Invalid(field.NewPath("IPv4"), cidrConfig.Spec.IPv4, "wrong CIDR format, IP doesn't match network IP address"))
+		isDual, err := netutils.IsDualStackCIDRStrings(cidrConfig.Spec.CIDRs)
-			}
+		if err != nil || !isDual {
-			if prefix.String() != cidrConfig.Spec.IPv4 {
+			allErrs = append(allErrs, field.Invalid(fieldPath, cidrConfig.Spec, "may specify no more than one IP for each IP family, i.e 192.168.0.0/24 and 2001:db8::/64"))
-				allErrs = append(allErrs, field.Invalid(field.NewPath("IPv4"), cidrConfig.Spec.IPv4, "CIDR not in canonical format"))
+			return allErrs
-			}
-			if !prefix.Addr().Is4() {
-				allErrs = append(allErrs, field.Invalid(field.NewPath("IPv4"), cidrConfig.Spec.IPv4, "not IPv4 family CIDR"))
-			}
 		}
 	}
 
-	if cidrConfig.Spec.IPv6 != "" {
+	for i, cidr := range cidrConfig.Spec.CIDRs {
-		prefix, err := netip.ParsePrefix(cidrConfig.Spec.IPv6)
+		allErrs = append(allErrs, validateCIDR(cidr, fieldPath.Index(i))...)
-		if err != nil {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("IPv6"), cidrConfig.Spec.IPv6, err.Error()))
-		} else {
-			if prefix.Addr() != prefix.Masked().Addr() {
-				allErrs = append(allErrs, field.Invalid(field.NewPath("IPv6"), cidrConfig.Spec.IPv6, "wrong CIDR format, IP doesn't match network IP address"))
-			}
-			if prefix.String() != cidrConfig.Spec.IPv6 {
-				allErrs = append(allErrs, field.Invalid(field.NewPath("IPv6"), cidrConfig.Spec.IPv6, "CIDR not in RFC 5952 canonical format"))
-			}
-			if !prefix.Addr().Is6() {
-				allErrs = append(allErrs, field.Invalid(field.NewPath("IPv6"), cidrConfig.Spec.IPv6, "not IPv6 family CIDR"))
-			}
-		}
 	}
 
 	return allErrs
 }
 
+func validateCIDR(cidr string, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	prefix, err := netip.ParsePrefix(cidr)
+	if err != nil {
+		allErrs = append(allErrs, field.Invalid(fldPath, cidr, err.Error()))
+	} else {
+		if prefix.Addr() != prefix.Masked().Addr() {
+			allErrs = append(allErrs, field.Invalid(fldPath, cidr, "wrong CIDR format, IP doesn't match network IP address"))
+		}
+		if prefix.String() != cidr {
+			allErrs = append(allErrs, field.Invalid(fldPath, cidr, "CIDR not in RFC 5952 canonical format"))
+		}
+	}
+	return allErrs
+}
+
 // ValidateServiceCIDRUpdate tests if an update to a ServiceCIDR is valid.
 func ValidateServiceCIDRUpdate(update, old *networking.ServiceCIDR) field.ErrorList {
 	var allErrs field.ErrorList
 	allErrs = append(allErrs, apivalidation.ValidateObjectMetaUpdate(&update.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata"))...)
-	allErrs = append(allErrs, apivalidation.ValidateImmutableField(update.Spec.IPv4, old.Spec.IPv4, field.NewPath("spec").Child("ipv4"))...)
+	allErrs = append(allErrs, apivalidation.ValidateImmutableField(update.Spec.CIDRs, old.Spec.CIDRs, field.NewPath("spec").Child("cidrs"))...)
-	allErrs = append(allErrs, apivalidation.ValidateImmutableField(update.Spec.IPv6, old.Spec.IPv6, field.NewPath("spec").Child("ipv6"))...)
 
 	return allErrs
 }

pkg/apis/networking/validation/validation_test.go

@@ -2061,6 +2061,17 @@ func TestValidateServiceCIDR(t *testing.T) {
 				},
 			},
 		},
+		"three-ipranges": {
+			expectedErrors: 1,
+			ipRange: &networking.ServiceCIDR{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-name",
+				},
+				Spec: networking.ServiceCIDRSpec{
+					CIDRs: []string{"192.168.0.0/24", "fd00::/64", "10.0.0.0/16"},
+				},
+			},
+		},
 		"good-iprange-ipv4": {
 			expectedErrors: 0,
 			ipRange: &networking.ServiceCIDR{
@@ -2068,7 +2079,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv4: "192.168.0.0/24",
+					CIDRs: []string{"192.168.0.0/24"},
 				},
 			},
 		},
@@ -2079,7 +2090,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv6: "fd00:1234::/64",
+					CIDRs: []string{"fd00:1234::/64"},
 				},
 			},
 		},
@@ -2090,8 +2101,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv4: "192.168.0.0/24",
+					CIDRs: []string{"192.168.0.0/24", "fd00:1234::/64"},
-					IPv6: "fd00:1234::/64",
 				},
 			},
 		},
@@ -2102,7 +2112,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv4: "sadasdsad",
+					CIDRs: []string{"asdasdasd"},
 				},
 			},
 		},
@@ -2113,7 +2123,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv4: "192.168.0.1",
+					CIDRs: []string{"192.168.0.1"},
 				},
 			},
 		},
@@ -2124,7 +2134,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv4: "192.168.0.1/24",
+					CIDRs: []string{"192.168.0.1/24"},
 				},
 			},
 		},
@@ -2135,7 +2145,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv6: "fd00:1234::2/64",
+					CIDRs: []string{"fd00:1234::2/64"},
 				},
 			},
 		},
@@ -2146,7 +2156,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv6: "FD00:1234::2/64",
+					CIDRs: []string{"FD00:1234::2/64"},
 				},
 			},
 		},
@@ -2157,8 +2167,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv4: "192.168.0.0/24",
+					CIDRs: []string{"192.168.0.0/24", "FD00:1234::/64"},
-					IPv6: "FD00:1234::/64",
 				},
 			},
 		},
@@ -2169,8 +2178,7 @@ func TestValidateServiceCIDR(t *testing.T) {
 					Name: "test-name",
 				},
 				Spec: networking.ServiceCIDRSpec{
-					IPv4: "192.168.007.0/24",
+					CIDRs: []string{"192.168.007.0/24", "fd00:1234::/64"},
-					IPv6: "fd00:1234::/64",
 				},
 			},
 		},
@@ -2193,8 +2201,7 @@ func TestValidateServiceCIDRUpdate(t *testing.T) {
 			ResourceVersion: "1",
 		},
 		Spec: networking.ServiceCIDRSpec{
-			IPv4: "192.168.0.0/24",
+			CIDRs: []string{"192.168.0.0/24", "fd00:1234::/64"},
-			IPv6: "2001:db2::/64",
 		},
 	}
 
@@ -2213,27 +2220,18 @@ func TestValidateServiceCIDRUpdate(t *testing.T) {
 		},
 
 		{
-			name: "Failed update, update spec.IPv4",
+			name: "Failed update, update spec.CIDRs single stack",
 			svc: func(svc *networking.ServiceCIDR) *networking.ServiceCIDR {
 				out := svc.DeepCopy()
-				out.Spec.IPv4 = "10.0.0.0/16"
+				out.Spec.CIDRs = []string{"10.0.0.0/16"}
 				return out
 			}, expectErr: true,
 		},
 		{
-			name: "Failed update, update spec.IPv6",
+			name: "Failed update, update spec.CIDRs dual stack",
 			svc: func(svc *networking.ServiceCIDR) *networking.ServiceCIDR {
 				out := svc.DeepCopy()
-				out.Spec.IPv6 = "fd00:1:2:3::/64"
+				out.Spec.CIDRs = []string{"10.0.0.0/24", "fd00:1234::/64"}
-				return out
-			}, expectErr: true,
-		},
-		{
-			name: "Failed update, update spec.IPv4 and spec.IPv6",
-			svc: func(svc *networking.ServiceCIDR) *networking.ServiceCIDR {
-				out := svc.DeepCopy()
-				out.Spec.IPv4 = "10.0.0.0/16"
-				out.Spec.IPv6 = "fd00:1:2:3::/64"
 				return out
 			}, expectErr: true,
 		},

staging/src/k8s.io/api/networking/v1alpha1/types.go

@@ -87,8 +87,8 @@ type IPAddressList struct {
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +k8s:prerelease-lifecycle-gen:introduced=1.27
 
-// ServiceCIDR defines a range of IPs using CIDR format (192.168.0.0/24 or 2001:db2::/64).
+// ServiceCIDR defines a range of IP addresses using CIDR format (e.g. 192.168.0.0/24 or 2001:db2::/64).
-// This range is used by the cluster to allocate the ClusterIPs associated to the Services object.
+// This range is used to allocate ClusterIPs to Service objects.
 type ServiceCIDR struct {
 	metav1.TypeMeta `json:",inline"`
 	// Standard object's metadata.
@@ -107,14 +107,11 @@ type ServiceCIDR struct {
 
 // ServiceCIDRSpec define the CIDRs the user wants to use for allocating ClusterIPs for Services.
 type ServiceCIDRSpec struct {
-	// IPv4 defines an IPv4 IP block in CIDR notation (e.g. "192.168.0.0/24").
+	// CIDRs defines the IP blocks in CIDR notation (e.g. "192.168.0.0/24" or "2001:db8::/64")
+	// from which to assign service cluster IPs. Max of two CIDRs is allowed, one of each IP family.
 	// This field is immutable.
 	// +optional
-	IPv4 string `json:"ipv4,omitempty" protobuf:"bytes,1,opt,name=ipv4"`
+	CIDRs []string `json:"cidrs,omitempty" protobuf:"bytes,1,opt,name=cidrs"`
-	// IPv6 defines an IPv6 IP block in CIDR notation (e.g. "2001:db8::/64").
-	// This field is immutable.
-	// +optional
-	IPv6 string `json:"ipv6,omitempty" protobuf:"bytes,2,opt,name=ipv6"`
 }
 
 const (