From c9a0fd2face6f3b9c9d50ee925aa8c901946e982 Mon Sep 17 00:00:00 2001
From: Allen Porter <allen.porter@gmail.com>
Date: Tue, 7 Mar 2023 20:47:52 -0800
Subject: [PATCH] Add support for external diff commands (#108)

Add support for running an external diff command, and update README with
examples with dyff.

Issue #101
---
 README.md                                 | 34 ++++++++++++-
 flux_local/command.py                     | 15 ++++++
 flux_local/tool/diff.py                   | 62 +++++++++++++++++++++--
 flux_local/tool/selector.py               |  2 +
 tests/tool/test_flux_local.py             |  3 +-
 tests/tool/testdata/diff_hr_external.yaml | 11 ++++
 tests/tool/testdata/diff_ks_external.yaml |  8 +++
 7 files changed, 129 insertions(+), 6 deletions(-)
 create mode 100644 tests/tool/testdata/diff_hr_external.yaml
 create mode 100644 tests/tool/testdata/diff_ks_external.yaml

diff --git a/README.md b/README.md
index 86fede6c..3453921f 100644
--- a/README.md
+++ b/README.md
@@ -125,7 +125,7 @@ $ flux-local diff ks apps
 Additionally `flux-local` can inflate a `HelmRelease` locally and show diffs in the output
 objects. This is similar to `flux diff` but for HelmReleases:
 
-```bash
+```diff
 $ flux-local diff hr -n podinfo podinfo
 ---
 
@@ -146,6 +146,38 @@ $ flux-local diff hr -n podinfo podinfo
 ...
 ```
 
+You may also use an external diff program such as [dyff](https://github.com/homeport/dyff) which
+is more compact for diffing yaml resources:
+```bash
+$ git status
+On branch dev
+Your branch is up to date with 'origin/dev'.
+
+Changes not staged for commit:
+	modified:   home/dev/hajimari-values.yaml
+
+$ export DIFF="dyff between --omit-header --color on"
+# flux-local diff ks home --path clusters/dev/
+
+spec.chart.spec.version  (HelmRelease/hajimari/hajimari)
+  ± value change
+    - 2.0.2
+    + 2.0.1
+
+$ flux-local diff hr hajimari -n hajimari --path clusters/dev/
+
+metadata.labels.helm.sh/chart  (ClusterRoleBinding/default/hajimari)
+  ± value change
+    - hajimari-2.0.2
+    + hajimari-2.0.1
+
+metadata.labels.helm.sh/chart  (PersistentVolumeClaim/default/hajimari-data)
+  ± value change
+    - hajimari-2.0.2
+    + hajimari-2.0.1
+```
+
+
 ### flux-local test
 
 You can verify that the resources in the cluster are formatted properly before commit or as part
diff --git a/flux_local/command.py b/flux_local/command.py
index 9f3b0e81..2ead32a6 100644
--- a/flux_local/command.py
+++ b/flux_local/command.py
@@ -8,6 +8,7 @@
 from dataclasses import dataclass
 from pathlib import Path
 from collections.abc import Sequence
+import os
 
 from .exceptions import CommandException
 
@@ -40,6 +41,13 @@ class Command(Task):
     """Current working directory."""
 
     exc: type[CommandException] = CommandException
+    """Exception to throw in case of an error."""
+
+    retcodes: list[int] | None = None
+    """Non-zero error codes that are allowed to indicate success (e.g. for diff)."""
+
+    env: dict[str, str] | None = None
+    """Environment variables for the subprocess."""
 
     @property
     def string(self) -> str:
@@ -53,15 +61,22 @@ def __str__(self) -> str:
     async def run(self, stdin: bytes | None = None) -> bytes:
         """Run the command, returning stdout."""
         _LOGGER.debug("Running command: %s", self)
+        env = {
+            **os.environ,
+            **(self.env if self.env else {}),
+        }
         proc = await asyncio.create_subprocess_shell(
             self.string,
             stdin=subprocess.PIPE,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             cwd=self.cwd,
+            env=env,
         )
         out, err = await proc.communicate(stdin)
         if proc.returncode:
+            if self.retcodes and proc.returncode in self.retcodes:
+                return out
             errors = [f"Command '{self}' failed with return code {proc.returncode}"]
             if out:
                 errors.append(out.decode("utf-8"))
diff --git a/flux_local/tool/diff.py b/flux_local/tool/diff.py
index d881a2a1..b2ebcfa9 100644
--- a/flux_local/tool/diff.py
+++ b/flux_local/tool/diff.py
@@ -1,6 +1,7 @@
 """Flux-local diff action."""
 
 import asyncio
+import os
 from argparse import ArgumentParser
 from argparse import _SubParsersAction as SubParsersAction
 from contextlib import contextmanager
@@ -8,12 +9,13 @@
 import difflib
 import logging
 import pathlib
+import shlex
 import tempfile
-from typing import cast, Generator, Any
+from typing import cast, Generator, Any, AsyncGenerator
 import yaml
 
 
-from flux_local import git_repo
+from flux_local import git_repo, command
 
 from . import selector
 from .visitor import HelmVisitor, ObjectOutput, ResourceKey
@@ -45,6 +47,48 @@ def perform_object_diff(
                 yield line
 
 
+async def perform_external_diff(
+    cmd: list[str],
+    a: ObjectOutput,
+    b: ObjectOutput,
+) -> AsyncGenerator[str, None]:
+    """Generate diffs between the two output objects."""
+    with tempfile.TemporaryDirectory() as tmpdir:
+        for kustomization_key in set(a.content.keys()) | set(b.content.keys()):
+            _LOGGER.debug(
+                "Diffing results for Kustomization %s",
+                kustomization_key,
+            )
+            a_resources = a.content.get(kustomization_key, {})
+            b_resources = b.content.get(kustomization_key, {})
+            keys = set(a_resources.keys()) | set(b_resources.keys())
+
+            a_file = pathlib.Path(tmpdir) / "a.yaml"
+            a_file.write_text(
+                "\n".join(
+                    [
+                        "\n".join(a_resources.get(resource_key, []))
+                        for resource_key in keys
+                    ]
+                )
+            )
+            b_file = pathlib.Path(tmpdir) / "b.yaml"
+            b_file.write_text(
+                "\n".join(
+                    [
+                        "\n".join(b_resources.get(resource_key, []))
+                        for resource_key in keys
+                    ]
+                )
+            )
+
+            out = await command.Command(
+                cmd + [str(a_file), str(b_file)], retcodes=[0, 1]
+            ).run()
+            if out:
+                yield out.decode("utf-8")
+
+
 def omit_none(obj: Any) -> dict[str, Any]:
     """Creates a dictionary with None values missing."""
     return {k: v for k, v in obj if v is not None}
@@ -203,10 +247,15 @@ async def run(  # type: ignore[no-untyped-def]
         _LOGGER.debug("Diffing content")
         if output == "yaml":
             result = perform_yaml_diff(orig_content, content, unified)
+        elif external_diff := os.environ.get("DIFF"):
+            async for line in perform_external_diff(
+                shlex.split(external_diff), orig_content, content
+            ):
+                print(line)
         else:
             result = perform_object_diff(orig_content, content, unified)
-        for line in result:
-            print(line)
+            for line in result:
+                print(line)
 
 
 class DiffHelmReleaseAction:
@@ -315,6 +364,11 @@ async def run(  # type: ignore[no-untyped-def]
         if output == "yaml":
             for line in perform_yaml_diff(orig_helm_content, helm_content, unified):
                 print(line)
+        elif external_diff := os.environ.get("DIFF"):
+            async for line in perform_external_diff(
+                shlex.split(external_diff), orig_helm_content, helm_content
+            ):
+                print(line)
         else:
             for line in perform_object_diff(orig_helm_content, helm_content, unified):
                 print(line)
diff --git a/flux_local/tool/selector.py b/flux_local/tool/selector.py
index aa9b3f6c..5a247d4f 100644
--- a/flux_local/tool/selector.py
+++ b/flux_local/tool/selector.py
@@ -75,6 +75,7 @@ def build_ks_selector(  # type: ignore[no-untyped-def]
         selector.kustomization.namespace = None
     selector.kustomization.skip_crds = kwargs["skip_crds"]
     selector.kustomization.skip_secrets = kwargs["skip_secrets"]
+    selector.cluster_policy.enabled = False
     return selector
 
 
@@ -103,6 +104,7 @@ def build_hr_selector(  # type: ignore[no-untyped-def]
         selector.helm_release.namespace = None
     selector.helm_release.skip_crds = kwargs["skip_crds"]
     selector.helm_release.skip_secrets = kwargs["skip_secrets"]
+    selector.cluster_policy.enabled = False
     return selector
 
 
diff --git a/tests/tool/test_flux_local.py b/tests/tool/test_flux_local.py
index 9043657b..ea0a01bb 100644
--- a/tests/tool/test_flux_local.py
+++ b/tests/tool/test_flux_local.py
@@ -10,7 +10,8 @@
 @pytest.mark.golden_test("testdata/*.yaml")
 async def test_flux_local_golden(golden: GoldenTestFixture) -> None:
     """Test commands in golden files."""
+    env = golden.get("env")
     args = golden["args"]
-    result = await run(Command(["flux-local"] + args))
+    result = await run(Command(["flux-local"] + args, env=env))
     if golden.get("stdout"):
         assert result == golden.out["stdout"]
diff --git a/tests/tool/testdata/diff_hr_external.yaml b/tests/tool/testdata/diff_hr_external.yaml
new file mode 100644
index 00000000..f1a2bbc1
--- /dev/null
+++ b/tests/tool/testdata/diff_hr_external.yaml
@@ -0,0 +1,11 @@
+env:
+  DIFF: diff
+args:
+- diff
+- hr
+- podinfo
+- -n
+- podinfo
+- --path
+- tests/testdata/cluster/
+stdout: ''
diff --git a/tests/tool/testdata/diff_ks_external.yaml b/tests/tool/testdata/diff_ks_external.yaml
new file mode 100644
index 00000000..7fcd0833
--- /dev/null
+++ b/tests/tool/testdata/diff_ks_external.yaml
@@ -0,0 +1,8 @@
+env:
+  DIFF: diff
+args:
+- diff
+- ks
+- apps
+- --path
+- tests/testdata/cluster/