fix(api): change an existing VLAN membership instead of re-creating it

set_interface() always POSTed to vlans-ports, treating every membership as
new. Moving a port that already carries the VLAN from untagged to tagged is
not a create, though, and the switch says so:

    POST vlans-ports {"vlan_id":10,"port_id":"10","port_mode":"POM_TAGGED_STATIC"}
      -> 400 {"message":"Association exists"}

Only 409 was handled as "already there"; v7 firmware answers 400. The
membership is its own resource named {vlan_id}-{port_id} and takes a PUT:

    PUT vlans-ports/10-10 -> 200

So the exact case anyone hits first failed outright — a port holding VLAN 10
untagged alongside 30/40/50 tagged, with VLAN 10 to become tagged too.

The response body is now carried into both error messages. The ports PUT just
above already did this; here it was dropped, so "Association exists" — which
names the cause outright — never reached the caller. The message read
"vlans-ports POST HTTP 400" and nothing more.

Verified against a 2530-24G-PoEP on REST v7: set_interface("10", trunk
vlan 30), where that membership already exists, now completes and leaves the
port exactly as it was.
This commit is contained in:
Christian Manivong
2026-08-18 16:22:21 +07:00
parent 0dcede037f
commit e3bbac0656
12 changed files with 272 additions and 20 deletions
+69
View File
@@ -651,3 +651,72 @@ class TestDriverGetVlans:
# Ports 1-4 are untagged in VLAN 1
assert pvids["1"] == 1
assert pvids["4"] == 1
# ===========================================================================
# set_interface over the REST API — changing an existing VLAN membership
# ===========================================================================
def _api_driver(post_status: int = 400, post_body: str = '{"message":"Association exists"}'):
"""Driver on the REST transport with a scripted API client."""
with patch("napalm_procurve.procurve.ConnectHandler"):
drv = ProcurveDriver(hostname="192.168.0.1", username="manager", password="secret")
drv._transport = "api"
api = MagicMock()
post_resp = MagicMock(ok=post_status < 400, status_code=post_status, text=post_body)
api.post.return_value = post_resp
api.put.return_value = MagicMock(ok=True, status_code=200, text="{}")
drv._api = api
return drv, api
class TestApiSetInterfaceExistingMembership:
"""A port that already carries the VLAN needs its mode changed, not a new row.
Reproduced on a 2530-24G-PoEP (REST v7): port 10 holds VLAN 10 untagged
alongside 30/40/50 tagged. Posting the tagged membership answers
`400 {"message":"Association exists"}` — only `409` was treated as
"already there". The resource is `{vlan_id}-{port_id}` and takes a PUT.
"""
def test_trunk_falls_back_to_put_on_association_exists(self):
drv, api = _api_driver(post_status=400)
drv.set_interface("10", {"mode": "trunk", "trunk_vlans": [10]})
api.put.assert_called_once()
path = api.put.call_args[0][0]
assert path == "vlans-ports/10-10"
assert api.put.call_args[1]["json"]["port_mode"] == "POM_TAGGED_STATIC"
def test_access_falls_back_to_put_on_association_exists(self):
drv, api = _api_driver(post_status=400)
drv.set_interface("10", {"mode": "access", "access_vlan": 20})
assert api.put.call_args[0][0] == "vlans-ports/20-10"
assert api.put.call_args[1]["json"]["port_mode"] == "POM_UNTAGGED"
def test_conflict_status_also_falls_back(self):
"""Other firmware answers 409 for the same situation."""
drv, api = _api_driver(post_status=409)
drv.set_interface("10", {"mode": "trunk", "trunk_vlans": [10]})
api.put.assert_called_once()
def test_a_new_membership_still_uses_post_alone(self):
drv, api = _api_driver(post_status=200)
drv.set_interface("11", {"mode": "trunk", "trunk_vlans": [30]})
api.post.assert_called_once()
api.put.assert_not_called()
def test_a_failing_put_reports_the_response_body(self):
"""The message used to drop it, so "Association exists" never surfaced."""
drv, api = _api_driver(post_status=400)
api.put.return_value = MagicMock(ok=False, status_code=403, text='{"message":"denied"}')
with pytest.raises(Exception) as exc:
drv.set_interface("10", {"mode": "trunk", "trunk_vlans": [10]})
assert "denied" in str(exc.value)
def test_an_unrelated_post_failure_reports_the_response_body(self):
drv, api = _api_driver(post_status=500, post_body='{"message":"boom"}')
with pytest.raises(Exception) as exc:
drv.set_interface("10", {"mode": "trunk", "trunk_vlans": [10]})
assert "boom" in str(exc.value)
api.put.assert_not_called()