feat: Include location in the result output (#1196)

This will allow outputters to point to the specific location that caused the
result to be created.

Signed-off-by: James Alseth <[email protected]>

Author: jalseth

output/result.go

@@ -1,41 +1,101 @@
 package output
 
 import (
+	"encoding/json"
 	"fmt"
 	"slices"
 )
 
 // Result describes the result of a single rule evaluation.
 type Result struct {
 	Message  string         `json:"msg"`
+	Location *Location      `json:"loc,omitempty"`
 	Metadata map[string]any `json:"metadata,omitempty"`
 	Outputs  []string       `json:"outputs,omitempty"`
 }
 
+// Location describes the origin location in the configuration file that
+// caused the result to be produced.
+type Location struct {
+	File string      `json:"file,omitempty"`
+	Line json.Number `json:"line,omitempty"`
+}
+
+func (l Location) String() string {
+	return fmt.Sprintf("%s L%s", l.File, l.Line.String())
+}
+
+const (
+	msgField = "msg"
+	locField = "_loc"
+)
+
+var reservedFields = []string{
+	msgField,
+	locField,
+}
+
 // NewResult creates a new result. An error is returned if the
 // metadata could not be successfully parsed.
 func NewResult(metadata map[string]any) (Result, error) {
-	if _, ok := metadata["msg"]; !ok {
-		return Result{}, fmt.Errorf("rule missing msg field: %v", metadata)
+	if metadata == nil {
+		return Result{}, fmt.Errorf("metadata must be supplied")
 	}
-	if _, ok := metadata["msg"].(string); !ok {
-		return Result{}, fmt.Errorf("msg field must be string: %v", metadata)
+	msg, ok := lookup[string](metadata, msgField)
+	if !ok {
+		return Result{}, fmt.Errorf("%q field must be present and a string", msgField)
 	}
 
 	result := Result{
-		Message:  metadata["msg"].(string),
+		Message:  msg,
 		Metadata: make(map[string]any),
 	}
 
+	if loc, ok := metadata[locField]; ok {
+		if l := parseLocation(loc); l != nil {
+			result.Location = l
+		}
+	}
+
 	for k, v := range metadata {
-		if k != "msg" {
+		if !slices.Contains(reservedFields, k) {
 			result.Metadata[k] = v
 		}
 	}
 
 	return result, nil
 }
 
+func parseLocation(location any) *Location {
+	loc, ok := location.(map[string]any)
+	if !ok {
+		return nil
+	}
+
+	l := &Location{}
+	if file, ok := lookup[string](loc, "file"); ok {
+		l.File = file
+	}
+	if line, ok := lookup[json.Number](loc, "line"); ok {
+		l.Line = line
+	}
+
+	if l.File == "" && l.Line.String() == "" {
+		return nil
+	}
+
+	return l
+}
+
+func lookup[T any](m map[string]any, k string) (value T, ok bool) {
+	x, ok := m[k]
+	if !ok {
+		return
+	}
+	value, ok = x.(T)
+	return
+}
+
 // Passed returns true if the result did not fail a policy.
 func (r Result) Passed() bool {
 	return r.Message == ""

output/result_test.go

@@ -1,10 +1,86 @@
 package output
 
 import (
+	"encoding/json"
+	"strconv"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
+func TestNewResult(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		desc    string
+		input   map[string]any
+		want    Result
+		wantErr bool
+	}{
+		{
+			desc:    "no metadata is an error",
+			wantErr: true,
+		},
+		{
+			desc:    "missing msg is an error",
+			input:   map[string]any{},
+			wantErr: true,
+		},
+		{
+			desc:    "non-string msg is an error",
+			input:   map[string]any{"msg": 123},
+			wantErr: true,
+		},
+		{
+			desc:  "msg only",
+			input: map[string]any{"msg": "message"},
+			want: Result{
+				Message:  "message",
+				Metadata: make(map[string]any),
+			},
+		},
+		{
+			desc: "msg with location and metadata",
+			input: map[string]any{
+				"msg": "message",
+				"_loc": map[string]any{
+					"file": "some_file.json",
+					"line": json.Number("123"),
+				},
+				"other": "metadata",
+			},
+			want: Result{
+				Message: "message",
+				Location: &Location{
+					File: "some_file.json",
+					Line: json.Number("123"),
+				},
+				Metadata: map[string]any{"other": "metadata"},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
+			got, err := NewResult(tc.input)
+			if gotErr := err != nil; gotErr != tc.wantErr {
+				t.Fatalf("NewResult() error = %v, want %v", err, tc.wantErr)
+			}
+			if tc.wantErr {
+				return
+			}
+			if diff := cmp.Diff(got, tc.want); diff != "" {
+				t.Errorf("NewResult() produced an unexpected diff:\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestCheckResultsHelpers(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		desc             string
 		results          CheckResults
@@ -70,6 +146,8 @@ func TestCheckResultsHelpers(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
 			if gotFailure := tc.results.HasFailure(); gotFailure != tc.wantHasFailure {
 				t.Errorf("HasFailure() = %v, want %v", gotFailure, tc.wantHasFailure)
 			}
@@ -87,6 +165,8 @@ func TestCheckResultsHelpers(t *testing.T) {
 }
 
 func TestExitCode(t *testing.T) {
+	t.Parallel()
+
 	warning := CheckResult{
 		Warnings: []Result{{}},
 	}
@@ -110,15 +190,21 @@ func TestExitCode(t *testing.T) {
 		{results: CheckResults{warning, failure}, expected: 1},
 	}
 
-	for _, testCase := range testCases {
-		actual := testCase.results.ExitCode()
-		if actual != testCase.expected {
-			t.Errorf("Unexpected error code. expected %v, actual %v", testCase.expected, actual)
-		}
+	for i, testCase := range testCases {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			t.Parallel()
+
+			actual := testCase.results.ExitCode()
+			if actual != testCase.expected {
+				t.Errorf("Unexpected error code. expected %v, actual %v", testCase.expected, actual)
+			}
+		})
 	}
 }
 
 func TestExitCodeFailOnWarn(t *testing.T) {
+	t.Parallel()
+
 	warning := CheckResult{
 		Warnings: []Result{{}},
 	}
@@ -137,10 +223,14 @@ func TestExitCodeFailOnWarn(t *testing.T) {
 		{results: CheckResults{warning, failure}, expected: 2},
 	}
 
-	for _, testCase := range testCases {
-		actual := testCase.results.ExitCodeFailOnWarn()
-		if actual != testCase.expected {
-			t.Errorf("Unexpected error code. expected %v, actual %v", testCase.expected, actual)
-		}
+	for i, testCase := range testCases {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			t.Parallel()
+
+			actual := testCase.results.ExitCodeFailOnWarn()
+			if actual != testCase.expected {
+				t.Errorf("Unexpected error code. expected %v, actual %v", testCase.expected, actual)
+			}
+		})
 	}
 }

policy/engine.go

@@ -391,7 +391,6 @@ func (e *Engine) check(ctx context.Context, path string, config any, namespace s
 		checkResult.Failures = append(checkResult.Failures, failures...)
 		checkResult.Warnings = append(checkResult.Warnings, warnings...)
 		checkResult.Exceptions = append(checkResult.Exceptions, exceptions...)
-
 		checkResult.Queries = append(checkResult.Queries, exceptionQueryResult, ruleQueryResult)
 	}
 

tests/source-location/data.yaml

@@ -0,0 +1,3 @@
+loc:
+  file: test.txt
+  line: 123

tests/source-location/policy/main.rego

@@ -0,0 +1,7 @@
+package main
+
+deny contains {
+    "msg": "test",
+    "other": "metadata",
+    "_loc": input.loc
+}

tests/source-location/test.bats

@@ -0,0 +1,9 @@
+#!/usr/bin/env bats
+
+@test "Location is included in results" {
+  run $CONFTEST test -o json data.yaml
+
+  echo $output
+  [[ "$output" =~ "\"file\": \"test.txt\"" ]]
+  [[ "$output" =~ "\"line\": 123" ]]
+}