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.
This commit is contained in:
		
							parent
							
								
									bce2f4728b
								
							
						
					
					
						commit
						6baad71b1f
					
				|  | @ -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 | ||||
| 	decryptedSecretMutex sync.Mutex | ||||
| 	decryptedSecrets     map[string]*decryptedSecret | ||||
| } | ||||
| 
 | ||||
| func NewLogger(writer io.Writer, logLevel string) *zap.SugaredLogger { | ||||
|  | @ -50,6 +56,7 @@ func New(logger *zap.SugaredLogger, kubeContext string, runner Runner) *execer { | |||
| 		logger:           logger, | ||||
| 		kubeContext:      kubeContext, | ||||
| 		runner:           runner, | ||||
| 		decryptedSecrets: make(map[string]*decryptedSecret), | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
|  | @ -125,14 +132,26 @@ 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.Debugf("Preparing to decrypt secret %v", absPath) | ||||
| 	helm.decryptedSecretMutex.Lock() | ||||
| 
 | ||||
| 	secret, ok := helm.decryptedSecrets[absPath] | ||||
| 
 | ||||
| 	// Cache miss
 | ||||
| 	if !ok { | ||||
| 
 | ||||
| 		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() | ||||
|  | @ -142,12 +161,6 @@ func (helm *execer) DecryptSecret(context HelmContext, name string, flags ...str | |||
| 			return "", err | ||||
| 		} | ||||
| 
 | ||||
| 	tmpFile, err := ioutil.TempFile("", "secret") | ||||
| 	if err != nil { | ||||
| 		return "", err | ||||
| 	} | ||||
| 	defer tmpFile.Close() | ||||
| 
 | ||||
| 		// 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 { | ||||
|  | @ -155,28 +168,34 @@ func (helm *execer) DecryptSecret(context HelmContext, name string, flags ...str | |||
| 		} | ||||
| 		decFilename := strings.Replace(absPath, ".yaml", decSuffix, 1) | ||||
| 
 | ||||
| 	// 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) | ||||
| 		secretBytes, err := ioutil.ReadFile(decFilename) | ||||
| 		if err != nil { | ||||
| 			return "", err | ||||
| 		} | ||||
| 	defer decFile.Close() | ||||
| 
 | ||||
| 	_, err = io.Copy(tmpFile, decFile) | ||||
| 	if err != nil { | ||||
| 		return "", err | ||||
| 	} | ||||
| 
 | ||||
| 	if err := decFile.Close(); 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() | ||||
| 	} | ||||
| 
 | ||||
| 	tmpFile, err := ioutil.TempFile("", "secret") | ||||
| 	if err != nil { | ||||
| 		return "", err | ||||
| 	} | ||||
| 	_, err = tmpFile.Write(secret.bytes) | ||||
| 	if err != nil { | ||||
| 		return "", err | ||||
| 	} | ||||
| 
 | ||||
| 	return tmpFile.Name(), err | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -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) | ||||
| 	} | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue