diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 00000000..e0506380 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,133 @@ + +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, caste, color, religion, or sexual +identity and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment for our +community include: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best not just for us as individuals, but for the overall + community + +Examples of unacceptable behavior include: + +* The use of sexualized language or imagery, and sexual attention or advances of + any kind +* Trolling, insulting or derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email address, + without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject +comments, commits, code, wiki edits, issues, and other contributions that are +not aligned to this Code of Conduct, and will communicate reasons for moderation +decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. +Examples of representing our community include using an official email address, +posting via an official social media account, or acting as an appointed +representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported to the community leaders responsible for enforcement at +abuse@swisscom.com. +All complaints will be reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of +actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved, including unsolicited interaction with +those enforcing the Code of Conduct, for a specified period of time. This +includes avoiding interactions in community spaces as well as external channels +like social media. Violating these terms may lead to a temporary or permanent +ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. No public or +private interaction with the people involved, including unsolicited interaction +with those enforcing the Code of Conduct, is allowed during this period. +Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within the +community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +[https://www.contributor-covenant.org/version/2/1/code_of_conduct.html][v2.1]. + +Community Impact Guidelines were inspired by +[Mozilla's code of conduct enforcement ladder][Mozilla CoC]. + +For answers to common questions about this code of conduct, see the FAQ at +[https://www.contributor-covenant.org/faq][FAQ]. Translations are available at +[https://www.contributor-covenant.org/translations][translations]. + +[homepage]: https://www.contributor-covenant.org +[v2.1]: https://www.contributor-covenant.org/version/2/1/code_of_conduct.html +[Mozilla CoC]: https://github.com/mozilla/diversity +[FAQ]: https://www.contributor-covenant.org/faq +[translations]: https://www.contributor-covenant.org/translations diff --git a/api/v1/ipaddress_types.go b/api/v1/ipaddress_types.go index bbab8fdd..06569808 100644 --- a/api/v1/ipaddress_types.go +++ b/api/v1/ipaddress_types.go @@ -28,7 +28,7 @@ type IpAddressSpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster // Important: Run "make" to regenerate code after modifying this file - //+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$` + //+kubebuilder:validation:Format=cidr //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'ipAddress' is immutable" //+kubebuilder:validation:Required IpAddress string `json:"ipAddress"` @@ -63,6 +63,7 @@ type IpAddressStatus struct { //+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` //+kubebuilder:printcolumn:name="ID",type=string,JSONPath=`.status.id` //+kubebuilder:printcolumn:name="URL",type=string,JSONPath=`.status.url` +//+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // +kubebuilder:resource:shortName=ip // IpAddress is the Schema for the ipaddresses API diff --git a/api/v1/ipaddressclaim_types.go b/api/v1/ipaddressclaim_types.go index 7f02cef6..3f4d270f 100644 --- a/api/v1/ipaddressclaim_types.go +++ b/api/v1/ipaddressclaim_types.go @@ -29,7 +29,7 @@ type IpAddressClaimSpec struct { // Important: Run "make" to regenerate code after modifying this file //+kubebuilder:validation:Required - //+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$` + //+kubebuilder:validation:Format=cidr //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefix' is immutable" ParentPrefix string `json:"parentPrefix"` @@ -63,8 +63,9 @@ type IpAddressClaimStatus struct { //+kubebuilder:subresource:status //+kubebuilder:storageversion //+kubebuilder:printcolumn:name="IpAddress",type=string,JSONPath=`.status.ipAddress` -//+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` //+kubebuilder:printcolumn:name="IpAssigned",type=string,JSONPath=`.status.conditions[?(@.type=="IPAssigned")].status` +//+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` +//+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // +kubebuilder:resource:shortName=ipc // IpAddressClaim is the Schema for the ipaddressclaims API diff --git a/api/v1/prefix_types.go b/api/v1/prefix_types.go index 113083a8..96e31eac 100644 --- a/api/v1/prefix_types.go +++ b/api/v1/prefix_types.go @@ -29,7 +29,7 @@ type PrefixSpec struct { // Important: Run "make" to regenerate code after modifying this file //+kubebuilder:validation:Required - //+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$` + //+kubebuilder:validation:Format=cidr //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'prefix' is immutable" Prefix string `json:"prefix"` @@ -65,6 +65,7 @@ type PrefixStatus struct { // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` // +kubebuilder:printcolumn:name="ID",type=string,JSONPath=`.status.id` // +kubebuilder:printcolumn:name="URL",type=string,JSONPath=`.status.url` +// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // +kubebuilder:resource:shortName=px // Prefix is the Schema for the prefixes API type Prefix struct { diff --git a/api/v1/prefixclaim_types.go b/api/v1/prefixclaim_types.go index e693cb5a..0b9be4e5 100644 --- a/api/v1/prefixclaim_types.go +++ b/api/v1/prefixclaim_types.go @@ -29,12 +29,12 @@ type PrefixClaimSpec struct { // Important: Run "make" to regenerate code after modifying this file //+kubebuilder:validation:Required - //+kubebuilder:validation:Pattern=`^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$` + //+kubebuilder:validation:Format=cidr //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefix' is immutable" ParentPrefix string `json:"parentPrefix"` //+kubebuilder:validation:Required - //+kubebuilder:validation:Pattern=`^\/([1-9]|[12][0-9]|3[0-2])$` + //+kubebuilder:validation:Pattern=`^\/[0-9]|[1-9][0-9]|1[01][0-9]|12[0-8]$` //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'prefixLength' is immutable" PrefixLength string `json:"prefixLength"` @@ -65,8 +65,9 @@ type PrefixClaimStatus struct { // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Prefix",type=string,JSONPath=`.status.prefix` -// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` // +kubebuilder:printcolumn:name="PrefixAssigned",type=string,JSONPath=`.status.conditions[?(@.type=="PrefixAssigned")].status` +// +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 // PrefixClaim is the Schema for the prefixclaims API type PrefixClaim struct { diff --git a/config/crd/bases/ipam.netbox.dev_ipaddressclaims.yaml b/config/crd/bases/ipam.netbox.dev_ipaddressclaims.yaml index e416e917..374aa6f0 100644 --- a/config/crd/bases/ipam.netbox.dev_ipaddressclaims.yaml +++ b/config/crd/bases/ipam.netbox.dev_ipaddressclaims.yaml @@ -20,12 +20,15 @@ spec: - jsonPath: .status.ipAddress name: IpAddress type: string - - jsonPath: .status.conditions[?(@.type=="Ready")].status - name: Ready - type: string - jsonPath: .status.conditions[?(@.type=="IPAssigned")].status name: IpAssigned type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date name: v1 schema: openAPIV3Schema: @@ -60,7 +63,7 @@ spec: description: type: string parentPrefix: - pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$ + format: cidr type: string x-kubernetes-validations: - message: Field 'parentPrefix' is immutable diff --git a/config/crd/bases/ipam.netbox.dev_ipaddresses.yaml b/config/crd/bases/ipam.netbox.dev_ipaddresses.yaml index c41ed979..d29c1917 100644 --- a/config/crd/bases/ipam.netbox.dev_ipaddresses.yaml +++ b/config/crd/bases/ipam.netbox.dev_ipaddresses.yaml @@ -29,6 +29,9 @@ spec: - jsonPath: .status.url name: URL type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date name: v1 schema: openAPIV3Schema: @@ -63,7 +66,7 @@ spec: description: type: string ipAddress: - pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$ + format: cidr type: string x-kubernetes-validations: - message: Field 'ipAddress' is immutable diff --git a/config/crd/bases/ipam.netbox.dev_prefixclaims.yaml b/config/crd/bases/ipam.netbox.dev_prefixclaims.yaml index e8f49276..f3fb46cd 100644 --- a/config/crd/bases/ipam.netbox.dev_prefixclaims.yaml +++ b/config/crd/bases/ipam.netbox.dev_prefixclaims.yaml @@ -20,12 +20,15 @@ spec: - jsonPath: .status.prefix name: Prefix type: string - - jsonPath: .status.conditions[?(@.type=="Ready")].status - name: Ready - type: string - jsonPath: .status.conditions[?(@.type=="PrefixAssigned")].status name: PrefixAssigned type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date name: v1 schema: openAPIV3Schema: @@ -60,13 +63,13 @@ spec: description: type: string parentPrefix: - pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$ + format: cidr type: string x-kubernetes-validations: - message: Field 'parentPrefix' is immutable rule: self == oldSelf prefixLength: - pattern: ^\/([1-9]|[12][0-9]|3[0-2])$ + pattern: ^\/[0-9]|[1-9][0-9]|1[01][0-9]|12[0-8]$ type: string x-kubernetes-validations: - message: Field 'prefixLength' is immutable diff --git a/config/crd/bases/ipam.netbox.dev_prefixes.yaml b/config/crd/bases/ipam.netbox.dev_prefixes.yaml index e7b2f56e..25678ed1 100644 --- a/config/crd/bases/ipam.netbox.dev_prefixes.yaml +++ b/config/crd/bases/ipam.netbox.dev_prefixes.yaml @@ -29,6 +29,9 @@ spec: - jsonPath: .status.url name: URL type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date name: v1 schema: openAPIV3Schema: @@ -63,7 +66,7 @@ spec: description: type: string prefix: - pattern: ^((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\/([1-9]|[12][0-9]|3[0-2])$ + format: cidr type: string x-kubernetes-validations: - message: Field 'prefix' is immutable diff --git a/config/samples/ip-address-claim-no-tenant.yaml b/config/samples/ip-address-claim-no-tenant.yaml deleted file mode 100644 index 4df3f9ec..00000000 --- a/config/samples/ip-address-claim-no-tenant.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: netbox.dev/v1 -kind: IpAddressClaim -metadata: - labels: - app.kubernetes.io/name: netbox-operator - app.kubernetes.io/managed-by: kustomize - name: ipaddressclaim-sample-2 -spec: - tenant: "non-existant-tenant" - description: "some description" - comments: "your comments" - preserveInNetbox: true - parentPrefix: "2.0.0.0/16" diff --git a/go.mod b/go.mod index f2256697..537bfb0e 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( github.com/imdario/mergo v0.3.6 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/klauspost/compress v1.17.9 // indirect + github.com/klauspost/compress v1.17.10 // indirect github.com/magiconair/properties v1.8.7 // indirect github.com/mailru/easyjson v0.7.7 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect diff --git a/go.sum b/go.sum index ee46a5be..9dcad1d2 100644 --- a/go.sum +++ b/go.sum @@ -79,8 +79,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2qeMA= -github.com/klauspost/compress v1.17.9/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= +github.com/klauspost/compress v1.17.10 h1:oXAz+Vh0PMUvJczoi+flxpnBEPxoER1IaAnU/NMPtT0= +github.com/klauspost/compress v1.17.10/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= diff --git a/internal/controller/expected_netboxmock_calls_test.go b/internal/controller/expected_netboxmock_calls_test.go index e4b51acf..7d45c4c8 100644 --- a/internal/controller/expected_netboxmock_calls_test.go +++ b/internal/controller/expected_netboxmock_calls_test.go @@ -234,7 +234,7 @@ func mockTenancyTenancyTenantsList(tenancyMock *mock_interfaces.MockTenancyInter } // ----------------------------- -// Restet Mock Functions +// Reset Mock Functions // ----------------------------- func resetMockFunctions(ipamMockA *mock_interfaces.MockIpamInterface, ipamMockB *mock_interfaces.MockIpamInterface, tenancyMock *mock_interfaces.MockTenancyInterface) { diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go index 5194025a..b257a1fd 100644 --- a/internal/controller/ipaddress_controller.go +++ b/internal/controller/ipaddress_controller.go @@ -118,9 +118,10 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } // get name of parent prefix - parentPrefixName := strings.Replace(ipAddressClaim.Spec.ParentPrefix, "/", "-", -1) - - leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace} + leaseLockerNSN := types.NamespacedName{ + Name: convertCIDRToLeaseLockName(ipAddressClaim.Spec.ParentPrefix), + Namespace: r.OperatorNamespace, + } ll, err = leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.NamespacedName.String()) if err != nil { return ctrl.Result{}, err @@ -132,13 +133,14 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // create lock locked := ll.TryLock(lockCtx) if !locked { - logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName)) - r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName) + logger.Info(fmt.Sprintf("failed to lock parent prefix %s", ipAddressClaim.Spec.ParentPrefix)) + r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", + ipAddressClaim.Spec.ParentPrefix) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil } - debugLogger.Info(fmt.Sprintf("sucessfully locked parent prefix %s", parentPrefixName)) + debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", ipAddressClaim.Spec.ParentPrefix)) } // 2. reserve or update ip address in netbox diff --git a/internal/controller/ipaddressclaim_controller.go b/internal/controller/ipaddressclaim_controller.go index 7326a046..b744e4a6 100644 --- a/internal/controller/ipaddressclaim_controller.go +++ b/internal/controller/ipaddressclaim_controller.go @@ -95,9 +95,10 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque debugLogger.Info("ipaddress object matching ipaddress claim was not found, creating new ipaddress object") // 2. check if lease for parent prefix is available - parentPrefixName := strings.ReplaceAll(o.Spec.ParentPrefix, "/", "-") - - leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace} + leaseLockerNSN := types.NamespacedName{ + Name: convertCIDRToLeaseLockName(o.Spec.ParentPrefix), + Namespace: r.OperatorNamespace, + } ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+ipAddressName) if err != nil { return ctrl.Result{}, err @@ -110,21 +111,24 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque locked := ll.TryLock(lockCtx) if !locked { // lock for parent prefix was not available, rescheduling - logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName)) - r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName) + logger.Info(fmt.Sprintf("failed to lock parent prefix %s", o.Spec.ParentPrefix)) + r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", + o.Spec.ParentPrefix) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil } - debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", parentPrefixName)) + debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", o.Spec.ParentPrefix)) // 4. try to reclaim ip address h := generateIpAddressRestorationHash(o) ipAddressModel, err := r.NetboxClient.RestoreExistingIpByHash(config.GetOperatorConfig().NetboxRestorationHashFieldName, h) if err != nil { - return ctrl.Result{}, err + if setConditionErr := r.SetConditionAndCreateEvent(ctx, o, ipamv1.ConditionIpAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil { + return ctrl.Result{}, fmt.Errorf("error updating status: %w, looking up ip by hash failed: %w", setConditionErr, err) + } + return ctrl.Result{Requeue: true}, nil } - // TODO: set condition for each error if ipAddressModel == nil { // ip address cannot be restored from netbox @@ -137,9 +141,12 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque }, }) if err != nil { - return ctrl.Result{}, err + if setConditionErr := r.SetConditionAndCreateEvent(ctx, o, ipamv1.ConditionIpAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil { + return ctrl.Result{}, fmt.Errorf("error updating status: %w, when assignment of ip address failed: %w", setConditionErr, err) + } + return ctrl.Result{Requeue: true}, nil } - debugLogger.Info(fmt.Sprintf("ip address is not reserved in netbox, assignined new ip address: %s", ipAddressModel.IpAddress)) + debugLogger.Info(fmt.Sprintf("ip address is not reserved in netbox, assigned new ip address: %s", ipAddressModel.IpAddress)) } else { // 5.b reassign reserved ip address from netbox // do nothing, ip address restored diff --git a/internal/controller/netbox_testdata_test.go b/internal/controller/netbox_testdata_test.go index 04b4bd80..30147a93 100644 --- a/internal/controller/netbox_testdata_test.go +++ b/internal/controller/netbox_testdata_test.go @@ -24,6 +24,7 @@ import ( "github.com/netbox-community/go-netbox/v3/netbox/client/tenancy" netboxModels "github.com/netbox-community/go-netbox/v3/netbox/models" ipamv1 "github.com/netbox-community/netbox-operator/api/v1" + "github.com/netbox-community/netbox-operator/pkg/netbox/api" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -40,6 +41,7 @@ var comments = "integration test comment" var siteSlug = "mars-ip-claim" var ipAddress = "1.0.0.1/32" +var ipAddressFamily = int64(api.IPv4Family) var parentPrefix = "1.0.0.0/28" var siteId = int64(2) @@ -136,6 +138,7 @@ func mockedResponseExpectedAvailableIpAddress() []*netboxModels.AvailableIP { return []*netboxModels.AvailableIP{ { Address: ipAddress, + Family: ipAddressFamily, }, } } diff --git a/internal/controller/prefix_controller.go b/internal/controller/prefix_controller.go index 36ecdde6..f709c036 100644 --- a/internal/controller/prefix_controller.go +++ b/internal/controller/prefix_controller.go @@ -119,9 +119,10 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } // get the name of the parent prefix - parentPrefixName := strings.Replace(prefixClaim.Spec.ParentPrefix, "/", "-", -1) - - leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace} + leaseLockerNSN := types.NamespacedName{ + Name: convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix), + Namespace: r.OperatorNamespace, + } ll, err = leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.NamespacedName.String()) if err != nil { return ctrl.Result{}, err @@ -132,13 +133,14 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // create lock if locked := ll.TryLock(lockCtx); !locked { - logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName)) - r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName) + logger.Info(fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Spec.ParentPrefix)) + r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", + prefixClaim.Spec.ParentPrefix) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil } - debugLogger.Info("sucessfully locked parent prefix %s", parentPrefixName) + debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix) } /* 2. reserve or update Prefix in netbox */ diff --git a/internal/controller/prefixclaim_controller.go b/internal/controller/prefixclaim_controller.go index 2c9876ef..8c59257b 100644 --- a/internal/controller/prefixclaim_controller.go +++ b/internal/controller/prefixclaim_controller.go @@ -19,7 +19,6 @@ package controller import ( "context" "fmt" - "strings" "time" "github.com/netbox-community/netbox-operator/pkg/netbox/models" @@ -91,8 +90,10 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) debugLogger.Info("the prefix was not found, will create a new prefix object now") /* 2. check if the lease for parent prefix is available */ - parentPrefixName := strings.ReplaceAll(prefixClaim.Spec.ParentPrefix, "/", "-") - leaseLockerNSN := types.NamespacedName{Name: parentPrefixName, Namespace: r.OperatorNamespace} + leaseLockerNSN := types.NamespacedName{ + Name: convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix), + Namespace: r.OperatorNamespace, + } ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+prefixName) if err != nil { return ctrl.Result{}, err @@ -105,21 +106,24 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) locked := ll.TryLock(lockCtx) if !locked { // lock for parent prefix was not available, rescheduling - logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefixName)) - r.Recorder.Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", parentPrefixName) + logger.Info(fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Spec.ParentPrefix)) + r.Recorder.Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", + prefixClaim.Spec.ParentPrefix) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil } - debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", parentPrefixName)) + debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix)) // 4. try to reclaim Prefix using restorationHash h := generatePrefixRestorationHash(prefixClaim) prefixModel, err := r.NetboxClient.RestoreExistingPrefixByHash(h) if err != nil { - return ctrl.Result{}, err + if setConditionErr := r.SetConditionAndCreateEvent(ctx, prefixClaim, ipamv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil { + return ctrl.Result{}, fmt.Errorf("error updating status: %w, when look up of prefix by hash failed: %w", setConditionErr, err) + } + return ctrl.Result{Requeue: true}, nil } - // TODO: set condition for each error if prefixModel == nil { // Prefix cannot be restored from netbox @@ -135,7 +139,10 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) }, }) if err != nil { - return ctrl.Result{}, err + if setConditionErr := r.SetConditionAndCreateEvent(ctx, prefixClaim, ipamv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil { + return ctrl.Result{}, fmt.Errorf("error updating status: %w, when failed to get matching prefix: %w", setConditionErr, err) + } + return ctrl.Result{Requeue: true}, nil } debugLogger.Info(fmt.Sprintf("prefix is not reserved in netbox, assignined new prefix: %s", prefixModel.Prefix)) } else { @@ -215,38 +222,6 @@ func (r *PrefixClaimReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -func (r *PrefixClaimReconciler) GetAvailablePrefix(o *ipamv1.PrefixClaim) (*models.Prefix, error) { - var availablePrefix *models.Prefix - var err error - if availablePrefix, err = r.NetboxClient.GetAvailablePrefixByClaim( - &models.PrefixClaim{ - ParentPrefix: o.Spec.ParentPrefix, - PrefixLength: o.Spec.PrefixLength, - Metadata: &models.NetboxMetadata{ - Tenant: o.Spec.Tenant, - }, - }, - ); err != nil { - return nil, err - } - - if _, err = r.NetboxClient.ReserveOrUpdatePrefix( - &models.Prefix{ - Prefix: availablePrefix.Prefix, - Metadata: &models.NetboxMetadata{ - Comments: o.Spec.Comments, - Custom: map[string]string{}, - Description: o.Spec.Description, - Site: o.Spec.Site, - Tenant: o.Spec.Tenant, - }, - }); err != nil { - return nil, err - } - - return availablePrefix, nil -} - // TODO(henrybear327): Duplicated code, consider refactoring this func (r *PrefixClaimReconciler) SetConditionAndCreateEvent(ctx context.Context, o *ipamv1.PrefixClaim, condition metav1.Condition, eventType string, conditionMessageAppend string) error { if len(conditionMessageAppend) > 0 { diff --git a/internal/controller/utils.go b/internal/controller/utils.go new file mode 100644 index 00000000..b335b130 --- /dev/null +++ b/internal/controller/utils.go @@ -0,0 +1,25 @@ +/* +Copyright 2024 Swisscom (Schweiz) AG. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "strings" +) + +func convertCIDRToLeaseLockName(cidr string) string { + return strings.ReplaceAll(strings.ReplaceAll(cidr, "/", "-"), ":", "-") +} diff --git a/pkg/netbox/api/ip_address_claim.go b/pkg/netbox/api/ip_address_claim.go index 2c77db82..7da1c59f 100644 --- a/pkg/netbox/api/ip_address_claim.go +++ b/pkg/netbox/api/ip_address_claim.go @@ -23,10 +23,20 @@ import ( "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" "github.com/netbox-community/netbox-operator/pkg/netbox/models" + "github.com/netbox-community/netbox-operator/pkg/netbox/utils" ) +type IPFamily int64 + const ( - ipMask = "/32" + IPv4Family IPFamily = iota + 4 + _ // Skip 5 + IPv6Family +) + +const ( + ipMaskIPv4 = "/32" + ipMaskIPv6 = "/128" ) func (r *NetboxClient) RestoreExistingIpByHash(customFieldName string, hash string) (*models.IPAddress, error) { @@ -57,6 +67,10 @@ func (r *NetboxClient) RestoreExistingIpByHash(customFieldName string, hash stri // GetAvailableIpAddressByClaim searches an available IpAddress in Netbox matching IpAddressClaim requirements func (r *NetboxClient) GetAvailableIpAddressByClaim(ipAddressClaim *models.IPAddressClaim) (*models.IPAddress, error) { + _, err := r.GetTenantDetails(ipAddressClaim.Metadata.Tenant) + if err != nil { + return nil, err + } responseParentPrefix, err := r.GetPrefix(&models.Prefix{ Prefix: ipAddressClaim.ParentPrefix, @@ -66,7 +80,7 @@ func (r *NetboxClient) GetAvailableIpAddressByClaim(ipAddressClaim *models.IPAdd return nil, err } if len(responseParentPrefix.Payload.Results) == 0 { - return nil, errors.New("parent prefix not found") + return nil, utils.NetboxNotFoundError("parent prefix") } parentPrefixId := responseParentPrefix.Payload.Results[0].ID @@ -75,6 +89,15 @@ func (r *NetboxClient) GetAvailableIpAddressByClaim(ipAddressClaim *models.IPAdd return nil, err } + var ipMask string + if responseAvailableIPs.Payload[0].Family == int64(IPv4Family) { + ipMask = ipMaskIPv4 + } else if responseAvailableIPs.Payload[0].Family == int64(IPv6Family) { + ipMask = ipMaskIPv6 + } else { + return nil, errors.New("available ip has unknown IP family") + } + ipAddress, err := r.SetIpAddressMask(responseAvailableIPs.Payload[0].Address, ipMask) if err != nil { return nil, err diff --git a/pkg/netbox/api/ip_address_claim_test.go b/pkg/netbox/api/ip_address_claim_test.go index 3631a3f1..7d1ddd8e 100644 --- a/pkg/netbox/api/ip_address_claim_test.go +++ b/pkg/netbox/api/ip_address_claim_test.go @@ -17,6 +17,7 @@ limitations under the License. package api import ( + "errors" "testing" "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" @@ -35,20 +36,33 @@ func TestIPAddressClaim(t *testing.T) { mockIPAddress := mock_interfaces.NewMockIpamInterface(ctrl) mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) - parentPrefixId := int64(3) - parentPrefix := "10.114.0.0" - ipAddress1 := "10.112.140.1/24" - ipAddress2 := "10.112.140.2/24" - singleIpAddress2 := "10.112.140.2/32" + // test data for IPv4 ip address claim + parentPrefixIdV4 := int64(3) + parentPrefixV4 := "10.114.0.0" + ipAddressV4_1 := "10.112.140.1/24" + ipAddressV4_2 := "10.112.140.2/24" + singleIpAddressV4_2 := "10.112.140.2/32" - // example of available IP address - availAddress := func() []*netboxModels.AvailableIP { + // example of available IPv4 IP addresses + availAddressesIPv4 := func() []*netboxModels.AvailableIP { return []*netboxModels.AvailableIP{ - {Address: ipAddress1}, - {Address: ipAddress2}, + { + Address: ipAddressV4_1, + Family: int64(IPv4Family), + }, + { + Address: ipAddressV4_2, + Family: int64(IPv4Family), + }, } } + // test data for IPv6 ip address claim + parentPrefixIdV6 := int64(4) + parentPrefixV6 := "2001:db8:85a3:8d3::/64" + ipAddressV6 := "2001:db8:85a3:8d3::2/64" + singleIpAddressV6 := "2001:db8:85a3:8d3::2/128" + // example of tenant tenantId := int64(2) tenantName := "Tenant1" @@ -68,10 +82,10 @@ func TestIPAddressClaim(t *testing.T) { t.Run("Fetch available IP address's claim by parent prefix.", func(t *testing.T) { // ip address mock input - input := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixId) + input := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixIdV4) // ip address mock output output := &ipam.IpamPrefixesAvailableIpsListOK{ - Payload: availAddress(), + Payload: availAddressesIPv4(), } mockIPAddress.EXPECT().IpamPrefixesAvailableIpsList(input, nil).Return(output, nil) @@ -81,37 +95,40 @@ func TestIPAddressClaim(t *testing.T) { Ipam: mockIPAddress, } - actual, err := client.GetAvailableIpAddressesByParentPrefix(parentPrefixId) + actual, err := client.GetAvailableIpAddressesByParentPrefix(parentPrefixIdV4) // assert error return AssertNil(t, err) assert.Len(t, actual.Payload, 2) - assert.Equal(t, ipAddress1, actual.Payload[0].Address) - assert.Equal(t, ipAddress2, actual.Payload[1].Address) + assert.Equal(t, ipAddressV4_1, actual.Payload[0].Address) + assert.Equal(t, ipAddressV4_2, actual.Payload[1].Address) }) - t.Run("Fetch first available IP address by claim.", func(t *testing.T) { + t.Run("Fetch first available IP address by claim (IPv4).", func(t *testing.T) { inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) // ip address mock input - input := ipam.NewIpamPrefixesListParams().WithPrefix(&parentPrefix) + input := ipam.NewIpamPrefixesListParams().WithPrefix(&parentPrefixV4) // ip address mock output output := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { - ID: parentPrefixId, - Prefix: &parentPrefix, + ID: parentPrefixIdV4, + Prefix: &parentPrefixV4, }, }, }, } - inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixId) + inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixIdV4) outputIps := &ipam.IpamPrefixesAvailableIpsListOK{ Payload: []*netboxModels.AvailableIP{ - {Address: ipAddress2}, + { + Address: ipAddressV4_2, + Family: int64(IPv4Family), + }, }} mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes() @@ -125,7 +142,7 @@ func TestIPAddressClaim(t *testing.T) { } actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{ - ParentPrefix: parentPrefix, + ParentPrefix: parentPrefixV4, Metadata: &models.NetboxMetadata{ Tenant: tenantName, }, @@ -134,7 +151,109 @@ func TestIPAddressClaim(t *testing.T) { // assert error AssertNil(t, err) // assert nil output - assert.Equal(t, singleIpAddress2, actual.IpAddress) + assert.Equal(t, singleIpAddressV4_2, actual.IpAddress) + }) + + t.Run("Fetch first available IP address by claim (IPv6).", func(t *testing.T) { + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + // ip address mock input + input := ipam.NewIpamPrefixesListParams().WithPrefix(&parentPrefixV6) + // ip address mock output + output := &ipam.IpamPrefixesListOK{ + Payload: &ipam.IpamPrefixesListOKBody{ + Results: []*netboxModels.Prefix{ + { + ID: parentPrefixIdV6, + Prefix: &parentPrefixV6, + }, + }, + }, + } + + inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixIdV6) + outputIps := &ipam.IpamPrefixesAvailableIpsListOK{ + Payload: []*netboxModels.AvailableIP{ + { + Address: ipAddressV6, + Family: int64(IPv6Family), + }, + }} + + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes() + mockIPAddress.EXPECT().IpamPrefixesList(input, nil).Return(output, nil) + mockIPAddress.EXPECT().IpamPrefixesAvailableIpsList(inputIps, nil).Return(outputIps, nil) + + // init client + client := &NetboxClient{ + Tenancy: mockTenancy, + Ipam: mockIPAddress, + } + + actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{ + ParentPrefix: parentPrefixV6, + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + // assert error + AssertNil(t, err) + // assert nil output + assert.Equal(t, singleIpAddressV6, actual.IpAddress) + }) + + t.Run("Fetch first available IP address by claim (invalid IP family).", func(t *testing.T) { + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + // ip address mock input + input := ipam.NewIpamPrefixesListParams().WithPrefix(&parentPrefixV6) + // ip address mock output + output := &ipam.IpamPrefixesListOK{ + Payload: &ipam.IpamPrefixesListOKBody{ + Results: []*netboxModels.Prefix{ + { + ID: parentPrefixIdV6, + Prefix: &parentPrefixV6, + }, + }, + }, + } + + inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixIdV6) + outputIps := &ipam.IpamPrefixesAvailableIpsListOK{ + Payload: []*netboxModels.AvailableIP{ + { + Address: ipAddressV6, + Family: int64(5), + }, + }} + + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes() + mockIPAddress.EXPECT().IpamPrefixesList(input, nil).Return(output, nil) + mockIPAddress.EXPECT().IpamPrefixesAvailableIpsList(inputIps, nil).Return(outputIps, nil) + + // init client + client := &NetboxClient{ + Tenancy: mockTenancy, + Ipam: mockIPAddress, + } + + actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{ + ParentPrefix: parentPrefixV6, + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + // assert error + AssertError(t, err, "available ip has unknown IP family") + + var expected *models.IPAddress + + assert.Equal(t, expected, actual) }) t.Run("Fetch IP address's claim with incorrect parent prefix.", func(t *testing.T) { @@ -142,7 +261,7 @@ func TestIPAddressClaim(t *testing.T) { inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) // ip address mock input - input := ipam.NewIpamPrefixesListParams().WithPrefix(&parentPrefix) + input := ipam.NewIpamPrefixesListParams().WithPrefix(&parentPrefixV4) // ip address mock output output := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ @@ -160,14 +279,16 @@ func TestIPAddressClaim(t *testing.T) { } actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{ - ParentPrefix: parentPrefix, + ParentPrefix: parentPrefixV4, Metadata: &models.NetboxMetadata{ Tenant: tenantName, }, }) + expectedErrMsg := "failed to fetch parent prefix: not found" + // assert error - AssertError(t, err, "parent prefix not found") + AssertError(t, err, expectedErrMsg) // assert nil output assert.Nil(t, actual) }) @@ -175,7 +296,7 @@ func TestIPAddressClaim(t *testing.T) { t.Run("Fetch IP address's claim with exhausted parent prefix.", func(t *testing.T) { // ip address mock input - input := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixId) + input := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixIdV4) // ip address mock output output := &ipam.IpamPrefixesAvailableIpsListOK{ Payload: []*netboxModels.AvailableIP{}, @@ -188,7 +309,7 @@ func TestIPAddressClaim(t *testing.T) { Ipam: mockIPAddress, } - actual, err := client.GetAvailableIpAddressesByParentPrefix(parentPrefixId) + actual, err := client.GetAvailableIpAddressesByParentPrefix(parentPrefixIdV4) // assert error AssertError(t, err, "parent prefix exhausted") @@ -231,3 +352,78 @@ func TestIPAddressClaim(t *testing.T) { assert.Equal(t, ipAddressRestore, actual.IpAddress) }) } + +func TestIPAddressClaim_GetNoAvailableIPAddressWithTenancyChecks(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) + + parentPrefix := "10.112.140.0/24" + t.Run("No IP address asigned with an error when getting the tenant list", func(t *testing.T) { + + tenantName := "Tenant1" + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + // expected error + expectedErrorMsg := "cannot get the list" // testcase-defined error + + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(nil, errors.New(expectedErrorMsg)).AnyTimes() + + // init client + client := &NetboxClient{ + Tenancy: mockTenancy, + } + + actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{ + ParentPrefix: parentPrefix, + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + // assert error + assert.Containsf(t, err.Error(), expectedErrorMsg, "Error should contain: %v, got: %v", expectedErrorMsg, err.Error()) + // assert nil output + assert.Equal(t, actual, (*models.IPAddress)(nil)) + }) + + t.Run("No IP address asigned with non-existing tenant", func(t *testing.T) { + + // non existing tenant + nonExistingTenant := "non-existing-tenant" + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&nonExistingTenant) + + // empty tenant list + emptyTenantList := &tenancy.TenancyTenantsListOK{ + Payload: &tenancy.TenancyTenantsListOKBody{ + Results: []*netboxModels.Tenant{}, + }, + } + + // expected error + expectedErrorMsg := "failed to fetch tenant: not found" + + // mock empty list call + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(emptyTenantList, nil).AnyTimes() + + // init client + client := &NetboxClient{ + Tenancy: mockTenancy, + } + + actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{ + ParentPrefix: parentPrefix, + Metadata: &models.NetboxMetadata{ + Tenant: nonExistingTenant, + }, + }) + + // assert error + assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err) + // assert nil output + assert.Equal(t, actual, (*models.IPAddress)(nil)) + }) +} diff --git a/pkg/netbox/api/prefix_claim.go b/pkg/netbox/api/prefix_claim.go index ccac1ba0..a6de143a 100644 --- a/pkg/netbox/api/prefix_claim.go +++ b/pkg/netbox/api/prefix_claim.go @@ -53,20 +53,40 @@ func (r *NetboxClient) RestoreExistingPrefixByHash(hash string) (*models.Prefix, }, nil } -func isRequestingTheEntireParentPrefix(prefixClaim *models.PrefixClaim) (bool, error) { +func validatePrefixLengthOrError(prefixClaim *models.PrefixClaim, prefixFamily int64) error { parentPrefixSplit := strings.Split(prefixClaim.ParentPrefix, "/") if len(parentPrefixSplit) != 2 { - return false, errors.New("invalid parent prefix format") + return errors.New("invalid parent prefix format") } - if "/"+parentPrefixSplit[1] /* e.g. /24 */ == prefixClaim.PrefixLength /* e.g. /24 */ { - return true, nil + parentPrefixLength, err := strconv.Atoi(parentPrefixSplit[1]) + if err != nil { + return err + } + + requestedPrefixLength, err := strconv.Atoi(strings.TrimPrefix(prefixClaim.PrefixLength, "/")) + if err != nil { + return err } - return false, nil + + if parentPrefixLength == requestedPrefixLength { + return errors.New("requesting the entire parent prefix range is disallowed") + } else if parentPrefixLength > requestedPrefixLength { + return errors.New("requested prefix size must be smaller than the parent prefix size") + } else if prefixFamily == int64(IPv4Family) && requestedPrefixLength > 32 { + return errors.New("requested prefix length must be smaller than 32 for IPv4") + } + + return nil } // 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) + if err != nil { + return nil, err + } + responseParentPrefix, err := r.GetPrefix(&models.Prefix{ Prefix: prefixClaim.ParentPrefix, Metadata: prefixClaim.Metadata, @@ -78,17 +98,12 @@ func (r *NetboxClient) GetAvailablePrefixByClaim(prefixClaim *models.PrefixClaim return nil, errors.New("parent prefix not found") } - parentPrefixId := responseParentPrefix.Payload.Results[0].ID - - // We reject target prefix size == parent prefix size - if ret, err := isRequestingTheEntireParentPrefix(prefixClaim); err != nil { + if err := validatePrefixLengthOrError(prefixClaim, *responseParentPrefix.Payload.Results[0].Family.Value); err != nil { return nil, err - } else if ret { - // The issue lies in the prefix deletion. - // We do not want the operator to delete the parent prefix as a side effect of deleting any allocated prefixes - return nil, errors.New("requesting for the entire parent prefix range is disallowed") } + parentPrefixId := responseParentPrefix.Payload.Results[0].ID + /* Notes regarding the available prefix returned by netbox The available prefixes API do NOT allow us to pass in the desired prefix size. And we observed the API's behavior as the following: diff --git a/pkg/netbox/api/prefix_claim_test.go b/pkg/netbox/api/prefix_claim_test.go index 7f4812b5..f05c20ac 100644 --- a/pkg/netbox/api/prefix_claim_test.go +++ b/pkg/netbox/api/prefix_claim_test.go @@ -38,13 +38,20 @@ func TestPrefixClaim_GetAvailablePrefixesByParentPrefix(t *testing.T) { //prefix mock input parentPrefixId := int64(3) availablePrefixListInput := ipam.NewIpamPrefixesAvailablePrefixesListParams().WithID(parentPrefixId) + //prefix mock output childPrefix1 := "10.112.140.0/24" childPrefix2 := "10.120.180.0/24" availablePrefixListOutput := &ipam.IpamPrefixesAvailablePrefixesListOK{ Payload: []*netboxModels.AvailablePrefix{ - {Prefix: childPrefix1}, - {Prefix: childPrefix2}, + { + Prefix: childPrefix1, + Family: int64(IPv4Family), + }, + { + Prefix: childPrefix2, + Family: int64(IPv4Family), + }, }, } @@ -110,6 +117,7 @@ func TestPrefixClaim_GetAvailablePrefixByClaim_WithWrongParent(t *testing.T) { prefixListInput := ipam. NewIpamPrefixesListParams(). WithPrefix(&prefix) + prefixListOutput := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{}, @@ -129,6 +137,9 @@ func TestPrefixClaim_GetAvailablePrefixByClaim_WithWrongParent(t *testing.T) { actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ ParentPrefix: prefix, PrefixLength: "/28", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, }) assert.Nil(t, actual) assert.EqualError(t, err, "parent prefix not found") @@ -162,12 +173,16 @@ func TestPrefixClaim_GetBestFitPrefixByClaim(t *testing.T) { prefixListInput := ipam. NewIpamPrefixesListParams(). WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 prefixListOutput := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { ID: parentPrefixId, Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, }, }, }, @@ -196,6 +211,282 @@ func TestPrefixClaim_GetBestFitPrefixByClaim(t *testing.T) { actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ ParentPrefix: parentPrefix, PrefixLength: "/28", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + assert.Nil(t, err) + assert.Equal(t, prefix, actual.Prefix) +} + +func TestPrefixClaim_InvalidIPv4PrefixLength(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl) + mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) + + // example of tenant + tenantId := int64(2) + tenantName := "Tenant1" + tenantOutputSlug := "tenant1" + expectedTenant := &tenancy.TenancyTenantsListOK{ + Payload: &tenancy.TenancyTenantsListOKBody{ + Results: []*netboxModels.Tenant{ + { + ID: tenantId, + Name: &tenantName, + Slug: &tenantOutputSlug, + }, + }, + }, + } + + parentPrefix := "10.112.140.0/24" + parentPrefixId := int64(1) + prefixListInput := ipam. + NewIpamPrefixesListParams(). + WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 + prefixListOutput := &ipam.IpamPrefixesListOK{ + Payload: &ipam.IpamPrefixesListOKBody{ + Results: []*netboxModels.Prefix{ + { + ID: parentPrefixId, + Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, + }, + }, + }, + } + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + mockPrefixIpam.EXPECT().IpamPrefixesList(prefixListInput, nil).Return(prefixListOutput, nil).AnyTimes() + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes() + + netboxClient := &NetboxClient{ + Ipam: mockPrefixIpam, + Tenancy: mockTenancy, + } + + actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: parentPrefix, + PrefixLength: "/33", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + var expectedPrefix *models.Prefix + + assert.Error(t, err) + assert.Equal(t, expectedPrefix, actual) +} + +func TestPrefixClaim_FailWhenRequestingEntirePrefix(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl) + mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) + + // example of tenant + tenantId := int64(2) + tenantName := "Tenant1" + tenantOutputSlug := "tenant1" + expectedTenant := &tenancy.TenancyTenantsListOK{ + Payload: &tenancy.TenancyTenantsListOKBody{ + Results: []*netboxModels.Tenant{ + { + ID: tenantId, + Name: &tenantName, + Slug: &tenantOutputSlug, + }, + }, + }, + } + + parentPrefix := "10.112.140.0/24" + parentPrefixId := int64(1) + prefixListInput := ipam. + NewIpamPrefixesListParams(). + WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 + prefixListOutput := &ipam.IpamPrefixesListOK{ + Payload: &ipam.IpamPrefixesListOKBody{ + Results: []*netboxModels.Prefix{ + { + ID: parentPrefixId, + Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, + }, + }, + }, + } + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + mockPrefixIpam.EXPECT().IpamPrefixesList(prefixListInput, nil).Return(prefixListOutput, nil).AnyTimes() + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes() + + netboxClient := &NetboxClient{ + Ipam: mockPrefixIpam, + Tenancy: mockTenancy, + } + + actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: parentPrefix, + PrefixLength: "/24", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + var expectedPrefix *models.Prefix + + assert.Error(t, err) + assert.Equal(t, expectedPrefix, actual) +} + +func TestPrefixClaim_FailWhenPrefixLargerThanParent(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl) + mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) + + // example of tenant + tenantId := int64(2) + tenantName := "Tenant1" + tenantOutputSlug := "tenant1" + expectedTenant := &tenancy.TenancyTenantsListOK{ + Payload: &tenancy.TenancyTenantsListOKBody{ + Results: []*netboxModels.Tenant{ + { + ID: tenantId, + Name: &tenantName, + Slug: &tenantOutputSlug, + }, + }, + }, + } + + parentPrefix := "10.112.140.0/24" + parentPrefixId := int64(1) + prefixListInput := ipam. + NewIpamPrefixesListParams(). + WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 + prefixListOutput := &ipam.IpamPrefixesListOK{ + Payload: &ipam.IpamPrefixesListOKBody{ + Results: []*netboxModels.Prefix{ + { + ID: parentPrefixId, + Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, + }, + }, + }, + } + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + mockPrefixIpam.EXPECT().IpamPrefixesList(prefixListInput, nil).Return(prefixListOutput, nil).AnyTimes() + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes() + + netboxClient := &NetboxClient{ + Ipam: mockPrefixIpam, + Tenancy: mockTenancy, + } + + actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: parentPrefix, + PrefixLength: "/20", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + var expectedPrefix *models.Prefix + + assert.Error(t, err) + assert.Equal(t, expectedPrefix, actual) +} + +func TestPrefixClaim_ValidIPv6PrefixLength(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl) + mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) + + // example of tenant + tenantId := int64(2) + tenantName := "Tenant1" + tenantOutputSlug := "tenant1" + expectedTenant := &tenancy.TenancyTenantsListOK{ + Payload: &tenancy.TenancyTenantsListOKBody{ + Results: []*netboxModels.Tenant{ + { + ID: tenantId, + Name: &tenantName, + Slug: &tenantOutputSlug, + }, + }, + }, + } + + parentPrefix := "2001:db8:85a3:8d3::/30" + parentPrefixId := int64(1) + prefix := "2001:db8:85a3:8d3::/33" + prefixListInput := ipam. + NewIpamPrefixesListParams(). + WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv6Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV6 + prefixListOutput := &ipam.IpamPrefixesListOK{ + Payload: &ipam.IpamPrefixesListOKBody{ + Results: []*netboxModels.Prefix{ + { + ID: parentPrefixId, + Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, + }, + }, + }, + } + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + prefixAvailableListInput := ipam.NewIpamPrefixesAvailablePrefixesListParams().WithID(parentPrefixId) + prefixAvailableListOutput := &ipam.IpamPrefixesAvailablePrefixesListOK{ + Payload: []*netboxModels.AvailablePrefix{ + { + Prefix: prefix, + }, + }, + } + + mockPrefixIpam.EXPECT().IpamPrefixesList(prefixListInput, nil).Return(prefixListOutput, nil).AnyTimes() + mockPrefixIpam.EXPECT().IpamPrefixesAvailablePrefixesList(prefixAvailableListInput, nil).Return(prefixAvailableListOutput, nil).AnyTimes() + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes() + + netboxClient := &NetboxClient{ + Ipam: mockPrefixIpam, + Tenancy: mockTenancy, + } + + actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: parentPrefix, + PrefixLength: "/33", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, }) assert.Nil(t, err) @@ -232,12 +523,16 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSize(t *test prefixListInput := ipam. NewIpamPrefixesListParams(). WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 prefixListOutput := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { ID: parentPrefixId, Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, }, }, }, @@ -272,6 +567,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSize(t *test actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ ParentPrefix: parentPrefix, PrefixLength: "/28", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, }) assert.Nil(t, err) @@ -306,12 +604,16 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSizeCriteria prefixListInput := ipam. NewIpamPrefixesListParams(). WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 prefixListOutput := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { ID: parentPrefixId, Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, }, }, }, @@ -340,6 +642,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSizeCriteria _, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ ParentPrefix: parentPrefix, PrefixLength: "/28", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, }) assert.True(t, errors.Is(err, ErrNoPrefixMatchsSizeCriteria)) @@ -375,12 +680,16 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidFormatFromNetbox(t *testing.T prefixListInput := ipam. NewIpamPrefixesListParams(). WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 prefixListOutput := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { ID: parentPrefixId, Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, }, }, }, @@ -415,6 +724,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidFormatFromNetbox(t *testing.T actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ ParentPrefix: parentPrefix, PrefixLength: "/28", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, }) assert.Nil(t, err) @@ -449,12 +761,16 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidPrefixClaim(t *testing.T) { prefixListInput := ipam. NewIpamPrefixesListParams(). WithPrefix(&parentPrefix) + + prefixFamily := int64(IPv4Family) + prefixFamilyLabel := netboxModels.PrefixFamilyLabelIPV4 prefixListOutput := &ipam.IpamPrefixesListOK{ Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { ID: parentPrefixId, Prefix: &parentPrefix, + Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily}, }, }, }, @@ -483,7 +799,92 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidPrefixClaim(t *testing.T) { _, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ ParentPrefix: parentPrefix, PrefixLength: "/28.", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, }) assert.True(t, errors.Is(err, strconv.ErrSyntax)) } + +func TestPrefixClaim_GetNoAvailablePrefixesWithNonExistingTenant(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) + + // non-existing tenant + tenantName := "non-existing-tenant" + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + // expected error + expectedErrorMsg := "failed to fetch tenant: not found" + + // empty tenant list + emptyTenantList := &tenancy.TenancyTenantsListOK{ + Payload: &tenancy.TenancyTenantsListOKBody{ + Results: []*netboxModels.Tenant{}, + }, + } + + parentPrefix := "10.112.140.0/24" + + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(emptyTenantList, nil).AnyTimes() + + netboxClient := &NetboxClient{ + Tenancy: mockTenancy, + } + + prefix, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: parentPrefix, + PrefixLength: "/28.", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err) + assert.Equal(t, prefix, (*models.Prefix)(nil)) +} + +func TestPrefixClaim_GetNoAvailablePrefixesWithErrorWhenGettingTenantList(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) + + // non-existing tenant + tenantName := "non-existing tenant" + + inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) + + // expected errors + getTenantDetailsErrorMsg := "failed to fetch Tenant details" + tenancyTenantsListErrorMsg := "cannot get the list" // testcase defined error + + parentPrefix := "10.112.140.0/24" + + mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(nil, errors.New(tenancyTenantsListErrorMsg)).AnyTimes() + + netboxClient := &NetboxClient{ + Tenancy: mockTenancy, + } + + prefix, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{ + ParentPrefix: parentPrefix, + PrefixLength: "/28.", + Metadata: &models.NetboxMetadata{ + Tenant: tenantName, + }, + }) + + // assert 1st level error - GetTenantDetails() + assert.Containsf(t, err.Error(), getTenantDetailsErrorMsg, "expected error containing %q, got %s", getTenantDetailsErrorMsg, err) + + // assert 2nd level error - TenanyTenantsList() + assert.Containsf(t, err.Error(), tenancyTenantsListErrorMsg, "expected error containing %q, got %s", tenancyTenantsListErrorMsg, err) + + // assert nil output + assert.Equal(t, prefix, (*models.Prefix)(nil)) +} diff --git a/pkg/netbox/api/tenancy.go b/pkg/netbox/api/tenancy.go index 016fa3c4..12b816a6 100644 --- a/pkg/netbox/api/tenancy.go +++ b/pkg/netbox/api/tenancy.go @@ -17,8 +17,6 @@ limitations under the License. package api import ( - "errors" - "github.com/netbox-community/go-netbox/v3/netbox/client/tenancy" "github.com/netbox-community/netbox-operator/pkg/netbox/models" @@ -32,7 +30,7 @@ func (r *NetboxClient) GetTenantDetails(name string) (*models.Tenant, error) { return nil, utils.NetboxError("failed to fetch Tenant details", err) } if len(response.Payload.Results) == 0 { - return nil, errors.New("tenant not found") + return nil, utils.NetboxNotFoundError("tenant") } return &models.Tenant{ diff --git a/pkg/netbox/api/tenancy_test.go b/pkg/netbox/api/tenancy_test.go index c2428c1b..d8238dbf 100644 --- a/pkg/netbox/api/tenancy_test.go +++ b/pkg/netbox/api/tenancy_test.go @@ -80,5 +80,5 @@ func TestTenancy_GetWrongTenantDetails(t *testing.T) { actual, err := netboxClient.GetTenantDetails(wrongTenant) assert.Nil(t, actual) - assert.EqualError(t, err, "tenant not found") + assert.EqualError(t, err, "failed to fetch tenant: not found") } diff --git a/pkg/netbox/utils/error.go b/pkg/netbox/utils/error.go index 223ed4f8..71befab3 100644 --- a/pkg/netbox/utils/error.go +++ b/pkg/netbox/utils/error.go @@ -17,10 +17,17 @@ limitations under the License. package utils import ( + "fmt" + "github.com/pkg/errors" ) +var ErrNotFound = errors.New("not found") + func NetboxError(message string, err error) error { + return fmt.Errorf(message+": %w", err) +} - return errors.Errorf(message, err.Error()) +func NetboxNotFoundError(name string) error { + return NetboxError("failed to fetch "+name, ErrNotFound) }