From f3e10d8af331e92d889a12189b864a90194167b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Valais?= <mael@vls.dev>
Date: Fri, 8 Nov 2024 16:33:21 +0100
Subject: [PATCH 1/6] annot-exclusion: it is now possible to exclude labels and
 annotations

---
 .../charts/venafi-kubernetes-agent/README.md  |  20 +++
 .../templates/configmap.yaml                  |   4 +
 .../values.schema.json                        |  17 ++
 .../venafi-kubernetes-agent/values.yaml       |  17 ++
 pkg/agent/config.go                           |  32 +++-
 pkg/agent/run.go                              |   7 +
 pkg/datagatherer/k8s/dynamic.go               |  63 ++++++-
 pkg/datagatherer/k8s/dynamic_test.go          | 158 ++++++++++++------
 8 files changed, 266 insertions(+), 52 deletions(-)

diff --git a/deploy/charts/venafi-kubernetes-agent/README.md b/deploy/charts/venafi-kubernetes-agent/README.md
index 8064c0bb..8ad5a8d2 100644
--- a/deploy/charts/venafi-kubernetes-agent/README.md
+++ b/deploy/charts/venafi-kubernetes-agent/README.md
@@ -423,6 +423,26 @@ Control Plane.
 > ```yaml
 > helm.sh/release.v1
 > ```
