Updating Interface.tagged_vlans via API improperly allowed on interface with mode: tagged-all

radek-senfeld commented 7 months ago

Deployment Type

Self-hosted

NetBox Version

v3.7.5, 4.0.3

Python Version

3.9

Steps to Reproduce

Choose any interface.
Set the interface's 801.1Q Mode to Tagged (all) (in API terms, "mode": "tagged-all")
Use a PATCH request to set a non-empty value for the interface's tagged_vlans property
Use a second PATCH request to change only the interface's description property, leaving tagged_vlans and untagged_vlans out of the patch
Retrieve the interface via the API.

$ curl -X PATCH -H "Content-type: application/json"  -H "Accept: application/json" -H "Authorization: Token $TOKEN"  http://localhost/api/dcim/interfaces/1/  --data '{"description": "Patching the description and tagged_vlans", "tagged_vlans": [ 1 ] }'

$ curl -X PATCH -H "Content-type: application/json"  -H "Accept: application/json" -H "Authorization: Token $TOKEN"  http://localhost/api/dcim/interfaces/1/  --data '{"description": "Patching the description only" }'

Expected Behavior

The first PATCH should be rejected with an API error message indicating that setting tagged_vlan is not allowed on an interface with "mode": "tagged_all".

Observed Behavior

The first (invalid) PATCH is accepted, and reflected as specified. The second (valid) PATCH is accepted, and results in the unexpected obliteration of the tagged_vlan result.

jeffgdotorg commented 7 months ago

Thanks for reporting a suspected bug.

It appears you're using the pynetbox API library. To eliminate the possibility that a bug in pynetbox underlies this behavior, please try to reproduce the problem by operating the API directly using curl or the requests library.

radek-senfeld commented 7 months ago

Thanks for reporting a suspected bug.

It appears you're using the pynetbox API library. To eliminate the possibility that a bug in pynetbox underlies this behavior, please try to reproduce the problem by operating the API directly using curl or the requests library.

Yes, we're using pynetbox API library.

As formerly shown, the bug is not connected to the pynetbox library. It happens on the server side whenever the attribute "tagged_vlans" is missing from the PATCH request. Just give it a go, please..

As I stated, I'm pretty sure it could affect all other SerializedPKRelatedField fields in similar situation.

radek-senfeld commented 6 months ago

Tested via Requests. VLANs got nuked.

r = requests.patch("https://netbox.domain.tld/api/dcim/interfaces/69/", json={"description": "Patching just the description will vaporize VLANs"}, headers={"Authorization": "Token deadbeefbabe"})

jeremystretch commented 6 months ago

I was not able to reproduce this on v3.7.8. After assigning both tagged VLANs and an untagged VLAN to an interface, sending the following request did not change anything except its description (as intended):

curl -X PATCH \
-H "Authorization: Token $TOKEN" \
-H "Content-Type: application/json" \
-H "Accept: application/json; indent=4" \
http://netbox:8000/api/dcim/interfaces/1/ \
--data '{"description": "testing"}'

Please try upgrading to v3.7.8.

radek-senfeld commented 6 months ago

I was able to reproduce the issue on v3.7.8 using the curl command.

jeremystretch commented 6 months ago

@radek-senfeld then you'll need to provide more information about how someone else can reproduce the problem. As it stands, it appears to be an issue with your specific deployment or configuration.

jeffgdotorg commented 6 months ago

@radek-senfeld please try to keep in mind that every minute the maintainers spend fiddling with variables in an attempt to reproduce the behavior you're seeing is a minute that they're not working on another issue. The more specific and detailed you can be in your steps to reproduce, the quicker the team can validate the problem and get on with fixing it.

MarianRychtecky commented 6 months ago

Hi Jeff, this is Marian from Radek's team. We took some time to install a fresh instance of Netbox and test it on a new environment. Doing so will give us more details about where the problem could be. We tested the instance we use as a production clone and found the problem persistent. Now, we will test in a new instance and provide you with all the details.

github-actions[bot] commented 6 months ago

This is a reminder that additional information is needed in order to further triage this issue. If the requested details are not provided, the issue will soon be closed automatically.

MarianRychtecky commented 6 months ago

We need a couple more days for testing with v.4

jeremystretch commented 6 months ago

@MarianRychtecky as no further detail has been provided in the past weeks, I'm going to close out this bug report. If, once you've completed your testing, you determine that you're able to reproduce the bug on NetBox v4.0.3 or later and can provide detailed instructions for doing so, please submit a new bug report with that information.

radek-senfeld commented 6 months ago

This issue persists on NetBox v4.0.3.

Being dev myself I completely understand the PITA of an incomplete bug report. Still our installation is very close to a vanilla one. Will test complete vanilla later this day as I can't imagine why are you unable to reproduce the issue.

radek-senfeld commented 6 months ago

>>> import requests
>>> r = requests.patch("https://netbox-v4.domain/api/dcim/interfaces/5048/", json={"description": "Patching just the description will vaporize tagged_vlans"}, headers={"Authorization": "Token {token}"})

radek-senfeld commented 6 months ago

Parsing thru the source code again and I guess the problem is related to the "tagged" vs "tagged-all" interface mode.

radek-senfeld commented 6 months ago

Indeed! In "tagged" mode the Interface.tagged_vlans persist the PATCH while in "tagged-all" mode they don't.

It's misleading that at least in v3.7.* the tagged_vlans get saved even in "tagged-all" mode to be deleted the next save.

jeffgdotorg commented 6 months ago

I haven't managed to reproduce the problem with a pristine 4.0.3 system and curl. I associated the solitary VLAN with a site and not with a VLAN group, in case that differs from your approach.

ubuntu@nb403-repro-15924:/opt/netbox$ curl -H "Accept: application/json" \
 -H "Authorization: Token ${TOKEN}" http://localhost/api/dcim/interfaces/1/

{
  "id": 1,
  "url": "http://localhost/api/dcim/interfaces/1/",
  "display": "eth0",
  "device": {
    "id": 1,
    "url": "http://localhost/api/dcim/devices/1/",
    "display": "sw01",
    "name": "sw01",
    "description": ""
  },
  "vdcs": [],
  "module": null,
  "name": "eth0",
  "label": "",
  "type": {
    "value": "1000base-t",
    "label": "1000BASE-T (1GE)"
  },
  "enabled": true,
  "parent": null,
  "bridge": null,
  "lag": null,
  "mtu": null,
  "mac_address": null,
  "speed": null,
  "duplex": null,
  "wwn": null,
  "mgmt_only": false,
  "description": "",
  "mode": {
    "value": "tagged",
    "label": "Tagged"
  },
  "rf_role": null,
  "rf_channel": null,
  "poe_mode": null,
  "poe_type": null,
  "rf_channel_frequency": null,
  "rf_channel_width": null,
  "tx_power": null,
  "untagged_vlan": null,
  "tagged_vlans": [
    {
      "id": 1,
      "url": "http://localhost/api/ipam/vlans/1/",
      "display": "vlan-forty-two (42)",
      "vid": 42,
      "name": "vlan-forty-two",
      "description": ""
    }
  ],
  "mark_connected": false,
  "cable": null,
  "cable_end": "",
  "wireless_link": null,
  "link_peers": [],
  "link_peers_type": null,
  "wireless_lans": [],
  "vrf": null,
  "l2vpn_termination": null,
  "connected_endpoints": null,
  "connected_endpoints_type": null,
  "connected_endpoints_reachable": null,
  "tags": [],
  "custom_fields": {},
  "created": "2024-05-30T21:23:46.637971Z",
  "last_updated": "2024-05-30T21:33:35.324288Z",
  "count_ipaddresses": 0,
  "count_fhrp_groups": 0,
  "_occupied": false
}

ubuntu@nb403-repro-15924:/opt/netbox$ curl -X PATCH -H "Content-type: application/json" \
 -H "Accept: application/json" -H "Authorization: Token $TOKEN" \
 http://localhost/api/dcim/interfaces/1/ \
 --data '{"description": "Patching just the description, do not ask to touch the vlans"}'

{
  "id": 1,
  "url": "http://localhost/api/dcim/interfaces/1/",
  "display": "eth0",
  "device": {
    "id": 1,
    "url": "http://localhost/api/dcim/devices/1/",
    "display": "sw01",
    "name": "sw01",
    "description": ""
  },
  "vdcs": [],
  "module": null,
  "name": "eth0",
  "label": "",
  "type": {
    "value": "1000base-t",
    "label": "1000BASE-T (1GE)"
  },
  "enabled": true,
  "parent": null,
  "bridge": null,
  "lag": null,
  "mtu": null,
  "mac_address": null,
  "speed": null,
  "duplex": null,
  "wwn": null,
  "mgmt_only": false,
  "description": "Patching just the description, do not ask to touch the vlans",
  "mode": {
    "value": "tagged",
    "label": "Tagged"
  },
  "rf_role": null,
  "rf_channel": null,
  "poe_mode": null,
  "poe_type": null,
  "rf_channel_frequency": null,
  "rf_channel_width": null,
  "tx_power": null,
  "untagged_vlan": null,
  "tagged_vlans": [
    {
      "id": 1,
      "url": "http://localhost/api/ipam/vlans/1/",
      "display": "vlan-forty-two (42)",
      "vid": 42,
      "name": "vlan-forty-two",
      "description": ""
    }
  ],
  "mark_connected": false,
  "cable": null,
  "cable_end": "",
  "wireless_link": null,
  "link_peers": [],
  "link_peers_type": null,
  "wireless_lans": [],
  "vrf": null,
  "l2vpn_termination": null,
  "connected_endpoints": null,
  "connected_endpoints_type": null,
  "connected_endpoints_reachable": null,
  "tags": [],
  "custom_fields": {},
  "created": "2024-05-30T21:23:46.637971Z",
  "last_updated": "2024-05-30T21:34:41.344480Z",
  "count_ipaddresses": 0,
  "count_fhrp_groups": 0,
  "_occupied": false
}

ubuntu@nb403-repro-15924:/opt/netbox$ curl -H "Accept: application/json" \
 -H "Authorization: Token ${TOKEN}" http://localhost/api/dcim/interfaces/1/

{
  "id": 1,
  "url": "http://localhost/api/dcim/interfaces/1/",
  "display": "eth0",
  "device": {
    "id": 1,
    "url": "http://localhost/api/dcim/devices/1/",
    "display": "sw01",
    "name": "sw01",
    "description": ""
  },
  "vdcs": [],
  "module": null,
  "name": "eth0",
  "label": "",
  "type": {
    "value": "1000base-t",
    "label": "1000BASE-T (1GE)"
  },
  "enabled": true,
  "parent": null,
  "bridge": null,
  "lag": null,
  "mtu": null,
  "mac_address": null,
  "speed": null,
  "duplex": null,
  "wwn": null,
  "mgmt_only": false,
  "description": "Patching just the description, do not ask to touch the vlans",
  "mode": {
    "value": "tagged",
    "label": "Tagged"
  },
  "rf_role": null,
  "rf_channel": null,
  "poe_mode": null,
  "poe_type": null,
  "rf_channel_frequency": null,
  "rf_channel_width": null,
  "tx_power": null,
  "untagged_vlan": null,
  "tagged_vlans": [
    {
      "id": 1,
      "url": "http://localhost/api/ipam/vlans/1/",
      "display": "vlan-forty-two (42)",
      "vid": 42,
      "name": "vlan-forty-two",
      "description": ""
    }
  ],
  "mark_connected": false,
  "cable": null,
  "cable_end": "",
  "wireless_link": null,
  "link_peers": [],
  "link_peers_type": null,
  "wireless_lans": [],
  "vrf": null,
  "l2vpn_termination": null,
  "connected_endpoints": null,
  "connected_endpoints_type": null,
  "connected_endpoints_reachable": null,
  "tags": [],
  "custom_fields": {},
  "created": "2024-05-30T21:23:46.637971Z",
  "last_updated": "2024-05-30T21:34:41.344480Z",
  "count_ipaddresses": 0,
  "count_fhrp_groups": 0,
  "_occupied": false
}

jeffgdotorg commented 5 months ago

@radek-senfeld @MarianRychtecky please see my latest comments. If your efforts at reproducing the problem on a recent 4.0.x release have been fruitful, I can reopen the issue, but my own efforts have been unsuccessful.

MarianRychtecky commented 5 months ago

Hi Jeff, Radek did explain where the issue is:

In "tagged" mode, the Interface.tagged_vlans persist the PATCH, while in "tagged-all" mode, they don't.

It's misleading that, at least in v3.7.*, the tagged_vlans get saved even in "tagged-all" mode to be deleted the next save.

We changed the settings on all the interfaces from "tagged-all" (incl. VLANs) to "tagged" (incl. VLANs). When the mode is "tagged," then PATCH will modify it, and it will work correctly. Initially, when the mode was "tagged-all," the first API call would save it, but the next run would remove all VLANs.

We would recommend an update in API. When the mode is "tagged-all" and includes the attribute "tagged_vlans," then an exception should appear with the message "Specifying VLANs in tagged-all mode is not available."

Let me know if any more clarification is needed. For us, the change from "tagged-all" to "tagged" worked.

Thanks.

jeffgdotorg commented 5 months ago

@MarianRychtecky @radek-senfeld I came back around to this issue while catching up on my revisions-needed backlog. Now that I've understood the crux (I hope), I'm reopening it as a low-severity validated bug. Thanks for your patient understanding.

I'm moving the issue on to needs owner status. If one of you would like to work it through to a PR, please say so and a maintainer will assign it to you. Otherwise another developer with the requisite skills and capacity can pick it up any time.

netbox-community / netbox