Merge pull request #23 from helmfile/fix-helmfile-deps-chartify

Fix helmfile deps not to remove entries for charts that are being chartified

pkg/app/run.go

@@ -80,8 +80,11 @@ func (r *Run) withPreparedCharts(helmfileCommand string, opts state.ChartPrepare
 			Namespace:   rel.Namespace,
 			KubeContext: rel.KubeContext,
 		}
-		if chart := releaseToChart[key]; chart != "" {
-			rel.Chart = chart
+		if chart := releaseToChart[key]; chart != rel.Chart {
+			// In this case we assume that the chart is downloaded and modified by Helmfile and chartify.
+			// So we take note of the local filesystem path to the modified version of the chart
+			// and use it later via the Release.ChartPathOrName() func.
+			rel.ChartPath = chart
 		}
 	}
 

pkg/state/state.go

@@ -183,6 +183,11 @@ type RepositorySpec struct {
 type ReleaseSpec struct {
 	// Chart is the name of the chart being installed to create this release
 	Chart string `yaml:"chart,omitempty"`
+
+	// ChartPath is the downloaded and modified version of the remote Chart specified by the Chart field.
+	// This field is empty when the release is going to use the remote chart as-is, without any modifications(e.g. chartify).
+	ChartPath string `yaml:"chartPath,omitempty"`
+
 	// Directory is an alias to Chart which may be of more fit when you want to use a local/remote directory containing
 	// K8s manifests or Kustomization as a chart
 	Directory string `yaml:"directory,omitempty"`
@@ -315,6 +320,16 @@ type ReleaseSpec struct {
 	SkipDeps *bool `yaml:"skipDeps,omitempty"`
 }
 
+// ChartPathOrName returns ChartPath if it is non-empty, and returns Chart otherwise.
+// This is useful to redirect helm commands like `helm template`, `helm diff`, and `helm upgrade --install` to
+// our modified version of the chart, in case the user configured Helmfile to do modify the chart before being passed to Helm.
+func (r ReleaseSpec) ChartPathOrName() string {
+	if r.ChartPath != "" {
+		return r.ChartPath
+	}
+	return r.Chart
+}
+
 type Release struct {
 	ReleaseSpec
 
@@ -804,7 +819,7 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme
 			for prep := range jobQueue {
 				release := prep.release
 				flags := prep.flags
-				chart := normalizeChart(st.basePath, release.Chart)
+				chart := normalizeChart(st.basePath, release.ChartPathOrName())
 				var relErr *ReleaseError
 				context := st.createHelmContext(release, workerIndex)
 
@@ -1396,7 +1411,7 @@ func (st *HelmState) TemplateReleases(helm helmexec.Interface, outputDir string,
 		}
 
 		if len(errs) == 0 {
-			if err := helm.TemplateRelease(release.Name, release.Chart, flags...); err != nil {
+			if err := helm.TemplateRelease(release.Name, release.ChartPathOrName(), flags...); err != nil {
 				errs = append(errs, err)
 			}
 		}
@@ -1572,7 +1587,7 @@ func (st *HelmState) LintReleases(helm helmexec.Interface, additionalValues []st
 		}
 
 		if len(errs) == 0 {
-			if err := helm.Lint(release.Name, release.Chart, flags...); err != nil {
+			if err := helm.Lint(release.Name, release.ChartPathOrName(), flags...); err != nil {
 				errs = append(errs, err)
 			}
 		}
@@ -1870,7 +1885,7 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st
 				buf := &bytes.Buffer{}
 				if prep.upgradeDueToSkippedDiff {
 					results <- diffResult{release, &ReleaseError{ReleaseSpec: release, err: nil, Code: HelmDiffExitCodeChanged}, buf}
-				} else if err := helm.DiffRelease(st.createHelmContextWithWriter(release, buf), release.Name, normalizeChart(st.basePath, release.Chart), suppressDiff, flags...); err != nil {
+				} else if err := helm.DiffRelease(st.createHelmContextWithWriter(release, buf), release.Name, normalizeChart(st.basePath, release.ChartPathOrName()), suppressDiff, flags...); err != nil {
 					switch e := err.(type) {
 					case helmexec.ExitError:
 						// Propagate any non-zero exit status from the external command like `helm` that is failed under the hood

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-6d97d4c7ff",
+		want:    "foo-values-789768d94b",
 	})
 
 	run(testcase{
 		subject: "different bytes content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    []byte(`{"k":"v"}`),
-		want:    "foo-values-74fcbfbd68",
+		want:    "foo-values-7d669b768f",
 	})
 
 	run(testcase{
 		subject: "different map content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    map[string]interface{}{"k": "v"},
-		want:    "foo-values-7b954b6b6b",
+		want:    "foo-values-dd597bd55",
 	})
 
 	run(testcase{
 		subject: "different chart",
 		release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
-		want:    "foo-values-5c54676568",
+		want:    "foo-values-5d6c9f7f59",
 	})
 
 	run(testcase{
 		subject: "different name",
 		release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
-		want:    "bar-values-5fff459546",
+		want:    "bar-values-b6cdc56c",
 	})
 
 	run(testcase{
 		subject: "specific ns",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
-		want:    "myns-foo-values-cfd9959dd",
+		want:    "myns-foo-values-7cddc65b5",
 	})
 
 	for id, n := range ids {