diff --git a/api/v1/prefixclaim_types.go b/api/v1/prefixclaim_types.go index 0b9be4e5..c8ddcff6 100644 --- a/api/v1/prefixclaim_types.go +++ b/api/v1/prefixclaim_types.go @@ -24,14 +24,20 @@ 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(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"` + + // TODO(henrybear327): validate the key and value are all of type string + //+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]$` @@ -56,16 +62,19 @@ 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 + + ParentPrefix string `json:"parentPrefix,omitempty"` // Due to the fact that we can use ParentPrefixSelector to assign parent prefixes, we use this field to store exactly which parent prefix we are using for prefix allocation + 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 // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Prefix",type=string,JSONPath=`.status.prefix` // +kubebuilder:printcolumn:name="PrefixAssigned",type=string,JSONPath=`.status.conditions[?(@.type=="PrefixAssigned")].status` +// +kubebuilder:printcolumn:name="ParentPrefix",type=string,JSONPath=`.status.parentPrefix` // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // +kubebuilder:resource:shortName=pxc @@ -118,3 +127,17 @@ var ConditionPrefixAssignedFalse = metav1.Condition{ Reason: "PrefixCRNotCreated", Message: "Failed to fetch new Prefix from NetBox", } + +var ConditionParentPrefixComputedTrue = metav1.Condition{ + Type: "ParentPrefixComputed", + Status: "True", + Reason: "ParentPrefixComputed", + Message: "The parent prefix was computed successfully", +} + +var ConditionParentPrefixComputedFalse = metav1.Condition{ + Type: "ParentPrefixComputed", + Status: "False", + Reason: "ParentPrefixNotComputed", + Message: "The parent prefix was not able to be computed", +} diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index a934effa..602fbef3 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -320,6 +320,13 @@ func (in *PrefixClaimList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PrefixClaimSpec) DeepCopyInto(out *PrefixClaimSpec) { *out = *in + if in.ParentPrefixSelector != nil { + in, out := &in.ParentPrefixSelector, &out.ParentPrefixSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.CustomFields != nil { in, out := &in.CustomFields, &out.CustomFields *out = make(map[string]string, len(*in)) diff --git a/config/crd/bases/netbox.dev_prefixclaims.yaml b/config/crd/bases/netbox.dev_prefixclaims.yaml index a0a6902d..e1ad3caa 100644 --- a/config/crd/bases/netbox.dev_prefixclaims.yaml +++ b/config/crd/bases/netbox.dev_prefixclaims.yaml @@ -23,6 +23,9 @@ spec: - jsonPath: .status.conditions[?(@.type=="PrefixAssigned")].status name: PrefixAssigned type: string + - jsonPath: .status.parentPrefix + name: ParentPrefix + type: string - jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string @@ -52,7 +55,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 +73,15 @@ spec: x-kubernetes-validations: - message: Field 'parentPrefix' is immutable rule: self == oldSelf + parentPrefixSelector: + additionalProperties: + type: string + description: 'TODO(henrybear327): validate the key and value are all + of type string' + 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 @@ -84,9 +98,11 @@ spec: - message: Field 'tenant' is immutable rule: self == oldSelf required: - - parentPrefix - prefixLength type: object + x-kubernetes-validations: + - rule: (!has(self.parentPrefix) && has(self.parentPrefixSelector)) || + (has(self.parentPrefix) && !has(self.parentPrefixSelector)) status: description: PrefixClaimStatus defines the observed state of PrefixClaim properties: @@ -159,11 +175,9 @@ spec: - type type: object type: array + parentPrefix: + type: string prefix: - 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 type: string prefixName: type: string diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index 75b00a5e..5862809e 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -4,4 +4,5 @@ resources: - netbox_v1_ipaddressclaim.yaml - netbox_v1_prefix.yaml - netbox_v1_prefixclaim.yaml +- netbox_v1_prefixclaim_customfields.yaml #+kubebuilder:scaffold:manifestskustomizesamples diff --git a/config/samples/netbox_v1_prefixclaim_customfields.yaml b/config/samples/netbox_v1_prefixclaim_customfields.yaml new file mode 100644 index 00000000..00f9178f --- /dev/null +++ b/config/samples/netbox_v1_prefixclaim_customfields.yaml @@ -0,0 +1,19 @@ +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: "Dunder-Mifflin, Inc." + site: "DataCenter" + description: "some description" + comments: "your comments" + preserveInNetbox: true + prefixLength: "/31" + parentPrefixSelector: # notice that the keys and values are case-sensitive + environment: "Production" + poolName: "Pool 1" + # environment: "production" + # poolName: "pool 3" diff --git a/config/samples/netbox_v1_prefixclaim_customfields_bool_int.yaml b/config/samples/netbox_v1_prefixclaim_customfields_bool_int.yaml new file mode 100644 index 00000000..f2353420 --- /dev/null +++ b/config/samples/netbox_v1_prefixclaim_customfields_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-customfields-sample-4 +spec: + tenant: "Dunder-Mifflin, Inc." + site: "DataCenter" + description: "some description" + comments: "your comments" + preserveInNetbox: true + prefixLength: "/31" + parentPrefixSelector: # notice that 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 index c54d74ef..2b4af5ef 100644 --- 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 index 18164d37..6eaeb1ae 100644 --- 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) diff --git a/internal/controller/prefix_controller.go b/internal/controller/prefix_controller.go index 0f9d0ad1..80021c0c 100644 --- a/internal/controller/prefix_controller.go +++ b/internal/controller/prefix_controller.go @@ -132,9 +132,19 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } + if prefixClaim.Status.ParentPrefix == "" { + // the parent prefix is not computed + if err := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, "the parent prefix is not computed"); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{ + Requeue: true, + }, nil + } + // get the name of the parent prefix leaseLockerNSN := types.NamespacedName{ - Name: convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix), + Name: convertCIDRToLeaseLockName(prefixClaim.Status.ParentPrefix), Namespace: r.OperatorNamespace, } ll, err = leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.NamespacedName.String()) @@ -147,14 +157,13 @@ 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", prefixClaim.Spec.ParentPrefix)) - r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", - prefixClaim.Spec.ParentPrefix) + errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.ParentPrefix) + r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil } - debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix) + debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Status.ParentPrefix) } /* 2. reserve or update Prefix in netbox */ @@ -218,7 +227,6 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } debugLogger.Info(fmt.Sprintf("reserved prefix in netbox, prefix: %s", prefix.Spec.Prefix)) - if err = r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyTrue, corev1.EventTypeNormal, ""); err != nil { return ctrl.Result{}, err } diff --git a/internal/controller/prefixclaim_controller.go b/internal/controller/prefixclaim_controller.go index 9d4dd98f..1419414f 100644 --- a/internal/controller/prefixclaim_controller.go +++ b/internal/controller/prefixclaim_controller.go @@ -75,7 +75,49 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } - /* 1. check if the matching Prefix object exists */ + /* 1. compute and assign the parent prefix if required */ + // The current design will use prefixClaim.Status.ParentPrefix for storing the computed parent prefix, and as the source of truth for future parent prefix references + if prefixClaim.Status.ParentPrefix == "" /* parent prefix not yet computed/assigned */ { + if prefixClaim.Spec.ParentPrefix != "" { + // ParentPrefix takes precedence over ParentPrefixSelector + prefixClaim.Status.ParentPrefix = prefixClaim.Spec.ParentPrefix + } else if len(prefixClaim.Spec.ParentPrefixSelector) > 0 { + // The main idea is that we select one of the available parent prefixes as the ParentPrefix for all subsequent computation + // + // The existing algorithm for prefix allocation within a ParentPrefix remains unchanged + + // fetch available prefixes from netbox + parentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixByParentPrefixSelector(prefixClaim.Spec.ParentPrefixSelector, prefixClaim.Spec.Tenant, prefixClaim.Spec.PrefixLength) + if err != nil || len(parentPrefixCandidates) == 0 { + errorMsg := fmt.Sprintf("no parent prefix can be obtained with the query conditions set in ParentPrefixSelector, err = %v, number of candidates = %v", err, len(parentPrefixCandidates)) + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixComputedFalse, corev1.EventTypeWarning, errorMsg); err != nil { + return ctrl.Result{}, err + } + + // we requeue as this might be a temporary exhausation + return ctrl.Result{Requeue: true}, nil + } + + // set prefixClaim.Spec.ParentPrefix + // TODO(henrybear327): use best-fit algorithm to pick a parent prefix + parentPrefixCandidate := parentPrefixCandidates[0] + prefixClaim.Status.ParentPrefix = parentPrefixCandidate.Prefix + } else { + // this case should not be triggered anymore, as we have validation rules put in place on the CR + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixComputedFalse, corev1.EventTypeWarning, "either ParentPrefixSelector or ParentPrefix needs to be set"); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{Requeue: false}, nil + } + + // set status, and condition field + msg := fmt.Sprintf("parentPrefix is computed: %v", prefixClaim.Status.ParentPrefix) + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixComputedTrue, corev1.EventTypeNormal, msg); err != nil { + return ctrl.Result{}, err + } + } + + /* 2. check if the matching Prefix object exists */ prefix := &netboxv1.Prefix{} prefixName := prefixClaim.ObjectMeta.Name prefixLookupKey := types.NamespacedName{ @@ -89,9 +131,9 @@ 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 */ + /* 3. check if the lease for parent prefix is available */ leaseLockerNSN := types.NamespacedName{ - Name: convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix), + Name: convertCIDRToLeaseLockName(prefixClaim.Status.ParentPrefix), Namespace: r.OperatorNamespace, } ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+prefixName) @@ -102,20 +144,19 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) lockCtx, cancel := context.WithCancel(ctx) defer cancel() - /* 3. try to lock the lease for the parent prefix */ + /* 4. try to lock the lease for the parent prefix */ locked := ll.TryLock(lockCtx) if !locked { // lock for parent prefix was not available, rescheduling - 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) + errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.ParentPrefix) + r.Recorder.Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil } - debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix)) + debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Status.ParentPrefix)) - // 4. try to reclaim Prefix using restorationHash + // 5. try to reclaim Prefix using restorationHash h := generatePrefixRestorationHash(prefixClaim) prefixModel, err := r.NetboxClient.RestoreExistingPrefixByHash(h) if err != nil { @@ -127,12 +168,12 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) if prefixModel == nil { // Prefix cannot be restored from netbox - // 5.a assign new available Prefix + // 6.a assign new available Prefix // get available Prefix under parent prefix in netbox with equal mask length prefixModel, err = r.NetboxClient.GetAvailablePrefixByClaim( &models.PrefixClaim{ - ParentPrefix: prefixClaim.Spec.ParentPrefix, + ParentPrefix: prefixClaim.Status.ParentPrefix, PrefixLength: prefixClaim.Spec.PrefixLength, Metadata: &models.NetboxMetadata{ Tenant: prefixClaim.Spec.Tenant, @@ -146,13 +187,13 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) } debugLogger.Info(fmt.Sprintf("prefix is not reserved in netbox, assignined new prefix: %s", prefixModel.Prefix)) } else { - // 5.b reassign reserved Prefix from netbox + // 6.b reassign reserved Prefix from netbox // do nothing, Prefix restored debugLogger.Info(fmt.Sprintf("reassign reserved prefix from netbox, prefix: %s", prefixModel.Prefix)) } - /* 6-1, create the Prefix object */ + /* 7.a create the Prefix object */ prefixResource := generatePrefixFromPrefixClaim(prefixClaim, prefixModel.Prefix, logger) err = controllerutil.SetControllerReference(prefixClaim, prefixResource, r.Scheme) if err != nil { @@ -170,7 +211,7 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } } else { // Prefix object exists - /* 6-2. update fields of the Prefix object */ + /* 7.b update fields of the Prefix object */ debugLogger.Info("update prefix resource") if err := r.Client.Get(ctx, prefixLookupKey, prefix); err != nil { return ctrl.Result{}, err diff --git a/internal/controller/prefixclaim_helpers.go b/internal/controller/prefixclaim_helpers.go index 63799c88..dfa7f4fb 100644 --- a/internal/controller/prefixclaim_helpers.go +++ b/internal/controller/prefixclaim_helpers.go @@ -19,6 +19,7 @@ package controller import ( "crypto/sha1" "fmt" + "sort" "github.com/go-logr/logr" netboxv1 "github.com/netbox-community/netbox-operator/api/v1" @@ -62,21 +63,44 @@ func generatePrefixSpec(claim *netboxv1.PrefixClaim, prefix string, logger logr. } func generatePrefixRestorationHash(claim *netboxv1.PrefixClaim) string { + parentPrefixSelectorStr := "" + if len(claim.Spec.ParentPrefixSelector) > 0 { + // we generate the string by + // a) sort all keys in non-decreasing order (to avoid reordering the field in the CR causing a different hash to be generated) + // b) concat all the keys and values in the sequence of key1_value1_..._keyN_valueN + + keyList := make([]string, 0, len(claim.Spec.ParentPrefixSelector)) + for key := range claim.Spec.ParentPrefixSelector { + keyList = append(keyList, key) + } + sort.Strings(keyList) + + for _, key := range keyList { + if len(parentPrefixSelectorStr) > 0 { + parentPrefixSelectorStr += "_" + } + parentPrefixSelectorStr += key + "_" + claim.Spec.ParentPrefixSelector[key] + } + } + rd := PrefixClaimRestorationData{ - Namespace: claim.Namespace, - Name: claim.Name, - ParentPrefix: claim.Spec.ParentPrefix, - PrefixLength: claim.Spec.PrefixLength, - Tenant: claim.Spec.Tenant, + Namespace: claim.Namespace, + Name: claim.Name, + ParentPrefix: claim.Status.ParentPrefix, + PrefixLength: claim.Spec.PrefixLength, + Tenant: claim.Spec.Tenant, + ParentPrefixSelector: parentPrefixSelectorStr, } - return fmt.Sprintf("%x", sha1.Sum([]byte(rd.Namespace+rd.Name+rd.ParentPrefix+rd.PrefixLength+rd.Tenant))) + + return fmt.Sprintf("%x", sha1.Sum([]byte(rd.Namespace+rd.Name+rd.ParentPrefix+rd.PrefixLength+rd.Tenant+rd.ParentPrefixSelector))) } type PrefixClaimRestorationData struct { // only use immutable fields - Namespace string - Name string - ParentPrefix string - PrefixLength string - Tenant string + Namespace string + Name string + ParentPrefix string + PrefixLength string + Tenant string + ParentPrefixSelector string } diff --git a/internal/controller/prefixclaim_helpers_test.go b/internal/controller/prefixclaim_helpers_test.go new file mode 100644 index 00000000..f3f911ea --- /dev/null +++ b/internal/controller/prefixclaim_helpers_test.go @@ -0,0 +1,38 @@ +package controller + +import ( + "testing" + + netboxv1 "github.com/netbox-community/netbox-operator/api/v1" +) + +func testPrefixClaimHash(t *testing.T, prefixClaim *netboxv1.PrefixClaim, expectedHash string) { + generatedHash := generatePrefixRestorationHash(prefixClaim) + + if generatedHash != expectedHash { + t.Errorf("hash mistatch: expected %#v, got %#v from %#v", expectedHash, generatedHash, prefixClaim) + } +} + +func TestBackwardCompatibilityOfGeneratePrefixRestorationHash(t *testing.T) { + { + // output observed when applied config/samples/netbox_v1_prefixclaim.yaml on commit 064e6b + // concatenated string = "defaultprefixclaim-sample2.0.0.0/16/28Dunder-Mifflin, Inc." + // sha1 = "a0601ac7e6d196a82c0e61f9be17313113c3043f" + prefixClaim := &netboxv1.PrefixClaim{ + Spec: netboxv1.PrefixClaimSpec{ + ParentPrefix: "2.0.0.0/16", // not used, as we read from the ParentPrefix in Status + PrefixLength: "/28", + Tenant: "Dunder-Mifflin, Inc.", + ParentPrefixSelector: nil, // TODO(henrybear327): check the default value of this + }, + Status: netboxv1.PrefixClaimStatus{ + ParentPrefix: "2.0.0.0/16", + }, + } + prefixClaim.Namespace = "default" + prefixClaim.Name = "prefixclaim-sample" + + testPrefixClaimHash(t, prefixClaim, "a0601ac7e6d196a82c0e61f9be17313113c3043f") + } +} diff --git a/kind/load-data-job/local-demo-data.sql b/kind/load-data-job/local-demo-data.sql index 51c9bdb5..148aefef 100644 --- a/kind/load-data-job/local-demo-data.sql +++ b/kind/load-data-job/local-demo-data.sql @@ -1,28 +1,91 @@ --- create custom fields +-- create Custom Fields INSERT INTO public.extras_customfield (id, type, name, label, description, required, filter_logic, "default", weight, validation_minimum, validation_maximum, validation_regex, created, last_updated, related_object_type_id, group_name, search_weight, is_cloneable, choice_set_id, ui_editable, ui_visible, comments) VALUES (2, 'text', 'netboxOperatorRestorationHash', 'Netbox Restoration Hash', 'Used to rediscover previously claimed IP Addresses', false, 'exact', NULL, 100, NULL, NULL, '', '2024-06-13 15:17:08.65334+00', '2024-06-13 15:17:08.653359+00', NULL, 'netbox-operator', 100, false, NULL, 'hidden', 'always', ''); INSERT INTO public.extras_customfield (id, type, name, label, description, required, filter_logic, "default", weight, validation_minimum, validation_maximum, validation_regex, created, last_updated, related_object_type_id, group_name, search_weight, is_cloneable, choice_set_id, ui_editable, ui_visible, comments) VALUES (3, 'text', 'example_field', 'Example Field', 'example description', false, 'exact', NULL, 100, NULL, NULL, '', '2024-06-13 15:17:08.65334+00', '2024-06-13 15:17:08.653359+00', NULL, 'netbox-operator', 100, false, NULL, 'hidden', 'always', ''); --- for IP +INSERT INTO public.extras_customfield (id, type, name, label, description, required, filter_logic, "default", weight, validation_minimum, validation_maximum, validation_regex, created, last_updated, related_object_type_id, group_name, search_weight, is_cloneable, choice_set_id, ui_editable, ui_visible, comments) +VALUES (4, 'text', 'environment', 'Environment', 'Custom field 1 for ParentPrefixSelector', false, 'exact', NULL, 100, NULL, NULL, '', '2024-06-13 15:17:08.65334+00', '2024-06-13 15:17:08.653359+00', NULL, 'netbox-operator', 100, false, NULL, 'hidden', 'always', ''); + +INSERT INTO public.extras_customfield (id, type, name, label, description, required, filter_logic, "default", weight, validation_minimum, validation_maximum, validation_regex, created, last_updated, related_object_type_id, group_name, search_weight, is_cloneable, choice_set_id, ui_editable, ui_visible, comments) +VALUES (5, 'text', 'poolName', 'Pool Name', 'Custom field 2 for ParentPrefixSelector', false, 'exact', NULL, 100, NULL, NULL, '', '2024-06-13 15:17:08.65334+00', '2024-06-13 15:17:08.653359+00', NULL, 'netbox-operator', 100, false, NULL, 'hidden', 'always', ''); + +INSERT INTO public.extras_customfield (id, type, name, label, description, required, filter_logic, "default", weight, validation_minimum, validation_maximum, validation_regex, created, last_updated, related_object_type_id, group_name, search_weight, is_cloneable, choice_set_id, ui_editable, ui_visible, comments) +VALUES (6, 'boolean', 'cfDataTypeBool', 'cf Data Type Bool', 'Custom field 3 for ParentPrefixSelector', false, 'exact', NULL, 100, NULL, NULL, '', '2024-06-13 15:17:08.65334+00', '2024-06-13 15:17:08.653359+00', NULL, 'netbox-operator', 100, false, NULL, 'hidden', 'always', ''); + +INSERT INTO public.extras_customfield (id, type, name, label, description, required, filter_logic, "default", weight, validation_minimum, validation_maximum, validation_regex, created, last_updated, related_object_type_id, group_name, search_weight, is_cloneable, choice_set_id, ui_editable, ui_visible, comments) +VALUES (7, 'integer', 'cfDataTypeInteger', 'cf Data Type Integer', 'Custom field 4 for ParentPrefixSelector', false, 'exact', NULL, 100, NULL, NULL, '', '2024-06-13 15:17:08.65334+00', '2024-06-13 15:17:08.653359+00', NULL, 'netbox-operator', 100, false, NULL, 'hidden', 'always', ''); + +-- associate custom fields with IP INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) VALUES (2, 2, 69); INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) VALUES (3, 3, 69); --- for Prefix +-- associate custom fields with Prefix INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) VALUES (4, 2, 70); INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) VALUES (5, 3, 70); --- misc +INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) +VALUES (6, 4, 70); + +INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) +VALUES (7, 5, 70); + +INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) +VALUES (8, 6, 70); + +INSERT INTO public.extras_customfield_object_types (id, customfield_id, objecttype_id) +VALUES (9, 7, 70); + +-- insert User Token +INSERT INTO public.users_token (id, created, expires, key, write_enabled, description, user_id, allowed_ips, last_used) +VALUES (1, '2024-06-14 12:20:13.317942+00', NULL, '0123456789abcdef0123456789abcdef01234567', true, 'test-token', 1, '{}', NULL); + +-- insert Tenant INSERT INTO public.tenancy_tenant (created, last_updated, custom_field_data, id, name, slug, description, comments, group_id) VALUES ('2024-06-14 09:57:11.709344+00', '2024-06-14 09:57:11.709359+00', '{"cust_id": null}', 100, 'MY_TENANT', 'my_tenant', '', '', NULL); + +-- insert Prefix +-- 2.0.0.0/16 INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{}', '2.0.0.0/16', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); -INSERT INTO public.users_token (id, created, expires, key, write_enabled, description, user_id, allowed_ips, last_used) -VALUES (1, '2024-06-14 12:20:13.317942+00', NULL, '0123456789abcdef0123456789abcdef01234567', true, 'test-token', 1, '{}', NULL); + +-- 3.0.0.0/24 - 3.0.8.0/24 (watch out for the upper/lower-case) +-- Pool 1, Production +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "Production", "poolName": "Pool 1", "cfDataTypeBool": true, "cfDataTypeInteger": 1}', '3.0.0.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "Production", "poolName": "Pool 1", "cfDataTypeBool": true, "cfDataTypeInteger": 1}', '3.0.1.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +-- Pool 1, Development +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "Development", "poolName": "Pool 1", "cfDataTypeBool": false, "cfDataTypeInteger": 2}', '3.0.2.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +-- Pool 2, Production +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "Production", "poolName": "Pool 2", "cfDataTypeBool": true, "cfDataTypeInteger": 3}', '3.0.3.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "Production", "poolName": "Pool 2", "cfDataTypeBool": true, "cfDataTypeInteger": 3}', '3.0.4.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +-- Pool 2, Development +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "Development", "poolName": "Pool 2", "cfDataTypeBool": false, "cfDataTypeInteger": 4}', '3.0.5.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +-- pool 3, production +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "production", "poolName": "pool 3", "cfDataTypeBool": true, "cfDataTypeInteger": 5}', '3.0.6.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "production", "poolName": "pool 3", "cfDataTypeBool": true, "cfDataTypeInteger": 5}', '3.0.7.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); + +-- pool 3, development +INSERT INTO public.ipam_prefix (created, last_updated, custom_field_data, prefix, status, is_pool, description, role_id, site_id, tenant_id, vlan_id, vrf_id, _children, _depth, mark_utilized, comments) +VALUES ('2024-06-14 10:01:10.094083+00', '2024-06-14 10:01:10.094095+00', '{"environment": "development", "poolName": "pool 3", "cfDataTypeBool": false, "cfDataTypeInteger": 6}', '3.0.8.0/24', 'active', false, '', NULL, NULL, 100, NULL, NULL, 0, 0, false, ''); diff --git a/pkg/netbox/api/helper.go b/pkg/netbox/api/helper.go index 222f9e92..0e07a2fa 100644 --- a/pkg/netbox/api/helper.go +++ b/pkg/netbox/api/helper.go @@ -18,30 +18,43 @@ package api import ( "fmt" + "log" "net/url" "github.com/go-openapi/runtime" "github.com/go-openapi/strfmt" ) +type CustomFieldEntry struct { + key string + value string +} + type CustomFieldStringFilter struct { - CustomFieldName string - CustomFieldValue string + entries []CustomFieldEntry } -func newCustomFieldStringFilterOperation(name string, value string) func(co *runtime.ClientOperation) { +func newCustomFieldStringFilterOperation(entries []CustomFieldEntry) func(co *runtime.ClientOperation) { return func(co *runtime.ClientOperation) { co.Params = &CustomFieldStringFilter{ - CustomFieldName: name, - CustomFieldValue: value, + entries: entries, } } } func (o *CustomFieldStringFilter) WriteToRequest(r runtime.ClientRequest, reg strfmt.Registry) error { - err := r.SetQueryParam(fmt.Sprintf("cf_%s", url.QueryEscape(o.CustomFieldName)), o.CustomFieldValue) - if err != nil { - return err + // We currently write the request by ANDing all the custom fields + + // The custom field query format is like the following: http://localhost:8080/ipam/prefixes/?q=&cf_poolName=Pool+2&cf_environment=Production + // The GitHub issue related to supporting multiple custom field in a query: https://github.com/netbox-community/netbox/issues/7163 + for _, entry := range o.entries { + err := r.SetQueryParam(fmt.Sprintf("cf_%s", url.QueryEscape(entry.key)), entry.value) + if err != nil { + return err + } } + + log.Println("GetQueryParams", r.GetQueryParams()) + return nil } diff --git a/pkg/netbox/api/ip_address_claim.go b/pkg/netbox/api/ip_address_claim.go index 7da1c59f..470e0bb1 100644 --- a/pkg/netbox/api/ip_address_claim.go +++ b/pkg/netbox/api/ip_address_claim.go @@ -22,6 +22,7 @@ import ( "net" "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" + "github.com/netbox-community/netbox-operator/pkg/config" "github.com/netbox-community/netbox-operator/pkg/netbox/models" "github.com/netbox-community/netbox-operator/pkg/netbox/utils" ) @@ -39,8 +40,13 @@ const ( ipMaskIPv6 = "/128" ) -func (r *NetboxClient) RestoreExistingIpByHash(customFieldName string, hash string) (*models.IPAddress, error) { - customIpSearch := newCustomFieldStringFilterOperation(customFieldName, hash) +func (r *NetboxClient) RestoreExistingIpByHash(hash string) (*models.IPAddress, error) { + customIpSearch := newCustomFieldStringFilterOperation([]CustomFieldEntry{ + { + key: config.GetOperatorConfig().NetboxRestorationHashFieldName, + value: hash, + }, + }) list, err := r.Ipam.IpamIPAddressesList(ipam.NewIpamIPAddressesListParams(), nil, customIpSearch) 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 index 7d1ddd8e..563a65b8 100644 --- a/pkg/netbox/api/ip_address_claim_test.go +++ b/pkg/netbox/api/ip_address_claim_test.go @@ -24,7 +24,6 @@ import ( "github.com/netbox-community/go-netbox/v3/netbox/client/tenancy" netboxModels "github.com/netbox-community/go-netbox/v3/netbox/models" "github.com/netbox-community/netbox-operator/gen/mock_interfaces" - "github.com/netbox-community/netbox-operator/pkg/config" "github.com/netbox-community/netbox-operator/pkg/netbox/models" "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -346,7 +345,7 @@ func TestIPAddressClaim(t *testing.T) { mockIPAddress.EXPECT().IpamIPAddressesList(ipam.NewIpamIPAddressesListParams(), nil, gomock.Any()).Return(output, nil) - actual, err := client.RestoreExistingIpByHash(config.GetOperatorConfig().NetboxRestorationHashFieldName, input) + actual, err := client.RestoreExistingIpByHash(input) assert.Nil(t, err) assert.Equal(t, ipAddressRestore, actual.IpAddress) diff --git a/pkg/netbox/api/prefix_claim.go b/pkg/netbox/api/prefix_claim.go index a6de143a..47259188 100644 --- a/pkg/netbox/api/prefix_claim.go +++ b/pkg/netbox/api/prefix_claim.go @@ -22,13 +22,24 @@ import ( "strconv" "strings" + "github.com/go-openapi/runtime" "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" "github.com/netbox-community/netbox-operator/pkg/config" "github.com/netbox-community/netbox-operator/pkg/netbox/models" ) +var ( + // TODO(henrybear327): centralize errors + ErrNoPrefixMatchsSizeCriteria = errors.New("no available prefix matches size criterias") +) + func (r *NetboxClient) RestoreExistingPrefixByHash(hash string) (*models.Prefix, error) { - customPrefixSearch := newCustomFieldStringFilterOperation(config.GetOperatorConfig().NetboxRestorationHashFieldName, hash) + customPrefixSearch := newCustomFieldStringFilterOperation([]CustomFieldEntry{ + { + key: config.GetOperatorConfig().NetboxRestorationHashFieldName, + value: hash, + }, + }) list, err := r.Ipam.IpamPrefixesList(ipam.NewIpamPrefixesListParams(), nil, customPrefixSearch) if err != nil { return nil, err @@ -80,6 +91,52 @@ func validatePrefixLengthOrError(prefixClaim *models.PrefixClaim, prefixFamily i return nil } +func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(customFields map[string]string, tenant, prefixLength string) ([]*models.Prefix, error) { + var conditions func(co *runtime.ClientOperation) + customFieldEntries := make([]CustomFieldEntry, 0, len(customFields)) + for k, v := range customFields { + customFieldEntries = append(customFieldEntries, CustomFieldEntry{ + key: k, + value: v, + }) + } + conditions = newCustomFieldStringFilterOperation(customFieldEntries) + + list, err := r.Ipam.IpamPrefixesList(ipam.NewIpamPrefixesListParams(), nil, conditions) + if err != nil { + return nil, err + } + + // TODO: find a better way? + if list.Payload.Count != nil && *list.Payload.Count == 0 { + return nil, nil + } + + prefixes := make([]*models.Prefix, 0) + for _, prefix := range list.Payload.Results { + if prefix.Prefix != nil { + // if we can allocate a prefix from it, we can take it as a parent prefix + if _, err := r.getAvailablePrefixByPrefix(tenant, *prefix.Prefix, prefixLength); err == nil { + prefixes = append(prefixes, &models.Prefix{ + Prefix: *prefix.Prefix, + }) + } + } + } + + return prefixes, nil +} + +func (r *NetboxClient) getAvailablePrefixByPrefix(tenant, prefix, prefixLength string) (*models.Prefix, error) { + return r.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: prefix, + PrefixLength: prefixLength, + Metadata: &models.NetboxMetadata{ + Tenant: tenant, + }, + }) +} + // GetAvailablePrefixByClaim searches an available Prefix in Netbox matching PrefixClaim requirements func (r *NetboxClient) GetAvailablePrefixByClaim(prefixClaim *models.PrefixClaim) (*models.Prefix, error) { _, err := r.GetTenantDetails(prefixClaim.Metadata.Tenant) @@ -159,8 +216,6 @@ func (r *NetboxClient) GetAvailablePrefixesByParentPrefix(parentPrefixId int64) return responseAvailablePrefixes, nil } -var ErrNoPrefixMatchsSizeCriteria = errors.New("no available prefix matches size criterias") - func getSmallestMatchingPrefix(prefixList *ipam.IpamPrefixesAvailablePrefixesListOK, prefixClaimLengthString string) (string, bool, error) { // input valiation if len(prefixClaimLengthString) == 0 {