chore: merge executor worktree (worktree-agent-a30d26dba3410e0da) — plan 24-04
This commit is contained in:
commit
d00c47ecb1
4 changed files with 588 additions and 3 deletions
|
|
@ -0,0 +1,182 @@
|
|||
---
|
||||
phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud
|
||||
plan: 04
|
||||
subsystem: aws-route53
|
||||
tags: [route53, integration-health, dns, node-dns, delegation-check]
|
||||
dependency-graph:
|
||||
requires:
|
||||
- "lib/services/route53-factory.ts (isRoute53Configured / getRoute53Client, plan 24-01)"
|
||||
- "lib/types/route53.ts (plan 24-01)"
|
||||
- "migrations/102_route53_tables.sql (route53_zones, plan 24-01)"
|
||||
provides:
|
||||
- "lib/services/route53-dns-delegation.ts (normalizeNsList / compareNsDelegation / resolveLiveNs / checkAllZoneDelegations)"
|
||||
- "checkRoute53() registered in lib/services/integration-health.ts"
|
||||
- "HealthStatus union gained 'degraded' member"
|
||||
- "IntegrationHealth gained nsDelegationMismatches? / nsDelegationErrors? fields"
|
||||
affects:
|
||||
- "app/api/dashboard/integration-health/route.ts (now returns a route53 entry)"
|
||||
- "/admin/integrations and /status pages (consume the health list; both already fall back safely on an unrecognized status string)"
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns:
|
||||
- "dedicated dns.Resolver() instance pinned to public resolvers, never the process-global dns module (D-12, T-24-13)"
|
||||
- "bounded-concurrency worker-pool batch check (checkAllZoneDelegations, T-24-14)"
|
||||
key-files:
|
||||
created:
|
||||
- lib/services/route53-dns-delegation.ts
|
||||
- lib/services/route53-dns-delegation.test.ts
|
||||
modified:
|
||||
- lib/services/integration-health.ts
|
||||
decisions:
|
||||
- "Used Node's dns module directly (Resolver + setServers(['1.1.1.1','8.8.8.8'])), not the DoH-over-HTTPS fallback — plan 24-01's checkpoint confirmed EGRESS-OK from inside the pulse-app container."
|
||||
- "Chosen degraded-state HealthStatus member: added a new 'degraded' member to the union (no existing member represented 'reachable, authenticated, but a secondary check found a problem') — every existing member was either a full pass ('ok'/'unknown'), a hard failure ('auth_failed'/'unreachable'/'not_configured'), or operator-suppressed ('disabled')."
|
||||
- "Implemented a local, private sanitizeAwsError()/isAwsAuthError() inside integration-health.ts instead of importing from lib/services/route53-record-validation.ts — that file is scoped to plan 24-03, which runs in a sibling parallel worktree and was not available in this isolated worktree (24-04's depends_on only lists 24-01). See Deviations below."
|
||||
metrics:
|
||||
duration: "~45 min, 2 tasks, TDD RED/GREEN on Task 1"
|
||||
completed: "2026-08-05"
|
||||
---
|
||||
|
||||
# Phase 24 Plan 4: Route 53 Integration Health + D-12 DNS Delegation Check Summary
|
||||
|
||||
Added `checkRoute53()` to the existing integration-health aggregator (auth probe via
|
||||
`ListHostedZonesCommand`) plus a D-12-specific extension: every synced hosted zone's
|
||||
Route-53-authoritative name servers are compared against a **live public DNS lookup**
|
||||
(dedicated `dns.Resolver()` pinned to `1.1.1.1`/`8.8.8.8`, never the process-global
|
||||
resolver) and a mismatch degrades the reported health to a new `'degraded'`
|
||||
`HealthStatus` member — no manually-maintained "expected NS" field anywhere.
|
||||
|
||||
## What Was Built
|
||||
|
||||
**Task 1 — `lib/services/route53-dns-delegation.ts` (TDD RED/GREEN):**
|
||||
- Pure half, fully unit-tested (13/13 assertions, no network I/O in tests):
|
||||
- `normalizeNsList(input: unknown): string[]` — lowercase, strip trailing dot,
|
||||
de-dupe, sort; `[]` for `null`/`undefined`/non-array input.
|
||||
- `compareNsDelegation(authoritative, live)` — set-diff mismatch detection.
|
||||
Empty authoritative list is treated as unjudgeable (`mismatch: false`, no false
|
||||
alarm); a non-empty authoritative list with an empty live answer is a real
|
||||
delegation problem (`mismatch: true`).
|
||||
- I/O half (not unit-tested per plan instruction — network calls are flaky in CI;
|
||||
covered by manual verification in 24-VALIDATION.md):
|
||||
- `resolveLiveNs(domain, timeoutMs=5000)` — constructs `new Resolver()` (callback
|
||||
API from `'dns'`) and calls `.setServers(['1.1.1.1','8.8.8.8'])` **on that
|
||||
instance only**. The process-global `dns.setServers()` is never called anywhere
|
||||
in this file (T-24-13) — verified by grep in the acceptance criteria.
|
||||
- `checkAllZoneDelegations(zones, opts?)` — bounded-concurrency (default 5) worker
|
||||
pool over the zone list; a lookup error yields `{ mismatch: false, error }` (an
|
||||
unreachable resolver is an infra problem, not delegation drift — T-24-14); zones
|
||||
whose `authoritativeNameServers` normalizes to `[]` are skipped.
|
||||
|
||||
**Task 2 — `checkRoute53()` registered in `lib/services/integration-health.ts`:**
|
||||
- `IntegrationHealth` gained two optional fields: `nsDelegationMismatches?: string[] | null`
|
||||
and `nsDelegationErrors?: string[] | null` (kept separate from `error`, per 24-PATTERNS.md).
|
||||
- `checkRoute53()` placed next to `checkDattoRmm()`: config gate via `isRoute53Configured()`
|
||||
→ `getRoute53Client().send(new ListHostedZonesCommand({ MaxItems: 1 }))` auth probe,
|
||||
timed for `latencyMs`. Auth errors (`InvalidClientTokenId`, `SignatureDoesNotMatch`,
|
||||
`AccessDenied`, `UnrecognizedClientException`, or HTTP 401/403 via `$metadata.httpStatusCode`)
|
||||
map to `status: 'auth_failed'`; any other error maps to `'unreachable'`. Both branches
|
||||
redact the error through a local `sanitizeAwsError()` before it reaches `IntegrationHealth.error`.
|
||||
- D-12 delegation step: `SELECT id, name, authoritative_name_servers FROM route53_zones
|
||||
WHERE is_deleted = false ORDER BY name LIMIT 50`, fed to `checkAllZoneDelegations()`.
|
||||
A non-empty mismatch list downgrades `status` from `'ok'` to the new `'degraded'`
|
||||
member and sets a summary `error` string (`'NS delegation mismatch for N zone(s): ...'`,
|
||||
noting truncation if the 50-zone cap was hit). The entire delegation step is wrapped
|
||||
in its own try/catch — a Postgres failure or blocked resolver degrades to
|
||||
`nsDelegationErrors` and leaves the auth-probe status untouched, never throwing out
|
||||
of `checkIntegrationHealth()`'s `Promise.all` (T-24-16).
|
||||
- Registered `checkRoute53(),` as a bare (unwrapped) entry in the `Promise.all` array
|
||||
alongside `checkAutotask()` / `checkDattoRmm()` / `checkItglue()` / `checkS1()`.
|
||||
- No changes to `applyDisableOverlay()` — it already keys off `item.key` generically,
|
||||
so `'route53'` is covered automatically (D-10, display-only; `integration_settings`
|
||||
grep count unchanged from before this task).
|
||||
- **Bug fix (Rule 1):** `summarize()`'s status-bucketing `if/else if` chain didn't
|
||||
account for the new `'degraded'` status — it would have silently fallen through
|
||||
uncounted (not `ok`, not `failed`, not `notConfigured`), breaking the invariant that
|
||||
bucket counts sum to `total`. Added `'degraded'` to the `failed` bucket (and thus
|
||||
`hasIssues`) alongside `'auth_failed'`/`'unreachable'`.
|
||||
- **Type fix (Rule 3):** the plan's example passed `MaxItems: '1'` (string) to
|
||||
`ListHostedZonesCommand`; this SDK version (`@aws-sdk/client-route-53` ^3.1104.0)
|
||||
types `MaxItems` as `number`. Changed to `MaxItems: 1`.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 1 - Bug] `summarize()` didn't bucket the new `'degraded'` status**
|
||||
- Found during: Task 2, after adding `'degraded'` to `HealthStatus`.
|
||||
- Fix: added `'degraded'` to the existing `failed`-bucket branch.
|
||||
- Files modified: `lib/services/integration-health.ts`
|
||||
- Commit: ea04672
|
||||
|
||||
**2. [Rule 3 - Blocking issue] `ListHostedZonesCommand({ MaxItems: '1' })` failed `tsc`**
|
||||
- Found during: Task 2 verification (`npx tsc --noEmit`).
|
||||
- Issue: this SDK version types `MaxItems` as `number`, not `string` as the plan's
|
||||
action text described.
|
||||
- Fix: `MaxItems: 1`.
|
||||
- Files modified: `lib/services/integration-health.ts`
|
||||
- Commit: ea04672
|
||||
|
||||
**3. [Rule 3 - Blocking issue] `lib/services/route53-record-validation.ts` (plan 24-03)
|
||||
does not exist in this worktree**
|
||||
- Found during: Task 2, reading `<read_first>` / `<action>` which reference
|
||||
`sanitizeAwsError` from that file.
|
||||
- Why: this plan's `depends_on` frontmatter lists only `24-01`; plan 24-03 (which owns
|
||||
`sanitizeAwsError`) runs concurrently in a sibling parallel worktree in this same wave
|
||||
and is not merged into this worktree's history.
|
||||
- Fix: implemented a local, private `sanitizeAwsError()` + `isAwsAuthError()` pair
|
||||
inside `lib/services/integration-health.ts`, using the **identical redaction rules**
|
||||
spec'd in 24-03-PLAN.md (`AKIA[0-9A-Z]{16}` → `[redacted-key-id]`, `arn:aws:[^\s"']+`
|
||||
→ `[redacted-arn]`, 12-digit account ids → `[redacted-account-id]`, truncate to 500
|
||||
chars) so behavior is consistent regardless of which implementation ships. Documented
|
||||
inline with a NOTE comment pointing at this deviation.
|
||||
- **Follow-up for a human/future plan:** once 24-03 lands on `master`, the local copy in
|
||||
`integration-health.ts` should be replaced with an import from
|
||||
`@/lib/services/route53-record-validation` to keep a single source of truth — flagging
|
||||
this explicitly since it is a cross-plan duplication introduced by parallel worktree
|
||||
execution, not by design.
|
||||
- Files modified: `lib/services/integration-health.ts`
|
||||
- Commit: ea04672
|
||||
|
||||
### Verification Note (not a deviation)
|
||||
|
||||
The plan's Task 2 acceptance criteria includes an optional curl check against the running
|
||||
`pulse-app` container's `/api/dashboard/integration-health` route. That container
|
||||
(confirmed via `docker inspect`) has no source-code volume mount — it runs a pre-built
|
||||
standalone image from before this plan's commits, and the route additionally redirects
|
||||
unauthenticated requests (307) per `middleware.ts`. Live end-to-end verification against
|
||||
the running container was therefore not performed in this worktree; `npx tsc --noEmit`,
|
||||
`npx vitest run lib/services/route53-dns-delegation.test.ts` (13/13 passing), and the full
|
||||
`npm test` suite (480/482 passing — the 2 failures are pre-existing/unrelated, see below)
|
||||
are the verifications actually run. A container rebuild + authenticated curl is left to
|
||||
the orchestrator/human at merge time if desired.
|
||||
|
||||
### Out-of-Scope Discovery (logged, not fixed)
|
||||
|
||||
`npm test` (full suite) surfaced the same 2 pre-existing failures in
|
||||
`lib/services/analyzer/itglue-search.test.ts` already logged in this phase's
|
||||
`deferred-items.md` by plan 24-01. Neither that file nor `itglue-search.ts` were touched
|
||||
by this plan.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
All created/modified files confirmed present:
|
||||
- FOUND: lib/services/route53-dns-delegation.ts
|
||||
- FOUND: lib/services/route53-dns-delegation.test.ts
|
||||
- FOUND: lib/services/integration-health.ts (modified)
|
||||
|
||||
All commits confirmed present in `git log`:
|
||||
- 7396f07 test(24-04): add failing test for NS normalization and delegation comparison
|
||||
- 06ebae5 feat(24-04): implement NS normalization and delegation-comparison module
|
||||
- ea04672 feat(24-04): register checkRoute53() in the integration health aggregator
|
||||
|
||||
## TDD Gate Compliance
|
||||
|
||||
Task 1 followed RED → GREEN: `test(24-04)` commit (7396f07) precedes the `feat(24-04)`
|
||||
implementation commit (06ebae5); no REFACTOR commit was needed (implementation matched
|
||||
the test contract on first pass). Task 2 is `type="auto"` without `tdd="true"` per the
|
||||
plan, so no RED/GREEN gate applied there — verified with `tsc` + full `npm test` instead.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None beyond what's already covered by this plan's own `<threat_model>` (T-24-13, T-24-14,
|
||||
T-24-03, T-24-15, T-24-16 — all addressed as designed, see "What Was Built" above). No new
|
||||
network endpoints, auth paths, or schema changes were introduced outside that register.
|
||||
Loading…
Add table
Add a link
Reference in a new issue