feat(rmm): device-sync worker, provider trait, suppression + dedupe (PMS-103) #62

Merged
nrupard merged 3 commits from feat/pms-103-rmm-sync-worker into main 2026-06-04 17:33:43 +02:00
Owner

Summary

Closes the five PMS-100 blockers that the story-verification pass surfaced - device-sync worker, RmmProvider trait, real encryption key, suppression + dedupe + TicketService routing on alert ingest, missing GET/PUT connection endpoints - in one PR so the worker ships with the integration it actually needs. Mirrors the PMS-92 shape used for the notifications dispatcher.

  • RmmService::new(db) is gone; the only constructors are with_encryption_key(db, key) and with_dependencies(db, key, ticket_service). src/api/router.rs:110 now wires the real encryption_key (same value already threaded into BillingService + NotificationsService) plus a TicketService clone, so rmm_connections.api_key_encrypted / api_secret_encrypted stop being encrypted under [0u8; 32] and alert-ingest tickets stop bypassing the canonical ticket-creation path.
  • RmmProvider trait lives in src/modules/rmm/provider.rs with list_devices / list_alerts / ping. TacticalRmmProvider is the first real implementation (Token auth against /agents/ and /alerts/). mesh_central / datto / connectwise / ninja_rmm fall back to a fail-loud UnimplementedProvider so an operator using one of those sees a real last_error instead of silent no-ops.
  • migrations/007_rmm_mesh_central_provider.sql adds mesh_central to the rmm_connections.provider CHECK constraint so PMS-105's "MeshCentral provider variant" claim matches the schema.
  • RmmSyncWorker (src/modules/rmm/worker.rs) spawned from src/main.rs next to the notifications DispatcherWorker. Tick is 60s; per active connection past its sync_interval_minutes window the worker decrypts credentials, builds the right provider, pulls devices, UPSERTs rmm_device_mappings on (rmm_connection_id, rmm_device_id), match-or-creates assets (serial -> hostname order), stamps last_sync_at, writes asset_audit_log with action='created' or 'synced', and flips sync_status to success (clearing last_error) or failed (storing the error). Errors per connection are isolated so one bad provider does not stall the rest.
  • ingest_alert at src/modules/rmm/service.rs:296-393 consults rule.suppression_rules.min_severity and rule.suppression_rules.dedupe_window_minutes before creating a ticket. ticket_template JSONB is rendered via the notifications render_template helper. Ticket creation goes through TicketService::create_ticket so the result lands with validation, automation, audit log, and the notifications dispatch wire-up shipped in PMS-92.
  • GET /api/v1/rmm/connections/{id} and PUT /api/v1/rmm/connections/{id} land. PUT uses COALESCE so missing fields leave the existing column untouched; api_key / api_secret only re-encrypt when the payload supplies them.
  • tests/rmm.rs covers the full loop:
    • one worker tick against a static mock provider (asserts mapping + asset + audit + sync_status)
    • alert ingest routes through TicketService (asserts T-prefixed ticket_number + template-rendered title/description)
    • low-severity alert suppressed before any ticket
    • second alert inside dedupe window does not open a new ticket
    • canary-based encryption guard mirrors the PMS-92 regression test
  • CI: .forgejo/workflows/check.yml adds --test rmm to the integration-tests step.

Test plan

  • CI green: fmt, clippy, compile, unit tests, integration tests including new --test rmm.
  • Boot the dev stack (just dev) and confirm both notifications dispatcher worker started and rmm sync worker started log at startup.
  • Seed an active rmm_connections row pointing at a real Tactical RMM (or a stub); confirm the worker flips sync_status to success or failed within ~60s and stamps last_sync_at.
  • POST to /api/v1/rmm/alerts with a valid HMAC signature against a rule that has suppression_rules.min_severity set; confirm a low-severity alert produces no ticket and a critical one produces exactly one ticket through TicketService (visible in tickets.source = 'rmm' with a sequenced ticket_number).
  • PUT to /api/v1/rmm/connections/{id} with only is_active = false; confirm api_key_encrypted does NOT change in the database (PUT preserves credentials when omitted).
  • After merge, set PMS-103 to "Done" in YouTrack. PMS-100 closes once all five original AC items pass.

#PMS-103

