L2VPNTerminationImportForm bulk update validation fails when interface/vlan fields omitted #542

New Issue

MrUnknownDE · 2026-04-05T16:41:46+02:00

MrUnknownDE commented

2026-04-05 16:41:46 +02:00

Originally created by @adionit7 on 1/17/2026

Summary

The L2VPNTerminationImportForm validation logic incorrectly handles bulk updates when interface and/or vlan fields are omitted from the CSV. The form validation fails to distinguish between new instances (where these fields are required) and existing instances being updated (where they should be preserved if omitted).

Environment

NetBox version: main (development branch)
Python version: 3.12+
Django version: (as specified by NetBox requirements)

Steps to Reproduce

Create an existing L2VPN termination with an assigned interface or VLAN
Prepare a CSV for bulk update with only id and l2vpn columns (omitting interface and vlan):
id,l2vpn
1,L2VPN 1
2,L2VPN 1
3,L2VPN 1
Attempt to import/update via bulk import in the NetBox UI
Observe validation error or unexpected behavior

Expected Behavior

When performing a bulk update via CSV import:

If interface and/or vlan columns are omitted from the CSV, the existing values should be preserved
The form should only require interface or vlan for new instances, not for updates
No validation errors should occur when updating existing terminations without specifying these fields
The bulk update should succeed and only update the fields specified in the CSV

Actual Behavior

The form validation incorrectly checks if not self.instance instead of if not self.instance.pk
This causes the validation to fail for updates when interface/vlan are not in the CSV
The validation logic treats all instances the same way, not distinguishing between new and existing objects
The test test_bulk_update_objects_with_permission is currently skipped with a TODO comment (line 683 in netbox/vpn/tests/test_views.py)

Code Location

Form: netbox/vpn/forms/bulk_import.py - L2VPNTerminationImportForm.clean() method (line ~340-352)
Test: netbox/vpn/tests/test_views.py - L2VPNTerminationTestCase.test_bulk_update_objects_with_permission() (line ~683-685)

Current Code (Problematic)

def clean(self):
super().clean()

if self.cleaned_data.get('device') and self.cleaned_data.get('virtual_machine'):
    raise ValidationError(_('Cannot import device and VM interface terminations simultaneously.'))

# This check is incorrect - self.instance is always truthy
if not self.instance and not (self.cleaned_data.get('interface') or self.cleaned_data.get('vlan')):
    raise ValidationError(_('Each termination must specify either an interface or a VLAN.'))

if self.cleaned_data.get('interface') and self.cleaned_data.get('vlan'):
    raise ValidationError(_('Cannot assign both an interface and a VLAN.'))

# if this is an update we might not have interface or vlan in the form data
if self.cleaned_data.get('interface') or self.cleaned_data.get('vlan'):
    self.instance.assigned_object = self.cleaned_data.get('interface') or self.cleaned_data.get('vlan')

Proposed Solution

Update the validation logic in L2VPNTerminationImportForm.clean() to:

Check if not self.instance.pk instead of if not self.instance to properly detect new vs existing instances
Only require interface or vlan for new instances (not self.instance.pk)
Preserve existing assigned_object values when these fields are omitted during updates

Fix

Change line ~342 from:
if not self.instance and not (self.cleaned_data.get('interface') or self.cleaned_data.get('vlan')):

To:
if not self.instance.pk and not (self.cleaned_data.get('interface') or self.cleaned_data.get('vlan')):

Why This Works

self.instance is always a model instance object (truthy), even for new objects
self.instance.pk is None for new instances and has a value for existing instances
This pattern is already used correctly in other forms (e.g., RackImportForm at netbox/dcim/forms/bulk_import.py:332)

I'd like to volunteer for this issue!

