CHANGELOG.md

@@ -1,5 +1,37 @@
 # Changelog
 
+## 2026-09-10 22:58 UTC — Code-review fixes on the KVM branch: deletion, validation, lazy SSH, guest agent
+
+Five findings from a review of `feat/kvm-provider`, all fixed on the branch.
+
+- **A Proxy can always finish deleting.** The delete path used to drop the finalizer
+  only when `Get` answered not-found, so a permanent error there retried forever —
+  a KVM host removed from the config was enough. It now calls `Delete` anyway, since
+  that is idempotent and can finish where `Get` cannot, and
+  `crawl.example.com/skip-cleanup: "true"` abandons the instance for whatever stays
+  stuck. In the KVM provider, deleting over an unconfigured host is a no-op success;
+  `Get` still fails permanently there so a typo cannot look like a missing VM.
+- **The guest agent ships as a vendor-data script**, superseding the cloud-config
+  merge in the entry below. Merging could never be made safe: cloud-init drops
+  vendor-data keys the user-data defines, and the strategy cannot be overridden from
+  either document. The Proxy's cloud-init now reaches the seed byte for byte in any
+  format — Jinja, MIME, gzip, scripts — where the round-trip previously re-typed
+  unquoted scalars (`version: 1.10` arrived as `1.1`), dropped comments and reordered
+  keys. New `installGuestAgent` setting, default on.
+- **A missing `kvm-ssh` Secret no longer crash-loops the manager.** The SSH material
+  is read on first use and cached, so only KVM Proxies fail and they recover when the
+  Secret appears. A missing file is transient, an unparseable key permanent, and
+  startup still logs a warning.
+- **Host names and image keys are validated.** Either can end up in a providerID or a
+  volume file name, where a slash made the VM unaddressable for good.
+- Verified end to end: unit suite, the integration test against a real hypervisor
+  (22 s, agent reporting the IP), both in-cluster Proxies Ready in 2 min with 204
+  through squid, the guest showing `qemu-guest-agent` active with its cloud-config
+  comments intact, and the manager staying up with the Secret pointed away.
+- Key files: `internal/provider/kvm/{seed.go,conn.go,kvm.go}`,
+  `internal/controller/proxy_controller.go`, `internal/provider/config_kvm.go`,
+  `api/v1alpha1/proxy_types.go`.
+
 ## 2026-09-10 21:04 UTC — KVM provider passes in-cluster; guest agent merged into cloud-config
 
 - Two KVM Proxies with real cloud-configs reach `Ready` from inside the kind cluster

docs/plans-executions/2026-09-04-1404-kvm-provider.md

@@ -631,6 +631,26 @@ and the `beelink` host entries in `config/manager/providers_config.yaml`
 and `config/samples/providers-config.yaml`, which are this machine's dev
 setup rather than the repo's defaults.
 
+### Run 8: review fixes
+
+A `/code-review high` pass over the branch found five defects, fixed in
+[plan 2026-09-10-2222](../plans/2026-09-10-2222-kvm-review-fixes.md) with
+its own execution log. Two of them land on Run 7's conclusions above and
+supersede them:
+
+- The guest-agent **merge described in Run 7 is gone**. It could not be made
+  safe for user-data that is not a plain cloud-config, and its YAML
+  round-trip re-typed unquoted scalars. The bootstrap now ships as a
+  vendor-data shell script, which cloud-init never merges, and the Proxy's
+  cloud-init reaches the seed byte for byte.
+- The read-only-filesystem fix from Run 7 still stands, but the eager SSH
+  key load it sat next to does not: a missing `kvm-ssh` Secret aborted the
+  whole provider set and crash-looped the manager. The material is read on
+  first use now.
+
+Re-validated the same way: integration test against beelink, both Proxies
+recreated in kind, 204 through squid.
+
 ### MR opened
 
 [PR #5](https://gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/pulls/5),

docs/plans-executions/2026-09-10-2222-kvm-review-fixes.md

@@ -6,7 +6,7 @@ Plan: [docs/plans/2026-09-10-2222-kvm-review-fixes.md](../plans/2026-09-10-2222-
 - [x] Step 2 — Validate names that become identifiers (finding 2)
 - [x] Step 3 — Read the SSH material lazily (finding 3)
 - [x] Step 4 — Guest agent via vendor-data script (findings 4 and 5)
-- [ ] Step 5 — Validate, record, push
+- [x] Step 5 — Validate, record, push
 
 ## Step 0 — plan committed
 
@@ -162,6 +162,61 @@ across seven formats. One consequence worth remembering: a user-data that
 sets `vendor_data: {enabled: false}` disables the agent, and cloud-init
 documents that as intended behaviour.
 
+## Step 5 — Validation
+
+Unit suite, `go vet` and `gofmt` clean across the repo. Then the two real
+runs.
+
+**Integration against beelink**, unchanged environment from the KVM plan's
+log, passed in 22 s on a warm cache, with
+`virDomainInterfaceAddresses source=DomainInterfaceAddressesSrcAgent
+interfaces=2` — the agent installed by the new vendor script answering, on
+a stock Debian 13 image.
+
+**In-cluster**, both Proxies deleted and recreated against the rebuilt
+image: Ready with an address 2 min 3 s after `kubectl apply`, 204 through
+squid from the dev box for both. Inside the debug VM:
+
+```sh
+ssh novakj@192.168.0.94 'systemctl is-active qemu-guest-agent; cloud-init status; \
+  sudo head -4 /var/lib/cloud/instance/user-data.txt; sudo ls /var/lib/cloud/instance/scripts/vendor/'
+# active
+# status: done
+# #cloud-config
+# # Login for debugging: user "novakj", password "kvm-dev", passwordless sudo.
+# ...
+# part-001
+```
+
+Two things worth having seen with your own eyes there: the user's **comment
+lines survived** into the guest, which the old merge stripped, and the
+script really did land in the vendor directory rather than being merged
+into the user's config.
+
+**Negative check for the Secret**, done by pointing the deployment's volume
+at a name that does not exist rather than deleting the real Secret:
+
+```sh
+kubectl -n egress-proxies-operator-system patch deploy egress-proxies-operator-controller-manager \
+  --type=json -p '[{"op":"replace","path":"/spec/template/spec/volumes/1/secret/secretName","value":"kvm-ssh-absent"}]'
+```
+
+The manager came up and stayed up, 0 restarts, logging
+`WARNING: kvm ssh material is not usable ... open /etc/proxy-operator/kvm/id_ed25519:
+no such file or directory`. The Kubernetes Proxies kept reconciling; the
+KVM ones failed per reconcile and retried. Before this branch the same
+config exited the process before any controller started. Patched back
+afterwards, everything Ready.
+
+Worth noting: the KVM Proxies kept reporting `Ready` during that window
+rather than degrading visibly. That is existing behaviour, not a
+regression — a transient provider error rides the workqueue and does not
+clear a healthy status — but it does mean the only signal of a missing
+Secret is the log and the reconcile errors. Also, the ListByTag sweep on
+beelink still reports `managed=2` where one domain is an orphan from an
+older failed run; unrelated to this work, and the same leftover noted in
+the KVM plan's Run 6.
+
 Worth noting: the review filed this as a KVM finding, but the fix is mostly
 in the controller, because KVM is the only provider that resolves a
 providerID segment against operator config at all. GCP and Kubernetes hand