docs(fleet): add remote access runbook - #1152
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR adds comprehensive documentation for fleet remote access infrastructure, establishing canonical runbooks and operational procedures for Tailscale ACL enforcement, RustDesk transport, and KVM2 watcher integration with NATS publishing. Additionally, it expands environment variable support for Tailscale API key credential handling across shared configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is still clean. I came back expecting to find something — anything — to roast in the incremental diff, and instead the author went and fixed all four review findings in two surgical commits. That's annoying. How am I supposed to maintain my curmudgeon credentials? The The new The 📊 Overall: Like watching someone actually read the review feedback and apply it correctly. Deeply unsettling. Ship it. Files Reviewed (14 files, 2 incremental)Original review (12 files):
Incremental review (2 new files + 5 modified):
Reviewed by glm-5.1 · incremental from 1acaf22 → 1084e81 Reviewed by glm-5.1 · 341,216 tokens |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
pmoves/docs/AGENTS/CODEX_ECOSYSTEM_TRAVERSAL.md (1)
167-167: Minor readability tweak recommended for the new heuristic line.This bullet is a bit long/dense; splitting it into two shorter clauses will make scan-time faster in the heuristics block.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/docs/AGENTS/CODEX_ECOSYSTEM_TRAVERSAL.md` at line 167, Split the long heuristic bullet into two shorter clauses: first state the trigger and immediate remediation steps ("If the task touches VPN mesh, remote desktop, stale-node cleanup, or VPS onboarding, start with FLEET_REMOTE_ACCESS_RUNBOOK.md, then RUSTDESK_SELF_HOSTED.md"), and then a separate clause that describes the final administrative step ("Finish by signing the lane in AGNOTE4482PHI.t1.md"). Update the heuristic line referencing FLEET_REMOTE_ACCESS_RUNBOOK.md, RUSTDESK_SELF_HOSTED.md, and AGNOTE4482PHI.t1.md so it becomes two concise sentences or two bullets for improved readability.pmoves/docs/operations/RUSTDESK_SELF_HOSTED.md (1)
194-199: Clarify NATS_URL configuration guidance.Line 196 states "Set
Environment=NATS_URL=..." but the systemd unit file already includesEnvironment=NATS_URL=nats://nats:pmoves@nats:4222as a default (perpmoves/scripts/fleet/fleet-audit-watcher.service). Consider rephrasing to: "The service defaults toEnvironment=NATS_URL=nats://nats:pmoves@nats:4222; override if your NATS broker is at a different reachable address."📝 Proposed clarification
Operational notes: - Create `/var/log/pmoves` before starting the service. The systemd unit uses `ReadWritePaths=/var/log/pmoves`, and the service will fail early if the directory does not exist. -- Set `Environment=NATS_URL=...` in `fleet-audit-watcher.service` to a broker reachable from KVM2. +- The service defaults to `Environment=NATS_URL=nats://nats:pmoves@nats:4222`. Override this in `fleet-audit-watcher.service` if your NATS broker is at a different reachable address from KVM2. - The repo default NATS config binds port `4222` to localhost only, so the watcher cannot publish remotely until one PMOVES node exposes NATS on a Tailscale-reachable interface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/docs/operations/RUSTDESK_SELF_HOSTED.md` around lines 194 - 199, Update the docs text to clarify that the systemd unit already sets a default NATS_URL and instruct how to override it: state that the service defaults to Environment=NATS_URL=nats://nats:pmoves@nats:4222 (as defined in fleet-audit-watcher.service / pmoves/scripts/fleet/fleet-audit-watcher.service) and only change the Environment value if your NATS broker is at a different reachable address.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/CLAUDE.md:
- Around line 267-279: Add the new fleet-audit-watcher NATS topology and service
entry: in .claude/context/nats-subjects.md document the watcher’s published
subjects (e.g., fleet.audit.watcher.events, fleet.audit.watcher.errors), message
direction (publisher: fleet-audit-watcher, subscribers/consumers), and any
QoS/retry semantics and required ACL tag usage (TAILSCALE_API_KEY vs
TAILSCALE_AUTHKEY); in .claude/context/services-catalog.md add a service entry
for fleet-audit-watcher including exposed port(s) if any or NATS client-only
note, required runtime deps (nats CLI, /var/log/pmoves, NATS broker reachable
via Tailscale), healthcheck endpoint or startup probe guidance, and operational
notes about the default NATS bind (localhost:4222) and implications for KVM2
reachability.
In `@pmoves/docs/operations/FLEET_REMOTE_ACCESS_RUNBOOK.md`:
- Around line 108-130: Update the documentation for the GET
/api/v2/tailnet/{tailnet}/acl entry in FLEET_REMOTE_ACCESS_RUNBOOK.md: verify
against Tailscale's public API whether GET on /api/v2/tailnet/{tailnet}/acl is
supported, and if it is not publicly documented remove the GET example or mark
it explicitly as "undocumented/private/internal" with guidance to avoid relying
on it; keep the POST/PUT examples for policy updates intact and reference the
exact endpoint string GET /api/v2/tailnet/{tailnet}/acl in the note so readers
can see which entry was verified/changed.
In `@pmoves/docs/PMOVES.AI` PLANS/PMOVES_TAILSCALE_STATUS.md:
- Around line 8-17: Update the "implemented" checklist lines to include explicit
evidence links or pointers: for the ACL policy reference the actual artifact
`pmoves/configs/tailscale-acl-policy.json` (and add a permalink or commit SHA),
for the remote-access runbooks add direct anchors or file links to
`../operations/FLEET_REMOTE_ACCESS_RUNBOOK.md`,
`../operations/RUSTDESK_SELF_HOSTED.md`, and `../TAILSCALE_NODE_HYGIENE.md`
showing the verification steps, and for enrollment/KVM2 watcher add links to the
smoke/log artifact or CI job that proves deployment (e.g. a permalink to
`pmoves/scripts/fleet/generate-enrollment.py` run output or the
`fleet-audit-watcher.sh` / `fleet-audit-watcher.service` logs); ensure each
implemented-status line includes the evidence link or path and a one-line
descriptor of what the evidence demonstrates.
In `@pmoves/docs/TAILSCALE_NODE_HYGIENE.md`:
- Around line 43-51: Update the curl examples in TAILSCALE_NODE_HYGIENE.md to
use HTTP Basic auth instead of a Bearer header: replace the Authorization header
lines currently showing -H "Authorization: Bearer $TAILSCALE_API_KEY" with
curl's -u 'tskey-<api-key>:' form for both the devices list request and the
DELETE request (the two curl blocks shown) so the API key is supplied as the
username per Tailscale API requirements.
---
Nitpick comments:
In `@pmoves/docs/AGENTS/CODEX_ECOSYSTEM_TRAVERSAL.md`:
- Line 167: Split the long heuristic bullet into two shorter clauses: first
state the trigger and immediate remediation steps ("If the task touches VPN
mesh, remote desktop, stale-node cleanup, or VPS onboarding, start with
FLEET_REMOTE_ACCESS_RUNBOOK.md, then RUSTDESK_SELF_HOSTED.md"), and then a
separate clause that describes the final administrative step ("Finish by signing
the lane in AGNOTE4482PHI.t1.md"). Update the heuristic line referencing
FLEET_REMOTE_ACCESS_RUNBOOK.md, RUSTDESK_SELF_HOSTED.md, and AGNOTE4482PHI.t1.md
so it becomes two concise sentences or two bullets for improved readability.
In `@pmoves/docs/operations/RUSTDESK_SELF_HOSTED.md`:
- Around line 194-199: Update the docs text to clarify that the systemd unit
already sets a default NATS_URL and instruct how to override it: state that the
service defaults to Environment=NATS_URL=nats://nats:pmoves@nats:4222 (as
defined in fleet-audit-watcher.service /
pmoves/scripts/fleet/fleet-audit-watcher.service) and only change the
Environment value if your NATS broker is at a different reachable address.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4b4f193-0900-4e72-ae27-55095b146ace
📒 Files selected for processing (12)
.claude/CLAUDE.mddocs/SECRETS_ONBOARDING.mdpmoves/docs/AGENTS/AGNOTE4482PHI.t1.mdpmoves/docs/AGENTS/CODEX_CLAUDE_PARITY_MAP.mdpmoves/docs/AGENTS/CODEX_ECOSYSTEM_TRAVERSAL.mdpmoves/docs/AGENTS/CODEX_OPERATOR_HOME.mdpmoves/docs/PMOVES.AI PLANS/PMOVES_TAILSCALE_STATUS.mdpmoves/docs/PMOVES.AI PLANS/README_DOCS_INDEX.mdpmoves/docs/TAILSCALE_NODE_HYGIENE.mdpmoves/docs/operations/FLEET_REMOTE_ACCESS_RUNBOOK.mdpmoves/docs/operations/RUSTDESK_SELF_HOSTED.mdpmoves/env.shared.example
| - `GET /api/v2/tailnet/{tailnet}/devices` — list devices (`devices:core:read`) | ||
| - `DELETE /api/v2/device/{deviceId}` — delete a device (`devices:core`) | ||
| - `GET /api/v2/tailnet/{tailnet}/acl` — fetch policy file (`policy_file:read`) | ||
| - `POST /api/v2/tailnet/{tailnet}/acl` — update policy file (`policy_file`) | ||
|
|
||
| `-` can be used as a shorthand tailnet ID when the API key belongs to the active tailnet. | ||
|
|
||
| Examples: | ||
|
|
||
| ```bash | ||
| curl -fsS \ | ||
| -H "Authorization: Bearer $TAILSCALE_API_KEY" \ | ||
| "https://api.tailscale.com/api/v2/tailnet/-/devices" | ||
|
|
||
| curl -fsS -X DELETE \ | ||
| -H "Authorization: Bearer $TAILSCALE_API_KEY" \ | ||
| "https://api.tailscale.com/api/v2/device/<deviceId>" | ||
|
|
||
| curl -fsS \ | ||
| -H "Authorization: Bearer $TAILSCALE_API_KEY" \ | ||
| -H "Accept: application/hujson" \ | ||
| "https://api.tailscale.com/api/v2/tailnet/-/acl" | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the correct Tailscale API v2 endpoints for listing devices, deleting a device, and getting/updating ACL policy?
💡 Result:
The Tailscale API v2 endpoints are: - List devices: GET https://api.tailscale.com/api/v2/tailnet/{tailnet}/devices - Delete device: DELETE https://api.tailscale.com/api/v2/device/{deviceID} - Get ACL policy: No direct read endpoint found in public documentation; ACL policy is managed via the tailnet policy file through the admin console, GitOps, or potentially undocumented API (Pulumi provider suggests update-only). Update likely via POST/PUT to /api/v2/tailnet/{tailnet}/acl with policy JSON. Base URL: https://api.tailscale.com/api/v2/ Authentication: API key via HTTP Basic auth (tskey-KEY:). Official interactive docs: https://tailscale.com/api
Citations:
- 1: https://tailscale.com/api
- 2: https://tailscale.com/kb/1101/api
- 3: https://stackoverflow.com/questions/74132318/tailscale-list-all-devices-using-api
- 4: https://tailscale.com/docs/features/access-control/device-management/how-to/remove
- 5: https://github.com/tailscale/tailscale/blob/main/api.md
- 6: https://tailscale.com/docs/features/tailnet-policy-file/manage-tailnet-policies
The GET /api/v2/tailnet/{tailnet}/acl endpoint appears to be undocumented in official Tailscale API documentation.
The list devices and delete device endpoints are confirmed correct. However, the GET ACL endpoint is not found in Tailscale's public API documentation—only POST/PUT for updates is documented. This suggests the endpoint may be private or undocumented. Verify that this endpoint actually exists and works for reading ACL policy before documenting it in an operational runbook, or clarify its status (internal API, undocumented, etc.).
🧰 Tools
🪛 LanguageTool
[style] ~113-~113: To form a complete sentence, be sure to include a subject.
Context: ...update policy file (policy_file) - can be used as a shorthand tailnet ID when ...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/docs/operations/FLEET_REMOTE_ACCESS_RUNBOOK.md` around lines 108 -
130, Update the documentation for the GET /api/v2/tailnet/{tailnet}/acl entry in
FLEET_REMOTE_ACCESS_RUNBOOK.md: verify against Tailscale's public API whether
GET on /api/v2/tailnet/{tailnet}/acl is supported, and if it is not publicly
documented remove the GET example or mark it explicitly as
"undocumented/private/internal" with guidance to avoid relying on it; keep the
POST/PUT examples for policy updates intact and reference the exact endpoint
string GET /api/v2/tailnet/{tailnet}/acl in the note so readers can see which
entry was verified/changed.
|
Addressed the current docs review pass in What changed:
Validation note:
|
1 participant
Summary
TAILSCALE_API_KEYadmin-secret contract and the Known Roads rule for translating raw rebuild manifests to make-target bring-upTesting
git diff --checkReviewer Notes
Summary by CodeRabbit
New Features
Documentation
Chores