pulp / pulp_deb

Debian repository plugin for Pulp (pulpproject.org)
GNU General Public License v2.0
61 stars 78 forks source link

Add initial RBAC support #1062

Open maggu opened 6 months ago

maggu commented 6 months ago

Add RBAC support for APT repositories. Partially fixes #860.

hstct commented 5 months ago

The Deb CI / test / test (docs) (pull_request) pipeline is running endless had to cancel it. I think you may need to rebase your branch onto the latest main branch.

maggu commented 5 months ago

@hstct Okay. Done now.

quba42 commented 5 months ago

@maggu First of all thanks for this work.

Since this is an entirely new feature for pulp_deb and I don't have any experience how RBAC works for other Pulp plugins, I am somewhat unsure how to go about reviewing this.

One thing that would really help us, is any amount of write up of use cases how you want to use RBAC in pulp_deb that you can do. Perhaps a post in https://discourse.pulpproject.org/c/development/8 to accompany this PR would be a good place to start. Alternatively a post on the accompanying issue might also work: https://github.com/pulp/pulp_deb/issues/860.

I am trying to understand what the goal and scope for "initial RBAC support" in pulp_deb should be.

Does this request make any sense to you?

maggu commented 5 months ago

@quba42 Absolutely. I'll delegate that task to colleagues who have a clearer view of our use cases than I do. Thanks.

maggu commented 3 months ago

This PR looks pretty good. Are you thinking of adding any tests?

Thank you for review and valuable feedback! Sorry it's taken me quite some time to reply. For now my purpose has been to resolve our most urgent business needs, but tests should obviously be added and hopefully we can help out with that as well.

maggu commented 3 months ago

I'm going on vacation now, so won't work on this for some time. I'm planning on making it a priority once I get back though. In the meantime, please feel free to make edits if you want to.

maggu commented 2 weeks ago

I added a few basic functional tests as well now. Comments? They cover the cases mentioned in #860 at least, and some more. I suppose they would benefit from being further extended at some point, but perhaps it is good enough for this PR?

(They made me discover that I had mistakenly written view_debrepository instead of view_aptrepository in viewsets/publication.py by the way. It's corrected now.)