Merge pull request #1273 from roboll/fix-chartify

fix: Follow-up fixes for #1172
roboll · May 27, 2020 · 930f91a · 930f91a
2 parents 16288df + 1cfce32
commit 930f91a
Show file tree

Hide file tree

Showing 8 changed files with 201 additions and 19 deletions.
diff --git a/docs/advanced-features.md b/docs/advanced-features.md
@@ -89,3 +89,49 @@ At this point, Helmfile can generate a complete kustomization from the base kust
 which can be included in the temporary chart.
 
 After all, Helmfile just installs the temporary chart like standard charts, which allows you to manage everything with Helmfile regardless of each app is declared using a Helm chart or a kustomization.
+
+Please also see [test/advanced/helmfile.yaml](https://github.com/roboll/helmfile/tree/master/test/advanced/helmfile.yaml) for an example of kustomization support and more.
+
+## Adhoc Customization of Helm charts
+
+You can add/update any Kubernetes resource field rendered from a Helm chart by specifying `releases[].strategicMergePatches`:
+
+```
+repositories:
+- name: incubator
+  url: https://kubernetes-charts-incubator.storage.googleapis.com
+
+releases:
+- name: raw1
+  chart: incubator/raw
+  values:
+  - resources:
+    - apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: raw1
+        namespace: default
+      data:
+        foo: FOO
+  strategicMergePatches:
+    - apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: raw1
+        namespace: default
+      data:
+        bar: BAR
+```
+
+Running `helmfile template` on the above example results in a ConfigMap called `raw` whose `data` is:
+
+```yaml
+foo: FOO
+bar: BAR
+```
+
+Please note that the second `data` field `bar` is coming from the strategic-merge patch defined in the above helmfile.yaml.
+
+There's also `releases[].jsonPatches` that works similarly to `strategicMergePatches` but has additional capability to remove fields.
+
+Please also see [test/advanced/helmfile.yaml](https://github.com/roboll/helmfile/tree/master/test/advanced/helmfile.yaml) for an example of patching support and more.
diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go
@@ -1,7 +1,10 @@
 package state
 
 import (
+	"github.com/roboll/helmfile/pkg/helmexec"
 	"github.com/variantdev/chartify"
+	"os"
+	"path/filepath"
 	"strings"
 )
 
@@ -19,11 +22,22 @@ func (st *HelmState) appendHelmXFlags(flags []string, release *ReleaseSpec) ([]s
 	return flags, nil
 }
 
-func (st *HelmState) PrepareChartify(release *ReleaseSpec) (bool, *chartify.ChartifyOpts) {
+func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) (bool, *chartify.ChartifyOpts, error) {
 	var opts chartify.ChartifyOpts
 
 	var shouldRun bool
 
+	opts.EnableKustomizeAlphaPlugins = true
+
+	opts.ChartVersion = release.Version
+
+	dir := filepath.Join(st.basePath, release.Chart)
+	if stat, _ := os.Stat(dir); stat != nil && stat.IsDir() {
+		if exists, err := st.fileExists(filepath.Join(dir, "Chart.yaml")); err == nil && !exists {
+			shouldRun = true
+		}
+	}
+
 	for _, d := range release.Dependencies {
 		var dep string
 
@@ -52,7 +66,7 @@ func (st *HelmState) PrepareChartify(release *ReleaseSpec) (bool, *chartify.Char
 	if len(jsonPatches) > 0 {
 		generatedFiles, err := st.generateTemporaryValuesFiles(jsonPatches, release.MissingFileHandler)
 		if err != nil {
-			return false, nil
+			return false, nil, err
 		}
 
 		for _, f := range generatedFiles {
@@ -68,7 +82,7 @@ func (st *HelmState) PrepareChartify(release *ReleaseSpec) (bool, *chartify.Char
 	if len(strategicMergePatches) > 0 {
 		generatedFiles, err := st.generateTemporaryValuesFiles(strategicMergePatches, release.MissingFileHandler)
 		if err != nil {
-			return false, nil
+			return false, nil, err
 		}
 
 		for _, f := range generatedFiles {
@@ -80,5 +94,14 @@ func (st *HelmState) PrepareChartify(release *ReleaseSpec) (bool, *chartify.Char
 		shouldRun = true
 	}
 
-	return shouldRun, &opts
+	if shouldRun {
+		generatedFiles, err := st.generateValuesFiles(helm, release, workerIndex)
+		if err != nil {
+			return false, nil, err
+		}
+
+		opts.ValuesFiles = generatedFiles
+	}
+
+	return shouldRun, &opts, nil
 }
diff --git a/pkg/state/state.go b/pkg/state/state.go
@@ -718,6 +718,7 @@ func PrepareCharts(helm helmexec.Interface, st *HelmState, dir string, concurren
 	type downloadResults struct {
 		releaseName string
 		chartPath   string
+		err         error
 	}
 	errs := []error{}
 
@@ -739,11 +740,17 @@ func PrepareCharts(helm helmexec.Interface, st *HelmState, dir string, concurren
 			}
 			close(jobQueue)
 		},
-		func(_ int) {
+		func(workerIndex int) {
 			for release := range jobQueue {
 				var chartPath string
 
-				if shouldChartify, opts := st.PrepareChartify(release); shouldChartify {
+				shouldChartify, opts, err := st.PrepareChartify(helm, release, workerIndex)
+				if err != nil {
+					results <- &downloadResults{err: err}
+					return
+				}
+
+				if shouldChartify {
 					c := chartify.New(
 						chartify.HelmBin(st.DefaultHelmBinary),
 						chartify.UseHelm3(helm3),
@@ -787,12 +794,18 @@ func PrepareCharts(helm helmexec.Interface, st *HelmState, dir string, concurren
 					}
 				}
 
-				results <- &downloadResults{release.Name, chartPath}
+				results <- &downloadResults{releaseName: release.Name, chartPath: chartPath}
 			}
 		},
 		func() {
 			for i := 0; i < len(st.Releases); i++ {
 				downloadRes := <-results
+
+				if downloadRes.err != nil {
+					errs = append(errs, downloadRes.err)
+
+					return
+				}
 				temp[downloadRes.releaseName] = downloadRes.chartPath
 			}
 		},
@@ -1886,12 +1899,7 @@ func (st *HelmState) generateTemporaryValuesFiles(values []interface{}, missingF
 	return generatedFiles, nil
 }
 
-func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) {
-	flags := []string{}
-	if release.Namespace != "" {
-		flags = append(flags, "--namespace", release.Namespace)
-	}
-
+func (st *HelmState) generateVanillaValuesFiles(release *ReleaseSpec) ([]string, error) {
 	values := []interface{}{}
 	for _, v := range release.Values {
 		switch typedValue := v.(type) {
@@ -1918,12 +1926,14 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R
 		return nil, err
 	}
 
-	for _, f := range generatedFiles {
-		flags = append(flags, "--values", f)
-	}
-
 	release.generatedValues = append(release.generatedValues, generatedFiles...)
 
+	return generatedFiles, nil
+}
+
+func (st *HelmState) generateSecretValuesFiles(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) {
+	var generatedFiles []string
+
 	for _, value := range release.Secrets {
 		paths, skip, err := st.storage().resolveFile(release.MissingFileHandler, "secrets", release.ValuesPathPrefix+value)
 		if err != nil {
@@ -1944,9 +1954,45 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R
 			return nil, err
 		}
 
-		release.generatedValues = append(release.generatedValues, valfile)
-		flags = append(flags, "--values", valfile)
+		generatedFiles = append(generatedFiles, valfile)
+	}
+
+	release.generatedValues = append(release.generatedValues, generatedFiles...)
+
+	return generatedFiles, nil
+}
+
+func (st *HelmState) generateValuesFiles(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) {
+	valuesFiles, err := st.generateVanillaValuesFiles(release)
+	if err != nil {
+		return nil, err
+	}
+
+	secretValuesFiles, err := st.generateSecretValuesFiles(helm, release, workerIndex)
+	if err != nil {
+		return nil, err
+	}
+
+	files := append(valuesFiles, secretValuesFiles...)
+
+	return files, nil
+}
+
+func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) {
+	flags := []string{}
+	if release.Namespace != "" {
+		flags = append(flags, "--namespace", release.Namespace)
 	}
+
+	generatedFiles, err := st.generateValuesFiles(helm, release, workerIndex)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, f := range generatedFiles {
+		flags = append(flags, "--values", f)
+	}
+
 	if len(release.SetValues) > 0 {
 		for _, set := range release.SetValues {
 			if set.Value != "" {

diff --git a/test/advanced/helmfile.yaml b/test/advanced/helmfile.yaml
@@ -0,0 +1,49 @@
+repositories:
+- name: incubator
+  url: https://kubernetes-charts-incubator.storage.googleapis.com
+
+releases:
+- name: kustomapp
+  chart: ./kustomapp
+  values:
+  - namePrefix: kustomapp-
+- name: raw1
+  chart: incubator/raw
+  values:
+  - resources:
+    - apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: raw1
+        namespace: default
+      data:
+        foo: FOO
+  strategicMergePatches:
+    - apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: raw1
+        namespace: default
+      data:
+        bar: BAR
+- name: raw2
+  chart: incubator/raw
+  values:
+  - resources:
+    - apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: raw2
+        namespace: default
+      data:
+        foo: FOO
+  jsonPatches:
+  - target:
+      version: v1
+      kind: ConfigMap
+      name: raw2
+      namespace: default
+    patch:
+      - op: replace
+        path: /data/baz
+        value: "BAZ"
diff --git a/test/advanced/kustomapp/all.yaml b/test/advanced/kustomapp/all.yaml
@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: kustomapp
+  namespace: default
+data:
+  kustomappfoo: FOO
diff --git a/test/advanced/kustomapp/kustomization.yaml b/test/advanced/kustomapp/kustomization.yaml
@@ -0,0 +1,2 @@
+resources:
+- all.yaml
diff --git a/test/advanced/kustomapp2/all.yaml b/test/advanced/kustomapp2/all.yaml
@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: kustomapp2
+  namespace: default
+data:
+  kustomappfoo: FOO
diff --git a/test/advanced/kustomapp2/kustomization.yaml b/test/advanced/kustomapp2/kustomization.yaml
@@ -0,0 +1,2 @@
+resources:
+- all.yaml