diff --git a/pkg/action/dependency.go b/pkg/action/dependency.go index b12887bde..c18e100d4 100644 --- a/pkg/action/dependency.go +++ b/pkg/action/dependency.go @@ -37,6 +37,7 @@ type Dependency struct { Verify bool Keyring string SkipRefresh bool + SkipDownloadIfExists bool ColumnWidth uint Username string Password string diff --git a/pkg/cmd/dependency.go b/pkg/cmd/dependency.go index 5978c902a..26323b3a1 100644 --- a/pkg/cmd/dependency.go +++ b/pkg/cmd/dependency.go @@ -133,4 +133,5 @@ func addDependencySubcommandFlags(f *pflag.FlagSet, client *action.Dependency) { f.BoolVar(&client.InsecureSkipTLSVerify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the chart download") f.BoolVar(&client.PlainHTTP, "plain-http", false, "use insecure HTTP connections for the chart download") f.StringVar(&client.CaFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") + f.BoolVar(&client.SkipDownloadIfExists, "skip-download-if-exists", false, "skip download of the chart if it already exists in the charts directory") } diff --git a/pkg/cmd/dependency_build.go b/pkg/cmd/dependency_build.go index 7e5c731b7..eef2cce11 100644 --- a/pkg/cmd/dependency_build.go +++ b/pkg/cmd/dependency_build.go @@ -61,16 +61,17 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command { } man := &downloader.Manager{ - Out: out, - ChartPath: chartpath, - Keyring: client.Keyring, - SkipUpdate: client.SkipRefresh, - Getters: getter.All(settings), - RegistryClient: registryClient, - RepositoryConfig: settings.RepositoryConfig, - RepositoryCache: settings.RepositoryCache, - ContentCache: settings.ContentCache, - Debug: settings.Debug, + Out: out, + ChartPath: chartpath, + Keyring: client.Keyring, + SkipUpdate: client.SkipRefresh, + SkipDownloadIfExists: client.SkipDownloadIfExists, + Getters: getter.All(settings), + RegistryClient: registryClient, + RepositoryConfig: settings.RepositoryConfig, + RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, + Debug: settings.Debug, } if client.Verify { man.Verify = downloader.VerifyIfPossible diff --git a/pkg/cmd/dependency_update.go b/pkg/cmd/dependency_update.go index 7f805c37b..47baa574d 100644 --- a/pkg/cmd/dependency_update.go +++ b/pkg/cmd/dependency_update.go @@ -65,16 +65,17 @@ func newDependencyUpdateCmd(_ *action.Configuration, out io.Writer) *cobra.Comma } man := &downloader.Manager{ - Out: out, - ChartPath: chartpath, - Keyring: client.Keyring, - SkipUpdate: client.SkipRefresh, - Getters: getter.All(settings), - RegistryClient: registryClient, - RepositoryConfig: settings.RepositoryConfig, - RepositoryCache: settings.RepositoryCache, - ContentCache: settings.ContentCache, - Debug: settings.Debug, + Out: out, + ChartPath: chartpath, + Keyring: client.Keyring, + SkipUpdate: client.SkipRefresh, + SkipDownloadIfExists: client.SkipDownloadIfExists, + Getters: getter.All(settings), + RegistryClient: registryClient, + RepositoryConfig: settings.RepositoryConfig, + RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, + Debug: settings.Debug, } if client.Verify { man.Verify = downloader.VerifyAlways diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index ee4f8abe3..c15ffc2bb 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -29,6 +29,8 @@ import ( "path/filepath" "strings" + "github.com/Masterminds/semver/v3" + "helm.sh/helm/v4/internal/fileutil" ifs "helm.sh/helm/v4/internal/third_party/dep/fs" "helm.sh/helm/v4/internal/urlutil" @@ -149,12 +151,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven } } - name := filepath.Base(u.Path) - if u.Scheme == registry.OCIScheme { - idx := strings.LastIndexByte(name, ':') - name = fmt.Sprintf("%s-%s.tgz", name[:idx], name[idx+1:]) - } - + name := c.getChartName(u.String()) destfile := filepath.Join(dest, name) if err := fileutil.AtomicWriteFile(destfile, data, 0644); err != nil { return destfile, nil, err @@ -299,11 +296,7 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena // Note, this does make an assumption that the name/version is unique to a // hash when a provenance file is used. If this isn't true, this section of code // will need to be reworked. - name := filepath.Base(u.Path) - if u.Scheme == registry.OCIScheme { - idx := strings.LastIndexByte(name, ':') - name = fmt.Sprintf("%s-%s.tgz", name[:idx], name[idx+1:]) - } + name := c.getChartName(u.String()) // Copy chart to a known location with the right name for verification and then // clean it up. @@ -352,7 +345,12 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena func (c *ChartDownloader) ResolveChartVersion(ref, version string) (string, *url.URL, error) { u, err := url.Parse(ref) if err != nil { - return "", nil, fmt.Errorf("invalid chart URL format: %s", ref) + return "", nil, err + } + + u, err = c.appendTagToURLIfNeeded(u, version) + if err != nil { + return "", nil, err } if registry.IsOCI(u.String()) { @@ -571,6 +569,66 @@ func (c *ChartDownloader) scanReposForURL(u string, rf *repo.File) (*repo.Entry, return nil, ErrNoOwnerRepo } +func (c *ChartDownloader) getChartName(url string) string { + name := filepath.Base(url) + if registry.IsOCI(url) { + idx := strings.LastIndexByte(name, ':') + name = fmt.Sprintf("%s-%s.tgz", name[:idx], name[idx+1:]) + } + + return name +} + +func (c *ChartDownloader) appendTagToURLIfNeeded(chartURL *url.URL, version string) (*url.URL, error) { + if !registry.IsOCI(chartURL.String()) { + return chartURL, nil + } + + refAlreadyHasTagOrDigest := strings.Contains(chartURL.Path, ":") || strings.Contains(chartURL.Path, "@") + if refAlreadyHasTagOrDigest { + return chartURL, nil + } + + tag, err := c.getOciTag(chartURL.String(), version) + if err != nil { + return nil, err + } + + chartURL.Path = fmt.Sprintf("%s:%s", chartURL.Path, tag) + + return chartURL, nil +} + +func (c *ChartDownloader) getOciTag(ref, version string) (string, error) { + var tag string + + // Evaluate whether an explicit version has been provided. Otherwise, determine version to use + _, errSemVer := semver.NewVersion(version) + if errSemVer == nil { + tag = version + } else { + // Retrieve list of repository tags + tags, err := c.RegistryClient.Tags(strings.TrimPrefix(ref, fmt.Sprintf("%s://", registry.OCIScheme))) + if err != nil { + return "", err + } + if len(tags) == 0 { + return "", fmt.Errorf("unable to locate any tags in provided repository: %s", ref) + } + + // Determine if version provided + // If empty, try to get the highest available tag + // If exact version, try to find it + // If semver constraint string, try to find a match + tag, err = registry.GetTagMatchingVersionOrConstraint(tags, version) + if err != nil { + return "", err + } + } + + return tag, nil +} + func loadRepoConfig(file string) (*repo.File, error) { r, err := repo.LoadFile(file) if err != nil && !errors.Is(err, fs.ErrNotExist) { diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 6043fbaaa..0d8444ba1 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -27,6 +27,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "strings" "sync" @@ -70,6 +71,8 @@ type Manager struct { Keyring string // SkipUpdate indicates that the repository should not be updated first. SkipUpdate bool + // SkipDownloadIfExists indicates that the chart should not be downloaded if it already exists. + SkipDownloadIfExists bool // Getter collection for the operation Getters []getter.Provider RegistryClient *registry.Client @@ -273,6 +276,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps)) var saveError error churls := make(map[string]struct{}) + skippedCharts := make([]string, 0) for _, dep := range deps { // No repository means the chart is in charts directory if dep.Repository == "" { @@ -326,8 +330,6 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { continue } - fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository) - dl := ChartDownloader{ Out: m.Out, Verify: m.Verify, @@ -356,6 +358,27 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { getter.WithTagName(version)) } + if m.SkipDownloadIfExists { + u, err := url.Parse(churl) + if err != nil { + return err + } + + u, err = dl.appendTagToURLIfNeeded(u, version) + if err != nil { + return err + } + + name := dl.getChartName(u.String()) + if _, err = os.Stat(filepath.Join(destPath, name)); err == nil { + fmt.Fprintf(m.Out, "Already exists locally %s from repo %s\n", dep.Name, dep.Repository) + + skippedCharts = append(skippedCharts, name) + continue + } + } + + fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository) if _, _, err = dl.DownloadTo(churl, version, tmpPath); err != nil { saveError = fmt.Errorf("could not download %s: %w", churl, err) break @@ -367,7 +390,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { // TODO: this should probably be refactored to be a []error, so we can capture and provide more information rather than "last error wins". if saveError == nil { // now we can move all downloaded charts to destPath and delete outdated dependencies - if err := m.safeMoveDeps(deps, tmpPath, destPath); err != nil { + if err := m.safeMoveDeps(deps, tmpPath, destPath, skippedCharts); err != nil { return err } } else { @@ -394,13 +417,14 @@ func parseOCIRef(chartRef string) (string, string, error) { // It does this by first matching the file name to an expected pattern, then loading // the file to verify that it is a chart. // -// Any charts in dest that do not exist in source are removed (barring local dependencies) +// Any charts in dest that do not exist in source and not in skippedCharts are removed (barring local dependencies) +// skippedCharts is a list of charts that were skipped during the download process. // // Because it requires tar file introspection, it is more intensive than a basic move. // // This will only return errors that should stop processing entirely. Other errors // will emit log messages or be ignored. -func (m *Manager) safeMoveDeps(deps []*chart.Dependency, source, dest string) error { +func (m *Manager) safeMoveDeps(deps []*chart.Dependency, source, dest string, skippedCharts []string) error { existsInSourceDirectory := map[string]bool{} isLocalDependency := map[string]bool{} sourceFiles, err := os.ReadDir(source) @@ -441,7 +465,7 @@ func (m *Manager) safeMoveDeps(deps []*chart.Dependency, source, dest string) er fmt.Fprintln(m.Out, "Deleting outdated charts") // find all files that exist in dest that do not exist in source; delete them (outdated dependencies) for _, file := range destFiles { - if !file.IsDir() && !existsInSourceDirectory[file.Name()] { + if !file.IsDir() && !existsInSourceDirectory[file.Name()] && !slices.Contains(skippedCharts, file.Name()) { fname := filepath.Join(dest, file.Name()) ch, err := loader.LoadFile(fname) if err != nil { diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 9e27f183f..c90fecf1e 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -302,6 +302,105 @@ version: 0.1.0` } } +func TestDownloadAll_SkipDownloadIfExists(t *testing.T) { + // Set up a fake repo + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + ) + defer srv.Stop() + if err := srv.LinkIndices(); err != nil { + t.Fatal(err) + } + + chartPath := t.TempDir() + contentCache := t.TempDir() + b := bytes.NewBuffer(nil) + g := getter.Providers{getter.Provider{ + Schemes: []string{"http", "https"}, + New: getter.NewHTTPGetter, + }} + m := &Manager{ + Out: b, + RepositoryConfig: filepath.Join(srv.Root(), "repositories.yaml"), + RepositoryCache: srv.Root(), + ChartPath: chartPath, + Getters: g, + ContentCache: contentCache, + } + + dep := &chart.Dependency{ + Name: "local-subchart", + Repository: srv.URL(), + Version: "0.1.0", + } + + // First download without SkipDownloadIfExists flag - should download the chart + if err := m.downloadAll([]*chart.Dependency{dep}); err != nil { + t.Fatal(err) + } + + chartFile := filepath.Join(chartPath, "charts", "local-subchart-0.1.0.tgz") + if _, err := os.Stat(chartFile); errors.Is(err, fs.ErrNotExist) { + t.Fatal("chart should have been downloaded") + } + + // Get the file info to check modification time later + fileInfo1, err := os.Stat(chartFile) + if err != nil { + t.Fatal(err) + } + + // Second download with SkipDownloadIfExists flag - should NOT re-download + m.SkipDownloadIfExists = true + b.Reset() + if err := m.downloadAll([]*chart.Dependency{dep}); err != nil { + t.Fatal(err) + } + + // Verify the chart file still exists + if _, err := os.Stat(chartFile); errors.Is(err, fs.ErrNotExist) { + t.Fatal("chart should still exist") + } + + // Verify the file was NOT re-downloaded by checking modification time hasn't changed + fileInfo2, err := os.Stat(chartFile) + if err != nil { + t.Fatal(err) + } + + if !fileInfo1.ModTime().Equal(fileInfo2.ModTime()) { + t.Errorf("chart file should not have been re-downloaded, but modification time changed: %v vs %v", fileInfo1.ModTime(), fileInfo2.ModTime()) + } + + output := b.String() + if !bytes.Contains([]byte(output), []byte("Already exists locally")) { + t.Errorf("expected 'Already exists locally' message in output, got: %s", output) + } + + // Third test: Delete the file and verify it gets re-downloaded when flag is disabled + if err := os.Remove(chartFile); err != nil { + t.Fatal(err) + } + + m.SkipDownloadIfExists = false + b.Reset() + if err := m.downloadAll([]*chart.Dependency{dep}); err != nil { + t.Fatal(err) + } + + // Verify the file was re-downloaded + if _, err := os.Stat(chartFile); errors.Is(err, fs.ErrNotExist) { + t.Fatal("chart should have been re-downloaded") + } + + // Verify the output does NOT contain "Already exists locally" + output = b.String() + if bytes.Contains([]byte(output), []byte("Already exists locally")) { + t.Error("should not skip download when file doesn't exist") + } +} + func TestUpdateBeforeBuild(t *testing.T) { // Set up a fake repo srv := repotest.NewTempServer(