Fix/secrets ordering

.circleci/config.yml

@@ -88,6 +88,7 @@ jobs:
           command: |
             cp ~/build/helmfile ~/project/helmfile
             cp ~/build/diff-yamls ~/project/diff-yamls
+            cp ~/build/yamldiff ~/project/yamldiff
       - run: make -C .circleci helm2
       - run: make -C .circleci kustomize
       - run: make -C .circleci minikube
@@ -110,6 +111,7 @@ jobs:
         command: |
           cp ~/build/helmfile ~/project/helmfile
           cp ~/build/diff-yamls ~/project/diff-yamls
+          cp ~/build/yamldiff ~/project/yamldiff
     - run: make -C .circleci helm
     - run: make -C .circleci vault
     - run: make -C .circleci sops

.gitignore

@@ -2,7 +2,8 @@ dist/
 .idea/
 helmfile
 helmfile.lock
-diff-yamls
+/diff-yamls
+/yamldiff
 test/integration/tmp
 vendor/
 *.log

Makefile

@@ -18,7 +18,8 @@ check:
 .PHONY: check
 
 build-test-tools:
-	go build test/diff-yamls.go
+	go build test/diff-yamls/diff-yamls.go
+	go build test/yamldiff/yamldiff.go
 .PHONY: build-test-tools
 
 test:

go.mod

