Merge pull request #1744 from simonbaird/multliple-terms-in-exclude-note

Fix description text for multi-term exclude advice

acceptance/examples/reject.rego

@@ -47,7 +47,7 @@ deny contains result if {
 		},
 		{
 			"code": "main.reject_with_term",
-			"term": "term2",
+			"term": ["term2", "term3"],
 			"collections": ["A"],
 			"effective_on": "2022-01-01T00:00:00Z",
 			"msg": "Fails always (term2)",

features/__snapshots__/validate_image.snap

@@ -1622,9 +1622,12 @@ Error: success criteria not met
             "collections": [
               "A"
             ],
-            "description": "This rule will always fail. To exclude this rule add \"main.reject_with_term:term2\" to the `exclude` section of the policy configuration.",
+            "description": "This rule will always fail. To exclude this rule add one or more of \"main.reject_with_term:term2\", \"main.reject_with_term:term3\" to the `exclude` section of the policy configuration.",
             "solution": "None",
-            "term": "term2",
+            "term": [
+              "term2",
+              "term3"
+            ],
             "title": "Reject with term rule"
           }
         },
@@ -1741,8 +1744,8 @@ Results:
   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.
+  Description: This rule will always fail. To exclude this rule add one or more of "main.reject_with_term:term2",
+  "main.reject_with_term:term3" to the `exclude` section of the policy configuration.
   Solution: None
 
 ✕ [Violation] main.rejector

internal/evaluator/conftest_evaluator.go

@@ -111,10 +111,7 @@ func trim(results *[]Outcome) {
 				continue
 			}
 
-			if term, ok := results[i].Metadata[metadataTerm]; ok {
-				code = fmt.Sprintf("%s:%s", code, term)
-			}
-			results[i].Metadata[metadataDescription] = fmt.Sprintf("%s. To exclude this rule add %q to the `exclude` section of the policy configuration.", strings.TrimSuffix(description, "."), code)
+			results[i].Metadata[metadataDescription] = fmt.Sprintf("%s. To exclude this rule add %s to the `exclude` section of the policy configuration.", strings.TrimSuffix(description, "."), excludeDirectives(code, results[i].Metadata[metadataTerm]))
 		}
 
 		return results
@@ -128,6 +125,41 @@ func trim(results *[]Outcome) {
 	}
 }
 
+// Used above to suggest what to exclude to skip a certain violation.
+// Use the term if one is provided so it's as specific as possible.
+func excludeDirectives(code string, rawTerm any) string {
+	output := []string{}
+
+	if term, ok := rawTerm.(string); ok && term != "" {
+		// A single term was provided
+		output = append(output, fmt.Sprintf(`"%s:%s"`, code, term))
+	}
+
+	if rawTerms, ok := rawTerm.([]any); ok {
+		// Multiple terms were provided
+		for _, t := range rawTerms {
+			if term, ok := t.(string); ok && term != "" {
+				output = append(output, fmt.Sprintf(`"%s:%s"`, code, term))
+			}
+		}
+	}
+
+	if len(output) == 0 {
+		// No terms were provided (or some unexpected edge case)
+		output = append(output, fmt.Sprintf(`"%s"`, code))
+	}
+
+	prefix := ""
+	if len(output) > 1 {
+		// For required tasks I think just the first one would be sufficient, but I'm
+		// not sure if that's always true, so let's give some slightly vague advice
+		prefix = "one or more of "
+	}
+
+	// Put it all together and return a string
+	return fmt.Sprintf("%s%s", prefix, strings.Join(output, ", "))
+}
+
 type testRunner interface {
 	Run(context.Context, []string) ([]Outcome, Data, error)
 }

internal/evaluator/conftest_evaluator_test.go

@@ -23,6 +23,7 @@ import (
 	"context"
 	"embed"
 	"encoding/json"
+	"fmt"
 	"io"
 	"io/fs"
 	"os"
@@ -1983,6 +1984,58 @@ func TestNewConftestEvaluatorComputeIncludeExclude(t *testing.T) {
 	}
 }
 
+// This test is not high value but it should make Codecov happier
+func TestExcludeDirectives(t *testing.T) {
+	cases := []struct {
+		code     string
+		term     any
+		expected string
+	}{
+		// Normal behavior
+		{
+			code:     "foo",
+			term:     nil,
+			expected: `"foo"`,
+		},
+		{
+			code:     "foo",
+			term:     "bar",
+			expected: `"foo:bar"`,
+		},
+		{
+			code:     "foo",
+			term:     []any{"bar", "baz"},
+			expected: `one or more of "foo:bar", "foo:baz"`,
+		},
+		// Unlikely edge cases
+		{
+			code:     "foo",
+			term:     "",
+			expected: `"foo"`,
+		},
+		{
+			code:     "foo",
+			term:     []any{nil},
+			expected: `"foo"`,
+		},
+		{
+			code:     "foo",
+			term:     []any{nil, ""},
+			expected: `"foo"`,
+		},
+		{
+			code:     "foo",
+			term:     []any{nil, "bar", 42},
+			expected: `"foo:bar"`,
+		},
+	}
+	for i, tt := range cases {
+		t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) {
+			assert.Equal(t, excludeDirectives(tt.code, tt.term), tt.expected)
+		})
+	}
+}
+
 var testCapabilities string
 
 func init() {