fix: ensure plain http is supported across all helmfile commands (#1858)

fix: ensure plain http is supported across helmfile commands

Signed-off-by: purpleclay <purpleclaygh@gmail.com>

pkg/state/state.go

@@ -2665,26 +2665,46 @@ func (st *HelmState) kubeConnectionFlags(release *ReleaseSpec) []string {
 }
 
 func (st *HelmState) appendChartDownloadFlags(flags []string, release *ReleaseSpec) []string {
-	var repoSkipTLSVerify, repoPlainHttp bool
 	repo, _ := st.GetRepositoryAndNameFromChartName(release.Chart)
-	if repo != nil {
-		repoPlainHttp = repo.PlainHttp
-		repoSkipTLSVerify = repo.SkipTLSVerify
-	}
-
-	if release.PlainHttp || st.HelmDefaults.PlainHttp || repoPlainHttp {
+	if st.needsPlainHttp(release, repo) {
 		flags = append(flags, "--plain-http")
 		// --insecure-skip-tls-verify nullifies --plain-http in helm, omit it if PlainHttp is specified
 		return flags
 	}
 
-	if release.InsecureSkipTLSVerify || st.HelmDefaults.InsecureSkipTLSVerify || repoSkipTLSVerify {
+	if st.needsInsecureSkipTLSVerify(release, repo) {
 		flags = append(flags, "--insecure-skip-tls-verify")
 	}
 
 	return flags
 }
 
+func (st *HelmState) needsPlainHttp(release *ReleaseSpec, repo *RepositorySpec) bool {
+	var repoPlainHttp, relPlainHttp bool
+	if repo != nil {
+		repoPlainHttp = repo.PlainHttp
+	}
+
+	if release != nil {
+		relPlainHttp = release.PlainHttp
+	}
+
+	return relPlainHttp || st.HelmDefaults.PlainHttp || repoPlainHttp
+}
+
+func (st *HelmState) needsInsecureSkipTLSVerify(release *ReleaseSpec, repo *RepositorySpec) bool {
+	var repoSkipTLSVerify, relSkipTLSVerify bool
+	if repo != nil {
+		repoSkipTLSVerify = repo.SkipTLSVerify
+	}
+
+	if release != nil {
+		relSkipTLSVerify = release.InsecureSkipTLSVerify
+	}
+
+	return relSkipTLSVerify || st.HelmDefaults.InsecureSkipTLSVerify || repoSkipTLSVerify
+}
+
 func (st *HelmState) timeoutFlags(release *ReleaseSpec) []string {
 	var flags []string
 
@@ -2848,11 +2868,21 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec,
 	// `helm template --validate` and `helm upgrade --dry-run` ignore `--kube-version` flag.
 	// For the moment, not specifying kubeVersion.
 	flags = st.appendApiVersionsFlags(flags, release, "")
-
 	flags = st.appendConnectionFlags(flags, release)
-
 	flags = st.appendChartDownloadFlags(flags, release)
 
+	// `helm diff` does not support the `--plain-http` flag, this needs to be removed
+	repo, _ := st.GetRepositoryAndNameFromChartName(release.Chart)
+	if st.needsPlainHttp(release, repo) {
+		var cleanFlags []string
+		for _, flag := range flags {
+			if flag != "--plain-http" {
+				cleanFlags = append(cleanFlags, flag)
+			}
+		}
+		flags = cleanFlags
+	}
+
 	for _, flag := range flags {
 		if flag == "--insecure-skip-tls-verify" {
 			diffVersion, err := helmexec.GetPluginVersion("diff", settings.PluginsDirectory)
@@ -3847,10 +3877,9 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm
 		st.logger.Debugf("chart already exists at %s", chartPath)
 	} else {
 		flags := st.chartOCIFlags(release)
-
-		// apprnd flags about keyring and verify
 		flags = st.appendVerifyFlags(flags, release)
 		flags = st.appendKeyringFlags(flags, release)
+		flags = st.appendChartDownloadFlags(flags, release)
 
 		err := helm.ChartPull(qualifiedChartName, chartPath, flags...)
 		if err != nil {

pkg/state/state_test.go

@@ -1871,6 +1871,19 @@ func TestHelmState_DiffFlags(t *testing.T) {
 			helm:          &exectest.Helm{},
 			wantDiffFlags: []string{"--api-versions", "helmfile.test/v1", "--api-versions", "helmfile.test/v2", "--kube-version", "1.21"},
 		},
+		{
+			name: "release with kubeversion and plain http which is ignored",
+			releases: []ReleaseSpec{
+				{
+					Name:        "releaseName",
+					Chart:       "foo",
+					KubeVersion: "1.21",
+					PlainHttp:   true,
+				},
+			},
+			helm:          &exectest.Helm{},
+			wantDiffFlags: []string{"--kube-version", "1.21"},
+		},
 	}
 	for i := range tests {
 		tt := tests[i]
@@ -3582,6 +3595,98 @@ func TestAppendChartDownloadFlags(t *testing.T) {
 	}
 }
 
+func TestNeedsPlainHttp(t *testing.T) {
+	tests := []struct {
+		name     string
+		release  *ReleaseSpec
+		repo     *RepositorySpec
+		defaults HelmSpec
+		expected bool
+	}{
+		{
+			name: "PlainHttp in Release",
+			release: &ReleaseSpec{
+				PlainHttp: true,
+			},
+			expected: true,
+		},
+		{
+			name: "PlainHttp in Repository",
+			repo: &RepositorySpec{
+				PlainHttp: true,
+			},
+			expected: true,
+		},
+		{
+			name: "PlainHttp in HelmDefaults",
+			defaults: HelmSpec{
+				PlainHttp: true,
+			},
+			expected: true,
+		},
+		{
+			name:     "PlainHttp not set",
+			expected: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st := &HelmState{
+				ReleaseSetSpec: ReleaseSetSpec{
+					HelmDefaults: tt.defaults,
+				},
+			}
+			require.Equal(t, tt.expected, st.needsPlainHttp(tt.release, tt.repo))
+		})
+	}
+}
+
+func TestNeedsInsecureSkipTLSVerify(t *testing.T) {
+	tests := []struct {
+		name     string
+		release  *ReleaseSpec
+		repo     *RepositorySpec
+		defaults HelmSpec
+		expected bool
+	}{
+		{
+			name: "InsecureSkipTLSVerify in Release",
+			release: &ReleaseSpec{
+				InsecureSkipTLSVerify: true,
+			},
+			expected: true,
+		},
+		{
+			name: "SkipTLSVerify in Repository",
+			repo: &RepositorySpec{
+				SkipTLSVerify: true,
+			},
+			expected: true,
+		},
+		{
+			name: "InsecureSkipTLSVerify in HelmDefaults",
+			defaults: HelmSpec{
+				InsecureSkipTLSVerify: true,
+			},
+			expected: true,
+		},
+		{
+			name:     "InsecureSkipTLSVerify not set",
+			expected: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st := &HelmState{
+				ReleaseSetSpec: ReleaseSetSpec{
+					HelmDefaults: tt.defaults,
+				},
+			}
+			require.Equal(t, tt.expected, st.needsInsecureSkipTLSVerify(tt.release, tt.repo))
+		})
+	}
+}
+
 func TestHideChartURL(t *testing.T) {
 	tests := []struct {
 		input    string