From e452050dc040614daac9a8c21aca78f7b252d471 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 23 Aug 2025 11:37:39 +0000
Subject: [PATCH] Fix error propagation in helmfile diff when Kubernetes is
 unreachable

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
---
 pkg/state/diff_error_propagation_test.go | 67 ++++++++++++++++++++++++
 pkg/state/state.go                       | 37 ++++++++-----
 2 files changed, 92 insertions(+), 12 deletions(-)
 create mode 100644 pkg/state/diff_error_propagation_test.go

diff --git a/pkg/state/diff_error_propagation_test.go b/pkg/state/diff_error_propagation_test.go
new file mode 100644
index 00000000..a40d85d5
--- /dev/null
+++ b/pkg/state/diff_error_propagation_test.go
@@ -0,0 +1,67 @@
+package state
+
+import (
+	"errors"
+	"sync"
+	"testing"
+
+	"go.uber.org/zap"
+
+	"github.com/helmfile/helmfile/pkg/exectest"
+	"github.com/helmfile/helmfile/pkg/helmexec"
+)
+
+func TestIsReleaseInstalled_HandlesConnectionError(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+	
+	state := &HelmState{
+		logger: logger,
+	}
+
+	// Create a custom helm mock that fails on List operations 
+	helm := &CustomFailingHelm{
+		Helm: &exectest.Helm{
+			DiffMutex:     &sync.Mutex{},
+			ChartsMutex:   &sync.Mutex{},
+			ReleasesMutex: &sync.Mutex{},
+			Helm3:         true,
+		},
+	}
+
+	release := ReleaseSpec{
+		Name:      "test-release",
+		Chart:     "test/chart",
+		Namespace: "default",
+	}
+
+	// This should return an error due to connection failure
+	_, err := state.isReleaseInstalled(helmexec.HelmContext{}, helm, release)
+
+	// Verify that error was propagated
+	if err == nil {
+		t.Fatalf("expected isReleaseInstalled to return error when Kubernetes is unreachable, but got no error")
+	}
+
+	if err.Error() == "" {
+		t.Fatalf("expected isReleaseInstalled to return meaningful error when Kubernetes is unreachable, but got empty error")
+	}
+
+	// Check if the error contains the expected message
+	expectedMsg := "Kubernetes cluster unreachable"
+	if err.Error() != expectedMsg && !contains(err.Error(), "Kubernetes cluster unreachable") {
+		t.Fatalf("expected error to contain 'Kubernetes cluster unreachable', but got: %v", err.Error())
+	}
+}
+
+// CustomFailingHelm wraps exectest.Helm and overrides List to simulate failures
+type CustomFailingHelm struct {
+	*exectest.Helm
+}
+
+func (h *CustomFailingHelm) List(context helmexec.HelmContext, filter string, flags ...string) (string, error) {
+	return "", errors.New("Kubernetes cluster unreachable: Get \"http://localhost:8080/version\": dial tcp [::1]:8080: connect: connection refused")
+}
+
+func contains(s, substr string) bool {
+	return len(s) >= len(substr) && s[:len(substr)] == substr || (len(s) > len(substr) && contains(s[1:], substr))
+}
\ No newline at end of file
diff --git a/pkg/state/state.go b/pkg/state/state.go
index c45b94db..19d576d7 100644
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@@ -1887,7 +1887,7 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 	mu := &sync.RWMutex{}
 	installedReleases := map[string]bool{}
 
-	isInstalled := func(r *ReleaseSpec) bool {
+	isInstalled := func(r *ReleaseSpec) (bool, error) {
 		id := ReleaseToID(r)
 
 		mu.RLock()
@@ -1895,19 +1895,19 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 		mu.RUnlock()
 
 		if ok {
-			return v
+			return v, nil
 		}
 
 		v, err := st.isReleaseInstalled(st.createHelmContext(r, 0), helm, *r)
 		if err != nil {
-			st.logger.Warnf("confirming if the release is already installed or not: %v", err)
-		} else {
-			mu.Lock()
-			installedReleases[id] = v
-			mu.Unlock()
+			return false, err
 		}
 
-		return v
+		mu.Lock()
+		installedReleases[id] = v
+		mu.Unlock()
+
+		return v, nil
 	}
 
 	releases := []*ReleaseSpec{}
@@ -1952,12 +1952,25 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 					suppressDiff = true
 				}
 
-				if opt.SkipDiffOnInstall && !isInstalled(release) {
-					results <- diffPrepareResult{release: release, upgradeDueToSkippedDiff: true, suppressDiff: suppressDiff}
-					continue
+				if opt.SkipDiffOnInstall {
+					installed, err := isInstalled(release)
+					if err != nil {
+						errs = append(errs, err)
+					} else if !installed {
+						results <- diffPrepareResult{release: release, upgradeDueToSkippedDiff: true, suppressDiff: suppressDiff}
+						continue
+					}
 				}
 
-				disableValidation := release.DisableValidationOnInstall != nil && *release.DisableValidationOnInstall && !isInstalled(release)
+				var disableValidation bool
+				if release.DisableValidationOnInstall != nil && *release.DisableValidationOnInstall {
+					installed, err := isInstalled(release)
+					if err != nil {
+						errs = append(errs, err)
+					} else {
+						disableValidation = !installed
+					}
+				}
 
 				// TODO We need a long-term fix for this :)
 				// See https://github.com/roboll/helmfile/issues/737