[Freeipa-devel] Please review: V4/AD user short names design draft

Wed Mar 1 16:04:52 UTC 2017

On Wed, 2017-03-01 at 16:47 +0100, Martin Babinsky wrote:
> On 03/01/2017 04:32 PM, Simo Sorce wrote:
> > On Wed, 2017-03-01 at 16:17 +0100, Martin Babinsky wrote:
> >> On 03/01/2017 03:42 PM, Simo Sorce wrote:
> >>> On Tue, 2017-02-28 at 13:29 +0100, Martin Babinsky wrote:
> >>>> Hello list,
> >>>>
> >>>> I have put together a draft of design page describing server-side
> >>>> implementation of user short name -> fully-qualified name resolution.[1]
> >>>>
> >>>> In the end I have taken the liberty to change a few aspects of the
> >>>> design we have agreed on before and I will be grad if we can discuss
> >>>> them further.
> >>>>
> >>>> Me and Honza have discussed the object that should hold the domain
> >>>> resolution order and given the fact that IPA domain can also be a part
> >>>> of this list, we have decided that this information is no longer bound
> >>>> to trust configuration and should be a part of the global config instead.
> >>>>
> >>>> Also we have purposefully cut down the API only to a raw manipulation of
> >>>> the attribute using an option of `ipa config-mod`. The reasons for this
> >>>> are twofold:
> >>>>
> >>>>    * the developer resources are quite scarce and it may be good to
> >>>> follow YAGNI[2] principle to implement the dumbest API now and not to
> >>>> invest into more high-level interface unless there is a demand for it
> >>>>
> >>>>    * we can imagine that the manipulation of the domain resolution order
> >>>> is a rare operation (ideally only once all trusts are established), so I
> >>>> am not convinced that it is worth investing into designing higher-level API
> >>>>
> >>>> I propose we first develop the "dumber" parts first to unblock the SSSD
> >>>> part. If we have spare cycle afterwards then we can design and implement
> >>>> more bells-and-whistles afterwards.
> >>>>
> >>>> [1] https://www.freeipa.org/page/V4/AD_User_Short_Names
> >>>> [2] https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it
> >>>
> >>> Thank you Martin,
> >>> this is a good initial proposal.
> >>>
> >>> I have a few issues with this design:
> >>> - It conflates the idea of ordering with the idea of shortening user
> >>> names
> >>
> >> I fail to see where the conflation takes place. The ordered list is
> >> stored on the server. The client then uses it to expand short names. I
> >> guess I am just missing something.
> >
> > The attribute is called ipaNTDomainResolutionOrder, nothing in that
> > attribute says anything about making names become short names.
> > If it were ipaNTShortNameDomainResolutionOrder then it would be
> > specific, as it is it seem just to refer to the order in which domain
> > are resolved, but that is somethign we want in order to determine which
> > domains SSSD is going to make use short names too, not just the order in
> > which domains are resolved.
> > I hope this makes it clearer.
> >
> >>> - It allows only for one setting for all the machines, no way to treat
> >>> different groups of machines differently
> >>>
> >>
> >> Yes it was discussed that the setting will be global. I would implement
> >> local overrides only when there is a demand for the feature given
> >> development time is short.
> >
> > Demand is immediate, and it is obvious IMO.
> >
> 
> Such demand was not made clear during previous discussions and was not 
> mentioned by SSSD guys either AFAIK.

I guess this is why we do reviews :-)

> >>> The first one is probably just a matter of using a more specific name
> >>> for the new attribute, or, perhaps not use a new attribute at all but
> >>> just use ipaConfigString with an agreed syntax like:
> >>> ipaConfigString: Domains Use Short Name List: aaa bbb ccc ddd
> >>>
> >>> The side effect of using ipaConfigString is that we can set this on
> >>> older servers too, so people do not have to upgrade their servers to use
> >>> this. Old servers will not have any validation, but that is ok, sssd
> >>> must be prepared to receive a bad list and deal with it appropriately
> >>> anyway.
> >>>
> >>
> >> No more 'ipaConfigString' attribute values, please. Me and everyone else
> >> fixing e.g. replication issues can relate to the pain of doing CRUD
> >> operations involving them.
> >
> > ipaConfigString was devised explicitly so that configuration options
> > could be added without replication issues because the string can be
> > accepted by any server version.
> > So what replication issues are there ?
> > What has CRUD to do with it ?
> >
> 
> Well consider client doing a) retrieve ipaDomainResolutionOrder and 
> split it by delimiter, or b) retrieve values of ipaConfigString, iterate 
> until you find one that starts with "Domains Use Short Name list:", 
> strip off the rest of the value and split it by delimiter.

I do not see any problem with this.

> I just feel anything involving 'ipaConfigString' leads to design smell, 
> sorry. Yes it is my personal opinion but I think there are more people 
> sharing it. If not, I am happy to hear counterarguments.

I am asking why, can you bring some evidence ?
I am all about feelings, they are important, but I want data to make a
decision.

> >> If the admin wishes old servers to server new clients this information,
> >
> > They do not "wish", this is pretty much what happens all the time ...
> >
> >> all he has to do is upgrade a single replica, set the attribute value
> >> there and let replication take care of the rest.
> >
> > Come on, really ?
> > If you have RHEL6 it is not a matter of "simply" upgrading a single
> > replica, it means upgrade of the whole infrastructure ...
> >
> 
> There is plenty of features not available to deplyments with RHEL6 
> masters, I simply fail to see why this one should be special.

It is not that it is special, my problem with that statement is that you
assume that it is easy to upgrade servers. It is not, and decisions
based on that assumption end up being very bad decisions for our users.
So please do not ever assume that our users can "just upgrade one of
their replicas".

> >> Yes, the management CLI
> >> will not be available on the old masters but that is the case of new
> >> features anyway.
> >
> > I do not think we need any management UI in the short term to be honest,
> > just a way to set a string.
> > That will cut most development time that can be spent instead on dealing
> > with allowing smaller groups of machines to be affected instead.
> >
> >>> The second one is something we *may* address later, and use the setting
> >>> in cn=ipaConfig as a default, but there are two reasons why I think a
> >>> setting applicable to just a host group makes sense:
> >>> - it allows to test the setting on a small set of machines to see if
> >>> everything works right, this is going to be especially important on
> >>> existing setups, where people do not want to risk all machines
> >>> misbehaving at once if something goes wrong.
> >>> - it allows to migrate machines slowly, in some cases people may need to
> >>> change local files/application settings on machines if the usernames
> >>> change, so they may need a controlled roll out before changing a setting
> >>> globally.
> >>>
> >>> This may achieved by adding this setting to an ID View for example, then
> >>> only hosts in that IDView would get this. Or a new object could be
> >>> created that has members, the former has the advantage of being already
> >>> in place and SSSD already downloads that data, the latter allows to
> >>> target an even smaller set of hosts unrelated to previous ID views
> >>> settings.
> >>>
> >>> Simo.
> >>>
> >>
> >> That is an interesting proposal but I am afraid we may not get to
> >> implement that during 4.5 development. I can certainly mention the
> >> possibility in the design so that we can return to it when a need arises.
> >
> > My take is: cut API/UI work, and do the underlying infrastructure work
> > for the widest set of serves/clients possible instead.
> >
> > It is much more important to get the underlying gears done than to add
> > UI candy, that can be delayed.
> >
> > Simo.
> >
> 
> I agree, we just have to come to agreement of *which* gears are really 
> necessary.

Indeed, but adding attributes to ipaConfig and the ID Views is not hard,
it is a matter of extending two objectclasses instead of one ... if we
decide that Id Views are a good abstraction point.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York