From 3a71cdc5f8fd462fc669b8551059682de033e424 Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Wed, 18 Nov 2020 14:14:27 +0000 Subject: [PATCH 1/5] Implement simple fieldfilter to select fields This will allow field filtering to be more generic for different resource types. Signed-off-by: Charlie Egan --- go.mod | 2 + go.sum | 3 ++ pkg/datagatherer/k8s/fieldfilter.go | 48 ++++++++++++++++++++ pkg/datagatherer/k8s/fieldfilter_test.go | 58 ++++++++++++++++++++++++ 4 files changed, 111 insertions(+) create mode 100644 pkg/datagatherer/k8s/fieldfilter.go create mode 100644 pkg/datagatherer/k8s/fieldfilter_test.go diff --git a/go.mod b/go.mod index 331d42fa..0c292ea5 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,8 @@ require ( github.com/Azure/go-autorest/autorest v0.11.8 // indirect github.com/Azure/go-autorest/autorest/to v0.4.0 // indirect github.com/Azure/go-autorest/autorest/validation v0.3.0 // indirect + github.com/Jeffail/gabs v1.1.1 + github.com/Jeffail/gabs/v2 v2.6.0 github.com/aws/aws-sdk-go v1.34.10 github.com/cenkalti/backoff v2.0.0+incompatible github.com/d4l3k/messagediff v1.2.1 diff --git a/go.sum b/go.sum index a7197de8..ed69c71d 100644 --- a/go.sum +++ b/go.sum @@ -90,6 +90,9 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/Jeffail/gabs v1.1.1 h1:V0uzR08Hj22EX8+8QMhyI9sX2hwRu+/RJhJUmnwda/E= github.com/Jeffail/gabs v1.1.1/go.mod h1:6xMvQMK4k33lb7GUUpaAPh6nKMmemQeg5d4gn7/bOXc= +github.com/Jeffail/gabs v1.4.0 h1://5fYRRTq1edjfIrQGvdkcd22pkYUrHZ5YC/H2GJVAo= +github.com/Jeffail/gabs/v2 v2.6.0 h1:WdCnGaDhNa4LSRTMwhLZzJ7SRDXjABNP13SOKvCpL5w= +github.com/Jeffail/gabs/v2 v2.6.0/go.mod h1:xCn81vdHKxFUuWWAaD5jCTQDNPBMh5pPs9IJ+NcziBI= github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/PuerkitoBio/purell v1.0.0/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= diff --git a/pkg/datagatherer/k8s/fieldfilter.go b/pkg/datagatherer/k8s/fieldfilter.go new file mode 100644 index 00000000..3137ee10 --- /dev/null +++ b/pkg/datagatherer/k8s/fieldfilter.go @@ -0,0 +1,48 @@ +package k8s + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/Jeffail/gabs/v2" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func Select(fields []string, resource *unstructured.Unstructured) error { + // convert the object to JSON for field filtering + asJSON, err := json.Marshal(resource) + if err != nil { + return fmt.Errorf("failed to marshal json for resource: %s", err) + } + + // parse the JSON for processing in gabs + jsonParsed, err := gabs.ParseJSON(asJSON) + if err != nil { + return fmt.Errorf("failed to parse generated json for resource: %s", err) + } + + // craft a new object containing only selected fields + filteredObject := gabs.New() + for _, v := range fields { + // also support JSONPointers for keys containing '.' chars + if strings.HasPrefix(v, "/") { + gObject, err := jsonParsed.JSONPointer(v) + if err != nil { + return fmt.Errorf("failed to select data at JSONPointer: %s", v) + } + pathComponents := strings.Split(v, "/") + filteredObject.Set(gObject.Data(), pathComponents[1:]...) + } else { + filteredObject.SetP(jsonParsed.Path(v).Data(), v) + } + } + + // load the filtered JSON back into the resource + err = json.Unmarshal(filteredObject.Bytes(), resource) + if err != nil { + return fmt.Errorf("failed to update resource: %s", err) + } + + return nil +} diff --git a/pkg/datagatherer/k8s/fieldfilter_test.go b/pkg/datagatherer/k8s/fieldfilter_test.go new file mode 100644 index 00000000..0079e90e --- /dev/null +++ b/pkg/datagatherer/k8s/fieldfilter_test.go @@ -0,0 +1,58 @@ +package k8s + +import ( + "encoding/json" + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestFieldSelector(t *testing.T) { + resource := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "example", + "namespace": "example", + "last-applied-configuration": "secret", + }, + "type": "kubernetes.io/tls", + "data": map[string]interface{}{ + "tls.crt": "cert data", + "tls.key": "secret", + }, + }, + } + + fieldsToSelect := []string{ + "apiVersion", + "kind", + "metadata.name", + "metadata.namespace", + "type", + "/data/tls.crt", + } + + err := Select(fieldsToSelect, resource) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + bytes, err := json.MarshalIndent(resource, "", " ") + expectedJSON := `{ + "apiVersion": "v1", + "data": { + "tls.crt": "cert data" + }, + "kind": "Secret", + "metadata": { + "name": "example", + "namespace": "example" + }, + "type": "kubernetes.io/tls" +}` + if string(bytes) != expectedJSON { + t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(bytes), expectedJSON) + } +} From 27d31ea041a5469a495eb45014c2350e4e1fb53a Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Wed, 18 Nov 2020 14:19:39 +0000 Subject: [PATCH 2/5] Make selected fields optional If the desired fields are missing in the object, then just roll over and set no value. Signed-off-by: Charlie Egan --- pkg/datagatherer/k8s/fieldfilter.go | 7 ++++-- pkg/datagatherer/k8s/fieldfilter_test.go | 27 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/pkg/datagatherer/k8s/fieldfilter.go b/pkg/datagatherer/k8s/fieldfilter.go index 3137ee10..ecc16b62 100644 --- a/pkg/datagatherer/k8s/fieldfilter.go +++ b/pkg/datagatherer/k8s/fieldfilter.go @@ -29,12 +29,15 @@ func Select(fields []string, resource *unstructured.Unstructured) error { if strings.HasPrefix(v, "/") { gObject, err := jsonParsed.JSONPointer(v) if err != nil { - return fmt.Errorf("failed to select data at JSONPointer: %s", v) + // fail to select field if missing, just continue + continue } pathComponents := strings.Split(v, "/") filteredObject.Set(gObject.Data(), pathComponents[1:]...) } else { - filteredObject.SetP(jsonParsed.Path(v).Data(), v) + if jsonParsed.ExistsP(v) { + filteredObject.SetP(jsonParsed.Path(v).Data(), v) + } } } diff --git a/pkg/datagatherer/k8s/fieldfilter_test.go b/pkg/datagatherer/k8s/fieldfilter_test.go index 0079e90e..af5cf75f 100644 --- a/pkg/datagatherer/k8s/fieldfilter_test.go +++ b/pkg/datagatherer/k8s/fieldfilter_test.go @@ -56,3 +56,30 @@ func TestFieldSelector(t *testing.T) { t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(bytes), expectedJSON) } } + +func TestFieldSelectorMissingSelectedField(t *testing.T) { + resource := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Secret", + }, + } + + fieldsToSelect := []string{ + "kind", + "missing", + "/missing", + } + + err := Select(fieldsToSelect, resource) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + bytes, err := json.MarshalIndent(resource, "", " ") + expectedJSON := `{ + "kind": "Secret" +}` + if string(bytes) != expectedJSON { + t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(bytes), expectedJSON) + } +} From 7ed0f9e9d8d112acb5977d3f624daaa6a31dd01b Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Wed, 18 Nov 2020 14:31:53 +0000 Subject: [PATCH 3/5] Add Redact to also allow removing of fields Plan to use this for managedFields in all objects Signed-off-by: Charlie Egan --- pkg/datagatherer/k8s/fieldfilter.go | 41 +++++++++++++ pkg/datagatherer/k8s/fieldfilter_test.go | 78 +++++++++++++++++++++++- 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/pkg/datagatherer/k8s/fieldfilter.go b/pkg/datagatherer/k8s/fieldfilter.go index ecc16b62..6ead5f12 100644 --- a/pkg/datagatherer/k8s/fieldfilter.go +++ b/pkg/datagatherer/k8s/fieldfilter.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) +// Select removes all but the supplied fields from the resource func Select(fields []string, resource *unstructured.Unstructured) error { // convert the object to JSON for field filtering asJSON, err := json.Marshal(resource) @@ -49,3 +50,43 @@ func Select(fields []string, resource *unstructured.Unstructured) error { return nil } + +// Redact removes the supplied fields from the resource +func Redact(fields []string, resource *unstructured.Unstructured) error { + // convert the object to JSON for field filtering + asJSON, err := json.Marshal(resource) + if err != nil { + return fmt.Errorf("failed to marshal json for resource: %s", err) + } + + // parse the JSON for processing in gabs + jsonParsed, err := gabs.ParseJSON(asJSON) + if err != nil { + return fmt.Errorf("failed to parse generated json for resource: %s", err) + } + + // craft a new object excluding redacted fields + for _, v := range fields { + // also support JSONPointers for keys containing '.' chars + if strings.HasPrefix(v, "/") { + pathComponents := strings.Split(v, "/")[1:] + if jsonParsed.Exists(pathComponents...) { + jsonParsed.Delete(pathComponents...) + } + } else { + if jsonParsed.ExistsP(v) { + jsonParsed.DeleteP(v) + } + } + } + + fmt.Println(jsonParsed.String()) + + // load the filtered JSON back into the resource + err = json.Unmarshal(jsonParsed.Bytes(), resource) + if err != nil { + return fmt.Errorf("failed to update resource: %s", err) + } + + return nil +} diff --git a/pkg/datagatherer/k8s/fieldfilter_test.go b/pkg/datagatherer/k8s/fieldfilter_test.go index af5cf75f..9d25faaf 100644 --- a/pkg/datagatherer/k8s/fieldfilter_test.go +++ b/pkg/datagatherer/k8s/fieldfilter_test.go @@ -7,7 +7,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func TestFieldSelector(t *testing.T) { +func TestSelect(t *testing.T) { resource := &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "v1", @@ -57,7 +57,7 @@ func TestFieldSelector(t *testing.T) { } } -func TestFieldSelectorMissingSelectedField(t *testing.T) { +func TestSelectMissingSelectedField(t *testing.T) { resource := &unstructured.Unstructured{ Object: map[string]interface{}{ "kind": "Secret", @@ -65,7 +65,7 @@ func TestFieldSelectorMissingSelectedField(t *testing.T) { } fieldsToSelect := []string{ - "kind", + "kind", // required for unstructured unmarshal "missing", "/missing", } @@ -83,3 +83,75 @@ func TestFieldSelectorMissingSelectedField(t *testing.T) { t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(bytes), expectedJSON) } } + +func TestRedact(t *testing.T) { + resource := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "example", + "namespace": "example", + "last-applied-configuration": "secret", + }, + "type": "kubernetes.io/tls", + "data": map[string]interface{}{ + "tls.crt": "cert data", + "tls.key": "secret", + }, + }, + } + + fieldsToRedact := []string{ + "metadata.last-applied-configuration", + "/data/tls.key", + } + + err := Redact(fieldsToRedact, resource) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + bytes, err := json.MarshalIndent(resource, "", " ") + expectedJSON := `{ + "apiVersion": "v1", + "data": { + "tls.crt": "cert data" + }, + "kind": "Secret", + "metadata": { + "name": "example", + "namespace": "example" + }, + "type": "kubernetes.io/tls" +}` + if string(bytes) != expectedJSON { + t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(bytes), expectedJSON) + } +} + +func TestRedactMissingField(t *testing.T) { + resource := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Secret", + }, + } + + fieldsToRedact := []string{ + "missing", + "/missing", + } + + err := Redact(fieldsToRedact, resource) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + bytes, err := json.MarshalIndent(resource, "", " ") + expectedJSON := `{ + "kind": "Secret" +}` + if string(bytes) != expectedJSON { + t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(bytes), expectedJSON) + } +} From 6f453d74ffd9eaf99e8385c2c9fc10b3a7349225 Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Wed, 18 Nov 2020 14:48:16 +0000 Subject: [PATCH 4/5] Update dynamic DG to use fieldfilter functions Signed-off-by: Charlie Egan --- pkg/datagatherer/k8s/dynamic.go | 62 ++++++++-------------------- pkg/datagatherer/k8s/dynamic_test.go | 18 ++++---- pkg/datagatherer/k8s/fieldfilter.go | 2 - 3 files changed, 27 insertions(+), 55 deletions(-) diff --git a/pkg/datagatherer/k8s/dynamic.go b/pkg/datagatherer/k8s/dynamic.go index 1dda11c6..90e94368 100644 --- a/pkg/datagatherer/k8s/dynamic.go +++ b/pkg/datagatherer/k8s/dynamic.go @@ -164,9 +164,6 @@ func (g *DataGathererDynamic) Fetch() (interface{}, error) { } func redactList(list *unstructured.UnstructuredList) error { - // In principal we could only redact the list if it's kind is SecretList or - // a generic mixed List, however the test suite does not set the list kind - // and it is safer to always check for Secrets. for i := range list.Items { // Determine the kind of items in case this is a generic 'mixed' list. gvks, _, err := scheme.Scheme.ObjectKinds(&list.Items[i]) @@ -174,59 +171,34 @@ func redactList(list *unstructured.UnstructuredList) error { return errors.WithStack(err) } - object := list.Items[i] + resource := list.Items[i] for _, gvk := range gvks { // If this item is a Secret then we need to redact it. if gvk.Kind == "Secret" && (gvk.Group == "core" || gvk.Group == "") { + Select([]string{ + "kind", + "apiVersion", + "metadata.name", + "metadata.namespace", + "type", + "/data/tls.crt", + "/data/ca.crt", + }, &resource) - // If the secret is a tls secret, we redact all data other then - // the tls.crt and ca.crt. This is because we need to inspect - // the certificate to make recommendations. - if object.Object["type"] == "kubernetes.io/tls" { - secretData, ok := object.Object["data"].(map[string]interface{}) - if ok { - for k := range secretData { - // Only these two keys will be sent, all others are - // deleted - if k != "tls.crt" && k != "ca.crt" { - delete(secretData, k) - } - } - } else { - // If secret is not string mapping, redact all secret data - object.Object["data"] = map[string]interface{}{} - } - } else { - // Redact all secret data for non-tls secrets - object.Object["data"] = map[string]interface{}{} - } - - metadata, metadataPresent := object.Object["metadata"].(map[string]interface{}) - if metadataPresent { - // Redact last-applied-configuration annotation if set - annotations, present := metadata["annotations"].(map[string]interface{}) - if present { - _, annotationPresent := annotations["kubectl.kubernetes.io/last-applied-configuration"] - if annotationPresent { - annotations["kubectl.kubernetes.io/last-applied-configuration"] = "redacted" - } - metadata["annotations"] = annotations - } - } // break when the object has been processed as a secret, no // other kinds have redact modifications break } - metadata, metadataPresent := object.Object["metadata"].(map[string]interface{}) - if metadataPresent { - // Drop managed fields if set - if _, present := metadata["managedFields"]; present { - delete(metadata, "managedFields") - } - } + // remove managedFields from all resources + Redact([]string{ + "metadata.managedFields", + }, &resource) } + + // update the object in the list + list.Items[i] = resource } return nil } diff --git a/pkg/datagatherer/k8s/dynamic_test.go b/pkg/datagatherer/k8s/dynamic_test.go index d59b3d41..16c63320 100644 --- a/pkg/datagatherer/k8s/dynamic_test.go +++ b/pkg/datagatherer/k8s/dynamic_test.go @@ -41,7 +41,10 @@ func getObject(version, kind, name, namespace string, withManagedFields bool) *u func getSecret(name, namespace string, data map[string]interface{}, isTLS bool, withLastApplied bool) *unstructured.Unstructured { object := getObject("v1", "Secret", name, namespace, false) - object.Object["data"] = data + + if data != nil { + object.Object["data"] = data + } object.Object["type"] = "Opaque" if isTLS { @@ -56,10 +59,6 @@ func getSecret(name, namespace string, data map[string]interface{}, isTLS bool, metadata["annotations"] = map[string]interface{}{ "kubectl.kubernetes.io/last-applied-configuration": string(jsonData), } - } else { // generate an expected redacted secret - metadata["annotations"] = map[string]interface{}{ - "kubectl.kubernetes.io/last-applied-configuration": "redacted", - } } return object @@ -164,8 +163,8 @@ func TestDynamicGatherer_Fetch(t *testing.T) { }, false, true), }, expected: asUnstructuredList( - getSecret("testsecret", "testns1", map[string]interface{}{}, false, false), - getSecret("anothertestsecret", "testns2", map[string]interface{}{}, false, false), + getSecret("testsecret", "testns1", nil, false, false), + getSecret("anothertestsecret", "testns2", nil, false, false), ), }, "Secret of type kubernetes.io/tls should have crts and not keys": { @@ -188,7 +187,7 @@ func TestDynamicGatherer_Fetch(t *testing.T) { "ca.crt": "value", }, true, false), // all other keys removed - getSecret("anothertestsecret", "testns2", map[string]interface{}{}, true, false), + getSecret("anothertestsecret", "testns2", nil, true, false), ), }, "Foos in different namespaces should be returned if they are in the namespace list for the gatherer": { @@ -240,6 +239,9 @@ func TestDynamicGatherer_Fetch(t *testing.T) { } if diff, equal := messagediff.PrettyDiff(test.expected, res); !equal { t.Errorf("\n%s", diff) + expectedJSON, _ := json.MarshalIndent(test.expected, "", " ") + gotJSON, _ := json.MarshalIndent(res, "", " ") + t.Fatalf("unexpected JSON: \ngot \n%s\nwant\n%s", string(gotJSON), expectedJSON) } }) } diff --git a/pkg/datagatherer/k8s/fieldfilter.go b/pkg/datagatherer/k8s/fieldfilter.go index 6ead5f12..2fc86b40 100644 --- a/pkg/datagatherer/k8s/fieldfilter.go +++ b/pkg/datagatherer/k8s/fieldfilter.go @@ -80,8 +80,6 @@ func Redact(fields []string, resource *unstructured.Unstructured) error { } } - fmt.Println(jsonParsed.String()) - // load the filtered JSON back into the resource err = json.Unmarshal(jsonParsed.Bytes(), resource) if err != nil { From 0521da0fa4622b2d3a4129f6ccd1fa19c3c1e75f Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Wed, 18 Nov 2020 15:06:16 +0000 Subject: [PATCH 5/5] Export redact and select field lists Signed-off-by: Charlie Egan --- pkg/datagatherer/k8s/dynamic.go | 10 +--------- pkg/datagatherer/k8s/fieldfilter.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/datagatherer/k8s/dynamic.go b/pkg/datagatherer/k8s/dynamic.go index 90e94368..5a8ed59d 100644 --- a/pkg/datagatherer/k8s/dynamic.go +++ b/pkg/datagatherer/k8s/dynamic.go @@ -176,15 +176,7 @@ func redactList(list *unstructured.UnstructuredList) error { for _, gvk := range gvks { // If this item is a Secret then we need to redact it. if gvk.Kind == "Secret" && (gvk.Group == "core" || gvk.Group == "") { - Select([]string{ - "kind", - "apiVersion", - "metadata.name", - "metadata.namespace", - "type", - "/data/tls.crt", - "/data/ca.crt", - }, &resource) + Select(SecretSelectedFields, &resource) // break when the object has been processed as a secret, no // other kinds have redact modifications diff --git a/pkg/datagatherer/k8s/fieldfilter.go b/pkg/datagatherer/k8s/fieldfilter.go index 2fc86b40..92fb47ee 100644 --- a/pkg/datagatherer/k8s/fieldfilter.go +++ b/pkg/datagatherer/k8s/fieldfilter.go @@ -9,6 +9,18 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) +// SecretSelectedFields is the list of fields sent from Secret objects to the +// backend +var SecretSelectedFields = []string{ + "kind", + "apiVersion", + "metadata.name", + "metadata.namespace", + "type", + "/data/tls.crt", + "/data/ca.crt", +} + // Select removes all but the supplied fields from the resource func Select(fields []string, resource *unstructured.Unstructured) error { // convert the object to JSON for field filtering