diff --git a/AGENTS.md b/AGENTS.md index 1a68b0f..cebb106 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -405,7 +405,8 @@ Node tags are flexible key-value pairs that allow custom metadata to be attached | `lat` | GPS latitude override | Overrides node-reported latitude for map display | | `lon` | GPS longitude override | Overrides node-reported longitude for map display | | `elevation` | GPS elevation override | Overrides node-reported elevation | -| `role` | Node role/purpose | Used for website presentation and filtering (e.g., "gateway", "repeater", "sensor") | + +**Infrastructure status** is determined by node adoption (records in `user_profile_nodes`), not by tags. When OIDC is enabled, adopted nodes are highlighted on the map with distinct icons and a legend. The `role` tag is still read and displayed in map popups for informational purposes but does not drive infrastructure logic. **Important Notes:** - All tags are optional - nodes can function without any tags diff --git a/alembic/versions/20260503_1800_remove_obsolete_node_tags.py b/alembic/versions/20260503_1800_remove_obsolete_node_tags.py new file mode 100644 index 0000000..b9ae22a --- /dev/null +++ b/alembic/versions/20260503_1800_remove_obsolete_node_tags.py @@ -0,0 +1,25 @@ +"""remove obsolete role and member_id node tags + +Revision ID: 20260503_1800 +Revises: d7a9bbe85a9e +Create Date: 2026-05-03 18:00:00.000000 + +""" + +from typing import Sequence, Union + +from alembic import op + +revision: str = "20260503_1800" +down_revision: Union[str, None] = "d7a9bbe85a9e" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.execute("DELETE FROM node_tags WHERE key = 'role' AND value = 'infra'") + op.execute("DELETE FROM node_tags WHERE key = 'member_id'") + + +def downgrade() -> None: + pass diff --git a/docs/i18n.md b/docs/i18n.md index 8e560ae..f13ce89 100644 --- a/docs/i18n.md +++ b/docs/i18n.md @@ -303,9 +303,9 @@ Map page content: | Key | English | Context | |-----|---------|---------| | `show_labels` | Show Labels | Toggle to show node labels | -| `infrastructure_only` | Infrastructure Only | Toggle to show only infrastructure nodes | -| `legend` | Legend: | Map legend header | -| `infrastructure` | Infrastructure | Infrastructure node category | +| `infrastructure_only` | Infrastructure Only | Toggle to show only infrastructure nodes (OIDC required, only rendered when `OIDC_ENABLED=true`; data sourced from node adoption, not tags) | +| `legend` | Legend: | Map legend header (only rendered when `OIDC_ENABLED=true`) | +| `infrastructure` | Infrastructure | Infrastructure node category (only rendered when `OIDC_ENABLED=true`; based on adoption status) | | `public` | Public | Public node category | | `nodes_on_map` | {{count}} nodes on map | Status text with coordinates | | `nodes_none_have_coordinates` | {{count}} nodes (none have coordinates) | Status text without coordinates | diff --git a/docs/plans/20260503-1800-infra-node-refactor/plan.md b/docs/plans/20260503-1800-infra-node-refactor/plan.md new file mode 100644 index 0000000..b21a379 --- /dev/null +++ b/docs/plans/20260503-1800-infra-node-refactor/plan.md @@ -0,0 +1,933 @@ +# Infra Node Refactor — Replace `role=infra` Tag with Adopted Nodes + +**Date:** 2026-05-03 +**Status:** Draft + +## Overview + +Replace the `role=infra` NodeTag convention with the existing UserProfileNode adoption model as the source of truth for "infrastructure" status. Currently, infrastructure nodes are identified by a `role=infra` tag, which is a manual, unstructured convention. The adoption system (`user_profile_nodes` table) already links operators to nodes in a first-class, enforced relationship. This refactor makes adopted nodes the canonical infrastructure indicator across the Map, Prometheus metrics, and alerting. + +No new models or tables. No new API endpoints. An Alembic migration is included to clean up obsolete tags. + +## Decisions + +1. **Adopted = Infrastructure** — A node is considered "infrastructure" if it has a record in the `user_profile_nodes` table (i.e., it has been adopted by an operator). The `is_infra` concept is renamed to `is_adopted` throughout. + +2. **Promote `adopted` label over `role` tag** — The `meshcore_node_last_seen_timestamp_seconds` metric changes from a `role` label (derived from `NodeTag`) to an `adopted` boolean label (`"true"`/`"false"`). This is a breaking change for existing Prometheus queries. + +3. **Retain the `role` tag for display** — The `role` tag is still read and displayed in map popups (line `roleHtml` in `map.js`), but it no longer drives the infrastructure/public icon distinction. The map icon color and filter now use adoption status. + +4. **Rename `infra_center` to `adopted_center`** — The server-computed geographic centroid of infrastructure/adopted nodes is renamed for clarity. The API response field changes from `infra_center` to `adopted_center`. + +5. **Rename `infra_nodes` debug field to `adopted_nodes`** — The debug count in map data is renamed for consistency. + +6. **Map category filter changes** — The "Infrastructure Only" filter option now shows only adopted nodes. The label remains "Infrastructure Only" (existing i18n key `map.infrastructure_only`) since the user-facing concept is unchanged. + +7. **Prometheus alert rule updated** — The `NodeNotSeen` alert changes from `role="infra"` selector to `adopted="true"` selector. This is a documentation + config change in `etc/prometheus/alerts.yml`. + +8. **No `role` tag filter in metrics query** — The metrics `collect_metrics()` function replaces the `NodeTag` subquery JOIN with a `UserProfileNode` existence check. This is simpler and uses the enforced relationship. + +9. **OIDC-disabled = no adoption UI** — When `OIDC_ENABLED=false`, there are no user profiles and therefore no adopted nodes. The map must not display adoption-dependent UI elements: no "Infrastructure Only" filter option, no icon distinction, no indicator dot in popups, and no legend. All nodes render identically (green markers) since the adoption concept does not apply. The server still computes `adopted_center` and `is_adopted` fields (they will always be `null`/`false`), but the client ignores them when OIDC is disabled. + +10. **Data cleanup via Alembic migration** — An Alembic migration removes obsolete `role=infra` and `member_id` tags from the `node_tags` table. These tags are superseded by the adoption system and should not linger in the database. + +11. **Add `meshcore_nodes_adopted` metric** — A new Prometheus gauge exposes the count of adopted nodes, complementing the existing `meshcore_nodes_total` and `meshcore_nodes_active` metrics. + +12. **New map icon colors: green (normal) and blue (adopted)** — The existing red (#ef4444) for infrastructure nodes is confusing (red implies offline/broken). Colors change to green (#22c55e / #15803d) for normal nodes and blue (#3b82f6 / #1e40af) for adopted nodes. + +## Terminology + +| Term | Meaning | +|---|---| +| Adopted node | A node with a record in `user_profile_nodes`, linked to an operator's `UserProfile` | +| `is_adopted` | Boolean flag replacing `is_infra` — true when the node has an adoption record | +| `adopted_center` | Geographic centroid of all adopted nodes (replaces `infra_center`) | +| `adopted` label | Prometheus metric label (`"true"`/`"false"`) replacing `role` tag-derived value | +| `role` tag | Optional NodeTag with key `"role"` — retained for display, no longer drives infra logic | +| `member_id` tag | Obsolete NodeTag key from the pre-adoption member system — removed by migration | +| OIDC-disabled | Deployment where `OIDC_ENABLED=false`; no user profiles, no adoption, no infra distinction | + +## Current State + +### How `role=infra` Works Today + +| Layer | File | Lines | What it does | +|-------|------|-------|-------------| +| **Server map data** | `web/app.py` | 707–708, 743 | Iterates node tags looking for `key == "role"`, sets `role` variable. Sets `is_infra: role == "infra"` on each node dict. | +| **Server infra center** | `web/app.py` | 754–772 | Filters nodes where `is_infra` is truthy, computes `infra_center` centroid. Passes `infra_nodes` count in debug. | +| **Map client — icon** | `map.js` | 52–54 | `node.is_infra` chooses red (#ef4444) vs blue (#3b82f6) marker icon | +| **Map client — popup** | `map.js` | 86–92 | `node.is_infra` renders a colored dot indicator with "Infrastructure"/"Public" title | +| **Map client — filter** | `map.js` | 202–204, 286 | Category dropdown with "Infrastructure Only" option filters on `node.is_infra` | +| **Map client — centering** | `map.js` | 25–31, 125, 330–340 | `infraCenter` from server used as anchor point; initial map fit to infra nodes bounds | +| **Map client — legend** | `map.js` | 252–260 | Red = Infrastructure, Blue = Public | +| **Prometheus metrics** | `api/metrics.py` | 147–176 | LEFT JOINs `NodeTag` where `key == "role"`, exposes `role` label on `meshcore_node_last_seen_timestamp_seconds` | +| **Prometheus alert** | `etc/prometheus/alerts.yml` | 9–16 | `NodeNotSeen` alert selects `role="infra"` nodes not seen for 48h | + +### How Adopted Nodes Already Work + +| Layer | File | Lines | What it does | +|-------|------|-------|-------------| +| **Model** | `common/models/user_profile_node.py` | 20–60 | Join table `user_profile_nodes` with composite PK, unique constraint on `node_id` (one adopter per node) | +| **API adopt/release** | `api/routes/adoptions.py` | — | `POST /api/v1/adoptions` (operator/admin), `DELETE /api/v1/adoptions/{public_key}` | +| **Node API** | `api/routes/nodes.py` | 17–34, 45–47 | `_get_adopted_by()` extracts adopter info; `adopted_by` query param filters nodes | +| **Node schema** | `common/schemas/nodes.py` | 75–85, 103–105 | `AdoptedByUser` schema; `NodeRead.adopted_by` optional field | +| **Map server** | `web/app.py` | 726–732 | Already reads `adopted_by` from API response and sets `owner` on node dict | +| **Map client** | `map.js` | 69–76, 218–233 | Popups show owner info; member dropdown conditionally shown when `config.oidc_enabled` | + +### OIDC-Conditional Pattern (Existing) + +The codebase already uses `config.oidc_enabled` to conditionally render OIDC-dependent UI: + +| File | Line | Condition | +|------|------|-----------| +| `map.js` | 218 | `config.oidc_enabled && profiles.length > 0` — member filter dropdown | +| `nodes.js` | 57, 147 | `config.oidc_enabled` — adopted_by filter, profile select | +| `advertisements.js` | 60, 191 | `config.oidc_enabled` — adopted_by filter, profile select | +| `node-detail.js` | 296 | `!config.oidc_enabled \|\| !config.user` — hide adoption section | +| `components.js` | 36, 597 | `config.oidc_enabled` — role checks, auth section | + +### Obsolete Tags + +| Tag Key | Status | Why obsolete | +|---------|--------|-------------| +| `role` (value=`infra`) | Superseded | Adoption status (`user_profile_nodes`) now determines infrastructure | +| `member_id` | Superseded | The old member→node mapping system was replaced by the adoption model (`UserProfileNode`) | + +These tags may exist in existing databases. The Alembic migration (Phase 5) will clean them up. + +### Seed Files + +| File | Contains | Action needed | +|------|----------|---------------| +| `example/seed/node_tags.yaml` | `role: gateway` on example node | Remove `role` key from example | +| `seed/node_tags.yaml` | If present, may contain `role` or `member_id` entries | Remove those keys | + +### Tests + +| Test File | Tests | Lines | +|-----------|-------|-------| +| `tests/test_web/test_map.py` | `TestMapDataInfrastructure` (4 tests) | 278–420 | +| `tests/test_api/test_metrics.py` | `test_node_last_seen_timestamp_with_role` | 199–225 | + +--- + +## Implementation + +### Phase 1: Update Server-Side Map Data (`web/app.py`) + +**File:** `src/meshcore_hub/web/app.py` + +Replace the `role=infra` detection with adoption-based detection. + +#### 1.1 Change node dict construction (L742–744) + +```python +# Before: +"role": role, +"is_infra": role == "infra", +"owner": owner, + +# After: +"role": role, +"is_adopted": adopted_info is not None, +"owner": owner, +``` + +The `role` tag is still read and passed through for display in popups. The `is_infra` key becomes `is_adopted`, sourced from whether the node has `adopted_by` data (which the API already populates). + +Note: `adopted_info` is already computed at L726 (`adopted_info = node.get("adopted_by")`). + +#### 1.2 Rename infra_center to adopted_center (L754–772) + +```python +# Before (L754): +infra_nodes = [n for n in nodes_with_location if n.get("is_infra")] +infra_count = len(infra_nodes) + +# After: +adopted_nodes = [n for n in nodes_with_location if n.get("is_adopted")] +adopted_count = len(adopted_nodes) +``` + +```python +# Before (L767): +infra_center: dict[str, float] | None = None +if infra_nodes: + infra_center = { + "lat": sum(n["lat"] for n in infra_nodes) / len(infra_nodes), + "lon": sum(n["lon"] for n in infra_nodes) / len(infra_nodes), + } + +# After: +adopted_center: dict[str, float] | None = None +if adopted_nodes: + adopted_center = { + "lat": sum(n["lat"] for n in adopted_nodes) / len(adopted_nodes), + "lon": sum(n["lon"] for n in adopted_nodes) / len(adopted_nodes), + } +``` + +#### 1.3 Update response dict (L774–787) + +```python +# Before: +return JSONResponse({ + "nodes": nodes_with_location, + "profiles": list(profiles_by_id.values()), + "center": {"lat": center_lat, "lon": center_lon}, + "infra_center": infra_center, + "debug": { + "total_nodes": total_nodes, + "nodes_with_coords": nodes_with_coords, + "infra_nodes": infra_count, + "error": error, + }, +}) + +# After: +return JSONResponse({ + "nodes": nodes_with_location, + "profiles": list(profiles_by_id.values()), + "center": {"lat": center_lat, "lon": center_lon}, + "adopted_center": adopted_center, + "debug": { + "total_nodes": total_nodes, + "nodes_with_coords": nodes_with_coords, + "adopted_nodes": adopted_count, + "error": error, + }, +}) +``` + +### Phase 2: Update Prometheus Metrics (`api/metrics.py`) + +**File:** `src/meshcore_hub/api/metrics.py` + +Replace the `role` label (from NodeTag) with `adopted` label (from UserProfileNode), and add a new `meshcore_nodes_adopted` gauge. + +#### 2.1 Add `UserProfileNode` to imports + +`UserProfileNode` is **not** currently imported in `metrics.py` (L14–23). Add it to the import block: + +```python +from meshcore_hub.common.models import ( + Advertisement, + EventLog, + Message, + Node, + Telemetry, + TracePath, + UserProfile, + UserProfileNode, # ADD +) +``` + +`NodeTag` can be removed from the import list after the refactor (no longer used). + +#### 2.2 Replace the node_last_seen gauge query (L147–176) + +```python +# Before: +node_last_seen = Gauge( + "meshcore_node_last_seen_timestamp_seconds", + "Unix timestamp of when the node was last seen", + ["public_key", "node_name", "adv_type", "role"], + registry=registry, +) +role_subq = ( + select(NodeTag.node_id, NodeTag.value.label("role")) + .where(NodeTag.key == "role") + .subquery() +) +nodes_with_last_seen = session.execute( + select( + Node.public_key, + Node.name, + Node.adv_type, + Node.last_seen, + role_subq.c.role, + ) + .outerjoin(role_subq, Node.id == role_subq.c.node_id) + .where(Node.last_seen.isnot(None)) +).all() +for public_key, name, adv_type, last_seen, role in nodes_with_last_seen: + node_last_seen.labels( + public_key=public_key, + node_name=name or "", + adv_type=adv_type or "unknown", + role=role or "", + ).set(last_seen.timestamp()) + +# After: +node_last_seen = Gauge( + "meshcore_node_last_seen_timestamp_seconds", + "Unix timestamp of when the node was last seen", + ["public_key", "node_name", "adv_type", "adopted"], + registry=registry, +) +adopted_subq = ( + select(UserProfileNode.node_id) + .subquery() +) +nodes_with_last_seen = session.execute( + select( + Node.public_key, + Node.name, + Node.adv_type, + Node.last_seen, + adopted_subq.c.node_id.isnot(None).label("is_adopted"), + ) + .outerjoin(adopted_subq, Node.id == adopted_subq.c.node_id) + .where(Node.last_seen.isnot(None)) +).all() +for public_key, name, adv_type, last_seen, is_adopted in nodes_with_last_seen: + node_last_seen.labels( + public_key=public_key, + node_name=name or "", + adv_type=adv_type or "unknown", + adopted="true" if is_adopted else "false", + ).set(last_seen.timestamp()) +``` + +#### 2.3 Add `meshcore_nodes_adopted` gauge + +Insert after the `nodes_with_location` gauge (after L145), before the node_last_seen gauge: + +```python +nodes_adopted = Gauge( + "meshcore_nodes_adopted", + "Number of adopted nodes (nodes with an adoption record)", + registry=registry, +) +adopted_count = ( + session.execute(select(func.count(UserProfileNode.node_id))).scalar() or 0 +) +nodes_adopted.set(adopted_count) +``` + +#### 2.4 Remove `NodeTag` import + +`NodeTag` is only used in the `role_subq` block. After replacing it with `UserProfileNode`, remove `NodeTag` from the import list (L19). + +### Phase 3: Update Prometheus Alert Rule (`etc/prometheus/alerts.yml`) + +**File:** `etc/prometheus/alerts.yml` + +```yaml +# Before: +- alert: NodeNotSeen + expr: time() - meshcore_node_last_seen_timestamp_seconds{role="infra"} > 48 * 3600 + for: 5m + labels: + severity: warning + annotations: + summary: "Node {{ $labels.node_name }} ({{ $labels.role }}) not seen for 48+ hours" + description: "Node {{ $labels.public_key }} ({{ $labels.adv_type }}, role={{ $labels.role }}) last seen {{ $value | humanizeDuration }} ago." + +# After: +- alert: NodeNotSeen + expr: time() - meshcore_node_last_seen_timestamp_seconds{adopted="true"} > 48 * 3600 + for: 5m + labels: + severity: warning + annotations: + summary: "Adopted node {{ $labels.node_name }} not seen for 48+ hours" + description: "Adopted node {{ $labels.public_key }} ({{ $labels.adv_type }}) last seen {{ $value | humanizeDuration }} ago." +``` + +### Phase 4: Update Map Client (`map.js`) + +**File:** `src/meshcore_hub/web/static/js/spa/pages/map.js` + +All adoption-dependent UI is gated on `config.oidc_enabled`. When OIDC is disabled, the map renders all nodes with identical green markers, no infrastructure filter, no legend, and no indicator dots. + +Icon colors change from red (infra) / blue (normal) to **blue (adopted) / green (normal)**. + +| Node type | Before | After | +|-----------|--------|-------| +| Adopted (OIDC enabled) | Red `#ef4444` / `#b91c1c` | Blue `#3b82f6` / `#1e40af` | +| Normal | Blue `#3b82f6` / `#1e40af` | Green `#22c55e` / `#15803d` | +| All nodes (OIDC disabled) | Red/blue split | Green only | + +#### 4.1 Add `oidcEnabled` parameter to `createNodeIcon()` and `createPopupContent()` signatures + +Both functions currently take only `node`. Add `oidcEnabled` as a second parameter: + +```javascript +// Before: +function createNodeIcon(node) { +function createPopupContent(node) { + +// After: +function createNodeIcon(node, oidcEnabled) { +function createPopupContent(node, oidcEnabled) { +``` + +Update the call sites (L295–296) to pass `config.oidc_enabled`: + +```javascript +// Before at L295–296: +const marker = L.marker([node.lat, node.lon], { icon: createNodeIcon(node) }).addTo(map); +marker.bindPopup(createPopupContent(node)); + +// After: +const marker = L.marker([node.lat, node.lon], { icon: createNodeIcon(node, config.oidc_enabled) }).addTo(map); +marker.bindPopup(createPopupContent(node, config.oidc_enabled)); +``` + +#### 4.2 Update `createNodeIcon()` — new colors + conditional (L47–54, L295) + +```javascript +// Before: +const iconHtml = node.is_infra + ? '
' + : ''; + +// After: +const iconHtml = (oidcEnabled && node.is_adopted) + ? '' + : ''; +``` + +Adopted nodes get blue markers. Normal nodes (and all nodes when OIDC disabled) get green markers. + +#### 4.3 Update `createPopupContent()` — conditional indicator with new colors (L86–92) + +```javascript +// Before (L87): +if (typeof node.is_infra !== 'undefined') { + const dotColor = node.is_infra ? '#ef4444' : '#3b82f6'; + const borderColor = node.is_infra ? '#b91c1c' : '#1e40af'; + const title = node.is_infra ? t('map.infrastructure') : t('map.public'); + +// After: +if (oidcEnabled && typeof node.is_adopted !== 'undefined') { + const dotColor = node.is_adopted ? '#3b82f6' : '#22c55e'; + const borderColor = node.is_adopted ? '#1e40af' : '#15803d'; + const title = node.is_adopted ? t('map.infrastructure') : t('map.public'); +``` + +The popup indicator dot is only rendered when OIDC is enabled. When OIDC is disabled, `infraIndicatorHtml` stays empty and no dot appears. + +#### 4.4 Update `getAnchorPoint()` — rename parameter (L25) + +```javascript +// Before: +function getAnchorPoint(nodes, infraCenter) { + if (infraCenter) return infraCenter; + +// After: +function getAnchorPoint(nodes, adoptedCenter) { + if (adoptedCenter) return adoptedCenter; +``` + +#### 4.5 Update data destructuring — rename variable (L125) + +```javascript +// Before: +const infraCenter = data.infra_center || null; + +// After: +const adoptedCenter = data.adopted_center || null; +``` + +#### 4.6 Update category filter — conditional on OIDC (L202–204) + +```javascript +// Before: + + +// After: + +``` + +When OIDC is disabled, the dropdown shows only "All Nodes". + +#### 4.7 Update legend — conditional on OIDC with new colors (L252–260) + +```javascript +// Before: +