Handle null observedGeneration in condition Equal()

[bobh66]

Description of your changes

Modified the condition Equial() method to check for unset ObservedGeneration fields
when comparing two conditions. If at least one of the ObservedGenerations is 0 then the comparison
will ignore the field since it will always be False.

This can happen when a provider does not set the observedGeneration in a Synced condition during Observe(), which causes it to appear to be different than the Synced condition that is set by the managed reconciler.

Fixes #902

I have:

• Read and followed Crossplane's contribution process.
• Run earthly +reviewable to ensure this PR is ready for review.
• Added or updated unit tests.
  - [ ] Linked a PR or a docs tracking issue to document this change.
• Added backport release-x.y labels to auto-backport this PR.

Need help with this checklist? See the cheat sheet.

[coderabbitai]

📝 Walkthrough

Walkthrough

The Condition.Equal() method now treats ObservedGeneration as optional: if either condition has ObservedGeneration == 0, equality compares only Type, Status, Reason, and Message; otherwise ObservedGeneration is included in the comparison.

Changes

Cohort / File(s)	Summary
Condition Equality Logic apis/common/condition.go	Added guard clause in Equal() so that when either condition's ObservedGeneration is 0, equality ignores ObservedGeneration and compares Type, Status, Reason, and Message; otherwise all five fields are compared.
Condition Tests apis/common/condition_test.go	Added test IdenticalWithMissingObservedGeneration to assert equality when one condition lacks ObservedGeneration; updated DifferentObservedGeneration case to assert inequality when both have differing non-zero ObservedGeneration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Thank you for the clear change and tests — would you like me to add a note to the changelog or link this behavior to the related issue (#902)?

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Breaking Changes	❌ Error	The PR modifies exported Condition.Equal() method behavior in apis/common package, a public API breaking change, but lacks the required 'breaking-change' label.	Add the 'breaking-change' label to acknowledge the behavior modification, or document why the change is backward-compatible and the label can be waived.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and descriptively summarizes the main change: handling null/unset observedGeneration in the condition Equal() method.
Description check	✅ Passed	The description is directly related to the changeset, explaining the rationale, the specific problem being solved, and referencing issue #902.
Linked Issues check	✅ Passed	The code changes fully implement the requirement from issue #902: modify Condition.Equal() to ignore ObservedGeneration when either value is 0, comparing only Type, Status, Reason, and Message [#902].
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #902: the implementation in condition.go and corresponding test updates in condition_test.go are both in-scope.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[ulucinar]

Hi @bobh66,
Thanks for investigating this issue and proposing a fix. Please also see my comment here.

The .metadata.generation starts from 1, so observedGeneration being 0 effectively means it's not set. crossplane-runtime will always set the status conditions with a non-zero observedGeneration (because the resource's .metadata.generation cannot be 0). And if the provider cooperates (sets the observedGeneration, they should again always be non-zero). So only for a non-cooperating provider (a provider that does not set the observedGeneration on the status conditions it sets), the observedGeneration can be 0. And in that case, knowing the provider does not set observedGenerations, we can safely ignore them in comparisons.

[jbw976]

Thanks @bobh66 for taking this on and @ulucinar for the thoughtful review and impact assessment.

Let's get this merged, so it's included in v2.2 ✅

[github-actions]

Successfully created backport PR for release-2.0:

• [Backport release-2.0] Handle null observedGeneration in condition Equal() #915