add support for IPv6

bruelea · bruelea · commit 0d227575e62b · 2024-09-24T14:44:46.000+02:00
diff --git a/api/v1/ipaddress_types.go b/api/v1/ipaddress_types.go
@@ -28,7 +28,7 @@ type IpAddressSpec struct {
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 	// Important: Run "make" to regenerate code after modifying this file
 
-	//+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$`
+	//+kubebuilder:validation:Format=cidr
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'ipAddress' is immutable"
 	//+kubebuilder:validation:Required
 	IpAddress string `json:"ipAddress"`
diff --git a/api/v1/ipaddressclaim_types.go b/api/v1/ipaddressclaim_types.go
@@ -29,7 +29,7 @@ type IpAddressClaimSpec struct {
 	// Important: Run "make" to regenerate code after modifying this file
 
 	//+kubebuilder:validation:Required
-	//+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$`
+	//+kubebuilder:validation:Format=cidr
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefix' is immutable"
 	ParentPrefix string `json:"parentPrefix"`
 
diff --git a/api/v1/prefix_types.go b/api/v1/prefix_types.go
@@ -29,7 +29,7 @@ type PrefixSpec struct {
 	// Important: Run "make" to regenerate code after modifying this file
 
 	//+kubebuilder:validation:Required
-	//+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$`
+	//+kubebuilder:validation:Format=cidr
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'prefix' is immutable"
 	Prefix string `json:"prefix"`
 
diff --git a/api/v1/prefixclaim_types.go b/api/v1/prefixclaim_types.go
@@ -29,12 +29,12 @@ type PrefixClaimSpec struct {
 	// Important: Run "make" to regenerate code after modifying this file
 
 	//+kubebuilder:validation:Required
-	//+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$`
+	//+kubebuilder:validation:Format=cidr
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefix' is immutable"
 	ParentPrefix string `json:"parentPrefix"`
 
 	//+kubebuilder:validation:Required
-	//+kubebuilder:validation:Pattern=`^\/([1-9]|[12][0-9]|3[0-2])$`
+	//+kubebuilder:validation:Pattern=`^\/[0-9]|[1-9][0-9]|11[0-9]|12[0-8]$`
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'prefixLength' is immutable"
 	PrefixLength string `json:"prefixLength"`
 
diff --git a/config/crd/bases/netbox.dev_ipaddressclaims.yaml b/config/crd/bases/netbox.dev_ipaddressclaims.yaml
@@ -60,7 +60,7 @@ spec:
               description:
                 type: string
               parentPrefix:
-                pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$
+                format: cidr
                 type: string
                 x-kubernetes-validations:
                 - message: Field 'parentPrefix' is immutable
diff --git a/config/crd/bases/netbox.dev_ipaddresses.yaml b/config/crd/bases/netbox.dev_ipaddresses.yaml
@@ -63,7 +63,7 @@ spec:
               description:
                 type: string
               ipAddress:
-                pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$
+                format: cidr
                 type: string
                 x-kubernetes-validations:
                 - message: Field 'ipAddress' is immutable
diff --git a/config/crd/bases/netbox.dev_prefixclaims.yaml b/config/crd/bases/netbox.dev_prefixclaims.yaml
@@ -60,13 +60,13 @@ spec:
               description:
                 type: string
               parentPrefix:
-                pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$
+                format: cidr
                 type: string
                 x-kubernetes-validations:
                 - message: Field 'parentPrefix' is immutable
                   rule: self == oldSelf
               prefixLength:
-                pattern: ^\/([1-9]|[12][0-9]|3[0-2])$
+                pattern: ^\/[0-9]|[1-9][0-9]|11[0-9]|12[0-8]$
                 type: string
                 x-kubernetes-validations:
                 - message: Field 'prefixLength' is immutable
diff --git a/config/crd/bases/netbox.dev_prefixes.yaml b/config/crd/bases/netbox.dev_prefixes.yaml
@@ -63,7 +63,7 @@ spec:
               description:
                 type: string
               prefix:
-                pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$
+                format: cidr
                 type: string
                 x-kubernetes-validations:
                 - message: Field 'prefix' is immutable
diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go
@@ -118,9 +118,10 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 
 		// get name of parent prefix
-		parentPrefixName := strings.Replace(ipAddressClaim.Spec.ParentPrefix, "/", "-", -1)
-
-		leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace}
+		leaseLockerNSN := types.NamespacedName{
+			Name:      convertCIDRToLeaseLockName(ipAddressClaim.Spec.ParentPrefix),
+			Namespace: r.OperatorNamespace,
+		}
 		ll, err = leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.NamespacedName.String())
 		if err != nil {
 			return ctrl.Result{}, err
@@ -132,13 +133,14 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		// create lock
 		locked := ll.TryLock(lockCtx)
 		if !locked {
-			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName))
-			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName)
+			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", ipAddressClaim.Spec.ParentPrefix))
+			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
+				ipAddressClaim.Spec.ParentPrefix)
 			return ctrl.Result{
 				RequeueAfter: 2 * time.Second,
 			}, nil
 		}
