From 5d030730255a132b7d0268623ceef0a395e56573 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sun, 10 Nov 2024 18:15:26 +0000 Subject: [PATCH 1/3] Strip attributes in List resources --- flux_local/tool/visitor.py | 25 +++-- tests/tool/test_visitor.py | 214 +++++++++++++++++++++++++++++++++++++ 2 files changed, 231 insertions(+), 8 deletions(-) create mode 100644 tests/tool/test_visitor.py diff --git a/flux_local/tool/visitor.py b/flux_local/tool/visitor.py index 12934337..38f9b20b 100644 --- a/flux_local/tool/visitor.py +++ b/flux_local/tool/visitor.py @@ -177,6 +177,21 @@ def update_manifest(self, manifest: Manifest) -> None: helm_release.images.sort() +def strip_resource_attributes(resource: dict[str, Any], strip_attributes: list[str]) -> None: + """Strip any annotations from kustomize that contribute to diff noise when objects are re-ordered in the output.""" + strip_attrs(resource["metadata"], strip_attributes) + # Remove common noisy labels in commonly used templates + if ( + (spec := resource.get("spec")) + and (templ := spec.get("template")) + and (meta := templ.get("metadata")) + ): + strip_attrs(meta, strip_attributes) + if resource["kind"] == "List" and (items := resource.get("items")): + for item in items: + strip_attrs(item["metadata"], strip_attributes) + + class ObjectOutput(ResourceOutput): """Resource visitor that builds outputs for objects within the kustomization.""" @@ -208,14 +223,8 @@ async def call_async( ) continue # Remove common noisy labels - strip_attrs(metadata, self.strip_attributes) - # Remove common noisy labels in commonly used templates - if ( - (spec := resource.get("spec")) - and (templ := spec.get("template")) - and (meta := templ.get("metadata")) - ): - strip_attrs(meta, self.strip_attributes) + strip_resource_attributes(resource, self.strip_attributes) + resource_key = ResourceKey( kind=kind, kustomization_path=str(kustomization_path), diff --git a/tests/tool/test_visitor.py b/tests/tool/test_visitor.py new file mode 100644 index 00000000..444cd2e3 --- /dev/null +++ b/tests/tool/test_visitor.py @@ -0,0 +1,214 @@ +"""Tests for the visitor module.""" + +import base64 +from pathlib import Path +from typing import Any +import yaml + +import pytest + +from flux_local.exceptions import HelmException +from flux_local.manifest import ( + HelmRelease, + ValuesReference, + Secret, + ConfigMap, + Kustomization, + HelmChart, + SubstituteReference, +) +from flux_local.git_repo import ResourceSelector, PathSelector, build_manifest +from flux_local.tool.visitor import strip_resource_attributes + +STRIP_ATTRIBUTES = [ + "app.kubernetes.io/version", + "chart", +] + +@pytest.mark.parametrize( + ("metadata", "expected_metadata"), + [ + ( + { + "labels": { + "app.kubernetes.io/version": "1.0.0", + "app.kubernetes.io/managed-by": "Helm", + } + }, + { + "labels": { + "app.kubernetes.io/managed-by": "Helm", + }, + } + ), + ( + { + "annotations": { + "app.kubernetes.io/version": "1.0.0", + "app.kubernetes.io/managed-by": "Helm", + } + }, + { + "annotations": { + "app.kubernetes.io/managed-by": "Helm", + }, + } + ), + ( + {}, + {}, + ), + ] +) +def test_strip_resource_attributes(metadata: dict[str, Any], expected_metadata: dict[str, Any]) -> None: + """Test the strip_resource_attributes function.""" + resource = { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "my-configmap", + "namespace": "default", + **metadata, + }, + "data": { + "key1": "value1", + "key2": "value2", + }, + } + strip_resource_attributes(resource, STRIP_ATTRIBUTES) + assert resource == { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "my-configmap", + "namespace": "default", + **expected_metadata, + }, + "data": { + "key1": "value1", + "key2": "value2", + }, + } + + +def test_strip_deployment_metadata() -> None: + """Test the strip_resource_attributes function.""" + resource = yaml.load("""apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + labels: + app: nginx +spec: + replicas: 3 + selector: + matchLabels: + app: nginx + template: + metadata: + labels: + app: nginx + app.kubernetes.io/version: 1.0.0 + spec: + containers: + - name: nginx + image: nginx:1.14.2 + ports: + - containerPort: 80 +""", Loader=yaml.Loader) + + strip_resource_attributes(resource, STRIP_ATTRIBUTES) + assert yaml.dump(resource, sort_keys=False) == """apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + labels: + app: nginx +spec: + replicas: 3 + selector: + matchLabels: + app: nginx + template: + metadata: + labels: + app: nginx + spec: + containers: + - name: nginx + image: nginx:1.14.2 + ports: + - containerPort: 80 +""" + + + +def test_strip_list_metadata() -> None: + """Test the stripping metadata from a list resource.""" + resource = yaml.load("""apiVersion: v1 +items: +- apiVersion: stable.example.com/v1 + kind: CronTab + metadata: + annotations: + app: my-cron-tab + app.kubernetes.io/version: 1.0.0 + creationTimestamp: '2021-06-20T07:35:27Z' + generation: 1 + name: my-new-cron-object + namespace: default + resourceVersion: '1326' + uid: 9aab1d66-628e-41bb-a422-57b8b3b1f5a9 + spec: + cronSpec: '* * * * */5' + image: my-awesome-cron-image +kind: List +metadata: + resourceVersion: '' + selfLink: '' + +""", Loader=yaml.Loader) + + strip_resource_attributes(resource, STRIP_ATTRIBUTES) + assert yaml.dump(resource, sort_keys=False) == """apiVersion: v1 +items: +- apiVersion: stable.example.com/v1 + kind: CronTab + metadata: + annotations: + app: my-cron-tab + creationTimestamp: '2021-06-20T07:35:27Z' + generation: 1 + name: my-new-cron-object + namespace: default + resourceVersion: '1326' + uid: 9aab1d66-628e-41bb-a422-57b8b3b1f5a9 + spec: + cronSpec: '* * * * */5' + image: my-awesome-cron-image +kind: List +metadata: + resourceVersion: '' + selfLink: '' +""" + + +def test_strip_list_corner_cases() -> None: + """Test corner cases of handling metadata.""" + resource = yaml.load("""apiVersion: v1 +kind: List +metadata: + resourceVersion: '' + selfLink: '' +items: + +""", Loader=yaml.Loader) + + strip_resource_attributes(resource, STRIP_ATTRIBUTES) + assert yaml.dump(resource, sort_keys=False) == """apiVersion: v1 +kind: List +metadata: + resourceVersion: '' + selfLink: '' +items: null +""" From 39bbdf0315e9e932dc80607a9dd4bbc69dfc6095 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sun, 10 Nov 2024 18:22:01 +0000 Subject: [PATCH 2/3] Remove unused imports --- tests/tool/test_visitor.py | 74 +++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/tests/tool/test_visitor.py b/tests/tool/test_visitor.py index 444cd2e3..a295a378 100644 --- a/tests/tool/test_visitor.py +++ b/tests/tool/test_visitor.py @@ -1,23 +1,11 @@ """Tests for the visitor module.""" -import base64 -from pathlib import Path from typing import Any import yaml import pytest -from flux_local.exceptions import HelmException -from flux_local.manifest import ( - HelmRelease, - ValuesReference, - Secret, - ConfigMap, - Kustomization, - HelmChart, - SubstituteReference, -) -from flux_local.git_repo import ResourceSelector, PathSelector, build_manifest + from flux_local.tool.visitor import strip_resource_attributes STRIP_ATTRIBUTES = [ @@ -25,6 +13,7 @@ "chart", ] + @pytest.mark.parametrize( ("metadata", "expected_metadata"), [ @@ -39,10 +28,10 @@ "labels": { "app.kubernetes.io/managed-by": "Helm", }, - } + }, ), ( - { + { "annotations": { "app.kubernetes.io/version": "1.0.0", "app.kubernetes.io/managed-by": "Helm", @@ -52,15 +41,17 @@ "annotations": { "app.kubernetes.io/managed-by": "Helm", }, - } + }, ), ( {}, {}, ), - ] + ], ) -def test_strip_resource_attributes(metadata: dict[str, Any], expected_metadata: dict[str, Any]) -> None: +def test_strip_resource_attributes( + metadata: dict[str, Any], expected_metadata: dict[str, Any] +) -> None: """Test the strip_resource_attributes function.""" resource = { "apiVersion": "v1", @@ -93,7 +84,8 @@ def test_strip_resource_attributes(metadata: dict[str, Any], expected_metadata: def test_strip_deployment_metadata() -> None: """Test the strip_resource_attributes function.""" - resource = yaml.load("""apiVersion: apps/v1 + resource = yaml.load( + """apiVersion: apps/v1 kind: Deployment metadata: name: nginx-deployment @@ -115,10 +107,14 @@ def test_strip_deployment_metadata() -> None: image: nginx:1.14.2 ports: - containerPort: 80 -""", Loader=yaml.Loader) - +""", + Loader=yaml.Loader, + ) + strip_resource_attributes(resource, STRIP_ATTRIBUTES) - assert yaml.dump(resource, sort_keys=False) == """apiVersion: apps/v1 + assert ( + yaml.dump(resource, sort_keys=False) + == """apiVersion: apps/v1 kind: Deployment metadata: name: nginx-deployment @@ -140,12 +136,13 @@ def test_strip_deployment_metadata() -> None: ports: - containerPort: 80 """ - + ) def test_strip_list_metadata() -> None: """Test the stripping metadata from a list resource.""" - resource = yaml.load("""apiVersion: v1 + resource = yaml.load( + """apiVersion: v1 items: - apiVersion: stable.example.com/v1 kind: CronTab @@ -166,11 +163,15 @@ def test_strip_list_metadata() -> None: metadata: resourceVersion: '' selfLink: '' - -""", Loader=yaml.Loader) - + +""", + Loader=yaml.Loader, + ) + strip_resource_attributes(resource, STRIP_ATTRIBUTES) - assert yaml.dump(resource, sort_keys=False) == """apiVersion: v1 + assert ( + yaml.dump(resource, sort_keys=False) + == """apiVersion: v1 items: - apiVersion: stable.example.com/v1 kind: CronTab @@ -191,24 +192,31 @@ def test_strip_list_metadata() -> None: resourceVersion: '' selfLink: '' """ + ) def test_strip_list_corner_cases() -> None: """Test corner cases of handling metadata.""" - resource = yaml.load("""apiVersion: v1 + resource = yaml.load( + """apiVersion: v1 kind: List metadata: resourceVersion: '' selfLink: '' items: - -""", Loader=yaml.Loader) - + +""", + Loader=yaml.Loader, + ) + strip_resource_attributes(resource, STRIP_ATTRIBUTES) - assert yaml.dump(resource, sort_keys=False) == """apiVersion: v1 + assert ( + yaml.dump(resource, sort_keys=False) + == """apiVersion: v1 kind: List metadata: resourceVersion: '' selfLink: '' items: null """ + ) From 7e2c6c39adab33c1e283673a74cd8b756c1c00a6 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sun, 10 Nov 2024 18:25:21 +0000 Subject: [PATCH 3/3] Harden item handling --- flux_local/tool/visitor.py | 6 ++++-- tests/tool/test_visitor.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/flux_local/tool/visitor.py b/flux_local/tool/visitor.py index 38f9b20b..009f1e09 100644 --- a/flux_local/tool/visitor.py +++ b/flux_local/tool/visitor.py @@ -187,9 +187,11 @@ def strip_resource_attributes(resource: dict[str, Any], strip_attributes: list[s and (meta := templ.get("metadata")) ): strip_attrs(meta, strip_attributes) - if resource["kind"] == "List" and (items := resource.get("items")): + if resource["kind"] == "List" and (items := resource.get("items")) and isinstance(items, list): for item in items: - strip_attrs(item["metadata"], strip_attributes) + if not (item_meta := item.get("metadata")): + continue + strip_attrs(item_meta, strip_attributes) class ObjectOutput(ResourceOutput): diff --git a/tests/tool/test_visitor.py b/tests/tool/test_visitor.py index a295a378..81cbc22a 100644 --- a/tests/tool/test_visitor.py +++ b/tests/tool/test_visitor.py @@ -195,7 +195,7 @@ def test_strip_list_metadata() -> None: ) -def test_strip_list_corner_cases() -> None: +def test_strip_list_null_items() -> None: """Test corner cases of handling metadata.""" resource = yaml.load( """apiVersion: v1 @@ -220,3 +220,31 @@ def test_strip_list_corner_cases() -> None: items: null """ ) + + +def test_strip_list_item_without_metdata() -> None: + """Test corner cases of handling metadata.""" + resource = yaml.load( + """apiVersion: v1 +kind: List +metadata: + resourceVersion: '' + selfLink: '' +items: +- kind: CronTab +""", + Loader=yaml.Loader, + ) + + strip_resource_attributes(resource, STRIP_ATTRIBUTES) + assert ( + yaml.dump(resource, sort_keys=False) + == """apiVersion: v1 +kind: List +metadata: + resourceVersion: '' + selfLink: '' +items: +- kind: CronTab +""" + )