diff --git a/README.md b/README.md index 18f12631..23a662d5 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ DMT includes **9 specialized linters** to validate different aspects of your Dec | [**NoCyrillic**](pkg/linters/no-cyrillic/README.md) | Character encoding | Cyrillic characters in code/config files | | [**OpenAPI**](pkg/linters/openapi/README.md) | OpenAPI schemas | Schema validation, CRD definitions, naming conventions | | [**RBAC**](pkg/linters/rbac/README.md) | Security policies | Role bindings, service accounts, wildcards | -| [**Templates**](pkg/linters/templates/README.md) | Kubernetes templates | VPA/PDB settings, Prometheus rules, Grafana dashboards, service ports, mount-points | +| [**Templates**](pkg/linters/templates/README.md) | Kubernetes templates | VPA/PDB settings, Prometheus rules, Grafana dashboards, service ports, mount-points, Ingress/Gateway API enablement, deprecated annotations | ### 🚀 Module Bootstrapping diff --git a/internal/modules/https_certificate_reuse_exclude_test.go b/internal/modules/https_certificate_reuse_exclude_test.go new file mode 100644 index 00000000..48828f32 --- /dev/null +++ b/internal/modules/https_certificate_reuse_exclude_test.go @@ -0,0 +1,66 @@ +/* +Copyright 2026 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 modules + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/config" + "github.com/deckhouse/dmt/pkg/config/global" +) + +// TestRemapHTTPSCertificateReuseExcludeRules proves that a module's +// .dmtlint.yaml exclude-rules for https-certificate-reuse (parsed into +// config.TemplatesExcludeRules by viper/mapstructure) actually reach the +// pkg.TemplatesExcludeRules the rule itself reads from — the same path +// IngressEnablement/GatewayEnablement already use. +func TestRemapHTTPSCertificateReuseExcludeRules(t *testing.T) { + configSettings := &config.LintersSettings{ + Templates: config.TemplatesSettings{ + ExcludeRules: config.TemplatesExcludeRules{ + HTTPSCertificateReuse: config.PathRuleExclude{ + Files: config.StringRuleExcludeList{"templates/legacy-certificate.yaml"}, + Directories: config.DirectoryRuleExcludeList{"templates/vendor/"}, + }, + }, + }, + } + + settings := remapLinterSettings(configSettings, &global.Linters{}) + + excludes := settings.Templates.ExcludeRules.HTTPSCertificateReuse + require.Equal(t, pkg.StringRuleExcludeList{"templates/legacy-certificate.yaml"}, excludes.Files) + require.Equal(t, pkg.DirectoryRuleExcludeList{"templates/vendor/"}, excludes.Directories) +} + +// TestRemapHTTPSCertificateReuseRuleLevel proves the rule-level impact +// override (global org-wide config, mirroring how every other Templates +// rule's level is wired) reaches pkg.TemplatesLinterRules too. +func TestRemapHTTPSCertificateReuseRuleLevel(t *testing.T) { + settings := remapLinterSettings(&config.LintersSettings{}, &global.Linters{ + Templates: global.TemplatesLinterConfig{ + Rules: global.TemplatesLinterRules{ + HTTPSCertificateReuseRule: global.RuleConfig{Impact: pkg.Warn.String()}, + }, + }, + }) + + require.Equal(t, pkg.Warn, *settings.Templates.Rules.HTTPSCertificateReuseRule.GetLevel()) +} diff --git a/internal/modules/module.go b/internal/modules/module.go index e1a0d1a0..518c6470 100644 --- a/internal/modules/module.go +++ b/internal/modules/module.go @@ -418,6 +418,10 @@ func mapTemplatesRules(linterSettings *pkg.LintersSettings, configSettings *conf rules.HelmRenderRule.SetLevel(globalRules.HelmRenderRule.Impact, fallbackImpact) rules.OpenAPIValuesQuoteRule.SetLevel(globalRules.OpenAPIValuesQuoteRule.Impact, fallbackImpact) rules.SchemaValidationRule.SetLevel(globalRules.SchemaValidationRule.Impact, fallbackImpact) + rules.DeprecatedHTTPRouteAnnotationsRule.SetLevel(globalRules.DeprecatedHTTPRouteAnnotationsRule.Impact, fallbackImpact) + rules.IngressEnablementRule.SetLevel(globalRules.IngressEnablementRule.Impact, fallbackImpact) + rules.GatewayEnablementRule.SetLevel(globalRules.GatewayEnablementRule.Impact, fallbackImpact) + rules.HTTPSCertificateReuseRule.SetLevel(globalRules.HTTPSCertificateReuseRule.Impact, fallbackImpact) } // mapOpenAPIRules configures OpenAPI linter rules @@ -552,6 +556,14 @@ func mapTemplatesExclusionsAndSettings(linterSettings *pkg.LintersSettings, conf excludes.MountPoints = pkg.StringRuleExcludeList(configExcludes.MountPoints) excludes.OpenAPIValuesQuote = pkg.StringRuleExcludeList(configExcludes.OpenAPIValuesQuote) excludes.SchemaValidation = configExcludes.SchemaValidation.Get() + excludes.DeprecatedHTTPRouteAnnotations.Files = pkg.StringRuleExcludeList(configExcludes.DeprecatedHTTPRouteAnnotations.Files) + excludes.DeprecatedHTTPRouteAnnotations.Directories = pkg.DirectoryRuleExcludeList(configExcludes.DeprecatedHTTPRouteAnnotations.Directories) + excludes.IngressEnablement.Files = pkg.StringRuleExcludeList(configExcludes.IngressEnablement.Files) + excludes.IngressEnablement.Directories = pkg.DirectoryRuleExcludeList(configExcludes.IngressEnablement.Directories) + excludes.GatewayEnablement.Files = pkg.StringRuleExcludeList(configExcludes.GatewayEnablement.Files) + excludes.GatewayEnablement.Directories = pkg.DirectoryRuleExcludeList(configExcludes.GatewayEnablement.Directories) + excludes.HTTPSCertificateReuse.Files = pkg.StringRuleExcludeList(configExcludes.HTTPSCertificateReuse.Files) + excludes.HTTPSCertificateReuse.Directories = pkg.DirectoryRuleExcludeList(configExcludes.HTTPSCertificateReuse.Directories) // Additional settings linterSettings.Templates.PrometheusRuleSettings.Disable = configSettings.Templates.PrometheusRules.Disable diff --git a/pkg/config.go b/pkg/config.go index a98025a7..b0c56ae5 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -137,23 +137,27 @@ type TemplatesLinterConfig struct { GrafanaDashboardsSettings GrafanaDashboardsSettings } type TemplatesLinterRules struct { - VPARule RuleConfig - PDBRule RuleConfig - IngressRule RuleConfig - PrometheusRule RuleConfig - GrafanaRule RuleConfig - KubeRBACProxyRule RuleConfig - ServicePortRule RuleConfig - ClusterDomainRule RuleConfig - RegistryRule RuleConfig - HTTPRouteRule RuleConfig - EnabledModulesRule RuleConfig - CRDEnabledModulesRule RuleConfig - WebhookConfigurationRule RuleConfig - MountPointsRule RuleConfig - HelmRenderRule RuleConfig - OpenAPIValuesQuoteRule RuleConfig - SchemaValidationRule RuleConfig + VPARule RuleConfig + PDBRule RuleConfig + IngressRule RuleConfig + PrometheusRule RuleConfig + GrafanaRule RuleConfig + KubeRBACProxyRule RuleConfig + ServicePortRule RuleConfig + ClusterDomainRule RuleConfig + RegistryRule RuleConfig + HTTPRouteRule RuleConfig + EnabledModulesRule RuleConfig + CRDEnabledModulesRule RuleConfig + WebhookConfigurationRule RuleConfig + MountPointsRule RuleConfig + HelmRenderRule RuleConfig + OpenAPIValuesQuoteRule RuleConfig + SchemaValidationRule RuleConfig + DeprecatedHTTPRouteAnnotationsRule RuleConfig + IngressEnablementRule RuleConfig + GatewayEnablementRule RuleConfig + HTTPSCertificateReuseRule RuleConfig } type PrometheusRuleSettings struct { @@ -164,17 +168,28 @@ type GrafanaDashboardsSettings struct { Disable bool } type TemplatesExcludeRules struct { - VPAAbsent KindRuleExcludeList - PDBAbsent KindRuleExcludeList - ServicePort ServicePortExcludeList - KubeRBACProxy StringRuleExcludeList - Ingress KindRuleExcludeList - HTTPRoute KindRuleExcludeList - EnabledModules EnabledModulesExcludeRule - WebhookConfiguration KindRuleExcludeList - MountPoints StringRuleExcludeList - OpenAPIValuesQuote StringRuleExcludeList - SchemaValidation KindRuleExcludeList + VPAAbsent KindRuleExcludeList + PDBAbsent KindRuleExcludeList + ServicePort ServicePortExcludeList + KubeRBACProxy StringRuleExcludeList + Ingress KindRuleExcludeList + HTTPRoute KindRuleExcludeList + EnabledModules EnabledModulesExcludeRule + WebhookConfiguration KindRuleExcludeList + MountPoints StringRuleExcludeList + OpenAPIValuesQuote StringRuleExcludeList + SchemaValidation KindRuleExcludeList + DeprecatedHTTPRouteAnnotations PathRuleExclude + IngressEnablement PathRuleExclude + GatewayEnablement PathRuleExclude + HTTPSCertificateReuse PathRuleExclude +} + +// PathRuleExclude excludes specific files and whole directories (both relative +// to the module root) from a rule that scans template source files. +type PathRuleExclude struct { + Files StringRuleExcludeList + Directories DirectoryRuleExcludeList } type EnabledModulesExcludeRule struct { diff --git a/pkg/config/global/global.go b/pkg/config/global/global.go index a5b2fca7..6b590b13 100644 --- a/pkg/config/global/global.go +++ b/pkg/config/global/global.go @@ -140,23 +140,27 @@ type TemplatesLinterConfig struct { } type TemplatesLinterRules struct { - VPARule RuleConfig `mapstructure:"vpa"` - PDBRule RuleConfig `mapstructure:"pdb"` - IngressRule RuleConfig `mapstructure:"ingress"` - HTTPRouteRule RuleConfig `mapstructure:"httproute"` - PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` - GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` - KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` - ServicePortRule RuleConfig `mapstructure:"service-port"` - ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` - RegistryRule RuleConfig `mapstructure:"registry"` - EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` - CRDEnabledModulesRule RuleConfig `mapstructure:"crd-enabled-modules"` - WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` - MountPointsRule RuleConfig `mapstructure:"mount-points"` - HelmRenderRule RuleConfig `mapstructure:"helm-render"` - OpenAPIValuesQuoteRule RuleConfig `mapstructure:"openapi-values-quote"` - SchemaValidationRule RuleConfig `mapstructure:"schema-validation"` + VPARule RuleConfig `mapstructure:"vpa"` + PDBRule RuleConfig `mapstructure:"pdb"` + IngressRule RuleConfig `mapstructure:"ingress"` + HTTPRouteRule RuleConfig `mapstructure:"httproute"` + PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` + GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` + KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` + ServicePortRule RuleConfig `mapstructure:"service-port"` + ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` + RegistryRule RuleConfig `mapstructure:"registry"` + EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + CRDEnabledModulesRule RuleConfig `mapstructure:"crd-enabled-modules"` + WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` + MountPointsRule RuleConfig `mapstructure:"mount-points"` + HelmRenderRule RuleConfig `mapstructure:"helm-render"` + OpenAPIValuesQuoteRule RuleConfig `mapstructure:"openapi-values-quote"` + SchemaValidationRule RuleConfig `mapstructure:"schema-validation"` + DeprecatedHTTPRouteAnnotationsRule RuleConfig `mapstructure:"deprecated-httproute-annotations"` + IngressEnablementRule RuleConfig `mapstructure:"ingress-enablement"` + GatewayEnablementRule RuleConfig `mapstructure:"gateway-enablement"` + HTTPSCertificateReuseRule RuleConfig `mapstructure:"https-certificate-reuse"` } func (c LinterConfig) IsWarn() bool { diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index db57c480..a681f021 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -234,37 +234,45 @@ type TemplatesSettings struct { } type TemplatesLinterRules struct { - VPARule RuleConfig `mapstructure:"vpa"` - PDBRule RuleConfig `mapstructure:"pdb"` - IngressRule RuleConfig `mapstructure:"ingress"` - HTTPRouteRule RuleConfig `mapstructure:"httproute"` - PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` - GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` - KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` - ServicePortRule RuleConfig `mapstructure:"service-port"` - ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` - RegistryRule RuleConfig `mapstructure:"registry"` - EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` - CRDEnabledModulesRule RuleConfig `mapstructure:"crd-enabled-modules"` - WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` - MountPointsRule RuleConfig `mapstructure:"mount-points"` - HelmRenderRule RuleConfig `mapstructure:"helm-render"` - OpenAPIValuesQuoteRule RuleConfig `mapstructure:"openapi-values-quote"` - SchemaValidationRule RuleConfig `mapstructure:"schema-validation"` + VPARule RuleConfig `mapstructure:"vpa"` + PDBRule RuleConfig `mapstructure:"pdb"` + IngressRule RuleConfig `mapstructure:"ingress"` + HTTPRouteRule RuleConfig `mapstructure:"httproute"` + PrometheusRule RuleConfig `mapstructure:"prometheus-rules"` + GrafanaRule RuleConfig `mapstructure:"grafana-dashboards"` + KubeRBACProxyRule RuleConfig `mapstructure:"kube-rbac-proxy"` + ServicePortRule RuleConfig `mapstructure:"service-port"` + ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` + RegistryRule RuleConfig `mapstructure:"registry"` + EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + CRDEnabledModulesRule RuleConfig `mapstructure:"crd-enabled-modules"` + WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` + MountPointsRule RuleConfig `mapstructure:"mount-points"` + HelmRenderRule RuleConfig `mapstructure:"helm-render"` + OpenAPIValuesQuoteRule RuleConfig `mapstructure:"openapi-values-quote"` + SchemaValidationRule RuleConfig `mapstructure:"schema-validation"` + DeprecatedHTTPRouteAnnotationsRule RuleConfig `mapstructure:"deprecated-httproute-annotations"` + IngressEnablementRule RuleConfig `mapstructure:"ingress-enablement"` + GatewayEnablementRule RuleConfig `mapstructure:"gateway-enablement"` + HTTPSCertificateReuseRule RuleConfig `mapstructure:"https-certificate-reuse"` } type TemplatesExcludeRules struct { - VPAAbsent KindRuleExcludeList `mapstructure:"vpa"` - PDBAbsent KindRuleExcludeList `mapstructure:"pdb"` - ServicePort ServicePortExcludeList `mapstructure:"service-port"` - KubeRBACProxy StringRuleExcludeList `mapstructure:"kube-rbac-proxy"` - Ingress KindRuleExcludeList `mapstructure:"ingress"` - HTTPRoute KindRuleExcludeList `mapstructure:"httproute"` - EnabledModules EnabledModulesExcludeRule `mapstructure:"enabled-modules"` - WebhookConfiguration KindRuleExcludeList `mapstructure:"webhook-configuration-annotations"` - MountPoints StringRuleExcludeList `mapstructure:"mount-points"` - OpenAPIValuesQuote StringRuleExcludeList `mapstructure:"openapi-values-quote"` - SchemaValidation KindRuleExcludeList `mapstructure:"schema-validation"` + VPAAbsent KindRuleExcludeList `mapstructure:"vpa"` + PDBAbsent KindRuleExcludeList `mapstructure:"pdb"` + ServicePort ServicePortExcludeList `mapstructure:"service-port"` + KubeRBACProxy StringRuleExcludeList `mapstructure:"kube-rbac-proxy"` + Ingress KindRuleExcludeList `mapstructure:"ingress"` + HTTPRoute KindRuleExcludeList `mapstructure:"httproute"` + EnabledModules EnabledModulesExcludeRule `mapstructure:"enabled-modules"` + WebhookConfiguration KindRuleExcludeList `mapstructure:"webhook-configuration-annotations"` + MountPoints StringRuleExcludeList `mapstructure:"mount-points"` + OpenAPIValuesQuote StringRuleExcludeList `mapstructure:"openapi-values-quote"` + SchemaValidation KindRuleExcludeList `mapstructure:"schema-validation"` + DeprecatedHTTPRouteAnnotations PathRuleExclude `mapstructure:"deprecated-httproute-annotations"` + IngressEnablement PathRuleExclude `mapstructure:"ingress-enablement"` + GatewayEnablement PathRuleExclude `mapstructure:"gateway-enablement"` + HTTPSCertificateReuse PathRuleExclude `mapstructure:"https-certificate-reuse"` } type EnabledModulesExcludeRule struct { @@ -272,6 +280,13 @@ type EnabledModulesExcludeRule struct { Directories DirectoryRuleExcludeList `mapstructure:"directories"` } +// PathRuleExclude excludes specific files and whole directories (both relative +// to the module root) from a rule that scans template source files. +type PathRuleExclude struct { + Files StringRuleExcludeList `mapstructure:"files"` + Directories DirectoryRuleExcludeList `mapstructure:"directories"` +} + type GrafanaDashboardsExcludeList struct { Disable bool `mapstructure:"disable"` } diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index e5f53049..20a5bc5d 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -27,6 +27,10 @@ Proper template validation prevents runtime issues, ensures applications are pro | [mount-points](#mount-points) | Validates that mount-points.yaml directories are used as volumeMounts in pod controllers | ✅ | enabled | | [openapi-values-quote](#openapi-values-quote) | Requires templates to quote OpenAPI string values that have no `pattern`/`enum`/`format` | ✅ | enabled | | [schema-validation](#schema-validation) | Strictly decodes every rendered standard Kubernetes resource against its API type | ✅ | enabled | +| [deprecated-httproute-annotations](#deprecated-httproute-annotations) | Flags deprecated annotation keys (e.g. `alb.network.deckhouse.io/response-headers-to-add`) | ✅ | enabled | +| [ingress-enablement](#ingress-enablement) | Requires Ingress creation to be gated by `helm_lib_module_ingress_enabled` | ✅ | enabled | +| [gateway-enablement](#gateway-enablement) | Requires HTTPRoute/ListenerSet creation to be gated by `helm_lib_module_gateway_enabled` | ✅ | enabled | +| [https-certificate-reuse](#https-certificate-reuse) | Requires a custom certificate to be copied once and reused by Ingress and Gateway API via `helm_lib_module_https_secret_name`'s plain and two-prefix forms | ✅ | enabled | "Configurable" means that this rule can be configured using the `.dmtlint.yaml` file, including customizing the rule's parameters and/or disabling the rule. @@ -2936,3 +2940,501 @@ linters-settings: Whichever `k8s.io/api` is in `go.mod`. Bumping that dependency is the whole of updating this rule — there is nothing else to regenerate. + +--- + +### deprecated-httproute-annotations + +**Purpose:** Flags annotation keys used to work around a missing native +HTTPRoute setting — an ALB-specific annotation standing in for a field +Gateway API's HTTPRoute now exposes directly — so authors migrate to the +native field instead of copying the annotation-based workaround into new +templates. + +**Description:** + +Scans all template files (`.yaml`, `.yml`, `.tpl`) for a small built-in list of +banned annotation keys and reports every occurrence, together with a concrete +workaround snippet for the replacement. Today the list has one entry: +`alb.network.deckhouse.io/response-headers-to-add`, deprecated in favor of the +native Gateway API `ResponseHeaderModifier` HTTPRoute filter. + +**What it checks:** + +1. Only runs when the module actually renders an `Ingress`, `HTTPRoute`, or + `ListenerSet` — every banned annotation is specific to those resources, so a + module with none of them is skipped entirely +2. All files in the `templates/` directory +3. Presence of a banned annotation key, as plain text — it does not matter + whether the key appears as a YAML annotation or inside a Helm expression + +**Why it matters:** + +`alb.network.deckhouse.io/response-headers-to-add` predates Gateway API's own +`ResponseHeaderModifier` filter and applies to every rule of the HTTPRoute +object uniformly (there is no way to target one rule). The native filter is +per-rule, standards-based, and portable to any Gateway API implementation — +the annotation is a legacy-only escape hatch. + +**Examples:** + +❌ **Incorrect** - Setting the deprecated annotation: + +```yaml +# templates/httproute.yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard + annotations: + alb.network.deckhouse.io/response-headers-to-add: '{"Strict-Transport-Security":"max-age=31536000; includeSubDomains"}' +spec: + rules: + - backendRefs: + - name: dashboard + port: 443 +``` + +**Error:** +``` +Error: Annotation "alb.network.deckhouse.io/response-headers-to-add" must not be used: deprecated in favor of the native Gateway API HTTPRoute ResponseHeaderModifier filter. Add this filter to the relevant HTTPRoute rule instead: + rules: + - backendRefs: [...] + matches: [...] + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + add: + - name: Strict-Transport-Security + value: max-age=31536000; includeSubDomains +``` + +✅ **Correct** - Using the native HTTPRoute filter: + +```yaml +# templates/httproute.yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + rules: + - backendRefs: + - name: dashboard + port: 443 + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + add: + - name: Strict-Transport-Security + value: max-age=31536000; includeSubDomains +``` + +**Configuration:** + +The rule supports excluding specific files and directories (paths are relative +to the module root): + +```yaml +# .dmtlint.yaml +linters-settings: + templates: + exclude-rules: + deprecated-httproute-annotations: + files: + - templates/legacy-ingress.yaml + directories: + - templates/vendor/ +``` + +--- + +### ingress-enablement + +**Purpose:** Ensures an Ingress's creation can be turned off the same way every +other module's can: via `global.modules.ingress.enabled` or the module's own +`.ingress.enabled` override. + +**Description:** + +Scans every template file that emits a `kind: Ingress` manifest and reports the +ones that never reference `helm_lib_module_ingress_enabled` — the shared +`helm_lib` helper that checks the module override first, then the global +setting, defaulting to enabled when neither is set — anywhere in the same +file. + +**What it checks:** + +1. Only runs when the module actually renders an `Ingress`: a module with none + has nothing for this check to say +2. Every file in `templates/` whose rendered output would contain + `kind: Ingress` +3. That the same file also references `helm_lib_module_ingress_enabled` + +**This is a same-file, textual heuristic, not a template-scope analysis.** It +does not verify that the helper actually gates the specific manifest it +found — only that both the `kind: Ingress` line and the helper name appear +somewhere in the same file. In every module observed so far the guard and the +manifest it protects live in the same file (`{{- if eq (include +"helm_lib_module_ingress_enabled" .) "true" }}` wrapping the whole +document), so this catches the case that actually matters — an Ingress with no +enablement check at all — without needing a real Helm control-flow parser. + +**Why it matters:** + +An Ingress that never checks the shared helper renders unconditionally: it +cannot be disabled by an operator who sets `ingress.enabled: false` at either +the global or the module level, and every module is expected to honor that +knob the same way. + +**Examples:** + +❌ **Incorrect** - Ingress with no enablement check: + +```yaml +# templates/ingress.yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard + namespace: d8-my-module +spec: + rules: + - host: dashboard.example.com +``` + +**Error:** +``` +Error: File creates a Ingress object but never checks "helm_lib_module_ingress_enabled", so its creation cannot be controlled via global.modules.ingress.enabled or myModule.ingress.enabled. Guard the manifest with {{- if eq (include "helm_lib_module_ingress_enabled" .) "true" }} ... {{- end }} (requires lib_helm v1.72.21+) so it can be disabled the same way every other module's Ingress does. +``` + +The exact `.Values` path named in the finding is computed from the module's own +name (`myModule` above is `my-module` converted to camelCase), so it always +matches what that module's own values.yaml actually calls it. + +✅ **Correct** - Guarded by the shared helper: + +```yaml +# templates/ingress.yaml +{{- if eq (include "helm_lib_module_ingress_enabled" .) "true" }} +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard + namespace: d8-my-module +spec: + rules: + - host: dashboard.example.com +{{- end }} +``` + +**Configuration:** + +```yaml +# .dmtlint.yaml +linters-settings: + templates: + exclude-rules: + ingress-enablement: + files: + - templates/ingress.yaml # module has a documented reason to skip the helper + directories: + - templates/vendor/ +``` + +--- + +### gateway-enablement + +**Purpose:** The Gateway API counterpart of [ingress-enablement](#ingress-enablement): +ensures HTTPRoute and ListenerSet creation can be turned off via +`global.modules.gatewayAPI.enabled` or the module's own +`.gatewayAPI.enabled` override. + +**Description:** + +Scans every template file that emits a `kind: HTTPRoute` or `kind: +ListenerSet` manifest and reports the ones that never reference +`helm_lib_module_gateway_enabled` — the shared helper that requires both an +enabled flag and a resolvable Gateway (module, then global, then +`global.discovery.gatewayAPIDefaultGateway`) — anywhere in the same file. + +**What it checks:** + +1. Only runs when the module actually renders an `HTTPRoute` or `ListenerSet`: + a module with neither has nothing for this check to say +2. Every file in `templates/` whose rendered output would contain + `kind: HTTPRoute` or `kind: ListenerSet` +3. That the same file also references `helm_lib_module_gateway_enabled` + +Same same-file heuristic and trade-off as `ingress-enablement` — see that +rule's description for the reasoning. + +**Why it matters:** + +Unlike Ingress, Gateway API has no safe default: a module cannot assume a +Gateway exists the way it can assume an `nginx` IngressClass exists. A +HTTPRoute/ListenerSet pair that skips `helm_lib_module_gateway_enabled` will +either render with no usable parent Gateway, or fail to respect an operator's +explicit `gatewayAPI.enabled: false`. + +**Examples:** + +❌ **Incorrect** - HTTPRoute with no enablement check: + +```yaml +# templates/httproute.yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard + namespace: d8-my-module +spec: + hostnames: + - dashboard.example.com +``` + +**Error:** +``` +Error: File creates a Gateway API (HTTPRoute/ListenerSet) object but never checks "helm_lib_module_gateway_enabled", so its creation cannot be controlled via global.modules.gatewayAPI.enabled or myModule.gatewayAPI.enabled, with a Gateway resolvable via global.discovery.gatewayAPIDefaultGateway, global.modules.gatewayAPI.gateway, or myModule.gatewayAPI.gateway. Guard the manifest with {{- if eq (include "helm_lib_module_gateway_enabled" .) "true" }} ... {{- end }} (requires lib_helm v1.72.21+) so it can be disabled the same way every other module's Gateway API (HTTPRoute/ListenerSet) does. +``` + +As with `ingress-enablement`, the `.Values` paths named in the finding are +computed from the module's own name. + +✅ **Correct** - Guarded by the shared helper: + +```yaml +# templates/httproute.yaml +{{- $moduleGateway := dict }} +{{- include "helm_lib_module_gateway" (list . $moduleGateway) }} +{{- if and (eq (include "helm_lib_module_gateway_enabled" .) "true") .Values.global.modules.publicDomainTemplate }} +apiVersion: gateway.networking.k8s.io/v1 +kind: ListenerSet +metadata: + name: dashboard + namespace: d8-my-module +spec: + parentRef: + name: {{ $moduleGateway.name }} + namespace: {{ $moduleGateway.namespace }} + listeners: + - name: http + protocol: HTTP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard + namespace: d8-my-module +spec: + hostnames: + - dashboard.example.com +{{- end }} +``` + +**Configuration:** + +```yaml +# .dmtlint.yaml +linters-settings: + templates: + exclude-rules: + gateway-enablement: + files: + - templates/multicluster/api-proxy/httproute.yaml + directories: + - templates/vendor/ +``` + +--- + +### https-certificate-reuse + +> **Warning:** Requires modules to vendor `lib_helm` (`deckhouse_lib_helm`) +> **v1.72.21 or newer** — the two-prefix form of `helm_lib_module_https_secret_name` +> was only added in that release. + +**Purpose:** Ensures a module that serves the same certificate over both +Ingress and Gateway API copies its `CustomCertificate`-mode certificate +exactly once, and that both flows reuse that one copy via +`helm_lib_module_https_secret_name` — Ingress with the plain, one-prefix +form, HTTPRoute/ListenerSet with the two-prefix form, linking back to the +exact same base prefix. + +**Description:** + +`helm_lib_module_https_secret_name` has an optional third argument: +`{{ include "helm_lib_module_https_secret_name" (list . "base-prefix" "gateway-prefix") }}`. +In `CertManager` mode it resolves to `gateway-prefix`'s own secret — Gateway +API needs its own `cert-manager` `Certificate`, validated through a separate +`ClusterIssuer`. In `CustomCertificate` mode it ignores the override and +resolves to `base-prefix`'s secret instead, since custom certificate data is +the same regardless of which resource consumes it. That is the entire point +of the two-prefix form: a module using it never needs a second +`CustomCertificate` copy for its Gateway API flow. + +This rule scans every template file for `helm_lib_module_https_copy_custom_certificate` +calls (which actually create a `CustomCertificate`-mode `Secret`) and for +`helm_lib_module_https_secret_name` calls, classifying each +`helm_lib_module_https_secret_name` call by the kind(s) declared in its own +YAML document — a file is split on `---` separators first, since one file +commonly bundles a `HTTPRoute`/`ListenerSet` alongside the cert-manager +`Certificate` that feeds it. A call inside a `kind: Certificate` document is +excluded entirely: a `Certificate`'s own `secretName` always uses the plain +form to declare a new target secret for cert-manager to populate, which +isn't a manifest "reusing" a shared secret. Otherwise a call counts as an +`Ingress` or Gateway API reference if its own document declares `kind: +Ingress` or `kind: HTTPRoute`/`kind: ListenerSet` respectively. It reports, +deduplicated by location: + +1. A secret prefix copied by more than one + `helm_lib_module_https_copy_custom_certificate` call +2. An `Ingress`-file reference using the two-prefix form — Ingress never + needs a Gateway-API-specific override +3. A `HTTPRoute`/`ListenerSet`-file reference using the plain one-prefix + form — without the override argument, that manifest names its own, + independent secret instead of reusing the Ingress flow's +4. A two-prefix form's override prefix that is *also* independently copied +5. Two copied prefixes follow the `-ingress-tls` / `-httproute-tls` + naming convention for the same stem (e.g. `istio-ingress-tls` and + `istio-httproute-tls`) — unlike checks 1-4, this one fires purely from the + `helm_lib_module_https_copy_custom_certificate` calls themselves, so it + also catches a module that copies both variants but hasn't wired up (or + has wired up incorrectly) either flow's reference to + `helm_lib_module_https_secret_name` at all. It runs last and only adds a + finding at a location none of checks 1-4 already reported. + +**What it checks:** + +1. Only runs when the module renders both an `Ingress` and a + `HTTPRoute`/`ListenerSet`: reuse across flows is only possible when both + exist +2. Every file in `templates/` for calls to + `helm_lib_module_https_copy_custom_certificate` and + `helm_lib_module_https_secret_name` +3. Whether the five conditions above hold, as described + +**This is a textual, whole-module heuristic**, not a value-flow analysis: it +only recognizes prefixes passed as string literals, and classifies a +`helm_lib_module_https_secret_name` call by the resource kind(s) declared in +its own `---`-delimited document (not by full YAML parsing, so a document +with more than one `kind:` line, however unusual, is classified by whichever +kinds match). A module that legitimately copies more than one certificate for +unrelated services — each with its own prefix, referenced consistently by +that service's own Ingress and Gateway API manifests — is not flagged by +checks 1-4, since those are scoped to whether a given prefix's *own* +consumers use it correctly, not to how many distinct prefixes exist in the +module. The rule deliberately does not check that a `HTTPRoute`/`ListenerSet` +link's base prefix is also referenced by some `Ingress` file in the module: +that produced false positives when a module's Ingress and Gateway API +manifests for the same certificate live in separate directories the rule +doesn't otherwise correlate. Check 5 is a narrower exception: it relies on +the `-ingress-tls` / `-httproute-tls` naming convention rather +than any reference, so a module that names two genuinely unrelated +certificates with that same convention (e.g. `auth-ingress-tls` and +`auth-httproute-tls` for two unrelated purposes) would be a false positive in +principle — in practice this convention is used consistently for the paired +case throughout the module ecosystem. + +**Why it matters:** + +A module that copies the same custom certificate data under two prefixes +ships an extra `Secret` that serves no purpose: the Gateway API flow already +gets the base prefix's certificate in `CustomCertificate` mode through the +two-prefix form, and the override prefix only matters in `CertManager` mode, +where it names a `cert-manager` `Certificate`'s target secret — not a +`CustomCertificate` copy. A Gateway API manifest that uses the plain form +instead of linking back to the Ingress flow's secret gets its own +independent secret in every mode, including `CustomCertificate`, which is +either a second wasteful copy or — if no matching copy exists at all under +that name — a broken reference. + +**Examples:** + +❌ **Incorrect** - copying the certificate once per flow, and referencing +the Gateway-API-only copy with the plain form instead of linking back to the +Ingress flow's secret: + +```yaml +# templates/custom-certificate.yaml +{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-my-module" "my-module-ingress-tls") }} +{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-my-module" "my-module-httproute-tls") }} +``` + +```yaml +# templates/httproute.yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + tls: + secretRef: + name: {{ include "helm_lib_module_https_secret_name" (list . "my-module-httproute-tls") }} +``` + +**Error:** +``` +Error: HTTPRoute/ListenerSet references its TLS secret with the plain form of helm_lib_module_https_secret_name (list . "my-module-httproute-tls"). It is recommended to use the extended form instead (list . "" "my-module-httproute-tls"), which allows reusing the custom certificate secret in case CustomCertificate mode is enabled. +``` + +Copying the override prefix as well (rather than fixing the reference) is +flagged too, once the reference is corrected to link back to it: + +``` +Error: File copies a custom certificate under secret prefix "my-module-httproute-tls", but "my-module-httproute-tls" is only meant to be the Gateway-API-specific override in the two-prefix form of helm_lib_module_https_secret_name (list . "my-module-ingress-tls" "my-module-httproute-tls"), found in templates/httproute.yaml:8. Under CustomCertificate mode both the Ingress and Gateway API flows already resolve to "my-module-ingress-tls"'s copy, so copying one under "my-module-httproute-tls" too is a duplicate — remove this helm_lib_module_https_copy_custom_certificate call and let the two-prefix form share "my-module-ingress-tls"'s certificate. +``` + +❌ **Incorrect** (check 5) - both copies exist, but neither flow's manifest +references `helm_lib_module_https_secret_name` at all yet (or does so +incorrectly elsewhere) — this is caught from the copy calls alone: + +```yaml +# templates/custom-certificate.yaml +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "istio-ingress-tls") }} +{{ $moduleGateway := dict }} +{{ include "helm_lib_module_gateway" (list . $moduleGateway) }} +{{ if $moduleGateway }} +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "istio-httproute-tls") }} +{{ end }} +``` + +**Error:** +``` +Error: File copies a custom certificate under secret prefix "istio-httproute-tls", which by naming convention is the Gateway API/HTTPRoute variant of "istio-ingress-tls" — also copied via helm_lib_module_https_copy_custom_certificate, in templates/custom-certificate.yaml:1. Copy the certificate once, under "istio-ingress-tls", and reference it from the Gateway API flow with the two-prefix form of helm_lib_module_https_secret_name (list . "istio-ingress-tls" "istio-httproute-tls") instead of copying it separately. +``` + +✅ **Correct** - copying the certificate once and sharing it: + +```yaml +# templates/custom-certificate.yaml +{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-my-module" "my-module-ingress-tls") }} +``` + +```yaml +# templates/httproute.yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + tls: + secretRef: + name: {{ include "helm_lib_module_https_secret_name" (list . "my-module-ingress-tls" "my-module-httproute-tls") }} +``` + +**Configuration:** + +```yaml +# .dmtlint.yaml +linters-settings: + templates: + exclude-rules: + https-certificate-reuse: + files: + - templates/legacy-certificate.yaml + directories: + - templates/vendor/ +``` diff --git a/pkg/linters/templates/rules/deprecated_httproute_annotations.go b/pkg/linters/templates/rules/deprecated_httproute_annotations.go new file mode 100644 index 00000000..5d47221f --- /dev/null +++ b/pkg/linters/templates/rules/deprecated_httproute_annotations.go @@ -0,0 +1,141 @@ +/* +Copyright 2026 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 ( + "context" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + DeprecatedHTTPRouteAnnotationsRuleName = "deprecated-httproute-annotations" +) + +// deprecatedAnnotation is one annotation key that must no longer appear in +// module templates, along with the reason it was banned and a workaround +// snippet demonstrating the replacement, both surfaced in the finding. +type deprecatedAnnotation struct { + Key string + Reason string + Workaround string +} + +// deprecatedAnnotations is the list of annotations this rule flags. Add an entry +// here to ban another annotation; the scan and reporting are shared. +var deprecatedAnnotations = []deprecatedAnnotation{ + { + Key: "alb.network.deckhouse.io/response-headers-to-add", + Reason: "deprecated in favor of the native Gateway API HTTPRoute ResponseHeaderModifier filter", + Workaround: ` rules: + - backendRefs: [...] + matches: [...] + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + add: + - name: Strict-Transport-Security + value: max-age=31536000; includeSubDomains`, + }, +} + +type DeprecatedHTTPRouteAnnotationsRule struct { + pkg.RuleMeta + pkg.PathRule + + module pkg.Module + errorList *errors.LintRuleErrorsList +} + +func NewDeprecatedHTTPRouteAnnotationsRule(excludeFileRules []pkg.StringRuleExclude, + excludeDirectoryRules []pkg.DirectoryRuleExclude, + m pkg.Module, errorList *errors.LintRuleErrorsList) *DeprecatedHTTPRouteAnnotationsRule { + return &DeprecatedHTTPRouteAnnotationsRule{ + RuleMeta: pkg.RuleMeta{ + Name: DeprecatedHTTPRouteAnnotationsRuleName, + }, + PathRule: pkg.PathRule{ + ExcludeStringRules: excludeFileRules, + ExcludeDirectoryRules: excludeDirectoryRules, + }, + module: m, + errorList: errorList.WithRule(DeprecatedHTTPRouteAnnotationsRuleName), + } +} + +var _ pkg.Rule = (*DeprecatedHTTPRouteAnnotationsRule)(nil) + +// Check scans every template file for the annotation keys in deprecatedAnnotations +// and reports each occurrence, regardless of whether the key appears as a plain +// YAML annotation or inside a Helm expression — the key text itself is what must +// no longer be used. +// +// The rule only runs when the module actually renders an Ingress, HTTPRoute, or +// ListenerSet: every entry in deprecatedAnnotations is specific to those +// resources, so a module with none of them has nothing for this check to say. +func (r *DeprecatedHTTPRouteAnnotationsRule) Check(_ context.Context) { + m := r.module + + if !storageHasKind(m, "Ingress", "HTTPRoute", "ListenerSet") { + return + } + + templatesPath := filepath.Join(m.GetPath(), "templates") + if _, err := os.Stat(templatesPath); os.IsNotExist(err) { + return + } + + files := fsutils.GetFiles(templatesPath, true, fsutils.FilterFileByExtensions(".yaml", ".yml", ".tpl")) + + for _, filePath := range files { + relPath := fsutils.Rel(m.GetPath(), filePath) + + if !r.Enabled(relPath) { + continue + } + + content, err := os.ReadFile(filePath) + if err != nil { + r.errorList.WithFilePath(relPath).Errorf("Failed to read file: %v", err) + continue + } + + r.checkContent(relPath, content) + } +} + +func (r *DeprecatedHTTPRouteAnnotationsRule) checkContent(relPath string, content []byte) { + for _, annotation := range deprecatedAnnotations { + re := regexp.MustCompile(regexp.QuoteMeta(annotation.Key)) + + for _, loc := range re.FindAllIndex(content, -1) { + line := strings.Count(string(content[:loc[0]]), "\n") + 1 + + r.errorList.WithFilePath(relPath). + WithLineNumber(line). + WithValue(annotation.Key). + Errorf("Annotation %q must not be used: %s. Add this filter to the relevant HTTPRoute rule instead:\n%s", + annotation.Key, annotation.Reason, annotation.Workaround) + } + } +} diff --git a/pkg/linters/templates/rules/deprecated_httproute_annotations_test.go b/pkg/linters/templates/rules/deprecated_httproute_annotations_test.go new file mode 100644 index 00000000..0713655f --- /dev/null +++ b/pkg/linters/templates/rules/deprecated_httproute_annotations_test.go @@ -0,0 +1,218 @@ +/* +Copyright 2026 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" + "testing" + + "github.com/gojuno/minimock/v3" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/deckhouse/dmt/internal/mocks" + "github.com/deckhouse/dmt/internal/storage" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +// writeTemplatesModule builds a temporary module directory containing the given +// template files (keyed by path relative to the module root) and returns the +// module path. +func writeTemplatesModule(t *testing.T, templateFiles map[string]string) string { + t.Helper() + + modulePath := filepath.Join(t.TempDir(), "module") + require.NoError(t, os.MkdirAll(modulePath, 0o755)) + + for relPath, content := range templateFiles { + fullPath := filepath.Join(modulePath, relPath) + require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + require.NoError(t, os.WriteFile(fullPath, []byte(content), 0o600)) + } + + return modulePath +} + +// kindOnlyStorage builds a minimal rendered-object store containing one bare +// object per kind given — enough for storageHasKind to see them, which is all +// these rules read from GetStorage(). +func kindOnlyStorage(kinds ...string) map[storage.ResourceIndex]storage.StoreObject { + out := make(map[storage.ResourceIndex]storage.StoreObject, len(kinds)) + + for i, kind := range kinds { + u := unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "v1", + "kind": kind, + "metadata": map[string]any{"name": fmt.Sprintf("obj-%d", i)}, + }} + + idx := storage.ResourceIndex{Kind: u.GetKind(), Name: u.GetName(), Namespace: u.GetNamespace()} + out[idx] = storage.StoreObject{Unstructured: u} + } + + return out +} + +// templatesMockModule builds a Module mock rooted at modulePath whose rendered +// storage contains one bare object per kind in storageKinds. +func templatesMockModule(t *testing.T, modulePath string, storageKinds ...string) *mocks.ModuleMock { + t.Helper() + + m := mocks.NewModuleMock(minimock.NewController(t)) + // Optional: the storage gate in each rule's Check may return before GetPath + // or GetName is ever called (see the "does not run at all" test cases). + m.GetPathMock.Optional().Return(modulePath) + m.GetNameMock.Optional().Return("my-module") + m.GetStorageMock.Return(kindOnlyStorage(storageKinds...)) + + return m +} + +func TestDeprecatedHTTPRouteAnnotationsRule_Check(t *testing.T) { + const httprouteWithAnnotation = `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: x + annotations: + alb.network.deckhouse.io/response-headers-to-add: '{"Strict-Transport-Security":"max-age=31536000"}' +` + + tests := []struct { + name string + templateFiles map[string]string + storageKinds []string + exclude []pkg.StringRuleExclude + wantCount int + wantContains []string + wantLines []int + }{ + { + name: "flags the deprecated response-headers-to-add annotation", + templateFiles: map[string]string{ + "templates/httproute.yaml": httprouteWithAnnotation, + }, + storageKinds: []string{"HTTPRoute"}, + wantCount: 1, + wantContains: []string{ + `alb.network.deckhouse.io/response-headers-to-add`, + "ResponseHeaderModifier", + "Strict-Transport-Security", + "responseHeaderModifier", + }, + wantLines: []int{6}, + }, + { + name: "flags multiple occurrences across files", + templateFiles: map[string]string{ + "templates/a.yaml": `metadata: + annotations: + alb.network.deckhouse.io/response-headers-to-add: '{}' +`, + "templates/b.yaml": `metadata: + annotations: + alb.network.deckhouse.io/response-headers-to-add: '{}' +`, + }, + storageKinds: []string{"HTTPRoute"}, + wantCount: 2, + }, + { + name: "ignores files that never use the annotation", + templateFiles: map[string]string{ + "templates/httproute.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: x + annotations: + alb.network.deckhouse.io/backend-tls-settings: '{"mode": "SIMPLE"}' +`, + }, + storageKinds: []string{"HTTPRoute"}, + wantCount: 0, + }, + { + name: "does not run at all when the module ships no Ingress/HTTPRoute/ListenerSet", + templateFiles: map[string]string{ + "templates/httproute.yaml": httprouteWithAnnotation, + }, + storageKinds: nil, // e.g. a module whose only resources are a Deployment and a Service + wantCount: 0, + }, + { + name: "an excluded file is skipped even though the module ships HTTPRoute", + templateFiles: map[string]string{ + "templates/httproute.yaml": httprouteWithAnnotation, + }, + storageKinds: []string{"HTTPRoute"}, + exclude: []pkg.StringRuleExclude{"templates/httproute.yaml"}, + wantCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modulePath := writeTemplatesModule(t, tt.templateFiles) + + errorList := errors.NewLintRuleErrorsList() + NewDeprecatedHTTPRouteAnnotationsRule(tt.exclude, nil, templatesMockModule(t, modulePath, tt.storageKinds...), errorList).Check(t.Context()) + + errs := errorList.GetErrors() + require.Len(t, errs, tt.wantCount) + + for _, want := range tt.wantContains { + found := false + + for i := range errs { + if containsStr(errs[i].Text, want) { + found = true + break + } + } + + require.Truef(t, found, "expected a finding containing %q, got %+v", want, errs) + } + + for i, wantLine := range tt.wantLines { + if i < len(errs) { + require.Equalf(t, wantLine, errs[i].LineNumber, "unexpected line for finding %d: %s", i, errs[i].Text) + } + } + }) + } +} + +func TestDeprecatedHTTPRouteAnnotationsRule_DirectoryExclusion(t *testing.T) { + modulePath := writeTemplatesModule(t, map[string]string{ + "templates/vendor/httproute.yaml": `metadata: + annotations: + alb.network.deckhouse.io/response-headers-to-add: '{}' +`, + }) + + errorList := errors.NewLintRuleErrorsList() + NewDeprecatedHTTPRouteAnnotationsRule( + nil, + []pkg.DirectoryRuleExclude{"templates/vendor/"}, + templatesMockModule(t, modulePath, "HTTPRoute"), + errorList, + ).Check(t.Context()) + + require.Empty(t, errorList.GetErrors()) +} diff --git a/pkg/linters/templates/rules/enablement_helpers.go b/pkg/linters/templates/rules/enablement_helpers.go new file mode 100644 index 00000000..9d30d9a4 --- /dev/null +++ b/pkg/linters/templates/rules/enablement_helpers.go @@ -0,0 +1,126 @@ +/* +Copyright 2026 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 ( + "bytes" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +// kindLineRe returns a regexp matching a manifest's `kind:` field on its own +// line for any of the given kinds — the same shape a rendered Kubernetes YAML +// document uses, regardless of the Helm expressions around it. It is used to +// test "does this template file ever emit an object of this kind" without +// rendering the chart. +func kindLineRe(kinds ...string) *regexp.Regexp { + escaped := make([]string, len(kinds)) + for i, k := range kinds { + escaped[i] = regexp.QuoteMeta(k) + } + + return regexp.MustCompile(`(?m)^kind:\s*(` + strings.Join(escaped, "|") + `)\s*$`) +} + +// storageHasKind reports whether module's rendered objects include at least one +// of the given kinds. It gates the enablement/annotation rules so they only run +// on modules that actually ship the kind of resource they check — a module with +// no Ingress has nothing for ingress-enablement to say, and likewise for Gateway +// API and HTTPRoute/ListenerSet. +func storageHasKind(m pkg.Module, kinds ...string) bool { + for _, object := range m.GetStorage() { + kind := object.Unstructured.GetKind() + + for _, k := range kinds { + if kind == k { + return true + } + } + } + + return false +} + +// checkKindGatedByHelper scans every template file of module for kindRe and +// reports each file that matches it but never mentions helperName anywhere in +// the same file. kindLabel names the resource kind(s) in the finding text, +// and valuesHint names the concrete `.Values` path(s) an author would set to +// control it, so the finding says exactly what to change, not just which +// helper to call. +// +// This is a textual, same-file heuristic: it does not verify that helperName +// actually gates the specific manifest kindRe matched, only that both appear +// somewhere in the same file. See IngressEnablementRule.Check for why that +// trade-off was chosen over a full Helm-template control-flow parser. +func checkKindGatedByHelper( + m pkg.Module, + errorList *errors.LintRuleErrorsList, + pathRule pkg.PathRule, + kindRe *regexp.Regexp, + helperName string, + kindLabel string, + valuesHint string, +) { + templatesPath := filepath.Join(m.GetPath(), "templates") + if _, err := os.Stat(templatesPath); os.IsNotExist(err) { + return + } + + files := fsutils.GetFiles(templatesPath, true, fsutils.FilterFileByExtensions(".yaml", ".yml", ".tpl")) + helperBytes := []byte(helperName) + + for _, filePath := range files { + relPath := fsutils.Rel(m.GetPath(), filePath) + + if !pathRule.Enabled(relPath) { + continue + } + + content, err := os.ReadFile(filePath) + if err != nil { + errorList.WithFilePath(relPath).Errorf("Failed to read file: %v", err) + continue + } + + loc := kindRe.FindIndex(content) + if loc == nil { + continue + } + + if bytes.Contains(content, helperBytes) { + continue + } + + line := bytes.Count(content[:loc[0]], []byte("\n")) + 1 + + errorList.WithFilePath(relPath). + WithLineNumber(line). + Errorf( + "File creates a %s object but never checks %q, so its creation cannot be "+ + "controlled via %s. Guard the manifest with "+ + "{{- if eq (include %q .) \"true\" }} ... {{- end }} (requires lib_helm "+ + "v1.72.21+) so it can be disabled the same way every other module's %s does.", + kindLabel, helperName, valuesHint, helperName, kindLabel, + ) + } +} diff --git a/pkg/linters/templates/rules/gateway_enablement.go b/pkg/linters/templates/rules/gateway_enablement.go new file mode 100644 index 00000000..e4092a07 --- /dev/null +++ b/pkg/linters/templates/rules/gateway_enablement.go @@ -0,0 +1,92 @@ +/* +Copyright 2026 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 ( + "context" + "fmt" + + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + GatewayEnablementRuleName = "gateway-enablement" + + // gatewayEnabledHelper is the shared helm_lib helper that decides whether a + // module's Gateway API resources (HTTPRoute, ListenerSet) should be created: + // it checks the module's own `.gatewayAPI.enabled` override (or the + // global `global.modules.gatewayAPI.enabled`), AND requires that a Gateway + // actually resolves (module, then global, then + // global.discovery.gatewayAPIDefaultGateway) — unlike Ingress there is no + // safe default gateway, so both conditions matter. + gatewayEnabledHelper = "helm_lib_module_gateway_enabled" +) + +type GatewayEnablementRule struct { + pkg.RuleMeta + pkg.PathRule + + module pkg.Module + errorList *errors.LintRuleErrorsList +} + +func NewGatewayEnablementRule(excludeFileRules []pkg.StringRuleExclude, + excludeDirectoryRules []pkg.DirectoryRuleExclude, + m pkg.Module, errorList *errors.LintRuleErrorsList) *GatewayEnablementRule { + return &GatewayEnablementRule{ + RuleMeta: pkg.RuleMeta{ + Name: GatewayEnablementRuleName, + }, + PathRule: pkg.PathRule{ + ExcludeStringRules: excludeFileRules, + ExcludeDirectoryRules: excludeDirectoryRules, + }, + module: m, + errorList: errorList.WithRule(GatewayEnablementRuleName), + } +} + +var _ pkg.Rule = (*GatewayEnablementRule)(nil) + +// Check scans every template file that emits a `kind: HTTPRoute` or +// `kind: ListenerSet` manifest and reports the ones that never reference +// helm_lib_module_gateway_enabled anywhere in the same file. See +// IngressEnablementRule.Check for the same-file heuristic this shares and why it +// was chosen. +// +// The rule only runs when the module actually renders an HTTPRoute or +// ListenerSet: a module with neither has nothing for this check to say. +func (r *GatewayEnablementRule) Check(_ context.Context) { + if !storageHasKind(r.module, "HTTPRoute", "ListenerSet") { + return + } + + camelModuleName := modules.ToLowerCamel(r.module.GetName()) + valuesHint := fmt.Sprintf( + "global.modules.gatewayAPI.enabled or %[1]s.gatewayAPI.enabled, with a Gateway resolvable "+ + "via global.discovery.gatewayAPIDefaultGateway, global.modules.gatewayAPI.gateway, or %[1]s.gatewayAPI.gateway", + camelModuleName, + ) + + checkKindGatedByHelper( + r.module, r.errorList, r.PathRule, + kindLineRe("HTTPRoute", "ListenerSet"), gatewayEnabledHelper, + "Gateway API (HTTPRoute/ListenerSet)", valuesHint, + ) +} diff --git a/pkg/linters/templates/rules/gateway_enablement_test.go b/pkg/linters/templates/rules/gateway_enablement_test.go new file mode 100644 index 00000000..45b0e66a --- /dev/null +++ b/pkg/linters/templates/rules/gateway_enablement_test.go @@ -0,0 +1,176 @@ +/* +Copyright 2026 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 ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +func TestGatewayEnablementRule_Check(t *testing.T) { + const ungatedHTTPRoute = `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard + namespace: d8-my-module +` + + const ungatedListenerSet = `apiVersion: gateway.networking.k8s.io/v1 +kind: ListenerSet +metadata: + name: dashboard + namespace: d8-my-module +` + + tests := []struct { + name string + templateFiles map[string]string + storageKinds []string + exclude []pkg.StringRuleExclude + wantCount int + wantContains []string + wantLines []int + }{ + { + name: "flags an HTTPRoute with no enablement check at all", + templateFiles: map[string]string{ + "templates/httproute.yaml": ungatedHTTPRoute, + }, + storageKinds: []string{"HTTPRoute"}, + wantCount: 1, + wantContains: []string{ + "helm_lib_module_gateway_enabled", "Gateway API", + "global.modules.gatewayAPI.enabled", "myModule.gatewayAPI.enabled", + "global.discovery.gatewayAPIDefaultGateway", + }, + wantLines: []int{2}, + }, + { + name: "flags a ListenerSet with no enablement check at all", + templateFiles: map[string]string{ + "templates/listenerset.yaml": ungatedListenerSet, + }, + storageKinds: []string{"ListenerSet"}, + wantCount: 1, + }, + { + name: "passes a ListenerSet and HTTPRoute guarded by helm_lib_module_gateway_enabled in the same file", + templateFiles: map[string]string{ + "templates/httproute.yaml": `{{- if and (eq (include "helm_lib_module_gateway_enabled" .) "true") .Values.global.modules.publicDomainTemplate }} +apiVersion: gateway.networking.k8s.io/v1 +kind: ListenerSet +metadata: + name: dashboard + namespace: d8-my-module +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard + namespace: d8-my-module +{{- end }} +`, + }, + storageKinds: []string{"HTTPRoute", "ListenerSet"}, + wantCount: 0, + }, + { + name: "ignores files that never create Gateway API objects", + templateFiles: map[string]string{ + "templates/ingress.yaml": `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard +`, + }, + storageKinds: []string{"HTTPRoute"}, // module has one, just not from this file + wantCount: 0, + }, + { + name: "does not run at all when the module ships neither HTTPRoute nor ListenerSet", + templateFiles: map[string]string{ + "templates/httproute.yaml": ungatedHTTPRoute, + }, + storageKinds: nil, // e.g. a module whose HTTPRoute never actually rendered + wantCount: 0, + }, + { + name: "an excluded file is skipped even though the module ships an HTTPRoute", + templateFiles: map[string]string{ + "templates/httproute.yaml": ungatedHTTPRoute, + }, + storageKinds: []string{"HTTPRoute"}, + exclude: []pkg.StringRuleExclude{"templates/httproute.yaml"}, + wantCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modulePath := writeTemplatesModule(t, tt.templateFiles) + + errorList := errors.NewLintRuleErrorsList() + NewGatewayEnablementRule(tt.exclude, nil, templatesMockModule(t, modulePath, tt.storageKinds...), errorList).Check(t.Context()) + + errs := errorList.GetErrors() + require.Len(t, errs, tt.wantCount) + + for _, want := range tt.wantContains { + found := false + + for i := range errs { + if containsStr(errs[i].Text, want) { + found = true + break + } + } + + require.Truef(t, found, "expected a finding containing %q, got %+v", want, errs) + } + + for i, wantLine := range tt.wantLines { + if i < len(errs) { + require.Equalf(t, wantLine, errs[i].LineNumber, "unexpected line for finding %d: %s", i, errs[i].Text) + } + } + }) + } +} + +func TestGatewayEnablementRule_DirectoryExclusion(t *testing.T) { + modulePath := writeTemplatesModule(t, map[string]string{ + "templates/vendor/httproute.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +`, + }) + + errorList := errors.NewLintRuleErrorsList() + NewGatewayEnablementRule( + nil, + []pkg.DirectoryRuleExclude{"templates/vendor/"}, + templatesMockModule(t, modulePath, "HTTPRoute"), + errorList, + ).Check(t.Context()) + + require.Empty(t, errorList.GetErrors()) +} diff --git a/pkg/linters/templates/rules/https_certificate_reuse.go b/pkg/linters/templates/rules/https_certificate_reuse.go new file mode 100644 index 00000000..1cb8a8a9 --- /dev/null +++ b/pkg/linters/templates/rules/https_certificate_reuse.go @@ -0,0 +1,416 @@ +/* +Copyright 2026 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 ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + HTTPSCertificateReuseRuleName = "https-certificate-reuse" +) + +var ( + // httpsCopyCustomCertificateRe matches a call to + // helm_lib_module_https_copy_custom_certificate, capturing its third + // argument (secret_name_prefix). The second argument (namespace) is not + // captured; it may be a quoted literal or a bare Helm expression. + httpsCopyCustomCertificateRe = regexp.MustCompile( + `helm_lib_module_https_copy_custom_certificate"\s*\(\s*list\s+\S+\s+(?:"[^"]*"|\S+)\s+"([^"]+)"\s*\)`, + ) + + // httpsSecretNameRe matches a call to helm_lib_module_https_secret_name, + // capturing its secret_name_prefix (always present) and, when the + // two-prefix form is used, the Gateway-API-specific override prefix. + httpsSecretNameRe = regexp.MustCompile( + `helm_lib_module_https_secret_name"\s*\(\s*list\s+\S+\s+"([^"]+)"(?:\s+"([^"]+)")?\s*\)`, + ) + + // yamlDocumentSeparatorRe matches a "---" document separator on its own + // line, the convention templates use to emit multiple manifests from one + // file. + yamlDocumentSeparatorRe = regexp.MustCompile(`(?m)^---[ \t]*$`) +) + +// certFlowSuffixes are the conventional customcertificate secret_name_prefix +// suffixes used throughout the module ecosystem: a module's Ingress-flow copy +// is commonly named "-ingress-tls" (or bare "ingress-tls") and its +// Gateway API/HTTPRoute-flow copy "-httproute-tls" (or bare +// "httproute-tls"). +var certFlowSuffixes = map[string]string{ + "ingress-tls": "ingress", + "httproute-tls": "httproute", +} + +// certFlowStem splits a customcertificate secret_name_prefix into its logical +// service stem and flow (ingress or httproute), if it follows that naming +// convention. "istio-httproute-tls" -> ("istio", "httproute", true); +// "httproute-tls" -> ("", "httproute", true); "service-a-tls" -> ("", "", false). +func certFlowStem(prefix string) (string, string, bool) { + for suffix, f := range certFlowSuffixes { + if prefix == suffix { + return "", f, true + } + + if trimmed, found := strings.CutSuffix(prefix, "-"+suffix); found { + return trimmed, f, true + } + } + + return "", "", false +} + +// splitYAMLDocuments returns the [start, end) byte ranges of each YAML +// document in content, split on "---" document separators. A file with no +// separator is a single document spanning the whole content. +func splitYAMLDocuments(content []byte) [][2]int { + seps := yamlDocumentSeparatorRe.FindAllIndex(content, -1) + if len(seps) == 0 { + return [][2]int{{0, len(content)}} + } + + docs := make([][2]int, 0, len(seps)+1) + start := 0 + + for _, sep := range seps { + docs = append(docs, [2]int{start, sep[0]}) + start = sep[1] + } + + docs = append(docs, [2]int{start, len(content)}) + + return docs +} + +// httpsCertCopy is one occurrence of helm_lib_module_https_copy_custom_certificate. +type httpsCertCopy struct { + prefix string + relPath string + line int +} + +// httpsSecretRef is one occurrence of helm_lib_module_https_secret_name. +// linked is true for the two-prefix form (list . base override); for the +// plain one-prefix form, override is empty and linked is false. +type httpsSecretRef struct { + base string + override string + linked bool + relPath string + line int +} + +type HTTPSCertificateReuseRule struct { + pkg.RuleMeta + pkg.PathRule + + module pkg.Module + errorList *errors.LintRuleErrorsList +} + +func NewHTTPSCertificateReuseRule(excludeFileRules []pkg.StringRuleExclude, + excludeDirectoryRules []pkg.DirectoryRuleExclude, + m pkg.Module, errorList *errors.LintRuleErrorsList) *HTTPSCertificateReuseRule { + return &HTTPSCertificateReuseRule{ + RuleMeta: pkg.RuleMeta{ + Name: HTTPSCertificateReuseRuleName, + }, + PathRule: pkg.PathRule{ + ExcludeStringRules: excludeFileRules, + ExcludeDirectoryRules: excludeDirectoryRules, + }, + module: m, + errorList: errorList.WithRule(HTTPSCertificateReuseRuleName), + } +} + +var _ pkg.Rule = (*HTTPSCertificateReuseRule)(nil) + +// Check enforces that a module serving HTTPS over both Ingress and Gateway +// API copies its CustomCertificate-mode certificate exactly once per secret, +// and that both flows reuse that one copy via +// helm_lib_module_https_secret_name: the Ingress manifest with the plain, +// one-prefix form ({{ include "helm_lib_module_https_secret_name" (list . "base") }}), +// the HTTPRoute/ListenerSet manifest with the two-prefix form +// ({{ include "helm_lib_module_https_secret_name" (list . "base" "override") }}), +// linking back to the exact same "base". In CertManager mode the two-prefix +// form resolves "override" to its own secret (Gateway API needs its own +// cert-manager Certificate, validated through a separate ClusterIssuer), but +// in CustomCertificate mode it ignores the override and resolves to "base"'s +// secret instead — so the Gateway API flow never needs, and must never copy, +// a second CustomCertificate secret of its own. +// +// This is a textual, whole-module heuristic scanning every template file for +// helm_lib_module_https_copy_custom_certificate and +// helm_lib_module_https_secret_name calls, classifying each +// helm_lib_module_https_secret_name call by the kind(s) declared in its own +// YAML document — a file is split on "---" separators first (see +// splitYAMLDocuments), since one file commonly bundles a HTTPRoute/ListenerSet +// alongside the cert-manager Certificate that feeds it. A call inside a +// `kind: Certificate` document is excluded entirely: a Certificate's own +// secretName always uses the plain form to declare a new target secret for +// cert-manager to populate, which isn't the Ingress or Gateway API manifest +// "consuming" a shared secret that this rule is about. Otherwise a call +// counts as an Ingress or Gateway API reference if its document declares +// `kind: Ingress` or `kind: HTTPRoute`/`kind: ListenerSet` respectively. It +// reports, deduplicated by location: +// +// 1. A secret_name_prefix copied by more than one +// helm_lib_module_https_copy_custom_certificate call — only one copy is +// ever needed for a given secret. +// 2. An Ingress-file reference using the two-prefix form — Ingress has no +// use for a Gateway-API-specific override, since it never needs to +// diverge from the shared secret. +// 3. A HTTPRoute/ListenerSet-file reference using the plain one-prefix +// form — without the override, that manifest names its own, +// independent secret instead of reusing the Ingress flow's. +// 4. A two-prefix form's override that is ALSO independently copied — that +// copy is unreachable, since the link already made the base's copy +// available to the Gateway API flow in CustomCertificate mode. +// 5. Two copied prefixes follow the "-ingress-tls" / "-httproute-tls" +// naming convention for the same stem (see certFlowStem). Unlike checks +// 1-4, this one fires purely from the copy calls themselves, so it also +// catches a module that copies both variants but hasn't wired up (or has +// wired up incorrectly) either flow's reference yet; it runs last and +// only adds a finding where none of the above already reported the +// exact same location. +// +// The rule only runs when the module renders both an Ingress and a +// HTTPRoute/ListenerSet: reuse across flows is only possible when both flows +// exist. +func (r *HTTPSCertificateReuseRule) Check(_ context.Context) { + m := r.module + + if !storageHasKind(m, "Ingress") || !storageHasKind(m, "HTTPRoute", "ListenerSet") { + return + } + + templatesPath := filepath.Join(m.GetPath(), "templates") + if _, err := os.Stat(templatesPath); os.IsNotExist(err) { + return + } + + files := fsutils.GetFiles(templatesPath, true, fsutils.FilterFileByExtensions(".yaml", ".yml", ".tpl")) + ingressKindRe := kindLineRe("Ingress") + gatewayKindRe := kindLineRe("HTTPRoute", "ListenerSet") + certificateKindRe := kindLineRe("Certificate") + + var copies []httpsCertCopy + + var ingressRefs []httpsSecretRef + + var gatewayRefs []httpsSecretRef + + for _, filePath := range files { + relPath := fsutils.Rel(m.GetPath(), filePath) + + if !r.Enabled(relPath) { + continue + } + + content, err := os.ReadFile(filePath) + if err != nil { + r.errorList.WithFilePath(relPath).Errorf("Failed to read file: %v", err) + continue + } + + for _, loc := range httpsCopyCustomCertificateRe.FindAllSubmatchIndex(content, -1) { + copies = append(copies, httpsCertCopy{ + prefix: string(content[loc[2]:loc[3]]), + relPath: relPath, + line: bytes.Count(content[:loc[0]], []byte("\n")) + 1, + }) + } + + // A file often bundles several manifests (e.g. a HTTPRoute alongside + // the cert-manager Certificate that feeds it) separated by "---", so + // classify each helm_lib_module_https_secret_name call by the kind(s) + // declared in its own document, not by every kind anywhere in the + // file. A Certificate's own secretName always uses the plain form — + // it's declaring a new target secret for cert-manager to populate, + // not an Ingress or Gateway API manifest consuming an existing one — + // so calls inside a Certificate document are excluded entirely. + for _, doc := range splitYAMLDocuments(content) { + docContent := content[doc[0]:doc[1]] + + if certificateKindRe.Match(docContent) { + continue + } + + isIngressDoc := ingressKindRe.Match(docContent) + isGatewayDoc := gatewayKindRe.Match(docContent) + + if !isIngressDoc && !isGatewayDoc { + continue + } + + for _, loc := range httpsSecretNameRe.FindAllSubmatchIndex(docContent, -1) { + ref := httpsSecretRef{ + base: string(docContent[loc[2]:loc[3]]), + linked: loc[4] >= 0, + relPath: relPath, + line: bytes.Count(content[:doc[0]+loc[0]], []byte("\n")) + 1, + } + if ref.linked { + ref.override = string(docContent[loc[4]:loc[5]]) + } + + if isIngressDoc { + ingressRefs = append(ingressRefs, ref) + } + + if isGatewayDoc { + gatewayRefs = append(gatewayRefs, ref) + } + } + } + } + + copiedAt := make(map[string][]httpsCertCopy, len(copies)) + + for _, c := range copies { + copiedAt[c.prefix] = append(copiedAt[c.prefix], c) + } + + reported := make(map[string]bool) + reportOnce := func(relPath string, line int, value, format string, args ...any) { + key := fmt.Sprintf("%s:%d", relPath, line) + if reported[key] { + return + } + + reported[key] = true + + r.errorList.WithFilePath(relPath). + WithLineNumber(line). + WithValue(value). + Errorf(format, args...) + } + + // 1. A secret copied by more than one helm_lib_module_https_copy_custom_certificate call. + for _, locs := range copiedAt { + if len(locs) < 2 { + continue + } + + for _, dupe := range locs[1:] { + reportOnce(dupe.relPath, dupe.line, dupe.prefix, + "Secret prefix %q is copied by more than one helm_lib_module_https_copy_custom_certificate "+ + "call (also at %s:%d) — only one copy is needed for a given secret; every consumer should "+ + "reuse it via helm_lib_module_https_secret_name instead of copying it again.", + dupe.prefix, locs[0].relPath, locs[0].line, + ) + } + } + + // 2. An Ingress-file reference using the two-prefix (Gateway-API-override) form. + for _, ref := range ingressRefs { + if !ref.linked { + continue + } + + reportOnce(ref.relPath, ref.line, ref.base, + "Ingress references its TLS secret with the two-prefix form of helm_lib_module_https_secret_name "+ + "(list . %q %q), but Ingress has no Gateway-API-specific override to apply — use the plain "+ + "form (list . %q) instead.", + ref.base, ref.override, ref.base, + ) + } + + // 3. A HTTPRoute/ListenerSet-file reference using the plain one-prefix form. + for _, ref := range gatewayRefs { + if ref.linked { + continue + } + + reportOnce(ref.relPath, ref.line, ref.base, + "HTTPRoute/ListenerSet references its TLS secret with the plain form of "+ + "helm_lib_module_https_secret_name (list . %[1]q). It is recommended to use the extended form "+ + "instead (list . \"\" %[1]q), which allows reusing the custom certificate "+ + "secret in case CustomCertificate mode is enabled.", + ref.base, + ) + } + + // 4. A two-prefix reference whose override is also independently copied. + for _, ref := range gatewayRefs { + if !ref.linked { + continue + } + + for _, dupe := range copiedAt[ref.override] { + reportOnce(dupe.relPath, dupe.line, dupe.prefix, + "File copies a custom certificate under secret prefix %q, but %q is only meant to be the "+ + "Gateway-API-specific override in the two-prefix form of helm_lib_module_https_secret_name "+ + "(list . %q %q), found in %s:%d. Under CustomCertificate mode both the Ingress and Gateway "+ + "API flows already resolve to %q's copy, so copying one under %q too is a duplicate — "+ + "remove this helm_lib_module_https_copy_custom_certificate call and let the two-prefix "+ + "form share %q's certificate.", + dupe.prefix, ref.override, ref.base, ref.override, ref.relPath, ref.line, + ref.base, ref.override, ref.base, + ) + } + } + + // 5. Two copied prefixes follow the "-ingress-tls" / "-httproute-tls" + // naming convention for the same stem. Runs last and only adds a + // finding where none of the checks above already reported this exact + // location: unlike them, it fires purely from the copy calls + // themselves, so it also catches a module that copies both variants + // but hasn't wired up (or has wired up incorrectly) either flow's + // reference yet, which the reference-based checks above cannot see. + ingressCopyByStem := make(map[string]httpsCertCopy) + + for _, c := range copies { + if stem, flow, ok := certFlowStem(c.prefix); ok && flow == "ingress" { + if _, seen := ingressCopyByStem[stem]; !seen { + ingressCopyByStem[stem] = c + } + } + } + + for _, c := range copies { + stem, flow, ok := certFlowStem(c.prefix) + if !ok || flow != "httproute" { + continue + } + + ingressCopy, hasIngress := ingressCopyByStem[stem] + if !hasIngress { + continue + } + + reportOnce(c.relPath, c.line, c.prefix, + "File copies a custom certificate under secret prefix %q, which by naming convention is the "+ + "Gateway API/HTTPRoute variant of %q — also copied via helm_lib_module_https_copy_custom_certificate, "+ + "in %s:%d. Copy the certificate once, under %q, and reference it from the Gateway API flow with the "+ + "two-prefix form of helm_lib_module_https_secret_name (list . %q %q) instead of copying it separately.", + c.prefix, ingressCopy.prefix, ingressCopy.relPath, ingressCopy.line, + ingressCopy.prefix, ingressCopy.prefix, c.prefix, + ) + } +} diff --git a/pkg/linters/templates/rules/https_certificate_reuse_test.go b/pkg/linters/templates/rules/https_certificate_reuse_test.go new file mode 100644 index 00000000..7fbf41f3 --- /dev/null +++ b/pkg/linters/templates/rules/https_certificate_reuse_test.go @@ -0,0 +1,407 @@ +/* +Copyright 2026 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 ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +func TestHTTPSCertificateReuseRule_Check(t *testing.T) { + const ingressUsingSharedSecret = `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard +spec: + tls: + - secretName: {{ include "helm_lib_module_https_secret_name" (list . "my-module-ingress-tls") }} +` + + const ingressNoSecretRef = `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard +` + + const httprouteLinkedToSharedSecret = `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + hostnames: + - dashboard.example.com + tls: + secretRef: + name: {{ include "helm_lib_module_https_secret_name" (list . "my-module-ingress-tls" "my-module-httproute-tls") }} +` + + const copyShared = `{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-my-module" "my-module-ingress-tls") }} +` + + const copyOverride = `{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-my-module" "my-module-httproute-tls") }} +` + + tests := []struct { + name string + templateFiles map[string]string + storageKinds []string + exclude []pkg.StringRuleExclude + wantCount int + wantContains []string + wantLines []int + }{ + { + name: "passes the canonical correct usage: copied once, ingress plain, gateway linked", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-certificate.yaml": copyShared, + "templates/httproute.yaml": httprouteLinkedToSharedSecret, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 0, + }, + { + // Real-world reproduction: templates/httproute.yaml bundles the + // HTTPRoute together with the cert-manager Certificate that + // feeds it, separated by "---". The Certificate's own secretName + // legitimately uses the plain form (it names a NEW target secret + // for cert-manager, not an existing shared one) and must not be + // classified as "the Gateway API flow's own TLS secret + // reference" just because the file also contains a HTTPRoute. + name: "passes a plain-form reference inside a Certificate document bundled with a HTTPRoute", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-certificate.yaml": copyShared, + "templates/httproute.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + tls: + secretRef: + name: {{ include "helm_lib_module_https_secret_name" (list . "my-module-ingress-tls" "my-module-httproute-tls") }} +--- +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: dashboard-httproute +spec: + secretName: {{ include "helm_lib_module_https_secret_name" (list . "my-module-httproute-tls") }} +`, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 0, + }, + { + name: "flags the override prefix also being independently copied", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-certificate.yaml": copyShared + copyOverride, + "templates/httproute.yaml": httprouteLinkedToSharedSecret, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 1, + wantContains: []string{ + "my-module-httproute-tls", "my-module-ingress-tls", + "helm_lib_module_https_secret_name", "duplicate", + }, + wantLines: []int{2}, + }, + { + name: "flags a secret prefix copied by two separate copy calls", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-cert-a.yaml": copyShared, + "templates/custom-cert-b.yaml": copyShared, + "templates/httproute.yaml": httprouteLinkedToSharedSecret, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 1, + wantContains: []string{"my-module-ingress-tls", "more than one", "only one copy is needed"}, + }, + { + name: "flags a HTTPRoute reference using the plain form instead of the two-prefix form", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-certificate.yaml": copyShared, + "templates/httproute.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + tls: + secretRef: + name: {{ include "helm_lib_module_https_secret_name" (list . "my-module-httproute-tls") }} +`, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 1, + wantContains: []string{"my-module-httproute-tls", "plain form", "extended form"}, + }, + { + name: "flags a ListenerSet reference using the plain form (real-world istio/api-proxy reproduction)", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressNoSecretRef, + "templates/custom-certificate.yaml": `{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "istio-ingress-tls") }} +{{ $moduleGateway := dict }} +{{ include "helm_lib_module_gateway" (list . $moduleGateway) }} +{{ if $moduleGateway }} +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "istio-httproute-tls") }} +{{ if .Values.istio.multicluster.enabled }} +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "api-proxy-httproute-tls") }} +{{ end }} +{{ end }} +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "api-proxy-ingress-tls") }} +`, + "templates/listenerset.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: ListenerSet +metadata: + name: istio +spec: + listeners: + - tls: + certificateRefs: + - name: {{ include "helm_lib_module_https_secret_name" (list . "istio-httproute-tls") }} +`, + "templates/multicluster/api-proxy/listenerset.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: ListenerSet +metadata: + name: api-proxy +spec: + listeners: + - tls: + certificateRefs: + - name: {{ include "helm_lib_module_https_secret_name" (list . "api-proxy-httproute-tls") }} +`, + }, + storageKinds: []string{"Ingress", "ListenerSet"}, + // Both signals fire here, at four distinct locations: the two + // ListenerSet references using the plain form (check 3), and the + // two "-httproute-tls" copies flagged separately by the naming + // convention check (check 6), since it can't tell they're the + // same underlying problem as the bad references above. + wantCount: 4, + wantContains: []string{ + "istio-httproute-tls", "api-proxy-httproute-tls", "plain form", "extended form", + "naming convention", + }, + }, + { + name: "flags per-stem duplicate copies from the copy calls alone, with no secret_name reference anywhere", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressNoSecretRef, + "templates/custom-certificate.yaml": `{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "istio-ingress-tls") }} +{{ $moduleGateway := dict }} +{{ include "helm_lib_module_gateway" (list . $moduleGateway) }} +{{ if $moduleGateway }} +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "istio-httproute-tls") }} +{{ if .Values.istio.multicluster.enabled }} +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "api-proxy-httproute-tls") }} +{{ end }} +{{ end }} +{{ include "helm_lib_module_https_copy_custom_certificate" (list . "d8-istio" "api-proxy-ingress-tls") }} +`, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 2, + wantContains: []string{ + "istio-httproute-tls", "istio-ingress-tls", "naming convention", + "api-proxy-httproute-tls", "api-proxy-ingress-tls", + }, + }, + { + // The rule used to also flag a HTTPRoute link whose base didn't + // match any prefix an Ingress file referenced, but that produced + // false positives when a module's Ingress and Gateway API + // manifests for the same logical certificate live in separate + // directories the rule doesn't otherwise correlate — removed. + name: "passes a HTTPRoute link whose base the Ingress flow never references", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-certificate.yaml": copyShared, + "templates/httproute.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + tls: + secretRef: + name: {{ include "helm_lib_module_https_secret_name" (list . "some-other-secret" "some-other-secret-httproute") }} +`, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 0, + }, + { + name: "flags an Ingress reference using the two-prefix form", + templateFiles: map[string]string{ + "templates/ingress.yaml": `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard +spec: + tls: + - secretName: {{ include "helm_lib_module_https_secret_name" (list . "my-module-ingress-tls" "my-module-httproute-tls") }} +`, + "templates/custom-certificate.yaml": copyShared, + "templates/httproute.yaml": httprouteLinkedToSharedSecret, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + wantCount: 1, + wantContains: []string{"my-module-ingress-tls", "plain form", "no Gateway-API-specific override"}, + }, + { + name: "passes two unrelated certificates, each copied once and correctly linked, for two different services", + templateFiles: map[string]string{ + "templates/dex/ingress.yaml": `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dex +spec: + tls: + - secretName: {{ include "helm_lib_module_https_secret_name" (list . "ingress-tls") }} +`, + "templates/kubeconfig-generator/ingress.yaml": `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: kubeconfig +spec: + tls: + - secretName: {{ include "helm_lib_module_https_secret_name" (list . "kubeconfig-ingress-tls") }} +`, + "templates/custom-certificate.yaml": `{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-user-authn" "ingress-tls") }} +`, + "templates/kubeconfig-generator/custom-certificate.yaml": `{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-user-authn" "kubeconfig-ingress-tls") }} +`, + "templates/listenerset.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: ListenerSet +metadata: + name: user-authn +spec: + listeners: + - tls: + certificateRefs: + - name: {{ include "helm_lib_module_https_secret_name" (list . "ingress-tls" "httproute-tls") }} + - tls: + certificateRefs: + - name: {{ include "helm_lib_module_https_secret_name" (list . "kubeconfig-ingress-tls" "kubeconfig-httproute-tls") }} +`, + }, + storageKinds: []string{"Ingress", "ListenerSet"}, + wantCount: 0, + }, + { + name: "does not run when the module has no Ingress", + templateFiles: map[string]string{ + "templates/custom-certificate.yaml": copyShared + copyOverride, + "templates/httproute.yaml": httprouteLinkedToSharedSecret, + }, + storageKinds: []string{"HTTPRoute"}, + wantCount: 0, + }, + { + name: "does not run when the module has no Gateway API resource", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-certificate.yaml": copyShared + copyOverride, + }, + storageKinds: []string{"Ingress"}, + wantCount: 0, + }, + { + name: "an excluded file is skipped even though it copies the duplicate certificate", + templateFiles: map[string]string{ + "templates/ingress.yaml": ingressUsingSharedSecret, + "templates/custom-certificate.yaml": copyShared + copyOverride, + "templates/httproute.yaml": httprouteLinkedToSharedSecret, + }, + storageKinds: []string{"Ingress", "HTTPRoute"}, + exclude: []pkg.StringRuleExclude{"templates/custom-certificate.yaml"}, + wantCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modulePath := writeTemplatesModule(t, tt.templateFiles) + + errorList := errors.NewLintRuleErrorsList() + NewHTTPSCertificateReuseRule(tt.exclude, nil, templatesMockModule(t, modulePath, tt.storageKinds...), errorList).Check(t.Context()) + + errs := errorList.GetErrors() + require.Len(t, errs, tt.wantCount) + + for _, want := range tt.wantContains { + found := false + + for i := range errs { + if containsStr(errs[i].Text, want) { + found = true + break + } + } + + require.Truef(t, found, "expected a finding containing %q, got %+v", want, errs) + } + + for i, wantLine := range tt.wantLines { + if i < len(errs) { + require.Equalf(t, wantLine, errs[i].LineNumber, "unexpected line for finding %d: %s", i, errs[i].Text) + } + } + }) + } +} + +func TestHTTPSCertificateReuseRule_DirectoryExclusion(t *testing.T) { + modulePath := writeTemplatesModule(t, map[string]string{ + "templates/ingress.yaml": `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard +spec: + tls: + - secretName: {{ include "helm_lib_module_https_secret_name" (list . "my-module-ingress-tls") }} +`, + "templates/vendor/custom-certificate.yaml": `{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-my-module" "my-module-ingress-tls") }} +{{- include "helm_lib_module_https_copy_custom_certificate" (list . "d8-my-module" "my-module-httproute-tls") }} +`, + "templates/httproute.yaml": `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: dashboard +spec: + tls: + secretRef: + name: {{ include "helm_lib_module_https_secret_name" (list . "my-module-ingress-tls" "my-module-httproute-tls") }} +`, + }) + + errorList := errors.NewLintRuleErrorsList() + NewHTTPSCertificateReuseRule( + nil, + []pkg.DirectoryRuleExclude{"templates/vendor/"}, + templatesMockModule(t, modulePath, "Ingress", "HTTPRoute"), + errorList, + ).Check(t.Context()) + + require.Empty(t, errorList.GetErrors()) +} diff --git a/pkg/linters/templates/rules/ingress_enablement.go b/pkg/linters/templates/rules/ingress_enablement.go new file mode 100644 index 00000000..224d6059 --- /dev/null +++ b/pkg/linters/templates/rules/ingress_enablement.go @@ -0,0 +1,93 @@ +/* +Copyright 2026 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 ( + "context" + "fmt" + + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + IngressEnablementRuleName = "ingress-enablement" + + // ingressEnabledHelper is the shared helm_lib helper that decides whether a + // module's Ingress should be created: it checks the module's own + // `.ingress.enabled` override first, then falls back to the global + // `global.modules.ingress.enabled`, defaulting to true when neither is set. + ingressEnabledHelper = "helm_lib_module_ingress_enabled" +) + +type IngressEnablementRule struct { + pkg.RuleMeta + pkg.PathRule + + module pkg.Module + errorList *errors.LintRuleErrorsList +} + +func NewIngressEnablementRule(excludeFileRules []pkg.StringRuleExclude, + excludeDirectoryRules []pkg.DirectoryRuleExclude, + m pkg.Module, errorList *errors.LintRuleErrorsList) *IngressEnablementRule { + return &IngressEnablementRule{ + RuleMeta: pkg.RuleMeta{ + Name: IngressEnablementRuleName, + }, + PathRule: pkg.PathRule{ + ExcludeStringRules: excludeFileRules, + ExcludeDirectoryRules: excludeDirectoryRules, + }, + module: m, + errorList: errorList.WithRule(IngressEnablementRuleName), + } +} + +var _ pkg.Rule = (*IngressEnablementRule)(nil) + +// Check scans every template file that emits a `kind: Ingress` manifest and +// reports the ones that never reference helm_lib_module_ingress_enabled anywhere +// in the same file. Without that helper (or an equivalent check on the same +// values), the Ingress renders unconditionally and cannot be turned off via +// either global.modules.ingress.enabled or the module's own ingress.enabled +// override — the two supported ways to disable it. +// +// This is a textual, same-file heuristic, not a template-scope analysis: a file +// that emits several Ingress manifests but only guards one of them with the +// helper will not be flagged. In every module observed so far the guard and the +// manifest it protects live in the same file, so this trade-off catches the +// common and important case — an Ingress with no enablement check at all — +// without the cost of a real Helm-template control-flow parser. +// +// The rule only runs when the module actually renders an Ingress object: a +// module with none has nothing for this check to say. +func (r *IngressEnablementRule) Check(_ context.Context) { + if !storageHasKind(r.module, "Ingress") { + return + } + + camelModuleName := modules.ToLowerCamel(r.module.GetName()) + valuesHint := fmt.Sprintf("global.modules.ingress.enabled or %s.ingress.enabled", camelModuleName) + + checkKindGatedByHelper( + r.module, r.errorList, r.PathRule, + kindLineRe("Ingress"), ingressEnabledHelper, + "Ingress", valuesHint, + ) +} diff --git a/pkg/linters/templates/rules/ingress_enablement_test.go b/pkg/linters/templates/rules/ingress_enablement_test.go new file mode 100644 index 00000000..6591e3f5 --- /dev/null +++ b/pkg/linters/templates/rules/ingress_enablement_test.go @@ -0,0 +1,174 @@ +/* +Copyright 2026 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 ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +func TestIngressEnablementRule_Check(t *testing.T) { + const ungatedIngress = `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard + namespace: d8-my-module +spec: + rules: + - host: dashboard.example.com +` + + tests := []struct { + name string + templateFiles map[string]string + storageKinds []string + exclude []pkg.StringRuleExclude + wantCount int + wantContains []string + wantLines []int + }{ + { + name: "flags an Ingress with no enablement check at all", + templateFiles: map[string]string{ + "templates/ingress.yaml": ungatedIngress, + }, + storageKinds: []string{"Ingress"}, + wantCount: 1, + wantContains: []string{ + "helm_lib_module_ingress_enabled", "Ingress", + "global.modules.ingress.enabled", "myModule.ingress.enabled", + }, + wantLines: []int{2}, + }, + { + name: "passes an Ingress guarded by helm_lib_module_ingress_enabled in the same file", + templateFiles: map[string]string{ + "templates/ingress.yaml": `{{- if eq (include "helm_lib_module_ingress_enabled" .) "true" }} +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard + namespace: d8-my-module +spec: + rules: + - host: dashboard.example.com +{{- end }} +`, + }, + storageKinds: []string{"Ingress"}, + wantCount: 0, + }, + { + name: "ignores files that never create an Ingress", + templateFiles: map[string]string{ + "templates/deployment.yaml": `apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-app +`, + }, + storageKinds: []string{"Ingress"}, // module has one, just not from this file + wantCount: 0, + }, + { + name: "does not confuse an unrelated kind field with Ingress", + templateFiles: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: my-config +data: + note: "this is not an IngressClass" +`, + }, + storageKinds: []string{"Ingress"}, + wantCount: 0, + }, + { + name: "does not run at all when the module ships no Ingress", + templateFiles: map[string]string{ + "templates/ingress.yaml": ungatedIngress, + }, + storageKinds: nil, // e.g. a module whose Ingress never actually rendered + wantCount: 0, + }, + { + name: "an excluded file is skipped even though the module ships an Ingress", + templateFiles: map[string]string{ + "templates/ingress.yaml": ungatedIngress, + }, + storageKinds: []string{"Ingress"}, + exclude: []pkg.StringRuleExclude{"templates/ingress.yaml"}, + wantCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modulePath := writeTemplatesModule(t, tt.templateFiles) + + errorList := errors.NewLintRuleErrorsList() + NewIngressEnablementRule(tt.exclude, nil, templatesMockModule(t, modulePath, tt.storageKinds...), errorList).Check(t.Context()) + + errs := errorList.GetErrors() + require.Len(t, errs, tt.wantCount) + + for _, want := range tt.wantContains { + found := false + + for i := range errs { + if containsStr(errs[i].Text, want) { + found = true + break + } + } + + require.Truef(t, found, "expected a finding containing %q, got %+v", want, errs) + } + + for i, wantLine := range tt.wantLines { + if i < len(errs) { + require.Equalf(t, wantLine, errs[i].LineNumber, "unexpected line for finding %d: %s", i, errs[i].Text) + } + } + }) + } +} + +func TestIngressEnablementRule_DirectoryExclusion(t *testing.T) { + modulePath := writeTemplatesModule(t, map[string]string{ + "templates/vendor/ingress.yaml": `apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: dashboard +`, + }) + + errorList := errors.NewLintRuleErrorsList() + NewIngressEnablementRule( + nil, + []pkg.DirectoryRuleExclude{"templates/vendor/"}, + templatesMockModule(t, modulePath, "Ingress"), + errorList, + ).Check(t.Context()) + + require.Empty(t, errorList.GetErrors()) +} diff --git a/pkg/linters/templates/templates.go b/pkg/linters/templates/templates.go index 285e3005..aee9040a 100644 --- a/pkg/linters/templates/templates.go +++ b/pkg/linters/templates/templates.go @@ -111,6 +111,22 @@ func (l *Templates) rules() []pkg.Rule { rules.NewHelmRenderRule(m, level(cfg.Rules.HelmRenderRule)), rules.NewOpenAPIValuesQuoteRule(cfg.ExcludeRules.OpenAPIValuesQuote.Get(), m, level(cfg.Rules.OpenAPIValuesQuoteRule)), rules.NewSchemaValidationRule(cfg.ExcludeRules.SchemaValidation.Get(), m, level(cfg.Rules.SchemaValidationRule)), + rules.NewDeprecatedHTTPRouteAnnotationsRule( + cfg.ExcludeRules.DeprecatedHTTPRouteAnnotations.Files.Get(), + cfg.ExcludeRules.DeprecatedHTTPRouteAnnotations.Directories.Get(), + m, level(cfg.Rules.DeprecatedHTTPRouteAnnotationsRule)), + rules.NewIngressEnablementRule( + cfg.ExcludeRules.IngressEnablement.Files.Get(), + cfg.ExcludeRules.IngressEnablement.Directories.Get(), + m, level(cfg.Rules.IngressEnablementRule)), + rules.NewGatewayEnablementRule( + cfg.ExcludeRules.GatewayEnablement.Files.Get(), + cfg.ExcludeRules.GatewayEnablement.Directories.Get(), + m, level(cfg.Rules.GatewayEnablementRule)), + rules.NewHTTPSCertificateReuseRule( + cfg.ExcludeRules.HTTPSCertificateReuse.Files.Get(), + cfg.ExcludeRules.HTTPSCertificateReuse.Directories.Get(), + m, level(cfg.Rules.HTTPSCertificateReuseRule)), ) } diff --git a/pkg/scopes/static.go b/pkg/scopes/static.go index 670aeb63..c9fed746 100644 --- a/pkg/scopes/static.go +++ b/pkg/scopes/static.go @@ -130,10 +130,14 @@ var staticRules = map[string]set.Set{ templates.ID: set.New( templatesrules.CRDEnabledModulesRuleName, templatesrules.ClusterDomainRuleName, + templatesrules.DeprecatedHTTPRouteAnnotationsRuleName, templatesrules.EnabledModulesRuleName, + templatesrules.GatewayEnablementRuleName, templatesrules.GrafanaRuleName, templatesrules.HTTPRouteRuleName, + templatesrules.HTTPSCertificateReuseRuleName, templatesrules.HelmRenderRuleName, + templatesrules.IngressEnablementRuleName, templatesrules.IngressRuleName, templatesrules.KubeRbacProxyRuleName, templatesrules.MountPointsRuleName, diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/expected.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/expected.yaml new file mode 100644 index 00000000..adac35ef --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/expected.yaml @@ -0,0 +1,11 @@ +description: > + A module whose .dmtlint.yaml excludes templates/httproute.yaml from the + deprecated-httproute-annotations rule via exclude-rules.deprecated-httproute-annotations.files + must not be flagged for that file, even though it still sets the deprecated + alb.network.deckhouse.io/response-headers-to-add annotation. This proves the + exclude-rules configuration is actually wired from .dmtlint.yaml through to + the rule, not just reachable via a direct Go constructor call. +module: module +expectAbsent: + - linter: templates + rule: deprecated-httproute-annotations diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/.dmtlint.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/.dmtlint.yaml new file mode 100644 index 00000000..0e506f11 --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/.dmtlint.yaml @@ -0,0 +1,6 @@ +linters-settings: + templates: + exclude-rules: + deprecated-httproute-annotations: + files: + - templates/httproute.yaml diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/module.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/module.yaml new file mode 100644 index 00000000..891ac0d5 --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-deprecated-httproute-annotations +namespace: e2e-deprecated-httproute-annotations diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/openapi/config-values.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/openapi/values.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/templates/httproute.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/templates/httproute.yaml new file mode 100644 index 00000000..a362a6ae --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations-excluded/module/templates/httproute.yaml @@ -0,0 +1,14 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: e2e-deprecated-httproute-annotations + namespace: e2e-deprecated-httproute-annotations + annotations: + alb.network.deckhouse.io/response-headers-to-add: '{"Strict-Transport-Security":"max-age=31536000; includeSubDomains"}' +spec: + hostnames: + - e2e.example.com + rules: + - backendRefs: + - name: e2e + port: 80 diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations/expected.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations/expected.yaml new file mode 100644 index 00000000..aadec274 --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations/expected.yaml @@ -0,0 +1,11 @@ +description: > + A template that still sets the deprecated + `alb.network.deckhouse.io/response-headers-to-add` annotation must be flagged + by the deprecated-httproute-annotations rule instead of the native Gateway API + ResponseHeaderModifier filter. +module: module +expect: + - linter: templates + rule: deprecated-httproute-annotations + level: error + textContains: "alb.network.deckhouse.io/response-headers-to-add" diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations/module/module.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/module.yaml new file mode 100644 index 00000000..891ac0d5 --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-deprecated-httproute-annotations +namespace: e2e-deprecated-httproute-annotations diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations/module/openapi/config-values.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations/module/openapi/values.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/deprecated-httproute-annotations/module/templates/httproute.yaml b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/templates/httproute.yaml new file mode 100644 index 00000000..a362a6ae --- /dev/null +++ b/test/e2e/testdata/templates/deprecated-httproute-annotations/module/templates/httproute.yaml @@ -0,0 +1,14 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: e2e-deprecated-httproute-annotations + namespace: e2e-deprecated-httproute-annotations + annotations: + alb.network.deckhouse.io/response-headers-to-add: '{"Strict-Transport-Security":"max-age=31536000; includeSubDomains"}' +spec: + hostnames: + - e2e.example.com + rules: + - backendRefs: + - name: e2e + port: 80 diff --git a/test/e2e/testdata/templates/gateway-enablement-missing/expected.yaml b/test/e2e/testdata/templates/gateway-enablement-missing/expected.yaml new file mode 100644 index 00000000..2fb63286 --- /dev/null +++ b/test/e2e/testdata/templates/gateway-enablement-missing/expected.yaml @@ -0,0 +1,11 @@ +description: > + A module that creates an HTTPRoute without ever checking + helm_lib_module_gateway_enabled cannot have that HTTPRoute disabled via + global or module configuration, and must be flagged by the + gateway-enablement rule. +module: module +expect: + - linter: templates + rule: gateway-enablement + level: error + textContains: "helm_lib_module_gateway_enabled" diff --git a/test/e2e/testdata/templates/gateway-enablement-missing/module/module.yaml b/test/e2e/testdata/templates/gateway-enablement-missing/module/module.yaml new file mode 100644 index 00000000..86e7b2d6 --- /dev/null +++ b/test/e2e/testdata/templates/gateway-enablement-missing/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-gateway-enablement-missing +namespace: e2e-gateway-enablement-missing diff --git a/test/e2e/testdata/templates/gateway-enablement-missing/module/openapi/config-values.yaml b/test/e2e/testdata/templates/gateway-enablement-missing/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/gateway-enablement-missing/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/gateway-enablement-missing/module/openapi/values.yaml b/test/e2e/testdata/templates/gateway-enablement-missing/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/gateway-enablement-missing/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/gateway-enablement-missing/module/templates/httproute.yaml b/test/e2e/testdata/templates/gateway-enablement-missing/module/templates/httproute.yaml new file mode 100644 index 00000000..df2e2de3 --- /dev/null +++ b/test/e2e/testdata/templates/gateway-enablement-missing/module/templates/httproute.yaml @@ -0,0 +1,12 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: e2e-gateway-enablement-missing + namespace: e2e-gateway-enablement-missing +spec: + hostnames: + - e2e.example.com + rules: + - backendRefs: + - name: e2e + port: 80 diff --git a/test/e2e/testdata/templates/ingress-enablement-missing/expected.yaml b/test/e2e/testdata/templates/ingress-enablement-missing/expected.yaml new file mode 100644 index 00000000..b9f1014d --- /dev/null +++ b/test/e2e/testdata/templates/ingress-enablement-missing/expected.yaml @@ -0,0 +1,10 @@ +description: > + A module that creates an Ingress without ever checking + helm_lib_module_ingress_enabled cannot have that Ingress disabled via global + or module configuration, and must be flagged by the ingress-enablement rule. +module: module +expect: + - linter: templates + rule: ingress-enablement + level: error + textContains: "helm_lib_module_ingress_enabled" diff --git a/test/e2e/testdata/templates/ingress-enablement-missing/module/module.yaml b/test/e2e/testdata/templates/ingress-enablement-missing/module/module.yaml new file mode 100644 index 00000000..d2dc361a --- /dev/null +++ b/test/e2e/testdata/templates/ingress-enablement-missing/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-ingress-enablement-missing +namespace: e2e-ingress-enablement-missing diff --git a/test/e2e/testdata/templates/ingress-enablement-missing/module/openapi/config-values.yaml b/test/e2e/testdata/templates/ingress-enablement-missing/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/ingress-enablement-missing/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/ingress-enablement-missing/module/openapi/values.yaml b/test/e2e/testdata/templates/ingress-enablement-missing/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/ingress-enablement-missing/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/ingress-enablement-missing/module/templates/ingress.yaml b/test/e2e/testdata/templates/ingress-enablement-missing/module/templates/ingress.yaml new file mode 100644 index 00000000..22d477f6 --- /dev/null +++ b/test/e2e/testdata/templates/ingress-enablement-missing/module/templates/ingress.yaml @@ -0,0 +1,17 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: e2e-ingress-enablement-missing + namespace: e2e-ingress-enablement-missing +spec: + rules: + - host: e2e.example.com + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: e2e + port: + number: 80 diff --git a/test/e2e/testdata/templates/no-ingress-gateway-objects/expected.yaml b/test/e2e/testdata/templates/no-ingress-gateway-objects/expected.yaml new file mode 100644 index 00000000..c3016a12 --- /dev/null +++ b/test/e2e/testdata/templates/no-ingress-gateway-objects/expected.yaml @@ -0,0 +1,14 @@ +description: > + A module that ships no Ingress, HTTPRoute, or ListenerSet at all must not + trigger deprecated-httproute-annotations, ingress-enablement, or gateway-enablement — + even though the module's ConfigMap contains the literal banned annotation + string, which would otherwise trip deprecated-httproute-annotations. All three rules + gate on the module actually rendering one of those kinds. +module: module +expectAbsent: + - linter: templates + rule: deprecated-httproute-annotations + - linter: templates + rule: ingress-enablement + - linter: templates + rule: gateway-enablement diff --git a/test/e2e/testdata/templates/no-ingress-gateway-objects/module/module.yaml b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/module.yaml new file mode 100644 index 00000000..2bf1fa27 --- /dev/null +++ b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-no-ingress-gateway-objects +namespace: e2e-no-ingress-gateway-objects diff --git a/test/e2e/testdata/templates/no-ingress-gateway-objects/module/openapi/config-values.yaml b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/no-ingress-gateway-objects/module/openapi/values.yaml b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/no-ingress-gateway-objects/module/templates/configmap.yaml b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/templates/configmap.yaml new file mode 100644 index 00000000..19fff124 --- /dev/null +++ b/test/e2e/testdata/templates/no-ingress-gateway-objects/module/templates/configmap.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-no-ingress-gateway-objects + namespace: e2e-no-ingress-gateway-objects +data: + # This string would trip the deprecated-httproute-annotations rule if it ran, but the + # module has no Ingress/HTTPRoute/ListenerSet at all, so the rule must not run. + note: "alb.network.deckhouse.io/response-headers-to-add"