-
Notifications
You must be signed in to change notification settings - Fork 602
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
Change root of JSON presenter to a mapping (instead of a sequence) #163
Conversation
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
grype/presenter/json/test-fixtures/snapshot/TestEmptyJsonPresenter.golden
Show resolved
Hide resolved
} | ||
} | ||
], | ||
"directory": "/some/path" |
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.
Having the golden files is a good way to visualize how this output is shaping up. I think this is going to be very helpful for consumers. One thing that jumps at me is that a consumer would need to check for the key existence instead of asking for a value that advertises what the source is.
There is at least one potential caveat in the case of a directory being a one-to-one mapping like in here: If we want to add any other key information to the directory scan besides the path, this would need to have a breaking change to include that.
The pattern(s) that the JSON presenter follows in other parts of the object is what I would like to see. For example, in the "artifact"
object, there are a few keys that advertise what their values are:
"name": "package-1",
"type": "deb",
Which is great to consume, as the consumer doesn't need to figure out what it is dealing with. This is an example on how that would look like if it didn't advertise the values as it is today:
"deb": "package-1",
And that is the similarity that I see with the proposed approach in this PR. Can we consider making this new section a generic name that encompasses directory
,image
and any other future source? I don't have any strong opinions on the key name, but following what grype
has internally this could be:
"source": {
"type": "directory",
"target": {
"path": "/some/path"
}
}
For an image it could probably be something like:
"source": {
"type": "directory",
"target": {
"tag": "latest",
"repo": "alpine:3.12",
}
}
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.
Good suggestion, I'll incorporate in syft then a follow up PR in grype.
from an offline chat, changing |
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
99c90d5
to
314e9b5
Compare
Changes the JSON presenter from an enumeration of vulnerability findings...
...to a JSON mapping at the root of the document:
And additionally adds the image or directory source information (just as in syft).