*Originally created by @adionit7 on 1/17/2026* ## Summary The `L2VPNTerminationImportForm` validation logic incorrectly handles bulk updates when `interface` and/or `vlan` fields are omitted from the CSV. The form validation fails to distinguish between new instances (where these fields are required) and existing instances being updated (where they should be preserved if omitted). ## Environment - NetBox version: main (development branch) - Python version: 3.12+ - Django version: (as specified by NetBox requirements) ## Steps to Reproduce 1. Create an existing L2VPN termination with an assigned interface or VLAN 2. Prepare a CSV for bulk update with only `id` and `l2vpn` columns (omitting `interface` and `vlan`): id,l2vpn 1,L2VPN 1 2,L2VPN 1 3,L2VPN 1 3. Attempt to import/update via bulk import in the NetBox UI 4. Observe validation error or unexpected behavior ## Expected Behavior When performing a bulk update via CSV import: - If interface and/or vlan columns are omitted from the CSV, the existing values should be preserved - The form should only require interface or vlan for new instances, not for updates - No validation errors should occur when updating existing terminations without specifying these fields - The bulk update should succeed and only update the fields specified in the CSV ## Actual Behavior - The form validation incorrectly checks if not self.instance instead of if not self.instance.pk - This causes the validation to fail for updates when interface/vlan are not in the CSV - The validation logic treats all instances the same way, not distinguishing between new and existing objects - The test test_bulk_update_objects_with_permission is currently skipped with a TODO comment (line 683 in netbox/vpn/tests/test_views.py) ## Code Location - Form: netbox/vpn/forms/bulk_import.py - L2VPNTerminationImportForm.clean() method (line ~340-352) - Test: netbox/vpn/tests/test_views.py - L2VPNTerminationTestCase.test_bulk_update_objects_with_permission() (line ~683-685) ## Current Code (Problematic) def clean(self): super().clean() if self.cleaned_data.get('device') and self.cleaned_data.get('virtual_machine'): raise ValidationError(_('Cannot import device and VM interface terminations simultaneously.')) # This check is incorrect - self.instance is always truthy if not self.instance and not (self.cleaned_data.get('interface') or self.cleaned_data.get('vlan')): raise ValidationError(_('Each termination must specify either an interface or a VLAN.')) if self.cleaned_data.get('interface') and self.cleaned_data.get('vlan'): raise ValidationError(_('Cannot assign both an interface and a VLAN.')) # if this is an update we might not have interface or vlan in the form data if self.cleaned_data.get('interface') or self.cleaned_data.get('vlan'): self.instance.assigned_object = self.cleaned_data.get('interface') or self.cleaned_data.get('vlan') ## Proposed Solution Update the validation logic in L2VPNTerminationImportForm.clean() to: 1. Check if not self.instance.pk instead of if not self.instance to properly detect new vs existing instances 2. Only require interface or vlan for new instances (not self.instance.pk) 3. Preserve existing assigned_object values when these fields are omitted during updates ## Fix Change line ~342 from: if not self.instance and not (self.cleaned_data.get('interface') or self.cleaned_data.get('vlan')): To: if not self.instance.pk and not (self.cleaned_data.get('interface') or self.cleaned_data.get('vlan')): ## Why This Works - self.instance is always a model instance object (truthy), even for new objects - self.instance.pk is None for new instances and has a value for existing instances - This pattern is already used correctly in other forms (e.g., RackImportForm at netbox/dcim/forms/bulk_import.py:332) I'd like to volunteer for this issue!

MrUnknownDE closed this issue

2026-04-05 16:41:46 +02:00

Sign in to join this conversation.

Branches Tags

main

21455-sql-indexes-audit

21357-register-model-actions

feature

20924-plugin-ui-components

21766-improve-test-coverage-for-sortable-table-columns

21157-export-template-public-models

21025-pre-render-config-contexts

21364-swagger

feature-ip-prefix-link

20911-dropdown-3

fix_module_substitution

21160-filterset

20911-dropdown-2

20044-elevation-stuck-lightmode

7604-filter-modifiers-v3

20660-script-load

19724-graphql

14884-script

19720-macaddress-interface-generic-relation

fix-19669-api-image-download

7604-filter-modifiers

19275-fixes-interface-bulk-edit

fix-17794-get_field_value_return_list

11507-show-aggregate-and-rir-on-api

9583-add_column_specific_search_field_to_tables

No Label

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github/netbox#542