test: make unit and envtest suites hermetic and deterministic (#67)

[senolcolak]

Summary

First PR of the testing-foundation series. Makes the operator's test suites hermetic (no network, no source mutation) and deterministic, and separates the pure-unit layer from the API-server integration layer.

Closes #67.

What changed

• Build-tag split. Controller envtest suites are gated behind //go:build envtest. The unit layer (webhook defaulter/validator specs, fixture builders) runs with plain go test — no Kubernetes API server, no KUBEBUILDER_ASSETS.
• Webhook suite → pure unit. Specs already call Default/Validate directly, so the envtest + webhook-server bootstrap (which nothing exercised through admission) is replaced by a minimal RunSpecs entrypoint with RandomizeAllSpecs enabled.
• Typed builders replace captured YAML. test/builders/monitor.go reproduces only the fields the controller consumes, via functional options, with a contract test (monitor_test.go) that asserts the selector labels, mon container --fsid/--id=/--public-addr=/--setuser-match-path= args, the ROOK_CEPH_MON_HOST secretKeyRef, and the three named volumes — so the builders cannot silently drift from what the controller rewrites. Deletes the 4 fixtures under contrib/k8s/test/.
• Hermetic make targets. test-unit, test-unit-repeat (loops 20×, -race, fresh shuffle/Ginkgo seed each run to expose order dependence), test-envtest, test-all, test. None mutate source or clone Rook over the network.
• Design doc under docs/superpowers/specs/.

Verification

• go build ./... and go vet ./... clean in both plain and -tags envtest modes.
• Unit suites pass with -race -shuffle=on.
• envtest controller suite passes 14/14 specs against the new Go builders.

Known follow-ups (not blockers)

• Controller envtest suite has order dependence: AfterEach cleanup does not wait for finalized Deployment deletion #78 — controller envtest suite has a pre-existing order-dependence bug (namespaceCleanUp deletes finalizer-bearing Deployments without waiting). RandomizeAllSpecs is left disabled on the controller suite only (package ordering and per-run seeds still shuffle) with a NOTE(#67) explaining the mechanism; the fix lands in Expand controller, webhook, API, and lifecycle test coverage #68.
• Deferred to later PRs: an envtest-gated smoke test that the webhook is wired to the right GVK; validating builders against real Rook output (drift guard).

[Copilot]

🟡 Changes recommended

The new monitor keyring secret builder stores base64 text in Secret.Data instead of decoded bytes, which makes the fixture diverge from real Kubernetes behavior and can mask controller-copy semantics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR introduces a hermetic, deterministic testing foundation by separating pure unit tests (webhooks + typed fixture builders) from controller envtest integration tests, and replacing file-based YAML fixtures with typed Go builders.

Changes:

• Split envtest suites behind //go:build envtest and simplify webhook tests into pure unit specs with randomized order.
• Replace captured Rook monitor YAML fixtures with typed builders plus a contract test to guard against drift.
• Add hermetic make test-unit, test-unit-repeat, and test-envtest targets; update make test to run the layered suite without mutating source or cloning dependencies.

File summaries

File	Description
test/builders/monitor.go	Adds typed builders for monitor Deployment/Secrets/ConfigMap to replace YAML fixtures.
test/builders/monitor_test.go	Adds contract tests to lock down the builder fields the controller relies on.
pkg/webhook/v1alpha1/suite_test.go	Converts webhook suite to pure unit Ginkgo entrypoint (no envtest/bootstrap).
pkg/controller/suite_test.go	Gates controller suite behind envtest build tag and switches fixtures to builders.
pkg/controller/remotecluster_controller_test.go	Adds envtest build tag to exclude from unit runs.
pkg/controller/remotearbiter_controller_test.go	Adds envtest build tag to exclude from unit runs.
Makefile	Adds hermetic unit/envtest targets and makes test run them without pretty/deps.
docs/superpowers/specs/2026-09-03-testing-foundation-design.md	Documents the testing-foundation architecture and rollout plan.
contrib/k8s/test/override-configmap.yaml	Removes YAML fixture (replaced by typed builder).
contrib/k8s/test/mon-deployment.yaml	Removes YAML fixture (replaced by typed builder).
contrib/k8s/test/keyring-secret.yaml	Removes YAML fixture (replaced by typed builder).
contrib/k8s/test/env-var-secret.yaml	Removes YAML fixture (replaced by typed builder).

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The envtest path still depends on missing/unvendored Rook CRDs (contrib/k8s/3rdparty) and the new keyring builder currently mishandles YAML base64 Secret data unless decoding is added, breaking the “fresh checkout” hermetic goal.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

docs/superpowers/specs/2026-09-03-testing-foundation-design.md:38

• This design doc states there is no build/test CI in the repo, but then claims #66 has already shipped .github/workflows/ci.yaml (and references shipped jobs). In this PR branch, .github/workflows/ci.yaml is not present, so these statements are inconsistent and will mislead readers about current status vs planned work.

test/builders/monitor.go:221

• MonitorKeyringSecret currently stores the base64 string bytes into Secret.Data; once defaultKeyringData is decoded, this should use the decoded byte slice so the controller copies/mounts the real keyring content.

	return &corev1.Secret{
		TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Secret"},
		ObjectMeta: metav1.ObjectMeta{Name: o.name, Namespace: o.namespace},
		Data:       map[string][]byte{"keyring": []byte(defaultKeyringData)},
	}

test/builders/monitor.go:34

• defaultKeyringData is the base64 string from the YAML Secret data: field, but corev1.Secret.Data must contain decoded bytes. Decode the base64 string once at build time so envtests exercise the controller with a realistic keyring payload.

	// keyring value copied verbatim from the captured keyring-secret.yaml.
	defaultKeyringData = "Clttb24uXQoJa2V5ID0gQVFCVm1RTnAxSm94SkJBQW5GUHVZUGZhcUZORkJIWUhCb0N0VGc9PQoJY2FwcyBtb24gPSAiYWxsb3cgKiIKCgpbY2xpZW50LmFkbWluXQoJa2V5ID0gQVFCVm1RTnAxbEF2SlJBQVp0V3ZZdDZnYXpPMWdTR1M5SEtYSFE9PQoJY2FwcyBtZHMgPSAiYWxsb3cgKiIKCWNhcHMgbW9uID0gImFsbG93ICoiCgljYXBzIG9zZCA9ICJhbGxvdyAqIgoJY2FwcyBtZ3IgPSAiYWxsb3cgKiIK"
)

