Merge pull request #1656 from simonbaird/text-output-group-by-result

simonbaird · web-flow · commit 62077017c964 · 2024-05-31T16:10:35.000-04:00
Rework validate image human readable text output
diff --git a/features/__snapshots__/validate_image.snap b/features/__snapshots__/validate_image.snap
@@ -1722,52 +1722,58 @@ Error: success criteria not met
 
 ---
 [detailed failures output:${TMPDIR}/output.txt - 1]
-Name:       Unnamed
-ImageRef:   ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
-Summary:    Violations: 3, Warnings: 0, Successes: 4
-Violations:
-[31m✕[0m [31mmain.reject_with_term[0m
-  Fails always (term1)
-  Reject with term rule
-  Description: This rule will always fail. To exclude this rule add
-  "main.reject_with_term:term1" to the `exclude` section of the policy
-  configuration.
+Success: false
+Result: FAILURE
+Violations: 3, Warnings: 0, Successes: 4
+Component: Unnamed
+ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+
+Results:
+[31m✕[0m [31m[Violation] main.reject_with_term[0m
+  ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+  Reason: Fails always (term1)
+  Title: Reject with term rule
+  Description: This rule will always fail. To exclude this rule add "main.reject_with_term:term1" to the `exclude` section of the
+  policy configuration.
   Solution: None
 
-[31m✕[0m [31mmain.reject_with_term[0m
-  Fails always (term2)
-  Reject with term rule
-  Description: This rule will always fail. To exclude this rule add
-  "main.reject_with_term:term2" to the `exclude` section of the policy
-  configuration.
+[31m✕[0m [31m[Violation] main.reject_with_term[0m
+  ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+  Reason: Fails always (term2)
+  Title: Reject with term rule
+  Description: This rule will always fail. To exclude this rule add "main.reject_with_term:term2" to the `exclude` section of the
+  policy configuration.
   Solution: None
 
-[31m✕[0m [31mmain.rejector[0m
-  Fails always
-  Reject rule
-  Description: This rule will always fail. To exclude this rule add
-  "main.rejector" to the `exclude` section of the policy configuration.
+[31m✕[0m [31m[Violation] main.rejector[0m
+  ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+  Reason: Fails always
+  Title: Reject rule
+  Description: This rule will always fail. To exclude this rule add "main.rejector" to the `exclude` section of the policy
+  configuration.
   Solution: None
 
-Successes:
-[32m✓[0m [32mbuiltin.attestation.signature_check[0m
-  Attestation signature check passed
+[32m✓[0m [32m[Success] builtin.attestation.signature_check[0m
+  ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+  Title: Attestation signature check passed
   Description: The attestation signature matches available signing materials.
 
-[32m✓[0m [32mbuiltin.attestation.syntax_check[0m
-  Attestation syntax check passed
+[32m✓[0m [32m[Success] builtin.attestation.syntax_check[0m
+  ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+  Title: Attestation syntax check passed
   Description: The attestation has correct syntax.
 
-[32m✓[0m [32mbuiltin.image.signature_check[0m
-  Image signature check passed
+[32m✓[0m [32m[Success] builtin.image.signature_check[0m
+  ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+  Title: Image signature check passed
   Description: The image signature matches available signing materials.
 
-[32m✓[0m [32mmain.acceptor[0m
-  Allow rule
+[32m✓[0m [32m[Success] main.acceptor[0m
+  ImageRef: ${REGISTRY}/acceptance/image@sha256:${REGISTRY_acceptance/image:latest_DIGEST}
+  Title: Allow rule
   Description: This rule will never fail
 
 
-
 ---
 [future failure is a deny when using effective-date flag:stdout - 1]
 {
diff --git a/internal/applicationsnapshot/report.go b/internal/applicationsnapshot/report.go
@@ -310,7 +310,19 @@ func generateMarkdownSummary(r *Report) ([]byte, error) {
 var efs embed.FS
 
 func generateTextReport(r *Report) ([]byte, error) {
-	return utils.RenderFromTemplatesWithMain(r, "text_report.tmpl", efs)
+	// Prepare some template input
+	input := struct {
+		Report     *Report
+		TestReport TestReport
+	}{
+		// This includes everything in the yaml/json output
+		Report: r,
+		// This has useful stuff we want to output, so let's reuse it
+		// even though this is not what it was originally designed for
+		TestReport: r.toAppstudioReport(),
+	}
+
+	return utils.RenderFromTemplatesWithMain(input, "text_report.tmpl", efs)
 }
 
 func writeMarkdownField(buffer *bytes.Buffer, name string, value any, icon string) {
diff --git a/internal/applicationsnapshot/templates/_components.tmpl b/internal/applicationsnapshot/templates/_components.tmpl
@@ -0,0 +1,25 @@
+{{- if gt (len .) 1 -}}
+{{/*
+  When there are multiple components we provide totals for each
+  component and change the format to make it more readable as a list
+*/}}
+Components:
+{{ range . -}}
+- Name: {{ .Name }}
+  ImageRef: {{ .ContainerImage }}
+  Violations: {{ len .Violations }}, Warnings: {{ len .Warnings }}, Successes: {{ .SuccessCount }}
+
+{{ end -}}
+
+{{- else -}}
+{{/*
+  When there is only one component the summary counts are the same
+  as the overall counts, so we don't need to repeat them.
+  (Using range here even though we know there is only one item.)
+*/}}
+{{- range . -}}
+Component: {{ .Name }}
+ImageRef: {{ .ContainerImage }}
+
+{{ end -}}
+{{- end -}}
diff --git a/internal/applicationsnapshot/templates/_results.tmpl b/internal/applicationsnapshot/templates/_results.tmpl
@@ -1,18 +1,42 @@
-{{- $color := .Color -}}
+{{- $type := .Type -}}
+{{- $wrap := 130 -}}
 {{- $indent := 2 -}}
-{{- $wrap := 78 -}}
-{{- range .Results -}}
-{{- colorIndicator $color }} {{ colorText $color .Metadata.code }}
-{{ if and (ne .Message "Pass") .Message -}}
-  {{- indentWrap $indent $wrap .Message }}
-{{ end -}}
-{{ if .Metadata.title -}}
-  {{- indentWrap $indent $wrap .Metadata.title }}
-{{ end -}}
-{{ if .Metadata.description -}}
-  {{- indentWrap $indent $wrap (printf "Description: %s" .Metadata.description) }}
-{{ end -}}
-{{ if and (ne .Message "Pass") .Metadata.solution -}}
-  {{- indentWrap $indent $wrap (printf "Solution: %s" .Metadata.solution) }}
-{{ end }}
-{{ end -}}
+
+{{- range .Components -}}
+  {{- $imageRef := .ContainerImage -}}
+
+  {{ $results := "" }}
+  {{- if eq $type "Violation" -}}{{- $results = .Violations -}}
+  {{- else if eq $type "Warning" -}}{{- $results = .Warnings -}}
+  {{- else if eq $type "Success" -}}{{- $results = .Successes  -}}
+  {{- end -}}
+
+  {{- range $results -}}
+    {{/* Assume .Metadata.code is always present */}}
+    {{- colorIndicator $type }} {{ colorText $type (printf "[%s] %s" $type .Metadata.code) }}{{ nl -}}
+
+    {{- if $imageRef -}}
+      {{- indentWrap $indent $wrap (printf "ImageRef: %s" $imageRef ) }}{{ nl -}}
+    {{- end -}}
+
+    {{/* For a success the message is generally just "Pass" so don't show it */}}
+    {{- if and (ne $type "Success") .Message -}}
+      {{- indentWrap $indent $wrap (printf "Reason: %s" .Message) }}{{ nl -}}
+    {{- end -}}
+
+    {{- if .Metadata.title }}
+      {{- indentWrap $indent $wrap (printf "Title: %s" .Metadata.title) }}{{ nl -}}
+    {{- end -}}
+
+    {{- if .Metadata.description -}}
+      {{- indentWrap $indent $wrap (printf "Description: %s" .Metadata.description) -}}{{ nl -}}
+    {{- end -}}
+
+    {{/* Don't show the solution text for a success either */}}
+    {{- if and (ne $type "Success") .Metadata.solution -}}
+      {{- indentWrap $indent $wrap (printf "Solution: %s" .Metadata.solution) -}}{{ nl -}}
+    {{- end -}}
+
+    {{- nl -}}
+  {{- end -}}
+{{- end -}}
diff --git a/internal/applicationsnapshot/templates/text_report.tmpl b/internal/applicationsnapshot/templates/text_report.tmpl
@@ -1,17 +1,24 @@
-{{ range .Components -}}
-Name:       {{ .Name }}
-ImageRef:   {{ .ContainerImage }}
-Summary:    Violations: {{ len .Violations }}, Warnings: {{ len .Warnings }}, Successes: {{ .SuccessCount }}
-{{ if gt (len .Violations) 0 -}}
-Violations:
-{{ template "_results.tmpl" (toMap "Results" .Violations "Color" "red") }}
+{{- $t := .TestReport -}}
+{{- $r := .Report -}}
+{{- $c := $r.Components -}}
+
+{{- $showSuccesses := (index $c 0).Successes -}}
+
+Success: {{ $r.Success }}
+Result: {{ $t.Result }}
+Violations: {{ $t.Failures }}, Warnings: {{ $t.Warnings }}, Successes: {{ $t.Successes }}{{ nl -}}
+
+{{- template "_components.tmpl" $c -}}
+
+Results:{{ nl -}}
+{{- if gt $t.Failures 0 -}}
+  {{- template "_results.tmpl" (toMap "Components" $c "Type" "Violation") -}}
 {{- end -}}
-{{ if gt (len .Warnings) 0 -}}
-Warnings:
-{{ template "_results.tmpl" (toMap "Results" .Warnings "Color" "yellow") }}
+
+{{- if gt $t.Warnings 0 -}}
+  {{- template "_results.tmpl" (toMap "Components" $c "Type" "Warning") -}}
 {{- end -}}
-{{ if and (gt .SuccessCount 0) (.Successes) -}}
-Successes:
-{{ template "_results.tmpl" (toMap "Results" .Successes "Color" "green") }}
+
+{{- if and (gt $t.Successes 0) $showSuccesses -}}
+  {{- template "_results.tmpl" (toMap "Components" $c "Type" "Success") -}}
 {{- end -}}
-{{- end }}
diff --git a/internal/utils/templates.go b/internal/utils/templates.go
@@ -147,6 +147,11 @@ func toMap(values ...interface{}) (map[string]interface{}, error) {
 	return m, nil
 }
 
+// Can make it easier to get the right number of line breaks
+func nl() (string, error) {
+	return "\n", nil
+}
+
 // For use in template.Funcs above
 // Todo maybe: Use reflect to find the functions and make this dynamic
 var templateHelpers = template.FuncMap{
@@ -156,4 +161,5 @@ var templateHelpers = template.FuncMap{
 	"wrap":           wrap,
 	"indentWrap":     indentWrap,
 	"toMap":          toMap,
+	"nl":             nl,
 }
