From 4a10a71c03b27d42f4ef8dd04e6a78ba051394f0 Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:13:04 +0100 Subject: [PATCH 1/5] fix(promsd): look lane tags up by name, not by a computed slug promsd turned a tag name into a slug by replacing the colon with a hyphen. NetBox deletes the colon when a client sends no slug, so a tag created in the UI got a slug promsd never asked for. NetBox answered 400 and the whole refresh failed. Each refresh now reads the slug of every lane tag from NetBox by exact name. A tag NetBox lacks, or a query NetBox refuses with 400, empties that lane only. /healthz lists those tags in unavailable_tags. Other errors still fail the refresh and keep the last snapshot. Closes dmfdeploy/dmfdeploy#795 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- README.md | 4 ++- src/dmf_promsd/cache.py | 16 ++++++--- src/dmf_promsd/main.py | 1 + src/dmf_promsd/netbox.py | 54 ++++++++++++++++++++++-------- tests/netbox45.py | 40 ++++++++++++++++------ tests/test_main.py | 71 ++++++++++++++++++++++++++++++++++++++++ tests/test_netbox.py | 5 +-- 7 files changed, 161 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index c19e2bc..73a6e65 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,8 @@ Part of [dmfdeploy](https://github.com/dmfdeploy/dmfdeploy), an independent open The dmfdeploy platform keeps NetBox as its source of truth for inventory. `dmf-promsd` reads NetBox services, devices and virtual machines, and serves the ones that carry a monitoring tag as Prometheus [HTTP service discovery](https://prometheus.io/docs/prometheus/latest/http_sd/) target groups. Prometheus then scrapes or probes new workloads without anyone editing its configuration. +The service looks each lane tag up in NetBox by name and filters by the slug NetBox returns, so a tag created in the NetBox UI works with whatever slug NetBox chose. If a lane tag doesn't exist in NetBox, or NetBox refuses to filter by it, that lane serves no targets from it, the other lanes keep working, and `unavailable_tags` in `/healthz` lists the tag. + A record joins a lane when it has the matching tag in NetBox and the lane reads that record type: | Tag | Endpoint | Reads | Lane | @@ -22,7 +24,7 @@ A record with the `cluster_service` and `cluster_namespace` custom fields uses t The service skips a record in a lane when it lacks a required field: `metrics_port` for scrape, `probe_module` for probe, or an address for any lane. It keeps the skip counts in memory and does not return them from `/healthz`. -`GET /healthz` returns the status, product name and version, the cache settings (`cache_ttl_seconds`, `http_sd_refresh` and `freshness_window_seconds`), `snapshot_age_seconds`, `source_counts` (the number of services, devices and virtual machines fetched from NetBox) and `last_error`. +`GET /healthz` returns the status, product name and version, the cache settings (`cache_ttl_seconds`, `http_sd_refresh` and `freshness_window_seconds`), `snapshot_age_seconds`, `source_counts` (the number of services, devices and virtual machines fetched from NetBox), `unavailable_tags` and `last_error`. ## Run it diff --git a/src/dmf_promsd/cache.py b/src/dmf_promsd/cache.py index 74d2703..009623b 100644 --- a/src/dmf_promsd/cache.py +++ b/src/dmf_promsd/cache.py @@ -17,6 +17,8 @@ class CacheSnapshot: lanes: dict[str, list[dict[str, Any]]] skipped: dict[str, dict[str, int]] source_counts: dict[str, int] + # Lane tags NetBox has no tag for, or refused to filter by. Their lanes serve fewer records. + unavailable_tags: list[str] @dataclass(frozen=True) @@ -70,11 +72,16 @@ def refresh(self) -> bool: try: records: list[Record] = [] source_counts: dict[str, int] = {} + slugs = self._client.tag_slugs(lane.tag for lane in LANES) + unavailable = {lane.tag for lane in LANES} - slugs.keys() for kind in RECORD_KINDS: - tags = [lane.tag for lane in LANES if kind in lane.kinds] - kind_records, unreadable = self._client.list_records(kind, tags) - records.extend(kind_records) - source_counts[kind] = len(kind_records) + unreadable + kind_slugs = [ + slugs[lane.tag] for lane in LANES if kind in lane.kinds and lane.tag in slugs + ] + listing = self._client.list_records(kind, kind_slugs) + records.extend(listing.records) + source_counts[kind] = len(listing.records) + listing.unreadable + unavailable |= {tag for tag, slug in slugs.items() if slug in listing.rejected} payloads, skipped = build_payloads(records) except Exception as exc: # noqa: BLE001, a failed refresh keeps the last snapshot with self._lock: @@ -86,6 +93,7 @@ def refresh(self) -> bool: lanes=payloads, skipped=skipped, source_counts=source_counts, + unavailable_tags=sorted(unavailable), ) with self._lock: self._snapshot = snapshot diff --git a/src/dmf_promsd/main.py b/src/dmf_promsd/main.py index b3f84c7..79b361d 100644 --- a/src/dmf_promsd/main.py +++ b/src/dmf_promsd/main.py @@ -25,6 +25,7 @@ def _health_payload(cache: PromSDCache, refresh_hint: str) -> dict[str, Any]: "freshness_window_seconds": cache.freshness_window_seconds, "snapshot_age_seconds": state.age_seconds, "source_counts": state.snapshot.source_counts if state.snapshot else {}, + "unavailable_tags": state.snapshot.unavailable_tags if state.snapshot else [], "last_error": state.last_error, } diff --git a/src/dmf_promsd/netbox.py b/src/dmf_promsd/netbox.py index 433a2ff..befc0c6 100644 --- a/src/dmf_promsd/netbox.py +++ b/src/dmf_promsd/netbox.py @@ -1,17 +1,13 @@ from __future__ import annotations from collections.abc import Iterable, Mapping -from typing import Any +from typing import Any, NamedTuple import httpx from .records import DEVICES, SERVICES, VIRTUAL_MACHINES, Record - -def _tag_slug(name: str) -> str: - # NetBox ?tag= filters by tag SLUG (colon -> hyphen), not the name. - return name.replace(":", "-") - +_TAGS_PATH = "/api/extras/tags/" _LIST_PATHS = { SERVICES: "/api/ipam/services/", @@ -51,6 +47,13 @@ def __init__(self, status_code: int, body: str) -> None: super().__init__(f"NetBox API {status_code}: {body}") +class Listing(NamedTuple): + records: list[Record] + unreadable: int + # Tag slugs whose query NetBox refused. Their records are missing from `records`. + rejected: list[str] + + class NetboxClient: def __init__( self, @@ -98,22 +101,47 @@ def _list_paginated( query = None return results - def list_tagged(self, kind: str, tags: Iterable[str]) -> list[dict[str, Any]]: + def tag_slugs(self, names: Iterable[str]) -> dict[str, str]: + """Map each tag name that exists in NetBox to the slug NetBox gave it. + + NetBox derives the slug when a client sends none, and the rule differs between + clients, so the slug is read back instead of computed. Names NetBox doesn't know + are left out. + """ + wanted = list(names) + params = [("limit", "1000"), *(("name", name) for name in wanted)] + found = {tag["name"]: tag["slug"] for tag in self._list_paginated(_TAGS_PATH, params)} + return {name: found[name] for name in wanted if name in found} + + def list_tagged( + self, kind: str, slugs: Iterable[str] + ) -> tuple[list[dict[str, Any]], list[str]]: path = _LIST_PATHS[kind] # NetBox ?tag= filters are ANDed; query each tag separately and union by # id so an object tagged with ANY of the tags is included (OR). merged: dict[Any, dict[str, Any]] = {} - for tag in tags: - for obj in self._list_paginated(path, [("limit", "1000"), ("tag", _tag_slug(tag))]): + rejected: list[str] = [] + for slug in slugs: + try: + objects = self._list_paginated(path, [("limit", "1000"), ("tag", slug)]) + except NetboxAPIError as exc: + # A 400 is about this query alone. Any other error means NetBox is + # unwell, and the caller keeps the last snapshot. + if exc.status_code != 400: + raise + rejected.append(slug) + continue + for obj in objects: merged[obj.get("id", id(obj))] = obj - return list(merged.values()) + return list(merged.values()), rejected - def list_records(self, kind: str, tags: Iterable[str]) -> tuple[list[Record], int]: + def list_records(self, kind: str, slugs: Iterable[str]) -> Listing: records: list[Record] = [] unreadable = 0 - for raw in self.list_tagged(kind, tags): + objects, rejected = self.list_tagged(kind, slugs) + for raw in objects: try: records.append(parse_record(kind, raw)) except (KeyError, TypeError, AttributeError): unreadable += 1 - return records, unreadable + return Listing(records, unreadable, rejected) diff --git a/tests/netbox45.py b/tests/netbox45.py index b82e15b..787f667 100644 --- a/tests/netbox45.py +++ b/tests/netbox45.py @@ -204,9 +204,9 @@ class FakeNetbox: """The three list endpoints promsd calls, answering as NetBox v4.5.0 does. While `down` is set, every request gets the 503 of a proxy whose NetBox is - gone. A request without the token is a 403. `?tag=` matches tag slugs, repeated - `tag` parameters must all match, and a slug that is neither seeded nor on a - record is a 400. Every match comes back in one page. + gone. A request without the token is a 403. `?name=` on the tags endpoint matches + exactly. `?tag=` matches tag slugs, repeated `tag` parameters must all match, and a + slug that is neither seeded nor on a record is a 400. Every match comes back in one page. """ def __init__( @@ -215,18 +215,28 @@ def __init__( services: Iterable[dict[str, Any]] = (), devices: Iterable[dict[str, Any]] = (), virtual_machines: Iterable[dict[str, Any]] = (), + tag_slugs: dict[str, str] | None = None, + absent_tags: Iterable[str] = (), ) -> None: self._records = { _LIST_PATHS["services"]: list(services), _LIST_PATHS["devices"]: list(devices), _LIST_PATHS["virtual_machines"]: list(virtual_machines), } - self._slugs = {_slug(name) for name in SEEDED_TAGS} | { - tag["slug"] - for records in self._records.values() - for record in records - for tag in record["tags"] - } + # A tag created in the NetBox UI gets NetBox's own slug, `monitoringscrape` for + # `monitoring:scrape`. `tag_slugs` overrides the slug of a named tag everywhere. + overrides = tag_slugs or {} + self._tags = {name: overrides.get(name, _slug(name)) for name in SEEDED_TAGS} + for records in self._records.values(): + for record in records: + for record_tag in record["tags"]: + record_tag["slug"] = overrides.get(record_tag["name"], record_tag["slug"]) + self._tags[record_tag["name"]] = record_tag["slug"] + for name in absent_tags: + del self._tags[name] + self._slugs = set(self._tags.values()) + # Slugs NetBox knows but answers 400 for on a list endpoint. + self.refused_slugs: set[str] = set() self.requests: list[httpx.Request] = [] self.down = False @@ -239,11 +249,21 @@ def __call__(self, request: httpx.Request) -> httpx.Response: return httpx.Response(503, text="Service Unavailable") if request.headers.get("Authorization") not in {f"Token {TOKEN}", f"Bearer {TOKEN}"}: return httpx.Response(403, json={"detail": "Invalid v1 token"}) + if request.url.path == "/api/extras/tags/": + names = request.url.params.get_list("name") + found = [ + {"id": i, "name": name, "slug": slug} + for i, (name, slug) in enumerate(self._tags.items(), 1) + if name in names + ] + return httpx.Response( + 200, json={"count": len(found), "next": None, "previous": None, "results": found} + ) records = self._records.get(request.url.path) if records is None: return httpx.Response(404, json={"detail": "Not found."}) slugs = request.url.params.get_list("tag") - unknown = [slug for slug in slugs if slug not in self._slugs] + unknown = [slug for slug in slugs if slug not in self._slugs or slug in self.refused_slugs] if unknown: choice = f"Select a valid choice. {unknown[0]} is not one of the available choices." return httpx.Response(400, json={"tag": [choice]}) diff --git a/tests/test_main.py b/tests/test_main.py index 1f828b3..f5b11e9 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -56,6 +56,7 @@ def test_lifespan_refreshes_on_startup_and_stops_on_shutdown(): "http_sd_refresh": "11s", "freshness_window_seconds": 14, "source_counts": {"services": 0, "devices": 1, "virtual_machines": 0}, + "unavailable_tags": [], "last_error": None, } assert ready.status_code == 200 @@ -232,3 +233,73 @@ def test_a_record_promsd_cannot_read_costs_the_others_nothing(): assert [group["targets"] for group in scrape] == [["192.0.2.11:9100"]] assert health["source_counts"]["devices"] == 2 assert health["last_error"] is None + + +def _serve(netbox: FakeNetbox) -> tuple[dict[str, list[str]], dict]: + client = netbox.client() + app = create_app(Settings(netbox_url=BASE_URL, netbox_token=TOKEN), client=client) + try: + with TestClient(app) as http: + served = { + lane: [t for group in http.get(f"/sd/{lane}").json() for t in group["targets"]] + for lane in ("scrape", "probe", "snmp") + } + health = http.get("/healthz").json() + ready = http.get("/readyz") + finally: + client.close() + assert ready.status_code == 200 + return served, health + + +def _one_device_per_lane() -> list[dict]: + return [ + device("node-1", ["monitoring:scrape"], address="192.0.2.11/24", metrics_port=9100), + device("encoder-1", ["monitoring:probe"], address="192.0.2.21/24", probe_module="icmp"), + device("switch-1", ["monitoring:snmp"], address="192.0.2.31/24"), + ] + + +# promsd must use the slug NetBox chose, not one it computes from the tag name. +def test_lanes_follow_the_slugs_netbox_chose(): + ui_slugs = {f"monitoring:{lane}": f"monitoring{lane}" for lane in ("scrape", "probe", "snmp")} + served, health = _serve(FakeNetbox(devices=_one_device_per_lane(), tag_slugs=ui_slugs)) + + assert served == { + "scrape": ["192.0.2.11:9100"], + "probe": ["192.0.2.21"], + "snmp": ["192.0.2.31"], + } + assert health["last_error"] is None + assert health["unavailable_tags"] == [] + + +def test_legacy_hyphen_slugs_still_work(): + served, _ = _serve(FakeNetbox(devices=_one_device_per_lane())) + + assert served == { + "scrape": ["192.0.2.11:9100"], + "probe": ["192.0.2.21"], + "snmp": ["192.0.2.31"], + } + + +# A lane tag NetBox lacks empties its lane only, and /healthz names the tag. +def test_a_missing_lane_tag_empties_only_its_lane(): + netbox = FakeNetbox(devices=_one_device_per_lane(), absent_tags=["monitoring:probe"]) + served, health = _serve(netbox) + + assert served == {"scrape": ["192.0.2.11:9100"], "probe": [], "snmp": ["192.0.2.31"]} + assert health["last_error"] is None + assert health["unavailable_tags"] == ["monitoring:probe"] + + +# NetBox's 400 for one lane's query costs the other lanes nothing. +def test_a_refused_lane_query_empties_only_its_lane(): + netbox = FakeNetbox(devices=_one_device_per_lane()) + netbox.refused_slugs.add("monitoring-snmp") + served, health = _serve(netbox) + + assert served == {"scrape": ["192.0.2.11:9100"], "probe": ["192.0.2.21"], "snmp": []} + assert health["last_error"] is None + assert health["unavailable_tags"] == ["monitoring:snmp"] diff --git a/tests/test_netbox.py b/tests/test_netbox.py index a6368d3..c62686c 100644 --- a/tests/test_netbox.py +++ b/tests/test_netbox.py @@ -36,11 +36,12 @@ def handler(request: httpx.Request) -> httpx.Response: ) try: - results = client.list_tagged("services", ("monitoring:scrape",)) + results, rejected = client.list_tagged("services", ("monitoring-scrape",)) finally: client.close() assert [item["id"] for item in results] == [1, 2] + assert rejected == [] assert len(requests) == 2 - # NetBox ?tag= filters by slug (colon -> hyphen), not the tag name. + # NetBox ?tag= filters by slug, not the tag name. assert requests[0].url.params.get("tag") == "monitoring-scrape" From 905a3329f5ac7026b70d16e3b8f6db00162b0994 Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:17:07 +0100 Subject: [PATCH 2/5] fix(promsd): fail the refresh on any record query error The previous commit treated every 400 on a record query as a lane tag NetBox refused. A malformed request or a 400 on a later page then replaced the last good snapshot with a partial one and cleared last_error. Lane tags are already resolved by exact name before any record query, so a missing tag is known at the lookup. A lane whose tag NetBox doesn't have is empty and /healthz lists the tag in missing_tags. No query names it. Any error on a record query fails the refresh, keeps the last good snapshot and sets last_error. Refs dmfdeploy/dmfdeploy#795 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- README.md | 4 +- src/dmf_promsd/cache.py | 15 +++--- src/dmf_promsd/main.py | 2 +- src/dmf_promsd/netbox.py | 34 +++--------- tests/netbox45.py | 36 ++++++++++--- tests/test_main.py | 112 ++++++++++++++++++++++++++++++--------- tests/test_netbox.py | 3 +- 7 files changed, 134 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 73a6e65..0dfa040 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Part of [dmfdeploy](https://github.com/dmfdeploy/dmfdeploy), an independent open The dmfdeploy platform keeps NetBox as its source of truth for inventory. `dmf-promsd` reads NetBox services, devices and virtual machines, and serves the ones that carry a monitoring tag as Prometheus [HTTP service discovery](https://prometheus.io/docs/prometheus/latest/http_sd/) target groups. Prometheus then scrapes or probes new workloads without anyone editing its configuration. -The service looks each lane tag up in NetBox by name and filters by the slug NetBox returns, so a tag created in the NetBox UI works with whatever slug NetBox chose. If a lane tag doesn't exist in NetBox, or NetBox refuses to filter by it, that lane serves no targets from it, the other lanes keep working, and `unavailable_tags` in `/healthz` lists the tag. +The service looks each lane tag up in NetBox by name and filters by the slug NetBox returns, so a tag created in the NetBox UI works with whatever slug NetBox chose. If a lane tag doesn't exist in NetBox, that lane is empty, the other lanes keep working, and `missing_tags` in `/healthz` lists the tag. Any error on a record query fails the refresh, and the service keeps serving the last good snapshot. A record joins a lane when it has the matching tag in NetBox and the lane reads that record type: @@ -24,7 +24,7 @@ A record with the `cluster_service` and `cluster_namespace` custom fields uses t The service skips a record in a lane when it lacks a required field: `metrics_port` for scrape, `probe_module` for probe, or an address for any lane. It keeps the skip counts in memory and does not return them from `/healthz`. -`GET /healthz` returns the status, product name and version, the cache settings (`cache_ttl_seconds`, `http_sd_refresh` and `freshness_window_seconds`), `snapshot_age_seconds`, `source_counts` (the number of services, devices and virtual machines fetched from NetBox), `unavailable_tags` and `last_error`. +`GET /healthz` returns the status, product name and version, the cache settings (`cache_ttl_seconds`, `http_sd_refresh` and `freshness_window_seconds`), `snapshot_age_seconds`, `source_counts` (the number of services, devices and virtual machines fetched from NetBox), `missing_tags` (the lane tags NetBox doesn't have) and `last_error`. ## Run it diff --git a/src/dmf_promsd/cache.py b/src/dmf_promsd/cache.py index 009623b..0e51298 100644 --- a/src/dmf_promsd/cache.py +++ b/src/dmf_promsd/cache.py @@ -17,8 +17,8 @@ class CacheSnapshot: lanes: dict[str, list[dict[str, Any]]] skipped: dict[str, dict[str, int]] source_counts: dict[str, int] - # Lane tags NetBox has no tag for, or refused to filter by. Their lanes serve fewer records. - unavailable_tags: list[str] + # Lane tags NetBox doesn't have. No record can carry one, so its lane is empty. + missing_tags: list[str] @dataclass(frozen=True) @@ -73,15 +73,14 @@ def refresh(self) -> bool: records: list[Record] = [] source_counts: dict[str, int] = {} slugs = self._client.tag_slugs(lane.tag for lane in LANES) - unavailable = {lane.tag for lane in LANES} - slugs.keys() + missing = {lane.tag for lane in LANES} - slugs.keys() for kind in RECORD_KINDS: kind_slugs = [ slugs[lane.tag] for lane in LANES if kind in lane.kinds and lane.tag in slugs ] - listing = self._client.list_records(kind, kind_slugs) - records.extend(listing.records) - source_counts[kind] = len(listing.records) + listing.unreadable - unavailable |= {tag for tag, slug in slugs.items() if slug in listing.rejected} + kind_records, unreadable = self._client.list_records(kind, kind_slugs) + records.extend(kind_records) + source_counts[kind] = len(kind_records) + unreadable payloads, skipped = build_payloads(records) except Exception as exc: # noqa: BLE001, a failed refresh keeps the last snapshot with self._lock: @@ -93,7 +92,7 @@ def refresh(self) -> bool: lanes=payloads, skipped=skipped, source_counts=source_counts, - unavailable_tags=sorted(unavailable), + missing_tags=sorted(missing), ) with self._lock: self._snapshot = snapshot diff --git a/src/dmf_promsd/main.py b/src/dmf_promsd/main.py index 79b361d..8dd6001 100644 --- a/src/dmf_promsd/main.py +++ b/src/dmf_promsd/main.py @@ -25,7 +25,7 @@ def _health_payload(cache: PromSDCache, refresh_hint: str) -> dict[str, Any]: "freshness_window_seconds": cache.freshness_window_seconds, "snapshot_age_seconds": state.age_seconds, "source_counts": state.snapshot.source_counts if state.snapshot else {}, - "unavailable_tags": state.snapshot.unavailable_tags if state.snapshot else [], + "missing_tags": state.snapshot.missing_tags if state.snapshot else [], "last_error": state.last_error, } diff --git a/src/dmf_promsd/netbox.py b/src/dmf_promsd/netbox.py index befc0c6..2e95f32 100644 --- a/src/dmf_promsd/netbox.py +++ b/src/dmf_promsd/netbox.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Iterable, Mapping -from typing import Any, NamedTuple +from typing import Any import httpx @@ -47,13 +47,6 @@ def __init__(self, status_code: int, body: str) -> None: super().__init__(f"NetBox API {status_code}: {body}") -class Listing(NamedTuple): - records: list[Record] - unreadable: int - # Tag slugs whose query NetBox refused. Their records are missing from `records`. - rejected: list[str] - - class NetboxClient: def __init__( self, @@ -113,35 +106,22 @@ def tag_slugs(self, names: Iterable[str]) -> dict[str, str]: found = {tag["name"]: tag["slug"] for tag in self._list_paginated(_TAGS_PATH, params)} return {name: found[name] for name in wanted if name in found} - def list_tagged( - self, kind: str, slugs: Iterable[str] - ) -> tuple[list[dict[str, Any]], list[str]]: + def list_tagged(self, kind: str, slugs: Iterable[str]) -> list[dict[str, Any]]: path = _LIST_PATHS[kind] # NetBox ?tag= filters are ANDed; query each tag separately and union by # id so an object tagged with ANY of the tags is included (OR). merged: dict[Any, dict[str, Any]] = {} - rejected: list[str] = [] for slug in slugs: - try: - objects = self._list_paginated(path, [("limit", "1000"), ("tag", slug)]) - except NetboxAPIError as exc: - # A 400 is about this query alone. Any other error means NetBox is - # unwell, and the caller keeps the last snapshot. - if exc.status_code != 400: - raise - rejected.append(slug) - continue - for obj in objects: + for obj in self._list_paginated(path, [("limit", "1000"), ("tag", slug)]): merged[obj.get("id", id(obj))] = obj - return list(merged.values()), rejected + return list(merged.values()) - def list_records(self, kind: str, slugs: Iterable[str]) -> Listing: + def list_records(self, kind: str, slugs: Iterable[str]) -> tuple[list[Record], int]: records: list[Record] = [] unreadable = 0 - objects, rejected = self.list_tagged(kind, slugs) - for raw in objects: + for raw in self.list_tagged(kind, slugs): try: records.append(parse_record(kind, raw)) except (KeyError, TypeError, AttributeError): unreadable += 1 - return Listing(records, unreadable, rejected) + return records, unreadable diff --git a/tests/netbox45.py b/tests/netbox45.py index 787f667..9e03e09 100644 --- a/tests/netbox45.py +++ b/tests/netbox45.py @@ -11,7 +11,7 @@ import itertools import re from collections import defaultdict -from collections.abc import Iterable, Iterator +from collections.abc import Callable, Iterable, Iterator from typing import Any import httpx @@ -232,13 +232,23 @@ def __init__( for record_tag in record["tags"]: record_tag["slug"] = overrides.get(record_tag["name"], record_tag["slug"]) self._tags[record_tag["name"]] = record_tag["slug"] - for name in absent_tags: - del self._tags[name] self._slugs = set(self._tags.values()) - # Slugs NetBox knows but answers 400 for on a list endpoint. - self.refused_slugs: set[str] = set() + # Answers 400 to a list request this returns true for, such as one for a later page. + self.refuse: Callable[[httpx.Request], bool] = lambda request: False + # Page size of list endpoints; None puts every match in one page. + self.page_size: int | None = None self.requests: list[httpx.Request] = [] self.down = False + for name in absent_tags: + self.remove_tag(name) + + def remove_tag(self, name: str) -> None: + """Delete a tag. NetBox strips it from every object that carried it.""" + self._tags.pop(name, None) + self._slugs = set(self._tags.values()) + for records in self._records.values(): + for record in records: + record["tags"] = [t for t in record["tags"] if t["name"] != name] def client(self, token: str = TOKEN) -> NetboxClient: return NetboxClient(base_url=BASE_URL, token=token, transport=httpx.MockTransport(self)) @@ -262,15 +272,27 @@ def __call__(self, request: httpx.Request) -> httpx.Response: records = self._records.get(request.url.path) if records is None: return httpx.Response(404, json={"detail": "Not found."}) + if self.refuse(request): + return httpx.Response(400, json={"detail": "Bad request."}) slugs = request.url.params.get_list("tag") - unknown = [slug for slug in slugs if slug not in self._slugs or slug in self.refused_slugs] + unknown = [slug for slug in slugs if slug not in self._slugs] if unknown: choice = f"Select a valid choice. {unknown[0]} is not one of the available choices." return httpx.Response(400, json={"tag": [choice]}) matches = [ record for record in records if set(slugs) <= {tag["slug"] for tag in record["tags"]} ] + offset = int(request.url.params.get("offset", 0)) + size = self.page_size or max(len(matches), 1) + following = None + if offset + size < len(matches): + following = str(request.url.copy_set_param("offset", offset + size)) return httpx.Response( 200, - json={"count": len(matches), "next": None, "previous": None, "results": matches}, + json={ + "count": len(matches), + "next": following, + "previous": None, + "results": matches[offset : offset + size], + }, ) diff --git a/tests/test_main.py b/tests/test_main.py index f5b11e9..9e104a2 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -7,6 +7,7 @@ from fastapi.testclient import TestClient from dmf_promsd import __version__ +from dmf_promsd.cache import PromSDCache from dmf_promsd.main import create_app from dmf_promsd.settings import Settings from tests.netbox45 import BASE_URL, TOKEN, FakeNetbox, device, service, virtual_machine @@ -56,7 +57,7 @@ def test_lifespan_refreshes_on_startup_and_stops_on_shutdown(): "http_sd_refresh": "11s", "freshness_window_seconds": 14, "source_counts": {"services": 0, "devices": 1, "virtual_machines": 0}, - "unavailable_tags": [], + "missing_tags": [], "last_error": None, } assert ready.status_code == 200 @@ -235,15 +236,19 @@ def test_a_record_promsd_cannot_read_costs_the_others_nothing(): assert health["last_error"] is None +def _lanes(http: TestClient) -> dict[str, list[str]]: + return { + lane: [t for group in http.get(f"/sd/{lane}").json() for t in group["targets"]] + for lane in ("scrape", "probe", "snmp") + } + + def _serve(netbox: FakeNetbox) -> tuple[dict[str, list[str]], dict]: client = netbox.client() app = create_app(Settings(netbox_url=BASE_URL, netbox_token=TOKEN), client=client) try: with TestClient(app) as http: - served = { - lane: [t for group in http.get(f"/sd/{lane}").json() for t in group["targets"]] - for lane in ("scrape", "probe", "snmp") - } + served = _lanes(http) health = http.get("/healthz").json() ready = http.get("/readyz") finally: @@ -260,28 +265,23 @@ def _one_device_per_lane() -> list[dict]: ] +ALL_LANES = {"scrape": ["192.0.2.11:9100"], "probe": ["192.0.2.21"], "snmp": ["192.0.2.31"]} + + # promsd must use the slug NetBox chose, not one it computes from the tag name. def test_lanes_follow_the_slugs_netbox_chose(): ui_slugs = {f"monitoring:{lane}": f"monitoring{lane}" for lane in ("scrape", "probe", "snmp")} served, health = _serve(FakeNetbox(devices=_one_device_per_lane(), tag_slugs=ui_slugs)) - assert served == { - "scrape": ["192.0.2.11:9100"], - "probe": ["192.0.2.21"], - "snmp": ["192.0.2.31"], - } + assert served == ALL_LANES assert health["last_error"] is None - assert health["unavailable_tags"] == [] + assert health["missing_tags"] == [] def test_legacy_hyphen_slugs_still_work(): served, _ = _serve(FakeNetbox(devices=_one_device_per_lane())) - assert served == { - "scrape": ["192.0.2.11:9100"], - "probe": ["192.0.2.21"], - "snmp": ["192.0.2.31"], - } + assert served == ALL_LANES # A lane tag NetBox lacks empties its lane only, and /healthz names the tag. @@ -289,17 +289,79 @@ def test_a_missing_lane_tag_empties_only_its_lane(): netbox = FakeNetbox(devices=_one_device_per_lane(), absent_tags=["monitoring:probe"]) served, health = _serve(netbox) - assert served == {"scrape": ["192.0.2.11:9100"], "probe": [], "snmp": ["192.0.2.31"]} + assert served == {**ALL_LANES, "probe": []} assert health["last_error"] is None - assert health["unavailable_tags"] == ["monitoring:probe"] + assert health["missing_tags"] == ["monitoring:probe"] + # No record query names a tag NetBox doesn't have. + queried = {r.url.params["tag"] for r in netbox.requests if "tag" in r.url.params} + assert queried == {"monitoring-scrape", "monitoring-snmp"} + + +# A device that carries two lane tags still serves only the lane whose tag exists. +def test_a_device_with_two_lane_tags_serves_only_the_existing_lane(): + both = device( + "node-1", + ["monitoring:scrape", "monitoring:snmp"], + address="192.0.2.11/24", + metrics_port=9100, + ) + served, health = _serve(FakeNetbox(devices=[both], absent_tags=["monitoring:snmp"])) + + assert served == {"scrape": ["192.0.2.11:9100"], "probe": [], "snmp": []} + assert health["missing_tags"] == ["monitoring:snmp"] + + +def _refused_from_page(offset: int): + def refuse(request) -> bool: + return ( + request.url.path == "/api/dcim/devices/" + and int(request.url.params.get("offset", 0)) >= offset + ) + return refuse -# NetBox's 400 for one lane's query costs the other lanes nothing. -def test_a_refused_lane_query_empties_only_its_lane(): + +# A 400 on a record query is not a missing tag. The refresh fails and the last snapshot stays. +def test_a_refused_record_query_keeps_the_last_snapshot(): + for first_bad_offset in (0, 1): + netbox = FakeNetbox(devices=[*_one_device_per_lane(), _extra_scrape_device()]) + netbox.page_size = 1 + client = netbox.client() + cache = PromSDCache(client=client, ttl_seconds=60) + try: + assert cache.refresh() + good = cache.get_snapshot() + assert good is not None + + netbox.refuse = _refused_from_page(first_bad_offset) + assert not cache.refresh() + + state = cache.state() + assert state.snapshot is good + assert state.last_error is not None and "NetBox API 400" in state.last_error + finally: + client.close() + + +def _extra_scrape_device() -> dict: + return device("node-2", ["monitoring:scrape"], address="192.0.2.12/24", metrics_port=9100) + + +# A tag deleted after a good snapshot empties its lane on the next refresh. +def test_a_tag_removed_after_a_good_snapshot_empties_its_lane(): netbox = FakeNetbox(devices=_one_device_per_lane()) - netbox.refused_slugs.add("monitoring-snmp") - served, health = _serve(netbox) + client = netbox.client() + cache = PromSDCache(client=client, ttl_seconds=60) + try: + assert cache.refresh() + assert [g["targets"] for g in cache.lane_payload("snmp")] == [["192.0.2.31"]] - assert served == {"scrape": ["192.0.2.11:9100"], "probe": ["192.0.2.21"], "snmp": []} - assert health["last_error"] is None - assert health["unavailable_tags"] == ["monitoring:snmp"] + netbox.remove_tag("monitoring:snmp") + assert cache.refresh() + + assert cache.lane_payload("snmp") == [] + assert [g["targets"] for g in cache.lane_payload("scrape")] == [["192.0.2.11:9100"]] + snapshot = cache.get_snapshot() + assert snapshot is not None and snapshot.missing_tags == ["monitoring:snmp"] + finally: + client.close() diff --git a/tests/test_netbox.py b/tests/test_netbox.py index c62686c..b8f2b54 100644 --- a/tests/test_netbox.py +++ b/tests/test_netbox.py @@ -36,12 +36,11 @@ def handler(request: httpx.Request) -> httpx.Response: ) try: - results, rejected = client.list_tagged("services", ("monitoring-scrape",)) + results = client.list_tagged("services", ("monitoring-scrape",)) finally: client.close() assert [item["id"] for item in results] == [1, 2] - assert rejected == [] assert len(requests) == 2 # NetBox ?tag= filters by slug, not the tag name. assert requests[0].url.params.get("tag") == "monitoring-scrape" From ca4da6053b3d1716d456a5070d44fef2640ce909 Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:20:27 +0100 Subject: [PATCH 3/5] fix(promsd): keep a missing lane empty when its tag appears mid-refresh The tag lookup and the record queries are separate requests. A tag created and attached between them let the records carry a tag whose lane the lookup had marked missing, so that lane served a partial set. build_payloads now takes the missing tags and gives those lanes an empty payload and no skip counts. The next refresh finds the tag and serves every record. Refs dmfdeploy/dmfdeploy#795 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- src/dmf_promsd/cache.py | 2 +- src/dmf_promsd/sd.py | 9 +++++++-- tests/netbox45.py | 15 ++++++++++++++- tests/test_main.py | 29 +++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/dmf_promsd/cache.py b/src/dmf_promsd/cache.py index 0e51298..a54887d 100644 --- a/src/dmf_promsd/cache.py +++ b/src/dmf_promsd/cache.py @@ -81,7 +81,7 @@ def refresh(self) -> bool: kind_records, unreadable = self._client.list_records(kind, kind_slugs) records.extend(kind_records) source_counts[kind] = len(kind_records) + unreadable - payloads, skipped = build_payloads(records) + payloads, skipped = build_payloads(records, missing) except Exception as exc: # noqa: BLE001, a failed refresh keeps the last snapshot with self._lock: self._last_error = str(exc) diff --git a/src/dmf_promsd/sd.py b/src/dmf_promsd/sd.py index b9aa246..faeccfa 100644 --- a/src/dmf_promsd/sd.py +++ b/src/dmf_promsd/sd.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections import Counter -from collections.abc import Callable, Iterable, Mapping +from collections.abc import Callable, Collection, Iterable, Mapping from dataclasses import dataclass from typing import Any @@ -175,11 +175,16 @@ def _build_lane( def build_payloads( - records: Iterable[Record], + records: Iterable[Record], missing_tags: Collection[str] = () ) -> tuple[dict[str, list[dict[str, Any]]], dict[str, dict[str, int]]]: records = list(records) payloads: dict[str, list[dict[str, Any]]] = {} skipped: dict[str, dict[str, int]] = {} for lane in LANES: + if lane.tag in missing_tags: + # The tag can reach records after the lookup that found it missing. The lane + # stays empty until a refresh finds the tag, so it never serves a partial set. + payloads[lane.name], skipped[lane.name] = [], {} + continue payloads[lane.name], skipped[lane.name] = _build_lane(records, lane) return payloads, skipped diff --git a/tests/netbox45.py b/tests/netbox45.py index 9e03e09..fbf4cdb 100644 --- a/tests/netbox45.py +++ b/tests/netbox45.py @@ -237,11 +237,20 @@ def __init__( self.refuse: Callable[[httpx.Request], bool] = lambda request: False # Page size of list endpoints; None puts every match in one page. self.page_size: int | None = None + # Runs once after the next tag lookup is answered, to change NetBox mid-refresh. + self.after_tag_lookup: Callable[[], None] | None = None self.requests: list[httpx.Request] = [] self.down = False for name in absent_tags: self.remove_tag(name) + def attach_tag(self, name: str, records: Iterable[dict[str, Any]]) -> None: + """Create a tag if needed and put it on the given records.""" + slug = self._tags.setdefault(name, _slug(name)) + self._slugs.add(slug) + for record in records: + record["tags"].append({**tag(name), "slug": slug}) + def remove_tag(self, name: str) -> None: """Delete a tag. NetBox strips it from every object that carried it.""" self._tags.pop(name, None) @@ -266,9 +275,13 @@ def __call__(self, request: httpx.Request) -> httpx.Response: for i, (name, slug) in enumerate(self._tags.items(), 1) if name in names ] - return httpx.Response( + response = httpx.Response( 200, json={"count": len(found), "next": None, "previous": None, "results": found} ) + if self.after_tag_lookup is not None: + after, self.after_tag_lookup = self.after_tag_lookup, None + after() + return response records = self._records.get(request.url.path) if records is None: return httpx.Response(404, json={"detail": "Not found."}) diff --git a/tests/test_main.py b/tests/test_main.py index 9e104a2..6a727fb 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -365,3 +365,32 @@ def test_a_tag_removed_after_a_good_snapshot_empties_its_lane(): assert snapshot is not None and snapshot.missing_tags == ["monitoring:snmp"] finally: client.close() + + +# The tag can appear between the lookup that misses it and the record queries. The lane +# stays empty for that refresh, and the next refresh serves every record. +def test_a_tag_created_mid_refresh_does_not_fill_its_lane_partially(): + tags = ["monitoring:scrape", "monitoring:snmp"] + both = device("node-1", tags, address="192.0.2.11/24", metrics_port=9100) + scrape_only = device( + "switch-1", ["monitoring:scrape"], address="192.0.2.31/24", metrics_port=9100 + ) + netbox = FakeNetbox(devices=[both, scrape_only], absent_tags=["monitoring:snmp"]) + netbox.after_tag_lookup = lambda: netbox.attach_tag("monitoring:snmp", [both, scrape_only]) + client = netbox.client() + cache = PromSDCache(client=client, ttl_seconds=60) + try: + assert cache.refresh() + snapshot = cache.get_snapshot() + assert snapshot is not None + assert snapshot.lanes["snmp"] == [] + assert snapshot.skipped["snmp"] == {} + assert snapshot.missing_tags == ["monitoring:snmp"] + assert len(snapshot.lanes["scrape"]) == 2 + + assert cache.refresh() + targets = [g["targets"] for g in cache.lane_payload("snmp")] + assert sorted(targets) == [["192.0.2.11"], ["192.0.2.31"]] + assert cache.get_snapshot().missing_tags == [] + finally: + client.close() From 3e6dccce6ad4a391dcae019d27242064c595993a Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:59:07 +0100 Subject: [PATCH 4/5] fix(promsd): fail the refresh when no lane tag is visible NetBox hides objects a token may not view, so a token without extras.view_tag sees an empty tag list. That looked like "no lane tags exist", so promsd published empty lanes, cleared last_error and stayed ready. The refresh now fails when none of the lane tags is visible, keeps the last good snapshot, and names the missing permission in last_error. The README now lists tag view access among the token permissions. Refs dmfdeploy/dmfdeploy#795 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- README.md | 2 +- src/dmf_promsd/cache.py | 8 +++++ tests/test_tag_visibility.py | 62 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 tests/test_tag_visibility.py diff --git a/README.md b/README.md index 0dfa040..9632f6a 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ The service skips a record in a lane when it lacks a required field: `metrics_po ## Run it -You need Python 3.12 or later, and a NetBox instance with an API token that can read services, devices and virtual machines. +You need Python 3.12 or later, and a NetBox instance with an API token with view access to services, devices, virtual machines and tags (`extras | tag | view`). NetBox hides objects a token can't view, so without tag access promsd sees no lane tags. In that case the refresh fails, the service keeps the last good lanes, and `last_error` reports that no lane tag is visible and the token may lack `extras.view_tag`. The service reports not ready until a refresh succeeds or when the last good lanes go stale. Supply the NetBox address and token through environment variables. The service refuses to start without `NETBOX_URL` and `NETBOX_TOKEN`. Set the token from your secret store at run time and keep it out of files in this repository. diff --git a/src/dmf_promsd/cache.py b/src/dmf_promsd/cache.py index a54887d..2a0f963 100644 --- a/src/dmf_promsd/cache.py +++ b/src/dmf_promsd/cache.py @@ -74,6 +74,14 @@ def refresh(self) -> bool: source_counts: dict[str, int] = {} slugs = self._client.tag_slugs(lane.tag for lane in LANES) missing = {lane.tag for lane in LANES} - slugs.keys() + if not slugs: + # NetBox hides objects the token may not view, so a token without tag + # access sees no tags at all. Publishing empty lanes would pass for + # "no lane tags exist", so fail the refresh and keep the last snapshot. + raise RuntimeError( + "No lane tag is visible in NetBox. The token may lack permission " + "to view tags (extras.view_tag)." + ) for kind in RECORD_KINDS: kind_slugs = [ slugs[lane.tag] for lane in LANES if kind in lane.kinds and lane.tag in slugs diff --git a/tests/test_tag_visibility.py b/tests/test_tag_visibility.py new file mode 100644 index 0000000..4a87544 --- /dev/null +++ b/tests/test_tag_visibility.py @@ -0,0 +1,62 @@ +from __future__ import annotations + +from dmf_promsd.cache import PromSDCache +from tests.netbox45 import FakeNetbox, device + +LANE_TAGS = ["monitoring:scrape", "monitoring:probe", "monitoring:snmp"] + + +def _devices() -> list[dict]: + return [ + device("node-1", ["monitoring:scrape"], address="192.0.2.11/24", metrics_port=9100), + device("switch-1", ["monitoring:snmp"], address="192.0.2.31/24"), + ] + + +# A token without tag-view access sees an empty tag list, the same as a NetBox with no +# lane tags. The refresh fails so the lanes don't go empty without a signal. +def test_no_visible_lane_tag_fails_the_refresh_and_keeps_the_last_snapshot(): + netbox = FakeNetbox(devices=_devices()) + client = netbox.client() + cache = PromSDCache(client=client, ttl_seconds=60) + try: + assert cache.refresh() + good = cache.get_snapshot() + assert good is not None + + for name in LANE_TAGS: + netbox.remove_tag(name) + assert not cache.refresh() + + state = cache.state() + assert state.snapshot is good + assert state.last_error is not None + assert "No lane tag is visible" in state.last_error + assert "extras.view_tag" in state.last_error + finally: + client.close() + + +def test_no_visible_lane_tag_on_first_refresh_leaves_no_snapshot(): + client = FakeNetbox(devices=_devices(), absent_tags=LANE_TAGS).client() + cache = PromSDCache(client=client, ttl_seconds=60) + try: + assert not cache.refresh() + assert cache.get_snapshot() is None + finally: + client.close() + + +# One visible lane tag proves the token can view tags, so the others are truly missing. +def test_one_visible_lane_tag_keeps_the_missing_lane_case(): + absent = ["monitoring:probe", "monitoring:snmp"] + client = FakeNetbox(devices=_devices(), absent_tags=absent).client() + cache = PromSDCache(client=client, ttl_seconds=60) + try: + assert cache.refresh() + snapshot = cache.get_snapshot() + assert snapshot is not None + assert snapshot.missing_tags == sorted(absent) + assert cache.state().last_error is None + finally: + client.close() From 0ebf20141ef50660120f9078b8c2a868ef93e3a1 Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Fri, 6 Nov 2026 19:22:28 +0100 Subject: [PATCH 5/5] fix(cache): fail the refresh when any lane tag is unseen Refs dmfdeploy/dmfdeploy#795 NetBox can limit tag view permission to some tags, so a hidden lane tag looks the same as a missing one. Treating it as missing published an empty lane and left the service ready. Fail the refresh instead when any lane tag is not seen, keep the last snapshot, and name the unseen tags in last_error. netbox-sot creates all three lane tags, so an unseen one is a misconfiguration or a permission gap. This removes the missing-tags path: the snapshot field, the build_payloads branch and the /healthz field. Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- README.md | 6 +-- src/dmf_promsd/cache.py | 23 ++++------ src/dmf_promsd/main.py | 1 - src/dmf_promsd/sd.py | 9 +--- tests/netbox45.py | 15 +------ tests/test_main.py | 85 ++++++------------------------------ tests/test_tag_visibility.py | 48 ++++++++++++++------ 7 files changed, 63 insertions(+), 124 deletions(-) diff --git a/README.md b/README.md index 9632f6a..aa9aa01 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Part of [dmfdeploy](https://github.com/dmfdeploy/dmfdeploy), an independent open The dmfdeploy platform keeps NetBox as its source of truth for inventory. `dmf-promsd` reads NetBox services, devices and virtual machines, and serves the ones that carry a monitoring tag as Prometheus [HTTP service discovery](https://prometheus.io/docs/prometheus/latest/http_sd/) target groups. Prometheus then scrapes or probes new workloads without anyone editing its configuration. -The service looks each lane tag up in NetBox by name and filters by the slug NetBox returns, so a tag created in the NetBox UI works with whatever slug NetBox chose. If a lane tag doesn't exist in NetBox, that lane is empty, the other lanes keep working, and `missing_tags` in `/healthz` lists the tag. Any error on a record query fails the refresh, and the service keeps serving the last good snapshot. +The service looks each lane tag up in NetBox by name and filters by the slug NetBox returns, so a tag created in the NetBox UI works with whatever slug NetBox chose. Every lane tag must exist in NetBox and be visible to the token. If any lane tag is missing or hidden, the refresh fails, the service keeps serving the last good lanes, and `last_error` in `/healthz` names the tags it didn't see. Any error on a record query fails the refresh the same way. The service reports not ready once the kept lanes go stale. A record joins a lane when it has the matching tag in NetBox and the lane reads that record type: @@ -24,11 +24,11 @@ A record with the `cluster_service` and `cluster_namespace` custom fields uses t The service skips a record in a lane when it lacks a required field: `metrics_port` for scrape, `probe_module` for probe, or an address for any lane. It keeps the skip counts in memory and does not return them from `/healthz`. -`GET /healthz` returns the status, product name and version, the cache settings (`cache_ttl_seconds`, `http_sd_refresh` and `freshness_window_seconds`), `snapshot_age_seconds`, `source_counts` (the number of services, devices and virtual machines fetched from NetBox), `missing_tags` (the lane tags NetBox doesn't have) and `last_error`. +`GET /healthz` returns the status, product name and version, the cache settings (`cache_ttl_seconds`, `http_sd_refresh` and `freshness_window_seconds`), `snapshot_age_seconds`, `source_counts` (the number of services, devices and virtual machines fetched from NetBox) and `last_error`. ## Run it -You need Python 3.12 or later, and a NetBox instance with an API token with view access to services, devices, virtual machines and tags (`extras | tag | view`). NetBox hides objects a token can't view, so without tag access promsd sees no lane tags. In that case the refresh fails, the service keeps the last good lanes, and `last_error` reports that no lane tag is visible and the token may lack `extras.view_tag`. The service reports not ready until a refresh succeeds or when the last good lanes go stale. +You need Python 3.12 or later, and a NetBox instance with an API token with view access to services, devices, virtual machines and all three lane tags (`extras | tag | view`). NetBox hides objects a token can't view, so promsd can't tell a hidden lane tag from a missing one. In either case the refresh fails, the service keeps the last good lanes, and `last_error` names the tags it didn't see and mentions `extras.view_tag`. The service reports not ready until a refresh succeeds, or once the last good lanes go stale. Supply the NetBox address and token through environment variables. The service refuses to start without `NETBOX_URL` and `NETBOX_TOKEN`. Set the token from your secret store at run time and keep it out of files in this repository. diff --git a/src/dmf_promsd/cache.py b/src/dmf_promsd/cache.py index 2a0f963..a9b3bcf 100644 --- a/src/dmf_promsd/cache.py +++ b/src/dmf_promsd/cache.py @@ -17,8 +17,6 @@ class CacheSnapshot: lanes: dict[str, list[dict[str, Any]]] skipped: dict[str, dict[str, int]] source_counts: dict[str, int] - # Lane tags NetBox doesn't have. No record can carry one, so its lane is empty. - missing_tags: list[str] @dataclass(frozen=True) @@ -73,23 +71,21 @@ def refresh(self) -> bool: records: list[Record] = [] source_counts: dict[str, int] = {} slugs = self._client.tag_slugs(lane.tag for lane in LANES) - missing = {lane.tag for lane in LANES} - slugs.keys() - if not slugs: - # NetBox hides objects the token may not view, so a token without tag - # access sees no tags at all. Publishing empty lanes would pass for - # "no lane tags exist", so fail the refresh and keep the last snapshot. + unseen = sorted({lane.tag for lane in LANES} - slugs.keys()) + if unseen: + # NetBox hides objects the token may not view, so a hidden lane tag looks + # the same as a missing one. Publishing an empty lane would pass for "no + # records", so fail the refresh and keep the last snapshot. raise RuntimeError( - "No lane tag is visible in NetBox. The token may lack permission " - "to view tags (extras.view_tag)." + f"Lane tags not seen in NetBox: {', '.join(unseen)}. Each is either " + "missing from NetBox or not visible to the token (extras.view_tag)." ) for kind in RECORD_KINDS: - kind_slugs = [ - slugs[lane.tag] for lane in LANES if kind in lane.kinds and lane.tag in slugs - ] + kind_slugs = [slugs[lane.tag] for lane in LANES if kind in lane.kinds] kind_records, unreadable = self._client.list_records(kind, kind_slugs) records.extend(kind_records) source_counts[kind] = len(kind_records) + unreadable - payloads, skipped = build_payloads(records, missing) + payloads, skipped = build_payloads(records) except Exception as exc: # noqa: BLE001, a failed refresh keeps the last snapshot with self._lock: self._last_error = str(exc) @@ -100,7 +96,6 @@ def refresh(self) -> bool: lanes=payloads, skipped=skipped, source_counts=source_counts, - missing_tags=sorted(missing), ) with self._lock: self._snapshot = snapshot diff --git a/src/dmf_promsd/main.py b/src/dmf_promsd/main.py index 8dd6001..b3f84c7 100644 --- a/src/dmf_promsd/main.py +++ b/src/dmf_promsd/main.py @@ -25,7 +25,6 @@ def _health_payload(cache: PromSDCache, refresh_hint: str) -> dict[str, Any]: "freshness_window_seconds": cache.freshness_window_seconds, "snapshot_age_seconds": state.age_seconds, "source_counts": state.snapshot.source_counts if state.snapshot else {}, - "missing_tags": state.snapshot.missing_tags if state.snapshot else [], "last_error": state.last_error, } diff --git a/src/dmf_promsd/sd.py b/src/dmf_promsd/sd.py index faeccfa..b9aa246 100644 --- a/src/dmf_promsd/sd.py +++ b/src/dmf_promsd/sd.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections import Counter -from collections.abc import Callable, Collection, Iterable, Mapping +from collections.abc import Callable, Iterable, Mapping from dataclasses import dataclass from typing import Any @@ -175,16 +175,11 @@ def _build_lane( def build_payloads( - records: Iterable[Record], missing_tags: Collection[str] = () + records: Iterable[Record], ) -> tuple[dict[str, list[dict[str, Any]]], dict[str, dict[str, int]]]: records = list(records) payloads: dict[str, list[dict[str, Any]]] = {} skipped: dict[str, dict[str, int]] = {} for lane in LANES: - if lane.tag in missing_tags: - # The tag can reach records after the lookup that found it missing. The lane - # stays empty until a refresh finds the tag, so it never serves a partial set. - payloads[lane.name], skipped[lane.name] = [], {} - continue payloads[lane.name], skipped[lane.name] = _build_lane(records, lane) return payloads, skipped diff --git a/tests/netbox45.py b/tests/netbox45.py index fbf4cdb..9e03e09 100644 --- a/tests/netbox45.py +++ b/tests/netbox45.py @@ -237,20 +237,11 @@ def __init__( self.refuse: Callable[[httpx.Request], bool] = lambda request: False # Page size of list endpoints; None puts every match in one page. self.page_size: int | None = None - # Runs once after the next tag lookup is answered, to change NetBox mid-refresh. - self.after_tag_lookup: Callable[[], None] | None = None self.requests: list[httpx.Request] = [] self.down = False for name in absent_tags: self.remove_tag(name) - def attach_tag(self, name: str, records: Iterable[dict[str, Any]]) -> None: - """Create a tag if needed and put it on the given records.""" - slug = self._tags.setdefault(name, _slug(name)) - self._slugs.add(slug) - for record in records: - record["tags"].append({**tag(name), "slug": slug}) - def remove_tag(self, name: str) -> None: """Delete a tag. NetBox strips it from every object that carried it.""" self._tags.pop(name, None) @@ -275,13 +266,9 @@ def __call__(self, request: httpx.Request) -> httpx.Response: for i, (name, slug) in enumerate(self._tags.items(), 1) if name in names ] - response = httpx.Response( + return httpx.Response( 200, json={"count": len(found), "next": None, "previous": None, "results": found} ) - if self.after_tag_lookup is not None: - after, self.after_tag_lookup = self.after_tag_lookup, None - after() - return response records = self._records.get(request.url.path) if records is None: return httpx.Response(404, json={"detail": "Not found."}) diff --git a/tests/test_main.py b/tests/test_main.py index 6a727fb..7c97f11 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -57,7 +57,6 @@ def test_lifespan_refreshes_on_startup_and_stops_on_shutdown(): "http_sd_refresh": "11s", "freshness_window_seconds": 14, "source_counts": {"services": 0, "devices": 1, "virtual_machines": 0}, - "missing_tags": [], "last_error": None, } assert ready.status_code == 200 @@ -275,7 +274,6 @@ def test_lanes_follow_the_slugs_netbox_chose(): assert served == ALL_LANES assert health["last_error"] is None - assert health["missing_tags"] == [] def test_legacy_hyphen_slugs_still_work(): @@ -284,33 +282,6 @@ def test_legacy_hyphen_slugs_still_work(): assert served == ALL_LANES -# A lane tag NetBox lacks empties its lane only, and /healthz names the tag. -def test_a_missing_lane_tag_empties_only_its_lane(): - netbox = FakeNetbox(devices=_one_device_per_lane(), absent_tags=["monitoring:probe"]) - served, health = _serve(netbox) - - assert served == {**ALL_LANES, "probe": []} - assert health["last_error"] is None - assert health["missing_tags"] == ["monitoring:probe"] - # No record query names a tag NetBox doesn't have. - queried = {r.url.params["tag"] for r in netbox.requests if "tag" in r.url.params} - assert queried == {"monitoring-scrape", "monitoring-snmp"} - - -# A device that carries two lane tags still serves only the lane whose tag exists. -def test_a_device_with_two_lane_tags_serves_only_the_existing_lane(): - both = device( - "node-1", - ["monitoring:scrape", "monitoring:snmp"], - address="192.0.2.11/24", - metrics_port=9100, - ) - served, health = _serve(FakeNetbox(devices=[both], absent_tags=["monitoring:snmp"])) - - assert served == {"scrape": ["192.0.2.11:9100"], "probe": [], "snmp": []} - assert health["missing_tags"] == ["monitoring:snmp"] - - def _refused_from_page(offset: int): def refuse(request) -> bool: return ( @@ -321,7 +292,7 @@ def refuse(request) -> bool: return refuse -# A 400 on a record query is not a missing tag. The refresh fails and the last snapshot stays. +# A 400 on a record query fails the refresh and the last snapshot stays. def test_a_refused_record_query_keeps_the_last_snapshot(): for first_bad_offset in (0, 1): netbox = FakeNetbox(devices=[*_one_device_per_lane(), _extra_scrape_device()]) @@ -347,50 +318,20 @@ def _extra_scrape_device() -> dict: return device("node-2", ["monitoring:scrape"], address="192.0.2.12/24", metrics_port=9100) -# A tag deleted after a good snapshot empties its lane on the next refresh. -def test_a_tag_removed_after_a_good_snapshot_empties_its_lane(): - netbox = FakeNetbox(devices=_one_device_per_lane()) +# With a lane tag unseen on the first refresh there is no snapshot to serve, so the +# service is not ready and says which tags it didn't see. +def test_a_missing_lane_tag_on_the_first_refresh_leaves_the_service_not_ready(): + netbox = FakeNetbox(devices=_one_device_per_lane(), absent_tags=["monitoring:probe"]) client = netbox.client() - cache = PromSDCache(client=client, ttl_seconds=60) + app = create_app(Settings(netbox_url=BASE_URL, netbox_token=TOKEN), client=client) try: - assert cache.refresh() - assert [g["targets"] for g in cache.lane_payload("snmp")] == [["192.0.2.31"]] - - netbox.remove_tag("monitoring:snmp") - assert cache.refresh() - - assert cache.lane_payload("snmp") == [] - assert [g["targets"] for g in cache.lane_payload("scrape")] == [["192.0.2.11:9100"]] - snapshot = cache.get_snapshot() - assert snapshot is not None and snapshot.missing_tags == ["monitoring:snmp"] + with TestClient(app) as http: + ready = http.get("/readyz") + health = http.get("/healthz").json() finally: client.close() - -# The tag can appear between the lookup that misses it and the record queries. The lane -# stays empty for that refresh, and the next refresh serves every record. -def test_a_tag_created_mid_refresh_does_not_fill_its_lane_partially(): - tags = ["monitoring:scrape", "monitoring:snmp"] - both = device("node-1", tags, address="192.0.2.11/24", metrics_port=9100) - scrape_only = device( - "switch-1", ["monitoring:scrape"], address="192.0.2.31/24", metrics_port=9100 - ) - netbox = FakeNetbox(devices=[both, scrape_only], absent_tags=["monitoring:snmp"]) - netbox.after_tag_lookup = lambda: netbox.attach_tag("monitoring:snmp", [both, scrape_only]) - client = netbox.client() - cache = PromSDCache(client=client, ttl_seconds=60) - try: - assert cache.refresh() - snapshot = cache.get_snapshot() - assert snapshot is not None - assert snapshot.lanes["snmp"] == [] - assert snapshot.skipped["snmp"] == {} - assert snapshot.missing_tags == ["monitoring:snmp"] - assert len(snapshot.lanes["scrape"]) == 2 - - assert cache.refresh() - targets = [g["targets"] for g in cache.lane_payload("snmp")] - assert sorted(targets) == [["192.0.2.11"], ["192.0.2.31"]] - assert cache.get_snapshot().missing_tags == [] - finally: - client.close() + assert ready.status_code == 503 + assert ready.json()["reason"] == "no snapshot" + assert "monitoring:probe" in health["last_error"] + assert "missing_tags" not in health diff --git a/tests/test_tag_visibility.py b/tests/test_tag_visibility.py index 4a87544..b1e991f 100644 --- a/tests/test_tag_visibility.py +++ b/tests/test_tag_visibility.py @@ -13,6 +13,13 @@ def _devices() -> list[dict]: ] +def _devices_for_every_lane() -> list[dict]: + return [ + *_devices(), + device("encoder-1", ["monitoring:probe"], address="192.0.2.21/24", probe_module="icmp"), + ] + + # A token without tag-view access sees an empty tag list, the same as a NetBox with no # lane tags. The refresh fails so the lanes don't go empty without a signal. def test_no_visible_lane_tag_fails_the_refresh_and_keeps_the_last_snapshot(): @@ -31,32 +38,47 @@ def test_no_visible_lane_tag_fails_the_refresh_and_keeps_the_last_snapshot(): state = cache.state() assert state.snapshot is good assert state.last_error is not None - assert "No lane tag is visible" in state.last_error + for name in LANE_TAGS: + assert name in state.last_error assert "extras.view_tag" in state.last_error finally: client.close() -def test_no_visible_lane_tag_on_first_refresh_leaves_no_snapshot(): - client = FakeNetbox(devices=_devices(), absent_tags=LANE_TAGS).client() +# A token limited to some lane tags can't tell a hidden tag from a missing one, so any +# unseen lane tag fails the refresh and the earlier lanes stay. +def test_a_partly_visible_lane_tag_set_fails_the_refresh_and_keeps_every_lane(): + netbox = FakeNetbox(devices=_devices_for_every_lane()) + client = netbox.client() cache = PromSDCache(client=client, ttl_seconds=60) try: + assert cache.refresh() + good = cache.get_snapshot() + assert good is not None + assert good.lanes["probe"] and good.lanes["snmp"] + + netbox.remove_tag("monitoring:probe") + netbox.remove_tag("monitoring:snmp") assert not cache.refresh() - assert cache.get_snapshot() is None + + state = cache.state() + assert state.snapshot is good + assert cache.lane_payload("probe") == good.lanes["probe"] + assert cache.lane_payload("snmp") == good.lanes["snmp"] + assert state.last_error is not None + assert "monitoring:probe, monitoring:snmp" in state.last_error + assert "monitoring:scrape" not in state.last_error + assert "extras.view_tag" in state.last_error finally: client.close() -# One visible lane tag proves the token can view tags, so the others are truly missing. -def test_one_visible_lane_tag_keeps_the_missing_lane_case(): - absent = ["monitoring:probe", "monitoring:snmp"] - client = FakeNetbox(devices=_devices(), absent_tags=absent).client() +def test_an_unseen_lane_tag_on_the_first_refresh_leaves_no_snapshot(): + client = FakeNetbox(devices=_devices(), absent_tags=["monitoring:probe"]).client() cache = PromSDCache(client=client, ttl_seconds=60) try: - assert cache.refresh() - snapshot = cache.get_snapshot() - assert snapshot is not None - assert snapshot.missing_tags == sorted(absent) - assert cache.state().last_error is None + assert not cache.refresh() + assert cache.get_snapshot() is None + assert cache.state().last_error is not None finally: client.close()