From 3d42cb4c6a46c04bc4e50b69dffa2aacc8b5810f Mon Sep 17 00:00:00 2001 From: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:58:20 +0530 Subject: [PATCH] fix: only skip broken symlinks when matched by .helmignore Instead of silently skipping all broken symlinks unconditionally, pass the error to the walkFn callback so chart loaders can check .helmignore patterns before deciding whether to error. - internal/sympath/walk.go: call walkFn with the error instead of returning nil for broken symlinks - pkg/chart/v2/loader/directory.go: skip broken symlink errors only when the file matches a .helmignore rule - internal/chart/v3/loader/directory.go: same fix for v3 charts - Add TestLoadDirWithBrokenSymlinkNotInHelmignore to verify that broken symlinks not in .helmignore still cause an error Signed-off-by: Kartik Suryavanshi Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> --- internal/chart/v3/loader/directory.go | 4 +++ internal/chart/v3/loader/load_test.go | 40 +++++++++++++++++++++++++++ internal/sympath/walk.go | 9 +++--- pkg/chart/v2/loader/directory.go | 4 +++ pkg/chart/v2/loader/load_test.go | 40 +++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 5 deletions(-) diff --git a/internal/chart/v3/loader/directory.go b/internal/chart/v3/loader/directory.go index dfe3af3b2..ff3926935 100644 --- a/internal/chart/v3/loader/directory.go +++ b/internal/chart/v3/loader/directory.go @@ -77,6 +77,10 @@ func LoadDir(dir string) (*chart.Chart, error) { n = filepath.ToSlash(n) if err != nil { + // For broken symlinks, respect .helmignore before erroring. + if os.IsNotExist(err) && fi != nil && rules.Ignore(n, fi) { + return nil + } return err } if fi.IsDir() { diff --git a/internal/chart/v3/loader/load_test.go b/internal/chart/v3/loader/load_test.go index 9b8762e51..8f14d56f1 100644 --- a/internal/chart/v3/loader/load_test.go +++ b/internal/chart/v3/loader/load_test.go @@ -90,6 +90,46 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirWithBrokenSymlinkNotInHelmignore(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests require unix") + } + + tmpDir := t.TempDir() + + chartYAML := `apiVersion: v2 +name: test +version: 0.1.0 +` + valuesYAML := `{} +` + if err := os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), []byte(chartYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "values.yaml"), []byte(valuesYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create a broken symlink NOT listed in .helmignore + brokenLink := filepath.Join(tmpDir, "broken") + if err := os.Symlink("nonexistent", brokenLink); err != nil { + t.Fatal(err) + } + + // Empty .helmignore — broken symlink is not ignored + if err := os.WriteFile(filepath.Join(tmpDir, ".helmignore"), []byte(""), 0644); err != nil { + t.Fatal(err) + } + + l, err := Loader(tmpDir) + if err != nil { + t.Fatal(err) + } + if _, err := l.Load(); err == nil { + t.Fatal("loading chart with broken symlink not in .helmignore should fail") + } +} + func TestLoadDirWithBrokenSymlinkInHelmignore(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink tests require unix") diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index 4aca38693..8eca7da47 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -68,11 +68,10 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { resolved, err := filepath.EvalSymlinks(path) if err != nil { if os.IsNotExist(err) { - // If a symlink's target does not exist (broken symlink), - // silently skip it. Broken symlinks do not contribute - // content and should not cause errors, especially when - // they match .helmignore patterns. - return nil + // Pass the broken symlink error to walkFn so callers + // (e.g. chart loaders) can decide whether to skip it + // based on .helmignore rules. + return walkFn(path, info, err) } return err } diff --git a/pkg/chart/v2/loader/directory.go b/pkg/chart/v2/loader/directory.go index 82578d924..8fd0b516b 100644 --- a/pkg/chart/v2/loader/directory.go +++ b/pkg/chart/v2/loader/directory.go @@ -77,6 +77,10 @@ func LoadDir(dir string) (*chart.Chart, error) { n = filepath.ToSlash(n) if err != nil { + // For broken symlinks, respect .helmignore before erroring. + if os.IsNotExist(err) && fi != nil && rules.Ignore(n, fi) { + return nil + } return err } if fi.IsDir() { diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index 56954b16a..8f61b3632 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -90,6 +90,46 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirWithBrokenSymlinkNotInHelmignore(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests require unix") + } + + tmpDir := t.TempDir() + + chartYAML := `apiVersion: v2 +name: test +version: 0.1.0 +` + valuesYAML := `{} +` + if err := os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), []byte(chartYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "values.yaml"), []byte(valuesYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create a broken symlink NOT listed in .helmignore + brokenLink := filepath.Join(tmpDir, "broken") + if err := os.Symlink("nonexistent", brokenLink); err != nil { + t.Fatal(err) + } + + // Empty .helmignore — broken symlink is not ignored + if err := os.WriteFile(filepath.Join(tmpDir, ".helmignore"), []byte(""), 0644); err != nil { + t.Fatal(err) + } + + l, err := Loader(tmpDir) + if err != nil { + t.Fatal(err) + } + if _, err := l.Load(); err == nil { + t.Fatal("loading chart with broken symlink not in .helmignore should fail") + } +} + func TestLoadDirWithBrokenSymlinkInHelmignore(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink tests require unix")