feat: validate only gitops instances affected by the changed files

.github/workflows/test.yml

@@ -24,3 +24,22 @@ jobs:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
       - name: Run hook against PR branch
         run: ./git-branch-linearity.sh ${{ github.base_ref }}
+
+  python-tests:
+    runs-on: ubuntu-24.04
+    strategy:
+      matrix:
+        # Oldest supported version and the one of the nix shell
+        python-version: ["3.10", "3.14"]
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+      - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97  # v7.0.0
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          pipx install poetry==2.4.1
+          poetry env use python
+          poetry install
+      - name: Run Python tests
+        run: poetry run pytest tests

.pre-commit-hooks.yaml

@@ -30,18 +30,14 @@
 -   id: gitops-values-validation
     name: GitOps values JSON schema validation
     language: python
-    entry: python -m kp_pre_commit_hooks.gitops_values_validation
+    entry: gitops-values-validation --changed-files
     stages: [pre-commit]
+    files: ^gitops/
+    # Always run so that commits only deleting files are still checked:
+    # deleted files are never passed to hooks, the script lists the staged ones itself
     always_run: true
-    pass_filenames: false
-    additional_dependencies:
-      - jsonschema==4.19.0
-      - requests==2.31.0
-      - ruamel-yaml==0.17.32
-      - urllib3[secure]==1.26.15
-      - pyyaml==6.0.1
-      - termcolor==2.4.0
-      - semver==3.0.2
+    # No parallel runs: values files headers may be rewritten and the schema cache is shared
+    require_serial: true
 
 
 -   id: kafka-check-schemas

README.md

@@ -80,6 +80,37 @@ repos:
       - id: terraform-repo-compliance
 ```
 
+#### `gitops-values-validation`
+
+Validate the values files of a gitops repository (`gitops/<application>/<service>/`) against the JSON schema
+of the `platform-managed-chart` version declared in `Chart.yaml` / `Chart-<env>.yaml`, plus Kpler specific rules:
+- service names are unique across applications and match their folder
+- topic names follow the naming convention and `maxLocalTopicBytes` stays within the allowed limits
+- the `$schema` header of each values file matches the chart version (it is fixed automatically when wrong)
+
+The JSON schemas are downloaded from S3 and cached in `~/.cache/pre-commit/kp-pre-commit-hooks/schemas`, so the
+[Twingate VPN](https://kpler.atlassian.net/wiki/spaces/KSD/pages/243562083/Install+and+configure+the+Twingate+VPN+client)
+must be up when a schema is not cached yet. The hook requires Python 3.10 or later.
+
+On commit, only the service instances depending on the changed files (`Chart.yaml`, `Chart-<env>.yaml`,
+`values.yaml`, `values-<env>.yaml`, `values-<env>-<instance>.yaml`) are validated. As pre-commit never passes
+deleted files to hooks, the staged deleted files are added by the script itself. Repository-level constraints
+(unique service names) are always checked. Run it with `--all-files` (e.g. in CI) to validate every instance:
+```bash
+pre-commit run gitops-values-validation --all-files
+```
+
+Add these lines in your `.pre-commit-config.yaml` file to enable this pre-commit hook:
+```yaml
+repos:
+  # [...]
+  - repo: https://github.com/Kpler/kp-pre-commit-hooks.git
+    rev: v0.63.0  # Use the latest version (v0.63.0+ to only validate the changed files)
+    hooks:
+      # [...]
+      - id: gitops-values-validation
+```
+
 [example of implementation]: https://github.com/Kpler/template-kafka-stream-msk/blob/main/src/ci/scala/schema_generator/VulcanSchemaGenerator.scala
 [sbt command]: https://github.com/Kpler/template-kafka-stream-msk/blob/main/build.sbt#L75
 
@@ -94,12 +125,28 @@ pre-commit try-repo path_to_this_repo/kp-pre-commit-hooks/ check-branch-linearit
 ```
 
 
