diff --git a/gns3server/api/routes/mcp/__init__.py b/gns3server/api/routes/mcp/__init__.py index 3f2425b00..2369fe658 100644 --- a/gns3server/api/routes/mcp/__init__.py +++ b/gns3server/api/routes/mcp/__init__.py @@ -1052,7 +1052,6 @@ async def marker_definition( 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, 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]]: """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. 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 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' """ 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) if val is not None: params[opt] = val diff --git a/gns3server/api/routes/mcp/links.py b/gns3server/api/routes/mcp/links.py index 9ddf2b377..7e1afc6a2 100644 --- a/gns3server/api/routes/mcp/links.py +++ b/gns3server/api/routes/mcp/links.py @@ -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"): if params.get(opt) is not None: body[opt] = params[opt] - # direction: "tx"/"rx" set a one-way filter; "both"/omitted = no filter. - if params.get("direction") in ("tx", "rx"): - body["direction"] = params["direction"] + # No direction: a definition fans out to every link and auto-selects its + # capture node on each, so tx/rx (which is relative to that node) has no + # consistent meaning. Encode direction in the BPF instead. return conn.http_call("post", base, json_data=body).json() 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"): if params.get(opt) is not None: 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: - 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() # action == "delete" diff --git a/tests/api/routes/mcp/test_handlers.py b/tests/api/routes/mcp/test_handlers.py index 5cd8a8bc0..fc8cd00d2 100644 --- a/tests/api/routes/mcp/test_handlers.py +++ b/tests/api/routes/mcp/test_handlers.py @@ -441,39 +441,44 @@ class TestLinkMarker: 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" - 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 with patch(f"{BASE}.{self.mod}._get_connector") as m: conn = _mock_conn({"name": "arp"}) m.return_value = conn marker_definition_handler( - {"project_id": "p", "action": "update", - "def_name": "arp", "direction": "both"}, ctx, + {"project_id": "p", "action": "create", + "bpf": "arp", "tag": 1, "color": "#fff"}, ctx, ) conn.http_call.assert_called_with( - "put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/arp", - json_data={"direction": None}, + "post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions", + 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 with patch(f"{BASE}.{self.mod}._get_connector") as m: conn = _mock_conn({"name": "arp"}) m.return_value = conn marker_definition_handler( - {"project_id": "p", "action": "update", - "def_name": "arp", "direction": "tx"}, ctx, + {"project_id": "p", "action": "create", + "bpf": "arp", "direction": "tx"}, ctx, ) conn.http_call.assert_called_with( - "put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/arp", - json_data={"direction": "tx"}, + "post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions", + 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 with patch(f"{BASE}.{self.mod}._get_connector") as m: conn = _mock_conn({"name": "arp"}) @@ -487,30 +492,24 @@ class TestMarkerDefinition: 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 with patch(f"{BASE}.{self.mod}._get_connector") as m: conn = _mock_conn({"name": "arp"}) m.return_value = conn marker_definition_handler( - {"project_id": "p", "action": "create", - "bpf": "arp", "direction": "both"}, ctx, + {"project_id": "p", "action": "update", + "def_name": "arp", "tag": 1, "direction": "rx"}, ctx, ) conn.http_call.assert_called_with( - "post", "http://192.168.1.3:3080/v3/projects/p/marker-definitions", - json_data={"bpf": "arp"}, + "put", "http://192.168.1.3:3080/v3/projects/p/marker-definitions/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 - with patch(f"{BASE}.{self.mod}._get_connector") as m: - conn = _mock_conn({"name": "arp"}) - m.return_value = conn - 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"}, + with patch(f"{BASE}.{self.mod}._get_connector"): + result = marker_definition_handler( + {"project_id": "p", "action": "update", "def_name": "arp"}, ctx, ) + assert "error" in result