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>

pkg/state/chart_dependencies_rewrite_test.go

@@ -10,6 +10,7 @@ import (
 	"go.uber.org/zap"
 
 	"github.com/helmfile/helmfile/pkg/filesystem"
+	"github.com/helmfile/helmfile/pkg/yaml"
 )
 
 func TestRewriteChartDependencies(t *testing.T) {
@@ -558,14 +559,39 @@ dependencies:
 		t.Errorf("goroutine error: %v", err)
 	}
 
-	// Verify Chart.yaml is still valid
+	// 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)
 	}
 
-	// Should still have the dependencies section
-	if !strings.Contains(string(data), "dependencies:") {
-		t.Errorf("Chart.yaml should still have dependencies section")
+	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 chartMeta ChartMeta
+	if err := yaml.Unmarshal(data, &chartMeta); err != nil {
+		t.Fatalf("Chart.yaml is not valid YAML after concurrent rewrites: %v", err)
+	}
+
+	if chartMeta.Name != "test-chart" {
+		t.Errorf("expected chart name 'test-chart', got %q", chartMeta.Name)
+	}
+	if len(chartMeta.Dependencies) != 1 {
+		t.Fatalf("expected 1 dependency, got %d", len(chartMeta.Dependencies))
+	}
+	if chartMeta.Dependencies[0].Name != "dep1" {
+		t.Errorf("expected dependency name 'dep1', got %q", chartMeta.Dependencies[0].Name)
+	}
+	if !strings.HasPrefix(chartMeta.Dependencies[0].Repository, "file://") {
+		t.Errorf("expected dependency repository to start with 'file://', got %q", chartMeta.Dependencies[0].Repository)
 	}
 }

pkg/state/state.go

@@ -1471,7 +1471,8 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error)
 
 	var chartMeta ChartMeta
 	if err := yaml.Unmarshal(data, &chartMeta); err != nil {
-		return cleanup, err
+		mu.Unlock()
+		return func() {}, err
 	}
 
 	// Rewrite relative file:// dependencies to absolute paths
@@ -1487,7 +1488,8 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error)
 				absPath := filepath.Join(chartPath, relPath)
 				absPath, err = filepath.Abs(absPath)
 				if err != nil {
-					return cleanup, fmt.Errorf("failed to resolve absolute path for dependency %s: %w", dep.Name, err)
+					mu.Unlock()
+					return 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)
@@ -1501,11 +1503,13 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error)
 	if modified {
 		updatedData, err := yaml.Marshal(&chartMeta)
 		if err != nil {
-			return cleanup, fmt.Errorf("failed to marshal Chart.yaml: %w", err)
+			mu.Unlock()
+			return func() {}, fmt.Errorf("failed to marshal Chart.yaml: %w", err)
 		}
 
 		if err := os.WriteFile(chartYamlPath, updatedData, 0644); err != nil {
-			return cleanup, fmt.Errorf("failed to write Chart.yaml: %w", err)
+			mu.Unlock()
+			return func() {}, fmt.Errorf("failed to write Chart.yaml: %w", err)
 		}
 
 		st.logger.Debugf("Rewrote Chart.yaml with absolute dependency paths at %s", chartYamlPath)