Commit Graph

4 Commits

Author SHA1 Message Date
yxxhero fc31dbfc5e
fix: eliminate race condition in rewriteChartDependencies (#2541)
* fix: eliminate race condition in rewriteChartDependencies by copying chart before modifying

Instead of modifying the original Chart.yaml in-place (which causes race
conditions when multiple releases reference the same local chart), copy the
chart to a temporary directory and rewrite the copy's dependencies. This
eliminates the need for per-chart mutex locks and prevents file corruption
when concurrent goroutines process releases sharing the same local chart.

Fixes #2502

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address PR review comments for rewriteChartDependencies

- Handle non-NotExist errors from st.fs.Stat to surface permission/IO failures
- Reword function doc to clarify temp copy is conditional on rewrite being needed
- Assert rewrittenPath vs tempDir based on expectModified in test table

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for issue #2502 race condition with shared local chart

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: separate environments and releases with --- in helmfile.yaml

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct file:// path and remove --skip-deps for dependency build

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct file:// dependency path (5 levels up to test/integration/)

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: remove output validation from race condition test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: assert WriteFile/MkdirTemp/RemoveAll/CopyDir in DefaultFileSystem test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: add strategicMergePatches to trigger chartify in race condition test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: scope test values under raw subchart and align ConfigMap name with strategic merge patches

The race condition test values.yaml had templates at the top level instead
of scoped under the raw subchart key, causing helm template to produce no
output and chartify's ReplaceWithRendered to fail with an empty
helmx.1.rendered directory. Also align the ConfigMap name to match the
strategicMergePatches target.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-04-20 10:15:47 +08:00
yxxhero acb7ce36fc
fix: add mutex lock for concurrent rewriteChartDependencies access (#2509)
* fix: add mutex lock for concurrent rewriteChartDependencies access

Prevent race conditions when multiple goroutines concurrently access
the same Chart.yaml by introducing a per-chart-path mutex via sync.Map.

Signed-off-by: yxxhero <yxxhero@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: unlock mutex on all error paths and improve race condition test validation

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/135e06b8-99e8-42cb-859d-524b2d2b1907

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: remove duplicate mutex, reuse getNamedRWMutex, restore on write failure, add test barrier

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/f93746bf-82fa-46b1-b8f0-b34bc9aa749c

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: early unlock when unmodified, assert exact restore in race test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/5981aea4-2799-4560-9272-315d28d4b7d7

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: improve comment wording and strengthen race test start barrier

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/9e4d6f4f-cdc1-4e9e-bdc6-81061ebc1dcc

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: yxxhero <yxxhero@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
2026-04-13 11:59:52 +08:00
Etienne Champetier 4bdb6f097c
fix: keep all chart dependencies key / values (#2501)
* feat: Refactor TestRewriteChartDependencies

Signed-off-by: Etienne Champetier <e.champetier@ateme.com>

* fix: keep all chart dependencies key / values

In rewriteChartDependencies we were only parsing name / repository / version,
thus dropping keys like condition / import-values.

This at least fixes the use of condition.

Signed-off-by: Etienne Champetier <e.champetier@ateme.com>

---------

Signed-off-by: Etienne Champetier <e.champetier@ateme.com>
2026-03-26 13:16:54 +08:00
Shane Starcher 61f4a316a6
fix: rewrite relative file:// chart dependencies to absolute paths (#2334)
Fixes an issue where Chart.yaml dependencies with relative file:// paths
fail during chartification because the paths become invalid when the chart
is copied to chartify's temporary directory.

The rewriteChartDependencies function now converts relative file://
dependencies to absolute paths before chartification, then restores the
original Chart.yaml afterwards. Absolute file:// and other repository
types (https, oci) are left unchanged.

Includes comprehensive test coverage for various dependency scenarios.

Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
Co-authored-by: Shane Starcher <shane.starcher@gmail.com>
2025-12-20 09:08:39 +08:00