diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go index 0d7952b..3a71086 100644 --- a/internal/controller/ipaddress_controller.go +++ b/internal/controller/ipaddress_controller.go @@ -45,7 +45,7 @@ import ( ) const IpAddressFinalizerName = "ipaddress.netbox.dev/finalizer" -const LastIpAddressManagedCustomFieldsAnnotationName = "ipaddress.netbox.dev/managed-custom-fields" +const IPManagedCustomFieldsAnnotationName = "ipaddress.netbox.dev/managed-custom-fields" // IpAddressReconciler reconciles a IpAddress object type IpAddressReconciler struct { @@ -163,7 +163,7 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } - ipAddressModel, err := generateNetboxIpAddressModelFromIpAddressSpec(&o.Spec, req, annotations[LastIpAddressManagedCustomFieldsAnnotationName]) + ipAddressModel, err := generateNetboxIpAddressModelFromIpAddressSpec(&o.Spec, req, annotations[IPManagedCustomFieldsAnnotationName]) if err != nil { return ctrl.Result{}, err } @@ -193,7 +193,7 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( annotations = make(map[string]string, 1) } - annotations[LastIpAddressManagedCustomFieldsAnnotationName], err = generateLastMetadataAnnotation(o.Spec.CustomFields) + annotations[IPManagedCustomFieldsAnnotationName], err = generateLastMetadataAnnotation(o.Spec.CustomFields) if err != nil { logger.Error(err, "failed to update last metadata annotation") return ctrl.Result{Requeue: true}, nil diff --git a/internal/controller/iprange_controller.go b/internal/controller/iprange_controller.go index e3b4ef3..5bbfe90 100644 --- a/internal/controller/iprange_controller.go +++ b/internal/controller/iprange_controller.go @@ -42,7 +42,7 @@ import ( ) const IpRangeFinalizerName = "iprange.netbox.dev/finalizer" -const LastIpRangeManagedCustomFieldsAnnotationName = "iprange.netbox.dev/managed-custom-fields" +const IPRManagedCustomFieldsAnnotationName = "iprange.netbox.dev/managed-custom-fields" // IpRangeReconciler reconciles a IpRange object type IpRangeReconciler struct { @@ -135,7 +135,7 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, err } - ipRangeModel, err := r.generateNetboxIpRangeModelFromIpRangeSpec(o, req, annotations[LastIpRangeManagedCustomFieldsAnnotationName]) + ipRangeModel, err := r.generateNetboxIpRangeModelFromIpRangeSpec(o, req, annotations[IPRManagedCustomFieldsAnnotationName]) if err != nil { return ctrl.Result{}, err } @@ -166,7 +166,7 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct annotations = make(map[string]string, 1) } - annotations[LastIpRangeManagedCustomFieldsAnnotationName], err = generateLastMetadataAnnotation(o.Spec.CustomFields) + annotations[IPRManagedCustomFieldsAnnotationName], err = generateLastMetadataAnnotation(o.Spec.CustomFields) if err != nil { logger.Error(err, "failed to update last metadata annotation") return ctrl.Result{Requeue: true}, nil diff --git a/internal/controller/iprange_controller_test.go b/internal/controller/iprange_controller_test.go index b32f913..52284d3 100644 --- a/internal/controller/iprange_controller_test.go +++ b/internal/controller/iprange_controller_test.go @@ -16,69 +16,83 @@ limitations under the License. package controller -// import ( -// "context" - -// . "github.com/onsi/ginkgo/v2" -// . "github.com/onsi/gomega" -// "k8s.io/apimachinery/pkg/api/errors" -// "k8s.io/apimachinery/pkg/types" -// "sigs.k8s.io/controller-runtime/pkg/reconcile" - -// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - -// netboxdevv1 "github.com/netbox-community/netbox-operator/api/v1" -// ) - -// var _ = Describe("IpRange Controller", func() { -// Context("When reconciling a resource", func() { -// const resourceName = "test-resource" - -// ctx := context.Background() - -// typeNamespacedName := types.NamespacedName{ -// Name: resourceName, -// Namespace: "default", // TODO(user):Modify as needed -// } -// iprange := &netboxdevv1.IpRange{} - -// BeforeEach(func() { -// By("creating the custom resource for the Kind IpRange") -// err := k8sClient.Get(ctx, typeNamespacedName, iprange) -// if err != nil && errors.IsNotFound(err) { -// resource := &netboxdevv1.IpRange{ -// ObjectMeta: metav1.ObjectMeta{ -// Name: resourceName, -// Namespace: "default", -// }, -// // TODO(user): Specify other spec details if needed. -// } -// Expect(k8sClient.Create(ctx, resource)).To(Succeed()) -// } -// }) - -// AfterEach(func() { -// // TODO(user): Cleanup logic after each test, like removing the resource instance. -// resource := &netboxdevv1.IpRange{} -// err := k8sClient.Get(ctx, typeNamespacedName, resource) -// Expect(err).NotTo(HaveOccurred()) - -// By("Cleanup the specific resource instance IpRange") -// Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) -// }) -// It("should successfully reconcile the resource", func() { -// By("Reconciling the created resource") -// controllerReconciler := &IpRangeReconciler{ -// Client: k8sClient, -// Scheme: k8sClient.Scheme(), -// } - -// _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ -// NamespacedName: typeNamespacedName, -// }) -// Expect(err).NotTo(HaveOccurred()) -// // TODO(user): Add more specific assertions depending on your controller's reconciliation logic. -// // Example: If you expect a certain status condition after reconciliation, verify it here. -// }) -// }) -// }) +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/netbox-community/netbox-operator/pkg/netbox/api" + "github.com/netbox-community/netbox-operator/pkg/netbox/models" + apimodel "github.com/netbox-community/netbox-operator/pkg/netbox/models" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + netboxv1 "github.com/netbox-community/netbox-operator/api/v1" +) + +var ipRangeRecondiler *IpRangeReconciler + +var _ = Describe("IpRange Controller", func() { + Context("When generating NetBox IpRange Model form IpRangeSpec", func() { + // dummy reconciler + ipRangeRecondiler = &IpRangeReconciler{ + NetboxClient: &api.NetboxClient{ + Ipam: ipamMockIpAddress, + Tenancy: tenancyMock, + Dcim: dcimMock, + }, + } + + // default IpRange + ipRange := &netboxv1.IpRange{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + Spec: netboxv1.IpRangeSpec{ + StartAddress: "1.0.0.1/32", + EndAddress: "1.0.0.5/32", + Comments: "a comment", + Description: "a description", + Tenant: "a tenant", + CustomFields: map[string]string{"custom_field_2": "valueToBeSet"}, + }} + ipRange.Name = "test-claim" + + // default managedCustomFieldsAnnotation + managedCustomFieldsAnnotation := "{\"custom_field_1\":\"valueToBeRemoved\"}" + + // default request + req := reconcile.Request{ + NamespacedName: client.ObjectKey{ + Name: "test-claim", + Namespace: "default", + }, + } + + It("should create the correct ip range model", func() { + ipRangeModel, err := ipRangeRecondiler.generateNetboxIpRangeModelFromIpRangeSpec(ipRange, req, managedCustomFieldsAnnotation) + + Expect(ipRangeModel).To(Equal(&models.IpRange{ + Metadata: &apimodel.NetboxMetadata{ + Comments: "a comment", + Description: "default/test-claim // a description // managed by netbox-operator, please don't edit it in Netbox unless you know what you're doing", + Custom: map[string]string{"custom_field_2": "valueToBeSet", "custom_field_1": ""}, + Tenant: "a tenant", + }, + StartAddress: "1.0.0.1/32", + EndAddress: "1.0.0.5/32", + })) + + Expect(err).To(BeNil()) + }) + + It("should return error if parsing of annotation fails", func() { + invalidManagedCustomFieldsAnnotation := "{:\"valueToBeRemoved\"}" + ipRangeModel, err := ipRangeRecondiler.generateNetboxIpRangeModelFromIpRangeSpec(ipRange, req, invalidManagedCustomFieldsAnnotation) + + Expect(ipRangeModel).To(BeNil()) + + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/internal/controller/iprangeclaim_controller.go b/internal/controller/iprangeclaim_controller.go index 52c921b..c352bb8 100644 --- a/internal/controller/iprangeclaim_controller.go +++ b/internal/controller/iprangeclaim_controller.go @@ -117,6 +117,11 @@ func (r *IpRangeClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } + err = addFinalizer(ctx, r.Client, o, IpRangeClaimFinalizerName) + if err != nil { + return ctrl.Result{}, err + } + err = r.Client.Create(ctx, ipRangeResource) if err != nil { errSetCondition := r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeAssignedFalse, corev1.EventTypeWarning, "", err) diff --git a/internal/controller/prefix_controller.go b/internal/controller/prefix_controller.go index 17482fd..75a3145 100644 --- a/internal/controller/prefix_controller.go +++ b/internal/controller/prefix_controller.go @@ -46,7 +46,7 @@ import ( ) const PrefixFinalizerName = "prefix.netbox.dev/finalizer" -const LastPrefixManagedCustomFieldsAnnotationName = "prefix.netbox.dev/managed-custom-fields" +const PXManagedCustomFieldsAnnotationName = "prefix.netbox.dev/managed-custom-fields" // PrefixReconciler reconciles a Prefix object type PrefixReconciler struct { @@ -177,7 +177,7 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - prefixModel, err := generateNetboxPrefixModelFromPrefixSpec(&prefix.Spec, req, annotations[LastPrefixManagedCustomFieldsAnnotationName]) + prefixModel, err := generateNetboxPrefixModelFromPrefixSpec(&prefix.Spec, req, annotations[PXManagedCustomFieldsAnnotationName]) if err != nil { return ctrl.Result{}, err } @@ -205,7 +205,7 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr annotations = make(map[string]string, 1) } - annotations[LastPrefixManagedCustomFieldsAnnotationName], err = generateLastMetadataAnnotation(prefix.Spec.CustomFields) + annotations[PXManagedCustomFieldsAnnotationName], err = generateLastMetadataAnnotation(prefix.Spec.CustomFields) if err != nil { logger.Error(err, "failed to update last metadata annotation") return ctrl.Result{Requeue: true}, nil diff --git a/internal/controller/utils.go b/internal/controller/utils.go index b7c07be..84f07f7 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -31,14 +31,14 @@ func convertCIDRToLeaseLockName(cidr string) string { return strings.ReplaceAll(strings.ReplaceAll(cidr, "/", "-"), ":", "-") } -func generateLastMetadataAnnotation(cusotmFields map[string]string) (string, error) { - if cusotmFields == nil { - cusotmFields = make(map[string]string) +func generateLastMetadataAnnotation(customFields map[string]string) (string, error) { + if customFields == nil { + customFields = make(map[string]string) } - lastIpRangeMetadata, err := json.Marshal(cusotmFields) + lastIpRangeMetadata, err := json.Marshal(customFields) if err != nil { - return "", fmt.Errorf("failed to marshal lastIpRangeMetadata annotation: %w", err) + return "", fmt.Errorf("failed to marshal custom fields to JSON: %w", err) } return string(lastIpRangeMetadata), nil diff --git a/pkg/netbox/api/ip_range_claim.go b/pkg/netbox/api/ip_range_claim.go index 245ca8e..cf1968a 100644 --- a/pkg/netbox/api/ip_range_claim.go +++ b/pkg/netbox/api/ip_range_claim.go @@ -113,7 +113,7 @@ func searchAvailableIpRange(availableIps *ipam.IpamPrefixesAvailableIpsListOK, r // it will search for the first available range of IPs with the required size // it will return the start and end IP of the range var startAddress, endAddress string - consecutiveCount := 0 + consecutiveCount := 1 ips := make([]net.IP, len(availableIps.Payload)) var err error @@ -129,15 +129,11 @@ func searchAvailableIpRange(availableIps *ipam.IpamPrefixesAvailableIpsListOK, r return bytes.Compare(ips[i], ips[j]) < 0 }) - for i := range ips { + for i := 1; i < len(ips); i++ { currentIp := ips[i] + previousIP := ips[i-1] - var previousIP net.IP - if i > 0 { - previousIP = ips[i-1] - } - - if i == 0 || areConsecutiveIPs(previousIP, currentIp) { + if areConsecutiveIPs(previousIP, currentIp) { consecutiveCount++ if consecutiveCount == requiredSize { startAddress = ips[i-requiredSize+1].String() diff --git a/pkg/netbox/api/ip_range_claim_test.go b/pkg/netbox/api/ip_range_claim_test.go index bbe1828..14b26bd 100644 --- a/pkg/netbox/api/ip_range_claim_test.go +++ b/pkg/netbox/api/ip_range_claim_test.go @@ -36,7 +36,7 @@ func TestIPRangeClaim(t *testing.T) { mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl) // test data for IPv4 ip range claim - parentPrefixIdV4 := int64(3) + parentPrefixId := int64(3) parentPrefixV4 := "10.114.0.0" requestedRangeSize := 3 ipRangeV4_1 := "10.112.140.1/24" @@ -57,19 +57,19 @@ func TestIPRangeClaim(t *testing.T) { Family: int64(IPv4Family), }, { - Address: ipRangeV4_3, + Address: ipRangeV4_5, Family: int64(IPv4Family), }, { - Address: ipRangeV4_5, + Address: ipRangeV4_3, Family: int64(IPv4Family), }, { - Address: ipRangeV4_6, + Address: ipRangeV4_7, Family: int64(IPv4Family), }, { - Address: ipRangeV4_7, + Address: ipRangeV4_6, Family: int64(IPv4Family), }, { @@ -111,14 +111,14 @@ func TestIPRangeClaim(t *testing.T) { Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { - ID: parentPrefixIdV4, + ID: parentPrefixId, Prefix: &parentPrefixV4, }, }, }, } - inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixIdV4) + inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixId) outputIps := &ipam.IpamPrefixesAvailableIpsListOK{ Payload: availableIpAdressesIPv4(), } @@ -148,7 +148,7 @@ func TestIPRangeClaim(t *testing.T) { assert.Equal(t, expectedIp_7, actual.EndAddress) }) - t.Run("Fetch first available IP range by claim (IPv4).", func(t *testing.T) { + t.Run("Fail first available IP range by claim (IPv6) if not enough consequiteve ips.", func(t *testing.T) { inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName) @@ -159,14 +159,14 @@ func TestIPRangeClaim(t *testing.T) { Payload: &ipam.IpamPrefixesListOKBody{ Results: []*netboxModels.Prefix{ { - ID: parentPrefixIdV4, - Prefix: &parentPrefixV4, + ID: parentPrefixId, + Prefix: &parentPrefixV6, }, }, }, } - inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixIdV4) + inputIps := ipam.NewIpamPrefixesAvailableIpsListParams().WithID(parentPrefixId) outputIps := &ipam.IpamPrefixesAvailableIpsListOK{ Payload: []*netboxModels.AvailableIP{ { @@ -201,6 +201,20 @@ func TestIPRangeClaim(t *testing.T) { AssertError(t, err, "not enough consecutive IPs available") }) + t.Run("Fail with invalid input when searching Available Ip Range", func(t *testing.T) { + payload := &ipam.IpamPrefixesAvailableIpsListOK{ + Payload: []*netboxModels.AvailableIP{ + {Address: "invalid ip address"}, + }, + } + + startAddress, endAddress, err := searchAvailableIpRange(payload, 3, int64(4)) + + assert.Equal(t, "", startAddress) + assert.Equal(t, "", endAddress) + AssertError(t, err, "failed to parse IP address: invalid CIDR address: invalid ip address") + }) + t.Run("Reclaim IP Range", func(t *testing.T) { input := "dummy-hash"