Repository: roboll/helmfile
Pull request #1622 (2020-12-15 01:09:33)

feat: Stabilize helmfile-diff output
This finishes the work started at #1619 by introducing the following changes:

- You can now set `HELMFILE_TEMPDIR` and `CHARTIFY_TEMPDIR` as the directory to persist temporary values files rendered by helmfile and temporary charts generated by chartify. Note that chartify is used internally when you use kustomize integration or raw K8s manifests as a chart.
- Helmfile uses the hash sum of the release config and the values file content in the file name of the temporary values file.

  In combination with `HELMFILE_TEMPDIR` and `CHARTIFY_TEMPDIR`, this enables helmfile to use the stable file names for temporary values files, which contribute to stabilize log messages like `Comparing release=appset, chart=PATH/TO/TEMPORARY/CHART`, where the `PATH/TO/TEMPORARY/CHART` had been randomized due to formerly random helmfile tempdir and temporary values file names, and random chartify tempdir.

You can try this feature with a script like:

```
tempdir=$(mktemp -d -t helmfile)

HELMFILE_TEMPDIR=${tempdir} CHARTIFY_TEMPDIR=${tempdir} helmfile diff

rm -rf ${tempdir}
```


Patch:
diff --git a/go.mod b/go.mod
index 7099c46..34e2d26 100644
--- a/go.mod
+++ b/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/Masterminds/semver/v3 v3.1.0
 	github.com/Masterminds/sprig/v3 v3.1.0
 	github.com/aryann/difflib v0.0.0-20170710044230-e206f873d14a
+	github.com/davecgh/go-spew v1.1.1
 	github.com/frankban/quicktest v1.11.2 // indirect
 	github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
 	github.com/go-test/deep v1.0.7
@@ -24,7 +25,7 @@ require (
 	github.com/spf13/cobra v1.1.1
 	github.com/tatsushid/go-prettytable v0.0.0-20141013043238-ed2d14c29939
 	github.com/urfave/cli v1.22.5
-	github.com/variantdev/chartify v0.5.0
+	github.com/variantdev/chartify v0.6.0
 	github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363
 	github.com/variantdev/vals v0.11.0
 	go.uber.org/multierr v1.6.0
diff --git a/go.sum b/go.sum
index 1ad1e4e..a8a902e 100644
--- a/go.sum
+++ b/go.sum
@@ -626,6 +626,8 @@ github.com/variantdev/chartify v0.4.9 h1:06foIMnJj31q/l1JZ+54anDLwqtP8zAOv5qVEn2
 github.com/variantdev/chartify v0.4.9/go.mod h1:jqlUJIzcrIVSfg8FC4g+IoC5WB83TBl8rnVVEv6g8MQ=
 github.com/variantdev/chartify v0.5.0 h1:I6T6oobjLfYmwZ4dUjRsO9AdGKPCMtfzt0CXR0ovl9k=
 github.com/variantdev/chartify v0.5.0/go.mod h1:jqlUJIzcrIVSfg8FC4g+IoC5WB83TBl8rnVVEv6g8MQ=
+github.com/variantdev/chartify v0.6.0 h1:QQ00a8Vtuhk6F9jeTZJEXV2g0zRXhYG43xovWZrc3ac=
+github.com/variantdev/chartify v0.6.0/go.mod h1:qF4XzQlkfH/6k2jAi1hLas+lK4zSCa8kY+r5JdmLA68=
 github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363 h1:KrfQBEUn+wEOQ/6UIfoqRDvn+Q/wZridQ7t0G1vQqKE=
 github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363/go.mod h1:pH1TQsNSLj2uxMo9NNl9zdGy01Wtn+/2MT96BrKmVyE=
 github.com/variantdev/vals v0.11.0 h1:818ztGk5yPTiixbUeE3LkkBGGATEAKtWq09n8q8PFqc=
diff --git a/pkg/state/state.go b/pkg/state/state.go
index 7bbbbd4..192f19a 100644
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@@ -1474,6 +1474,7 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 	numReleases := len(releases)
 	jobs := make(chan *ReleaseSpec, numReleases)
 	results := make(chan diffPrepareResult, numReleases)
+	resultsMap := map[string]diffPrepareResult{}
 
 	rs := []diffPrepareResult{}
 	errs := []error{}
@@ -1568,12 +1569,18 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 						errs = append(errs, e)
 					}
 				} else if res.release != nil {
-					rs = append(rs, res)
+					resultsMap[ReleaseToID(res.release)] = res
 				}
 			}
 		},
 	)
 
