Add option to select smart diffing for migrations to catalog compile

This commit adds a new option `-m`, `--migration-diff-output` which can
be used to select smarter diffing for a migration which Commodore knows
about.

This commit also adds the Kapitan 0.29 to 0.30 migration as the only
known migration. The smart diffing for this migration ignores reordered
K8s objects in multi-document YAML streams, and suppresses diffs which
only contain changes `app.kubernetes.io/managed-by: Tiller` ->
`app.kubernetes.io/managed-by: Helm` and `heritage: Tiller` ->
`heritage: Helm`.

Author: simu

commodore/catalog.py

@@ -1,12 +1,16 @@
+import difflib
+
 from pathlib import Path as P
-from typing import Iterable
+from typing import Iterable, Tuple
 
 import click
+import yaml
 
 from . import git
 from .helpers import rm_tree_contents, lieutenant_query
 from .cluster import Cluster
-from .config import Config
+from .config import Config, Migration
+from .k8sobject import K8sObject
 
 
 def fetch_customer_catalog(config: Config, cluster: Cluster):
@@ -112,6 +116,64 @@ def _push_catalog(cfg: Config, repo, commit_message):
         click.echo(" > Skipping commit+push to catalog in local mode...")
 
 
+def _ignore_kapitan_029_030_non_semantic(line):
+    """
+    Ignore non-semantic changes in the (already sorted by K8s object) diff between
+    Kapitan 0.29 and 0.30.
+
+    This includes:
+    * Change of "app.kubernetes.io/managed-by: Tiller" to
+      "app.kubernetes.io/managed-by: Helm"
+    * Change of "heritage: Tiller" to "heritage: Helm"
+    * `null` objects not emitted in multi-object YAML documents anymore
+    """
+    line = line.strip()
+    # We don't care about context and metadata lines
+    if not (line.startswith("-") or line.startswith("+")):
+        return True
+    # Ignore changes where we don't emit a null object preceded or followed
+    # by a stream separator anymore
+    if line in ("-null", "----"):
+        return True
+    # Ignore changes which are only about Tiller -> Helm as object manager
+    if line.startswith("-") and (
+        line.endswith("app.kubernetes.io/managed-by: Tiller")
+        or line.endswith("heritage: Tiller")
+    ):
+        return True
+    if line.startswith("+") and (
+        line.endswith("app.kubernetes.io/managed-by: Helm")
+        or line.endswith("heritage: Helm")
+    ):
+        return True
+
+    return False
+
+
+def _kapitan_029_030_difffunc(
+    before_text: str, after_text: str, fromfile: str = "", tofile: str = ""
+) -> Tuple[Iterable[str], bool]:
+    before_objs = sorted(yaml.safe_load_all(before_text), key=K8sObject)
+    before_sorted_lines = yaml.dump_all(before_objs).split("\n")
+
+    after_objs = sorted(yaml.safe_load_all(after_text), key=K8sObject)
+    after_sorted_lines = yaml.dump_all(after_objs).split("\n")
+
+    diff = difflib.unified_diff(
+        before_sorted_lines,
+        after_sorted_lines,
+        lineterm="",
+        fromfile=fromfile,
+        tofile=tofile,
+    )
+    diff_lines = list(diff)
+    suppress_diff = all(
+        _ignore_kapitan_029_030_non_semantic(line) for line in diff_lines[2:]
+    )
+
+    return diff_lines, suppress_diff
+
+
 def update_catalog(cfg: Config, targets: Iterable[str], repo):
     click.secho("Updating catalog repository...", bold=True)
     # pylint: disable=import-outside-toplevel
@@ -122,7 +184,11 @@ def update_catalog(cfg: Config, targets: Iterable[str], repo):
     for target_name in targets:
         dir_util.copy_tree(str(cfg.inventory.output_dir / target_name), str(catalogdir))
 
-    difftext, changed = git.stage_all(repo)
+    if cfg.migration == Migration.KAP_029_030:
+        difftext, changed = git.stage_all(repo, diff_func=_kapitan_029_030_difffunc)
+    else:
+        difftext, changed = git.stage_all(repo)
+
     if changed:
         indented = textwrap.indent(difftext, "     ")
         message = f" > Changes:\n{indented}"

commodore/cli.py

@@ -5,7 +5,7 @@
 from dotenv import load_dotenv, find_dotenv
 from commodore import __git_version__, __version__
 from .catalog import catalog_list
