-
Notifications
You must be signed in to change notification settings - Fork 912
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
new(CI): add CodeQL security scanning to Falco. #2171
Conversation
Signed-off-by: Andrea Terzolo <[email protected]> Co-authored-by: Chris Aniszczyk <[email protected]>
@Andreagit97: The label(s) 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. |
/area CI |
Good catch! 🤩 /milestone 0.33.0 |
Signed-off-by: Andrea Terzolo <[email protected]>
we use python only in out tests Signed-off-by: Andrea Terzolo <[email protected]>
Signed-off-by: Andrea Terzolo <[email protected]>
Unfortunately CodeQL runs the analysis also on our libraries since they are part of the build process... I've also tried to exclude some paths through the configuration file with
You can read more about it here 👉 https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#specifying-directories-to-scan |
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.
Unfortunately CodeQL runs the analysis also on our libraries since they are part of the build process...
I see. If we can't do anything different, I think this may be acceptable. After all the job should not be required, so even if we have some noise we can live with it. Plus, this may help us spot issues in libs too.
.github/workflows/codeql.yaml
Outdated
|
||
on: | ||
push: | ||
branches: [ "master", release/0.26.2, release/0.32.2 ] |
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.
I don't think we need to also point release branches here. After all, release branches will be expected to contain only cherry-picked commits that have been merged in mailine first. What do you think?
Alternatively, is something like release/*
a viable option in GHA?
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.
Agree I left them from the previous PR but I think we don't need the release branches since they are just cherry-picked commits, I will remove them, thank you 🚀
Signed-off-by: Andrea Terzolo <[email protected]> Co-authored-by: Jason Dellaluce <[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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, jasondellaluce The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
LGTM label has been added. Git tree hash: 5636ce79cb7d32678b8c9ea252abe7a3196d3313
|
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.
LGTM 🤩
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area CI
What this PR does / why we need it:
This PR recalls the one opened by @caniszczyk here 👉 #1499
Don't know why this has not been merged at the time, since I find it very useful for us 🤔 BTW I will try to reopen it now, thank you again @caniszczyk for your great job! I've just updated the version of some GH actions :)
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?: