feat: k5s-stack-namespaces hook

.github/workflows/ci.yml

@@ -52,6 +52,7 @@ jobs:
           stevedore-release-scope --help > /dev/null
           service-yaml-check > /dev/null
           no-production-newtonsoft --help > /dev/null
+          k5s-stack-namespaces > /dev/null
 
       - name: try-repo each hook against a fixture consumer
         run: |
@@ -77,6 +78,14 @@ jobs:
             image: pinpredict/oms
           YAML
 
+          # Sibling k5s lane overlays with distinct namespaces, so
+          # k5s-stack-namespaces exercises its grouping path rather than
+          # passing on an empty file set.
+          mkdir -p "$fixture/overlays"
+          printf 'namespace: rig-base\n' > "$fixture/k5s.yaml"
+          printf 'namespace: rig-5x\n' > "$fixture/overlays/perf-5x.yaml"
+          printf 'namespace: rig-10x\n' > "$fixture/overlays/perf-10x.yaml"
+
           # A tracked, clean production source so no-production-newtonsoft has
           # something to actually scan — with zero matching files it would pass
           # without exercising the git enumeration path.
@@ -86,7 +95,7 @@ jobs:
           git -c init.defaultBranch=main init -q --template= "$fixture"
           git -C "$fixture" add -A
 
-          for hook in stevedore-release-scope service-yaml-check no-production-newtonsoft; do
+          for hook in stevedore-release-scope service-yaml-check no-production-newtonsoft k5s-stack-namespaces; do
             echo "::group::try-repo $hook"
             (cd "$fixture" && pre-commit try-repo "$GITHUB_WORKSPACE" "$hook" --all-files)
             echo "::endgroup::"

.pre-commit-hooks.yaml

@@ -68,3 +68,19 @@
   language: script
   files: '(^|/)go\.mod$|(^|/)\.tool-versions$'
   pass_filenames: false
