Close Step 11 test gaps: CEL envtest cases, defaults assertion, quota/permanent/adopt flows; make test runs -race
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -15,7 +15,7 @@ Pairs with [docs/plans/2026-08-07-1747-proxy-operator.md](../plans/2026-08-07-17
|
||||
- [x] Step 8 — GCP provider (`internal/provider/gcp/`)
|
||||
- [x] Step 9 — Orphan GC + metrics
|
||||
- [x] Step 10 — Wiring, config, docs
|
||||
- [ ] Step 11 — Tests
|
||||
- [x] Step 11 — Tests
|
||||
- [ ] Verification (vet/test/kind e2e) + commit, push, open MR
|
||||
|
||||
## Step 0 — Branch and scaffold
|
||||
@@ -1009,3 +1009,55 @@ a local process doesn't have; in-cluster deployments keep the secure
|
||||
default from the kustomize patch. The `providers` map wrapping happens
|
||||
*before* any consumer sees it, so the reconciler and GC only ever hold
|
||||
instrumented providers.
|
||||
|
||||
## Step 11 — Tests
|
||||
|
||||
Most of the plan's Step 11 list was deliberately front-loaded into the
|
||||
step that built each component (the action-table suite, computePhase and
|
||||
SpecHash tables, lease-store matrix incl. the concurrent `-race` case,
|
||||
discovery httptest suite, health threshold/CONNECT tests, GCP
|
||||
`buildInsertRequest` + classification, name-derivation tests from
|
||||
Step 2). This step closed what remained — the envtest-only coverage —
|
||||
and audited the list item by item.
|
||||
|
||||
Added to `internal/controller/proxy_controller_test.go`:
|
||||
|
||||
- **The CEL cases only a real API server can test** (fake clients run
|
||||
neither CEL nor structural defaulting): six invalid-create rejections
|
||||
(Managed-without-provider, External-with-provider,
|
||||
External-without-endpoint, Managed-with-endpoint, cloudInit
|
||||
both/neither), mode-mutation rejection, provider mutation *and removal*
|
||||
rejection (the `has(self.x)==has(oldSelf.x)` form exists exactly for
|
||||
the removal case), and the `+kubebuilder:default={}` assertion — a
|
||||
Proxy created with no `healthCheck` comes back with every nested
|
||||
default materialized, plus port and maxLeases defaults.
|
||||
- **Ready-through-health**: Managed proxy walks to Running (phase still
|
||||
Provisioning — "no health verdict yet must not be Ready"), then a fake
|
||||
`HealthSnapshotter` supplies a healthy snapshot and the phase flips to
|
||||
Ready with latency in status.
|
||||
- **Quota + permanent, envtest edition**: quota → condition
|
||||
QuotaExceeded, `RequeueAfter = QuotaRetry`, nil error, phase *not*
|
||||
Failed; then permanent → Failed and the generation latch provably stops
|
||||
further provider calls.
|
||||
- **Adopt**: strip the spec-hash annotation off a Running proxy (as an
|
||||
operator upgrade with a changed hash-input struct would), reconcile,
|
||||
and assert the annotation is restored byte-identical, the providerID
|
||||
unchanged, and zero provider deletes.
|
||||
|
||||
One repo-wide change: `make test` now runs with `-race` (the plan's
|
||||
"everything runs with -race" was previously only true of the manual
|
||||
verification runs, not the canonical target):
|
||||
|
||||
```make
|
||||
go test -race $$(go list ./... | grep -v /e2e) -coverprofile cover.out
|
||||
```
|
||||
|
||||
Everything green on the first run of the new specs; full suite ~9 s for
|
||||
the controller package under race, `-short` still skips envtest in 0.6 s.
|
||||
|
||||
Worth noting: the provider-removal CEL test has a subtlety worth keeping —
|
||||
removing `provider` alone would also trip the required-iff rule, so the
|
||||
test flips mode and adds an endpoint in the same update to isolate the
|
||||
immutability rules as the thing that rejects. The plan's remaining
|
||||
checklist item is Verification: the throwaway-kind-cluster run of the
|
||||
README quickstart, then push + MR.
|
||||
|
||||
Reference in New Issue
Block a user