@@ -19,6 +19,10 @@ require (
 	github.com/hashicorp/go-version v1.2.1
 	github.com/howeyc/gopass v0.0.0-20190910152052-7cb4b85ec19c // indirect
 	github.com/imdario/mergo v0.3.11
+	github.com/jessevdk/go-flags v1.4.0
+	github.com/kylelemons/godebug v1.1.0
+	github.com/logrusorgru/aurora v2.0.3+incompatible
+	github.com/mattn/go-isatty v0.0.12
 	github.com/pierrec/lz4 v2.3.0+incompatible // indirect
 	github.com/r3labs/diff v1.1.0
 	github.com/spf13/cobra v1.1.1

pkg/state/create.go

@@ -277,28 +277,34 @@ func (c *StateCreator) scatterGatherEnvSecretFiles(st *HelmState, envSecretFiles
 	inputsSize := len(inputs)
 
 	type secretResult struct {
+		id     int
 		result map[string]interface{}
 		err    error
 		path   string
 	}
 
-	secrets := make(chan string, inputsSize)
+	type secretInput struct {
+		id   int
+		path string
+	}
+
+	secrets := make(chan secretInput, inputsSize)
 	results := make(chan secretResult, inputsSize)
 
 	st.scatterGather(0, inputsSize,
 		func() {
-			for _, secretFile := range envSecretFiles {
-				secrets <- secretFile
+			for i, secretFile := range envSecretFiles {
+				secrets <- secretInput{i, secretFile}
 			}
 			close(secrets)
 		},
 		func(id int) {
-			for path := range secrets {
+			for secret := range secrets {
 				release := &ReleaseSpec{}
 				flags := st.appendConnectionFlags([]string{}, helm, release)
-				decFile, err := helm.DecryptSecret(st.createHelmContext(release, 0), path, flags...)
+				decFile, err := helm.DecryptSecret(st.createHelmContext(release, 0), secret.path, flags...)
 				if err != nil {
-					results <- secretResult{nil, err, path}
+					results <- secretResult{secret.id, nil, err, secret.path}
 					continue
 				}
 				defer func() {
@@ -308,28 +314,35 @@ func (c *StateCreator) scatterGatherEnvSecretFiles(st *HelmState, envSecretFiles
 				}()
 				bytes, err := readFile(decFile)
 				if err != nil {
-					results <- secretResult{nil, fmt.Errorf("failed to load environment secrets file \"%s\": %v", path, err), path}
+					results <- secretResult{secret.id, nil, fmt.Errorf("failed to load environment secrets file \"%s\": %v", secret.path, err), secret.path}
 					continue
 				}
 				m := map[string]interface{}{}
 				if err := yaml.Unmarshal(bytes, &m); err != nil {
-					results <- secretResult{nil, fmt.Errorf("failed to load environment secrets file \"%s\": %v", path, err), path}
+					results <- secretResult{secret.id, nil, fmt.Errorf("failed to load environment secrets file \"%s\": %v", secret.path, err), secret.path}
 					continue
 				}
 				// All the nested map key should be string. Otherwise we get strange errors due to that
 				// mergo or reflect is unable to merge map[interface{}]interface{} with map[string]interface{} or vice versa.
 				// See https://github.com/roboll/helmfile/issues/677
 				vals, err := maputil.CastKeysToStrings(m)
 				if err != nil {
-					results <- secretResult{nil, fmt.Errorf("failed to load environment secrets file \"%s\": %v", path, err), path}
+					results <- secretResult{secret.id, nil, fmt.Errorf("failed to load environment secrets file \"%s\": %v", secret.path, err), secret.path}
 					continue
 				}
-				results <- secretResult{vals, nil, path}
+				results <- secretResult{secret.id, vals, nil, secret.path}
 			}
 		},
 		func() {
+			sortedSecrets := make([]secretResult, inputsSize)
+
 			for i := 0; i < inputsSize; i++ {
 				result := <-results
+				sortedSecrets[result.id] = result
+			}
+			close(results)
+
+			for _, result := range sortedSecrets {
 				if result.err != nil {
 					errs = append(errs, result.err)
 				} else {
@@ -338,7 +351,6 @@ func (c *StateCreator) scatterGatherEnvSecretFiles(st *HelmState, envSecretFiles
 					}
 				}
 			}
-			close(results)
 		},
 	)
 

test/integration/run.sh

@@ -164,6 +164,34 @@ if [[ helm_major_version -eq 3 ]]; then
   ${helmfile} -f ${dir}/secretssops.yaml -e direct build || fail "\"helmfile build\" shouldn't fail"
 
   test_pass "secretssops.2"
+
+    test_start "secretssops.3 - should order secrets correctly"
+
+    tmp=$(mktemp -d)
+    direct=${tmp}/direct.build.yaml
+    reverse=${tmp}/reverse.build.yaml
+    golden_dir=${dir}/secrets-golden
+
+    info "Building secrets output"
+
+    info "Comparing build/direct output ${direct} with ${golden_dir}"
+    for i in $(seq 10); do
+        info "Comparing build/direct #$i"
+        ${helmfile} -f ${dir}/secretssops.yaml -e direct template --skip-deps > ${direct} || fail "\"helmfile template\" shouldn't fail"
+        ./yamldiff ${golden_dir}/direct.build.yaml ${direct} || fail "\"helmfile template\" should be consistent"
+        echo code=$?
+    done
+
+    info "Comparing build/reverse output ${direct} with ${golden_dir}"
+    for i in $(seq 10); do
+        info "Comparing build/reverse #$i"
+        ${helmfile} -f ${dir}/secretssops.yaml -e reverse template --skip-deps > ${reverse} || fail "\"helmfile template\" shouldn't fail"
+        ./yamldiff ${golden_dir}/reverse.build.yaml ${reverse} || fail "\"helmfile template\" should be consistent"
+        echo code=$?
+    done
+
+    test_pass "secretssops.3"
+
 fi
 
 # ALL DONE -----------------------------------------------------------------------------------------------------------

test/integration/secrets-golden/direct.build.yaml

@@ -0,0 +1,15 @@
+---
+# Source: raw/templates/resources.yaml
+apiVersion: v1
+kind: Secret
+metadata:
+  labels:
+    app: raw
+    chart: raw-0.2.3
+    heritage: Helm
+    release: raw
+  name: common-secret
+stringData:
+  key_1: value_1
+  key_2: value_2
+  key_shared: value_2

test/integration/secrets-golden/reverse.build.yaml

@@ -0,0 +1,15 @@
+---
+# Source: raw/templates/resources.yaml
+apiVersion: v1
+kind: Secret
+metadata:
+  labels:
+    app: raw
+    chart: raw-0.2.3
+    heritage: Helm
+    release: raw
+  name: common-secret
+stringData:
+  key_1: value_1
+  key_2: value_2
+  key_shared: value_1

test/integration/secretssops.yaml

@@ -25,6 +25,13 @@ releases:
     chart: center/incubator/raw
     version: 0.2.3
     values:
-      - mysecret: {{ .Environment.Values.key_1 }}
-      - mysecret: {{ .Environment.Values.key_2 }}
-      - mysecret: {{ .Environment.Values.key_shared }}
+      - templates:
+        - |
+          apiVersion: v1
+          kind: Secret
+          metadata:
+            name: common-secret
+          stringData:
+            key_1: {{ .Environment.Values.key_1 }}
+            key_2: {{ .Environment.Values.key_2 }}
+            key_shared: {{ .Environment.Values.key_shared }}

test/yamldiff/yamldiff.go

@@ -0,0 +1,147 @@
+package main
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+
+	"strings"
+
+	"github.com/jessevdk/go-flags"
+	"github.com/kylelemons/godebug/pretty"
+	"github.com/logrusorgru/aurora"
+	"github.com/mattn/go-isatty"
+	"gopkg.in/yaml.v2"
+)
+
+var version = "latest"
+
+func main() {
+	var opts struct {
+		NoColor bool   `long:"no-color" description:"disable colored output" required:"false"`
+		Version func() `long:"version" description:"print version and exit"`
+		// Ignore  []string `long:"ignore" description:"ignore vaules by yaml path" required:"false"`
+	}
+
+	opts.Version = func() {
+		fmt.Fprintf(os.Stderr, "%v\n", version)
+		os.Exit(0)
+	}
+
+	args, err := flags.Parse(&opts)
+	if err != nil {
+		if err.(*flags.Error).Type == flags.ErrHelp {
+			os.Exit(0)
+		}
+		os.Exit(1)
+	}
+	if len(args) != 2 {
+		fmt.Println("requires exactly two arguments to diff, use `yamldiff <file1> <file2>`")
+		os.Exit(1)
+	}
+
+	formatter := newFormatter(opts.NoColor)
+
+	errors := stat(args[0], args[1])
+	failOnErr(formatter, errors...)
+
+	yaml1, err := unmarshal(args[0])
+	if err != nil {
+		failOnErr(formatter, err)
+	}
+	yaml2, err := unmarshal(args[1])
+	if err != nil {
+		failOnErr(formatter, err)
+	}
+
+	diff, ndiff := computeDiff(formatter, yaml1, yaml2)
+	if ndiff > 0 {
+		fmt.Println(diff)
+		os.Exit(2)
+	}
+}
+
+func stat(filenames ...string) []error {
+	var errs []error
+	for _, filename := range filenames {
+		if filename == "-" {
+			continue
+		}
+		_, err := os.Stat(filename)
+		if err != nil {
+			errs = append(errs, fmt.Errorf("cannot find file: %v. Does it exist?", filename))
+		}
+	}
+	return errs
+}
+
+func unmarshal(filename string) (interface{}, error) {
+	var contents []byte
+	var err error
+	if filename == "-" {
+		contents, err = ioutil.ReadAll(os.Stdin)
+	} else {
+		contents, err = ioutil.ReadFile(filename)
+	}
+	if err != nil {
+		return nil, err
+	}
+	var ret interface{}
+	err = yaml.Unmarshal(contents, &ret)
+	if err != nil {
+		return nil, err
+	}
+	return ret, nil
+}
+
+func failOnErr(formatter aurora.Aurora, errs ...error) {
+	if len(errs) == 0 {
+		return
+	}
+	var errMessages []string
+	for _, err := range errs {
+		errMessages = append(errMessages, err.Error())
+	}
+	fmt.Fprintf(os.Stderr, "%v\n\n", formatter.Red(strings.Join(errMessages, "\n")))
+	os.Exit(1)
+}
+
+func computeDiff(formatter aurora.Aurora, a interface{}, b interface{}) (string, int) {
+	diffs := make([]string, 0)
+	ndiffs := 0
+	for _, s := range strings.Split(pretty.Compare(a, b), "\n") {
+		switch {
+		case strings.HasPrefix(s, "+"):
+			diffs = append(diffs, formatter.Bold(formatter.Green(s)).String())
+			ndiffs += 1
+		case strings.HasPrefix(s, "-"):
+			diffs = append(diffs, formatter.Bold(formatter.Red(s)).String())
+			ndiffs += 1
+		default:
+			diffs = append(diffs, s)
+		}
+	}
+	return strings.Join(diffs, "\n"), ndiffs
+}
+
+func newFormatter(noColor bool) aurora.Aurora {
+	var formatter aurora.Aurora
+	if noColor || !isTerminal() {
+		formatter = aurora.NewAurora(false)
+	} else {
+		formatter = aurora.NewAurora(true)
+	}
+	return formatter
+}
+
+func isTerminal() bool {
+	fd := os.Stdout.Fd()
+	switch {
+	case isatty.IsTerminal(fd):
+		return true
+	case isatty.IsCygwinTerminal(fd):
+		return true
+	default:
+		return false
+	}
+}