bodhi icon indicating copy to clipboard operation
bodhi copied to clipboard

Packit permissions for Bodhi updates

Open majamassarini opened this issue 1 year ago • 2 comments

Hi Bodhi team, I am Maja from the Packit team.

More than an issue this is a request for a feature.

We are now able to automatically release a package going through the phases of submitting a dist-git PR, creating a Koji build and a Bodhi update on behalf of a packager but the Packit Bot needs

  • to belong to the packager group
  • to be granted the commit access level for every package it releases to be able to do the Bodhi updates. We would like to get rid of the commit access level. We assumed we need it based on our experience: we can not manage to create a Bodhi update with lower access levels, but there is no documentation - or at least we could not find it - which describe this constraint and since Packit belongs to the packager group we can already do a Koji build for every package without asking further permissions. The Packit Bot does not have to commit into the package dist-git repo thus it sounds dangerous to ask for having such a permission.

One of the following solutions could help us:

  1. have an ad hoc permission setting just to enable the creation of Bodhi updates
  2. give to the collaborator or ticket access level the permission to create Bodhi updates
  3. give to Packit the permission to create Bodhi updates for every package but the maintainer must enable this behaviour manually through a setting.

Let us know if one of these solutions is feasible. And if we can help with it.

Thank you!

majamassarini avatar Jul 28 '22 12:07 majamassarini

Hi. We have a setting for this in Bodhi config: if a user is in one of the defined admin_packager_groups the ACL validation mechanism is bypassed and there's no need have commit privileges.

Currently admin_packager_groups is set to ['provenpackager', 'releng', 'security_respons']. I don't see any of this to be a feasible choice where adding Packit user, so the solution I see are:

  • ask (releng? fedora-infra?) for a new group without dist-git commit access to be created for bots like packit and add packit there + update Bodhi config. This is the quickest solution, because once the group is created we only have to change Bodhi config in ansible and redeploy. Also, the group can be used for any other future bots needs.
  • hack Bodhi code to add another config field (sigh) to be able to specify single users to bypass ACL validation.

Thoughts?

mattiaverga avatar Jul 28 '22 16:07 mattiaverga

Hi @mattiaverga ,

thanks for your prompt reply! We will speak with fedora-infra and then we will let you know.

thanks again!

majamassarini avatar Jul 29 '22 12:07 majamassarini

Has this been discussed with the Fedora infra? (link would be nice)

praiskup avatar Oct 13 '22 08:10 praiskup

Has this been discussed with the Fedora infra? (link would be nice)

Before opening this issue we wrote this one in fedora-infra. We did not yet go back to infra again.

majamassarini avatar Oct 13 '22 09:10 majamassarini

Hi! Packit lead here.

We discussed this during the today's team meeting and I have a few notes to this to push this forward:

  • Firstly, it would be nice to have the requirements to create Bodhi updates stated somewhere. (Sorry if it is mentioned anywhere and I haven't realised.) By now, we need to just try and see what works. And guess if there isn't anything else in the permission check.
  • Current state works for us, but we would like to get rid of the permissions we don't need. (So, it's not blocking us but would be really really nice to solve.)

