diff --git a/ParentPrefixSelectorGuide.md b/ParentPrefixSelectorGuide.md new file mode 100644 index 00000000..daeda957 --- /dev/null +++ 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 ``. + +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 index d997b9f2..f039d545 100644 --- 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 index 7219b45b..826a9909 100644 --- 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 `` + // 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 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 9f322aee..5cfc979d 100644 --- 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 `` + 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 index 75b00a5e..8e12c49b 100644 --- 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 new file mode 100644 index 00000000..d51083fb --- /dev/null +++ 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 new file mode 100644 index 00000000..771b3f69 --- /dev/null +++ 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 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..ad634714 100644 --- a/internal/controller/prefix_controller.go +++ b/internal/controller/prefix_controller.go @@ -132,29 +132,42 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - // get the name of the parent prefix - 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 + if prefixClaim.Status.SelectedParentPrefix == "" { + // the parent prefix is not selected + if err := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, "the parent prefix is not selected"); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{ + Requeue: true, + }, nil } - lockCtx, cancel := context.WithCancel(ctx) - defer cancel() + if prefixClaim.Status.SelectedParentPrefix != msgCanNotInferParentPrefix { + // we can't restore from the restoration hash - // 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) - return ctrl.Result{ - RequeueAfter: 2 * time.Second, - }, nil + // get the name of the parent prefix + leaseLockerNSN := types.NamespacedName{ + Name: convertCIDRToLeaseLockName(prefixClaim.Status.SelectedParentPrefix), + Namespace: r.OperatorNamespace, + } + ll, err = leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.NamespacedName.String()) + if err != nil { + return ctrl.Result{}, err + } + + lockCtx, cancel := context.WithCancel(ctx) + defer cancel() + + // create lock + if locked := ll.TryLock(lockCtx); !locked { + errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.SelectedParentPrefix) + r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) + return ctrl.Result{ + RequeueAfter: 2 * time.Second, + }, nil + } + debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Status.SelectedParentPrefix) } - debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix) } /* 2. reserve or update Prefix in netbox */ @@ -218,7 +231,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 5743f2a3..fa35f9b9 100644 --- a/internal/controller/prefixclaim_controller.go +++ b/internal/controller/prefixclaim_controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + "errors" "fmt" "time" @@ -40,6 +41,10 @@ import ( "github.com/netbox-community/netbox-operator/pkg/netbox/api" ) +const ( + msgCanNotInferParentPrefix = "Prefix restored from hash, cannot infer the parent prefix" +) + // PrefixClaimReconciler reconciles a PrefixClaim object type PrefixClaimReconciler struct { client.Client @@ -75,7 +80,106 @@ 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 selected parent prefix, + // and as the source of truth for future parent prefix references + if prefixClaim.Status.SelectedParentPrefix == "" /* parent prefix not yet selected/assigned */ { + if prefixClaim.Spec.ParentPrefix != "" { + prefixClaim.Status.SelectedParentPrefix = prefixClaim.Spec.ParentPrefix + + // set status, and condition field + msg := fmt.Sprintf("parentPrefix is provided in CR: %v", prefixClaim.Status.SelectedParentPrefix) + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, msg); err != nil { + return ctrl.Result{}, err + } + } else if len(prefixClaim.Spec.ParentPrefixSelector) > 0 { + // we first check if a prefix can be restored from the netbox + + // since the parent prefix is not part of the restoration hash computation + // we can quickly check to see if the prefix with the restoration hash is matched in NetBox + h := generatePrefixRestorationHash(prefixClaim) + canBeRestored, err := r.NetboxClient.RestoreExistingPrefixByHash(h) + if err != nil { + msg := fmt.Sprintf("failed to look up prefix by hash: %v", err.Error()) + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, msg); err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{Requeue: true}, nil + } + + if canBeRestored != nil { + // Yes, so we will claim the prefix directly + /* + Because the parent prefix isn't part of the restoration hash, + we can't be 100% certain what was the original parent prefix when we performed the initial allocation when restoring. + + Consider the following case: + + 1) A prefix (P1) is allocated from (P), with preserveInNetBox set to true. + |------------------------| Parent prefix (P) + |-----| Allocated prefix (P1) + + 2) Prefix (P1) is deleted using the NetBox operator (but still visible in NetBox because of preserveInNetBox being true) + |------------------------| Parent prefix (P) + |-----| Allocated prefix (P1) + + 3) In NetBox, another prefix (P2) is created manually + |------------------------| Parent prefix (P) + |---------------| Manually added prefix (P2) + |-----| Allocated prefix (P1) + + 4) Perform prefix restoration + + Now we won't know if P or P2 is the parent prefix. + But this doesn't matter since we are certain which was the original prefix we allocated and we can recover exactly that one. + */ + + // since we can't infer the parent prefix + // we write a special string in the ParentPrefix status field indicating the situation + prefixClaim.Status.SelectedParentPrefix = msgCanNotInferParentPrefix + + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, msgCanNotInferParentPrefix); err != nil { + return ctrl.Result{}, err + } + } else { + // No, so we need to select one parent prefix from prefix candidates + + // 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) + 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.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, errorMsg); err != nil { + return ctrl.Result{}, err + } + + // we requeue as this might be a temporary prefix exhausation + return ctrl.Result{Requeue: true}, nil + } + + // TODO(henrybear327): use best-fit algorithm to pick a parent prefix + parentPrefixCandidate := parentPrefixCandidates[0] + prefixClaim.Status.SelectedParentPrefix = parentPrefixCandidate.Prefix + + // set status, and condition field + msg := fmt.Sprintf("parentPrefix is selected: %v", prefixClaim.Status.SelectedParentPrefix) + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, msg); err != nil { + return ctrl.Result{}, err + } + } + } 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.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, "either ParentPrefixSelector or ParentPrefix needs to be set"); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + } + + /* 2. check if the matching Prefix object exists */ prefix := &netboxv1.Prefix{} prefixName := prefixClaim.ObjectMeta.Name prefixLookupKey := types.NamespacedName{ @@ -89,33 +193,39 @@ 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 */ - 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 - } + if prefixClaim.Status.SelectedParentPrefix != msgCanNotInferParentPrefix { + // we can't restore from the restoration hash - lockCtx, cancel := context.WithCancel(ctx) - defer cancel() - - /* 3. 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) - return ctrl.Result{ - RequeueAfter: 2 * time.Second, - }, nil - } - debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix)) + /* 3. check if the lease for parent prefix is available */ + leaseLockerNSN := types.NamespacedName{ + Name: convertCIDRToLeaseLockName(prefixClaim.Status.SelectedParentPrefix), + Namespace: r.OperatorNamespace, + } + ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+prefixName) + if err != nil { + return ctrl.Result{}, err + } + + lockCtx, cancel := context.WithCancel(ctx) + defer cancel() + + /* 4. try to lock the lease for the parent prefix */ + locked := ll.TryLock(lockCtx) + if !locked { + // lock for parent prefix was not available, rescheduling + errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.SelectedParentPrefix) + 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.Status.SelectedParentPrefix)) + } // else { + // we can restore from the restoration hash + // we skip directly to try to reclaim Prefix using restorationHash + // } - // 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 +237,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.SelectedParentPrefix, PrefixLength: prefixClaim.Spec.PrefixLength, Metadata: &models.NetboxMetadata{ Tenant: prefixClaim.Spec.Tenant, @@ -140,6 +250,18 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) }, }) if err != nil { + if errors.Is(err, api.ErrParentPrefixExhausted) { + msg := fmt.Sprintf("%v, will restart the parent prefix selection process", err.Error()) + if setConditionErr := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, msg); setConditionErr != nil { + return ctrl.Result{}, fmt.Errorf("error updating status: %w, when failed to get matching prefix: %w", setConditionErr, err) + } + + // we reset the selected parent prefix, since this one is already exhausted + prefixClaim.Status.SelectedParentPrefix = "" + + return ctrl.Result{Requeue: true}, 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) } @@ -147,13 +269,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 { @@ -171,7 +293,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 6108f2e3..e53cbfaa 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" @@ -63,21 +64,51 @@ 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.Spec.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 rd.ComputeHash() } 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 +} + +func (rd *PrefixClaimRestorationData) ComputeHash() string { + if rd == nil { + return "" + } + return fmt.Sprintf("%x", sha1.Sum([]byte(rd.Namespace+rd.Name+rd.ParentPrefix+rd.PrefixLength+rd.Tenant+rd.ParentPrefixSelector))) } diff --git a/internal/controller/prefixclaim_helpers_test.go b/internal/controller/prefixclaim_helpers_test.go new file mode 100644 index 00000000..8486e372 --- /dev/null +++ b/internal/controller/prefixclaim_helpers_test.go @@ -0,0 +1,54 @@ +/* +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 ( + "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{ + SelectedParentPrefix: "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..37f4a8da 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, 5, 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, 5, 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, 5, 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/errors.go b/pkg/netbox/api/errors.go new file mode 100644 index 00000000..fe7100c5 --- /dev/null +++ b/pkg/netbox/api/errors.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 api + +import "errors" + +var ( + ErrParentPrefixExhausted = errors.New("parent prefix exhausted") + ErrParentPrefixNotFound = errors.New("parent prefix not found") + ErrWrongMatchingPrefixSubnetFormat = errors.New("wrong matchingPrefix subnet format") +) diff --git a/pkg/netbox/api/helper.go b/pkg/netbox/api/helper.go index 222f9e92..e2a87b7d 100644 --- a/pkg/netbox/api/helper.go +++ b/pkg/netbox/api/helper.go @@ -24,24 +24,44 @@ import ( "github.com/go-openapi/strfmt" ) -type CustomFieldStringFilter struct { - CustomFieldName string - CustomFieldValue string +type CustomFieldEntry struct { + key string + value string } -func newCustomFieldStringFilterOperation(name string, value string) func(co *runtime.ClientOperation) { +type QueryFilter struct { + netBoxFields map[string]string + customFields []CustomFieldEntry +} + +func newQueryFilterOperation(netBoxFields map[string]string, customFields []CustomFieldEntry) func(co *runtime.ClientOperation) { return func(co *runtime.ClientOperation) { - co.Params = &CustomFieldStringFilter{ - CustomFieldName: name, - CustomFieldValue: value, + co.Params = &QueryFilter{ + netBoxFields: netBoxFields, + customFields: customFields, } } } -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 +func (o *QueryFilter) WriteToRequest(r runtime.ClientRequest, reg strfmt.Registry) error { + // We currently write the request by ANDing all the custom fields + + // The idea is to provide filtering of tenant and site here + // Doing string filtering on tenant and site doesn't really work though, so we will use tenant_id and site_id instead + // The query format is like the following: http://localhost:8080/ipam/prefixes/?q=&site_id=2 + for key, value := range o.netBoxFields { + if err := r.SetQueryParam(url.QueryEscape(key), url.QueryEscape(value)); err != nil { + return err + } + } + + // 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.customFields { + if err := r.SetQueryParam(fmt.Sprintf("cf_%s", url.QueryEscape(entry.key)), entry.value); err != nil { + return err + } } + return nil } diff --git a/pkg/netbox/api/ip_address_claim.go b/pkg/netbox/api/ip_address_claim.go index 7da1c59f..9ad35ca1 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 := newQueryFilterOperation(nil, []CustomFieldEntry{ + { + key: config.GetOperatorConfig().NetboxRestorationHashFieldName, + value: hash, + }, + }) list, err := r.Ipam.IpamIPAddressesList(ipam.NewIpamIPAddressesListParams(), nil, customIpSearch) if err != nil { return nil, err @@ -115,7 +121,7 @@ func (r *NetboxClient) GetAvailableIpAddressesByParentPrefix(parentPrefixId int6 return nil, err } if len(responseAvailableIPs.Payload) == 0 { - return nil, errors.New("parent prefix exhausted") + return nil, ErrParentPrefixExhausted } return responseAvailableIPs, nil } diff --git a/pkg/netbox/api/ip_address_claim_test.go b/pkg/netbox/api/ip_address_claim_test.go index 1acc7250..6d209f9b 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" @@ -312,7 +311,7 @@ func TestIPAddressClaim(t *testing.T) { actual, err := client.GetAvailableIpAddressesByParentPrefix(parentPrefixIdV4) // assert error - AssertError(t, err, "parent prefix exhausted") + AssertError(t, err, ErrParentPrefixExhausted.Error()) // assert nil output AssertNil(t, actual) }) @@ -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 be1d58d3..99a0e265 100644 --- a/pkg/netbox/api/prefix_claim.go +++ b/pkg/netbox/api/prefix_claim.go @@ -22,13 +22,26 @@ 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" + + netboxv1 "github.com/netbox-community/netbox-operator/api/v1" +) + +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 := newQueryFilterOperation(nil, []CustomFieldEntry{ + { + key: config.GetOperatorConfig().NetboxRestorationHashFieldName, + value: hash, + }, + }) list, err := r.Ipam.IpamPrefixesList(ipam.NewIpamPrefixesListParams(), nil, customPrefixSearch) if err != nil { return nil, err @@ -80,6 +93,75 @@ func validatePrefixLengthOrError(prefixClaim *models.PrefixClaim, prefixFamily i return nil } +func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(prefixClaimSpec *netboxv1.PrefixClaimSpec) ([]*models.Prefix, error) { + fieldEntries := make(map[string]string) + + if tenant, ok := prefixClaimSpec.ParentPrefixSelector["tenant"]; ok { + details, err := r.GetTenantDetails(tenant) + if err != nil { + return nil, err + } + + fieldEntries["tenant_id"] = strconv.Itoa(int(details.Id)) + } + + if site, ok := prefixClaimSpec.ParentPrefixSelector["site"]; ok { + details, err := r.GetSiteDetails(site) + if err != nil { + return nil, err + } + + fieldEntries["site_id"] = strconv.Itoa(int(details.Id)) + } + + var conditions func(co *runtime.ClientOperation) + parentPrefixSelectorEntries := make([]CustomFieldEntry, 0, len(prefixClaimSpec.ParentPrefixSelector)) + for k, v := range prefixClaimSpec.ParentPrefixSelector { + parentPrefixSelectorEntries = append(parentPrefixSelectorEntries, CustomFieldEntry{ + key: k, + value: v, + }) + } + + conditions = newQueryFilterOperation(fieldEntries, parentPrefixSelectorEntries) + + 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 && r.isParentPrefixCandidate(prefixClaimSpec, *prefix.Prefix) { + prefixes = append(prefixes, &models.Prefix{ + Prefix: *prefix.Prefix, + }) + } + } + + return prefixes, nil +} + +func (r *NetboxClient) isParentPrefixCandidate(prefixClaimSpec *netboxv1.PrefixClaimSpec, prefix string) bool { + // if we can allocate a prefix from it, we can take it as a parent prefix + if _, err := r.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: prefix, + PrefixLength: prefixClaimSpec.PrefixLength, + Metadata: &models.NetboxMetadata{ + Tenant: prefixClaimSpec.Tenant, + Site: prefixClaimSpec.Site, + }, + }); err == nil { + return true + } + return false +} + // 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) @@ -103,7 +185,7 @@ func (r *NetboxClient) GetAvailablePrefixByClaim(prefixClaim *models.PrefixClaim return nil, err } if len(responseParentPrefix.Payload.Results) == 0 { - return nil, errors.New("parent prefix not found") + return nil, ErrParentPrefixNotFound } if err := validatePrefixLengthOrError(prefixClaim, *responseParentPrefix.Payload.Results[0].Family.Value); err != nil { @@ -143,7 +225,7 @@ func (r *NetboxClient) GetAvailablePrefixByClaim(prefixClaim *models.PrefixClaim // [2] https://github.com/netbox-community/go-netbox/v3/commits/hack/v3.4.5-0/ matchingPrefixSplit := strings.Split(matchingPrefix, "/") if len(matchingPrefixSplit) != 2 { - return nil, errors.New("wrong matchingPrefix subnet format") + return nil, ErrWrongMatchingPrefixSubnetFormat } matchingPrefix = matchingPrefixSplit[0] + prefixClaim.PrefixLength } // else { @@ -162,13 +244,11 @@ func (r *NetboxClient) GetAvailablePrefixesByParentPrefix(parentPrefixId int64) return nil, err } if len(responseAvailablePrefixes.Payload) == 0 { - return nil, errors.New("parent prefix exhausted") + return nil, ErrParentPrefixExhausted } 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 { diff --git a/pkg/netbox/api/prefix_claim_test.go b/pkg/netbox/api/prefix_claim_test.go index 354313c1..f68f8df0 100644 --- a/pkg/netbox/api/prefix_claim_test.go +++ b/pkg/netbox/api/prefix_claim_test.go @@ -89,7 +89,7 @@ func TestPrefixClaim_GetNoAvailablePrefixesByParentPrefix(t *testing.T) { actual, err := netboxClient.GetAvailablePrefixesByParentPrefix(parentPrefixId) assert.Nil(t, actual) - assert.EqualError(t, err, "parent prefix exhausted") + assert.ErrorIs(t, err, ErrParentPrefixExhausted) } func TestPrefixClaim_GetAvailablePrefixByClaim_WithWrongParent(t *testing.T) {