From 365d7a1fa764f70c0dcde496653cbb7681abe028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Valais?= Date: Tue, 12 Nov 2024 19:33:24 +0100 Subject: [PATCH] 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"},