+#### **config.excludeAnnotationKeysRegex** ~ `array`
+> Default value:
+> ```yaml
+> []
+> ```
+
+You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.  
+  
+If you would like to exclude annotations keys that contain the word  
+`secret`, use the regular expression `.*secret.*`. The leading and ending .*  
+are important if you want to filter out keys that contain `secret` anywhere in the key string.  
+  
+Note that the annotation `kubectl.kubernetes.io/last-applied-configuration` is already excluded by default, you don't need to exclude it explicitly.  
+  
+Example: excludeAnnotationKeysRegex: [".*secret.*"]
+#### **config.excludeLabelKeysRegex** ~ `array`
+> Default value:
+> ```yaml
+> []
+> ```
 #### **config.configmap.name** ~ `unknown`
 > Default value:
 > ```yaml
diff --git a/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml b/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml
index af236b40..9942e21f 100644
--- a/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml
+++ b/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml
@@ -13,6 +13,10 @@ data:
     cluster_description: {{ .Values.config.clusterDescription | quote }}
     server: {{ .Values.config.server | quote }}
     period: {{ .Values.config.period | quote }}
+    exclude-annotation-keys-regex:
+      {{ .Values.config.excludeAnnotationKeysRegex | nindent 6 }}
+    exclude-label-keys-regex:
+      {{ .Values.config.excludeLabelKeysRegex | nindent 6 }}
     venafi-cloud:
       uploader_id: "no"
       upload_path: "/v1/tlspk/upload/clusterdata"
diff --git a/deploy/charts/venafi-kubernetes-agent/values.schema.json b/deploy/charts/venafi-kubernetes-agent/values.schema.json
index a6e2c0b5..7e0228fe 100644
--- a/deploy/charts/venafi-kubernetes-agent/values.schema.json
+++ b/deploy/charts/venafi-kubernetes-agent/values.schema.json
@@ -165,6 +165,12 @@
         "configmap": {
           "$ref": "#/$defs/helm-values.config.configmap"
         },
+        "excludeAnnotationKeysRegex": {
+          "$ref": "#/$defs/helm-values.config.excludeAnnotationKeysRegex"
+        },
+        "excludeLabelKeysRegex": {
+          "$ref": "#/$defs/helm-values.config.excludeLabelKeysRegex"
+        },
         "ignoredSecretTypes": {
           "$ref": "#/$defs/helm-values.config.ignoredSecretTypes"
         },
@@ -206,6 +212,17 @@
     },
     "helm-values.config.configmap.key": {},
     "helm-values.config.configmap.name": {},
+    "helm-values.config.excludeAnnotationKeysRegex": {
+      "default": [],
+      "description": "You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.\n\nIf you would like to exclude annotations keys that contain the word\n`secret`, use the regular expression `.*secret.*`. The leading and ending .*\nare important if you want to filter out keys that contain `secret` anywhere in the key string.\n\nNote that the annotation `kubectl.kubernetes.io/last-applied-configuration` is already excluded by default, you don't need to exclude it explicitly.\n\nExample: excludeAnnotationKeysRegex: [\".*secret.*\"]",
+      "items": {},
+      "type": "array"
+    },
+    "helm-values.config.excludeLabelKeysRegex": {
+      "default": [],
+      "items": {},
+      "type": "array"
+    },
     "helm-values.config.ignoredSecretTypes": {
       "items": {
         "$ref": "#/$defs/helm-values.config.ignoredSecretTypes[0]"
diff --git a/deploy/charts/venafi-kubernetes-agent/values.yaml b/deploy/charts/venafi-kubernetes-agent/values.yaml
index b0fc45ef..093cc90f 100644
--- a/deploy/charts/venafi-kubernetes-agent/values.yaml
+++ b/deploy/charts/venafi-kubernetes-agent/values.yaml
@@ -238,6 +238,23 @@ config:
   - bootstrap.kubernetes.io/token
   - helm.sh/release.v1
 
+  # You can configure Venafi Kubernetes Agent to exclude some annotations or
+  # labels from being pushed to the Venafi Control Plane. All Kubernetes objects
+  # are affected. The objects are still pushed, but the specified annotations
+  # and labels are removed before being sent to the Venafi Control Plane.
+  #
+  # If you would like to exclude annotations keys that contain the word
+  # `secret`, use the regular expression `.*secret.*`. The leading and ending .*
+  # are important if you want to filter out keys that contain `secret` anywhere
+  # in the key string.
+  #
+  # Note that the annotation `kubectl.kubernetes.io/last-applied-configuration`
+  # is already excluded by default, you don't need to exclude it explicitly.
+  #
+  # Example: excludeAnnotationKeysRegex: [".*secret.*"]
+  excludeAnnotationKeysRegex: []
+  excludeLabelKeysRegex: []
+
   # Specify ConfigMap details to load config from an existing resource.
   # This should be blank by default unless you have you own config.
   configmap:
diff --git a/pkg/agent/config.go b/pkg/agent/config.go
index 5bccf1d9..e413a313 100644
--- a/pkg/agent/config.go
+++ b/pkg/agent/config.go
@@ -5,6 +5,7 @@ import (
 	"io"
 	"net/url"
 	"os"
+	"regexp"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -54,6 +55,12 @@ type Config struct {
 	InputPath string `yaml:"input-path"`
 	// For testing purposes.
 	OutputPath string `yaml:"output-path"`
+
+	// Skips annotation keys that match the given set of regular expressions.
+	// Example: ".*someprivateannotation.*".
+	ExcludeAnnotationKeysRegex []string `yaml:"exclude-annotation-keys-regex"`
+	// Skips label keys that match the given set of regular expressions.
+	ExcludeLabelKeysRegex []string `yaml:"exclude-label-keys-regex"`
 }
 
 type Endpoint struct {
@@ -339,7 +346,9 @@ type CombinedConfig struct {
 	VenConnNS   string
 
 	// VenafiCloudKeypair and VenafiCloudVenafiConnection modes only.
-	DisableCompression bool
+	DisableCompression         bool
+	ExcludeAnnotationKeysRegex []*regexp.Regexp
+	ExcludeLabelKeysRegex      []*regexp.Regexp
 
 	// Only used for testing purposes.
 	OutputPath string
@@ -585,6 +594,27 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 		res.DisableCompression = flags.DisableCompression
 	}
 
+	// Validation of the config fields exclude_annotation_keys_regex and
+	// exclude_label_keys_regex.
+	{
+		for i, regex := range cfg.ExcludeAnnotationKeysRegex {
+			r, err := regexp.Compile(regex)
+			if err != nil {
+				errs = multierror.Append(errs, fmt.Errorf("invalid exclude_annotation_keys_regex[%d]: %w", i, err))
+				continue
+			}
+			res.ExcludeAnnotationKeysRegex = append(res.ExcludeAnnotationKeysRegex, r)
+		}
+		for i, regex := range cfg.ExcludeLabelKeysRegex {
+			r, err := regexp.Compile(regex)
+			if err != nil {
+				errs = multierror.Append(errs, fmt.Errorf("invalid exclude_label_keys_regex[%d]: %w", i, err))
+				continue
+			}
+			res.ExcludeLabelKeysRegex = append(res.ExcludeLabelKeysRegex, r)
+		}
+	}
+
 	if errs != nil {
 		return CombinedConfig{}, nil, errs
 	}
diff --git a/pkg/agent/run.go b/pkg/agent/run.go
index 0fa7730b..54832dbe 100644
--- a/pkg/agent/run.go
+++ b/pkg/agent/run.go
@@ -34,6 +34,7 @@ import (
 	"github.com/jetstack/preflight/api"
 	"github.com/jetstack/preflight/pkg/client"
 	"github.com/jetstack/preflight/pkg/datagatherer"
+	"github.com/jetstack/preflight/pkg/datagatherer/k8s"
 	"github.com/jetstack/preflight/pkg/kubeconfig"
 	"github.com/jetstack/preflight/pkg/logs"
 	"github.com/jetstack/preflight/pkg/version"
@@ -176,6 +177,12 @@ func Run(cmd *cobra.Command, args []string) (returnErr error) {
 			return fmt.Errorf("failed to instantiate %q data gatherer  %q: %v", kind, dgConfig.Name, err)
 		}
 
+		dynDg, isDynamicGatherer := newDg.(*k8s.DataGathererDynamic)
+		if isDynamicGatherer {
+			dynDg.ExcludeAnnotKeys = config.ExcludeAnnotationKeysRegex
+			dynDg.ExcludeLabelKeys = config.ExcludeLabelKeysRegex
+		}
+
 		log.V(logs.Debug).Info("Starting DataGatherer", "name", dgConfig.Name)
 
 		// start the data gatherers and wait for the cache sync
diff --git a/pkg/datagatherer/k8s/dynamic.go b/pkg/datagatherer/k8s/dynamic.go
index 68753648..6636c7ea 100644
--- a/pkg/datagatherer/k8s/dynamic.go
+++ b/pkg/datagatherer/k8s/dynamic.go
@@ -3,6 +3,7 @@ package k8s
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -260,6 +261,9 @@ type DataGathererDynamic struct {
 	// informer watches the events around the targeted resource and updates the cache
 	informer     k8scache.SharedIndexInformer
 	registration k8scache.ResourceEventHandlerRegistration
+
+	ExcludeAnnotKeys []*regexp.Regexp
+	ExcludeLabelKeys []*regexp.Regexp
 }
 
 // Run starts the dynamic data gatherer's informers for resource collection.
@@ -338,7 +342,7 @@ func (g *DataGathererDynamic) Fetch() (interface{}, int, error) {
 	}
 
 	// Redact Secret data
-	err := redactList(items)
+	err := redactList(items, g.ExcludeAnnotKeys, g.ExcludeLabelKeys)
 	if err != nil {
 		return nil, -1, errors.WithStack(err)
 	}
@@ -349,7 +353,7 @@ func (g *DataGathererDynamic) Fetch() (interface{}, int, error) {
 	return list, len(items), nil
 }
 
-func redactList(list []*api.GatheredResource) error {
+func redactList(list []*api.GatheredResource, excludeAnnotKeys, excludeLabelKeys []*regexp.Regexp) error {
 	for i := range list {
 		if item, ok := list[i].Resource.(*unstructured.Unstructured); ok {
 			// Determine the kind of items in case this is a generic 'mixed' list.
@@ -374,6 +378,43 @@ func redactList(list []*api.GatheredResource) error {
 
 			// remove managedFields from all resources
 			Redact(RedactFields, resource)
+
+			annotsRaw, ok, err := unstructured.NestedFieldNoCopy(resource.Object, "metadata", "annotations")
+			if err != nil {
+				return fmt.Errorf("wasn't able to find the metadata.annotations field: %w", err)
+			}
+			if ok {
+				annots, ok := annotsRaw.(map[string]interface{})
+				if !ok {
+					return fmt.Errorf("metadata.annotations isn't a map on the resource %s in namespace %s: %w", resource.GetName(), resource.GetNamespace(), err)
+				}
+				for key := range annots {
+					for _, excludeAnnotKey := range excludeAnnotKeys {
+						if excludeAnnotKey.MatchString(key) {
+							delete(annots, key)
+						}
+					}
+				}
+			}
+
+			labelsRaw, ok, err := unstructured.NestedFieldNoCopy(resource.Object, "metadata", "labels")
+			if err != nil {
+				return fmt.Errorf("wasn't able to find the metadata.labels field for the resource %s in namespace %s: %w", resource.GetName(), resource.GetNamespace(), err)
+			}
+			if ok {
+				labels, ok := labelsRaw.(map[string]interface{})
+				if !ok {
+					return fmt.Errorf("metadata.labels isn't a map on the resource %s in namespace %s: %w", resource.GetName(), resource.GetNamespace(), err)
+				}
+				for key := range labels {
+					for _, excludeLabelKey := range excludeLabelKeys {
+						if excludeLabelKey.MatchString(key) {
+							delete(labels, key)
+						}
+					}
+				}
+			}
+
 			continue
 		}
 
@@ -386,6 +427,24 @@ func redactList(list []*api.GatheredResource) error {
 			item.GetObjectMeta().SetManagedFields(nil)
 			delete(item.GetObjectMeta().GetAnnotations(), "kubectl.kubernetes.io/last-applied-configuration")
 
+			annots := item.GetObjectMeta().GetAnnotations()
+			for key := range annots {
+				for _, excludeAnnotKey := range excludeAnnotKeys {
+					if excludeAnnotKey.MatchString(key) {
+						delete(annots, key)
+					}
+				}
+			}
+
+			labels := item.GetObjectMeta().GetLabels()
+			for key := range labels {
+				for _, excludeLabelKey := range excludeLabelKeys {
+					if excludeLabelKey.MatchString(key) {
+						delete(labels, key)
+					}
+				}
+			}
+
 			resource := item.(runtime.Object)
 			gvks, _, err := scheme.Scheme.ObjectKinds(resource)
 			if err != nil {
diff --git a/pkg/datagatherer/k8s/dynamic_test.go b/pkg/datagatherer/k8s/dynamic_test.go
index d64bc876..8cbdb5ce 100644
--- a/pkg/datagatherer/k8s/dynamic_test.go
+++ b/pkg/datagatherer/k8s/dynamic_test.go
@@ -5,13 +5,15 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
+	"regexp"
 	"sort"
 	"strings"
 	"sync"
 	"testing"
 	"time"
 
-	"github.com/d4l3k/messagediff"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v2"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -52,6 +54,19 @@ func getObject(version, kind, name, namespace string, withManagedFields bool) *u
 	}
 }
 
+func getObjectAnnot(version, kind, name, namespace string, annotations, labels map[string]interface{}) *unstructured.Unstructured {
+	obj := getObject(version, kind, name, namespace, false)
+
+	metadata, _ := obj.Object["metadata"].(map[string]interface{})
+	if annotations == nil {
+		annotations = make(map[string]interface{})
+	}
+	metadata["annotations"] = annotations
+	metadata["labels"] = labels
+
+	return obj
+}
+
 func getSecret(name, namespace string, data map[string]interface{}, isTLS bool, withLastApplied bool) *unstructured.Unstructured {
 	object := getObject("v1", "Secret", name, namespace, false)
 
@@ -367,12 +382,14 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 	// check the expected result
 	emptyScheme := runtime.NewScheme()
 	tests := map[string]struct {
-		config        ConfigDynamic
-		addObjects    []runtime.Object
-		deleteObjects map[string]string
-		updateObjects map[string]runtime.Object
-		expected      []*api.GatheredResource
-		err           bool
+		config            ConfigDynamic
+		excludeAnnotsKeys []string
+		excludeLabelKeys  []string
+		addObjects        []runtime.Object
+		deleteObjects     map[string]string
+		updateObjects     map[string]runtime.Object
+		expected          []*api.GatheredResource
+		err               bool
 	}{
 		"fetches the default namespace": {
 			addObjects: []runtime.Object{
@@ -582,6 +599,32 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 				},
 			},
 		},
+		"excluded annotations are removed on secrets and CRDs": {
+			config:            ConfigDynamic{GroupVersionResource: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}},
+			excludeAnnotsKeys: []string{".*secret.*"},
+			addObjects: []runtime.Object{
+				getObjectAnnot("v1", "Secret", "s0", "n1", map[string]interface{}{"normal-annot": "value"}, nil),
+				getObjectAnnot("v1", "Secret", "s1", "n1", nil, map[string]interface{}{"normal-label": "value"}),
+				getObjectAnnot("v1", "Secret", "s2", "n1", map[string]interface{}{"super-secret-annot": "value"}, nil),
+				getObjectAnnot("v1", "Secret", "s3", "n1", nil, map[string]interface{}{"super-secret-label": "value"}),
+
+				getObjectAnnot("route.openshift.io/v1", "Route", "r0", "n1", map[string]interface{}{"normal-annot": "value"}, nil),
+				getObjectAnnot("route.openshift.io/v1", "Route", "r1", "n1", nil, map[string]interface{}{"normal-label": "value"}),
+				getObjectAnnot("route.openshift.io/v1", "Route", "r2", "n1", map[string]interface{}{"super-secret-annot": "value"}, nil),
+				getObjectAnnot("route.openshift.io/v1", "Route", "r3", "n1", nil, map[string]interface{}{"super-secret-label": "value"}),
+			},
+			expected: []*api.GatheredResource{
+				{Resource: getObjectAnnot("v1", "Secret", "s0", "n1", map[string]interface{}{"normal-annot": "value"}, nil)},
+				{Resource: getObjectAnnot("v1", "Secret", "s1", "n1", nil, map[string]interface{}{"normal-label": "value"})},
+				{Resource: getObjectAnnot("v1", "Secret", "s2", "n1", nil, nil)},
+				{Resource: getObjectAnnot("v1", "Secret", "s3", "n1", nil, nil)},
+
+				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r0", "n1", map[string]interface{}{"normal-annot": "value"}, nil)},
+				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r1", "n1", nil, map[string]interface{}{"normal-label": "value"})},
+				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r2", "n1", nil, nil)},
+				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r3", "n1", nil, nil)},
+			},
+		},
 	}
 
 	for name, tc := range tests {
@@ -622,6 +665,14 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 			factory.Start(ctx.Done())
 			k8scache.WaitForCacheSync(ctx.Done(), testInformer.HasSynced)
 
+			dgd := dg.(*DataGathererDynamic)
+			for _, key := range tc.excludeAnnotsKeys {
+				dgd.ExcludeAnnotKeys = append(dgd.ExcludeAnnotKeys, regexp.MustCompile(key))
+			}
+			for _, key := range tc.excludeLabelKeys {
+				dgd.ExcludeLabelKeys = append(dgd.ExcludeLabelKeys, regexp.MustCompile(key))
+			}
+
 			// start data gatherer informer
 			dynamiDg := dg
 			go func() {
@@ -662,7 +713,7 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 			if waitTimeout(&wg, 30*time.Second) {
 				t.Fatalf("unexpected timeout")
 			}
-			res, count, err := dynamiDg.Fetch()
+			res, expectCount, err := dynamiDg.Fetch()
 			if err != nil && !tc.err {
 				t.Errorf("expected no error but got: %v", err)
 			}
@@ -685,16 +736,8 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 				// sorting list of expected results by name
 				sortGatheredResources(tc.expected)
 
-				if diff, equal := messagediff.PrettyDiff(tc.expected, list); !equal {
-					t.Errorf("\n%s", diff)
-					expectedJSON, _ := json.MarshalIndent(tc.expected, "", "  ")
-					gotJSON, _ := json.MarshalIndent(list, "", "  ")
-					t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(gotJSON), expectedJSON)
-				}
-
-				if len(list) != count {
-					t.Errorf("wrong count of resources reported: got %d, want %d", count, len(list))
-				}
+				assert.Equal(t, tc.expected, list)
+				assert.Len(t, list, expectCount, "unexpected number of resources returned")
 			}
 		})
 	}
@@ -707,12 +750,14 @@ func TestDynamicGathererNativeResources_Fetch(t *testing.T) {
 	// check the expected result
 	podGVR := schema.GroupVersionResource{Group: corev1.SchemeGroupVersion.Group, Version: corev1.SchemeGroupVersion.Version, Resource: "pods"}
 	tests := map[string]struct {
-		config        ConfigDynamic
-		addObjects    []runtime.Object
-		deleteObjects map[string]string
-		updateObjects map[string]runtime.Object
-		expected      []*api.GatheredResource
-		err           bool
+		config            ConfigDynamic
+		excludeAnnotsKeys []string
+		excludeLabelKeys  []string
+		addObjects        []runtime.Object
+		deleteObjects     map[string]string
+		updateObjects     map[string]runtime.Object
+		expected          []*api.GatheredResource
+		err               bool
 	}{
 		"only a Pod should be returned if GVR selects pods": {
 			addObjects: []runtime.Object{
@@ -878,6 +923,27 @@ func TestDynamicGathererNativeResources_Fetch(t *testing.T) {
 				},
 			},
 		},
+		// Pod is the only native resource that we test out of lack of time
+		// (would require a lot of changes to the testing func). Ideally we
+		// should test all native resources such as Service, Deployment,
+		// Ingress, Namespace, and so on.
+		"excluded annotations are removed native resources: pods, namespaces, etc": {
+			config:            ConfigDynamic{GroupVersionResource: podGVR},
+			excludeAnnotsKeys: []string{"secret"},
+			excludeLabelKeys:  []string{"secret"},
+			addObjects: []runtime.Object{
+				&corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p0", UID: "p0", Namespace: "n1", Annotations: map[string]string{"normal-annot": "bar"}}},
+				&corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", UID: "p1", Namespace: "n1", Labels: map[string]string{"normal-label": "bar"}}},
+				&corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2", UID: "p2", Namespace: "n1", Annotations: map[string]string{"super-secret-annot": "bar"}}},
+				&corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p3", UID: "p3", Namespace: "n1", Labels: map[string]string{"super-secret-label": "bar"}}},
+			},
+			expected: []*api.GatheredResource{
+				{Resource: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p0", UID: "p0", Namespace: "n1", Annotations: map[string]string{"normal-annot": "bar"}}, TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}}},
+				{Resource: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", UID: "p1", Namespace: "n1", Labels: map[string]string{"normal-label": "bar"}}, TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}}},
+				{Resource: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2", UID: "p2", Namespace: "n1", Annotations: map[string]string{}}, TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}}},
+				{Resource: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p3", UID: "p3", Namespace: "n1", Labels: map[string]string{}}, TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}}},
+			},
+		},
 	}
 
 	for name, tc := range tests {
@@ -916,6 +982,13 @@ func TestDynamicGathererNativeResources_Fetch(t *testing.T) {
 			//start test Informer
 			factory.Start(ctx.Done())
 			k8scache.WaitForCacheSync(ctx.Done(), testInformer.HasSynced)
+			dgd := dg.(*DataGathererDynamic)
+			for _, key := range tc.excludeAnnotsKeys {
+				dgd.ExcludeAnnotKeys = append(dgd.ExcludeAnnotKeys, regexp.MustCompile(key))
+			}
+			for _, key := range tc.excludeLabelKeys {
+				dgd.ExcludeLabelKeys = append(dgd.ExcludeLabelKeys, regexp.MustCompile(key))
+			}
 
 			// start data gatherer informer
 			dynamiDg := dg
@@ -956,39 +1029,26 @@ func TestDynamicGathererNativeResources_Fetch(t *testing.T) {
 			if waitTimeout(&wg, 5*time.Second) {
 				t.Fatalf("unexpected timeout")
 			}
-			res, count, err := dynamiDg.Fetch()
-			if err != nil && !tc.err {
-				t.Errorf("expected no error but got: %v", err)
-			}
-			if err == nil && tc.err {
-				t.Errorf("expected to get an error but didn't get one")
+			rawRes, count, err := dynamiDg.Fetch()
+			if tc.err {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
 			}
 
 			if tc.expected != nil {
-				items, ok := res.(map[string]interface{})
-				if !ok {
-					t.Errorf("expected result be an map[string]interface{} but wasn't")
-				}
+				res, ok := rawRes.(map[string]interface{})
+				require.Truef(t, ok, "expected result be an map[string]interface{} but wasn't")
+				actual := res["items"].([]*api.GatheredResource)
+				require.Truef(t, ok, "expected result be an []*api.GatheredResource but wasn't")
 
-				list, ok := items["items"].([]*api.GatheredResource)
-				if !ok {
-					t.Errorf("expected result be an []*api.GatheredResource but wasn't")
-				}
 				// sorting list of results by name
-				sortGatheredResources(list)
+				sortGatheredResources(actual)
 				// sorting list of expected results by name
 				sortGatheredResources(tc.expected)
 
-				if diff, equal := messagediff.PrettyDiff(tc.expected, list); !equal {
-					t.Errorf("\n%s", diff)
-					expectedJSON, _ := json.MarshalIndent(tc.expected, "", "  ")
-					gotJSON, _ := json.MarshalIndent(list, "", "  ")
-					t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(gotJSON), expectedJSON)
-				}
-
-				if len(list) != count {
-					t.Errorf("wrong count of resources reported: got %d, want %d", count, len(list))
-				}
+				assert.Equal(t, tc.expected, actual)
+				assert.Len(t, actual, count)
 			}
 		})
 	}

From 5ad0b72b0f1c06f84ceb99fda6402110b3238c33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Valais?= <mael@vls.dev>
Date: Tue, 12 Nov 2024 16:53:40 +0100
Subject: [PATCH 2/6] annot-exclusion: helm template failing due to missing
 toYaml

---
 .../charts/venafi-kubernetes-agent/templates/configmap.yaml   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml b/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml
index 9942e21f..18927824 100644
--- a/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml
+++ b/deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml
@@ -14,9 +14,9 @@ data:
     server: {{ .Values.config.server | quote }}
     period: {{ .Values.config.period | quote }}
     exclude-annotation-keys-regex:
-      {{ .Values.config.excludeAnnotationKeysRegex | nindent 6 }}
+      {{ .Values.config.excludeAnnotationKeysRegex | toYaml | nindent 6 }}
     exclude-label-keys-regex:
-      {{ .Values.config.excludeLabelKeysRegex | nindent 6 }}
+      {{ .Values.config.excludeLabelKeysRegex | toYaml | nindent 6 }}
     venafi-cloud:
       uploader_id: "no"
       upload_path: "/v1/tlspk/upload/clusterdata"

From a20e5a373a8de42e19f97e2d4a7dd70ffdea4f65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Valais?= <mael@vls.dev>
Date: Tue, 12 Nov 2024 19:33:24 +0100
Subject: [PATCH 3/6] annot-exclusion: fix unit test and use a realistic regex
 in tests

---
 .../charts/venafi-kubernetes-agent/README.md  |  6 +-
 .../values.schema.json                        |  2 +-
 .../venafi-kubernetes-agent/values.yaml       | 10 +--
 pkg/datagatherer/k8s/dynamic_test.go          | 73 +++++++++++--------
 4 files changed, 49 insertions(+), 42 deletions(-)

diff --git a/deploy/charts/venafi-kubernetes-agent/README.md b/deploy/charts/venafi-kubernetes-agent/README.md
index 8ad5a8d2..f148b8f8 100644
--- a/deploy/charts/venafi-kubernetes-agent/README.md
+++ b/deploy/charts/venafi-kubernetes-agent/README.md
@@ -431,13 +431,11 @@ Control Plane.
 
 You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.  
   
-If you would like to exclude annotations keys that contain the word  
-`secret`, use the regular expression `.*secret.*`. The leading and ending .*  
-are important if you want to filter out keys that contain `secret` anywhere in the key string.  
+If you would like to exclude annotations keys that contain the word `word`, use the regular expression `.*word.*`. The leading and ending .* are important if you want to filter out keys that contain `word` anywhere in the key string.  
   
 Note that the annotation `kubectl.kubernetes.io/last-applied-configuration` is already excluded by default, you don't need to exclude it explicitly.  
   
-Example: excludeAnnotationKeysRegex: [".*secret.*"]
+Example: excludeAnnotationKeysRegex: ["kapp\.k14s\.io\/original.*"]
 #### **config.excludeLabelKeysRegex** ~ `array`
 > Default value:
 > ```yaml
diff --git a/deploy/charts/venafi-kubernetes-agent/values.schema.json b/deploy/charts/venafi-kubernetes-agent/values.schema.json
index 7e0228fe..b4b755f1 100644
--- a/deploy/charts/venafi-kubernetes-agent/values.schema.json
+++ b/deploy/charts/venafi-kubernetes-agent/values.schema.json
@@ -214,7 +214,7 @@
     "helm-values.config.configmap.name": {},
     "helm-values.config.excludeAnnotationKeysRegex": {
       "default": [],
-      "description": "You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.\n\nIf you would like to exclude annotations keys that contain the word\n`secret`, use the regular expression `.*secret.*`. The leading and ending .*\nare important if you want to filter out keys that contain `secret` anywhere in the key string.\n\nNote that the annotation `kubectl.kubernetes.io/last-applied-configuration` is already excluded by default, you don't need to exclude it explicitly.\n\nExample: excludeAnnotationKeysRegex: [\".*secret.*\"]",
+      "description": "You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.\n\nIf you would like to exclude annotations keys that contain the word `word`, use the regular expression `.*word.*`. The leading and ending .* are important if you want to filter out keys that contain `word` anywhere in the key string.\n\nNote that the annotation `kubectl.kubernetes.io/last-applied-configuration` is already excluded by default, you don't need to exclude it explicitly.\n\nExample: excludeAnnotationKeysRegex: [\"kapp\\.k14s\\.io\\/original.*\"]",
       "items": {},
       "type": "array"
     },