-		debugLogger.Info(fmt.Sprintf("sucessfully locked parent prefix %s", parentPrefixName))
+		debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", ipAddressClaim.Spec.ParentPrefix))
 	}
 
 	// 2. reserve or update ip address in netbox
diff --git a/internal/controller/ipaddressclaim_controller.go b/internal/controller/ipaddressclaim_controller.go
@@ -95,9 +95,10 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		debugLogger.Info("ipaddress object matching ipaddress claim was not found, creating new ipaddress object")
 
 		// 2. check if lease for parent prefix is available
-		parentPrefixName := strings.ReplaceAll(o.Spec.ParentPrefix, "/", "-")
-
-		leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace}
+		leaseLockerNSN := types.NamespacedName{
+			Name:      convertCIDRToLeaseLockName(o.Spec.ParentPrefix),
+			Namespace: r.OperatorNamespace,
+		}
 		ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+ipAddressName)
 		if err != nil {
 			return ctrl.Result{}, err
@@ -110,21 +111,24 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		locked := ll.TryLock(lockCtx)
 		if !locked {
 			// lock for parent prefix was not available, rescheduling
-			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName))
-			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName)
+			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", o.Spec.ParentPrefix))
+			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
+				o.Spec.ParentPrefix)
 			return ctrl.Result{
 				RequeueAfter: 2 * time.Second,
 			}, nil
 		}
-		debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", parentPrefixName))
+		debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", o.Spec.ParentPrefix))
 
 		// 4. try to reclaim ip address
 		h := generateIpAddressRestorationHash(o)
 		ipAddressModel, err := r.NetboxClient.RestoreExistingIpByHash(config.GetOperatorConfig().NetboxRestorationHashFieldName, h)
 		if err != nil {
+			if setConditionErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil {
+				return ctrl.Result{}, fmt.Errorf("error updating status: %w, looking up ip by hash failed: %w", setConditionErr, err)
+			}
 			return ctrl.Result{}, err
 		}
-		// TODO: set condition for each error
 
 		if ipAddressModel == nil {
 			// ip address cannot be restored from netbox
@@ -137,6 +141,9 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 					},
 				})
 			if err != nil {
+				if setConditionErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil {
+					return ctrl.Result{}, fmt.Errorf("error updating status: %w, when assignment of ip address failed: %w", setConditionErr, err)
+				}
 				return ctrl.Result{}, err
 			}
 			debugLogger.Info(fmt.Sprintf("ip address is not reserved in netbox, assignined new ip address: %s", ipAddressModel.IpAddress))
diff --git a/internal/controller/netbox_testdata_test.go b/internal/controller/netbox_testdata_test.go
@@ -40,6 +40,7 @@ var comments = "integration test comment"
 var siteSlug = "mars-ip-claim"
 
 var ipAddress = "1.0.0.1/32"
+var ipAddressFamily = int64(4)
 var parentPrefix = "1.0.0.0/28"
 
 var siteId = int64(2)
@@ -136,6 +137,7 @@ func mockedResponseExpectedAvailableIpAddress() []*netboxModels.AvailableIP {
 	return []*netboxModels.AvailableIP{
 		{
 			Address: ipAddress,
+			Family:  ipAddressFamily,
 		},
 	}
 }
diff --git a/internal/controller/prefix_controller.go b/internal/controller/prefix_controller.go
@@ -119,9 +119,10 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		}
 
 		// get the name of the parent prefix
-		parentPrefixName := strings.Replace(prefixClaim.Spec.ParentPrefix, "/", "-", -1)
-
-		leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace}
+		leaseLockerNSN := types.NamespacedName{
+			Name:      convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix),
+			Namespace: r.OperatorNamespace,
+		}
 		ll, err = leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.NamespacedName.String())
 		if err != nil {
 			return ctrl.Result{}, err
@@ -132,13 +133,14 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
 		// create lock
 		if locked := ll.TryLock(lockCtx); !locked {
-			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName))
-			r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName)
+			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Spec.ParentPrefix))
+			r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
+				prefixClaim.Spec.ParentPrefix)
 			return ctrl.Result{
 				RequeueAfter: 2 * time.Second,
 			}, nil
 		}
-		debugLogger.Info("sucessfully locked parent prefix %s", parentPrefixName)
+		debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix)
 	}
 
 	/* 2. reserve or update Prefix in netbox */
diff --git a/internal/controller/prefixclaim_controller.go b/internal/controller/prefixclaim_controller.go
@@ -19,7 +19,6 @@ package controller
 import (
 	"context"
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/netbox-community/netbox-operator/pkg/netbox/models"
@@ -91,8 +90,10 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		debugLogger.Info("the prefix was not found, will create a new prefix object now")
 
 		/* 2. check if the lease for parent prefix is available */
-		parentPrefixName := strings.ReplaceAll(prefixClaim.Spec.ParentPrefix, "/", "-")
-		leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace}
+		leaseLockerNSN := types.NamespacedName{
+			Name:      convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix),
+			Namespace: r.OperatorNamespace,
+		}
 		ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+prefixName)
 		if err != nil {
 			return ctrl.Result{}, err
@@ -105,18 +106,22 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		locked := ll.TryLock(lockCtx)
 		if !locked {
 			// lock for parent prefix was not available, rescheduling
-			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName))
-			r.Recorder.Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName)
+			logger.Info(fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Spec.ParentPrefix))
+			r.Recorder.Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
+				prefixClaim.Spec.ParentPrefix)
 			return ctrl.Result{
 				RequeueAfter: 2 * time.Second,
 			}, nil
 		}
