From 2333f093c11a858a683a695fa52d28b1eee7c8d9 Mon Sep 17 00:00:00 2001 From: Purple Clay Date: Mon, 13 Jan 2025 12:55:23 +0000 Subject: [PATCH] fix: ensure development versions of charts can be used across helmfile commands (#1865) Signed-off-by: purpleclay --- pkg/app/app_template_test.go | 16 ++++++------ pkg/helmexec/exec.go | 4 +-- pkg/helmexec/exec_test.go | 12 ++++----- pkg/state/state.go | 47 ++++++++++++++++++++---------------- pkg/state/state_test.go | 23 ++++++++++++++++++ 5 files changed, 65 insertions(+), 37 deletions(-) diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index 0ffb26ce..921bcb84 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -236,7 +236,7 @@ releases: }, selectors: []string{"name=test2"}, templated: []exectest.Release{ - {Name: "test2", Flags: []string{}}, + {Name: "test2"}, }, }) }) @@ -249,8 +249,8 @@ releases: }, selectors: []string{"name=test3"}, templated: []exectest.Release{ - {Name: "test2", Flags: []string{}}, - {Name: "test3", Flags: []string{}}, + {Name: "test2"}, + {Name: "test3"}, }, }) }) @@ -264,8 +264,8 @@ releases: }, selectors: []string{"name=test3"}, templated: []exectest.Release{ - {Name: "test2", Flags: []string{}}, - {Name: "test3", Flags: []string{}}, + {Name: "test2"}, + {Name: "test3"}, }, }) }) @@ -279,7 +279,7 @@ releases: }, selectors: []string{"name=test2"}, templated: []exectest.Release{ - {Name: "test2", Flags: []string{}}, + {Name: "test2"}, }, }) }) @@ -293,8 +293,8 @@ releases: }, selectors: []string{"name=test3"}, templated: []exectest.Release{ - {Name: "test2", Flags: []string{}}, - {Name: "test3", Flags: []string{}}, + {Name: "test2"}, + {Name: "test3"}, }, }) }) diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index 283910ef..a2044470 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -519,8 +519,8 @@ func (helm *execer) ChartPull(chart string, path string, flags ...string) error if helmVersionConstraint.Check(helm.version) { // in the 3.7.0 version, the chart pull has been replaced with helm pull // https://github.com/helm/helm/releases/tag/v3.7.0 - ociChartURL, ociChartTag := resolveOciChart(chart) - helmArgs = []string{"pull", ociChartURL, "--version", ociChartTag, "--destination", path, "--untar"} + ociChartURL, _ := resolveOciChart(chart) + helmArgs = []string{"pull", ociChartURL, "--destination", path, "--untar"} helmArgs = append(helmArgs, flags...) } else { helmArgs = []string{"chart", "pull", chart} diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index 58d4ce3d..b68c3e76 100644 --- a/pkg/helmexec/exec_test.go +++ b/pkg/helmexec/exec_test.go @@ -826,20 +826,20 @@ exec: helm --kubeconfig config --kube-context dev chart pull chart helmVersion: "v3.10.0", chartName: "repo/helm-charts:0.14.0", chartPath: "path1", - chartFlags: []string{"--untardir", "/tmp/dir"}, + chartFlags: []string{"--untardir", "/tmp/dir", "--version", "0.14.0"}, listResult: `Pulling repo/helm-charts:0.14.0 -exec: helm --kubeconfig config --kube-context dev pull oci://repo/helm-charts --version 0.14.0 --destination path1 --untar --untardir /tmp/dir +exec: helm --kubeconfig config --kube-context dev pull oci://repo/helm-charts --destination path1 --untar --untardir /tmp/dir --version 0.14.0 `, }, { name: "more then v3.7.0 with rc", helmBin: "helm", helmVersion: "v3.14.0-rc.1+g69dcc92", - chartName: "repo/helm-charts:0.14.0", + chartName: "repo/helm-charts", chartPath: "path1", - chartFlags: []string{"--untardir", "/tmp/dir"}, - listResult: `Pulling repo/helm-charts:0.14.0 -exec: helm --kubeconfig config --kube-context dev pull oci://repo/helm-charts --version 0.14.0 --destination path1 --untar --untardir /tmp/dir + chartFlags: []string{"--untardir", "/tmp/dir", "--devel"}, + listResult: `Pulling repo/helm-charts +exec: helm --kubeconfig config --kube-context dev pull oci://repo/helm-charts --destination path1 --untar --untardir /tmp/dir --devel `, }, } diff --git a/pkg/state/state.go b/pkg/state/state.go index de31e035..421491b4 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1368,7 +1368,8 @@ 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 := st.chartVersionFlags(release) + var fetchFlags []string + fetchFlags = st.appendChartVersionFlags(fetchFlags, release) fetchFlags = append(fetchFlags, "--untar", "--untardir", chartPath) if err := helm.Fetch(chartName, fetchFlags...); err != nil { results <- &chartPrepareResult{err: err} @@ -2722,7 +2723,8 @@ func (st *HelmState) timeoutFlags(release *ReleaseSpec) []string { } func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSpec, workerIndex int, opt *SyncOpts) ([]string, []string, error) { - flags := st.chartVersionFlags(release) + var flags []string + flags = st.appendChartVersionFlags(flags, release) if release.EnableDNS != nil && *release.EnableDNS || release.EnableDNS == nil && st.HelmDefaults.EnableDNS { flags = append(flags, "--enable-dns") } @@ -2810,9 +2812,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseSpec, workerIndex int, opt *TemplateOpts) ([]string, []string, error) { var flags []string - - flags = st.chartVersionFlags(release) - + flags = st.appendChartVersionFlags(flags, release) flags = st.appendHelmXFlags(flags, release) postRenderer := "" @@ -2840,7 +2840,8 @@ func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseS func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, disableValidation bool, workerIndex int, opt *DiffOpts) ([]string, []string, error) { settings := cli.New() - flags := st.chartVersionFlags(release) + var flags []string + flags = st.appendChartVersionFlags(flags, release) disableOpenAPIValidation := false if release.DisableOpenAPIValidation != nil { @@ -2944,9 +2945,7 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, return append(flags, common...), files, nil } -func (st *HelmState) chartVersionFlags(release *ReleaseSpec) []string { - flags := []string{} - +func (st *HelmState) appendChartVersionFlags(flags []string, release *ReleaseSpec) []string { if release.Version != "" { flags = append(flags, "--version", release.Version) } @@ -3880,6 +3879,7 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm flags = st.appendVerifyFlags(flags, release) flags = st.appendKeyringFlags(flags, release) flags = st.appendChartDownloadFlags(flags, release) + flags = st.appendChartVersionFlags(flags, release) err := helm.ChartPull(qualifiedChartName, chartPath, flags...) if err != nil { @@ -3915,31 +3915,36 @@ func (st *HelmState) IsOCIChart(chart string) bool { return repo.OCI } -func (st *HelmState) getOCIQualifiedChartName(release *ReleaseSpec, helm helmexec.Interface) (qualifiedChartName, chartName, chartVersion string, err error) { - chartVersion = "latest" - if release.Version != "" { +func (st *HelmState) getOCIQualifiedChartName(release *ReleaseSpec, helm helmexec.Interface) (string, string, string, error) { + chartVersion := "latest" + if st.isDevelopment(release) && release.Version == "" { + // omit version, otherwise --devel flag is ignored by helm and helm-diff + chartVersion = "" + } else if release.Version != "" { chartVersion = release.Version } + if !st.IsOCIChart(release.Chart) { + return "", "", chartVersion, nil + } + + var qualifiedChartName, chartName string if strings.HasPrefix(release.Chart, "oci://") { - split := strings.Split(release.Chart, "/") - chartName = split[len(split)-1] + parts := strings.Split(release.Chart, "/") + chartName = parts[len(parts)-1] qualifiedChartName = strings.Replace(fmt.Sprintf("%s:%s", release.Chart, chartVersion), "oci://", "", 1) } else { var repo *RepositorySpec repo, chartName = st.GetRepositoryAndNameFromChartName(release.Chart) - if repo == nil { - return - } - if !repo.OCI { - return - } qualifiedChartName = fmt.Sprintf("%s/%s:%s", repo.URL, chartName, chartVersion) } + qualifiedChartName = strings.TrimSuffix(qualifiedChartName, ":") + if chartVersion == "latest" && helm.IsVersionAtLeast("3.8.0") { return "", "", "", fmt.Errorf("the version for OCI charts should be semver compliant, the latest tag is not supported anymore for helm >= 3.8.0") } - return + + return qualifiedChartName, chartName, chartVersion, nil } func (st *HelmState) FullFilePath() (string, error) { diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index a2fbc9d7..9072ea56 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -3216,6 +3216,8 @@ func TestFullFilePath(t *testing.T) { } func TestGetOCIQualifiedChartName(t *testing.T) { + devel := true + tests := []struct { state HelmState expected []struct { @@ -3303,6 +3305,27 @@ func TestGetOCIQualifiedChartName(t *testing.T) { {"registry/chart-path/chart-name:0.1.2", "chart-name", "0.1.2"}, }, }, + { + state: HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + Repositories: []RepositorySpec{}, + Releases: []ReleaseSpec{ + { + Chart: "oci://registry/chart-path/chart-name", + Devel: &devel, + }, + }, + }, + }, + helmVersion: "3.13.3", + expected: []struct { + qualifiedChartName string + chartName string + chartVersion string + }{ + {"registry/chart-path/chart-name", "chart-name", ""}, + }, + }, } for _, tt := range tests {