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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