-from .config import Config
+from .config import Config, Migration
 from .helpers import clean_working_tree
 from .compile import compile as _compile
 from .component.template import ComponentTemplater
@@ -130,6 +130,18 @@ def clean(config: Config, verbose):
     default=True,
     help="Whether to fetch Jsonnet and Kapitan dependencies in local mode. By default dependencies are fetched.",
 )
+@click.option(
+    "-m",
+    "--migration-diff-output",
+    help=(
+        "Specify a migration that you expect to happen for the cluster catalog. "
+        + "Currently Commodore only knows the Kapitan 0.29 to 0.30 migration. "
+        + "When the Kapitan 0.29 to 0.30 migration is selected, Commodore will suppress "
+        + "noise (changing managed-by labels, and reordered objects) caused by the "
+        + "migration in the diff output."
+    ),
+    type=click.Choice([m.value for m in Migration], case_sensitive=False),
+)
 @verbosity
 @pass_config
 # pylint: disable=too-many-arguments
@@ -147,6 +159,7 @@ def compile_catalog(
     global_repo_revision_override,
     tenant_repo_revision_override,
     fetch_dependencies,
+    migration_diff_output,
 ):
     config.update_verbosity(verbose)
     config.api_url = api_url
@@ -158,6 +171,7 @@ def compile_catalog(
     config.usermail = git_author_email
     config.global_repo_revision_override = global_repo_revision_override
     config.tenant_repo_revision_override = tenant_repo_revision_override
+    config.migration = migration_diff_output
     if config.push and (
         config.global_repo_revision_override or config.tenant_repo_revision_override
     ):

commodore/config.py

@@ -1,7 +1,8 @@
 import textwrap
 
+from enum import Enum
 from pathlib import Path as P
-from typing import Dict, List
+from typing import Dict, List, Optional
 
 import click
 from git import Repo
@@ -10,13 +11,18 @@
 from .inventory import Inventory
 
 
+class Migration(Enum):
+    KAP_029_030 = "kapitan-0.29-to-0.30"
+
+
 # pylint: disable=too-many-instance-attributes,too-many-public-methods
 class Config:
     _inventory: Inventory
     _components: Dict[str, Component]
     _config_repos: Dict[str, Repo]
     _component_aliases: Dict[str, str]
     _deprecation_notices: List[str]
+    _migration: Optional[Migration]
 
     # pylint: disable=too-many-arguments
     def __init__(
@@ -46,6 +52,7 @@ def __init__(
         self._deprecation_notices = []
         self._global_repo_revision_override = None
         self._tenant_repo_revision_override = None
+        self._migration = None
 
     @property
     def verbose(self):
@@ -124,6 +131,15 @@ def tenant_repo_revision_override(self):
     def tenant_repo_revision_override(self, rev):
         self._tenant_repo_revision_override = rev
 
+    @property
+    def migration(self):
+        return self._migration
+
+    @migration.setter
+    def migration(self, migration):
+        if migration and migration != "":
+            self._migration = Migration(migration)
+
     @property
     def inventory(self):
         return self._inventory

commodore/git.py

@@ -1,6 +1,9 @@
 import difflib
 import hashlib
 
+from typing import Iterable, List, Tuple
+from typing_extensions import Protocol
+
 import click
 
 from git import Repo, Actor
@@ -126,7 +129,63 @@ def _compute_similarity(change):
     return similarity_diff
 
 
-def stage_all(repo):
+class DiffFunc(Protocol):
+    def __call__(
+        self, before_text: str, after_text: str, fromfile: str = "", tofile: str = ""
+    ) -> Tuple[Iterable[str], bool]:
+        ...
+
+
+def _default_difffunc(
+    before_text: str, after_text: str, fromfile: str = "", tofile: str = ""
+) -> Tuple[Iterable[str], bool]:
+    before_lines = before_text.split("\n")
+    after_lines = after_text.split("\n")
+    diff_lines = difflib.unified_diff(
+        before_lines, after_lines, lineterm="", fromfile=fromfile, tofile=tofile
+    )
+    # never suppress diffs in default difffunc
+    return diff_lines, False
+
+
+def _process_diffs(
+    change_type: str, changes: Iterable, diff_func: DiffFunc
+) -> Iterable[str]:
+    difftext = []
+    for c in changes:
+        # Because we're diffing the staged changes, the diff objects
+        # are backwards, and "added" files are actually being deleted
+        # and vice versa for "deleted" files.
+        if change_type == "A":
+            difftext.append(click.style(f"Deleted file {c.b_path}", fg="red"))
+        elif change_type == "D":
+            difftext.append(click.style(f"Added file {c.b_path}", fg="green"))
+        elif change_type == "R":
+            difftext.append(
+                click.style(f"Renamed file {c.b_path} => {c.a_path}", fg="yellow")
+            )
+        else:
+            # Other changes should produce a usable diff
+            # The diff objects are backwards, so use b_blob as before
+            # and a_blob as after.
+            before = c.b_blob.data_stream.read().decode("utf-8")
+            after = c.a_blob.data_stream.read().decode("utf-8")
+            diff_lines, suppress_diff = diff_func(
+                before, after, fromfile=c.b_path, tofile=c.a_path
+            )
+            if not suppress_diff:
+                if c.renamed_file:
+                    # Just compute similarity ratio for renamed files
+                    # similar to git's diffing
+                    difftext.append("\n".join(_compute_similarity(c)).strip())
+                    continue
+                diff_lines = [_colorize_diff(line) for line in diff_lines]
+                difftext.append("\n".join(diff_lines).strip())
+
+    return difftext
+
+
+def stage_all(repo, diff_func: DiffFunc = _default_difffunc):
     index = repo.index
 
     # Stage deletions
@@ -149,40 +208,12 @@ def stage_all(repo):
         diff = index.diff(_NULL_TREE(repo))
 
     changed = False
-    difftext = []
+    difftext: List[str] = []
     if diff:
         changed = True
         for ct in diff.change_type:
-            for c in diff.iter_change_type(ct):
-                # Because we're diffing the staged changes, the diff objects
-                # are backwards, and "added" files are actually being deleted
-                # and vice versa for "deleted" files.
-                if ct == "A":
-                    difftext.append(click.style(f"Deleted file {c.b_path}", fg="red"))
-                elif ct == "D":
-                    difftext.append(click.style(f"Added file {c.b_path}", fg="green"))
-                elif ct == "R":
-                    difftext.append(
-                        click.style(
-                            f"Renamed file {c.b_path} => {c.a_path}", fg="yellow"
-                        )
-                    )
-                else:
-                    if c.renamed_file:
-                        # Just compute similarity ratio for renamed files
-                        # similar to git's diffing
-                        difftext.append("\n".join(_compute_similarity(c)).strip())
-                        continue
-                    # Other changes should produce a usable diff
-                    # The diff objects are backwards, so use b_blob as before
-                    # and a_blob as after.
-                    before = c.b_blob.data_stream.read().decode("utf-8").split("\n")
-                    after = c.a_blob.data_stream.read().decode("utf-8").split("\n")
-                    diff_lines = difflib.unified_diff(
-                        before, after, lineterm="", fromfile=c.b_path, tofile=c.a_path
-                    )
-                    diff_lines = [_colorize_diff(line) for line in diff_lines]
-                    difftext.append("\n".join(diff_lines).strip())
+            ct_difftext = _process_diffs(ct, diff.iter_change_type(ct), diff_func)
+            difftext.extend(ct_difftext)
 
     return "\n".join(difftext), changed
 

commodore/k8sobject.py

@@ -0,0 +1,38 @@
+class K8sObject:
+    def __init__(self, obj):
+        self._obj = {}
+        if obj:
+            self._obj = obj
+        self._kind = self._obj.get("kind", "")
+        self._name = self._obj.get("metadata", {}).get("name", "")
+        self._namespace = self._obj.get("metadata", {}).get("namespace", "")
+
+    def __lt__(self, other):
+        if self._kind != other._kind:
+            return self._kind < other._kind
+        if self._namespace != other._namespace:
+            return self._namespace < other._namespace
+        return self._name < other._name
+
+    def __gt__(self, other):
+        if self._kind != other._kind:
+            return self._kind > other._kind
+        if self._namespace != other._namespace:
+            return self._namespace > other._namespace
+        return self._name > other._name
+
+    def __eq__(self, other):
+        return (
+            self._kind == other._kind
+            and self._namespace == other._namespace
+            and self._name == other._name
+        )
+
+    def __le__(self, other):
+        return not self.__gt__(other)
+
+    def __ge__(self, other):
+        return not self.__lt__(other)
+
+    def __ne__(self, other):
+        return not self.__eq__(other)