Re: [foreman-dev] Opinions from plugin maintainers wanted: permissions and roles

[Lukas Zapletal]

> Let's continue the discussion here since it might read more people. I think
> that as a user I don't care that my installation consists of core and several
> plugins and I want to have Viewer role that gathers all view permission for
> the whole app.
>
> This does not in conflict with also providing "$plugin Viewer" and "$plugin
> Manager" roles so if user wants to create a user group from subset of
> permission he can still do it.
>
> If you want to keep current Viewer and Manager roles to contain only core
> permissions then I'd suggest renaming them to Core Viewer and Core Manager

The price for that is too high in my eyes. I think these roles and
permissions should be strictly separated for now and forever and we
need to come up with different approach of handling that. What I like
the best is a help text, better documentation and renaming the core
roles to something that is more obvious.

Allowing plugins to modify core roles will end up with a mess that is
very difficult to clean! Both adding and deleting permissions for
existing roles during upgrades is very challenging, we usually want to
tell administrators "hey, danger ahead, during upgrade all these users
will get/lost some permissions" so it's a dilemma to do this via
migration/seed or explicitly ask the user to do the change in upgrade
notes. When reviewing these changes, we need to be careful and we are
doing great job in core, but I wonder what happens if we open the
doors to any plugin to basically play around with the two most
important roles in the application.

-- 
Later,
  Lukas @lzap Zapletal

-- 
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.

Re: [foreman-dev] Revert removal of @host.params for host_param

[Ewoud Kohl van Wijngaarden]

Hello,

My opinion comes as a non-core developer but more as a user. I'd prefer
not to relearn anything so any change is something I'd like to avoid.
Even when you automatically change any existing template then you still
haven't changed the knowledge inside the users head. Given this has been
the interface for quite a long time, I don't think this should be
changed lightly.

On the other hand, decoupling from the activerecord object is a good
thing. To me the proxy object sounds like an ideal middle ground because
you get the decoupling, but don't have to change the interface for the
user.

>From that point of view I'd suggest to revert change and decide on the
proxy object. I also undestand that you may not have time to implement
the proxy right now, but going from an iterative perspective it is an
option to do that when a macro is added or changed. Then the interface
to the user can be formalised.

Another thing that's important to note is that those affected (users)
will most likely not even see the PR. That's a problem I don't know how
to solve but something we should all be aware of.

On Tue, Jan 17, 2017 at 08:52:20AM +0100, Marek Hulán wrote:
> Hello
> 
> The discussion has diverged into something different. People agree we should 
> add an extra layer but implemented through proxy objects. It's also clear no 
> one starts actively working on this right now. At the same time I want to add 
> new macro, let's say rpm_distro? which returns true/false based on 
> @host.operatingsystem, how do I do it?
> 
> So far the answer was to add a new macro. Now with the proxy object, should I 
> define it in host so one would use @host.rpm_distro?. Honestly I don't want 
> to 
> add more stuff into Host object, not even through concerns. So should I wait 
> until we have a proxy layer? That means I can't add any improvements. For me 
> the best answer is add new macro called "rpm_distro?". This will result in 
> two 
> approaches in future, we'll have both @host.param and rpm_distro?
> 
> I think the best approach is just reverting the deprecation warning and add a 
> value to host_param macro. It can check that user is asking for a parameter 
> that does not exist and raise a exception of a specific class so rendering 
> could display nicer error message such as "You tried to use parameter with 
> name abc but it's not defined for host xyz" instead of "Undefined method 
> upcase on nil". We could extend Host#param for this too but it's not right, 
> this exception is specific for template rendering, other internal usage of 
> this method might rely on fact it returns nil.
> 
> My probably last comment in this thread - the PR was opened Nov 1st 2016, 
> deprecation was suggested Nov 2nd, merged Jan 3rd 2017 [1]. I know a lot of 
> devs were offline during December but still, please try to raise you concerns 
> in PRs rather than revert stuff.
> 
> [1] https://github.com/theforeman/foreman/pull/3983
> 
> --
> Marek
> 
> On pondělí 16. ledna 2017 14:59:43 CET Tomer Brisker wrote:
> > Hi,
> > I think that while there are benefits to moving away from the host object,
> > we have a de-facto API based on it.
> > A way to change without breaking user's template would be to use a "proxy"
> > object that maintains the same endpoints and allows adding functionality or
> > handling changes in the underlying objects without breaking the way users
> > use them.
> > So the templates will still have a "@host" object with the same methods as
> > before, except it will in fact be proxy object and not an active record.
> > For example, we could have a nice error message if the actual host is nil.
> > We could also leverage method_missing to easily handle passing anything not
> > defined in the api to the host (possibly only if safe mode is off), so that
> > any users relying on active record methods etc. won't have breakage.
> > 
> > Tomer
> > 
> > On Mon, Jan 16, 2017 at 2:49 PM, Stephen Benjamin 
> > 
> > wrote:
> > > On Mon, Jan 16, 2017 at 3:24 AM, Ondrej Prazak  wrote:
> > > > +1 for keeping the macros. IMHO, just because we did something a certain
> > > 
> > > way for a long time should not prevent us from changing it if there are
> > > reasons for a change. This is also not the first change in core that
> > > affected plugin(s) in a negative way and I doubt it will be the last.
> > > Breaking plugin tests is unpleasant, but it is still a second best
> > > scenario.
> > > 
> > > 
> > > The plugin test failure is relatively minor, the main objection here
> > > is the number of users who rely on this interface now need to change.
> > > After raising this issue we got some PR's that help the migrations
> > > which is nice, but there's
> > > organizations who have less sophisticated technical users who learned
> > > basic ERB who have to re-learn this, not to mention that git repos of
> > > users using foreman_templates now have to update, and we now have lots
> > > of incorrect info 

Re: [foreman-dev] Opinions from plugin maintainers wanted: permissions and roles

[Marek Hulán]

On úterý 17. ledna 2017 15:46:56 CET Lukas Zapletal wrote:
> On Mon, Jan 16, 2017 at 6:55 PM, oprazak  wrote:
> > So if a plugin is installed, user has to go and find what role/permission
> > is missing or ask someone who can grant permissions.
> 
> I do not think the proposed approach is the best, here is my lengthy
> explanation:
> 
> https://github.com/theforeman/foreman/pull/4168#issuecomment-273187422

Let's continue the discussion here since it might read more people. I think 
that as a user I don't care that my installation consists of core and several 
plugins and I want to have Viewer role that gathers all view permission for 
the whole app.

This does not in conflict with also providing "$plugin Viewer" and "$plugin 
Manager" roles so if user wants to create a user group from subset of 
permission he can still do it.

If you want to keep current Viewer and Manager roles to contain only core 
permissions then I'd suggest renaming them to Core Viewer and Core Manager

--
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.