fix ssa patch by applying all managed fields at once#928
fix ssa patch by applying all managed fields at once#928TehilaTheStudent wants to merge 1 commit intocrossplane:mainfrom
Conversation
📝 WalkthroughWalkthroughExtracts SSA-managed fields from the resolved object and merges them into the JSON merge patch generated by prepareJSONMerge, so previously SSA-owned fields are preserved when applying the patch; returns the merged patch or the original when no managed fields are present or extraction is non-fatal. Changes
Sequence DiagramsequenceDiagram
participant Reconciler
participant prepareJSONMerge
participant mergeWithManagedFields
participant ManagedFieldsExtractor
Reconciler->>prepareJSONMerge: prepareJSONMerge(patch, resolved)
rect rgba(100,150,200,0.5)
Note over prepareJSONMerge: generate JSON merge patch (desired vs current)
prepareJSONMerge->>prepareJSONMerge: generatePatch()
end
rect rgba(150,200,100,0.5)
Note over prepareJSONMerge,mergeWithManagedFields: preserve SSA-managed fields
prepareJSONMerge->>mergeWithManagedFields: mergeWithManagedFields(patch, resolved)
mergeWithManagedFields->>ManagedFieldsExtractor: extract managedFields from resolved
ManagedFieldsExtractor-->>mergeWithManagedFields: managedFields payload (if any)
mergeWithManagedFields->>mergeWithManagedFields: marshal & jsonpatch.MergePatch into patch
mergeWithManagedFields-->>prepareJSONMerge: mergedPatch or original patch
end
prepareJSONMerge-->>Reconciler: return final patch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
pkg/reconciler/managed/api.go (1)
59-61: Consider enriching error messages with a bit more user-facing context.Per the coding guidelines, error messages should be meaningful to end users and include context about what was being attempted. These messages are clear to developers but might benefit from a touch more context, e.g.:
- errExtractManagedFields = "cannot extract managed fields" + errExtractManagedFields = "cannot extract previously managed fields from resolved resource"That said, since these are wrapped further up the call chain (ultimately under
errPatchManaged), the current messages may be sufficient in practice. Just wanted to raise it — what do you think?As per coding guidelines, "Ensure all error messages are meaningful to end users, not just developers - avoid technical jargon, include context about what the user was trying to do".
pkg/reconciler/managed/api_test.go (2)
577-586: The long expected JSON string on line 584 is hard to review — could you clarify the intent?I'm trying to understand what's being verified here. The
resolvedobject hasmanagedFieldsclaiming ownership ofotherRefId, butotherRefIdisn't actually set on the resolved object. So themergeWithManagedFieldscall shouldn't merge any additional field values. Is the intent to verify that managed fields metadata flowing through the patch doesn't cause any issues?If so, it might help readability to add a brief inline comment explaining what this test case exercises beyond "PatchWithManagedFieldsMerged" — e.g., "verifies that managed fields referencing absent values don't inject phantom fields into the patch."
Also, the long JSON literal is fragile and hard to diff-review. Would you consider constructing the expected output programmatically or at least formatting it across multiple lines? Just a thought — not blocking.
507-538: Test helpers look good — consider adding error handling forSetNestedField.The test structure is solid and table-driven tests are well-organized. One suggestion:
unstructured.SetNestedFieldreturns an error, and while it's unlikely to fail with these controlled, static inputs, it's worth checking for consistency. You might convert the helpers to accept*testing.Tand uset.Fatal()if setup fails, which would surface any surprises:Example for the helper at line 507
- withManagedFields := func() *unstructured.Unstructured { + withManagedFields := func(t *testing.T) *unstructured.Unstructured { + t.Helper() u := &unstructured.Unstructured{} u.SetGroupVersionKind(schema.GroupVersionKind{ Group: "example.com", Version: "v1", Kind: "TestResource", }) u.SetName("test-resource") - unstructured.SetNestedField(u.Object, "new-ref-id", "spec", "forProvider", "someRefId") + if err := unstructured.SetNestedField(u.Object, "new-ref-id", "spec", "forProvider", "someRefId"); err != nil { + t.Fatal(err) + }Same pattern applies to lines 623 and 645 if you'd like to make them consistent. That said, the current approach is functional—this is really just a polish suggestion.
TehilaTheStudent
force-pushed
the
fix-ssa-patch
branch
from
17a5ae6 to
a27ab73
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/reconciler/managed/api.go`:
- Around line 249-254: In mergeWithManagedFields, do not silently swallow the
runtime.DefaultUnstructuredConverter.ToUnstructured error and return the
unmerged patch; instead propagate the conversion error so reconciliation can
surface the failure. Locate the function mergeWithManagedFields and replace the
current return patch, nil behavior when ToUnstructured fails with returning an
error (e.g., wrap or return err) so callers receive a non-nil error rather than
getting the original unmerged patch that can trigger the delete/loop bug.
🧹 Nitpick comments (3)
pkg/reconciler/managed/api_test.go (3)
609-728: Great test structure! Could we also cover the error paths?The four happy-path scenarios cover the key functional cases well — thank you for the
DifferentFieldManagercase, that's an important boundary to test.
mergeWithManagedFieldshas three distinct error branches (errExtractManagedFieldsat line 258,errMarshalManagedFieldsat line 267,errMergePatchesat line 272) that are currently untested. Adding at least one error-path case (e.g., an object whoseExtractIntofails) would strengthen confidence in the error wrapping and ensure the error messages make it to the caller. Would you be open to adding that?
629-629: Nit: inconsistent error handling forSetNestedField.On line 515 the return value is explicitly discarded with
_ = unstructured.SetNestedField(...), but here the error is silently dropped. A small consistency fix:- unstructured.SetNestedField(u.Object, "old-ref-value", "spec", "forProvider", "someRefId") + _ = unstructured.SetNestedField(u.Object, "old-ref-value", "spec", "forProvider", "someRefId")Same applies to line 651.
577-592: Quick question about the expected patch containingmanagedFieldsmetadata.In this test,
existinghas nomanagedFieldswhileresolveddoes, so the JSON merge diff naturally includesmetadata.managedFieldsin the patch. In production, bothexisting(deep copy ofmg) andresolved(mgafterResolveReferences) should have identicalmanagedFields, so the diff wouldn't contain them.The test still validates the merge logic correctly, but the expected patch with embedded
managedFieldscould be confusing for future readers. Would it help to add a brief comment noting this is a test-specific artifact, or alternatively adjust both objects to have the samemanagedFieldsso the expected patch is more representative of production behavior?
| func mergeWithManagedFields(patch []byte, obj runtime.Object, fieldManager string) ([]byte, error) { | ||
| // ExtractInto requires an unstructured representation of the object. | ||
| u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) | ||
| if err != nil { | ||
| return patch, nil //nolint:nilerr // Conversion errors are non-fatal; return the original patch. | ||
| } |
There was a problem hiding this comment.
Silently swallowing the conversion error re-introduces the original bug.
Thanks for the clear nolint annotation! I have a question about the intent here though: if ToUnstructured fails, the function returns the unmerged patch — which is exactly the broken behavior this PR is fixing (omitted fields → API server deletes them → infinite loop). While a failure here seems unlikely (since json.Marshal(resolved) already succeeded a few lines earlier), silently falling back to the buggy path feels risky.
Could we propagate the error instead, so that a reconciliation retry can surface what went wrong? That way, if this ever does fail, operators get a clear signal rather than a mysterious infinite loop.
Suggested fix
- u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
- if err != nil {
- return patch, nil //nolint:nilerr // Conversion errors are non-fatal; return the original patch.
- }
+ u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
+ if err != nil {
+ return nil, errors.Wrap(err, "cannot convert resolved object to unstructured for managed fields extraction")
+ }🤖 Prompt for AI Agents
In `@pkg/reconciler/managed/api.go` around lines 249 - 254, In
mergeWithManagedFields, do not silently swallow the
runtime.DefaultUnstructuredConverter.ToUnstructured error and return the
unmerged patch; instead propagate the conversion error so reconciliation can
surface the failure. Locate the function mergeWithManagedFields and replace the
current return patch, nil behavior when ToUnstructured fails with returning an
error (e.g., wrap or return err) so callers receive a non-nil error rather than
getting the original unmerged patch that can trigger the delete/loop bug.
|
I don't know if it's relevant or not, but I wanted to leave a breadcrumb to this issue in provider-kubernetes where SSA has issues with "apply only" fields. |
TehilaTheStudent
force-pushed
the
fix-ssa-patch
branch
from
a27ab73 to
fc40c97
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
pkg/reconciler/managed/api.go (1)
59-61: Error messages are clear and descriptive — thank you!The new error strings (
errExtractManagedFields,errMarshalManagedFields,errMergePatches) follow the existing naming conventions and provide meaningful context for debugging. As per coding guidelines, "Ensure all error messages are meaningful to end users, not just developers - avoid technical jargon, include context about what the user was trying to do."The messages here lean slightly technical ("extract previously managed fields from resolved resource"). For an end-user who encounters this in a condition or event, would it be clearer to hint at why this matters? Something like:
"cannot preserve previously resolved reference fields"That said, these errors are internal to the reconciler and not surfaced directly as conditions, so the current phrasing is reasonable. Just a thought!
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/reconciler/managed/api.go` around lines 59 - 61, Update the error strings errExtractManagedFields, errMarshalManagedFields, and errMergePatches in pkg/reconciler/managed/api.go to be more user-facing and state why the failure matters (e.g., preserving previously-resolved reference/managed fields during reconciliation), so replace the technical phrases like "cannot extract previously managed fields from resolved resource" with clearer messages such as "cannot preserve previously resolved reference fields" or similar; ensure each message still uniquely maps to its error symbol (errExtractManagedFields, errMarshalManagedFields, errMergePatches) and retains enough context for debugging while avoiding heavy internal jargon.pkg/reconciler/managed/api_test.go (3)
668-717: Consider adding an error-path test case forExtractIntofailure.The current test cases cover the happy paths well. One scenario that's missing is what happens when
managedfields.ExtractIntoreturns an error — this would exercise theerrExtractManagedFieldserror wrapping at Line 258 ofapi.go.I understand this might be tricky to trigger with well-formed unstructured objects, but if there's a way to craft an input that causes
ExtractIntoto fail (e.g., malformedmanagedFieldsentries), it would strengthen confidence in the error handling. Would you like me to help investigate what inputs would trigger that path?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/reconciler/managed/api_test.go` around lines 668 - 717, Add a failing-path test case named "ExtractIntoError" that constructs an obj whose metadata.managedFields contains a deliberately malformed entry so managedfields.ExtractInto returns an error (for example, an unstructured.Unstructured with metadata.managedFields as a []interface{} containing an entry where "fieldsV1" is an invalid type/contents), call the same function under test (the code-path that invokes managedfields.ExtractInto in api.go), and assert the returned error is non-nil and wraps errExtractManagedFields; reference the test map in pkg/reconciler/managed/api_test.go and the error symbol errExtractManagedFields and the call site that uses managedfields.ExtractInto so the new case follows the existing "cases" structure.
577-595: The "PatchWithManagedFieldsMerged" test case exercises the key bug fix — great addition!The comment explaining why
managedFieldsappears in the expected patch (Lines 578-582) is very helpful context. One thought: the expected patch string (Lines 589-593) is a long concatenated string that could be brittle if JSON field ordering changes. Sincejson.Marshalon Go structs is deterministic, this should be stable, but if it ever becomes a maintenance burden, you could consider comparing parsed JSON objects instead of raw strings.Not blocking — just a heads-up for future-proofing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/reconciler/managed/api_test.go` around lines 577 - 595, The test "PatchWithManagedFieldsMerged" uses a long concatenated JSON string in want.patch which is brittle to field ordering; change the assertion to parse both the expected JSON string (want.patch) and the actual produced patch into generic JSON structures (e.g., map[string]interface{} via json.Unmarshal) and compare them with a structural comparison (reflect.DeepEqual or cmp.Diff) instead of direct string equality—update the assertion around the code that reads the produced patch (the variable holding the actual patch in the test) to unmarshal both values and assert equality of the parsed objects.
507-527: Consider extracting shared helper logic or usingt.Helper()for error reporting.Both
newManagedFieldsObj(Line 507) andwithManagedFields(Line 624) construct very similarUnstructuredobjects with managed fields, differing only in the field name/value and managed fields content. This is fine since they serve distinct purposes, but two things stood out:
The
_ = unstructured.SetNestedField(...)calls (Lines 515, 632, 654) silently discard errors. If the test data is ever malformed, these would fail silently and produce confusing test failures downstream. Would it make sense to refactor these helpers to accepttesting.Tand callt.Fatalon error?Minor DRY opportunity: a small shared builder (e.g.,
newTestUnstructured(gvk, name)) could reduce the boilerplate acrossnewManagedFieldsObj,withoutManagedFields,withManagedFields, andwithDifferentManager. Totally optional — just flagging in case it helps readability as more test cases are added.Also applies to: 624-644
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/reconciler/managed/api_test.go` around lines 507 - 527, The tests silently discard errors from unstructured.SetNestedField in helpers like newManagedFieldsObj and withManagedFields; change these helper constructors to accept t *testing.T, call t.Helper(), and check the error returned from SetNestedField (calling t.Fatalf or t.Fatal on error) so malformed test data fails fast with clear messages; optionally factor the repeated GVK/name setup into a small shared builder (e.g., newTestUnstructured or buildUnstructured(gvk, name)) used by newManagedFieldsObj, withoutManagedFields, withManagedFields, and withDifferentManager to reduce boilerplate while preserving their specific managedFields setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/reconciler/managed/api.go`:
- Around line 249-254: The mergeWithManagedFields function currently swallows
errors from runtime.DefaultUnstructuredConverter.ToUnstructured and returns the
original patch (patch, nil), which can reintroduce the bug this PR fixes; change
this to propagate the conversion error instead of silently degrading: in
mergeWithManagedFields when ToUnstructured returns an error, return nil and the
error (or wrap it with context mentioning mergeWithManagedFields and
ToUnstructured) so callers can handle/fail loudly; if you intentionally want a
non-fatal path, add a clear comment explaining the specific guarded scenario and
why returning the unmerged patch is safe.
---
Nitpick comments:
In `@pkg/reconciler/managed/api_test.go`:
- Around line 668-717: Add a failing-path test case named "ExtractIntoError"
that constructs an obj whose metadata.managedFields contains a deliberately
malformed entry so managedfields.ExtractInto returns an error (for example, an
unstructured.Unstructured with metadata.managedFields as a []interface{}
containing an entry where "fieldsV1" is an invalid type/contents), call the same
function under test (the code-path that invokes managedfields.ExtractInto in
api.go), and assert the returned error is non-nil and wraps
errExtractManagedFields; reference the test map in
pkg/reconciler/managed/api_test.go and the error symbol errExtractManagedFields
and the call site that uses managedfields.ExtractInto so the new case follows
the existing "cases" structure.
- Around line 577-595: The test "PatchWithManagedFieldsMerged" uses a long
concatenated JSON string in want.patch which is brittle to field ordering;
change the assertion to parse both the expected JSON string (want.patch) and the
actual produced patch into generic JSON structures (e.g., map[string]interface{}
via json.Unmarshal) and compare them with a structural comparison
(reflect.DeepEqual or cmp.Diff) instead of direct string equality—update the
assertion around the code that reads the produced patch (the variable holding
the actual patch in the test) to unmarshal both values and assert equality of
the parsed objects.
- Around line 507-527: The tests silently discard errors from
unstructured.SetNestedField in helpers like newManagedFieldsObj and
withManagedFields; change these helper constructors to accept t *testing.T, call
t.Helper(), and check the error returned from SetNestedField (calling t.Fatalf
or t.Fatal on error) so malformed test data fails fast with clear messages;
optionally factor the repeated GVK/name setup into a small shared builder (e.g.,
newTestUnstructured or buildUnstructured(gvk, name)) used by
newManagedFieldsObj, withoutManagedFields, withManagedFields, and
withDifferentManager to reduce boilerplate while preserving their specific
managedFields setup.
In `@pkg/reconciler/managed/api.go`:
- Around line 59-61: Update the error strings errExtractManagedFields,
errMarshalManagedFields, and errMergePatches in pkg/reconciler/managed/api.go to
be more user-facing and state why the failure matters (e.g., preserving
previously-resolved reference/managed fields during reconciliation), so replace
the technical phrases like "cannot extract previously managed fields from
resolved resource" with clearer messages such as "cannot preserve previously
resolved reference fields" or similar; ensure each message still uniquely maps
to its error symbol (errExtractManagedFields, errMarshalManagedFields,
errMergePatches) and retains enough context for debugging while avoiding heavy
internal jargon.
Signed-off-by: TehilaTheStudent <tehila14916@gmail.com>
TehilaTheStudent
force-pushed
the
fix-ssa-patch
branch
from
fc40c97 to
f2f92cb
Compare
I changed the implementation to return the original patch to not break the reconciliation for errors in managedfields.extractInto |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/reconciler/managed/api.go (1)
59-60:errMergePatchescould use a bit more context.
"cannot merge patches"on its own is quite opaque. Adding subject context would help operators diagnose the error — e.g.,"cannot merge managed fields patch with resolved reference patch". As per coding guidelines, error messages should include context about what the user was trying to do.✏️ Suggested improvement
- errMergePatches = "cannot merge patches" + errMergePatches = "cannot merge managed fields patch with resolved reference patch"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/reconciler/managed/api.go` around lines 59 - 60, The error constant errMergePatches ("cannot merge patches") is too vague; update its value to include subject/context (e.g., "cannot merge managed fields patch with resolved reference patch") so operators know what was being merged; change the constant definition errMergePatches in pkg/reconciler/managed/api.go and adjust any places that return or wrap errMergePatches to preserve the new contextual message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/reconciler/managed/api_test.go`:
- Around line 577-595: Rename the test case "PatchWithManagedFieldsMerged" to
"PatchFallsBackWhenExtractIntoFails" and update its reason string to state that
the test verifies the fallback path when ExtractInto fails (due to managedFields
claiming non-existent fields), rather than asserting that previously managed
fields are merged; update the test's comment and any references to the fixture
newManagedFieldsObj() and the mergeWithManagedFields behavior so they document
that ExtractInto triggers the fallback and the expected patch is the original
diff including managedFields metadata from resolved.
---
Duplicate comments:
In `@pkg/reconciler/managed/api.go`:
- Around line 250-253: The current code swallows conversion errors from
runtime.DefaultUnstructuredConverter.ToUnstructured (when converting obj into u)
by returning (patch, nil), which allows an SSA patch to be applied without
managed fields; instead, propagate the conversion error so reconciliation
retries and surfaces the failure—update the ToUnstructured error handling in the
function that constructs patch (referencing
runtime.DefaultUnstructuredConverter.ToUnstructured, variable u and patch) to
return the error (e.g., return patch, err) rather than nil, so callers receive
and surface the conversion failure.
---
Nitpick comments:
In `@pkg/reconciler/managed/api.go`:
- Around line 59-60: The error constant errMergePatches ("cannot merge patches")
is too vague; update its value to include subject/context (e.g., "cannot merge
managed fields patch with resolved reference patch") so operators know what was
being merged; change the constant definition errMergePatches in
pkg/reconciler/managed/api.go and adjust any places that return or wrap
errMergePatches to preserve the new contextual message.
| "PatchWithManagedFieldsMerged": { | ||
| // Verifies that managed fields referencing fields not in the current patch | ||
| // are still included in the final patch to maintain SSA field ownership. | ||
| // Note: The expected patch includes managedFields metadata because existing | ||
| // has none while resolved does - this is a test artifact. In production, | ||
| // both objects would have identical managedFields so the diff wouldn't include them. | ||
| reason: "Should merge previously managed fields into the patch to maintain SSA field ownership.", | ||
| args: args{ | ||
| existing: withoutManagedFields(), | ||
| resolved: newManagedFieldsObj(), | ||
| }, | ||
| want: want{ | ||
| patch: `{"apiVersion":"example.com/v1","kind":"TestResource",` + | ||
| `"metadata":{"managedFields":[{"fieldsType":"FieldsV1",` + | ||
| `"fieldsV1":{"f:spec":{"f:forProvider":{"f:otherRefId":{}}}},` + | ||
| `"manager":"managed.crossplane.io/api-simple-reference-resolver",` + | ||
| `"operation":"Apply"}]},"spec":{"forProvider":{"someRefId":"new-ref-id"}}}`, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
PatchWithManagedFieldsMerged test name and reason don't match the actual scenario.
The newManagedFieldsObj() fixture has a managedFields entry claiming ownership of spec.forProvider.otherRefId, but that field doesn't exist on the object. When mergeWithManagedFields runs, ExtractInto will fail for exactly the edge case described by provider-kubernetes#420, so the code falls back to returning the original diff patch. The managedFields block in the expected output comes from the JSON diff between existing (no managed fields) and resolved (has managed fields metadata) — it's not actually the result of the merge logic.
The test is therefore exercising the fallback path, not "previously managed fields merged into the patch." Could we rename it (e.g., PatchFallsBackWhenExtractIntoFails) and update the reason string so it accurately documents what's being validated? The happy path is correctly covered by TestMergeWithManagedFields.WithManagedFields, so there's no coverage gap — this is purely a documentation/readability concern.
✏️ Suggested rename
- "PatchWithManagedFieldsMerged": {
- // Verifies that managed fields referencing fields not in the current patch
- // are still included in the final patch to maintain SSA field ownership.
- // Note: The expected patch includes managedFields metadata because existing
- // has none while resolved does - this is a test artifact. In production,
- // both objects would have identical managedFields so the diff wouldn't include them.
- reason: "Should merge previously managed fields into the patch to maintain SSA field ownership.",
+ "PatchFallsBackWhenExtractIntoFails": {
+ // Verifies that when ExtractInto fails because a managed-fields entry references
+ // a spec field that is absent from the object (the edge case in provider-kubernetes#420),
+ // prepareJSONMerge falls back to returning the raw diff patch unchanged.
+ // The managedFields block in the expected output is a diff artifact (existing has none,
+ // resolved does) and is not produced by the merge logic itself.
+ reason: "Should fall back to the raw diff patch when ExtractInto fails due to a managed field referencing an absent spec field.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/reconciler/managed/api_test.go` around lines 577 - 595, Rename the test
case "PatchWithManagedFieldsMerged" to "PatchFallsBackWhenExtractIntoFails" and
update its reason string to state that the test verifies the fallback path when
ExtractInto fails (due to managedFields claiming non-existent fields), rather
than asserting that previously managed fields are merged; update the test's
comment and any references to the fixture newManagedFieldsObj() and the
mergeWithManagedFields behavior so they document that ExtractInto triggers the
fallback and the expected patch is the original diff including managedFields
metadata from resolved.
2 participants
Description of your changes
When APISimpleReferenceResolver resolves references and applies an SSA patch, it only included newly resolved fields. The API server interpreted missing previously-managed fields as intentional deletions, causing an infinite reconciliation loop.
This adds mergeWithManagedFields which extracts fields previously managed by
managed.crossplane.io/api-simple-reference-resolverand merges them into the patch before applying.Fixes #694
I have:
./nix.sh flake checkto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.