starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Interface Bulk Upload can create invalid WWN values #11759

New Issue
Closed
opened 2025-12-29 21:49:31 +01:00 by adam · 13 comments
adam commented 2025-12-29 21:49:31 +01:00
Owner
Copy Link

Originally created by @cruse1977 on GitHub (Oct 21, 2025).

Originally assigned to: @cmdrrobin on GitHub.

NetBox Edition

NetBox Community

NetBox Version

v4.4.4

Python Version

3.10

Steps to Reproduce

  1. Create a device example - "dmi-akron-sw01",
  2. Bulk Upload an interface with the following data
device, name, type, wwn
"dmi01-akron-sw01", "test-interface2", "25gbase-x-sfp28", ""

Expected Behavior

In the changelog, wwn should either be set to null, or disallowed due to an empty string

Observed Behavior

Interface allows to be created - however this then prevents branches from merging:

        "Applying change DCIM | interface Eth1/1/1 created by admin using default",
        "['Invalid WWN format: ']: (dcim.interface:pk=81563) field_value was ''"

Leaving this blank in the UI and creating an interface WWN is always set to null:

Image

However with the CSV its "" - causing branch merges to fail:

Image
Originally created by @cruse1977 on GitHub (Oct 21, 2025). Originally assigned to: @cmdrrobin on GitHub. ### NetBox Edition NetBox Community ### NetBox Version v4.4.4 ### Python Version 3.10 ### Steps to Reproduce 1. Create a device example - "dmi-akron-sw01", 2. Bulk Upload an interface with the following data ``` device, name, type, wwn "dmi01-akron-sw01", "test-interface2", "25gbase-x-sfp28", "" ``` ### Expected Behavior In the changelog, wwn should either be set to null, or disallowed due to an empty string ### Observed Behavior Interface allows to be created - however this then prevents branches from merging: ``` "Applying change DCIM | interface Eth1/1/1 created by admin using default", "['Invalid WWN format: ']: (dcim.interface:pk=81563) field_value was ''" ``` Leaving this blank in the UI and creating an interface WWN is always set to null: <img width="1384" height="511" alt="Image" src="https://github.com/user-attachments/assets/5f6abebe-b139-4ddb-b980-c0f20f0c4c03" /> However with the CSV its "" - causing branch merges to fail: <img width="1519" height="642" alt="Image" src="https://github.com/user-attachments/assets/e0a5d618-a67f-443d-9807-87b5736fef07" />
adam added the type: bugstatus: acceptednetboxseverity: low labels 2025-12-29 21:49:31 +01:00
adam closed this issue 2025-12-29 21:49:31 +01:00
adam commented 2025-12-29 21:49:32 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

We don't use CSV import, but Python scripts to create bulk interfaces. I noticed that when an interface is created the wwn value is set to "". This should be null.

Maybe try to not set/import wwn values? Or maybe do not add "" for the wwn column in your CSV file.

@cmdrrobin commented on GitHub (Oct 28, 2025): We don't use CSV import, but Python scripts to create bulk interfaces. I noticed that when an interface is created the `wwn` value is set to `""`. This should be `null`. Maybe try to not set/import `wwn` values? Or maybe do not add `""` for the wwn column in your CSV file.
adam commented 2025-12-29 21:49:32 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

FWIW I think Netbox API should do something about this. Either give an error or allow users to add wwn as "" value. (same goes for mode and probable for others as well)

@cmdrrobin commented on GitHub (Oct 28, 2025): FWIW I think Netbox API should do something about this. Either give an error or allow users to add `wwn` as `""` value. (same goes for `mode` and probable for others as well)
adam commented 2025-12-29 21:49:32 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

Update from our side, we have the same issue when using import function (dcim/interfaces/import).

We tested it with the following YAML file:

- device: device1.local
  name: vlan123
  type: virtual
  mtu: 9216

When importing this, it creates the interface with wwn as "", not as null.

For the record we use Netbox version 4.3.3.

@cmdrrobin commented on GitHub (Oct 28, 2025): Update from our side, we have the same issue when using import function (`dcim/interfaces/import`). We tested it with the following YAML file: ```yaml - device: device1.local name: vlan123 type: virtual mtu: 9216 ``` When importing this, it creates the interface with `wwn` as `""`, not as `null`. For the record we use Netbox version 4.3.3.
adam commented 2025-12-29 21:49:33 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Oct 28, 2025):

@cmdrrobin would you like to volunteer a PR for this issue?

@jeremystretch commented on GitHub (Oct 28, 2025): @cmdrrobin would you like to volunteer a PR for this issue?
adam commented 2025-12-29 21:49:33 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

Sure! I will try to look into it!

@cmdrrobin commented on GitHub (Oct 28, 2025): Sure! I will try to look into it!
adam commented 2025-12-29 21:49:34 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Oct 28, 2025):

That's a little non-committal. 🙂 Can I assign you as an owner, or should we leave this open for someone else?

@jeremystretch commented on GitHub (Oct 28, 2025): That's a little non-committal. 🙂 Can I assign you as an owner, or should we leave this open for someone else?
adam commented 2025-12-29 21:49:34 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

fine by me. Need to dive into the Netbox code

@cmdrrobin commented on GitHub (Oct 28, 2025): fine by me. Need to dive into the Netbox code
adam commented 2025-12-29 21:49:34 +01:00
Author
Owner
Copy Link

@pheus commented on GitHub (Oct 28, 2025):

Hi all! Sorry to jump in, and thanks for the quick PR (#20702)!

From my testing, this isn’t limited to WWNField: any model field with null=True can hit the same issue during bulk import when the column is present but left blank (""), which then shows up in the changelog/staged changes.

Suggestion

  • Keep the field‑level fix for WWNField (great for API/plugins).
  • Also normalize at the import boundary (e.g., NetBoxModelImportForm.clean()): coerce empty/whitespace strings to None for any forward, concrete null=True field. This would cover CSV/JSON/YAML across core and plugins.

I have a small PoC and can open a follow‑up PR or add to #20702. Happy to help either way. Thanks again!

@pheus commented on GitHub (Oct 28, 2025): Hi all! Sorry to jump in, and thanks for the quick PR (#20702)! From my testing, this isn’t limited to `WWNField`: any model field with `null=True` can hit the same issue during bulk import when the column is present but left blank (`""`), which then shows up in the changelog/staged changes. **Suggestion** - Keep the field‑level fix for `WWNField` (great for API/plugins). - Also normalize at the import boundary (e.g., `NetBoxModelImportForm.clean()`): coerce empty/whitespace strings to `None` for any forward, concrete `null=True` field. This would cover CSV/JSON/YAML across core and plugins. I have a small PoC and can open a follow‑up PR or add to #20702. Happy to help either way. Thanks again!
adam commented 2025-12-29 21:49:34 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

Hi @pheus thanks for the suggestions. If you want I can close my PR and let you create a follow-up.

@cmdrrobin commented on GitHub (Oct 28, 2025): Hi @pheus thanks for the suggestions. If you want I can close my PR and let you create a follow-up.
adam commented 2025-12-29 21:49:34 +01:00
Author
Owner
Copy Link

@pheus commented on GitHub (Oct 28, 2025):

Hi @cmdrrobin! Thanks for jumping on this so quickly!

It wasn’t my intention to take it away from you. I ran into the behavior while working on a plugin and meant to comment earlier, but got pulled into other things - sorry for the delay.

I’m happy to proceed however you prefer:

  • Option A: Keep your PR as-is and I’ll share a small PoC for the import-form normalization (empty string → None for any null=True field) so you can fold it in.
  • Option B: You close yours and I’ll open a follow-up PR adds the import-form change.

Totally your call! I appreciate the work you’ve already put in and I’m glad to help either way!

@pheus commented on GitHub (Oct 28, 2025): Hi @cmdrrobin! Thanks for jumping on this so quickly! It wasn’t my intention to take it away from you. I ran into the behavior while working on a plugin and meant to comment earlier, but got pulled into other things - sorry for the delay. I’m happy to proceed however you prefer: - **Option A:** Keep your PR as-is and I’ll share a small PoC for the import-form normalization (empty string → `None` for any `null=True` field) so you can fold it in. - **Option B:** You close yours and I’ll open a follow-up PR adds the import-form change. Totally your call! I appreciate the work you’ve already put in and I’m glad to help either way!
adam commented 2025-12-29 21:49:35 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

Option A is fine. I can add the changes to the PR.

@cmdrrobin commented on GitHub (Oct 28, 2025): Option A is fine. I can add the changes to the PR.
adam commented 2025-12-29 21:49:35 +01:00
Author
Owner
Copy Link

@pheus commented on GitHub (Oct 28, 2025):

Thanks, @cmdrrobin! I’ve opened a small PR to your branch that adds the import‑form normalization so it’s easy to review and merge.

Since you’re finishing this up, would you be able to include tests for the change? I expect the maintainers will want coverage. The new test support for import scenarios (see #19944) should help.

Happy to answer questions or provide pointers, but I’m good with you owning the tests. Thanks again!

@pheus commented on GitHub (Oct 28, 2025): Thanks, @cmdrrobin! I’ve opened a small [PR](https://github.com/cmdrrobin/netbox/pull/1) to your branch that adds the import‑form normalization so it’s easy to review and merge. Since you’re finishing this up, would you be able to include tests for the change? I expect the maintainers will want coverage. The new test support for import scenarios (see #19944) should help. Happy to answer questions or provide pointers, but I’m good with you owning the tests. Thanks again!
adam commented 2025-12-29 21:49:35 +01:00
Author
Owner
Copy Link

@cmdrrobin commented on GitHub (Oct 28, 2025):

Yes will do and thanks for making these changes.
Will try later tonight otherwise tomorrow morning.

@cmdrrobin commented on GitHub (Oct 28, 2025): Yes will do and thanks for making these changes. Will try later tonight otherwise tomorrow morning.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label netbox severity: low status: accepted type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#11759
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?