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

new(CI): add CodeQL security scanning to Falco. #2171

Merged
merged 5 commits into from
Aug 23, 2022

Conversation

Andreagit97
Copy link
Member

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?:

new(CI): add CodeQL security scanning to Falco.

Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Chris Aniszczyk <[email protected]>
@poiana
Copy link
Contributor

poiana commented Aug 19, 2022

@Andreagit97: The label(s) area/ci cannot be applied, because the repository doesn't have them.

In response to this:

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?:

new(CI): add CodeQL security scanning to Falco.

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.

@leogr
Copy link
Member

leogr commented Aug 19, 2022

/area CI

@poiana poiana added the area/ci label Aug 19, 2022
@leogr
Copy link
Member

leogr commented Aug 19, 2022

Good catch! 🤩
Thank you, @Andreagit97

/milestone 0.33.0

@poiana poiana added this to the 0.33.0 milestone Aug 19, 2022
we use python only in out tests

Signed-off-by: Andrea Terzolo <[email protected]>
@Andreagit97
Copy link
Member Author

Andreagit97 commented Aug 19, 2022

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 paths and paths-ignore keywords but I faced the following warning :(

The "paths"/"paths-ignore" fields of the config only have effect for JavaScript, Python, and Ruby

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

Copy link
Contributor

@jasondellaluce jasondellaluce left a 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.


on:
push:
branches: [ "master", release/0.26.2, release/0.32.2 ]
Copy link
Contributor

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?

Copy link
Member Author

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]>
Copy link
Contributor

@jasondellaluce jasondellaluce left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Aug 22, 2022

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana
Copy link
Contributor

poiana commented Aug 22, 2022

LGTM label has been added.

Git tree hash: 5636ce79cb7d32678b8c9ea252abe7a3196d3313

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

LGTM 🤩

@poiana poiana merged commit 02fce93 into falcosecurity:master Aug 23, 2022
@Andreagit97 Andreagit97 deleted the codeQL_CI branch October 15, 2022 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants