Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
radek-senfeld opened this issue May 2, 2024 · 19 comments · May be fixed by #17211
Open
Assignees
Labels
netbox severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@radek-senfeld
Copy link

radek-senfeld commented May 2, 2024

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.

@radek-senfeld radek-senfeld added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels May 2, 2024
@jeffgdotorg
Copy link
Contributor

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.

@jeffgdotorg jeffgdotorg added status: revisions needed This issue requires additional information to be actionable and removed status: needs triage This issue is awaiting triage by a maintainer labels May 2, 2024
@jeffgdotorg jeffgdotorg removed their assignment May 2, 2024
@radek-senfeld
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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

@jeremystretch
Copy link
Member

@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
Copy link
Contributor

@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
Copy link

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.

Copy link
Contributor

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.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label May 24, 2024
@MarianRychtecky
Copy link

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

@jeremystretch
Copy link
Member

@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.

@jeremystretch jeremystretch closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2024
@jeremystretch jeremystretch removed type: bug A confirmed report of unexpected behavior in the application status: revisions needed This issue requires additional information to be actionable pending closure Requires immediate attention to avoid being closed for inactivity labels May 24, 2024
@radek-senfeld
Copy link
Author

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
Copy link
Author

>>> 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}"})

image

@radek-senfeld
Copy link
Author

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

@radek-senfeld
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

@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.

@jeffgdotorg jeffgdotorg reopened this Jun 26, 2024
@jeffgdotorg jeffgdotorg added type: bug A confirmed report of unexpected behavior in the application status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: low Does not significantly disrupt application functionality, or a workaround is available labels Jun 26, 2024
@jeffgdotorg jeffgdotorg changed the title Updating Interface.tagged_vlans via API Updating Interface.tagged_vlans via API improerly allowed on interface with mode: tagged-all Jun 26, 2024
@arthanson arthanson changed the title Updating Interface.tagged_vlans via API improerly allowed on interface with mode: tagged-all Updating Interface.tagged_vlans via API improperly allowed on interface with mode: tagged-all Aug 7, 2024
@DanSheps DanSheps added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Aug 20, 2024
@DanSheps DanSheps self-assigned this Aug 20, 2024
DanSheps added a commit that referenced this issue Aug 20, 2024
@jeremystretch jeremystretch added the netbox label Nov 1, 2024 — with Linear
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
netbox severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
5 participants