-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
refactor(deps): Merge trivy-iac into Trivy #6005
Conversation
Note: I've left out |
@knqyf263 and @nikpivkin I understand that this PR is quite big so please take your time to review. Besides the new files being added, most of the changes are actually using the new files (rather than the trivy-iac package). Please let me know if I can do anything to help you review this PR. I've also broken the merge of misconfig scanning into this PR and the defsec changes, in this PR. |
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.
Can we make a new directory iac
under pkg
and put detection
, scanners
and adapters
there? We should be going to reorganize the structure in the future, but a dedicated directory helps us set the owners for now.
extrafs
can be located under pkg
directly as it is like a common utility.
Thanks that's a great point about code owners. I've updated it. |
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.
There are many tests about checks, like test/testdata/dockerfile/DS001
. Since logic and checks are separated, I feel like Trivy should not have the tests for checks. What do you think?
Yeah that's a valid point. They should be moved. I've moved them into the checks repo: aquasecurity/trivy-checks#70 |
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.
There are several files under test/
. Are they integration tests? It looks unit tests to me.
test/attribute_test.go
Outdated
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.
Is it a unit test? If so, the file should be next to the relevant Go file.
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.
Yeah they really are just terraform functional tests. I've relocated them under the terraform directory. Also deleted another test which was not relevant rules_test.go
.
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.
Thanks! I guess those tests were added for tfsec. When creating defsec, they were just copied from tfsec.
Yes because last time the merge was too big to review so we split it up. The second PR is here that merges remaining pieces from defsec into Trivy. It was stacked on top of this PR but since this merged first, I will recreate it and ask for a review. |
Description
Merges current state of trivy-iac into Trivy.
Related issues
trivy-iac
into Trivy #5626Required PRs to be reviewed before
Checklist