* 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>
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>
When lookup cannot find the requested resource (apierrors.IsNotFound),
add slog.Debug() calls with structured fields (apiVersion, kind,
namespace, name) so that users running helm template --debug can see
why lookup returned an empty map instead of silently swallowing the
not-found result.
Fixes: https://github.com/helm/helm/issues/32101
Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
slog.Error on line 157 passes printf-style positional args (%q, %s)
instead of key-value pairs. The slog API treats these as unkeyed
attributes, producing garbled log output.
Two nearby slog.Warn calls wrap fmt.Sprintf unnecessarily. Convert
all three calls to use proper structured key-value arguments.
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Assisted-by: Grok/grok-4
- Distinguish unowned resources from those whose ownership could not be
verified (e.g. RBAC/network errors). Unverifiable resources are now
returned in a separate list with the underlying error so log output
and the "kept" summary describe what actually happened.
- Only prefix the kept summary with the resource-policy header when
there are policy-kept resources, so users with only ownership-skipped
resources don't see a misleading header.
- Update verifyOwnershipBeforeDelete doc comment to reflect that
not-found resources are excluded from all returned lists.
- Assert success of Releases.Create in uninstall tests so fixture
setup failures are surfaced immediately.
Signed-off-by: Evans Mungai <mbuevans@gmail.com>
- Close log stream after reading (prevents connection/fd leak)
- Strengthen tests to assert on output headers rather than error paths
- Remove unused import
Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
When a test pod contains multiple containers (e.g. Istio/Consul/Vault
sidecars), 'helm test --logs' failed with 'a container name must be
specified'. This happened because GetPodLogs called the Kubernetes log
API without specifying a container name.
The fix fetches the pod spec first, then iterates over all containers
(init containers + regular containers) and requests logs for each one
explicitly. Errors from individual containers are collected and returned
together via errors.Join rather than aborting on the first failure.
Also fixes a typo: hooksByWight -> hooksByWeight.
Closes#6902
Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
Helm now requires Go 1.26 (#32078); the cloner[T] type-assertion fallback
in transport.go was a defensive shim for Go versions before
http.Transport.Clone() existed. The fallback path is unreachable on
supported Go versions.
Refs: #31386
Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
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>
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>
`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>
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>
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>
George Jenkins
Evans Mungai
Terry Howe
Ogulcan Aydogan
Austin Abro
Robert Sirchia
Sebastien Tardif
Kai Tanaka
box4wangjing
Sumit Solanki
Matthieu MOREL
Matheus Pimenta
Benoit Tigeot
Johannes Lohmer