Merge pull request #204 from jetstack/fieldfilter

Implement more generic means of filtering uploaded objects

Author: charlieegan3

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

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=

pkg/datagatherer/k8s/dynamic.go

@@ -164,69 +164,33 @@ 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])
 		if err != nil {
 			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(SecretSelectedFields, &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
 }

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)
 			}
 		})
 	}

pkg/datagatherer/k8s/fieldfilter.go

@@ -0,0 +1,102 @@
+package k8s
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+
+	"github.com/Jeffail/gabs/v2"
+	"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
+	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 {
+				// fail to select field if missing, just continue
+				continue
+			}
+			pathComponents := strings.Split(v, "/")
+			filteredObject.Set(gObject.Data(), pathComponents[1:]...)
+		} else {
+			if jsonParsed.ExistsP(v) {
+				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
+}
+
+// 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)
+			}
+		}
+	}
+
+	// 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
+}