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

ci: Add write permission to security events when building ig. #2315

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

eiffel-fl
Copy link
Member

@eiffel-fl eiffel-fl commented Dec 20, 2023

This is needed by CodeQL to be able to report events.

Signed-off-by: Francis Laniel flaniel@linux.microsoft.com
Fixes: f04d95b ("ci: Add CWE checks for ig.")
[1]: github/codeql#8843 (comment)

@mauriciovasquezbernal
Copy link
Member

Otherwise, it explodes when run on main.

Can you describe what's exploding in this case? Issue on our end? Limitation of CodeQL? Anything else?

@eiffel-fl
Copy link
Member Author

Otherwise, it explodes when run on main.

Can you describe what's exploding in this case? Issue on our end? Limitation of CodeQL? Anything else?

I updated the commit message with the corresponding documentation.

@mauriciovasquezbernal
Copy link
Member

I can't see how https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#scanning-pull-requests explains the issue, above that it says it can also run on push -> https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#scanning-on-push

The failing run says Resource not accessible by integration, according to github/codeql#8843 (comment) it could be it's missing security-events: write.

I see those permissions are set in

permissions:
security-events: write

but that's part of the btfgen job, should it be moved to the build-ig one?

I can also see the CodeQl checks worked fine for kubectl-gadget (https://github.com/inspektor-gadget/inspektor-gadget/actions/runs/7272757833/job/19815436935), hence I think there is something off with the ig case.

@eiffel-fl
Copy link
Member Author

I can't see how https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#scanning-pull-requests explains the issue, above that it says it can also run on push -> https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#scanning-on-push

The failing run says Resource not accessible by integration, according to github/codeql#8843 (comment) it could be it's missing security-events: write.

I see those permissions are set in

permissions:
security-events: write

but that's part of the btfgen job, should it be moved to the build-ig one?

I can also see the CodeQl checks worked fine for kubectl-gadget (https://github.com/inspektor-gadget/inspektor-gadget/actions/runs/7272757833/job/19815436935), hence I think there is something off with the ig case.

Indeed, I forgot to add this permission to build-ig in the previous PR while I did it for the other checks, I guess this is what happens when you work again on PR opened since ages.
Anyway, it should fix the issue, but I do not understand why it did not break inside the previous PR.

@eiffel-fl eiffel-fl changed the title ci: Only run CodeQL checks on pull request. ci: Add write permission to security events when building ig. Dec 20, 2023
Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

LGTM once the permissions are removed from the btfgen job.

.github/workflows/inspektor-gadget.yml Show resolved Hide resolved
This is needed by CodeQL to be able to report events.

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
Fixes: f04d95b ("ci: Add CWE checks for ig.")
[1]: github/codeql#8843 (comment)
@eiffel-fl eiffel-fl merged commit 2b5a8af into main Dec 20, 2023
53 checks passed
@eiffel-fl eiffel-fl deleted the francis/codeql/pr branch December 20, 2023 16:32
@eiffel-fl
Copy link
Member Author

Thank you for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants