mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-10-28 19:22:22 +02:00
marker: let callers pin the capture node via capture_node_id
A marker is single-sided — only the chosen capture node's uBridge installs the mark filter — and dir=tx|rx is interpreted from that node's perspective. Until now the observer was always auto-picked (_choose_marker_side), so dir=tx meant "the auto-chosen endpoint is sending", which is unpredictable and makes the direction filter hard to render meaningfully in the Web UI. Add an optional create-only capture_node_id to MarkerCreate: when set, the marker is pinned to that endpoint's uBridge (validated as a link endpoint and a marker-capable type); when omitted, behavior is unchanged (auto-pick). The chosen id is already echoed back as capture_node_id and in MARK signals, so the UI can always render the observer regardless of who picked it. capture_node_id is create-only (changing it would silently flip the meaning of stored direction; recreate instead) and is not accepted on project-level definitions — they are link-agnostic and have no endpoints, so inherited markers keep auto-picking per link. Plumbed through REST create_marker, the MCP link_marker tool, and base Link.start_marker. update_marker does not forward it.
This commit is contained in:
parent
6749b872fa
commit
71fa778d50
@ -125,6 +125,28 @@ Because the listener ignores unknown keys, `dir` is **additive**: an older serve
|
|||||||
drops it and an older uBridge simply omits it — either way the system falls back to
|
drops it and an older uBridge simply omits it — either way the system falls back to
|
||||||
undirected rendering with no error.
|
undirected rendering with no error.
|
||||||
|
|
||||||
|
### Choosing the capture node
|
||||||
|
|
||||||
|
Since `dir` is relative to the capture node, *which* endpoint is the observer decides what
|
||||||
|
`tx`/`rx` mean. By default the server auto-picks (first started marker-capable endpoint, in
|
||||||
|
link-endpoint order). To pin it — e.g. so `dir=tx` unambiguously means "vpcs1 is sending" —
|
||||||
|
pass `capture_node_id` on marker **create**:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{ "bpf": "icmp", "direction": "tx", "capture_node_id": "<vpcs1 node uuid>" }
|
||||||
|
```
|
||||||
|
|
||||||
|
The value must be one of the link's two endpoints and a marker-capable type (`vpcs`, `qemu`,
|
||||||
|
`docker`, `iou`, `dynamips`, `cloud`); any other id is rejected with `409`. Omit it to keep
|
||||||
|
the auto-pick. The chosen id is echoed back as `capture_node_id` in the marker entry and in
|
||||||
|
each `MARK` signal's `node=<id>`, so the Web UI always knows the observer regardless of who
|
||||||
|
picked it.
|
||||||
|
|
||||||
|
`capture_node_id` is **create-only**: it is fixed once the marker exists (changing the
|
||||||
|
observer would silently flip the meaning of stored `direction`, so recreate the marker
|
||||||
|
instead). It is not accepted on project-level definitions — a definition is link-agnostic and
|
||||||
|
has no endpoints to choose from, so inherited markers always auto-pick per link.
|
||||||
|
|
||||||
## API Endpoints
|
## API Endpoints
|
||||||
|
|
||||||
All endpoints require a JWT bearer token (`POST /v3/access/users/authenticate`). The
|
All endpoints require a JWT bearer token (`POST /v3/access/users/authenticate`). The
|
||||||
@ -167,12 +189,17 @@ extra request.
|
|||||||
"name": "icmp",
|
"name": "icmp",
|
||||||
"bpf": "icmp",
|
"bpf": "icmp",
|
||||||
"tag": 1,
|
"tag": 1,
|
||||||
|
"direction": "tx",
|
||||||
|
"capture_node_id": "a37e2235-e21f-46c9-a2ab-ba0f8c5465e6",
|
||||||
"color": "#ff5722",
|
"color": "#ff5722",
|
||||||
"highlight_duration": 800,
|
"highlight_duration": 800,
|
||||||
"enabled": true
|
"enabled": true
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
`direction` and `capture_node_id` are both optional and create-only (see
|
||||||
|
[Direction](#direction)).
|
||||||
|
|
||||||
**Definition create body** (`MarkerDefinitionCreate`, shared by POST and PUT):
|
**Definition create body** (`MarkerDefinitionCreate`, shared by POST and PUT):
|
||||||
|
|
||||||
```json
|
```json
|
||||||
@ -224,7 +251,8 @@ extra request.
|
|||||||
| `enabled` | bool | Whether the marker is active |
|
| `enabled` | bool | Whether the marker is active |
|
||||||
| `color` | string \| null | Hex color render hint, e.g. `#ff5722` |
|
| `color` | string \| null | Hex color render hint, e.g. `#ff5722` |
|
||||||
| `highlight_duration` | int \| null | UI highlight duration in ms after a match; `null` = UI default |
|
| `highlight_duration` | int \| null | UI highlight duration in ms after a match; `null` = UI default |
|
||||||
| `capture_node_id` | string | Server-chosen node whose uBridge hosts the marker |
|
| `direction` | string \| null | `tx` / `rx` filter relative to the capture node; `null` = both |
|
||||||
|
| `capture_node_id` | string | Node whose uBridge hosts the marker — caller-set on create, else auto-picked |
|
||||||
| `inherited_from` | string | Source definition name — present on inherited markers only |
|
| `inherited_from` | string | Source definition name — present on inherited markers only |
|
||||||
|
|
||||||
### Definition
|
### Definition
|
||||||
|
|||||||
@ -466,6 +466,7 @@ async def create_marker(
|
|||||||
bpf=marker_data.bpf,
|
bpf=marker_data.bpf,
|
||||||
tag=marker_data.tag,
|
tag=marker_data.tag,
|
||||||
direction=marker_data.direction,
|
direction=marker_data.direction,
|
||||||
|
capture_node_id=marker_data.capture_node_id,
|
||||||
color=marker_data.color,
|
color=marker_data.color,
|
||||||
highlight_duration=marker_data.highlight_duration,
|
highlight_duration=marker_data.highlight_duration,
|
||||||
)
|
)
|
||||||
|
|||||||
@ -1016,6 +1016,7 @@ async def link_marker(
|
|||||||
tag: Annotated[int | None, Field(description="Numeric tag for packet correlation")] = None,
|
tag: Annotated[int | None, Field(description="Numeric tag for packet correlation")] = None,
|
||||||
enabled: Annotated[bool | None, Field(description="Enable or disable the marker (for update action)")] = None,
|
enabled: Annotated[bool | None, Field(description="Enable or disable the marker (for update action)")] = None,
|
||||||
direction: Annotated[str | None, Field(description="Direction filter: 'tx' for capture node sending only, 'rx' for capture node receiving only (omit for both)")] = None,
|
direction: Annotated[str | None, Field(description="Direction filter: 'tx' for capture node sending only, 'rx' for capture node receiving only (omit for both)")] = None,
|
||||||
|
capture_node_id: Annotated[str | None, Field(description="UUID of the endpoint whose uBridge hosts the marker (the observer; tx/rx are from its perspective). Must be a link endpoint and marker-capable. Omit to auto-pick.")] = 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,
|
||||||
) -> list[dict[str, Any]]:
|
) -> list[dict[str, Any]]:
|
||||||
@ -1034,7 +1035,7 @@ async def link_marker(
|
|||||||
and cannot be modified or deleted via this tool.
|
and cannot be modified or deleted via this tool.
|
||||||
"""
|
"""
|
||||||
params = {"project_id": project_id, "link_id": link_id, "action": action}
|
params = {"project_id": project_id, "link_id": link_id, "action": action}
|
||||||
for opt in ("bpf", "marker_name", "name", "tag", "enabled", "direction", "color", "highlight_duration"):
|
for opt in ("bpf", "marker_name", "name", "tag", "enabled", "direction", "capture_node_id", "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
|
||||||
|
|||||||
@ -395,7 +395,7 @@ def link_marker_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
|
|||||||
if not bpf:
|
if not bpf:
|
||||||
return {"error": "bpf is required for create action"}
|
return {"error": "bpf is required for create action"}
|
||||||
body: dict[str, Any] = {"bpf": bpf}
|
body: dict[str, Any] = {"bpf": bpf}
|
||||||
for opt in ("name", "tag", "direction", "color", "highlight_duration"):
|
for opt in ("name", "tag", "direction", "capture_node_id", "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]
|
||||||
return conn.http_call("post", base, json_data=body).json()
|
return conn.http_call("post", base, json_data=body).json()
|
||||||
|
|||||||
@ -334,7 +334,7 @@ class Link:
|
|||||||
|
|
||||||
raise NotImplementedError
|
raise NotImplementedError
|
||||||
|
|
||||||
async def start_marker(self, name, bpf, tag=None, direction=None):
|
async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None):
|
||||||
"""
|
"""
|
||||||
Attach a traffic-insight marker to this link (base — UDPLink overrides).
|
Attach a traffic-insight marker to this link (base — UDPLink overrides).
|
||||||
"""
|
"""
|
||||||
|
|||||||
@ -310,6 +310,31 @@ class UDPLink(Link):
|
|||||||
"traffic insight"
|
"traffic insight"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def _node_by_id(self, node_id):
|
||||||
|
"""
|
||||||
|
Resolve a caller-chosen capture node by id, validating it is an
|
||||||
|
endpoint of this link and marker-capable. Used when the caller
|
||||||
|
(REST/MCP) explicitly pins the observer side instead of letting
|
||||||
|
``_choose_marker_side`` auto-pick.
|
||||||
|
|
||||||
|
:param node_id: node id (UUID or str) the caller requested
|
||||||
|
:returns: a ``self._nodes`` entry (node/adapter_number/port_number)
|
||||||
|
"""
|
||||||
|
|
||||||
|
target = str(node_id)
|
||||||
|
for node in self._nodes:
|
||||||
|
if str(node["node"].id) != target:
|
||||||
|
continue
|
||||||
|
if node["node"].node_type not in _MARKER_CAPABLE_TYPES:
|
||||||
|
raise ControllerError(
|
||||||
|
f"Node {node_id} ({node['node'].node_type}) cannot host a "
|
||||||
|
f"marker — no uBridge bridge to attach the filter to"
|
||||||
|
)
|
||||||
|
return node
|
||||||
|
raise ControllerNotFoundError(
|
||||||
|
f"Node {node_id} is not an endpoint of link {self._id}"
|
||||||
|
)
|
||||||
|
|
||||||
async def node_updated(self, node):
|
async def node_updated(self, node):
|
||||||
"""
|
"""
|
||||||
Called when a node member of the link is updated
|
Called when a node member of the link is updated
|
||||||
@ -322,7 +347,7 @@ class UDPLink(Link):
|
|||||||
# explicitly deletes a marker via the REST API, and a marker is torn
|
# explicitly deletes a marker via the REST API, and a marker is torn
|
||||||
# down automatically only when its link is deleted.
|
# down automatically only when its link is deleted.
|
||||||
|
|
||||||
async def start_marker(self, name, bpf, tag=None, direction=None, color=None, highlight_duration=None, inherited_from=None):
|
async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None, color=None, highlight_duration=None, inherited_from=None):
|
||||||
"""
|
"""
|
||||||
Attach a traffic-insight marker to this link.
|
Attach a traffic-insight marker to this link.
|
||||||
|
|
||||||
@ -335,6 +360,11 @@ class UDPLink(Link):
|
|||||||
:param name: stable filter name — echoed in MARK signals + pcap identity
|
:param name: stable filter name — echoed in MARK signals + pcap identity
|
||||||
:param bpf: libpcap BPF expression
|
:param bpf: libpcap BPF expression
|
||||||
:param tag: optional correlation id
|
:param tag: optional correlation id
|
||||||
|
:param capture_node_id: optional explicit observer node. When set the
|
||||||
|
marker is pinned to that endpoint's uBridge (and ``direction`` is
|
||||||
|
interpreted from its perspective); validated by ``_node_by_id``.
|
||||||
|
Omitted = auto-pick via ``_choose_marker_side``. Ignored for
|
||||||
|
inherited markers (project defs are link-agnostic → always auto).
|
||||||
:param color: optional hex color for the Web UI (e.g. '#ff5722'); stored
|
:param color: optional hex color for the Web UI (e.g. '#ff5722'); stored
|
||||||
with the link and persisted in the topology, never sent to uBridge
|
with the link and persisted in the topology, never sent to uBridge
|
||||||
:param highlight_duration: optional UI-only hint (milliseconds) for how
|
:param highlight_duration: optional UI-only hint (milliseconds) for how
|
||||||
@ -350,7 +380,10 @@ class UDPLink(Link):
|
|||||||
if not result.get("valid"):
|
if not result.get("valid"):
|
||||||
raise ControllerError(f"Invalid BPF expression: {result.get('error', 'unknown error')}")
|
raise ControllerError(f"Invalid BPF expression: {result.get('error', 'unknown error')}")
|
||||||
|
|
||||||
marker_side = self._choose_marker_side()
|
if capture_node_id and not inherited_from:
|
||||||
|
marker_side = self._node_by_id(capture_node_id)
|
||||||
|
else:
|
||||||
|
marker_side = self._choose_marker_side()
|
||||||
marker_entry = {
|
marker_entry = {
|
||||||
"bpf": bpf,
|
"bpf": bpf,
|
||||||
"tag": tag,
|
"tag": tag,
|
||||||
|
|||||||
@ -180,6 +180,15 @@ class MarkerCreate(BaseModel):
|
|||||||
pattern=r"^(tx|rx)$",
|
pattern=r"^(tx|rx)$",
|
||||||
description="Direction filter: 'tx' = capture node sending only, 'rx' = capture node receiving only. Omitted or null = both directions.",
|
description="Direction filter: 'tx' = capture node sending only, 'rx' = capture node receiving only. Omitted or null = both directions.",
|
||||||
)
|
)
|
||||||
|
capture_node_id: Optional[UUID] = Field(
|
||||||
|
None,
|
||||||
|
description=(
|
||||||
|
"Which endpoint's uBridge hosts this marker (the 'observer'). "
|
||||||
|
"tx/rx in `direction` are interpreted from this node's perspective. "
|
||||||
|
"Must be one of the link's two endpoints and a marker-capable type. "
|
||||||
|
"Omitted = server auto-picks (first started marker-capable endpoint)."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class MarkerDefinitionCreate(BaseModel):
|
class MarkerDefinitionCreate(BaseModel):
|
||||||
|
|||||||
@ -98,6 +98,40 @@ async def test_start_marker_stores_entry(project):
|
|||||||
assert "inherited_from" not in entry
|
assert "inherited_from" not in entry
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_start_marker_pins_capture_node(project):
|
||||||
|
# Auto-pick would choose node1 (first endpoint); pin to node2 explicitly.
|
||||||
|
with _valid_bpf():
|
||||||
|
link = await _make_link(project)
|
||||||
|
chosen = link._nodes[1]["node"].id
|
||||||
|
auto = link._nodes[0]["node"].id
|
||||||
|
assert chosen != auto # sanity: the pin must actually mean something
|
||||||
|
await link.start_marker("icmp", "icmp", capture_node_id=chosen)
|
||||||
|
|
||||||
|
assert link.markers["icmp"]["capture_node_id"] == chosen
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_start_marker_rejects_non_endpoint_capture_node(project):
|
||||||
|
|
||||||
|
with _valid_bpf():
|
||||||
|
link = await _make_link(project)
|
||||||
|
with pytest.raises(ControllerNotFoundError):
|
||||||
|
await link.start_marker("icmp", "icmp", capture_node_id="11111111-2222-3333-4444-555555555555")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_start_marker_capture_node_ignored_for_inherited(project):
|
||||||
|
# Definitions are link-agnostic: an inherited marker must auto-pick even
|
||||||
|
# if a capture_node_id leaks through, never trusting the caller's pin.
|
||||||
|
with _valid_bpf():
|
||||||
|
link = await _make_link(project)
|
||||||
|
leaked = link._nodes[1]["node"].id
|
||||||
|
await link.start_marker("m", "icmp", capture_node_id=leaked, inherited_from="arp")
|
||||||
|
|
||||||
|
assert link.markers["m"]["capture_node_id"] == link._nodes[0]["node"].id
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_start_marker_rejects_duplicate(project):
|
async def test_start_marker_rejects_duplicate(project):
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user