-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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: introduce --exclude-owned
flag to exclude K8S Resources with Owner References
#5059
Conversation
|
4886b23
to
3712866
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.
@thapabishwa thank you for the contribution, maybe its best to add the exclude logic in : trivy-kubernetes
project here
Hi @chen-keinan. Thanks for your swift response. I have concerns about adding these changes to the For instance, if these changes are integrated into
However, by choosing to retain the new flag, complete control is handed over to the end user, enabling them to decide what to scan for:
Please let me know your thoughts on it. |
@thapabishwa the flag remain in trivy cli however trivy should be aware to k8s internal notions for example : one way to do it is to introduce
once you have the flag in client its easy to include you functionality and also will help us in the future to introduce additional options |
Thank you, @chen-keinan. I've understood the advantages of adding this changes into the Thank you for your prompt reply. |
PR on Pulling it here as I cannot add reviewers on that PR. |
@thapabishwa PR has been approved and merged. Note: you'll need to update cli docs after adding new flag :: use this command : |
Thanks @chen-keinan. Upgraded Please let me know what you think about it. |
b3025e6
to
bf6bd8f
Compare
@thapabishwa FYI your PR is depend on #5058 as |
Thanks @chen-keinan . Appreciate your feedback. I've addressed to the comments you've made. |
2bc7c22
to
44eb279
Compare
I'm do not think it can be merged before #5058 as changes to trivy-kubernets lib will impact trivy k8s functionality , fixing the integration test will still keep trivy broken. |
@chen-keinan I took some time to rethink the changes made to the In the current implementation, resources are skipped just if they have ownerRef. I think, this exclusion criteria might not be optimal as resources can be owned by custom resources(like CRDs). As a consequence, I think it is more sensible to exclude only the resources that are owned by built-in workloads. I've created another PR in Please let me know what you think about 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.
@thapabishwa thank you for the contribution lgtm 🚀 .
lets wait for #5019 to get approved and merged (will also solve the integration test issue)
Thanks @chen-keinan. Could you also take a look at aquasecurity/trivy-kubernetes#215? I think it might be beneficial for us to consider aquasecurity/trivy-kubernetes#215 PR before merging the current one. |
@thapabishwa approved and merged , please update trivy-kubernetes version to |
wait until triy-kubernetes v0.5.7-0.20230830053006-95e88d51f82b
version is updated on go.mod file
Hey @chen-keinan. Sorry, I made some mistake on last PR to Please take a look at this PR and let me know what you think about it. aquasecurity/trivy-kubernetes#216 |
new tag for trivy-kubernetes: |
@chen-keinan Thanks a lot. Appreciate your support and patience while I addressed the mistakes I made. It means a lot. Updated |
d37eabe
to
25614f8
Compare
Rebased onto main to resolve |
@thapabishwa Please rebase your branch with upstream and k8s integration test issue will be fixed |
- filter artifacts using trivy-kubernetes library - upgrade dependencies - generate docs
25614f8
to
13dcf57
Compare
Thanks @chen-keinan . I've rebased the branch onto latest main. |
@chen-keinan @knqyf263 . I've addressed to all the comments. Please let me know once this is merged. |
Thanks |
--exclude-owned
flag to exclude K8S Resources with Owner References
Description
This PR introduces the
--exclude-owned
flag to the Kubernetes resource handling. The purpose of this flag is to allow exclusion of Kubernetes objects that have an owner reference. This can be useful when we want to filter out resources that are managed or owned by other objects.Use cases
--exclude-owner
is enabled, this flag streamlines scanning by skipping resources that are managed by higher-level controllers.Before
After
Related PRs
--all-namespaces
flag #5043Checklist