Conversation
When Deployed() succeeds but releaserToV1Release() fails, prepareUpgrade returned err (nil) instead of cerr (the conversion error), causing a silent nil return that could lead to nil pointer dereferences downstream. Closes helm#32007 Signed-off-by: Rhys McNeill <rhysmcneill7@hotmail.co.uk>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the prepareUpgrade function in pkg/action/upgrade.go where the wrong error variable is returned when release conversion fails. When the Deployed() call succeeds but the subsequent conversion to V1 format fails, the code was returning the error from the Deployed() call (which is nil) instead of the conversion error, silently swallowing the failure. This could cause nil pointer dereferences downstream.
Changes:
- Fixed line 256 in
pkg/action/upgrade.goto returncerrinstead oferrwhen release conversion fails
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gjenkins8
approved these changes
Apr 20, 2026
Member
|
Good catch. Thanks! |
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.
Closes #32007
What this PR does / why we need it:
During an upgrade, if the last release is not in a deployed state, Helm fetches the currently deployed release and converts it. If that conversion fails, the wrong error variable is returned — the error from the fetch (which is nil on success) rather than the actual conversion error. This silently swallows the failure, returning nil across the board, which could cause a nil pointer panic further down the call chain.
This PR fixes the single-line variable mix-up so the correct error is propagated.
Special notes for your reviewer:
All existing upgrade tests pass locally. A dedicated regression test for this path is not straightforward to add because the storage layer is a concrete type rather than an interface, so the conversion failure cannot be triggered with the current test infrastructure. Happy to discuss a follow-up if maintainers would like that addressed separately.
If applicable:
docs neededlabel should be applied if so)