From 8d764de336b3b64f5c3e77a7dd5b0d5af0314efb Mon Sep 17 00:00:00 2001 From: kacerr Date: Wed, 16 Sep 2026 11:17:56 +0000 Subject: [PATCH] Write the generic provider contract Fully abstract: MUST/SHOULD/MAY rules with the caller behind each one, 19 behaviour-profile axes with stable codes, credential hygiene, testing obligations, the checklist and the contract's known limits. Co-Authored-By: Claude Fable 5.1 --- .../2026-09-16-1108-provider-docs-split.md | 65 ++ docs/provider-contract.md | 860 ++++++++++++++++++ 2 files changed, 925 insertions(+) create mode 100644 docs/plans-executions/2026-09-16-1108-provider-docs-split.md create mode 100644 docs/provider-contract.md diff --git a/docs/plans-executions/2026-09-16-1108-provider-docs-split.md b/docs/plans-executions/2026-09-16-1108-provider-docs-split.md new file mode 100644 index 0000000..b1a6fc9 --- /dev/null +++ b/docs/plans-executions/2026-09-16-1108-provider-docs-split.md @@ -0,0 +1,65 @@ +# Execution: Split the provider blueprint into contract, appendix, template and index + +Plan: [docs/plans/2026-09-16-1108-provider-docs-split.md](../plans/2026-09-16-1108-provider-docs-split.md) + +- [x] Step 0 — Save and commit the plan +- [x] Step 1 — Write `docs/provider-contract.md` +- [ ] Step 2 — Write `docs/provider-spec-template.md` +- [ ] Step 3 — Write `docs/providers-existing.md` +- [ ] Step 4 — Rewrite `docs/providers.md` as the index; link edits +- [ ] Step 5 — Verification sweep +- [ ] Step 6 — Final summary, CHANGELOG after confirmation + +## Step 0 — Save and commit the plan + +Committed as `cf89ac4` on the existing `feat/provider-blueprint` branch, on +its own, before any document work. The pre-existing local modification of +`.claude/settings.json` was left out of the commit. + +## Step 1 — Write `docs/provider-contract.md` + +2026-09-16 11:17. Written from scratch rather than edited down from the old +blueprint, so that nothing provider-specific could survive by accident. Every +MUST names its caller by function (`reconcileManaged`, `replaceInstance`, +`reconcileDelete`, `providerFailure`, `deletionFailure`, `Sweeper.sweep`, +`Monitor.checkOnce`, `Monitor.run`, `registry.Build`) and states what breaks. + +**Axis codes frozen** (19, §4.1 to §4.19): `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`. The appendix table and the template are keyed +on these strings; the §4.20 summary table's last column gives the template +entry number each axis maps to. + +**Two rules that were not in the old blueprint**, both found by reading the +callers while verifying: + +- `Sweeper.sweep` loops providers sequentially in one goroutine, and no + caller puts a deadline on the context (only `Monitor.checkOnce` uses + `WithTimeout`). So a hung `ListByTag` stalls orphan GC for every provider. + Recorded in §1.5 and the `timeouts` axis. +- The Stopped branch of `reconcileManaged` calls `Delete` and requeues + without clearing `ProviderID`, then deletes again on every poll until + NotFound. A backend that reports a freshly created, never started instance + as stopped would have it deleted right after creation. Recorded as a MUST + in §2.2 and as the `provisioning-state` axis with three allowed profiles. + +Checks run on the draft (all clean): + +```sh +grep -niE "kubernetes|gcp|kvm|libvirt|squid|google|compute engine" docs/provider-contract.md +grep -n "—" docs/provider-contract.md +grep -nE '\.go:[0-9]+|:[0-9]+-[0-9]+' docs/provider-contract.md +grep -ohE "(internal|cmd|config|docs|api|hack|test)/[A-Za-z0-9_./-]+" docs/provider-contract.md | sort -u | while read p; do test -e "$p" || echo "MISSING $p"; done +``` + +The path check reports three expected misses: `config/samples/proxy_.yaml` +is a pattern, `docs/provider-spec-template.md` is written in Step 2, and +`internal/provider/providertest` is a proposal named in §13. + +Worth noting: the contract refers to "the operator pod" and "the cluster's +own API" rather than naming the platform, so that the abstractness grep stays +meaningful. The `hack/providers-dev.yaml` rule is phrased as "only if the type +needs no credentials" because that file exists for `make run-dev` without any +Secret. diff --git a/docs/provider-contract.md b/docs/provider-contract.md new file mode 100644 index 0000000..6181de3 --- /dev/null +++ b/docs/provider-contract.md @@ -0,0 +1,860 @@ +# The provider contract + +This document is the specification a provider must meet. It is written for +whoever builds the next one, human or agent, and it deliberately says nothing +about how any existing provider is built. Those details are in +[providers-existing.md](providers-existing.md); the questionnaire a new +provider's design must answer is [provider-spec-template.md](provider-spec-template.md). +Read the three in that order: this contract, then the template, then the +appendix for worked examples. + +Rule keywords: **MUST** means breaking the rule breaks a caller, and every MUST +below names the caller and what breaks. **SHOULD** means every current provider +does it and a new one needs a written reason not to. **MAY** means it is +allowed and the choice must be recorded in the provider's spec. + +Code references are a file path plus a function or symbol name, never a line +number. Where a rule is enforced by a caller, that caller is one of: + +- the **reconciler**, `internal/controller/proxy_controller.go`; +- **orphan GC**, `internal/gc/gc.go`; +- the **provider health monitor**, `internal/providerhealth/monitor.go`; +- the **decorators**, `internal/provider/metrics.go` and + `internal/provider/tracing.go`; +- the **composition root**, `cmd/main.go`. + +## 1. What a provider is and who calls it + +### 1.1 The types + +A provider is a Go type implementing `provider.Provider` +(`internal/provider/provider.go`): four methods over one VM-like instance, +identified by an opaque `providerID` string. + +- `Create(ctx, CreateRequest) (providerID, error)` +- `Get(ctx, providerID) (*Instance, error)` +- `Delete(ctx, providerID) error` +- `ListByTag(ctx) ([]Instance, error)` + +`CreateRequest` carries `Name` (from `NameFromUID` in `internal/provider/name.go`, +22 characters, `proxy-` plus base32), `UID` (the owning Proxy's UID), +`Namespace`, `ProxyName`, `Placement` (`Region`, `Zone`, `MachineType`, +`Image`), `CloudInit` (already resolved from inline text or a Secret) and +`Port`. + +`Instance` carries `ID`, `IP`, `State` (one of `StateProvisioning`, +`StateRunning`, `StateStopped`, `StateTerminated`), `UID` and `CreatedAt`. + +The GC marks are the constants `LabelManaged` (value `LabelManagedYes`) and +`LabelUID`, also in `provider.go`. + +Providers are named instances of a type: two entries of one type with +different config blocks are two providers. The configured name is what +`Proxy.spec.provider` refers to, and it is immutable, so a Proxy never moves +between providers. + +### 1.2 The callers + +Nothing above the provider watches backend resources. Everything the operator +knows about an instance comes from polling `Get`. + +- **The reconciler** calls `Create`, `Get` and `Delete` from `reconcileManaged`, + `replaceInstance` and `reconcileDelete`, and branches only on the error class + (`providerFailure`, `deletionFailure`) and on `Instance.State`. It maps the + CRD's placement to `Placement` one to one in `placementFrom`. Its poll + intervals are set in `applyDefaults`: `ProvisioningPoll` and `DeletionPoll` + 10s, `DriftPoll` 2m, `RequeueNow` 1s, `QuotaRetry` 5m. Up to 3 Proxies + reconcile at once (`MaxConcurrentReconciles` in `SetupWithManager`). +- **Orphan GC** calls `ListByTag` and `Delete` from `Sweeper.sweep`, every + `Interval` (10m), and never deletes an instance younger than `MinAge` (10m). + It loops over providers one after another in a single goroutine. +- **The provider health monitor** calls the optional `Checker` from + `Monitor.checkOnce`, every `--provider-check-interval`, all providers + concurrently under one timeout. +- **The composition root** builds every provider through `registry.Build` + (`internal/provider/registry/registry.go`) before the manager starts, on + every replica, then wraps each one in `WithTracing(WithMetrics(...))`. + +**Where the calls come from.** The reconciler, GC, the health engine and the +monitor run on the leader only. The constructors run on every replica at +startup. + +**Concurrency.** Up to 3 reconciles, a GC sweep and a health-check round can +call the same provider at the same time, and GC's `Delete` can overlap the +reconciler's `Delete` for the same instance. A provider **MUST be safe for +concurrent use, including two calls for the same instance name**. Nothing +above the provider serialises them. + +### 1.3 What the instance is for + +The instance is a forward proxy with its own egress path. Two parties connect +to `http://:`: + +- the health engine, from the operator pod, which sends a real `CONNECT` + through it (`probe` in `internal/health/probe.go`); +- clients, which get `ip` and `port` from the discovery API (`viewOf` in + `internal/discovery/handlers.go`). + +So the IP a provider reports **MUST be reachable on that port from the +operator's pod network and from the clients**. Whatever that takes on the +backend (firewall rules, security groups, routing) is either done by the +provider or documented as a prerequisite in the provider's guide. + +The provider does not know which proxy software runs. The proxy is installed +and configured by the Proxy's `cloudInit`; building install logic into the +operator is a stated non-goal (`docs/prompts/__initial-prompt.md`, non-goals). +A backend that cannot take user-data (see `readiness` in §4) documents that +it ignores the field and how the proxy gets configured instead. + +### 1.4 Where it runs + +The manager container (`config/manager/manager.yaml`) runs as non-root with a +read-only root filesystem, a 64Mi `emptyDir` at `/tmp` and a 128Mi memory +limit, from a distroless static image (`Dockerfile`) that carries the public +CA roots and nothing else. So a provider: + +- **MUST stream large payloads** rather than buffering them in memory; +- **MUST write scratch files only under `/tmp`**, and delete them; +- gets credentials either as mounted files (the providers ConfigMap at + `/etc/proxy-operator`, optional Secret mounts) or from the environment or + the pod's own identity. See §7. + +### 1.5 Time and cancellation + +**No caller puts a deadline on the context.** The reconcile and GC contexts +are cancelled on shutdown and on loss of leadership, and otherwise never. +The only bounded context is the health monitor's (`Monitor.checkOnce`, one +timeout per round). + +- **MUST honour `ctx`**: return promptly once it is cancelled, on every + method. +- **SHOULD bound every backend call itself**: a dial timeout, a per-request + timeout, a cap on any wait loop. What breaks otherwise: a hung `Create`, + `Get` or `Delete` pins one of the 3 reconcile workers until the manager + stops; a hung `ListByTag` stalls orphan GC for every provider, because + `Sweeper.sweep` runs them sequentially; a `Check` that ignores its context + wedges a monitor round. Choose budgets against the polls: a `Get` that + routinely outlives `ProvisioningPoll` gains nothing. + +## 2. The contract, method by method + +### 2.1 Create(ctx, CreateRequest) (providerID, error) + +- **MUST be idempotent by `req.Name`.** A repeat call for a name that already + exists returns that instance's providerID with a nil error, creates no + duplicate and returns no error. + *Caller:* the reconciler records the ID in status only after `Create` + returns (`reconcileManaged`). A crash between the two, or a lost status + write, runs `Create` again for the same Proxy; `replaceInstance` recovers + from a crash between its two writes the same way. +- **MUST mark the instance as managed**, with `LabelManaged=LabelManagedYes` + and `LabelUID=req.UID` or the backend's equivalent, in the same call that + creates it. When the backend's create call cannot carry the mark, the + window between creation and marking **MUST be far shorter than `MinAge`** + and documented in the provider's spec (`gc-mark` in §4). + *Caller:* orphan GC finds instances only through the mark and never deletes + one whose UID is empty (`Sweeper.sweep`). An instance that exists without + the mark leaks if the operator crashes before marking it. +- **MUST return a providerID that is self-contained.** `Get` and `Delete` + receive only the ID, and the config or spec may have changed since + `Create` (a zone edit during a replacement). Put whatever `Get` needs to + find the instance into the ID, and validate config values that end up in + the ID so they cannot contain the separator. See `primary-key` in §4 for + backends that allocate their own identifier. +- **MUST NOT let `Get` answer NotFound for an ID that `Create` just + returned.** A backend with eventual consistency between create and read + needs a read-after-write guarantee or a retry inside `Create`. + *Caller:* on NotFound the reconciler clears the ID and creates again + (`reconcileManaged`), so a lagging read makes a duplicate. +- **MUST honour `ctx`** (§1.5). +- **MAY return once the request is submitted, or block until the instance is + running** (`create-return` in §4). Submit-and-return is preferred: readiness + is discovered by polling `Get`, and a blocking `Create` holds one of the 3 + workers for its whole duration. A provider that blocks MUST still honour + cancellation and SHOULD cap the wait. +- **SHOULD reject a request it can never satisfy** with `ErrPermanent` + before touching the backend: a missing or unknown zone, machine type or + image, or any placement value not in its catalogue. The CRD cannot validate + `placement`, because it is provider-agnostic, so this error is the user's + only feedback. The message SHOULD name the offending field and value. +- **Catalogue changes do not replace instances.** The spec hash + (`specHashInput` in `internal/controller/spechash.go`) covers `placement`, + the resolved `cloudInit` and `port`, not the provider config. When a + placement value names a config entry, changing what that entry means affects + only new instances. The provider's guide MUST say so and tell users to add a + new key rather than edit one in use. +- **MAY ignore `Placement` fields that do not apply.** If a field is ignored, + the provider's guide MUST say so, because the field is still part of the + spec hash: changing it replaces the instance and has no other effect. +- **SHOULD deliver `CloudInit` as user-data**, byte for byte, without parsing + or re-rendering it. A provider that cannot documents that it ignores the + field. +- **MUST NOT log `CloudInit` contents**, put them in span attributes, or + include them in error messages. It can hold secrets. Log its byte count. + Test this with a sentinel string (§11). + +### 2.2 Get(ctx, providerID) (*Instance, error) + +- **MUST return `ErrNotFound` when, and only when, the instance is gone.** + *Caller:* `ErrNotFound` triggers three actions. In normal reconciling the + reconciler clears the ID and creates again (`reconcileManaged`). During + replacement it is the signal to create the new instance + (`replaceInstance`). During deletion it releases the finalizer + (`reconcileDelete`). A false NotFound during an outage creates a duplicate + or loses the instance. "Unreachable", "wrong credentials" and "cannot tell" + are Transient or Permanent, never NotFound. +- **MUST keep returning the instance, not NotFound, while a delete is in + progress** and the name is still taken. + *Caller:* the replacement instance gets the same name. `replaceInstance` + waits for NotFound before creating it; a NotFound too early makes `Create` + adopt the dying instance. +- **MUST report `StateRunning` only with a non-empty `IP`.** Running without + an address yet is `StateProvisioning`. + *Caller:* the IP from a Running answer goes straight into status and to the + health engine (`reconcileManaged`). +- **MUST report `StateStopped` or `StateTerminated` only when the instance + will not recover by itself.** SHOULD map states it does not recognise to + `StateStopped`. + *Caller:* the reconciler reacts to both states identically: it calls + `Delete` and requeues after `DeletionPoll` without clearing the ID, then + polls `Get` and deletes again until NotFound, then creates + (`reconcileManaged`). Instances are disposable, so this is the safe answer + for anything unknown, but it makes the next rule critical. +- **MUST NOT report a freshly created instance that has not been started yet + as `StateStopped`.** A backend with no provisioning state of its own, where + "defined but not yet started" and "stopped" look the same, MUST resolve this + in one of two ways (`provisioning-state` in §4): block `Create` until the + instance is started, or record enough at creation (written metadata, a task + handle) for `Get` to answer `StateProvisioning`. + *Caller:* the same branch of `reconcileManaged` would delete the instance + right after it was created, forever. +- **During an asynchronous operation or a backend lock** (a clone or a delete + still running, an instance locked by the backend), `Get` MUST return either + the last known state or `ErrTransient`, never NotFound and never + Permanent (`async-ops` in §4). +- **MUST fill `UID` and `CreatedAt` by reading them back from the backend**, + never from memory. + *Caller:* GC compares `UID` with the live Proxies and deletes anything whose + `CreatedAt` is older than `MinAge`. **A zero `CreatedAt` counts as very + old** (`Sweeper.sweep`), so a provider that forgets it makes instances still + being created collectable. An unparsable timestamp giving the zero time is + accepted: it never shields an orphan forever. +- **SHOULD return `ErrPermanent` for an ID it can never resolve**: a malformed + ID, a location no longer in the config. + *Caller:* `reconcileDelete` then tries `Delete` anyway and releases the + finalizer if that works. If the provider returned Transient, deletion would + poll forever. + +### 2.3 Delete(ctx, providerID) error + +- **MUST be idempotent.** Deleting something already gone returns nil. + *Caller:* GC and the reconciler race, and a crash repeats the call. +- **MUST accept being called again while an earlier delete is still in + progress.** Treat "already deleting" as success, and SHOULD keep that cheap. + *Caller:* while `Get` still returns the instance, `reconcileDelete` and the + Stopped branch of `reconcileManaged` call `Delete` again every + `DeletionPoll` until `Get` returns NotFound. +- **MUST remove everything `Create` made**: disks, volumes, seeds, addresses, + uploaded files. Either delete them explicitly or rely on the backend + cascading (`secondary-resources` in §4). + *Caller:* GC sees only what `ListByTag` returns, so secondary resources + have no other cleanup path. +- **MAY return before the instance is gone** (`delete-return` in §4), as long + as `Get` keeps showing it until it is. +- **SHOULD succeed for an ID whose `Get` fails permanently** when there is + nothing left the provider could reach, and log that it did nothing. + *Caller:* on the deletion path a permanent error is not latched; it retries + forever (`deletionFailure`), and the Proxy is then only released through the + `AnnotationSkipCleanup` annotation (`api/v1alpha1/proxy_types.go`). + +### 2.4 ListByTag(ctx) ([]Instance, error) + +- **MUST return only instances that carry the managed mark**, each with an + `ID` that `Delete` accepts, plus `UID` and `CreatedAt`. `State` and `IP` + are not used by GC. + *Caller:* GC deletes every listed instance whose UID is not live and whose + age is past `MinAge` (`Sweeper.sweep`). Listing something without the mark + is how an operator deletes an instance it does not own. +- **MUST return an error rather than a list it cannot vouch for.** An error + makes GC skip that provider for this sweep (`Sweeper.sweep`). +- **MAY leave out instances it could not read.** Leaving something out is + always safe, because GC never deletes what is not listed. Pick a policy + (`list-partial` in §4) and document it. +- **SHOULD cover the whole scope the provider can create in**: every + namespace, zone, node or host. Anything outside that scope is never + collected. +- **Scope caveat:** GC protects instances by UID, not by installation. Two + operator installs sharing one backend delete each other's instances (§13). + Two provider entries of one install that share a scope are safe, because + they share the live-UID set. + +## 3. What a container-style backend and a VM-style backend have in common + +The contract above is the same for both, but the profiles in §4 differ. +A backend that runs a container has an instance that is ready seconds after +`Create`, gets its address from the platform, is deleted in one call and has +no secondary resources; a backend that boots a VM has a provisioning window +of minutes, may discover the address only after the guest reports it, may +need several deletes, and leaves disks and seeds behind unless told +otherwise. Neither is preferred. What matters is that the provider's spec +names its profile on every axis, because the appendix table and the test list +are keyed by those names. + +## 4. Behaviour profiles + +Each axis below has a stable code. The provider's spec (the template) answers +every axis; the appendix maps each existing provider to its profile on every +axis. A profile is not a permission to skip a MUST in §2; it says which of +the allowed ways of meeting it the provider chose. + +### 4.1 `readiness` + +*When is an instance usable after `Create`?* + +- **immediate**: the platform starts it at once; `Get` reports Running + within a poll or two. +- **provisioning-window**: minutes of booting; `Get` reports Provisioning + for that time. + +Callers tolerate both: the reconciler polls at `ProvisioningPoll` either +way. An immediate provider still MUST report Provisioning until the IP is +known. + +### 4.2 `create-return` + +*Does `Create` return once the request is submitted or once the instance is +running?* + +- **submit**: preferred. Readiness comes from `Get`. +- **blocking**: allowed. Costs one of 3 workers for the duration, and any + first-use setup (an image download) runs inside it. MUST honour `ctx`, + SHOULD cap the wait, and MUST keep every step idempotent so a cut-off + attempt resumes. + +### 4.3 `delete-return` + +*Does `Delete` return before the instance is gone?* + +- **returns-early**: `Get` keeps returning the instance until it is gone + (§2.2), and repeated `Delete` calls are cheap. +- **waits**: `Delete` returns nil only when nothing is left. Bound the wait + and honour `ctx`. + +### 4.4 `ip-source` + +*Where does `Instance.IP` come from?* + +- **assigned-at-create**: the backend reports it with the instance. +- **discovered**: an agent in the guest, a DHCP lease, a neighbour table. + Discovery failures mean "no address yet", never an error; `Get` returns + Provisioning. The provider's guide MUST say what the guest needs (an agent + package, outbound access to install it) and what happens without it. + +### 4.5 `gc-mark` + +*Where do `LabelManaged` and `LabelUID` live?* + +- **native-labels**: labels or tags the backend indexes; `ListByTag` can + filter server-side. +- **written-metadata**: a description, an annotation or a metadata element + written by the provider; `ListByTag` lists everything and filters + client-side. +- **mark-window**: the create call cannot carry the mark and a second call + writes it. Allowed only when the window is a few seconds, documented, and + the unmarked instance is cleaned by the same Proxy's next reconcile (§2.1). + +### 4.6 `created-at` + +*Where does `CreatedAt` come from?* + +- **backend-timestamp**: the backend's own creation time. +- **written-at-create**: the provider writes the time into the mark + metadata in the create call. A backend with no timestamp MUST do this. + +### 4.7 `list-partial` + +*What does `ListByTag` do when part of its scope is unreachable?* + +- **fail-whole**: one unreachable part fails the list; GC skips the provider + this sweep. Nothing is collected until every part answers. +- **omit-unreachable**: unreachable parts are left out and the rest is + returned. Orphans in the reachable parts are collected; the provider MUST + log what it left out. + +Both are safe (§2.4). A single unreadable instance MAY always be skipped. + +### 4.8 `quota` + +*Can the provider tell "no capacity" from other failures?* + +- **detected**: a recognisable quota, rate-limit or out-of-resources signal + maps to `ErrQuotaExceeded`. +- **absent**: capacity errors map to `ErrTransient`. The guide MUST say so, + and the `Checker` SHOULD expose a capacity figure in `Details` instead. + +### 4.9 `not-found` + +*How does the provider know an instance is gone?* + +- **status-404**: a distinct not-found status or error code from the + backend. +- **error-code**: a code or message the backend uses for "does not exist", + matched exactly and covered by the classify table test with real captured + responses. +- **list-and-miss**: absence from a listing. Allowed only when the listing + can never be partial; otherwise a missing part reads as "gone" and §2.2 is + broken. + +### 4.10 `primary-key` + +*Is `req.Name` the backend's key?* + +- **name-is-key**: the backend addresses instances by name. `Create` looks + the name up, the ID is `/`. +- **allocated-id**: the backend allocates an identifier and the name is a + label. `Create` MUST first find an existing instance by name (idempotency), + then allocate. Allocation MUST be safe against a concurrent `Create` for + another Proxy: a deterministic identifier derived from `req.UID`, or an + allocate-then-verify loop. The ID SHOULD embed the allocated key and the + name; `Get` and `Delete` use the allocated key. If instances can move + between locations (migration), the ID MUST NOT depend on the location. + +### 4.11 `provisioning-state` + +*Does the backend have a state for "being created"?* + +- **native**: the backend reports a provisioning or staging state, mapped to + `StateProvisioning`. +- **create-blocks**: no such state; `Create` blocks until the instance is + started, so `Get` never sees "defined but not started". +- **marker**: no such state and `Create` returns early; the provider records + at creation (metadata or a task handle) what lets `Get` answer + `StateProvisioning` for an instance that was never started, and + `StateStopped` for one that stopped afterwards. + +### 4.12 `async-ops` + +*Does the backend run create and delete as asynchronous tasks?* + +- **none**: calls complete synchronously. +- **task-handle**: the backend returns a task the provider can poll. The + provider SHOULD keep the handle where `Get` can find it (metadata or the + ID) so a failed create can be reported instead of read as NotFound. +- **backend-lock**: the backend locks the instance during a task. `Get` + returns the last known state or Transient while locked (§2.2); `Delete` + on a locked instance returns Transient, not Permanent. + +### 4.13 `secondary-resources` + +*What else does `Create` make, and who removes it?* + +- **none**. +- **cascade**: the backend deletes them with the instance (auto-delete + flags). The provider MUST set those flags at create. +- **explicit**: the provider deletes them by name in `Delete`, tolerating + their absence. Resources that can exist without an instance (a crash + between creating them and creating the instance) are a documented GC blind + spot unless `ListByTag` covers them. + +### 4.14 `credentials` + +*Where do credentials and trust material come from?* + +- **mounted-files**: paths in the config block, pointing into an optional + Secret mount (§7). +- **environment**: standard SDK environment discovery. +- **workload-identity**: the pod's own identity. + +TLS trust for an HTTPS backend with a private CA is one of **system-roots**, +**ca-file** (a path in the config block) or **insecure** (a boolean whose +name starts with `insecure`, warned about at startup). + +### 4.15 `client-kind` + +*What talks to the backend?* + +- **vendor-sdk**: put it behind a small interface (the seam) so a fake can + stand in. +- **thin-client**: a `net/http` client written in the provider package. + Preferred when no maintained SDK exists (`CLAUDE.md`: stdlib over + third-party). It MUST set transport timeouts, MUST carry the backend's + status and body in one error type that `classify` reads, and SHOULD wrap + its transport for child spans (§9). +- **rpc-over-ssh** or another non-HTTP transport: one pooled connection per + target, a keepalive, and a documented answer to "what does a cancelled call + do to the shared connection". + +### 4.16 `timeouts` + +*How is each backend call bounded?* (§1.5) + +- **per-call**: every request has its own deadline, independent of `ctx`. +- **ctx-only**: the provider relies on `ctx`. Allowed only for backends + whose client already enforces a timeout; the spec MUST cite it. + +### 4.17 `logging` + +*Which log tiers does the provider produce?* The logger MUST come from +`logf.FromContext(ctx)` at call time (§9). + +- **tiered**: V(1) one line per operation outcome and per failed backend + call; V(2) request and response detail; V(5) raw wire if there is one. +- **outcome-only**: V(1) only. +- **none**: allowed only for a backend whose client logs on its own. + +### 4.18 `child-spans` + +*How do backend calls appear under the operation span?* (§9) + +- **sdk-instrumentation**: the SDK emits spans itself. +- **transport-wrapper**: a parent-gated `otelhttp` transport. +- **span-decorator**: a decorator over the seam that starts one span per + call. +- **none**: a documented gap, with log correlation through `traceID` only. + +### 4.19 `backend-test` + +*How is the provider proven against the real backend?* + +- **cluster-e2e**: the kind end-to-end suite. +- **integration-tag**: a test behind `//go:build integration`, skipped + without its environment variables, with a `make test-integration-` + target and a section in `docs/testing.md`. +- **manual-runbook**: a documented by-hand run. Allowed only when the + backend cannot be reached from CI or a developer machine. + +### 4.20 Summary + +| Axis | Profiles | Who tolerates the difference | Template entry | +|---|---|---|---| +| `readiness` | immediate, provisioning-window | reconciler polls either way | 7 | +| `create-return` | submit, blocking | reconciler; worker budget | 10 | +| `delete-return` | returns-early, waits | reconciler re-calls Delete | 11 | +| `ip-source` | assigned-at-create, discovered | reconciler gates Running on IP | 7 | +| `gc-mark` | native-labels, written-metadata, mark-window | GC | 8 | +| `created-at` | backend-timestamp, written-at-create | GC `MinAge` | 8 | +| `list-partial` | fail-whole, omit-unreachable | GC | 12 | +| `quota` | detected, absent | reconciler `QuotaRetry` | 13 | +| `not-found` | status-404, error-code, list-and-miss | reconciler | 13 | +| `primary-key` | name-is-key, allocated-id | reconciler idempotency | 2 | +| `provisioning-state` | native, create-blocks, marker | reconciler Stopped branch | 9 | +| `async-ops` | none, task-handle, backend-lock | reconciler | 10 | +| `secondary-resources` | none, cascade, explicit | GC blind spots | 11 | +| `credentials` | mounted-files, environment, workload-identity | deployment | 14 | +| `client-kind` | vendor-sdk, thin-client, rpc-over-ssh | tests, tracing | 17 | +| `timeouts` | per-call, ctx-only | worker and GC budget | 18 | +| `logging` | tiered, outcome-only, none | operators | 20 | +| `child-spans` | sdk-instrumentation, transport-wrapper, span-decorator, none | tracing | 17 | +| `backend-test` | cluster-e2e, integration-tag, manual-runbook | CI | 21 | + +## 5. Error taxonomy + +Every error a provider method returns **MUST be built with +`provider.Wrap(class, op, providerName, id, err)`** (`internal/provider/errors.go`). +`op` is one of `create`, `get`, `delete` or `list`, and `providerName` is the +**configured name** (`ProviderConfig.Name`), not the type. +*Caller:* the decorators label metrics and spans with the class +(`resultLabel`), and the reconciler branches on `provider.Class`. + +`Wrap` keeps both `errors.Is(err, class)` and `errors.As` to the backend's +error type working (`Error.Unwrap` returns both). + +| Class | Use it for | During normal reconciling | While deleting | +|---|---|---|---| +| `ErrNotFound` | the instance does not exist | Get: recreate or advance (§2.2); other ops: returned as the error, so exponential backoff | Get: release the finalizer | +| `ErrQuotaExceeded` | quota, rate limit, out of capacity | `Provisioned=False` with reason `QuotaExceeded`, requeue after `QuotaRetry`, not counted as an error (`providerFailure`) | requeue after `QuotaRetry` (`deletionFailure`) | +| `ErrTransient` | network, 5xx, timeouts, cancellation, anything unknown | error returned, so workqueue exponential backoff | same | +| `ErrPermanent` | bad config, bad argument, auth or permission denied, malformed ID | **latched**: `Provisioned=False` with reason `PermanentError`, no more provider calls until `metadata.generation` changes (`reconcileManaged`) | error returned and retried forever; a Permanent `Get` also triggers `Delete` (`reconcileDelete`) | + +Pitfalls: + +- **Unclassified means Transient.** `provider.Class` returns `ErrTransient` + for any error not built with `Wrap`. That is the safe default, but a + permanent failure that was never classified retries forever, and a quota + error backs off far too fast. +- **Permanent is sticky.** Use it only for errors that editing the Proxy spec + or fixing the config can resolve. A permission-denied status that the + backend also uses for quota is the hard case: read the backend's reason if + it gives one, and if it does not, pick Permanent and document it. +- **Do not return NotFound from `Create`.** The reconciler treats it as + Transient there, but it misleads the metrics and anyone reading logs. +- **NotFound detection** follows the `not-found` profile (§4.9). A backend + that reports "does not exist" as a generic server error needs the exact + message or code matched, with real captured responses in the test table. +- **Classify in one `classify` function** with a table-driven test covering + every class, including "not a backend error at all, so Transient". + +## 6. Identity, names, marks, CreatedAt + +- **Names:** `NameFromUID(uid)` gives `proxy-` plus 16 lowercase base32 + characters. It is a valid DNS label. A backend with stricter rules SHOULD + map it deterministically rather than invent a new scheme, because + idempotency depends on the name being the same every time. A backend that + keys on an allocated identifier follows `primary-key` (§4.10). +- **providerID:** opaque to everyone except the provider. Keep it + `/`-shaped or `/`-shaped, parse + it strictly, and return `ErrPermanent` for a malformed one (§2.2). +- **Marks:** `LabelManaged` with value `LabelManagedYes`, and `LabelUID`. + Use native labels or tags where the backend has them; otherwise write them + into metadata in the create call (§4.5). +- **`CreatedAt`:** the backend's creation timestamp, or written into the mark + metadata at creation (§4.6). +- **Provider names:** a configured `name` MUST be a lowercase DNS-1123 label + of at most 63 characters (`spec.provider` in + `api/v1alpha1/proxy_types.go`). The `ProviderHealth` object is named after + the provider; `Monitor.Start` warns and `Monitor.publish` skips the object + for a name that is not a valid object name. Config loading does not check + this yet (§13). + +## 7. Credentials, TLS and secret hygiene + +- **The constructor MUST NOT dial the backend.** `registry.Build` aborts the + whole provider set on a constructor error, and the manager exits: one + unreachable backend must not take the other providers down. Fail in the + constructor only on config errors. +- **Credential files SHOULD be read lazily.** A mount marked `optional: true` + is empty when the Secret is missing; the constructor logs a warning and the + first call fails Transient until the Secret lands. A file that exists but + does not parse is Permanent. +- **Credentials MUST NOT appear in logs, span attributes, error strings or + `Error.Err`.** Backend errors that echo a request URL or header with a + token in it MUST be sanitised before wrapping. The same holds for + `CloudInit` (§2.1). Test both with a sentinel (§11). +- **TLS trust** follows `credentials` (§4.14). An insecure toggle MUST be + named so that it reads as insecure, default off, and MUST be logged as a + warning at startup when on. +- **Rotation.** State in the spec whether rotating the Secret takes effect + without a restart. Reading files on every dial makes rotation free; caching + them after first load needs a restart and must say so. +- **Scratch files** only under `/tmp` (§1.4), removed when the call ends. + +## 8. Optional interfaces: `Checker` + +`provider.Checker` is `Check(ctx) []TargetStatus` (`internal/provider/check.go`). +`cmd/main.go` type-asserts it on the **unwrapped** provider before adding the +decorators, and the monitor treats a nil Checker as unsupported, not failed +(`Monitor.run`). + +- **MUST never fail as a whole.** Every failure is a `TargetStatus` with + `Ready=false`. + *Caller:* `Monitor.checkOnce` has no error path per provider. +- **MUST return promptly once `ctx` ends.** The monitor gives the whole round + one timeout. +- **MUST return one entry per target, in a stable order.** Targets are what + can fail independently: an apiserver, a project, each host or node. + *Caller:* the monitor logs transitions per target (`logTransitions`) and + the gauges are labelled `{provider,target}`. +- **MUST NOT disrupt reconciles.** A check that shares a connection with + provisioning must not tear that connection down when it times out. +- **SHOULD use the shared reasons** (`ReasonReachable`, `ReasonTimeout`, + `ReasonUnreachable`, `ReasonUnauthorized`, `ReasonForbidden`, + `ReasonAPIError`). Add provider-specific reasons only for failures an admin + would fix differently, and document each one in the guide. +- **SHOULD check with the same credentials, verb and scope that `ListByTag` + and `Create` use**, and be cheap: one page, one object. +- **MAY fill `Details`** with facts useful when triaging: versions, capacity, + the account in use. +- The `status.type` field of `ProviderHealth` + (`api/v1alpha1/providerhealth_types.go`) lists the known types in its doc + comment; add the new type there and run `make manifests`. + +## 9. Observability + +Tracing is OpenTelemetry, set up in `internal/tracing` from the standard +`OTEL_*` environment variables. With no exporter configured, spans are no-ops +and logs are unchanged ([architecture.md §10](architecture.md#10-tracing-internaltracing)). + +**Layer 1, operation spans and metrics: nothing to write.** `cmd/main.go` +wraps every provider in `WithTracing(WithMetrics(...))`: + +- `WithTracing` opens one client span per call, named `provider.create`, + `provider.get`, `provider.delete` or `provider.list`, with + `provider.name`, `provider.id` and `provider.result` (the error class) as + attributes. `ErrNotFound` keeps the span status Ok, because polling to + NotFound is expected. +- `WithMetrics` counts each call as + `proxy_operator_provider_requests_total{provider,op,result}` + (`internal/metrics/metrics.go`). A `Checker` feeds + `proxy_operator_provider_target_up`, + `proxy_operator_provider_check_duration_seconds` and + `proxy_operator_provider_check_timestamp_seconds`, labelled + `{provider,target}`. + +Rules: + +- **MUST NOT open its own per-operation span or count its own requests.** + They would duplicate the decorators. +- **MUST pass the caller's `ctx` to every backend call and take the logger + from `logf.FromContext(ctx)` at call time.** `tracing.StartSpan` + (`internal/tracing/logger.go`) puts a logger carrying `traceID` and + `spanID` into `ctx`. A logger cached at construction, or a backend call + made with `context.Background()`, loses the trace. +- **Health checks bypass the decorators.** A `Checker` runs under the + monitor's `providerhealth.check` span, and the same `ctx` rules apply. + +**Layer 2, child spans per backend call: SHOULD.** One operation span says +that `Create` took 40 seconds; child spans say which backend call took them. +Follow `child-spans` (§4.18): + +- HTTP backends: wrap the transport with `otelhttp`, gated on an existing + parent span as `tracing.RestConfigWrapper` does + (`internal/tracing/transport.go`), so background traffic (token refreshes, + keepalives) does not become root spans. Check first whether the SDK already + does this. +- Non-HTTP backends: start child spans yourself with + `tracing.Start(ctx, ".")` around each call and around long + steps such as a dial or a download, in a decorator over the seam, not at + call sites. +- **MUST NOT put secrets or CloudInit in span attributes.** Put identifiers, + sizes and error codes there. + +**Layer 3, logs.** Follow `logging` (§4.17). Log lines join a trace through +the `traceID` key from layer 1. + +## 10. Configuration and wiring + +The providers file is parsed by `LoadConfig` (`internal/provider/config.go`) +with `UnmarshalStrict`, so unknown keys already fail. + +- Add a `Config` struct and a `ProviderConfig` field for it. +- Add the block to `setBlocks`, so a block that does not match `Type` is + rejected. +- Add the type to the allow-list in `Config.validate`. The same list exists a + second time as the constructor map in `cmd/main.go`; both must change + (§13). +- Add required-field validation in its own `config_.go` when it is + more than a few lines, with rows in `config_test.go`. +- **Validate every config value that is pasted into an identifier**: a + location name that becomes part of the providerID must not contain the + separator; a catalogue key that becomes a file or volume name must be a + safe filename. +- The constructor has the signature `registry.Constructor`: + `func(ctx, provider.ProviderConfig) (provider.Provider, error)`. Register + it in the map in `cmd/main.go`. Flags a provider needs are defined there + too and passed through a `NewWith...` variant, never read from the + environment inside the package. +- Decorators are applied by the composition root, in the order + `WithTracing(WithMetrics(p))`, after the Checker has been collected from + the unwrapped provider. + +## 11. Testing obligations + +`CLAUDE.md` applies: stdlib `testing`, table-driven, tests next to source, +`t.Parallel()` where safe. + +- **A fake behind the seam.** Every unit test runs against an in-memory + fake of the backend, never the network. +- **Contract tests**, each against the fake: + - repeat `Create` returns the same ID and creates nothing; + - `Get` after `Delete` returns `ErrNotFound`; + - `Delete` twice succeeds; + - malformed ID is `ErrPermanent` from `Get` and `Delete`; + - an unknown backend state maps to `StateStopped`; + - `StateRunning` requires an IP, and a running instance without one is + `StateProvisioning`; + - `UID` and `CreatedAt` are read back from the fake, not remembered; + - `ListByTag` excludes unmarked instances and instances with an empty UID + are still listed with `UID == ""`; + - `Get` during an in-progress delete still returns the instance; + - `Get` during an asynchronous operation or lock returns the last state or + Transient (`async-ops`); + - a created but not yet started instance is never `StateStopped` + (`provisioning-state`); + - a cancelled `ctx` makes every method return promptly. +- **Classify**: one table test covering every class, every backend code the + provider matches, and "not a backend error at all, so Transient". Use + captured real responses for message matching. +- **Checker**: a table test for the reasons, a test that `Check` returns + promptly with an expired `ctx`, and a test that it returns one entry per + target in config order. +- **Sentinel tests**: a `CloudInit` containing a sentinel string and a + credential containing another never appear in any log line, span attribute + or error string. +- **Tracing**: with an in-memory span recorder + (`sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(tracetest.NewSpanRecorder()))`, + as in `internal/provider/tracing_test.go`), the child spans are children + of the operation span. +- **Real-backend test** per `backend-test` (§4.19). It runs the whole + lifecycle: create, create again, poll until Running, list, delete, delete + again, confirm it is gone and that no secondary resource remains. Keep it + compiling with `go vet -tags=integration ./...`. + +## 12. Checklist for adding a provider + +1. **Spec first.** Copy `docs/provider-spec-template.md` to + `docs/-provider-spec.md` and answer every entry. Unanswered entries + block implementation. +2. **Package** `internal/provider//` with the provider type, a + compile-time assertion for `provider.Provider` and one for + `provider.Checker`, the seam interface, and a logger built from + `logf.FromContext(ctx).WithName("").WithValues("provider", name)`. +3. **Config** per §10, including `config_test.go` rows. +4. **Constructor** `New(ctx, provider.ProviderConfig)` registered in + `cmd/main.go`; no dialing. +5. **Contract** per §2, tested per §11. +6. **Errors** per §5, `classify` plus its table test, `ProviderConfig.Name` + in every `Wrap`. +7. **Checker** per §8, with the `status.type` comment updated and + `make manifests` run. +8. **Tracing and logging** per §9 and §4.17, §4.18. +9. **Real-backend test** per §4.19, with a `make test-integration-` + target and a section in `docs/testing.md`. +10. **RBAC**: if the provider talks to the cluster's own API, add + `+kubebuilder:rbac` markers in their own comment block, never inside a + doc comment, then `make manifests` and check + `git diff config/rbac/role.yaml`. +11. **Deployment**: an `optional: true` credentials mount in + `config/manager/manager.yaml`; a commented entry in + `config/manager/providers_config.yaml` and + `config/samples/providers-config.yaml`; a `config/samples/proxy_.yaml` + whose cloud-init installs a proxy on the sample image; a Makefile helper + for the Secret if there is one; `hack/providers-dev.yaml` only if the + type needs no credentials. +12. **Docs**: `docs/-provider.md` (prerequisites, config reference, + placement mapping, what lands on the backend, the health check, logging, + known gaps); a row and a section in `providers-existing.md`; a row in the + `architecture.md` components table and a block in its §5 and §11 + (§10 too if the provider adds spans); a row in the README "Implemented + providers" table; a `CHANGELOG.md` entry once the user has confirmed it + works. +13. **Optional:** a demo script under `docs/demo/`. + +## 13. Known limits of the contract + +These are facts a new provider has to live with today. None is designed or +scheduled. + +- **Adoption is not verified.** `Create` adopts any existing instance with + the right name without checking its `LabelUID`. +- **`StateStopped` and `StateTerminated` are indistinguishable** to the + reconciler; both mean delete and recreate. +- **Config is read once per process.** Adding a provider or a location, or + rotating credentials a provider caches, needs a manager restart. +- **GC has no installation identity.** Two installs sharing one backend + delete each other's instances, because GC only knows its own live UIDs. +- **An unconfigured provider latches the Proxy.** A Proxy naming a provider + missing from the config is latched `PermanentError`; adding the provider + and restarting does not change `metadata.generation`, and `spec.provider` + is immutable, so the Proxy stays latched until it is recreated. +- **Provider names are not validated** as DNS-1123 at config load. +- **The type list exists twice**: `Config.validate` and the constructor map + in `cmd/main.go`. +- **`Delete` is resubmitted on every deletion poll** until `Get` says + NotFound. +- **Callers set no deadline** (§1.5); each provider bounds itself. +- **There is no conformance suite.** The §11 contract tests are written per + provider; `internal/provider/providertest` is a proposal in the appendix. +- **Stale comments** in `internal/provider/provider.go`, + `internal/provider/registry/registry.go` and `api/v1alpha1/proxy_types.go` + still describe a `mock` provider that no longer exists. + +## 14. Related documents + +- [providers.md](providers.md): the index. +- [provider-spec-template.md](provider-spec-template.md): the questionnaire. +- [providers-existing.md](providers-existing.md): how the existing providers + meet this contract, and their gaps. +- [architecture.md §5](architecture.md#5-what-provider-calls-do-in-the-outside-world), + [§10](architecture.md#10-tracing-internaltracing) and + [§11](architecture.md#11-provider-health-checks-internalproviderhealth). +- [testing.md](testing.md).