+	for _, r := range releases {
+		if p, ok := resultsMap[ReleaseToID(r)]; ok {
+			rs = append(rs, p)
+		}
+	}
+
 	return rs, errs
 }
 
@@ -2411,7 +2418,7 @@ func (st *HelmState) generateTemporaryReleaseValuesFiles(release *ReleaseSpec, v
 				return generatedFiles, fmt.Errorf("failed to render values files \"%s\": %v", typedValue, err)
 			}
 
-			valfile, err := ioutil.TempFile("", "values")
+			valfile, err := createTempValuesFile(release, yamlBytes)
 			if err != nil {
 				return generatedFiles, err
 			}
@@ -2420,19 +2427,24 @@ func (st *HelmState) generateTemporaryReleaseValuesFiles(release *ReleaseSpec, v
 			if _, err := valfile.Write(yamlBytes); err != nil {
 				return generatedFiles, fmt.Errorf("failed to write %s: %v", valfile.Name(), err)
 			}
-			st.logger.Debugf("successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes))
+
+			st.logger.Debugf("Successfully generated the value file at %s. produced:\n%s", path, string(yamlBytes))
+
 			generatedFiles = append(generatedFiles, valfile.Name())
 		case map[interface{}]interface{}, map[string]interface{}:
-			valfile, err := ioutil.TempFile("", "values")
+			valfile, err := createTempValuesFile(release, typedValue)
 			if err != nil {
 				return generatedFiles, err
 			}
 			defer valfile.Close()
+
 			encoder := yaml.NewEncoder(valfile)
 			defer encoder.Close()
+
 			if err := encoder.Encode(typedValue); err != nil {
 				return generatedFiles, err
 			}
+
 			generatedFiles = append(generatedFiles, valfile.Name())
 		default:
 			return generatedFiles, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue)
diff --git a/pkg/state/temp.go b/pkg/state/temp.go
new file mode 100644
index 0000000..71a8406
--- /dev/null
+++ b/pkg/state/temp.go
@@ -0,0 +1,90 @@
+package state
+
+import (
+	"errors"
+	"fmt"
+	"hash/fnv"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/davecgh/go-spew/spew"
+)
+
+func createTempValuesFile(release *ReleaseSpec, data interface{}) (*os.File, error) {
+	p, err := tempValuesFilePath(release, data)
+	if err != nil {
+		return nil, err
+	}
+
+	f, err := os.Create(*p)
+	if err != nil {
+		return nil, err
+	}
+
+	return f, nil
+}
+
+func tempValuesFilePath(release *ReleaseSpec, data interface{}) (*string, error) {
+	id, err := generateValuesID(release, data)
+	if err != nil {
+		panic(err)
+	}
+
+	workDir := os.Getenv("HELMFILE_TEMPDIR")
+	if workDir == "" {
+		workDir, err = ioutil.TempDir(os.TempDir(), "helmfile")
+		if err != nil {
+			panic(err)
+		}
+	}
+
+	d := filepath.Join(workDir, id)
+
+	info, err := os.Stat(d)
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		panic(err)
+	} else if info == nil {
+
+	}
+
+	return &d, nil
+}
+
+func generateValuesID(release *ReleaseSpec, data interface{}) (string, error) {
+	var id []string
+
+	if release.Namespace != "" {
+		id = append(id, release.Namespace)
+	}
+
+	id = append(id, release.Name, "values")
+
+	hash, err := HashObject([]interface{}{release, data})
+	if err != nil {
+		return "", err
+	}
+
+	id = append(id, hash)
+
+	return strings.Join(id, "-"), nil
+}
+
+func HashObject(obj interface{}) (string, error) {
+	hash := fnv.New32a()
+
+	hash.Reset()
+
+	printer := spew.ConfigState{
+		Indent:         " ",
+		SortKeys:       true,
+		DisableMethods: true,
+		SpewKeys:       true,
+	}
+	printer.Fprintf(hash, "%#v", obj)
+
+	sum := fmt.Sprint(hash.Sum32())
+
+	return SafeEncodeString(sum), nil
+}
diff --git a/pkg/state/temp_rand.go b/pkg/state/temp_rand.go
new file mode 100644
index 0000000..7fc308c
--- /dev/null
+++ b/pkg/state/temp_rand.go
@@ -0,0 +1,33 @@
+/*
+Copyright 2015 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+// See k8s.io/apimachinery/pkg/util/rand/rand.go
+package state
+
+const (
+	// We omit vowels from the set of available characters to reduce the chances
+	// of "bad words" being formed.
+	alphanums = "bcdfghjklmnpqrstvwxz2456789"
+)
+
+// SafeEncodeString encodes s using the same characters as rand.String. This reduces the chances of bad words and
+// ensures that strings generated from hash functions appear consistent throughout the API.
+func SafeEncodeString(s string) string {
+	r := make([]byte, len(s))
+	for i, b := range []rune(s) {
+		r[i] = alphanums[(int(b) % len(alphanums))]
+	}
+	return string(r)
+}


Test patch:
diff --git a/pkg/state/temp_test.go b/pkg/state/temp_test.go
new file mode 100644
index 0000000..6fde6c3
--- /dev/null
+++ b/pkg/state/temp_test.go
@@ -0,0 +1,80 @@
+package state
+
+import (
+	"github.com/google/go-cmp/cmp"
+	"testing"
+)
+
+func TestGenerateID(t *testing.T) {
+	type testcase struct {
+		subject string
+		release ReleaseSpec
+		data    interface{}
+		want    string
+	}
+
+	ids := map[string]int{}
+
+	run := func(tc testcase) {
+		t.Helper()
+
+		t.Run(tc.subject, func(t *testing.T) {
+			t.Helper()
+
+			got, err := generateValuesID(&tc.release, tc.data)
+			if err != nil {
+				t.Fatalf("uenxpected error: %v", err)
+			}
+
+			if d := cmp.Diff(tc.want, got); d != "" {
+				t.Fatalf("unexpected result: want (-), got (+):\n%s", d)
+			}
+
+			ids[got]++
+		})
+	}
+
+	run(testcase{
+		subject: "baseline",
+		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
+		want:    "foo-values-67b55dc69b",
+	})
+
+	run(testcase{
+		subject: "different bytes content",
+		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
+		data:    []byte(`{"k":"v"}`),
+		want:    "foo-values-5988bf4947",
+	})
+
+	run(testcase{
+		subject: "different map content",
+		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
+		data:    map[string]interface{}{"k": "v"},
+		want:    "foo-values-5d6fb4db97",
+	})
+
+	run(testcase{
+		subject: "different chart",
+		release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
+		want:    "foo-values-58db655b79",
+	})
+
+	run(testcase{
+		subject: "different name",
+		release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
+		want:    "bar-values-797d6df4dc",
+	})
+
+	run(testcase{
+		subject: "specific ns",
+		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
+		want:    "myns-foo-values-5f867c6d49",
+	})
+
+	for id, n := range ids {
+		if n > 1 {
+			t.Fatalf("too many occurences of %s: %d", id, n)
+		}
+	}
+}