diff --git a/pkg/remote/remote_test.go b/pkg/remote/remote_test.go index 357f5707..0f0cc6a3 100644 --- a/pkg/remote/remote_test.go +++ b/pkg/remote/remote_test.go @@ -2,6 +2,7 @@ package remote import ( "fmt" + "github.com/google/go-cmp/cmp" "github.com/roboll/helmfile/pkg/helmexec" "github.com/roboll/helmfile/pkg/testhelper" "os" @@ -156,6 +157,75 @@ func TestRemote_SShGitHub(t *testing.T) { } } +func TestParse(t *testing.T) { + type testcase struct { + input string + getter, scheme, dir, file, query string + err string + } + + testcases := []testcase{ + { + input: "raw/incubator", + err: "parse url: missing scheme - probably this is a local file path? raw/incubator", + }, + { + input: "git::https://github.com/stakater/Forecastle.git@deployments/kubernetes/chart/forecastle?ref=v1.0.54", + getter: "git", + scheme: "https", + dir: "/stakater/Forecastle.git", + file: "deployments/kubernetes/chart/forecastle", + query: "ref=v1.0.54", + }, + } + + for i := range testcases { + tc := testcases[i] + + t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { + src, err := Parse(tc.input) + + var errMsg string + if err != nil { + errMsg = err.Error() + } + + if diff := cmp.Diff(tc.err, errMsg); diff != "" { + t.Fatalf("Unexpected error:\n%s", diff) + } + + var getter, scheme, dir, file, query string + if src != nil { + getter = src.Getter + scheme = src.Scheme + dir = src.Dir + file = src.File + query = src.RawQuery + } + + if diff := cmp.Diff(tc.getter, getter); diff != "" { + t.Fatalf("Unexpected getter:\n%s", diff) + } + + if diff := cmp.Diff(tc.scheme, scheme); diff != "" { + t.Fatalf("Unexpected scheme:\n%s", diff) + } + + if diff := cmp.Diff(tc.file, file); diff != "" { + t.Fatalf("Unexpected file:\n%s", diff) + } + + if diff := cmp.Diff(tc.dir, dir); diff != "" { + t.Fatalf("Unexpected dir:\n%s", diff) + } + + if diff := cmp.Diff(tc.query, query); diff != "" { + t.Fatalf("Unexpected query:\n%s", diff) + } + }) + } +} + type testGetter struct { get func(wd, src, dst string) error } diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index b17674de..611a6fef 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -36,11 +36,34 @@ func directoryExistsAt(path string) bool { type Chartify struct { Opts *chartify.ChartifyOpts - Chart string Clean func() } -func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) (*Chartify, func(), error) { +func (st *HelmState) goGetterChart(chart, dir string, force bool) (string, error) { + if dir != "" && chart == "" { + chart = dir + } + + _, err := remote.Parse(chart) + if err != nil { + if force { + return "", fmt.Errorf("Parsing url from dir failed due to error %q.\nContinuing the process assuming this is a regular Helm chart or a local dir.", err.Error()) + } + } else { + r := remote.NewRemote(st.logger, st.basePath, st.readFile, directoryExistsAt, fileExistsAt) + + fetchedDir, err := r.Fetch(chart) + if err != nil { + return "", fmt.Errorf("fetching %q: %v", chart, err) + } + + chart = fetchedDir + } + + return chart, nil +} + +func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSpec, chart string, workerIndex int) (*Chartify, func(), error) { chartify := &Chartify{ Opts: &chartify.ChartifyOpts{ WorkaroundOutputDirIssue: true, @@ -58,31 +81,6 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp var shouldRun bool - chart := release.Chart - if release.Directory != "" && chart == "" { - chart = release.Directory - } - - _, err := remote.Parse(chart) - if err != nil { - if release.ForceGoGetter { - return nil, clean, fmt.Errorf("Parsing url from directory of release %q failed due to error %q.\nContinuing the process assuming this is a regular Helm chart or a local directory.", release.Name, err.Error()) - } - } else { - r := remote.NewRemote(st.logger, st.basePath, st.readFile, directoryExistsAt, fileExistsAt) - - fetchedDir, err := r.Fetch(chart) - if err != nil { - return nil, clean, fmt.Errorf("fetching %q: %v", chart, err) - } - - chart = fetchedDir - - filesNeedCleaning = append(filesNeedCleaning, fetchedDir) - } - - chartify.Chart = chart - dir := filepath.Join(st.basePath, chart) if stat, _ := os.Stat(dir); stat != nil && stat.IsDir() { if exists, err := st.fileExists(filepath.Join(dir, "Chart.yaml")); err == nil && !exists { diff --git a/pkg/state/state.go b/pkg/state/state.go index 96856143..ee128580 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -859,9 +859,15 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre return } - var chartPath string + chartName := release.Chart - chartification, clean, err := st.PrepareChartify(helm, release, workerIndex) + chartPath, err := st.goGetterChart(chartName, release.Directory, release.ForceGoGetter) + if err != nil { + results <- &downloadResults{err: fmt.Errorf("release %q: %w", release.Name, err)} + return + } + + chartification, clean, err := st.PrepareChartify(helm, release, chartPath, workerIndex) defer clean() if err != nil { results <- &downloadResults{err: err} @@ -874,7 +880,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre chartify.UseHelm3(helm3), ) - out, err := c.Chartify(release.Name, chartification.Chart, chartify.WithChartifyOpts(chartification.Opts)) + out, err := c.Chartify(release.Name, chartPath, chartify.WithChartifyOpts(chartification.Opts)) if err != nil { results <- &downloadResults{err: err} return @@ -883,9 +889,14 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre chartPath = out } } else if !forceDownload { - chartPath = release.Chart - } else if pathExists(normalizeChart(st.basePath, release.Chart)) { - chartPath = normalizeChart(st.basePath, release.Chart) + // At this point, we are sure that either: + // 1. It is a local chart and we can use it in later process (helm upgrade/template/lint/etc) + // without any modification, or + // 2. It is a remote chart which can be safely handed over to helm, + // because the version of Helm used in this transaction support downloading the chart instead, + // and we don't need any modification to the chart + } else if pathExists(normalizeChart(st.basePath, chartPath)) { + chartPath = normalizeChart(st.basePath, chartPath) if !skipDeps { if err := helm.BuildDeps(release.Name, chartPath); err != nil { @@ -914,7 +925,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre fetchFlags = append(fetchFlags, "--version", release.Version) } - pathElems = append(pathElems, release.Name, release.Chart, chartVersion) + pathElems = append(pathElems, release.Name, chartName, chartVersion) chartPath = path.Join(pathElems...) @@ -925,7 +936,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre // only fetch chart if it is not already fetched if _, err := os.Stat(chartPath); os.IsNotExist(err) { fetchFlags = append(fetchFlags, "--untar", "--untardir", chartPath) - if err := helm.Fetch(release.Chart, fetchFlags...); err != nil { + if err := helm.Fetch(chartName, fetchFlags...); err != nil { results <- &downloadResults{err: err} return } diff --git a/pkg/state/state_gogetter_test.go b/pkg/state/state_gogetter_test.go new file mode 100644 index 00000000..006aa2ef --- /dev/null +++ b/pkg/state/state_gogetter_test.go @@ -0,0 +1,61 @@ +package state + +import ( + "fmt" + "github.com/google/go-cmp/cmp" + "github.com/roboll/helmfile/pkg/helmexec" + "io/ioutil" + "os" + "testing" +) + +func TestGoGetter(t *testing.T) { + logger := helmexec.NewLogger(os.Stderr, "warn") + + testcases := []struct { + chart, dir string + force bool + + out, err string + }{ + { + chart: "raw/incubator", + dir: "", + force: false, + out: "raw/incubator", + err: "", + }, + } + + for i, tc := range testcases { + t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { + d, err := ioutil.TempDir("", "testgogetter") + if err != nil { + panic(err) + } + defer os.RemoveAll(d) + + st := &HelmState{ + logger: logger, + readFile: ioutil.ReadFile, + basePath: d, + } + + out, err := st.goGetterChart(tc.chart, tc.dir, false) + + if diff := cmp.Diff(tc.out, out); diff != "" { + t.Fatalf("Unexpected out:\n%s", diff) + } + + var errMsg string + + if err != nil { + errMsg = err.Error() + } + + if diff := cmp.Diff(tc.err, errMsg); diff != "" { + t.Fatalf("Unexpected err:\n%s", diff) + } + }) + } +}