## Summary Closes the five PMS-100 blockers that the story-verification pass surfaced - device-sync worker, RmmProvider trait, real encryption key, suppression + dedupe + TicketService routing on alert ingest, missing GET/PUT connection endpoints - in one PR so the worker ships with the integration it actually needs. Mirrors the PMS-92 shape used for the notifications dispatcher. - `RmmService::new(db)` is gone; the only constructors are `with_encryption_key(db, key)` and `with_dependencies(db, key, ticket_service)`. `src/api/router.rs:110` now wires the real `encryption_key` (same value already threaded into `BillingService` + `NotificationsService`) plus a `TicketService` clone, so `rmm_connections.api_key_encrypted` / `api_secret_encrypted` stop being encrypted under `[0u8; 32]` and alert-ingest tickets stop bypassing the canonical ticket-creation path. - `RmmProvider` trait lives in `src/modules/rmm/provider.rs` with `list_devices` / `list_alerts` / `ping`. `TacticalRmmProvider` is the first real implementation (Token auth against `/agents/` and `/alerts/`). `mesh_central` / `datto` / `connectwise` / `ninja_rmm` fall back to a fail-loud `UnimplementedProvider` so an operator using one of those sees a real `last_error` instead of silent no-ops. - `migrations/007_rmm_mesh_central_provider.sql` adds `mesh_central` to the `rmm_connections.provider` CHECK constraint so PMS-105's "MeshCentral provider variant" claim matches the schema. - `RmmSyncWorker` (`src/modules/rmm/worker.rs`) spawned from `src/main.rs` next to the notifications `DispatcherWorker`. Tick is 60s; per active connection past its `sync_interval_minutes` window the worker decrypts credentials, builds the right provider, pulls devices, UPSERTs `rmm_device_mappings` on `(rmm_connection_id, rmm_device_id)`, match-or-creates `assets` (serial -> hostname order), stamps `last_sync_at`, writes `asset_audit_log` with `action='created'` or `'synced'`, and flips `sync_status` to `success` (clearing `last_error`) or `failed` (storing the error). Errors per connection are isolated so one bad provider does not stall the rest. - `ingest_alert` at `src/modules/rmm/service.rs:296-393` consults `rule.suppression_rules.min_severity` and `rule.suppression_rules.dedupe_window_minutes` before creating a ticket. `ticket_template` JSONB is rendered via the notifications `render_template` helper. Ticket creation goes through `TicketService::create_ticket` so the result lands with validation, automation, audit log, and the notifications dispatch wire-up shipped in PMS-92. - `GET /api/v1/rmm/connections/{id}` and `PUT /api/v1/rmm/connections/{id}` land. PUT uses `COALESCE` so missing fields leave the existing column untouched; `api_key` / `api_secret` only re-encrypt when the payload supplies them. - `tests/rmm.rs` covers the full loop: - one worker tick against a static mock provider (asserts mapping + asset + audit + sync_status) - alert ingest routes through TicketService (asserts T-prefixed ticket_number + template-rendered title/description) - low-severity alert suppressed before any ticket - second alert inside dedupe window does not open a new ticket - canary-based encryption guard mirrors the PMS-92 regression test - CI: `.forgejo/workflows/check.yml` adds `--test rmm` to the integration-tests step. ## Test plan - [ ] CI green: fmt, clippy, compile, unit tests, integration tests including new `--test rmm`. - [ ] Boot the dev stack (`just dev`) and confirm both `notifications dispatcher worker started` and `rmm sync worker started` log at startup. - [ ] Seed an active `rmm_connections` row pointing at a real Tactical RMM (or a stub); confirm the worker flips `sync_status` to `success` or `failed` within ~60s and stamps `last_sync_at`. - [ ] POST to `/api/v1/rmm/alerts` with a valid HMAC signature against a rule that has `suppression_rules.min_severity` set; confirm a low-severity alert produces no ticket and a critical one produces exactly one ticket through `TicketService` (visible in `tickets.source = 'rmm'` with a sequenced `ticket_number`). - [ ] PUT to `/api/v1/rmm/connections/{id}` with only `is_active = false`; confirm `api_key_encrypted` does NOT change in the database (`PUT` preserves credentials when omitted). - [ ] After merge, set PMS-103 to "Done" in YouTrack. PMS-100 closes once all five original AC items pass. #PMS-103
feat(rmm): sync worker, provider trait, prefs/dedupe, encryption key wire-up
Some checks failed
Check / fmt + clippy + compile + tests (pull_request) Failing after 9s
Build OCI container / Build and push mokosh-api image (push) Has been cancelled
184099244e
Closes the five PMS-100 blockers that the story-verification pass surfaced, in a single PR so the device-sync worker ships with the integration it needs to drain real traffic. Mirrors the PMS-92 shape on notifications.

