feat(component): block a resource whose live object is controlled by another owner

[sourcehawk]

Description

Closes #199

A second owner whose component renders an object that another owner already controls either fails with the API server's "Only one reference can have Controller set to true" text, or, for an Unowned() or scope-mismatched resource, silently takes the object's fields on every reconcile. This PR adds the component.BlockOnForeignController() resource option: before each apply the component reads the live object and, when it carries a controller reference to another owner, reports Blocked with controlled by <Kind> <name> and performs no apply, like any blocked guard. It compares controller references only, so two owners that both apply without one are still undetected; the docs state that a shared name is the operator's responsibility in that case.

Changes

• component.BlockOnForeignController() resource option; combining it with ReadOnly() is a build error.
• reconcileResources checks the live object's controller reference after the resource's own guard and before the apply, reading through ReconcileContext.APIReader when set so a stale cache cannot let an Unowned() apply through, and recording GuardBlocked and short-circuiting the remaining resources.
• The check covers every path that would delete the object: during suspension a resource another owner controls is neither scaled down nor deleted (it counts as suspended, so the component reports the usual Suspended condition), and a deletion asked for by Delete()/DeleteWhen()/GatedBy() or a disabled component feature gate is skipped. Each skip is logged with the controlling owner, and a delete of an object observed as safe carries the observed UID and resourceVersion as preconditions, so an owner that claims the object in between keeps it. Delete entries now carry their resolved options to make this possible.
• docs/component.md (options table, guards) and the docs/primitives.md Server-Side Apply section describe the option and the remaining gap; plugin references synced.

Related

• The apply field manager is the same for every owner of a kind, so two owners of one object take each other's fields #197, fix(component): stop two owners of one object from taking each other's fields #198: the per-owner field manager this builds on.
• fix(databaseserver): report a contract another server holds instead of rewriting it konsole-is/camunda-operator#262: the consumer-side guard this replaces.

Testing

Eight new envtest specs in pkg/component/create_foreign_controller_test.go: the second owner is blocked with the reason naming the first while the ConfigMap's data, owner references, managedFields and resourceVersion stay unchanged; an Unowned() registration is blocked by a controller reference set by other means and not blocked when there is none; the controlling owner itself is never blocked; the block clears once the reference is removed; a suspended component leaves the other owner's object untouched; and so do a disabled feature gate and a Delete() registration. Unit tests cover option resolution, the suspension path with a fake client (no Suspend, apply or delete reaches an object another owner controls), the check reading through APIReader when the cache is stale, and a delete (flag and delete-on-suspend) that loses the race to a claim at the moment of the delete, which leaves the object in place. make all passes (lint 0 issues). No E2E suite exercises the new option, so none was run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MZRpj8ChzesLeqLkWT31vS

[Copilot]

🟡 Changes recommended

The suspension-path reason string for foreign-controlled objects is inconsistent with the documented/advertised “same reason” behavior and should be aligned before approval.

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

Pull request overview

This PR introduces a new component resource option, component.BlockOnForeignController(), to prevent a component from applying (or suspending/deleting during suspension) a resource whose live object is already controlled by another owner via a controller owner reference. This turns otherwise noisy/opaque apply failures (or silent field takeovers in Unowned()/scope-mismatch cases) into an explicit Blocked condition with a readable reason.

Changes:

• Add component.BlockOnForeignController() resource option with validation (mutually exclusive with ReadOnly()).
• Extend reconciliation and suspension flows to detect a foreign controller owner reference on the live object and short-circuit with Blocked/Suspended without applying or deleting.
• Add envtest and unit tests, and update framework docs + synced plugin references to describe the new behavior and the remaining undetected contention case.

File summaries

File	Description
pkg/component/create.go	Adds the pre-apply foreign-controller check (foreignController) and blocks reconciliation with a GuardBlocked-equivalent status when another owner controls the live object.
pkg/component/resource_options.go	Introduces the option flag, public BlockOnForeignController() option, and validation that rejects combination with ReadOnly().
pkg/component/suspend.go	Adds a suspension-path check to avoid scaling down or deleting objects controlled by another owner.
pkg/component/resource_options_test.go	Adds option resolution and validation coverage for BlockOnForeignController.
pkg/component/suspend_test.go	Adds unit coverage ensuring suspension doesn’t touch a foreign-controlled object and still suspends normally when controlled by self/no controller.
pkg/component/create_foreign_controller_test.go	Adds envtest specs validating Blocked behavior and that the live object remains unchanged when blocked.
docs/component.md	Documents the new option and its interaction with guards and suspension behavior.
docs/primitives.md	Updates SSA section to recommend the new option and clarify the remaining contention gap for controller-less cases.
plugin/skills/building-components/references/component.md	Sync of the updated docs/component.md into plugin references.
plugin/skills/using-primitives/references/primitives.md	Sync of the updated docs/primitives.md into plugin references.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 1
• 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]

