From 7dec9489509423a3d64c9f68778cba8c2a36dbf3 Mon Sep 17 00:00:00 2001 From: KUOKA Yusuke Date: Sun, 16 Jun 2019 16:40:17 +0900 Subject: [PATCH] fix: helm chart referenced by http URL (#695) We unintentionally broke this since #593. Fixes #675 Fixes #687 --- pkg/app/app_test.go | 51 +++++++++++++++ pkg/state/chart_dependency.go | 15 ----- pkg/state/state.go | 11 ---- pkg/state/util.go | 40 ++++++++++++ pkg/state/util_test.go | 113 ++++++++++++++++++++++++++++++++++ 5 files changed, 204 insertions(+), 26 deletions(-) create mode 100644 pkg/state/util.go create mode 100644 pkg/state/util_test.go diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index bad18fda..77fd5e84 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -1025,6 +1025,57 @@ x: } } +func TestVisitDesiredStatesWithReleasesFiltered_RemoteTgzAsChart(t *testing.T) { + testcases := []struct { + expr, env, expected string + }{ + { + expected: "https://github.com/arangodb/kube-arangodb/releases/download/0.3.11/kube-arangodb-crd.tgz", + }, + } + for i := range testcases { + testcase := testcases[i] + t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { + files := map[string]string{ + "/path/to/helmfile.yaml": ` +releases: + - name: arangodb-crd + chart: https://github.com/arangodb/kube-arangodb/releases/download/0.3.11/kube-arangodb-crd.tgz +`, + } + + actual := []state.ReleaseSpec{} + + collectReleases := func(st *state.HelmState, helm helmexec.Interface) []error { + for _, r := range st.Releases { + actual = append(actual, r) + } + return []error{} + } + app := appWithFs(&App{ + KubeContext: "default", + Logger: helmexec.NewLogger(os.Stderr, "debug"), + Reverse: false, + Namespace: "", + Selectors: []string{}, + Env: "default", + }, files) + err := app.VisitDesiredStatesWithReleasesFiltered( + "helmfile.yaml", collectReleases, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(actual) != 1 { + t.Errorf("unexpected number of processed releases: expected=1, got=%d", len(actual)) + } + if actual[0].Chart != testcase.expected { + t.Errorf("unexpected chart: expected=%s, got=%s", testcase.expected, actual[0].Chart) + } + }) + } +} + func TestLoadDesiredStateFromYaml_DuplicateReleaseName(t *testing.T) { yamlFile := "example/path/to/yaml/file" yamlContent := []byte(`releases: diff --git a/pkg/state/chart_dependency.go b/pkg/state/chart_dependency.go index c8ceda06..1637d872 100644 --- a/pkg/state/chart_dependency.go +++ b/pkg/state/chart_dependency.go @@ -128,21 +128,6 @@ func (d *ResolvedDependencies) Get(chart, versionConstraint string) (string, err return "", fmt.Errorf("no resolved dependency found for \"%s\"", chart) } -func resolveRemoteChart(repoAndChart string) (string, string, bool) { - parts := strings.Split(repoAndChart, "/") - if isLocalChart(repoAndChart) { - return "", "", false - } - if len(parts) != 2 { - panic(fmt.Sprintf("unsupported format of chart name: %s", repoAndChart)) - } - - repo := parts[0] - chart := parts[1] - - return repo, chart, true -} - func (st *HelmState) mergeLockedDependencies() (*HelmState, error) { filename, unresolved, err := getUnresolvedDependenciess(st) if err != nil { diff --git a/pkg/state/state.go b/pkg/state/state.go index 337f6e53..3518d0fc 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1073,17 +1073,6 @@ func normalizeChart(basePath, chart string) string { } return filepath.Join(basePath, chart) } - -func isLocalChart(chart string) bool { - regex, _ := regexp.Compile("^[.]?./") - matched := regex.MatchString(chart) - if matched { - return true - } - - return chart == "" || chart[0] == '/' || len(strings.Split(chart, "/")) != 2 -} - func pathExists(chart string) bool { _, err := os.Stat(chart) return err == nil diff --git a/pkg/state/util.go b/pkg/state/util.go new file mode 100644 index 00000000..03d565c5 --- /dev/null +++ b/pkg/state/util.go @@ -0,0 +1,40 @@ +package state + +import ( + "regexp" + "strings" +) + +func isLocalChart(chart string) bool { + regex, _ := regexp.Compile("^[.]?./") + matched := regex.MatchString(chart) + if matched { + return true + } + + uriLike := strings.Index(chart, "://") > -1 + if uriLike { + return false + } + + return chart == "" || + chart[0] == '/' || + strings.Index(chart, "/") == -1 || + len(strings.Split(chart, "/")) != 2 +} + +func resolveRemoteChart(repoAndChart string) (string, string, bool) { + if isLocalChart(repoAndChart) { + return "", "", false + } + + parts := strings.Split(repoAndChart, "/") + if len(parts) != 2 { + return "", "", false + } + + repo := parts[0] + chart := parts[1] + + return repo, chart, true +} diff --git a/pkg/state/util_test.go b/pkg/state/util_test.go new file mode 100644 index 00000000..1d5d46d9 --- /dev/null +++ b/pkg/state/util_test.go @@ -0,0 +1,113 @@ +package state + +import "testing" + +func TestIsLocalChart(t *testing.T) { + testcases := []struct { + input string + expected bool + }{ + { + input: "mychart", + expected: true, + }, + { + input: "stable/mysql", + expected: false, + }, + { + input: "./charts/myapp", + expected: true, + }, + { + input: "charts/mysubsystem/myapp", + expected: true, + }, + { + input: "./charts/mysubsystem/myapp", + expected: true, + }, + { + // Regression test case for: + // * https://github.com/roboll/helmfile/issues/675 + // * https://github.com/roboll/helmfile/issues/687 + input: "https://github.com/arangodb/kube-arangodb/releases/download/0.3.11/kube-arangodb-crd.tgz", + expected: false, + }, + { + input: "https://example.com/bar.tgz", + expected: false, + }, + } + + for i := range testcases { + testcase := testcases[i] + + actual := isLocalChart(testcase.input) + + if testcase.expected != actual { + t.Errorf("unexpected result: isLocalChart(\"%s\"): expected=%v, got=%v", testcase.input, testcase.expected, actual) + } + } +} + +func TestResolveRemortChart(t *testing.T) { + testcases := []struct { + input string + repo string + chart string + remote bool + }{ + { + input: "mychart", + remote: false, + }, + { + input: "stable/mysql", + repo: "stable", + chart: "mysql", + remote: true, + }, + { + input: "./charts/myapp", + remote: false, + }, + { + input: "charts/mysubsystem/myapp", + remote: false, + }, + { + input: "./charts/mysubsystem/myapp", + remote: false, + }, + { + // Regression test case for: + // * https://github.com/roboll/helmfile/issues/675 + // * https://github.com/roboll/helmfile/issues/687 + input: "https://github.com/arangodb/kube-arangodb/releases/download/0.3.11/kube-arangodb-crd.tgz", + remote: false, + }, + { + input: "https://example.com/bar.tgz", + remote: false, + }, + } + + for i := range testcases { + testcase := testcases[i] + + repo, chart, actual := resolveRemoteChart(testcase.input) + + if testcase.remote != actual { + t.Fatalf("unexpected result: reolveRemoteChart(\"%s\"): expected=%v, got=%v", testcase.input, testcase.remote, actual) + } + + if testcase.repo != repo { + t.Errorf("unexpected repo: %s: expected=%v, got=%v", testcase.input, testcase.repo, repo) + } + + if testcase.chart != chart { + t.Errorf("unexpected chart: %s: expected=%v, got=%v", testcase.input, testcase.chart, chart) + } + } +}