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..
Hi,
I fixed one rather embarassing issue in sudo code related to handling of
the special 'ALL' case. There are issues looking up service accounts, but
I'm not sure if they are related to this patchset or broken in master.,
I will investigate more.
There are some failures in downstram HBAC test, but I can't reproduce
them so far locally. I will keep digging.
Have a nice weekend!