mcp: drop direction from marker_definition tool

A marker definition fans out to every link and auto-selects its capture node
on each, so tx/rx is relative to a node that varies per link — the controller
already rejects it (409). Exposing direction on the MCP definition tool let an
agent ask for something that could only fail. Remove the parameter and the
handler's direction handling; the docstring now points to encoding direction in
the BPF (e.g. 'icmp and icmp[icmptype]==8'). Per-link link_marker keeps
direction, where the capture node is fixed.
This commit is contained in:
YueGuobin 2026-08-05 01:40:22 +08:00
parent f7b19dae99
commit 7ed4eeab29
No known key found for this signature in database
3 changed files with 39 additions and 41 deletions

View File

@ -1052,7 +1052,6 @@ async def marker_definition(
tag: Annotated[int | None, Field(description="Numeric tag for packet correlation")] = None, tag: Annotated[int | None, Field(description="Numeric tag for packet correlation")] = None,
color: Annotated[str | None, Field(description="Hex color for UI highlight, e.g. '#ff5722'")] = None, color: Annotated[str | None, Field(description="Hex color for UI highlight, e.g. '#ff5722'")] = None,
highlight_duration: Annotated[int | None, Field(description="UI highlight duration in milliseconds")] = None, highlight_duration: Annotated[int | None, Field(description="UI highlight duration in milliseconds")] = None,
direction: Annotated[str | None, Field(description="Direction filter: 'tx' (capture node sending only), 'rx' (receiving only), or 'both' (no filter — on update this clears a previously set direction). Omit to leave unchanged on update.")] = None,
) -> list[dict[str, Any]]: ) -> list[dict[str, Any]]:
"""Manage project-level marker definitions — traffic-insight rules that apply to ALL links. """Manage project-level marker definitions — traffic-insight rules that apply to ALL links.
@ -1061,14 +1060,20 @@ async def marker_definition(
On delete, 'global-{name}' is removed from every link. On delete, 'global-{name}' is removed from every link.
Create requires: project_id, action='create', bpf Create requires: project_id, action='create', bpf
Update requires: project_id, action='update', def_name, and at least one of (bpf, tag, direction, color, highlight_duration) Update requires: project_id, action='update', def_name, and at least one of (bpf, tag, color, highlight_duration)
Delete requires: project_id, action='delete', def_name Delete requires: project_id, action='delete', def_name
List requires: project_id, action='list' List requires: project_id, action='list'
A definition has NO direction (tx/rx): it fans out to every link and auto-selects
its capture node on each, so a fixed direction has no consistent meaning. Encode
the direction you want in the BPF instead (e.g. 'icmp and icmp[icmptype]==8' for
echo requests only). For a capture-node-relative direction on a single link, use
the per-link `link_marker` tool.
Common BPF examples: 'arp', 'icmp', 'ospf', 'tcp port 22', 'udp port 53' Common BPF examples: 'arp', 'icmp', 'ospf', 'tcp port 22', 'udp port 53'
""" """
params = {"project_id": project_id, "action": action} params = {"project_id": project_id, "action": action}
for opt in ("bpf", "def_name", "name", "tag", "direction", "color", "highlight_duration"): for opt in ("bpf", "def_name", "name", "tag", "color", "highlight_duration"):
val = locals().get(opt) val = locals().get(opt)
if val is not None: if val is not None:
params[opt] = val params[opt] = val

View File

@ -460,9 +460,9 @@ def marker_definition_handler(params: dict[str, Any], gns3_ctx: dict[str, Any])
for opt in ("name", "tag", "color", "highlight_duration"): for opt in ("name", "tag", "color", "highlight_duration"):
if params.get(opt) is not None: if params.get(opt) is not None:
body[opt] = params[opt] body[opt] = params[opt]
# direction: "tx"/"rx" set a one-way filter; "both"/omitted = no filter. # No direction: a definition fans out to every link and auto-selects its
if params.get("direction") in ("tx", "rx"): # capture node on each, so tx/rx (which is relative to that node) has no
body["direction"] = params["direction"] # consistent meaning. Encode direction in the BPF instead.
return conn.http_call("post", base, json_data=body).json() return conn.http_call("post", base, json_data=body).json()
def_name = params.get("def_name") def_name = params.get("def_name")
@ -476,14 +476,8 @@ def marker_definition_handler(params: dict[str, Any], gns3_ctx: dict[str, Any])
for opt in ("bpf", "tag", "color", "highlight_duration"): for opt in ("bpf", "tag", "color", "highlight_duration"):
if params.get(opt) is not None: if params.get(opt) is not None:
body[opt] = params[opt] body[opt] = params[opt]
# direction tri-state: omitted=preserve, "tx"/"rx"=set, "both"=clear (→ null).
direction = params.get("direction")
if direction == "both":
body["direction"] = None
elif direction in ("tx", "rx"):
body["direction"] = direction
if not body: if not body:
return {"error": "At least one update field is required (bpf, tag, direction, color, highlight_duration)"} return {"error": "At least one update field is required (bpf, tag, color, highlight_duration)"}
return conn.http_call("put", url, json_data=body).json() return conn.http_call("put", url, json_data=body).json()
# action == "delete" # action == "delete"

View File

@ -441,39 +441,44 @@ class TestLinkMarker:
class TestMarkerDefinition: class TestMarkerDefinition:
"""marker_definition_handler direction tri-state (same semantics as link markers).""" """marker_definition_handler build create/update bodies.
A definition has NO direction: it fans out to every link and auto-selects its
capture node on each, so tx/rx (relative to that node) has no consistent
meaning — any direction passed is ignored, never reaching the request body.
"""
mod = "links" mod = "links"
def test_update_direction_both_clears(self, ctx): def test_create_builds_body(self, ctx):
from gns3server.api.routes.mcp.links import marker_definition_handler from gns3server.api.routes.mcp.links import marker_definition_handler
with patch(f"{BASE}.{self.mod}._get_connector") as m: with patch(f"{BASE}.{self.mod}._get_connector") as m:
conn = _mock_conn({"name": "arp"}) conn = _mock_conn({"name": "arp"})
m.return_value = conn m.return_value = conn
marker_definition_handler( marker_definition_handler(
{"project_id": "p", "action": "update", {"project_id": "p", "action": "create",
"def_name": "arp", "direction": "both"}, ctx, "bpf": "arp", "tag": 1, "color": "#fff"}, ctx,
) )
conn.http_call.assert_called_with( conn.http_call.assert_called_with(
"put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/arp", "post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions",
json_data={"direction": None}, json_data={"bpf": "arp", "tag": 1, "color": "#fff"},
) )
def test_update_direction_tx_sets(self, ctx): def test_create_ignores_direction(self, ctx):
from gns3server.api.routes.mcp.links import marker_definition_handler from gns3server.api.routes.mcp.links import marker_definition_handler
with patch(f"{BASE}.{self.mod}._get_connector") as m: with patch(f"{BASE}.{self.mod}._get_connector") as m:
conn = _mock_conn({"name": "arp"}) conn = _mock_conn({"name": "arp"})
m.return_value = conn m.return_value = conn
marker_definition_handler( marker_definition_handler(
{"project_id": "p", "action": "update", {"project_id": "p", "action": "create",
"def_name": "arp", "direction": "tx"}, ctx, "bpf": "arp", "direction": "tx"}, ctx,
) )
conn.http_call.assert_called_with( conn.http_call.assert_called_with(
"put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/arp", "post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions",
json_data={"direction": "tx"}, json_data={"bpf": "arp"},
) )
def test_update_direction_omitted_preserved(self, ctx): def test_update_builds_body(self, ctx):
from gns3server.api.routes.mcp.links import marker_definition_handler from gns3server.api.routes.mcp.links import marker_definition_handler
with patch(f"{BASE}.{self.mod}._get_connector") as m: with patch(f"{BASE}.{self.mod}._get_connector") as m:
conn = _mock_conn({"name": "arp"}) conn = _mock_conn({"name": "arp"})
@ -487,30 +492,24 @@ class TestMarkerDefinition:
json_data={"tag": 1}, json_data={"tag": 1},
) )
def test_create_direction_both_omitted(self, ctx): def test_update_ignores_direction(self, ctx):
from gns3server.api.routes.mcp.links import marker_definition_handler from gns3server.api.routes.mcp.links import marker_definition_handler
with patch(f"{BASE}.{self.mod}._get_connector") as m: with patch(f"{BASE}.{self.mod}._get_connector") as m:
conn = _mock_conn({"name": "arp"}) conn = _mock_conn({"name": "arp"})
m.return_value = conn m.return_value = conn
marker_definition_handler( marker_definition_handler(
{"project_id": "p", "action": "create", {"project_id": "p", "action": "update",
"bpf": "arp", "direction": "both"}, ctx, "def_name": "arp", "tag": 1, "direction": "rx"}, ctx,
) )
conn.http_call.assert_called_with( conn.http_call.assert_called_with(
"post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions", "put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/arp",
json_data={"bpf": "arp"}, json_data={"tag": 1},
) )
def test_create_direction_tx(self, ctx): def test_update_requires_a_field(self, ctx):
from gns3server.api.routes.mcp.links import marker_definition_handler from gns3server.api.routes.mcp.links import marker_definition_handler
with patch(f"{BASE}.{self.mod}._get_connector") as m: with patch(f"{BASE}.{self.mod}._get_connector"):
conn = _mock_conn({"name": "arp"}) result = marker_definition_handler(
m.return_value = conn {"project_id": "p", "action": "update", "def_name": "arp"}, ctx,
marker_definition_handler(
{"project_id": "p", "action": "create",
"bpf": "arp", "direction": "tx"}, ctx,
)
conn.http_call.assert_called_with(
"post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions",
json_data={"bpf": "arp", "direction": "tx"},
) )
assert "error" in result