Azure / ShieldGuard

Enables best security practices for your project from day zero.
MIT License
8 stars 6 forks source link

Implement text presenter for better readability #36

Closed everjing closed 1 year ago

everjing commented 1 year ago

This PR enables presenting report in text form with better readability over the default form JSON. Below are TEXT vs JSON:

FAIL - tempDataAndPolicy/foo.yaml - main - foo is not allowed
FAIL - tempDataAndPolicy/foo.yaml - main - foo is not allowed
FAIL - tempDataAndPolicy/foo.yaml - main - baz is not allowed
WARN - tempDataAndPolicy/foo.yaml - main - foo is not allowed
15 tests, 11 passed, 3 failures 1 warnings, 0 exceptions
Error: test failed: found 3 failure(s), 1 warning(s)
[
  {
    "filename": "tempDataAndPolicy/foo.yaml",
    "namespace": "main",
    "success": 11,
    "failures": [
      {
        "query": "data.main.deny_name",
        "rule": {
          "name": "name"
        },
        "message": "foo is not allowed"
      },
      {
        "query": "data.main.deny_other",
        "rule": {
          "name": "other"
        },
        "message": "foo is not allowed"
      },
      {
        "query": "data.main.deny_name",
        "rule": {
          "name": "name"
        },
        "message": "baz is not allowed"
      }
    ],
    "warnings": [
      {
        "query": "data.main.warn_name",
        "rule": {
          "name": "name"
        },
        "message": "foo is not allowed"
      }
    ],
    "exceptions": []
  }
]

Error: test failed: found 3 failure(s), 1 warning(s)

bcho commented 1 year ago

Thanks for your change! We can start with making the code to be compilable first. After that, we can define a very basic text output and get it merge.

The source information can be found from the query result struct: https://github.com/Azure/ShieldGuard/blob/493fd8d6860cbdac16f1a3a0588e357e4d75f628/sg/internal/result/types.go#L25 . We can omit then namespace information at this moment since we always use main namespace at this moment.

everjing commented 1 year ago

Thanks for your change! We can start with making the code to be compilable first. After that, we can define a very basic text output and get it merge.

The source information can be found from the query result struct:

https://github.com/Azure/ShieldGuard/blob/493fd8d6860cbdac16f1a3a0588e357e4d75f628/sg/internal/result/types.go#L25

. We can omit then namespace information at this moment since we always use main namespace at this moment.

From iterating list of QueryResults, I aggregated into lists of Result by kind (failures, warnings, etc) and when printing out, it will iterate list of Result and can only extract information from Result type. But Result does not have a field to get the source, any suggestions?

bcho commented 1 year ago

Thanks for your change! We can start with making the code to be compilable first. After that, we can define a very basic text output and get it merge. The source information can be found from the query result struct: https://github.com/Azure/ShieldGuard/blob/493fd8d6860cbdac16f1a3a0588e357e4d75f628/sg/internal/result/types.go#L25

. We can omit then namespace information at this moment since we always use main namespace at this moment.

From iterating list of QueryResults, I aggregated into lists of Result by kind (failures, warnings, etc) and when printing out, it will iterate list of Result and can only extract information from Result type. But Result does not have a field to get the source, any suggestions?

A simple way is to carry the source location with the result when aggregate them. Please check the code: https://github.com/Azure/ShieldGuard/blob/493fd8d6860cbdac16f1a3a0588e357e4d75f628/sg/internal/result/presenter/query_result.go#L38-L45 And please read carefully on the JSON presenter implementation.

everjing commented 1 year ago

The text is presented with all contents in one line as shown in image below image

Please advise how to print results in multiple lines shown as below. Thanks!

FAIL - tempDataAndPolicy/foo.yaml - main - foo is not allowed.
FAIL - tempDataAndPolicy/foo.yaml - main - foo is not allowed.
FAIL - tempDataAndPolicy/foo.yaml - main - baz is not allowed.
WARN - tempDataAndPolicy/foo.yaml - main - foo is not allowed.
15 tests, 11 passed, 3 failures 1 warnings, 0 exceptions.
Error: test failed: found 3 failure(s), 1 warning(s)
everjing commented 1 year ago

Thanks for your change! We can start with making the code to be compilable first. After that, we can define a very basic text output and get it merge. The source information can be found from the query result struct: https://github.com/Azure/ShieldGuard/blob/493fd8d6860cbdac16f1a3a0588e357e4d75f628/sg/internal/result/types.go#L25

. We can omit then namespace information at this moment since we always use main namespace at this moment.

From iterating list of QueryResults, I aggregated into lists of Result by kind (failures, warnings, etc) and when printing out, it will iterate list of Result and can only extract information from Result type. But Result does not have a field to get the source, any suggestions?

A simple way is to carry the source location with the result when aggregate them. Please check the code:

https://github.com/Azure/ShieldGuard/blob/493fd8d6860cbdac16f1a3a0588e357e4d75f628/sg/internal/result/presenter/query_result.go#L38-L45

And please read carefully on the JSON presenter implementation.

Thanks, very helpful suggestions! I figured it out and implemented text presenter in commit a0dbaa0(https://github.com/Azure/ShieldGuard/pull/36/commits/a0dbaa0c4fe7cc6aa1cc815dc6b0ede046567013)`

everjing commented 1 year ago

Commit [4b73153](https://github.com/Azure/ShieldGuard/pull/36/commits/4b73153a7998cb53c684ec8f1006aae8e22a8895)enabled starting a new line for each result, which is what we want. image Please check this out and let me know your thoughts. Thanks!