Cache secrets and concurrent decryption (#790)

Related to #782 and #444 

- Allows concurrent decryption of different secrets files
- Caches decrypted secrets by original file path and returns decrypted results from memory
- Secrets being run through an instance of helmexec will be cached and run as fast as possible concurrently

NB: This particular PR doesn't make _all_ calls to secrets cached and concurrent.  Environment Secrets in particular seem to not be evaluated with a ScatterGather(), and doesn't use the same helmexec instance as other parts of the code, so it doesn't take advantage of these changes.  Some reworking of the plumbing there would be needed.

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
 	}
 

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)
 	}