[v0.14] Prevent false drift from null field rendering #4997
Merged
Conversation
* Prevent false drift from Helm v4 null field rendering Helm v4 can emit explicit null values for omitted fields (e.g., spec.strategy.rollingUpdate: null), while Helm v3 omitted these fields entirely. When combined with Kubernetes server-side defaulting, this creates false drift detection: the merge patch interprets null as "delete this field," conflicting with defaulted live resources. Solution: - Add normalizeNullPatch to recursively strip null-valued fields from merge patches before drift comparison - Handle nested structures (maps and arrays) to cover all potential Helm v4 null emissions beyond just deployment strategy * Add fast-path optimization for null patch normalization in diff.go Add bytes.Contains fast-path check before expensive unmarshal/walk/ marshal operations when patch doesn't contain 'null'. This provides ~55x performance improvement for patches without nulls: - WithoutNulls: 127.9 ns/op (vs 7063 ns/op with nulls) - 97% fewer allocations (1 vs 72 allocs/op) - 97% less memory (176 B vs 5404 B per op) Also add benchmark tests and fast-path test coverage. (cherry picked from commit 7a7732f) Refers to rancher#4969
Collaborator
|
Why are we backporting a Helm v4 fix to Fleet 0.14 ? 🤔 |
Contributor
Author
As it turned out, the cause is not Helm 4, but an issue that we have introduced and backported down to Fleet v0.11. But the changes are the right one to fix this issue anyway. |
Collaborator
|
Ok, then please adapt the title to reflect that. |
thardeck
approved these changes
Apr 17, 2026
Closed
1 task
thardeck
added a commit
that referenced
this pull request
Apr 17, 2026
* Revert "[v0.14] Prevent false drift from null field rendering (#4997)" This reverts commit fcd6873. * Revert "[v0.14] Remove k8s.io/kubernetes, use client-go scheme (#4902)" This reverts commit 58f0054. * Align k8s.io/kubernetes to v1.34.6 to match other k8s libraries After reverting the k8s.io/kubernetes removal (PR #4902), bump k8s.io/kubernetes from v1.34.5 to v1.34.6 and update all replace directives to v0.34.6, matching the kubernetes dependencies that were already at v0.34.6 on this branch (bumped in #4901). Refs: #4969
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick from #4664
Refers to #4969
Additional Information
Checklist