On Wed, Jun 29, 2016 at 07:00:13PM +0200, Jakub Hrozek wrote:
On Wed, Jun 29, 2016 at 04:36:23PM +0200, Sumit Bose wrote:
> On Wed, Jun 29, 2016 at 12:05:42PM +0200, Lukas Slebodnik wrote:
> > On (29/06/16 11:54), Pavel Březina wrote:
> > >On 06/28/2016 06:24 PM, Jakub Hrozek wrote:
> > >> Hi,
> > >>
> > >> here is my branch that implements using the fully qualified names in
sysdb
> > >> for users and groups:
> > >>
https://github.com/jhrozek/sssd/commits/sysdb-fqdn
> > >>
> > >> The SSSD code changes are done. There are some downstream tests that
are
> > >> failing (some sudo and some HBAC tests), but I think I can chase
those
> > >> down quickly.
> > >>
> > >> But -- I still don't have the upgrade code written and I only
tested the
> > >> IPA overrides and IPA-AD trust code manually, so there might be a lot
of
> > >> bugs there. It is also a lot of stuff to review.
> > >>
> > >> With that in mind, I would like to ask other developers for opinions
--
> > >> should we still pursue pushing this refactoring into 1.14? Or should
we
> > >> release 1.14.0 first and push this patchset as the first thing into
> > >> 1.15?
> > >
> > >Well there are few places where we told ourselves that it will be fixed as
a
> > >side effect of those patches. Given the scale I'm fine with postponing
it to
> > >1.15 but than we need to fix those.
> > >
> > >However, it still may be better to do it for 1.14 if doable.
> > >
> > +1 for 1.14 if possible
>
> I would prefer this as well. My first tests didn't went too bad. Besides
> other things I tested Smartcard and 2-factor authentication which both
> worked as expected.
>
> I added 3 fixes to my copy at
>
https://fedorapeople.org/cgit/sbose/public_git/sssd.git/log/?h=jhrozek_sy...
Thank you for the fixes and checking the code in general, I will check
these out later.
Thanks, merged to the branch.
>
> This first is needed if there is no home-directory defined in AD. The
> other 2 I came across while checking if lookups by UPN with alternative
> suffix are working. The lookups were working but the output changed
> depending on the lookup history. The functionality from the patches is
> not present in current master as well so chances are that there might
> similar issues here.
>
>
>
> About cache updates. I think this is the critical part in the decision
> here. If updates are not working or are too complex we cannot commit the
> refactoring.
Yes. The most complex part is parsing the qualified names which might
be in any format, given by the full_name_format option. To solve this,
I just pass a name context to the upgrade function.
The upgrade so far doesn't look too bad to me, at least users and groups
from main and trusted domains work:
https://github.com/jhrozek/sssd/commit/8492cfeaa3ca789c204e78bfdaebdfdb38...
I still need to upgrade override objects and sudo rules because these
contain usernames as well.
The memberof plugin is forcibly disabled during the upgrade and all
attributes are written in a single transaction from the upgrade. I think
this is safer, especially with complex group memberships, where the
memberof plugin might otherwise process a single entry multiple times.
The upgrade is done now. Please review..I tested user and group lookups
also with overrides. I force-pushed the code into my github branch.
I also fixed the timestamp integration tests.
>
> Maybe we should save a copy of the old cache to allow downgrades
> without loosing the cache content if something goes wrong with the
> update?
Yes. We can call sssctl to do this.
sssctl can do the upgrade now, I just need to add the upgrade to the
specfile..
The only thing remaining in this branch is to fix the failing downstream
tests IMO..