diff --git a/internal/third_party/dep/fs/fs.go b/internal/third_party/dep/fs/fs.go index 717eff04d..6e2720f3b 100644 --- a/internal/third_party/dep/fs/fs.go +++ b/internal/third_party/dep/fs/fs.go @@ -73,7 +73,7 @@ func renameByCopy(src, dst string) error { cerr = fmt.Errorf("copying directory failed: %w", cerr) } } else { - cerr = copyFile(src, dst) + cerr = CopyFile(src, dst) if cerr != nil { cerr = fmt.Errorf("copying file failed: %w", cerr) } @@ -139,7 +139,7 @@ func CopyDir(src, dst string) error { } else { // This will include symlinks, which is what we want when // copying things. - if err = copyFile(srcPath, dstPath); err != nil { + if err = CopyFile(srcPath, dstPath); err != nil { return fmt.Errorf("copying file failed: %w", err) } } @@ -148,11 +148,11 @@ func CopyDir(src, dst string) error { return nil } -// copyFile copies the contents of the file named src to the file named +// CopyFile copies the contents of the file named src to the file named // by dst. The file will be created if it does not already exist. If the // destination file exists, all its contents will be replaced by the contents // of the source file. The file mode will be copied from the source. -func copyFile(src, dst string) (err error) { +func CopyFile(src, dst string) (err error) { if sym, err := IsSymlink(src); err != nil { return fmt.Errorf("symlink check failed: %w", err) } else if sym { diff --git a/internal/third_party/dep/fs/fs_test.go b/internal/third_party/dep/fs/fs_test.go index 4c59d17fe..610771bc3 100644 --- a/internal/third_party/dep/fs/fs_test.go +++ b/internal/third_party/dep/fs/fs_test.go @@ -326,7 +326,7 @@ func TestCopyFile(t *testing.T) { srcf.Close() destf := filepath.Join(dir, "destf") - if err := copyFile(srcf.Name(), destf); err != nil { + if err := CopyFile(srcf.Name(), destf); err != nil { t.Fatal(err) } @@ -366,7 +366,7 @@ func TestCopyFileSymlink(t *testing.T) { for symlink, dst := range testcases { t.Run(symlink, func(t *testing.T) { var err error - if err = copyFile(symlink, dst); err != nil { + if err = CopyFile(symlink, dst); err != nil { t.Fatalf("failed to copy symlink: %s", err) } @@ -438,7 +438,7 @@ func TestCopyFileFail(t *testing.T) { defer cleanup() fn := filepath.Join(dstdir, "file") - if err := copyFile(srcf.Name(), fn); err == nil { + if err := CopyFile(srcf.Name(), fn); err == nil { t.Fatalf("expected error for %s, got none", fn) } } diff --git a/pkg/action/install.go b/pkg/action/install.go index b13bbfb8b..db130c6e9 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -815,6 +815,7 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) ( }, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, RegistryClient: c.registryClient, } diff --git a/pkg/action/pull.go b/pkg/action/pull.go index b4779f8d2..c1f77e44c 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -88,6 +88,7 @@ func (p *Pull) Run(chartRef string) (string, error) { RegistryClient: p.cfg.RegistryClient, RepositoryConfig: p.Settings.RepositoryConfig, RepositoryCache: p.Settings.RepositoryCache, + ContentCache: p.Settings.ContentCache, } if registry.IsOCI(chartRef) { diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go index c5f87cf24..19563cba3 100644 --- a/pkg/cli/environment.go +++ b/pkg/cli/environment.go @@ -91,6 +91,8 @@ type EnvSettings struct { QPS float32 // ColorMode controls colorized output (never, auto, always) ColorMode string + // ContentCache is the location where cached charts are stored + ContentCache string } func New() *EnvSettings { @@ -109,6 +111,7 @@ func New() *EnvSettings { RegistryConfig: envOr("HELM_REGISTRY_CONFIG", helmpath.ConfigPath("registry/config.json")), RepositoryConfig: envOr("HELM_REPOSITORY_CONFIG", helmpath.ConfigPath("repositories.yaml")), RepositoryCache: envOr("HELM_REPOSITORY_CACHE", helmpath.CachePath("repository")), + ContentCache: envOr("HELM_CONTENT_CACHE", helmpath.CachePath("content")), BurstLimit: envIntOr("HELM_BURST_LIMIT", defaultBurstLimit), QPS: envFloat32Or("HELM_QPS", defaultQPS), ColorMode: envColorMode(), @@ -161,6 +164,7 @@ func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.RegistryConfig, "registry-config", s.RegistryConfig, "path to the registry config file") fs.StringVar(&s.RepositoryConfig, "repository-config", s.RepositoryConfig, "path to the file containing repository names and URLs") fs.StringVar(&s.RepositoryCache, "repository-cache", s.RepositoryCache, "path to the directory containing cached repository indexes") + fs.StringVar(&s.ContentCache, "content-cache", s.ContentCache, "path to the directory containing cached content (e.g. charts)") fs.IntVar(&s.BurstLimit, "burst-limit", s.BurstLimit, "client-side default throttling limit") fs.Float32Var(&s.QPS, "qps", s.QPS, "queries per second used when communicating with the Kubernetes API, not including bursting") fs.StringVar(&s.ColorMode, "color", s.ColorMode, "use colored output (never, auto, always)") diff --git a/pkg/cmd/dependency_build.go b/pkg/cmd/dependency_build.go index 16907facf..320fe12ae 100644 --- a/pkg/cmd/dependency_build.go +++ b/pkg/cmd/dependency_build.go @@ -69,6 +69,7 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command { RegistryClient: registryClient, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, Debug: settings.Debug, } if client.Verify { diff --git a/pkg/cmd/dependency_update.go b/pkg/cmd/dependency_update.go index 921e5ef49..b534fb48a 100644 --- a/pkg/cmd/dependency_update.go +++ b/pkg/cmd/dependency_update.go @@ -73,6 +73,7 @@ func newDependencyUpdateCmd(_ *action.Configuration, out io.Writer) *cobra.Comma RegistryClient: registryClient, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, Debug: settings.Debug, } if client.Verify { diff --git a/pkg/cmd/dependency_update_test.go b/pkg/cmd/dependency_update_test.go index 9646c6816..f1b39c4b7 100644 --- a/pkg/cmd/dependency_update_test.go +++ b/pkg/cmd/dependency_update_test.go @@ -45,6 +45,7 @@ func TestDependencyUpdateCmd(t *testing.T) { if err != nil { t.Fatal(err) } + contentCache := t.TempDir() ociChartName := "oci-depending-chart" c := createTestingMetadataForOCI(ociChartName, ociSrv.RegistryURL) @@ -69,7 +70,7 @@ func TestDependencyUpdateCmd(t *testing.T) { } _, out, err := executeActionCommand( - fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --plain-http", dir(chartname), dir("repositories.yaml"), dir()), + fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --content-cache %s --plain-http", dir(chartname), dir("repositories.yaml"), dir(), contentCache), ) if err != nil { t.Logf("Output: %s", out) @@ -112,7 +113,7 @@ func TestDependencyUpdateCmd(t *testing.T) { t.Fatal(err) } - _, out, err = executeActionCommand(fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --plain-http", dir(chartname), dir("repositories.yaml"), dir())) + _, out, err = executeActionCommand(fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --content-cache %s --plain-http", dir(chartname), dir("repositories.yaml"), dir(), contentCache)) if err != nil { t.Logf("Output: %s", out) t.Fatal(err) @@ -133,11 +134,12 @@ func TestDependencyUpdateCmd(t *testing.T) { if err := chartutil.SaveDir(c, dir()); err != nil { t.Fatal(err) } - cmd := fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --registry-config %s/config.json --plain-http", + cmd := fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --registry-config %s/config.json --content-cache %s --plain-http", dir(ociChartName), dir("repositories.yaml"), dir(), - dir()) + dir(), + contentCache) _, out, err = executeActionCommand(cmd) if err != nil { t.Logf("Output: %s", out) @@ -179,8 +181,9 @@ func TestDependencyUpdateCmd_DoNotDeleteOldChartsOnError(t *testing.T) { // Chart repo is down srv.Stop() + contentCache := t.TempDir() - _, output, err = executeActionCommand(fmt.Sprintf("dependency update %s --repository-config %s --repository-cache %s --plain-http", dir(chartname), dir("repositories.yaml"), dir())) + _, output, err = executeActionCommand(fmt.Sprintf("dependency update %s --repository-config %s --repository-cache %s --content-cache %s --plain-http", dir(chartname), dir("repositories.yaml"), dir(), contentCache)) if err == nil { t.Logf("Output: %s", output) t.Fatal("Expected error, got nil") @@ -232,9 +235,11 @@ func TestDependencyUpdateCmd_WithRepoThatWasNotAdded(t *testing.T) { t.Fatal(err) } + contentCache := t.TempDir() + _, out, err := executeActionCommand( - fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s", dir(chartname), - dir("repositories.yaml"), dir()), + fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --content-cache %s", dir(chartname), + dir("repositories.yaml"), dir(), contentCache), ) if err != nil { diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go index d53b1d981..b254b887e 100644 --- a/pkg/cmd/install.go +++ b/pkg/cmd/install.go @@ -287,6 +287,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options Getters: p, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, Debug: settings.Debug, RegistryClient: client.GetRegistryClient(), } diff --git a/pkg/cmd/package.go b/pkg/cmd/package.go index 40c503222..fc56e936a 100644 --- a/pkg/cmd/package.go +++ b/pkg/cmd/package.go @@ -100,6 +100,7 @@ func newPackageCmd(out io.Writer) *cobra.Command { RegistryClient: registryClient, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, } if err := downloadManager.Update(); err != nil { diff --git a/pkg/cmd/pull_test.go b/pkg/cmd/pull_test.go index 58e1862ae..ed8ea442e 100644 --- a/pkg/cmd/pull_test.go +++ b/pkg/cmd/pull_test.go @@ -212,15 +212,18 @@ func TestPullCmd(t *testing.T) { }, } + contentCache := t.TempDir() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { outdir := srv.Root() - cmd := fmt.Sprintf("fetch %s -d '%s' --repository-config %s --repository-cache %s --registry-config %s --plain-http", + cmd := fmt.Sprintf("fetch %s -d '%s' --repository-config %s --repository-cache %s --registry-config %s --content-cache %s --plain-http", tt.args, outdir, filepath.Join(outdir, "repositories.yaml"), outdir, filepath.Join(outdir, "config.json"), + contentCache, ) // Create file or Dir before helm pull --untar, see: https://github.com/helm/helm/issues/7182 if tt.existFile != "" { diff --git a/pkg/cmd/show_test.go b/pkg/cmd/show_test.go index ab8cafc37..5ccb4bcad 100644 --- a/pkg/cmd/show_test.go +++ b/pkg/cmd/show_test.go @@ -64,14 +64,17 @@ func TestShowPreReleaseChart(t *testing.T) { }, } + contentTmp := t.TempDir() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { outdir := srv.Root() - cmd := fmt.Sprintf("show all '%s' %s --repository-config %s --repository-cache %s", + cmd := fmt.Sprintf("show all '%s' %s --repository-config %s --repository-cache %s --content-cache %s", tt.args, tt.flags, filepath.Join(outdir, "repositories.yaml"), outdir, + contentTmp, ) //_, out, err := executeActionCommand(cmd) _, _, err := executeActionCommand(cmd) diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index c3288286b..4f204037a 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -210,6 +210,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { Getters: p, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, + ContentCache: settings.ContentCache, Debug: settings.Debug, } if err := man.Update(); err != nil { diff --git a/pkg/downloader/cache.go b/pkg/downloader/cache.go index d9b925756..cecfc8bd7 100644 --- a/pkg/downloader/cache.go +++ b/pkg/downloader/cache.go @@ -17,6 +17,7 @@ package downloader import ( "crypto/sha256" + "errors" "fmt" "io" "log/slog" @@ -31,11 +32,17 @@ import ( // digests in index files providing a common key for checking content. type Cache interface { // Get returns a reader for the given key. - Get(key [sha256.Size]byte, prov bool) (string, error) + Get(key [sha256.Size]byte, cacheType string) (string, error) // Put stores the given reader for the given key. - Put(key [sha256.Size]byte, data io.Reader, prov bool) (string, error) + Put(key [sha256.Size]byte, data io.Reader, cacheType string) (string, error) } +// CacheChart specifies the content is a chart +var CacheChart = ".chart" + +// CacheProv specifies the content is a provenance file +var CacheProv = ".prov" + // TODO: The cache assumes files because much of Helm assumes files. Convert // Helm to pass content around instead of file locations. @@ -45,8 +52,8 @@ type DiskCache struct { } // Get returns a reader for the given key. -func (c *DiskCache) Get(key [sha256.Size]byte, prov bool) (string, error) { - p := c.fileName(key, prov) +func (c *DiskCache) Get(key [sha256.Size]byte, cacheType string) (string, error) { + p := c.fileName(key, cacheType) fi, err := os.Stat(p) if err != nil { return "", err @@ -58,16 +65,16 @@ func (c *DiskCache) Get(key [sha256.Size]byte, prov bool) (string, error) { // directories should never happen unless something outside helm is operating // on this content. if fi.IsDir() { - return p, os.ErrInvalid + return p, errors.New("is a directory") } return p, nil } // Put stores the given reader for the given key. // It returns the path to the stored file. -func (c *DiskCache) Put(key [sha256.Size]byte, data io.Reader, prov bool) (string, error) { +func (c *DiskCache) Put(key [sha256.Size]byte, data io.Reader, cacheType string) (string, error) { // TODO: verify the key and digest of the key are the same. - p := c.fileName(key, prov) + p := c.fileName(key, cacheType) if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil { slog.Error("failed to create cache directory") return p, err @@ -77,10 +84,6 @@ func (c *DiskCache) Put(key [sha256.Size]byte, data io.Reader, prov bool) (strin // fileName generates the filename in a structured manner where the first part is the // directory and the full hash is the filename. -func (c *DiskCache) fileName(id [sha256.Size]byte, prov bool) string { - suffix := ".tgz" - if prov { - suffix = ".prov" - } - return filepath.Join(c.Root, fmt.Sprintf("%02x", id[0]), fmt.Sprintf("%x", id)+suffix) +func (c *DiskCache) fileName(id [sha256.Size]byte, cacheType string) string { + return filepath.Join(c.Root, fmt.Sprintf("%02x", id[0]), fmt.Sprintf("%x", id)+cacheType) } diff --git a/pkg/downloader/cache_test.go b/pkg/downloader/cache_test.go new file mode 100644 index 000000000..340c77aba --- /dev/null +++ b/pkg/downloader/cache_test.go @@ -0,0 +1,122 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package downloader + +import ( + "bytes" + "crypto/sha256" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// compiler check to ensure DiskCache implements the Cache interface. +var _ Cache = (*DiskCache)(nil) + +func TestDiskCache_PutAndGet(t *testing.T) { + // Setup a temporary directory for the cache + tmpDir := t.TempDir() + cache := &DiskCache{Root: tmpDir} + + // Test data + content := []byte("hello world") + key := sha256.Sum256(content) + + // --- Test case 1: Put and Get a regular file (prov=false) --- + t.Run("PutAndGetTgz", func(t *testing.T) { + // Put the data into the cache + path, err := cache.Put(key, bytes.NewReader(content), CacheChart) + require.NoError(t, err, "Put should not return an error") + + // Verify the file exists at the returned path + _, err = os.Stat(path) + require.NoError(t, err, "File should exist after Put") + + // Get the file from the cache + retrievedPath, err := cache.Get(key, CacheChart) + require.NoError(t, err, "Get should not return an error for existing file") + assert.Equal(t, path, retrievedPath, "Get should return the same path as Put") + + // Verify content + data, err := os.ReadFile(retrievedPath) + require.NoError(t, err) + assert.Equal(t, content, data, "Content of retrieved file should match original content") + }) + + // --- Test case 2: Put and Get a provenance file (prov=true) --- + t.Run("PutAndGetProv", func(t *testing.T) { + provContent := []byte("provenance data") + provKey := sha256.Sum256(provContent) + + path, err := cache.Put(provKey, bytes.NewReader(provContent), CacheProv) + require.NoError(t, err) + + retrievedPath, err := cache.Get(provKey, CacheProv) + require.NoError(t, err) + assert.Equal(t, path, retrievedPath) + + data, err := os.ReadFile(retrievedPath) + require.NoError(t, err) + assert.Equal(t, provContent, data) + }) + + // --- Test case 3: Get a non-existent file --- + t.Run("GetNonExistent", func(t *testing.T) { + nonExistentKey := sha256.Sum256([]byte("does not exist")) + _, err := cache.Get(nonExistentKey, CacheChart) + assert.ErrorIs(t, err, os.ErrNotExist, "Get for a non-existent key should return os.ErrNotExist") + }) + + // --- Test case 4: Put an empty file --- + t.Run("PutEmptyFile", func(t *testing.T) { + emptyContent := []byte{} + emptyKey := sha256.Sum256(emptyContent) + + path, err := cache.Put(emptyKey, bytes.NewReader(emptyContent), CacheChart) + require.NoError(t, err) + + // Get should return ErrNotExist for empty files + _, err = cache.Get(emptyKey, CacheChart) + assert.ErrorIs(t, err, os.ErrNotExist, "Get for an empty file should return os.ErrNotExist") + + // But the file should exist + _, err = os.Stat(path) + require.NoError(t, err, "Empty file should still exist on disk") + }) + + // --- Test case 5: Get a directory --- + t.Run("GetDirectory", func(t *testing.T) { + dirKey := sha256.Sum256([]byte("i am a directory")) + dirPath := cache.fileName(dirKey, CacheChart) + err := os.MkdirAll(dirPath, 0755) + require.NoError(t, err) + + _, err = cache.Get(dirKey, CacheChart) + assert.EqualError(t, err, "is a directory") + }) +} + +func TestDiskCache_fileName(t *testing.T) { + cache := &DiskCache{Root: "/tmp/cache"} + key := sha256.Sum256([]byte("some data")) + + assert.Equal(t, filepath.Join("/tmp/cache", "13", "1307990e6ba5ca145eb35e99182a9bec46531bc54ddf656a602c780fa0240dee.chart"), cache.fileName(key, CacheChart)) + assert.Equal(t, filepath.Join("/tmp/cache", "13", "1307990e6ba5ca145eb35e99182a9bec46531bc54ddf656a602c780fa0240dee.prov"), cache.fileName(key, CacheProv)) +} diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index bdf65011c..693e6b009 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -23,12 +23,14 @@ import ( "fmt" "io" "io/fs" + "log/slog" "net/url" "os" "path/filepath" "strings" "helm.sh/helm/v4/internal/fileutil" + ifs "helm.sh/helm/v4/internal/third_party/dep/fs" "helm.sh/helm/v4/internal/urlutil" "helm.sh/helm/v4/pkg/getter" "helm.sh/helm/v4/pkg/helmpath" @@ -76,6 +78,11 @@ type ChartDownloader struct { RepositoryConfig string RepositoryCache string + // ContentCache is the location where Cache stores its files by default + // In previous versions of Helm the charts were put in the RepositoryCache. The + // repositories and charts are stored in 2 difference caches. + ContentCache string + // Cache specifies the cache implementation to use. Cache Cache } @@ -93,7 +100,11 @@ type ChartDownloader struct { // (if provenance was verified), or an error if something bad happened. func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *provenance.Verification, error) { if c.Cache == nil { - c.Cache = &DiskCache{Root: c.RepositoryCache} + if c.ContentCache == "" { + return "", nil, errors.New("content cache must be set") + } + c.Cache = &DiskCache{Root: c.ContentCache} + slog.Debug("setup up default downloader cache") } hash, u, err := c.ResolveChartVersion(ref, version) if err != nil { @@ -119,11 +130,12 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven return "", nil, err } copy(digest32[:], digest) - if pth, err := c.Cache.Get(digest32, false); err == nil { + if pth, err := c.Cache.Get(digest32, CacheChart); err == nil { fdata, err := os.ReadFile(pth) if err == nil { found = true data = bytes.NewBuffer(fdata) + slog.Debug("found chart in cache", "id", hash) } } } @@ -154,11 +166,12 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven found = false var body *bytes.Buffer if hash != "" { - if pth, err := c.Cache.Get(digest32, true); err == nil { + if pth, err := c.Cache.Get(digest32, CacheProv); err == nil { fdata, err := os.ReadFile(pth) if err == nil { found = true body = bytes.NewBuffer(fdata) + slog.Debug("found provenance in cache", "id", hash) } } } @@ -192,7 +205,11 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven // DownloadToCache retrieves resources while using a content based cache. func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provenance.Verification, error) { if c.Cache == nil { - c.Cache = &DiskCache{Root: c.RepositoryCache} + if c.ContentCache == "" { + return "", nil, errors.New("content cache must be set") + } + c.Cache = &DiskCache{Root: c.ContentCache} + slog.Debug("setup up default downloader cache") } digestString, u, err := c.ResolveChartVersion(ref, version) @@ -221,9 +238,13 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena var pth string // only fetch from the cache if we have a digest if len(digest) > 0 { - pth, err = c.Cache.Get(digest32, false) + pth, err = c.Cache.Get(digest32, CacheChart) + if err == nil { + slog.Debug("found chart in cache", "id", digestString) + } } if len(digest) == 0 || err != nil { + slog.Debug("attempting to download chart", "ref", ref, "version", version) if err != nil && !os.IsNotExist(err) { return "", nil, err } @@ -236,21 +257,24 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena // Generate the digest if len(digest) == 0 { - h := sha256.New() - digest32 = [sha256.Size]byte(h.Sum(data.Bytes())) + digest32 = sha256.Sum256(data.Bytes()) } - pth, err = c.Cache.Put(digest32, data, false) + pth, err = c.Cache.Put(digest32, data, CacheChart) if err != nil { return "", nil, err } + slog.Debug("put downloaded chart in cache", "id", hex.EncodeToString(digest32[:])) } // If provenance is requested, verify it. ver := &provenance.Verification{} if c.Verify > VerifyNever { - ppth, err := c.Cache.Get(digest32, true) - if err != nil { + + ppth, err := c.Cache.Get(digest32, CacheProv) + if err == nil { + slog.Debug("found provenance in cache", "id", digestString) + } else { if !os.IsNotExist(err) { return pth, ver, err } @@ -264,14 +288,41 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena return pth, ver, nil } - ppth, err = c.Cache.Put(digest32, body, true) + ppth, err = c.Cache.Put(digest32, body, CacheProv) if err != nil { return "", nil, err } + slog.Debug("put downloaded provenance file in cache", "id", hex.EncodeToString(digest32[:])) } if c.Verify != VerifyLater { - ver, err = VerifyChart(pth, ppth, c.Keyring) + + // provenance files pin to a specific name so this needs to be accounted for + // when verifying. + // 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:]) + } + + // Copy chart to a known location with the right name for verification and then + // clean it up. + tmpdir := filepath.Dir(filepath.Join(c.ContentCache, "tmp")) + if err := os.MkdirAll(tmpdir, 0755); err != nil { + return pth, ver, err + } + tmpfile := filepath.Join(tmpdir, name) + err = ifs.CopyFile(pth, tmpfile) + if err != nil { + return pth, ver, err + } + // Not removing the tmp dir itself because a concurrent process may be using it + defer os.RemoveAll(tmpfile) + + ver, err = VerifyChart(tmpfile, ppth, c.Keyring) if err != nil { // Fail always in this case, since it means the verification step // failed. diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 5b5f96751..649448fef 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -16,10 +16,14 @@ limitations under the License. package downloader import ( + "crypto/sha256" + "encoding/hex" "os" "path/filepath" "testing" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/getter" @@ -198,15 +202,19 @@ func TestDownloadTo(t *testing.T) { t.Fatal(err) } + contentCache := t.TempDir() + c := ChartDownloader{ Out: os.Stderr, Verify: VerifyAlways, Keyring: "testdata/helm-test-key.pub", RepositoryConfig: repoConfig, RepositoryCache: repoCache, + ContentCache: contentCache, Getters: getter.All(&cli.EnvSettings{ RepositoryConfig: repoConfig, RepositoryCache: repoCache, + ContentCache: contentCache, }), Options: []getter.Option{ getter.WithBasicAuth("username", "password"), @@ -250,6 +258,7 @@ func TestDownloadTo_TLS(t *testing.T) { repoConfig := filepath.Join(srv.Root(), "repositories.yaml") repoCache := srv.Root() + contentCache := t.TempDir() c := ChartDownloader{ Out: os.Stderr, @@ -257,9 +266,11 @@ func TestDownloadTo_TLS(t *testing.T) { Keyring: "testdata/helm-test-key.pub", RepositoryConfig: repoConfig, RepositoryCache: repoCache, + ContentCache: contentCache, Getters: getter.All(&cli.EnvSettings{ RepositoryConfig: repoConfig, RepositoryCache: repoCache, + ContentCache: contentCache, }), Options: []getter.Option{ getter.WithTLSClientConfig( @@ -304,15 +315,18 @@ func TestDownloadTo_VerifyLater(t *testing.T) { if err := srv.LinkIndices(); err != nil { t.Fatal(err) } + contentCache := t.TempDir() c := ChartDownloader{ Out: os.Stderr, Verify: VerifyLater, RepositoryConfig: repoConfig, RepositoryCache: repoCache, + ContentCache: contentCache, Getters: getter.All(&cli.EnvSettings{ RepositoryConfig: repoConfig, RepositoryCache: repoCache, + ContentCache: contentCache, }), } cname := "/signtest-0.1.0.tgz" @@ -366,3 +380,108 @@ func TestScanReposForURL(t *testing.T) { t.Fatalf("expected ErrNoOwnerRepo, got %v", err) } } + +func TestDownloadToCache(t *testing.T) { + srv := repotest.NewTempServer(t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + ) + defer srv.Stop() + if err := srv.CreateIndex(); err != nil { + t.Fatal(err) + } + if err := srv.LinkIndices(); err != nil { + t.Fatal(err) + } + + // The repo file needs to point to our server. + repoFile := filepath.Join(srv.Root(), "repositories.yaml") + repoCache := srv.Root() + contentCache := t.TempDir() + + c := ChartDownloader{ + Out: os.Stderr, + Verify: VerifyNever, + RepositoryConfig: repoFile, + RepositoryCache: repoCache, + Getters: getter.All(&cli.EnvSettings{ + RepositoryConfig: repoFile, + RepositoryCache: repoCache, + ContentCache: contentCache, + }), + Cache: &DiskCache{Root: contentCache}, + } + + // Case 1: Chart not in cache, download it. + t.Run("download and cache chart", func(t *testing.T) { + // Clear cache for this test + os.RemoveAll(contentCache) + os.MkdirAll(contentCache, 0755) + c.Cache = &DiskCache{Root: contentCache} + + pth, v, err := c.DownloadToCache("test/signtest", "0.1.0") + require.NoError(t, err) + require.NotNil(t, v) + + // Check that the file exists at the returned path + _, err = os.Stat(pth) + require.NoError(t, err, "chart should exist at returned path") + + // Check that it's in the cache + digest, _, err := c.ResolveChartVersion("test/signtest", "0.1.0") + require.NoError(t, err) + digestBytes, err := hex.DecodeString(digest) + require.NoError(t, err) + var digestArray [sha256.Size]byte + copy(digestArray[:], digestBytes) + + cachePath, err := c.Cache.Get(digestArray, CacheChart) + require.NoError(t, err, "chart should now be in cache") + require.Equal(t, pth, cachePath) + }) + + // Case 2: Chart is in cache, get from cache. + t.Run("get chart from cache", func(t *testing.T) { + // The cache should be populated from the previous test. + // To prove it's coming from cache, we can stop the server. + // But repotest doesn't support restarting. + // Let's just call it again and assume it works if it's fast and doesn't error. + pth, v, err := c.DownloadToCache("test/signtest", "0.1.0") + require.NoError(t, err) + require.NotNil(t, v) + + _, err = os.Stat(pth) + require.NoError(t, err, "chart should exist at returned path") + }) + + // Case 3: Download with verification + t.Run("download and verify", func(t *testing.T) { + // Clear cache + os.RemoveAll(contentCache) + os.MkdirAll(contentCache, 0755) + c.Cache = &DiskCache{Root: contentCache} + c.Verify = VerifyAlways + c.Keyring = "testdata/helm-test-key.pub" + + _, v, err := c.DownloadToCache("test/signtest", "0.1.0") + require.NoError(t, err) + require.NotNil(t, v) + require.NotEmpty(t, v.FileHash, "verification should have a file hash") + + // Check that both chart and prov are in cache + digest, _, err := c.ResolveChartVersion("test/signtest", "0.1.0") + require.NoError(t, err) + digestBytes, err := hex.DecodeString(digest) + require.NoError(t, err) + var digestArray [sha256.Size]byte + copy(digestArray[:], digestBytes) + + _, err = c.Cache.Get(digestArray, CacheChart) + require.NoError(t, err, "chart should be in cache") + _, err = c.Cache.Get(digestArray, CacheProv) + require.NoError(t, err, "provenance file should be in cache") + + // Reset for other tests + c.Verify = VerifyNever + c.Keyring = "" + }) +} diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index b43165975..8b77a77c0 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -75,6 +75,9 @@ type Manager struct { RegistryClient *registry.Client RepositoryConfig string RepositoryCache string + + // ContentCache is a location where a cache of charts can be stored + ContentCache string } // Build rebuilds a local charts directory from a lockfile. @@ -331,6 +334,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { Keyring: m.Keyring, RepositoryConfig: m.RepositoryConfig, RepositoryCache: m.RepositoryCache, + ContentCache: m.ContentCache, RegistryClient: m.RegistryClient, Getters: m.Getters, Options: []getter.Option{ diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index f01a5d7ad..b7121a4ce 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -488,12 +488,14 @@ func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Depe Schemes: []string{"http", "https"}, New: getter.NewHTTPGetter, }} + contentCache := t.TempDir() m := &Manager{ ChartPath: dir(chartName), Out: b, Getters: g, RepositoryConfig: dir("repositories.yaml"), RepositoryCache: dir(), + ContentCache: contentCache, } // First build will update dependencies and create Chart.lock file.