-
Notifications
You must be signed in to change notification settings - Fork 84
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: submit dbus-broker builds to Coverity automatically #363
base: main
Are you sure you want to change the base?
Conversation
Let's make a full use of Coverity and submit the builds for analysis automatically every midnight. We can't do that for every PR, since there are quite strict rate limits that limit how many builds we can submit per day and per week (see [0]). The action (and the script) requires two environment variables to be set - $COVERITY_SCAN_TOKEN for authentication (can be found here [1]), and $COVERITY_SCAN_NOTIFICATION_EMAIL for sending the email notification when the build analysis is done. Originally this email used to be set to the email from the latest commit, but since the author of that commit might not even have permissions to see the Coverity report it's best to set it to one of the dbus-broker maintainers. Resolves: bus1#316 [0] https://scan.coverity.com/faq#frequency [1] https://scan.coverity.com/projects/dbus-broker?tab=project_settings
schedule: | ||
# Run Coverity daily at midnight | ||
- cron: '0 0 * * *' | ||
pull_request: |
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'm not sure secrets are available when PRs are opened. If they were it would be possible for anyone to "steal" them by opening PRs and sending them somewhere.
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.
With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository
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.
Yeah, this is mostly for maintainers (at least I do this in other repos) - if you open a PR from a branch directly in the upstream repo the secrets are accessible, so it makes debugging or changing the configuration less of a PITA. But in this case the action will fail even after the secrets are set up, since it's opened from a fork.
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.
it makes debugging or changing the configuration less of a PITA
Agreed. I think one option would be to special-case that by checking where PRs come from and skip it if they come from forks or, maybe, add a comment somewhere. Then again it should make it easier to spot PRs that should be reopened from branches if it fails loudly.
…_EMAIL to prevent it from sending notifications to the authors of the latest commits. That logic came from some internal script and doesn't make sense upstream. The idea was borrowed from bus1/dbus-broker#363
--silent --write-out "\n%{http_code}\n" \ | ||
--form project="$COVERITY_SCAN_PROJECT_NAME" \ | ||
--form token="${COVERITY_SCAN_TOKEN:?}" \ | ||
--form email="${COVERITY_SCAN_NOTIFICATION_EMAIL:?}" \ |
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 borrowed this avahi/avahi#600 :-) Though as far as I can remember thanks to that spammy approach some folks got subscribed to the systemd dashboard and helped to fix some issues :-)
…_EMAIL to prevent it from sending notifications to the authors of the latest commits. That logic came from some internal script and doesn't make sense upstream. The idea was borrowed from bus1/dbus-broker#363
Let's make a full use of Coverity and submit the builds for analysis automatically every midnight. We can't do that for every PR, since there are quite strict rate limits that limit how many builds we can submit per day and per week (see [0]).
The action (and the script) requires two environment variables to be set - $COVERITY_SCAN_TOKEN for authentication (can be found here [1]), and $COVERITY_SCAN_NOTIFICATION_EMAIL for sending the email notification when the build analysis is done. Originally this email used to be set to the email from the latest commit, but since the author of that commit might not even have permissions to see the Coverity report it's best to set it to one of the dbus-broker maintainers.
Resolves: #316
[0] https://scan.coverity.com/faq#frequency
[1] https://scan.coverity.com/projects/dbus-broker?tab=project_settings
As mentioned in the commit description (and the GH Actions file), there are two environment variables that need to be configured in the dbus-broker repo for this action to work properly. The configuration can be found under repo settings -> Secrets and variables -> Actions -> Repository secrets:
As for the notification email - this is really up to the maintainers to pick one (feel free to use mine, since that's already the case for other upstream repos). You'll get one email every day with the results of the analysis, and potentially a second one with description of each flaw that was detected if the analysis detected something new.
I gave the action a spin in mrc0mmand#3 and it seems to work as expected.
/cc @evverx