[dmt] fix add mount points validation rule#401
Open
diyliv wants to merge 13 commits into
Open
Conversation
Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
…tPoints Signed-off-by: diyliv <onlogn081@gmail.com>
Signed-off-by: diyliv <onlogn081@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new mount-points validation rule to keep mount-points.yaml in sync with actual volumeMount.mountPath usage in pod controllers, and wires it into both the templates and container linters with config support, docs, and tests.
Changes:
- Added a Templates-linter
mount-pointsrule that warns whenmount-points.yamldeclares directories not used as any controllermountPath. - Added a Container-linter
mount-pointsrule that warns when controllers usemountPaths not declared in anymount-points.yaml. - Wired config (global + runtime), updated documentation, and added unit + e2e test coverage.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/testdata/container/mount-points-missing/module/templates/deployment.yaml | E2E fixture with declared + undeclared mountPaths. |
| test/e2e/testdata/container/mount-points-missing/module/openapi/values.yaml | Minimal OpenAPI fixture for the e2e module. |
| test/e2e/testdata/container/mount-points-missing/module/openapi/config-values.yaml | Minimal OpenAPI fixture for the e2e module. |
| test/e2e/testdata/container/mount-points-missing/module/module.yaml | E2E module metadata fixture. |
| test/e2e/testdata/container/mount-points-missing/module/images/app/mount-points.yaml | E2E mount-points declaration fixture. |
| test/e2e/testdata/container/mount-points-missing/expected.yaml | E2E expected warning for undeclared mountPath. |
| pkg/linters/templates/templates.go | Registers the new templates mount-points rule in the templates linter. |
| pkg/linters/templates/rules/mount_points.go | Implements file → templates validation for mount-points.yaml. |
| pkg/linters/templates/rules/mount_points_test.go | Unit tests for templates mount-points rule scenarios. |
| pkg/linters/templates/README.md | Documents the new templates mount-points rule and configuration. |
| pkg/linters/container/rules/mount_points.go | Implements templates → file validation for undeclared mountPaths. |
| pkg/linters/container/rules/mount_points_test.go | Unit tests for container mount-points rule scenarios. |
| pkg/linters/container/rules.go | Registers the new container mount-points rule. |
| pkg/linters/container/container.go | Stores module path for rules needing module filesystem access. |
| pkg/config/linters_settings.go | Adds rule + exclude configuration fields for mount-points. |
| pkg/config/global/global.go | Adds global rule config entries for mount-points. |
| pkg/config.go | Adds runtime config entries for mount-points rules/excludes. |
| internal/module/module.go | Maps mount-points rule levels/excludes into effective linter settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
413
to
+417
| excludes.SeccompProfile = configExcludes.SeccompProfile.Get() | ||
| excludes.NoNewPrivileges = configExcludes.NoNewPrivileges.Get() | ||
| excludes.ImageDigest = configExcludes.ImageDigest.Get() | ||
| excludes.Description = pkg.StringRuleExcludeList(configExcludes.Description) | ||
| excludes.MountPoints = pkg.StringRuleExcludeList(configExcludes.MountPoints) |
|
|
||
| `/etc/app/certs` is declared in `mount-points.yaml` but no pod controller uses it as a mountPath. | ||
|
|
||
| **Error:** |
Comment on lines
+57
to
+63
| dirsByFile := collectMountPointsDirs(m, errorList) | ||
| if len(dirsByFile) == 0 { | ||
| return | ||
| } | ||
|
|
||
| templateMountPaths := collectTemplateMountPaths(m, errorList) | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mount_points.go): checks that all directories listed inmount-points.yamlare actually used asvolumeMount.mountPathin at least one pod controller (Deployment / StatefulSet / DaemonSet). One-way check: file → templates.filepath.Walkto find allmount-points.yamlfiles, parse each and extract thedirslist.GetStorage(), filters pod controllers viaIsPodController(), callsGetAllContainers()for each (main + init containers), collects allvolumeMount.mountPathinto a set.strings.TrimRight("/")) on both sides. If a dir from the file is not found in any controller — warning (not error).mount-points.yamlfile viaerrorList.WithFilePath()so the developer can quickly locate the source.dirs→ no errors.MountPointsRule RuleConfigto runtime config (pkg/config.go), global config with tagmapstructure:"mount-points"(pkg/config/global/global.go), rule level mapping inmapTemplatesRules()(internal/module/module.go).NewMountPointsRule().ValidateMountPoints(...)call added to theRun()method of the templates linter (templates.go).Context
Modules may contain
mount-points.yamlfiles describing directories that should be mounted into containers. If a directory is listed in the file but is not used asvolumeMount.mountPathin any pod, containerd v2 may crash when trying to mount something into a non-existent directory. The rule ensures thatmount-points.yamlalways matches actual usage in templates.Example
mount-points.yaml (
images/app/mount-points.yaml):Deployment in template (uses only
/etc/app):Result: warning —
mount-points.yaml references dir "/etc/app/certs" which is not used as a mountPath in any pod controller.Excluding directories
Per-line exclude is supported via DMT config. Use when pods are managed outside Helm (operator, webhook, static pods, bashible) — their
volumeMountsare not in templates, producing false positives.