Show total checks passed in successes count

Previously the "successes" field in the appstudio output showed just a count of each component with an overal pass. Change it to show a count of the number of passing checks. It seems more intuitive to do it this way, and I think probably the reason it's like this is because the relevant code pre-dates the ability to know how many passing checks there were. Notes: - To ensure the success count is present even when --show-success isn't used, there's an extra field added to the applicationsnapshot.Component struct. - I decide not to include the successCount value in the normal report output, hence the `json:"-"` in the struct definition. Res: https://issues.redhat.com/browse/HACBS-2694
enterprise-contract · Oct 3, 2023 · d40c6f3 · d40c6f3
1 parent 00cc303
commit d40c6f3
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 39 deletions.
diff --git a/cmd/validate/image.go b/cmd/validate/image.go
@@ -292,9 +292,13 @@ func validateImageCmd(validate imageValidationFunc) *cobra.Command {
 						res.component.Violations = out.Violations()
 						showSuccesses, _ := cmd.Flags().GetBool("show-successes")
 						res.component.Warnings = out.Warnings()
+
+						successes := out.Successes()
+						res.component.SuccessCount = len(successes)
 						if showSuccesses {
-							res.component.Successes = out.Successes()
+							res.component.Successes = successes
 						}
+
 						res.component.Signatures = out.Signatures
 						res.component.Attestations = out.Attestations
 						res.component.ContainerImage = out.ImageURL

diff --git a/features/__snapshots__/task_validate_image.snap b/features/__snapshots__/task_validate_image.snap
@@ -372,25 +372,25 @@ TUF_MIRROR not set. Skipping TUF root initialization.
 
 [Strict with warnings:results - 1]
 {
-  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":1,\"failures\":0,\"warnings\":1,\"result\":\"WARNING\"}"
+  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":3,\"failures\":0,\"warnings\":1,\"result\":\"WARNING\"}"
 }
 ---
 
 [Non strict with warnings:results - 1]
 {
-  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":1,\"failures\":0,\"warnings\":1,\"result\":\"WARNING\"}"
+  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":3,\"failures\":0,\"warnings\":1,\"result\":\"WARNING\"}"
 }
 ---
 
 [Golden container image:results - 1]
 {
-  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":1,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}"
+  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":4,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}"
 }
 ---
 
 [Initialize TUF succeeds:results - 1]
 {
-  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":1,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}"
+  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":4,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}"
 }
 ---
 
@@ -475,7 +475,7 @@ success: true
 {
   "timestamp": "${TIMESTAMP}",
   "namespace": "",
-  "successes": 1,
+  "successes": 3,
   "failures": 0,
   "warnings": 0,
   "result": "SUCCESS"
@@ -488,7 +488,7 @@ true
 
 [Outputs are there:results - 1]
 {
-  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":1,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}"
+  "TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":3,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}"
 }
 ---
 

diff --git a/features/__snapshots__/validate_image.snap b/features/__snapshots__/validate_image.snap
@@ -87,7 +87,7 @@ Error: success criteria not met
 {
   "timestamp": "${TIMESTAMP}",
   "namespace": "",
-  "successes": 0,
+  "successes": 4,
   "failures": 1,
   "warnings": 0,
   "result": "FAILURE"

diff --git a/internal/applicationsnapshot/report.go b/internal/applicationsnapshot/report.go
@@ -42,6 +42,7 @@ type Component struct {
 	Warnings     []evaluator.Result          `json:"warnings,omitempty"`
 	Successes    []evaluator.Result          `json:"successes,omitempty"`
 	Success      bool                        `json:"success"`
+	SuccessCount int                         `json:"-"`
 	Signatures   []signature.EntitySignature `json:"signatures,omitempty"`
 	Attestations []attestation.Attestation   `json:"attestations,omitempty"`
 }
@@ -198,12 +199,16 @@ func (r *Report) toSummary() summary {
 		c := componentSummary{
 			TotalViolations: len(cmp.Violations),
 			TotalWarnings:   len(cmp.Warnings),
-			TotalSuccesses:  len(cmp.Successes),
-			Success:         cmp.Success,
-			Name:            cmp.Name,
-			Violations:      condensedMsg(cmp.Violations),
-			Warnings:        condensedMsg(cmp.Warnings),
-			Successes:       condensedMsg(cmp.Successes),
+
+			// Because cmp.Successes does not get populated unless the --show-successes
+			// flag was set, cmp.SuccessCount is used here instead of len(cmp.Successes)
+			TotalSuccesses: cmp.SuccessCount,
+
+			Success:    cmp.Success,
+			Name:       cmp.Name,
+			Violations: condensedMsg(cmp.Violations),
+			Warnings:   condensedMsg(cmp.Warnings),
+			Successes:  condensedMsg(cmp.Successes),
 		}
 		pr.Components = append(pr.Components, c)
 	}
@@ -247,12 +252,12 @@ func (r *Report) toAppstudioReport() TestReport {
 	}
 
 	hasFailures := false
-	for _, component := range r.Components {
-		result.Failures += len(component.Violations)
-		result.Warnings += len(component.Warnings)
-		if component.Success {
-			result.Successes += 1
-		} else {
+	for _, component := range r.toSummary().Components {
+		result.Failures += component.TotalViolations
+		result.Warnings += component.TotalWarnings
+		result.Successes += component.TotalSuccesses
+
+		if !component.Success {
 			// It is possible, although quite unusual, that a component has no
 			// listed violations but is still marked as not successful.
 			hasFailures = true

diff --git a/internal/applicationsnapshot/report_test.go b/internal/applicationsnapshot/report_test.go
@@ -314,7 +314,8 @@ func Test_ReportSummary(t *testing.T) {
 						},
 					},
 				},
-				Success: false,
+				Success:      false,
+				SuccessCount: 1,
 			},
 			want: summary{
 				Components: []componentSummary{
@@ -361,7 +362,8 @@ func Test_ReportSummary(t *testing.T) {
 						},
 					},
 				},
-				Success: false,
+				Success:      false,
+				SuccessCount: 1,
 			},
 			want: summary{
 				Snapshot: "snappy",
@@ -409,12 +411,16 @@ func Test_ReportAppstudio(t *testing.T) {
 				"failures": 0,
 				"namespace": "",
 				"result": "SUCCESS",
-				"successes": 3,
+				"successes": 42,
 				"timestamp": "0",
 				"warnings": 0
 			}`,
-			components: []Component{{Success: true}, {Success: true}, {Success: true}},
-			success:    true,
+			components: []Component{
+				{Success: true, SuccessCount: 39},
+				{Success: true, SuccessCount: 2},
+				{Success: true, SuccessCount: 1},
+			},
+			success: true,
 		},
 		{
 			name: "warning",
@@ -428,8 +434,8 @@ func Test_ReportAppstudio(t *testing.T) {
 				"warnings": 1
 			}`,
 			components: []Component{
-				{Success: true},
-				{Success: true, Warnings: []evaluator.Result{{Message: "this is a warning"}}},
+				{Success: true, SuccessCount: 1},
+				{Success: true, SuccessCount: 1, Warnings: []evaluator.Result{{Message: "this is a warning"}}},
 			},
 			success: true,
 		},
@@ -445,7 +451,7 @@ func Test_ReportAppstudio(t *testing.T) {
 				"warnings": 0
 			}`,
 			components: []Component{
-				{Success: true},
+				{Success: true, SuccessCount: 1},
 				{Success: false, Violations: []evaluator.Result{{Message: "this is a violation"}}},
 			},
 			success: false,
@@ -461,7 +467,7 @@ func Test_ReportAppstudio(t *testing.T) {
 				"timestamp": "0",
 				"warnings": 0
 			}`,
-			components: []Component{{Success: false}, {Success: true}},
+			components: []Component{{Success: false}, {Success: true, SuccessCount: 1}},
 			success:    false,
 		},
 		{
@@ -476,7 +482,7 @@ func Test_ReportAppstudio(t *testing.T) {
 				"warnings": 1
 			}`,
 			components: []Component{
-				{Success: true},
+				{Success: true, SuccessCount: 1},
 				{Success: false, Violations: []evaluator.Result{{Message: "this is a violation"}}},
 				{Success: false, Warnings: []evaluator.Result{{Message: "this is a warning"}}},
 			},
@@ -508,7 +514,7 @@ func Test_ReportAppstudio(t *testing.T) {
 				"warnings": 0
 			}`,
 			snapshot:   "snappy",
-			components: []Component{{Success: true}, {Success: true}, {Success: true}},
+			components: []Component{{Success: true, SuccessCount: 3}, {Success: true}, {Success: true}},
 			success:    true,
 		},
 	}
@@ -557,11 +563,11 @@ func Test_ReportHACBS(t *testing.T) {
 				"failures": 0,
 				"namespace": "",
 				"result": "SUCCESS",
-				"successes": 3,
+				"successes": 42,
 				"timestamp": "0",
 				"warnings": 0
 			}`,
-			components: []Component{{Success: true}, {Success: true}, {Success: true}},
+			components: []Component{{Success: true, SuccessCount: 42}, {Success: true}, {Success: true}},
 			success:    true,
 		},
 		{
@@ -576,8 +582,8 @@ func Test_ReportHACBS(t *testing.T) {
 				"warnings": 1
 			}`,
 			components: []Component{
-				{Success: true},
-				{Success: true, Warnings: []evaluator.Result{{Message: "this is a warning"}}},
+				{Success: true, SuccessCount: 1},
+				{Success: true, SuccessCount: 1, Warnings: []evaluator.Result{{Message: "this is a warning"}}},
 			},
 			success: true,
 		},
@@ -593,7 +599,7 @@ func Test_ReportHACBS(t *testing.T) {
 				"warnings": 0
 			}`,
 			components: []Component{
-				{Success: true},
+				{Success: true, SuccessCount: 1},
 				{Success: false, Violations: []evaluator.Result{{Message: "this is a violation"}}},
 			},
 			success: false,
@@ -609,7 +615,7 @@ func Test_ReportHACBS(t *testing.T) {
 				"timestamp": "0",
 				"warnings": 0
 			}`,
-			components: []Component{{Success: false}, {Success: true}},
+			components: []Component{{Success: false}, {Success: true, SuccessCount: 1}},
 			success:    false,
 		},
 		{
@@ -624,7 +630,7 @@ func Test_ReportHACBS(t *testing.T) {
 				"warnings": 1
 			}`,
 			components: []Component{
-				{Success: true},
+				{Success: true, SuccessCount: 1},
 				{Success: false, Violations: []evaluator.Result{{Message: "this is a violation"}}},
 				{Success: false, Warnings: []evaluator.Result{{Message: "this is a warning"}}},
 			},
@@ -656,7 +662,7 @@ func Test_ReportHACBS(t *testing.T) {
 				"warnings": 0
 			}`,
 			snapshot:   "snappy",
-			components: []Component{{Success: true}, {Success: true}, {Success: true}},
+			components: []Component{{Success: true, SuccessCount: 1}, {Success: true, SuccessCount: 1}, {Success: true, SuccessCount: 1}},
 			success:    true,
 		},
 	}