Remove ParseJSONQuery function and replace it with unstructured (#469)

* Remove ParseJSONQuery function and replace it with unstructured
functions

* Better error formatting

* Fix the attributes

Author: tremes

pkg/gatherers/clusterconfig/install_plans.go

@@ -8,6 +8,7 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/client-go/dynamic"
@@ -21,6 +22,20 @@ import (
 // InstallPlansTopX is the Maximal number of Install plans by non-unique instances count
 const InstallPlansTopX = 100
 
+type collectedPlan struct {
+	Namespace string
+	Name      string
+	CSV       string
+	Count     int
+}
+
+// InstallPlanAnonymizer implements serialization of top x installplans
+type InstallPlanAnonymizer struct {
+	v     map[string]*collectedPlan
+	total int
+	limit int
+}
+
 // GatherInstallPlans collects Top x InstallPlans from all openshift namespaces.
 // Because InstallPlans have unique generated names, it groups them by namespace and the "template"
 // for name generation from field generateName.
@@ -78,11 +93,12 @@ func gatherInstallPlans(ctx context.Context,
 				return nil, []error{err}
 			}
 			jsonMap := u.UnstructuredContent()
-			var items []interface{}
-			err = failEarly(
-				func() error { return utils.ParseJSONQuery(jsonMap, "metadata.continue?", &cont) },
-				func() error { return utils.ParseJSONQuery(jsonMap, "items", &items) },
-			)
+			// continue will not be always present - we can ignore the return bool value
+			cont, _, err = unstructured.NestedString(jsonMap, "metadata", "continue")
+			if err != nil {
+				return nil, []error{err}
+			}
+			items, err := utils.NestedSliceWrapper(jsonMap, "items")
 			if err != nil {
 				return nil, []error{err}
 			}
@@ -105,21 +121,21 @@ func gatherInstallPlans(ctx context.Context,
 func collectInstallPlan(recs map[string]*collectedPlan, item interface{}) []error {
 	// Get common prefix
 	csv := "[NONE]"
-	var clusterServiceVersionNames []interface{}
-	var ns, genName string
 	var itemMap map[string]interface{}
 	var ok bool
 	if itemMap, ok = item.(map[string]interface{}); !ok {
 		return []error{fmt.Errorf("cannot cast item to map %v", item)}
 	}
 
-	err := failEarly(
-		func() error {
-			return utils.ParseJSONQuery(itemMap, "spec.clusterServiceVersionNames", &clusterServiceVersionNames)
-		},
-		func() error { return utils.ParseJSONQuery(itemMap, "metadata.namespace", &ns) },
-		func() error { return utils.ParseJSONQuery(itemMap, "metadata.generateName", &genName) },
-	)
+	clusterServiceVersionNames, err := utils.NestedSliceWrapper(itemMap, "spec", "clusterServiceVersionNames")
+	if err != nil {
+		return []error{err}
+	}
+	ns, err := utils.NestedStringWrapper(itemMap, "metadata", "namespace")
+	if err != nil {
+		return []error{err}
+	}
+	genName, err := utils.NestedStringWrapper(itemMap, "metadata", "generateName")
 	if err != nil {
 		return []error{err}
 	}
@@ -138,30 +154,6 @@ func collectInstallPlan(recs map[string]*collectedPlan, item interface{}) []erro
 	return nil
 }
 
-func failEarly(fns ...func() error) error {
-	for _, f := range fns {
-		err := f()
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-type collectedPlan struct {
-	Namespace string
-	Name      string
-	CSV       string
-	Count     int
-}
-
-// InstallPlanAnonymizer implements serialization of top x installplans
-type InstallPlanAnonymizer struct {
-	v     map[string]*collectedPlan
-	total int
-	limit int
-}
-
 // Marshal implements serialization of InstallPlan
 func (a InstallPlanAnonymizer) Marshal(_ context.Context) ([]byte, error) {
 	if a.limit == 0 {

pkg/gatherers/clusterconfig/install_plans_test.go

@@ -15,8 +15,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer/yaml"
 	dynamicfake "k8s.io/client-go/dynamic/fake"
 	kubefake "k8s.io/client-go/kubernetes/fake"
-
-	"github.com/openshift/insights-operator/pkg/utils"
 )
 
 //nolint: funlen, lll, gocyclo
@@ -79,8 +77,7 @@ func Test_InstallPlans_Gather(t *testing.T) {
 				}
 				gv, _ := schema.ParseGroupVersion(installplan.GetAPIVersion())
 				gvr := schema.GroupVersionResource{Version: gv.Version, Group: gv.Group, Resource: "installplans"}
-				var ns string
-				err = utils.ParseJSONQuery(installplan.Object, "metadata.namespace", &ns)
+				ns, _, err := unstructured.NestedString(installplan.Object, "metadata", "namespace")
 				if err != nil {
 					t.Fatal("unable to read ns ", err)
 				}

pkg/gatherers/clusterconfig/olm_operators.go

@@ -64,17 +64,16 @@ func gatherOLMOperators(ctx context.Context, dynamicClient dynamic.Interface) ([
 	if err != nil {
 		return nil, []error{err}
 	}
-	var refs []interface{}
 	olms := []olmOperator{}
 	errs := []error{}
 	for _, i := range olmOperators.Items {
 		newOlm := olmOperator{
 			Name: i.GetName(),
 		}
-		err := utils.ParseJSONQuery(i.Object, "status.components.refs", &refs)
+		refs, err := utils.NestedSliceWrapper(i.Object, "status", "components", "refs")
 		if err != nil {
 			// if no references are found then add an error and OLM operator with only name and continue
-			errs = append(errs, fmt.Errorf("cannot find \"status.components.refs\" in %s definition: %v", i.GetName(), err))
+			errs = append(errs, fmt.Errorf("cannot find \"status.components.refs\" in %s definition", i.GetName()))
 			olms = append(olms, newOlm)
 			continue
 		}
@@ -182,14 +181,13 @@ func getCsvFromRef(ctx context.Context, dynamicClient dynamic.Interface, csvRef
 // parseCsv tries to parse "status.conditions" and "spec.displayName" from the input map.
 // Returns an error if any of the values cannot be parsed.
 func parseCsv(csv map[string]interface{}) (name string, conditions []interface{}, err error) {
-	err = utils.ParseJSONQuery(csv, "status.conditions", &conditions)
+	conditions, err = utils.NestedSliceWrapper(csv, "status", "conditions")
 	if err != nil {
 		return "", nil, err
 	}
-	err = utils.ParseJSONQuery(csv, "spec.displayName", &name)
+	name, err = utils.NestedStringWrapper(csv, "spec", "displayName")
 	if err != nil {
 		return "", nil, err
 	}
-
-	return name, conditions, nil
+	return
 }

pkg/gatherers/clusterconfig/olm_operators_test.go

@@ -57,7 +57,7 @@ func Test_OLMOperators_Gather(t *testing.T) {
 			"Operator doesn't have CSV reference",
 			"testdata/olm_operator_2.yaml",
 			"testdata/csv_1.yaml",
-			fmt.Errorf("cannot find \"status.components.refs\" in test-olm-operator-with-no-ref definition: key refs wasn't found in map[] "),
+			fmt.Errorf("cannot find \"status.components.refs\" in test-olm-operator-with-no-ref definition"),
 			olmOperator{
 				Name: "test-olm-operator-with-no-ref",
 			},
@@ -66,7 +66,7 @@ func Test_OLMOperators_Gather(t *testing.T) {
 			"Operator CSV doesn't have the displayName",
 			"testdata/olm_operator_1.yaml",
 			"testdata/csv_2.yaml",
-			fmt.Errorf("cannot read test-olm-operator.v1.2.3 ClusterServiceVersion attributes: key displayName wasn't found in map[] "),
+			fmt.Errorf("cannot read test-olm-operator.v1.2.3 ClusterServiceVersion attributes: can't find spec.displayName"),
 			olmOperator{
 				Name:    "test-olm-operator",
 				Version: "v1.2.3",
@@ -116,7 +116,7 @@ func Test_OLMOperators_Gather(t *testing.T) {
 				}
 			}
 			if len(records) != 1 {
-				t.Fatalf("unexpected number or records %d", len(records))
+				t.Fatalf("unexpected number of records %d", len(records))
 			}
 			ooa, ok := records[0].Item.(record.JSONMarshaller).Object.([]olmOperator)
 			if !ok {

pkg/gatherers/clusterconfig/operators.go

@@ -151,12 +151,15 @@ func collectClusterOperatorResources(ctx context.Context,
 			if clusterResource == nil {
 				continue
 			}
-			var kind, name, apiVersion string
-			err = failEarly(
-				func() error { return utils.ParseJSONQuery(clusterResource.Object, "kind", &kind) },
-				func() error { return utils.ParseJSONQuery(clusterResource.Object, "apiVersion", &apiVersion) },
-				func() error { return utils.ParseJSONQuery(clusterResource.Object, "metadata.name", &name) },
-			)
+			kind, err := utils.NestedStringWrapper(clusterResource.Object, "kind")
+			if err != nil {
+				continue
+			}
+			apiVersion, err := utils.NestedStringWrapper(clusterResource.Object, "apiVersion")
+			if err != nil {
+				continue
+			}
+			name, err := utils.NestedStringWrapper(clusterResource.Object, "metadata", "name")
 			if err != nil {
 				continue
 			}

pkg/utils/parse_json_query.go

@@ -0,0 +1,41 @@
+package utils
+
+import (
+	"fmt"
+	"strings"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func NestedStringWrapper(obj map[string]interface{}, fields ...string) (string, error) {
+	s, ok, err := unstructured.NestedString(obj, fields...)
+	if !ok {
+		return "", fmt.Errorf("can't find %s", formatSlice(fields...))
+	}
+	if err != nil {
+		return "", err
+	}
+
+	return s, nil
+}
+
+func NestedSliceWrapper(obj map[string]interface{}, fields ...string) ([]interface{}, error) {
+	s, ok, err := unstructured.NestedSlice(obj, fields...)
+	if !ok {
+		return nil, fmt.Errorf("can't find %s", formatSlice(fields...))
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	return s, nil
+}
+
+func formatSlice(s ...string) string {
+	var str string
+	for _, f := range s {
+		str += fmt.Sprintf("%s.", f)
+	}
+	str = strings.TrimRight(str, ".")
+	return str
+}