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

[Simo Sorce]

On Wed, 2017-03-01 at 17:29 +0100, Martin Basti wrote:
> 
> On 01.03.2017 17:04, Simo Sorce wrote:
> > 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 disagree,
> 
> ipaConfigString evokes that this is IPA configuration, but AFAIK the 
> SSSD is the consumer of data and it is unrelated to configuration of IPA 
> server. If you plan to extend usage of 'ipaDomainResolutionOrder' to 
> more entries than one, then is better to have separate attribute that 
> allows better LDAP searches (debugging, support).
> Why SSSD instead of downloading the exact attribute content should do a 
> parsing of messy values that can be inside ipaConfigString?
> 
> Why we suddenly plan to support older servers with a new feature? In 
> past access to new features required to upgrade freeipa, why we should 
> increase complexity of code and ldap searches? Any plugin that involve 
> ipaConfingString must be handled in special way, we basically cannot use 
> framework defaults -> increases bugs, devel time, prone to future 
> regressions. So in future when we implement UI for this we will suffer.
> 
> ipaConfigString is multivalued attribute, domains basically have to be 
> only one string to keep order (single value attribute) => additional 
> complications on both SSSD side and IPA framework side if somebody set 
> domain order as multiple values instead one. With single valued 
> attribute this is handled by free by LDAP.
> 
> Even for users is more natural to set string of domains to one attribute 
> instead of adding a new value with a special prefix and list domain to 
> multivalued attribute, the second is more error prone with worse UX.
> 
> I would like to have clean design, separate attributes for separate 
> features, otherwise we can just create ipaUltimateAtr and put JSON inside.


Given we are now talking about an attribute to reuse in multiple
objectclasss I tend to agree with all these points, my initial comments
on this were related to the single global option case.

Simo. 

Martin^2
> 
> >
> >> 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