diff --git a/pkg/filesystem/fs.go b/pkg/filesystem/fs.go index 89bd55ce..7b4f0dca 100644 --- a/pkg/filesystem/fs.go +++ b/pkg/filesystem/fs.go @@ -27,7 +27,10 @@ func (fs fileStat) Sys() any { return nil } type FileSystem struct { ReadFile func(string) ([]byte, error) ReadDir func(string) ([]fs.DirEntry, error) + WriteFile func(string, []byte, fs.FileMode) error DeleteFile func(string) error + MkdirTemp func(string, string) (string, error) + RemoveAll func(string) error FileExists func(string) (bool, error) Glob func(string) ([]string, error) FileExistsAt func(string) bool @@ -44,7 +47,10 @@ type FileSystem struct { func DefaultFileSystem() *FileSystem { dfs := FileSystem{ ReadDir: os.ReadDir, + WriteFile: os.WriteFile, DeleteFile: os.Remove, + MkdirTemp: os.MkdirTemp, + RemoveAll: os.RemoveAll, Glob: filepath.Glob, Getwd: os.Getwd, Chdir: os.Chdir, @@ -107,6 +113,15 @@ func FromFileSystem(params FileSystem) *FileSystem { if params.CopyDir != nil { dfs.CopyDir = params.CopyDir } + if params.WriteFile != nil { + dfs.WriteFile = params.WriteFile + } + if params.MkdirTemp != nil { + dfs.MkdirTemp = params.MkdirTemp + } + if params.RemoveAll != nil { + dfs.RemoveAll = params.RemoveAll + } return dfs } diff --git a/pkg/filesystem/fs_test.go b/pkg/filesystem/fs_test.go index 94219179..d84a03a2 100644 --- a/pkg/filesystem/fs_test.go +++ b/pkg/filesystem/fs_test.go @@ -252,4 +252,8 @@ func TestFs_DefaultBuilder(t *testing.T) { assert.NotNil(t, ffs.Chdir) assert.NotNil(t, ffs.Abs) assert.NotNil(t, ffs.EvalSymlinks) + assert.NotNil(t, ffs.WriteFile) + assert.NotNil(t, ffs.MkdirTemp) + assert.NotNil(t, ffs.RemoveAll) + assert.NotNil(t, ffs.CopyDir) } diff --git a/pkg/state/chart_dependencies_rewrite_test.go b/pkg/state/chart_dependencies_rewrite_test.go index fda69c33..d185b650 100644 --- a/pkg/state/chart_dependencies_rewrite_test.go +++ b/pkg/state/chart_dependencies_rewrite_test.go @@ -1,6 +1,7 @@ package state import ( + "fmt" "os" "path/filepath" "strings" @@ -67,12 +68,10 @@ dependencies: expectModified: true, expectError: false, validate: func(t *testing.T, modifiedChartYaml string) { - // Should have been converted to absolute path if strings.Contains(modifiedChartYaml, "file://../relative-chart") { t.Errorf("relative path should have been converted to absolute") } - // Should now have an absolute path if !strings.Contains(modifiedChartYaml, "file://") { t.Errorf("should still have file:// prefix") } @@ -100,22 +99,18 @@ dependencies: expectModified: true, expectError: false, validate: func(t *testing.T, modifiedChartYaml string) { - // HTTPS repo should remain unchanged if !strings.Contains(modifiedChartYaml, "https://charts.example.com") { t.Errorf("https repository should not be modified") } - // Relative path should be converted if strings.Contains(modifiedChartYaml, "file://../relative-chart") { t.Errorf("relative file:// path should have been converted") } - // Absolute path should remain unchanged if !strings.Contains(modifiedChartYaml, "file:///absolute/chart") { t.Errorf("absolute file:// path should not be modified") } - // OCI repo should remain unchanged if !strings.Contains(modifiedChartYaml, "oci://registry.example.com") { t.Errorf("oci repository should not be modified") } @@ -140,7 +135,6 @@ dependencies: expectModified: true, expectError: false, validate: func(t *testing.T, modifiedChartYaml string) { - // All relative paths should be converted if strings.Contains(modifiedChartYaml, "file://../chart1") || strings.Contains(modifiedChartYaml, "file://./chart2") || strings.Contains(modifiedChartYaml, "file://../../../chart3") { @@ -187,14 +181,12 @@ extra-field: aaa for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create temporary directory for test tempDir, err := os.MkdirTemp("", "helmfile-test-") if err != nil { t.Fatalf("failed to create temp dir: %v", err) } defer os.RemoveAll(tempDir) - // Create Chart.yaml if provided if tt.chartYaml != "" { chartYamlPath := filepath.Join(tempDir, "Chart.yaml") if err := os.WriteFile(chartYamlPath, []byte(tt.chartYaml), 0644); err != nil { @@ -202,22 +194,13 @@ extra-field: aaa } } - // Create HelmState with logger logger := zap.NewNop().Sugar() st := &HelmState{ logger: logger, fs: filesystem.DefaultFileSystem(), } - // Read original content if it exists - var originalContent []byte - chartYamlPath := filepath.Join(tempDir, "Chart.yaml") - if _, err := os.Stat(chartYamlPath); err == nil { - originalContent, _ = os.ReadFile(chartYamlPath) - } - - // Call rewriteChartDependencies - cleanup, err := st.rewriteChartDependencies(tempDir) + rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir) if tt.expectError { if err == nil { @@ -226,44 +209,39 @@ extra-field: aaa return } + if tt.expectModified { + if rewrittenPath == tempDir { + t.Errorf("expected rewrittenPath != tempDir when modifications are needed, got same path") + } + } else { + if rewrittenPath != tempDir { + t.Errorf("expected rewrittenPath == tempDir when no modifications are needed, got %q", rewrittenPath) + } + } + if err != nil { t.Fatalf("unexpected error: %v", err) } + defer cleanup() if tt.chartYaml == "" { return } - modifiedChartBytes, err := os.ReadFile(filepath.Join(tempDir, "Chart.yaml")) + modifiedChartBytes, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml")) if err != nil { t.Fatalf("failed to read Chart.yaml: %v", err) } modifiedChartYaml := string(modifiedChartBytes) - // Validate the modified Chart.yaml if tt.validate != nil { tt.validate(t, modifiedChartYaml) } - - // Call cleanup and verify restoration - cleanup() - - // Read restored content - restoredContent, err := os.ReadFile(chartYamlPath) - if err != nil { - t.Fatalf("failed to read restored Chart.yaml: %v", err) - } - - // Verify content was restored - if string(restoredContent) != string(originalContent) { - t.Errorf("cleanup did not restore original content\noriginal:\n%s\nrestored:\n%s", - string(originalContent), string(restoredContent)) - } }) } } -func TestRewriteChartDependencies_CleanupRestoresOriginal(t *testing.T) { +func TestRewriteChartDependencies_OriginalNotModified(t *testing.T) { tempDir, err := os.MkdirTemp("", "helmfile-test-") if err != nil { t.Fatalf("failed to create temp dir: %v", err) @@ -290,33 +268,44 @@ dependencies: fs: filesystem.DefaultFileSystem(), } - cleanup, err := st.rewriteChartDependencies(tempDir) + rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir) if err != nil { t.Fatalf("unexpected error: %v", err) } - // Verify modification happened - modifiedContent, err := os.ReadFile(chartYamlPath) + t.Cleanup(func() { + if rewrittenPath == tempDir { + return + } + + cleanup() + + if _, statErr := os.Stat(rewrittenPath); !os.IsNotExist(statErr) { + t.Errorf("expected rewritten chart path %q to be removed after cleanup", rewrittenPath) + } + }) + + if rewrittenPath == tempDir { + t.Errorf("expected a different path when modifications are needed, got same path") + } + + modifiedContent, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml")) if err != nil { t.Fatalf("failed to read modified Chart.yaml: %v", err) } if string(modifiedContent) == originalChart { - t.Errorf("Chart.yaml should have been modified") + t.Errorf("Chart.yaml in the copy should have been modified") } - // Call cleanup - cleanup() - - // Verify restoration - restoredContent, err := os.ReadFile(chartYamlPath) + originalContent, err := os.ReadFile(chartYamlPath) if err != nil { - t.Fatalf("failed to read restored Chart.yaml: %v", err) + t.Fatalf("failed to read original Chart.yaml: %v", err) } - if string(restoredContent) != originalChart { - t.Errorf("cleanup did not restore original content\nexpected:\n%s\ngot:\n%s", - originalChart, string(restoredContent)) + if string(originalContent) != originalChart { + t.Errorf("original Chart.yaml should not have been modified\nexpected:\n%s\ngot:\n%s", + originalChart, string(originalContent)) } } @@ -355,21 +344,30 @@ dependencies: fs: filesystem.DefaultFileSystem(), } - cleanup, err := st.rewriteChartDependencies(tempDir) + rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir) if err != nil { t.Fatalf("unexpected error: %v", err) } - defer cleanup() - // Read modified content - modifiedContent, err := os.ReadFile(chartYamlPath) + t.Cleanup(func() { + if rewrittenPath == tempDir { + return + } + + cleanup() + + if _, statErr := os.Stat(rewrittenPath); !os.IsNotExist(statErr) { + t.Errorf("expected rewritten chart path %q to be removed after cleanup", rewrittenPath) + } + }) + + modifiedContent, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml")) if err != nil { t.Fatalf("failed to read modified Chart.yaml: %v", err) } content := string(modifiedContent) - // Verify fields are preserved requiredFields := []string{ "apiVersion: v2", "name: test-chart", @@ -386,10 +384,17 @@ dependencies: } } - // Verify the dependency was rewritten if strings.Contains(content, "file://../relative-chart") { t.Errorf("relative path should have been converted to absolute") } + + originalContent, err := os.ReadFile(chartYamlPath) + if err != nil { + t.Fatalf("failed to read original Chart.yaml: %v", err) + } + if string(originalContent) != chartYaml { + t.Errorf("original Chart.yaml should not have been modified") + } } func TestRewriteChartDependencies_ErrorHandling(t *testing.T) { @@ -397,7 +402,6 @@ func TestRewriteChartDependencies_ErrorHandling(t *testing.T) { name string setupFunc func(tempDir string) error expectError bool - errorMsg string }{ { name: "invalid yaml in Chart.yaml", @@ -446,7 +450,7 @@ dependencies: fs: filesystem.DefaultFileSystem(), } - _, err = st.rewriteChartDependencies(tempDir) + _, _, err = st.rewriteChartDependencies(tempDir) if tt.expectError && err == nil { t.Errorf("expected error but got none") @@ -465,8 +469,6 @@ func TestRewriteChartDependencies_WindowsStylePath(t *testing.T) { } defer os.RemoveAll(tempDir) - // Test with backslash (Windows-style) paths - // Note: file:// URLs should use forward slashes, but test handling of edge cases chartYaml := `apiVersion: v2 name: test-chart version: 1.0.0 @@ -487,20 +489,29 @@ dependencies: fs: filesystem.DefaultFileSystem(), } - cleanup, err := st.rewriteChartDependencies(tempDir) + rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir) if err != nil { t.Fatalf("unexpected error: %v", err) } - defer cleanup() - // Should handle the path correctly - data, err := os.ReadFile(chartYamlPath) + t.Cleanup(func() { + if rewrittenPath == tempDir { + return + } + + cleanup() + + if _, statErr := os.Stat(rewrittenPath); !os.IsNotExist(statErr) { + t.Errorf("expected rewritten chart path %q to be removed after cleanup", rewrittenPath) + } + }) + + data, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml")) if err != nil { t.Fatalf("failed to read Chart.yaml: %v", err) } content := string(data) - // The relative path should have been converted to absolute if strings.Contains(content, "file://./subdir/chart") { t.Errorf("relative path with ./ should have been converted") } @@ -527,9 +538,6 @@ dependencies: t.Fatalf("failed to write Chart.yaml: %v", err) } - // Run multiple goroutines concurrently on the same chart path. - // A readiness WaitGroup ensures all goroutines are blocked before the start barrier is released, - // so calls to rewriteChartDependencies overlap as much as possible. numGoroutines := 10 var wg sync.WaitGroup var readyWg sync.WaitGroup @@ -542,7 +550,6 @@ dependencies: go func() { defer wg.Done() - // Signal that this goroutine is ready, then wait for the start signal. readyWg.Done() <-ready @@ -552,16 +559,44 @@ dependencies: fs: filesystem.DefaultFileSystem(), } - cleanup, err := st.rewriteChartDependencies(tempDir) + rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir) if err != nil { errCh <- err return } defer cleanup() + + data, readErr := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml")) + if readErr != nil { + errCh <- readErr + return + } + + type ChartDependency struct { + Name string `yaml:"name"` + Repository string `yaml:"repository"` + Version string `yaml:"version"` + } + type ChartMeta struct { + APIVersion string `yaml:"apiVersion"` + Name string `yaml:"name"` + Version string `yaml:"version"` + Dependencies []ChartDependency `yaml:"dependencies,omitempty"` + } + + var meta ChartMeta + if unmarshalErr := yaml.Unmarshal(data, &meta); unmarshalErr != nil { + errCh <- unmarshalErr + return + } + + if meta.Name != "test-chart" { + errCh <- fmt.Errorf("expected chart name 'test-chart', got %q", meta.Name) + return + } }() } - // Wait until all goroutines are ready, then release them simultaneously. readyWg.Wait() close(ready) @@ -572,10 +607,9 @@ dependencies: t.Errorf("goroutine error: %v", err) } - // Verify Chart.yaml is still valid by unmarshaling and checking expected fields data, err := os.ReadFile(chartYamlPath) if err != nil { - t.Fatalf("failed to read Chart.yaml: %v", err) + t.Fatalf("failed to read original Chart.yaml: %v", err) } type ChartDependency struct { @@ -592,21 +626,22 @@ dependencies: var chartMeta ChartMeta if err := yaml.Unmarshal(data, &chartMeta); err != nil { - t.Fatalf("Chart.yaml is not valid YAML after concurrent rewrites: %v", err) + t.Fatalf("original Chart.yaml is not valid YAML: %v", err) } if chartMeta.Name != "test-chart" { - t.Errorf("expected chart name 'test-chart', got %q", chartMeta.Name) + t.Errorf("expected original chart name 'test-chart', got %q", chartMeta.Name) } - if len(chartMeta.Dependencies) != 1 { - t.Fatalf("expected 1 dependency, got %d", len(chartMeta.Dependencies)) + if len(chartMeta.Dependencies) == 0 { + t.Fatalf("expected original Chart.yaml to contain at least one dependency, got %d", len(chartMeta.Dependencies)) } - if chartMeta.Dependencies[0].Name != "dep1" { - t.Errorf("expected dependency name 'dep1', got %q", chartMeta.Dependencies[0].Name) + + const wantDependencyName = "dep1" + if chartMeta.Dependencies[0].Name != wantDependencyName { + t.Errorf("expected first dependency name %q, got %q", wantDependencyName, chartMeta.Dependencies[0].Name) } - // The cleanup from each goroutine must have restored the original relative path. const wantRepository = "file://../relative-chart" if chartMeta.Dependencies[0].Repository != wantRepository { - t.Errorf("expected dependency repository %q (original relative path), got %q", wantRepository, chartMeta.Dependencies[0].Repository) + t.Errorf("expected original dependency repository %q, got %q", wantRepository, chartMeta.Dependencies[0].Repository) } } diff --git a/pkg/state/state.go b/pkg/state/state.go index fe4e6452..f51a0dd1 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1409,30 +1409,26 @@ type PrepareChartKey struct { // rewriteChartDependencies rewrites relative file:// dependencies in Chart.yaml to absolute paths // to ensure they can be resolved from chartify's temporary directory. -// When the file is actually rewritten the returned cleanup function restores the original content -// and releases the per-chart mutex; when no rewrite is needed a no-op cleanup is returned and the -// mutex is never held past this call. -func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) { +// Instead of modifying the original chart in-place (which causes race conditions when multiple +// releases reference the same local chart), it creates a temporary copy only when a rewrite is +// needed and rewrites that copy. When a temp copy is created, it reflects the current chart +// contents so prepare hooks or other steps that mutate the local chart directory are honored. +// The returned cleanup function removes the temporary directory when one was created and is +// otherwise a no-op. +func (st *HelmState) rewriteChartDependencies(chartPath string) (string, func(), error) { chartYamlPath := filepath.Join(chartPath, "Chart.yaml") - // Check if Chart.yaml exists - if _, err := os.Stat(chartYamlPath); os.IsNotExist(err) { - return func() {}, nil + if _, err := st.fs.Stat(chartYamlPath); os.IsNotExist(err) { + return chartPath, func() {}, nil + } else if err != nil { + return chartPath, func() {}, err } - // Reuse the existing per-path RWMutex for exclusive access to the chart directory. - mu := st.getNamedRWMutex(chartPath) - mu.Lock() - - // Read Chart.yaml - data, err := os.ReadFile(chartYamlPath) + data, err := st.fs.ReadFile(chartYamlPath) if err != nil { - mu.Unlock() - return func() {}, err + return chartPath, func() {}, err } - originalContent := data - // Parse Chart.yaml type ChartDependency struct { Name string `yaml:"name"` Repository string `yaml:"repository"` @@ -1444,26 +1440,21 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) } var chartMeta ChartMeta - if err := yaml.Unmarshal(originalContent, &chartMeta); err != nil { - mu.Unlock() - return func() {}, err + if err := yaml.Unmarshal(data, &chartMeta); err != nil { + return chartPath, func() {}, err } - // Rewrite relative file:// dependencies to absolute paths modified := false for i := range chartMeta.Dependencies { dep := &chartMeta.Dependencies[i] if strings.HasPrefix(dep.Repository, "file://") { relPath := strings.TrimPrefix(dep.Repository, "file://") - // Check if it's a relative path if !filepath.IsAbs(relPath) { - // Convert to absolute path relative to the chart directory absPath := filepath.Join(chartPath, relPath) absPath, err = filepath.Abs(absPath) if err != nil { - mu.Unlock() - return func() {}, fmt.Errorf("failed to resolve absolute path for dependency %s: %w", dep.Name, err) + return chartPath, func() {}, fmt.Errorf("failed to resolve absolute path for dependency %s: %w", dep.Name, err) } st.logger.Debugf("Rewriting Chart dependency %s from %s to file://%s", dep.Name, dep.Repository, absPath) @@ -1473,37 +1464,44 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) } } - // If nothing changed, release the lock immediately. Chart.yaml has already been read and - // unmarshaled above, but no file write is needed and the lock is not held beyond this function. if !modified { - mu.Unlock() - return func() {}, nil + return chartPath, func() {}, nil } updatedData, err := yaml.Marshal(&chartMeta) if err != nil { - mu.Unlock() - return func() {}, fmt.Errorf("failed to marshal Chart.yaml: %w", err) + return chartPath, func() {}, fmt.Errorf("failed to marshal Chart.yaml: %w", err) } - if err := os.WriteFile(chartYamlPath, updatedData, 0644); err != nil { - // File may be truncated/partially written; restore original content before releasing the lock. - if restoreErr := os.WriteFile(chartYamlPath, originalContent, 0644); restoreErr != nil { - st.logger.Warnf("Failed to restore Chart.yaml at %s after write error (%v): %v", chartYamlPath, err, restoreErr) - } - mu.Unlock() - return func() {}, fmt.Errorf("failed to write Chart.yaml: %w", err) + tempDir, err := st.fs.MkdirTemp("", "chart-deps-rewrite-*") + if err != nil { + return chartPath, func() {}, fmt.Errorf("failed to create temp directory for chart rewrite: %w", err) } - st.logger.Debugf("Rewrote Chart.yaml with absolute dependency paths at %s", chartYamlPath) - - // Return cleanup that restores original content and releases the lock. - return func() { - if restoreErr := os.WriteFile(chartYamlPath, originalContent, 0644); restoreErr != nil { - st.logger.Warnf("Failed to restore original Chart.yaml at %s: %v", chartYamlPath, restoreErr) + if err := st.fs.CopyDir(chartPath, tempDir); err != nil { + if removeErr := st.fs.RemoveAll(tempDir); removeErr != nil { + st.logger.Warnf("Failed to remove temp chart directory %s: %v", tempDir, removeErr) } - mu.Unlock() - }, nil + return chartPath, func() {}, fmt.Errorf("failed to copy chart to temp directory: %w", err) + } + + tempChartYamlPath := filepath.Join(tempDir, "Chart.yaml") + if err := st.fs.WriteFile(tempChartYamlPath, updatedData, 0644); err != nil { + if removeErr := st.fs.RemoveAll(tempDir); removeErr != nil { + st.logger.Warnf("Failed to remove temp chart directory %s: %v", tempDir, removeErr) + } + return chartPath, func() {}, fmt.Errorf("failed to write Chart.yaml: %w", err) + } + + st.logger.Debugf("Rewrote Chart.yaml with absolute dependency paths at %s", tempChartYamlPath) + + cleanup := func() { + if removeErr := st.fs.RemoveAll(tempDir); removeErr != nil { + st.logger.Warnf("Failed to remove temp chart directory %s: %v", tempDir, removeErr) + } + } + + return tempDir, cleanup, nil } // Otherwise, if a chart is not a helm chart, it will call "chartify" to turn it into a chart. @@ -1515,11 +1513,12 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re // This prevents errors like "Error: directory /tmp/chartify.../argocd-application not found" // when Chart.yaml contains dependencies like "file://../argocd-application" if st.fs.DirectoryExistsAt(chartPath) { - restoreChart, err := st.rewriteChartDependencies(chartPath) + rewrittenPath, cleanupTempChart, err := st.rewriteChartDependencies(chartPath) if err != nil { return "", false, fmt.Errorf("failed to rewrite chart dependencies: %w", err) } - defer restoreChart() + chartPath = rewrittenPath + defer cleanupTempChart() } c := chartify.New( diff --git a/test/integration/run.sh b/test/integration/run.sh index a7116e8f..6b3cd92a 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -96,6 +96,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes # TEST CASES---------------------------------------------------------------------------------------------------------- +. ${dir}/test-cases/issue-2502-race-condition-local-chart.sh . ${dir}/test-cases/chart-deps-condition.sh . ${dir}/test-cases/fetch-forl-local-chart.sh . ${dir}/test-cases/suppress-output-line-regex.sh diff --git a/test/integration/test-cases/issue-2502-race-condition-local-chart.sh b/test/integration/test-cases/issue-2502-race-condition-local-chart.sh new file mode 100644 index 00000000..544b8434 --- /dev/null +++ b/test/integration/test-cases/issue-2502-race-condition-local-chart.sh @@ -0,0 +1,50 @@ +# Integration test for issue #2502: Race condition when multiple releases share a local chart +# https://github.com/helmfile/helmfile/issues/2502 +# +# When multiple releases reference the same local chart, concurrent goroutines +# race on rewriting Chart.yaml dependencies, causing: +# "Error: validation: chart.metadata.name is required" +# +# This test verifies the fix works WITHOUT --concurrency 1 workaround. + +issue_2502_input_dir="${cases_dir}/issue-2502-race-condition-local-chart/input" + +issue_2502_tmp=$(mktemp -d) +actual="${issue_2502_tmp}/actual.yaml" + +cleanup_issue_2502() { + if [ -n "${issue_2502_tmp}" ] && [ -d "${issue_2502_tmp}" ]; then + rm -rf "${issue_2502_tmp}" + fi +} +trap cleanup_issue_2502 EXIT + +test_start "issue #2502: race condition with shared local chart" + +info "Running helmfile template with 5 releases sharing the same local chart (default concurrency)" + +# Run WITHOUT --concurrency 1 to test the fix. +# Before the fix, this would intermittently fail with: +# "Error: validation: chart.metadata.name is required" +# Run multiple iterations to increase chance of catching a race. +pass=0 +iterations=5 +for i in $(seq 1 ${iterations}); do + if ${helmfile} -f ${issue_2502_input_dir}/helmfile.yaml -e test template > ${actual} 2>&1; then + pass=$((pass + 1)) + else + cat ${actual} + fail "helmfile template failed on iteration ${i}/${iterations} (race condition on shared local chart)" + fi +done + +if [ ${pass} -ne ${iterations} ]; then + fail "Expected ${iterations}/${iterations} passes but got ${pass}/${iterations}" +fi + +info "All ${iterations} iterations passed successfully" + +cleanup_issue_2502 +trap - EXIT + +test_pass "issue #2502: race condition with shared local chart" diff --git a/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/Chart.yaml b/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/Chart.yaml new file mode 100644 index 00000000..665abaf0 --- /dev/null +++ b/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/Chart.yaml @@ -0,0 +1,10 @@ +dependencies: + - name: raw + repository: file://../../../../../charts/raw + version: 0.0.1 +apiVersion: v2 +appVersion: "1.0.0" +description: A test chart for race condition reproduction +name: my-chart +type: application +version: 0.1.0 diff --git a/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/values.yaml b/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/values.yaml new file mode 100644 index 00000000..20aeae25 --- /dev/null +++ b/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/values.yaml @@ -0,0 +1,9 @@ +raw: + templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: test-cm + data: + key: value diff --git a/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helmfile.yaml b/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helmfile.yaml new file mode 100644 index 00000000..705fed2a --- /dev/null +++ b/test/integration/test-cases/issue-2502-race-condition-local-chart/input/helmfile.yaml @@ -0,0 +1,49 @@ +environments: + test: {} +--- +releases: + - name: app + chart: helm/my-chart + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: test-cm + data: + key: value + - name: app-2 + chart: helm/my-chart + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: test-cm + data: + key: value + - name: app-3 + chart: helm/my-chart + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: test-cm + data: + key: value + - name: app-4 + chart: helm/my-chart + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: test-cm + data: + key: value + - name: app-5 + chart: helm/my-chart + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: test-cm + data: + key: value