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 <noreply@anthropic.com>
This commit is contained in:
@@ -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_<type>.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.
|
||||||
@@ -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://<Instance.IP>:<spec.port>`:
|
||||||
|
|
||||||
|
- 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 `<location>/<name>`.
|
||||||
|
- **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-<type>`
|
||||||
|
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
|
||||||
|
`<location>/<name>`-shaped or `<location>/<allocated-key>`-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, "<type>.<call>")` 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 `<Type>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_<type>.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/<type>-provider-spec.md` and answer every entry. Unanswered entries
|
||||||
|
block implementation.
|
||||||
|
2. **Package** `internal/provider/<type>/` 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("<type>").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-<type>`
|
||||||
|
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_<type>.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/<type>-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).
|
||||||
Reference in New Issue
Block a user