Files
shopdb-flask/docs/adr/ADR-006-collector-contract.md
cproudlock 5fa5160420 Apply skill-driven review fixes: security, hook isolation, tests, docs
Addresses findings from a 6-lens review against the project skills
(defining-asset-contract, enforcing-plugin-contract, hardening-flask-config,
integrating-plugin-hooks, pinning-flask-behavior, simplifying-python).

Security (hardening-flask-config):
- Load per-plugin COLLECTOR_API_KEY_<PLUGIN> from env in create_app. from_object
  only copies class attributes, so per-plugin keys (ADR-006) were dead in real
  deploys and silently fell back to the shared key.
- EMPLOYEE_DB_USER/PASSWORD no longer default to root/rootpassword (no safe
  default for a secret; unset fails loud). Documented in .env.example + DEPLOY.md.
- COLLECTOR_API_KEY + per-plugin + EMPLOYEE_DB_* added to .env.example/DEPLOY.md.

Hook isolation (integrating-plugin-hooks):
- collector _collector_plugins and dashboard get_navigation now re-raise in
  dev/test and log+isolate in prod, instead of silently swallowing a broken
  plugin hook.

Plugin loader (enforcing-plugin-contract):
- enable_plugin/install_plugin read dependencies+version from the manifest
  instead of instantiating the plugin class.
- _register_plugin_components rejects a second plugin claiming an already-used
  api_prefix (reset per app in init_app).

Tests (pinning-flask-behavior):
- test_identifiers.py: gauge/maintenance round-trip on computer/printer/network
  create+update; per-type seed yields the 12 identifier keys.
- contract tests for apply_collector_payload presence + schema-declarers-implement.
- security tests for per-plugin key env loading + no employee-db password default.

Docs/contract sync (defining-asset-contract):
- PLUGIN-HOOKS.md documents apply_collector_payload; stale 0.2.0 -> 0.3.0.
- ADR-006 documents apply_collector_payload + single-dispatch rationale.
- ADR-001 enumerates the expanded shopdb.api import surface.

Simplify (simplifying-python):
- De-duplicate the 21-entry settings defaults: shared build_default_settings()
  used by both the /settings/seed route and the CLI (were drifting copies).
- Remove dead AssetStatus import + redundant AssetType local import in computers
  plugin; comment the statusid=1 collector default.

153 tests pass (was 145), naming/style green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-26 19:25:52 -04:00

149 lines
6.4 KiB
Markdown

