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

polkit and Coverity Scan #517

Open
evverx opened this issue Oct 31, 2024 · 14 comments · May be fixed by #523
Open

polkit and Coverity Scan #517

evverx opened this issue Oct 31, 2024 · 14 comments · May be fixed by #523

Comments

@evverx
Copy link

evverx commented Oct 31, 2024

Is your feature request related to a problem? Please describe.
I went to https://scan.coverity.com/ and found https://scan.coverity.com/projects/polkit there. Looks like it hasn't been updated since 2014.

Describe the solution you'd like
It would be great to send polkit to Coverity Scan automatically by analogy with bus1/dbus-broker#316.

Describe alternatives you've considered
I can send it there manually by analogy with that I do with dbus-broker but it's not ideal.

@pwithnall
Copy link
Contributor

I’m currently the only admin for that Coverity account. I’m happy to make the current maintainers of polkit also be admins there. I don’t have time to maintain or run the Coverity scans any more. Is there a canonical list of the current maintainers somewhere?

@evverx
Copy link
Author

evverx commented Nov 4, 2024

I think in terms of setting up a GitHub action sending data to Coverity Scan automatically it should be enough to add @mrc0mmand there to test the integration like bus1/dbus-broker#363. I don't know if @mrc0mmand has access to the repository secrets here on GitHub but I don't think it should be a problem to pass the coverity token to the maintainers with that kind of access to the repository.

@pwithnall
Copy link
Contributor

@jrybar-rh, who are the current maintainers of polkit? From recent commit history it looks like just you have merge rights. I’d love to give permissions for Coverity to someone, but I want to double check I’m giving it to the right people!

@evverx
Copy link
Author

evverx commented Nov 4, 2024

(Just to be absolutely clear I don't need any access to Coverity. I already send polkit to another instance)

@mrc0mmand
Copy link
Member

@pwithnall I think you can give the permissions to @jrybar-rh for now and he can then extend this to other people when needed.

@pwithnall
Copy link
Contributor

I’ve invited @jrybar-rh to Coverity using their redhat.com address

@jrybar-rh
Copy link
Member

@mrc0mmand added to the project on Coverity as maintainer. BTW polkit is tested in OSH for Fedora, just sayin'.
Frantisek, I'll leave this issue for you to close whenever you're ok with it.

mrc0mmand added a commit that referenced this issue Nov 6, 2024
Let's reintroduce regular Coverity builds. Since there's a pretty strict
rate limit [0], do one nightly build each day, and upload it to Coverity
for analysis. The results can be then found in the project dashboard [1].

[0] https://scan.coverity.com/faq#frequency
[1] https://scan.coverity.com/projects/polkit?tab=overview

Resolves: #517
@pwithnall
Copy link
Contributor

Looks like you’re all set up there now. I’ll remove myself as an admin, as I’m no longer running Coverity builds :)

@pwithnall
Copy link
Contributor

Hmm, I can’t see a way to remove myself as an admin. Please feel free to remove me yourselves, from https://scan.coverity.com/projects/polkit?tab=members

@jrybar-rh
Copy link
Member

Thank you, @pwithnall.

@mrc0mmand
Copy link
Member

@pwithnall do you want stay as a member or be removed completely from the Coverity project?

@pwithnall
Copy link
Contributor

Please remove me completely :)

@mrc0mmand
Copy link
Member

Please remove me completely :)

Done. Thanks a lot for the access to the project!

@evverx
Copy link
Author

evverx commented Nov 6, 2024

BTW polkit is tested in OSH for Fedora, just sayin'

I think systemd-ci-incubator#2 would still be useful because it can show newly introduced findings when PRs are opened. As far as I understand it was added to Packit to make it possible to catch things as early as possible instead of waiting for releases or "cron" builds. For the same reason I think it's useful to run dfuzzer when PRs are opened (#515). As far as I can remember some distros like openSUSE run it before releases (but I don't think they run anything under ASan/UBSan/Valgrind. I'm not sure their CI infrastructure pulled the change introduced by @mrc0mmand allowing dfuzzer to poke properties either).

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 a pull request may close this issue.

4 participants