From be14292938df31ef868af7d278bb638b12800d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Est=C3=A9vez?= Date: Tue, 20 May 2025 11:22:20 +0200 Subject: [PATCH 1/3] Fix: Changed implementation to allow non standard convention for chart tgz names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alberto Estévez --- pkg/clients/helm/client.go | 69 +++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/pkg/clients/helm/client.go b/pkg/clients/helm/client.go index 3759453..9d382cd 100644 --- a/pkg/clients/helm/client.go +++ b/pkg/clients/helm/client.go @@ -20,7 +20,6 @@ import ( "fmt" "net/url" "os" - "path" "path/filepath" "strings" @@ -180,11 +179,7 @@ func (hc *client) pullLatestChartVersion(spec *v1beta1.ChartSpec, creds *RepoCre } }() - if err := hc.pullChart(spec, creds, tmpDir); err != nil { - return "", err - } - - chartFileName, err := getChartFileName(tmpDir) + chartFileName, err := hc.pullChart(spec, creds, tmpDir) if err != nil { return "", err } @@ -196,7 +191,17 @@ func (hc *client) pullLatestChartVersion(spec *v1beta1.ChartSpec, creds *RepoCre return chartFilePath, nil } -func (hc *client) pullChart(spec *v1beta1.ChartSpec, creds *RepoCreds, chartDir string) error { +func (hc *client) pullChart(spec *v1beta1.ChartSpec, creds *RepoCreds, chartDir string) (string, error) { + tmpDir, err := os.MkdirTemp(chartDir, "") + if err != nil { + return "", err + } + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + hc.log.WithValues("tmpDir", tmpDir).Info("failed to remove temporary directory") + } + }() + pc := hc.pullClient chartRef := spec.URL @@ -211,7 +216,7 @@ func (hc *client) pullChart(spec *v1beta1.ChartSpec, creds *RepoCreds, chartDir } else if registry.IsOCI(spec.URL) { ociURL, version, err := resolveOCIChartVersion(spec.URL) if err != nil { - return err + return "", err } pc.Version = version chartRef = ociURL.String() @@ -219,21 +224,32 @@ func (hc *client) pullChart(spec *v1beta1.ChartSpec, creds *RepoCreds, chartDir pc.Username = creds.Username pc.Password = creds.Password - pc.DestDir = chartDir + pc.DestDir = tmpDir if creds.Username != "" && creds.Password != "" { err := hc.login(spec, creds, pc.InsecureSkipTLSverify) if err != nil { - return err + return "", err } } o, err := pc.Run(chartRef) hc.log.Debug(o) + + if err != nil { + return "", errors.Wrap(err, errFailedToPullChart) + } + + chartFileName, err := getChartFileName(tmpDir) if err != nil { - return errors.Wrap(err, errFailedToPullChart) + return "", err } - return nil + + chartFilePath := filepath.Join(chartDir, chartFileName) + if err := os.Rename(filepath.Join(tmpDir, chartFileName), chartFilePath); err != nil { + return "", err + } + return chartFilePath, nil } func (hc *client) login(spec *v1beta1.ChartSpec, creds *RepoCreds, insecure bool) error { @@ -257,6 +273,7 @@ func (hc *client) login(spec *v1beta1.ChartSpec, creds *RepoCreds, insecure bool func (hc *client) PullAndLoadChart(spec *v1beta1.ChartSpec, creds *RepoCreds) (*chart.Chart, error) { var chartFilePath string var err error + switch { case spec.URL == "" && (spec.Version == "" || spec.Version == devel): chartFilePath, err = hc.pullLatestChartVersion(spec, creds) @@ -264,7 +281,7 @@ func (hc *client) PullAndLoadChart(spec *v1beta1.ChartSpec, creds *RepoCreds) (* return nil, err } case registry.IsOCI(spec.URL): - u, v, err := resolveOCIChartVersion(spec.URL) + _, v, err := resolveOCIChartVersion(spec.URL) if err != nil { return nil, err } @@ -275,24 +292,27 @@ func (hc *client) PullAndLoadChart(spec *v1beta1.ChartSpec, creds *RepoCreds) (* return nil, err } } else { - chartFilePath = resolveChartFilePath(path.Base(u.Path), v) + chartFilePath, err = hc.pullChart(spec, creds, chartCache) + if err != nil { + return nil, err + } } case spec.URL != "": - u, err := url.Parse(spec.URL) + chartFilePath, err = hc.pullChart(spec, creds, chartCache) if err != nil { - return nil, errors.Wrap(err, errFailedToParseURL) + return nil, err } - chartFilePath = filepath.Join(chartCache, path.Base(u.Path)) default: - chartFilePath = resolveChartFilePath(spec.Name, spec.Version) + chartFilePath, err = hc.pullChart(spec, creds, chartCache) + if err != nil { + return nil, err + } } if _, err := os.Stat(chartFilePath); os.IsNotExist(err) { - if err = hc.pullChart(spec, creds, chartCache); err != nil { - return nil, err - } - } else if err != nil { return nil, errors.Wrap(err, errFailedToCheckIfLocalChartExists) + } else if err != nil { + return nil, err } chart, err := loader.Load(chartFilePath) @@ -359,11 +379,6 @@ func resolveOCIChartVersion(chartURL string) (*url.URL, string, error) { return ociURL, "", nil } -func resolveChartFilePath(name string, version string) string { - filename := fmt.Sprintf("%s-%s.tgz", name, version) - return filepath.Join(chartCache, filename) -} - func resolveOCIChartRef(repository string, name string) string { return strings.Join([]string{strings.TrimSuffix(repository, "/"), name}, "/") } From dd6592b5ca2e53d80206733c6fb9d350f7c058ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Est=C3=A9vez?= Date: Tue, 20 May 2025 13:10:20 +0200 Subject: [PATCH 2/3] Fix: Separate into functions to reduce cyclomatic complexity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alberto Estévez --- pkg/clients/helm/client.go | 64 +++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/pkg/clients/helm/client.go b/pkg/clients/helm/client.go index 9d382cd..04f016f 100644 --- a/pkg/clients/helm/client.go +++ b/pkg/clients/helm/client.go @@ -271,42 +271,9 @@ func (hc *client) login(spec *v1beta1.ChartSpec, creds *RepoCreds, insecure bool } func (hc *client) PullAndLoadChart(spec *v1beta1.ChartSpec, creds *RepoCreds) (*chart.Chart, error) { - var chartFilePath string - var err error - - switch { - case spec.URL == "" && (spec.Version == "" || spec.Version == devel): - chartFilePath, err = hc.pullLatestChartVersion(spec, creds) - if err != nil { - return nil, err - } - case registry.IsOCI(spec.URL): - _, v, err := resolveOCIChartVersion(spec.URL) - if err != nil { - return nil, err - } - - if v == "" { - chartFilePath, err = hc.pullLatestChartVersion(spec, creds) - if err != nil { - return nil, err - } - } else { - chartFilePath, err = hc.pullChart(spec, creds, chartCache) - if err != nil { - return nil, err - } - } - case spec.URL != "": - chartFilePath, err = hc.pullChart(spec, creds, chartCache) - if err != nil { - return nil, err - } - default: - chartFilePath, err = hc.pullChart(spec, creds, chartCache) - if err != nil { - return nil, err - } + chartFilePath, err := hc.resolveChartFilePath(spec, creds) + if err != nil { + return nil, err } if _, err := os.Stat(chartFilePath); os.IsNotExist(err) { @@ -382,3 +349,28 @@ func resolveOCIChartVersion(chartURL string) (*url.URL, string, error) { func resolveOCIChartRef(repository string, name string) string { return strings.Join([]string{strings.TrimSuffix(repository, "/"), name}, "/") } + +func (hc *client) resolveChartFilePath(spec *v1beta1.ChartSpec, creds *RepoCreds) (string, error) { + switch { + case spec.URL == "" && (spec.Version == "" || spec.Version == devel): + return hc.pullLatestChartVersion(spec, creds) + case registry.IsOCI(spec.URL): + return hc.resolveOCIChart(spec, creds) + case spec.URL != "": + return hc.pullChart(spec, creds, chartCache) + default: + return hc.pullChart(spec, creds, chartCache) + } +} + +func (hc *client) resolveOCIChart(spec *v1beta1.ChartSpec, creds *RepoCreds) (string, error) { + _, v, err := resolveOCIChartVersion(spec.URL) + if err != nil { + return "", err + } + + if v == "" { + return hc.pullLatestChartVersion(spec, creds) + } + return hc.pullChart(spec, creds, chartCache) +} From 2fb082e2e1f33b48c0e264472e0719da4e479bac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Est=C3=A9vez?= Date: Wed, 21 May 2025 23:16:31 +0200 Subject: [PATCH 3/3] Fix: Delete tgz in destination folder if it already exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alberto Estévez --- pkg/clients/helm/client.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/clients/helm/client.go b/pkg/clients/helm/client.go index 04f016f..fe8a169 100644 --- a/pkg/clients/helm/client.go +++ b/pkg/clients/helm/client.go @@ -246,9 +246,15 @@ func (hc *client) pullChart(spec *v1beta1.ChartSpec, creds *RepoCreds, chartDir } chartFilePath := filepath.Join(chartDir, chartFileName) + if _, err := os.Stat(chartFilePath); err == nil { + if err := os.Remove(chartFilePath); err != nil { + return "", errors.Wrap(err, "failed to remove existing file") + } + } if err := os.Rename(filepath.Join(tmpDir, chartFileName), chartFilePath); err != nil { return "", err } + return chartFilePath, nil }