* fix: protect FailingKubeClient.RecordedWaitOptions from concurrent access
Add a sync.Mutex to guard the append to RecordedWaitOptions in
GetWaiterWithOptions, fixing a data race detected by -race when
concurrent goroutines (e.g. upgrade + rollback) both call
GetWaiterWithOptions on the same FailingKubeClient instance.
Fixes race failures in TestUpgradeRelease_Interrupted_RollbackOnFailure
and TestInstallRelease_RollbackOnFailure_Interrupted.
Signed-off-by: Terry Howe <thowe@nvidia.com>
* fix: extract appendRecordedWaitOptionsLocked helper with defer unlock
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
---------
Signed-off-by: Terry Howe <thowe@nvidia.com>
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
(cherry picked from commit a5552edf9f)
Commands like 'helm registry login', 'helm push', and 'helm pull' were
writing success messages ("Login Succeeded", "Pushed:", "Pulled:",
"Digest:") to stderr instead of stdout. The root cause was that
newDefaultRegistryClient and newRegistryClientWithTLS hard-coded
os.Stderr as the registry client writer, ignoring the out io.Writer
that main() passes as os.Stdout.
Thread out io.Writer through newRegistryClient, newDefaultRegistryClient,
and newRegistryClientWithTLS, and update all call sites in pkg/cmd.
Fixes#13464
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
(cherry picked from commit c2f1b238a1)
`isVersionRange` checked for `x`/`X` across the entire version
string, misclassifying exact versions like `1.0.0-fix`,
`2.0.0-next`, or `1.0.0+exp` as ranges.
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
(cherry picked from commit 740174a2b1)
From Matt's comment
> The check for " || " should remove the spaces and have "||". Spaces around the || aren't required.
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
(cherry picked from commit b79d7f1881)
When using version ranges like ^1 or ~1.10, Helm incorrectly showed
warnings about falling back to closest version. Only show the warning
when an exact version is requested but not found.
Fixes: https://github.com/helm/helm/issues/31757
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
(cherry picked from commit 1e145ee2b2)
The previous change suppressed ctx.Err() whenever there were no
resource-specific errors, which incorrectly swallowed context.Canceled
and other non-deadline errors signalling an external interruption.
Refine the condition: only suppress context.DeadlineExceeded when there
are no resource-specific errors (resources are Unknown/NotFound, meaning
the delete wait succeeded or resources were already gone). Any other
context error — including context.Canceled — is always propagated.
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
(cherry picked from commit 5e09ee78ee)
During informer initialization there is a brief window where watched
resources appear as Unknown before their real statuses are delivered.
The statusObserver skips Unknown resources when waiting for deletion
(they may have been deleted before the watch started), but if *all*
resources are in that transient Unknown state the skipped-resource list
is empty. AggregateStatus on an empty slice returns the desired status,
causing cancel() to be called immediately — before any real status event
has arrived.
Guard against this by tracking the count of Unknown-skipped resources.
When every resource was Unknown-skipped and none have a definitive status
yet, defer the early-cancel decision until at least one resource reports
a real status. This preserves the correct behaviour for resources that
were genuinely deleted before the watch started (they eventually receive
a NotFound or stay Unknown, and the aggregate succeeds), while fixing
the race for resources that are transiently Unknown at startup.
Also tighten the ctx.Err() check in waitForDelete: only append a
deadline error when there are resource-specific errors to accompany it.
A timeout while all resources are Unknown or NotFound is not itself an
error — the resources are in an acceptable state for a delete wait.
Fixes: TestStatusWaitForDelete/error_when_not_all_objects_are_deleted
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
(cherry picked from commit 4e24ee41a4)
Only user-supplied nils should survive coalescing. Chart-default nils
defaults, not just user overrides. This caused:
- %!s(<nil>) in templates using Bitnami common.secrets.key (#31919)
- pluck fallbacks returning nil instead of falling through to globals
(#31971)
Fixes#31919Fixes#31971
Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>
Three test cases that cover the regression scenarios introduced
by the #31644 nil preservation fix:
- subchart default nils should be cleaned up
when parent doesn't set those keys (#31919)
- user-supplied null should erase subchart defaults (#31919)
- subchart default nil should not shadow global values via pluck (#31971)
Tests are expected to fail until the regression is fixed.
Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>
When running 'helm pull --debug', no debug output was printed because
the HTTP getter did not emit any slog.Debug messages. This adds
slog.Debug calls to log the URL being fetched and the response status
when debug-level logging is enabled.
Fixeshelm/helm#31098
Signed-off-by: Cairon <cairon-ab@users.noreply.github.com>
Move implementation to pkg/kubeenv per review; kube.RetryingRoundTripper
remains a type alias for API compatibility. pkg/cli uses kubeenv only.
Signed-off-by: Sumit Solanki <sumit.solanki@ibm.com>
The previous fix (increasing timeout / reducing deletion delay) did not
work because the flakiness is not a timing problem at all.
Root cause: fluxcd/cli-utils HasSynced() returns true after the initial
list item is *popped* from DeltaFIFO, which is before AddFunc delivers
the ResourceUpdateEvent to the collector. This creates a race where the
SyncEvent can arrive at the statusObserver *before* the pod's Current
status is recorded. When that happens:
- statusObserver sees pod as Unknown
- Unknown is skipped for WaitForDelete (by design, to handle resources
that were already deleted before watching started)
- AggregateStatus([], NotFoundStatus) == NotFoundStatus → cancel()
- The watch context is cancelled before DeleteFunc can fire
- Final check: pod still Current → error
The test intent is to verify that waitForDeleteCtx (not the cancelled
generalCtx) is selected. A non-existent resource satisfies this:
- With waitForDeleteCtx=Background(): informer syncs with empty list
→ Unknown → cancel → success ✓
- With generalCtx (cancelled, wrong): context immediately done
→ ctx.Err() appended → error returned ✓
Remove the goroutine-based deletion and the pod creation to eliminate
the race while preserving the context-selection assertion.
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
TestMethodContextOverridesGeneralContext/WaitForDelete used a 1s
timeout with a 500ms deletion delay, leaving only ~500ms for the
fake watcher to propagate the delete event. On loaded CI runners
this window is too tight and causes intermittent failures.
Increase the timeout to 5s and reduce the deletion delay to 100ms
so there is ample headroom. Apply the same deletion-delay reduction
to TestStatusWaitForDelete which shares the same pattern.
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
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#32007
Signed-off-by: Rhys McNeill <rhysmcneill7@hotmail.co.uk>
Move the retrying HTTP round-tripper used by EnvSettings into pkg/cli so
pkg/cli no longer imports pkg/kube, avoiding import cycles for Helm
library consumers while preserving retry behavior for transient API
server errors.
Add pkg/cli/roundtripper_test.go with parity coverage for the moved logic.
Signed-off-by: Sumit Solanki <sumit.solanki@ibm.com>
Made-with: Cursor
Move the retrying round tripper used by EnvSettings into pkg/cli so
pkg/cli no longer imports pkg/kube. This preserves retry behavior while
breaking the import edge that triggers cycles for Helm library consumers
(such as the kustomize integration described in #31965).
Signed-off-by: Sumit Solanki <sumit.solanki@ibm.com>
The toTOML doc comment said "returns empty string on marshal error"
but the implementation actually returns err.Error(). Fix the comment
to match the real behavior. Also mention mustToToml as the strict
alternative.
Signed-off-by: Ilya Kiselev <kis-ilya-a@yandex.ru>
Add `mustToToml` that panics on marshal error, consistent with
`mustToYaml` and `mustToJson`. This makes it possible for chart authors
to get a hard failure when TOML serialization fails, rather than having
to inspect the output manually.
`toToml` behavior is unchanged in this commit.
Closes#31430
Signed-off-by: Ilya Kiselev <kis-ilya-a@yandex.ru>
Terry Howe
Benoit Tigeot
George Jenkins
Matheus Pimenta
Evans Mungai
Johannes Lohmer
Johannes Lohmer
Cairon
Sumit Solanki
Anmol Virdi
Rhys McNeill
abhay1999
Debasish Mohanty
Ilya Kiselev
Matthieu MOREL