diff --git a/api/v1/iprangeclaim_types.go b/api/v1/iprangeclaim_types.go index d3a7f7ea..fe859510 100644 --- a/api/v1/iprangeclaim_types.go +++ b/api/v1/iprangeclaim_types.go @@ -181,7 +181,7 @@ var ConditionIpRangeAssignedFalse = metav1.Condition{ Message: "Failed to fetch new IP Range from NetBox", } -var ConditionIpRangeAssignedFalseSizeMissmatch = metav1.Condition{ +var ConditionIpRangeAssignedFalseSizeMismatch = metav1.Condition{ Type: "IPRangeAssigned", Status: "False", Reason: "IPRangeCRNotCreated", diff --git a/config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml b/config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml index a437b0c2..8a9daea5 100644 --- a/config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml +++ b/config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml @@ -14,7 +14,6 @@ spec: prefixLength: "/31" parentPrefixSelector: tenant: "MY_TENANT" - site: "DM-Buffalo" family: "IPv4" environment: "Production" - poolName: "Pool 1" \ No newline at end of file + poolName: "Pool 1" diff --git a/internal/controller/expected_netboxmock_calls_test.go b/internal/controller/expected_netboxmock_calls_test.go index e8c049ac..5837e7fd 100644 --- a/internal/controller/expected_netboxmock_calls_test.go +++ b/internal/controller/expected_netboxmock_calls_test.go @@ -41,7 +41,7 @@ func mockIpAddressListWithIpAddressFilter(ipamMock *mock_interfaces.MockIpamInte return &ipam.IpamIPAddressesListOK{Payload: nil}, err } fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList was called with expected input\n") - return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressList()}, nil + return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHash)}, nil }).MinTimes(1) } @@ -92,6 +92,22 @@ func mockIpAddressListWithHashFilter(ipamMock *mock_interfaces.MockIpamInterface }).MinTimes(1) } +func mockIpAddressListWithHashFilterMismatch(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) { + ipamMock.EXPECT().IpamIPAddressesList(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamIPAddressesListOK, error) { + got := params.(*ipam.IpamIPAddressesListParams) + diff := deep.Equal(got, ExpectedIpAddressListParamsWithIpAddressData) + // skip check for the 3rd input parameter as it is a method, method is a non comparable type + if len(diff) > 0 { + err := fmt.Errorf("netboxmock: unexpected call to ipam.IpamIPAddressesList, diff to expected params diff: %+v", diff) + catchUnexpectedParams <- err + return &ipam.IpamIPAddressesListOK{Payload: nil}, err + } + fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList (empty reslut) was called with expected input,\n") + return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHashMismatch)}, nil + }).MinTimes(1) +} + func mockPrefixesListWithPrefixFilter(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) { ipamMock.EXPECT().IpamPrefixesList(gomock.Any(), gomock.Any()). DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamPrefixesListOK, error) { diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go index 0e1b0900..24a98dd2 100644 --- a/internal/controller/ipaddress_controller.go +++ b/internal/controller/ipaddress_controller.go @@ -170,10 +170,30 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( netboxIpAddressModel, err := r.NetboxClient.ReserveOrUpdateIpAddress(ipAddressModel) if err != nil { - updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse, - corev1.EventTypeWarning, o.Spec.IpAddress) - return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+ - "after reservation of ip in netbox failed: %w", updateStatusErr, err) + if errors.Is(err, api.ErrRestorationHashMismatch) && o.Status.IpAddressId == 0 { + // if there is a restoration hash mismatch and the IpAddressId status field is not set, + // delete the ip address so it can be recreated by the ip address claim controller + // this will only affect resources that are created by a claim controller (and have a restoration hash custom field + logger.Info("restoration hash mismatch, deleting ip address custom resource", "ipaddress", o.Spec.IpAddress) + err = r.Client.Delete(ctx, o) + if err != nil { + if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse, + corev1.EventTypeWarning, err.Error()); updateStatusErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+ + "after deletion of ip address cr failed: %w", updateStatusErr, err) + } + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, nil + + } + + if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse, + corev1.EventTypeWarning, err.Error()); updateStatusErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+ + "after reservation of ip in netbox failed: %w", updateStatusErr, err) + } + return ctrl.Result{Requeue: true}, nil } // 3. unlock lease of parent prefix diff --git a/internal/controller/ipaddress_controller_test.go b/internal/controller/ipaddress_controller_test.go index b5f8f56c..4ecc5019 100644 --- a/internal/controller/ipaddress_controller_test.go +++ b/internal/controller/ipaddress_controller_test.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" netboxv1 "github.com/netbox-community/netbox-operator/api/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" ) var _ = Describe("IpAddress Controller", Ordered, func() { @@ -51,6 +52,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() { cr *netboxv1.IpAddress, // our CR as typed object IpamMocksIpAddress []func(*mock_interfaces.MockIpamInterface, chan error), TenancyMocks []func(*mock_interfaces.MockTenancyInterface, chan error), + restorationHashMismatch bool, // To check for deletion if restoration hash does not match expectedConditionReady bool, // Expected state of the ConditionReady condition expectedCRStatus netboxv1.IpAddressStatus, // Expected status of the CR ) { @@ -81,31 +83,40 @@ var _ = Describe("IpAddress Controller", Ordered, func() { By("Creating IpAddress CR") Eventually(k8sClient.Create(ctx, cr), timeout, interval).Should(Succeed()) - // check that reconcile loop did run a least once by checking that conditions are set createdCR := &netboxv1.IpAddress{} - Eventually(func() bool { - err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR) - return err == nil && len(createdCR.Status.Conditions) > 0 - }, timeout, interval).Should(BeTrue()) - - // Now check if conditions are set as expected - Eventually(func() bool { - err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR) - return err == nil && - apismeta.IsStatusConditionTrue(createdCR.Status.Conditions, netboxv1.ConditionIpaddressReadyTrue.Type) == expectedConditionReady - }, timeout, interval).Should(BeTrue()) - - // Check that the expected ip address is present in the status - Expect(createdCR.Status.IpAddressId).To(Equal(expectedCRStatus.IpAddressId)) - - // Cleanup the netbox resources - Expect(k8sClient.Delete(ctx, createdCR)).Should(Succeed()) - - // Wait until the resource is deleted to make sure that it will not interfere with the next test case - Eventually(func() bool { - err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR) - return err != client.IgnoreNotFound(err) - }, timeout, interval).Should(BeTrue()) + + if restorationHashMismatch { + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR) + return apierrors.IsNotFound(err) + }, timeout, interval).Should(BeTrue()) + } else { + + // check that reconcile loop did run a least once by checking that conditions are set + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR) + return err == nil && len(createdCR.Status.Conditions) > 0 + }, timeout, interval).Should(BeTrue()) + + // Now check if conditions are set as expected + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR) + return err == nil && + apismeta.IsStatusConditionTrue(createdCR.Status.Conditions, netboxv1.ConditionIpaddressReadyTrue.Type) == expectedConditionReady + }, timeout, interval).Should(BeTrue()) + + // Check that the expected ip address is present in the status + Expect(createdCR.Status.IpAddressId).To(Equal(expectedCRStatus.IpAddressId)) + + // Cleanup the netbox resources + Expect(k8sClient.Delete(ctx, createdCR)).Should(Succeed()) + + // Wait until the resource is deleted to make sure that it will not interfere with the next test case + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR) + return err != client.IgnoreNotFound(err) + }, timeout, interval).Should(BeTrue()) + } catchCtxCancel() }, @@ -119,7 +130,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() { []func(*mock_interfaces.MockTenancyInterface, chan error){ mockTenancyTenancyTenantsList, }, - true, ExpectedIpAddressStatus), + false, true, ExpectedIpAddressStatus), Entry("Create IpAddress CR, ip address already reserved in NetBox, preserved in netbox, ", defaultIpAddressCR(true), []func(*mock_interfaces.MockIpamInterface, chan error){ @@ -129,7 +140,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() { []func(*mock_interfaces.MockTenancyInterface, chan error){ mockTenancyTenancyTenantsList, }, - true, ExpectedIpAddressStatus), + false, true, ExpectedIpAddressStatus), Entry("Create IpAddress CR, ip address already reserved in NetBox", defaultIpAddressCR(false), []func(*mock_interfaces.MockIpamInterface, chan error){ @@ -140,7 +151,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() { []func(*mock_interfaces.MockTenancyInterface, chan error){ mockTenancyTenancyTenantsList, }, - true, ExpectedIpAddressStatus), + false, true, ExpectedIpAddressStatus), Entry("Create IpAddress CR, reserve or update failure", defaultIpAddressCR(false), []func(*mock_interfaces.MockIpamInterface, chan error){ @@ -151,6 +162,15 @@ var _ = Describe("IpAddress Controller", Ordered, func() { []func(*mock_interfaces.MockTenancyInterface, chan error){ mockTenancyTenancyTenantsList, }, - false, ExpectedIpAddressFailedStatus), + false, false, ExpectedIpAddressFailedStatus), + Entry("Create IpAddress CR, restoration hash mismatch", + defaultIpAddressCreatedByClaim(true), + []func(*mock_interfaces.MockIpamInterface, chan error){ + mockIpAddressListWithHashFilterMismatch, + }, + []func(*mock_interfaces.MockTenancyInterface, chan error){ + mockTenancyTenancyTenantsList, + }, + true, false, nil), ) }) diff --git a/internal/controller/iprange_controller.go b/internal/controller/iprange_controller.go index f1e5c575..8b9ffa73 100644 --- a/internal/controller/iprange_controller.go +++ b/internal/controller/iprange_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" "encoding/json" + "errors" "fmt" "maps" "strconv" @@ -142,13 +143,29 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct netboxIpRangeModel, err := r.NetboxClient.ReserveOrUpdateIpRange(ipRangeModel) if err != nil { - if loggingErr := r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse, - corev1.EventTypeWarning, fmt.Sprintf("%s-%s ", o.Spec.StartAddress, o.Spec.EndAddress), err); loggingErr != nil { - return ctrl.Result{}, fmt.Errorf("logging error: %w. Original error from ReserveOrUpdateIpRange: %w", loggingErr, err) + if errors.Is(err, api.ErrRestorationHashMismatch) && o.Status.IpRangeId == 0 { + // if there is a restoration hash mismatch and the IpRangeId status field is not set, + // delete the ip range so it can be recreated by the ip range claim controller + // this will only affect resources that are created by a claim controller (and have a restoration hash custom field + logger.Info("restoration hash mismatch, deleting ip range custom resource", "ip-range-start", o.Spec.StartAddress, "ip-range-end", o.Spec.EndAddress) + err = r.Client.Delete(ctx, o) + if err != nil { + if err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse, + corev1.EventTypeWarning, "", err); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, nil + } + + if err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse, + corev1.EventTypeWarning, fmt.Sprintf("%s-%s ", o.Spec.StartAddress, o.Spec.EndAddress), err); err != nil { + return ctrl.Result{}, err } // The decision to not return the error message (just logging it) is to not trigger printing the stacktrace on api errors - return ctrl.Result{}, nil + return ctrl.Result{Requeue: true}, nil } // 3. unlock lease of parent prefix diff --git a/internal/controller/iprangeclaim_controller.go b/internal/controller/iprangeclaim_controller.go index c352bb8a..4c48392d 100644 --- a/internal/controller/iprangeclaim_controller.go +++ b/internal/controller/iprangeclaim_controller.go @@ -322,7 +322,7 @@ func (r *IpRangeClaimReconciler) restoreOrAssignIpRangeAndSetCondition(ctx conte availableIpRanges, err := r.NetboxClient.GetAvailableIpAddressesByIpRange(ipRangeModel.Id) if len(availableIpRanges.Payload) != o.Spec.Size { ll.Unlock() - err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeAssignedFalseSizeMissmatch, corev1.EventTypeWarning, "", err) + err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeAssignedFalseSizeMismatch, corev1.EventTypeWarning, "", err) if err != nil { return nil, ctrl.Result{}, err } diff --git a/internal/controller/netbox_testdata_test.go b/internal/controller/netbox_testdata_test.go index 670c5012..0bed0e22 100644 --- a/internal/controller/netbox_testdata_test.go +++ b/internal/controller/netbox_testdata_test.go @@ -53,8 +53,11 @@ var tenantSlug = "test-tenant-slug" var restorationHash = "6f6c67651f0b43b2969ba2ae35c74fc91815513b" -var customFields = map[string]string{"example_field": "example value"} -var customFieldsWithHash = map[string]string{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash} +var customFieldsCR = map[string]string{"example_field": "example value"} +var customFieldsWithHashCR = map[string]string{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash} + +var customFieldsWithHash = map[string]interface{}{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash} +var customFieldsWithHashMismatch = map[string]interface{}{"example_field": "example value", "netboxOperatorRestorationHash": "different hash"} var netboxLabel = "Status" var value = "active" @@ -72,7 +75,7 @@ func defaultIpAddressCR(preserveInNetbox bool) *netboxv1.IpAddress { Spec: netboxv1.IpAddressSpec{ IpAddress: ipAddress, Tenant: tenant, - CustomFields: customFields, + CustomFields: customFieldsCR, Comments: comments, Description: description, PreserveInNetbox: preserveInNetbox, @@ -89,7 +92,7 @@ func defaultIpAddressCreatedByClaim(preserveInNetbox bool) *netboxv1.IpAddress { Spec: netboxv1.IpAddressSpec{ IpAddress: ipAddress, Tenant: tenant, - CustomFields: customFieldsWithHash, + CustomFields: customFieldsWithHashCR, Comments: comments, Description: description, PreserveInNetbox: preserveInNetbox, @@ -106,7 +109,7 @@ func defaultIpAddressClaimCR() *netboxv1.IpAddressClaim { Spec: netboxv1.IpAddressClaimSpec{ ParentPrefix: parentPrefix, Tenant: tenant, - CustomFields: customFields, + CustomFields: customFieldsCR, Comments: comments, Description: description, PreserveInNetbox: false, @@ -176,6 +179,22 @@ func mockedResponsePrefixList() *ipam.IpamPrefixesListOKBody { } } +func mockedResponseIPAddressListWithHash(customFields map[string]interface{}) *ipam.IpamIPAddressesListOKBody { + return &ipam.IpamIPAddressesListOKBody{ + Results: []*netboxModels.IPAddress{ + { + ID: mockedResponseIPAddress().ID, + Address: mockedResponseIPAddress().Address, + Comments: mockedResponseIPAddress().Comments, + CustomFields: customFields, + Description: mockedResponseIPAddress().Description, + Display: mockedResponseIPAddress().Display, + Tenant: mockedResponseIPAddress().Tenant, + }, + }, + } +} + func mockedResponseIPAddressList() *ipam.IpamIPAddressesListOKBody { return &ipam.IpamIPAddressesListOKBody{ Results: []*netboxModels.IPAddress{ diff --git a/internal/controller/prefix_controller.go b/internal/controller/prefix_controller.go index f0b1f0b3..6c3b7df2 100644 --- a/internal/controller/prefix_controller.go +++ b/internal/controller/prefix_controller.go @@ -184,8 +184,29 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr netboxPrefixModel, err := r.NetboxClient.ReserveOrUpdatePrefix(prefixModel) if err != nil { - updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, prefix.Spec.Prefix) - return ctrl.Result{}, fmt.Errorf("failed at update prefix status: %w, "+"after reservation of prefix in netbox failed: %w", updateStatusErr, err) + if errors.Is(err, api.ErrRestorationHashMismatch) && prefix.Status.PrefixId == 0 { + // if there is a restoration hash mismatch and the PrefixId status field is not set, + // delete the prefix so it can be recreated by the prefix claim controller + // this will only affect resources that are created by a claim controller (and have a restoration hash custom field + logger.Info("restoration hash mismatch, deleting prefix custom resource", "prefix", prefix.Spec.Prefix) + err = r.Client.Delete(ctx, prefix) + if err != nil { + if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, + corev1.EventTypeWarning, err.Error()); updateStatusErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+ + "after deletion of prefix cr failed: %w", updateStatusErr, err) + } + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, nil + } + + if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, + corev1.EventTypeWarning, err.Error()); updateStatusErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+ + "after reservation of prefix netbox failed: %w", updateStatusErr, err) + } + return ctrl.Result{Requeue: true}, nil } /* 3. unlock lease of parent prefix */ diff --git a/pkg/netbox/api/errors.go b/pkg/netbox/api/errors.go index 302c8fbc..2492bc95 100644 --- a/pkg/netbox/api/errors.go +++ b/pkg/netbox/api/errors.go @@ -23,4 +23,5 @@ var ( ErrParentPrefixNotFound = errors.New("parent prefix not found") ErrWrongMatchingPrefixSubnetFormat = errors.New("wrong matchingPrefix subnet format") ErrInvalidIpFamily = errors.New("invalid IP Family") + ErrRestorationHashMismatch = errors.New("restoration hash mismatch") ) diff --git a/pkg/netbox/api/ip_address.go b/pkg/netbox/api/ip_address.go index ee3df7ad..3d13666b 100644 --- a/pkg/netbox/api/ip_address.go +++ b/pkg/netbox/api/ip_address.go @@ -17,10 +17,12 @@ limitations under the License. package api import ( + "fmt" "net/http" "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" netboxModels "github.com/netbox-community/go-netbox/v3/netbox/models" + "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" @@ -33,14 +35,18 @@ func (r *NetboxClient) ReserveOrUpdateIpAddress(ipAddress *models.IPAddress) (*n } desiredIPAddress := &netboxModels.WritableIPAddress{ - Address: &ipAddress.IpAddress, - Comments: ipAddress.Metadata.Comments + warningComment, - CustomFields: ipAddress.Metadata.Custom, - Description: TruncateDescription(ipAddress.Metadata.Description), - Status: "active", + Address: &ipAddress.IpAddress, + Description: TruncateDescription(""), + Status: "active", } - if ipAddress.Metadata.Tenant != "" { + if ipAddress.Metadata != nil { + desiredIPAddress.CustomFields = ipAddress.Metadata.Custom + desiredIPAddress.Comments = ipAddress.Metadata.Comments + warningComment + desiredIPAddress.Description = TruncateDescription(ipAddress.Metadata.Description) + } + + if ipAddress.Metadata != nil && ipAddress.Metadata.Tenant != "" { tenantDetails, err := r.GetTenantDetails(ipAddress.Metadata.Tenant) if err != nil { return nil, err @@ -52,7 +58,22 @@ func (r *NetboxClient) ReserveOrUpdateIpAddress(ipAddress *models.IPAddress) (*n if len(responseIpAddress.Payload.Results) == 0 { return r.CreateIpAddress(desiredIPAddress) } - //update ip address since it does exist + + ipToUpdate := responseIpAddress.Payload.Results[0] + + // if the desired ip address has a restoration hash + // check that the ip address to update has the same restoration hash + restorationHashKey := config.GetOperatorConfig().NetboxRestorationHashFieldName + if ipAddress.Metadata != nil { + if restorationHash, ok := ipAddress.Metadata.Custom[restorationHashKey]; ok { + if ipToUpdate.CustomFields != nil && ipToUpdate.CustomFields.(map[string]interface{})[restorationHashKey] == restorationHash { + //update ip address since it does exist and the restoration hash matches + return r.UpdateIpAddress(ipToUpdate.ID, desiredIPAddress) + } + return nil, fmt.Errorf("%w, assigned ip address %s", ErrRestorationHashMismatch, ipAddress.IpAddress) + } + } + ipAddressId := responseIpAddress.Payload.Results[0].ID return r.UpdateIpAddress(ipAddressId, desiredIPAddress) } diff --git a/pkg/netbox/api/ip_address_test.go b/pkg/netbox/api/ip_address_test.go index b2850936..b01fcb80 100644 --- a/pkg/netbox/api/ip_address_test.go +++ b/pkg/netbox/api/ip_address_test.go @@ -24,6 +24,7 @@ import ( "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" "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/pkg/config" "github.com/netbox-community/netbox-operator/pkg/netbox/models" "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -37,7 +38,7 @@ const ( ) func TestIPAddress(t *testing.T) { - ctrl := gomock.NewController(t) + ctrl := gomock.NewController(t, gomock.WithOverridableExpectations()) defer ctrl.Finish() mockIPAddress := mock_interfaces.NewMockIpamInterface(ctrl) mockPrefixTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) @@ -61,14 +62,21 @@ func TestIPAddress(t *testing.T) { } } + expectedHash := "fioaf9289rjfhaeuih" + + customFields := map[string]interface{}{ + config.GetOperatorConfig().NetboxRestorationHashFieldName: expectedHash, + } + // example output IP address expectedIPAddress := func() *netboxModels.IPAddress { return &netboxModels.IPAddress{ - ID: int64(1), - Address: &ipAddress, - Display: ipAddress, - Comments: Comments, - Description: Description, + ID: int64(1), + Address: &ipAddress, + Display: ipAddress, + Comments: Comments, + Description: Description, + CustomFields: customFields, Tenant: &netboxModels.NestedTenant{ ID: tenantId, }, @@ -93,6 +101,21 @@ func TestIPAddress(t *testing.T) { } } + ipAddressModel := func(restorationHash string) *models.IPAddress { + model := &models.IPAddress{ + IpAddress: ipAddress, + } + if restorationHash != "" { + if model.Metadata == nil { + model.Metadata = &models.NetboxMetadata{} + } + model.Metadata.Custom = map[string]string{ + config.GetOperatorConfig().NetboxRestorationHashFieldName: restorationHash, + } + } + return model + } + t.Run("Retrieve Existing static IP Address.", func(t *testing.T) { // id, address conversion from int64 to string @@ -254,4 +277,90 @@ func TestIPAddress(t *testing.T) { AssertNil(t, err) }) + t.Run("Check ReserveOrUpdate without hash", func(t *testing.T) { + inputList := ipam.NewIpamIPAddressesListParams().WithAddress(&ipAddress) + outputList := &ipam.IpamIPAddressesListOK{ + Payload: &ipam.IpamIPAddressesListOKBody{ + Results: []*netboxModels.IPAddress{ + { + ID: expectedIPAddress().ID, + Address: expectedIPAddress().Address, + Display: expectedIPAddress().Display, + }}, + }, + } + + outputUpdate := &ipam.IpamIPAddressesUpdateOK{ + Payload: expectedIPAddress(), + } + + mockIPAddress.EXPECT().IpamIPAddressesList(inputList, nil).Return(outputList, nil).AnyTimes() + // use gomock.Any() because the input contains a pointer + mockIPAddress.EXPECT().IpamIPAddressesUpdate(gomock.Any(), nil).Return(outputUpdate, nil) + + client := &NetboxClient{ + Ipam: mockIPAddress, + } + + ipAddressModel := ipAddressModel("") + _, err := client.ReserveOrUpdateIpAddress(ipAddressModel) + AssertNil(t, err) + }) + + t.Run("Check ReserveOrUpdate with hash", func(t *testing.T) { + inputList := ipam.NewIpamIPAddressesListParams().WithAddress(&ipAddress) + outputList := &ipam.IpamIPAddressesListOK{ + Payload: &ipam.IpamIPAddressesListOKBody{ + Results: []*netboxModels.IPAddress{ + { + ID: expectedIPAddress().ID, + Address: expectedIPAddress().Address, + Display: expectedIPAddress().Display, + CustomFields: expectedIPAddress().CustomFields, + }}, + }, + } + + outputUpdate := &ipam.IpamIPAddressesUpdateOK{ + Payload: expectedIPAddress(), + } + + mockIPAddress.EXPECT().IpamIPAddressesList(inputList, nil).Return(outputList, nil).AnyTimes() + // use gomock.Any() because the input contains a pointer + mockIPAddress.EXPECT().IpamIPAddressesUpdate(gomock.Any(), nil).Return(outputUpdate, nil) + + client := &NetboxClient{ + Ipam: mockIPAddress, + } + + ipAddressModel := ipAddressModel(expectedHash) + _, err := client.ReserveOrUpdateIpAddress(ipAddressModel) + AssertNil(t, err) + }) + + t.Run("Check ReserveOrUpdate with hash mismatch", func(t *testing.T) { + inputList := ipam.NewIpamIPAddressesListParams().WithAddress(&ipAddress) + outputList := &ipam.IpamIPAddressesListOK{ + Payload: &ipam.IpamIPAddressesListOKBody{ + Results: []*netboxModels.IPAddress{ + { + ID: expectedIPAddress().ID, + Address: expectedIPAddress().Address, + Display: expectedIPAddress().Display, + CustomFields: expectedIPAddress().CustomFields, + }}, + }, + } + + mockIPAddress.EXPECT().IpamIPAddressesList(inputList, nil).Return(outputList, nil).AnyTimes() + + client := &NetboxClient{ + Ipam: mockIPAddress, + } + + expectedHash := "iwfohs7v82fe9w0" + ipAddressModel := ipAddressModel(expectedHash) + _, err := client.ReserveOrUpdateIpAddress(ipAddressModel) + AssertError(t, err, "restoration hash mismatch, assigned ip address 10.112.140.0") + }) } diff --git a/pkg/netbox/api/ip_range.go b/pkg/netbox/api/ip_range.go index be00ff81..7d7cf9b4 100644 --- a/pkg/netbox/api/ip_range.go +++ b/pkg/netbox/api/ip_range.go @@ -17,10 +17,12 @@ limitations under the License. package api import ( + "fmt" "net/http" "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" netboxModels "github.com/netbox-community/go-netbox/v3/netbox/models" + "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" @@ -41,7 +43,13 @@ func (r *NetboxClient) ReserveOrUpdateIpRange(ipRange *models.IpRange) (*netboxM Status: "active", } - if ipRange.Metadata.Tenant != "" { + if ipRange.Metadata != nil { + desiredIpRange.CustomFields = ipRange.Metadata.Custom + desiredIpRange.Comments = ipRange.Metadata.Comments + warningComment + desiredIpRange.Description = TruncateDescription(ipRange.Metadata.Description) + } + + if ipRange.Metadata != nil && ipRange.Metadata.Tenant != "" { tenantDetails, err := r.GetTenantDetails(ipRange.Metadata.Tenant) if err != nil { return nil, err @@ -53,6 +61,22 @@ func (r *NetboxClient) ReserveOrUpdateIpRange(ipRange *models.IpRange) (*netboxM if len(responseIpRange.Payload.Results) == 0 { return r.CreateIpRange(desiredIpRange) } + + ipRangeToUpdate := responseIpRange.Payload.Results[0] + + // if the desired ip address has a restoration hash + // check that the ip address to update has the same restoration hash + restorationHashKey := config.GetOperatorConfig().NetboxRestorationHashFieldName + if ipRange.Metadata != nil { + if restorationHash, ok := ipRange.Metadata.Custom[restorationHashKey]; ok { + if ipRangeToUpdate.CustomFields != nil && ipRangeToUpdate.CustomFields.(map[string]interface{})[restorationHashKey] == restorationHash { + //update ip address since it does exist and the restoration hash matches + return r.UpdateIpRange(ipRangeToUpdate.ID, desiredIpRange) + } + return nil, fmt.Errorf("%w, assigned ip range %s-%s", ErrRestorationHashMismatch, ipRange.StartAddress, ipRange.EndAddress) + } + } + //update ip range since it does exist ipRangeId := responseIpRange.Payload.Results[0].ID return r.UpdateIpRange(ipRangeId, desiredIpRange) diff --git a/pkg/netbox/api/ip_range_test.go b/pkg/netbox/api/ip_range_test.go index e08dd965..8290ccd4 100644 --- a/pkg/netbox/api/ip_range_test.go +++ b/pkg/netbox/api/ip_range_test.go @@ -24,6 +24,7 @@ import ( "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" "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/pkg/config" "github.com/netbox-community/netbox-operator/pkg/netbox/models" "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -183,6 +184,55 @@ func TestIpRange(t *testing.T) { assert.Equal(t, expectedIPRange().Tenant.Slug, actual.Tenant.Slug) }) + t.Run("ReserveOrUpdate, restoration hash mismatch", func(t *testing.T) { + + // ip range mock input + listInput := ipam.NewIpamIPRangesListParams(). + WithStartAddress(&startAddress). + WithEndAddress(&endAddress) + + wrongHash := "89hqvs0ud89qhdi" + // ip range mock output + listOutput := &ipam.IpamIPRangesListOK{ + Payload: &ipam.IpamIPRangesListOKBody{ + Results: []*netboxModels.IPRange{ + { + ID: expectedIPRange().ID, + StartAddress: &startAddress, + EndAddress: &endAddress, + CustomFields: map[string]interface{}{ + config.GetOperatorConfig().NetboxRestorationHashFieldName: wrongHash, + }, + Comments: expectedIPRange().Comments, + Description: expectedIPRange().Description, + Tenant: expectedIPRange().Tenant, + }, + }, + }, + } + + mockIpam.EXPECT().IpamIPRangesList(listInput, nil).Return(listOutput, nil).AnyTimes() + + // init client + client := &NetboxClient{ + Ipam: mockIpam, + } + + expectedHash := "ffjrep8b29fdaikb" + _, err := client.ReserveOrUpdateIpRange(&models.IpRange{ + StartAddress: startAddress, + EndAddress: endAddress, + Metadata: &models.NetboxMetadata{ + Custom: map[string]string{ + config.GetOperatorConfig().NetboxRestorationHashFieldName: expectedHash, + }, + }, + }) + + // assert error return + AssertError(t, err, "restoration hash mismatch, assigned ip range 10.112.140.1-10.112.140.3") + }) + t.Run("ReserveOrUpdate, update existing ip range", func(t *testing.T) { // ip range mock input diff --git a/pkg/netbox/api/prefix.go b/pkg/netbox/api/prefix.go index 2dbdc827..1c15830a 100644 --- a/pkg/netbox/api/prefix.go +++ b/pkg/netbox/api/prefix.go @@ -17,10 +17,12 @@ limitations under the License. package api import ( + "fmt" "net/http" "github.com/netbox-community/go-netbox/v3/netbox/client/ipam" netboxModels "github.com/netbox-community/go-netbox/v3/netbox/models" + "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" @@ -43,7 +45,13 @@ func (r *NetboxClient) ReserveOrUpdatePrefix(prefix *models.Prefix) (*netboxMode Status: "active", } - if prefix.Metadata.Tenant != "" { + if prefix.Metadata != nil { + desiredPrefix.CustomFields = prefix.Metadata.Custom + desiredPrefix.Comments = prefix.Metadata.Comments + warningComment + desiredPrefix.Description = TruncateDescription(prefix.Metadata.Description) + } + + if prefix.Metadata != nil && prefix.Metadata.Tenant != "" { tenantDetails, err := r.GetTenantDetails(prefix.Metadata.Tenant) if err != nil { return nil, err @@ -51,7 +59,7 @@ func (r *NetboxClient) ReserveOrUpdatePrefix(prefix *models.Prefix) (*netboxMode desiredPrefix.Tenant = &tenantDetails.Id } - if prefix.Metadata.Site != "" { + if prefix.Metadata != nil && prefix.Metadata.Site != "" { siteDetails, err := r.GetSiteDetails(prefix.Metadata.Site) if err != nil { return nil, err @@ -64,6 +72,21 @@ func (r *NetboxClient) ReserveOrUpdatePrefix(prefix *models.Prefix) (*netboxMode return r.CreatePrefix(desiredPrefix) } + prefixToUpdate := responsePrefix.Payload.Results[0] + + // if the desired ip address has a restoration hash + // check that the ip address to update has the same restoration hash + restorationHashKey := config.GetOperatorConfig().NetboxRestorationHashFieldName + if prefix.Metadata != nil { + if restorationHash, ok := prefix.Metadata.Custom[restorationHashKey]; ok { + if prefixToUpdate.CustomFields != nil && prefixToUpdate.CustomFields.(map[string]interface{})[restorationHashKey] == restorationHash { + //update ip address since it does exist and the restoration hash matches + return r.UpdatePrefix(prefixToUpdate.ID, desiredPrefix) + } + return nil, fmt.Errorf("%w, assigned prefix %s", ErrRestorationHashMismatch, prefix.Prefix) + } + } + //update ip address since it does exist prefixId := responsePrefix.Payload.Results[0].ID return r.UpdatePrefix(prefixId, desiredPrefix) diff --git a/pkg/netbox/api/prefix_test.go b/pkg/netbox/api/prefix_test.go index 0ab45ecd..3929903b 100644 --- a/pkg/netbox/api/prefix_test.go +++ b/pkg/netbox/api/prefix_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" "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" @@ -382,22 +383,6 @@ func TestPrefix_ReserveOrUpdate(t *testing.T) { mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) mockDcim := mock_interfaces.NewMockDcimInterface(ctrl) - //prefix mock input - prefixToCreate := &netboxModels.WritablePrefix{ - Comments: comments + warningComment, - Description: description + warningComment, - CustomFields: make(map[string]string), - Prefix: prefixPtr, - Site: &siteOutputId, - Tenant: &tenantOutputId, - Status: "active", - } - - createPrefixInput := ipam. - NewIpamPrefixesCreateParams(). - WithDefaults(). - WithData(prefixToCreate) - //prefix mock output createPrefixOutput := &ipam.IpamPrefixesCreateCreated{ Payload: &netboxModels.Prefix{ @@ -418,7 +403,8 @@ func TestPrefix_ReserveOrUpdate(t *testing.T) { mockTenancy.EXPECT().TenancyTenantsList(tenantListRequestInput, nil).Return(tenantListRequestOutput, nil).AnyTimes() mockDcim.EXPECT().DcimSitesList(siteListRequestInput, nil).Return(siteListRequestOutput, nil).AnyTimes() mockIpam.EXPECT().IpamPrefixesList(prefixListRequestInput, nil).Return(emptyPrefixListOutput, nil) - mockIpam.EXPECT().IpamPrefixesCreate(createPrefixInput, nil).Return(createPrefixOutput, nil) + // use go mock Any as the input parameter contains pointers + mockIpam.EXPECT().IpamPrefixesCreate(gomock.Any(), nil).Return(createPrefixOutput, nil) netboxClient := &NetboxClient{ Ipam: mockIpam, @@ -495,4 +481,46 @@ func TestPrefix_ReserveOrUpdate(t *testing.T) { // without manipulation by the code assert.Nil(t, err) }) + + t.Run("restoration hash mismatch", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockIpam := mock_interfaces.NewMockIpamInterface(ctrl) + + wrongHash := "89327r7fhui" + //prefix mock output + prefixListOutput := &ipam.IpamPrefixesListOK{ + Payload: &ipam.IpamPrefixesListOKBody{ + Results: []*netboxModels.Prefix{ + { + ID: prefixId, + CustomFields: map[string]interface{}{ + config.GetOperatorConfig().NetboxRestorationHashFieldName: wrongHash, + }, + Display: prefix, + Prefix: &prefix, + }, + }, + }, + } + + mockIpam.EXPECT().IpamPrefixesList(prefixListRequestInput, nil).Return(prefixListOutput, nil) + + netboxClient := &NetboxClient{ + Ipam: mockIpam, + } + + expectedHash := "jfioaw0e9gh" + prefixModel := models.Prefix{ + Prefix: prefix, + Metadata: &models.NetboxMetadata{ + Custom: map[string]string{config.GetOperatorConfig().NetboxRestorationHashFieldName: expectedHash}, + }, + } + + _, err := netboxClient.ReserveOrUpdatePrefix(&prefixModel) + // skip assertion on retured values as the payload of IpamPrefixesCreate() is returened + // without manipulation by the code + AssertError(t, err, "restoration hash mismatch, assigned prefix 10.112.140.0/24") + }) }