From 9b64d65d9478cbe7b9bbd9d9049160108b01f6da Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 15 Dec 2020 10:24:27 +0900 Subject: [PATCH] feat: Stabilize helmfile-diff output (#1622) 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} ``` --- go.mod | 3 +- go.sum | 2 + pkg/state/state.go | 20 ++++++++-- pkg/state/temp.go | 90 ++++++++++++++++++++++++++++++++++++++++++ pkg/state/temp_rand.go | 33 ++++++++++++++++ pkg/state/temp_test.go | 80 +++++++++++++++++++++++++++++++++++++ 6 files changed, 223 insertions(+), 5 deletions(-) create mode 100644 pkg/state/temp.go create mode 100644 pkg/state/temp_rand.go create mode 100644 pkg/state/temp_test.go diff --git a/go.mod b/go.mod index 7099c467..34e2d26a 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 1ad1e4e8..a8a902e2 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 7bbbbd40..192f19ae 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 00000000..71a8406c --- /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 00000000..7fc308cb --- /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) +} diff --git a/pkg/state/temp_test.go b/pkg/state/temp_test.go new file mode 100644 index 00000000..6fde6c37 --- /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) + } + } +}