# Deep Review: 20260316-111459-pr-238

| | |
|---|---|
| **Date** | 2026-03-16 11:14 |
| **PR** | [#238](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/238) — Split RDD finalizer into /cleanup and /owned |
| **Branch** | `split-finalizers` |
| **Commits** | `9de9269` Split RDD finalizer into /cleanup and /owned |
| **Reviewers** | Claude Opus 4.6, Codex GPT 5.4, Gemini 3.1 Pro |
| **Verdict** | **Merge with fixes** — upgrade path for legacy finalizers must be addressed |
| **Wall-clock time** | 12 min 27 s |

---

## Consolidated Review

### Executive Summary

This PR splits the single shared `rdd.rancherdesktop.io/cleanup` finalizer into two purpose-specific finalizers: `/cleanup` for self-protection (a resource's own controller runs cleanup before deletion) and `/owned` for cascade blocking (an owner prevents child deletion until explicit release via `DeleteOwnedResources`). The split fixes a real bug where `DeleteOwnedResources` stripped the only finalizer on a child resource, bypassing the child controller's cleanup logic. The implementation is correct for new resources but leaves existing resources stranded on upgrade: legacy template ConfigMaps still carry `/cleanup`, which `DeleteOwnedResources` no longer strips.

---

### Critical Issues

None.

---

### Important Issues

1. **Legacy template ConfigMaps become stuck in Terminating after upgrade** — `pkg/controllers/base/finalizer.go:159-176` [Claude Opus 4.6, Codex GPT 5.4, Gemini 3.1 Pro]

```go
patch := client.MergeFrom(item.DeepCopy())
if controllerutil.RemoveFinalizer(&item, OwnedFinalizerName) {
    if err := c.Patch(ctx, &item, patch); err != nil {
        // ...
    }
}
if err := c.Delete(ctx, &item); err != nil && client.IgnoreNotFound(err) != nil {
    // ...
}
```

Before this PR, the LimaVM webhook created template ConfigMaps with `rdd.rancherdesktop.io/cleanup` (the old `FinalizerName`). After this PR, `DeleteOwnedResources` strips only `/owned`. When a user deletes a LimaVM created before the upgrade:

- `DeleteOwnedResources` tries to strip `/owned` from the template ConfigMap — not present, so no-op.
- `c.Delete` marks the ConfigMap for deletion.
- The old `/cleanup` finalizer blocks actual deletion. No controller removes `/cleanup` from child ConfigMaps — `RemoveCleanupFinalizer` runs only for the resource's own controller.
- The template ConfigMap is stuck in Terminating permanently.

All three agents identified this issue. Claude rated it important/gap; Codex rated it important/regression; Gemini rated it critical/regression. The correct classification is **important, regression**: the PR introduces the incompatibility, but recovery is straightforward in the single-user embedded context (delete the kine database or manually patch the finalizer).

Fix: Migrate legacy children. The simplest approach: in the LimaVM reconciler, when fetching the template ConfigMap during normal reconciliation, check whether it carries `/cleanup` without `/owned`. If so, patch to replace `/cleanup` with `/owned`. This keeps the migration close to the only controller that creates these ConfigMaps and avoids polluting the generic `DeleteOwnedResources` helper with knowledge of the legacy scheme.

---

### Suggestions

1. **Stale comment claims owned resources "have no finalizers"** — `pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:60-64` [Claude Opus 4.6]

```go
// Delete owned resources and remove the finalizer in one pass. This is safe
// because owned resources (ConfigMaps) delete instantly and have no finalizers.
// If we later own resources with complex teardown, split this into two reconcile
// cycles: delete owned resources, then verify they are gone before removing
// the finalizer.
```

After this PR, template ConfigMaps carry the `/owned` finalizer. The practical conclusion (delete in one pass) remains correct because `DeleteOwnedResources` strips `/owned` before issuing the delete, but the stated reasoning is wrong. Blame confirms these lines predate this PR (`de1f2298`), but the PR made the comment inaccurate.

Fix: Rewrite the comment to explain that `DeleteOwnedResources` strips `/owned` before deletion, and the two-cycle caveat applies if a child ever needs `/cleanup` alongside `/owned`.

2. **Missing test case for legacy child with only `/cleanup` finalizer** — `pkg/controllers/base/finalizer_test.go` [Claude Opus 4.6, Codex GPT 5.4]

```go
// Missing case:
{
    name:          "config map with cleanup finalizer only (legacy)",
    finalizers:    []string{CleanupFinalizerName},
    expectDeleted: false,
},
```

The test covers no finalizers, owned-only, and both-finalizers. The missing case is a child with only `/cleanup` — the upgrade scenario from Important Issue #1. `RemoveFinalizer` at line 162 returns false (different code path), and the resource gets stuck in Terminating.

Fix: Add the test case above. After implementing the migration fix, add a corresponding integration test that creates a legacy-finalizered ConfigMap and verifies LimaVM deletion cleans it up.

3. **`EnsureOwnedFinalizer` and `RemoveOwnedFinalizer` are unused** — `pkg/controllers/base/finalizer.go:60-78` [Gemini 3.1 Pro]

```go
func EnsureOwnedFinalizer(ctx context.Context, c client.Client, obj client.Object) (bool, error) {
```

The LimaVM webhook applies `OwnedFinalizerName` directly in the struct literal (controller.go:211), and `DeleteOwnedResources` removes it via `controllerutil.RemoveFinalizer` with a direct patch (finalizer.go:162). Neither caller uses the new helpers.

Fix: Either remove these functions or refactor existing code to use them. They provide a consistent API surface for future controllers, so keeping them is reasonable — but document that intent.

---

### Design Observations

1. **`DeleteOwnedResources` is now schema-version-sensitive** (in-scope) [Codex GPT 5.4]

The helper assumes every child that should be unblocked already uses `/owned`. Encoding a short compatibility window (stripping the legacy finalizer from owned resources) in the helper would eliminate the entire class of upgrade bugs across all controllers, rather than requiring each controller to handle its own migration. The tradeoff: this would contradict the design principle that `DeleteOwnedResources` does not strip `/cleanup`. A targeted migration in the owning controller is safer and keeps the helper's contract clean.

2. **The `/owned` finalizer doubles as a user-facing deletion guard** (in-scope) [Gemini 3.1 Pro]

Child resources like template ConfigMaps do not need `/owned` for the garbage collection loop to work — `DeleteOwnedResources` can delete resources without a finalizer. The `/owned` finalizer's primary value is blocking accidental `kubectl delete` attempts via the `ValidateDelete` webhook. This layered approach (webhook + finalizer) is sound but worth documenting explicitly so future contributors understand the finalizer's dual purpose.

---

### Testing Assessment

The unit tests cover the three key scenarios for the new design: no finalizer, owned-only (deleted), and both finalizers (owned stripped, cleanup remains). The BATS integration test verifies that LimaVM gets `/cleanup` and the template ConfigMap gets `/owned`.

Untested scenarios ranked by risk:

1. **Upgrade path**: Child resource with only the legacy `/cleanup` finalizer processed by `DeleteOwnedResources`. Highest risk — this is the scenario from Important Issue #1.
2. **Webhook validation during cascade deletion**: Verifying that `ValidateDelete` allows deletion of a template ConfigMap immediately after `DeleteOwnedResources` strips `/owned`.
3. **Concurrent owner deletion**: Two owners deleting simultaneously, both calling `DeleteOwnedResources` on a shared child. The patch/delete is naturally idempotent (NotFound handled at line 166-168), so this is likely safe but untested.

---

### Documentation Assessment

The new `docs/design/controllers.md` documents the finalizer design clearly, including the "Why Two Finalizers?" rationale. Cross-links from `README.md` and `discovery.md` are appropriate.

One gap: the design doc should mention the upgrade path for existing resources, or state explicitly that migration from the single-finalizer scheme is required.

---

## Agent Performance Retro

### Claude Opus 4.6
- **Unique contributions**: Identified the stale comment in `limavm_lifecycle.go:60-64`.
- **Accuracy**: All findings verified. Correctly classified the upgrade issue as "important, gap" (though "regression" is more precise since this PR introduced the incompatibility).
- **Depth**: Traced through `DeleteOwnedResources`, the webhook, and the reconciler. Used git blame to confirm origins. Explored the design tradeoff between helper-level and controller-level migration.
- **Signal-to-noise**: Excellent. Every finding is actionable. No false positives.

### Codex GPT 5.4
- **Unique contributions**: None beyond the shared upgrade finding.
- **Accuracy**: All findings verified. Correctly classified the upgrade issue as "important, regression."
- **Depth**: Traced the full deletion path from `DeleteOwnedResources` through to the parent finalizer removal. Concise analysis.
- **Signal-to-noise**: High signal, very concise review. No false positives.

### Gemini 3.1 Pro
- **Unique contributions**: Identified unused helper functions (`EnsureOwnedFinalizer`, `RemoveOwnedFinalizer`). Flagged the ConfigMap orphan issue (pre-existing gap, not a regression).
- **Accuracy**: Rated the upgrade issue as "critical, regression" — overcalibrated for an embedded single-user context. The ConfigMap orphan finding is legitimate but pre-existing.
- **Depth**: Read the webhook code, traced the admission flow, and identified the orphan scenario. Good cross-component analysis.
- **Signal-to-noise**: Good, but the orphan finding predates this PR and the severity calibration was off.

### Summary Table

| Metric | Claude Opus 4.6 | Codex GPT 5.4 | Gemini 3.1 Pro |
|--------|-----------------|----------------|----------------|
| Duration | 4:16 | 3:20 | 4:26 |
| Critical | 0 | 0 | 1 (overcalibrated) |
| Important | 1 | 1 | 1 |
| Suggestion | 2 | 1 | 1 |
| Design observations | 2 | 1 | 1 |
| False positives | 0 | 0 | 0 |
| Unique insights | 1 | 0 | 2 |

Claude provided the most thorough analysis with the best severity calibration. Gemini contributed the most unique findings but overcalibrated severity. Codex was accurate and concise but offered no unique insights beyond the shared finding.

---

## Skill Improvement Recommendations

- The prompt's instruction to "calibrate severity for an embedded single-user context" worked for Claude and Codex but not Gemini. Consider adding a concrete example to the repo context (e.g., "an issue that requires deleting the kine database to recover is IMPORTANT, not CRITICAL").
- All three agents found the same upgrade issue, confirming it as the PR's central risk. The prompt dimensions are comprehensive for this type of change.
- No aspects were missed by all agents.
- The clarification round was unnecessary — all findings were clear and consistent.

---

## Appendix: Original Reviews

### Claude Opus 4.6

## Executive Summary

This PR cleanly separates a single shared finalizer into two purpose-specific finalizers: `/cleanup` for self-protection (a resource's own controller runs cleanup before deletion) and `/owned` for cascade blocking (an owner prevents child deletion until explicit release). The split fixes a real bug where `DeleteOwnedResources` stripped the only finalizer on a child resource, bypassing the child's own cleanup logic. The implementation is correct for new installations but leaves existing resources stranded on upgrade.

## Findings

### Critical Issues

None.

### Important Issues

1. **Existing template ConfigMaps get stuck in Terminating on upgrade** — `pkg/controllers/base/finalizer.go:162`, `pkg/controllers/lima/limavm/controller.go:210-211` (important, gap)

```go
// Old code created template ConfigMaps with:
Finalizers: []string{base.FinalizerName},   // "rdd.rancherdesktop.io/cleanup"

// New code creates them with:
Finalizers: []string{base.OwnedFinalizerName}, // "rdd.rancherdesktop.io/owned"
```

Template ConfigMaps already persisted in kine/sqlite carry the old `rdd.rancherdesktop.io/cleanup` string. When a LimaVM is deleted after upgrading to this code:

- `DeleteOwnedResources` (line 162) tries to strip `rdd.rancherdesktop.io/owned` — not present, so no-op.
- `c.Delete` (line 176) marks the ConfigMap for deletion.
- The `ValidateDelete` webhook (controller.go line 276) checks `HasOwnedFinalizer` — not present, so allows deletion.
- But the old `/cleanup` finalizer still blocks actual deletion. No controller removes `/cleanup` from child resources — `RemoveCleanupFinalizer` is only called by a resource's *own* controller.
- The template ConfigMap gets stuck in Terminating forever.

Confirmed via blame: lines 210-211 were changed by this PR (`9de92692`), and the old value was `FinalizerName = "rdd.rancherdesktop.io/cleanup"`.

In the single-user embedded context this is recoverable (delete the kine db or manually patch the finalizer), but it will silently break deletion for any user who upgrades with existing LimaVM resources.

Fix: Add migration logic in the reconciler (or `DeleteOwnedResources`) to strip the legacy `/cleanup` finalizer from owned resources. A simple approach: in `DeleteOwnedResources`, also strip `CleanupFinalizerName` from owned resources (resources that pass the `IsOwnedByUID` check). Alternatively, strip the legacy finalizer during reconciliation when setting the owner reference at `limavm_controller.go:232-243`.

### Suggestions

1. **Stale comment claims owned resources "have no finalizers"** — `pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:60-63` (suggestion, gap)

```go
// Delete owned resources and remove the finalizer in one pass. This is safe
// because owned resources (ConfigMaps) delete instantly and have no finalizers.
// If we later own resources with complex teardown, split this into two reconcile
// cycles: delete owned resources, then verify they are gone before removing
// the finalizer.
```

After this PR, template ConfigMaps *do* have the `/owned` finalizer. `DeleteOwnedResources` strips it before the delete call, so the practical conclusion (delete in one pass) is still correct, but the stated reasoning is now wrong. Confirmed via blame: lines 60-64 are from `de1f2298` (pre-existing), not this PR.

Fix: Update the comment to explain that `DeleteOwnedResources` strips `/owned` before deletion, and the "two reconcile cycles" caveat applies if a child ever needs a `/cleanup` finalizer alongside `/owned`.

2. **Missing test case: child with only cleanup finalizer** — `pkg/controllers/base/finalizer_test.go` (suggestion, gap)

The test covers no finalizers, owned-only, and both-finalizers. The missing case is a child with only the `/cleanup` finalizer (e.g., the upgrade scenario above). In this case, `RemoveFinalizer` at line 162 returns false (different code path — the patch is skipped), and `c.Delete` marks the resource for deletion but it stays stuck. This is the exact behavior that will happen on upgrade.

Fix: Add a test case:
```go
{
    name:          "config map with cleanup finalizer only (legacy)",
    finalizers:    []string{CleanupFinalizerName},
    expectDeleted: false,
},
```

### Design Observations

1. **The owned finalizer could also protect against accidental direct deletion** (in-scope)

   The `ValidateDelete` webhook at `controller.go:276` checks only `HasOwnedFinalizer` to block direct deletion. After `DeleteOwnedResources` strips `/owned`, the webhook allows the delete, and the ConfigMap is deleted. This is correct.

   But if a resource has both `/cleanup` and `/owned`, `DeleteOwnedResources` strips `/owned`, issues a delete, and the resource enters Terminating. The child controller sees `IsBeingDeleted` and runs cleanup, then removes `/cleanup`. The webhook's check is now redundant in this path since the finalizer itself already blocks deletion. The webhook's value is preventing *direct* `kubectl delete` attempts that bypass `DeleteOwnedResources` — a user-facing UX guardrail. This layered approach (webhook + finalizer) is sound.

2. **`DeleteOwnedResources` could strip both owned and legacy finalizers in one pass** (in-scope)

   Rather than adding separate migration logic, `DeleteOwnedResources` could strip *both* `/owned` and `/cleanup` from owned resources. This would handle the upgrade path naturally: any child resource identified via `IsOwnedByUID` gets both stripped, regardless of which was set. The `/cleanup` finalizer on the *owner itself* is untouched (it's never an "item" in the loop). This is simpler than a separate migration step and eliminates the category of upgrade bugs permanently. The tradeoff: if a child resource ever legitimately needs `/cleanup` for its own controller's cleanup, this approach would bypass it. But the design doc already states `DeleteOwnedResources` should *not* strip `/cleanup`, so this would contradict the stated design principle. A targeted migration (strip legacy `/cleanup` only when `/owned` is absent on an owned resource) would be safer.

### Testing Assessment

Test coverage is good for the new two-finalizer design:

1. Unit test covers the three key scenarios: no finalizer, owned-only (deleted), both (owned stripped, cleanup remains).
2. BATS integration test verifies the LimaVM gets `/cleanup` (line 56) and the template ConfigMap gets `/owned` (line 77).

Untested scenarios, ranked by risk:

1. **Upgrade path**: Child resource with only the legacy `/cleanup` finalizer being processed by `DeleteOwnedResources`. This is the highest-risk gap (see Important Issue #1 and Suggestion #2).
2. **Concurrent owner deletion**: Two owners deleting simultaneously, both calling `DeleteOwnedResources` on a shared child. The patch/delete is naturally idempotent (NotFound is handled at line 166-168), so this is likely fine, but untested.

### Documentation Assessment

The new `docs/design/controllers.md` is well-written and provides clear documentation of the finalizer design, including the "Why Two Finalizers?" rationale. The cross-links from `README.md` and `discovery.md` are appropriate.

One gap: the design doc should mention the upgrade path for existing resources, or explicitly state that migration from the single-finalizer scheme is required.

---

### Codex GPT 5.4

### Executive Summary

This PR cleanly separates RDD's single finalizer into `/cleanup` and `/owned`, and the new Lima tests plus helper updates cover the intended steady-state behavior. The main issue is upgrade compatibility: resources created before this PR still carry the old single `rdd.rancherdesktop.io/cleanup` finalizer, but the new code no longer treats that value as an owned-resource blocker, which leaves legacy Lima template `ConfigMap`s stuck terminating and lets their parent `LimaVM` delete anyway.

### Findings

**Critical Issues**

None.

**Important Issues**

1. **Legacy template ConfigMaps become undeletable after upgrade** — `pkg/controllers/base/finalizer.go:159-162` (important, regression)

```go
// Strip the owned finalizer to unblock deletion. The cleanup finalizer (if any)
// remains so the child's own controller can still run its cleanup logic.
patch := client.MergeFrom(item.DeepCopy())
if controllerutil.RemoveFinalizer(&item, OwnedFinalizerName) {
```

Before this PR, Lima template `ConfigMap`s were created with the shared `rdd.rancherdesktop.io/cleanup` finalizer in `pkg/controllers/lima/limavm/controller.go:210-211` on `upstream/main`; after this PR, new ones use `/owned` at `pkg/controllers/lima/limavm/controller.go:210-211`, and `DeleteOwnedResources()` now strips only `/owned` at `pkg/controllers/base/finalizer.go:159-162`. On an upgraded daemon, any pre-existing template `ConfigMap` still carrying the old `/cleanup` value will keep that finalizer forever: `DeleteOwnedResources()` will call `Delete()` without removing it, and no controller ever clears finalizers on template `ConfigMap`s. The LimaVM deletion path at `pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:65-71` then removes the parent `/cleanup` finalizer immediately, so the `LimaVM` disappears while the template `ConfigMap` is left stuck in `Terminating`. `git blame` shows the behavior change was introduced on `pkg/controllers/base/finalizer.go:159-162` and `pkg/controllers/lima/limavm/controller.go:210-211` by commit `9de92692`.

Fix: add a compatibility path for legacy children. The simplest option is to treat the legacy `rdd.rancherdesktop.io/cleanup` value as removable by `DeleteOwnedResources()` for child resources during a transition period. A more explicit option is to migrate existing template `ConfigMap`s from `/cleanup` to `/owned` before relying on the new semantics. Either way, add an upgrade test that creates a legacy Lima template `ConfigMap` and verifies `LimaVM` deletion fully cleans it up.

**Suggestions**

1. **Add an upgrade-regression test for the old finalizer value** — `pkg/controllers/base/finalizer_test.go:75-78` (suggestion, enhancement)

```go
{
    name:          "config map with both finalizers strips only owned",
    finalizers:    []string{CleanupFinalizerName, OwnedFinalizerName},
    expectDeleted: false,
},
```

The updated test suite proves the new split behavior, but it does not exercise the real upgrade case: an existing child object with only the legacy `rdd.rancherdesktop.io/cleanup` finalizer. That is the high-risk path for this PR, and the current test at `pkg/controllers/base/finalizer_test.go:75-78` instead codifies only the new dual-finalizer model. `git blame` shows those lines were added in `9de92692`.

Fix: add a case with `finalizers: []string{CleanupFinalizerName}` that models a pre-upgrade child, then assert the chosen compatibility behavior explicitly. Ideally add an integration-level LimaVM lifecycle test as well, since the bug crosses the webhook, helper, and reconciler layers.

### Design Observations

1. **`DeleteOwnedResources()` now depends on a migration story, not just a new convention** (in-scope)

The split is a good model, but the helper has become schema-version-sensitive: it assumes every child that should be unblocked already uses `/owned`. Encoding a short compatibility window in the helper eliminates an entire class of upgrade bugs across controllers, instead of requiring each controller to remember its own migration.

### Testing Assessment

1. The highest-risk missing scenario is upgrading an existing LimaVM whose template `ConfigMap` still has the legacy `rdd.rancherdesktop.io/cleanup` finalizer, then deleting the LimaVM.
2. There is no test for manually deleting a legacy template `ConfigMap` after upgrade; with `ValidateDelete()` now checking only `/owned` at `pkg/controllers/lima/limavm/controller.go:276`, that path should be verified too.
3. There is no controller-level test proving that a child left with `/cleanup` triggers its own reconciler and eventually clears that finalizer before the parent finalizer is removed.

### Documentation Assessment

The new controller-framework docs are clear for new objects, but they do not mention upgrade compatibility for resources still carrying the legacy single finalizer. If this PR keeps a transition path, `docs/design/controllers.md` should document it briefly.

---

### Gemini 3.1 Pro

### Executive Summary

This PR correctly addresses an architectural limitation where a single finalizer forced `DeleteOwnedResources` to short-circuit child resources' self-cleanup logic, successfully splitting responsibilities into `/cleanup` and `/owned` finalizers. However, the change breaks backward compatibility for existing resources (e.g. legacy `TemplateConfigMap`s) that still hold the old `/cleanup` finalizer but rely entirely on `DeleteOwnedResources` for cascade garbage collection.

### Findings

#### Critical Issues

1. **Upgrade compatibility broken for existing `TemplateConfigMap`s** — `pkg/controllers/base/finalizer.go:162-164` `(critical, regression)`

```go
			patch := client.MergeFrom(item.DeepCopy())
			if controllerutil.RemoveFinalizer(&item, OwnedFinalizerName) {
				// Use Patch instead of Update for PartialObjectMetadata
```

Before this PR, `TemplateConfigMap`s created by the `LimaVM` mutating webhook were assigned the `rdd.rancherdesktop.io/cleanup` finalizer. With this update, `DeleteOwnedResources` has been modified to exclusively strip `OwnedFinalizerName` (`rdd.rancherdesktop.io/owned`). When a user deletes a `LimaVM` that was created prior to this upgrade, `DeleteOwnedResources` will issue a `Delete` on its child `TemplateConfigMap` without stripping the legacy `/cleanup` finalizer. Because the ConfigMap is not a custom resource and lacks a controller to execute its own cleanup, it will become permanently stuck in the `Terminating` state, preventing full deletion of the parent.

Fix: Provide a migration path for legacy resources. Modify the `LimaVMReconciler` to inspect the `templateConfigMap` upon fetching it; if it contains `CleanupFinalizerName`, strip it and append `OwnedFinalizerName` via a `Patch` before continuing normal operation.

#### Important Issues

2. **Template ConfigMap orphaned if LimaVM creation fails** — `pkg/controllers/lima/limavm/controller.go:226-236` `(important, gap)`

```go
	// Create the ConfigMap (this triggers ConfigMap admission webhook for validation)
	if err := d.Client.Create(ctx, templateConfigMap, options...); err != nil {
		return fmt.Errorf("failed to create template ConfigMap: %w", err)
	}
```

The mutating webhook generates the `templateConfigMap` and assigns it the `OwnedFinalizerName` finalizer synchronously before the `LimaVM` itself is created in etcd. At this stage, it cannot attach an OwnerReference because the `LimaVM` UID is not yet available. If the `LimaVM` resource creation is subsequently rejected by the API server (e.g., due to a validating webhook, RBAC limits, or an etcd transient error), the ConfigMap is permanently orphaned. Because it lacks an OwnerReference, `DeleteOwnedResources` will never be able to discover and clean it up. Furthermore, because it holds the `/owned` finalizer, the `ConfigMapValidator.ValidateDelete` hook blocks users from manually deleting it.

Fix: Defer the creation of the `templateConfigMap` from the mutating webhook to the `LimaVMReconciler`. Creating the ConfigMap in the reconciler ensures the `LimaVM` UID is available to atomically assign an OwnerReference, preventing side-effect leaks during admission requests.

#### Suggestions

3. **Unused finalizer management functions** — `pkg/controllers/base/finalizer.go:60-75` `(suggestion, enhancement)`

```go
// EnsureOwnedFinalizer adds the owned finalizer to a child object if not already present.
// Returns true if the finalizer was added and the object has been updated.
func EnsureOwnedFinalizer(ctx context.Context, c client.Client, obj client.Object) (bool, error) {
```

The newly introduced helper functions `EnsureOwnedFinalizer` and `RemoveOwnedFinalizer` are entirely unused in the codebase. The `LimaVM` webhook applies the `OwnedFinalizerName` directly within its struct initialization (line 211 in `lima/limavm/controller.go`), and `DeleteOwnedResources` removes it via `controllerutil.RemoveFinalizer` combined with a direct patch (line 162 in `base/finalizer.go`).

Fix: Either remove these functions to eliminate dead code, or refactor existing components to utilize them for better consistency across controllers.

### Design Observations

**Two Finalizers vs One** `(in-scope)`
The introduction of two distinct finalizers (`/cleanup` for self-protection and `/owned` for cascade blocking) cleanly separates responsibilities. It ensures that `DeleteOwnedResources` does not prematurely short-circuit a child resource's teardown logic. However, this model highlights that child resources not strictly requiring self-protection (like generic `ConfigMap`s) technically do not need an explicit finalizer at all, given RDD completely relies on `DeleteOwnedResources` for manual cascade deletion rather than standard Kubernetes GC. The `/owned` finalizer on these children primarily acts as a user-facing lock to prevent accidental manual deletion, which is useful but not strictly necessary for the garbage collection loop to successfully function.

### Testing Assessment

The PR includes strong unit tests inside `finalizer_test.go` that reliably assert `DeleteOwnedResources` strips the owned finalizer while correctly leaving the cleanup finalizer intact.

Additional untested scenarios ranked by risk:
1. **Webhook validation during cascade deletion:** A test verifying that the `ConfigMapValidator.ValidateDelete` successfully allows deletion of a `TemplateConfigMap` immediately after `DeleteOwnedResources` executes its patch, confirming the finalizer is successfully hidden from the webhook's `OldObject` validation.
2. **Backward compatibility finalizer migration:** If the fix for legacy finalizers is implemented, a test should verify that a `ConfigMap` holding the legacy `/cleanup` finalizer is successfully migrated to `/owned` and subsequently deleted.

### Documentation Assessment

The PR brings excellent architectural documentation in `docs/design/controllers.md` that concisely outlines the dual-finalizer design, why two independent finalizers are necessary, and how `DeleteOwnedResources` parses them. The inline documentation surrounding the finalizer constant definitions is appropriately updated. No documentation gaps were identified.