diff --git a/deploy/charts/venafi-kubernetes-agent/values.yaml b/deploy/charts/venafi-kubernetes-agent/values.yaml
index 093cc90f..cdd24794 100644
--- a/deploy/charts/venafi-kubernetes-agent/values.yaml
+++ b/deploy/charts/venafi-kubernetes-agent/values.yaml
@@ -243,15 +243,15 @@ config:
   # are affected. The objects are still pushed, but the specified annotations
   # and labels are removed before being sent to the Venafi Control Plane.
   #
-  # If you would like to exclude annotations keys that contain the word
-  # `secret`, use the regular expression `.*secret.*`. The leading and ending .*
-  # are important if you want to filter out keys that contain `secret` anywhere
-  # in the key string.
+  # If you would like to exclude annotations keys that contain the word `word`,
+  # use the regular expression `.*word.*`. The leading and ending .* are
+  # important if you want to filter out keys that contain `word` anywhere in the
+  # key string.
   #
   # Note that the annotation `kubectl.kubernetes.io/last-applied-configuration`
   # is already excluded by default, you don't need to exclude it explicitly.
   #
-  # Example: excludeAnnotationKeysRegex: [".*secret.*"]
+  # Example: excludeAnnotationKeysRegex: ["kapp\.k14s\.io\/original.*"]
   excludeAnnotationKeysRegex: []
   excludeLabelKeysRegex: []
 
diff --git a/pkg/datagatherer/k8s/dynamic_test.go b/pkg/datagatherer/k8s/dynamic_test.go
index 8cbdb5ce..d925fae0 100644
--- a/pkg/datagatherer/k8s/dynamic_test.go
+++ b/pkg/datagatherer/k8s/dynamic_test.go
@@ -380,7 +380,6 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 	// init the datagatherer's informer with the client
 	// add/delete resources watched by the data gatherer
 	// check the expected result
-	emptyScheme := runtime.NewScheme()
 	tests := map[string]struct {
 		config            ConfigDynamic
 		excludeAnnotsKeys []string
@@ -599,31 +598,41 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 				},
 			},
 		},
