Skip to content
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

scanner - Do not add unsuported artifacts to scan queue #18931

Closed
wants to merge 1 commit into from

Conversation

dioguerra
Copy link

@dioguerra dioguerra commented Jul 13, 2023

Thank you for contributing to Harbor!

Comprehensive Summary of your change

When collectScanningArtifacts is run, the Scannel will return supported bool if the current artifact is scanneable. If it is scaneable this artifact should be added to the artifacts to be scanned and skipped otherwise.

It looks like the artifact might be added to the artifacts to be scanned list even if it is not supported by the remote Scanner.

Issue being fixed

Fixes #(issue)
Not exactly sure it fixes my issue. I dont know how to reproduce, but here is a reference on why I was investigating this:
#18455

/release-note/enhancement

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/update"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed. (still understanding how to do this, tried with a couple of images and signatures and looks ok)
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@dioguerra dioguerra requested a review from a team as a code owner July 13, 2023 11:42
Signed-off-by: Diogo Guerra <diogo.filipe.tomas.guerra@cern.ch>
@dioguerra
Copy link
Author

dioguerra commented Jul 13, 2023

For reference, the original changes are here: https://github.com/goharbor/harbor/blame/f21b1481bb5ba3efb9e3c1dd8c4e704d9dcc44a1/src/controller/scan/base_controller.go#L181

Maybe i'm missing something @heww ?

@Vad1mo Vad1mo added release-note/update Update or Fix release-note/enhancement Label to mark PR to be added under release notes as enhancement and removed release-note/enhancement Label to mark PR to be added under release notes as enhancement labels Jul 13, 2023
@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Merging #18931 (8eca1d2) into main (df4dc3c) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #18931      +/-   ##
==========================================
+ Coverage   67.33%   67.42%   +0.08%     
==========================================
  Files         981      986       +5     
  Lines      107342   107976     +634     
  Branches     2698     2698              
==========================================
+ Hits        72282    72803     +521     
- Misses      31176    31273      +97     
- Partials     3884     3900      +16     
Flag Coverage Δ
unittests 67.42% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/controller/scan/base_controller.go 60.36% <100.00%> (+0.14%) ⬆️

... and 22 files with indirect coverage changes

@github-actions
Copy link

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions bot added the Stale label Sep 16, 2023
@github-actions
Copy link

This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main.

@github-actions github-actions bot closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants