diff --git a/docs/BACKUP-KINDS.md b/docs/BACKUP-KINDS.md new file mode 100644 index 0000000..d93c7bc --- /dev/null +++ b/docs/BACKUP-KINDS.md @@ -0,0 +1,118 @@ +# Fleet config backups: adding a kind, and how health is judged + +This is about the `backups` plugin - the per-PC CONFIG backups the collector +takes off shopfloor machines (NTLARS settings, part-marker configs). It is not +about backing up the ShopDB database itself; that is `docs/BACKUP-RESTORE.md`. + +Two things this covers: + +1. [Adding a new backup kind](#adding-a-new-backup-kind) +2. [How the Backup badge decides good vs stale](#how-the-backup-badge-decides-good-vs-stale) + +--- + +## Adding a new backup kind + +A kind is one class in `plugins/backups/services/registry.py`, added to +`REGISTRY` at the bottom of that file: + +```python +REGISTRY = {k.key: k for k in (NtlarsKind(), PartMarkerKind())} +``` + +Subclass `BackupKind` and override what applies. `NtlarsKind` is the fullest +example (parseable, renderable, has an info panel); `PartMarkerKind` is the +lean one. + +| Member | What it is | +|--------|------------| +| `key` | Wire value. This is what lands in `backuprevisions.backupkind` and what the fleet table shows on the badge. Keep it short and lowercase. | +| `displayname` | Human label for the UI. | +| `storagebackend` | `'shopdb'` (bytes live in the DB, deduped on a semantic hash) or `'share'` (file lives on the SMB share, deduped on a raw byte hash). | +| `assettypes` | Which core asset types this kind can attach to; `['*']` for any. | +| `emptytext` | Text when an asset has no revisions of this kind. `None` HIDES the panel, which is the right default - a kind applies to an asset TYPE, but whether a given machine ever has that backup is per-machine. A part-marker panel on all 144 machines is noise. | +| `parse(raw)` | Opaque kinds return `None`; parseable kinds return a projection dict. | +| `formats()` / `render(...)` | Download formats, for `shopdb` kinds. | +| `resolveassetid(payload)` | Map a collector payload to the asset the backup belongs to. | +| `infopanel()` / `buildinfo(...)` | Optional at-a-glance card (ADR-010). Declared by the KIND, not hardcoded in the plugin, so a successor technology ships its own card by adding a class. | +| `sharedir(...)` | Conventional UNC directory for a `share` kind. Advisory - the authoritative path is whatever the collector reported, because the PC is what actually wrote the file. | + +### What you do NOT have to touch + +The GE-Enforce **Enforcement Reports** Backup column needs no change. It reads +`backuprevisions` generically: newest revision per host, whatever the kind, and +shows `backupkind` on the badge. A new kind inherits the badge, the colour and +the tooltip with no work. + +Same for the staleness rule below - it is time-based and kind-agnostic. + +--- + +## How the Backup badge decides good vs stale + +### The trap this is built around + +**A revision is only written when the config CHANGES.** Dedup compares against +the latest revision for the chain `(asset, kind, sourcehostname)`, so a machine +whose config has been stable for six months has a six-month-old newest revision +and is perfectly healthy. The question worth answering is not "when was the last +backup taken" - it is "is this still being checked". + +So the timestamp the fleet table uses is `backuprevisions.lastseenat`: the last +time the collector CONFIRMED this config, whether or not anything changed. It +moves on every successful collection; the revision does not. + +That was shown as a raw date at first, and it read as neglect. At the default +`backups_intervalhours` of **24**, the collector only attempts once a day, so a +day-old confirmation IS the healthy steady state. The date made a working system +look like a stalled one, and made the reader do arithmetic against a setting +they would have to go and look up. + +### What it does now + +`GET /api/geenforce/reports` returns, per host: + +| Field | Meaning | +|-------|---------| +| `backupkind` | Which kind was most recently confirmed. `null` = no backup at all. | +| `backuplastseen` | When it was last CONFIRMED (ISO). Tooltip only. | +| `backupok` | `true` good, `false` stale, `null` nothing to judge. | +| `backupstaleafterdays` | The threshold in force, so the UI can explain itself. | + +`backupok` is deliberately **tri-state**. `null` means there is no revision for +that host, or the check is disabled - and it renders as no badge, never green. +"Never seen" must not read as healthy. + +The threshold is the backups plugin's own `backups_staledays` setting (default +**3**), read through `plugins.backups.services.staleness.staledays()` rather +than re-derived, so there is ONE definition of stale. `0` disables the check. +The import is guarded, so a lean site build without the backups plugin returns +`null` instead of failing. + +### Why time-based rather than per-kind + +Every kind answers the same question the same way: something confirmed this +recently, or it did not. Making the rule per-kind would mean each new kind has +to define health before it can show a badge, for no gain. If a kind ever needs +its own window - a weekly backup that should not be judged on a 3-day rule - +add a threshold override on `BackupKind` and have `_backup_stale_cutoff` prefer +it; the tri-state contract stays as it is. + +### Where the code lives + +- `plugins/geenforce/api/routes.py` - `_attach_backup_state` (newest revision + per host), `_backup_stale_cutoff` (threshold), `_backup_ok` (verdict) +- `plugins/geenforce/frontend/views/EnforcementReports.vue` - `backupClass` + (green / red / none), `backupTitle` (the hover text) +- `plugins/backups/services/staleness.py` - the shared threshold, also behind + the dashboard's stale-backups card + +Tests: `tests/test_plugins/test_geenforce_reporting.py`, the backup-verdict +block - recent-is-good, older-than-threshold-is-stale, and none-is-not-green. + +--- + +## See also + +- `docs/BACKUP-RESTORE.md` - backing up the ShopDB database itself +- `docs/geenforce-api-cutover.md` - the fleet reporting path these fields ride on diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index aa12163..d501f43 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -955,9 +955,12 @@ def _asset_facts(hostnames): 'mapx': asset.mapx, 'mapy': asset.mapy, 'machinenumber': None, 'machineassetid': None, + 'machinepluginid': None, 'toolassetnumber': None, 'toolassetid': None, + 'toolpluginid': None, 'displayrole': None, 'backupkind': None, 'backuplastseen': None, + 'backuplastseenat': None, } _attach_controlled_assets(facts, assetidtohost) @@ -993,11 +996,24 @@ def _attach_controlled_assets(facts, assetidtohost): AssetRelationship.sourceassetid.in_(assetidtohost)) .all()) - toolassetids = set() + # assetid -> the PLUGIN extension id the detail routes key on. /machines/:id + # and /measuringtools/:id take machineid / measuringtoolid, NOT the core + # assetid; linking with the assetid lands on whichever record happens to + # share that number, which looks plausible and is wrong. Frontend + # getAssetRoute resolves the same way. + toolids = {} try: from plugins.measuringtools.models import MeasuringTool - toolassetids = {row.assetid for row in - MeasuringTool.query.with_entities(MeasuringTool.assetid)} + toolids = {row.assetid: row.measuringtoolid for row in + MeasuringTool.query.with_entities( + MeasuringTool.assetid, MeasuringTool.measuringtoolid)} + except ImportError: + pass + machineids = {} + try: + from plugins.machines.models import Machine + machineids = {row.assetid: row.machineid for row in + Machine.query.with_entities(Machine.assetid, Machine.machineid)} except ImportError: pass @@ -1005,12 +1021,17 @@ def _attach_controlled_assets(facts, assetidtohost): entry = facts.get(assetidtohost.get(sourceassetid)) if not entry: continue - if asset.assetid in toolassetids: + if asset.assetid in toolids: entry['toolassetnumber'] = asset.assetnumber entry['toolassetid'] = asset.assetid + entry['toolpluginid'] = toolids.get(asset.assetid) else: entry['machinenumber'] = asset.assetnumber entry['machineassetid'] = asset.assetid + # None when the machines plugin is not installed or the asset has no + # extension row: the frontend then renders the number without a link + # rather than a link that goes somewhere wrong. + entry['machinepluginid'] = machineids.get(asset.assetid) def _attach_display_roles(facts, hostnames): @@ -1059,6 +1080,51 @@ def _attach_backup_state(facts, hostnames): entry['backuplastseen'] = ( revision.lastseenat.isoformat() + 'Z' if revision.lastseenat else None) + # Raw value kept beside the wire string so the verdict compares + # datetimes rather than re-parsing its own output. + entry['backuplastseenat'] = revision.lastseenat + + +def _backup_ok(lastseenat, cutoff): + """Tri-state verdict for one PC's backup: True good, False stale, None unknown. + + A BOOLEAN rather than a date, because the question is "is this backed up?" + and every backup kind answers it the same way. Adding a new kind needs no + change here - it reports a revision like the others and inherits the badge. + + None means there is nothing to judge: no backup revision for this host at + all, or the staleness check is disabled. The caller shows no badge rather + than a green one, since "never seen" must not read as healthy. + """ + if cutoff is None or lastseenat is None: + return None + return lastseenat >= cutoff + + +def _backup_stale_cutoff(): + """(cutoff datetime, threshold days) for 'this PC's backup has gone quiet'. + + A raw date makes the reader do the arithmetic, and the arithmetic is not + obvious: the collector only ATTEMPTS every backups_intervalhours (24 by + default), so a confirmation from yesterday is the healthy steady state, not + a warning. What answers the question is the verdict. + + The threshold is the backups plugin's own backups_staledays, read through + its service so there is ONE definition of stale rather than a second one + drifting here. Returns (None, 0) when the plugin is absent or the check is + disabled, and callers then show the date alone. + """ + try: + from plugins.backups.services import staleness + except ImportError: + return None, 0 + try: + days = int(staleness.staledays()) + except (TypeError, ValueError): + return None, 0 + if days <= 0: + return None, 0 + return service._utcnow() - timedelta(days=days), days def _report_stale_cutoff(): @@ -1108,6 +1174,7 @@ def list_reports(): if report.hostname}) stalecutoff, stalafter = _report_stale_cutoff() + backupcutoff, backupafterdays = _backup_stale_cutoff() latest_cache = {} data = [] for report in reports: @@ -1142,13 +1209,23 @@ def list_reports(): 'mapy': known.get('mapy'), 'machinenumber': known.get('machinenumber'), 'machineassetid': known.get('machineassetid'), + 'machinepluginid': known.get('machinepluginid'), 'toolassetnumber': known.get('toolassetnumber'), 'toolassetid': known.get('toolassetid'), + 'toolpluginid': known.get('toolpluginid'), # Reported by the device wins; the DashboardDefault mapping is a # fallback for hosts on an older client that does not send it. 'displayrole': report.subtype or known.get('displayrole'), 'backupkind': known.get('backupkind'), 'backuplastseen': known.get('backuplastseen'), + # VERIFIED, not "last backup taken". Dedup means an unchanged config + # writes no new revision, so lastseenat is the last time the + # collector CHECKED and found the backup intact - which is the + # question. backupok says whether that check is recent enough to + # trust; the date alone reads as "nothing has happened since", which + # is the healthy steady state and looks like neglect. + 'backupok': _backup_ok(known.get('backuplastseenat'), backupcutoff), + 'backupstaleafterdays': backupafterdays, 'scopename': report.scopename, 'phase': report.phase, 'appliedversion': report.appliedversion, diff --git a/plugins/geenforce/frontend/views/EnforcementReports.vue b/plugins/geenforce/frontend/views/EnforcementReports.vue index 0f1eeb7..414fe5b 100644 --- a/plugins/geenforce/frontend/views/EnforcementReports.vue +++ b/plugins/geenforce/frontend/views/EnforcementReports.vue @@ -61,23 +61,33 @@ - + {{ report.machinenumber }} - {{ report.machinenumber }} + {{ report.toolassetnumber }} + {{ report.toolassetnumber }} {{ report.displayrole }} - -
{{ report.location }}
@@ -99,15 +109,19 @@ {{ report.installed }} {{ report.skipped }} {{ report.failed }} - - - - - + + + {{ report.backupkind }} + - {{ formatDate(report.lastcheckin || report.receivedat) }} @@ -191,6 +205,27 @@ function statusClass(status) { return { ok: 'badge-success', selfhealed: 'badge-info', failed: 'badge-danger' }[status] || '' } +// null verdict = nothing to judge (no revision, or the check is disabled). +// Deliberately NOT green: "never seen" must not read as healthy. +function backupClass(report) { + if (report.backupok === true) return 'badge-success' + if (report.backupok === false) return 'badge-danger' + return '' +} + +function backupTitle(report) { + const seen = report.backuplastseen ? formatDate(report.backuplastseen) : 'never' + if (report.backupok === true) { + return `${report.backupkind}: checked and verified ${seen}.` + + ' Config unchanged since, which is why the date does not move.' + } + if (report.backupok === false) { + return `${report.backupkind}: last confirmed ${seen}, more than` + + ` ${report.backupstaleafterdays} day(s) ago - the backup has stopped running.` + } + return `${report.backupkind}: last confirmed ${seen}.` +} + function staleTitle(report) { const received = report.receivedat ? formatDate(report.receivedat) : 'never' const checkin = report.lastcheckin ? formatDate(report.lastcheckin) : 'not reported' diff --git a/tests/test_plugins/test_geenforce_reporting.py b/tests/test_plugins/test_geenforce_reporting.py index daedef3..ae814a3 100644 --- a/tests/test_plugins/test_geenforce_reporting.py +++ b/tests/test_plugins/test_geenforce_reporting.py @@ -336,3 +336,128 @@ def test_a_future_checkin_cannot_mask_server_side_silence(client, db, app, row = client.get('/api/geenforce/reports', headers=auth_headers).get_json()['data'][0] assert row['isstale'] is True + + +def test_asset_link_uses_the_plugin_id_not_the_assetid(client, db, app, auth_headers): + """/machines/:id keys on machineid, NOT the core assetid. + + Returning the assetid made the fleet table link to whichever machine + happened to carry that number - a wrong page that looks right rather than a + 404. The frontend links on machinepluginid for exactly this reason. + """ + from shopdb.core.models import Asset, AssetType + from shopdb.core.models.relationship import RelationshipType, AssetRelationship + from plugins.computers.models import Computer + from plugins.machines.models import Machine + + _seed_and_publish(app) + secret = _token(client, auth_headers, ['geenforce.report']) + + computertype = AssetType.query.filter_by(assettype='computer').first() \ + or AssetType(assettype='computer') + machinetype = AssetType.query.filter_by(assettype='machine').first() \ + or AssetType(assettype='machine') + db.session.add_all([computertype, machinetype]) + db.session.flush() + + pcasset = Asset(assetnumber='WJBAY01', assettypeid=computertype.assettypeid) + machineasset = Asset(assetnumber='6905', assettypeid=machinetype.assettypeid) + db.session.add_all([pcasset, machineasset]) + db.session.flush() + db.session.add(Computer(assetid=pcasset.assetid, hostname='WJBAY01')) + machine = Machine(assetid=machineasset.assetid) + db.session.add(machine) + db.session.flush() + + controls = RelationshipType.query.filter_by(relationshiptype='controls').first() + if not controls: + controls = RelationshipType(relationshiptype='controls') + db.session.add(controls) + db.session.flush() + db.session.add(AssetRelationship( + sourceassetid=pcasset.assetid, targetassetid=machineasset.assetid, + relationshiptypeid=controls.relationshiptypeid, label='collector:machine')) + db.session.commit() + + client.post('/api/geenforce/report', json={ + 'hostname': 'WJBAY01', 'scopename': 'gea-shopfloor-cmm', 'counts': {}, + }, headers={'X-API-Key': secret}) + + row = [r for r in client.get('/api/geenforce/reports', + headers=auth_headers).get_json()['data'] + if r['hostname'] == 'WJBAY01'][0] + assert row['machinenumber'] == '6905' + assert row['machineassetid'] == machineasset.assetid + assert row['machinepluginid'] == machine.machineid + + +# -- backup verdict ---------------------------------------------------------- +# +# The column used to show a raw date, which read as neglect: dedup means an +# unchanged config writes no new revision, so a day-old confirmation IS the +# healthy steady state at the default 24h collection interval. + +def _backup_revision(db, hostname, kind, ageda): + from datetime import timedelta + from plugins.backups.models import BackupRevision + from plugins.geenforce.service import _utcnow + from shopdb.core.models import Asset, AssetType + from plugins.computers.models import Computer + + computertype = AssetType.query.filter_by(assettype='computer').first() \ + or AssetType(assettype='computer') + db.session.add(computertype) + db.session.flush() + asset = Asset(assetnumber=hostname, assettypeid=computertype.assettypeid) + db.session.add(asset) + db.session.flush() + db.session.add(Computer(assetid=asset.assetid, hostname=hostname)) + db.session.add(BackupRevision(assetid=asset.assetid, backupkind=kind, + sourcehostname=hostname, + contenthash='0' * 64, + lastseenat=_utcnow() - timedelta(days=ageda))) + db.session.commit() + + +def _row_for(client, auth_headers, hostname): + return [r for r in client.get('/api/geenforce/reports', + headers=auth_headers).get_json()['data'] + if r['hostname'] == hostname][0] + + +def test_a_recent_check_is_good_even_though_nothing_changed(client, db, app, auth_headers): + _seed_and_publish(app) + secret = _token(client, auth_headers, ['geenforce.report']) + _backup_revision(db, 'WJBAK01', 'ntlars', ageda=1) + client.post('/api/geenforce/report', json={ + 'hostname': 'WJBAK01', 'scopename': 'gea-shopfloor-cmm', 'counts': {}}, + headers={'X-API-Key': secret}) + + row = _row_for(client, auth_headers, 'WJBAK01') + assert row['backupkind'] == 'ntlars' + assert row['backupok'] is True, 'a day old is the healthy steady state' + assert row['backupstaleafterdays'] == 3 + + +def test_a_check_older_than_the_threshold_is_stale(client, db, app, auth_headers): + _seed_and_publish(app) + secret = _token(client, auth_headers, ['geenforce.report']) + _backup_revision(db, 'WJBAK02', 'ntlars', ageda=9) + client.post('/api/geenforce/report', json={ + 'hostname': 'WJBAK02', 'scopename': 'gea-shopfloor-cmm', 'counts': {}}, + headers={'X-API-Key': secret}) + + assert _row_for(client, auth_headers, 'WJBAK02')['backupok'] is False + + +def test_no_backup_at_all_is_unknown_not_good(client, db, app, auth_headers): + """None, never True: 'never seen' must not render as a green badge.""" + _seed_and_publish(app) + secret = _token(client, auth_headers, ['geenforce.report']) + client.post('/api/geenforce/report', json={ + 'hostname': 'WJBAK03', 'scopename': 'gea-shopfloor-cmm', 'counts': {}}, + headers={'X-API-Key': secret}) + + row = _row_for(client, auth_headers, 'WJBAK03') + assert row['backupkind'] is None + assert row['backupok'] is None