From c91762d1294c831a9717166e9f635ee1b9f90811 Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Tue, 1 Sep 2026 18:26:24 +0200 Subject: [PATCH 1/2] feat(template): Check that Helm defined templates carry the chart name Adds a script called by prek which walks our charts and checks whether all defines/calls use a namespaced name https://github.com/stackabletech/issues/issues/882 --- template/.pre-commit-config.yaml.j2 | 6 ++ template/scripts/check_namespaced_defines.py | 93 ++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 template/scripts/check_namespaced_defines.py diff --git a/template/.pre-commit-config.yaml.j2 b/template/.pre-commit-config.yaml.j2 index 9b181f8a..33206497 100644 --- a/template/.pre-commit-config.yaml.j2 +++ b/template/.pre-commit-config.yaml.j2 @@ -66,6 +66,12 @@ repos: - repo: local hooks: + - id: check-namespaced-defines + name: check-namespaced-defines + language: system + entry: python3 scripts/check_namespaced_defines.py + files: ^deploy/helm/[^/]+/templates/ + - id: regenerate-charts name: regenerate-charts language: system diff --git a/template/scripts/check_namespaced_defines.py b/template/scripts/check_namespaced_defines.py new file mode 100644 index 00000000..679cae73 --- /dev/null +++ b/template/scripts/check_namespaced_defines.py @@ -0,0 +1,93 @@ +""" +Fails when a Helm defined template, or a call to one, is not prefixed with the +name of the chart it lives in. + +To run tests for this script: + python3 -m unittest check_namespaced_defines.py +""" + +import re +import unittest +from pathlib import Path + +NAME_PATTERN = r'\b(?:define|include|template)\s+"([^"]+)"' +CHART_PATTERN = r"(?:^|/)deploy/helm/([^/]+)/templates/" + + +def chart_of(path): + """The chart whose templates directory holds path, or None if it is outside one.""" + found = re.search(CHART_PATTERN, Path(path).as_posix()) + return found.group(1) if found else None + + +def unprefixed(text, chart): + """The defined template names in text, and the calls to them, lacking the chart prefix.""" + names = re.findall(NAME_PATTERN, text) + return sorted({name for name in names if not name.startswith(f"{chart}.")}) + + +class TestCoreMethods(unittest.TestCase): + def test_chart_of(self): + self.assertEqual( + chart_of("deploy/helm/trino-operator/templates/x.yaml"), "trino-operator" + ) + # An absolute path has to give the same answer, or a manual run invents a chart name. + self.assertEqual( + chart_of("/tmp/wt/deploy/helm/secret-operator/templates/a/b.yaml"), + "secret-operator", + ) + self.assertIsNone(chart_of("deploy/helm/trino-operator/values.yaml")) + self.assertIsNone(chart_of("README.md")) + + def test_prefixed_is_accepted(self): + text = '{{- define "trino-operator.labels" -}}\n{{ include "trino-operator.chart" . }}\n' + self.assertEqual(unprefixed(text, "trino-operator"), []) + + def test_unprefixed_define_is_reported(self): + self.assertEqual( + unprefixed('{{- define "helper.thing" -}}', "trino-operator"), + ["helper.thing"], + ) + + def test_unprefixed_call_is_reported(self): + # A renamed definition whose call sites did not move with it stops the chart rendering. + text = ( + '{{ include "operator.fullname" . }}\n{{ template "operator.labels" . }}\n' + ) + self.assertEqual( + unprefixed(text, "trino-operator"), ["operator.fullname", "operator.labels"] + ) + + def test_keyword_must_stand_alone(self): + # Without a word boundary any identifier ending in the keyword matches. + self.assertEqual( + unprefixed('{{ .Values.xinclude "bar.baz" }}', "foo-operator"), [] + ) + + +if __name__ == "__main__": + import sys + + if not sys.argv[1:]: + print(f"usage: {sys.argv[0]} deploy/helm//templates/*", file=sys.stderr) + sys.exit(2) + + failed = False + for path in sys.argv[1:]: + chart = chart_of(path) + if chart is None: + print( + f"{sys.argv[0]}: {path} is not inside deploy/helm//templates/", + file=sys.stderr, + ) + sys.exit(2) + names = unprefixed(Path(path).read_text(), chart) + if names: + failed = True + print( + f"{path}: defined templates and the calls to them must be prefixed with '{chart}.'" + ) + for name in names: + print(f" {name}") + + sys.exit(1 if failed else 0) From f73e52ce9c161803707105683e12ff591e0dd24c Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Fri, 4 Sep 2026 10:18:50 +0200 Subject: [PATCH 2/2] fix(template): Apply review feedback * Rename vairable * Fix the locale so it doesn't depend on the machine * Switch to a named capture group --- template/scripts/check_namespaced_defines.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/template/scripts/check_namespaced_defines.py b/template/scripts/check_namespaced_defines.py index 679cae73..a26c3e66 100644 --- a/template/scripts/check_namespaced_defines.py +++ b/template/scripts/check_namespaced_defines.py @@ -11,13 +11,13 @@ from pathlib import Path NAME_PATTERN = r'\b(?:define|include|template)\s+"([^"]+)"' -CHART_PATTERN = r"(?:^|/)deploy/helm/([^/]+)/templates/" +CHART_PATTERN = r"(?:^|/)deploy/helm/(?P[^/]+)/templates/" def chart_of(path): """The chart whose templates directory holds path, or None if it is outside one.""" found = re.search(CHART_PATTERN, Path(path).as_posix()) - return found.group(1) if found else None + return found.group("chart") if found else None def unprefixed(text, chart): @@ -81,13 +81,14 @@ def test_keyword_must_stand_alone(self): file=sys.stderr, ) sys.exit(2) - names = unprefixed(Path(path).read_text(), chart) - if names: + text = Path(path).read_text(encoding="utf-8") + unprefixed_names = unprefixed(text, chart) + if unprefixed_names: failed = True print( f"{path}: defined templates and the calls to them must be prefixed with '{chart}.'" ) - for name in names: + for name in unprefixed_names: print(f" {name}") sys.exit(1 if failed else 0)