-### Local Debugging of Schema Validation Logic
+#### Development environment
+
+Use `nix-shell` (or [direnv](https://direnv.net/) with the provided `.envrc`) to get Python, Poetry and
+pre-commit, or install Python 3.10+ and Poetry yourself. Then:
+```bash
+poetry install
+poetry run pytest tests
+```
+
+The tests use a minimal schema (`tests/test_data/schema-platform-managed-chart-strict.json`) holding only the parts of
+the real schema that carry the Kpler specific rules (`additionalChecks`), so they run without the VPN, e.g. in CI.
+To run them against the real schemas, with the Twingate VPN up:
+```bash
+poetry run pytest tests --real-schema
+```
 
-Prereq:
-`poetry install`
+#### Local Debugging of Schema Validation Logic
 
 An example for testing against a repo:
-`poetry run python kp_pre_commit_hooks/gitops_values_validation.py ~/repos/mt-inbox-gitops`
+`poetry run gitops-values-validation ~/repos/mt-inbox-gitops`
 
 Send the entire gitops repository path in for it to parse through the gitops repository for validation.
+To only validate the instances affected by some files, as the hook does, run it from the gitops repository root
+with `--changed-files`:
+`poetry -P path_to_this_repo/kp-pre-commit-hooks run gitops-values-validation --changed-files gitops/app/service/values-dev.yaml`

kp_pre_commit_hooks/gitops_values_validation.py

@@ -1,5 +1,7 @@
+import argparse
 import json
 import re
+import subprocess
 import sys
 import textwrap
 from collections import defaultdict
@@ -235,6 +237,13 @@ def iter_service_instances_config(self):
             _, env, instance = instance_values_file.stem.split("-", maxsplit=2)
             yield ServiceInstanceConfig(application_name, service_name, env, instance, instance_values_file.parent, self)
 
+    def iter_service_instances_config_affected_by(self, changed_files: Sequence[Path]):
+        """Yield only the service instances whose configuration depends on one of the changed files"""
+        resolved_changed_files = {f.resolve() for f in changed_files}
+        for config in self.iter_service_instances_config():
+            if any(source_file.resolve() in resolved_changed_files for source_file in config.source_files):
+                yield config
+
     def validate_unique_service_names(self) -> list[SchemaValidationError]:
         """Validate that service names are unique across all applications"""
         service_to_apps = defaultdict(set)
@@ -317,19 +326,24 @@ def configuration(self) -> dict:
         return ValuesFile.merge_values(self.values_files)
 
     @property
-    def values_files(self) -> list[ValuesFile]:
-        """Get list of values files"""
-        candidate_files = [
-            "values.yaml",
-            f"values-{self.env}.yaml",
-            f"values-{self.env}-{self.instance}.yaml"
-        ]
+    def values_file_paths(self) -> list[Path]:
+        """Get candidate values files paths, existing or not"""
         return [
-            ValuesFile(self.path.joinpath(file))
-            for file in candidate_files
-            if self.path.joinpath(file).exists()
+            self.path / "values.yaml",
+            self.path / f"values-{self.env}.yaml",
+            self.path / f"values-{self.env}-{self.instance}.yaml",
         ]
 
+    @property
+    def values_files(self) -> list[ValuesFile]:
+        """Get list of values files"""
+        return [ValuesFile(path) for path in self.values_file_paths if path.exists()]
+
+    @property
+    def source_files(self) -> list[Path]:
+        """Get every file (existing or not) the instance configuration is built from"""
+        return [self.path / "Chart.yaml", self.path / f"Chart-{self.env}.yaml", *self.values_file_paths]
+
     @property
     def helm_chart(self) -> HelmChart:
         """Get HelmChart from Chart.yaml and optional Chart-{env}.yaml"""
@@ -681,8 +695,42 @@ def display_errors(
 # Main Entry Point
 ###############################################################################
 
-if __name__ == "__main__":
-    gitops_path = Path(sys.argv[1]) if len(sys.argv) >= 2 else Path.cwd()
+def get_staged_deleted_files(git_path: Path) -> list[Path]:
+    """Get files deleted in the git index, as pre-commit never passes deleted files to hooks"""
+    # --no-renames reports the old path of a renamed file as deleted,
+    # --relative makes paths relative to the current directory like the ones passed by pre-commit
+    output = subprocess.run(
+        ["git", "diff", "--staged", "--name-only", "--no-renames", "--relative", "--diff-filter=D", "-z"],
+        cwd=git_path,
+        capture_output=True,
+        text=True,
+        check=True,
+    ).stdout
+    return [git_path / file for file in output.split("\0") if file]
+
+
+def parse_args(argv: Optional[Sequence[str]] = None) -> tuple[Path, Optional[list[Path]]]:
+    """Parse CLI arguments into the gitops repository path and the changed files (None to validate everything)"""
+    parser = argparse.ArgumentParser(description="Validate the values files of a gitops repository")
+    parser.add_argument(
+        "--changed-files",
+        action="store_true",
+        help="validate only the instances affected by the given files and the staged deleted files",
+    )
+    parser.add_argument("paths", nargs="*", type=Path, help="gitops repository path, or changed files with --changed-files")
+    args = parser.parse_args(argv)
+
+    if args.changed_files:
+        return Path.cwd(), args.paths
+    if len(args.paths) > 1:
+        parser.error("only one gitops repository path is expected, use --changed-files to pass changed files")
+    return (args.paths[0] if args.paths else Path.cwd()), None
+
+
+def main(argv: Optional[Sequence[str]] = None) -> int:
+    gitops_path, changed_files = parse_args(argv)
+    if changed_files is not None:
+        changed_files = [*changed_files, *get_staged_deleted_files(gitops_path)]
     gitops_repository = GitOpsRepository(gitops_path)
 
     try:
@@ -700,7 +748,12 @@ def display_errors(
             print(colorize("Repository constraints PASSED", "green"))
 
         # Individual service instance validations
-        for service_instance_config in gitops_repository.iter_service_instances_config():
+        service_instances_config = (
+            gitops_repository.iter_service_instances_config()
+            if changed_files is None
+            else gitops_repository.iter_service_instances_config_affected_by(changed_files)
+        )
+        for service_instance_config in service_instances_config:
             print(f"Checking {service_instance_config} ", end="")
 
             validator = ServiceInstanceConfigValidator(service_instance_config)
@@ -715,12 +768,16 @@ def display_errors(
                 # We always try to sync the schema header version, in case it was one of the error detected
                 service_instance_config.sync_values_files_schema_header_version()
 
-        sys.exit(1 if errors_found else 0)
+        return 1 if errors_found else 0
 
     except UnauthorizedToDownloadSchema as error:
         print(
             f"\n\n{colorize('FATAL:', 'red')} Unauthorized to download schema at {error.schema_url}\n"
             "       Please check that your Twingate VPN Client is up and running configured.\n"
             f"       More info at {TWINGATE_DOC_URL}\n\n"
         )
-        sys.exit(1)
+        return 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())