From c7faca383e4b1e39e2d020e75889ec08b2ccf2df Mon Sep 17 00:00:00 2001 From: Paramoshka Date: Thu, 27 Aug 2026 14:37:01 +0300 Subject: [PATCH 1/3] [feat] Update Ingress linter for unsafe annotations Signed-off-by: Paramoshka --- pkg/linters/templates/README.md | 66 ++++-- pkg/linters/templates/rules/ingress.go | 42 +++- pkg/linters/templates/rules/ingress_test.go | 189 ++++++++++++++++++ test/e2e/README.md | 2 +- .../templates/ingress-snippet/expected.yaml | 10 +- 5 files changed, 277 insertions(+), 32 deletions(-) create mode 100644 pkg/linters/templates/rules/ingress_test.go diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index 2b92d784..f4684500 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -14,7 +14,7 @@ Proper template validation prevents runtime issues, ensures applications are pro | [pdb](#pdb) | Validates PodDisruptionBudgets for deployments and statefulsets | ✅ | enabled | | [kube-rbac-proxy](#kube-rbac-proxy) | Validates kube-rbac-proxy CA certificates in namespaces | ✅ | enabled | | [service-port](#service-port) | Validates services use named target ports | ✅ | enabled | -| [ingress-rules](#ingress-rules) | Validates Ingress configuration snippets | ✅ | enabled | +| [ingress-rules](#ingress-rules) | Warns about unsafe Ingress snippet annotations and validates HSTS | ✅ | enabled | | [httproute-rules](#httproute-rules) | Validates that every Ingress has a companion HTTPRoute backed by a ListenerSet | ✅ | enabled | | [prometheus-rules](#prometheus-rules) | Validates Prometheus rules with promtool and proper templates | ✅ | enabled | | [grafana-dashboards](#grafana-dashboards) | Validates Grafana dashboard templates | ✅ | enabled | @@ -885,29 +885,47 @@ linters-settings: ### ingress-rules -**Purpose:** Ensures Ingress resources include required security configuration snippets, specifically the Strict-Transport-Security (HSTS) header for enforcing HTTPS connections. +**Purpose:** Reports unsafe ingress-nginx snippet annotations and ensures that +Ingresses using `configuration-snippet` preserve HSTS during migration. **Description:** -Validates that Ingress objects with `nginx.ingress.kubernetes.io/configuration-snippet` annotation contain the required HSTS header configuration using the `helm_lib_module_ingress_configuration_snippet` helper. +The rule reports a warning when an Ingress uses any of these Critical +annotations: + +- `nginx.ingress.kubernetes.io/configuration-snippet`; +- `nginx.ingress.kubernetes.io/server-snippet`; +- `nginx.ingress.kubernetes.io/auth-snippet`; +- `nginx.ingress.kubernetes.io/modsecurity-snippet`; +- `nginx.ingress.kubernetes.io/stream-snippet`. + +These annotations allow arbitrary NGINX configuration and require manual +migration. The rule does not inspect arbitrary directives or suggest +replacements for them. **What it checks:** -1. Ingresses with `nginx.ingress.kubernetes.io/configuration-snippet` annotation -2. Configuration snippet contains `add_header Strict-Transport-Security` -3. Recommends using `helm_lib_module_ingress_configuration_snippet` helper +1. Every unsafe snippet annotation produces a migration warning. +2. An Ingress using `configuration-snippet` must also preserve HSTS through + either: + - `nginx.ingress.kubernetes.io/ingress-nginx-hsts: "true"` (preferred), or + - the legacy `add_header Strict-Transport-Security` directive inside the + snippet. +3. A legacy HSTS directive is accepted during migration, but the unsafe + annotation warning remains. **Why it matters:** -HSTS (HTTP Strict-Transport-Security): -- Forces browsers to use HTTPS only -- Prevents protocol downgrade attacks -- Protects against man-in-the-middle attacks -- Required for security compliance +- Snippet annotations require unsafe ingress-nginx controller options and allow + arbitrary NGINX directives. +- The dedicated HSTS annotation preserves the required fixed policy without + enabling arbitrary snippets. +- Keeping the legacy HSTS check during migration avoids weakening existing + Ingress security. **Examples:** -❌ **Incorrect** - Missing HSTS header: +❌ **Unsafe and missing HSTS:** ```yaml # templates/ingress.yaml @@ -934,13 +952,10 @@ spec: name: http ``` -**Error:** -``` -Error: Ingress annotation "nginx.ingress.kubernetes.io/configuration-snippet" does not contain required snippet "{{ include "helm_lib_module_ingress_configuration_snippet" . | nindent 6 }}". -Object: dashboard -``` +This produces a warning for the unsafe annotation and an error because neither +the preferred nor legacy HSTS configuration is present. -✅ **Correct** - Using Helm library helper: +⚠️ **Legacy HSTS during migration:** ```yaml # templates/ingress.yaml @@ -968,13 +983,24 @@ spec: name: http ``` -The helper includes the HSTS header: +The helper renders the legacy HSTS header: ```nginx add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; ``` -✅ **Correct** - Ingress without configuration-snippet (not checked): +This temporarily satisfies the HSTS check, but the unsafe annotation warning +remains. + +✅ **Preferred HSTS configuration:** + +```yaml +metadata: + annotations: + nginx.ingress.kubernetes.io/ingress-nginx-hsts: "true" +``` + +✅ **Ingress without snippet annotations:** ```yaml # templates/ingress.yaml diff --git a/pkg/linters/templates/rules/ingress.go b/pkg/linters/templates/rules/ingress.go index 2385387c..afec3967 100644 --- a/pkg/linters/templates/rules/ingress.go +++ b/pkg/linters/templates/rules/ingress.go @@ -32,10 +32,21 @@ import ( ) const ( - IngressRuleName = "ingress-rules" - snippet = `{{ include "helm_lib_module_ingress_configuration_snippet" . | nindent 6 }}` + IngressRuleName = "ingress-rules" + nginxAnnotationPrefix = "nginx.ingress.kubernetes.io/" + configurationSnippetAnnotation = nginxAnnotationPrefix + "configuration-snippet" + ingressNginxHSTSAnnotation = nginxAnnotationPrefix + "ingress-nginx-hsts" + legacyHSTSDirective = "add_header Strict-Transport-Security" ) +var unsafeIngressAnnotations = []string{ + configurationSnippetAnnotation, + nginxAnnotationPrefix + "server-snippet", + nginxAnnotationPrefix + "auth-snippet", + nginxAnnotationPrefix + "modsecurity-snippet", + nginxAnnotationPrefix + "stream-snippet", +} + type IngressRule struct { pkg.RuleMeta pkg.KindRule @@ -87,12 +98,27 @@ func (r *IngressRule) checkObject(object storage.StoreObject) { return } - for key, value := range ingress.GetAnnotations() { - if key == "nginx.ingress.kubernetes.io/configuration-snippet" { - if !strings.Contains(value, "add_header Strict-Transport-Security") { - errorList.WithObjectID(object.Unstructured.GetName()). - Errorf("Ingress annotation %q does not contain required snippet %q.", key, snippet) - } + annotations := ingress.GetAnnotations() + objectErrors := errorList.WithObjectID(object.Unstructured.GetName()) + + for _, annotation := range unsafeIngressAnnotations { + if _, found := annotations[annotation]; !found { + continue } + + objectErrors.Warnf("Ingress annotation %q is unsafe and requires manual migration.", annotation) + } + + configurationSnippet, found := annotations[configurationSnippetAnnotation] + if !found { + return + } + + hasSafeHSTS := annotations[ingressNginxHSTSAnnotation] == "true" + + hasLegacyHSTS := strings.Contains(configurationSnippet, legacyHSTSDirective) + if !hasSafeHSTS && !hasLegacyHSTS { + objectErrors.Errorf("Ingress annotation %q requires annotation %q to be set to %q to preserve HSTS.", + configurationSnippetAnnotation, ingressNginxHSTSAnnotation, "true") } } diff --git a/pkg/linters/templates/rules/ingress_test.go b/pkg/linters/templates/rules/ingress_test.go new file mode 100644 index 00000000..903b2b1a --- /dev/null +++ b/pkg/linters/templates/rules/ingress_test.go @@ -0,0 +1,189 @@ +/* +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/gojuno/minimock/v3" + "github.com/stretchr/testify/assert" + "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" +) + +func TestIngressRuleWarnsAboutUnsafeAnnotations(t *testing.T) { + for _, annotation := range unsafeIngressAnnotations { + t.Run(annotation, func(t *testing.T) { + value := "value" + if annotation == configurationSnippetAnnotation { + value = legacyHSTSDirective + } + + findings := runIngressRule(t, "Ingress", map[string]string{annotation: value}, nil) + + require.Len(t, findings, 1) + assert.Equal(t, pkg.Warn, findings[0].Level) + assert.Contains(t, findings[0].Text, annotation) + }) + } +} + +func TestIngressRuleHSTSCompatibility(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + wantWarnings int + wantErrors int + }{ + { + name: "safe HSTS annotation", + annotations: map[string]string{ + configurationSnippetAnnotation: "proxy_set_header X-Test value;", + ingressNginxHSTSAnnotation: "true", + }, + wantWarnings: 1, + }, + { + name: "legacy HSTS directive", + annotations: map[string]string{ + configurationSnippetAnnotation: legacyHSTSDirective + " value;", + }, + wantWarnings: 1, + }, + { + name: "HSTS is missing", + annotations: map[string]string{ + configurationSnippetAnnotation: "proxy_set_header X-Test value;", + }, + wantWarnings: 1, + wantErrors: 1, + }, + { + name: "configuration snippet is empty", + annotations: map[string]string{ + configurationSnippetAnnotation: "", + }, + wantWarnings: 1, + wantErrors: 1, + }, + { + name: "safe HSTS annotation is false", + annotations: map[string]string{ + configurationSnippetAnnotation: "proxy_set_header X-Test value;", + ingressNginxHSTSAnnotation: "false", + }, + wantWarnings: 1, + wantErrors: 1, + }, + { + name: "safe HSTS annotation is empty", + annotations: map[string]string{ + configurationSnippetAnnotation: "proxy_set_header X-Test value;", + ingressNginxHSTSAnnotation: "", + }, + wantWarnings: 1, + wantErrors: 1, + }, + { + name: "safe HSTS annotation has another value", + annotations: map[string]string{ + configurationSnippetAnnotation: "proxy_set_header X-Test value;", + ingressNginxHSTSAnnotation: "TRUE", + }, + wantWarnings: 1, + wantErrors: 1, + }, + { + name: "safe annotations without configuration snippet", + annotations: map[string]string{ + ingressNginxHSTSAnnotation: "true", + nginxAnnotationPrefix + "proxy-ssl-use-controller-certificate": "true", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + findings := runIngressRule(t, "Ingress", tt.annotations, nil) + + var warnings, ruleErrors int + + for _, finding := range findings { + switch finding.Level { + case pkg.Warn: + warnings++ + case pkg.Error: + ruleErrors++ + + assert.Contains(t, finding.Text, ingressNginxHSTSAnnotation) + } + } + + assert.Equal(t, tt.wantWarnings, warnings) + assert.Equal(t, tt.wantErrors, ruleErrors) + }) + } +} + +func TestIngressRuleReportsUnsafeAnnotationsInStableOrder(t *testing.T) { + findings := runIngressRule(t, "Ingress", map[string]string{ + nginxAnnotationPrefix + "stream-snippet": "stream {}", + nginxAnnotationPrefix + "server-snippet": "return 200;", + }, nil) + + require.Len(t, findings, 2) + assert.Contains(t, findings[0].Text, nginxAnnotationPrefix+"server-snippet") + assert.Contains(t, findings[1].Text, nginxAnnotationPrefix+"stream-snippet") +} + +func TestIngressRuleSkipsExcludedAndNonIngressResources(t *testing.T) { + exclude := []pkg.KindRuleExclude{{Kind: "Ingress", Name: "test"}} + annotations := map[string]string{configurationSnippetAnnotation: "value"} + + assert.Empty(t, runIngressRule(t, "Ingress", annotations, exclude)) + assert.Empty(t, runIngressRule(t, "Deployment", annotations, nil)) +} + +func runIngressRule(t *testing.T, kind string, annotations map[string]string, exclude []pkg.KindRuleExclude) []pkg.LinterError { + t.Helper() + + object := unstructured.Unstructured{} + object.SetAPIVersion("networking.k8s.io/v1") + object.SetKind(kind) + object.SetName("test") + object.SetAnnotations(annotations) + + objects := map[storage.ResourceIndex]storage.StoreObject{ + {Kind: kind, Name: object.GetName()}: { + Unstructured: object, + AbsPath: "/test/ingress.yaml", + }, + } + + module := mocks.NewModuleMock(minimock.NewController(t)) + module.GetStorageMock.Return(objects) + + errorList := errors.NewLintRuleErrorsList() + NewIngressRule(exclude, module, errorList).Check(t.Context()) + + return errorList.GetErrors() +} diff --git a/test/e2e/README.md b/test/e2e/README.md index 737301c0..e4d1f50b 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -132,7 +132,7 @@ go test ./test/e2e/ -run 'TestE2E//' -v | `templates/vpa-misconfigured` | `vpa` (updateMode `Auto`, missing `resourcePolicy.containerPolicies`) | | `templates/pdb-mismatch` | `pdb` (PDB selector does not match controller pod labels) | | `templates/pdb-helm-hook` | `pdb` (PDB carries helm hook annotations) | -| `templates/ingress-snippet` | `ingress-rules` (configuration-snippet missing HSTS) | +| `templates/ingress-snippet` | `ingress-rules` (unsafe configuration-snippet warning and missing HSTS error) | | `templates/monitoring-missing-yaml` | `prometheus-rules` + `grafana-dashboards` (monitoring/ without templates/monitoring.yaml) | | `templates/grafana-dashboard` | `grafana-dashboards` (deprecated panel type, missing prometheus datasource variable) | | `templates/prometheus-promtool` | `prometheus-rules` (invalid PromQL via promtool) | diff --git a/test/e2e/testdata/templates/ingress-snippet/expected.yaml b/test/e2e/testdata/templates/ingress-snippet/expected.yaml index 266218e2..61c10bcd 100644 --- a/test/e2e/testdata/templates/ingress-snippet/expected.yaml +++ b/test/e2e/testdata/templates/ingress-snippet/expected.yaml @@ -1,9 +1,13 @@ description: > - An Ingress whose configuration-snippet annotation does not include the - required HSTS snippet must be flagged by the ingress-rules rule. + An Ingress with an unsafe configuration-snippet and no HSTS protection must + produce a migration warning and an HSTS error. module: module expect: + - linter: templates + rule: ingress-rules + level: warn + textContains: "is unsafe and requires manual migration" - linter: templates rule: ingress-rules level: error - textContains: "does not contain required snippet" + textContains: "to preserve HSTS" From 6a7814984211c5ff2c950f463650dc501ce3b9f1 Mon Sep 17 00:00:00 2001 From: Paramoshka Date: Fri, 28 Aug 2026 12:23:36 +0300 Subject: [PATCH 2/3] [feat] Fail on unsafe Ingress annotations Signed-off-by: Paramoshka --- pkg/linters/templates/README.md | 14 ++--- pkg/linters/templates/rules/ingress.go | 2 +- pkg/linters/templates/rules/ingress_test.go | 52 ++++++++----------- test/e2e/README.md | 2 +- .../templates/ingress-snippet/expected.yaml | 4 +- 5 files changed, 34 insertions(+), 40 deletions(-) diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index f4684500..7cbdb281 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -14,7 +14,7 @@ Proper template validation prevents runtime issues, ensures applications are pro | [pdb](#pdb) | Validates PodDisruptionBudgets for deployments and statefulsets | ✅ | enabled | | [kube-rbac-proxy](#kube-rbac-proxy) | Validates kube-rbac-proxy CA certificates in namespaces | ✅ | enabled | | [service-port](#service-port) | Validates services use named target ports | ✅ | enabled | -| [ingress-rules](#ingress-rules) | Warns about unsafe Ingress snippet annotations and validates HSTS | ✅ | enabled | +| [ingress-rules](#ingress-rules) | Rejects unsafe Ingress snippet annotations and validates HSTS | ✅ | enabled | | [httproute-rules](#httproute-rules) | Validates that every Ingress has a companion HTTPRoute backed by a ListenerSet | ✅ | enabled | | [prometheus-rules](#prometheus-rules) | Validates Prometheus rules with promtool and proper templates | ✅ | enabled | | [grafana-dashboards](#grafana-dashboards) | Validates Grafana dashboard templates | ✅ | enabled | @@ -890,7 +890,7 @@ Ingresses using `configuration-snippet` preserve HSTS during migration. **Description:** -The rule reports a warning when an Ingress uses any of these Critical +The rule reports an error when an Ingress uses any of these Critical annotations: - `nginx.ingress.kubernetes.io/configuration-snippet`; @@ -905,14 +905,14 @@ replacements for them. **What it checks:** -1. Every unsafe snippet annotation produces a migration warning. +1. Every unsafe snippet annotation produces a migration error. 2. An Ingress using `configuration-snippet` must also preserve HSTS through either: - `nginx.ingress.kubernetes.io/ingress-nginx-hsts: "true"` (preferred), or - the legacy `add_header Strict-Transport-Security` directive inside the snippet. 3. A legacy HSTS directive is accepted during migration, but the unsafe - annotation warning remains. + annotation error remains. **Why it matters:** @@ -952,8 +952,8 @@ spec: name: http ``` -This produces a warning for the unsafe annotation and an error because neither -the preferred nor legacy HSTS configuration is present. +This produces an error for the unsafe annotation and another error because +neither the preferred nor legacy HSTS configuration is present. ⚠️ **Legacy HSTS during migration:** @@ -989,7 +989,7 @@ The helper renders the legacy HSTS header: add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; ``` -This temporarily satisfies the HSTS check, but the unsafe annotation warning +This temporarily satisfies the HSTS check, but the unsafe annotation error remains. ✅ **Preferred HSTS configuration:** diff --git a/pkg/linters/templates/rules/ingress.go b/pkg/linters/templates/rules/ingress.go index afec3967..2d7d3856 100644 --- a/pkg/linters/templates/rules/ingress.go +++ b/pkg/linters/templates/rules/ingress.go @@ -106,7 +106,7 @@ func (r *IngressRule) checkObject(object storage.StoreObject) { continue } - objectErrors.Warnf("Ingress annotation %q is unsafe and requires manual migration.", annotation) + objectErrors.Errorf("Ingress annotation %q is unsafe and requires manual migration.", annotation) } configurationSnippet, found := annotations[configurationSnippetAnnotation] diff --git a/pkg/linters/templates/rules/ingress_test.go b/pkg/linters/templates/rules/ingress_test.go index 903b2b1a..3795ebb2 100644 --- a/pkg/linters/templates/rules/ingress_test.go +++ b/pkg/linters/templates/rules/ingress_test.go @@ -30,7 +30,7 @@ import ( "github.com/deckhouse/dmt/pkg/errors" ) -func TestIngressRuleWarnsAboutUnsafeAnnotations(t *testing.T) { +func TestIngressRuleReportsUnsafeAnnotationsAsErrors(t *testing.T) { for _, annotation := range unsafeIngressAnnotations { t.Run(annotation, func(t *testing.T) { value := "value" @@ -41,7 +41,7 @@ func TestIngressRuleWarnsAboutUnsafeAnnotations(t *testing.T) { findings := runIngressRule(t, "Ingress", map[string]string{annotation: value}, nil) require.Len(t, findings, 1) - assert.Equal(t, pkg.Warn, findings[0].Level) + assert.Equal(t, pkg.Error, findings[0].Level) assert.Contains(t, findings[0].Text, annotation) }) } @@ -49,10 +49,10 @@ func TestIngressRuleWarnsAboutUnsafeAnnotations(t *testing.T) { func TestIngressRuleHSTSCompatibility(t *testing.T) { tests := []struct { - name string - annotations map[string]string - wantWarnings int - wantErrors int + name string + annotations map[string]string + wantErrors int + wantHSTSError bool }{ { name: "safe HSTS annotation", @@ -60,30 +60,30 @@ func TestIngressRuleHSTSCompatibility(t *testing.T) { configurationSnippetAnnotation: "proxy_set_header X-Test value;", ingressNginxHSTSAnnotation: "true", }, - wantWarnings: 1, + wantErrors: 1, }, { name: "legacy HSTS directive", annotations: map[string]string{ configurationSnippetAnnotation: legacyHSTSDirective + " value;", }, - wantWarnings: 1, + wantErrors: 1, }, { name: "HSTS is missing", annotations: map[string]string{ configurationSnippetAnnotation: "proxy_set_header X-Test value;", }, - wantWarnings: 1, - wantErrors: 1, + wantErrors: 2, + wantHSTSError: true, }, { name: "configuration snippet is empty", annotations: map[string]string{ configurationSnippetAnnotation: "", }, - wantWarnings: 1, - wantErrors: 1, + wantErrors: 2, + wantHSTSError: true, }, { name: "safe HSTS annotation is false", @@ -91,8 +91,8 @@ func TestIngressRuleHSTSCompatibility(t *testing.T) { configurationSnippetAnnotation: "proxy_set_header X-Test value;", ingressNginxHSTSAnnotation: "false", }, - wantWarnings: 1, - wantErrors: 1, + wantErrors: 2, + wantHSTSError: true, }, { name: "safe HSTS annotation is empty", @@ -100,8 +100,8 @@ func TestIngressRuleHSTSCompatibility(t *testing.T) { configurationSnippetAnnotation: "proxy_set_header X-Test value;", ingressNginxHSTSAnnotation: "", }, - wantWarnings: 1, - wantErrors: 1, + wantErrors: 2, + wantHSTSError: true, }, { name: "safe HSTS annotation has another value", @@ -109,8 +109,8 @@ func TestIngressRuleHSTSCompatibility(t *testing.T) { configurationSnippetAnnotation: "proxy_set_header X-Test value;", ingressNginxHSTSAnnotation: "TRUE", }, - wantWarnings: 1, - wantErrors: 1, + wantErrors: 2, + wantHSTSError: true, }, { name: "safe annotations without configuration snippet", @@ -125,21 +125,15 @@ func TestIngressRuleHSTSCompatibility(t *testing.T) { t.Run(tt.name, func(t *testing.T) { findings := runIngressRule(t, "Ingress", tt.annotations, nil) - var warnings, ruleErrors int + require.Len(t, findings, tt.wantErrors) for _, finding := range findings { - switch finding.Level { - case pkg.Warn: - warnings++ - case pkg.Error: - ruleErrors++ - - assert.Contains(t, finding.Text, ingressNginxHSTSAnnotation) - } + assert.Equal(t, pkg.Error, finding.Level) } - assert.Equal(t, tt.wantWarnings, warnings) - assert.Equal(t, tt.wantErrors, ruleErrors) + if tt.wantHSTSError { + assert.Contains(t, findings[len(findings)-1].Text, ingressNginxHSTSAnnotation) + } }) } } diff --git a/test/e2e/README.md b/test/e2e/README.md index e4d1f50b..0acc185f 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -132,7 +132,7 @@ go test ./test/e2e/ -run 'TestE2E//' -v | `templates/vpa-misconfigured` | `vpa` (updateMode `Auto`, missing `resourcePolicy.containerPolicies`) | | `templates/pdb-mismatch` | `pdb` (PDB selector does not match controller pod labels) | | `templates/pdb-helm-hook` | `pdb` (PDB carries helm hook annotations) | -| `templates/ingress-snippet` | `ingress-rules` (unsafe configuration-snippet warning and missing HSTS error) | +| `templates/ingress-snippet` | `ingress-rules` (unsafe configuration-snippet error and missing HSTS error) | | `templates/monitoring-missing-yaml` | `prometheus-rules` + `grafana-dashboards` (monitoring/ without templates/monitoring.yaml) | | `templates/grafana-dashboard` | `grafana-dashboards` (deprecated panel type, missing prometheus datasource variable) | | `templates/prometheus-promtool` | `prometheus-rules` (invalid PromQL via promtool) | diff --git a/test/e2e/testdata/templates/ingress-snippet/expected.yaml b/test/e2e/testdata/templates/ingress-snippet/expected.yaml index 61c10bcd..ea3e766a 100644 --- a/test/e2e/testdata/templates/ingress-snippet/expected.yaml +++ b/test/e2e/testdata/templates/ingress-snippet/expected.yaml @@ -1,11 +1,11 @@ description: > An Ingress with an unsafe configuration-snippet and no HSTS protection must - produce a migration warning and an HSTS error. + produce a migration error and an HSTS error. module: module expect: - linter: templates rule: ingress-rules - level: warn + level: error textContains: "is unsafe and requires manual migration" - linter: templates rule: ingress-rules From eb83a1e7c2d640319d5589ee47cd5a17d7bc059f Mon Sep 17 00:00:00 2001 From: Paramoshka Date: Fri, 28 Aug 2026 15:25:14 +0300 Subject: [PATCH 3/3] [fix] Validate legacy HSTS directive --- pkg/linters/templates/README.md | 29 +++++++++++++-------- pkg/linters/templates/rules/ingress.go | 14 ++++++++-- pkg/linters/templates/rules/ingress_test.go | 26 +++++++++++++++++- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index 7cbdb281..dc5522ed 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -909,10 +909,10 @@ replacements for them. 2. An Ingress using `configuration-snippet` must also preserve HSTS through either: - `nginx.ingress.kubernetes.io/ingress-nginx-hsts: "true"` (preferred), or - - the legacy `add_header Strict-Transport-Security` directive inside the - snippet. -3. A legacy HSTS directive is accepted during migration, but the unsafe - annotation error remains. + - the canonical legacy HSTS directive produced by + `helm_lib_module_ingress_configuration_snippet` and shown below. +3. The canonical legacy HSTS directive is accepted during migration, but the + unsafe annotation error remains. **Why it matters:** @@ -2684,26 +2684,33 @@ Object: namespace = d8-my-module - d8-my-module ``` -### Issue: Ingress missing HSTS configuration +### Issue: Unsafe Ingress annotation or missing HSTS configuration **Symptom:** ``` -Error: Ingress annotation "nginx.ingress.kubernetes.io/configuration-snippet" does not contain required snippet +Error: Ingress annotation "nginx.ingress.kubernetes.io/configuration-snippet" is unsafe and requires manual migration. +Error: Ingress annotation "nginx.ingress.kubernetes.io/configuration-snippet" requires annotation "nginx.ingress.kubernetes.io/ingress-nginx-hsts" to be set to "true" to preserve HSTS. ``` -**Cause:** Ingress configuration-snippet missing Strict-Transport-Security header. +**Cause:** The Ingress uses an unsafe snippet annotation. A +`configuration-snippet` without either the dedicated HSTS annotation or the +legacy canonical HSTS directive also risks losing HSTS during migration. **Solutions:** -1. **Use Helm helper:** +1. **Preserve HSTS with the safe annotation:** ```yaml annotations: - nginx.ingress.kubernetes.io/configuration-snippet: | -{{- include "helm_lib_module_ingress_configuration_snippet" . | nindent 6 }} + nginx.ingress.kubernetes.io/ingress-nginx-hsts: "true" ``` -2. **Exclude Ingress:** +2. **Migrate the remaining snippet directives to safe annotations or other + Kubernetes resources, then remove the unsafe snippet annotation.** Adding + the HSTS annotation resolves only the HSTS error; the unsafe annotation + error remains until the snippet is removed or excluded. + +3. **Temporarily exclude the Ingress if no safe migration is available:** ```yaml # .dmtlint.yaml diff --git a/pkg/linters/templates/rules/ingress.go b/pkg/linters/templates/rules/ingress.go index 2d7d3856..936107aa 100644 --- a/pkg/linters/templates/rules/ingress.go +++ b/pkg/linters/templates/rules/ingress.go @@ -36,7 +36,7 @@ const ( nginxAnnotationPrefix = "nginx.ingress.kubernetes.io/" configurationSnippetAnnotation = nginxAnnotationPrefix + "configuration-snippet" ingressNginxHSTSAnnotation = nginxAnnotationPrefix + "ingress-nginx-hsts" - legacyHSTSDirective = "add_header Strict-Transport-Security" + legacyHSTSDirective = `add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;` ) var unsafeIngressAnnotations = []string{ @@ -116,9 +116,19 @@ func (r *IngressRule) checkObject(object storage.StoreObject) { hasSafeHSTS := annotations[ingressNginxHSTSAnnotation] == "true" - hasLegacyHSTS := strings.Contains(configurationSnippet, legacyHSTSDirective) + hasLegacyHSTS := hasLegacyHSTSDirective(configurationSnippet) if !hasSafeHSTS && !hasLegacyHSTS { objectErrors.Errorf("Ingress annotation %q requires annotation %q to be set to %q to preserve HSTS.", configurationSnippetAnnotation, ingressNginxHSTSAnnotation, "true") } } + +func hasLegacyHSTSDirective(configurationSnippet string) bool { + for line := range strings.SplitSeq(configurationSnippet, "\n") { + if strings.TrimSpace(line) == legacyHSTSDirective { + return true + } + } + + return false +} diff --git a/pkg/linters/templates/rules/ingress_test.go b/pkg/linters/templates/rules/ingress_test.go index 3795ebb2..45144df1 100644 --- a/pkg/linters/templates/rules/ingress_test.go +++ b/pkg/linters/templates/rules/ingress_test.go @@ -65,10 +65,34 @@ func TestIngressRuleHSTSCompatibility(t *testing.T) { { name: "legacy HSTS directive", annotations: map[string]string{ - configurationSnippetAnnotation: legacyHSTSDirective + " value;", + configurationSnippetAnnotation: " " + legacyHSTSDirective, }, wantErrors: 1, }, + { + name: "commented legacy HSTS directive", + annotations: map[string]string{ + configurationSnippetAnnotation: "# " + legacyHSTSDirective, + }, + wantErrors: 2, + wantHSTSError: true, + }, + { + name: "legacy HSTS directive disables HSTS", + annotations: map[string]string{ + configurationSnippetAnnotation: `add_header Strict-Transport-Security "max-age=0" always;`, + }, + wantErrors: 2, + wantHSTSError: true, + }, + { + name: "report-only header is not HSTS", + annotations: map[string]string{ + configurationSnippetAnnotation: `add_header Strict-Transport-Security-Report-Only "max-age=31536000" always;`, + }, + wantErrors: 2, + wantHSTSError: true, + }, { name: "HSTS is missing", annotations: map[string]string{