-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add govulncheck presubmit to expose go vulnerabilities in a PR #30591
Conversation
0ed8d1d
to
d9c8c8d
Compare
d9c8c8d
to
2eaa81f
Compare
I don't see any issues with this @ArkaSaha30. Would you be able to run this on the EKS cluster instead of the default cluster like it's currently set up? You can use https://github.com/kubernetes/test-infra/pull/30556/files as a guide on what updates are needed. |
/hold until kubernetes/sig-security#101 is merged |
2eaa81f
to
200757c
Compare
/retest |
Will this job just create the vulnerability artifact that can be reviewed for informational purposes, or is the intent to have some kind of gatekeeping element to fail if (for example) there are any CVEs found? If it's for gatekeeping, I'm not currently understanding how/where the job will fail out of the test. |
This will be mostly for informational purposes (atleast for now) since we want the job to not block a PR right now. This will allow us to observe and iterate over the accuracy of the scan results and then potentially in future act as gatekeeping for a PR if needed |
Awesome, thank you @ArkaSaha30 and @PushkarJ |
200757c
to
bbce756
Compare
Made a small change to the presubmit after testing locally:
The |
Signed-off-by: ArkaSaha30 <[email protected]>
bbce756
to
7b2eeb5
Compare
Pushing the changes as per discussion threads: |
Signed-off-by: ArkaSaha30 <[email protected]>
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.
This is much more clean! Well done.
/lgtm
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ArkaSaha30, rjsadow The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold for kubernetes/kubernetes#120562 |
@ArkaSaha30 @PushkarJ are we ok to close this based on kubernetes/kubernetes#120562 (comment) or do you see any other uses for having a separate job? |
The default verify script will make sure the PR doesn't make things worse, but I think we'd want a periodic job to check if existing state has issues, right? |
@liggitt yes we need the periodic one for sure. We are tracking it separately with this issue kubernetes/sig-security#100 |
I'm going to close this pr for now since kubernetes/kubernetes#120562 will add it to /close |
@rjsadow: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
This PR will add a govulncheck presubmit to check for go vulnerabilities when a PR is opened for go module changes. It will trigger the script from
sig-security/sig-security-tooling/govulncheck/hack/govulncheck-presubmit.sh
kubernetes/hack/verify-govulncheck.sh
.Fixes: kubernetes/sig-security#99
Parent Issue: kubernetes/sig-security#95