mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-08-27 20:40:13 +03:00
fix(marker): fix inheritance hook, topology load, update sync, and asdict
Bugs found via end-to-end testing of project-level marker definitions: 1. New links didn't inherit — apply_defs_to_new_link is async but was called without await in UDPLink.create(). 2. Project load crashed — load_project passed marker_definitions to Project.__init__. Now popped in load_project and restored separately in Project.open() (it backs a read-only property). 3. PUT on a definition didn't sync to links — update_marker's guard rejected even the project-layer sync call. Added an `inherited` bypass flag used by update_marker_definition. 4. _ubridge_add_marker_filter raised re.PatternError — the name regex used (?i)(?!global) look-around, invalid in Python's re module. Dropped the prefix check there: "global-*" names are legitimate at the uBridge boundary (inherited definitions); forbidden only at the user-facing schema. 5. GET /links hid inherited markers — asdict()'s runtime branch used _persist_markers() (which filters inherited markers). Restored self._markers for the runtime branch; only the topology_dump branch filters (inherited markers are rebuilt from definitions on load). 6. Duplicate "already exists" warnings on project open — open() fanned out definitions to all links, but UDPLink.create() had already done so via its inheritance hook. Removed the redundant fan-out in open().
This commit is contained in:
parent
84845f8504
commit
179f072c33
@ -1093,7 +1093,10 @@ class BaseNode:
|
||||
# mark <bpf> [tag <id>] [pcap <path>] — tag/pcap keyword pairs, any order.
|
||||
# name travels from the controller REST layer (MarkerCreate schema) but is
|
||||
# validated here too as defense-in-depth against hand-edited topology files.
|
||||
_MARKER_NAME_RE = re.compile(r"^(?i)(?!global)[A-Za-z0-9][A-Za-z0-9_.-]*$")
|
||||
# Note: "global-*" names are legitimate here — they come from project-level
|
||||
# marker definitions (inherit_marker). The prefix is only forbidden at the
|
||||
# user-facing schema layer, not at the uBridge boundary.
|
||||
_MARKER_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]*$")
|
||||
if not _MARKER_NAME_RE.match(name):
|
||||
raise UbridgeError(f"Invalid marker name: {name!r}")
|
||||
cmd = 'bridge add_packet_filter {bridge} {name} mark "{bpf}"'.format(
|
||||
|
||||
@ -741,6 +741,9 @@ class Controller:
|
||||
topo_data.pop("version")
|
||||
topo_data.pop("revision")
|
||||
topo_data.pop("type")
|
||||
# marker_definitions is restored by Project.open() from the topology
|
||||
# file; it must not be passed to Project.__init__.
|
||||
topo_data.pop("marker_definitions", None)
|
||||
|
||||
if topo_data["project_id"] in self._projects:
|
||||
project = self._projects[topo_data["project_id"]]
|
||||
|
||||
@ -641,7 +641,7 @@ class Link:
|
||||
"capture_compute_id": self.capture_compute_id,
|
||||
"link_type": self._link_type,
|
||||
"filters": self._filters,
|
||||
"markers": self._persist_markers(),
|
||||
"markers": self._markers,
|
||||
"suspend": self._suspended,
|
||||
"link_style": self._link_style,
|
||||
"wireshark": self._wireshark,
|
||||
|
||||
@ -970,7 +970,7 @@ class Project:
|
||||
marker_name = f"global-{name}"
|
||||
if marker_name in link.markers and link.markers[marker_name].get("inherited_from") == name:
|
||||
await link.update_marker(
|
||||
marker_name, bpf=d["bpf"], tag=d.get("tag"), color=d.get("color")
|
||||
marker_name, bpf=d["bpf"], tag=d.get("tag"), color=d.get("color"), inherited=True
|
||||
)
|
||||
self.dump()
|
||||
self.emit_notification("project.updated", self.asdict())
|
||||
@ -1410,7 +1410,6 @@ class Project:
|
||||
"auto_start",
|
||||
"auto_close",
|
||||
"auto_open",
|
||||
"marker_definitions",
|
||||
"scene_height",
|
||||
"scene_width",
|
||||
"zoom",
|
||||
@ -1427,6 +1426,12 @@ class Project:
|
||||
if val is not None:
|
||||
setattr(self, key, val)
|
||||
|
||||
# marker_definitions is loaded separately (it is not a __init__ kwarg
|
||||
# nor a simple attribute — it backs a read-only property).
|
||||
defs = project_data.get("marker_definitions")
|
||||
if isinstance(defs, dict):
|
||||
self._marker_definitions = defs
|
||||
|
||||
topology = project_data["topology"]
|
||||
for compute in topology.get("computes", []):
|
||||
compute_id = compute.get("compute_id")
|
||||
@ -1493,10 +1498,9 @@ class Project:
|
||||
for drawing_data in topology.get("drawings", []):
|
||||
await self.add_drawing(dump=False, **drawing_data)
|
||||
|
||||
# After every link is loaded, apply project-level marker definitions
|
||||
# so inherited markers are present from the start.
|
||||
for link in list(self._links.values()):
|
||||
await self.apply_defs_to_new_link(link)
|
||||
# Note: project-level marker definitions are applied to each link
|
||||
# inside UDPLink.create() (the inheritance hook), so they are
|
||||
# already present once links are loaded — no separate fan-out here.
|
||||
|
||||
self.dump()
|
||||
# We catch all error to be able to roll back the .gns3 to the previous state
|
||||
|
||||
@ -147,7 +147,7 @@ class UDPLink(Link):
|
||||
self._created = True
|
||||
# New links automatically inherit every active project-level marker
|
||||
# definition so the user doesn't have to reconfigure.
|
||||
self._project.apply_defs_to_new_link(self)
|
||||
await self._project.apply_defs_to_new_link(self)
|
||||
|
||||
async def update(self):
|
||||
"""
|
||||
@ -391,7 +391,7 @@ class UDPLink(Link):
|
||||
self._project.emit_notification("link.updated", self.asdict())
|
||||
self._project.dump()
|
||||
|
||||
async def update_marker(self, name, bpf=None, tag=None, enabled=None, color=None):
|
||||
async def update_marker(self, name, bpf=None, tag=None, enabled=None, color=None, inherited=False):
|
||||
"""
|
||||
Update an existing marker's BPF/tag/enabled/color. Any change pushes via
|
||||
``self.update()``; uBridge picks up the new params on the next NIO
|
||||
@ -402,13 +402,15 @@ class UDPLink(Link):
|
||||
:param tag: new tag id (None = keep existing)
|
||||
:param enabled: toggle (None = keep existing)
|
||||
:param color: new hex color (None = keep existing)
|
||||
:param inherited: set by project-level sync to bypass the inheritance
|
||||
guard (the project layer is the legitimate editor)
|
||||
"""
|
||||
|
||||
marker_info = self._markers.get(name)
|
||||
if not marker_info:
|
||||
raise ControllerNotFoundError(f"Marker '{name}' not found on link {self._id}")
|
||||
|
||||
if marker_info.get("inherited_from"):
|
||||
if marker_info.get("inherited_from") and not inherited:
|
||||
raise ControllerError(
|
||||
f"Marker '{name}' is inherited from the project-level "
|
||||
f"definition '{marker_info['inherited_from']}'. "
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user