ateapi: authorize MintCert against the store, not the worker cache

[NekoPunch (orangeCatDeveloper)]

Fixes #964

Resume can deny MintCert after the worker assignment is committed because authorization reads an asynchronously updated worker cache. The issue has representative CI failures and a reproduction.

Design decisions

• Authorize from the store, not the worker cache. Credential minting must use committed assignment state. Cache retries only reduce the stale window; a version barrier would add coordination with the replica's watch. A direct store read avoids both. Authorization already reads the actor from the store, so this adds one worker point read rather than a new service dependency. Store failures return Internal; an authoritative mismatch returns PermissionDenied.

• Identify a worker by (namespace, pod). Kubernetes makes a Pod name unique within its namespace. Pool identifies ownership and configuration, not the Pod itself. Keeping pool in the key would require carrying that redundant attribute into MintCert; using (namespace, pod) lets the request locate the worker and then validates the authenticated atelet node and requested Pod UID against the stored record. Both storage backends now use the same logical identity.

• Use Pod UID as an incarnation check, not as part of the key. (namespace, pod) names the current logical worker, while Pod UID distinguishes delete-and-recreate incarnations. Keeping UID out of the key prevents multiple incarnations from coexisting; comparing it during authorization and mutation rejects stale requests and snapshots.

• Keep worker_pool as an immutable attribute. Pool still drives scheduling, metrics, WorkerPool lookup, sandbox configuration, and CLI output. It remains on the worker record, and both PostgreSQL and Redis reject changing it in place.

• Preserve the existing relabel-as-replacement behavior. A pool relabel runs the existing worker cleanup path, deletes the old record, and recreates it under the current pool. This PR preserves that behavior while changing the key; it does not decide whether relabel should become nondisruptive.

• Key the workqueue by (namespace, pod). Including pool or UID would give old and new events different keys, allowing them to run concurrently on the two syncer workers. One key serializes and coalesces events for the same Pod name, and reconcile reads the latest Pod state from the informer.

• Make deletion conditional on the record inspected. Both ateapi replicas run the syncer, so a worker can change between read and delete. DeleteWorker atomically checks namespace, pool, pod, Pod UID, and version. Version alone cannot distinguish an ABA delete-and-recreate because a new record starts again at version 1.

• Release actors only from the worker placement inspected. Worker deletion and actor update are separate operations. Before changing actor state, the syncer compares actor UID plus namespace, pool, pod, and Pod UID. A stale snapshot therefore cannot affect an actor that has moved or a new Pod incarnation that reused the name.

The worker cache remains in scheduling and worker-count paths, where eventual consistency is acceptable; only credential authorization stops depending on it.

Evidence

Before, in run 31628573958, attempt 1:

18:50:06.844788  UpdateWorker commits the assignment
18:50:06.856480  ActorIdentity denies: worker has no actor assignment
18:50:06.858441  ResumeActor fails at CallAteletRestore

The denied read occurred 11.7ms after the authoritative write.

The regression suite covers these counterexamples:

MintCert reads the cache          PermissionDenied after a committed assignment
precondition checks version only  replacement incarnation or another pool is deleted
release compares ns/pod only      a stale snapshot crashes the reassigned actor
queue key includes pool           a relabel leaves two independently runnable items
PostgreSQL permits pool mutation  stored key fields and worker proto diverge

After, local go test ./cmd/ateapi/... -race -v passes the corresponding cases on this branch:

--- PASS: TestMintCertUsesAnAssignmentAsSoonAsItIsWritten
--- PASS: TestSyncer_StaleWorkerSnapshotDoesNotCrashReassignedActor
--- PASS: TestSyncer_PoolRelabelEnqueuesOneKey
--- PASS: TestSyncer_PoolRelabel_RebuildsRecord
  atepg    --- PASS: TestContractSuite/WorkerIdentityIsNamespaceAndPod
  atepg    --- PASS: TestContractSuite/DeleteWorker_VersionConflict
  atepg    --- PASS: TestContractSuite/DeleteWorker_RejectsReplacedIncarnation
  atepg    --- PASS: TestContractSuite/DeleteWorker_RejectsOtherPool
  ateredis --- PASS: TestContractSuite/WorkerIdentityIsNamespaceAndPod
  ateredis --- PASS: TestContractSuite/DeleteWorker_VersionConflict
  ateredis --- PASS: TestContractSuite/DeleteWorker_RejectsReplacedIncarnation
  ateredis --- PASS: TestContractSuite/DeleteWorker_RejectsOtherPool

Boundaries and rollout

The project has other known flaky tests. This PR fixes only the stale-cache MintCert failure described in #964; the rest are not addressed here.

This PR contains no schema or data migration and no compatibility layer for the old worker identity:

• An existing PostgreSQL table keeps its (namespace, pool, pod) primary key. Before deploying the new code, migration must remove duplicate (namespace, pod) rows and replace that primary key.

• Existing Redis worker keys include pool and are not read by the new code. Their records must be rewritten. Resetting them is safe only after all actors have no live worker placement; the syncer can rebuild worker metadata from Pods, but it cannot reconstruct actor assignments.

• Old and new ateapi versions must not write the same store concurrently. Existing PostgreSQL worker page tokens also become invalid after migration.

• Tests pass

• Appropriate changes to documentation are included in the PR

[Taahir Ahmed (ahmedtd)]

Hmmm, I'm not sure we want to remove caching on reads here.

In the analogous situation in Kubernetes, we return an error code indicating that the denial could be due to watch/cache lag, and the caller must retry.

Julian Gutierrez Oschmann (@juli4n) WDYT?

[NekoPunch (orangeCatDeveloper)]

Thanks — could you point me to the Kubernetes mechanism you have in mind? I want to make sure I'm comparing against the same consistency model.

I have two concerns with retrying a cache-based denial here.

First, the worker cache doesn't expose a freshness watermark/resource version today, so after a stale read the caller has no way to know when the cache has observed the assignment it is waiting for. A dropped watch event (see #949) can leave the cache stale until the next successful relist, so retrying is effectively polling an eventually consistent replica without a freshness condition. And unlike the controller-style case, there isn't an independent reconciliation loop here today: MintCert is on the synchronous ResumeActor path, so retry-on-lag would introduce a new retry protocol with its own bounds and failure semantics.

More importantly, retry-on-denial only handles one direction of staleness. Suspend clears the worker's assignment in the store before clearing the actor's back-pointer. During that window, the cache can still contain the old worker→actor assignment while the actor in the store still points back to that worker, so the reciprocal check can still succeed after the authoritative worker assignment has already been cleared. There is no denial to retry in that case.

That's the main reason I was leaning toward reading the worker's authorization state from the authoritative store. There is also Kubernetes precedent for bypassing cached state when making a stale decision would be costly — e.g. GC does a live lookup before acting on deletion decisions, and NamespaceLifecycle falls back to / forces live lookup when its local namespace state may be stale. Cost-wise, minting here is lifecycle-rate rather than request-rate, and this handler already reads the actor from the store, so this is one additional point read per activation.

If we want to preserve the cache on this path, I think we'd need something stronger than a retryable denial: a freshness/version mechanism that lets the serving replica prove its cache has observed the relevant assignment state. That seems like substantially more consistency machinery than a point read here. WDYT?