Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/v1/prefix_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
henrybear327 marked this conversation as resolved.
Show resolved Hide resolved
type PrefixSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file
Expand All @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions api/v1/prefixclaim_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/netbox.dev_prefixclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/netbox.dev_prefixes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion config/samples/netbox_v1_prefix.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion config/samples/netbox_v1_prefixclaim.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions gen/mock_interfaces/netbox_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions internal/controller/prefixclaim_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions internal/controller/prefixclaim_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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())
Expand All @@ -123,6 +125,7 @@ var _ = BeforeSuite(func() {
NetboxClient: &api.NetboxClient{
Ipam: ipamMockIpAddress,
Tenancy: tenancyMock,
Dcim: dcimMock,
},
OperatorNamespace: OperatorNamespace,
RestConfig: k8sManager.GetConfig(),
Expand All @@ -136,6 +139,7 @@ var _ = BeforeSuite(func() {
NetboxClient: &api.NetboxClient{
Ipam: ipamMockIpAddressClaim,
Tenancy: tenancyMock,
Dcim: dcimMock,
},
OperatorNamespace: OperatorNamespace,
RestConfig: k8sManager.GetConfig(),
Expand Down
2 changes: 2 additions & 0 deletions pkg/netbox/api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -108,6 +109,7 @@ func GetNetboxClient() (*NetboxClient, error) {
Ipam: auxNetboxClient.Ipam,
Tenancy: auxNetboxClient.Tenancy,
Extras: auxNetboxClient.Extras,
Dcim: auxNetboxClient.Dcim,
}

return netboxClient, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/netbox/api/ip_address_claim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
8 changes: 8 additions & 0 deletions pkg/netbox/api/prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions pkg/netbox/api/prefix_claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading