[dmt] add NamespaceLabelsRule to container exclusion settings#394
Open
riptide-01 wants to merge 15 commits into
Open
[dmt] add NamespaceLabelsRule to container exclusion settings#394riptide-01 wants to merge 15 commits into
riptide-01 wants to merge 15 commits into
Conversation
Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
…ainerRules struct Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
26be33e to
01475d2
Compare
- Introduced new test cases in `framework_test.go` to validate the behavior of the `Match` function with respect to expected lint findings and rules that should pass. - Enhanced the `framework.go` file to support the new `ExpectPass` feature, allowing specification of rules that must not produce any matching findings. - Updated the README to document the new `expectPass` functionality. - Added test data for a namespace that should be ignored by the container linter on the `object-namespace-labels` rule. This commit improves the testing framework for linting rules and ensures better compliance with expected behavior in various scenarios. Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
…iguration Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
3012031 to
84a9740
Compare
Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
…rom `.dmt.yaml` to `.dmtlint.yaml` across multiple linters. Signed-off-by: Smyslov Maxim <maksim.smyslov@flant.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the container linter’s object-namespace-labels rule properly honor namespace exclusions configured via .dmtlint.yaml, and adds e2e coverage (including a new expectPass assertion) to prevent regressions.
Changes:
- Wire
object-namespace-labelsexclude entries from module/global config through module mapping intoNamespaceLabelsRule, and applyKindRule.Enabled()inside the rule. - Fix the global config mapstructure key for the rule (
namespace-labels→object-namespace-labels). - Add e2e fixtures for both the “violation” and “excluded namespace” scenarios, plus an
expectPassmatcher and unit tests in the e2e framework.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 30 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/testdata/container/namespace-labels/module/templates/manifests.yaml | Adds a fixture namespace + PrometheusRule that should trigger object-namespace-labels. |
| test/e2e/testdata/container/namespace-labels/module/openapi/values.yaml | Minimal OpenAPI scaffold for the new e2e fixture module. |
| test/e2e/testdata/container/namespace-labels/module/openapi/config-values.yaml | Minimal OpenAPI scaffold for the new e2e fixture module. |
| test/e2e/testdata/container/namespace-labels/module/module.yaml | Declares the fixture module metadata. |
| test/e2e/testdata/container/namespace-labels/expected.yaml | Defines expected failing finding for object-namespace-labels. |
| test/e2e/testdata/container/ignore-namespace-labels/module/templates/manifests.yaml | Adds a fixture namespace + PrometheusRule that should be excluded from object-namespace-labels. |
| test/e2e/testdata/container/ignore-namespace-labels/module/openapi/values.yaml | Minimal OpenAPI scaffold for the exclusion fixture module. |
| test/e2e/testdata/container/ignore-namespace-labels/module/openapi/config-values.yaml | Minimal OpenAPI scaffold for the exclusion fixture module. |
| test/e2e/testdata/container/ignore-namespace-labels/module/module.yaml | Declares the exclusion fixture module metadata. |
| test/e2e/testdata/container/ignore-namespace-labels/module/.dmtlint.yaml | Configures exclude-rules.object-namespace-labels to ignore the fixture namespace. |
| test/e2e/testdata/container/ignore-namespace-labels/expected.yaml | Uses expectPass to assert the rule produces no matching findings. |
| test/e2e/README.md | Documents the new expectPass field and its matching semantics. |
| test/e2e/framework.go | Adds ExpectPass to case spec and enforces it in Match(). |
| test/e2e/framework_test.go | Adds unit tests for expectPass matching behavior. |
| pkg/linters/templates/README.md | Clarifies “Configurable” meaning and updates config filename references in examples. |
| pkg/linters/rbac/README.md | Updates “Configurable” wording and configuration examples (needs follow-up fixes). |
| pkg/linters/openapi/README.md | Updates “Configurable” wording and configuration examples (needs follow-up fixes). |
| pkg/linters/no-cyrillic/README.md | Updates “Configurable” wording and configuration examples (needs follow-up fixes). |
| pkg/linters/module/README.md | Updates “Configurable” wording (needs follow-up fixes). |
| pkg/linters/images/README.md | Updates “Configurable” wording (needs follow-up fixes). |
| pkg/linters/hooks/README.md | Updates “Configurable” wording and configuration examples (needs follow-up fixes). |
| pkg/linters/docs/README.md | Updates “Configurable” wording (needs follow-up fixes). |
| pkg/linters/container/rules/namespace_labels.go | Adds exclude support via embedded KindRule and checks Enabled() before running the rule. |
| pkg/linters/container/rules.go | Passes configured exclude list into NewNamespaceLabelsRule(...). |
| pkg/linters/container/README.md | Marks rule configurable and documents exclusions (currently has multiple Markdown/YAML corruption issues to fix). |
| pkg/config/linters_settings.go | Adds object-namespace-labels to module-level container.exclude-rules settings mapping. |
| pkg/config/global/global.go | Fixes global mapstructure key to object-namespace-labels. |
| pkg/config.go | Adds NamespaceLabelsRule into runtime container exclude rules struct. |
| internal/module/module.go | Wires module config exclusions into the container linter config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | [no-new-privileges](#no-new-privileges) | Validates containers don't allow privilege escalation | ✅ | enabled | | ||
| | [seccomp-profile](#seccomp-profile) | Validates seccomp profile configuration | ✅ | enabled | | ||
|
|
||
| "Configurable" means that this rule can be configured using the `.dmt.yaml` file, including customizing the rule's parameters and/or disabling the rule. |
| Exclude specific `d8-*` namespaces from this check when the Prometheus watcher label is intentionally omitted: | ||
|
|
||
| ```yaml | ||
| # .dmt.yaml |
| ``` | ||
|
|
||
| ✅ **Correct** - Non-root with deckhouse user: | ||
| ✅ .dmtlint.yaml** - Non-root with deckhouse user: |
|
|
||
| ✅ **Correct** - Read-only filesystem: | ||
|
|
||
| .dmtlint.yaml |
| Pod running in hostNetwork and it's container port doesn't fit the range [4200,4299] | ||
| ``` | ||
|
|
||
| .dmtlint.yaml |
Comment on lines
+366
to
371
| ### Configuration in M.dmtlint.yamlectory | ||
|
|
||
| You can also place a `.dmt.yaml` configuration file directly in your module directory: | ||
|
|
||
| .dmtlint.yaml | ||
| ```yaml | ||
| # modules/my-module/.dmt.yaml |
| | [cyrillic-in-english](#cyrillic-in-english) | Validates English documentation doesn't contain cyrillic characters | ✅ | enabled | | ||
| | [no-lang-key](#no-lang-key) | Validates documentation front matter doesn't contain `lang` key | ✅ | enabled | | ||
|
|
||
| "Configurable" means that this rule can be configured using the `.dmt.yaml` file, including customizing the rule's parameters and/or disabling the rule. |
| | [**package-yaml**](#package-yaml) | Validates `package.yaml` metadata and new requirements schema | ✅ Yes | | ||
| | [**legacy-release-file**](#legacy-release-file) | Checks for deprecated `release.yaml` file | ❌ No | | ||
|
|
||
| "Configurable" means that this rule can be configured using the `.dmt.yaml` file, including customizing the rule's parameters and/or disabling the rule. |
| | [**werf**](#werf) | Validates werf.yaml configuration | ✅ Yes | | ||
| | [**patches**](#patches) | Validates patch file structure and documentation | ✅ Yes | | ||
|
|
||
| "Configurable" means that this rule can be configured using the `.dmt.yaml` file, including customizing the rule's parameters and/or disabling the rule. |
| ``` | ||
| dnsPolicy must be `ClusterFirstWithHostNet` when hostNetwork is `true` | ||
| ``` | ||
| ``.dmtlint.yaml |
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.
Description
The
object-namespace-labelscontainer rule could be documented as configurable, but excluding namespaces via.dmt.yamlhad no effect.This PR wires the exclude list through config and module mapping, applies
KindRule.Enabled()in the rule implementation, fixes the global config mapstructure key (namespace-labels→object-namespace-labels), and adds e2e fixtures plus anexpectPassassertion helper.Before / After
object-namespace-labelsunderlinters-settings.container.exclude-ruleswas ignoredNamespaceLabelsRuled8-*namespace withPrometheusRuleresourceskind+nameexclude entriesmapstructure:"namespace-labels"(mismatched name)mapstructure:"object-namespace-labels"(consistent with rule ID)namespace-labels) and excluded namespace (ignore-namespace-labels)expectPassfield with unit testsWhy do we need it, and what problem does it solve?
Some modules intentionally omit
prometheus.deckhouse.io/rules-watcher-enabledon specificd8-*namespaces. Users tried to disable the check viaexclude-rules, but it did not work because:NamespaceLabelsRulewas missing from the exclude-rules config structs and module mapping.Enabled()— unlike other container rules with exclude support.namespace-labelsinstead ofobject-namespace-labels).E2e tests lock in both the positive case (missing label is reported) and the exclude case (configured namespace is ignored).