On Wed, Jul 06, 2016 at 09:02:05PM +0200, Jakub Hrozek wrote:
> On Wed, Jul 06, 2016 at 06:34:32PM +0200, Jakub Hrozek wrote:
> > On Wed, Jul 06, 2016 at 11:13:02AM +0200, Lukas Slebodnik wrote:
> > > On (03/07/16 12:50), Jakub Hrozek wrote:
> > > >On Fri, Jul 01, 2016 at 11:01:53PM +0200, Jakub Hrozek wrote:
> > > >> On Fri, Jul 01, 2016 at 03:24:39PM +0200, Jakub Hrozek wrote:
> > > >> > 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!
> > > >
> > > >Hi,
> > > >I found some other bugs. To ease the review, I pushed them to:
> > > >
https://github.com/jhrozek/sssd/commits/sysdb-fqdn
> > > >I included Pavel's sudo patch as well because otherwise I would do
some
> > > >changes and then he would overwrite them back :)
> > > >
> > > >So far I kept the patches separate mostly to make it clear where the
> > > >bugs were, but I would like to squash them later.
> > > >
> > > >More downstream tests were fixed with these patches, but a new set is
> > > >running at the moment.
> > >
> > > There are some failures in nss-srv unit test
> > > [ RUN ] test_nss_getorigbyname
> > > "orig_name@nss_test" != "orig_name"
> > > ../sssd/src/tests/cmocka/test_nss_srv.c:2125: error: Failure!
> > >
> > > [ FAILED ] test_nss_getorigbyname
> > > [ RUN ] test_nss_getorigbyname_extra_attrs
> > > "orig_name@nss_test" != "orig_name"
> > > ../sssd/src/tests/cmocka/test_nss_srv.c:2225: error: Failure!
> > >
> > > [ FAILED ] test_nss_getorigbyname_extra_attrs
> > > [ RUN ] test_nss_getorigbyname_multi_value_attrs
> > > "orig_name@nss_test" != "orig_name"
> > > ../sssd/src/tests/cmocka/test_nss_srv.c:2353: error: Failure!
> > >
> > > [ FAILED ] test_nss_getorigbyname_multi_value_attrs
> > >
> > > and you need to rebase patches on top of current master
> > > because there is a small conflict in error codes.
> > >
> > > It would be good to push patches with thisrefactoring before
> > > 1.14.0. We can fix small bugs later (1.14.1)
> > >
> >
> > I pushed another patchset into my branch that includes one fix for HBAC,
> > one fix for SUDO (both squashed and both found by downstream tests) and
> > several patches from Sumit.
> >
> > I'm still running additional tests and I would like to work on the
> > upgrade from the specfile, but I think this patchset can be merged
> > already.
>
> Of course I forgot to cherry-pick one Sumit's patch. I included it now
> and re-ran tests again. I will send a report to our internal list
> (because the tests are downstream..) when they finish..
I did some testing as well which now all went well, including the update
of a cache with more then 500 entries on a offline system. I think as
well the patches are ready to be pushed. I'm just waiting for a CI run
to finish before giving my ACK.
ACK
bye,
Sumit
>
> bye,
> Sumit
>
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel(a)lists.fedorahosted.org
> >
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>