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

feat: support scan remote repository #3131

Merged
merged 7 commits into from
Dec 11, 2022
Merged

feat: support scan remote repository #3131

merged 7 commits into from
Dec 11, 2022

Conversation

penndev6
Copy link
Contributor

@penndev6 penndev6 commented Nov 5, 2022

Description

support scan remote repository through the deployed vulnerability database server. The detail see in #3121 .

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@penndev6 penndev6 requested a review from knqyf263 as a code owner November 5, 2022 12:43
@penndev6
Copy link
Contributor Author

penndev6 commented Nov 5, 2022

Before:

image

After:

image

@penndev6
Copy link
Contributor Author

penndev6 commented Nov 5, 2022

@knqyf263 Hi, please take a look this pr. This feature is very useful for me. Thank you! And I think this is both a bug or a new feature, and I'm not sure if this feature is something you forgot to implement or is on your future features.

@penndev6
Copy link
Contributor Author

penndev6 commented Nov 8, 2022

@afdesk hi, Can you take a look at the CI error report?

@afdesk
Copy link
Contributor

afdesk commented Nov 9, 2022

hi @pikaqiu-go yes, sure. i'll take a look at this CI error

@penndev6
Copy link
Contributor Author

penndev6 commented Nov 9, 2022

@afdesk thank you!

@knqyf263 knqyf263 requested a review from afdesk November 13, 2022 17:42
@afdesk
Copy link
Contributor

afdesk commented Nov 13, 2022

@pikaqiu-go thanks for your contribution! it's really useful feature.

I've tested and it works fine.

there are two additional moments:

  1. we have to update the documentation: here and here
  2. I'd add an integration test.

@pikaqiu-go do you have a little more energy and time for it? or I can try to do it

@penndev6
Copy link
Contributor Author

Hi, @afdesk Thank you for reviewing the code for me. If it's not urgent, I can try to finish them.

@penndev6
Copy link
Contributor Author

Should I finsh them in this pr, maybe I can finish them in the new pr. Do I need to create issue and associate pr?

@afdesk
Copy link
Contributor

afdesk commented Nov 16, 2022

Hi, @afdesk Thank you for reviewing the code for me. If it's not urgent, I can try to finish them.

it'll be really cool! thanks a lot!

Should I finsh them in this pr, maybe I can finish them in the new pr. Do I need to create issue and associate pr?

Could you finish them in this PR?

I'm ready to help you in any time!

@penndev6
Copy link
Contributor Author

Hi, @afdesk Thank you for reviewing the code for me. If it's not urgent, I can try to finish them.

it'll be really cool! thanks a lot!

Should I finsh them in this pr, maybe I can finish them in the new pr. Do I need to create issue and associate pr?

Could you finish them in this PR?

I'm ready to help you in any time!

OK~

@penndev6 penndev6 requested a review from afdesk November 19, 2022 11:26
@penndev6
Copy link
Contributor Author

hi, @afdesk,please take a look~

@penndev6
Copy link
Contributor Author

I'm not sure if integration test is right.

@afdesk
Copy link
Contributor

afdesk commented Nov 21, 2022

hi, @afdesk,please take a look~

@pikaqiu-go thanks. I'm taking a look right now

Copy link
Contributor

@afdesk afdesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@afdesk
Copy link
Contributor

afdesk commented Nov 21, 2022

@pikaqiu-go I've updated the test data. I hope it's OK.
thanks for your efforts! it's really nice and useful

@afdesk
Copy link
Contributor

afdesk commented Nov 21, 2022

@knqyf263 I was a bit confused that Trivy doesn't have an integration test for repo scan. is it right? or i miss something

@knqyf263
Copy link
Collaborator

Oh, yes, we don't have integration tests for repository scanning since it used to be almost the same as fs scanning.

@penndev6
Copy link
Contributor Author

penndev6 commented Dec 8, 2022

@knqyf263 Hi, this pr has been stagnant for a long time. I wonder if there are other problems?

@knqyf263 knqyf263 merged commit 8744534 into aquasecurity:main Dec 11, 2022
@knqyf263
Copy link
Collaborator

@pikaqiu-go Thanks for your contribution🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trivy can’t scan remote repository through the deployed vulnerability database server
3 participants