mirror of
https://github.com/helmfile/helmfile.git
synced 2026-09-30 13:16:12 +02:00
Ankit
1341fd8659
* fix: skip chartification when chart renders zero resources
When a chart's templates render zero resources (e.g. everything is
gated behind a falsy `{{- if .Values.enabled }}`) and the release has
transformers, jsonPatches, or strategicMergePatches configured,
helmfile crashed with:
assertion failed: unexpected dir entry "" it must be the abs path
to the output directory
Root cause: chartify's replace.go runs `helm template --output-dir`
and expects exactly one directory entry under that output dir (the
rendered chart). When helm renders no resources, the output dir is
empty, so chartOutputDir stays "" and chartify's own assertion on it
being an absolute path fails. chartify (v0.28.0) doesn't expose a
typed/sentinel error for this, only the assertion text.
Since there's nothing to chartify when a release has no rendered
resources, treat this specific chartify failure as a no-op: keep
using the chart as-is and let helm template it normally (producing
the same empty output helm would have produced without chartify).
Any other chartify error is still surfaced unchanged.
Verified manually end-to-end with a real helm+kustomize:
- a chart with `enabled: false` + a transformer now runs without
error instead of crashing
- a chart with `enabled: true` + the same transformer still gets
transformed correctly (annotations applied), confirming the normal
chartify path is untouched
Added unit tests for the new error-matching helper in
pkg/state/issue_1757_test.go.
Fixes #1757
Signed-off-by: ankit090701 <ankitanku090701@gmail.com>
* fix: return original chart path from empty-render no-op, not the deps-rewrite temp copy
Review feedback on the original fix (#2724) found a real bug: the
empty-render no-op path returned chartPath after it may have already
been reassigned to the temp copy created by rewriteChartDependencies
(for charts with relative file:// deps). That temp dir is removed by
a deferred cleanupTempChart() as soon as processChartification
returns, so the caller was handed a path to a directory that no
longer existed, breaking any subsequent helm command with "chart not
found" - narrow (only local charts with relative file:// deps that
also render zero resources) but real and reproducible.
Capture originalChartPath before the rewrite and return that instead.
Also flatten the nested `if err != nil { if isChartifyEmptyRenderOutputError...`
into two sequential checks per review, and link the upstream tracking
issue (helmfile/chartify#206, opened by a maintainer during review) in
the error-matching constant's doc comment.
Added TestProcessChartification_EmptyRenderReturnsSurvivingPath, an
end-to-end test exercising the real processChartification ->
chartify.Chartify wiring (not just the isChartifyEmptyRenderOutputError
helper) with a chart that has a relative file:// dependency and renders
zero resources - the exact conditions that trigger the bug. Verified
passing against real helm+kustomize in a Linux container; skipped on
Windows due to an unrelated, pre-existing Windows path-handling issue
in chartify's dependency resolution (a drive letter embedded in a
file:// URL gets mis-joined), independent of the code path under test.
Signed-off-by: ankit090701 <ankitanku090701@gmail.com>
* fix: narrow chartify empty-render error match to avoid false positives
Per Copilot's automated review on this PR: the previous substring,
"it must be the abs path to the output directory", matches chartify's
assertion regardless of what chartOutputDir actually is. In the real
empty-render case chartOutputDir is "" (formatted via %q as `""`), but
the same assertion (chartify replace.go:151) would also fire if
chartOutputDir were ever a non-empty-but-still-relative path - a
different, genuine bug that should be surfaced as an error, not
silently treated as an empty-render no-op.
Narrow the match to include the `unexpected dir entry ""` prefix, so
it can only match the exact empty-string case. Verified against the
actual chartify v0.28.0 source (fmt.Errorf with %q on chartOutputDir)
that this is precisely what the empty-render case produces.
Added a test case asserting the same assertion text with a non-empty
dir entry is correctly NOT treated as the empty-render case. Re-ran
the full test suite (including the real helm+kustomize end-to-end
integration test) in a Linux container to confirm the narrower match
still catches the actual reported bug.
Signed-off-by: ankit090701 <ankitanku090701@gmail.com>
---------
Signed-off-by: ankit090701 <ankitanku090701@gmail.com>