diff --git a/pkg/app/app.go b/pkg/app/app.go index 8a06cbbf..817fb748 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -38,6 +38,7 @@ type App struct { FileOrDir string readFile func(string) ([]byte, error) + deleteFile func(string) error fileExists func(string) (bool, error) glob func(string) ([]string, error) abs func(string) (string, error) @@ -82,6 +83,7 @@ func New(conf ConfigProvider) *App { func Init(app *App) *App { app.readFile = ioutil.ReadFile + app.deleteFile = os.Remove app.glob = filepath.Glob app.abs = filepath.Abs app.getwd = os.Getwd @@ -482,6 +484,7 @@ func (a *App) loadDesiredStateFromYaml(file string, opts ...LoadOpts) (*state.He ld := &desiredStateLoader{ readFile: a.readFile, + deleteFile: a.deleteFile, fileExists: a.fileExists, env: a.Env, namespace: a.Namespace, diff --git a/pkg/app/desired_state_file_loader.go b/pkg/app/desired_state_file_loader.go index 5c76b7e2..3d7a7df3 100644 --- a/pkg/app/desired_state_file_loader.go +++ b/pkg/app/desired_state_file_loader.go @@ -28,6 +28,7 @@ type desiredStateLoader struct { namespace string readFile func(string) ([]byte, error) + deleteFile func(string) error fileExists func(string) (bool, error) abs func(string) (string, error) glob func(string) ([]string, error) @@ -150,6 +151,7 @@ func (ld *desiredStateLoader) loadFileWithOverrides(inheritedEnv, overrodeEnv *e func (a *desiredStateLoader) underlying() *state.StateCreator { c := state.NewCreator(a.logger, a.readFile, a.fileExists, a.abs, a.glob, a.valsRuntime, a.getHelm, a.overrideHelmBinary, a.remote) + c.DeleteFile = a.deleteFile c.LoadFile = a.loadFile return c } diff --git a/pkg/app/run.go b/pkg/app/run.go index 4e67bdf4..2e3f2eb0 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -63,7 +63,7 @@ func (r *Run) withPreparedCharts(forceDownload bool, helmfileCommand string, f f return err } - releaseToChart, errs := state.PrepareCharts(r.helm, r.state, dir, 2, helmfileCommand, forceDownload) + releaseToChart, errs := r.state.PrepareCharts(r.helm, dir, 2, helmfileCommand, forceDownload) if len(errs) > 0 { return fmt.Errorf("%v", errs) diff --git a/pkg/state/create.go b/pkg/state/create.go index b30a8105..3d50488a 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -44,6 +44,7 @@ type StateCreator struct { fileExists func(string) (bool, error) abs func(string) (string, error) glob func(string) ([]string, error) + DeleteFile func(string) error valsRuntime vals.Evaluator Strict bool @@ -281,6 +282,11 @@ func (c *StateCreator) scatterGatherEnvSecretFiles(st *HelmState, envSecretFiles results <- secretResult{nil, err, path} continue } + defer func() { + if err := c.DeleteFile(decFile); err != nil { + c.logger.Warnf("removing decrypted file %s: %w", decFile, err) + } + }() bytes, err := readFile(decFile) if err != nil { results <- secretResult{nil, fmt.Errorf("failed to load environment secrets file \"%s\": %v", path, err), path} diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 441bb317..bc70a6af 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -3,6 +3,7 @@ package state import ( "github.com/roboll/helmfile/pkg/remote" "io/ioutil" + "os" "path/filepath" "reflect" "testing" @@ -19,7 +20,9 @@ func createFromYaml(content []byte, file string, env string, logger *zap.Sugared logger: logger, readFile: ioutil.ReadFile, abs: filepath.Abs, - Strict: true, + + DeleteFile: os.Remove, + Strict: true, } return c.ParseAndLoad(content, filepath.Dir(file), file, env, true, nil) } diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index 8c45209e..adaac66a 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -22,7 +22,7 @@ func (st *HelmState) appendHelmXFlags(flags []string, release *ReleaseSpec) ([]s return flags, nil } -func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) (bool, *chartify.ChartifyOpts, error) { +func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) (bool, *chartify.ChartifyOpts, func(), error) { var opts chartify.ChartifyOpts opts.WorkaroundOutputDirIssue = true @@ -66,19 +66,25 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp shouldRun = true } + var filesNeedCleaning []string + + clean := func() { + st.removeFiles(filesNeedCleaning) + } + jsonPatches := release.JSONPatches if len(jsonPatches) > 0 { generatedFiles, err := st.generateTemporaryValuesFiles(jsonPatches, release.MissingFileHandler) if err != nil { - return false, nil, err + return false, nil, clean, err } + filesNeedCleaning = append(filesNeedCleaning, generatedFiles...) + for _, f := range generatedFiles { opts.JsonPatches = append(opts.JsonPatches, f) } - release.generatedValues = append(release.generatedValues, generatedFiles...) - shouldRun = true } @@ -86,14 +92,14 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp if len(strategicMergePatches) > 0 { generatedFiles, err := st.generateTemporaryValuesFiles(strategicMergePatches, release.MissingFileHandler) if err != nil { - return false, nil, err + return false, nil, clean, err } for _, f := range generatedFiles { opts.StrategicMergePatches = append(opts.StrategicMergePatches, f) } - release.generatedValues = append(release.generatedValues, generatedFiles...) + filesNeedCleaning = append(filesNeedCleaning, generatedFiles...) shouldRun = true } @@ -101,11 +107,13 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp if shouldRun { generatedFiles, err := st.generateValuesFiles(helm, release, workerIndex) if err != nil { - return false, nil, err + return false, nil, clean, err } + filesNeedCleaning = append(filesNeedCleaning, generatedFiles...) + opts.ValuesFiles = generatedFiles } - return shouldRun, &opts, nil + return shouldRun, &opts, clean, nil } diff --git a/pkg/state/state.go b/pkg/state/state.go index 953f5afb..71c041cc 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -219,8 +219,6 @@ type ReleaseSpec struct { StrategicMergePatches []interface{} `yaml:"strategicMergePatches,omitempty"` Adopt []string `yaml:"adopt,omitempty"` - // generatedValues are values that need cleaned up on exit - generatedValues []string //version of the chart that has really been installed cause desired version may be fuzzy (~2.0.0) installedVersion string } @@ -299,6 +297,7 @@ type syncPrepareResult struct { release *ReleaseSpec flags []string errors []*ReleaseError + files []string } // SyncReleases wrapper for executing helm upgrade on the releases @@ -349,10 +348,10 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu // TODO We need a long-term fix for this :) // See https://github.com/roboll/helmfile/issues/737 mut.Lock() - flags, flagsErr := st.flagsForUpgrade(helm, release, workerIndex) + flags, files, flagsErr := st.flagsForUpgrade(helm, release, workerIndex) mut.Unlock() if flagsErr != nil { - results <- syncPrepareResult{errors: []*ReleaseError{newReleaseFailedError(release, flagsErr)}} + results <- syncPrepareResult{errors: []*ReleaseError{newReleaseFailedError(release, flagsErr)}, files: files} continue } @@ -379,11 +378,11 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu } if len(errs) > 0 { - results <- syncPrepareResult{errors: errs} + results <- syncPrepareResult{errors: errs, files: files} continue } - results <- syncPrepareResult{release: release, flags: flags, errors: []*ReleaseError{}} + results <- syncPrepareResult{release: release, flags: flags, errors: []*ReleaseError{}, files: files} } }, func() { @@ -571,6 +570,13 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme } preps, prepErrs := st.prepareSyncReleases(helm, additionalValues, workerLimit, opts) + + defer func() { + for _, p := range preps { + st.removeFiles(p.files) + } + }() + if len(prepErrs) > 0 { return prepErrs } @@ -716,7 +722,7 @@ func (st *HelmState) getDeployedVersion(context helmexec.HelmContext, helm helme // Otheriwse, if a chart is not a helm chart, it will call "chartify" to turn it into a chart. // // If exists, it will also patch resources by json patches, strategic-merge patches, and injectors. -func PrepareCharts(helm helmexec.Interface, st *HelmState, dir string, concurrency int, helmfileCommand string, forceDownload bool) (map[string]string, []error) { +func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurrency int, helmfileCommand string, forceDownload bool) (map[string]string, []error) { temp := make(map[string]string, len(st.Releases)) type downloadResults struct { releaseName string @@ -747,7 +753,8 @@ func PrepareCharts(helm helmexec.Interface, st *HelmState, dir string, concurren for release := range jobQueue { var chartPath string - shouldChartify, opts, err := st.PrepareChartify(helm, release, workerIndex) + shouldChartify, opts, clean, err := st.PrepareChartify(helm, release, workerIndex) + defer clean() if err != nil { results <- &downloadResults{err: err} return @@ -847,15 +854,20 @@ func (st *HelmState) TemplateReleases(helm helmexec.Interface, outputDir string, } for i := range st.Releases { - release := st.Releases[i] + release := &st.Releases[i] if !release.Desired() { continue } - st.ApplyOverrides(&release) + st.ApplyOverrides(release) + + flags, files, err := st.flagsForTemplate(helm, release, 0) + + defer func() { + st.removeFiles(files) + }() - flags, err := st.flagsForTemplate(helm, &release, 0) if err != nil { errs = append(errs, err) } @@ -899,7 +911,7 @@ func (st *HelmState) TemplateReleases(helm helmexec.Interface, outputDir string, } } - if _, err := st.triggerCleanupEvent(&release, "template"); err != nil { + if _, err := st.triggerCleanupEvent(release, "template"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -944,7 +956,10 @@ func (st *HelmState) LintReleases(helm helmexec.Interface, additionalValues []st continue } - flags, err := st.flagsForLint(helm, &release, 0) + flags, files, err := st.flagsForLint(helm, &release, 0) + + defer st.removeFiles(files) + if err != nil { errs = append(errs, err) } @@ -992,6 +1007,7 @@ type diffPrepareResult struct { release *ReleaseSpec flags []string errors []*ReleaseError + files []string } func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValues []string, concurrency int, detailedExitCode, includeTests, suppressSecrets bool, opt ...DiffOpt) ([]diffPrepareResult, []error) { @@ -1038,7 +1054,7 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu // TODO We need a long-term fix for this :) // See https://github.com/roboll/helmfile/issues/737 mut.Lock() - flags, err := st.flagsForDiff(helm, release, workerIndex) + flags, files, err := st.flagsForDiff(helm, release, workerIndex) mut.Unlock() if err != nil { errs = append(errs, err) @@ -1087,9 +1103,9 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu for i, e := range errs { rsErrs[i] = newReleaseFailedError(release, e) } - results <- diffPrepareResult{errors: rsErrs} + results <- diffPrepareResult{errors: rsErrs, files: files} } else { - results <- diffPrepareResult{release: release, flags: flags, errors: []*ReleaseError{}} + results <- diffPrepareResult{release: release, flags: flags, errors: []*ReleaseError{}, files: files} } } }, @@ -1156,6 +1172,13 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st } preps, prepErrs := st.prepareDiffReleases(helm, additionalValues, workerLimit, detailedExitCode, includeTests, suppressSecrets, opts) + + defer func() { + for _, p := range preps { + st.removeFiles(p.files) + } + }() + if len(prepErrs) > 0 { return []ReleaseSpec{}, prepErrs } @@ -1291,21 +1314,6 @@ func (st *HelmState) TestReleases(helm helmexec.Interface, cleanup bool, timeout // Clean will remove any generated secrets func (st *HelmState) Clean() []error { - errs := []error{} - - for _, release := range st.Releases { - for _, value := range release.generatedValues { - err := st.removeFile(value) - if err != nil { - errs = append(errs, err) - } - } - } - - if len(errs) != 0 { - return errs - } - return nil } @@ -1668,7 +1676,7 @@ func (st *HelmState) timeoutFlags(helm helmexec.Interface, release *ReleaseSpec) return flags } -func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) { +func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) { flags := []string{} if release.Version != "" { flags = append(flags, "--version", release.Version) @@ -1710,7 +1718,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp flags = append(flags, "--create-namespace") } else if release.CreateNamespace != nil || st.HelmDefaults.CreateNamespace != nil { // createNamespace was set explicitly, but not running supported version of helm - error - return nil, fmt.Errorf("releases[].createNamespace requires Helm 3.2.0 or greater") + return nil, nil, fmt.Errorf("releases[].createNamespace requires Helm 3.2.0 or greater") } } @@ -1719,35 +1727,35 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp var err error flags, err = st.appendHelmXFlags(flags, release) if err != nil { - return nil, err + return nil, nil, err } - common, err := st.namespaceAndValuesFlags(helm, release, workerIndex) + common, clean, err := st.namespaceAndValuesFlags(helm, release, workerIndex) if err != nil { - return nil, err + return nil, clean, err } - return append(flags, common...), nil + return append(flags, common...), clean, nil } -func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) { +func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) { flags := []string{} var err error flags, err = st.appendHelmXFlags(flags, release) if err != nil { - return nil, err + return nil, nil, err } flags = st.appendApiVersionsFlags(flags) - common, err := st.namespaceAndValuesFlags(helm, release, workerIndex) + common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex) if err != nil { - return nil, err + return nil, files, err } - return append(flags, common...), nil + return append(flags, common...), files, nil } -func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) { +func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) { flags := []string{} if release.Version != "" { flags = append(flags, "--version", release.Version) @@ -1762,14 +1770,14 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, var err error flags, err = st.appendHelmXFlags(flags, release) if err != nil { - return nil, err + return nil, nil, err } - common, err := st.namespaceAndValuesFlags(helm, release, workerIndex) + common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex) if err != nil { - return nil, err + return nil, files, err } - return append(flags, common...), nil + return append(flags, common...), files, nil } func (st *HelmState) appendApiVersionsFlags(flags []string) []string { @@ -1788,18 +1796,18 @@ func (st *HelmState) isDevelopment(release *ReleaseSpec) bool { return result } -func (st *HelmState) flagsForLint(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) { - flags, err := st.namespaceAndValuesFlags(helm, release, workerIndex) +func (st *HelmState) flagsForLint(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) { + flags, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex) if err != nil { - return nil, err + return nil, files, err } flags, err = st.appendHelmXFlags(flags, release) if err != nil { - return nil, err + return nil, files, err } - return flags, nil + return flags, files, nil } func (st *HelmState) RenderValuesFileToBytes(path string) ([]byte, error) { @@ -1872,6 +1880,16 @@ func (st *HelmState) ExpandedHelmfiles() ([]SubHelmfileSpec, error) { return helmfiles, nil } +func (st *HelmState) removeFiles(files []string) { + for _, f := range files { + if err := st.removeFile(f); err != nil { + st.logger.Warnf("Removing %s: %v", err) + } else { + st.logger.Debugf("Removed %s", f) + } + } +} + func (st *HelmState) generateTemporaryValuesFiles(values []interface{}, missingFileHandler *string) ([]string, error) { generatedFiles := []string{} @@ -1880,47 +1898,47 @@ func (st *HelmState) generateTemporaryValuesFiles(values []interface{}, missingF case string: paths, skip, err := st.storage().resolveFile(missingFileHandler, "values", typedValue) if err != nil { - return nil, err + return generatedFiles, err } if skip { continue } if len(paths) > 1 { - return nil, fmt.Errorf("glob patterns in release values and secrets is not supported yet. please submit a feature request if necessary") + return generatedFiles, fmt.Errorf("glob patterns in release values and secrets is not supported yet. please submit a feature request if necessary") } path := paths[0] yamlBytes, err := st.RenderValuesFileToBytes(path) if err != nil { - return nil, fmt.Errorf("failed to render values files \"%s\": %v", typedValue, err) + return generatedFiles, fmt.Errorf("failed to render values files \"%s\": %v", typedValue, err) } valfile, err := ioutil.TempFile("", "values") if err != nil { - return nil, err + return generatedFiles, err } defer valfile.Close() if _, err := valfile.Write(yamlBytes); err != nil { - return nil, fmt.Errorf("failed to write %s: %v", valfile.Name(), err) + 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)) generatedFiles = append(generatedFiles, valfile.Name()) case map[interface{}]interface{}, map[string]interface{}: valfile, err := ioutil.TempFile("", "values") if err != nil { - return nil, err + return generatedFiles, err } defer valfile.Close() encoder := yaml.NewEncoder(valfile) defer encoder.Close() if err := encoder.Encode(typedValue); err != nil { - return nil, err + return generatedFiles, err } generatedFiles = append(generatedFiles, valfile.Name()) default: - return nil, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue) + return generatedFiles, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue) } } return generatedFiles, nil @@ -1953,8 +1971,6 @@ func (st *HelmState) generateVanillaValuesFiles(release *ReleaseSpec) ([]string, return nil, err } - release.generatedValues = append(release.generatedValues, generatedFiles...) - return generatedFiles, nil } @@ -1984,8 +2000,6 @@ func (st *HelmState) generateSecretValuesFiles(helm helmexec.Interface, release generatedFiles = append(generatedFiles, valfile) } - release.generatedValues = append(release.generatedValues, generatedFiles...) - return generatedFiles, nil } @@ -2005,17 +2019,21 @@ func (st *HelmState) generateValuesFiles(helm helmexec.Interface, release *Relea return files, nil } -func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, error) { +func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) { flags := []string{} if release.Namespace != "" { flags = append(flags, "--namespace", release.Namespace) } + var files []string + generatedFiles, err := st.generateValuesFiles(helm, release, workerIndex) if err != nil { - return nil, err + return nil, files, err } + files = generatedFiles + for _, f := range generatedFiles { flags = append(flags, "--values", f) } @@ -2025,7 +2043,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R if set.Value != "" { renderedValue, err := renderValsSecrets(st.valsRuntime, set.Value) if err != nil { - return nil, fmt.Errorf("Failed to render set value entry in %s for release %s: %v", st.FilePath, release.Name, err) + return nil, files, fmt.Errorf("Failed to render set value entry in %s for release %s: %v", st.FilePath, release.Name, err) } flags = append(flags, "--set", fmt.Sprintf("%s=%s", escape(set.Name), escape(renderedValue[0]))) } else if set.File != "" { @@ -2033,7 +2051,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R } else if len(set.Values) > 0 { renderedValues, err := renderValsSecrets(st.valsRuntime, set.Values...) if err != nil { - return nil, fmt.Errorf("Failed to render set values entry in %s for release %s: %v", st.FilePath, release.Name, err) + return nil, files, fmt.Errorf("Failed to render set values entry in %s for release %s: %v", st.FilePath, release.Name, err) } items := make([]string, len(renderedValues)) for i, raw := range renderedValues { @@ -2064,7 +2082,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R if len(envValErrs) != 0 { joinedEnvVals := strings.Join(envValErrs, "\n") errMsg := fmt.Sprintf("Environment Variables not found. Please make sure they are set and try again:\n%s", joinedEnvVals) - return nil, errors.New(errMsg) + return nil, files, errors.New(errMsg) } flags = append(flags, "--set", strings.Join(val, ",")) } @@ -2072,7 +2090,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R * END 'env' section for backwards compatibility **************/ - return flags, nil + return flags, files, nil } // renderValsSecrets helper function which renders 'ref+.*' secrets @@ -2172,7 +2190,7 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(interface{}) error) erro return nil } -func (st *HelmState) GenerateOutputDir(outputDir string, release ReleaseSpec) (string, error) { +func (st *HelmState) GenerateOutputDir(outputDir string, release *ReleaseSpec) (string, error) { // get absolute path of state file to generate a hash // use this hash to write helm output in a specific directory by state file and release name // ie. in a directory named stateFileName-stateFileHash-releaseName diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index bcb55315..840d71e3 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -705,7 +705,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { Version: tt.version, } - args, err := state.flagsForUpgrade(helm, tt.release, 0) + args, _, err := state.flagsForUpgrade(helm, tt.release, 0) if err != nil && tt.wantErr == "" { t.Errorf("unexpected error flagsForUpgrade: %v", err) }