Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow IPPool to have Prefix /0 #288

Merged
merged 2 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/v1alpha2/inclusterippool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ spec:
prefix:
description: Prefix is the network prefix to use.
maximum: 128
minimum: 0
type: integer
required:
- addresses
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ spec:
prefix:
description: Prefix is the network prefix to use.
maximum: 128
minimum: 0
type: integer
required:
- addresses
Expand Down
17 changes: 17 additions & 0 deletions internal/poolutil/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions internal/webhooks/inclusterippool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 59 additions & 1 deletion internal/webhooks/inclusterippool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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) {
Expand Down
Loading