-		debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", parentPrefixName))
+		debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix))
 
 		// 4. try to reclaim Prefix using restorationHash
 		h := generatePrefixRestorationHash(prefixClaim)
 		prefixModel, err := r.NetboxClient.RestoreExistingPrefixByHash(h)
 		if err != nil {
+			if setConditionErr := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil {
+				return ctrl.Result{}, fmt.Errorf("error updating status: %w, when look up of prefix by hash failed: %w", setConditionErr, err)
+			}
 			return ctrl.Result{}, err
 		}
 		// TODO: set condition for each error
@@ -135,6 +140,9 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 					},
 				})
 			if err != nil {
+				if setConditionErr := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil {
+					return ctrl.Result{}, fmt.Errorf("error updating status: %w, when failed to get matching prefix: %w", setConditionErr, err)
+				}
 				return ctrl.Result{}, err
 			}
 			debugLogger.Info(fmt.Sprintf("prefix is not reserved in netbox, assignined new prefix: %s", prefixModel.Prefix))
@@ -215,38 +223,6 @@ func (r *PrefixClaimReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *PrefixClaimReconciler) GetAvailablePrefix(o *netboxv1.PrefixClaim) (*models.Prefix, error) {
-	var availablePrefix *models.Prefix
-	var err error
-	if availablePrefix, err = r.NetboxClient.GetAvailablePrefixByClaim(
-		&models.PrefixClaim{
-			ParentPrefix: o.Spec.ParentPrefix,
-			PrefixLength: o.Spec.PrefixLength,
-			Metadata: &models.NetboxMetadata{
-				Tenant: o.Spec.Tenant,
-			},
-		},
-	); err != nil {
-		return nil, err
-	}
-
-	if _, err = r.NetboxClient.ReserveOrUpdatePrefix(
-		&models.Prefix{
-			Prefix: availablePrefix.Prefix,
-			Metadata: &models.NetboxMetadata{
-				Comments:    o.Spec.Comments,
-				Custom:      map[string]string{},
-				Description: o.Spec.Description,
-				Site:        o.Spec.Site,
-				Tenant:      o.Spec.Tenant,
-			},
-		}); err != nil {
-		return nil, err
-	}
-
-	return availablePrefix, nil
-}
-
 // TODO(henrybear327): Duplicated code, consider refactoring this
 func (r *PrefixClaimReconciler) SetConditionAndCreateEvent(ctx context.Context, o *netboxv1.PrefixClaim, condition metav1.Condition, eventType string, conditionMessageAppend string) error {
 	if len(conditionMessageAppend) > 0 {
diff --git a/internal/controller/utils.go b/internal/controller/utils.go
@@ -0,0 +1,25 @@
+/*
+Copyright 2024 Swisscom (Schweiz) AG.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	"strings"
+)
+
+func convertCIDRToLeaseLockName(cidr string) string {
+	return strings.ReplaceAll(strings.ReplaceAll(cidr, "/", "-"), ":", "-")
+}
diff --git a/pkg/netbox/api/ip_address_claim.go b/pkg/netbox/api/ip_address_claim.go
@@ -25,8 +25,17 @@ import (
 	"github.com/netbox-community/netbox-operator/pkg/netbox/models"
 )
 
+type IPFamily int64
+
+const (
+	IPv4Familiy IPFamily = iota + 4
+	_                    // Skip 5
+	IPv6Familiy
+)
+
 const (
-	ipMask = "/32"
+	ipMaskIPv4 = "/32"
+	ipMaskIPv6 = "/128"
 )
 
 func (r *NetboxClient) RestoreExistingIpByHash(customFieldName string, hash string) (*models.IPAddress, error) {
@@ -75,6 +84,15 @@ func (r *NetboxClient) GetAvailableIpAddressByClaim(ipAddressClaim *models.IPAdd
 		return nil, err
 	}
 
+	var ipMask string
+	if responseAvailableIPs.Payload[0].Family == int64(IPv4Familiy) {
+		ipMask = ipMaskIPv4
+	} else if responseAvailableIPs.Payload[0].Family == int64(IPv6Familiy) {
+		ipMask = ipMaskIPv6
+	} else {
+		return nil, errors.New("available ip has unknown IP family")
+	}
+
 	ipAddress, err := r.SetIpAddressMask(responseAvailableIPs.Payload[0].Address, ipMask)
 	if err != nil {
 		return nil, err
diff --git a/pkg/netbox/api/ip_address_claim_test.go b/pkg/netbox/api/ip_address_claim_test.go
diff --git a/pkg/netbox/api/prefix_claim.go b/pkg/netbox/api/prefix_claim.go
diff --git a/pkg/netbox/api/prefix_claim_test.go b/pkg/netbox/api/prefix_claim_test.go
