Implement dynamic selection of parent prefix from a set of custom fie…

…lds (#90) Implement dynamic selection of parent prefix from a set of custom fields Notes: According to [2], the `oneOf` validation will only come in the next version Reference: [1] https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/ [2] https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-ratcheting Signed-off-by: Hoanganh.Mai <[email protected]> Signed-off-by: Chun-Hung Tseng <[email protected]> Co-authored-by: Sergio <[email protected]> Co-authored-by: bruelea <[email protected]>
netbox-community · Nov 19, 2024 · f002bf8 · f002bf8
1 parent 5502c04
commit f002bf8
Show file tree

Hide file tree

Showing 21 changed files with 696 additions and 112 deletions.
diff --git a/ParentPrefixSelectorGuide.md b/ParentPrefixSelectorGuide.md
@@ -0,0 +1,56 @@
+# A guide of `ParentPrefixSelector` in `PrefixClaim`
+
+There are 2 ways to make a Prefix claim:
+- provide a `parentPrefix`
+- provide a `parentPrefixSelector`
+
+In this documentation, we will focus on the `parentPrefixSelector` only.
+
+# CRD format
+
+The following is a sample of utilizing the `parentPrefixSelector`:
+
+```bash
+apiVersion: netbox.dev/v1
+kind: PrefixClaim
+metadata:
+  labels:
+    app.kubernetes.io/name: netbox-operator
+    app.kubernetes.io/managed-by: kustomize
+  name: prefixclaim-customfields-sample
+spec:
+  tenant: "MY_TENANT"
+  site: "DM-Akron"
+  description: "some description"
+  comments: "your comments"
+  preserveInNetbox: true
+  prefixLength: "/31"
+  parentPrefixSelector: 
+    tenant: "MY_TENANT"
+    site: "DM-Buffalo"
+    environment: "Production"
+    poolName: "Pool 1"
+```
+
+The usage will be explained in the following sections.
+
+## Notes on `Spec.tenant` and `Spec.site`
+
+Please provide the *name*, not the *slug* value
+
+## `parentPrefixSelector`
+
+The `parentPrefixSelector` is a key-value map, where all the entries are of data type `<string-string>`.
+
+The map contains a set of query conditions for selecting a set of prefixes that can be used as the parent prefix.
+
+The query conditions will be chained by the AND operator, and exact match of the keys and values will be performed.
+
+The fields that can be used as query conditions in the `parentPrefixSelector` are:
+- `tenant` and `site` (in lowercase characters)
+    - these 2 fields are built-in fields from NetBox, so you do *not* need to create custom fields for them
+    - please provide the *name*, not the *slug* value
+    - if the entry for tenant or site is missing, it will *not* inherit from the tenant and site from the Spec
+- custom fields
+    - the data types tested and supported so far are `string`, `integer`, and `boolean`
+    - for `boolean` type, please use `true` and `false` as the value
diff --git a/README.md b/README.md
@@ -58,6 +58,13 @@ Use Cases for this Restoration:
 - Disaster Recovery: In case the cluster is lost, IP Addresses can be restored with the IPAddressClaim only
 - Sticky IPs: Some services do not handle changes to IPs well. This ensures the IP/Prefix assigned to a Custom Resource is always the same.
 
+# `ParentPrefixSelector` in `PrefixClaim`
+
+Please read [ParentPrefixSelector guide] for more information!
+
+[ParentPrefixSelector guide]: ./ParentPrefixSelectorGuide.md
+
+
 # Project Distribution
 
 Following are the steps to build the installer and distribute this project to users.

diff --git a/api/v1/prefixclaim_types.go b/api/v1/prefixclaim_types.go
@@ -24,24 +24,36 @@ import (
 // NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
 
 // PrefixClaimSpec defines the desired state of PrefixClaim
+// TODO: The reason for using a workaround please see https://github.com/netbox-community/netbox-operator/pull/90#issuecomment-2402112475
 // +kubebuilder:validation:XValidation:rule="!has(oldSelf.site) || has(self.site)", message="Site is required once set"
+// +kubebuilder:validation:XValidation:rule="(!has(self.parentPrefix) && has(self.parentPrefixSelector)) || (has(self.parentPrefix) && !has(self.parentPrefixSelector))"
 type PrefixClaimSpec struct {
+
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 	// Important: Run "make" to regenerate code after modifying this file
 
-	//+kubebuilder:validation:Required
 	//+kubebuilder:validation:Format=cidr
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefix' is immutable"
-	ParentPrefix string `json:"parentPrefix"`
+	ParentPrefix string `json:"parentPrefix,omitempty"`
+
+	// The `parentPrefixSelector` is a key-value map, where all the entries are of data type `<string-string>`
+	// The map contains a set of query conditions for selecting a set of prefixes that can be used as the parent prefix
+	// The query conditions will be chained by the AND operator, and exact match of the keys and values will be performed
+	// 2 built-in fields, namely `tenant` and `site`, along with custom fields, can be used
+	// For more information, please see ParentPrefixSelectorGuide.md
+	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefixSelector' is immutable"
+	ParentPrefixSelector map[string]string `json:"parentPrefixSelector,omitempty"`
 
 	//+kubebuilder:validation:Required
 	//+kubebuilder:validation:Pattern=`^\/[0-9]|[1-9][0-9]|1[01][0-9]|12[0-8]$`
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'prefixLength' is immutable"
 	PrefixLength string `json:"prefixLength"`
 
+	// Use the `name` value instead of the `slug` value
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'site' is immutable"
 	Site string `json:"site,omitempty"`
 
+	// Use the `name` value instead of the `slug` value
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'tenant' is immutable"
 	Tenant string `json:"tenant,omitempty"`
 
@@ -58,10 +70,15 @@ type PrefixClaimSpec struct {
 type PrefixClaimStatus struct {
 	// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
 	// Important: Run "make" to regenerate code after modifying this file
-	// Prefix status: container, active, reserved , deprecated
-	Prefix     string             `json:"prefix,omitempty"`
-	PrefixName string             `json:"prefixName,omitempty"`
-	Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
+	// Prefix status: container, active, reserved, deprecated
+
+	// Due to the fact that the parent prefix can be specified directly in `ParentPrefix` or selected from `ParentPrefixSelector`,
+	// we use this field to store exactly which parent prefix we are using for all subsequent reconcile loop calls,
+	// as Spec.ParentPrefix is an immutable field, we can't overwrite it
+	SelectedParentPrefix string             `json:"parentPrefix,omitempty"`
+	Prefix               string             `json:"prefix,omitempty"`
+	PrefixName           string             `json:"prefixName,omitempty"`
+	Conditions           []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
 }
 
 // +kubebuilder:object:root=true
@@ -120,3 +137,17 @@ var ConditionPrefixAssignedFalse = metav1.Condition{
 	Reason:  "PrefixCRNotCreated",
 	Message: "Failed to fetch new Prefix from NetBox",
 }
+
+var ConditionParentPrefixSelectedTrue = metav1.Condition{
+	Type:    "ParentPrefixSelected",
+	Status:  "True",
+	Reason:  "ParentPrefixSelected",
+	Message: "The parent prefix was selected successfully",
+}
+
+var ConditionParentPrefixSelectedFalse = metav1.Condition{
+	Type:    "ParentPrefixSelected",
+	Status:  "False",
+	Reason:  "ParentPrefixNotSelected",
+	Message: "The parent prefix was not able to be selected",
+}
diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/netbox.dev_prefixclaims.yaml b/config/crd/bases/netbox.dev_prefixclaims.yaml
@@ -52,7 +52,9 @@ spec:
           metadata:
             type: object
           spec:
-            description: PrefixClaimSpec defines the desired state of PrefixClaim
+            description: |-
+              PrefixClaimSpec defines the desired state of PrefixClaim
+              TODO: The reason for using a workaround please see https://github.com/netbox-community/netbox-operator/pull/90#issuecomment-2402112475
             properties:
               comments:
                 type: string
@@ -68,6 +70,19 @@ spec:
                 x-kubernetes-validations:
                 - message: Field 'parentPrefix' is immutable
                   rule: self == oldSelf
+              parentPrefixSelector:
+                additionalProperties:
+                  type: string
+                description: |-
+                  The `parentPrefixSelector` is a key-value map, where all the entries are of data type `<string-string>`
+                  The map contains a set of query conditions for selecting a set of prefixes that can be used as the parent prefix
+                  The query conditions will be chained by the AND operator, and exact match of the keys and values will be performed
+                  2 built-in fields, namely `tenant` and `site`, along with custom fields, can be used
+                  For more information, please see ParentPrefixSelectorGuide.md
+                type: object
+                x-kubernetes-validations:
+                - message: Field 'parentPrefixSelector' is immutable
+                  rule: self == oldSelf
               prefixLength:
                 pattern: ^\/[0-9]|[1-9][0-9]|1[01][0-9]|12[0-8]$
                 type: string
@@ -77,22 +92,25 @@ spec:
               preserveInNetbox:
                 type: boolean
               site:
+                description: Use the `name` value instead of the `slug` value
                 type: string
                 x-kubernetes-validations:
                 - message: Field 'site' is immutable
                   rule: self == oldSelf
               tenant:
+                description: Use the `name` value instead of the `slug` value
                 type: string
                 x-kubernetes-validations:
                 - message: Field 'tenant' is immutable
                   rule: self == oldSelf
             required:
-            - parentPrefix
             - prefixLength
             type: object
             x-kubernetes-validations:
             - message: Site is required once set
               rule: '!has(oldSelf.site) || has(self.site)'
+            - rule: (!has(self.parentPrefix) && has(self.parentPrefixSelector)) ||
+                (has(self.parentPrefix) && !has(self.parentPrefixSelector))
           status:
             description: PrefixClaimStatus defines the observed state of PrefixClaim
             properties:
@@ -165,11 +183,13 @@ spec:
                   - type
                   type: object
                 type: array
-              prefix:
+              parentPrefix:
                 description: |-
-                  INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
-                  Important: Run "make" to regenerate code after modifying this file
-                  Prefix status: container, active, reserved , deprecated
+                  Due to the fact that the parent prefix can be specified directly in `ParentPrefix` or selected from `ParentPrefixSelector`,
+                  we use this field to store exactly which parent prefix we are using for all subsequent reconcile loop calls,
+                  as Spec.ParentPrefix is an immutable field, we can't overwrite it
+                type: string
+              prefix:
                 type: string
               prefixName:
                 type: string

diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml
@@ -4,4 +4,6 @@ resources:
 - netbox_v1_ipaddressclaim.yaml
 - netbox_v1_prefix.yaml
 - netbox_v1_prefixclaim.yaml
+- netbox_v1_prefixclaim_parentprefixselector_bool_int.yaml
+- netbox_v1_prefixclaim_parentprefixselector.yaml
 #+kubebuilder:scaffold:manifestskustomizesamples
diff --git a/config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml b/config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml
@@ -0,0 +1,24 @@
+apiVersion: netbox.dev/v1
+kind: PrefixClaim
+metadata:
+  labels:
+    app.kubernetes.io/name: netbox-operator
+    app.kubernetes.io/managed-by: kustomize
+  name: prefixclaim-parentprefixselector-sample
+spec:
+  tenant: "MY_TENANT" # Use the `name` value instead of the `slug` value
+  site: "DM-Akron" # Use the `name` value instead of the `slug` value
+  description: "some description"
+  comments: "your comments"
+  preserveInNetbox: true
+  prefixLength: "/31"
+  parentPrefixSelector: # The keys and values are case-sensitive
+    # if the entry for tenant or site is missing, it will *not* inherit from the tenant and site from the Spec
+    tenant: "MY_TENANT" # Use the `name` value instead of the `slug` value
+    site: "DM-Buffalo" # Use the `name` value instead of the `slug` value
+
+    # custom fields of your interest
+    environment: "Production"
+    poolName: "Pool 1"
+    # environment: "production"
+    # poolName: "pool 3"
diff --git a/config/samples/netbox_v1_prefixclaim_parentprefixselector_bool_int.yaml b/config/samples/netbox_v1_prefixclaim_parentprefixselector_bool_int.yaml
@@ -0,0 +1,28 @@
+apiVersion: netbox.dev/v1
+kind: PrefixClaim
+metadata:
+  labels:
+    app.kubernetes.io/name: netbox-operator
+    app.kubernetes.io/managed-by: kustomize
+  name: prefixclaim-parentprefixselector-bool-int-sample
+spec:
+  tenant: "MY_TENANT" # Use the `name` value instead of the `slug` value
+  site: "DM-Akron" # Use the `name` value instead of the `slug` value
+  description: "some description"
+  comments: "your comments"
+  preserveInNetbox: true
+  prefixLength: "/31"
+  parentPrefixSelector: # The keys and values are case-sensitive
+    # should return a prefix in 3.0.0.0/24
+    environment: "Production"
+    poolName: "Pool 1"
+    # same condition as above, should return a prefix in 3.0.0.0/24
+    # cfDataTypeBool: "true"
+    # cfDataTypeInteger: "1"
+
+    # should return a prefix in 3.0.2.0/24
+    # environment: "Development"
+    # poolName: "Pool 1"
+    # same condition as above, should return a prefix in 3.0.0.0/24
+    # cfDataTypeBool: "false"
+    # cfDataTypeInteger: "2"
diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go
@@ -147,9 +147,8 @@ 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", ipAddressClaim.Spec.ParentPrefix))
-			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
-				ipAddressClaim.Spec.ParentPrefix)
+			errorMsg := fmt.Sprintf("failed to lock parent prefix %s", ipAddressClaim.Spec.ParentPrefix)
+			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
 			return ctrl.Result{
 				RequeueAfter: 2 * time.Second,
 			}, nil

diff --git a/internal/controller/ipaddressclaim_controller.go b/internal/controller/ipaddressclaim_controller.go
@@ -23,7 +23,6 @@ import (
 	"time"
 
 	netboxv1 "github.com/netbox-community/netbox-operator/api/v1"
-	"github.com/netbox-community/netbox-operator/pkg/config"
 	"github.com/netbox-community/netbox-operator/pkg/netbox/api"
 	"github.com/netbox-community/netbox-operator/pkg/netbox/models"
 	"github.com/swisscom/leaselocker"
@@ -111,9 +110,8 @@ 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", o.Spec.ParentPrefix))
-			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
-				o.Spec.ParentPrefix)
+			errorMsg := fmt.Sprintf("failed to lock parent prefix %s", o.Spec.ParentPrefix)
+			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
 			return ctrl.Result{
 				RequeueAfter: 2 * time.Second,
 			}, nil
@@ -122,7 +120,7 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 		// 4. try to reclaim ip address
 		h := generateIpAddressRestorationHash(o)
-		ipAddressModel, err := r.NetboxClient.RestoreExistingIpByHash(config.GetOperatorConfig().NetboxRestorationHashFieldName, h)
+		ipAddressModel, err := r.NetboxClient.RestoreExistingIpByHash(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)