diff --git a/api/v1alpha2/inclusterippool_types.go b/api/v1alpha2/inclusterippool_types.go index eb0609c..f87d5da 100644 --- a/api/v1alpha2/inclusterippool_types.go +++ b/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 diff --git a/config/crd/bases/ipam.cluster.x-k8s.io_globalinclusterippools.yaml b/config/crd/bases/ipam.cluster.x-k8s.io_globalinclusterippools.yaml index 5e8205a..af7e770 100644 --- a/config/crd/bases/ipam.cluster.x-k8s.io_globalinclusterippools.yaml +++ b/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 diff --git a/config/crd/bases/ipam.cluster.x-k8s.io_inclusterippools.yaml b/config/crd/bases/ipam.cluster.x-k8s.io_inclusterippools.yaml index e4f0c36..cba9991 100644 --- a/config/crd/bases/ipam.cluster.x-k8s.io_inclusterippools.yaml +++ b/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 diff --git a/internal/poolutil/pool_test.go b/internal/poolutil/pool_test.go index 9e940d0..7b294c6 100644 --- a/internal/poolutil/pool_test.go +++ b/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", diff --git a/internal/webhooks/inclusterippool.go b/internal/webhooks/inclusterippool.go index 454846f..e1efb98 100644 --- a/internal/webhooks/inclusterippool.go +++ b/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 diff --git a/internal/webhooks/inclusterippool_test.go b/internal/webhooks/inclusterippool_test.go index 993f5c1..78e3cab 100644 --- a/internal/webhooks/inclusterippool_test.go +++ b/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) {