From 904f303a341e7e01f952358b2c3bc6b4085f8f93 Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Sat, 7 Oct 2023 02:16:41 -0500 Subject: [PATCH] optimize OCI chart version check (#1052) * optimize OCI chart version check Signed-off-by: yxxhero * fix tests Signed-off-by: yxxhero --------- Signed-off-by: yxxhero --- pkg/state/state.go | 10 +++++++-- pkg/state/state_test.go | 50 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index 646f1d20..fc52bd9e 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -3467,7 +3467,10 @@ func (st *HelmState) Reverse() { } func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helmexec.Interface) (*string, error) { - qualifiedChartName, chartName, chartVersion := st.getOCIQualifiedChartName(release) + qualifiedChartName, chartName, chartVersion, err := st.getOCIQualifiedChartName(release, helm) + if err != nil { + return nil, err + } if qualifiedChartName == "" { return nil, nil @@ -3533,7 +3536,7 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm return &chartPath, nil } -func (st *HelmState) getOCIQualifiedChartName(release *ReleaseSpec) (qualifiedChartName, chartName, chartVersion string) { +func (st *HelmState) getOCIQualifiedChartName(release *ReleaseSpec, helm helmexec.Interface) (qualifiedChartName, chartName, chartVersion string, err error) { chartVersion = "latest" if release.Version != "" { chartVersion = release.Version @@ -3554,6 +3557,9 @@ func (st *HelmState) getOCIQualifiedChartName(release *ReleaseSpec) (qualifiedCh } qualifiedChartName = fmt.Sprintf("%s/%s:%s", repo.URL, chartName, chartVersion) } + 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 } diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 7404f5ec..2ea73411 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -17,6 +17,7 @@ import ( "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/testhelper" + "github.com/helmfile/helmfile/pkg/testutil" ) var logger = helmexec.NewLogger(io.Discard, "warn") @@ -3140,6 +3141,8 @@ func TestGetOCIQualifiedChartName(t *testing.T) { chartName string chartVersion string } + helmVersion string + wantErr bool }{ { state: HelmState{ @@ -3153,6 +3156,7 @@ func TestGetOCIQualifiedChartName(t *testing.T) { }, }, }, + helmVersion: "3.13.0", expected: []struct { qualifiedChartName string chartName string @@ -3161,6 +3165,35 @@ 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", + Version: "latest", + }, + }, + }, + }, + helmVersion: "3.13.0", + wantErr: true, + }, + { + state: HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + Repositories: []RepositorySpec{}, + Releases: []ReleaseSpec{ + { + Chart: "oci://registry/chart-path/chart-name", + Version: "latest", + }, + }, + }, + }, + helmVersion: "3.7.0", + }, { state: HelmState{ ReleaseSetSpec: ReleaseSetSpec{ @@ -3179,6 +3212,7 @@ func TestGetOCIQualifiedChartName(t *testing.T) { }, }, }, + helmVersion: "3.13.0", expected: []struct { qualifiedChartName string chartName string @@ -3191,11 +3225,19 @@ func TestGetOCIQualifiedChartName(t *testing.T) { for _, tt := range tests { t.Run(fmt.Sprintf("%+v", tt.expected), func(t *testing.T) { + helm := testutil.NewVersionHelmExec(tt.helmVersion) for i, r := range tt.state.Releases { - qualifiedChartName, chartName, chartVersion := tt.state.getOCIQualifiedChartName(&r) - require.Equalf(t, qualifiedChartName, tt.expected[i].qualifiedChartName, "qualifiedChartName got = %v, want %v", qualifiedChartName, tt.expected[i].qualifiedChartName) - require.Equalf(t, chartName, tt.expected[i].chartName, "chartName got = %v, want %v", chartName, tt.expected[i].chartName) - require.Equalf(t, chartVersion, tt.expected[i].chartVersion, "chartVersion got = %v, want %v", chartVersion, tt.expected[i].chartVersion) + qualifiedChartName, chartName, chartVersion, err := tt.state.getOCIQualifiedChartName(&r, helm) + if tt.wantErr { + require.Error(t, err, "getOCIQualifiedChartName() error = nil, want error") + return + } + require.NoError(t, err, "getOCIQualifiedChartName() error = %v, want nil", err) + if len(tt.expected) > 0 { + require.Equalf(t, qualifiedChartName, tt.expected[i].qualifiedChartName, "qualifiedChartName got = %v, want %v", qualifiedChartName, tt.expected[i].qualifiedChartName) + require.Equalf(t, chartName, tt.expected[i].chartName, "chartName got = %v, want %v", chartName, tt.expected[i].chartName) + require.Equalf(t, chartVersion, tt.expected[i].chartVersion, "chartVersion got = %v, want %v", chartVersion, tt.expected[i].chartVersion) + } } }) }