diff --git a/README.md b/README.md index cebd1f8f7..f3f30be6e 100644 --- a/README.md +++ b/README.md @@ -237,6 +237,14 @@ linters-settings: documentation: impact: error + exclude-rules: + # Silence the "file too large" warning for generated/bundled docs + # (size check only; content checks are unaffected) + file-size: + files: + - docs/generated-reference.md + directories: + - docs/generated/ templates: impact: error diff --git a/go.mod b/go.mod index 62ee25b0a..23f0c4fa7 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/spf13/viper v1.19.0 github.com/stretchr/testify v1.10.0 github.com/tidwall/gjson v1.18.0 - github.com/werf/nelm v1.24.3 + github.com/werf/nelm v1.26.1 golang.org/x/mod v0.27.0 gopkg.in/ini.v1 v1.67.0 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 5a1dafe4f..111debe49 100644 --- a/go.sum +++ b/go.sum @@ -415,8 +415,8 @@ github.com/werf/lockgate v0.1.1 h1:S400JFYjtWfE4i4LY9FA8zx0fMdfui9DPrBiTciCrx4= github.com/werf/lockgate v0.1.1/go.mod h1:0yIFSLq9ausy6ejNxF5uUBf/Ib6daMAfXuCaTMZJzIE= github.com/werf/logboek v0.6.1 h1:oEe6FkmlKg0z0n80oZjLplj6sXcBeLleCkjfOOZEL2g= github.com/werf/logboek v0.6.1/go.mod h1:Gez5J4bxekyr6MxTmIJyId1F61rpO+0/V4vjCIEIZmk= -github.com/werf/nelm v1.24.3 h1:KX5uk0rHymdOW6T3Nxrl2nuR1mI4c7FAsSQkI03ckIk= -github.com/werf/nelm v1.24.3/go.mod h1:Gy6XJ42rwJVA+UyB6ka9/DVFPzm+lh7lmcjLAZECdIs= +github.com/werf/nelm v1.26.1 h1:0gH8JuJlKZtPLpeRAt4H9mVLRzDQwNqhcllxcr2UTDg= +github.com/werf/nelm v1.26.1/go.mod h1:D3uU5e1dSBc0l0oo/iB6SjM2sLaXICqRDSWXAYF+vpk= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= diff --git a/internal/fsutils/fsutils.go b/internal/fsutils/fsutils.go index 3cb35c8e3..edbcc171b 100644 --- a/internal/fsutils/fsutils.go +++ b/internal/fsutils/fsutils.go @@ -17,6 +17,7 @@ limitations under the License. package fsutils import ( + "errors" "fmt" "os" "path/filepath" @@ -31,6 +32,40 @@ import ( // evalSymlinkCache is a cache for evaluated symlinks to avoid multiple evaluations var evalSymlinkCache sync.Map +// MaxLintableFileSize bounds how large a file dmt will read into memory while +// linting. Files above it are generated data blobs (bundled Grafana dashboards, +// rendered openapi, CRD bundles), not hand-written sources; reading a +// multi-gigabyte file just to scan it would exhaust memory and flood the log. +const MaxLintableFileSize = 10 << 20 // 10 MiB + +// ErrFileTooLarge is returned by ReadFile when a file exceeds MaxLintableFileSize. +var ErrFileTooLarge = errors.New("file too large to lint") + +// ReadFile reads the named file like os.ReadFile but refuses files larger than +// MaxLintableFileSize, returning ErrFileTooLarge instead of loading a huge file +// into memory. Callers that scan discovered files should treat ErrFileTooLarge +// as "skip this file" rather than a hard failure. +func ReadFile(path string) ([]byte, error) { + info, err := os.Stat(path) + if err != nil { + return nil, err + } + + if info.Size() > MaxLintableFileSize { + return nil, fmt.Errorf("%w: %s (%d bytes)", ErrFileTooLarge, path, info.Size()) + } + + return os.ReadFile(path) +} + +// IsFileTooLarge reports whether err was produced by ReadFile refusing an +// oversized file. Linters that scan discovered files use it to skip such files +// rather than failing. It exists so callers need not import the standard errors +// package, which many of them alias to dmt's own errors package. +func IsFileTooLarge(err error) bool { + return errors.Is(err, ErrFileTooLarge) +} + // IsDir checks if the given path is a directory func IsDir(path string) bool { fi, err := os.Stat(path) diff --git a/internal/fsutils/readfile_test.go b/internal/fsutils/readfile_test.go new file mode 100644 index 000000000..e36fe8b50 --- /dev/null +++ b/internal/fsutils/readfile_test.go @@ -0,0 +1,73 @@ +/* +Copyright 2025 Flant JSC + +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 fsutils + +import ( + "os" + "path/filepath" + "testing" +) + +func TestReadFile_ReadsNormalFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "ok.txt") + + want := []byte("hello") + if err := os.WriteFile(path, want, 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + got, err := ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + + if string(got) != string(want) { + t.Errorf("got %q, want %q", got, want) + } +} + +func TestReadFile_RejectsOversizedFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "huge.bin") + + f, err := os.Create(path) + if err != nil { + t.Fatalf("create: %v", err) + } + + if err := f.Truncate(MaxLintableFileSize + 1); err != nil { + t.Fatalf("truncate: %v", err) + } + + if err := f.Close(); err != nil { + t.Fatalf("close: %v", err) + } + + got, err := ReadFile(path) + if err == nil { + t.Fatalf("expected an error for an oversized file, got %d bytes", len(got)) + } + + if !IsFileTooLarge(err) { + t.Errorf("expected IsFileTooLarge to report true for %v", err) + } + + if got != nil { + t.Errorf("expected no data for an oversized file, got %d bytes", len(got)) + } +} diff --git a/internal/helm/guard.go b/internal/helm/guard.go new file mode 100644 index 000000000..c15bc7d17 --- /dev/null +++ b/internal/helm/guard.go @@ -0,0 +1,291 @@ +/* +Copyright 2025 Flant JSC + +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 helm + +import ( + "fmt" + "io" + "os" + "path/filepath" + + "github.com/werf/nelm/pkg/helm/pkg/ignore" +) + +// symlinkLoop describes a symbolic link that resolves to one of its own +// ancestors and was therefore skipped while preparing a chart for rendering. +type symlinkLoop struct { + // Path is the location of the offending link inside the chart. + Path string + // Resolved is the ancestor directory the link resolves to. + Resolved string +} + +// maxChartLoadBytes caps the cumulative size of the files nelm's chart loader +// would read into memory for a single chart. +// +// nelm's loader (GetFilesFromLocalFilesystem -> sympath.Walk) reads every +// regular file in the chart directory fully into RAM and follows symbolic links +// without any cycle detection. A symlink that resolves to one of its own +// ancestors, or one that points at a very large tree, therefore makes the walk +// read data without bound until the Go runtime aborts the process with an +// out-of-memory fatal error. This backstop is deliberately generous: a real +// chart's templates, values, CRDs and openapi schemas are far smaller, so +// tripping it means the directory is pathological rather than large. +const maxChartLoadBytes = 2 << 30 // 2 GiB + +// loadChartIgnoreRules loads the chart's .helmignore rules exactly the way +// nelm's loader does (parse the file at the chart root if present, then add the +// built-in defaults), so scanning and cleaning consider the same set of files +// nelm would actually read. +func loadChartIgnoreRules(chartDir string) (*ignore.Rules, error) { + rules := ignore.Empty() + + ifile := filepath.Join(chartDir, ignore.HelmIgnore) + if _, err := os.Stat(ifile); err == nil { + r, err := ignore.ParseFile(ifile) + if err != nil { + return nil, err + } + + rules = r + } + + rules.AddDefaults() + + return rules, nil +} + +// isIgnored reports whether path (relative to the chart root) is excluded by the +// chart's .helmignore rules, matching how nelm's loader classifies entries. +func isIgnored(rules *ignore.Rules, root, path string, info os.FileInfo) bool { + rel, err := filepath.Rel(root, path) + if err != nil { + return false + } + + return rules.Ignore(filepath.ToSlash(rel), info) +} + +// scanChartDir walks chartDir the same way nelm's loader does — following +// symbolic links and honouring .helmignore — but tracks the resolved real path +// of every directory currently on the descent stack so it can detect symlink +// loops, and sums the size of the regular files it would read so it can reject +// trees too large to load safely. +// +// It returns hasLoop=true when a symlink resolves to one of its own ancestors. +// The caller renders a cleaned copy of the directory in that case (see +// materializeCleanChartDir) instead of handing the loop to nelm's loader, which +// would follow it without bound and exhaust memory. An oversized tree is still a +// hard error, because cleaning cannot make it small enough to load safely. +// +// The walk only stats files (it never reads their contents), so this pass is +// cheap relative to the load nelm performs afterwards. +func scanChartDir(chartDir string) (bool, error) { + rules, err := loadChartIgnoreRules(chartDir) + if err != nil { + return false, err + } + + g := &chartDirGuard{root: chartDir, rules: rules, ancestors: make(map[string]struct{})} + if err := g.walk(chartDir); err != nil { + return false, err + } + + return g.hasLoop, nil +} + +type chartDirGuard struct { + root string + rules *ignore.Rules + ancestors map[string]struct{} + total int64 + hasLoop bool +} + +func (g *chartDirGuard) walk(dir string) error { + // Resolve to the real path so a symlink pointing back at an ancestor is + // recognised as a cycle rather than followed forever. + real, err := filepath.EvalSymlinks(dir) + if err != nil { + return fmt.Errorf("evaluate symlink %s: %w", dir, err) + } + + if _, ok := g.ancestors[real]; ok { + g.hasLoop = true + + return nil + } + + g.ancestors[real] = struct{}{} + defer delete(g.ancestors, real) + + entries, err := os.ReadDir(dir) + if err != nil { + return err + } + + for _, entry := range entries { + name := filepath.Join(dir, entry.Name()) + + // os.Stat follows symlinks, matching how the loader classifies entries. + info, err := os.Stat(name) + if err != nil { + return err + } + + if isIgnored(g.rules, g.root, name, info) { + continue + } + + if info.IsDir() { + if err := g.walk(name); err != nil { + return err + } + + continue + } + + if !info.Mode().IsRegular() { + continue + } + + g.total += info.Size() + if g.total > maxChartLoadBytes { + return fmt.Errorf( + "chart %q exceeds the %d-byte load limit; check for oversized files or a symbolic link pointing at a large tree", + g.root, maxChartLoadBytes, + ) + } + } + + return nil +} + +// materializeCleanChartDir copies chartDir into a fresh temporary directory, +// dereferencing symbolic links into real files and directories and skipping any +// directory that would form a symlink loop (a link resolving to one of its own +// ancestors). Files excluded by the chart's .helmignore are not copied, so the +// copy mirrors what nelm's loader would read. The returned directory therefore +// contains no symlinks and no cycles, so nelm's loader can walk it safely. +// +// It returns the path to the copy, the symlink loops it skipped (so the caller +// can report them), and a cleanup function the caller must invoke (typically via +// defer) to remove the copy. +func materializeCleanChartDir(chartDir string) (string, func(), []symlinkLoop, error) { + rules, err := loadChartIgnoreRules(chartDir) + if err != nil { + return "", nil, nil, err + } + + dst, err := os.MkdirTemp("", "dmt-chart-*") + if err != nil { + return "", nil, nil, err + } + + cleanup := func() { _ = os.RemoveAll(dst) } + + c := &chartCleaner{root: chartDir, rules: rules, ancestors: make(map[string]struct{})} + if err := c.copy(chartDir, dst); err != nil { + cleanup() + + return "", nil, nil, err + } + + return dst, cleanup, c.loops, nil +} + +type chartCleaner struct { + root string + rules *ignore.Rules + ancestors map[string]struct{} + loops []symlinkLoop +} + +func (c *chartCleaner) copy(srcDir, dstDir string) error { + real, err := filepath.EvalSymlinks(srcDir) + if err != nil { + return fmt.Errorf("evaluate symlink %s: %w", srcDir, err) + } + + if _, ok := c.ancestors[real]; ok { + c.loops = append(c.loops, symlinkLoop{Path: srcDir, Resolved: real}) + + return nil + } + + c.ancestors[real] = struct{}{} + defer delete(c.ancestors, real) + + if err := os.MkdirAll(dstDir, 0o755); err != nil { + return err + } + + entries, err := os.ReadDir(srcDir) + if err != nil { + return err + } + + for _, entry := range entries { + srcPath := filepath.Join(srcDir, entry.Name()) + dstPath := filepath.Join(dstDir, entry.Name()) + + // os.Stat follows symlinks so a symlinked file or directory is + // materialized as its real target rather than copied as a link. + info, err := os.Stat(srcPath) + if err != nil { + return err + } + + if isIgnored(c.rules, c.root, srcPath, info) { + continue + } + + switch { + case info.IsDir(): + if err := c.copy(srcPath, dstPath); err != nil { + return err + } + case info.Mode().IsRegular(): + if err := copyRegularFile(srcPath, dstPath, info.Mode().Perm()); err != nil { + return err + } + } + } + + return nil +} + +func copyRegularFile(src, dst string, perm os.FileMode) error { + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, perm) + if err != nil { + return err + } + + if _, err := io.Copy(out, in); err != nil { + out.Close() + + return err + } + + return out.Close() +} diff --git a/internal/helm/guard_test.go b/internal/helm/guard_test.go new file mode 100644 index 000000000..2d247176d --- /dev/null +++ b/internal/helm/guard_test.go @@ -0,0 +1,98 @@ +/* +Copyright 2025 Flant JSC + +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 helm + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestScanChartDirDetectsLoop verifies a symlink resolving to one of its own +// ancestors is reported as a loop, while a clean tree is not. +func TestScanChartDirDetectsLoop(t *testing.T) { + clean := t.TempDir() + writeGuardFile(t, filepath.Join(clean, "Chart.yaml"), "name: x\nversion: 0.1.0\n") + writeGuardFile(t, filepath.Join(clean, "templates", "cm.yaml"), "kind: ConfigMap\n") + + hasLoop, err := scanChartDir(clean) + require.NoError(t, err) + require.False(t, hasLoop, "clean chart must not report a loop") + + looped := t.TempDir() + writeGuardFile(t, filepath.Join(looped, "Chart.yaml"), "name: x\nversion: 0.1.0\n") + require.NoError(t, os.Symlink(looped, filepath.Join(looped, "loop"))) + + hasLoop, err = scanChartDir(looped) + require.NoError(t, err) + require.True(t, hasLoop, "symlink to an ancestor must be reported as a loop") +} + +// TestMaterializeCleanChartDir verifies the cleaned copy drops the symlink loop +// and honours .helmignore, so nelm's loader receives a safe, loop-free tree that +// mirrors what it would actually read. +func TestMaterializeCleanChartDir(t *testing.T) { + src := t.TempDir() + + writeGuardFile(t, filepath.Join(src, ".helmignore"), "*.ignored\n") + writeGuardFile(t, filepath.Join(src, "Chart.yaml"), "name: x\nversion: 0.1.0\n") + writeGuardFile(t, filepath.Join(src, "templates", "cm.yaml"), "kind: ConfigMap\n") + writeGuardFile(t, filepath.Join(src, "big.ignored"), "excluded by .helmignore\n") + + // A symlink loop: loop -> the chart root (an ancestor of itself). + require.NoError(t, os.Symlink(src, filepath.Join(src, "templates", "loop"))) + + dst, cleanup, loops, err := materializeCleanChartDir(src) + require.NoError(t, err) + + defer cleanup() + + // The skipped loop is reported back so the caller can surface it. + require.Len(t, loops, 1, "the symlink loop must be reported") + require.Equal(t, filepath.Join(src, "templates", "loop"), loops[0].Path) + + // Real chart content is materialized. + require.FileExists(t, filepath.Join(dst, "Chart.yaml")) + require.FileExists(t, filepath.Join(dst, "templates", "cm.yaml")) + + // .helmignore'd file is not copied. + _, err = os.Stat(filepath.Join(dst, "big.ignored")) + require.True(t, os.IsNotExist(err), "file excluded by .helmignore must not be copied") + + // The loop is dropped: the copy contains no symlinks and no re-entrant dir. + _, err = os.Lstat(filepath.Join(dst, "templates", "loop")) + require.True(t, os.IsNotExist(err), "symlink loop must not be present in the cleaned copy") + + require.NoError(t, filepath.Walk(dst, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + require.Zero(t, info.Mode()&os.ModeSymlink, "cleaned copy must not contain symlinks: %s", path) + + return nil + })) +} + +func writeGuardFile(t *testing.T, path, content string) { + t.Helper() + + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) +} diff --git a/internal/helm/render.go b/internal/helm/render.go index 16303abe0..c81852914 100644 --- a/internal/helm/render.go +++ b/internal/helm/render.go @@ -50,6 +50,13 @@ type Renderer struct { // values during linting. Overrides are applied to the chart and all of its // dependencies. HelmLibOverrides map[string][]byte + + // OnSymlinkLoop, when set, is called once for each symbolic link loop that + // was found in the chart directory and skipped while preparing a cleaned + // copy for rendering. It lets the caller surface the loop as a lint finding + // instead of silently dropping it. path is the offending link; resolved is + // the ancestor directory it points back to. + OnSymlinkLoop func(path, resolved string) } // RenderChartFromDir renders the chart located at chartDir with nelm's chart @@ -61,6 +68,36 @@ func (r Renderer) RenderChartFromDir(chartDir string, values map[string]any) (ma return nil, fmt.Errorf("helm chart must have a name") } + // nelm's chart loader reads every file in the directory into memory and + // follows symlinks without cycle detection, so a symlink loop makes it + // exhaust memory and abort the whole process with a fatal out-of-memory + // error. Pre-flight the directory: on a symlink loop, render a cleaned copy + // (loops skipped, symlinks dereferenced) so linting still proceeds; an + // oversized tree is rejected outright since cleaning cannot make it safe. + loadDir := chartDir + + hasLoop, err := scanChartDir(chartDir) + if err != nil { + return nil, fmt.Errorf("load chart: %w", err) + } + + if hasLoop { + cleaned, cleanup, loops, err := materializeCleanChartDir(chartDir) + if err != nil { + return nil, fmt.Errorf("load chart: prepare cleaned chart dir: %w", err) + } + + defer cleanup() + + if r.OnSymlinkLoop != nil { + for _, l := range loops { + r.OnSymlinkLoop(l.Path, l.Resolved) + } + } + + loadDir = cleaned + } + opts := helmopts.HelmOptions{ ChartLoadOpts: helmopts.ChartLoadOptions{ // deckhouse modules may omit Chart.yaml; provide sane defaults so the @@ -84,7 +121,7 @@ func (r Renderer) RenderChartFromDir(chartDir string, values map[string]any) (ma stdlog.SetOutput(io.Discard) - chrt, err := loader.LoadDir(chartDir, opts) + chrt, err := loader.LoadDir(loadDir, opts) stdlog.SetOutput(stdlogWriter) diff --git a/internal/module/controller.go b/internal/module/controller.go index 7ea6aac38..e9e1f4e94 100644 --- a/internal/module/controller.go +++ b/internal/module/controller.go @@ -43,6 +43,13 @@ func RunRender(m *Module, values chartutil.Values, objectStore *storage.Unstruct // Inject dmt's deterministic helm_lib helper stubs so image and // module-name references resolve to stable values during linting. HelmLibOverrides: helmLibOverrides(), + // A symlink loop in the chart is skipped (its files are not rendered) + // rather than crashing the loader; report it as a warning so the module + // is still linted but the loop is surfaced to the user. + OnSymlinkLoop: func(path, resolved string) { + errorList.WithModule(m.GetName()).WithFilePath(path).WithRule("symlink-loop"). + Warnf("symbolic link loop detected in chart and skipped: %s resolves to ancestor %s", path, resolved) + }, } files, err := renderer.RenderChartFromDir(m.GetPath(), values) diff --git a/internal/module/module.go b/internal/module/module.go index e5bc7622f..2606acd42 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -392,7 +392,18 @@ func mapExclusionRulesAndSettings(linterSettings *pkg.LintersSettings, configSet mapRBACExclusions(linterSettings, configSettings) mapHooksSettings(linterSettings, configSettings) mapModuleExclusionsAndSettings(linterSettings, configSettings) - // no excluded rules - mapDocumentationExclusionsAndSettings(linterSettings, configSettings) + mapDocumentationExclusions(linterSettings, configSettings) +} + +// mapDocumentationExclusions maps Documentation linter exclusion rules. Only the +// large-file size warning honours these excludes; the documentation content +// checks themselves are not affected. +func mapDocumentationExclusions(linterSettings *pkg.LintersSettings, configSettings *config.LintersSettings) { + excludes := &linterSettings.Documentation.ExcludeRules.FileSize + configExcludes := &configSettings.Documentation.ExcludeRules.FileSize + + excludes.Files = pkg.StringRuleExcludeList(configExcludes.Files) + excludes.Directories = pkg.DirectoryRuleExcludeList(configExcludes.Directories) } // mapContainerExclusions maps Container linter exclusion rules diff --git a/internal/module/sympath.go b/internal/module/sympath.go index 961964ba5..6fc2fe808 100644 --- a/internal/module/sympath.go +++ b/internal/module/sympath.go @@ -21,9 +21,12 @@ package module import ( "errors" "fmt" + "log/slog" "os" "path/filepath" "sort" + + "github.com/deckhouse/deckhouse/pkg/log" ) // Walk walks the file tree rooted at root, calling walkFn for each file or directory @@ -36,7 +39,7 @@ func Walk(root string, walkFn filepath.WalkFunc) error { if err != nil { err = walkFn(root, nil, err) } else { - err = symwalk(root, info, walkFn) + err = symwalk(root, info, walkFn, make(map[string]struct{})) } if errors.Is(err, filepath.SkipDir) { @@ -66,8 +69,12 @@ func readDirNames(dirname string) ([]string, error) { return names, nil } -// symwalk recursively descends path, calling walkFn. -func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { +// symwalk recursively descends path, calling walkFn. ancestors holds the +// resolved real paths of the directories currently on the recursion stack; it +// is used to detect symlink loops. Without it, a symlink resolving to one of its +// own ancestors would make the walk recurse forever, reading the same files into +// memory until the process is killed with an out-of-memory error. +func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc, ancestors map[string]struct{}) error { // Recursively walk symlinked directories. if IsSymlink(info) { resolved, err := filepath.EvalSymlinks(path) @@ -79,7 +86,7 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { return err } - if err := symwalk(path, info, walkFn); err != nil && !errors.Is(err, filepath.SkipDir) { + if err := symwalk(path, info, walkFn, ancestors); err != nil && !errors.Is(err, filepath.SkipDir) { return err } @@ -94,6 +101,25 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { return nil } + // Resolve to the real path so a symlink that points back at an ancestor is + // recognised as a loop and skipped rather than followed without bound. + real, err := filepath.EvalSymlinks(path) + if err != nil { + return walkFn(path, info, err) + } + + if _, ok := ancestors[real]; ok { + log.Warn("skipping already-visited path to avoid symbolic link loop", + slog.String("path", path), + slog.String("resolved", real), + ) + + return nil + } + + ancestors[real] = struct{}{} + defer delete(ancestors, real) + names, err := readDirNames(path) if err != nil { return walkFn(path, info, err) @@ -108,7 +134,7 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { return err } } else { - err = symwalk(filename, fileInfo, walkFn) + err = symwalk(filename, fileInfo, walkFn, ancestors) if err != nil { if (!fileInfo.IsDir() && !IsSymlink(fileInfo)) || !errors.Is(err, filepath.SkipDir) { return err diff --git a/pkg/config.go b/pkg/config.go index df87f1f6a..c87db1280 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -79,7 +79,19 @@ type LintersSettings struct { type DocumentationLinterConfig struct { LinterConfig - Rules DocumentationLinterRules + Rules DocumentationLinterRules + ExcludeRules DocumentationExcludeRules +} + +type DocumentationExcludeRules struct { + // FileSize excludes files/directories from the large-file size warning only, + // not from the documentation content checks themselves. + FileSize FileSizeExcludeRules +} + +type FileSizeExcludeRules struct { + Files StringRuleExcludeList + Directories DirectoryRuleExcludeList } type DocumentationLinterRules struct { diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index bd6cfffcc..d979d852a 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -369,5 +369,18 @@ func remapContainerRuleExclude(input *ContainerRuleExclude) *pkg.ContainerRuleEx } type DocumentationSettings struct { + ExcludeRules DocumentationExcludeRules `mapstructure:"exclude-rules"` + Impact string `mapstructure:"impact"` } + +type DocumentationExcludeRules struct { + // FileSize excludes files/directories from the large-file size warning only, + // not from the documentation content checks themselves. + FileSize FileSizeExcludeRules `mapstructure:"file-size"` +} + +type FileSizeExcludeRules struct { + Files StringRuleExcludeList `mapstructure:"files"` + Directories DirectoryRuleExcludeList `mapstructure:"directories"` +} diff --git a/pkg/linters/container/rules/mount_points.go b/pkg/linters/container/rules/mount_points.go index 18ff0ffe1..fbc9d7e56 100644 --- a/pkg/linters/container/rules/mount_points.go +++ b/pkg/linters/container/rules/mount_points.go @@ -28,6 +28,7 @@ import ( "github.com/deckhouse/deckhouse/pkg/log" + "github.com/deckhouse/dmt/internal/fsutils" "github.com/deckhouse/dmt/internal/storage" "github.com/deckhouse/dmt/pkg" "github.com/deckhouse/dmt/pkg/errors" @@ -148,7 +149,7 @@ func collectMountPointsDirs(modulePath string) map[string]bool { return nil } - data, err := os.ReadFile(path) + data, err := fsutils.ReadFile(path) if err != nil { log.Warn("mount-points read error", slog.String("path", path), log.Err(err)) return nil diff --git a/pkg/linters/docs/README.md b/pkg/linters/docs/README.md index 27ee07c9b..835968e61 100644 --- a/pkg/linters/docs/README.md +++ b/pkg/linters/docs/README.md @@ -297,6 +297,16 @@ linters-settings: - docs/GLOSSARY.md # Contains technical terms in multiple languages ``` +**Large files:** + +A documentation file larger than 10 MiB is not read into memory or scanned for cyrillic. Instead of failing, the linter reports a **warning** so the skipped file stays visible: + +``` +file is too large to check for Cyrillic letters and was skipped; exclude the file or its directory under documentation.exclude-rules.file-size to silence this warning +``` + +This warning is silenced only through the `exclude-rules.file-size` list (see [File-Size Exclusions](#file-size-exclusions)); the per-rule `exclude` above affects the content check, not the size warning. + ### no-lang-key **Purpose:** Ensures that documentation files don't contain the `lang` key in their YAML front matter. The language of the document should be determined by its file name convention (e.g., `.ru.md` suffix for Russian) rather than by a `lang` field in the front matter. @@ -386,6 +396,16 @@ linters-settings: - docs/LEGACY.md # Legacy file that still uses lang key ``` +**Large files:** + +A documentation file larger than 10 MiB is not read into memory or scanned for a `lang` key. Instead of failing, the linter reports a **warning** so the skipped file stays visible: + +``` +file is too large to check for a lang key and was skipped; exclude the file or its directory under documentation.exclude-rules.file-size to silence this warning +``` + +This warning is silenced only through the `exclude-rules.file-size` list (see [File-Size Exclusions](#file-size-exclusions)); the per-rule `exclude` above affects the content check, not the size warning. + --- ## Configuration @@ -427,6 +447,38 @@ linters-settings: - docs/GLOSSARY.md # Technical terms document ``` +### File-Size Exclusions + +Documentation files larger than 10 MiB are not scanned by the `cyrillic-in-english` and `no-lang-key` rules; the linter reports a warning for each skipped file instead (see the "Large files" notes above). These exclusions silence that warning **only** — they do not affect the content checks themselves. + +Exclude specific files: +```yaml +# .dmtlint.yaml +linters-settings: + documentation: + exclude-rules: + file-size: + files: + - docs/generated-reference.md + - docs/bundled-dashboard.md +``` + +Exclude entire directories: +```yaml +# .dmtlint.yaml +linters-settings: + documentation: + exclude-rules: + file-size: + directories: + - docs/generated/ +``` + +**Pattern matching:** +- Paths are relative to the module root +- `files` uses exact file-path matching; `directories` uses directory-prefix matching +- Applies only to the large-file size warning, not to the cyrillic/lang-key content checks + ## Common Issues ### Issue: Missing README.md @@ -545,6 +597,36 @@ Line 15: Check the документация for details. Проверьте документацию для получения подробностей. ``` +### Issue: Documentation file too large to check + +**Symptom:** +``` +Warning: file is too large to check for Cyrillic letters and was skipped; exclude the file or its directory under documentation.exclude-rules.file-size to silence this warning +File: docs/generated-reference.md +``` + +**Cause:** A documentation file exceeds 10 MiB, so it is not read into memory or scanned. This is usually a generated or bundled file rather than hand-written documentation. + +**Solutions:** + +1. **Split or trim the file** if it is genuinely documentation that should be checked. + +2. **Exclude it from the size check** if it is generated/bundled and the warning is noise: + + ```yaml + # .dmtlint.yaml + linters-settings: + documentation: + exclude-rules: + file-size: + files: + - docs/generated-reference.md + directories: + - docs/generated/ + ``` + + This suppresses the size warning only; the cyrillic and lang-key content checks are unaffected. + ### Issue: Empty README.md file **Symptom:** diff --git a/pkg/linters/docs/documentation.go b/pkg/linters/docs/documentation.go index e05bb1c3a..366c773c4 100644 --- a/pkg/linters/docs/documentation.go +++ b/pkg/linters/docs/documentation.go @@ -41,9 +41,16 @@ func (l *Documentation) Run(m *module.Module) { rules.NewBilingualRule().CheckBilingual(m, errorList.WithMaxLevel(l.cfg.Rules.BilingualRule.GetLevel())) - rules.NewCyrillicInEnglishRule().CheckFiles(m, errorList.WithMaxLevel(l.cfg.Rules.CyrillicInEnglishRule.GetLevel())) + sizeExcludeFiles := l.cfg.ExcludeRules.FileSize.Files.Get() + sizeExcludeDirs := l.cfg.ExcludeRules.FileSize.Directories.Get() - rules.NewNoLangKeyRule().CheckFiles(m, errorList.WithMaxLevel(l.cfg.Rules.NoLangKeyRule.GetLevel())) + rules.NewCyrillicInEnglishRule(). + WithFileSizeExcludes(sizeExcludeFiles, sizeExcludeDirs). + CheckFiles(m, errorList.WithMaxLevel(l.cfg.Rules.CyrillicInEnglishRule.GetLevel())) + + rules.NewNoLangKeyRule(). + WithFileSizeExcludes(sizeExcludeFiles, sizeExcludeDirs). + CheckFiles(m, errorList.WithMaxLevel(l.cfg.Rules.NoLangKeyRule.GetLevel())) } func (l *Documentation) Name() string { diff --git a/pkg/linters/docs/rules/cyrillic_in_english.go b/pkg/linters/docs/rules/cyrillic_in_english.go index 1dbe0351e..2d0270c3f 100644 --- a/pkg/linters/docs/rules/cyrillic_in_english.go +++ b/pkg/linters/docs/rules/cyrillic_in_english.go @@ -5,7 +5,6 @@ package rules import ( "fmt" - "os" "path/filepath" "regexp" "strings" @@ -17,6 +16,12 @@ import ( const ( CyrillicInEnglishRuleName = "cyrillic-in-english" + + // maxCyrillicReportLines and maxCyrillicLineWidth bound how much of a file is + // echoed back in a single finding, so a document full of Cyrillic cannot dump + // megabytes into the log. + maxCyrillicReportLines = 100 + maxCyrillicLineWidth = 200 ) var ( @@ -39,6 +44,19 @@ func NewCyrillicInEnglishRule() *CyrillicInEnglishRule { type CyrillicInEnglishRule struct { pkg.RuleMeta pkg.PathRule + + // sizeExcludes gates only the large-file size warning (not the Cyrillic + // content check), so a file/directory can be excluded from the size check + // alone. + sizeExcludes pkg.PathRule +} + +// WithFileSizeExcludes configures the files/directories excluded from the +// large-file size warning. +func (r *CyrillicInEnglishRule) WithFileSizeExcludes(files []pkg.StringRuleExclude, dirs []pkg.DirectoryRuleExclude) *CyrillicInEnglishRule { + r.sizeExcludes = pkg.PathRule{ExcludeStringRules: files, ExcludeDirectoryRules: dirs} + + return r } func (r *CyrillicInEnglishRule) CheckFiles(m pkg.Module, errorList *errors.LintRuleErrorsList) { @@ -81,7 +99,19 @@ func (r *CyrillicInEnglishRule) checkFile(m pkg.Module, fileName string, errorLi lines, err := getFileContent(fileName) if err != nil { + if fsutils.IsFileTooLarge(err) { + // Too large to scan; report it as a warning unless the file or its + // directory is excluded from the size check. + if r.sizeExcludes.Enabled(relPath) { + errorList.WithFilePath(relPath). + Warnf("file is too large to check for Cyrillic letters and was skipped; exclude the file or its directory under documentation.exclude-rules.file-size to silence this warning") + } + + return + } + errorList.WithFilePath(relPath).WithValue(err.Error()).Error("failed to read file") + return } @@ -95,7 +125,7 @@ func (r *CyrillicInEnglishRule) checkFile(m pkg.Module, fileName string, errorLi } func getFileContent(filename string) ([]string, error) { - fileBytes, err := os.ReadFile(filename) + fileBytes, err := fsutils.ReadFile(filename) if err != nil { return nil, err } @@ -112,6 +142,12 @@ func checkCyrillicLettersInString(line string) (string, bool) { line = strings.TrimSpace(line) + // Bound the width of a single reported line so a very long minified line + // cannot flood the log. + if runes := []rune(line); len(runes) > maxCyrillicLineWidth { + line = string(runes[:maxCyrillicLineWidth]) + "…" + } + cursor := cyrFillerRe.ReplaceAllString(line, "-") cursor = cyrPointerRe.ReplaceAllString(cursor, "^") cursor = strings.TrimRight(cursor, "-") @@ -123,15 +159,31 @@ func checkCyrillicLettersInArray(lines []string) (string, bool) { res := make([]string, 0) hasCyr := false + truncated := 0 for i, line := range lines { msg, has := checkCyrillicLettersInString(line) - if has { - hasCyr = true + if !has { + continue + } + + hasCyr = true + + // Cap the number of reported lines so a document full of Cyrillic cannot + // produce a multi-megabyte finding. + if len(res) >= maxCyrillicReportLines { + truncated++ - res = append(res, fmt.Sprintf("Line %d: %s", i+1, msg)) + continue } + + res = append(res, fmt.Sprintf("Line %d: %s", i+1, msg)) + } + + out := strings.Join(res, "\n") + if truncated > 0 { + out += fmt.Sprintf("\n… and %d more line(s) with Cyrillic letters (truncated)", truncated) } - return strings.Join(res, "\n"), hasCyr + return out, hasCyr } diff --git a/pkg/linters/docs/rules/no_lang_key.go b/pkg/linters/docs/rules/no_lang_key.go index 67ab8f2e2..1aefb46cd 100644 --- a/pkg/linters/docs/rules/no_lang_key.go +++ b/pkg/linters/docs/rules/no_lang_key.go @@ -5,7 +5,6 @@ package rules import ( "fmt" - "os" "path/filepath" "regexp" "strings" @@ -35,6 +34,18 @@ func NewNoLangKeyRule() *NoLangKeyRule { type NoLangKeyRule struct { pkg.RuleMeta pkg.PathRule + + // sizeExcludes gates only the large-file size warning (not the lang-key + // check), so a file/directory can be excluded from the size check alone. + sizeExcludes pkg.PathRule +} + +// WithFileSizeExcludes configures the files/directories excluded from the +// large-file size warning. +func (r *NoLangKeyRule) WithFileSizeExcludes(files []pkg.StringRuleExclude, dirs []pkg.DirectoryRuleExclude) *NoLangKeyRule { + r.sizeExcludes = pkg.PathRule{ExcludeStringRules: files, ExcludeDirectoryRules: dirs} + + return r } func (r *NoLangKeyRule) CheckFiles(m pkg.Module, errorList *errors.LintRuleErrorsList) { @@ -65,9 +76,21 @@ func (r *NoLangKeyRule) checkFile(m pkg.Module, fileName string, errorList *erro return } - content, err := os.ReadFile(fileName) + content, err := fsutils.ReadFile(fileName) if err != nil { + if fsutils.IsFileTooLarge(err) { + // Too large to scan; report it as a warning unless the file or its + // directory is excluded from the size check. + if r.sizeExcludes.Enabled(relPath) { + errorList.WithFilePath(relPath). + Warnf("file is too large to check for a lang key and was skipped; exclude the file or its directory under documentation.exclude-rules.file-size to silence this warning") + } + + return + } + errorList.WithFilePath(relPath).WithValue(err.Error()).Error("failed to read file") + return } diff --git a/pkg/linters/docs/rules/size_warning_test.go b/pkg/linters/docs/rules/size_warning_test.go new file mode 100644 index 000000000..a39d5fe24 --- /dev/null +++ b/pkg/linters/docs/rules/size_warning_test.go @@ -0,0 +1,126 @@ +/* +Copyright 2025 Flant JSC + +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 rules + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/gojuno/minimock/v3" + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/internal/mocks" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +// newModuleWithOversizedDoc creates a module whose docs/big.md is just over the +// size limit, and returns the module mock. +func newModuleWithOversizedDoc(t *testing.T) pkg.Module { + t.Helper() + + mc := minimock.NewController(t) + mockModule := mocks.NewModuleMock(mc) + + tempDir := t.TempDir() + mockModule.GetPathMock.Return(tempDir) + + docFile := filepath.Join(tempDir, "docs", "big.md") + require.NoError(t, os.MkdirAll(filepath.Dir(docFile), 0o755)) + + f, err := os.Create(docFile) + require.NoError(t, err) + require.NoError(t, f.Truncate(fsutils.MaxLintableFileSize+1)) + require.NoError(t, f.Close()) + + return mockModule +} + +func TestCyrillicInEnglish_OversizedFile_WarnsAndCanBeExcluded(t *testing.T) { + t.Run("warns when not excluded", func(t *testing.T) { + m := newModuleWithOversizedDoc(t) + errorList := errors.NewLintRuleErrorsList() + + NewCyrillicInEnglishRule().CheckFiles(m, errorList) + + errs := errorList.GetErrors() + require.Len(t, errs, 1) + require.True(t, strings.EqualFold(errs[0].Level.String(), "warn"), "level=%s", errs[0].Level.String()) + require.Contains(t, errs[0].Text, "too large") + }) + + t.Run("excluded by file", func(t *testing.T) { + m := newModuleWithOversizedDoc(t) + errorList := errors.NewLintRuleErrorsList() + + NewCyrillicInEnglishRule(). + WithFileSizeExcludes([]pkg.StringRuleExclude{"docs/big.md"}, nil). + CheckFiles(m, errorList) + + require.Empty(t, errorList.GetErrors()) + }) + + t.Run("excluded by directory", func(t *testing.T) { + m := newModuleWithOversizedDoc(t) + errorList := errors.NewLintRuleErrorsList() + + NewCyrillicInEnglishRule(). + WithFileSizeExcludes(nil, []pkg.DirectoryRuleExclude{"docs"}). + CheckFiles(m, errorList) + + require.Empty(t, errorList.GetErrors()) + }) +} + +func TestNoLangKey_OversizedFile_WarnsAndCanBeExcluded(t *testing.T) { + t.Run("warns when not excluded", func(t *testing.T) { + m := newModuleWithOversizedDoc(t) + errorList := errors.NewLintRuleErrorsList() + + NewNoLangKeyRule().CheckFiles(m, errorList) + + errs := errorList.GetErrors() + require.Len(t, errs, 1) + require.True(t, strings.EqualFold(errs[0].Level.String(), "warn"), "level=%s", errs[0].Level.String()) + require.Contains(t, errs[0].Text, "too large") + }) + + t.Run("excluded by file", func(t *testing.T) { + m := newModuleWithOversizedDoc(t) + errorList := errors.NewLintRuleErrorsList() + + NewNoLangKeyRule(). + WithFileSizeExcludes([]pkg.StringRuleExclude{"docs/big.md"}, nil). + CheckFiles(m, errorList) + + require.Empty(t, errorList.GetErrors()) + }) + + t.Run("excluded by directory", func(t *testing.T) { + m := newModuleWithOversizedDoc(t) + errorList := errors.NewLintRuleErrorsList() + + NewNoLangKeyRule(). + WithFileSizeExcludes(nil, []pkg.DirectoryRuleExclude{"docs"}). + CheckFiles(m, errorList) + + require.Empty(t, errorList.GetErrors()) + }) +} diff --git a/pkg/linters/no-cyrillic/README.md b/pkg/linters/no-cyrillic/README.md index 931f01185..88753827d 100644 --- a/pkg/linters/no-cyrillic/README.md +++ b/pkg/linters/no-cyrillic/README.md @@ -56,6 +56,16 @@ The linter intelligently skips files where cyrillic is expected or acceptable: - **Linter itself**: - `no_cyrillic.go` and `no_cyrillic_test.go` - The linter's own code +**Large files:** + +Files larger than 10 MiB are not read into memory or scanned. Such files are almost always generated data blobs (bundled Grafana dashboards, rendered OpenAPI, CRD bundles) rather than hand-written sources, and reading a multi-gigabyte file just to scan for cyrillic would exhaust memory. Instead of failing, the linter reports a **warning** so the skipped file stays visible: + +``` +file is too large (N bytes) to check for Cyrillic letters and was skipped; exclude the file or its directory in the no-cyrillic rules to silence this warning +``` + +To silence it, exclude the file or its directory via the standard `exclude-rules` (see [Configuration](#configuration)) — the same exclusions that skip a file from cyrillic checking also suppress this warning. + **Why it matters:** 1. **International Collaboration**: Code with cyrillic characters is inaccessible to developers who don't read Russian diff --git a/pkg/linters/no-cyrillic/rules/files.go b/pkg/linters/no-cyrillic/rules/files.go index 20b213990..799746d57 100644 --- a/pkg/linters/no-cyrillic/rules/files.go +++ b/pkg/linters/no-cyrillic/rules/files.go @@ -28,6 +28,19 @@ import ( const ( FilesRuleName = "files" + + // maxCheckableFileSize bounds how large a file the Cyrillic check will read + // into memory. Files above it are generated data blobs (bundled Grafana + // dashboards, rendered openapi, CRD bundles), not hand-written sources; + // reading a multi-gigabyte file just to scan for Cyrillic letters would + // exhaust memory, so such files are skipped. + maxCheckableFileSize = 10 << 20 // 10 MiB + + // maxCyrillicReportLines bounds how many offending lines a single finding + // echoes back, and maxCyrillicLineWidth bounds the width of each. Together + // they keep a file full of Cyrillic from dumping megabytes into the log. + maxCyrillicReportLines = 100 + maxCyrillicLineWidth = 200 ) var ( @@ -82,6 +95,26 @@ func (r *FilesRule) CheckFile(m pkg.Module, fileName string, errorList *errors.L return } + info, err := os.Stat(fileName) + if err != nil { + errorList.Error(err.Error()) + + return + } + + // Files too large to be hand-written sources are not scanned: reading a + // multi-gigabyte generated file into memory (and echoing its Cyrillic lines + // into a finding) would blow up memory and flood the log. Report it as a + // warning instead of failing. This is gated by r.Enabled(fName) above, so a + // user can silence it by excluding the file or its directory in the + // no-cyrillic exclude rules. + if info.Size() > maxCheckableFileSize { + errorList.WithFilePath(fName). + Warnf("file is too large (%d bytes) to check for Cyrillic letters and was skipped; exclude the file or its directory in the no-cyrillic rules to silence this warning", info.Size()) + + return + } + lines, err := getFileContent(fileName) if err != nil { errorList.Error(err.Error()) diff --git a/pkg/linters/no-cyrillic/rules/guard_test.go b/pkg/linters/no-cyrillic/rules/guard_test.go new file mode 100644 index 000000000..18c6ba3da --- /dev/null +++ b/pkg/linters/no-cyrillic/rules/guard_test.go @@ -0,0 +1,200 @@ +/* +Copyright 2025 Flant JSC + +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 rules + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/gojuno/minimock/v3" + + "github.com/deckhouse/dmt/internal/mocks" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +// writeOversizedCyrillicFile creates a file just over the size limit whose first +// line contains Cyrillic (so it would be reported if it were actually scanned). +func writeOversizedCyrillicFile(t *testing.T, path string) { + t.Helper() + + f, err := os.Create(path) + if err != nil { + t.Fatalf("create file: %v", err) + } + + if _, err := f.WriteString("greeting: Привет\n"); err != nil { + t.Fatalf("write: %v", err) + } + + if err := f.Truncate(maxCheckableFileSize + 1); err != nil { + t.Fatalf("truncate: %v", err) + } + + if err := f.Close(); err != nil { + t.Fatalf("close: %v", err) + } +} + +// TestFilesRule_CheckFile_WarnsOnOversizedFile verifies that a file larger than +// maxCheckableFileSize is not read into memory but reported as a warning (rather +// than silently skipped or read and echoed into the log). +func TestFilesRule_CheckFile_WarnsOnOversizedFile(t *testing.T) { + mc := minimock.NewController(t) + mockModule := mocks.NewModuleMock(mc) + + tempDir := t.TempDir() + mockModule.GetPathMock.Return(tempDir) + + testFile := filepath.Join(tempDir, "huge.yaml") + writeOversizedCyrillicFile(t, testFile) + + rule := NewFilesRule(nil, nil) + errorList := &errors.LintRuleErrorsList{} + + rule.CheckFile(mockModule, testFile, errorList) + + errs := errorList.GetErrors() + if len(errs) != 1 { + t.Fatalf("expected exactly one finding for an oversized file, got %d", len(errs)) + } + + if !strings.EqualFold(errs[0].Level.String(), "warn") { + t.Errorf("expected a warn-level finding, got %q", errs[0].Level.String()) + } + + if !strings.Contains(errs[0].Text, "too large") { + t.Errorf("expected the finding to mention the file is too large, got %q", errs[0].Text) + } +} + +// TestFilesRule_CheckFile_OversizedFileCanBeExcluded verifies the oversized-file +// warning honours the exclude rules, so a user can silence it by excluding the +// file or its directory. +func TestFilesRule_CheckFile_OversizedFileCanBeExcluded(t *testing.T) { + tests := []struct { + name string + relPath string + excludeFile []pkg.StringRuleExclude + excludeDirs []pkg.DirectoryRuleExclude + }{ + { + name: "excluded by file", + relPath: "huge.yaml", + excludeFile: []pkg.StringRuleExclude{"huge.yaml"}, + }, + { + name: "excluded by directory", + relPath: "big/huge.yaml", + excludeDirs: []pkg.DirectoryRuleExclude{"big"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mc := minimock.NewController(t) + mockModule := mocks.NewModuleMock(mc) + + tempDir := t.TempDir() + mockModule.GetPathMock.Return(tempDir) + + testFile := filepath.Join(tempDir, filepath.FromSlash(tt.relPath)) + if err := os.MkdirAll(filepath.Dir(testFile), 0700); err != nil { + t.Fatalf("mkdir: %v", err) + } + + writeOversizedCyrillicFile(t, testFile) + + rule := NewFilesRule(tt.excludeFile, tt.excludeDirs) + errorList := &errors.LintRuleErrorsList{} + + rule.CheckFile(mockModule, testFile, errorList) + + if errs := errorList.GetErrors(); len(errs) > 0 { + t.Errorf("expected excluded oversized file to produce no findings, got %d", len(errs)) + } + }) + } +} + +// TestFilesRule_CheckFile_CapsReportedLines verifies that a file with more +// Cyrillic lines than maxCyrillicReportLines produces a bounded finding with a +// truncation note rather than echoing every line. +func TestFilesRule_CheckFile_CapsReportedLines(t *testing.T) { + mc := minimock.NewController(t) + mockModule := mocks.NewModuleMock(mc) + + tempDir := t.TempDir() + mockModule.GetPathMock.Return(tempDir) + + var sb strings.Builder + for i := range maxCyrillicReportLines * 3 { + fmt.Fprintf(&sb, "line%d: Привет\n", i) + } + + testFile := filepath.Join(tempDir, "many.yaml") + if err := os.WriteFile(testFile, []byte(sb.String()), 0600); err != nil { + t.Fatalf("write file: %v", err) + } + + rule := NewFilesRule(nil, nil) + errorList := &errors.LintRuleErrorsList{} + + rule.CheckFile(mockModule, testFile, errorList) + + errs := errorList.GetErrors() + if len(errs) != 1 { + t.Fatalf("expected exactly one finding, got %d", len(errs)) + } + + value := fmt.Sprint(errs[0].ObjectValue) + + if !strings.Contains(value, "truncated") { + t.Errorf("expected the reported value to be truncated, got:\n%s", value) + } + + // At most maxCyrillicReportLines offending lines are echoed (each rendered as + // two lines: the source line and the cursor), plus the truncation note. + if got := strings.Count(value, "Привет"); got > maxCyrillicReportLines { + t.Errorf("expected at most %d echoed lines, got %d", maxCyrillicReportLines, got) + } +} + +// TestCheckCyrillicLettersInString_CapsLineWidth verifies that a single very +// long Cyrillic line is truncated in the reported message. +func TestCheckCyrillicLettersInString_CapsLineWidth(t *testing.T) { + long := strings.Repeat("Ы", maxCyrillicLineWidth*4) + + msg, has := checkCyrillicLettersInString(long) + if !has { + t.Fatal("expected Cyrillic to be detected") + } + + if !strings.Contains(msg, "…") { + t.Errorf("expected a truncated long line to be marked with an ellipsis, got:\n%s", msg) + } + + // The reported line (first line of msg) must be bounded to the configured + // width plus the ellipsis, not the full 4x-width input. + firstLine, _, _ := strings.Cut(msg, "\n") + if runes := []rune(firstLine); len(runes) > maxCyrillicLineWidth+1 { + t.Errorf("expected reported line width <= %d, got %d", maxCyrillicLineWidth+1, len(runes)) + } +} diff --git a/pkg/linters/no-cyrillic/rules/library.go b/pkg/linters/no-cyrillic/rules/library.go index 4c3cba79b..dfca84ae4 100644 --- a/pkg/linters/no-cyrillic/rules/library.go +++ b/pkg/linters/no-cyrillic/rules/library.go @@ -17,6 +17,7 @@ limitations under the License. package rules import ( + "fmt" "regexp" "strings" ) @@ -42,6 +43,13 @@ func checkCyrillicLettersInString(line string) (string, bool) { // Replace trim all spaces, because we do not need formatting here line = strings.TrimSpace(line) + // Bound the width of a single reported line: a minified file can hold a very + // long line, and echoing all of it (twice, with the cursor) would flood the + // log. + if runes := []rune(line); len(runes) > maxCyrillicLineWidth { + line = string(runes[:maxCyrillicLineWidth]) + "…" + } + // Make string with pointers to Cyrillic letters so user can detect hidden letters. cursor := cyrFillerRe.ReplaceAllString(line, "-") cursor = cyrPointerRe.ReplaceAllString(cursor, "^") @@ -55,15 +63,31 @@ func checkCyrillicLettersInArray(lines []string) (string, bool) { res := make([]string, 0) hasCyr := false + truncated := 0 for _, line := range lines { msg, has := checkCyrillicLettersInString(line) - if has { - hasCyr = true + if !has { + continue + } + + hasCyr = true - res = append(res, msg) + // Cap the number of reported lines so a file full of Cyrillic cannot + // produce a multi-megabyte finding. + if len(res) >= maxCyrillicReportLines { + truncated++ + + continue } + + res = append(res, msg) + } + + out := strings.Join(res, "\n") + if truncated > 0 { + out += fmt.Sprintf("\n… and %d more line(s) with Cyrillic letters (truncated)", truncated) } - return strings.Join(res, "\n"), hasCyr + return out, hasCyr } diff --git a/pkg/linters/openapi/rules/crds.go b/pkg/linters/openapi/rules/crds.go index e13951986..5e8454e72 100644 --- a/pkg/linters/openapi/rules/crds.go +++ b/pkg/linters/openapi/rules/crds.go @@ -18,7 +18,6 @@ package rules import ( "fmt" - "os" "path/filepath" "regexp" "strings" @@ -26,6 +25,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "sigs.k8s.io/yaml" + "github.com/deckhouse/dmt/internal/fsutils" "github.com/deckhouse/dmt/pkg" "github.com/deckhouse/dmt/pkg/errors" ) @@ -207,7 +207,7 @@ func (r *DeckhouseCRDsRule) Run(moduleName, path string, errorList *errors.LintR shortPath, _ := filepath.Rel(r.rootPath, path) - fileContent, err := os.ReadFile(path) + fileContent, err := fsutils.ReadFile(path) if err != nil { errorList.Errorf("Can't read file %s: %s", shortPath, err) return diff --git a/pkg/linters/templates/rules/grafana_dashboards.go b/pkg/linters/templates/rules/grafana_dashboards.go index 55a385a70..95f2cd9f5 100644 --- a/pkg/linters/templates/rules/grafana_dashboards.go +++ b/pkg/linters/templates/rules/grafana_dashboards.go @@ -70,7 +70,7 @@ func (r *GrafanaRule) ValidateGrafanaDashboards(m *module.Module, errorList *err return } - content, err := os.ReadFile(monitoringFilePath) + content, err := fsutils.ReadFile(monitoringFilePath) if err != nil { errorList.WithFilePath(monitoringFilePath). Errorf("Cannot read 'templates/monitoring.yaml' file: %s", err) @@ -121,7 +121,7 @@ func (r *GrafanaRule) validateDashboardFiles(m *module.Module, errorList *errors // validateDashboardFile validates a single grafana dashboard file func (r *GrafanaRule) validateDashboardFile(filePath string, errorList *errors.LintRuleErrorsList) { - content, err := os.ReadFile(filePath) + content, err := fsutils.ReadFile(filePath) if err != nil { errorList.WithFilePath(filePath).Errorf("failed to read dashboard file: %s", err) return diff --git a/pkg/linters/templates/rules/mount_points.go b/pkg/linters/templates/rules/mount_points.go index bbc5447fb..f5013d85d 100644 --- a/pkg/linters/templates/rules/mount_points.go +++ b/pkg/linters/templates/rules/mount_points.go @@ -23,6 +23,7 @@ import ( "gopkg.in/yaml.v3" + "github.com/deckhouse/dmt/internal/fsutils" "github.com/deckhouse/dmt/pkg" "github.com/deckhouse/dmt/pkg/errors" ) @@ -135,7 +136,7 @@ func collectMountPointsDirs(m pkg.Module, errorList *errors.LintRuleErrorsList) return nil } - data, err := os.ReadFile(path) + data, err := fsutils.ReadFile(path) if err != nil { errorList.Errorf("failed to read %s: %s", path, err) return nil diff --git a/pkg/linters/templates/rules/prometheus_rules.go b/pkg/linters/templates/rules/prometheus_rules.go index 29dfd6a0f..ca98062a6 100644 --- a/pkg/linters/templates/rules/prometheus_rules.go +++ b/pkg/linters/templates/rules/prometheus_rules.go @@ -28,6 +28,7 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/yaml" + "github.com/deckhouse/dmt/internal/fsutils" "github.com/deckhouse/dmt/internal/promtool" "github.com/deckhouse/dmt/internal/storage" "github.com/deckhouse/dmt/pkg" @@ -106,7 +107,7 @@ func (r *PrometheusRule) ValidatePrometheusRules(m pkg.Module, errorList *errors return } - content, err := os.ReadFile(monitoringFilePath) + content, err := fsutils.ReadFile(monitoringFilePath) if err != nil { errorList.WithFilePath(monitoringFilePath). Errorf("Cannot read 'templates/monitoring.yaml' file: %s", err) diff --git a/test/e2e/framework.go b/test/e2e/framework.go index e7cfaaf68..3fcf0a59d 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -64,15 +64,20 @@ const ( // // Matching semantics: // - linter is required and matched case-insensitively against LinterID. -// - rule, level and textContains are optional; when set they all must match. +// - rule, level, textContains and valueContains are optional; when set they +// all must match. // - textContains is a case-sensitive substring match against the message. +// - valueContains is a case-sensitive substring match against the finding's +// attached value (ObjectValue), where linters put details too large for the +// message itself. // - count is the expected number of matching findings; 0 means "at least one". type Finding struct { - Linter string `yaml:"linter"` - Rule string `yaml:"rule"` - Level string `yaml:"level"` - TextContains string `yaml:"textContains"` - Count int `yaml:"count"` + Linter string `yaml:"linter"` + Rule string `yaml:"rule"` + Level string `yaml:"level"` + TextContains string `yaml:"textContains"` + ValueContains string `yaml:"valueContains"` + Count int `yaml:"count"` } func (f Finding) String() string { @@ -89,6 +94,10 @@ func (f Finding) String() string { parts = append(parts, fmt.Sprintf("textContains=%q", f.TextContains)) } + if f.ValueContains != "" { + parts = append(parts, fmt.Sprintf("valueContains=%q", f.ValueContains)) + } + return strings.Join(parts, " ") } @@ -386,6 +395,10 @@ func findingMatches(exp Finding, got pkg.LinterError) bool { return false } + if exp.ValueContains != "" && !strings.Contains(fmt.Sprint(got.ObjectValue), exp.ValueContains) { + return false + } + return true } @@ -416,6 +429,10 @@ func copyDir(src, dst string) error { target := filepath.Join(dst, rel) + if info.Mode()&os.ModeSymlink != 0 { + return copySymlink(src, path, target) + } + if info.IsDir() { return os.MkdirAll(target, 0o755) } @@ -424,6 +441,65 @@ func copyDir(src, dst string) error { }) } +// copySymlink reproduces a symbolic link found under the source tree. Links that +// stay inside the module (including loops, e.g. templates/loop -> ..) are +// recreated as links so the copy faithfully preserves the layout under test; +// links pointing outside the module (e.g. a chart dependency symlinked from a +// shared lib dir) are dereferenced so their content still resolves in the +// isolated copy. +func copySymlink(srcRoot, path, dst string) error { + linkTarget, err := os.Readlink(path) + if err != nil { + return err + } + + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return err + } + + if symlinkStaysWithin(srcRoot, path, linkTarget) { + return os.Symlink(linkTarget, dst) + } + + // Dereference: copy whatever the link ultimately points to. + info, err := os.Stat(path) + if err != nil { + return err + } + + if info.IsDir() { + return copyDir(path, dst) + } + + return copyFile(path, dst, info.Mode()) +} + +// symlinkStaysWithin reports whether the link at path, pointing at linkTarget, +// resolves to a location inside srcRoot. +func symlinkStaysWithin(srcRoot, path, linkTarget string) bool { + resolved := linkTarget + if !filepath.IsAbs(resolved) { + resolved = filepath.Join(filepath.Dir(path), linkTarget) + } + + absRoot, err := filepath.Abs(srcRoot) + if err != nil { + return false + } + + absTarget, err := filepath.Abs(resolved) + if err != nil { + return false + } + + rel, err := filepath.Rel(absRoot, absTarget) + if err != nil { + return false + } + + return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + func copyFile(src, dst string, mode os.FileMode) error { in, err := os.Open(src) if err != nil { diff --git a/test/e2e/testdata/module/symlink-loop/expected.yaml b/test/e2e/testdata/module/symlink-loop/expected.yaml new file mode 100644 index 000000000..4f7c1d370 --- /dev/null +++ b/test/e2e/testdata/module/symlink-loop/expected.yaml @@ -0,0 +1,11 @@ +description: > + A chart containing a symbolic link loop (templates/loop -> the module root) + must not crash the helm loader with an out-of-memory error. dmt skips the loop + while rendering a cleaned copy of the chart and reports it as a warn-level + finding, so the module is still linted. +module: module +expect: + - linter: manager + rule: symlink-loop + level: warn + textContains: "symbolic link loop detected" diff --git a/test/e2e/testdata/module/symlink-loop/module/module.yaml b/test/e2e/testdata/module/symlink-loop/module/module.yaml new file mode 100644 index 000000000..8221f2778 --- /dev/null +++ b/test/e2e/testdata/module/symlink-loop/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-symlink-loop +namespace: e2e-symlink-loop diff --git a/test/e2e/testdata/module/symlink-loop/module/openapi/config-values.yaml b/test/e2e/testdata/module/symlink-loop/module/openapi/config-values.yaml new file mode 100644 index 000000000..03b0d8bfe --- /dev/null +++ b/test/e2e/testdata/module/symlink-loop/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/module/symlink-loop/module/openapi/values.yaml b/test/e2e/testdata/module/symlink-loop/module/openapi/values.yaml new file mode 100644 index 000000000..47180da56 --- /dev/null +++ b/test/e2e/testdata/module/symlink-loop/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/module/symlink-loop/module/templates/configmap.yaml b/test/e2e/testdata/module/symlink-loop/module/templates/configmap.yaml new file mode 100644 index 000000000..8c45e72e9 --- /dev/null +++ b/test/e2e/testdata/module/symlink-loop/module/templates/configmap.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-symlink-loop diff --git a/test/e2e/testdata/module/symlink-loop/module/templates/loop b/test/e2e/testdata/module/symlink-loop/module/templates/loop new file mode 120000 index 000000000..a96aa0ea9 --- /dev/null +++ b/test/e2e/testdata/module/symlink-loop/module/templates/loop @@ -0,0 +1 @@ +.. \ No newline at end of file diff --git a/test/e2e/testdata/no-cyrillic/truncated/expected.yaml b/test/e2e/testdata/no-cyrillic/truncated/expected.yaml new file mode 100644 index 000000000..649d71470 --- /dev/null +++ b/test/e2e/testdata/no-cyrillic/truncated/expected.yaml @@ -0,0 +1,12 @@ +description: > + A file with more Cyrillic lines (150) than the report cap (100) produces a + single no-cyrillic finding whose echoed value is truncated with a + "… (truncated)" note, instead of dumping every offending line into the log. +module: module +expect: + - linter: no-cyrillic + rule: files + level: error + textContains: "has cyrillic letters" + valueContains: "truncated" + count: 1 diff --git a/test/e2e/testdata/no-cyrillic/truncated/module/module.yaml b/test/e2e/testdata/no-cyrillic/truncated/module/module.yaml new file mode 100644 index 000000000..6de515714 --- /dev/null +++ b/test/e2e/testdata/no-cyrillic/truncated/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-cyrillic-truncated +namespace: e2e-cyrillic-truncated diff --git a/test/e2e/testdata/no-cyrillic/truncated/module/openapi/config-values.yaml b/test/e2e/testdata/no-cyrillic/truncated/module/openapi/config-values.yaml new file mode 100644 index 000000000..03b0d8bfe --- /dev/null +++ b/test/e2e/testdata/no-cyrillic/truncated/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/no-cyrillic/truncated/module/openapi/values.yaml b/test/e2e/testdata/no-cyrillic/truncated/module/openapi/values.yaml new file mode 100644 index 000000000..47180da56 --- /dev/null +++ b/test/e2e/testdata/no-cyrillic/truncated/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/no-cyrillic/truncated/module/templates/configmap.yaml b/test/e2e/testdata/no-cyrillic/truncated/module/templates/configmap.yaml new file mode 100644 index 000000000..3a394299b --- /dev/null +++ b/test/e2e/testdata/no-cyrillic/truncated/module/templates/configmap.yaml @@ -0,0 +1,155 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-cyrillic-truncated +data: + key0: "Привет" + key1: "Привет" + key2: "Привет" + key3: "Привет" + key4: "Привет" + key5: "Привет" + key6: "Привет" + key7: "Привет" + key8: "Привет" + key9: "Привет" + key10: "Привет" + key11: "Привет" + key12: "Привет" + key13: "Привет" + key14: "Привет" + key15: "Привет" + key16: "Привет" + key17: "Привет" + key18: "Привет" + key19: "Привет" + key20: "Привет" + key21: "Привет" + key22: "Привет" + key23: "Привет" + key24: "Привет" + key25: "Привет" + key26: "Привет" + key27: "Привет" + key28: "Привет" + key29: "Привет" + key30: "Привет" + key31: "Привет" + key32: "Привет" + key33: "Привет" + key34: "Привет" + key35: "Привет" + key36: "Привет" + key37: "Привет" + key38: "Привет" + key39: "Привет" + key40: "Привет" + key41: "Привет" + key42: "Привет" + key43: "Привет" + key44: "Привет" + key45: "Привет" + key46: "Привет" + key47: "Привет" + key48: "Привет" + key49: "Привет" + key50: "Привет" + key51: "Привет" + key52: "Привет" + key53: "Привет" + key54: "Привет" + key55: "Привет" + key56: "Привет" + key57: "Привет" + key58: "Привет" + key59: "Привет" + key60: "Привет" + key61: "Привет" + key62: "Привет" + key63: "Привет" + key64: "Привет" + key65: "Привет" + key66: "Привет" + key67: "Привет" + key68: "Привет" + key69: "Привет" + key70: "Привет" + key71: "Привет" + key72: "Привет" + key73: "Привет" + key74: "Привет" + key75: "Привет" + key76: "Привет" + key77: "Привет" + key78: "Привет" + key79: "Привет" + key80: "Привет" + key81: "Привет" + key82: "Привет" + key83: "Привет" + key84: "Привет" + key85: "Привет" + key86: "Привет" + key87: "Привет" + key88: "Привет" + key89: "Привет" + key90: "Привет" + key91: "Привет" + key92: "Привет" + key93: "Привет" + key94: "Привет" + key95: "Привет" + key96: "Привет" + key97: "Привет" + key98: "Привет" + key99: "Привет" + key100: "Привет" + key101: "Привет" + key102: "Привет" + key103: "Привет" + key104: "Привет" + key105: "Привет" + key106: "Привет" + key107: "Привет" + key108: "Привет" + key109: "Привет" + key110: "Привет" + key111: "Привет" + key112: "Привет" + key113: "Привет" + key114: "Привет" + key115: "Привет" + key116: "Привет" + key117: "Привет" + key118: "Привет" + key119: "Привет" + key120: "Привет" + key121: "Привет" + key122: "Привет" + key123: "Привет" + key124: "Привет" + key125: "Привет" + key126: "Привет" + key127: "Привет" + key128: "Привет" + key129: "Привет" + key130: "Привет" + key131: "Привет" + key132: "Привет" + key133: "Привет" + key134: "Привет" + key135: "Привет" + key136: "Привет" + key137: "Привет" + key138: "Привет" + key139: "Привет" + key140: "Привет" + key141: "Привет" + key142: "Привет" + key143: "Привет" + key144: "Привет" + key145: "Привет" + key146: "Привет" + key147: "Привет" + key148: "Привет" + key149: "Привет"