type options struct {

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new monitor keyring Secret builder stores base64-encoded fixture data as literal bytes, diverging from real Kubernetes-decoded Secret.Data and risking incorrect envtest behavior/fidelity.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

docs/superpowers/specs/2026-09-03-testing-foundation-design.md:31

• This status note claims #66/#68 already shipped and references .github/workflows/ci.yaml, but that workflow file is not present in this PR branch. As written, the doc reads as if CI work is already in-tree, which is misleading given this PR’s scope (#67 only).

This issue also appears on line 36 of the same file.

test/builders/monitor.go:233

• MonitorKeyringSecret sets Secret.Data["keyring"] to the literal bytes of defaultKeyringData, but defaultKeyringData is copied from the deleted YAML fixture’s data: field (base64-encoded on the wire). When the YAML was deserialized previously, Kubernetes decoded the base64 into the real keyring bytes; the new builder currently double-encodes the keyring and will cause the controller to propagate the wrong secret contents in envtest (it copies s.monitorKeyringSecret.Data verbatim). Decode the base64 string (or store the decoded keyring text directly) before assigning to Data.

func MonitorKeyringSecret(opts ...Option) *corev1.Secret {
	o := resolve(defaultKeyringSecret, "", opts)
	return &corev1.Secret{
		TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Secret"},
		ObjectMeta: metav1.ObjectMeta{Name: o.name, Namespace: o.namespace},
		Data:       map[string][]byte{"keyring": []byte(defaultKeyringData)},
	}

docs/superpowers/specs/2026-09-03-testing-foundation-design.md:36

• The #66 deliverable row states .github/workflows/ci.yaml and its jobs are already shipped, but that workflow isn’t present in this PR. Mark this as planned/future work (or remove detailed job claims) to avoid documenting non-existent artifacts.

| #66 | `.github/workflows/ci.yaml` — SHA-pinned, least-privilege, concurrency-cancel, timeouts, artifact-on-failure. Shipped jobs: `generate` (gen/helm/imports diff), `verify` (fmt/vet/lint/unit-race-shuffle/build), `envtest`, `helm` (lint+template), `vuln` (govulncheck); REUSE runs from the separate `reuse.yaml`. Documented required-check names. Split fast (PR) vs scheduled (nightly). |

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Lite

[marcelradke]

LGTM
I could run the unit tests
Seems a good preparation for further unit tests:
Replacing the yaml files to go-structures avoids local files and enables possible variations more easily.

[jrse]

@dependabot rebase