Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve opa logging #1011

Merged
merged 4 commits into from
Oct 11, 2021
Merged

Improve opa logging #1011

merged 4 commits into from
Oct 11, 2021

Conversation

yorinasub17
Copy link
Contributor

@yorinasub17 yorinasub17 commented Oct 7, 2021

Problem

Currently, the opa eval calls for multiple files happen concurrently and this results in the output logs interleaving. This makes it hard to read the command output because it outputs JSON. See the following example from a test run:

TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66: {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:   "result": [
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:     {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:       "expressions": [
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66: {}
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66: {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:         {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:   "result": [
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           "value": true,
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           "text": "data.enforce_source.allow",
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:     {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           "location": {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:             "row": 1,
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:             "col": 1
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           }
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:         }
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:       ]
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:     }
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:       "expressions": [
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:         {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           "value": true,
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           "text": "data.enforce_source.allow",
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           "location": {
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:   ]
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:             "row": 1,
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66: }
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:             "col": 1
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:           }
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:         }
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:       ]
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:     }
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66:   ]
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T12:01:06-05:00 logger.go:66: }

The other issue is that the output when opa checks fail with FailUndefined is {}, which makes it hard to debug failures.

Solution

This PR implements the following:

  • Update the logging so that it buffers the output and logs the entire output at once. This should be ok for opa eval calls because they happen relatively quickly compared to something like Docker or Terraform.
  • Add a new flag DebugQueryDataOnError which will rerun opa eval with a larger scoped result query so that you can see all the contents of all the defined expressions, making it easier to understand what went wrong in a failure.

With these changes, the above output improves to be:

TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: Output of command `opa eval --fail -i /var/folders/36/ljcx66pj73zd3sq1r1tsr_300000gn/T/terratest-opa-hcl2json-423489062/variables.json -d ../examples/terraform-opa-example/policy/enforce_source.rego data.enforce_source.allow`:
{
  "result": [
    {
      "expressions": [
        {
          "value": true,
          "text": "data.enforce_source.allow",
          "location": {
            "row": 1,
            "col": 1
          }
        }
      ]
    }
  ]
}
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: Output of command `opa eval --fail -i /var/folders/36/ljcx66pj73zd3sq1r1tsr_300000gn/T/terratest-opa-hcl2json-423489062/output.json -d ../examples/terraform-opa-example/policy/enforce_source.rego data.enforce_source.allow`:
{
  "result": [
    {
      "expressions": [
        {
          "value": true,
          "text": "data.enforce_source.allow",
          "location": {
            "row": 1,
            "col": 1
          }
        }
      ]
    }
  ]
}
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: opa eval passed on file /var/folders/36/ljcx66pj73zd3sq1r1tsr_300000gn/T/terratest-opa-hcl2json-423489062/variables.json (policy enforce_source.rego; query data.enforce_source.allow)
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: opa eval passed on file /var/folders/36/ljcx66pj73zd3sq1r1tsr_300000gn/T/terratest-opa-hcl2json-423489062/output.json (policy enforce_source.rego; query data.enforce_source.allow)
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: Output of command `opa eval --fail -i /var/folders/36/ljcx66pj73zd3sq1r1tsr_300000gn/T/terratest-opa-hcl2json-423489062/main.json -d ../examples/terraform-opa-example/policy/enforce_source.rego data.enforce_source.allow`:
{}
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: Failed opa eval on file /var/folders/36/ljcx66pj73zd3sq1r1tsr_300000gn/T/terratest-opa-hcl2json-423489062/main.json (policy enforce_source.rego; query data.enforce_source.allow)
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: DEBUG: rerunning opa eval to query for full data.
TestOPAEvalTerraformModuleFailsCheck 2021-10-07T14:41:52-05:00 logger.go:66: Output of command `opa eval --fail -i /var/folders/36/ljcx66pj73zd3sq1r1tsr_300000gn/T/terratest-opa-hcl2json-423489062/main.json -d ../examples/terraform-opa-example/policy/enforce_source.rego data`:
{
  "result": [
    {
      "expressions": [
        {
          "value": {
            "enforce_source": {
              "violation": [
                "instance_types"
              ]
            }
          },
          "text": "data",
          "location": {
            "row": 1,
            "col": 1
          }
        }
      ]
    }
  ]
}

Bonus fixes

  • Fix docs bug on website where it doesn't render the fail module.

@yorinasub17 yorinasub17 requested a review from brikis98 as a code owner October 7, 2021 19:47
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement 👍

modules/opa/eval.go Outdated Show resolved Hide resolved
test/terraform_opa_example_test.go Outdated Show resolved Hide resolved
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yorinasub17
Copy link
Contributor Author

Thanks for review! Going to merge this in now.

@yorinasub17 yorinasub17 merged commit c583606 into master Oct 11, 2021
@yorinasub17 yorinasub17 deleted the yori-improve-debuggability branch October 11, 2021 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants