From 751e5492530d6a7095882293d7ecf37467af12fa Mon Sep 17 00:00:00 2001 From: KUOKA Yusuke Date: Sun, 9 Sep 2018 17:20:44 +0900 Subject: [PATCH] fix: prevent panicing on invalid selector format (#323) Fixes #322 --- app_test.go | 104 +++++++++++++++++++++++++++++++++++++++ main.go | 3 +- state/release_filters.go | 2 +- 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/app_test.go b/app_test.go index 03279218..8725252b 100644 --- a/app_test.go +++ b/app_test.go @@ -198,3 +198,107 @@ releases: } } } + +// See https://github.com/roboll/helmfile/issues/322 +func TestFindAndIterateOverDesiredStates_Selectors(t *testing.T) { + absPaths := map[string]string{ + ".": "/path/to", + "/path/to/helmfile.d": "/path/to/helmfile.d", + } + dirs := map[string]bool{ + "helmfile.d": true, + } + files := map[string]string{ + "helmfile.yaml": ` +helmfiles: +- helmfile.d/a*.yaml +- helmfile.d/b*.yaml +`, + "/path/to/helmfile.d/a1.yaml": ` +releases: +- name: zipkin + chart: stable/zipkin +`, + "/path/to/helmfile.d/a2.yaml": ` +releases: +- name: prometheus + chart: stable/prometheus +`, + "/path/to/helmfile.d/b.yaml": ` +releases: +- name: grafana + chart: stable/grafana +`, + } + globMatches := map[string][]string{ + "/path/to/helmfile.d/a*.yaml": []string{"/path/to/helmfile.d/a1.yaml", "/path/to/helmfile.d/a2.yaml"}, + "/path/to/helmfile.d/b*.yaml": []string{"/path/to/helmfile.d/b.yaml"}, + } + fileExistsAt := func(path string) bool { + _, ok := files[path] + return ok + } + directoryExistsAt := func(path string) bool { + _, ok := dirs[path] + return ok + } + readFile := func(filename string) ([]byte, error) { + str, ok := files[filename] + if !ok { + return []byte(nil), fmt.Errorf("no file found: %s", filename) + } + return []byte(str), nil + } + glob := func(pattern string) ([]string, error) { + matches, ok := globMatches[pattern] + if !ok { + return []string(nil), fmt.Errorf("no file matched: %s", pattern) + } + return matches, nil + } + abs := func(path string) (string, error) { + a, ok := absPaths[path] + if !ok { + return "", fmt.Errorf("abs: unexpected path: %s", path) + } + return a, nil + } + app := &app{ + readFile: readFile, + glob: glob, + abs: abs, + fileExistsAt: fileExistsAt, + directoryExistsAt: directoryExistsAt, + kubeContext: "default", + logger: helmexec.NewLogger(os.Stderr, "debug"), + } + noop := func(st *state.HelmState, helm helmexec.Interface) []error { + return []error{} + } + + testcases := []struct { + label string + expectErr bool + errMsg string + }{ + {label: "name=prometheus", expectErr: false}, + {label: "name=", expectErr: true, errMsg: "Malformed label: name=. Expected label in form k=v or k!=v"}, + {label: "name!=", expectErr: true, errMsg: "Malformed label: name!=. Expected label in form k=v or k!=v"}, + {label: "name", expectErr: true, errMsg: "Malformed label: name. Expected label in form k=v or k!=v"}, + } + + for _, testcase := range testcases { + err := app.FindAndIterateOverDesiredStates( + "helmfile.yaml", noop, "", []string{testcase.label}, "default", + ) + if testcase.expectErr { + if err == nil { + t.Errorf("error expected but not happened for name=%s", testcase.label) + } else if err.Error() != testcase.errMsg { + t.Errorf("unexpected error message: expected=\"%s\", actual=\"%s\"", testcase.errMsg, err.Error()) + } + } else if !testcase.expectErr && err != nil { + t.Errorf("unexpected error for name=%s: %v", testcase.label, err) + } + } +} diff --git a/main.go b/main.go index b7150388..3cf93a5b 100644 --- a/main.go +++ b/main.go @@ -751,8 +751,7 @@ func (a *app) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin if len(labels) > 0 { err = st.FilterReleases(labels) if err != nil { - log.Print(err) - return nil, true, nil + return nil, false, err } } diff --git a/state/release_filters.go b/state/release_filters.go index a7a61992..a7bcae9d 100644 --- a/state/release_filters.go +++ b/state/release_filters.go @@ -62,7 +62,7 @@ func ParseLabels(l string) (LabelFilter, error) { kv := strings.Split(label, "=") lf.positiveLabels = append(lf.positiveLabels, kv) } else { // malformed case - err = 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