Validate update pool does not remove in use ips

Co-authored-by: Christian Ang <[email protected]>

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.

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) {