diff --git a/pkg/state/state.go b/pkg/state/state.go index 9c501774..de31e035 100644 --- a/pkg/state/state.go +++ b/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 { diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index c0262e61..a2fbc9d7 100644 --- a/pkg/state/state_test.go +++ b/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