-		"excluded annotations are removed on secrets and CRDs": {
-			config:            ConfigDynamic{GroupVersionResource: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}},
-			excludeAnnotsKeys: []string{".*secret.*"},
-			addObjects: []runtime.Object{
-				getObjectAnnot("v1", "Secret", "s0", "n1", map[string]interface{}{"normal-annot": "value"}, nil),
-				getObjectAnnot("v1", "Secret", "s1", "n1", nil, map[string]interface{}{"normal-label": "value"}),
-				getObjectAnnot("v1", "Secret", "s2", "n1", map[string]interface{}{"super-secret-annot": "value"}, nil),
-				getObjectAnnot("v1", "Secret", "s3", "n1", nil, map[string]interface{}{"super-secret-label": "value"}),
-
-				getObjectAnnot("route.openshift.io/v1", "Route", "r0", "n1", map[string]interface{}{"normal-annot": "value"}, nil),
-				getObjectAnnot("route.openshift.io/v1", "Route", "r1", "n1", nil, map[string]interface{}{"normal-label": "value"}),
-				getObjectAnnot("route.openshift.io/v1", "Route", "r2", "n1", map[string]interface{}{"super-secret-annot": "value"}, nil),
-				getObjectAnnot("route.openshift.io/v1", "Route", "r3", "n1", nil, map[string]interface{}{"super-secret-label": "value"}),
-			},
-			expected: []*api.GatheredResource{
-				{Resource: getObjectAnnot("v1", "Secret", "s0", "n1", map[string]interface{}{"normal-annot": "value"}, nil)},
-				{Resource: getObjectAnnot("v1", "Secret", "s1", "n1", nil, map[string]interface{}{"normal-label": "value"})},
-				{Resource: getObjectAnnot("v1", "Secret", "s2", "n1", nil, nil)},
-				{Resource: getObjectAnnot("v1", "Secret", "s3", "n1", nil, nil)},
-
-				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r0", "n1", map[string]interface{}{"normal-annot": "value"}, nil)},
-				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r1", "n1", nil, map[string]interface{}{"normal-label": "value"})},
-				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r2", "n1", nil, nil)},
-				{Resource: getObjectAnnot("route.openshift.io/v1", "Route", "r3", "n1", nil, nil)},
-			},
+		"excluded annotations are removed for unstructured-based gatherers such as secrets": {
+			config: ConfigDynamic{GroupVersionResource: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}},
+
+			// To give a realistic regex in this test case, let's use the
+			// example of the Kapp project that uses four annotations that all
+			// start with `kapp.k14s.io/original*`. These annotations are
+			// similar to `kubectl.kubernetes.io/last-applied-configuration` in
+			// that they may contain sensitive information. From [1], they may
+			// look like this:
+			//
+			//  kapp.k14s.io/original: |
+			//    {"apiVersion":"v1","kind":"Secret","spec":{"data": {"password": "cGFzc3dvcmQ=","username": "bXl1c2VybmFtZQ=="}}}
+			//  kapp.k14s.io/original-diff: |
+			//    - type: test
+			//      path: /data
+			//      value:
+			//      password: cygpcGVyUzNjcmV0UEBhc3N3b3JkIQ==
+			//      username: bXl1c2VybmFtZQ==
+			//
+			//  [1]: https://github.com/carvel-dev/kapp/issues/90#issuecomment-602074356
+			excludeAnnotsKeys: []string{`kapp\.k14s\.io\/original.*`},
+
+			// We haven't found convincing examples of labels that may contain
+			// sensitive information in the wild, so let's go with a dumb
+			// example.
+			excludeLabelKeys: []string{`.*sensitive.*`},
+
+			addObjects: []runtime.Object{getObjectAnnot("v1", "Secret", "s0", "n1",
+				map[string]interface{}{"kapp.k14s.io/original": "foo", "kapp.k14s.io/original-diff": "bar", "normal": "true"},
+				map[string]interface{}{"is-sensitive-label": "true", "prod": "true"},
+			)},
+			expected: []*api.GatheredResource{{Resource: getObjectAnnot("v1", "Secret", "s0", "n1",
+				map[string]interface{}{"normal": "true"},
+				map[string]interface{}{"prod": "true"},
+			)}},
 		},
 	}
 
@@ -632,12 +641,12 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 			var wg sync.WaitGroup
 			ctx := context.Background()
 			gvrToListKind := map[schema.GroupVersionResource]string{
-				schema.GroupVersionResource{Group: "foobar", Version: "v1", Resource: "foos"}:      "UnstructuredList",
-				schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}: "UnstructuredList",
-				schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}:         "UnstructuredList",
-				schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}:      "UnstructuredList",
+				{Group: "foobar", Version: "v1", Resource: "foos"}:      "UnstructuredList",
+				{Group: "apps", Version: "v1", Resource: "deployments"}: "UnstructuredList",
+				{Group: "", Version: "v1", Resource: "secrets"}:         "UnstructuredList",
+				{Group: "", Version: "v1", Resource: "namespaces"}:      "UnstructuredList",
 			}
