-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Modified to check the expiration time of the allowlist when determining the vulnerability of an artifact #18106
Conversation
a9f4b9b
to
e69adea
Compare
Codecov Report
@@ Coverage Diff @@
## main #18106 +/- ##
===========================================
- Coverage 67.49% 44.26% -23.24%
===========================================
Files 970 232 -738
Lines 106079 12808 -93271
Branches 2649 2582 -67
===========================================
- Hits 71602 5669 -65933
+ Misses 30645 6858 -23787
+ Partials 3832 281 -3551
Flags with carried forward coverage won't be shown. Click here to find out more.
|
82b0052
to
85077a7
Compare
85077a7
to
5b2bef9
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.
lgtm
} | ||
} else { | ||
for _, v := range vuls { | ||
if allowlist.Contains(v.ID) { |
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.
If change this line to if !allowlistIsExpired && allowlist.Contains(v.ID) {}
, then line 818-824 can be removed as logic duplicated with line 835-837?
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.
@hyeyoung-leee could you please update the code to resolve this comment? thanks
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.
71307e8
to
41f0a51
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.
lgtm
…ng the vulnerability of an artifact Signed-off-by: hyeyoung-lee <[email protected]>
@hyeyoung-leee Congratulations on your first merged PR on Harbor :) Welcome to the Club :) |
The allowlist is applied even after the expiration time has passed.
Signed-off-by: hyeyoung-leee [email protected]
Thank you for contributing to Harbor!
Comprehensive Summary of your change
Issue being fixed
Fixes #18078
Please indicate you've done the following: