Feature/check keys in parent prefix selector

Makefile

@@ -239,24 +239,24 @@ generate_mocks: ## TODO: auto install go install go.uber.org/mock/mockgen@latest
 	mockgen -destination ${GEN_DIR}/${NETBOX_MOCKS_OUTPUT_FILE} -source=${INTERFACE_DEFITIONS_DIR}
 
 # e2e tests
-E2E_PARAM := --namespace e2e --parallel 3 --apply-timeout 3m --assert-timeout 3m --delete-timeout 3m --error-timeout 3m --exec-timeout 3m # --skip-delete (add this argument for local debugging)
+E2E_PARAM := --namespace e2e --parallel 3 --apply-timeout 3m --assert-timeout 3m --delete-timeout 3m --error-timeout 3m --exec-timeout 3m --cleanup-timeout 3m # --skip-delete (add this argument for local debugging)
 .PHONY: create-kind-3.7.8
 create-kind-3.7.8:
 	./kind/local-env.sh --version 3.7.8
 .PHONY: test-e2e-3.7.8
-test-e2e-3.7.8: create-kind-3.7.8 deploy-kind install-$(GO_PACKAGE_NAME_CHAINSAW) 
+test-e2e-3.7.8: create-kind-3.7.8 deploy-kind install-$(GO_PACKAGE_NAME_CHAINSAW)
 	chainsaw test $(E2E_PARAM)
 
 .PHONY: create-kind-4.0.11
 create-kind-4.0.11:
 	./kind/local-env.sh --version 4.0.11
 .PHONY: test-e2e-4.0.11
-test-e2e-4.0.11: create-kind-4.0.11 deploy-kind install-$(GO_PACKAGE_NAME_CHAINSAW) 
+test-e2e-4.0.11: create-kind-4.0.11 deploy-kind install-$(GO_PACKAGE_NAME_CHAINSAW)
 	chainsaw test $(E2E_PARAM)
 
 .PHONY: create-kind-4.1.8
 create-kind-4.1.8:
 	./kind/local-env.sh --version 4.1.8
 .PHONY: test-e2e-4.1.8
-test-e2e-4.1.8: create-kind-4.1.8 deploy-kind install-$(GO_PACKAGE_NAME_CHAINSAW) 
+test-e2e-4.1.8: create-kind-4.1.8 deploy-kind install-$(GO_PACKAGE_NAME_CHAINSAW)
 	chainsaw test $(E2E_PARAM)

api/v1/prefixclaim_types.go

@@ -174,7 +174,7 @@ var ConditionPrefixAssignedFalse = metav1.Condition{
 	Type:    "PrefixAssigned",
 	Status:  "False",
 	Reason:  "PrefixCRNotCreated",
-	Message: "Failed to fetch new Prefix from NetBox",
+	Message: "Failed to assign prefix, prefix CR creation skipped",
 }
 
 var ConditionParentPrefixSelectedTrue = metav1.Condition{

internal/controller/prefixclaim_controller.go

@@ -154,10 +154,11 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 				// The existing algorithm for prefix allocation within a ParentPrefix remains unchanged
 
 				// fetch available prefixes from netbox
-				parentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixByParentPrefixSelector(&prefixClaim.Spec)
+				parentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixesByParentPrefixSelector(&prefixClaim.Spec)
 				if err != nil || len(parentPrefixCandidates) == 0 {
-					if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, fmt.Errorf("no parent prefix can be obtained with the query conditions set in ParentPrefixSelector, err = %w, number of candidates = %v", err, len(parentPrefixCandidates))); errReport != nil {
-						return ctrl.Result{}, errReport
+					r.EventStatusRecorder.Recorder().Event(prefixClaim, corev1.EventTypeWarning, netboxv1.ConditionPrefixAssignedFalse.Reason, netboxv1.ConditionPrefixAssignedFalse.Message+": "+err.Error())
+					if err := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); err != nil {
+						return ctrl.Result{}, err
 					}
 
 					// we requeue as this might be a temporary prefix exhausation

pkg/netbox/api/prefix_claim.go

@@ -19,10 +19,11 @@ package api
 import (
 	"errors"
 	"fmt"
+	"slices"
 	"strconv"
 	"strings"
 
-	"github.com/go-openapi/runtime"
+	"github.com/netbox-community/go-netbox/v3/netbox/client/extras"
 	"github.com/netbox-community/go-netbox/v3/netbox/client/ipam"
 	"github.com/netbox-community/netbox-operator/pkg/config"
 	"github.com/netbox-community/netbox-operator/pkg/netbox/models"
@@ -93,7 +94,7 @@ func validatePrefixLengthOrError(prefixClaim *models.PrefixClaim, prefixFamily i
 	return nil
 }
 
-func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(prefixClaimSpec *netboxv1.PrefixClaimSpec) ([]*models.Prefix, error) {
+func (r *NetboxClient) GetAvailablePrefixesByParentPrefixSelector(prefixClaimSpec *netboxv1.PrefixClaimSpec) ([]*models.Prefix, error) {
 	fieldEntries := make(map[string]string)
 
 	if tenant, ok := prefixClaimSpec.ParentPrefixSelector["tenant"]; ok {
@@ -125,16 +126,25 @@ func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(prefixClaimSpec
 		fieldEntries["family"] = family
 	}
 
-	var conditions func(co *runtime.ClientOperation)
-	parentPrefixSelectorEntries := make([]CustomFieldEntry, 0, len(prefixClaimSpec.ParentPrefixSelector))
+	parentPrefixSelectorCustomFields := make([]CustomFieldEntry, 0, len(prefixClaimSpec.ParentPrefixSelector))
 	for k, v := range prefixClaimSpec.ParentPrefixSelector {
-		parentPrefixSelectorEntries = append(parentPrefixSelectorEntries, CustomFieldEntry{
-			key:   k,
-			value: v,
-		})
+		switch k {
+		case "tenant", "site", "family":
+			// skip built in fields
+		default:
+			parentPrefixSelectorCustomFields = append(parentPrefixSelectorCustomFields, CustomFieldEntry{
+				key:   k,
+				value: v,
+			})
+		}
 	}
 
-	conditions = newQueryFilterOperation(fieldEntries, parentPrefixSelectorEntries)
+	err := r.customFieldsExistsOrErr(parentPrefixSelectorCustomFields)
+	if err != nil {
+		return nil, err
+	}
+
+	conditions := newQueryFilterOperation(fieldEntries, parentPrefixSelectorCustomFields)
 
 	list, err := r.Ipam.IpamPrefixesList(ipam.NewIpamPrefixesListParams(), nil, conditions)
 	if err != nil {
@@ -158,6 +168,48 @@ func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(prefixClaimSpec
 	return prefixes, nil
 }
 
+func (r *NetboxClient) customFieldsExistsOrErr(customfieldFilterEntries []CustomFieldEntry) error {
+	if len(customfieldFilterEntries) == 0 {
+		// as the parent prefix selector does not filter for custom fields
+		// the check can be skipped
+		return nil
+	}
+
+	responseGetCustomFieldsList, err := r.Extras.ExtrasCustomFieldsList(extras.NewExtrasCustomFieldsListParams(), nil)
+	if err != nil {
+		return err
+	}
+
+	existingCustomFields := responseGetCustomFieldsList.Payload.Results
+	if len(existingCustomFields) == 0 {
+		return fmt.Errorf("netbox custom fields list is nil or empty")
+	}
+
+	customFieldNames := make([]string, len(existingCustomFields))
+	for i, field := range existingCustomFields {
+		if field.Name == nil {
+			return fmt.Errorf("netbox custom field name is nil")
+		}
+		customFieldNames[i] = *field.Name
+	}
+
+	missingCustomFields := make([]string, 0)
+	for _, entry := range customfieldFilterEntries {
+		if !slices.Contains(customFieldNames, entry.key) {
+			missingCustomFields = append(missingCustomFields, entry.key)
+		}
+	}
+
+	if len(missingCustomFields) > 0 {
+		return fmt.Errorf(
+			"invalid parentPrefixSelector, netbox custom fields %s do not exist",
+			strings.Join(missingCustomFields, ", "),
+		)
+	}
+
+	return nil
+}
+
 func (r *NetboxClient) isParentPrefixCandidate(prefixClaimSpec *netboxv1.PrefixClaimSpec, prefix string) bool {
 	// if we can allocate a prefix from it, we can take it as a parent prefix
 	if _, err := r.GetAvailablePrefixByClaim(&models.PrefixClaim{

pkg/netbox/api/prefix_claim_test.go

@@ -22,9 +22,11 @@ import (
 	"testing"
 
 	"github.com/netbox-community/go-netbox/v3/netbox/client/dcim"
+	"github.com/netbox-community/go-netbox/v3/netbox/client/extras"
 	"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"
+	netboxv1 "github.com/netbox-community/netbox-operator/api/v1"
 	"github.com/netbox-community/netbox-operator/gen/mock_interfaces"
 	"github.com/netbox-community/netbox-operator/pkg/netbox/models"
 	"github.com/stretchr/testify/assert"
@@ -1047,3 +1049,213 @@ func TestPrefixClaim_GetAvailablePrefixIfNoSiteInSpec(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, prefix, actual.Prefix)
 }
+
+func TestPrefixClaim_GetAvailablePrefixByParentPrefixSelector(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	pxcSpec := netboxv1.PrefixClaimSpec{
+		ParentPrefixSelector: map[string]string{
+			"environment": "dev",
+			"family":      "IPv4",
+			"tenant":      "tenant",
+			"site":        "Site1",
+		},
+		PrefixLength: "/32",
+	}
+
+	mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)
+	mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl)
+	mockExtras := mock_interfaces.NewMockExtrasInterface(ctrl)
+	mockDcim := mock_interfaces.NewMockDcimInterface(ctrl)
+
+	// example of site
+	siteId := int64(3)
+	siteName := "Site1"
+	siteOutputSlug := "site1"
+	expectedSite := &dcim.DcimSitesListOK{
+		Payload: &dcim.DcimSitesListOKBody{
+			Results: []*netboxModels.Site{
+				{
+					ID:   siteId,
+					Name: &siteName,
+					Slug: &siteOutputSlug,
+				},
+			},
+		},
+	}
+	inputSite := dcim.NewDcimSitesListParams().WithName(&siteName)
+
+	// tenant
+	tenantName := "tenant"
+	tenantId := int64(2)
+	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()
+
+	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},
+				},
+			},
+		},
+	}
+
+	prefixListInputWithParam := ipam.NewIpamPrefixesListParams().WithPrefix(&parentPrefix)
+	prefixListOutputWithParam := &ipam.IpamPrefixesListOK{
+		Payload: &ipam.IpamPrefixesListOKBody{
+			Results: []*netboxModels.Prefix{
+				{
+					Prefix: &parentPrefix,
+					ID:     parentPrefixId,
+					Family: &netboxModels.PrefixFamily{Label: &prefixFamilyLabel, Value: &prefixFamily},
+				},
+			},
+		},
+	}
+
+	prefixAvailableListInput := ipam.NewIpamPrefixesAvailablePrefixesListParams().WithID(parentPrefixId)
+	prefixAvailableListOutput := &ipam.IpamPrefixesAvailablePrefixesListOK{
+		Payload: []*netboxModels.AvailablePrefix{
+			{
+				Family: prefixFamily,
+				Prefix: parentPrefix,
+			},
+		},
+	}
+
+	// get prefix to check if it's a candidate
+	expectedCustomFieldName := "environment"
+	expectedCustomFields := &extras.ExtrasCustomFieldsListOK{
+		Payload: &extras.ExtrasCustomFieldsListOKBody{
+			Results: []*netboxModels.CustomField{
+				{
+					Name: &expectedCustomFieldName,
+				},
+			},
+		},
+	}
+
+	mockPrefixIpam.EXPECT().IpamPrefixesList(prefixListInput, nil, gomock.Any()).Return(prefixListOutput, nil).Times(1)
+	mockPrefixIpam.EXPECT().IpamPrefixesList(prefixListInputWithParam, nil).Return(prefixListOutputWithParam, nil).Times(1)
+	mockPrefixIpam.EXPECT().IpamPrefixesAvailablePrefixesList(prefixAvailableListInput, nil).Return(prefixAvailableListOutput, nil).AnyTimes()
+	mockTenancy.EXPECT().TenancyTenantsList(gomock.Any(), nil).Return(expectedTenant, nil).AnyTimes()
+	mockDcim.EXPECT().DcimSitesList(inputSite, nil).Return(expectedSite, nil).AnyTimes()
+	mockExtras.EXPECT().ExtrasCustomFieldsList(extras.NewExtrasCustomFieldsListParams(), gomock.Any(), gomock.Any()).Return(expectedCustomFields, nil).AnyTimes()
+
+	netboxClient := &NetboxClient{
+		Ipam:    mockPrefixIpam,
+		Tenancy: mockTenancy,
+		Extras:  mockExtras,
+		Dcim:    mockDcim,
+	}
+
+	actual, err := netboxClient.GetAvailablePrefixesByParentPrefixSelector(&pxcSpec)
+
+	assert.Nil(t, err)
+	assert.Equal(t, parentPrefix, actual[0].Prefix)
+}
+
+func TestPrefixClaim_GetAvailablePrefixByParentPrefixSelectorFailIfNonExistingFieldInParentPrefixSelector(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	pxcSpec := netboxv1.PrefixClaimSpec{
+		ParentPrefixSelector: map[string]string{
+			"non-existing": "non-existing",
+			"environment":  "dev",
+			"family":       "IPv4",
+			"tenant":       "tenant",
+			"site":         "Site1",
+		},
+		PrefixLength: "/32",
+	}
+
+	mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)
+	mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl)
+	mockExtras := mock_interfaces.NewMockExtrasInterface(ctrl)
+	mockDcim := mock_interfaces.NewMockDcimInterface(ctrl)
+
+	// example of site
+	siteId := int64(3)
+	siteName := "Site1"
+	siteOutputSlug := "site1"
+	expectedSite := &dcim.DcimSitesListOK{
+		Payload: &dcim.DcimSitesListOKBody{
+			Results: []*netboxModels.Site{
+				{
+					ID:   siteId,
+					Name: &siteName,
+					Slug: &siteOutputSlug,
+				},
+			},
+		},
+	}
+	inputSite := dcim.NewDcimSitesListParams().WithName(&siteName)
+
+	// tenant
+	tenantName := "tenant"
+	tenantId := int64(2)
+	tenantOutputSlug := "tenant1"
+
+	expectedTenant := &tenancy.TenancyTenantsListOK{
+		Payload: &tenancy.TenancyTenantsListOKBody{
+			Results: []*netboxModels.Tenant{
+				{
+					ID:   tenantId,
+					Name: &tenantName,
+					Slug: &tenantOutputSlug,
+				},
+			},
+		},
+	}
+
+	// get prefix to check if it's a candidate
+	expectedCustomFieldName := "environment"
+	expectedCustomFields := &extras.ExtrasCustomFieldsListOK{
+		Payload: &extras.ExtrasCustomFieldsListOKBody{
+			Results: []*netboxModels.CustomField{
+				{
+					Name: &expectedCustomFieldName,
+				},
+			},
+		},
+	}
+
+	mockTenancy.EXPECT().TenancyTenantsList(gomock.Any(), nil).Return(expectedTenant, nil).AnyTimes()
+	mockDcim.EXPECT().DcimSitesList(inputSite, nil).Return(expectedSite, nil).AnyTimes()
+	mockExtras.EXPECT().ExtrasCustomFieldsList(extras.NewExtrasCustomFieldsListParams(), gomock.Any(), gomock.Any()).Return(expectedCustomFields, nil).AnyTimes()
+
+	netboxClient := &NetboxClient{
+		Ipam:    mockPrefixIpam,
+		Tenancy: mockTenancy,
+		Extras:  mockExtras,
+		Dcim:    mockDcim,
+	}
+
+	actual, err := netboxClient.GetAvailablePrefixesByParentPrefixSelector(&pxcSpec)
+
+	assert.Nil(t, actual)
+	AssertError(t, err, "invalid parentPrefixSelector, netbox custom fields non-existing do not exist")
+}