+
+- id: k5s-stack-namespaces
+  name: k5s stack overlays declare unique namespaces
+  description: |
+    Fails when two sibling k5s stack overlays declare the same `namespace:`. A
+    new lane is usually a copy of an existing one, and a namespace left
+    unchanged makes `k5s up` server-side-apply over the other lane's objects
+    with no error — Ready pods running a blend of two lanes' configuration,
+    which for a perf rig means every number it reports is measured against a
+    universe nobody described. Asserts uniqueness only, never a naming
+    convention; overlays that declare no namespace (inheriting the base stack's)
+    are skipped. Compares every stack in the changed files' directories, not
+    just the changed set, since a collision is a property of the whole set.
+  entry: k5s-stack-namespaces
+  language: python
+  files: '(^|/)(k5s\.yaml|komp\.yaml|overlays/.*\.ya?ml)$'

README.md

@@ -14,6 +14,7 @@ repos.
 | `stevedore-release-scope` | Assert that onboarding or retiring a service does not widen the shared image build contract: named services pair an `.stevedore.yaml` image id with a name-matching sibling chart, `docker-release` and `chart-release` receive the same `only:` selector, and `change_detection.shared_paths` carries the all-image signal without listing paths every onboarding touches. | `.stevedore.yaml`, `.github/workflows/ci.yml`, `charts/*/Chart.yaml` |
 | `no-production-newtonsoft` | Reject Newtonsoft.Json references in production .NET sources: a case-insensitive scan of every tracked source and build file, permitted only under the `--allow-prefix` paths (the approved test/benchmark projects) and on the one central `PackageVersion` line. Static half only — the transitive package-graph half needs `dotnet restore` and stays in the consumer's CI. | every commit (whole-tree scan) |
 | `check-go-version-sync` | Fails when a `go.mod` `go` directive and the governing `.tool-versions` `golang` pin drift apart. | `go.mod`, `.tool-versions` |
+| `k5s-stack-namespaces` | Fails when two sibling k5s stack overlays declare the same `namespace:`. A new lane is usually a copy of an existing one, and a namespace left unchanged makes `k5s up` server-side-apply over the other lane's objects with no error — Ready pods running a blend of two lanes' config. Asserts uniqueness only, never a naming convention. | `k5s.yaml`, `komp.yaml`, `overlays/*.yaml` |
 
 ## Using a hook
 
@@ -22,12 +23,12 @@ Reference this repo from a consumer's `.pre-commit-config.yaml`:
 ```yaml
 repos:
   - repo: https://github.com/pinpredict/pre-commit-hooks
-    rev: v0.4.0   # bump to upgrade
+    rev: v0.5.0   # bump to upgrade
     hooks:
       - id: check-go-version-sync
 ```
 
-**Current release: `v0.4.0`.** Pin an explicit tag rather than a branch;
+**Current release: `v0.5.0`.** Pin an explicit tag rather than a branch;
 `pre-commit autoupdate` rewrites the `rev:` to the latest tag when you want to
 move.
 
@@ -77,6 +78,40 @@ console script (`pinpredict_hooks/service_yaml_check.py`) with the paths —
 there is no `hooks/service-yaml-check.py` to run directly, for the packaging
 reason spelled out under [Repository layout](#repository-layout).
 
+### `k5s-stack-namespaces`
+
+```yaml
+  - repo: https://github.com/pinpredict/pre-commit-hooks
+    rev: v0.5.0
+    hooks:
+      - id: k5s-stack-namespaces
+```
+
+No arguments. A k5s rig is a base stack plus lane overlays
+(`k5s up -f overlays/perf-10x.yaml`), and a lane targeting a shared cluster
+declares its own `namespace:` so bringing one lane up cannot converge the rig
+another lane is standing in.
+
+What it catches is quiet: a new lane starts as a copy of an existing one, and if
+its `namespace:` is not changed with everything else, `k5s up` server-side-applies
+over the old lane's objects. Nothing errors — the pods are Ready and the rig is
+running a blend of two lanes' configuration. For a perf rig that means every
+number it reports was measured against a universe nobody described.
+
+Two deliberate choices:
+
+- **Uniqueness only, never a naming convention.** A repo's lane names are its own
+  business; pinning a pattern like `perf-rig-<lane>` would be one repo's
+  convention wearing an org hook's clothes.
+- **Files are read as written, not through k5s's `extends:` resolution.** Two
+  lanes that both *restate* a shared namespace is the collision; resolving first
+  would hide it. An overlay that declares no namespace at all — inheriting the
+  base stack's, the normal shape for a lane that only adjusts load — is skipped.
+
+It compares every stack in the changed files' **directories**, not just the
+changed set: a collision is a property of the whole set, and the lane that already
+owned the namespace is usually not part of the commit that collides with it.
+
 ## Releasing
 
 Bump the version in `pyproject.toml` **in the same PR as the hook change**,
@@ -144,7 +179,7 @@ A repo with no `.stevedore.yaml` at all is a clean no-op.
 
 ```yaml
 - repo: https://github.com/pinpredict/pre-commit-hooks
-  rev: v0.4.0
+  rev: v0.5.0
   hooks:
     - id: stevedore-release-scope
       args:
@@ -180,7 +215,7 @@ Nothing is exempt by default — name every permitted prefix:
 
 ```yaml
 - repo: https://github.com/pinpredict/pre-commit-hooks
-  rev: v0.4.0
+  rev: v0.5.0
   hooks:
     - id: no-production-newtonsoft
       args:
@@ -236,7 +271,7 @@ same toolchain. Each module's governing pin is the nearest ancestor
 
 ```yaml
 - repo: https://github.com/pinpredict/pre-commit-hooks
-  rev: v0.4.0
+  rev: v0.5.0
   hooks:
     - id: check-go-version-sync
 ```

pinpredict_hooks/k5s_stack_namespaces.py

@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+"""Assert that sibling k5s stack overlays do not share a namespace.
+
+A k5s rig is a base stack plus lane overlays (`k5s up -f overlays/perf-10x.yaml`).
+Each lane that targets a shared cluster declares its own `namespace:` so that
+bringing one lane up cannot converge the rig another lane is standing in.
+
+That is easy to get wrong in exactly one way, and the way is quiet. A new lane
+starts life as a copy of an existing one; if its `namespace:` is not changed with
+everything else, `k5s up` on the new lane server-side-applies over the old lane's
+objects. Nothing errors. The pods are Ready, the rig looks healthy, and it is
+running a blend of two lanes' configuration — for a perf rig, that means every
+number it reports is measured against a universe no one described.
+
+Generic on purpose: it asserts uniqueness, never a naming convention. A repo's
+lane names are its own business, and a hook that pinned `perf-rig-<lane>` would
+be one repo's convention wearing an org hook's clothes.
+
+A runtime suffix does not remove the need for this. k5s can append a per-engineer
+suffix (`namespaceSuffix: user`), which separates PEOPLE — but two lanes declaring
+the same base still resolve to the same namespace for any one engineer, so the
+declared values must be distinct regardless. The two mechanisms cover different
+axes and neither substitutes for the other.
+
+Overlays that declare NO namespace are skipped, not flagged: inheriting the base
+stack's namespace is the normal shape for a lane that only adjusts load, and it
+is `k5s down`-safe because there is only ever one such rig.
+"""
+
+from __future__ import annotations
+
+import sys
+from collections import defaultdict
+from pathlib import Path
+
+from ._common import MISSING, emit, load_yaml
+
+PROG = "k5s-stack-namespaces"
+
+
+def namespace_of(path: Path) -> str | None:
+    """Return the `namespace:` a stack file declares, or None.
+
+    Reads the file as written — deliberately NOT through k5s's merge or
+    `extends:` resolution. Two overlays that resolve to the same namespace only
+    because they share a fragment are the case this must catch, and resolving
+    first would hide it.
+    """
+    doc = load_yaml(path, PROG)
+    if doc is MISSING or not isinstance(doc, dict):
+        return None
+    ns = doc.get("namespace")
+    return ns if isinstance(ns, str) and ns else None
+
+
+def check(paths: list[Path]) -> list[str]:
+    """Group the given stacks by declared namespace and report any collision."""
+    by_namespace: dict[str, list[Path]] = defaultdict(list)
+    for path in sorted(set(paths)):
+        ns = namespace_of(path)
+        if ns is not None:
+            by_namespace[ns].append(path)
+
+    failures = []
+    for ns, owners in sorted(by_namespace.items()):
+        if len(owners) > 1:
+            listed = ", ".join(str(p) for p in owners)
+            failures.append(
+                f"namespace {ns!r} is declared by {len(owners)} stacks ({listed}) — "
+                f"`k5s up` on one would server-side-apply over the other's objects "
+                f"with no error, leaving a rig that blends both configurations. "
+                f"Give each its own namespace."
+            )
+    return failures
+
+
+def main(argv: list[str] | None = None) -> int:
+    # Defaulted so setuptools can wire this as a console script while a direct
+    # `python -m` invocation still works.
+    if argv is None:
+        argv = sys.argv[1:]
+
+    # pre-commit passes only the CHANGED files, but a collision is a property of
+    # the whole SET — a lane whose namespace was already taken is invisible if the
+    # file that took it is not in this commit. So the changed files only tell us
+    # which directories to look at; every stack in those directories is compared.
+    changed = [Path(a) for a in argv]
+    if not changed:
+        return 0
+    scope: set[Path] = set()
+    for path in changed:
+        for sibling in path.parent.glob("*.y*ml"):
+            if sibling.is_file():
+                scope.add(sibling)
+
+    failures = check(sorted(scope))
+    if failures:
+        emit(failures, PROG)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "pinpredict-pre-commit-hooks"
-version = "0.3.0"
+version = "0.5.0"
 description = "Shared pre-commit hooks used across PinPredict repositories"
 readme = "README.md"
 requires-python = ">=3.9"
@@ -18,6 +18,7 @@ dependencies = ["PyYAML>=6"]
 stevedore-release-scope = "pinpredict_hooks.stevedore_release_scope:main"
 service-yaml-check = "pinpredict_hooks.service_yaml_check:main"
 no-production-newtonsoft = "pinpredict_hooks.no_production_newtonsoft:main"
+k5s-stack-namespaces = "pinpredict_hooks.k5s_stack_namespaces:main"
 
 [tool.setuptools]
 packages = ["pinpredict_hooks"]

tests/test_k5s_stack_namespaces.py

@@ -0,0 +1,94 @@
+#!/usr/bin/env python3
+"""Regression tests for the k5s-stack-namespaces hook.
+
+Plain files in a temp dir — no git repo, no subprocess — so the suite stays fast
+enough to run as a pre-commit hook in this repo itself.
+"""
+
+from __future__ import annotations
+
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+
+from pinpredict_hooks.k5s_stack_namespaces import main  # noqa: E402
+
+
+def lane(ns: str | None, extends: str | None = None) -> str:
+    body = "services:\n  oms:\n    image: img\n"
+    if ns is not None:
+        body = f"namespace: {ns}\n" + body
+    if extends is not None:
+        body = f"extends:\n  - {extends}\n" + body
+    return body
+
+
+class K5sStackNamespacesTest(unittest.TestCase):
+    def setUp(self) -> None:
+        self._tmp = tempfile.TemporaryDirectory()
+        self.dir = Path(self._tmp.name) / "overlays"
+        self.dir.mkdir(parents=True)
+        self.addCleanup(self._tmp.cleanup)
+
+    def write(self, name: str, body: str) -> Path:
+        path = self.dir / name
+        path.write_text(body, encoding="utf-8")
+        return path
+
+    def test_unique_namespaces_pass(self) -> None:
+        a = self.write("perf-5x.yaml", lane("perf-rig-5x"))
+        self.write("perf-10x.yaml", lane("perf-rig-10x"))
+        self.assertEqual(main([str(a)]), 0)
+
+    def test_duplicate_namespace_fails_and_names_both_files(self) -> None:
+        # The realistic mistake: perf-20x started as a copy of perf-10x.
+        a = self.write("perf-10x.yaml", lane("perf-rig-10x"))
+        self.write("perf-20x.yaml", lane("perf-rig-10x"))
+        self.assertEqual(main([str(a)]), 1)
+
+    def test_collision_is_caught_when_only_the_new_file_is_staged(self) -> None:
+        """A collision is a property of the whole SET, not the changed files.
+
+        pre-commit passes only what this commit touched, so the lane that already
+        owned the namespace is usually absent from argv. Comparing just the staged
+        set would pass every time the older file is untouched — i.e. always.
+        """
+        self.write("perf-10x.yaml", lane("perf-rig-10x"))
+        new = self.write("perf-20x.yaml", lane("perf-rig-10x"))
+        self.assertEqual(main([str(new)]), 1)
+
+    def test_overlay_without_a_namespace_is_skipped(self) -> None:
+        """Inheriting the base stack's namespace is the normal lane shape."""
+        a = self.write("perf-5x.yaml", lane(None))
+        self.write("perf-10x.yaml", lane(None))
+        self.assertEqual(main([str(a)]), 0)
+
+    def test_shared_namespace_via_extends_is_still_caught(self) -> None:
+        """Files are read AS WRITTEN, not through k5s's extends resolution.
+
+        Two lanes that end up in one namespace because they both inherit it from a
+        shared fragment is the same collision; resolving first would hide it. Here
+        the fragment declares the namespace and neither lane overrides it, so
+        neither lane declares one — nothing to compare, and the hook stays quiet.
+        Restating it in both lanes is what it catches.
+        """
+        self.write("_shared.yaml", "namespace: perf-rig\n")
+        a = self.write("perf-5x.yaml", lane("perf-rig", extends="_shared.yaml"))
+        self.write("perf-10x.yaml", lane("perf-rig", extends="_shared.yaml"))
+        self.assertEqual(main([str(a)]), 1)
+
+    def test_no_files_is_a_clean_noop(self) -> None:
+        self.assertEqual(main([]), 0)
+
+    def test_malformed_yaml_is_fatal_not_skipped(self) -> None:
+        """Silently passing on unparseable YAML is how a guard stops guarding."""
+        bad = self.write("perf-5x.yaml", "namespace: [unclosed\n")
+        with self.assertRaises(SystemExit):
+            main([str(bad)])
+
+
+if __name__ == "__main__":
+    unittest.main()