From 9b8aa46faacc4a22d1ee2ee3558cd91828793d41 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Tue, 31 Mar 2026 21:48:03 +0800 Subject: [PATCH] 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 Signed-off-by: yxxhero --- pkg/state/chart_dependencies_rewrite_test.go | 66 ++++++++++++++++++++ pkg/state/state.go | 18 +++++- 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/pkg/state/chart_dependencies_rewrite_test.go b/pkg/state/chart_dependencies_rewrite_test.go index a51ae31a..a407f44a 100644 --- a/pkg/state/chart_dependencies_rewrite_test.go +++ b/pkg/state/chart_dependencies_rewrite_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "go.uber.org/zap" @@ -503,3 +504,68 @@ dependencies: t.Errorf("relative path with ./ should have been converted") } } + +func TestRewriteChartDependencies_RaceCondition(t *testing.T) { + tempDir, err := os.MkdirTemp("", "helmfile-test-") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + chartYaml := `apiVersion: v2 +name: test-chart +version: 1.0.0 +dependencies: + - name: dep1 + repository: file://../relative-chart + version: 1.0.0 +` + + chartYamlPath := filepath.Join(tempDir, "Chart.yaml") + if err := os.WriteFile(chartYamlPath, []byte(chartYaml), 0644); err != nil { + t.Fatalf("failed to write Chart.yaml: %v", err) + } + + // Run multiple goroutines concurrently on the same chart path + numGoroutines := 10 + var wg sync.WaitGroup + errCh := make(chan error, numGoroutines) + + for i := 0; i < numGoroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + + logger := zap.NewNop().Sugar() + st := &HelmState{ + logger: logger, + fs: filesystem.DefaultFileSystem(), + } + + cleanup, err := st.rewriteChartDependencies(tempDir) + if err != nil { + errCh <- err + return + } + defer cleanup() + }() + } + + wg.Wait() + close(errCh) + + for err := range errCh { + t.Errorf("goroutine error: %v", err) + } + + // Verify Chart.yaml is still valid + data, err := os.ReadFile(chartYamlPath) + if err != nil { + t.Fatalf("failed to read Chart.yaml: %v", err) + } + + // Should still have the dependencies section + if !strings.Contains(string(data), "dependencies:") { + t.Errorf("Chart.yaml should still have dependencies section") + } +} diff --git a/pkg/state/state.go b/pkg/state/state.go index 9fe36558..0a663a5c 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1417,7 +1417,18 @@ type PrepareChartKey struct { // // When running `helmfile template` on helm v2, or `helmfile lint` on both helm v2 and v3, // PrepareCharts will download and untar charts for linting and templating. -// +var chartDepsMutexMap sync.Map + +func (st *HelmState) getChartDepsMutex(chartPath string) *sync.Mutex { + mu, ok := chartDepsMutexMap.Load(chartPath) + if ok { + return mu.(*sync.Mutex) + } + newMu := &sync.Mutex{} + actualMu, _ := chartDepsMutexMap.LoadOrStore(chartPath, newMu) + return actualMu.(*sync.Mutex) +} + // rewriteChartDependencies rewrites relative file:// dependencies in Chart.yaml to absolute paths // to ensure they can be resolved from chartify's temporary directory func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) { @@ -1428,9 +1439,13 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) return func() {}, nil } + mu := st.getChartDepsMutex(chartPath) + mu.Lock() + // Read Chart.yaml data, err := os.ReadFile(chartYamlPath) if err != nil { + mu.Unlock() return func() {}, err } @@ -1440,6 +1455,7 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) if err := os.WriteFile(chartYamlPath, originalContent, 0644); err != nil { st.logger.Warnf("Failed to restore original Chart.yaml at %s: %v", chartYamlPath, err) } + mu.Unlock() } // Parse Chart.yaml