docs(24): capture phase context

This commit is contained in:
lorentz 2026-08-05 18:31:06 -04:00
parent 5f308f836f
commit 0805e387e3
2 changed files with 362 additions and 0 deletions

View file

@ -0,0 +1,214 @@
# Phase 24: AWS Route 53 DNS Sync - Context
**Gathered:** 2026-08-05
**Status:** Ready for planning
<domain>
## Phase Boundary
Sync DNS hosted zones/records from AWS Route 53 into Postgres on a schedule,
support full CRUD back to Route 53 from Pulse for common record types, track
record-level change history over time (both Pulse-initiated and externally
detected drift), log every sync and CRUD operation for audit (including
failures), and integrate into the existing per-system sync section
(scheduler, `/admin/sync` UI, health checks) alongside Autotask/Datto
RMM/Veeam/PAX8. AWS credentials are resolved via BWS (Bitwarden Secrets
Manager), not plaintext env vars.
</domain>
<decisions>
## Implementation Decisions
### CRUD Scope & Guardrails
- **D-01:** Writable record types are the common set only — A, AAAA, CNAME,
MX, TXT, SRV. NS and SOA are excluded from the write path (zone-delegation
records; editing them risks breaking the zone).
- **D-02:** Records only, not zones. Pulse can create/update/delete records
within hosted zones that already exist in Route 53. Hosted zone
creation/deletion (domain onboarding/decommissioning) stays outside Pulse
(AWS console or infra-as-code).
- **D-03:** Destructive record operations (update/delete) execute
immediately — no phishing-style staged/two-step approval gate. Every
change is logged with actor/timestamp/before/after so mistakes are
traceable after the fact, not blocked beforehand.
- **D-04:** CRUD is gated at `requireAdmin()` (admin + super-admin) — the
same bar as other write-capable admin surfaces in Pulse, not a stricter
super-admin-only gate.
### Change Tracking & Audit Schema
- **D-05:** Dedicated Route 53 tables, not a reuse of the phishing
pipeline's `audit_events` table. New migration introduces
`route53_zones` / `route53_records` / `route53_record_history` /
`route53_audit_log` (naming for planner/researcher to finalize) — mirrors
how Veeam and Datto RMM each own their tables rather than sharing a
cross-domain audit schema.
- **D-06:** Change history is written both for Pulse-initiated CRUD and for
sync-detected drift (a record changed outside Pulse, e.g. directly in the
AWS console). Each history row is tagged with a `source` field:
`pulse_crud` | `sync_detected_drift`, so the query "did someone change
this outside Pulse?" is answerable.
- **D-07:** Failed AWS API attempts (rate-limited, invalid record, AWS-side
error) are also logged in the audit trail — attempted before/after +
error message + `status: failed` — not just successful writes.
- **D-08:** Retention is unbounded — no purge job. Matches existing Pulse
convention; no audit/history table in this codebase currently has an
automatic retention/purge mechanism.
### Admin UI & Sync Integration
- **D-09:** New tile on `/admin/sync` (same list as Veeam/Datto RMM/PAX8)
plus a dedicated `/admin/sync/route53` detail page for zones, records,
and history — the existing per-integration pattern, not folded into an
existing page.
- **D-10:** `/admin/integrations` disable toggle for `route53` is
display-only (suppresses health-check display; scheduler/sync/CRUD keep
working underneath) — the default behavior per CLAUDE.md. Route 53 is
**not** a second PAX8-style exception that blocks sync/writes when
disabled.
- **D-11:** Sync cadence is incremental + periodic full — more frequent
incremental checks plus a daily full reconciliation, rather than a single
daily full sync. Trade-off (more API calls against Route 53 rate limits
for better real-time drift detection) accepted knowingly.
- **D-12:** The health-check row for Route 53 goes beyond the generic
auth-check + last-sync-age pattern used by other integrations — it also
includes a DNS-specific delegation check: compare each hosted zone's
Route-53-authoritative NS records against a **live public DNS lookup**
(e.g. Node's `dns` module or a DoH resolver) for that domain, flagging a
mismatch as degraded health. No manually-maintained "expected NS" field —
the live lookup is itself the source of truth to diff against.
### Claude's Discretion
- **Credentials & AWS account scope** — not discussed interactively (user
deliberately skipped this topic, treating it as already settled). Codebase
scouting found uncommitted infrastructure already in place:
`docker-entrypoint.sh` (new, untracked) plus diffs to `Dockerfile` and
`docker-compose.yml` that install the `bws` CLI and wrap the app's start
command as `bws run --project-id "$BWS_PROJECT_ID" -- node server.js`
when `BWS_ACCESS_TOKEN` is set, falling back to a plain `node server.js`
otherwise. **This means Bitwarden secret injection happens at the
container-entrypoint layer, before the Node process starts** — the app
itself never calls a BWS SDK; AWS credentials simply appear as normal
`process.env` values by the time `getRoute53Client()`-style code runs.
Researcher/planner should: (1) follow the exact existing
`lib/services/<name>-factory.ts` + `is<Name>Configured()` pattern used by
every other integration, reading credentials from `process.env`; (2)
confirm the actual env var names with the user (e.g. `AWS_ACCESS_KEY_ID`
/ `AWS_SECRET_ACCESS_KEY` / `AWS_REGION`, vs a `ROUTE53_*`-prefixed
variant) before finalizing the factory — this wasn't locked in
discussion; (3) add a `ROUTE53_*` (or `AWS_*`) row to CLAUDE.md's
integration env-prefix table once confirmed.
- **AWS account scope** — not discussed. Default assumption for planning
purposes is a single AWS account holding all client hosted zones (the
common MSP pattern), not per-client AWS accounts/cross-account IAM roles.
Flag during research if this assumption looks wrong once the actual AWS
setup is inspected.
- **Exact record-change diff granularity** (whole-recordset replace vs
individual value diffing) — left to researcher/planner, informed by how
the AWS SDK's `ChangeResourceRecordSets` API actually models a record
update.
- **Table/column naming inside the dedicated Route 53 schema** — D-05 locks
"dedicated tables," not literal names; researcher/planner should follow
existing migration conventions (`snake_case`, audit columns
`created_at`/`updated_at`/`synced_at`/`is_deleted`/`deleted_at`).
</decisions>
<canonical_refs>
## Canonical References
**Downstream agents MUST read these before planning or implementing.**
No external specs, ADRs, or docs reference AWS Route 53 anywhere in this
repo — ROADMAP.md's Phase 24 section has no "Canonical refs:" field, and no
seed file covers this integration. Requirements are fully captured in the
decisions above and in ROADMAP.md's Phase 24 Success Criteria.
### Closest existing analogs (not canonical docs, but the patterns to follow)
- `lib/services/veeam-factory.ts`, `lib/services/veeam-sync-service.ts`
factory + sync-service pattern for a full external integration with
scheduler + admin UI + health check
- `lib/services/datto-rmm-factory.ts`, `lib/services/datto-rmm-sync-service.ts`
— second reference implementation of the same pattern
- `lib/services/sync-scheduler.ts``sync_type` union, `ScheduleConfig`,
`defaultSchedules` array to extend for `route53-incremental`/`route53-full`
- `lib/services/integration-health.ts` — health-check aggregator to extend
- `app/admin/sync/page.tsx` — integration tile list (`id`/`category`/
`product`/`description`/`href`/`logo`/`color`) to extend with a `route53`
entry
- `migrations/081_*.sql``integration_settings` table backing the
`/admin/integrations` disable toggle (D-10)
</canonical_refs>
<code_context>
## Existing Code Insights
### Reusable Assets
- `components/admin/DataTable.tsx` — for the zones/records list on the new
`/admin/sync/route53` page
- `components/admin/DetailModal.tsx` — for record detail/history drill-down
(formatted/raw tab pattern already established)
- `app/admin/sync/page.tsx` tile array — extend with a `route53` entry
following the exact shape used for `veeam`/`datto-rmm`/`pax8`
### Established Patterns
- `lib/services/<name>-factory.ts` + `is<Name>Configured()` — credential
lazy-load + config-check pattern every integration follows; Route 53
client should match this exactly (see Claude's Discretion above)
- `lib/services/<name>-sync-service.ts` — incremental/full sync against
`lastTrackedModificationDateTime`-style cursors, batched via
`postgresClient.bulkUpsert()`
- `lib/services/sync-scheduler.ts` — node-cron singleton; new sync types
added to the `sync_type` string union and `defaultSchedules` seed array
- `lib/services/integration-health.ts` — per-integration health check,
cached ~5 minutes, read by both `/admin/integrations` and `/admin/sync`
- `migrations/081_*.sql` `integration_settings` — disable-toggle table with
`disabled_by`/`disabled_at`/`disabled_reason` audit columns; per
CLAUDE.md, PAX8 is currently the only integration where disabling also
blocks sync/writes — Route 53 explicitly does **not** join that list
(D-10)
### Integration Points
- `lib/services/sync-scheduler.ts` — add `route53-incremental` /
`route53-full` to the `sync_type` union and `defaultSchedules`
- `lib/services/integration-health.ts` — add a `route53` health entry
(auth check + last-sync age + the D-12 NS-delegation check)
- `app/admin/sync/page.tsx` — add `route53` tile; new
`app/admin/sync/route53/page.tsx` detail page
- New `app/api/route53/*` routes for zone/record read + CRUD + history
- New numbered migration (planner/researcher to confirm the current highest
migration number — duplicates exist at 002/004/009, alphabetical apply
order per CLAUDE.md) for the dedicated Route 53 tables (D-05)
- `docker-entrypoint.sh` / `Dockerfile` / `docker-compose.yml` — already
modified (uncommitted) to wire BWS secret injection; see Claude's
Discretion above
</code_context>
<specifics>
## Specific Ideas
- Health check must include a **live public DNS lookup** for NS delegation
mismatch (D-12) — not a manually-maintained "expected NS" field.
- Audit trail must capture failed AWS API attempts, not just successful
writes (D-07).
- Change history must distinguish Pulse-initiated changes from
sync-detected external drift via a `source` tag (D-06).
</specifics>
<deferred>
## Deferred Ideas
None — discussion stayed within phase scope. The "Credentials & AWS scope"
gray area was deliberately not discussed interactively (user judged it
already settled by the existing BWS/docker-entrypoint infrastructure) — see
Claude's Discretion above, not treated as out-of-scope or deferred to a
future phase.
</deferred>
---
*Phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud*
*Context gathered: 2026-08-05*

View file

@ -0,0 +1,148 @@
# Phase 24: AWS Route 53 DNS Sync - Discussion Log
> **Audit trail only.** Do not use as input to planning, research, or execution agents.
> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered.
**Date:** 2026-08-05
**Phase:** 24-AWS Route 53 DNS Sync
**Areas discussed:** CRUD scope & guardrails, Change tracking & audit schema, Admin UI & sync integration
---
## CRUD Scope & Guardrails
| Option | Description | Selected |
|--------|-------------|----------|
| Common types only | A, AAAA, CNAME, MX, TXT, SRV — the record types an MSP actually edits day-to-day. NS/SOA excluded. | ✓ |
| All record types | Includes NS/SOA — full parity with the AWS console, higher risk. | |
| Read-only for now | Ship sync + audit log first; defer writable types to a follow-up. | |
**User's choice:** Common types only
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Records only | Zones provisioned/decommissioned outside Pulse; Pulse only CRUDs records within existing zones. | ✓ |
| Zones + records | Pulse can also create/delete whole hosted zones. | |
**User's choice:** Records only
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Immediate, with full audit trail | Executes right away like other admin CRUD; every change logged with actor/before/after. | ✓ |
| Confirmation dialog only | Client-side "Are you sure?" modal, no server-side gate. | |
| Two-step approval gate | Mirrors phishing remediation — staged, second admin approves. | |
**User's choice:** Immediate, with full audit trail
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Admin + super-admin | Same bar as other write-capable admin surfaces (`requireAdmin()`). | ✓ |
| Super-admin only | Tighter gate given DNS-change blast radius. | |
| Any authenticated user | No role restriction. | |
**User's choice:** Admin + super-admin
**Notes:** —
---
## Change Tracking & Audit Schema
| Option | Description | Selected |
|--------|-------------|----------|
| Dedicated Route 53 tables | New `route53_*` tables scoped to this integration, mirrors Veeam/Datto RMM. | ✓ |
| Reuse phishing's audit_events table | Shared cross-domain audit table. | |
**User's choice:** Dedicated Route 53 tables
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Log both, tagged by source | History rows tagged `pulse_crud` / `sync_detected_drift`. | ✓ |
| Only log Pulse-initiated CRUD | Sync silently overwrites current-state tables, no drift history. | |
**User's choice:** Log both, tagged by source
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Log failed attempts too | Audit row captures attempted before/after + error + status=failed. | ✓ |
| Only log successful changes | Failed API calls just console.error'd. | |
**User's choice:** Log failed attempts too
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Unbounded, no purge | Matches existing Pulse convention — no history/audit table currently purges. | ✓ |
| Time-boxed retention | Scheduled purge job for rows older than N months/years. | |
**User's choice:** Unbounded, no purge
**Notes:** —
---
## Admin UI & Sync Integration
| Option | Description | Selected |
|--------|-------------|----------|
| New tile + detail page | `/admin/sync` tile + dedicated `/admin/sync/route53` page — existing Veeam/Datto RMM/PAX8 pattern. | ✓ |
| Fold into an existing page | Attach DNS management to an existing admin section. | |
**User's choice:** New tile + detail page
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Display-only toggle | Disabling suppresses health-check display only; matches CLAUDE.md default for every integration except PAX8. | ✓ |
| Blocks sync + CRUD like PAX8 | Second exception alongside PAX8 — disabling also skips scheduler and 403s writes. | |
**User's choice:** Display-only toggle
**Notes:** —
| Option | Description | Selected |
|--------|-------------|----------|
| Daily full sync | One scheduled job/day, matches pax8-daily/engagement-daily cadence. | |
| Incremental + periodic full | More frequent incremental checks plus daily full reconciliation. | ✓ |
**User's choice:** Incremental + periodic full
**Notes:** Accepted trade-off of more API calls against Route 53 rate limits for better real-time drift detection.
| Option | Description | Selected |
|--------|-------------|----------|
| Generic pattern | Same integration-health.ts shape — auth check + last-sync age. | |
| Add DNS-specific checks | Also flag degraded health on NS delegation mismatch vs registrar. | ✓ |
**User's choice:** Add DNS-specific checks
**Notes:** Follow-up clarified how to determine "expected" NS — see next row.
| Option | Description | Selected |
|--------|-------------|----------|
| Live public DNS lookup | Query a public resolver (DoH or Node `dns` module) for the domain's NS records, diff against Route 53's authoritative set. | ✓ |
| Manual expected-NS field | Admin manually records expected NS per zone; check diffs against stored value. | |
**User's choice:** Live public DNS lookup
**Notes:** No manually-maintained field — the live lookup is itself the source of truth.
---
## Claude's Discretion
- **Credentials & AWS account scope** — user deliberately did not select this
topic for discussion (treated as already settled by existing uncommitted
`docker-entrypoint.sh`/`Dockerfile`/`docker-compose.yml` BWS wiring found
during codebase scouting). Left to researcher/planner to confirm exact
env var names and follow the existing factory pattern. See CONTEXT.md
Claude's Discretion section for the full writeup.
- AWS account scope (single account vs per-client) — not discussed;
defaulted to single-account assumption.
- Exact record-change diff granularity — left to researcher/planner,
informed by the AWS SDK's `ChangeResourceRecordSets` shape.
- Table/column naming inside the dedicated Route 53 schema — locked concept
("dedicated tables"), not literal names.
## Deferred Ideas
None — discussion stayed within phase scope.