Merge ea88181d34 into f05c21b6b6

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

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")
 }

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

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

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) {

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 {

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(