fix(component): stop two owners of one object from taking each other's fields

[sourcehawk]

Description

Fixes #197

Two custom resources of one kind whose components render the same object silently took each other's fields on every reconcile, and both reported converged. The apply field manager was <Kind>/<component> for every owner of a kind, so from the API server's point of view it was one manager rewriting its own fields and neither owner ever saw a conflict. The manager is now <Kind>/<component>/<owner UID>, so each owner is a distinct manager, and when the framework sets a controller reference (the default) the second owner's apply is rejected by the API server (it carries a second controller: true owner reference), surfacing as an error on that owner's component instead of a silent takeover. For Unowned() resources or a scope mismatch, where no controller reference is set, the second owner's forced apply still takes its declared fields; managedFields then names each owner but the framework does not detect the contention. On upgrade, fields owned by the old manager are adopted on the next apply through forced ownership; fields the old manager held that are no longer in the desired state stay under the old name and are not pruned.

Changes

• The Server-Side Apply field manager is <Kind>/<component>/<owner UID> on both the reconcile and the suspension apply path, built by one helper (applyFieldOwner).
• When that readable form would exceed the API server's 128-character manager limit (neither kind nor component name is length-bounded), the manager is the hex-encoded SHA-256 of it instead: 64 characters, deterministic, still distinct per owner. Previously such a manager was rejected on every apply.
• docs/primitives.md documents the new manager format, why the owner is part of it, and the upgrade behaviour; the plugin skill references are synced.

Related

• A built-in guard that reports Blocked when another owner already controls the object (the issue's second option) is not included; it is tracked in Block a resource whose live object is controlled by another owner #199. With per-owner managers the second owner now fails loudly where a controller reference is set, and Block a resource whose live object is controlled by another owner #199 covers the Unowned()/scope-mismatch cases and a condition instead of an apply error.
• Consumer-side reproduction and workaround: Two DatabaseServers that name one DatabaseServerConfig loop over it and neither reports the conflict konsole-is/camunda-operator#260, fix(databaseserver): report a contract another server holds instead of rewriting it konsole-is/camunda-operator#262.

Testing

New envtest specs in pkg/component/create_field_manager_test.go reconcile two owners of the same kind against one ConfigMap. The specs were written first and failed on the unfixed code exactly as the issue describes: the second owner's reconcile succeeded and took the object. After the fix the second reconcile returns an error, the ConfigMap keeps the first owner's data and controller reference, and its only Apply manager is MockOperatorCRD/shared/<first owner UID>. A second spec checks that repeated reconciles of one owner keep a single manager, and a third applies with a 128-character component name and checks the API server accepts the 64-character hashed manager and reports it in managedFields. TestApplyFieldOwner unit-tests the boundary: exactly 128 stays readable, 129 hashes, and hashed managers still differ per owner. make all passes (fmt, lint, unit tests, scaffold and example tests, example builds). E2E was not run: nothing in e2e/ or examples/ inspects managed fields, and the behaviour is exercised against a real API server by envtest.

🤖 Generated with Claude Code

https://claude.ai/code/session_015o2rQvAQ5vRtq57JBxdRfE

[Copilot]

🟡 Changes recommended

The updated SSA docs/skill text overstates that the second owner’s apply is always rejected (it depends on controller owner-ref being set), which could mislead users in Unowned()/scope-mismatch cases.

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

Pull request overview

This PR fixes a Server-Side Apply (SSA) field-manager collision in pkg/component that allowed two owners of the same Kind to silently overwrite (“take”) each other’s managed fields when they rendered the same Kubernetes object, while both owners still reported convergence.

Changes:

• Make the SSA field manager per-owner by including the owner UID (<Kind>/<component>/<owner UID>) via a shared helper (applyFieldOwner) on both normal reconcile and suspension apply paths.
• Add envtest coverage to verify a second owner cannot silently take over a shared object and that repeated reconciles keep a single manager entry.
• Update primitives documentation (and synced plugin references) to describe the new manager format and upgrade behavior.

File summaries

File	Description
pkg/component/create.go	Introduces applyFieldOwner and applies it consistently on reconcile + suspension apply paths to prevent manager sharing across owners.
pkg/component/create_field_manager_test.go	Adds envtest specs reproducing the multi-owner collision and asserting correct failure/manager behavior after the fix.
docs/primitives.md	Documents the new SSA field manager format and upgrade behavior.
plugin/skills/using-primitives/references/primitives.md	Synced copy of docs/primitives.md with the same SSA field manager documentation updates.
plugin/skills/using-primitives/SKILL.md	Updates the skill guidance to reflect the new SSA field manager format and behavior.

Review details

• Files reviewed: 5/5 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]

🟢 Approval recommended

The change is well-scoped, consistently applied across code paths, and validated with targeted envtest coverage for the reported failure mode.

Review details

Suppressed comments (1)

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

pkg/component/create.go:202

• The GoDoc claims using the UID (instead of name) keeps the field manager "well under" the API server's 128-character limit, but component names are only validated as non-empty (no max length), so the full "//" string can still exceed 128 chars in edge cases. Suggest softening the wording to avoid implying a hard guarantee.

// applyFieldOwner returns the Server-Side Apply field manager for one component
// of one owner: "<Kind>/<component>/<owner UID>", for example
// "ExampleApp/web-interface/3d8a9d5e-1c2b-4f6e-9a7d-0b1c2d3e4f5a". The owner's
// UID is part of the name so that two owners of the same kind whose components
// render the same object are distinct managers to the API server. With a manager
// shared across owners, each owner's forced apply would relinquish the other's
// fields wholesale and neither would ever see a conflict. The UID rather than
// the owner's name keeps the manager well under the API server's 128-character
// limit and identical for cluster-scoped and namespaced owners.

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

[sourcehawk]

Review round 2 carried one suppressed finding, with no thread to reply in, so its disposition is recorded here.

pkg/component/create.go:202, GoDoc claims the UID keeps the manager "well under" the 128-character limit. Applied. The builder only rejects an empty component name, so the string has no upper bound; the GoDoc now describes the UID as a fixed 36 characters that leave the rest of the limit to the kind and the component name, instead of implying a guarantee.

[Copilot]

🟢 Approval recommended

The functional change and tests align with the described fix; the only remaining feedback is a small in-code comment wording correction.

Review details

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

[Copilot]

🟢 Approval recommended

The change is narrowly scoped, has targeted envtest coverage for the reported failure mode, and the remaining feedback is a minor doc wording refinement.

Review details

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

[Copilot]

🟢 Approval recommended

The changes consistently apply the per-owner SSA field manager across relevant paths, include targeted envtest coverage for the regression, and update documentation (including synced plugin references) to match the new behavior.

Review details

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

[Copilot]

🟢 Approval recommended

The core behavior change is well-contained, consistently applied across paths, and is backed by both unit tests and envtest coverage with only a minor documentation-comment nit noted.

Review details

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

[Copilot]

🟢 Approval recommended

The change addresses the documented SSA ownership bug with a deterministic, bounded field-manager scheme and is covered by both unit tests and envtest specs that reproduce and validate the fix.

Review details

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