From cf89ac4c9125088a5ea20a5f410de825bff6ea4a Mon Sep 17 00:00:00 2001 From: kacerr Date: Wed, 16 Sep 2026 11:13:01 +0000 Subject: [PATCH] Plan the split of the provider blueprint into contract, appendix, template and index Co-Authored-By: Claude Fable 5.1 --- .../2026-09-16-1108-provider-docs-split.md | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 docs/plans/2026-09-16-1108-provider-docs-split.md diff --git a/docs/plans/2026-09-16-1108-provider-docs-split.md b/docs/plans/2026-09-16-1108-provider-docs-split.md new file mode 100644 index 0000000..410bac5 --- /dev/null +++ b/docs/plans/2026-09-16-1108-provider-docs-split.md @@ -0,0 +1,99 @@ +# Plan: Split the provider blueprint into contract, appendix, template and index + +**Created:** 2026-09-16 11:08 + +## Context + +`docs/providers.md` is one 731-line blueprint that mixes the generic provider contract with how kubernetes, gcp and kvm happen to meet it, plus gaps and proposals. The user wants to hand an agent a self-contained set of rules for building a new provider (Proxmox is next), so the generic part must stand alone, state explicitly where providers are allowed to differ (a pod is ready at once, a VM has a provisioning window, kvm blocks in Create, gcp does not), and leave implementation detail to a separate appendix. A third file, a fill-in spec template, lets the user write the new provider's specifics in the same vocabulary. `docs/providers.md` becomes a short index so every existing link keeps working. + +Decisions already made by the user: two new files plus a template; `providers.md` becomes an index; the contract is fully abstract (never names an existing provider); work continues on the checked-out branch `feat/provider-blueprint`. + +Conventions for all four files: code references are `path` plus function or symbol name, never line numbers; rule keywords MUST / SHOULD / MAY as today, and every MUST names the caller and what breaks; no em-dashes in prose. The contract cites only shared-layer files (`internal/provider/*.go`, `internal/provider/registry`, `internal/controller`, `internal/gc`, `internal/providerhealth`, `internal/tracing`, `cmd/main.go`, `config/manager`, `api/v1alpha1`) and never `internal/provider/{kubernetes,gcp,kvm}`. + +## Facts verified against the code that the new contract must carry + +- Callers put no deadline on the context. Only `Monitor.checkOnce` (`internal/providerhealth/monitor.go`) uses `WithTimeout`; the reconcile and GC contexts are cancel-only. `Sweeper.sweep` (`internal/gc/gc.go`) loops providers sequentially, so one hung `ListByTag` stalls orphan GC for every provider. +- The reconciler's Stopped/Terminated branch in `reconcileManaged` (`internal/controller/proxy_controller.go`) deletes and requeues without clearing `ProviderID`, so a backend that reports a freshly created, not-yet-started instance as Stopped gets it deleted right after creation. kvm avoids this only by blocking `Create` through start. +- `Sweeper.sweep` treats a zero `CreatedAt` as collectable now, skips `UID == ""`, and skips anything younger than `MinAge` (10m). +- `cmd/main.go` type-asserts `provider.Checker` on the unwrapped provider before `WithTracing(WithMetrics(...))`; `registry.Build` fails the whole manager on a constructor error, so constructors must not dial. +- Manager runtime: non-root, read-only rootfs, `/tmp` emptyDir 64Mi, 128Mi limit, distroless static image with CA roots (`config/manager/manager.yaml`, `Dockerfile`). +- Corrections to earlier notes, for the appendix: kvm Checker `Details` also carry `user` and `pool`; the kubernetes provider has no seam interface (it injects `client.Client` via `newWithClient` and tests use the fake client); kvm `classify` has an `isClassified` pass-through for errors already wrapped with a class. + +## Files + +### 1. `docs/provider-contract.md` (new, the generic contract) + +Every behaviour axis gets a stable kebab-case code in backticks, used verbatim by the appendix table and the template so the three files can be cross-checked by grep. + +Sections: + +1. **Preamble**: audience, reading order (contract, template, appendix), rule keywords, "path plus symbol, never line numbers", where the rest lives. +2. **What a provider is and who calls it**: the types in `provider.go`, `name.go`; callers and entry points (`reconcileManaged`, `replaceInstance`, `reconcileDelete`, `providerFailure`, `deletionFailure`, `placementFrom`, poll intervals, `MaxConcurrentReconciles`; `Sweeper.sweep` and `MinAge`; `Monitor.checkOnce`; decorators; `registry.Build`); what the instance is for (health probe CONNECT in `internal/health`, discovery handing out ip and port); leader-only vs every-replica; concurrency (MUST be safe, including two calls for one name); runtime envelope; time and cancellation (no caller deadline, MUST honour cancel, SHOULD bound each backend call, what breaks: a pinned worker, a stalled GC sweep, a wedged check round). +3. **The contract, method by method**: current §2 rewritten abstractly, plus the new points. Create: idempotent by name, mark on create or within a documented window shorter than `MinAge`, self-contained ID, ctx, blocking vs submit, reject-early with `ErrPermanent` naming field and value, catalogue changes are outside the spec hash (`spechash.go`), MAY ignore Placement fields but MUST document, SHOULD deliver CloudInit, MUST NOT log it, and an eventually consistent backend MUST NOT let `Get` answer NotFound for an ID `Create` just returned. Get: NotFound only when gone, keep answering during delete, Running only with IP, Stopped/Terminated only when it will not recover, the "defined but not started" rule (block Create through start, or report Provisioning using written metadata or a task handle), behaviour during async operations and locks (last known state or Transient, never NotFound or Permanent), UID and CreatedAt read back, Permanent for an unresolvable ID. Delete: idempotent, re-entrant while deleting, removes secondary resources, MAY return before gone, SHOULD succeed where Get fails permanently. ListByTag: marked instances only with ID, UID, CreatedAt; error rather than an unvouched list; MAY omit unreadable parts; SHOULD cover the whole creatable scope; no installation identity. +4. **Behaviour profiles**: one subsection per axis with the question, allowed profiles, what callers tolerate, obligations per profile. Axes: `readiness`, `create-return`, `delete-return`, `ip-source`, `gc-mark`, `created-at`, `list-partial`, `quota`, `not-found`, `primary-key`, `provisioning-state`, `async-ops`, `secondary-resources`, `credentials`, `client-kind`, `timeouts`, `logging`, `child-spans`, `backend-test`. Closing table: code, profiles, tolerated by which caller, template entry. +5. **Error taxonomy**: `Wrap`, `Class`, `Error` fields, `Op` values, configured name not type; the class-to-reaction table; pitfalls; NotFound detection guidance (404, error code, list-and-miss, and why list-and-miss is forbidden with a partial list); the quota-vs-permission 403 case stated abstractly. +6. **Identity, names, marks, CreatedAt**: `NameFromUID` shape, providerID strict parsing, marks and written metadata, provider name as DNS-1123 (`Monitor` skip, `spec.provider` MaxLength), primary key not the name. +7. **Credentials, TLS and secret hygiene** (new): source and read time (constructor MUST NOT dial, SHOULD warn on a missing optional mount and fail at first use), never in logs, spans, error strings or `Error.Err`, TLS trust options and the naming of an insecure toggle, scratch only under `/tmp`. +8. **Optional interfaces: Checker**: current §5 rules; `status.type` comment in `api/v1alpha1/providerhealth_types.go` must list the new type and the CRD be regenerated. +9. **Observability**: the three layers, span and metric names, ctx and logger rules, child-span strategies by `client-kind`, no secrets or CloudInit in attributes, log tiers. +10. **Configuration and wiring**: `ProviderConfig`, `setBlocks`, the allow-list in `Config.validate`, `UnmarshalStrict`, `config_.go`, identifiers pasted into IDs, `registry.Constructor`, the map in `cmd/main.go`, decorator order, the duplicated type list. +11. **Testing obligations**: fake behind a seam; the contract test list (current checklist item 5 plus Get during an async operation, the not-yet-started state, NotFound after Create is forbidden); classify table test; Checker tests; sentinel tests for CloudInit and credentials; span recorder test; integration test behind `//go:build integration` with `_TEST_*` variables, `make test-integration-`, a section in `docs/testing.md`. +12. **Checklist for adding a provider**: current §8 generalised, plus: Makefile target, testing.md section, `hack/providers-dev.yaml` decision, `ProviderHealth` type comment and `make manifests`, RBAC diff, CHANGELOG, appendix row and section, the filled template committed as `docs/-provider-spec.md`, README row, architecture.md rows. +13. **Known limits of the contract**: adoption unverified; Stopped and Terminated indistinguishable; config read once; no installation identity in GC; unconfigured provider latches; type list duplicated; Delete resubmitted every poll; no caller deadline; no conformance suite. +14. **Related documents**. + +### 2. `docs/provider-spec-template.md` (new, the questionnaire) + +Preamble: copy to `docs/-provider-spec.md`, fill every Answer line, hand it to the implementing agent with the contract and appendix; unanswered entries block implementation. Header block: type, example configured name, backend and API version, SDK or thin client, author, date. + +Fixed entry format: Question; Allowed answers (profile names linking to the contract §4 subsection); What each answer obliges; `Answer:` blank; `Evidence:` blank. + +Entries: identity and providerID shape; primary key (`primary-key`); placement mapping (Region, Zone, MachineType, Image, ignored fields); image source; cloud-init delivery; machine type mapping; network and IP discovery (`ip-source`, `readiness`); GC marks and CreatedAt (`gc-mark`, `created-at`); state mapping table incl. not yet started (`provisioning-state`); Create blocking and task tracking (`create-return`, `async-ops`); Delete and secondary resources (`delete-return`, `secondary-resources`); ListByTag scope and partial failure (`list-partial`); error classification table (`not-found`, `quota`); credentials and TLS (`credentials`); config block fields, defaults, validation; Checker targets, reasons, Details; SDK vs thin client and child spans (`client-kind`, `child-spans`); timeouts (`timeouts`); concurrency; logging tiers (`logging`); tests (`backend-test`); deployment (mounts, samples, Makefile helper, RBAC, dev config); docs; known limits accepted at design time. + +### 3. `docs/providers-existing.md` (new, the appendix) + +Sources: current `providers.md` §6 table, §7 matrix and per-provider notes, §9 per-provider gaps and proposed features, `docs/kvm-provider.md`, the gcp and kubernetes package facts. Links to architecture.md §5, §11 and Decisions, and to the README table, rather than duplicating them. + +Sections: preamble; **profile table** with exactly one row per contract axis code and one column per existing provider, each cell a profile name plus a symbol as evidence; **other differences** (matrix rows that are not axes: Placement used, Port used, CloudInit delivered, states produced, Error.Provider correctness, credential rotation, connection handling, Checker Details, fakes, proxy software, reachability, distinct egress, adopted-UID check); **observability status** (current §6 table); one section each for **kubernetes**, **gcp**, **kvm** with the same sub-headings: package layout and seam, config block, ID format, Create, Get and state mapping table, IP source, Delete, ListByTag, classify table, Check (targets, reasons, Details), logging, tracing layer, tests and fakes, deployment bits, known gaps; **gaps shared by the existing providers**; **proposed features** (current list, unchanged). + +### 4. `docs/providers.md` (rewritten in place as the index) + +One paragraph on the split; one line each for contract, template, appendix; the per-provider guides (`docs/kvm-provider.md`, `docs/gcp-vm-validation.md`, `docs/gcp-in-specific-project.md`); the architecture.md sections; reading order for a new provider; a note that the pre-split blueprint is in git history under this path. + +### 5. One-line link edits + +- `README.md` "### Adding a provider": point at the contract, appendix and template, with `docs/providers.md` named as the index. Keep the "Start there" sentence. +- `CLAUDE.md` "## Architecture": same replacement for the providers.md sentence. +- `docs/architecture.md` components row "Provider contract": rules in `provider-contract.md`, profile table in `providers-existing.md`. +- Historical files under `docs/plans*` keep their links; they resolve to the index. + +## Steps (one commit per step, on `feat/provider-blueprint`) + +- **Step 0**: copy this plan to `docs/plans/-provider-docs-split.md` with the real timestamp and commit it alone, before anything else. +- **Step 1**: write `docs/provider-contract.md`; freeze the axis code list. Create `docs/plans-executions/-provider-docs-split.md` with the status checklist. +- **Step 2**: write `docs/provider-spec-template.md` from contract §4 and §12. +- **Step 3**: write `docs/providers-existing.md`, replacing every `file:line` from the old blueprint with `file` plus symbol. +- **Step 4**: rewrite `docs/providers.md` as the index; apply the three link edits. +- **Step 5**: run the verification below; fix findings. +- **Step 6**: final execution summary entry. CHANGELOG entry at the top only after the user confirms. Push and MR only when the user asks (per memory, push uses the token helper and the MR goes through the Gitea API). + +## Verification (read-only shell, from the repo root) + +1. Every MUST names a caller: `grep -n MUST docs/provider-contract.md` and confirm each bullet cites a shared-layer path or symbol and states what breaks. +2. Every cited path exists: extract `(internal|cmd|config|docs|api|hack|test)/...` tokens from the four files and `test -e` each. +3. Every cited symbol exists: extract backticked identifiers, drop paths, keywords and YAML keys, and `grep -rqw` each across `*.go`, `Makefile`, `*.yaml`. +4. Contract is abstract: `grep -niE "kubernetes|gcp|kvm|libvirt|squid|google|compute engine|internal/provider/(kubernetes|gcp|kvm)" docs/provider-contract.md` is empty. +5. Profiles line up 1:1: for each axis code from contract §4 headings, it appears in the appendix table and in the template; the appendix table has one row per code. +6. Links resolve: extract `](target)` from the four files plus README, CLAUDE.md and architecture.md; check file targets exist and anchors match slugified headings. +7. No em-dashes in the four files or in the added lines of the three edited files. +8. No line references: `grep -nE '\.go:[0-9]+|:[0-9]+-[0-9]+' ` is empty. +9. Nothing lost: compare the gap and proposal bullets of `git show HEAD:docs/providers.md` against the appendix and contract §13 by keyword. +10. `go build ./... && go test -short ./...` still pass (docs-only change). + +## Execution summary must record + +- Step 1: the frozen axis list; the two contract additions found in verification (sequential GC sweep; the Stopped branch re-deleting a not-yet-started instance); per-call timeout facts as verified for each existing provider. +- Step 3: the fact corrections applied (kvm Details keys, kubernetes seam, kvm `isClassified`); matrix rows dropped in favour of the profile table. +- Step 4: the exact link edits. +- Step 5: the verification commands as run with trimmed output. +- Worth noting: CHANGELOG headings keep the repo's own em-dash format; the stale `mock` comments in `provider.go`, `registry.go` and `proxy_types.go` stay listed as gaps and are not fixed here.