Fix: add logic to update site of prefix in netbox (#107)

* add logic to update site of prefix in netbox * add validation rule for site
netbox-community · Nov 11, 2024 · 3282272 · 3282272
1 parent 11c4c5f
commit 3282272
Show file tree

Hide file tree

Showing 22 changed files with 591 additions and 8 deletions.
diff --git a/api/v1/prefix_types.go b/api/v1/prefix_types.go
@@ -24,6 +24,7 @@ import (
 // NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
 
 // PrefixSpec defines the desired state of Prefix
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.site) || has(self.site)", message="Site is required once set"
 type PrefixSpec struct {
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 	// Important: Run "make" to regenerate code after modifying this file
@@ -33,6 +34,7 @@ type PrefixSpec struct {
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'prefix' is immutable"
 	Prefix string `json:"prefix"`
 
+	//+kubebuilder:validation:XValidation:rule="self == oldSelf || self != ''",message="Field 'site' is required once set"
 	Site string `json:"site,omitempty"`
 
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'tenant' is immutable"

diff --git a/api/v1/prefixclaim_types.go b/api/v1/prefixclaim_types.go
@@ -24,6 +24,7 @@ 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
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.site) || has(self.site)", message="Site is required once set"
 type PrefixClaimSpec struct {
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 	// Important: Run "make" to regenerate code after modifying this file
@@ -38,6 +39,7 @@ type PrefixClaimSpec struct {
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'prefixLength' is immutable"
 	PrefixLength string `json:"prefixLength"`
 
+	//+kubebuilder:validation:XValidation:rule="self == oldSelf || self != ''",message="Field 'site' is required once set"
 	Site string `json:"site,omitempty"`
 
 	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'tenant' is immutable"

diff --git a/config/crd/bases/netbox.dev_prefixclaims.yaml b/config/crd/bases/netbox.dev_prefixclaims.yaml
@@ -78,6 +78,9 @@ spec:
                 type: boolean
               site:
                 type: string
+                x-kubernetes-validations:
+                - message: Field 'site' is required once set
+                  rule: self == oldSelf || self != ''
               tenant:
                 type: string
                 x-kubernetes-validations:
@@ -87,6 +90,9 @@ spec:
             - parentPrefix
             - prefixLength
             type: object
+            x-kubernetes-validations:
+            - message: Site is required once set
+              rule: '!has(oldSelf.site) || has(self.site)'
           status:
             description: PrefixClaimStatus defines the observed state of PrefixClaim
             properties:

diff --git a/config/crd/bases/netbox.dev_prefixes.yaml b/config/crd/bases/netbox.dev_prefixes.yaml
@@ -75,6 +75,9 @@ spec:
                 type: boolean
               site:
                 type: string
+                x-kubernetes-validations:
+                - message: Field 'site' is required once set
+                  rule: self == oldSelf || self != ''
               tenant:
                 type: string
                 x-kubernetes-validations:
@@ -83,6 +86,9 @@ spec:
             required:
             - prefix
             type: object
+            x-kubernetes-validations:
+            - message: Site is required once set
+              rule: '!has(oldSelf.site) || has(self.site)'
           status:
             description: PrefixStatus defines the observed state of Prefix
             properties:

diff --git a/config/samples/netbox_v1_prefix.yaml b/config/samples/netbox_v1_prefix.yaml
@@ -7,7 +7,7 @@ metadata:
   name: prefix-sample
 spec:
   tenant: "Dunder-Mifflin, Inc."
-  site: "DataCenter"
+  site: "DM-Akron"
   description: "some description"
   comments: "your comments"
   preserveInNetbox: true

diff --git a/config/samples/netbox_v1_prefixclaim.yaml b/config/samples/netbox_v1_prefixclaim.yaml
@@ -7,7 +7,7 @@ metadata:
   name: prefixclaim-sample
 spec:
   tenant: "Dunder-Mifflin, Inc."
-  site: "DataCenter"
+  site: "DM-Akron"
   description: "some description"
   comments: "your comments"
   preserveInNetbox: true

diff --git a/gen/mock_interfaces/netbox_mocks.go b/gen/mock_interfaces/netbox_mocks.go
diff --git a/internal/controller/prefixclaim_controller.go b/internal/controller/prefixclaim_controller.go
@@ -136,6 +136,7 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 					PrefixLength: prefixClaim.Spec.PrefixLength,
 					Metadata: &models.NetboxMetadata{
 						Tenant: prefixClaim.Spec.Tenant,
+						Site:   prefixClaim.Spec.Site,
 					},
 				})
 			if err != nil {

diff --git a/internal/controller/prefixclaim_helpers.go b/internal/controller/prefixclaim_helpers.go
@@ -54,6 +54,7 @@ func generatePrefixSpec(claim *netboxv1.PrefixClaim, prefix string, logger logr.
 	return netboxv1.PrefixSpec{
 		Prefix:           prefix,
 		Tenant:           claim.Spec.Tenant,
+		Site:             claim.Spec.Site,
 		CustomFields:     customFields,
 		Description:      claim.Spec.Description,
 		Comments:         claim.Spec.Comments,

diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
@@ -58,6 +58,7 @@ var mockCtrl *gomock.Controller
 var ipamMockIpAddress *mock_interfaces.MockIpamInterface
 var ipamMockIpAddressClaim *mock_interfaces.MockIpamInterface
 var tenancyMock *mock_interfaces.MockTenancyInterface
+var dcimMock *mock_interfaces.MockDcimInterface
 var ctx context.Context
 var cancel context.CancelFunc
 
@@ -111,6 +112,7 @@ var _ = BeforeSuite(func() {
 	ipamMockIpAddress = mock_interfaces.NewMockIpamInterface(mockCtrl)
 	ipamMockIpAddressClaim = mock_interfaces.NewMockIpamInterface(mockCtrl)
 	tenancyMock = mock_interfaces.NewMockTenancyInterface(mockCtrl)
+	dcimMock = mock_interfaces.NewMockDcimInterface(mockCtrl)
 
 	k8sManager, err := ctrl.NewManager(cfg, k8sManagerOptions)
 	Expect(k8sManager.GetConfig()).NotTo(BeNil())
@@ -123,6 +125,7 @@ var _ = BeforeSuite(func() {
 		NetboxClient: &api.NetboxClient{
 			Ipam:    ipamMockIpAddress,
 			Tenancy: tenancyMock,
+			Dcim:    dcimMock,
 		},
 		OperatorNamespace: OperatorNamespace,
 		RestConfig:        k8sManager.GetConfig(),
@@ -136,6 +139,7 @@ var _ = BeforeSuite(func() {
 		NetboxClient: &api.NetboxClient{
 			Ipam:    ipamMockIpAddressClaim,
 			Tenancy: tenancyMock,
+			Dcim:    dcimMock,
 		},
 		OperatorNamespace: OperatorNamespace,
 		RestConfig:        k8sManager.GetConfig(),

diff --git a/pkg/netbox/api/client.go b/pkg/netbox/api/client.go
@@ -40,6 +40,7 @@ type NetboxClient struct {
 	Ipam    interfaces.IpamInterface
 	Tenancy interfaces.TenancyInterface
 	Extras  interfaces.ExtrasInterface
+	Dcim    interfaces.DcimInterface
 }
 
 // Checks that the Netbox host is properly configured for the operator to function.
@@ -108,6 +109,7 @@ func GetNetboxClient() (*NetboxClient, error) {
 		Ipam:    auxNetboxClient.Ipam,
 		Tenancy: auxNetboxClient.Tenancy,
 		Extras:  auxNetboxClient.Extras,
+		Dcim:    auxNetboxClient.Dcim,
 	}
 
 	return netboxClient, nil

diff --git a/pkg/netbox/api/ip_address_claim_test.go b/pkg/netbox/api/ip_address_claim_test.go
@@ -404,7 +404,7 @@ func TestIPAddressClaim_GetNoAvailableIPAddressWithTenancyChecks(t *testing.T) {
 		}
 
 		// expected error
-		expectedErrorMsg := "failed to fetch tenant: not found"
+		expectedErrorMsg := "failed to fetch tenant 'non-existing-tenant': not found"
 
 		// mock empty list call
 		mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(emptyTenantList, nil).AnyTimes()

diff --git a/pkg/netbox/api/prefix.go b/pkg/netbox/api/prefix.go
@@ -51,6 +51,14 @@ func (r *NetboxClient) ReserveOrUpdatePrefix(prefix *models.Prefix) (*netboxMode
 		desiredPrefix.Tenant = &tenantDetails.Id
 	}
 
+	if prefix.Metadata.Site != "" {
+		siteDetails, err := r.GetSiteDetails(prefix.Metadata.Site)
+		if err != nil {
+			return nil, err
+		}
+		desiredPrefix.Site = &siteDetails.Id
+	}
+
 	// create prefix since it doesn't exist
 	if len(responsePrefix.Payload.Results) == 0 {
 		return r.CreatePrefix(desiredPrefix)

diff --git a/pkg/netbox/api/prefix_claim.go b/pkg/netbox/api/prefix_claim.go
@@ -87,6 +87,14 @@ func (r *NetboxClient) GetAvailablePrefixByClaim(prefixClaim *models.PrefixClaim
 		return nil, err
 	}
 
+	// Don't assign an prefix if the requested site doesn't exist in netbox
+	if prefixClaim.Metadata.Site != "" {
+		_, err := r.GetSiteDetails(prefixClaim.Metadata.Site)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	responseParentPrefix, err := r.GetPrefix(&models.Prefix{
 		Prefix:   prefixClaim.ParentPrefix,
 		Metadata: prefixClaim.Metadata,