From 2f0483181770d915e09432aac403a7a331e2a836 Mon Sep 17 00:00:00 2001 From: Anton Bretting Date: Sat, 12 Feb 2022 12:28:08 +0100 Subject: [PATCH] Fix various golangci-lint errors (#2059) --- main.go | 5 +- pkg/app/app.go | 102 ++++++++------- pkg/app/app_test.go | 58 +++------ pkg/app/mocks_test.go | 2 - pkg/app/run.go | 10 +- pkg/app/two_pass_renderer_test.go | 14 ++- pkg/argparser/args.go | 6 +- pkg/argparser/args_test.go | 10 +- pkg/event/bus_test.go | 4 - pkg/exectest/helm.go | 2 - pkg/helmexec/exec_test.go | 200 ++++++++++++++++++++++++------ pkg/helmexec/runner.go | 5 - pkg/maputil/maputil.go | 4 +- pkg/state/create.go | 4 +- pkg/state/helmx.go | 12 +- pkg/state/release_filters.go | 8 +- pkg/state/state.go | 71 +++++------ pkg/state/state_exec_tmpl.go | 21 ++-- pkg/state/state_exec_tmpl_test.go | 5 - pkg/state/state_gogetter_test.go | 12 +- pkg/state/state_run.go | 3 +- pkg/state/state_test.go | 20 +-- pkg/state/temp.go | 4 +- pkg/state/util.go | 6 +- pkg/testhelper/testfs.go | 4 +- pkg/tmpl/context_funcs.go | 2 +- pkg/tmpl/context_funcs_test.go | 16 +-- pkg/tmpl/expand_secret_ref.go | 8 +- test/diff-yamls/diff-yamls.go | 7 +- 29 files changed, 362 insertions(+), 263 deletions(-) diff --git a/main.go b/main.go index 3b4c882d..5d975d30 100644 --- a/main.go +++ b/main.go @@ -737,7 +737,10 @@ type configImpl struct { func NewUrfaveCliConfigImpl(c *cli.Context) (configImpl, error) { if c.NArg() > 0 { - cli.ShowAppHelp(c) + err := cli.ShowAppHelp(c) + if err != nil { + return configImpl{}, err + } return configImpl{}, fmt.Errorf("err: extraneous arguments: %s", strings.Join(c.Args(), ", ")) } diff --git a/pkg/app/app.go b/pkg/app/app.go index aca5b9a3..30767b3d 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -567,7 +567,7 @@ func (a *App) ListReleases(c ListConfigProvider) error { } var keys []string - for k, _ := range r.Labels { + for k := range r.Labels { keys = append(keys, k) } sort.Strings(keys) @@ -949,7 +949,8 @@ func withBatches(templated *state.HelmState, batches [][]state.Release, helm hel var releaseIds []string for _, r := range targets { - releaseIds = append(releaseIds, state.ReleaseToID(&r)) + release := r + releaseIds = append(releaseIds, state.ReleaseToID(&release)) } logger.Debugf("processing releases in group %d/%d: %s", i+1, numBatches, strings.Join(releaseIds, ", ")) @@ -959,7 +960,7 @@ func withBatches(templated *state.HelmState, batches [][]state.Release, helm hel processed, errs := converge(&batchSt, helm) - if errs != nil && len(errs) > 0 { + if len(errs) > 0 { return false, errs } @@ -1181,7 +1182,8 @@ func (a *App) getSelectedReleases(r *Run, includeTransitiveNeeds bool) ([]state. // We iterate over allReleases rather than groupsByID // to preserve the order of releases for _, seq := range allReleases { - id := state.ReleaseToID(&seq) + release := seq + id := state.ReleaseToID(&release) rs := groupsByID[id] @@ -1199,7 +1201,7 @@ func (a *App) getSelectedReleases(r *Run, includeTransitiveNeeds bool) ([]state. // Otherwise we can't compute the DAG of releases correctly. r, deduped := selectedIds[id] if !deduped { - panic(fmt.Errorf("assertion error: release %q has never been selected. This shouldn't happen!", id)) + panic(fmt.Errorf("assertion error: release %q has never been selected. This shouldn't happen", id)) } deduplicated = append(deduplicated, r) @@ -1286,11 +1288,12 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { releasesWithNoChange := map[string]state.ReleaseSpec{} for _, r := range toApplyWithNeeds { - id := state.ReleaseToID(&r) + release := r + id := state.ReleaseToID(&release) _, uninstalled := releasesToBeDeleted[id] _, updated := releasesToBeUpdated[id] if !uninstalled && !updated { - releasesWithNoChange[id] = r + releasesWithNoChange[id] = release } } @@ -1336,7 +1339,8 @@ Do you really want to apply? var rs []state.ReleaseSpec for _, r := range subst.Releases { - if r2, ok := releasesToBeDeleted[state.ReleaseToID(&r)]; ok { + release := r + if r2, ok := releasesToBeDeleted[state.ReleaseToID(&release)]; ok { rs = append(rs, r2) } } @@ -1346,7 +1350,7 @@ Do you really want to apply? return subst.DeleteReleasesForSync(&affectedReleases, helm, c.Concurrency()) })) - if deletionErrs != nil && len(deletionErrs) > 0 { + if len(deletionErrs) > 0 { syncErrs = append(syncErrs, deletionErrs...) } } @@ -1357,7 +1361,8 @@ Do you really want to apply? var rs []state.ReleaseSpec for _, r := range subst.Releases { - if r2, ok := releasesToBeUpdated[state.ReleaseToID(&r)]; ok { + release := r + if r2, ok := releasesToBeUpdated[state.ReleaseToID(&release)]; ok { rs = append(rs, r2) } } @@ -1374,7 +1379,7 @@ Do you really want to apply? return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), &syncOpts) })) - if updateErrs != nil && len(updateErrs) > 0 { + if len(updateErrs) > 0 { syncErrs = append(syncErrs, updateErrs...) } } @@ -1405,16 +1410,18 @@ func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error releasesToDelete := map[string]state.ReleaseSpec{} for _, r := range toDelete { - id := state.ReleaseToID(&r) - releasesToDelete[id] = r + release := r + id := state.ReleaseToID(&release) + releasesToDelete[id] = release } releasesWithNoChange := map[string]state.ReleaseSpec{} for _, r := range toSync { - id := state.ReleaseToID(&r) + release := r + id := state.ReleaseToID(&release) _, uninstalled := releasesToDelete[id] if !uninstalled { - releasesWithNoChange[id] = r + releasesWithNoChange[id] = release } } @@ -1450,7 +1457,7 @@ Do you really want to delete? return subst.DeleteReleases(&affectedReleases, helm, c.Concurrency(), purge) })) - if deletionErrs != nil && len(deletionErrs) > 0 { + if len(deletionErrs) > 0 { errs = append(errs, deletionErrs...) } } @@ -1574,7 +1581,7 @@ func (a *App) lint(r *Run, c LintConfigProvider) (bool, []error, []error) { return lintErrs })) - if templateErrs != nil && len(templateErrs) > 0 { + if len(templateErrs) > 0 { errs = append(errs, templateErrs...) } } @@ -1626,7 +1633,7 @@ func (a *App) status(r *Run, c StatusesConfigProvider) (bool, []error) { return subst.ReleaseStatuses(helm, c.Concurrency()) })) - if templateErrs != nil && len(templateErrs) > 0 { + if len(templateErrs) > 0 { errs = append(errs, templateErrs...) } } @@ -1675,36 +1682,39 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { releasesToDelete := map[string]state.ReleaseSpec{} for _, r := range toDelete { - id := state.ReleaseToID(&r) - releasesToDelete[id] = r + release := r + id := state.ReleaseToID(&release) + releasesToDelete[id] = release } var toUpdate []state.ReleaseSpec for _, r := range toSyncWithNeeds { - if _, deleted := releasesToDelete[state.ReleaseToID(&r)]; !deleted { - if r.Installed == nil || *r.Installed { - toUpdate = append(toUpdate, r) - } else { - // TODO Emit error when the user opted to fail when the needed release is disabled, - // instead of silently ignoring it. - // See https://github.com/roboll/helmfile/issues/1018 + release := r + if _, deleted := releasesToDelete[state.ReleaseToID(&release)]; !deleted { + if release.Installed == nil || *release.Installed { + toUpdate = append(toUpdate, release) } + // TODO Emit error when the user opted to fail when the needed release is disabled, + // instead of silently ignoring it. + // See https://github.com/roboll/helmfile/issues/1018 } } releasesToUpdate := map[string]state.ReleaseSpec{} for _, r := range toUpdate { - id := state.ReleaseToID(&r) - releasesToUpdate[id] = r + release := r + id := state.ReleaseToID(&release) + releasesToUpdate[id] = release } releasesWithNoChange := map[string]state.ReleaseSpec{} for _, r := range toSyncWithNeeds { - id := state.ReleaseToID(&r) + release := r + id := state.ReleaseToID(&release) _, uninstalled := releasesToDelete[id] _, updated := releasesToUpdate[id] if !uninstalled && !updated { - releasesWithNoChange[id] = r + releasesWithNoChange[id] = release } } @@ -1745,7 +1755,8 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { var rs []state.ReleaseSpec for _, r := range subst.Releases { - if r2, ok := releasesToDelete[state.ReleaseToID(&r)]; ok { + release := r + if r2, ok := releasesToDelete[state.ReleaseToID(&release)]; ok { rs = append(rs, r2) } } @@ -1755,7 +1766,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { return subst.DeleteReleasesForSync(&affectedReleases, helm, c.Concurrency()) })) - if deletionErrs != nil && len(deletionErrs) > 0 { + if len(deletionErrs) > 0 { errs = append(errs, deletionErrs...) } } @@ -1765,8 +1776,9 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { var rs []state.ReleaseSpec for _, r := range subst.Releases { - if _, ok := releasesToDelete[state.ReleaseToID(&r)]; !ok { - rs = append(rs, r) + release := r + if _, ok := releasesToDelete[state.ReleaseToID(&release)]; !ok { + rs = append(rs, release) } } @@ -1781,7 +1793,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), opts) })) - if syncErrs != nil && len(syncErrs) > 0 { + if len(syncErrs) > 0 { errs = append(errs, syncErrs...) } } @@ -1824,11 +1836,12 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { releasesDisabled := map[string]state.ReleaseSpec{} for _, r := range selectedReleasesWithNeeds { - id := state.ReleaseToID(&r) - if r.Installed != nil && !*r.Installed { - releasesDisabled[id] = r + release := r + id := state.ReleaseToID(&release) + if release.Installed != nil && !*release.Installed { + releasesDisabled[id] = release } else { - toRender = append(toRender, r) + toRender = append(toRender, release) } } @@ -1908,11 +1921,12 @@ func (a *App) writeValues(r *Run, c WriteValuesConfigProvider) (bool, []error) { releasesToWrite := map[string]state.ReleaseSpec{} for _, r := range toRender { - id := state.ReleaseToID(&r) - if r.Installed != nil && !*r.Installed { + release := r + id := state.ReleaseToID(&release) + if release.Installed != nil && !*release.Installed { continue } - releasesToWrite[id] = r + releasesToWrite[id] = release } var errs []error @@ -2060,7 +2074,7 @@ type context struct { } func (c context) wrapErrs(errs ...error) error { - if errs != nil && len(errs) > 0 { + if len(errs) > 0 { for _, err := range errs { switch e := err.(type) { case *state.ReleaseError: diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 3f3e268b..0c2713fd 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -505,9 +505,9 @@ releases: errMsg string }{ {label: "name=prometheus", expectedCount: 1, expectErr: false}, - {label: "name=", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: Malformed label: name=. Expected label in form k=v or k!=v"}, - {label: "name!=", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: Malformed label: name!=. Expected label in form k=v or k!=v"}, - {label: "name", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: Malformed label: name. Expected label in form k=v or k!=v"}, + {label: "name=", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: malformed label: name=. Expected label in form k=v or k!=v"}, + {label: "name!=", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: malformed label: name!=. Expected label in form k=v or k!=v"}, + {label: "name", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: malformed label: name. Expected label in form k=v or k!=v"}, // See https://github.com/roboll/helmfile/issues/193 {label: "duplicatedNs=yes", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[2]: in /path/to/helmfile.d/b.yaml: duplicate release \"foo\" found in namespace \"zoo\" in kubecontext \"default\": there were 2 releases named \"foo\" matching specified selector"}, {label: "duplicatedCtx=yes", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[2]: in /path/to/helmfile.d/b.yaml: duplicate release \"foo\" found in namespace \"zoo\" in kubecontext \"default\": there were 2 releases named \"foo\" matching specified selector"}, @@ -873,9 +873,7 @@ tillerNs: INLINE_TILLER_NS_2 processed := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - processed = append(processed, r) - } + processed = append(processed, run.state.Releases...) return false, []error{} } @@ -1151,9 +1149,7 @@ x: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -1206,9 +1202,7 @@ releases: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -1264,9 +1258,7 @@ releases: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -1316,9 +1308,7 @@ releases: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -1365,9 +1355,7 @@ releases: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -1409,9 +1397,7 @@ releases: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -1453,9 +1439,7 @@ releases: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -1501,9 +1485,7 @@ releases: actual := []state.ReleaseSpec{} collectReleases := func(run *Run) (bool, []error) { - for _, r := range run.state.Releases { - actual = append(actual, r) - } + actual = append(actual, run.state.Releases...) return false, []error{} } app := appWithFs(&App{ @@ -2500,8 +2482,6 @@ func (d depsConfig) Args() string { // Mocking the command-line runner type mockRunner struct { - output []byte - err error } func (mock *mockRunner) ExecuteStdIn(cmd string, args []string, env map[string]string, stdin io.Reader) ([]byte, error) { @@ -2519,16 +2499,9 @@ func MockExecer(logger *zap.SugaredLogger, kubeContext string) helmexec.Interfac // mocking helmexec.Interface -type listKey struct { - filter string - flags string -} - type mockHelmExec struct { templated []mockTemplates repos []mockRepo - - updateDepsCallbacks map[string]func(string) error } type mockTemplates struct { @@ -2562,10 +2535,8 @@ func (helm *mockHelmExec) BuildDeps(name, chart string) error { } func (helm *mockHelmExec) SetExtraArgs(args ...string) { - return } func (helm *mockHelmExec) SetHelmBinary(bin string) { - return } func (helm *mockHelmExec) AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error { helm.repos = append(helm.repos, mockRepo{Name: name}) @@ -4715,7 +4686,10 @@ func captureStdout(f func()) string { go func() { var buf bytes.Buffer wg.Done() - io.Copy(&buf, reader) + _, err = io.Copy(&buf, reader) + if err != nil { + panic(err) + } out <- buf.String() }() wg.Wait() diff --git a/pkg/app/mocks_test.go b/pkg/app/mocks_test.go index cc00c77a..5657d3a4 100644 --- a/pkg/app/mocks_test.go +++ b/pkg/app/mocks_test.go @@ -42,11 +42,9 @@ func (helm *noCallHelmExec) BuildDeps(name, chart string) error { func (helm *noCallHelmExec) SetExtraArgs(args ...string) { helm.doPanic() - return } func (helm *noCallHelmExec) SetHelmBinary(bin string) { helm.doPanic() - return } func (helm *noCallHelmExec) AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error { helm.doPanic() diff --git a/pkg/app/run.go b/pkg/app/run.go index e412e861..ed680138 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -155,17 +155,19 @@ func (r *Run) diff(triggerCleanupEvent bool, detailedExitCode bool, c DiffConfig releasesToBeDeleted := map[string]state.ReleaseSpec{} for _, r := range deletingReleases { - id := state.ReleaseToID(&r) - releasesToBeDeleted[id] = r + release := r + id := state.ReleaseToID(&release) + releasesToBeDeleted[id] = release } releasesToBeUpdated := map[string]state.ReleaseSpec{} for _, r := range changedReleases { - id := state.ReleaseToID(&r) + release := r + id := state.ReleaseToID(&release) // If `helm-diff` detected changes but it is not being `helm delete`ed, we should run `helm upgrade` if _, ok := releasesToBeDeleted[id]; !ok { - releasesToBeUpdated[id] = r + releasesToBeUpdated[id] = release } } diff --git a/pkg/app/two_pass_renderer_test.go b/pkg/app/two_pass_renderer_test.go index ce945353..731b449b 100644 --- a/pkg/app/two_pass_renderer_test.go +++ b/pkg/app/two_pass_renderer_test.go @@ -1,11 +1,12 @@ package app import ( - "github.com/roboll/helmfile/pkg/remote" "os" "strings" "testing" + "github.com/roboll/helmfile/pkg/remote" + "github.com/roboll/helmfile/pkg/helmexec" "github.com/roboll/helmfile/pkg/state" "github.com/roboll/helmfile/pkg/testhelper" @@ -56,6 +57,9 @@ releases: var state state.HelmState err = yaml.Unmarshal(yamlBuf.Bytes(), &state) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if testfs.FileReaderCalls() > 2 { t.Error("reader should be called only twice") @@ -183,7 +187,10 @@ releases: rendered, _ := r.renderTemplatesToYaml("", "", yamlContent) var state state.HelmState - yaml.Unmarshal(rendered.Bytes(), &state) + err := yaml.Unmarshal(rendered.Bytes(), &state) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if len(state.Releases) != 1 { t.Fatal("there should be 1 release") @@ -210,6 +217,9 @@ func TestReadFromYaml_RenderTemplateWithNamespace(t *testing.T) { var state state.HelmState err = yaml.Unmarshal(yamlBuf.Bytes(), &state) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if state.Releases[0].Name != "namespace-myrelease" { t.Errorf("release name should be namespace-myrelease") diff --git a/pkg/argparser/args.go b/pkg/argparser/args.go index 724f433f..856d2197 100644 --- a/pkg/argparser/args.go +++ b/pkg/argparser/args.go @@ -2,8 +2,9 @@ package argparser import ( "fmt" - "github.com/roboll/helmfile/pkg/state" "strings" + + "github.com/roboll/helmfile/pkg/state" ) type keyVal struct { @@ -95,10 +96,9 @@ func GetArgs(args string, state *state.HelmState) []string { argArr = append(argArr, fmt.Sprintf("%s=%s", obj.key, obj.val)) } } else { - argArr = append(argArr, fmt.Sprintf("%s", obj.key)) + argArr = append(argArr, obj.key) } } - } state.HelmDefaults.Args = argArr diff --git a/pkg/argparser/args_test.go b/pkg/argparser/args_test.go index d12b456a..79925926 100644 --- a/pkg/argparser/args_test.go +++ b/pkg/argparser/args_test.go @@ -1,9 +1,10 @@ package argparser import ( - "github.com/roboll/helmfile/pkg/state" "strings" "testing" + + "github.com/roboll/helmfile/pkg/state" ) func TestGetArgs(t *testing.T) { @@ -44,10 +45,5 @@ func Test2(t *testing.T) { } func compareArgs(expectedArgs string, args []string) bool { - - if strings.Compare(strings.Join(args, " "), expectedArgs) != 0 { - return false - } - return true - + return strings.Compare(strings.Join(args, " "), expectedArgs) == 0 } diff --git a/pkg/event/bus_test.go b/pkg/event/bus_test.go index 42358607..c8eb8465 100644 --- a/pkg/event/bus_test.go +++ b/pkg/event/bus_test.go @@ -3,17 +3,13 @@ package event import ( "fmt" "io" - "os" "testing" "github.com/roboll/helmfile/pkg/environment" - "github.com/roboll/helmfile/pkg/helmexec" "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" ) -var logger = helmexec.NewLogger(os.Stdout, "warn") - type runner struct { } diff --git a/pkg/exectest/helm.go b/pkg/exectest/helm.go index 6319ce6e..5da0cb7a 100644 --- a/pkg/exectest/helm.go +++ b/pkg/exectest/helm.go @@ -79,10 +79,8 @@ func (helm *Helm) BuildDeps(name, chart string) error { } func (helm *Helm) SetExtraArgs(args ...string) { - return } func (helm *Helm) SetHelmBinary(bin string) { - return } func (helm *Helm) AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error { helm.Repo = []string{name, repository, cafile, certfile, keyfile, username, password, managed, passCredentials, skipTLSVerify} diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index 13463df2..8958a999 100644 --- a/pkg/helmexec/exec_test.go +++ b/pkg/helmexec/exec_test.go @@ -89,10 +89,14 @@ func Test_AddRepo_Helm_3_3_2(t *testing.T) { kubeContext: "dev", runner: &mockRunner{}, } - helm.AddRepo("myRepo", "https://repo.example.com/", "", "cert.pem", "key.pem", "", "", "", "", "") + err := helm.AddRepo("myRepo", "https://repo.example.com/", "", "cert.pem", "key.pem", "", "", "", "", "") expected := `Adding repo myRepo https://repo.example.com/ exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --force-update --cert-file cert.pem --key-file key.pem ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -102,93 +106,129 @@ func Test_AddRepo(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.AddRepo("myRepo", "https://repo.example.com/", "", "cert.pem", "key.pem", "", "", "", "", "") + err := helm.AddRepo("myRepo", "https://repo.example.com/", "", "cert.pem", "key.pem", "", "", "", "", "") expected := `Adding repo myRepo https://repo.example.com/ exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --cert-file cert.pem --key-file key.pem ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("myRepo", "https://repo.example.com/", "ca.crt", "", "", "", "", "", "", "") + err = helm.AddRepo("myRepo", "https://repo.example.com/", "ca.crt", "", "", "", "", "", "", "") expected = `Adding repo myRepo https://repo.example.com/ exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --ca-file ca.crt ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "", "", "", "", "") + err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "", "", "", "", "") expected = `Adding repo myRepo https://repo.example.com/ exec: helm --kube-context dev repo add myRepo https://repo.example.com/ ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("acrRepo", "", "", "", "", "", "", "acr", "", "") + err = helm.AddRepo("acrRepo", "", "", "", "", "", "", "acr", "", "") expected = `Adding repo acrRepo (acr) exec: az acr helm repo add --name acrRepo exec: az acr helm repo add --name acrRepo: ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("otherRepo", "", "", "", "", "", "", "unknown", "", "") + err = helm.AddRepo("otherRepo", "", "", "", "", "", "", "unknown", "", "") expected = `ERROR: unknown type 'unknown' for repository otherRepo ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", "", "") + err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", "", "") expected = `Adding repo myRepo https://repo.example.com/ exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --username example_user --password example_password ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("", "https://repo.example.com/", "", "", "", "", "", "", "", "") + err = helm.AddRepo("", "https://repo.example.com/", "", "", "", "", "", "", "", "") expected = `empty field name ` + if err != nil && err.Error() != "empty field name" { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", "true", "") + err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", "true", "") expected = `Adding repo myRepo https://repo.example.com/ exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --username example_user --password example_password --pass-credentials ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "", "", "", "", "true") + err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "", "", "", "", "true") expected = `Adding repo myRepo https://repo.example.com/ exec: helm --kube-context dev repo add myRepo https://repo.example.com/ --insecure-skip-tls-verify ` - + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if buffer.String() != expected { + t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) + } } func Test_UpdateRepo(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.UpdateRepo() + err := helm.UpdateRepo() expected := `Updating repo exec: helm --kube-context dev repo update ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.UpdateRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -198,19 +238,25 @@ func Test_SyncRelease(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.SyncRelease(HelmContext{}, "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs") + err := helm.SyncRelease(HelmContext{}, "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs") expected := `Upgrading release=release, chart=chart exec: helm --kube-context dev upgrade --install --reset-values release chart --timeout 10 --wait --wait-for-jobs ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.SyncRelease(HelmContext{}, "release", "chart") + err = helm.SyncRelease(HelmContext{}, "release", "chart") expected = `Upgrading release=release, chart=chart exec: helm --kube-context dev upgrade --install --reset-values release chart ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -220,11 +266,14 @@ func Test_SyncReleaseTillerless(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.SyncRelease(HelmContext{Tillerless: true, TillerNamespace: "foo"}, "release", "chart", + err := helm.SyncRelease(HelmContext{Tillerless: true, TillerNamespace: "foo"}, "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs") expected := `Upgrading release=release, chart=chart exec: helm --kube-context dev tiller run foo -- helm upgrade --install --reset-values release chart --timeout 10 --wait --wait-for-jobs ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -234,20 +283,26 @@ func Test_UpdateDeps(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.UpdateDeps("./chart/foo") + err := helm.UpdateDeps("./chart/foo") expected := `Updating dependency ./chart/foo exec: helm --kube-context dev dependency update ./chart/foo ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.UpdateDeps()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() helm.SetExtraArgs("--verify") - helm.UpdateDeps("./chart/foo") + err = helm.UpdateDeps("./chart/foo") expected = `Updating dependency ./chart/foo exec: helm --kube-context dev dependency update ./chart/foo --verify ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -257,20 +312,26 @@ func Test_BuildDeps(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.BuildDeps("foo", "./chart/foo") + err := helm.BuildDeps("foo", "./chart/foo") expected := `Building dependency release=foo, chart=./chart/foo exec: helm --kube-context dev dependency build ./chart/foo ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.BuildDeps()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() helm.SetExtraArgs("--verify") - helm.BuildDeps("foo", "./chart/foo") + err = helm.BuildDeps("foo", "./chart/foo") expected = `Building dependency release=foo, chart=./chart/foo exec: helm --kube-context dev dependency build ./chart/foo --verify ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.BuildDeps()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -286,13 +347,19 @@ func Test_DecryptSecret(t *testing.T) { return tmpFilePath, nil } - helm.DecryptSecret(HelmContext{}, "secretName") + _, err := helm.DecryptSecret(HelmContext{}, "secretName") + if err != nil { + if _, ok := err.(*os.PathError); ok { + } else { + t.Errorf("Error: %v", err) + } + } cwd, err := filepath.Abs(".") if err != nil { t.Errorf("Error: %v", err) } // Run again for caching - helm.DecryptSecret(HelmContext{}, "secretName") + _, err = helm.DecryptSecret(HelmContext{}, "secretName") expected := fmt.Sprintf(`Preparing to decrypt secret %v/secretName Decrypting secret %s/secretName @@ -300,6 +367,12 @@ exec: helm --kube-context dev secrets dec %s/secretName Preparing to decrypt secret %s/secretName Found secret in cache %s/secretName `, cwd, cwd, cwd, cwd, cwd) + if err != nil { + if _, ok := err.(*os.PathError); ok { + } else { + t.Errorf("Error: %v", err) + } + } if d := cmp.Diff(expected, buffer.String()); d != "" { t.Errorf("helmexec.DecryptSecret(): want (-), got (+):\n%s", d) } @@ -332,19 +405,25 @@ func Test_DiffRelease(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.DiffRelease(HelmContext{}, "release", "chart", false, "--timeout 10", "--wait", "--wait-for-jobs") + err := helm.DiffRelease(HelmContext{}, "release", "chart", false, "--timeout 10", "--wait", "--wait-for-jobs") expected := `Comparing release=release, chart=chart exec: helm --kube-context dev diff upgrade --reset-values --allow-unreleased release chart --timeout 10 --wait --wait-for-jobs ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.DiffRelease(HelmContext{}, "release", "chart", false) + err = helm.DiffRelease(HelmContext{}, "release", "chart", false) expected = `Comparing release=release, chart=chart exec: helm --kube-context dev diff upgrade --reset-values --allow-unreleased release chart ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -354,10 +433,13 @@ func Test_DiffReleaseTillerless(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.DiffRelease(HelmContext{Tillerless: true}, "release", "chart", false, "--timeout 10", "--wait", "--wait-for-jobs") + err := helm.DiffRelease(HelmContext{Tillerless: true}, "release", "chart", false, "--timeout 10", "--wait", "--wait-for-jobs") expected := `Comparing release=release, chart=chart exec: helm --kube-context dev tiller run -- helm diff upgrade --reset-values --allow-unreleased release chart --timeout 10 --wait --wait-for-jobs ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -367,10 +449,13 @@ func Test_DeleteRelease(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.DeleteRelease(HelmContext{}, "release") + err := helm.DeleteRelease(HelmContext{}, "release") expected := `Deleting release exec: helm --kube-context dev delete release ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.DeleteRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -379,10 +464,13 @@ func Test_DeleteRelease_Flags(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.DeleteRelease(HelmContext{}, "release", "--purge") + err := helm.DeleteRelease(HelmContext{}, "release", "--purge") expected := `Deleting release exec: helm --kube-context dev delete release --purge ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.DeleteRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -392,10 +480,13 @@ func Test_TestRelease(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.TestRelease(HelmContext{}, "release") + err := helm.TestRelease(HelmContext{}, "release") expected := `Testing release exec: helm --kube-context dev test release ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.TestRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -404,10 +495,13 @@ func Test_TestRelease_Flags(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.TestRelease(HelmContext{}, "release", "--cleanup", "--timeout", "60") + err := helm.TestRelease(HelmContext{}, "release", "--cleanup", "--timeout", "60") expected := `Testing release exec: helm --kube-context dev test release --cleanup --timeout 60 ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.TestRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -417,10 +511,13 @@ func Test_ReleaseStatus(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.ReleaseStatus(HelmContext{}, "myRelease") + err := helm.ReleaseStatus(HelmContext{}, "myRelease") expected := `Getting status myRelease exec: helm --kube-context dev status myRelease ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.ReleaseStatus()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -431,9 +528,12 @@ func Test_exec(t *testing.T) { logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "") env := map[string]string{} - helm.exec([]string{"version"}, env) + _, err := helm.exec([]string{"version"}, env) expected := `exec: helm version ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -446,26 +546,35 @@ func Test_exec(t *testing.T) { buffer.Reset() helm = MockExecer(logger, "dev") - helm.exec([]string{"diff", "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs"}, env) + _, err = helm.exec([]string{"diff", "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs"}, env) expected = `exec: helm --kube-context dev diff release chart --timeout 10 --wait --wait-for-jobs ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() - helm.exec([]string{"version"}, env) + _, err = helm.exec([]string{"version"}, env) expected = `exec: helm --kube-context dev version ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() helm.SetExtraArgs("foo") - helm.exec([]string{"version"}, env) + _, err = helm.exec([]string{"version"}, env) expected = `exec: helm --kube-context dev version foo ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -473,9 +582,12 @@ func Test_exec(t *testing.T) { buffer.Reset() helm = MockExecer(logger, "") helm.SetHelmBinary("overwritten") - helm.exec([]string{"version"}, env) + _, err = helm.exec([]string{"version"}, env) expected = `exec: overwritten version ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -485,10 +597,13 @@ func Test_Lint(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.Lint("release", "path/to/chart", "--values", "file.yml") + err := helm.Lint("release", "path/to/chart", "--values", "file.yml") expected := `Linting release=release, chart=path/to/chart exec: helm --kube-context dev lint path/to/chart --values file.yml ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.Lint()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -498,10 +613,13 @@ func Test_Fetch(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.Fetch("chart", "--version", "1.2.3", "--untar", "--untardir", "/tmp/dir") + err := helm.Fetch("chart", "--version", "1.2.3", "--untar", "--untardir", "/tmp/dir") expected := `Fetching chart exec: helm --kube-context dev fetch chart --version 1.2.3 --untar --untardir /tmp/dir ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.Lint()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -522,7 +640,10 @@ func Test_LogLevels(t *testing.T) { buffer.Reset() logger := NewLogger(&buffer, logLevel) helm := MockExecer(logger, "") - helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", "", "") + err := helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", "", "") + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -567,10 +688,13 @@ func Test_Template(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") helm := MockExecer(logger, "dev") - helm.TemplateRelease("release", "path/to/chart", "--values", "file.yml") + err := helm.TemplateRelease("release", "path/to/chart", "--values", "file.yml") expected := `Templating release=release, chart=path/to/chart exec: helm --kube-context dev template path/to/chart --name release --values file.yml ` + if err != nil { + t.Errorf("unexpected error: %v", err) + } if buffer.String() != expected { t.Errorf("helmexec.Template()\nactual = %v\nexpect = %v", buffer.String(), expected) } diff --git a/pkg/helmexec/runner.go b/pkg/helmexec/runner.go index 032e5164..ec573d71 100644 --- a/pkg/helmexec/runner.go +++ b/pkg/helmexec/runner.go @@ -14,11 +14,6 @@ import ( "go.uber.org/zap" ) -const ( - tmpPrefix = "helmfile-" - tmpSuffix = "-exec" -) - // Runner interface for shell commands type Runner interface { Execute(cmd string, args []string, env map[string]string) ([]byte, error) diff --git a/pkg/maputil/maputil.go b/pkg/maputil/maputil.go index 25930e4c..b49e1a45 100644 --- a/pkg/maputil/maputil.go +++ b/pkg/maputil/maputil.go @@ -158,11 +158,11 @@ func ParseKey(key string) []string { for _, rune := range key { split := false switch { - case escaped == false && rune == '\\': + case !escaped && rune == '\\': escaped = true continue case rune == '.': - split = escaped == false + split = !escaped } escaped = false if split { diff --git a/pkg/state/create.go b/pkg/state/create.go index 9ad20be1..43d7c010 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -358,13 +358,13 @@ func (c *StateCreator) scatterGatherEnvSecretFiles(st *HelmState, envSecretFiles for _, err := range errs { st.logger.Error(err) } - return fmt.Errorf("Failed loading environment secrets with %d errors", len(errs)) + return fmt.Errorf("failed loading environment secrets with %d errors", len(errs)) } return nil } func (st *HelmState) loadValuesEntries(missingFileHandler *string, entries []interface{}, remote *remote.Remote, ctxEnv *environment.Environment) (map[string]interface{}, error) { - envVals := map[string]interface{}{} + var envVals map[string]interface{} valuesEntries := append([]interface{}{}, entries...) ld := NewEnvironmentValuesLoader(st.storage(), st.readFile, st.logger, remote) diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index a69e3c18..5f897ffb 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -140,9 +140,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp filesNeedCleaning = append(filesNeedCleaning, generatedFiles...) - for _, f := range generatedFiles { - c.Opts.JsonPatches = append(c.Opts.JsonPatches, f) - } + c.Opts.JsonPatches = append(c.Opts.JsonPatches, generatedFiles...) shouldRun = true } @@ -154,9 +152,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp return nil, clean, err } - for _, f := range generatedFiles { - c.Opts.StrategicMergePatches = append(c.Opts.StrategicMergePatches, f) - } + c.Opts.StrategicMergePatches = append(c.Opts.StrategicMergePatches, generatedFiles...) filesNeedCleaning = append(filesNeedCleaning, generatedFiles...) @@ -170,9 +166,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp return nil, clean, err } - for _, f := range generatedFiles { - c.Opts.Transformers = append(c.Opts.Transformers, f) - } + c.Opts.Transformers = append(c.Opts.Transformers, generatedFiles...) filesNeedCleaning = append(filesNeedCleaning, generatedFiles...) diff --git a/pkg/state/release_filters.go b/pkg/state/release_filters.go index 5996d665..801e466b 100644 --- a/pkg/state/release_filters.go +++ b/pkg/state/release_filters.go @@ -54,15 +54,17 @@ func ParseLabels(l string) (LabelFilter, error) { lf.negativeLabels = [][]string{} var err error labels := strings.Split(l, ",") + reMissmatch := regexp.MustCompile("^[a-zA-Z0-9_-]+!=[a-zA-Z0-9_-]+$") + reMatch := regexp.MustCompile("^[a-zA-Z0-9_-]+=[a-zA-Z0-9_-]+$") for _, label := range labels { - if match, _ := regexp.MatchString("^[a-zA-Z0-9_-]+!=[a-zA-Z0-9_-]+$", label); match == true { // k!=v case + if match := reMissmatch.MatchString(label); match { // k!=v case kv := strings.Split(label, "!=") lf.negativeLabels = append(lf.negativeLabels, kv) - } else if match, _ := regexp.MatchString("^[a-zA-Z0-9_-]+=[a-zA-Z0-9_-]+$", label); match == true { // k=v case + } else if match := reMatch.MatchString(label); match { // k=v case kv := strings.Split(label, "=") lf.positiveLabels = append(lf.positiveLabels, kv) } else { // malformed case - return lf, fmt.Errorf("Malformed label: %s. Expected label in form k=v or k!=v", label) + return lf, fmt.Errorf("malformed label: %s. Expected label in form k=v or k!=v", label) } } return lf, err diff --git a/pkg/state/state.go b/pkg/state/state.go index 85f85018..4aa1817e 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -97,7 +97,6 @@ type HelmState struct { tempDir func(string, string) (string, error) directoryExistsAt func(string) bool - runner helmexec.Runner valsRuntime vals.Evaluator // RenderedValues is the helmfile-wide values that is `.Values` @@ -555,14 +554,12 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu }, func() { for i := 0; i < numReleases; { - select { - case r := <-results: - for _, e := range r.errors { - errs = append(errs, e) - } - res = append(res, r) - i++ + r := <-results + for _, e := range r.errors { + errs = append(errs, e) } + res = append(res, r) + i++ } }, ) @@ -739,12 +736,10 @@ func (st *HelmState) DeleteReleasesForSync(affectedReleases *AffectedReleases, h }, func() { for i := 0; i < len(releases); { - select { - case res := <-results: - if len(res.errors) > 0 { - for _, e := range res.errors { - errs = append(errs, e) - } + res := <-results + if len(res.errors) > 0 { + for _, e := range res.errors { + errs = append(errs, e) } } i++ @@ -875,12 +870,10 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme }, func() { for i := 0; i < len(preps); { - select { - case res := <-results: - if len(res.errors) > 0 { - for _, e := range res.errors { - errs = append(errs, e) - } + res := <-results + if len(res.errors) > 0 { + for _, e := range res.errors { + errs = append(errs, e) } } i++ @@ -1722,7 +1715,7 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu } if opts.Output != "" { - flags = append(flags, "--output", fmt.Sprintf("%s", opts.Output)) + flags = append(flags, "--output", opts.Output) } if opts.Set != nil { @@ -2028,8 +2021,7 @@ func (st *HelmState) Clean() []error { func (st *HelmState) GetReleasesWithOverrides() []ReleaseSpec { var rs []ReleaseSpec for _, r := range st.Releases { - var spec ReleaseSpec - spec = r + spec := r st.ApplyOverrides(&spec) rs = append(rs, spec) } @@ -2296,15 +2288,10 @@ func (st *HelmState) UpdateDeps(helm helmexec.Interface, includeTransitiveNeeds return nil } -func chartNameWithoutRepository(chart string) string { - chartSplit := strings.Split(chart, "/") - return chartSplit[len(chartSplit)-1] -} - // find "Chart.yaml" func findChartDirectory(topLevelDir string) (string, error) { var files []string - filepath.Walk(topLevelDir, func(path string, f os.FileInfo, err error) error { + err := filepath.Walk(topLevelDir, func(path string, f os.FileInfo, err error) error { if err != nil { return fmt.Errorf("error walking through %s: %v", path, err) } @@ -2316,6 +2303,9 @@ func findChartDirectory(topLevelDir string) (string, error) { } return nil }) + if err != nil { + return topLevelDir, err + } // Sort to get the shortest path sort.Strings(files) if len(files) > 0 { @@ -2323,15 +2313,13 @@ func findChartDirectory(topLevelDir string) (string, error) { return first, nil } - return topLevelDir, errors.New("No Chart.yaml found") + return topLevelDir, errors.New("no Chart.yaml found") } // appendConnectionFlags append all the helm command-line flags related to K8s API and Tiller connection including the kubecontext func (st *HelmState) appendConnectionFlags(flags []string, helm helmexec.Interface, release *ReleaseSpec) []string { adds := st.connectionFlags(helm, release) - for _, a := range adds { - flags = append(flags, a) - } + flags = append(flags, adds...) return flags } @@ -2963,7 +2951,10 @@ func (ar *AffectedReleases) DisplayAffectedReleases(logger *zap.SugaredLogger) { ) tbl.Separator = " " for _, release := range ar.Upgraded { - tbl.AddRow(release.Name, release.Chart, release.installedVersion) + err := tbl.AddRow(release.Name, release.Chart, release.installedVersion) + if err != nil { + logger.Warn("Could not add row, %v", err) + } } logger.Info(tbl.String()) } @@ -3041,7 +3032,7 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(interface{}) error) erro } //also exclude SelectorsInherited to true and explicit selectors if hf.SelectorsInherited && len(hf.Selectors) > 0 { - return fmt.Errorf("You cannot use 'SelectorsInherited: true' along with and explicit selector for path: %v", hf.Path) + return fmt.Errorf("you cannot use 'SelectorsInherited: true' along with and explicit selector for path: %v", hf.Path) } return nil } @@ -3056,7 +3047,10 @@ func (st *HelmState) GenerateOutputDir(outputDir string, release *ReleaseSpec, o } hasher := sha1.New() - io.WriteString(hasher, stateAbsPath) + _, err = io.WriteString(hasher, stateAbsPath) + if err != nil { + return "", err + } var stateFileExtension = filepath.Ext(st.FilePath) var stateFileName = st.FilePath[0 : len(st.FilePath)-len(stateFileExtension)] @@ -3120,7 +3114,10 @@ func (st *HelmState) GenerateOutputFilePath(release *ReleaseSpec, outputFileTemp } hasher := sha1.New() - io.WriteString(hasher, stateAbsPath) + _, err = io.WriteString(hasher, stateAbsPath) + if err != nil { + return "", err + } var stateFileExtension = filepath.Ext(st.FilePath) var stateFileName = st.FilePath[0 : len(st.FilePath)-len(stateFileExtension)] diff --git a/pkg/state/state_exec_tmpl.go b/pkg/state/state_exec_tmpl.go index e34d5b6f..16352087 100644 --- a/pkg/state/state_exec_tmpl.go +++ b/pkg/state/state_exec_tmpl.go @@ -90,27 +90,28 @@ func (st *HelmState) ExecuteTemplates() (*HelmState, error) { vals := st.Values() for i, rt := range st.Releases { - if rt.KubeContext == "" { - rt.KubeContext = r.HelmDefaults.KubeContext + release := rt + if release.KubeContext == "" { + release.KubeContext = r.HelmDefaults.KubeContext } - if rt.Labels == nil { - rt.Labels = map[string]string{} + if release.Labels == nil { + release.Labels = map[string]string{} } for k, v := range st.CommonLabels { - rt.Labels[k] = v + release.Labels[k] = v } successFlag := false - for it, prev := 0, &rt; it < 6; it++ { + for it, prev := 0, &release; it < 6; it++ { tmplData := st.createReleaseTemplateData(prev, vals) renderer := tmpl.NewFileRenderer(st.readFile, st.basePath, tmplData) - r, err := rt.ExecuteTemplateExpressions(renderer) + r, err := release.ExecuteTemplateExpressions(renderer) if err != nil { - return nil, fmt.Errorf("failed executing templates in release \"%s\".\"%s\": %v", st.FilePath, rt.Name, err) + return nil, fmt.Errorf("failed executing templates in release \"%s\".\"%s\": %v", st.FilePath, release.Name, err) } if reflect.DeepEqual(prev, r) { successFlag = true if err := updateBoolTemplatedValues(r); err != nil { - return nil, fmt.Errorf("failed executing templates in release \"%s\".\"%s\": %v", st.FilePath, rt.Name, err) + return nil, fmt.Errorf("failed executing templates in release \"%s\".\"%s\": %v", st.FilePath, release.Name, err) } st.Releases[i] = *r break @@ -118,7 +119,7 @@ func (st *HelmState) ExecuteTemplates() (*HelmState, error) { prev = r } if !successFlag { - return nil, fmt.Errorf("failed executing templates in release \"%s\".\"%s\": %s", st.FilePath, rt.Name, + return nil, fmt.Errorf("failed executing templates in release \"%s\".\"%s\": %s", st.FilePath, release.Name, "recursive references can't be resolved") } } diff --git a/pkg/state/state_exec_tmpl_test.go b/pkg/state/state_exec_tmpl_test.go index 23f7d9da..bfad30fb 100644 --- a/pkg/state/state_exec_tmpl_test.go +++ b/pkg/state/state_exec_tmpl_test.go @@ -17,11 +17,6 @@ func boolPtrToString(ptr *bool) string { return fmt.Sprintf("&%t", *ptr) } -func ptr(v interface{}) interface{} { - r := v - return reflect.ValueOf(r).Addr().Interface() -} - func TestHelmState_executeTemplates(t *testing.T) { tests := []struct { name string diff --git a/pkg/state/state_gogetter_test.go b/pkg/state/state_gogetter_test.go index 973d3515..c460fa92 100644 --- a/pkg/state/state_gogetter_test.go +++ b/pkg/state/state_gogetter_test.go @@ -2,11 +2,12 @@ package state import ( "fmt" - "github.com/google/go-cmp/cmp" - "github.com/roboll/helmfile/pkg/helmexec" "io/ioutil" "os" "testing" + + "github.com/google/go-cmp/cmp" + "github.com/roboll/helmfile/pkg/helmexec" ) func TestGoGetter(t *testing.T) { @@ -28,6 +29,7 @@ func TestGoGetter(t *testing.T) { } for i, tc := range testcases { + test := tc t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { d, err := ioutil.TempDir("", "testgogetter") if err != nil { @@ -41,9 +43,9 @@ func TestGoGetter(t *testing.T) { basePath: d, } - out, err := st.goGetterChart(tc.chart, tc.dir, "", false) + out, err := st.goGetterChart(test.chart, test.dir, "", false) - if diff := cmp.Diff(tc.out, out); diff != "" { + if diff := cmp.Diff(test.out, out); diff != "" { t.Fatalf("Unexpected out:\n%s", diff) } @@ -53,7 +55,7 @@ func TestGoGetter(t *testing.T) { errMsg = err.Error() } - if diff := cmp.Diff(tc.err, errMsg); diff != "" { + if diff := cmp.Diff(test.err, errMsg); diff != "" { t.Fatalf("Unexpected err:\n%s", diff) } }) diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index 038620e8..62f0b681 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -166,7 +166,8 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas var selectedReleaseIDs []string for _, r := range opts.SelectedReleases { - id := ReleaseToID(&r) + release := r + id := ReleaseToID(&release) selectedReleaseIDs = append(selectedReleaseIDs, id) } diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 698890fd..e35d6fc0 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -149,14 +149,6 @@ func boolValue(v bool) *bool { return &v } -// Mocking the command-line runner - -type mockRunner struct{} - -func (mock *mockRunner) Execute(cmd string, args []string, env map[string]string) ([]byte, error) { - return []byte{}, nil -} - func TestHelmState_flagsForUpgrade(t *testing.T) { enable := true disable := false @@ -1122,7 +1114,7 @@ func TestHelmState_SyncReleases(t *testing.T) { valsRuntime: valsRuntime, RenderedValues: map[string]interface{}{}, } - if errs := state.SyncReleases(&AffectedReleases{}, tt.helm, []string{}, 1); errs != nil && len(errs) > 0 { + if errs := state.SyncReleases(&AffectedReleases{}, tt.helm, []string{}, 1); len(errs) > 0 { if len(errs) != len(tt.wantErrorMsgs) { t.Fatalf("Unexpected errors: %v\nExpected: %v", errs, tt.wantErrorMsgs) } @@ -1613,7 +1605,7 @@ func TestHelmState_DiffReleases(t *testing.T) { RenderedValues: map[string]interface{}{}, } _, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, []string{}, false, false, false, false) - if errs != nil && len(errs) > 0 { + if len(errs) > 0 { t.Errorf("unexpected error: %v", errs) } if !reflect.DeepEqual(tt.helm.Diffed, tt.wantReleases) { @@ -1695,11 +1687,11 @@ func TestHelmState_SyncReleasesCleanup(t *testing.T) { "/path/to/someFile": `foo: FOO`, }) state = injectFs(state, testfs) - if errs := state.SyncReleases(&AffectedReleases{}, tt.helm, []string{}, 1); errs != nil && len(errs) > 0 { + if errs := state.SyncReleases(&AffectedReleases{}, tt.helm, []string{}, 1); len(errs) > 0 { t.Errorf("unexpected errors: %v", errs) } - if errs := state.Clean(); errs != nil && len(errs) > 0 { + if errs := state.Clean(); len(errs) > 0 { t.Errorf("unexpected errors: %v", errs) } @@ -1783,11 +1775,11 @@ func TestHelmState_DiffReleasesCleanup(t *testing.T) { `, }) state = injectFs(state, testfs) - if _, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, []string{}, false, false, false, false); errs != nil && len(errs) > 0 { + if _, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, []string{}, false, false, false, false); len(errs) > 0 { t.Errorf("unexpected errors: %v", errs) } - if errs := state.Clean(); errs != nil && len(errs) > 0 { + if errs := state.Clean(); len(errs) > 0 { t.Errorf("unexpected errors: %v", errs) } diff --git a/pkg/state/temp.go b/pkg/state/temp.go index 71a8406c..6a38a3e0 100644 --- a/pkg/state/temp.go +++ b/pkg/state/temp.go @@ -42,11 +42,9 @@ func tempValuesFilePath(release *ReleaseSpec, data interface{}) (*string, error) d := filepath.Join(workDir, id) - info, err := os.Stat(d) + _, err = os.Stat(d) if err != nil && !errors.Is(err, os.ErrNotExist) { panic(err) - } else if info == nil { - } return &d, nil diff --git a/pkg/state/util.go b/pkg/state/util.go index a3204cd0..805e74aa 100644 --- a/pkg/state/util.go +++ b/pkg/state/util.go @@ -13,14 +13,14 @@ func isLocalChart(chart string) bool { return true } - uriLike := strings.Index(chart, "://") > -1 + uriLike := strings.Contains(chart, "://") if uriLike { return false } return chart == "" || chart[0] == '/' || - strings.Index(chart, "/") == -1 || + !strings.Contains(chart, "/") || (len(strings.Split(chart, "/")) != 2 && len(strings.Split(chart, "/")) != 3) } @@ -30,7 +30,7 @@ func resolveRemoteChart(repoAndChart string) (string, string, bool) { return "", "", false } - uriLike := strings.Index(repoAndChart, "://") > -1 + uriLike := strings.Contains(repoAndChart, "://") if uriLike { return "", "", false } diff --git a/pkg/testhelper/testfs.go b/pkg/testhelper/testfs.go index 41a7d9ca..7378409a 100644 --- a/pkg/testhelper/testfs.go +++ b/pkg/testhelper/testfs.go @@ -20,7 +20,7 @@ type TestFs struct { func NewTestFs(files map[string]string) *TestFs { dirs := map[string]bool{} - for abs, _ := range files { + for abs := range files { for d := filepath.ToSlash(filepath.Dir(abs)); !dirs[d]; d = filepath.ToSlash(filepath.Dir(d)) { dirs[d] = true fmt.Fprintf(os.Stderr, "testfs: recognized dir: %s\n", d) @@ -102,7 +102,7 @@ func (f *TestFs) Glob(relPattern string) ([]string, error) { } matches := []string{} - for name, _ := range f.files { + for name := range f.files { matched, err := filepath.Match(pattern, name) if err != nil { return nil, err diff --git a/pkg/tmpl/context_funcs.go b/pkg/tmpl/context_funcs.go index 567e4bb9..57b7cb6b 100644 --- a/pkg/tmpl/context_funcs.go +++ b/pkg/tmpl/context_funcs.go @@ -56,7 +56,7 @@ func (c *Context) Exec(command string, args []interface{}, inputs ...string) (st for i, a := range args { switch a.(type) { case string: - strArgs[i] = a.(string) + strArgs[i] = fmt.Sprintf("%v", a) default: return "", fmt.Errorf("unexpected type of arg \"%s\" in args %v at index %d", reflect.TypeOf(a), args, i) } diff --git a/pkg/tmpl/context_funcs_test.go b/pkg/tmpl/context_funcs_test.go index cfc0a21d..43c69ba6 100644 --- a/pkg/tmpl/context_funcs_test.go +++ b/pkg/tmpl/context_funcs_test.go @@ -3,10 +3,11 @@ package tmpl import ( "encoding/json" "fmt" - "github.com/google/go-cmp/cmp" "path/filepath" "reflect" "testing" + + "github.com/google/go-cmp/cmp" ) func TestReadFile(t *testing.T) { @@ -207,14 +208,15 @@ func TestRequired(t *testing.T) { }, } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := Required(tt.args.warn, tt.args.val) - if (err != nil) != tt.wantErr { - t.Errorf("Required() error = %v, wantErr %v", err, tt.wantErr) + testCase := tt + t.Run(testCase.name, func(t *testing.T) { + got, err := Required(testCase.args.warn, testCase.args.val) + if (err != nil) != testCase.wantErr { + t.Errorf("Required() error = %v, wantErr %v", err, testCase.wantErr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("Required() got = %v, want %v", got, tt.want) + if !reflect.DeepEqual(got, testCase.want) { + t.Errorf("Required() got = %v, want %v", got, testCase.want) } }) } diff --git a/pkg/tmpl/expand_secret_ref.go b/pkg/tmpl/expand_secret_ref.go index 2e926df6..33d9bcb4 100644 --- a/pkg/tmpl/expand_secret_ref.go +++ b/pkg/tmpl/expand_secret_ref.go @@ -1,11 +1,11 @@ package tmpl import ( - "errors" "fmt" + "sync" + "github.com/roboll/helmfile/pkg/plugins" "github.com/variantdev/vals" - "sync" ) //to generate mock run mockgen -source=expand_secret_ref.go -destination=expand_secrets_mock.go -package=tmpl @@ -26,12 +26,12 @@ func fetchSecretValue(path string) (string, error) { rendered, ok := resultMap["key"] if !ok { - return "", errors.New(fmt.Sprintf("unexpected error occurred, %v doesn't have 'key' key", resultMap)) + return "", fmt.Errorf("unexpected error occurred, %v doesn't have 'key' key", resultMap) } result, ok := rendered.(string) if !ok { - return "", errors.New(fmt.Sprintf("expected %v to be string", rendered)) + return "", fmt.Errorf("expected %v to be string", rendered) } return result, nil diff --git a/test/diff-yamls/diff-yamls.go b/test/diff-yamls/diff-yamls.go index c53bb9eb..2f40b581 100644 --- a/test/diff-yamls/diff-yamls.go +++ b/test/diff-yamls/diff-yamls.go @@ -271,5 +271,10 @@ func (p *pair) add(node resource, source diffSource) error { } func main() { - rootCmd.Execute() + err := rootCmd.Execute() + + if err != nil { + fmt.Println(fmt.Errorf("unexpected error: %v", err)) + os.Exit(1) + } }