feat: adding ability for for charts to be pulled with plain HTTP (#1672)

* eat: adding ability for for charts to be pulled without HTTPS

accomplished by:

- Adding PlainHttp attribute to RepositorySpec., HelmDefault, ReleaseSpec
- Adding UnitTests for getOCIChart Flags.
- Adding funciton and unitTests for getChartDownload
- Changing and refactoring how Flags are added to getOCIChart.

Resolves #1224

Signed-off-by: Peter Halliday <peter.halliday@servicenow.com>

* Pass PlainHttp to OCI repo options, fix unit test

Signed-off-by: Pascal Rivard <privard@rbbn.com>

* Fix doc

Signed-off-by: Pascal Rivard <privard@rbbn.com>

* Use repository fields in non-OCI chart download options

Signed-off-by: Pascal Rivard <privard@rbbn.com>

* Update hashes in TestGenerateID

Signed-off-by: Pascal Rivard <privard@rbbn.com>

* Make sure repo exists when using its options

Signed-off-by: Pascal Rivard <privard@rbbn.com>

* Do not add TLS options if PlainHttp is set, adapt unit tests

Signed-off-by: Pascal Rivard <privard@rbbn.com>

* Fix doc

Signed-off-by: Pascal Rivard <privard@rbbn.com>

* Remove 'else if' from appendChartDownloadFlags

Signed-off-by: Pascal Rivard <privard@rbbn.com>

---------

Signed-off-by: Peter Halliday <peter.halliday@servicenow.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Co-authored-by: Peter Halliday <peter.halliday@servicenow.com>
Co-authored-by: Pascal Rivard <privard@rbbn.com>

docs/index.md

@@ -161,6 +161,10 @@ repositories:
 - name: skipTLS
   url: https://ss.my-insecure-domain.com
   skipTLSVerify: true
+# Advanced configuration: Connect to a repo served over plain http
+- name: plainHTTP
+  url: http://just.http.domain.com
+  plainHttp: true
 
 # context: kube-context # this directive is deprecated, please consider using helmDefaults.kubeContext
 
@@ -223,6 +227,8 @@ helmDefaults:
   cascade: "background"
   # insecureSkipTLSVerify is true if the TLS verification should be skipped when fetching remote chart
   insecureSkipTLSVerify: false
+  # plainHttp is true if fetching the remote chart should be done using HTTP
+  plainHttp: false
   # --wait flag for destroy/delete, if set to true, will wait until all resources are deleted before mark delete command as successful
   deleteWait: false
   # Timeout is the time in seconds to wait for helmfile destroy/delete (default 300)
@@ -339,6 +345,8 @@ releases:
     cascade: "background"
     # insecureSkipTLSVerify is true if the TLS verification should be skipped when fetching remote chart
     insecureSkipTLSVerify: false
+    # plainHttp is true if fetching the remote chart should be done using HTTP
+    plainHttp: false
     # suppressDiff skip the helm diff output. Useful for charts which produces large not helpful diff, default: false
     suppressDiff: false
     # suppressOutputLineRegex is a list of regex patterns to suppress output lines from helm diff (default []), available in helmfile v0.162.0 

pkg/state/state.go

@@ -199,6 +199,8 @@ type HelmSpec struct {
 	DisableOpenAPIValidation *bool `yaml:"disableOpenAPIValidation,omitempty"`
 	// InsecureSkipTLSVerify is true if the TLS verification should be skipped when fetching remote chart
 	InsecureSkipTLSVerify bool `yaml:"insecureSkipTLSVerify,omitempty"`
+	// PlainHttp is true if the remote charte should be fetched using HTTP and not HTTPS
+	PlainHttp bool `yaml:"plainHttp,omitempty"`
 	// Wait, if set to true, will wait until all resources are deleted before mark delete command as successful
 	DeleteWait bool `yaml:"deleteWait"`
 	// Timeout is the time in seconds to wait for helmfile delete command (default 300)
@@ -221,6 +223,7 @@ type RepositorySpec struct {
 	Keyring         string `yaml:"keyring,omitempty"`
 	PassCredentials bool   `yaml:"passCredentials,omitempty"`
 	SkipTLSVerify   bool   `yaml:"skipTLSVerify,omitempty"`
+	PlainHttp       bool   `yaml:"plainHttp,omitempty"`
 }
 
 type Inherit struct {
@@ -328,6 +331,9 @@ type ReleaseSpec struct {
 	// InsecureSkipTLSVerify is true if the TLS verification should be skipped when fetching remote chart.
 	InsecureSkipTLSVerify bool `yaml:"insecureSkipTLSVerify,omitempty"`
 
+	// PlainHttp is true if the remote charte should be fetched using HTTP and not HTTPS
+	PlainHttp bool `yaml:"plainHttp,omitempty"`
+
 	// These values are used in templating
 	VerifyTemplate    *string `yaml:"verifyTemplate,omitempty"`
 	WaitTemplate      *string `yaml:"waitTemplate,omitempty"`
@@ -2194,7 +2200,7 @@ func (st *HelmState) TestReleases(helm helmexec.Interface, cleanup bool, timeout
 		}
 
 		flags = st.appendConnectionFlags(flags, &release)
-		flags = st.appendChartDownloadTLSFlags(flags, &release)
+		flags = st.appendChartDownloadFlags(flags, &release)
 
 		return helm.TestRelease(st.createHelmContext(&release, workerIndex), release.Name, flags...)
 	})
@@ -2597,13 +2603,24 @@ func (st *HelmState) kubeConnectionFlags(release *ReleaseSpec) []string {
 	return flags
 }
 
-func (st *HelmState) appendChartDownloadTLSFlags(flags []string, release *ReleaseSpec) []string {
+func (st *HelmState) appendChartDownloadFlags(flags []string, release *ReleaseSpec) []string {
-	switch {
+	var repoSkipTLSVerify, repoPlainHttp bool
-	case release.InsecureSkipTLSVerify:
+	repo, _ := st.GetRepositoryAndNameFromChartName(release.Chart)
-		flags = append(flags, "--insecure-skip-tls-verify")
+	if repo != nil {
-	case st.HelmDefaults.InsecureSkipTLSVerify:
+		repoPlainHttp = repo.PlainHttp
+		repoSkipTLSVerify = repo.SkipTLSVerify
+	}
+
+	if release.PlainHttp || st.HelmDefaults.PlainHttp || repoPlainHttp {
+		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 {
 		flags = append(flags, "--insecure-skip-tls-verify")
 	}
+
 	return flags
 }
 
@@ -2673,7 +2690,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 	}
 
 	flags = st.appendConnectionFlags(flags, release)
-	flags = st.appendChartDownloadTLSFlags(flags, release)
+	flags = st.appendChartDownloadFlags(flags, release)
 
 	flags = st.appendHelmXFlags(flags, release)
 
@@ -2718,7 +2735,7 @@ func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseS
 	flags = st.appendPostRenderFlags(flags, release, postRenderer)
 	flags = st.appendPostRenderArgsFlags(flags, release, postRendererArgs)
 	flags = st.appendApiVersionsFlags(flags, release, kubeVersion)
-	flags = st.appendChartDownloadTLSFlags(flags, release)
+	flags = st.appendChartDownloadFlags(flags, release)
 	flags = st.appendShowOnlyFlags(flags, showOnly)
 
 	common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex)
@@ -2761,17 +2778,22 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec,
 
 	flags = st.appendConnectionFlags(flags, release)
 
-	if st.HelmDefaults.InsecureSkipTLSVerify || release.InsecureSkipTLSVerify {
+	flags = st.appendChartDownloadFlags(flags, release)
-		diffVersion, err := helmexec.GetPluginVersion("diff", settings.PluginsDirectory)
-		if err != nil {
-			return nil, nil, err
-		}
-		dv, _ := semver.NewVersion("v3.8.1")
 
-		if diffVersion.LessThan(dv) {
+	for _, flag := range flags {
-			return nil, nil, fmt.Errorf("insecureSkipTLSVerify is not supported by helm-diff plugin version %s, please use at least v3.8.1", diffVersion)
+		if flag == "--insecure-skip-tls-verify" {
+			diffVersion, err := helmexec.GetPluginVersion("diff", settings.PluginsDirectory)
+			if err != nil {
+				return nil, nil, err
+			}
+			dv, _ := semver.NewVersion("v3.8.1")
+
+			if diffVersion.LessThan(dv) {
+				return nil, nil, fmt.Errorf("insecureSkipTLSVerify is not supported by helm-diff plugin version %s, please use at least v3.8.1", diffVersion)
+			}
+
+			break
 		}
-		flags = st.appendChartDownloadTLSFlags(flags, release)
 	}
 
 	flags = st.appendHelmXFlags(flags, release)
@@ -2827,6 +2849,38 @@ func (st *HelmState) chartVersionFlags(release *ReleaseSpec) []string {
 	return flags
 }
 
+func (st *HelmState) chartOCIFlags(r *ReleaseSpec) []string {
+	flags := []string{}
+	repo, _ := st.GetRepositoryAndNameFromChartName(r.Chart)
+	if repo != nil {
+		if repo.PlainHttp {
+			flags = append(flags, "--plain-http")
+		} else {
+			// TLS options will nullify --plain-http in helm if passed, omit it them PlainHttp is specified
+			if repo.SkipTLSVerify {
+				flags = append(flags, "--insecure-skip-tls-verify")
+			}
+			if repo.CaFile != "" {
+				flags = append(flags, "--ca-file", repo.CaFile)
+			}
+			if repo.CertFile != "" && repo.KeyFile != "" {
+				flags = append(flags, "--cert-file", repo.CertFile, "--key-file", repo.KeyFile)
+			}
+		}
+		if repo.Verify {
+			flags = append(flags, "--verify")
+		}
+		if repo.Keyring != "" {
+			flags = append(flags, "--keyring", repo.Keyring)
+		}
+		if repo.RegistryConfig != "" {
+			flags = append(flags, "--registry-config", repo.RegistryConfig)
+		}
+	}
+
+	return flags
+}
+
 func (st *HelmState) appendValuesControlModeFlag(flags []string, reuseValues bool, resetValues bool) []string {
 	if !resetValues && (st.HelmDefaults.ReuseValues || reuseValues) {
 		flags = append(flags, "--reuse-values")
@@ -3661,29 +3715,7 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm
 	if st.fs.DirectoryExistsAt(chartPath) {
 		st.logger.Debugf("chart already exists at %s", chartPath)
 	} else {
-		flags := []string{}
+		flags := st.chartOCIFlags(release)
-		repo, _ := st.GetRepositoryAndNameFromChartName(release.Chart)
-		if repo != nil {
-			if repo.CaFile != "" {
-				flags = append(flags, "--ca-file", repo.CaFile)
-			}
-			if repo.CertFile != "" && repo.KeyFile != "" {
-				flags = append(flags, "--cert-file", repo.CertFile, "--key-file", repo.KeyFile)
-			}
-			if repo.SkipTLSVerify {
-				flags = append(flags, "--insecure-skip-tls-verify")
-			}
-			if repo.Verify {
-				flags = append(flags, "--verify")
-			}
-			if repo.Keyring != "" {
-				flags = append(flags, "--keyring", repo.Keyring)
-			}
-			if repo.RegistryConfig != "" {
-				flags = append(flags, "--registry-config", repo.RegistryConfig)
-			}
-		}
-
 		err := helm.ChartPull(qualifiedChartName, chartPath, flags...)
 		if err != nil {
 			return nil, err

pkg/state/state_test.go

@@ -3456,41 +3456,126 @@ func TestCommonDiffFlags(t *testing.T) {
 	}
 }
 
-func TestAppendChartDownloadTLSFlags(t *testing.T) {
+func TestAppendChartDownloadFlags(t *testing.T) {
 	tests := []struct {
-		name                         string
+		name     string
-		defaultInsecureSkipTLSVerify bool
+		spec     *ReleaseSetSpec
-		releaseInsecureSkipTLSVerify bool
+		expected []string
-		expected                     []string
 	}{
 		{
-			name:                         "defaultInsecureSkipTLSVerify is true and releaseInsecureSkipTLSVerify is false",
+			name: "SkipTLSVerify in repo",
-			defaultInsecureSkipTLSVerify: true,
+			spec: &ReleaseSetSpec{
-			releaseInsecureSkipTLSVerify: false,
+				Repositories: []RepositorySpec{
-			expected:                     []string{"--insecure-skip-tls-verify"},
+					{
+						Name:          "testrepo",
+						URL:           "registry/repo-path",
+						SkipTLSVerify: true,
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "testrepo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--insecure-skip-tls-verify",
+			},
 		},
 		{
-			name:                         "defaultInsecureSkipTLSVerify is false and releaseInsecureSkipTLSVerify is true",
+			name: "PlainHttp in repo, SkipTLSVerify everywhere",
-			defaultInsecureSkipTLSVerify: false,
+			spec: &ReleaseSetSpec{
-			releaseInsecureSkipTLSVerify: true,
+				HelmDefaults: HelmSpec{
-			expected:                     []string{"--insecure-skip-tls-verify"},
+					InsecureSkipTLSVerify: true,
+				},
+				Repositories: []RepositorySpec{
+					{
+						Name:          "testrepo",
+						URL:           "registry/repo-path",
+						SkipTLSVerify: true,
+						PlainHttp:     true,
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart:                 "testrepo/chartname",
+						InsecureSkipTLSVerify: true,
+					},
+				},
+			},
+			expected: []string{
+				"--plain-http",
+			},
 		},
 		{
-			name:                         "defaultInsecureSkipTLSVerify is false and releaseInsecureSkipTLSVerify is false",
+			name: "SkipTLSVerify in defaults",
-			defaultInsecureSkipTLSVerify: false,
+			spec: &ReleaseSetSpec{
-			releaseInsecureSkipTLSVerify: false,
+				HelmDefaults: HelmSpec{
-			expected:                     []string{},
+					InsecureSkipTLSVerify: true,
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--insecure-skip-tls-verify",
+			},
+		},
+		{
+			name: "SkipTLSVerify in release",
+			spec: &ReleaseSetSpec{
+				Releases: []ReleaseSpec{
+					{
+						Chart:                 "chartname",
+						InsecureSkipTLSVerify: true,
+					},
+				},
+			},
+			expected: []string{
+				"--insecure-skip-tls-verify",
+			},
+		},
+		{
+			name: "PlainHttp in defaults",
+			spec: &ReleaseSetSpec{
+				HelmDefaults: HelmSpec{
+					PlainHttp: true,
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--plain-http",
+			},
+		},
+		{
+			name: "PlainHttp in release",
+			spec: &ReleaseSetSpec{
+				Releases: []ReleaseSpec{
+					{
+						Chart:     "chartname",
+						PlainHttp: true,
+					},
+				},
+			},
+			expected: []string{
+				"--plain-http",
+			},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			st := &HelmState{}
+			st := &HelmState{
-			release := &ReleaseSpec{}
+				ReleaseSetSpec: *tt.spec,
-			st.HelmDefaults.InsecureSkipTLSVerify = tt.defaultInsecureSkipTLSVerify
+			}
-			release.InsecureSkipTLSVerify = tt.releaseInsecureSkipTLSVerify
 
-			result := st.appendChartDownloadTLSFlags([]string{}, release)
+			result := st.appendChartDownloadFlags([]string{}, &st.Releases[0])
 
 			require.Equal(t, tt.expected, result)
 		})
@@ -3790,3 +3875,216 @@ func TestGetOCIChartPath(t *testing.T) {
 		})
 	}
 }
+
+func TestHelmState_chartOCIFlags(t *testing.T) {
+	tests := []struct {
+		name     string
+		spec     *ReleaseSetSpec
+		expected []string
+	}{
+		{
+			name: "CaFile enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:   "oci-repo",
+						URL:    "registry/repo-path",
+						OCI:    true,
+						CaFile: "cafile",
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--ca-file",
+				"cafile",
+			},
+		},
+		{
+			name: "CertFile enabled and KeyFile disabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:     "oci-repo",
+						URL:      "registry/repo-path",
+						OCI:      true,
+						CertFile: "certfile",
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{},
+		},
+		{
+			name: "CertFile disabled and KeyFile enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:    "oci-repo",
+						URL:     "registry/repo-path",
+						OCI:     true,
+						KeyFile: "keyfile",
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{},
+		},
+		{
+			name: "CertFile and KeyFile enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:     "oci-repo",
+						URL:      "registry/repo-path",
+						OCI:      true,
+						CertFile: "certfile",
+						KeyFile:  "keyfile",
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--cert-file",
+				"certfile",
+				"--key-file",
+				"keyfile",
+			},
+		},
+		{
+			name: "SkipTLSVerify enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:          "oci-repo",
+						URL:           "registry/repo-path",
+						OCI:           true,
+						SkipTLSVerify: true,
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--insecure-skip-tls-verify",
+			},
+		},
+		{
+			name: "Verify enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:   "oci-repo",
+						URL:    "registry/repo-path",
+						OCI:    true,
+						Verify: true,
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--verify",
+			},
+		},
+		{
+			name: "Keyring enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:    "oci-repo",
+						URL:     "registry/repo-path",
+						OCI:     true,
+						Keyring: "keyring.pgp",
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--keyring",
+				"keyring.pgp",
+			},
+		},
+		{
+			name: "PlainHttp enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:      "oci-repo",
+						URL:       "registry/repo-path",
+						OCI:       true,
+						PlainHttp: true,
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--plain-http",
+			},
+		},
+		{
+			name: "PlainHttp and TLS options enabled",
+			spec: &ReleaseSetSpec{
+				Repositories: []RepositorySpec{
+					{
+						Name:          "oci-repo",
+						URL:           "registry/repo-path",
+						OCI:           true,
+						PlainHttp:     true,
+						CaFile:        "cafile",
+						CertFile:      "certfile",
+						KeyFile:       "keyfile",
+						SkipTLSVerify: true,
+					},
+				},
+				Releases: []ReleaseSpec{
+					{
+						Chart: "oci-repo/chartname",
+					},
+				},
+			},
+			expected: []string{
+				"--plain-http",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st := &HelmState{
+				ReleaseSetSpec: *tt.spec,
+			}
+			flags := st.chartOCIFlags(&tt.spec.Releases[0])
+			require.Equal(t, tt.expected, flags)
+		})
+	}
+}

pkg/state/temp_test.go

@@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) {
 	run(testcase{
 		subject: "baseline",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
-		want:    "foo-values-85888d5bcb",
+		want:    "foo-values-867bdd68d",
 	})
 
 	run(testcase{
 		subject: "different bytes content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    []byte(`{"k":"v"}`),
-		want:    "foo-values-77d5b4b567",
+		want:    "foo-values-55d7dc5f87",
 	})
 
 	run(testcase{
 		subject: "different map content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    map[string]any{"k": "v"},
-		want:    "foo-values-6b5c7d665c",
+		want:    "foo-values-58bdcf44f7",
 	})
 
 	run(testcase{
 		subject: "different chart",
 		release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
-		want:    "foo-values-5b86b7bc7",
+		want:    "foo-values-67dcddd85",
 	})
 
 	run(testcase{
 		subject: "different name",
 		release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
-		want:    "bar-values-bcb888989",
+		want:    "bar-values-6dd4989f97",
 	})
 
 	run(testcase{
 		subject: "specific ns",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
-		want:    "myns-foo-values-7599b8cdcf",
+		want:    "myns-foo-values-755d4487dc",
 	})
 
 	for id, n := range ids {