fix: address review feedback

- Pass all symlink evaluation errors through walkFn for consistency
  with the Walk contract (all errors are filtered by walkFn)
- Tighten test assertions to verify the error is specifically
  os.IsNotExist, not just any error

Signed-off-by: Kartik Suryavanshi <kartik.suryavanshi@outlook.com>
Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com>
pull/32264/head
Kartik Suryavanshi 3 months ago
parent 3d42cb4c6a
commit 8bc1c04c11

@ -125,8 +125,8 @@ version: 0.1.0
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := l.Load(); err == nil { if _, err := l.Load(); err == nil || !os.IsNotExist(err) {
t.Fatal("loading chart with broken symlink not in .helmignore should fail") t.Fatalf("expected broken symlink error (os.IsNotExist), got: %v", err)
} }
} }

@ -67,13 +67,9 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error {
if IsSymlink(info) { if IsSymlink(info) {
resolved, err := filepath.EvalSymlinks(path) resolved, err := filepath.EvalSymlinks(path)
if err != nil { if err != nil {
if os.IsNotExist(err) { // Pass the error to walkFn so callers (e.g. chart loaders)
// Pass the broken symlink error to walkFn so callers // can decide whether to skip it based on .helmignore rules.
// (e.g. chart loaders) can decide whether to skip it return walkFn(path, info, err)
// based on .helmignore rules.
return walkFn(path, info, err)
}
return err
} }
// This log message is to highlight a symlink that is being used within a chart, symlinks can be used for nefarious reasons. // This log message is to highlight a symlink that is being used within a chart, symlinks can be used for nefarious reasons.
slog.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved) slog.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved)

@ -125,8 +125,8 @@ version: 0.1.0
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := l.Load(); err == nil { if _, err := l.Load(); err == nil || !os.IsNotExist(err) {
t.Fatal("loading chart with broken symlink not in .helmignore should fail") t.Fatalf("expected broken symlink error (os.IsNotExist), got: %v", err)
} }
} }

Loading…
Cancel
Save