From 37482853ae7272e1474d96e3930a53976a051019 Mon Sep 17 00:00:00 2001 From: maheshrijal <62394512+maheshrijal@users.noreply.github.com> Date: Sun, 15 Feb 2026 22:44:58 +0530 Subject: [PATCH 1/2] feat(dependency): add untar support and harden extraction flow Signed-off-by: maheshrijal <62394512+maheshrijal@users.noreply.github.com> --- pkg/action/dependency.go | 3 + pkg/cmd/dependency.go | 6 +- pkg/cmd/dependency_build.go | 2 +- pkg/cmd/dependency_update.go | 4 +- pkg/cmd/dependency_update_test.go | 140 +++++++++++ pkg/downloader/manager.go | 400 +++++++++++++++++++++++++++++- pkg/downloader/manager_test.go | 393 ++++++++++++++++++++++++++++- 7 files changed, 937 insertions(+), 11 deletions(-) diff --git a/pkg/action/dependency.go b/pkg/action/dependency.go index b12887bde..8ddaf0b2c 100644 --- a/pkg/action/dependency.go +++ b/pkg/action/dependency.go @@ -35,6 +35,8 @@ import ( // It provides the implementation of 'helm dependency' and its respective subcommands. type Dependency struct { Verify bool + Untar bool + UntarDir string Keyring string SkipRefresh bool ColumnWidth uint @@ -51,6 +53,7 @@ type Dependency struct { func NewDependency() *Dependency { return &Dependency{ ColumnWidth: 80, + UntarDir: "charts", } } diff --git a/pkg/cmd/dependency.go b/pkg/cmd/dependency.go index 5978c902a..a3e723847 100644 --- a/pkg/cmd/dependency.go +++ b/pkg/cmd/dependency.go @@ -122,7 +122,7 @@ func newDependencyListCmd(out io.Writer) *cobra.Command { return cmd } -func addDependencySubcommandFlags(f *pflag.FlagSet, client *action.Dependency) { +func addDependencySubcommandFlags(f *pflag.FlagSet, client *action.Dependency, withUntar bool) { f.BoolVar(&client.Verify, "verify", false, "verify the packages against signatures") f.StringVar(&client.Keyring, "keyring", defaultKeyring(), "keyring containing public keys") f.BoolVar(&client.SkipRefresh, "skip-refresh", false, "do not refresh the local repository cache") @@ -133,4 +133,8 @@ 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") + if withUntar { + f.BoolVar(&client.Untar, "untar", false, "if set to true, will untar the dependency charts after downloading them and remove the chart archives") + f.StringVar(&client.UntarDir, "untardir", "charts", "if untar is specified, this flag specifies the directory (relative to chart root) into which dependencies are expanded") + } } diff --git a/pkg/cmd/dependency_build.go b/pkg/cmd/dependency_build.go index b8ac16e60..72df5b1e7 100644 --- a/pkg/cmd/dependency_build.go +++ b/pkg/cmd/dependency_build.go @@ -86,7 +86,7 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command { } f := cmd.Flags() - addDependencySubcommandFlags(f, client) + addDependencySubcommandFlags(f, client, false) return cmd } diff --git a/pkg/cmd/dependency_update.go b/pkg/cmd/dependency_update.go index 7f805c37b..a8973ac2e 100644 --- a/pkg/cmd/dependency_update.go +++ b/pkg/cmd/dependency_update.go @@ -68,6 +68,8 @@ func newDependencyUpdateCmd(_ *action.Configuration, out io.Writer) *cobra.Comma Out: out, ChartPath: chartpath, Keyring: client.Keyring, + Untar: client.Untar, + UntarDir: client.UntarDir, SkipUpdate: client.SkipRefresh, Getters: getter.All(settings), RegistryClient: registryClient, @@ -84,7 +86,7 @@ func newDependencyUpdateCmd(_ *action.Configuration, out io.Writer) *cobra.Comma } f := cmd.Flags() - addDependencySubcommandFlags(f, client) + addDependencySubcommandFlags(f, client, true) return cmd } diff --git a/pkg/cmd/dependency_update_test.go b/pkg/cmd/dependency_update_test.go index 3eaa51df1..fff9c5221 100644 --- a/pkg/cmd/dependency_update_test.go +++ b/pkg/cmd/dependency_update_test.go @@ -151,6 +151,132 @@ func TestDependencyUpdateCmd(t *testing.T) { } } +func TestDependencyUpdateCmd_Untar(t *testing.T) { + srv := setupMockRepoServer(t) + defer srv.Stop() + + dir := func(p ...string) string { + return filepath.Join(append([]string{srv.Root()}, p...)...) + } + + chartname := "depup-untar" + ch := createTestingMetadata(chartname, srv.URL()) + if err := chartutil.SaveDir(ch, dir()); err != nil { + t.Fatal(err) + } + + contentCache := t.TempDir() + cmd := fmt.Sprintf( + "dependency update '%s' --untar --repository-config %s --repository-cache %s --content-cache %s --plain-http", + dir(chartname), + dir("repositories.yaml"), + dir(), + contentCache, + ) + + _, out, err := executeActionCommand(cmd) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } + + for _, dependencyName := range []string{"reqtest", "compressedchart"} { + unpackedPath := dir(chartname, "charts", dependencyName) + if fi, err := os.Stat(unpackedPath); err != nil || !fi.IsDir() { + t.Fatalf("Expected unpacked chart directory %q: %v", unpackedPath, err) + } + } + + for _, archivePath := range []string{ + dir(chartname, "charts", "reqtest-0.1.0.tgz"), + dir(chartname, "charts", "compressedchart-0.1.0.tgz"), + } { + if _, err := os.Stat(archivePath); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("Expected chart archive to be deleted %q", archivePath) + } + } + + _, out, err = executeActionCommand(cmd) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } +} + +func TestDependencyUpdateCmd_UntarDir(t *testing.T) { + srv := setupMockRepoServer(t) + defer srv.Stop() + + dir := func(p ...string) string { + return filepath.Join(append([]string{srv.Root()}, p...)...) + } + + chartname := "depup-untardir" + ch := createTestingMetadata(chartname, srv.URL()) + if err := chartutil.SaveDir(ch, dir()); err != nil { + t.Fatal(err) + } + + contentCache := t.TempDir() + cmd := fmt.Sprintf( + "dependency update '%s' --untar --untardir vendor/charts --repository-config %s --repository-cache %s --content-cache %s --plain-http", + dir(chartname), + dir("repositories.yaml"), + dir(), + contentCache, + ) + + _, out, err := executeActionCommand(cmd) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } + + for _, dependencyName := range []string{"reqtest", "compressedchart"} { + unpackedPath := dir(chartname, "vendor", "charts", dependencyName) + if fi, err := os.Stat(unpackedPath); err != nil || !fi.IsDir() { + t.Fatalf("Expected unpacked chart directory %q: %v", unpackedPath, err) + } + } +} + +func TestDependencyUpdateCmd_UntarWithAliases(t *testing.T) { + srv := setupMockRepoServer(t) + defer srv.Stop() + + dir := func(p ...string) string { + return filepath.Join(append([]string{srv.Root()}, p...)...) + } + + chartname := "depup-untar-alias" + ch := createTestingMetadataWithAliases(chartname, srv.URL()) + if err := chartutil.SaveDir(ch, dir()); err != nil { + t.Fatal(err) + } + + contentCache := t.TempDir() + cmd := fmt.Sprintf( + "dependency update '%s' --untar --repository-config %s --repository-cache %s --content-cache %s --plain-http", + dir(chartname), + dir("repositories.yaml"), + dir(), + contentCache, + ) + + _, out, err := executeActionCommand(cmd) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } + + for _, dependencyName := range []string{"cache-a", "cache-b"} { + unpackedPath := dir(chartname, "charts", dependencyName) + if fi, err := os.Stat(unpackedPath); err != nil || !fi.IsDir() { + t.Fatalf("Expected unpacked chart directory %q: %v", unpackedPath, err) + } + } +} + func TestDependencyUpdateCmd_DoNotDeleteOldChartsOnError(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) @@ -300,6 +426,20 @@ func createTestingMetadataForOCI(name, registryURL string) *chart.Chart { } } +func createTestingMetadataWithAliases(name, baseURL string) *chart.Chart { + return &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: name, + Version: "1.2.3", + Dependencies: []*chart.Dependency{ + {Name: "reqtest", Alias: "cache-a", Version: "0.1.0", Repository: baseURL}, + {Name: "reqtest", Alias: "cache-b", Version: "0.1.0", Repository: baseURL}, + }, + }, + } +} + // createTestingChart creates a basic chart that depends on reqtest-0.1.0 // // The baseURL can be used to point to a particular repository server. diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 6043fbaaa..d47410c09 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -27,6 +27,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" "sync" @@ -68,6 +69,10 @@ type Manager struct { Debug bool // Keyring is the key ring file. Keyring string + // Untar indicates that downloaded dependencies should be unpacked. + Untar bool + // UntarDir is the root directory where unpacked dependencies are written. + UntarDir string // SkipUpdate indicates that the repository should not be updated first. SkipUpdate bool // Getter collection for the operation @@ -144,7 +149,8 @@ func (m *Manager) Build() error { } // Now we need to fetch every package here into charts/ - return m.downloadAll(lock.Dependencies) + // Use lock deps for download, and original metadata deps for extraction naming (aliases). + return m.downloadAll(lock.Dependencies, req) } // Update updates a local charts directory. @@ -200,7 +206,7 @@ func (m *Manager) Update() error { } // Now we need to fetch every package here into charts/ - if err := m.downloadAll(lock.Dependencies); err != nil { + if err := m.downloadAll(lock.Dependencies, req); err != nil { return err } @@ -242,7 +248,7 @@ func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) // // It will delete versions of the chart that exist on disk and might cause // a conflict. -func (m *Manager) downloadAll(deps []*chart.Dependency) error { +func (m *Manager) downloadAll(downloadDeps, extractionDeps []*chart.Dependency) error { repos, err := m.loadChartRepositories() if err != nil { return err @@ -270,10 +276,10 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { } defer os.RemoveAll(tmpPath) - fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps)) + fmt.Fprintf(m.Out, "Saving %d charts\n", len(downloadDeps)) var saveError error churls := make(map[string]struct{}) - for _, dep := range deps { + for _, dep := range downloadDeps { // No repository means the chart is in charts directory if dep.Repository == "" { fmt.Fprintf(m.Out, "Dependency %s did not declare a repository. Assuming it exists in the charts directory\n", dep.Name) @@ -367,9 +373,14 @@ 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(downloadDeps, tmpPath, destPath); err != nil { return err } + if m.Untar { + if err := m.untarDeps(extractionDeps, downloadDeps, destPath); err != nil { + return err + } + } } else { fmt.Fprintln(m.Out, "Save error occurred: ", saveError) return saveError @@ -377,6 +388,383 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { return nil } +type depExtractionTarget struct { + dep *chart.Dependency + archive string + extracted string + targetName string +} + +func (m *Manager) untarDeps(extractionDeps, resolvedDeps []*chart.Dependency, sourcePath string) error { + targets, err := m.buildExtractionTargets(extractionDeps, resolvedDeps, sourcePath) + if err != nil { + return err + } + if len(targets) == 0 { + return nil + } + + untarRoot, err := m.resolveUntarRoot() + if err != nil { + return err + } + if err := os.MkdirAll(untarRoot, 0755); err != nil { + return err + } + + tmpUntarRoot, err := os.MkdirTemp(m.ChartPath, "tmpuntar-") + if err != nil { + return err + } + defer os.RemoveAll(tmpUntarRoot) + tmpExtractRoot := filepath.Join(tmpUntarRoot, ".extract") + if err := os.MkdirAll(tmpExtractRoot, 0755); err != nil { + return err + } + + for i, target := range targets { + extractRoot := filepath.Join(tmpExtractRoot, strconv.Itoa(i)) + if err := os.MkdirAll(extractRoot, 0755); err != nil { + return err + } + if err := chartutil.ExpandFile(extractRoot, target.archive); err != nil { + return fmt.Errorf("failed to untar %s: %w", filepath.Base(target.archive), err) + } + + extractedPath := filepath.Join(extractRoot, target.extracted) + finalPath := filepath.Join(tmpUntarRoot, target.targetName) + if _, err := os.Stat(finalPath); err == nil { + return fmt.Errorf("failed to untar: a file or directory with the name %s already exists", filepath.Join(untarRoot, target.targetName)) + } else if !errors.Is(err, os.ErrNotExist) { + return err + } + if err := os.Rename(extractedPath, finalPath); err != nil { + return fmt.Errorf("failed to untar %s: %w", filepath.Base(target.archive), err) + } + } + + expectedTargets := make(map[string]struct{}, len(targets)) + for _, target := range targets { + expectedTargets[target.targetName] = struct{}{} + destPath := filepath.Join(untarRoot, target.targetName) + + if _, err := os.Stat(filepath.Join(tmpUntarRoot, target.targetName)); err != nil { + return fmt.Errorf("failed to untar %s: %w", filepath.Base(target.archive), err) + } + + info, err := os.Stat(destPath) + if err == nil && !info.IsDir() { + return fmt.Errorf("failed to untar: %s exists and is not a directory", destPath) + } + if err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } + + for _, target := range targets { + destPath := filepath.Join(untarRoot, target.targetName) + if err := os.RemoveAll(destPath); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + if err := fs.RenameWithFallback(filepath.Join(tmpUntarRoot, target.targetName), destPath); err != nil { + return fmt.Errorf("failed to untar %s: %w", filepath.Base(target.archive), err) + } + } + + if m.shouldCleanupUntarRoot(untarRoot) { + if err := m.cleanupOutdatedUnpackedCharts(extractionDeps, untarRoot, expectedTargets); err != nil { + return err + } + } + + if m.shouldCleanupUntarRoot(untarRoot) { + archives := make(map[string]struct{}, len(targets)) + for _, target := range targets { + archives[target.archive] = struct{}{} + } + for archive := range archives { + if err := os.Remove(archive); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } + } + + return nil +} + +func (m *Manager) buildExtractionTargets(extractionDeps, resolvedDeps []*chart.Dependency, sourcePath string) ([]depExtractionTarget, error) { + targets := make([]depExtractionTarget, 0, len(extractionDeps)) + seenTargetNames := make(map[string]string, len(extractionDeps)) + for i, dep := range extractionDeps { + if dep.Repository == "" { + continue + } + + targetName := dep.Name + if dep.Alias != "" { + targetName = dep.Alias + } + if previous, ok := seenTargetNames[targetName]; ok { + return nil, fmt.Errorf("dependency conflict detected: dependencies %q and %q resolve to the same target directory %q. Please set unique aliases", previous, dep.Name, targetName) + } + seenTargetNames[targetName] = dep.Name + + resolvedDep, err := findResolvedDependencyAtIndex(dep, resolvedDeps, i) + if err != nil { + return nil, err + } + archive, err := findDependencyArchive(sourcePath, dep, resolvedDep) + if err != nil { + return nil, err + } + ch, err := loader.LoadFile(archive) + if err != nil { + return nil, err + } + extracted := ch.Name() + if extracted == "" { + extracted = dep.Name + } + + targets = append(targets, depExtractionTarget{ + dep: dep, + archive: archive, + extracted: extracted, + targetName: targetName, + }) + } + + return targets, nil +} + +func findResolvedDependencyAtIndex(dep *chart.Dependency, resolvedDeps []*chart.Dependency, depIndex int) (*chart.Dependency, error) { + if depIndex >= 0 && depIndex < len(resolvedDeps) { + resolvedDep := resolvedDeps[depIndex] + if resolvedDep.Repository != "" && + resolvedDep.Name == dep.Name && + resolvedDep.Repository == dep.Repository && + dependencyVersionMatches(dep.Version, resolvedDep.Version) { + return resolvedDep, nil + } + } + return findResolvedDependency(dep, resolvedDeps) +} + +func findResolvedDependency(dep *chart.Dependency, resolvedDeps []*chart.Dependency) (*chart.Dependency, error) { + candidates := make([]*chart.Dependency, 0, 1) + for _, resolvedDep := range resolvedDeps { + if resolvedDep.Repository == "" { + continue + } + if resolvedDep.Name != dep.Name { + continue + } + if dep.Repository != resolvedDep.Repository { + continue + } + if !dependencyVersionMatches(dep.Version, resolvedDep.Version) { + continue + } + candidates = append(candidates, resolvedDep) + } + if len(candidates) == 1 { + return candidates[0], nil + } + if len(candidates) > 1 { + baseVersion := candidates[0].Version + sameVersion := true + for _, candidate := range candidates[1:] { + if candidate.Version != baseVersion { + sameVersion = false + break + } + } + if sameVersion { + return candidates[0], nil + } + return nil, fmt.Errorf("dependency conflict detected: found multiple resolved versions for dependency %q with constraint %q", dep.Name, dep.Version) + } + + return nil, nil +} + +func findDependencyArchive(sourcePath string, dep, resolvedDep *chart.Dependency) (string, error) { + matches, err := filepath.Glob(filepath.Join(sourcePath, fmt.Sprintf("%s-*.tgz", dep.Name))) + if err != nil { + return "", err + } + + versionConstraint := dep.Version + if resolvedDep != nil && resolvedDep.Version != "" { + versionConstraint = resolvedDep.Version + } + + for _, candidate := range matches { + ch, err := loader.LoadFile(candidate) + if err != nil { + continue + } + if ch.Metadata.Name == dep.Name && dependencyVersionMatches(versionConstraint, ch.Metadata.Version) { + return candidate, nil + } + } + + return "", fmt.Errorf("could not find downloaded dependency archive for %s-%s", dep.Name, dep.Version) +} + +func dependencyVersionMatches(versionConstraint, actualVersion string) bool { + if versionConstraint == "" || versionEquals(versionConstraint, actualVersion) { + return true + } + + constraint, err := semver.NewConstraint(versionConstraint) + if err != nil { + return false + } + actual, err := semver.NewVersion(actualVersion) + if err != nil { + return false + } + return constraint.Check(actual) +} + +func (m *Manager) resolveUntarRoot() (string, error) { + chartRoot, err := filepath.Abs(m.ChartPath) + if err != nil { + return "", err + } + chartRoot, err = filepath.EvalSymlinks(chartRoot) + if err != nil { + return "", err + } + + untarRoot := m.UntarDir + if untarRoot == "" { + untarRoot = "charts" + } else { + untarRoot = filepath.Clean(untarRoot) + } + if filepath.IsAbs(untarRoot) { + return "", fmt.Errorf("--untardir must be relative to the chart root") + } + untarRoot = filepath.Join(chartRoot, untarRoot) + untarRoot, err = resolvePathThroughExistingSymlinks(untarRoot) + if err != nil { + return "", err + } + + relToChartRoot, err := filepath.Rel(chartRoot, untarRoot) + if err != nil { + return "", err + } + if relToChartRoot == ".." || strings.HasPrefix(relToChartRoot, ".."+string(os.PathSeparator)) { + return "", fmt.Errorf("--untardir must stay within the chart root") + } + + info, err := os.Stat(untarRoot) + if err == nil && !info.IsDir() { + return "", fmt.Errorf("%q is not a directory", untarRoot) + } + if err != nil && !errors.Is(err, os.ErrNotExist) { + return "", err + } + + return untarRoot, nil +} + +func resolvePathThroughExistingSymlinks(path string) (string, error) { + path, err := filepath.Abs(path) + if err != nil { + return "", err + } + + current := path + var suffix []string + for { + if _, err := os.Lstat(current); err == nil { + break + } else if !errors.Is(err, os.ErrNotExist) { + return "", err + } + + parent, base := filepath.Split(current) + parent = filepath.Clean(parent) + if parent == current { + break + } + suffix = append([]string{base}, suffix...) + current = parent + } + + resolvedCurrent, err := filepath.EvalSymlinks(current) + if err != nil { + return "", err + } + + for _, part := range suffix { + resolvedCurrent = filepath.Join(resolvedCurrent, part) + } + + return filepath.Clean(resolvedCurrent), nil +} + +func (m *Manager) cleanupOutdatedUnpackedCharts(deps []*chart.Dependency, untarRoot string, expectedTargets map[string]struct{}) error { + localDependencyTargets := make(map[string]struct{}, len(deps)*2) + for _, dep := range deps { + if dep.Repository != "" { + continue + } + localDependencyTargets[dep.Name] = struct{}{} + if dep.Alias != "" { + localDependencyTargets[dep.Alias] = struct{}{} + } + } + + rootEntries, err := os.ReadDir(untarRoot) + if err != nil { + return err + } + for _, entry := range rootEntries { + if !entry.IsDir() { + continue + } + if _, ok := expectedTargets[entry.Name()]; ok { + continue + } + if _, ok := localDependencyTargets[entry.Name()]; ok { + continue + } + + candidate := filepath.Join(untarRoot, entry.Name()) + if _, err := loader.LoadDir(candidate); err != nil { + continue + } + + if err := os.RemoveAll(candidate); err != nil { + return err + } + } + + return nil +} + +func (m *Manager) shouldCleanupUntarRoot(untarRoot string) bool { + chartRoot, err := filepath.Abs(m.ChartPath) + if err != nil { + return false + } + chartRoot, err = filepath.EvalSymlinks(chartRoot) + if err != nil { + return false + } + defaultUntarRoot := filepath.Join(chartRoot, "charts") + resolvedUntarRoot, err := filepath.Abs(untarRoot) + if err != nil { + return false + } + return filepath.Clean(resolvedUntarRoot) == filepath.Clean(defaultUntarRoot) +} + func parseOCIRef(chartRef string) (string, string, error) { refTagRegexp := regexp.MustCompile(`^(oci://[^:]+(:[0-9]{1,5})?[^:]+):(.*)$`) caps := refTagRegexp.FindStringSubmatch(chartRef) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 9e27f183f..26b55e4de 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -18,10 +18,12 @@ package downloader import ( "bytes" "errors" + "io" "io/fs" "os" "path/filepath" "reflect" + "runtime" "testing" "time" @@ -267,7 +269,7 @@ func TestDownloadAll(t *testing.T) { if err := os.MkdirAll(filepath.Join(chartPath, "tmpcharts"), 0755); err != nil { t.Fatal(err) } - if err := m.downloadAll([]*chart.Dependency{signDep, localDep}); err != nil { + if err := m.downloadAll([]*chart.Dependency{signDep, localDep}, []*chart.Dependency{signDep, localDep}); err != nil { t.Error(err) } @@ -296,12 +298,73 @@ version: 0.1.0` Version: "0.1.0", } - err = m.downloadAll([]*chart.Dependency{badLocalDep}) + err = m.downloadAll([]*chart.Dependency{badLocalDep}, []*chart.Dependency{badLocalDep}) if err == nil { t.Fatal("Expected error for bad dependency name") } } +func TestDownloadAll_UntarCleansArchivesAndUsesAliases(t *testing.T) { + chartPath := t.TempDir() + m := &Manager{ + Out: new(bytes.Buffer), + RepositoryConfig: repoConfig, + RepositoryCache: repoCache, + ChartPath: chartPath, + Untar: true, + UntarDir: "charts", + } + + signtest, err := loader.LoadDir(filepath.Join("testdata", "signtest")) + if err != nil { + t.Fatal(err) + } + if err := chartutil.SaveDir(signtest, filepath.Join(chartPath, "testdata")); err != nil { + t.Fatal(err) + } + + local, err := loader.LoadDir(filepath.Join("testdata", "local-subchart")) + if err != nil { + t.Fatal(err) + } + if err := chartutil.SaveDir(local, filepath.Join(chartPath, "charts")); err != nil { + t.Fatal(err) + } + + remoteDep := &chart.Dependency{ + Name: signtest.Name(), + Repository: "file://./testdata/signtest", + Version: signtest.Metadata.Version, + } + remoteAliasDep := &chart.Dependency{ + Name: signtest.Name(), + Alias: "cache-a", + Repository: "file://./testdata/signtest", + Version: signtest.Metadata.Version, + } + localDep := &chart.Dependency{ + Name: local.Name(), + Repository: "", + Version: local.Metadata.Version, + } + + if err := m.downloadAll( + []*chart.Dependency{remoteDep, remoteAliasDep, localDep}, + []*chart.Dependency{remoteDep, remoteAliasDep, localDep}, + ); err != nil { + t.Fatal(err) + } + + _, err = os.Stat(filepath.Join(chartPath, "charts", "signtest")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(chartPath, "charts", "cache-a")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(chartPath, "charts", "signtest-0.1.0.tgz")) + assert.True(t, errors.Is(err, fs.ErrNotExist)) + _, err = os.Stat(filepath.Join(chartPath, "charts", "local-subchart")) + assert.NoError(t, err) +} + func TestUpdateBeforeBuild(t *testing.T) { // Set up a fake repo srv := repotest.NewTempServer( @@ -767,3 +830,329 @@ func TestWriteLock(t *testing.T) { assert.Error(t, err) }) } + +func TestBuildExtractionTargets_ConflictingTargetNames(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + createDependencyArchive(t, sourcePath, "compressedchart", "0.1.0") + + deps := []*chart.Dependency{ + {Name: "reqtest", Alias: "shared", Version: "0.1.0", Repository: "https://example.com/charts"}, + {Name: "compressedchart", Alias: "shared", Version: "0.1.0", Repository: "https://example.com/charts"}, + } + m := &Manager{} + + _, err := m.buildExtractionTargets(deps, deps, sourcePath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "resolve to the same target directory") +} + +func TestUntarDeps_AliasAndCleanup(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + createDependencyDir(t, sourcePath, "oldchart", "0.1.0") + + deps := []*chart.Dependency{ + {Name: "reqtest", Alias: "cache-a", Version: "0.1.0", Repository: "https://example.com/charts"}, + } + m := &Manager{ + Out: io.Discard, + ChartPath: chartPath, + Untar: true, + UntarDir: "charts", + } + + err := m.untarDeps(deps, deps, sourcePath) + assert.NoError(t, err) + + _, err = os.Stat(filepath.Join(sourcePath, "cache-a")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(sourcePath, "reqtest-0.1.0.tgz")) + assert.True(t, errors.Is(err, fs.ErrNotExist)) + _, err = os.Stat(filepath.Join(sourcePath, "oldchart")) + assert.True(t, errors.Is(err, fs.ErrNotExist)) +} + +func TestUntarDeps_MixedAliasAndNonAliasSameChart(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + + extractionDeps := []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + {Name: "reqtest", Alias: "cache-a", Version: "0.1.0", Repository: "https://example.com/charts"}, + } + resolvedDeps := []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + } + m := &Manager{ + Out: io.Discard, + ChartPath: chartPath, + Untar: true, + UntarDir: "charts", + } + + err := m.untarDeps(extractionDeps, resolvedDeps, sourcePath) + assert.NoError(t, err) + + _, err = os.Stat(filepath.Join(sourcePath, "reqtest")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(sourcePath, "cache-a")) + assert.NoError(t, err) +} + +func TestUntarDeps_Idempotent(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + + deps := []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + } + m := &Manager{ + Out: io.Discard, + ChartPath: chartPath, + Untar: true, + UntarDir: "charts", + } + + assert.NoError(t, m.untarDeps(deps, deps, sourcePath)) + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + assert.NoError(t, m.untarDeps(deps, deps, sourcePath)) +} + +func TestUntarDeps_NumericAlias(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + + deps := []*chart.Dependency{ + {Name: "reqtest", Alias: "0", Version: "0.1.0", Repository: "https://example.com/charts"}, + } + m := &Manager{ + Out: io.Discard, + ChartPath: chartPath, + Untar: true, + UntarDir: "charts", + } + + assert.NoError(t, m.untarDeps(deps, deps, sourcePath)) + _, err := os.Stat(filepath.Join(sourcePath, "0")) + assert.NoError(t, err) +} + +func TestUntarDeps_CustomUntarDirDoesNotCleanupOutdated(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + customUntarRoot := filepath.Join(chartPath, "vendor", "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + assert.NoError(t, os.MkdirAll(customUntarRoot, 0755)) + + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + createDependencyDir(t, customUntarRoot, "oldchart", "0.1.0") + + deps := []*chart.Dependency{ + {Name: "reqtest", Alias: "cache-a", Version: "0.1.0", Repository: "https://example.com/charts"}, + } + m := &Manager{ + Out: io.Discard, + ChartPath: chartPath, + Untar: true, + UntarDir: "vendor/charts", + } + + err := m.untarDeps(deps, deps, sourcePath) + assert.NoError(t, err) + + _, err = os.Stat(filepath.Join(customUntarRoot, "cache-a")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(customUntarRoot, "oldchart")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(sourcePath, "reqtest-0.1.0.tgz")) + assert.NoError(t, err) +} + +func TestUntarDeps_DefaultUntarRootPreservesLocalDependencies(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + createDependencyDir(t, sourcePath, "local-subchart", "0.1.0") + createDependencyDir(t, sourcePath, "oldchart", "0.1.0") + + extractionDeps := []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + {Name: "local-subchart", Version: "0.1.0", Repository: ""}, + } + m := &Manager{ + Out: io.Discard, + ChartPath: chartPath, + Untar: true, + UntarDir: "charts", + } + + err := m.untarDeps(extractionDeps, extractionDeps, sourcePath) + assert.NoError(t, err) + + _, err = os.Stat(filepath.Join(sourcePath, "reqtest")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(sourcePath, "local-subchart")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(sourcePath, "oldchart")) + assert.True(t, errors.Is(err, fs.ErrNotExist)) +} + +func TestResolveUntarRoot(t *testing.T) { + chartPath := t.TempDir() + + t.Run("default charts dir", func(t *testing.T) { + m := &Manager{ChartPath: chartPath} + untarRoot, err := m.resolveUntarRoot() + assert.NoError(t, err) + resolvedChartPath, err := filepath.EvalSymlinks(chartPath) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(resolvedChartPath, "charts"), untarRoot) + }) + + t.Run("relative path under chart root", func(t *testing.T) { + m := &Manager{ChartPath: chartPath, UntarDir: "vendor/charts"} + untarRoot, err := m.resolveUntarRoot() + assert.NoError(t, err) + resolvedChartPath, err := filepath.EvalSymlinks(chartPath) + assert.NoError(t, err) + assert.Equal(t, filepath.Join(resolvedChartPath, "vendor", "charts"), untarRoot) + }) + + t.Run("path traversal escapes chart root", func(t *testing.T) { + m := &Manager{ChartPath: chartPath, UntarDir: "../outside"} + _, err := m.resolveUntarRoot() + assert.Error(t, err) + assert.Contains(t, err.Error(), "must stay within the chart root") + }) + + t.Run("absolute untardir is rejected", func(t *testing.T) { + m := &Manager{ChartPath: chartPath, UntarDir: filepath.Join(os.TempDir(), "helm-outside")} + _, err := m.resolveUntarRoot() + assert.Error(t, err) + assert.Contains(t, err.Error(), "must be relative to the chart root") + }) + + t.Run("symlink escape is rejected", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink permissions are not portable on windows test environments") + } + + outside := filepath.Join(t.TempDir(), "outside") + assert.NoError(t, os.MkdirAll(outside, 0755)) + assert.NoError(t, os.Symlink(outside, filepath.Join(chartPath, "vendor"))) + + m := &Manager{ChartPath: chartPath, UntarDir: "vendor/charts"} + _, err := m.resolveUntarRoot() + assert.Error(t, err) + assert.Contains(t, err.Error(), "must stay within the chart root") + }) +} + +func TestShouldCleanupUntarRoot(t *testing.T) { + cwd, err := os.Getwd() + assert.NoError(t, err) + + t.Run("relative chart path default untar root", func(t *testing.T) { + m := &Manager{ChartPath: "."} + assert.True(t, m.shouldCleanupUntarRoot(filepath.Join(cwd, "charts"))) + }) + + t.Run("relative chart path custom untar root", func(t *testing.T) { + m := &Manager{ChartPath: "."} + assert.False(t, m.shouldCleanupUntarRoot(filepath.Join(cwd, "vendor", "charts"))) + }) +} + +func TestFindResolvedDependency_PicksMatchingVersion(t *testing.T) { + dep := &chart.Dependency{ + Name: "reqtest", + Version: "0.3.0", + Repository: "https://example.com/charts", + } + resolved, err := findResolvedDependency(dep, []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + {Name: "reqtest", Version: "0.3.0", Repository: "https://example.com/charts"}, + }) + assert.NoError(t, err) + assert.NotNil(t, resolved) + assert.Equal(t, "0.3.0", resolved.Version) +} + +func TestFindResolvedDependency_ReturnsErrorOnAmbiguousMatch(t *testing.T) { + dep := &chart.Dependency{ + Name: "reqtest", + Version: ">=0.1.0", + Repository: "https://example.com/charts", + } + resolved, err := findResolvedDependency(dep, []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + {Name: "reqtest", Version: "0.3.0", Repository: "https://example.com/charts"}, + }) + assert.Error(t, err) + assert.Nil(t, resolved) + assert.Contains(t, err.Error(), "found multiple resolved versions") +} + +func TestBuildExtractionTargets_PrefersResolvedDependencyByIndex(t *testing.T) { + chartPath := t.TempDir() + sourcePath := filepath.Join(chartPath, "charts") + assert.NoError(t, os.MkdirAll(sourcePath, 0755)) + createDependencyArchive(t, sourcePath, "reqtest", "0.1.0") + createDependencyArchive(t, sourcePath, "reqtest", "0.3.0") + + extractionDeps := []*chart.Dependency{ + {Name: "reqtest", Alias: "cache-a", Version: ">=0.1.0", Repository: "https://example.com/charts"}, + {Name: "reqtest", Alias: "cache-b", Version: ">=0.1.0", Repository: "https://example.com/charts"}, + } + resolvedDeps := []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: "https://example.com/charts"}, + {Name: "reqtest", Version: "0.3.0", Repository: "https://example.com/charts"}, + } + m := &Manager{} + + targets, err := m.buildExtractionTargets(extractionDeps, resolvedDeps, sourcePath) + assert.NoError(t, err) + assert.Len(t, targets, 2) + assert.Contains(t, targets[0].archive, "reqtest-0.1.0.tgz") + assert.Contains(t, targets[1].archive, "reqtest-0.3.0.tgz") +} + +func createDependencyArchive(t *testing.T, dest, name, version string) { + t.Helper() + c := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: name, + Version: version, + }, + } + _, err := chartutil.Save(c, dest) + assert.NoError(t, err) +} + +func createDependencyDir(t *testing.T, dest, name, version string) { + t.Helper() + c := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: name, + Version: version, + }, + } + err := chartutil.SaveDir(c, dest) + assert.NoError(t, err) +} From 839b699f7bb4935d5d4373e512b28248e112aea1 Mon Sep 17 00:00:00 2001 From: maheshrijal <62394512+maheshrijal@users.noreply.github.com> Date: Mon, 16 Feb 2026 11:13:58 +0530 Subject: [PATCH 2/2] fix(dependency): clarify untar archive cleanup behavior Signed-off-by: maheshrijal <62394512+maheshrijal@users.noreply.github.com> --- pkg/cmd/dependency.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/dependency.go b/pkg/cmd/dependency.go index a3e723847..e457b9048 100644 --- a/pkg/cmd/dependency.go +++ b/pkg/cmd/dependency.go @@ -134,7 +134,7 @@ func addDependencySubcommandFlags(f *pflag.FlagSet, client *action.Dependency, w 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") if withUntar { - f.BoolVar(&client.Untar, "untar", false, "if set to true, will untar the dependency charts after downloading them and remove the chart archives") + f.BoolVar(&client.Untar, "untar", false, "if set to true, will untar dependency charts after downloading them; with the default --untardir (charts/), chart archives are removed after extraction") f.StringVar(&client.UntarDir, "untardir", "charts", "if untar is specified, this flag specifies the directory (relative to chart root) into which dependencies are expanded") } }