-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: add version range to mariner provider #585
Conversation
Previously, this provider could only include upper bounds on ranges. Signed-off-by: Will Murphy <[email protected]>
) | ||
criterion: Optional[Criterion] = field( | ||
default=None, | ||
criterion: List[Criterion] = field( |
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.
This is the important change to the generated model. It might be worth backing out all. the required: True
, since it's noisy and might lead to a failure to parse the whole OVAL file if there's one malformed record. (The intended behavior of Vunnel is to skip malformed records but carry on and still generate some data.)
My understanding is that right now there will always be exactly 2 items on the list, at least right now.
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.
This Vunnel parser is written to ignore the failure to parse individual definitions anyway: https://github.com/anchore/vunnel/blob/main/src/vunnel/providers/mariner/parser.py#L45
Based on some experimentation, the new required: True
annotations are showing up because of bumping the xsdata
package to version 23.5.
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.
Based on offline discussion, we will pin xsdata
package back to the version that generated these models to avoid introducing unnecessary drift in the models. (Putting required: True
everywhere has no benefit on a field of type Optional
, since given a populated model we have to check for None
, and failing validation for this reason would be unfortunate, since the code defends against these None
s anyway.)
Signed-off-by: Will Murphy <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
Otherwise, grype-db has to parse and re-normalize these strings. Signed-off-by: Will Murphy <[email protected]>
This keeps the mariner models being generated by the same code that generated them initially, which reduces the risk of an incompatible generated change. Signed-off-by: Will Murphy <[email protected]>
@@ -54,7 +54,7 @@ orjson = "^3.8.6" | |||
SQLAlchemy = ">= 1.4.46, < 2.0" # note: 1.4.x currently required for enterprise | |||
mergedeep = "^1.3.4" | |||
importlib-metadata = "^7.0.1" | |||
xsdata = {extras = ["cli", "lxml", "soap"], version = ">=22.12,<25.0"} | |||
xsdata = {extras = ["cli", "lxml", "soap"], version = "=22.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.
Dev note: we might have to unpin this eventually to generate the models at a later date, but this pin keeps the diff less noisy until we want to tackle this issue
Previously, this provider could only include upper bounds on ranges.
Note that this will also require a grype-db PR to start putting the new field in the database.