Merge pull request #288 from kubernetes-sigs/prefix-0

Allow IPPool to have Prefix /0

Author: k8s-ci-robot

api/v1alpha2/inclusterippool_types.go

@@ -28,6 +28,7 @@ type InClusterIPPoolSpec struct {
 
 	// Prefix is the network prefix to use.
 	// +kubebuilder:validation:Maximum=128
+	// +kubebuilder:validation:Minimum=0
 	Prefix int `json:"prefix"`
 
 	// Gateway

config/crd/bases/ipam.cluster.x-k8s.io_globalinclusterippools.yaml

@@ -217,6 +217,7 @@ spec:
               prefix:
                 description: Prefix is the network prefix to use.
                 maximum: 128
+                minimum: 0
                 type: integer
             required:
             - addresses

config/crd/bases/ipam.cluster.x-k8s.io_inclusterippools.yaml

@@ -215,6 +215,7 @@ spec:
               prefix:
                 description: Prefix is the network prefix to use.
                 maximum: 128
+                minimum: 0
                 type: integer
             required:
             - addresses

internal/poolutil/pool_test.go

@@ -28,6 +28,23 @@ import (
 )
 
 var _ = Describe("PoolSpecToIPSet", func() {
+	It("should allow a pool spec with prefix /0", func() {
+		spec := &v1alpha2.InClusterIPPoolSpec{
+			Gateway: "192.168.0.1",
+			Prefix:  0,
+			Addresses: []string{
+				"192.168.0.3-192.168.0.10",
+			},
+		}
+		ipSet, err := PoolSpecToIPSet(spec)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(ipSet.Contains(mustParse("192.168.0.3"))).To(BeTrue())
+		Expect(ipSet.Contains(mustParse("192.168.0.3"))).To(BeTrue())
+		Expect(ipSet.Contains(mustParse("192.168.0.5"))).To(BeTrue())
+		Expect(ipSet.Contains(mustParse("192.168.0.6"))).To(BeTrue())
+		Expect(ipSet.Contains(mustParse("192.168.0.7"))).To(BeTrue())
+		Expect(IPSetCount(ipSet)).To(Equal(8))
+	})
 	It("converts a pool spec to a set", func() {
 		spec := &v1alpha2.InClusterIPPoolSpec{
 			Gateway: "192.168.0.1",

internal/webhooks/inclusterippool.go

@@ -185,8 +185,8 @@ func (webhook *InClusterIPPool) validate(_, newPool types.GenericInClusterPool)
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "addresses"), newPool.PoolSpec().Addresses, "addresses is required"))
 	}
 
-	if newPool.PoolSpec().Prefix == 0 {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "prefix"), newPool.PoolSpec().Prefix, "a valid prefix is required"))
+	if newPool.PoolSpec().Prefix < 0 {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "prefix"), newPool.PoolSpec().Addresses, "a valid prefix is required"))
 	}
 
 	var hasIPv4Addr, hasIPv6Addr bool

internal/webhooks/inclusterippool_test.go

@@ -419,12 +419,13 @@ func TestInvalidScenarios(t *testing.T) {
 			expectedError: "provided address is not a valid IP, range, nor CIDR",
 		},
 		{
-			testcase: "omitting a prefix should not be allowed",
+			testcase: "negative prefix not allowed",
 			spec: v1alpha2.InClusterIPPoolSpec{
 				Addresses: []string{
 					"10.0.0.25",
 					"10.0.0.26",
 				},
+				Prefix:  -1,
 				Gateway: "10.0.0.1",
 			},
 			expectedError: "a valid prefix is required",
@@ -648,6 +649,63 @@ func TestInvalidScenarios(t *testing.T) {
 	}
 }
 
+func TestIPPool_Prefix(t *testing.T) {
+	g := NewWithT(t)
+
+	scheme := runtime.NewScheme()
+	g.Expect(ipamv1.AddToScheme(scheme)).To(Succeed())
+
+	namespacedPool := &v1alpha2.InClusterIPPool{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my-pool",
+		},
+		Spec: v1alpha2.InClusterIPPoolSpec{
+			Addresses: []string{"10.0.0.10-10.0.0.20"},
+			Prefix:    0,
+			Gateway:   "10.0.0.1",
+		},
+	}
+
+	globalPool := &v1alpha2.GlobalInClusterIPPool{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my-pool",
+		},
+		Spec: v1alpha2.InClusterIPPoolSpec{
+			Addresses: []string{"10.0.0.10-10.0.0.20"},
+			Prefix:    0,
+			Gateway:   "10.0.0.1",
+		},
+	}
+	emptyPrefixPool := &v1alpha2.InClusterIPPool{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my-pool",
+		},
+		Spec: v1alpha2.InClusterIPPoolSpec{
+			Addresses: []string{"10.0.0.10-10.0.0.20"},
+			Gateway:   "10.0.0.1",
+		},
+	}
+
+	ips := []client.Object{
+		createIP("my-ip", "10.0.0.10", namespacedPool),
+		createIP("my-ip-2", "10.0.0.10", globalPool),
+	}
+
+	fakeClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(ips...).
+		WithIndex(&ipamv1.IPAddress{}, index.IPAddressPoolRefCombinedField, index.IPAddressByCombinedPoolRef).
+		Build()
+
+	webhook := InClusterIPPool{
+		Client: fakeClient,
+	}
+
+	g.Expect(webhook.validate(&v1alpha2.InClusterIPPool{}, namespacedPool)).Error().To(BeNil(), "should allow /0 prefix InClusterIPPool")
+	g.Expect(webhook.validate(&v1alpha2.GlobalInClusterIPPool{}, globalPool)).Error().To(BeNil(), "should allow /0 prefix GlobalInClusterIPPool")
+	g.Expect(webhook.validate(&v1alpha2.InClusterIPPool{}, emptyPrefixPool)).Error().To(BeNil(), "should allow empty prefix InClusterIPPool")
+}
+
 func runInvalidScenarioTests(t *testing.T, tt invalidScenarioTest, pool types.GenericInClusterPool, webhook InClusterIPPool) {
 	t.Helper()
 	t.Run(tt.testcase, func(t *testing.T) {