Bug 1949990: Extend OLM data with CSV display name

docs/insights-archive-sample/config/olm_operators.json

@@ -1,6 +1,7 @@
 [
     {
         "name": "eap.openshift-operators",
+        "displayName": "JBoss EAP",
         "version": "v2.1.1",
         "csv_conditions": [
             {
@@ -42,6 +43,7 @@
     },
     {
         "name": "elasticsearch-operator.openshift-operators-redhat",
+        "displayName": "OpenShift Elasticsearch Operator",
         "version": "4.6.0-202102200141.p0",
         "csv_conditions": [
             {
@@ -83,6 +85,7 @@
     },
     {
         "name": "kiali-ossm.openshift-operators",
+        "displayName": "Kiali Operator",
         "version": "v1.24.5",
         "csv_conditions": [
             {
@@ -124,6 +127,7 @@
     },
     {
         "name": "postgresql-operator-dev4devs-com.psql-test",
+        "displayName": "PostgreSQL Operator by Dev4Ddevs.com",
         "version": "v0.1.1",
         "csv_conditions": [
             {
@@ -165,6 +169,7 @@
     },
     {
         "name": "radanalytics-spark.openshift-operators",
+        "displayName": "Apache Spark Operator",
         "version": "v1.1.0",
         "csv_conditions": [
             {

pkg/gather/clusterconfig/olm_operators.go

@@ -2,11 +2,11 @@ package clusterconfig
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/klog/v2"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
@@ -17,13 +17,17 @@ import (
 
 var (
 	operatorGVR              = schema.GroupVersionResource{Group: "operators.coreos.com", Version: "v1", Resource: "operators"}
-	clusterServiceVersionGVR = schema.GroupVersionResource{Group: "operators.coreos.com", Version: "v1alpha1", Resource: "clusterserviceversions"}
+	clusterServiceVersionGVR = schema.GroupVersionResource{
+		Group:    "operators.coreos.com",
+		Version:  "v1alpha1",
+		Resource: "clusterserviceversions"}
 )
 
 type olmOperator struct {
-	Name       string        `json:"name"`
-	Version    string        `json:"version"`
-	Conditions []interface{} `json:"csv_conditions"`
+	Name        string        `json:"name"`
+	DisplayName string        `json:"displayName"`
+	Version     string        `json:"version"`
+	Conditions  []interface{} `json:"csv_conditions"`
 }
 
 // ClusterServiceVersion helper struct
@@ -64,32 +68,46 @@ func gatherOLMOperators(ctx context.Context, dynamicClient dynamic.Interface) ([
 		return nil, []error{err}
 	}
 	var refs []interface{}
-	var olms []olmOperator
+	olms := []olmOperator{}
+	errs := []error{}
 	for _, i := range olmOperators.Items {
+		newOlm := olmOperator{
+			Name: i.GetName(),
+		}
 		err := utils.ParseJSONQuery(i.Object, "status.components.refs", &refs)
 		if err != nil {
-			klog.Errorf("Cannot find \"status.components.refs\" in %s definition: %v", i.GetName(), err)
+			// 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))
+			olms = append(olms, newOlm)
 			continue
 		}
 		for _, r := range refs {
-			csvRef := getCSVRefFromRefs(r)
+			csvRef, err := findCSVRefInRefs(r)
+			if err != nil {
+				errs = append(errs, err)
+				olms = append(olms, newOlm)
+				continue
+			}
+			// CSV reference can still be nil
 			if csvRef == nil {
 				continue
 			}
-			conditions, err := getCSVConditions(ctx, dynamicClient, csvRef)
+			newOlm.Version = csvRef.Version
+
+			name, conditions, err := getCSVAndParse(ctx, dynamicClient, csvRef)
 			if err != nil {
-				klog.Errorf("failed to get %s conditions: %v", csvRef.Name, err)
+				// append the error and the OLM data we already have and continue
+				errs = append(errs, err)
+				olms = append(olms, newOlm)
 				continue
 			}
-			olmO := olmOperator{
-				Name:       i.GetName(),
-				Version:    csvRef.Version,
-				Conditions: conditions,
-			}
-			if isInArray(olmO, olms) {
+			newOlm.DisplayName = name
+			newOlm.Conditions = conditions
+
+			if isInArray(newOlm, olms) {
 				continue
 			}
-			olms = append(olms, olmO)
+			olms = append(olms, newOlm)
 		}
 	}
 	if len(olms) == 0 {
@@ -99,6 +117,9 @@ func gatherOLMOperators(ctx context.Context, dynamicClient dynamic.Interface) ([
 		Name: "config/olm_operators",
 		Item: record.JSONMarshaller{Object: olms},
 	}
+	if len(errs) != 0 {
+		return []record.Record{r}, errs
+	}
 	return []record.Record{r}, nil
 }
 
@@ -111,35 +132,67 @@ func isInArray(o olmOperator, a []olmOperator) bool {
 	return false
 }
 
-func getCSVRefFromRefs(r interface{}) *csvRef {
+//getCSVAndParse gets full CSV definition from csvRef and tries to parse the definition
+func getCSVAndParse(ctx context.Context, dynamicClient dynamic.Interface, csvRef *csvRef) (name string, conditions []interface{}, err error) {
+	csv, err := getCsvFromRef(ctx, dynamicClient, csvRef)
+	if err != nil {
+		return "", nil, fmt.Errorf("failed to get %s ClusterServiceVersion: %v", csvRef.Name, err)
+	}
+	name, conditions, err = parseCsv(csv)
+
+	if err != nil {
+		return "", nil, fmt.Errorf("cannot read %s ClusterServiceVersion attributes: %v", csvRef.Name, err)
+	}
+
+	return name, conditions, nil
+}
+
+//findCSVRefInRefs tries to find ClusterServiceVersion reference in the references
+//and parse the ClusterServiceVersion if successful.
+//It can return nil with no error if the CSV was not found
+func findCSVRefInRefs(r interface{}) (*csvRef, error) {
 	refMap, ok := r.(map[string]interface{})
 	if !ok {
-		klog.Errorf("Cannot convert %s to map[string]interface{}", r)
-		return nil
+		return nil, fmt.Errorf("cannot convert %s to map[string]interface{}", r)
 	}
 	// version is part of the name of ClusterServiceVersion
 	if refMap["kind"] == "ClusterServiceVersion" {
 		name := refMap["name"].(string)
+		if !strings.Contains(name, ".") {
+			return nil, fmt.Errorf("clusterserviceversion \"%s\" probably doesn't include version", name)
+		}
 		nameVer := strings.SplitN(name, ".", 2)
 		csvRef := &csvRef{
 			Name:      name,
 			Namespace: refMap["namespace"].(string),
 			Version:   nameVer[1],
 		}
-		return csvRef
+		return csvRef, nil
 	}
-	return nil
+	return nil, nil
 }
 
-func getCSVConditions(ctx context.Context, dynamicClient dynamic.Interface, csvRef *csvRef) ([]interface{}, error) {
+func getCsvFromRef(ctx context.Context, dynamicClient dynamic.Interface, csvRef *csvRef) (map[string]interface{}, error) {
 	csv, err := dynamicClient.Resource(clusterServiceVersionGVR).Namespace(csvRef.Namespace).Get(ctx, csvRef.Name, metav1.GetOptions{})
 	if err != nil {
 		return nil, err
 	}
+	return csv.Object, nil
+}
+
+//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{}) (string, []interface{}, error) {
 	var conditions []interface{}
-	err = utils.ParseJSONQuery(csv.Object, "status.conditions", &conditions)
+	var name string
+	err := utils.ParseJSONQuery(csv, "status.conditions", &conditions)
 	if err != nil {
-		return nil, err
+		return "", nil, err
 	}
-	return conditions, nil
+	err = utils.ParseJSONQuery(csv, "spec.displayName", &name)
+	if err != nil {
+		return "", nil, err
+	}
+
+	return name, conditions, nil
 }

pkg/gather/clusterconfig/olm_operators_test.go

@@ -2,8 +2,10 @@ package clusterconfig
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
+	"reflect"
 	"testing"
 
 	"github.com/openshift/insights-operator/pkg/record"
@@ -16,49 +18,114 @@ import (
 )
 
 func Test_OLMOperators_Gather(t *testing.T) {
-	olmOpContent, err := readFromFile("testdata/olm_operator_1.yaml")
-	if err != nil {
-		t.Fatal("test failed to read OLM operator data", err)
+	var cases = []struct {
+		testName            string
+		olmOperatorFileName string
+		csvFileName         string
+		expectedError       error
+		expecteOlmOperator  olmOperator
+	}{
+		{
+			"All OLM operator data is available",
+			"testdata/olm_operator_1.yaml",
+			"testdata/csv_1.yaml",
+			nil,
+			olmOperator{
+				Name:        "test-olm-operator",
+				DisplayName: "Testing operator",
+				Version:     "v1.2.3",
+				Conditions: []interface{}{
+					map[string]interface{}{
+						"lastTransitionTime": "2021-03-02T08:52:24Z",
+						"lastUpdateTime":     "2021-03-02T08:52:24Z",
+						"message":            "requirements not yet checked",
+						"phase":              "Pending",
+						"reason":             "RequirementsUnknown",
+					},
+					map[string]interface{}{
+						"lastTransitionTime": "2021-03-02T08:52:24Z",
+						"lastUpdateTime":     "2021-03-02T08:52:24Z",
+						"message":            "all requirements found, attempting install",
+						"phase":              "InstallReady",
+						"reason":             "AllRequirementsMet",
+					},
+				},
+			},
+		},
+		{
+			"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[] "),
+			olmOperator{
+				Name: "test-olm-operator-with-no-ref",
+			},
+		},
+		{
+			"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[] "),
+			olmOperator{
+				Name:    "test-olm-operator",
+				Version: "v1.2.3",
+			},
+		},
+		{
+			"Operator with unrecognizable CSV version",
+			"testdata/olm_operator_3.yaml",
+			"testdata/csv_1.yaml",
+			fmt.Errorf("clusterserviceversion \"name-without-version\" probably doesn't include version"),
+			olmOperator{
+				Name: "test-olm-operator-no-version",
+			},
+		},
 	}
 
-	csvContent, err := readFromFile("testdata/csv_1.yaml")
-	if err != nil {
-		t.Fatal("test failed to read CSV ", err)
-	}
-	client := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
-		operatorGVR:              "OperatorsList",
-		clusterServiceVersionGVR: "ClusterServiceVersionsList",
-	})
-	err = createUnstructuredResource(olmOpContent, client, operatorGVR)
-	if err != nil {
-		t.Fatal("cannot create OLM operator ", err)
-	}
-	err = createUnstructuredResource(csvContent, client, clusterServiceVersionGVR)
-	if err != nil {
-		t.Fatal("cannot create ClusterServiceVersion ", err)
-	}
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.testName, func(t *testing.T) {
+			olmOpContent, err := readFromFile(tt.olmOperatorFileName)
+			if err != nil {
+				t.Fatal("test failed to read OLM operator data", err)
+			}
 
-	ctx := context.Background()
-	records, errs := gatherOLMOperators(ctx, client)
-	if len(errs) > 0 {
-		t.Errorf("unexpected errors: %#v", errs)
-		return
-	}
-	if len(records) != 1 {
-		t.Fatalf("unexpected number or records %d", len(records))
-	}
-	ooa, ok := records[0].Item.(record.JSONMarshaller).Object.([]olmOperator)
-	if !ok {
-		t.Fatalf("returned item is not of type []olmOperator")
-	}
-	if ooa[0].Name != "test-olm-operator" {
-		t.Fatalf("unexpected name of gathered OLM operator %s", ooa[0].Name)
-	}
-	if ooa[0].Version != "v1.2.3" {
-		t.Fatalf("unexpected version of gathered OLM operator %s", ooa[0].Version)
-	}
-	if len(ooa[0].Conditions) != 2 {
-		t.Fatalf("unexpected number of conditions %s", ooa[0].Conditions...)
+			csvContent, err := readFromFile(tt.csvFileName)
+			if err != nil {
+				t.Fatal("test failed to read CSV ", err)
+			}
+			client := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
+				operatorGVR:              "OperatorsList",
+				clusterServiceVersionGVR: "ClusterServiceVersionsList",
+			})
+			err = createUnstructuredResource(olmOpContent, client, operatorGVR)
+			if err != nil {
+				t.Fatal("cannot create OLM operator ", err)
+			}
+			err = createUnstructuredResource(csvContent, client, clusterServiceVersionGVR)
+			if err != nil {
+				t.Fatal("cannot create ClusterServiceVersion ", err)
+			}
+
+			ctx := context.Background()
+			records, errs := gatherOLMOperators(ctx, client)
+			if len(errs) > 0 {
+				if errs[0].Error() != tt.expectedError.Error() {
+					t.Fatalf("unexpected errors: %v", errs[0].Error())
+				}
+			}
+			if len(records) != 1 {
+				t.Fatalf("unexpected number or records %d", len(records))
+			}
+			ooa, ok := records[0].Item.(record.JSONMarshaller).Object.([]olmOperator)
+			if !ok {
+				t.Fatalf("returned item is not of type []olmOperator")
+			}
+			sameOp := reflect.DeepEqual(ooa[0], tt.expecteOlmOperator)
+			if !sameOp {
+				t.Fatalf("Gathered %s operator is not equal to expected %s ", ooa[0], tt.expecteOlmOperator)
+			}
+		})
 	}
 }
 

pkg/gather/clusterconfig/testdata/csv_1.yaml

@@ -3,6 +3,8 @@ kind: ClusterServiceVersion
 metadata:
     name: test-olm-operator.v1.2.3
     namespace: test-olm-operator
+spec:
+    displayName: "Testing operator"    
 status:
     conditions:
         - lastTransitionTime: "2021-03-02T08:52:24Z"