For us, the priority of the solutions are as follows:

  1. The ideal state is to have a per-project permission to allow the creation of Bodhi updates. (It's explicit and Packit does not get any unnecessary rights.)
  • = Probably the second option mentioned by @mattiaverga (Do you want to see this configuration in Bodhi or in DistGit?)
  1. The second option for us is the first mentioned by @mattiaverga, to have a global group of Bodhi updaters.
  • This will simplify our UX and is probably easy to implement for you.
  • But a question is if it helps compared to the current solution. If having the global power of creating Bodhi updates is less dangerous than having dist-git commit rights to a set of projects.

So:

  • What needs to be done to achieve the ideal state (= option 1, per-project config), can we somehow help with that?
  • Do you think option 2 helps anyhow? Is it worth implementing it? (Either as a final step or as a middle step before doing option 2.)

Thank you very much in advance!

(And thanks Pavel for bringing up that we had already asked Fedora infra and had been asked to ask you..;)

lachmanfrantisek avatar Oct 20 '22 14:10 lachmanfrantisek

Hi! Packit lead here.

We discussed this during the today's team meeting and I have a few notes to this to push this forward:

* Firstly, it would be nice to have the requirements to create Bodhi updates stated somewhere. (Sorry if it is mentioned anywhere and I haven't realised.) By now, we need to just try and see what works. And guess if there isn't anything else in the permission check.

Bodhi relies on permissions set in DistGit, if a user has commit rights or is a project collaborator for that specific branch they can create the update. Before looking at DistGit, Bodhi always grant permissions to any user which belong to one of the admin_packager_groups I stated in my previous reply.

* Current state works for us, but we would like to get rid of the permissions we don't need. (So, it's not blocking us but would be really really nice to solve.)

For us, the priority of the solutions are as follows:

1. The ideal state is to have a per-project permission to allow the creation of Bodhi updates. (It's explicit and Packit does not get any unnecessary rights.)


* = Probably the second option mentioned by @mattiaverga (Do you want to see this configuration in Bodhi or in DistGit?)

For what I explained above, there's no way to have per-project permissions in Bodhi. Unless we want to hardcode them in the configuration file, but that would be a really ugly implementations IMO.

mattiaverga avatar Oct 20 '22 16:10 mattiaverga

@mattiaverga Thanks for the answer and sorry for being out for a few days.

Bodhi relies on permissions set in DistGit, if a user has commit rights or is a project collaborator for that specific branch they can create the update. For what I explained above, there's no way to have per-project permissions in Bodhi.

If I got this right you can't work per project on the Bodhi level, but if there is a way to set these permissions in Dist-git, you can use it.

So, we can add e.g. a new Access level (e.g. update) that does not have commit rights but can do the update. Or, reuse Collaborator with no branch being specified. Or, if we can have a separate list of updaters in the dist-git settings, will this work for you?

(As I've mentioned. We don't need this to happen now, but want to achieve an ideal long-term solution.)

lachmanfrantisek avatar Oct 25 '22 11:10 lachmanfrantisek

Regarding a new group being created and added to the admin_packager_groups, we need to ask elsewhere. (I'll ask in the Fedora-infra ticket Maja has mentioned.)

lachmanfrantisek avatar Oct 25 '22 11:10 lachmanfrantisek

@mattiaverga Thanks for the answer and sorry for being out for a few days.

Bodhi relies on permissions set in DistGit, if a user has commit rights or is a project collaborator for that specific branch they can create the update. For what I explained above, there's no way to have per-project permissions in Bodhi.

If I got this right you can't work per project on the Bodhi level, but if there is a way to set these permissions in Dist-git, you can use it.

So, we can add e.g. a new Access level (e.g. update) that does not have commit rights but can do the update. Or, reuse Collaborator with no branch being specified. Or, if we can have a separate list of updaters in the dist-git settings, will this work for you?

For non-side-tag updates, Bodhi just uses the hascommit pagure endpoint:

GET /api/0/<repo>/hascommit?user=<username>&branch=<branchname>
GET /api/0/<namespace>/<repo>/hascommit?user=<username>&branch=<branchname>

All Bodhi does is to list if the response is True or False. Username and branch are always specified in the request.

If you plan to make Packit work also work side-tag updates, it would be best to use the admin_packager_groups approach.

mattiaverga avatar Oct 25 '22 16:10 mattiaverga

OK, thanks for the explanation.

Can we leave this issue open until we get that new group?

lachmanfrantisek avatar Oct 26 '22 08:10 lachmanfrantisek

Here is a request for a new group to be created: https://pagure.io/fedora-infrastructure/issue/10959

lachmanfrantisek avatar Oct 27 '22 08:10 lachmanfrantisek

What about also allowing users with the ticket ACL to submit Bodhi updates? The ticket ACL isn't currently used for anything in src.fp.o (that instance disables Pagure's built in issue tracker), and it shouldn't be too difficult for Bodhi to query for that. Then, packagers could give packit that ACL.

gotmax23 avatar Oct 27 '22 15:10 gotmax23

What about also allowing users with the ticket ACL to submit Bodhi updates? The ticket ACL isn't currently used for anything in src.fp.o (that instance disables Pagure's built in issue tracker), and it shouldn't be too difficult for Bodhi to query for that. Then, packagers could give packit that ACL.

That could be an interesting workaround of the problem. I think I can write a PR in the next days to enable that. In the long term, I think it would be more appropriate if pagure-dist-git could create an apposite ACL for that, or even better if anyone could write a Pagure plugin to enable Packit in a project options (something like a check mark that makes hascommit endpoint to return True if the username is packit).

mattiaverga avatar Oct 29 '22 08:10 mattiaverga

It would be ideal to name the option as update or bodhi, rather than ticket, wdyt? Without this rename, it would feel soo incorrect and would be tough to document.

The most convenient (IMO) way of handling this problem would eventually be the dedicated approved_updaters groups that Franta is proposing anyway. The code is already in DistGit which is the most privileged action ... then just let Packit make the update without additional explicit perms per/repo. Does that sound too dangerous?

praiskup avatar Oct 29 '22 09:10 praiskup

Note that the package mainatainer needs to set bodhi_update config in the DistGit located .packit.yml file first (example) - which is an equivalent security measure to the ticket permission.

praiskup avatar Oct 29 '22 09:10 praiskup

@mattiaverga:

Note that the package mainatainer needs to set bodhi_update config in the DistGit located .packit.yml file first (example) - which is an equivalent security measure to the ticket permission.

In addition to making it easier for a Packit bug to affect a whole lot of packages, this would allow users who don't have admin access (users with commit or provenpackagers) to enable this feature without any other steps. I'm not suggesting that I don't trust the Packit developers or that I don't trust provenpackagers to follow the provenpackager policies. I just think that there's better and safer ways to accomplish your goal.

In the long term, I think it would be more appropriate if pagure-dist-git could create an apposite ACL for that, or even better if anyone could write a Pagure plugin to enable Packit in a project options (something like a check mark that makes hascommit endpoint to return True if the username is packit).

That would be nice.

@praiskup:

It would be ideal to name the option as update or bodhi, rather than ticket, wdyt? Without this rename, it would feel soo incorrect and would be tough to document.

Currently, ticket does nothing in distgit, and there's no documentation about this. Documentation that it's used to allow users to create Bodhi updates and nothing else wouldn't be any worse than the current situation, IMO. Pagure development is somewhat dead, so I don't think we should block on having a new ACL, even though I agree that's a better solution.

gotmax23 avatar Oct 29 '22 17:10 gotmax23

@mattiaverga:

Note that the package mainatainer needs to set bodhi_update config in the DistGit located .packit.yml file first (example) - which is an equivalent security measure to the ticket permission.

In addition to making it easier for a Packit bug to affect a whole lot of packages, this would allow users who don't have admin access (users with commit or provenpackagers) to enable this feature without any other steps. I'm not suggesting that I don't trust the Packit developers or that I don't trust provenpackagers to follow the provenpackager policies. I just think that there's better and safer ways to accomplish your goal.

I see your point, but note that this discussion is only about the last step, which is Packit to create an update for a stable/branched release. If we reach this point it means that Packit PRs have been merged, Koji builds have been completed (and tagged release-candidate) and, most important, Rawhide updates have been automatically pushed, since they're automatically created and don't require any validation.

I personally don't see any worsening of safety in enabling Packit pushing updates for any package, but since this is affecting the whole community it maybe needs to be discussed by FESCo. If they decide that it's not safer to do that, we can proceed with the workaround or the Pagure/pagure-dist-git plugin.

mattiaverga avatar Oct 30 '22 07:10 mattiaverga

I think this has been done, please reopen if I'm mistaken

mattiaverga avatar Feb 28 '23 07:02 mattiaverga