diff --git a/internal/webhooks/inclusterippool.go b/internal/webhooks/inclusterippool.go index 86342c9..b4ad739 100644 --- a/internal/webhooks/inclusterippool.go +++ b/internal/webhooks/inclusterippool.go @@ -110,7 +110,7 @@ func (webhook *InClusterIPPool) ValidateCreate(_ context.Context, obj runtime.Ob } // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type. -func (webhook *InClusterIPPool) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) error { +func (webhook *InClusterIPPool) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error { newPool, ok := newObj.(types.GenericInClusterPool) if !ok { return apierrors.NewBadRequest(fmt.Sprintf("expected a InClusterIPPool or an GlobalInClusterIPPool but got a %T", newObj)) @@ -119,7 +119,49 @@ func (webhook *InClusterIPPool) ValidateUpdate(_ context.Context, oldObj, newObj if !ok { return apierrors.NewBadRequest(fmt.Sprintf("expected a InClusterIPPool or an GlobalInClusterIPPool but got a %T", oldObj)) } - return webhook.validate(oldPool, newPool) + + err := webhook.validate(oldPool, newPool) + if err != nil { + return err + } + + oldPoolRef := corev1.TypedLocalObjectReference{ + APIGroup: pointer.String(v1alpha1.GroupVersion.Group), + Kind: oldPool.GetObjectKind().GroupVersionKind().Kind, + Name: oldPool.GetName(), + } + inUseAddresses, err := poolutil.ListAddressesInUse(ctx, webhook.Client, oldPool.GetNamespace(), oldPoolRef) + if err != nil { + return apierrors.NewInternalError(err) + } + + inUseBuilder := &netipx.IPSetBuilder{} + for _, address := range inUseAddresses { + ip, err := netip.ParseAddr(address.Spec.Address) + if err != nil { + // if an address we fetch for the pool is unparsable then it isn't in the pool ranges + continue + } + inUseBuilder.Add(ip) + } + + newPoolIPSet, err := poolutil.AddressesToIPSet(newPool.PoolSpec().Addresses) + if err != nil { + // these addresses are already validated, this shouldn't happen + return apierrors.NewInternalError(err) + } + + inUseBuilder.RemoveSet(newPoolIPSet) + outOfRangeIPSet, err := inUseBuilder.IPSet() + if err != nil { + panic("oh no") + } + + if outOfRange := outOfRangeIPSet.Ranges(); len(outOfRange) > 0 { + return apierrors.NewBadRequest(fmt.Sprintf("pool addresses does not contain allocated addresses: %v", outOfRange)) + } + + return nil } // ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type. diff --git a/internal/webhooks/inclusterippool_test.go b/internal/webhooks/inclusterippool_test.go index d9c0639..0cbf9ed 100644 --- a/internal/webhooks/inclusterippool_test.go +++ b/internal/webhooks/inclusterippool_test.go @@ -61,6 +61,7 @@ func TestPoolDeletionWithExistingIPAddresses(t *testing.T) { Kind: namespacedPool.GetObjectKind().GroupVersionKind().Kind, Name: namespacedPool.GetName(), }, + Address: "10.0.0.10", }, }, &ipamv1.IPAddress{ @@ -77,6 +78,7 @@ func TestPoolDeletionWithExistingIPAddresses(t *testing.T) { Kind: globalPool.GetObjectKind().GroupVersionKind().Kind, Name: globalPool.GetName(), }, + Address: "10.0.0.10", }, }, } @@ -98,6 +100,91 @@ func TestPoolDeletionWithExistingIPAddresses(t *testing.T) { g.Expect(webhook.ValidateDelete(ctx, namespacedPool)).To(Succeed(), "should allow deletion when no claims exist") g.Expect(webhook.ValidateDelete(ctx, globalPool)).To(Succeed(), "should allow deletion when no claims exist") + +} + +func TestUpdatingPoolInUseAddresses(t *testing.T) { + g := NewWithT(t) + + scheme := runtime.NewScheme() + g.Expect(ipamv1.AddToScheme(scheme)).To(Succeed()) + + namespacedPool := &v1alpha1.InClusterIPPool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-pool", + }, + Spec: v1alpha1.InClusterIPPoolSpec{ + Addresses: []string{"10.0.0.10-10.0.0.20"}, + Prefix: 24, + Gateway: "10.0.0.1", + }, + } + + globalPool := &v1alpha1.InClusterIPPool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-pool", + }, + Spec: v1alpha1.InClusterIPPoolSpec{ + Addresses: []string{"10.0.0.10-10.0.0.20"}, + Prefix: 24, + Gateway: "10.0.0.1", + }, + } + + ips := []client.Object{ + &ipamv1.IPAddress{ + TypeMeta: metav1.TypeMeta{ + Kind: "IPAddress", + APIVersion: "ipam.cluster.x-k8s.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-ip", + }, + Spec: ipamv1.IPAddressSpec{ + PoolRef: corev1.TypedLocalObjectReference{ + APIGroup: pointer.String(namespacedPool.GetObjectKind().GroupVersionKind().Group), + Kind: namespacedPool.GetObjectKind().GroupVersionKind().Kind, + Name: namespacedPool.GetName(), + }, + Address: "10.0.0.10", + }, + }, + &ipamv1.IPAddress{ + TypeMeta: metav1.TypeMeta{ + Kind: "IPAddress", + APIVersion: "ipam.cluster.x-k8s.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-ip-2", + }, + Spec: ipamv1.IPAddressSpec{ + PoolRef: corev1.TypedLocalObjectReference{ + APIGroup: pointer.String(globalPool.GetObjectKind().GroupVersionKind().Group), + Kind: globalPool.GetObjectKind().GroupVersionKind().Kind, + Name: globalPool.GetName(), + }, + Address: "10.0.0.10", + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(ips...). + WithIndex(&ipamv1.IPAddress{}, index.IPAddressPoolRefCombinedField, index.IPAddressByCombinedPoolRef). + Build() + + webhook := InClusterIPPool{ + Client: fakeClient, + } + + oldNamespacedPool := namespacedPool.DeepCopyObject() + oldGlobalPool := globalPool.DeepCopyObject() + namespacedPool.Spec.Addresses = []string{"10.0.0.15-10.0.0.20"} + globalPool.Spec.Addresses = []string{"10.0.0.15-10.0.0.20"} + + g.Expect(webhook.ValidateUpdate(ctx, oldNamespacedPool, namespacedPool)).NotTo(Succeed(), "should not allow removing in use IPs from addresses field in pool") + g.Expect(webhook.ValidateUpdate(ctx, oldGlobalPool, globalPool)).NotTo(Succeed(), "should not allow removing in use IPs from addresses field in pool") } func TestInClusterIPPoolDefaulting(t *testing.T) {