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

fix: violation not visible in json output #314

Merged
merged 1 commit into from
Feb 23, 2024
Merged

Conversation

vishal-chdhry
Copy link
Member

Explanation

Violations were not visible in json output because errors are parsed as {}. This PR converts violation from array of errors to array of strings, Violations are now visible.

Related issue

Milestone of this PR

What type of PR is this

Proposed Changes

Proof Manifests

Current behaviour:

"Rules": [
                  {
                    "Rule": {
                      "name": "detect-untrusted-flag",
                      "match": {
                        "any": [
                          {
                            "(Stages[].Commands[?Name=='RUN'].CmdLine[][] | length(@) \u003e `0`)": true
                          }
                        ]
                      },
                      "assert": {
                        "any": [
                          {
                            "message": "Dockerfile contains the '--allow-untrusted' which is not preferred",
                            "check": {
                              "~.(Stages[].Commands[?Name=='RUN'].CmdLine[][])": {
                                "(contains(@, '--allow-untrusted') \u0026\u0026 (contains(@, 'apk')))": false
                              }
                            }
                          }
                        ]
                      }
                    },
                    "Identifier": "",
                    "Error": null,
                    "Violations": [
                      {}
                    ]
                  }
                ]
              }
            ]
          }

Updated behaviour:

❯ ./../../../../kyverno-json scan --policy ./policy.yaml --payload ./payload.json --output json
....
        "Rules": [
          {
            "Rule": {
              "name": "deny-external-calls",
              "assert": {
                "all": [
                  {
                    "message": "HTTP calls are not allowed",
                    "check": {
                      "~.(Stages[].Commands[].Args[].Value)": {
                        "(contains(@, 'https://') || contains(@, 'http://'))": false
                      }
                    }
                  },
                  {
                    "message": "HTTP calls are not allowed",
                    "check": {
                      "~.(Stages[].Commands[].CmdLine[])": {
                        "(contains(@, 'https://') || contains(@, 'http://'))": false
                      }
                    }
                  },
                  {
                    "message": "curl is not allowed",
                    "check": {
                      "~.(Stages[].Commands[].CmdLine[])": {
                        "(contains(@, 'curl'))": false
                      }
                    }
                  },
                  {
                    "message": "wget is not allowed",
                    "check": {
                      "~.(Stages[].Commands[].CmdLine[])": {
                        "(contains(@, 'wget'))": false
                      }
                    }
                  }
                ]
              }
            },
            "Identifier": "",
            "Error": null,
            "Violations": [
              "HTTP calls are not allowed: all[0].check.~.(Stages[].Commands[].Args[].Value)[0].(contains(@, 'https://') || contains(@, 'http://')): Invalid value: true: Expected value: false",
              "wget is not allowed: all[3].check.~.(Stages[].Commands[].CmdLine[])[0].(contains(@, 'wget')): Invalid value: true: Expected value: false"
            ]
          }
        ]
      }
    ]
  }
]

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .

Further Comments

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Copy link

codecov bot commented Feb 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 20.48%. Comparing base (acadce6) to head (6d6a9d0).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #314   +/-   ##
=======================================
  Coverage   20.48%   20.48%           
=======================================
  Files          33       33           
  Lines        2143     2143           
=======================================
  Hits          439      439           
  Misses       1678     1678           
  Partials       26       26           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@eddycharly
Copy link
Member

Not sure we want to change that at the engine level... anyway, i wanted to make changes here so i'll probably revisit later.

@eddycharly eddycharly merged commit 7a0399b into main Feb 23, 2024
34 checks passed
@eddycharly eddycharly deleted the violation-as-string branch February 23, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants