# Deep Review: 20260320-180916-pr-243

| | |
|---|---|
| **Date** | 2026-03-20 18:09 |
| **PR** | [#243](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/243) — Encode owner Kind in owned finalizer, document webhook requirement |
| **Branch** | `owned-finalizer-webhook-docs` |
| **Commits** | `54602ea` Encode owner Kind in owned finalizer name<br>`eb0d203` Document validating webhook requirement for owned finalizer |
| **Reviewers** | Claude Opus 4.6, Codex GPT 5.4, Gemini 3.1 Pro |
| **Verdict** | **Merge with fixes** — BATS assertions reference the old error message and will fail in CI |
| **Wall-clock time** | 7 min 29 s |

---

## Consolidated Review

### Executive Summary

This PR encodes the owner Kind in the owned finalizer name (`owned-by-App` instead of `owned`), adds a generic `OwnedDeletionGuard[T]` webhook, refactors `ConfigMapValidator` to embed it, and documents the webhook requirement. The design is clean and the generics usage is correct. One concrete regression breaks BATS tests: the new generic error message does not match the assertions in `limavm-lifecycle.bats`. The documentation example also diverges from the actual error format.

### Critical Issues

1. **BATS assertions expect old error message** — `bats/tests/33-lima-controllers/limavm-lifecycle.bats:139-141,151` (critical, regression) [Codex GPT 5.4]

```bash
assert_output --partial "cannot delete template ConfigMap"
assert_output --partial "protected by the LimaVM controller"
assert_output --partial "delete the owning LimaVM resource instead"
```

The new `OwnedDeletionGuard.ValidateDelete` at `webhook.go:55` produces `cannot delete "<name>": owned by LimaVM; delete the LimaVM resource instead`. The BATS tests at lines 139–141 and 151 still assert the old phrasing (`"cannot delete template ConfigMap"`, `"protected by the LimaVM controller"`). Codex confirmed by running the tests: tests 13 and 14 fail. `git blame` confirms these lines predate the PR (commit `14d79c9f`) and were not updated.

Fix: update the BATS assertions to match the new error format. For example: `assert_output --partial "owned by LimaVM"` and `assert_output --partial "delete the LimaVM resource instead"`.

### Important Issues

1. **Documentation example diverges from actual error format** — `docs/design/controllers.md:52` (important, regression) [Claude Opus 4.6]

```markdown
(e.g., *"cannot delete LimaVM 'rd': owned by App; delete the App resource instead"*).
```

The example includes the resource Kind (`LimaVM`) before the name, but `OwnedDeletionGuard.ValidateDelete` at `webhook.go:55` uses only `obj.GetName()` — producing `cannot delete "rd"`, not `cannot delete LimaVM 'rd'`. This misleads readers about the actual error format.

Fix: change the example to `*"cannot delete 'rd': owned by App; delete the App resource instead"*`.

2. **No unit tests for `OwnedFinalizerOwner` or `OwnedDeletionGuard.ValidateDelete`** — `pkg/controllers/base/` (important, gap) [Claude Opus 4.6]

The PR introduces `OwnedFinalizerFor`, `OwnedFinalizerOwner`, `removeOwnedFinalizers`, and `OwnedDeletionGuard.ValidateDelete`. The existing `TestDeleteOwnedResources` covers `removeOwnedFinalizers` indirectly, and `OwnedFinalizerFor` is exercised through test setup at `finalizer_test.go:72`. Two functions lack direct tests:

- `OwnedFinalizerOwner` — the reverse parse, especially edge cases: no finalizer, multiple owned finalizers.
- `OwnedDeletionGuard.ValidateDelete` — that it rejects when an owned finalizer is present and allows when absent. The webhook rejection logic is the behavioral core of this PR.

Fix: add unit tests for both functions.

### Suggestions

1. **Stale `/owned` reference in `DeleteOwnedResources` docs** — `docs/design/controllers.md:65` (suggestion, gap) [Claude Opus 4.6]

```markdown
strips their `/owned` finalizer, and deletes them.
```

This text still says "strips their `/owned` finalizer" but the code now strips all `owned-by-*` finalizers via `removeOwnedFinalizers`. Since the PR updated surrounding documentation in the same section, this stale reference could have been caught.

Fix: change to "strips their `owned-by-*` finalizers."

2. **DryRun logging dropped from `ValidateDelete`** — `pkg/controllers/base/webhook.go:53` (suggestion, regression) [Claude Opus 4.6, Gemini 3.1 Pro]

```go
func (g *OwnedDeletionGuard[T]) ValidateDelete(_ context.Context, obj T) (admission.Warnings, error) {
```

The old `ConfigMapValidator.ValidateDelete` logged at V(1) during dry-run requests. The generic `OwnedDeletionGuard.ValidateDelete` ignores context entirely. The create/update paths still log dry-run via `validateTemplateConfigMap` at `controller.go:276`, but the delete path no longer does. Low priority since it is V(1) debug logging.

Fix: accept context and add the dry-run log, for consistency with other webhook handlers.

3. **`removeOwnedFinalizers` strips all owners indiscriminately** — `pkg/controllers/base/finalizer.go:269-281` (suggestion, gap) [Gemini 3.1 Pro]

```go
func removeOwnedFinalizers(obj client.Object) bool {
    finalizers := obj.GetFinalizers()
    filtered := make([]string, 0, len(finalizers))
    for _, f := range finalizers {
        if !strings.HasPrefix(f, ownedFinalizerPrefix) {
            filtered = append(filtered, f)
        }
    }
```

By encoding the owner Kind in the finalizer name, this PR enables multiple controllers to maintain independent cascade protections. But `removeOwnedFinalizers` (called at `finalizer.go:168`) strips all `owned-by-*` finalizers regardless of which owner triggered cleanup. If a resource ever has multiple owners, deleting one owner strips all ownership finalizers. In RDD's single-user embedded apiserver, multi-owner scenarios are unlikely, so this is a design note rather than a must-fix.

Fix: if multi-owner support becomes necessary, pass the owner Kind to `DeleteOwnedResources` and strip only the matching finalizer.

4. **ObjectSelector bypass via label removal** — `pkg/controllers/lima/limavm/controller.go:134-137` (suggestion, gap) [Codex GPT 5.4]

```go
ObjectSelector: metav1apply.LabelSelector().
    WithMatchLabels(map[string]string{
        controllers.TemplateConfigMapLabel: "true",
    }),
```

The validating webhook fires only for ConfigMaps with `TemplateConfigMapLabel`. A user who removes this label (via `kubectl patch`) and then deletes the ConfigMap bypasses the webhook entirely, leaving a stuck Terminating resource — exactly the failure mode the webhook exists to prevent. In RDD's single-user context this requires a deliberate two-step bypass, reducing practical risk. Broadening the webhook to match all ConfigMaps would add overhead for every ConfigMap operation in the namespace.

Fix: if this becomes a concern, either (a) reject label removal in `ValidateUpdate` when an owned finalizer is present, or (b) register the webhook for all ConfigMaps and return early for non-template ones.

---

## Design Observations

### Strengths

- **Clean generics usage**: `OwnedDeletionGuard[T]` eliminates per-resource boilerplate for delete validation. Embedding it in `ConfigMapValidator` (line 261) inherits `ValidateDelete` while allowing the struct to override `ValidateCreate`/`ValidateUpdate`. [Claude Opus 4.6, Gemini 3.1 Pro]

- **Self-documenting finalizers**: The `owned-by-<Kind>` naming scheme lets `kubectl get` immediately show which controller owns a resource and what to delete, without tracing owner references. [Claude Opus 4.6, Codex GPT 5.4, Gemini 3.1 Pro]

- **Forward-compatible cleanup**: Switching `DeleteOwnedResources` from a single hard-coded finalizer to prefix-based `removeOwnedFinalizers` ensures cleanup works regardless of which owner Kind set the finalizer. [Claude Opus 4.6, Codex GPT 5.4]

---

## Testing Assessment

1. **BATS assertions broken** (highest risk): tests 13 and 14 in `limavm-lifecycle.bats` fail because the assertions reference the old error message format.
2. **`OwnedDeletionGuard.ValidateDelete` has no unit test**: the webhook rejection logic — the behavioral core of this PR — is tested only indirectly through BATS integration tests.
3. **`OwnedFinalizerOwner` edge cases untested**: no owned finalizer, multiple owned finalizers.
4. **Multiple `owned-by-*` finalizers not tested**: `TestDeleteOwnedResources` covers the single-owner case; no test verifies behavior with mixed owners.

---

## Documentation Assessment

The new "Validating Webhook Requirement" section in `controllers.md` explains the finalizer-webhook contract clearly. Two issues: the example error message at line 52 does not match the code's actual format, and the `DeleteOwnedResources` description at line 65 still references the old `/owned` finalizer name.

---

## Commit Structure

Clean. The two commits represent distinct concepts: `54602ea` makes the code change (encode Kind in finalizer, refactor webhook), and `eb0d203` adds the documentation. No fixup commits.

---

## Agent Performance Retro

### [Claude]

Claude identified the documentation error message mismatch, the stale `/owned` reference, the missing unit tests, and the dry-run logging regression. Its analysis was accurate and well-supported by `git blame` verification. It missed the BATS test breakage — the most impactful finding — and did not flag the ObjectSelector bypass.

### [Codex]

Codex found the critical BATS test breakage by actually running the tests, and identified the label-removal bypass. Both findings were verified and actionable. It did not flag the documentation mismatch or the stale `/owned` reference. Best signal-to-noise ratio: every finding was concrete and correct.

### [Gemini]

Gemini raised the `removeOwnedFinalizers` multi-owner concern and the `OwnedFinalizerOwner` single-owner limitation. Both are valid design observations, though their severity was overweighted for RDD's single-user context. It also caught the dry-run logging regression (shared with Claude). It missed the BATS breakage and the documentation mismatch.

### Summary

| | [Claude] | [Codex] | [Gemini] |
|---|---|---|---|
| Duration | 3:47 | 3:18 | 4:13 |
| Critical | 0 | 1 | 0 |
| Important | 2 | 1 | 1 |
| Suggestion | 2 | 0 | 2 |
| Design observations | 3 | 2 | 2 |
| False positives | 0 | 0 | 0 |
| Unique insights | 3 | 2 | 2 |

Codex delivered the most value by finding the one must-fix regression and confirming it with a live test run. Claude provided the broadest coverage across documentation and testing gaps. Gemini contributed forward-looking design observations about multi-owner scenarios that, while theoretical for now, will become relevant if the ownership model expands. All three agents produced zero false positives.

---

## Appendix: Original Reviews

### Claude Opus 4.6

### Executive Summary

This PR encodes the owner `Kind` in the owned finalizer name and introduces a generic `OwnedDeletionGuard[T]` webhook to replace hand-rolled deletion validation. The code changes are clean, the generics usage is correct, and the `removeOwnedFinalizers` helper properly handles the transition from a single finalizer to prefix-based matching. There is one documentation accuracy issue introduced by the PR and a few testing gaps for the new helper functions.

### Findings

#### Critical Issues

None.

#### Important Issues

1. **Documentation example doesn't match actual error message** — `docs/design/controllers.md:52` (important, regression)

```markdown
2. The webhook calls `OwnedFinalizerOwner` on DELETE and rejects the request with a clear error
   (e.g., *"cannot delete LimaVM 'rd': owned by App; delete the App resource instead"*).
```

The example error message includes the resource Kind ("LimaVM") before the name, but the actual `OwnedDeletionGuard.ValidateDelete` at `webhook.go:55` formats the message as `cannot delete "rd": owned by App; delete the App resource instead` — it uses `obj.GetName()` only, not the Kind. The doc example misleads readers about what error users will actually see. Confirmed this line was introduced by commit `54602ea`.

Fix: Update the example to match the actual format: `*"cannot delete \"rd\": owned by App; delete the App resource instead"*`.

2. **Missing unit tests for new helper functions** — `pkg/controllers/base/` (important, gap)

This PR introduces four new functions (`OwnedFinalizerFor`, `OwnedFinalizerOwner`, `removeOwnedFinalizers`, `OwnedDeletionGuard.ValidateDelete`) but none have dedicated unit tests. The existing `TestDeleteOwnedResources` covers `removeOwnedFinalizers` indirectly through the integration path, and `OwnedFinalizerFor` is exercised via the test case setup at `finalizer_test.go:72`. However, there are no direct tests for:
- `OwnedFinalizerOwner` (the reverse parse — especially edge cases like no finalizer, prefix-only finalizer, multiple owned finalizers)
- `OwnedDeletionGuard.ValidateDelete` (that it rejects when an owned finalizer is present and allows when absent)

Fix: Add unit tests covering the new functions, particularly `OwnedFinalizerOwner` and `OwnedDeletionGuard.ValidateDelete`.

#### Suggestions

1. **Stale `/owned` reference in `DeleteOwnedResources` docs** — `docs/design/controllers.md:65` (suggestion, gap)

```markdown
`DeleteOwnedResources` finds all resources owned by a given object (via owner references),
strips their `/owned` finalizer, and deletes them.
```

This pre-existing text (commit `9de92692`) still says "strips their `/owned` finalizer" but the code now strips all `/owned-by-*` finalizers via `removeOwnedFinalizers`. Since this PR updated the surrounding documentation in the same section, this stale reference could have been caught.

Fix: Change to "strips their owned-by-* finalizers".

2. **DryRun logging dropped from `ValidateDelete`** — `pkg/controllers/base/webhook.go:53` (suggestion, regression)

```go
func (g *OwnedDeletionGuard[T]) ValidateDelete(_ context.Context, obj T) (admission.Warnings, error) {
```

The old `ConfigMapValidator.ValidateDelete` logged at V(1) during dry-run requests. The generic `OwnedDeletionGuard.ValidateDelete` ignores context entirely (`_`). This is a minor observability loss — the create/update paths still log dry-run via `validateTemplateConfigMap` at `controller.go:276`, but the delete path no longer does.

Fix: Consider accepting context and adding the dry-run log, for consistency with other webhook handlers in the codebase. Low priority since it's V(1) debug logging only.

### Design Observations

**Strengths**

- The generic `OwnedDeletionGuard[T]` is a well-executed use of Go generics. It eliminates per-resource boilerplate for the common pattern of rejecting deletes on owned resources, and embedding it in `ConfigMapValidator` (line 261) cleanly inherits `ValidateDelete` while allowing the struct to override `ValidateCreate`/`ValidateUpdate`.

- Switching `DeleteOwnedResources` from stripping a single hard-coded finalizer (`OwnedFinalizerName`) to prefix-based `removeOwnedFinalizers` (line 168) is the right design for forward compatibility — it ensures cleanup works correctly regardless of which owner Kind set the finalizer, without requiring the cleanup path to know the owner type.

- The finalizer naming scheme (`owned-by-<Kind>`) is self-documenting. A `kubectl get` immediately tells the operator which controller owns a resource and what to delete, without needing to trace owner references.

### Testing Assessment

The existing tests cover the main path adequately:
- `finalizer_test.go` tests `DeleteOwnedResources` end-to-end with the new finalizer format
- `limavm-lifecycle.bats:77` verifies the new finalizer name appears on actual template ConfigMaps

Untested scenarios, ranked by risk:
1. `OwnedDeletionGuard.ValidateDelete` — the webhook rejection logic is the behavioral core of this PR but has no unit test
2. `OwnedFinalizerOwner` with edge cases (no owned finalizer, prefix-only, multiple owners)
3. `removeOwnedFinalizers` with mixed owned/non-owned finalizers (partially covered by the "both finalizers" integration test case)

### Documentation Assessment

The `docs/design/controllers.md` updates are thorough. The new "Validating Webhook Requirement" section clearly explains the finalizer-webhook contract and the three-step lifecycle (set, reject, release). Two minor issues noted in findings: the error message example doesn't match the code format, and one pre-existing reference to `/owned` wasn't updated.

### Commit Structure

Clean. The two commits represent distinct concepts:
- `54602ea`: Code change (encode Kind in finalizer, refactor webhook)
- `eb0d203`: Documentation for the webhook requirement

No fixup commits or patches to earlier commits.

---

### Codex GPT 5.4

**Verdict**: Request changes

#### Executive Summary

This PR makes owned finalizers self-describing by encoding the owner `Kind`, adds a reusable `OwnedDeletionGuard`, and updates the LimaVM template ConfigMap to use the new `owned-by-LimaVM` finalizer. The overall direction is sound, but I found one concrete regression that breaks the existing Lima lifecycle BATS suite and one in-scope gap where the documented webhook/finalizer protection can still be bypassed by removing the selector label first.

#### Findings

**Critical Issues**

None.

**Important Issues**

1. **Generic delete message breaks the existing Lima lifecycle BATS suite** — `pkg/controllers/base/webhook.go:53-55` (important, regression)

```go
func (g *OwnedDeletionGuard[T]) ValidateDelete(_ context.Context, obj T) (admission.Warnings, error) {
	if owner := OwnedFinalizerOwner(obj); owner != "" {
		return nil, fmt.Errorf("cannot delete %q: owned by %s; delete the %s resource instead", obj.GetName(), owner, owner)
	}
	return nil, nil
}
```

The new error text at line 55 no longer matches the assertions in `bats/tests/33-lima-controllers/limavm-lifecycle.bats` lines 136–151, which still expect `"cannot delete template ConfigMap"` and `"protected by the LimaVM controller"`. I verified this by running `bats/tests/33-lima-controllers/limavm-lifecycle.bats`: tests 13 and 14 fail with `cannot delete "test-vm-template": owned by LimaVM; delete the LimaVM resource instead`.

Fix: update the BATS expectations to the new generic wording, or preserve the old Lima-specific phrasing in `ConfigMapValidator.ValidateDelete` instead of switching to the generic message.

2. **The delete guard is still bypassable by removing the selector label first** — `pkg/controllers/lima/limavm/controller.go:134-143`, `pkg/controllers/base/webhook.go:53-57` (important, gap)

```go
ObjectSelector: metav1apply.LabelSelector().
	WithMatchLabels(map[string]string{
		controllers.TemplateConfigMapLabel: "true",
	}),
```

The webhook only applies to ConfigMaps that still carry `TemplateConfigMapLabel` at lines 134-137, and `ValidateUpdate` at lines 270-271 only revalidates template data; it never prevents that label from being removed. Once the label is cleared, later DELETE requests no longer reach `OwnedDeletionGuard.ValidateDelete` at lines 53-57, so the API accepts the delete and the owned finalizer leaves the object stuck in `Terminating`, which is exactly the failure mode the new documentation says this mechanism should avoid.

Fix: do not rely on a mutable label for admission matching. The simplest fix is to register the validator for all ConfigMaps and have it return early unless the object has the template-owner characteristics you care about. If you keep `ObjectSelector`, then `ValidateUpdate` must reject removal of `TemplateConfigMapLabel` (and ideally reject clearing it whenever an owned finalizer is present).

**Suggestions**

None.

#### Design Observations

**Concerns**

- `(in-scope)` Using a mutable label as the webhook entry criterion undermines the "finalizer + webhook as one access-control gate" design. Registering the validator broadly and keying behavior off the finalizer itself would align the enforcement point with the protected state, eliminate the label-removal bypass, and simplify the mental model.

**Strengths**

- Encoding the owner `Kind` in the finalizer is a good simplification. It removes per-resource delete-message configuration and makes `DeleteOwnedResources`/`OwnedDeletionGuard` easier to reason about.
- Stripping all `owned-by-*` finalizers in `DeleteOwnedResources` is the right generic cleanup behavior for this model and avoids coupling deletion to a single hard-coded finalizer value.

#### Testing Assessment

1. Removing `lima.rancherdesktop.io/template-configmap` and then deleting the ConfigMap is not covered, even though it bypasses the documented protection path.
2. There is no direct unit test for `OwnedDeletionGuard.ValidateDelete` or `OwnedFinalizerOwner`, so the new generic error contract is only caught indirectly by BATS.
3. There is no coverage for objects carrying multiple `owned-by-*` finalizers, even though `DeleteOwnedResources` now removes all of them.

I ran `go test ./pkg/controllers/base ./pkg/controllers/lima/limavm/...`, which passed, and `bats/tests/33-lima-controllers/limavm-lifecycle.bats`, which failed in tests 13 and 14 for the message mismatch above.

#### Documentation Assessment

The new design doc explains the intended contract clearly, but after fixing the selector bypass it should explicitly say that the webhook must be matched by an immutable criterion or by the finalizer itself. As written, the LimaVM ConfigMap example overstates the guarantee.

---

### Gemini 3.1 Pro

#### Executive Summary

This PR successfully enhances the `owned` finalizer pattern by encoding the owner's `Kind` into the finalizer string. It introduces a clean, generic `OwnedDeletionGuard` webhook that provides users with clear, actionable error messages when attempting to delete protected resources. The design is elegant and significantly improves the developer experience and safety. However, the implementation of the finalizer removal logic during cleanup strips all owner finalizers indiscriminately, which circumvents the new namespaced design and could lead to premature resource deletion if resources are ever shared among multiple owners.

#### Findings

##### Critical Issues
None.

##### Important Issues
1. **`removeOwnedFinalizers` indiscriminately strips finalizers from all owners** — `pkg/controllers/base/finalizer.go:266-280` (gap/regression)

```go
func removeOwnedFinalizers(obj client.Object) bool {
	finalizers := obj.GetFinalizers()
	filtered := make([]string, 0, len(finalizers))
	for _, f := range finalizers {
		if !strings.HasPrefix(f, ownedFinalizerPrefix) {
			filtered = append(filtered, f)
		}
	}
// ...
```
By encoding the owner's `Kind` into the finalizer name (`owned-by-<Kind>`), this PR introduces a design that allows multiple controllers to safely share ownership of a child resource by managing their own distinct cascade protections. However, `removeOwnedFinalizers` (called by `DeleteOwnedResources` at line 168) strips *all* finalizers that match the `ownedFinalizerPrefix`, regardless of which owner triggered the cleanup.

If a resource is ever shared by multiple owners, deleting one owner will strip the finalizers for *all* owners and issue a `c.Delete(&item)`. K8s normally handles multiple owners by waiting for all finalizers to be removed before fully deleting a terminating object. By stripping all finalizers at once, `DeleteOwnedResources` bypasses this mechanism, causing the shared child to be immediately deleted out from under the other active owners. While multi-owner scenarios might be rare in RDD currently, this implementation directly contradicts the safety benefits of the new `Kind`-specific design.

**Fix:** Update `DeleteOwnedResources` to accept `ownerKind string` as an additional parameter, and change it to call `RemoveOwnedFinalizer(ctx, c, &item, ownerKind)` instead of the indiscriminate `removeOwnedFinalizers`.

##### Suggestions
1. **`OwnedFinalizerOwner` only surfaces the first owner found** — `pkg/controllers/base/finalizer.go:255-264` (gap)

```go
func OwnedFinalizerOwner(obj client.Object) string {
	for _, f := range obj.GetFinalizers() {
		if strings.HasPrefix(f, ownedFinalizerPrefix) {
			return strings.TrimPrefix(f, ownedFinalizerPrefix)
		}
	}
	return ""
}
```
If a resource has multiple owners and therefore multiple `owned-by-*` finalizers, this function will simply return the first one it iterates over. When the generic webhook validates a deletion, it will return an error message naming only this first owner.

**Fix:** If multiple `owned-by-*` finalizers are present, `OwnedFinalizerOwner` could return a comma-separated list of owners, or `OwnedDeletionGuard` could iterate over all finalizers and aggregate the errors.

2. **Dry-run logging removed from generic `ValidateDelete`** — `pkg/controllers/base/webhook.go:53-58` (regression)

```go
func (g *OwnedDeletionGuard[T]) ValidateDelete(_ context.Context, obj T) (admission.Warnings, error) {
```

The refactor to use the embedded `OwnedDeletionGuard` drops the `[DryRun]` logging that was previously present in `ConfigMapValidator.ValidateDelete`.

**Fix:** Reintroduce the `base.IsDryRun(ctx)` check and a corresponding `klog.V(1)` log inside `OwnedDeletionGuard.ValidateDelete`.

#### Design Observations

##### Strengths
- **Elegant Inheritance via Embedded Generics:** The use of an empty generic struct (`OwnedDeletionGuard[T]`) to provide default webhook validation methods is very clean. It allows controllers like `ConfigMapValidator` to seamlessly inherit the standard `DELETE` validation while overriding `CREATE`/`UPDATE` operations without repetitive boilerplate.
- **Self-Documenting Validation Errors:** Encoding the owner `Kind` into the finalizer string (`owned-by-<Kind>`) to provide explicit webhook error messages is a great pattern that significantly improves user experience.

#### Testing Assessment
1. **Missing multiple finalizer test:** `TestDeleteOwnedResources` covers removing a single `OwnedFinalizerFor("ConfigMap")`, but there are no test cases ensuring the proper behavior when an object carries multiple `owned-by-*` finalizers (especially if `DeleteOwnedResources` is refactored to only strip the specific owner's finalizer).

#### Documentation Assessment
The additions to `docs/design/controllers.md` are clear and perfectly explain the motivation and mechanics of the new `owned-by-<Kind>` pattern and its interaction with validating webhooks.