- Drop RmmService::new(); only with_encryption_key / with_dependencies survive. Router constructed RmmService with [0u8; 32], so rmm_connections.api_key_encrypted / api_secret_encrypted were ciphertext under a zero key (effectively plaintext). Router now threads the real encryption_key (same value already routed to BillingService + NotificationsService) plus a TicketService clone so alert ingest can use the canonical ticket creation path.
- RmmProvider trait (src/modules/rmm/provider.rs) abstracts the per-platform HTTP dialect. TacticalRmmProvider implements list_devices / list_alerts / ping against Tactical RMM's /agents/ and /alerts/ endpoints with Token auth. build_provider() returns a fail-loud UnimplementedProvider for mesh_central / datto / connectwise / ninja_rmm so operators see a clear last_error instead of silent no-ops.
- Migration 007_rmm_mesh_central_provider.sql adds mesh_central to the rmm_connections.provider CHECK constraint so PMS-105 (variant marked resolved) actually matches reality.
- RmmSyncWorker (src/modules/rmm/worker.rs) spawned from main.rs. Per-tick (60s) it picks every active connection past its sync_interval_minutes window, decrypts credentials, builds the right provider, list_devices() and UPSERTs rmm_device_mappings on (rmm_connection_id, rmm_device_id), match-or-creates assets via serial -> hostname order, stamps last_sync_at, writes asset_audit_log with action='created' or 'synced', and flips the connection's sync_status to 'success' (with last_error cleared) or 'failed' (with the error stored). Errors per connection are isolated so one bad provider does not stall the rest. sync_one(...) is public so the integration test can drive a deterministic tick.
- ingest_alert at src/modules/rmm/service.rs now consults rule.suppression_rules.min_severity and rule.suppression_rules.dedupe_window_minutes before creating a ticket. Suppressed / deduped alerts are dropped after a debug log. ticket_template JSONB is rendered via the notifications module's render_template helper (exported as crate::modules::notifications::render_template). Ticket creation routes through TicketService::create_ticket so the result lands with validation, automation, audit log, and the notifications dispatch wire-up shipped in PMS-92. Legacy direct-INSERT path kept under a tickets = None fallback for old test fixtures.
- GET /api/v1/rmm/connections/{id} + PUT /api/v1/rmm/connections/{id} land. PUT uses COALESCE so missing fields leave the existing column untouched; api_key / api_secret only re-encrypt when explicitly supplied.
- tests/rmm.rs covers the full loop: one worker tick against a static mock provider asserts mapping, asset creation with serial + last_sync_at + rmm_device_id, asset_audit_log row, and connection sync_status='success'. Alert ingest tests assert ticket landed via TicketService (T-prefixed ticket_number from the sequence, template-rendered title + description), suppression below min_severity drops the alert, and a second alert inside the dedupe window does not open a new ticket. Encryption guard: POST a canary in api_key and assert the persisted api_key_encrypted bytes do NOT contain it verbatim.
- CI: .forgejo/workflows/check.yml adds --test rmm to the integration-tests step.

#PMS-103
style: apply rustfmt to PMS-103 rmm sync worker work
Some checks failed
Check / fmt + clippy + compile + tests (pull_request) Failing after 27s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m22s
6aefab399e
CI fmt rewrapped a handful of long single-line expressions, expanded chained Option closures, and split long assert! / assert_eq! calls into multi-line form. Functional behavior unchanged.

#PMS-103
style: drop unused RmmService accessor methods
All checks were successful
Check / fmt + clippy + compile + tests (pull_request) Successful in 47s
Create release / Create release from merged PR (pull_request) Has been skipped
Build OCI container / Build and push mokosh-api image (push) Successful in 7m30s
0dfea6dbce
The encryption_key() and db() accessors were added speculatively for the sync worker, but the worker takes Database + key directly via RmmSyncWorker::new instead. Clippy's -D dead-code flags them; remove.

#PMS-103
nrupard deleted branch feat/pms-103-rmm-sync-worker 2026-06-04 17:33:43 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!62
No description provided.