Improve opa logging (#1011)

* Improve opa logging

* Fix examples rendering in test

* Remove residual comment from previous implementation

* Make rerun the default

docs/_data/examples.yml

@@ -113,9 +113,9 @@
   name: OPA Terraform Example
   image: /assets/img/logos/opa-logo.png
   files:
-    - url: /examples/terraform-opa-example/pass/main.tf
+    - url: /examples/terraform-opa-example/pass/main_pass.tf
       id: pass_terraform_main_code
-    - url: /examples/terraform-opa-example/fail/main.tf
+    - url: /examples/terraform-opa-example/fail/main_fail.tf
       id: fail_terraform_main_code
     - url: /examples/terraform-opa-example/policy/enforce_source.rego
       id: policy_main_code

modules/opa/eval.go

@@ -1,6 +1,8 @@
 package opa
 
 import (
+	"path/filepath"
+	"strings"
 	"sync"
 
 	"github.com/gruntwork-io/terratest/modules/logger"
@@ -26,6 +28,11 @@ type EvalOptions struct {
 
 	// When true, keep any temp files and folders that are created for the purpose of running opa eval.
 	DebugKeepTempFiles bool
+
+	// When true, disable the functionality where terratest reruns the opa check on the same file and query all elements
+	// on error. By default, terratest will rerun the opa eval call with `data` query so you can see all the contents
+	// evaluated.
+	DebugDisableQueryDataOnError bool
 }
 
 // FailMode signals whether `opa eval` should fail when the query returns an undefined value (FailUndefined), a
@@ -82,13 +89,23 @@ func asyncEval(
 	cmd := shell.Command{
 		Command: "opa",
 		Args:    formatOPAEvalArgs(options, jsonFilePath, resultQuery),
-		Logger:  options.Logger,
+
+		// Do not log output from shell package so we can log the full json without breaking it up. This is ok, because
+		// opa eval is typically very quick.
+		Logger: logger.Discard,
 	}
-	err := shell.RunCommandE(t, cmd)
+	err := runCommandWithFullLoggingE(t, options.Logger, cmd)
+	ruleBasePath := filepath.Base(options.RulePath)
 	if err == nil {
-		options.Logger.Logf(t, "opa eval passed on file %s", jsonFilePath)
+		options.Logger.Logf(t, "opa eval passed on file %s (policy %s; query %s)", jsonFilePath, ruleBasePath, resultQuery)
 	} else {
-		options.Logger.Logf(t, "Failed opa eval on file %s", jsonFilePath)
+		options.Logger.Logf(t, "Failed opa eval on file %s (policy %s; query %s)", jsonFilePath, ruleBasePath, resultQuery)
+		if options.DebugDisableQueryDataOnError == false {
+			options.Logger.Logf(t, "DEBUG: rerunning opa eval to query for full data.")
+			cmd.Args = formatOPAEvalArgs(options, jsonFilePath, "data")
+			// We deliberately ignore the error here as we want to only return the original error.
+			runCommandWithFullLoggingE(t, options.Logger, cmd)
+		}
 	}
 	errChan <- err
 }
@@ -114,3 +131,12 @@ func formatOPAEvalArgs(options *EvalOptions, jsonFilePath string, resultQuery st
 	)
 	return args
 }
+
+// runCommandWithFullLogging will log the command output in its entirety with buffering. This avoids breaking up the
+// logs when commands are run concurrently. This is a private function used in the context of opa only because opa runs
+// very quickly, and the output of opa is hard to parse if it is broken up by interleaved logs.
+func runCommandWithFullLoggingE(t testing.TestingT, logger *logger.Logger, cmd shell.Command) error {
+	output, err := shell.RunCommandAndGetOutputE(t, cmd)
+	logger.Logf(t, "Output of command `%s %s`:\n%s", cmd.Command, strings.Join(cmd.Args, " "), output)
+	return err
+}

test/terraform_opa_example_test.go

@@ -44,6 +44,6 @@ func TestOPAEvalTerraformModuleFailsCheck(t *testing.T) {
 		RulePath: policyPath,
 	}
 
-	// website::tag::6:: Here we expect the checks to fail, so we use `OPAEvalE` to check the error.
+	// website::tag::6:: Here we expect the checks to fail, so we use `OPAEvalE` to check the error. Note that on the files that failed, this function will rerun `opa eval` with the query set to `data`, so you can see the values of all the variables in the policy. This is useful for debugging failures.
 	require.Error(t, terraform.OPAEvalE(t, tfOpts, opaOpts, "data.enforce_source.allow"))
 }