From f51b1a1194d51c52289290291483457a0623d76b Mon Sep 17 00:00:00 2001
From: Travis Groth <travis@netfoo.org>
Date: Tue, 6 Aug 2019 06:37:33 -0400
Subject: [PATCH] - Allows concurrent decryption of different secrets files -
 Caches decrypted secrets by original file path and returns decrypted results
 from memory

---
 pkg/helmexec/exec.go      | 113 ++++++++++++++++++++++----------------
 pkg/helmexec/exec_test.go |  10 +++-
 2 files changed, 74 insertions(+), 49 deletions(-)

diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go
index 52a1d596..9a72f659 100644
--- a/pkg/helmexec/exec.go
+++ b/pkg/helmexec/exec.go
@@ -17,13 +17,19 @@ const (
 	command = "helm"
 )
 
+type decryptedSecret struct {
+	mutex sync.RWMutex
+	bytes []byte
+}
+
 type execer struct {
-	helmBinary      string
-	runner          Runner
-	logger          *zap.SugaredLogger
-	kubeContext     string
-	extra           []string
-	decryptionMutex sync.Mutex
+	helmBinary           string
+	runner               Runner
+	logger               *zap.SugaredLogger
+	kubeContext          string
+	extra                []string
+	decryptedSecretMutex sync.Mutex
+	decryptedSecrets     map[string]*decryptedSecret
 }
 
 func NewLogger(writer io.Writer, logLevel string) *zap.SugaredLogger {
@@ -46,10 +52,11 @@ func NewLogger(writer io.Writer, logLevel string) *zap.SugaredLogger {
 // New for running helm commands
 func New(logger *zap.SugaredLogger, kubeContext string, runner Runner) *execer {
 	return &execer{
-		helmBinary:  command,
-		logger:      logger,
-		kubeContext: kubeContext,
-		runner:      runner,
+		helmBinary:       command,
+		logger:           logger,
+		kubeContext:      kubeContext,
+		runner:           runner,
+		decryptedSecrets: make(map[string]*decryptedSecret),
 	}
 }
 
@@ -125,55 +132,67 @@ func (helm *execer) List(context HelmContext, filter string, flags ...string) (s
 }
 
 func (helm *execer) DecryptSecret(context HelmContext, name string, flags ...string) (string, error) {
-	// Prevents https://github.com/roboll/helmfile/issues/258
-	helm.decryptionMutex.Lock()
-	defer helm.decryptionMutex.Unlock()
-
 	absPath, err := filepath.Abs(name)
 	if err != nil {
 		return "", err
 	}
-	helm.logger.Infof("Decrypting secret %v", absPath)
-	preArgs := context.GetTillerlessArgs(helm.helmBinary)
-	env := context.getTillerlessEnv()
-	out, err := helm.exec(append(append(preArgs, "secrets", "dec", absPath), flags...), env)
-	helm.info(out)
-	if err != nil {
-		return "", err
-	}
 
-	tmpFile, err := ioutil.TempFile("", "secret")
-	if err != nil {
-		return "", err
-	}
-	defer tmpFile.Close()
+	helm.logger.Debugf("Preparing to decrypt secret %v", absPath)
+	helm.decryptedSecretMutex.Lock()
 
-	// HELM_SECRETS_DEC_SUFFIX is used by the helm-secrets plugin to define the output file
-	decSuffix := os.Getenv("HELM_SECRETS_DEC_SUFFIX")
-	if len(decSuffix) == 0 {
-		decSuffix = ".yaml.dec"
-	}
-	decFilename := strings.Replace(absPath, ".yaml", decSuffix, 1)
+	secret, ok := helm.decryptedSecrets[absPath]
 
-	// os.Rename seems to results in "cross-device link` errors in some cases
-	// Instead of moving, copy it to the destination temp file as a work-around
-	// See https://github.com/roboll/helmfile/issues/251#issuecomment-417166296f
-	decFile, err := os.Open(decFilename)
-	if err != nil {
-		return "", err
-	}
-	defer decFile.Close()
+	// Cache miss
+	if !ok {
 
-	_, err = io.Copy(tmpFile, decFile)
-	if err != nil {
-		return "", err
+		secret = &decryptedSecret{}
+		helm.decryptedSecrets[absPath] = secret
+
+		secret.mutex.Lock()
+		defer secret.mutex.Unlock()
+		helm.decryptedSecretMutex.Unlock()
+
+		helm.logger.Infof("Decrypting secret %v", absPath)
+		preArgs := context.GetTillerlessArgs(helm.helmBinary)
+		env := context.getTillerlessEnv()
+		out, err := helm.exec(append(append(preArgs, "secrets", "dec", absPath), flags...), env)
+		helm.info(out)
+		if err != nil {
+			return "", err
+		}
+
+		// HELM_SECRETS_DEC_SUFFIX is used by the helm-secrets plugin to define the output file
+		decSuffix := os.Getenv("HELM_SECRETS_DEC_SUFFIX")
+		if len(decSuffix) == 0 {
+			decSuffix = ".yaml.dec"
+		}
+		decFilename := strings.Replace(absPath, ".yaml", decSuffix, 1)
+
+		secretBytes, err := ioutil.ReadFile(decFilename)
+		if err != nil {
+			return "", err
+		}
+		secret.bytes = secretBytes
+
+		if err := os.Remove(decFilename); err != nil {
+			return "", err
+		}
+
+	} else {
+		// Cache hit
+		helm.logger.Debugf("Found secret in cache %v", absPath)
+
+		secret.mutex.RLock()
+		helm.decryptedSecretMutex.Unlock()
+		defer secret.mutex.RUnlock()
 	}
 
-	if err := decFile.Close(); err != nil {
+	tmpFile, err := ioutil.TempFile("", "secret")
+	if err != nil {
 		return "", err
 	}
-
-	if err := os.Remove(decFilename); err != nil {
+	_, err = tmpFile.Write(secret.bytes)
+	if err != nil {
 		return "", err
 	}
 
diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go
index 5fab7d60..59144ae8 100644
--- a/pkg/helmexec/exec_test.go
+++ b/pkg/helmexec/exec_test.go
@@ -228,10 +228,16 @@ func Test_DecryptSecret(t *testing.T) {
 	if err != nil {
 		t.Errorf("Error: %v", err)
 	}
-	expected := fmt.Sprintf(`Decrypting secret %s/secretName
+	// Run again for caching
+	helm.DecryptSecret(HelmContext{}, "secretName")
+
+	expected := fmt.Sprintf(`Preparing to decrypt secret %v/secretName
+Decrypting secret %s/secretName
 exec: helm secrets dec %s/secretName --kube-context dev
 exec: helm secrets dec %s/secretName --kube-context dev: 
-`, cwd, cwd, cwd)
+Preparing to decrypt secret %s/secretName
+Found secret in cache %s/secretName
+`, cwd, cwd, cwd, cwd, cwd, cwd)
 	if buffer.String() != expected {
 		t.Errorf("helmexec.DecryptSecret()\nactual = %v\nexpect = %v", buffer.String(), expected)
 	}