Handle reports with missing result property (#2696)

* Handle reports with missing result property

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

* Make use of type structs

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

* Fix import

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

* Fix cast from map to struct

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

api/policyreport/v1alpha2/policyreport_types.go

@@ -14,6 +14,8 @@ limitations under the License.
 package v1alpha2
 
 import (
+	"encoding/json"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -61,6 +63,13 @@ type PolicyReportSummary struct {
 	Skip int `json:"skip"`
 }
 
+func (prs PolicyReportSummary) ToMap() map[string]interface{} {
+	b, _ := json.Marshal(&prs)
+	var m map[string]interface{}
+	_ = json.Unmarshal(b, &m)
+	return m
+}
+
 // PolicyResult has one of the following values:
 //   - pass: indicates that the policy requirements are met
 //   - fail: indicates that the policy requirements are not met

pkg/policyreport/policyreport.go

@@ -1,6 +1,7 @@
 package policyreport
 
 import (
+	"encoding/json"
 	"fmt"
 	"reflect"
 	"strings"
@@ -98,8 +99,13 @@ func updateResults(oldReport, newReport map[string]interface{}, aggregatedReques
 		return nil, hasDuplicate, err
 	}
 
-	summary := updateSummary(results)
+	summaryResults := []report.PolicyReportResult{}
-	if err := unstructured.SetNestedMap(newReport, summary, "summary"); err != nil {
+	if err := mapToStruct(results, &summaryResults); err != nil {
+		return nil, hasDuplicate, err
+	}
+
+	summary := updateSummary(summaryResults)
+	if err := unstructured.SetNestedMap(newReport, summary.ToMap(), "summary"); err != nil {
 		return nil, hasDuplicate, err
 	}
 	return newReport, hasDuplicate, nil
@@ -172,40 +178,24 @@ func generateHashKey(result map[string]interface{}, dr deletedResource) (string,
 		resource["name"]), true
 }
 
-func updateSummary(results []interface{}) map[string]interface{} {
+func updateSummary(results []report.PolicyReportResult) report.PolicyReportSummary {
-	summary := make(map[string]interface{}, 5)
+	summary := report.PolicyReportSummary{}
 
 	for _, result := range results {
-		typedResult, ok := result.(map[string]interface{})
+		switch result.Result {
-		if !ok {
-			continue
-		}
-
-		switch typedResult["result"].(string) {
 		case report.StatusPass:
-			pass, _ := summary[report.StatusPass].(int64)
+			summary.Pass++
-			summary[report.StatusPass] = pass + 1
 		case report.StatusFail:
-			fail, _ := summary[report.StatusFail].(int64)
+			summary.Fail++
-			summary[report.StatusFail] = fail + 1
 		case report.StatusWarn:
-			warn, _ := summary[report.StatusWarn].(int64)
+			summary.Warn++
-			summary[report.StatusWarn] = warn + 1
 		case report.StatusError:
-			e, _ := summary[report.StatusError].(int64)
+			summary.Error++
-			summary[report.StatusError] = e + 1
 		case report.StatusSkip:
-			skip, _ := summary[report.StatusSkip].(int64)
+			summary.Skip++
-			summary[report.StatusSkip] = skip + 1
 		}
 	}
 
-	status := []string{report.StatusPass, report.StatusFail, report.StatusError, report.StatusSkip, report.StatusWarn}
-	for i := 0; i < 5; i++ {
-		if _, ok := summary[status[i]].(int64); !ok {
-			summary[status[i]] = int64(0)
-		}
-	}
 	return summary
 }
 
@@ -225,3 +215,8 @@ func isDeletedPolicyKey(key string) (policyName, ruleName string, isDelete bool)
 
 	return "", "", false
 }
+
+func mapToStruct(in, out interface{}) error {
+	jsonBytes, _ := json.Marshal(in)
+	return json.Unmarshal(jsonBytes, out)
+}

pkg/policyreport/policyreport_test.go

@@ -0,0 +1,70 @@
+package policyreport
+
+import (
+	"testing"
+
+	report "github.com/kyverno/kyverno/api/policyreport/v1alpha2"
+)
+
+var validReportStatuses = []string{"pass", "fail", "error", "skip", "warn"}
+
+func TestUpdateSummary_Successful(t *testing.T) {
+	results := []report.PolicyReportResult{}
+	for _, status := range validReportStatuses {
+		results = append(results, report.PolicyReportResult{
+			Result: report.PolicyResult(status),
+			Policy: "TestUpdateSummary_Successful",
+			Source: "Kyverno",
+		})
+	}
+
+	summary := updateSummary(results)
+
+	if summary.Pass != 1 {
+		t.Errorf("Was expecting status pass to have a count of 1")
+	}
+	if summary.Error != 1 {
+		t.Errorf("Was expecting status error to have a count of 1")
+	}
+	if summary.Fail != 1 {
+		t.Errorf("Was expecting status fail to have a count of 1")
+	}
+	if summary.Skip != 1 {
+		t.Errorf("Was expecting status skip to have a count of 1")
+	}
+	if summary.Warn != 1 {
+		t.Errorf("Was expecting status warn to have a count of 1")
+	}
+}
+func TestUpdateSummary_MissingResultField(t *testing.T) {
+	results := []report.PolicyReportResult{
+		{
+			Policy: "TestUpdateSummary_MissingResultField",
+			Source: "Kyverno",
+		},
+	}
+
+	defer func() {
+		if r := recover(); r != nil {
+			t.Error("Function should not cause a panic")
+		}
+	}()
+
+	summary := updateSummary(results)
+
+	if summary.Pass != 0 {
+		t.Errorf("Was expecting status pass to have a count of 0")
+	}
+	if summary.Error != 0 {
+		t.Errorf("Was expecting status error to have a count of 0")
+	}
+	if summary.Fail != 0 {
+		t.Errorf("Was expecting status fail to have a count of 0")
+	}
+	if summary.Skip != 0 {
+		t.Errorf("Was expecting status skip to have a count of 0")
+	}
+	if summary.Warn != 0 {
+		t.Errorf("Was expecting status warn to have a count of 0")
+	}
+}