Thanks for update, sending few more comments below in text On pondělí 23. ledna 2017 12:48:46 CET Lukas Zapletal wrote: > > Sorry I don't get it, especially the price. What's the difference between > > core and plugins in terms of permissions upgrades? If you rename plugin > > permission you need to provide the same migration as if you renamed it in > > core. > Once we merge everything into core roles, there is no easy way back. I > described above why I think a pull-mechanism (with separated roles) is > better that the proposal.
If you're concerned about the permissions table, this does not help. Permission are created there by plugin. If the permission is removed later, it should be removed from all roles anyway, user could already assign it to both core and plugin role. > > Currently we don't support plugin uninstallation, when we do we'll likely > > have a tool to say which permission should be removed. > > We had a need of removing a permission for existing plugin, this had > nothing to do with uninstallation. We added it by mistake basically. > But what happens if we decide to implement plugin uninstallation? The > proposal only makes it more tough. I suppose you wrote a migration that unassigned this permission from all filters and removed the permission from permissions table. Why does this not work for core? > > The only other option I see to make this usable is to precreate user group > > that plugins would assign their roles to. But I'd say that's the same > > thing > > just in different model. The user reports (see the mirroring BZ [1]) and > > especially the way they were reported convinces me we need to change the > > current status. If users open issues because they can't see a button even > > when they have role "viewers" then I think it's our fault. > > I think we need to improve how we report missing permissions overall, > that it will not be such a pain in finding particular role or > permissions and we can keep it separate. > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1304608 > > I really think we need to say "this is by design, add required roles > which are provided by plugins" here. Not all RFEs are valid. IMHO this is usability issue. And therefore valid RFE. > Anyway, summary of my opinion: > > * I am not against adding an API call to add permissions to roles in > general. > * I'd prefer pull mechanism rather than push. > * I do not like particularly modifications of core roles, I think we > should keep it separate and more focus on improving usability, > reporting and documentation around. > * I described my counter-proposal in the PR comment from this morning. I read your counter-proposal (I don't think it's "counter" :-) If I understand it correctly, we could break i down into following steps. 1) add DSL to extend core roles 2) make core roles locked for editing 3) add DSL to remove permissions from roles This thread started by discussion on automatically adding plugin permissions to core roles. So I guess that could only happen if we had 1-3. I'm fine with skipping automatically extending core roles until we're ready. > > There is one alternative proposal that comes to my mind - what if we > start tracking what created permissions. It could be core permission > installed during seed, plugin permission created by plugin API or > user-defined permission. This could help with permission deletion (if > it is not user-defined it will be less likely intrusive change to > delete it), it can possibly help with plugin uninstallation in the > future, it can improve debugging (ability to compare current state > with expected permission list). But I still think a pull mechanism API > would be better (that separates core and plugin permissions clearly > from user-defined). I find the locking better since it's similar concept to templates where we have the same problem. But I like the proposal in general for future plugin uninstallation. If audit can't give us the answer we should start tracking it, ideally the same way for all resources created by plugin. > Please do not understand this as hard-blocking your PR, I am trying to > write constructive notes on how to take a different approach. If you > think what is proposed in the PR is better, then just go ahead and I > will simply put my hands off from the review. I would love to hear > other opinions on that topic, I am surprised it's just us three/four > discussing this as everyone is using permissions and roles as > developers or as users. +1 -- Marek -- You received this message because you are subscribed to the Google Groups "foreman-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
