Fix migrateFieldManagersToSSA on update path

[mdbooth]

Summary

Fixes migrateFieldManagersToSSA when called in the update path.

resolves #495

Change Type

Bug Fix

Check List Before Merging

• This PR passes all pre-commit hook validations.
• This PR is fully tested and regression tests are included.
• Relevant documentation has been updated.

Additional Information

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bf9ef240-0643-4b74-9e3e-0a467a867388

📥 Commits

Reviewing files that changed from the base of the PR and between 69daf0c and 4810290.

📒 Files selected for processing (2)

• machinery/objects.go
• test/objects_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• test/objects_test.go

---

Walkthrough

The apply call path was refactored: callers now pass both desired and actual objects to apply. Optimistic-locking (SetResourceVersion) and managed-field migration are moved into apply, and migration now targets the actual object so existing field managers are considered.

Changes

Cohort / File(s)	Summary
Core Apply Logic machinery/objects.go	Refactored apply to accept desiredObject and actualObject. Moved desiredObject.SetResourceVersion(actualObject.GetResourceVersion()) into apply and placed it after migrateFieldManagersToSSA(ctx, actualObject). Updated call sites to pass actualObject. Switched internal uses and error handling to reference desiredObject where appropriate.
Field Manager Migration Test test/objects_test.go	Added TestObjectEngine_StaleManagedFieldMigration to verify a stale ManagedFields entry with operation Update is migrated to Apply during reconciliation. Added metav1 import for operation constants.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing migrateFieldManagersToSSA on the update path, which is the core objective of this PR.
Description check	✅ Passed	The description follows the template with all required sections filled: Summary, Change Type (Bug Fix), and completed checklist items. Documentation updates remain pending as indicated.
Linked Issues check	✅ Passed	The code changes directly address issue #495 by ensuring migrateFieldManagersToSSA receives the actual object's field-manager metadata during updates, fixing the no-op behavior.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the migrateFieldManagersToSSA issue: signature changes to pass actualObject, migration logic repositioning, and comprehensive test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/objects_test.go`:
- Around line 406-407: The cleanup call currently swallows all errors from
Client.Delete(ctx, configMap.DeepCopy()); change it to capture the returned
error (err := Client.Delete(...)) and only ignore apierrors.IsNotFound(err); for
any other non-nil error fail the test (e.g., t.Fatalf or require.NoError) so
setup failures surface before Reconcile runs; reference the Client.Delete call
and ensure you import/use apierrors.IsNotFound for the check.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ac12445a-0d54-4994-8f50-b52bd1234c7d

📥 Commits

Reviewing files that changed from the base of the PR and between 79d1013 and 69daf0c.

📒 Files selected for processing (2)

• machinery/objects.go
• test/objects_test.go

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.90%. Comparing base (79d1013) to head (4810290).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
machinery/objects.go	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #496   +/-   ##
=======================================
  Coverage   71.90%   71.90%           
=======================================
  Files          35       35           
  Lines        2983     2983           
=======================================
  Hits         2145     2145           
  Misses        710      710           
  Partials      128      128           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.