# ADR-006: Plugin collector contract pattern
- **Status:** ACCEPTED
- **Date:** 2026-05-08
- **Deciders:** cproudlock
- **Supersedes:** none
## Context
PC inventory data was collected by PowerShell scripts pushing to `/api/collector/pc` (`shopdb/core/api/collector.py`, ~374 LOC). The endpoint is hardcoded for PCs: it accepts a fixed schema and writes to the legacy `Machine` model.
Per ADR-001, `Machine` is being retired in favor of `Asset`. Per the project shift to PXE-driven imaging, PC inventory is moving to a new collection pipeline (PXE / GE-Enforce / manifest engine produces JSON about each PC). Other asset classes may want similar collector pipelines (printers via Zabbix, network gear via SNMP scan).
This calls for a generalizable contract: any plugin that wants to accept external collector input declares a JSON schema, and the framework wires the endpoint, auth, and idempotency.
## Decision
`BasePlugin` gets two new hooks (added in __contract_version__ 0.2.x -> the surface is carried at 0.3.0):
```python
def get_collector_schema(self) -> Optional[dict]:
"""Return JSON Schema describing the collector payload for this plugin.
Return None if the plugin does not accept collector input.
The schema must include:
- 'identityfield': name of the field that uniquely identifies an asset
across submissions (e.g., 'hostname' for PCs, 'macaddress' for network
devices). Used for idempotent upsert.
- 'fields': JSON Schema definitions for the rest of the payload.
"""
return None
def apply_collector_payload(self, payload: dict) -> dict:
"""Idempotently upsert an asset from a validated collector payload.
Called by /api/collector/<pluginname> after identity validation.
CONDITIONAL hook: required only when get_collector_schema returns
non-None. Default raises NotImplementedError (the dispatcher returns
500) so a schema-without-upsert fails loud. Returns a dict with at
least 'action' ('created'|'updated'|'noop'), 'assetid', 'warnings'.
"""
raise NotImplementedError
```
The pairing (schema present => apply implemented) is enforced by the
`test_schema_declaring_plugins_implement_apply` contract test.
A single dynamic dispatch route `/api/collector/<pluginname>` serves every
plugin that returns a schema (rather than registering a blueprint per plugin),
because Flask forbids `register_blueprint` after the first request and plugins
can be enabled at runtime. Auth is API-key, separate from JWT. Per-plugin keys via env vars:
- `COLLECTOR_API_KEY_<PLUGINNAME>` (preferred, plugin-specific)
- `COLLECTOR_API_KEY` (fallback, shared)
### Idempotent upsert
The endpoint uses the `identityfield` to find an existing `Asset` for the same identity. Found = update. Not found = insert. Existing relationships are preserved on update.
### Response contract
```json
{
"status": "ok",
"action": "created" | "updated" | "noop",
"assetid": 12345,
"identityvalue": "PC-1234",
"warnings": []
}
```
### Audit logging
Every collector submission produces an audit log entry: `{action, plugin, identityvalue, before/after diff}`. Audit retention per site policy.
### Schema discovery
The framework exposes the registered schemas at `/api/collector/_schemas` (read-only, JWT-protected) so external collector authors can introspect what payloads are accepted by which plugins.
## Concrete first user: computers plugin
The `computers` plugin is the first to implement `get_collector_schema`. The PXE pipeline conforms.
Initial computers collector schema (sketch, finalized when plugin is built):
```json
{
"identityfield": "hostname",
"fields": {
"hostname": "string, required",
"macaddress": "string, optional, secondary identity",
"osname": "string",
"osversion": "string",
"lastboottime": "datetime",
"currentuser": "string",
"ipaddress": "string",
"memorygb": "number",
"cputype": "string",
"imagename": "string (PXE image deployed)",
"imageappliedat": "datetime",
"installedsoftware": "array of {name, version}"
}
}
```
The PC re-image case is handled by the identity field: a freshly imaged PC keeps its hostname, so the existing `Asset` row is updated rather than duplicated. Existing `AssetRelationship` rows pointing at that PC (e.g., `controls` to a machine) are preserved across re-images.
## Migration of the existing endpoint
`shopdb/core/api/collector.py` (`/api/collector/pc`) is **deprecated** in v1 and **removed** before v1.0.
Migration path:
1. Implement `get_collector_schema` on the `computers` plugin. New endpoint `/api/collector/computers` is auto-registered.
2. Run both endpoints in parallel for one cycle of PXE imaging across the floor. PXE pipeline switches to `/api/collector/computers`.
3. Remove `shopdb/core/api/collector.py` and the legacy blueprint registration.
## Consequences
### Positive
- Generalizable across plugins. Sister sites adopting `printers`, `network`, etc. can wire their own collectors with no core change.
- Identity-based idempotency makes PC re-imaging safe by default.
- Audit logging is uniform across plugins.
- Schema discovery enables external tools to validate before submission.
### Negative
- Plugin authors must write a JSON schema. Slight learning curve, but JSON Schema is widely understood and the framework can ship a few examples.
- The `/api/collector/_schemas` endpoint plus per-plugin endpoints expand the public API surface; minor maintenance cost.
### Neutral
- API-key auth pattern stays as it is today (separate from JWT). Sites manage their own collector keys per plugin via env vars.
## Alternatives considered
1. **Keep `/api/collector/pc` and add new plugin-specific endpoints alongside.** Two ways to send PC data, plugin authors confused. Rejected.
2. **Use JWT for collectors instead of API key.** Collectors are headless processes (PXE pipeline, scripts), not interactive users. JWT lifecycle (refresh tokens, expiry) is the wrong tool. API key is simpler. Rejected.
3. **Plugins write directly to the database, no collector endpoint.** Skips audit logging and schema validation. Rejected.
## References
- `shopdb/core/api/collector.py` (legacy endpoint to be removed)
- `shopdb/plugins/base.py` (`get_collector_schema` + `apply_collector_payload` hooks)
- ADR-001 (asset model the collectors target)
- ADR-002 (collector schema is part of plugin contract; changes to the hook signature are major bumps)
- The PXE project (`/home/camp/projects/pxe/`) which feeds the computers collector