secsan: Add tags field to event data on new vulnerability notifications (PROJQUAY-5681) #1997
Conversation
Marcusk19
force-pushed
the
vulnerability_slack_notifications
branch
from
47297d0
to
fd2d203
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1997 +/- ##
=======================================
Coverage 70.80% 70.80%
=======================================
Files 436 436
Lines 40532 40536 +4
Branches 5283 5284 +1
=======================================
+ Hits 28697 28700 +3
- Misses 10158 10161 +3
+ Partials 1677 1675 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| tag_names = list( | ||
| registry_model.tag_names_for_manifest(manifest, TAG_LIMIT) | ||
| ) | ||
| print("TAG_NAMES: ", tag_names) |
There was a problem hiding this comment.
Forgot to remove this print statement
| @@ -56,6 +57,7 @@ | |||
|
|
|||
|
|
|||
| DEFAULT_SECURITY_SCANNER_V4_REINDEX_THRESHOLD = 86400 # 1 day | |||
| TAG_LIMIT = 100 | |||
There was a problem hiding this comment.
Why the limit? What subset is chosen when it's exceeded?
There was a problem hiding this comment.
From what I understand the limit is for retrieving the tag names that point to that manifest up to that limit. So at max there will only be 100 tags attached to the event data for the notification.
There was a problem hiding this comment.
Right, so how is that 100 chosen?
The limit will have to be documented somewhere, also.
Alternatively, is it possible to page through all the tags and emit multiple events if needed?
There was a problem hiding this comment.
The security notification worker already sets it as 100 which is why I chose that number. It's also possible to set it as None for no limit.
Is it better to just go through all tag names and not limit it to 100?
There was a problem hiding this comment.
Maybe? It could make the event object very large, which is why I suggested paging.
when building message notification, use get() to avoid exception when looking up event_data["tags"] use get() to avoid throwing error if key is not found
Marcusk19
force-pushed
the
vulnerability_slack_notifications
branch
from
710976c
to
600b605
Compare
Add tag field to event data for vulnerability notifications on new image pushes.
Missing tag field caused an exception for slack notifications, should allow slack notifications to fire for new images.