-
Notifications
You must be signed in to change notification settings - Fork 540
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: Remove scanResult from json output [MAGMA-1280] #2502
Conversation
|
f4546fe
to
b38a9db
Compare
b38a9db
to
2e876eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @snaftaly 🚢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Left some comments. Might be worth getting Snyk Open Source and/or Envelope reps to review as mentioned in the Slack thread (if they haven't already).
9d0db8b
to
96e72bb
Compare
The scanResult object was added to the json output where it should only have been added to calculate the the dockerfile warning string. This change removes the scanResult from the json output as it was not supposed to be exposed.
This refactor is done to not have a special case for array vs single result when formatting the results for JSON output. Note: dataToSend will now take the jsonData after formatting rather than the data before formatting. Since the formatting only changes the vulnerbilities and removes the unwanted scanResult, and since the vulnerbilities are removed from dataToSend before it is used, this change will not affect the CLI bahvior except for removing the unwanted scanResult.
96e72bb
to
b4bd660
Compare
Renaming the object as in IaC the logic does more than mapping the results.
Rename test fixtures used in format-test-results to align with the variable name change. This is done in a separate commit so it's clearer in git history that the files were changed and then renamed rather than deleted and created new files.
b4bd660
to
1949818
Compare
What does this PR do?
This PR removes the scanResult object from the json output.
It also refactors the code that creates the json output, therefore it's recommended to review it commit by commit.
How should this be manually tested?
Run:
snyk container test node:apline
Any background context you want to provide?
In a previous PR (#2093) the scanResult object was added in order to calculate the appropriate dockerfile analysis warning message shown in the textual output.
As a result the scanResult was also shown in the json output, although it was not intended to.
What are the relevant tickets?
https://snyksec.atlassian.net/browse/MAGMA-1280