🔵 Needs a closer look

Cached reads can miss a foreign controller and permit an Unowned() forced apply, while public discovery documentation also remains incomplete.

Review details

Suppressed comments (2)

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

pkg/component/resource_options.go:193

• The new public option is missing from the exported Builder.WithResource option list (builder.go:116-118) and from plugin/skills/building-components/SKILL.md:52-56; the distilled SSA guidance in plugin/skills/using-primitives/SKILL.md:177-181 also still ends at the old undetected-contention behavior. These published discovery surfaces should be updated alongside the API and generated references so consumers can find and correctly understand the option.

func BlockOnForeignController() ResourceOption {
	return func(c *resourceConfig) { c.blockOnForeignController = true }

pkg/component/create.go:402

• This safety check reads through the manager's cached Client, so it can observe the pre-controller state while the API server already has a foreign controller reference. In the Unowned() case the subsequent forced apply can then take the foreign owner's fields before the cache catches up, defeating the option's core guarantee. Prefer rec.APIReader when configured (as the status-conflict path already does), with Client as the fallback.

	if err := rec.Client.Get(ctx, client.ObjectKeyFromObject(obj), live); err != nil {

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[sourcehawk]

Round 2 of the Copilot review posted no comments and suppressed two findings; both were verified and applied in c5a3668:

• pkg/component/create.go, cached read: correct. ReconcileContext.APIReader exists and the status-conflict path already prefers it; a cached read can miss a controller reference the API server already carries, and for an Unowned() resource the forced apply would then take the other owner's fields. foreignController now reads through rec.APIReader when set and rec.Client otherwise, with a unit test where the cache is stale and the API reader holds the foreign controller.
• Discovery surfaces: correct. Builder.WithResource's GoDoc now lists the option (and the previously missing OrphanWhen and Unowned), and both plugin SKILL.md files (building-components option list, using-primitives SSA guidance) name BlockOnForeignController() and the remaining undetected case.

[Copilot]

🟡 Changes recommended

The controller check can read stale cached state and permit the field takeover it is intended to prevent.

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

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The read/apply sequence remains racy, and suspension aggregation discards the documented foreign-owner reason.

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

Review details

Suppressed comments (1)

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

pkg/component/suspend.go:124

• This owner-specific reason never reaches the component condition. Component.Reconcile aggregates these results through suspensionResults.summary(), which replaces every fully-suspended result with "All resources are suspended."; the new test only calls suspendResource directly and misses that public path. Preserve this exceptional reason during aggregation and cover a suspended component reconcile so the documented owner name is actually reported.

			return concepts.SuspensionStatusWithReason{
				Status: concepts.SuspensionStatusSuspended,
				Reason: fmt.Sprintf(
					"Resource %s is %s; nothing to suspend.", resource.Identity(), foreignControllerReason(controller),
				),

• Files reviewed: 14/14 changed files
• Comments generated: 1
• Review effort level: Balanced

[sourcehawk]

Round 3 of the Copilot review suppressed one finding; verified and applied in 4dd72de:

• pkg/component/suspend.go, dropped reason: correct. suspensionResults.summary() folds every all-suspended reason into All resources are suspended., so the owner name never reached the component condition and the docs overstated it. The skip now logs the controlling owner, the GoDoc and docs/component.md describe the condition the component actually reports, and a new envtest spec drives the suspended path through Component.Reconcile (condition Suspended, the other owner's object untouched, no Suspend, apply or delete reached).

The posted comment on the read/apply race was declined in its thread with the reasoning.

[Copilot]

🟡 Changes recommended

Feature-gate and explicit deletion paths can still delete an object protected by the new option.

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

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Non-atomic ownership checks can still delete an object that becomes foreign-controlled before deletion.

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

Review details

• Files reviewed: 19/19 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

Ownership checks, deletion race protection, documentation, and comprehensive tests are consistent and complete.

Review details

• Files reviewed: 19/19 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The option’s GoDoc overpromises protection for orphaning writes that bypass the controller check.

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

Review details

• Files reviewed: 19/19 changed files
• Comments generated: 1
• Review effort level: Balanced