-			cl := fake.NewSimpleDynamicClientWithCustomListKinds(emptyScheme, gvrToListKind, tc.addObjects...)
+			cl := fake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), gvrToListKind, tc.addObjects...)
 			// init the datagatherer's informer with the client
 			dg, err := tc.config.newDataGathererWithClient(ctx, cl, nil)
 			if err != nil {
@@ -927,7 +936,7 @@ func TestDynamicGathererNativeResources_Fetch(t *testing.T) {
 		// (would require a lot of changes to the testing func). Ideally we
 		// should test all native resources such as Service, Deployment,
 		// Ingress, Namespace, and so on.
-		"excluded annotations are removed native resources: pods, namespaces, etc": {
+		"excluded annotations are removed for typed resources gatherers such as pods": {
 			config:            ConfigDynamic{GroupVersionResource: podGVR},
 			excludeAnnotsKeys: []string{"secret"},
 			excludeLabelKeys:  []string{"secret"},

From 7271bc12aa1ae44f23118657fdd0e49fb813860f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Valais?= <mael@vls.dev>
Date: Wed, 13 Nov 2024 11:08:32 +0100
Subject: [PATCH 4/6] annot-exclusion: use concrete examples such as employee
 ID and Kapp

I've also reduced the size of the documentation in values.yaml; it now
only contains the essential information.
---
 .../charts/venafi-kubernetes-agent/README.md  |  6 ++--
 .../values.schema.json                        |  2 +-
 .../venafi-kubernetes-agent/values.yaml       | 28 ++++++++-----------
 pkg/datagatherer/k8s/dynamic_test.go          | 22 ++++++++++-----
 4 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/deploy/charts/venafi-kubernetes-agent/README.md b/deploy/charts/venafi-kubernetes-agent/README.md
index f148b8f8..af9098f9 100644
--- a/deploy/charts/venafi-kubernetes-agent/README.md
+++ b/deploy/charts/venafi-kubernetes-agent/README.md
@@ -431,11 +431,9 @@ Control Plane.
 
 You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.  
   
-If you would like to exclude annotations keys that contain the word `word`, use the regular expression `.*word.*`. The leading and ending .* are important if you want to filter out keys that contain `word` anywhere in the key string.  
+Dots is the only character that needs to be escaped in the regex. Use either double quotes with escaped single quotes or unquoted strings for the regex to avoid YAML parsing issues with `\.`.  
   
-Note that the annotation `kubectl.kubernetes.io/last-applied-configuration` is already excluded by default, you don't need to exclude it explicitly.  
-  
-Example: excludeAnnotationKeysRegex: ["kapp\.k14s\.io\/original.*"]
+Example: excludeAnnotationKeysRegex: ['^kapp\.k14s\.io/original.*']
 #### **config.excludeLabelKeysRegex** ~ `array`
 > Default value:
 > ```yaml
diff --git a/deploy/charts/venafi-kubernetes-agent/values.schema.json b/deploy/charts/venafi-kubernetes-agent/values.schema.json
index b4b755f1..dea7c43c 100644
--- a/deploy/charts/venafi-kubernetes-agent/values.schema.json
+++ b/deploy/charts/venafi-kubernetes-agent/values.schema.json
@@ -214,7 +214,7 @@
     "helm-values.config.configmap.name": {},
     "helm-values.config.excludeAnnotationKeysRegex": {
       "default": [],
-      "description": "You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.\n\nIf you would like to exclude annotations keys that contain the word `word`, use the regular expression `.*word.*`. The leading and ending .* are important if you want to filter out keys that contain `word` anywhere in the key string.\n\nNote that the annotation `kubectl.kubernetes.io/last-applied-configuration` is already excluded by default, you don't need to exclude it explicitly.\n\nExample: excludeAnnotationKeysRegex: [\"kapp\\.k14s\\.io\\/original.*\"]",
+      "description": "You can configure Venafi Kubernetes Agent to exclude some annotations or labels from being pushed to the Venafi Control Plane. All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being sent to the Venafi Control Plane.\n\nDots is the only character that needs to be escaped in the regex. Use either double quotes with escaped single quotes or unquoted strings for the regex to avoid YAML parsing issues with `\\.`.\n\nExample: excludeAnnotationKeysRegex: ['^kapp\\.k14s\\.io/original.*']",
       "items": {},
       "type": "array"
     },
diff --git a/deploy/charts/venafi-kubernetes-agent/values.yaml b/deploy/charts/venafi-kubernetes-agent/values.yaml
index cdd24794..bff466bb 100644
--- a/deploy/charts/venafi-kubernetes-agent/values.yaml
+++ b/deploy/charts/venafi-kubernetes-agent/values.yaml
@@ -114,7 +114,7 @@ podSecurityContext: {}
 securityContext:
   capabilities:
     drop:
-    - ALL
+      - ALL
   readOnlyRootFilesystem: true
   runAsNonRoot: true
 
@@ -230,28 +230,24 @@ config:
   # * https://kubernetes.io/docs/concepts/configuration/secret/#secret-types
   # * https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/#list-of-supported-fields
   ignoredSecretTypes:
-  - kubernetes.io/service-account-token
-  - kubernetes.io/dockercfg
-  - kubernetes.io/dockerconfigjson
-  - kubernetes.io/basic-auth
-  - kubernetes.io/ssh-auth
-  - bootstrap.kubernetes.io/token
-  - helm.sh/release.v1
+    - kubernetes.io/service-account-token
+    - kubernetes.io/dockercfg
+    - kubernetes.io/dockerconfigjson
+    - kubernetes.io/basic-auth
+    - kubernetes.io/ssh-auth
+    - bootstrap.kubernetes.io/token
+    - helm.sh/release.v1
 
   # You can configure Venafi Kubernetes Agent to exclude some annotations or
   # labels from being pushed to the Venafi Control Plane. All Kubernetes objects
   # are affected. The objects are still pushed, but the specified annotations
   # and labels are removed before being sent to the Venafi Control Plane.
   #
-  # If you would like to exclude annotations keys that contain the word `word`,
-  # use the regular expression `.*word.*`. The leading and ending .* are
-  # important if you want to filter out keys that contain `word` anywhere in the
-  # key string.
+  # Dots is the only character that needs to be escaped in the regex. Use either
+  # double quotes with escaped single quotes or unquoted strings for the regex
+  # to avoid YAML parsing issues with `\.`.
   #
-  # Note that the annotation `kubectl.kubernetes.io/last-applied-configuration`
-  # is already excluded by default, you don't need to exclude it explicitly.
-  #
-  # Example: excludeAnnotationKeysRegex: ["kapp\.k14s\.io\/original.*"]
+  # Example: excludeAnnotationKeysRegex: ['^kapp\.k14s\.io/original.*']
   excludeAnnotationKeysRegex: []
   excludeLabelKeysRegex: []
 
diff --git a/pkg/datagatherer/k8s/dynamic_test.go b/pkg/datagatherer/k8s/dynamic_test.go
index d925fae0..5af02bad 100644
--- a/pkg/datagatherer/k8s/dynamic_test.go
+++ b/pkg/datagatherer/k8s/dynamic_test.go
@@ -618,16 +618,24 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 			//      username: bXl1c2VybmFtZQ==
 			//
 			//  [1]: https://github.com/carvel-dev/kapp/issues/90#issuecomment-602074356
-			excludeAnnotsKeys: []string{`kapp\.k14s\.io\/original.*`},
-
-			// We haven't found convincing examples of labels that may contain
-			// sensitive information in the wild, so let's go with a dumb
-			// example.
-			excludeLabelKeys: []string{`.*sensitive.*`},
+			//
+			// The regular expression could be:
+			excludeAnnotsKeys: []string{`^kapp\.k14s\.io/original.*`},
+
+			// A somewhat realistic example of labels that would need to be
+			// excluded would be when a company declares ownership using
+			// sensitive identifiers (e.g., employee IDs), and the company
+			// doesn't want these IDs to be exposed. Let's imagine these
+			// employee IDs look like this:
+			//
+			//  company.com/employee-id: 12345
+			//
+			// The regular expression would then be:
+			excludeLabelKeys: []string{`^company\.com/employee-id$`},
 
 			addObjects: []runtime.Object{getObjectAnnot("v1", "Secret", "s0", "n1",
 				map[string]interface{}{"kapp.k14s.io/original": "foo", "kapp.k14s.io/original-diff": "bar", "normal": "true"},
-				map[string]interface{}{"is-sensitive-label": "true", "prod": "true"},
+				map[string]interface{}{`company.com/employee-id`: "12345", "prod": "true"},
 			)},
 			expected: []*api.GatheredResource{{Resource: getObjectAnnot("v1", "Secret", "s0", "n1",
 				map[string]interface{}{"normal": "true"},

From 93a02eca913b30e4ca84251adb04dcee6c57a981 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Valais?= <mael@vls.dev>
Date: Wed, 13 Nov 2024 13:31:03 +0100
Subject: [PATCH 5/6] annot-exclusion: unit test RemoveTypedKeys and
 RemoveUnstructuredKeys

---
 pkg/datagatherer/k8s/dynamic.go      | 128 ++++++++++-------
 pkg/datagatherer/k8s/dynamic_test.go | 197 +++++++++++++++++++++++++++
 2 files changed, 273 insertions(+), 52 deletions(-)

diff --git a/pkg/datagatherer/k8s/dynamic.go b/pkg/datagatherer/k8s/dynamic.go
index 6636c7ea..e02960c6 100644
--- a/pkg/datagatherer/k8s/dynamic.go
+++ b/pkg/datagatherer/k8s/dynamic.go
@@ -379,41 +379,8 @@ func redactList(list []*api.GatheredResource, excludeAnnotKeys, excludeLabelKeys
 			// remove managedFields from all resources
 			Redact(RedactFields, resource)
 
-			annotsRaw, ok, err := unstructured.NestedFieldNoCopy(resource.Object, "metadata", "annotations")
-			if err != nil {
-				return fmt.Errorf("wasn't able to find the metadata.annotations field: %w", err)
-			}
-			if ok {
-				annots, ok := annotsRaw.(map[string]interface{})
-				if !ok {
-					return fmt.Errorf("metadata.annotations isn't a map on the resource %s in namespace %s: %w", resource.GetName(), resource.GetNamespace(), err)
-				}
-				for key := range annots {
-					for _, excludeAnnotKey := range excludeAnnotKeys {
-						if excludeAnnotKey.MatchString(key) {
-							delete(annots, key)
-						}
-					}
-				}
-			}
-
-			labelsRaw, ok, err := unstructured.NestedFieldNoCopy(resource.Object, "metadata", "labels")
-			if err != nil {
-				return fmt.Errorf("wasn't able to find the metadata.labels field for the resource %s in namespace %s: %w", resource.GetName(), resource.GetNamespace(), err)
-			}
-			if ok {
-				labels, ok := labelsRaw.(map[string]interface{})
-				if !ok {
-					return fmt.Errorf("metadata.labels isn't a map on the resource %s in namespace %s: %w", resource.GetName(), resource.GetNamespace(), err)
-				}
-				for key := range labels {
-					for _, excludeLabelKey := range excludeLabelKeys {
-						if excludeLabelKey.MatchString(key) {
-							delete(labels, key)
-						}
-					}
-				}
-			}
+			RemoveUnstructuredKeys(excludeAnnotKeys, resource, "metadata", "annotations")
+			RemoveUnstructuredKeys(excludeLabelKeys, resource, "metadata", "labels")
 
 			continue
 		}
@@ -427,23 +394,8 @@ func redactList(list []*api.GatheredResource, excludeAnnotKeys, excludeLabelKeys
 			item.GetObjectMeta().SetManagedFields(nil)
 			delete(item.GetObjectMeta().GetAnnotations(), "kubectl.kubernetes.io/last-applied-configuration")
 
-			annots := item.GetObjectMeta().GetAnnotations()
-			for key := range annots {
-				for _, excludeAnnotKey := range excludeAnnotKeys {
-					if excludeAnnotKey.MatchString(key) {
-						delete(annots, key)
-					}
-				}
-			}
-
-			labels := item.GetObjectMeta().GetLabels()
-			for key := range labels {
-				for _, excludeLabelKey := range excludeLabelKeys {
-					if excludeLabelKey.MatchString(key) {
-						delete(labels, key)
-					}
-				}
-			}
+			RemoveTypedKeys(excludeAnnotKeys, item.GetObjectMeta().GetAnnotations())
+			RemoveTypedKeys(excludeLabelKeys, item.GetObjectMeta().GetLabels())
 
 			resource := item.(runtime.Object)
 			gvks, _, err := scheme.Scheme.ObjectKinds(resource)
@@ -470,6 +422,78 @@ func redactList(list []*api.GatheredResource, excludeAnnotKeys, excludeLabelKeys
 	return nil
 }
 
+// Meant for typed clientset objects.
+func RemoveTypedKeys(excludeAnnotKeys []*regexp.Regexp, m map[string]string) {
+	for key := range m {
+		for _, excludeAnnotKey := range excludeAnnotKeys {
+			if excludeAnnotKey.MatchString(key) {
+				delete(m, key)
+			}
+		}
+	}
+}
+
+// Meant for unstructured clientset objects. Removes the keys from the field
+// given as input. For example, let's say we have the following object:
+//
+//	{
+//	  "metadata": {
+//	    "annotations": {
+//	      "key1": "value1",
+//	      "key2": "value2"
+//	    }
+//	  }
+//	}
+//
+// Then, the following call:
+//
+//	RemoveUnstructuredKeys("^key1$", obj, "metadata", "annotations")
+//
+// Will result in:
+//
+//	{
+//	  "metadata": {
+//	    "annotations": {"key2": "value2"}
+//	  }
+//	}
+//
+// If the given path doesn't exist or leads to a non-map object, nothing
+// happens. The leaf object must either be a map[string]interface{} (that's
+// what's returned by the unstructured clientset) or a map[string]string (that's
+// what's returned by the typed clientset).
+func RemoveUnstructuredKeys(excludeKeys []*regexp.Regexp, obj *unstructured.Unstructured, path ...string) {
+	annotsRaw, ok, err := unstructured.NestedFieldNoCopy(obj.Object, path...)
+	if err != nil {
+		return
+	}
+	if !ok {
+		return
+	}
+
+	// The field may be nil since yaml.Unmarshal's omitempty might not be set on
+	// on this struct field.
+	if annotsRaw == nil {
+		return
+	}
+
+	// The only possible type in an unstructured.Unstructured object is
+	// map[string]interface{}. That's because the yaml.Unmarshal func is used
+	// with an empty map[string]interface{} object, which means all nested
+	// objects will be unmarshalled to a map[string]interface{}.
+	annots, ok := annotsRaw.(map[string]interface{})
+	if !ok {
+		return
+	}
+
+	for key := range annots {
+		for _, excludeAnnotKey := range excludeKeys {
+			if excludeAnnotKey.MatchString(key) {
+				delete(annots, key)
+			}
+		}
+	}
+}
+
 // generateExcludedNamespacesFieldSelector creates a field selector string from
 // a list of namespaces to exclude.
 func generateExcludedNamespacesFieldSelector(excludeNamespaces []string) fields.Selector {
diff --git a/pkg/datagatherer/k8s/dynamic_test.go b/pkg/datagatherer/k8s/dynamic_test.go
index 5af02bad..8002a153 100644
--- a/pkg/datagatherer/k8s/dynamic_test.go
+++ b/pkg/datagatherer/k8s/dynamic_test.go
@@ -1086,3 +1086,200 @@ func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) bool {
 		return true
 	}
 }
+
+func TestRemoveUnstructuredKeys(t *testing.T) {
+	t.Run("remove single key", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath:    []string{"metadata", "annotations"},
+		givenExclude: []string{"^key1$"},
+		givenObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+				"annotations": map[string]interface{}{
+					"key1": "value1",
+					"key2": "value2",
+				},
+			},
+		},
+		expectObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+			},
+		},
+	}))
+
+	t.Run("remove keys using multiple regexes", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath:    []string{"metadata", "annotations"},
+		givenExclude: []string{"^key1$", "^key2$"},
+		givenObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+				"annotations": map[string]interface{}{
+					"key1": "value1",
+					"key2": "value2",
+				},
+			},
+		},
+		expectObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+			},
+		},
+	}))
+
+	t.Run("remove multiple keys with a single regex", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath:    []string{"metadata", "annotations"},
+		givenExclude: []string{"key.*"},
+		givenObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+				"annotations": map[string]interface{}{
+					"key1": "value1",
+					"key2": "value2",
+				},
+			},
+		},
+		expectObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+			},
+		},
+	}))
+
+	t.Run("with no regex, the object is untouched", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath:    []string{"metadata", "annotations"},
+		givenExclude: []string{},
+		givenObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+				"annotations": map[string]interface{}{
+					"key1": "value1",
+					"key2": "value2",
+				},
+			},
+		},
+		expectObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name": "foo",
+				"annotations": map[string]interface{}{
+					"key1": "value1",
+					"key2": "value2",
+				},
+			},
+		},
+	}))
+
+	// The "leaf" field is the field that is at the end of the path. For
+	// example, "annotations" is the leaf field in metadata.annotations.
+	t.Run("works when the leaf field is not found", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath:    []string{"metadata", "annotations"},
+		givenExclude: []string{},
+
+		givenObj:  map[string]interface{}{"metadata": map[string]interface{}{}},
+		expectObj: map[string]interface{}{"metadata": map[string]interface{}{}},
+	}))
+
+	t.Run("works when the leaf field is nil", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath:    []string{"metadata", "annotations"},
+		givenExclude: []string{},
+		givenObj: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"name":        "foo",
+				"annotations": nil,
+			},
+		},
+		expectObj: map[string]interface{}{"metadata": map[string]interface{}{"name": "foo"}},
+	}))
+
+	t.Run("works when leaf field is unexpectedly not nil and not a known map", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath: []string{"metadata", "annotations"},
+		givenObj:  map[string]interface{}{"metadata": map[string]interface{}{"annotations": 42}},
+		expectObj: map[string]interface{}{"metadata": map[string]interface{}{"annotations": 42}},
+	}))
+
+	// The "intermediate" field is the field that is not at the end of the path.
+	// For example, "metadata" is the intermediate field in
+	// metadata.annotations.
+	t.Run("works when the intermediate field doesn't exist", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath: []string{"metadata", "annotations"},
+		givenObj:  map[string]interface{}{},
+		expectObj: map[string]interface{}{},
+	}))
+
+	t.Run("works when the intermediate field is nil", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath: []string{"metadata", "annotations"},
+		givenObj:  map[string]interface{}{"metadata": nil},
+	}))
+
+	t.Run("works when the intermediate field is unexpectedly not nil and not a map", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
+		givenPath: []string{"metadata", "annotations"},
+		givenObj:  map[string]interface{}{"metadata": 42},
+		expectObj: map[string]interface{}{"metadata": 42},
+	}))
+}
+
+type tc_RemoveUnstructuredKeys struct {
+	givenExclude []string
+	givenObj     map[string]interface{}
+	givenPath    []string
+	expectObj    map[string]interface{}
+}
+
+func run_TestRemoveUnstructuredKeys(tc tc_RemoveUnstructuredKeys) func(*testing.T) {
+	return func(t *testing.T) {
+		t.Helper()
+		RemoveUnstructuredKeys(toRegexps(tc.givenExclude), &unstructured.Unstructured{Object: tc.givenObj}, tc.givenPath...)
+	}
+}
+
+func TestRemoveTypedKeys(t *testing.T) {
+	t.Run("remove single key", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
+		givenExclude: []string{"^key1$"},
+		given:        map[string]string{"key1": "value1", "key2": "value2"},
+		expected:     map[string]string{"key2": "value2"},
+	}))
+
+	t.Run("remove keys using multiple regexes", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
+		givenExclude: []string{"^key1$", "^key2$"},
+		given:        map[string]string{"key1": "value1", "key2": "value2"},
+		expected:     map[string]string{},
+	}))
+
+	t.Run("remove multiple keys with a single regex", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
+		givenExclude: []string{"key.*"},
+		given:        map[string]string{"key1": "value1", "key2": "value2"},
+		expected:     map[string]string{},
+	}))
+
+	t.Run("with no regex, the object is untouched", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
+		givenExclude: []string{},
+		given:        map[string]string{"key1": "value1", "key2": "value2"},
+		expected:     map[string]string{"key1": "value1", "key2": "value2"},
+	}))
+
+	t.Run("works when the map is nil", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
+		givenExclude: []string{"^key1$"},
+		given:        nil,
+		expected:     nil,
+	}))
+}
+
+type tc_TestRemoveTypedKeys struct {
+	givenExclude []string
+	given        map[string]string
+	expected     map[string]string
+}
+
+func run_TestRemoveTypedKeys(tc tc_TestRemoveTypedKeys) func(t *testing.T) {
+	return func(t *testing.T) {
+		RemoveTypedKeys(toRegexps(tc.givenExclude), tc.given)
+		assert.Equal(t, tc.expected, tc.given)
+	}
+}
+
+func toRegexps(keys []string) []*regexp.Regexp {
+	var regexps []*regexp.Regexp
+	for _, key := range keys {
+		regexps = append(regexps, regexp.MustCompile(key))
+	}
+	return regexps
+}

From 8f99daaffb8df513de34188cd2b43b2c53b46e87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Valais?= <mael@vls.dev>
Date: Thu, 14 Nov 2024 09:31:11 +0100
Subject: [PATCH 6/6] annot-exclusion: TestRemoveUnstructuredKeys wasn't
 testing anything

Co-authored-by: Richard Wall <wallrj@users.noreply.github.com>
---
 pkg/datagatherer/k8s/dynamic_test.go | 78 +++++++++++++---------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/pkg/datagatherer/k8s/dynamic_test.go b/pkg/datagatherer/k8s/dynamic_test.go
index 8002a153..c421fcfb 100644
--- a/pkg/datagatherer/k8s/dynamic_test.go
+++ b/pkg/datagatherer/k8s/dynamic_test.go
@@ -1090,57 +1090,57 @@ func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) bool {
 func TestRemoveUnstructuredKeys(t *testing.T) {
 	t.Run("remove single key", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
 		givenPath:    []string{"metadata", "annotations"},
-		givenExclude: []string{"^key1$"},
+		givenExclude: []string{"^toexclude$"},
 		givenObj: map[string]interface{}{
 			"metadata": map[string]interface{}{
-				"name": "foo",
 				"annotations": map[string]interface{}{
-					"key1": "value1",
-					"key2": "value2",
+					"toexclude": "foo",
+					"tokeep":    "bar",
 				},
 			},
 		},
 		expectObj: map[string]interface{}{
 			"metadata": map[string]interface{}{
-				"name": "foo",
+				"annotations": map[string]interface{}{
+					"tokeep": "bar",
+				},
 			},
 		},
 	}))
 
 	t.Run("remove keys using multiple regexes", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
 		givenPath:    []string{"metadata", "annotations"},
-		givenExclude: []string{"^key1$", "^key2$"},
+		givenExclude: []string{"^toexclude1$", "^toexclude2$"},
 		givenObj: map[string]interface{}{
 			"metadata": map[string]interface{}{
-				"name": "foo",
 				"annotations": map[string]interface{}{
-					"key1": "value1",
-					"key2": "value2",
+					"toexclude1": "foo",
+					"toexclude2": "bar",
 				},
 			},
 		},
 		expectObj: map[string]interface{}{
-			"metadata": map[string]interface{}{
-				"name": "foo",
-			},
+			"metadata": map[string]interface{}{"annotations": map[string]interface{}{}},
 		},
 	}))
 
 	t.Run("remove multiple keys with a single regex", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
 		givenPath:    []string{"metadata", "annotations"},
-		givenExclude: []string{"key.*"},
+		givenExclude: []string{"toexclude.*"},
 		givenObj: map[string]interface{}{
 			"metadata": map[string]interface{}{
-				"name": "foo",
 				"annotations": map[string]interface{}{
-					"key1": "value1",
-					"key2": "value2",
+					"toexclude1": "foo",
+					"toexclude2": "bar",
+					"tokeep":     "baz",
 				},
 			},
 		},
 		expectObj: map[string]interface{}{
 			"metadata": map[string]interface{}{
-				"name": "foo",
+				"annotations": map[string]interface{}{
+					"tokeep": "baz",
+				},
 			},
 		},
 	}))
@@ -1150,19 +1150,15 @@ func TestRemoveUnstructuredKeys(t *testing.T) {
 		givenExclude: []string{},
 		givenObj: map[string]interface{}{
 			"metadata": map[string]interface{}{
-				"name": "foo",
 				"annotations": map[string]interface{}{
-					"key1": "value1",
-					"key2": "value2",
+					"tokeep1": "foo",
 				},
 			},
 		},
 		expectObj: map[string]interface{}{
 			"metadata": map[string]interface{}{
-				"name": "foo",
 				"annotations": map[string]interface{}{
-					"key1": "value1",
-					"key2": "value2",
+					"tokeep1": "foo",
 				},
 			},
 		},
@@ -1181,13 +1177,8 @@ func TestRemoveUnstructuredKeys(t *testing.T) {
 	t.Run("works when the leaf field is nil", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
 		givenPath:    []string{"metadata", "annotations"},
 		givenExclude: []string{},
-		givenObj: map[string]interface{}{
-			"metadata": map[string]interface{}{
-				"name":        "foo",
-				"annotations": nil,
-			},
-		},
-		expectObj: map[string]interface{}{"metadata": map[string]interface{}{"name": "foo"}},
+		givenObj:     map[string]interface{}{"metadata": map[string]interface{}{"annotations": nil}},
+		expectObj:    map[string]interface{}{"metadata": map[string]interface{}{"annotations": nil}},
 	}))
 
 	t.Run("works when leaf field is unexpectedly not nil and not a known map", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
@@ -1208,6 +1199,7 @@ func TestRemoveUnstructuredKeys(t *testing.T) {
 	t.Run("works when the intermediate field is nil", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
 		givenPath: []string{"metadata", "annotations"},
 		givenObj:  map[string]interface{}{"metadata": nil},
+		expectObj: map[string]interface{}{"metadata": nil},
 	}))
 
 	t.Run("works when the intermediate field is unexpectedly not nil and not a map", run_TestRemoveUnstructuredKeys(tc_RemoveUnstructuredKeys{
@@ -1228,36 +1220,37 @@ func run_TestRemoveUnstructuredKeys(tc tc_RemoveUnstructuredKeys) func(*testing.
 	return func(t *testing.T) {
 		t.Helper()
 		RemoveUnstructuredKeys(toRegexps(tc.givenExclude), &unstructured.Unstructured{Object: tc.givenObj}, tc.givenPath...)
+		assert.Equal(t, tc.expectObj, tc.givenObj)
 	}
 }
 
 func TestRemoveTypedKeys(t *testing.T) {
 	t.Run("remove single key", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
-		givenExclude: []string{"^key1$"},
-		given:        map[string]string{"key1": "value1", "key2": "value2"},
-		expected:     map[string]string{"key2": "value2"},
+		givenExclude: []string{"^toexclude$"},
+		given:        map[string]string{"toexclude": "foo", "tokeep": "bar"},
+		expected:     map[string]string{"tokeep": "bar"},
 	}))
 
 	t.Run("remove keys using multiple regexes", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
-		givenExclude: []string{"^key1$", "^key2$"},
-		given:        map[string]string{"key1": "value1", "key2": "value2"},
-		expected:     map[string]string{},
+		givenExclude: []string{"^toexclude1$", "^toexclude2$"},
+		given:        map[string]string{"toexclude1": "foo", "toexclude2": "bar", "tokeep": "baz"},
+		expected:     map[string]string{"tokeep": "baz"},
 	}))
 
 	t.Run("remove multiple keys with a single regex", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
-		givenExclude: []string{"key.*"},
-		given:        map[string]string{"key1": "value1", "key2": "value2"},
-		expected:     map[string]string{},
+		givenExclude: []string{"^toexclude.*"},
+		given:        map[string]string{"toexclude1": "foo", "toexclude2": "bar", "tokeep": "baz"},
+		expected:     map[string]string{"tokeep": "baz"},
 	}))
 
 	t.Run("with no regex, the object is untouched", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
 		givenExclude: []string{},
-		given:        map[string]string{"key1": "value1", "key2": "value2"},
-		expected:     map[string]string{"key1": "value1", "key2": "value2"},
+		given:        map[string]string{"tokeep1": "foo", "tokeep2": "bar"},
+		expected:     map[string]string{"tokeep1": "foo", "tokeep2": "bar"},
 	}))
 
 	t.Run("works when the map is nil", run_TestRemoveTypedKeys(tc_TestRemoveTypedKeys{
-		givenExclude: []string{"^key1$"},
+		givenExclude: []string{"^toexclude$"},
 		given:        nil,
 		expected:     nil,
 	}))
@@ -1271,6 +1264,7 @@ type tc_TestRemoveTypedKeys struct {
 
 func run_TestRemoveTypedKeys(tc tc_TestRemoveTypedKeys) func(t *testing.T) {
 	return func(t *testing.T) {
+		t.Helper()
 		RemoveTypedKeys(toRegexps(tc.givenExclude), tc.given)
 		assert.Equal(t, tc.expected, tc.given)
 	}