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>
This commit is contained in:
parent
eaf006a386
commit
9b8aa46faa
|
|
@ -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")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue