Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set
On Thu, Sep 17, 2015 at 02:50:09PM +0200, Pavel Březina wrote: > On 09/17/2015 01:26 PM, Pavel Březina wrote: > >On 09/17/2015 01:21 PM, Sumit Bose wrote: > >>On Thu, Sep 17, 2015 at 12:44:37PM +0200, Pavel Březina wrote: > >>>On 09/17/2015 12:37 PM, Sumit Bose wrote: > On Thu, Sep 17, 2015 at 12:11:17PM +0200, Pavel Březina wrote: > >https://fedorahosted.org/sssd/ticket/2790 > > > >This is just a partial fix. > > > >If a group contains member which doesn't have overrideDN specified, > >we fail > >to resolve membership with LOCAL views. > > > >There is a bigger issue when group contains ghost members, we do > >not apply > >overrides and just ignore the group. As code says, this is > >expected. Sumit, > >do you remember why it is this way? > > If there is an IPA idview applied to a client all objects have to be > completely resolved, i.e. the object must have been read from the > server > and it must have been checked if there is an override for the > object, to > return the right result to the caller. > > Since ghost entries are not resolved completely ENOENT is return to > indicate to the caller to call the backend to resolve all needed > objects completely, > > > > >sysdb_getgrgid_with_views: > >>if (el != NULL && el->num_values != 0) { > >>DEBUG(SSSDBG_TRACE_ALL, > >> "Group object [%s], contains ghost entries which > >>must be " \ > >> "resolved before overrides can be applied.\n", > >> ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); > >>ret = ENOENT; > >>goto done; > >>} > > > >As I see it, we can do: > >1) If group contains ghosts and memberuid, resolve memberuid > > I hope we can get rid of memberuid at some point. It was added as a > performance improvement at a time where we didn't had the memcache and > all request were handled by SSSD directly. Maybe this can be done > together with the planned sysdb changes which will store all entries > with a fully-qualified name. > > >2) For each ghost user either: > > a) Ignore > > b) Put the name in > > c) Try to find overrideObjectDN and apply possible name > >override, then put > >the name in > > I think b) should not be considered because it might return wrong > results. The other two options are valid. > >>> > >>>Is it possible to have unresolved user but its override values stored in > >>>sssd cache with IPA views? > >> > >>no, the override data is always written to the cache after the user > >>object. > > > >Then I think it will be best to keep it as is for IPA views, but resolve > >memberuid and put ghost in (1 and 2b) since it is not possible to have > >override object and unresolved user at the same time with local view. Do > >you agree? > > As per meeting discussion I went ahead and wrote the patch. > > > Hi, the patches are looking good, fix the issue, do not cause issues while testing with IPA overrides and pass CI (http://sssd-ci.duckdns.org/logs/job/26/79/summary.html), so ACK. bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set
On Fri, Sep 18, 2015 at 11:48:11AM +0200, Sumit Bose wrote: > Hi, > > the patches are looking good, fix the issue, do not cause issues while > testing with IPA overrides and pass CI > (http://sssd-ci.duckdns.org/logs/job/26/79/summary.html), so ACK. > > bye, > Sumit * 87e0dcaff945f8b8f30030309e16ba26935fcb7b * d5e26a3ec3fa1f217f0afd045a03b29d4f88fe1d * 9571c9ba5ee7f8aad24e9dec6c44ce21688fa044 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set
On Thu, Sep 17, 2015 at 12:11:17PM +0200, Pavel Březina wrote: > https://fedorahosted.org/sssd/ticket/2790 > > This is just a partial fix. > > If a group contains member which doesn't have overrideDN specified, we fail > to resolve membership with LOCAL views. > > There is a bigger issue when group contains ghost members, we do not apply > overrides and just ignore the group. As code says, this is expected. Sumit, > do you remember why it is this way? If there is an IPA idview applied to a client all objects have to be completely resolved, i.e. the object must have been read from the server and it must have been checked if there is an override for the object, to return the right result to the caller. Since ghost entries are not resolved completely ENOENT is return to indicate to the caller to call the backend to resolve all needed objects completely, > > sysdb_getgrgid_with_views: > >if (el != NULL && el->num_values != 0) { > >DEBUG(SSSDBG_TRACE_ALL, > > "Group object [%s], contains ghost entries which must be " > > \ > > "resolved before overrides can be applied.\n", > > ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); > >ret = ENOENT; > >goto done; > >} > > As I see it, we can do: > 1) If group contains ghosts and memberuid, resolve memberuid I hope we can get rid of memberuid at some point. It was added as a performance improvement at a time where we didn't had the memcache and all request were handled by SSSD directly. Maybe this can be done together with the planned sysdb changes which will store all entries with a fully-qualified name. > 2) For each ghost user either: > a) Ignore > b) Put the name in > c) Try to find overrideObjectDN and apply possible name override, then put > the name in I think b) should not be considered because it might return wrong results. The other two options are valid. HTH bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set
On 09/17/2015 12:37 PM, Sumit Bose wrote: On Thu, Sep 17, 2015 at 12:11:17PM +0200, Pavel Březina wrote: https://fedorahosted.org/sssd/ticket/2790 This is just a partial fix. If a group contains member which doesn't have overrideDN specified, we fail to resolve membership with LOCAL views. There is a bigger issue when group contains ghost members, we do not apply overrides and just ignore the group. As code says, this is expected. Sumit, do you remember why it is this way? If there is an IPA idview applied to a client all objects have to be completely resolved, i.e. the object must have been read from the server and it must have been checked if there is an override for the object, to return the right result to the caller. Since ghost entries are not resolved completely ENOENT is return to indicate to the caller to call the backend to resolve all needed objects completely, sysdb_getgrgid_with_views: if (el != NULL && el->num_values != 0) { DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost entries which must be " \ "resolved before overrides can be applied.\n", ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); ret = ENOENT; goto done; } As I see it, we can do: 1) If group contains ghosts and memberuid, resolve memberuid I hope we can get rid of memberuid at some point. It was added as a performance improvement at a time where we didn't had the memcache and all request were handled by SSSD directly. Maybe this can be done together with the planned sysdb changes which will store all entries with a fully-qualified name. 2) For each ghost user either: a) Ignore b) Put the name in c) Try to find overrideObjectDN and apply possible name override, then put the name in I think b) should not be considered because it might return wrong results. The other two options are valid. Is it possible to have unresolved user but its override values stored in sssd cache with IPA views? HTH bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set
On 09/17/2015 01:21 PM, Sumit Bose wrote: On Thu, Sep 17, 2015 at 12:44:37PM +0200, Pavel Březina wrote: On 09/17/2015 12:37 PM, Sumit Bose wrote: On Thu, Sep 17, 2015 at 12:11:17PM +0200, Pavel Březina wrote: https://fedorahosted.org/sssd/ticket/2790 This is just a partial fix. If a group contains member which doesn't have overrideDN specified, we fail to resolve membership with LOCAL views. There is a bigger issue when group contains ghost members, we do not apply overrides and just ignore the group. As code says, this is expected. Sumit, do you remember why it is this way? If there is an IPA idview applied to a client all objects have to be completely resolved, i.e. the object must have been read from the server and it must have been checked if there is an override for the object, to return the right result to the caller. Since ghost entries are not resolved completely ENOENT is return to indicate to the caller to call the backend to resolve all needed objects completely, sysdb_getgrgid_with_views: if (el != NULL && el->num_values != 0) { DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost entries which must be " \ "resolved before overrides can be applied.\n", ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); ret = ENOENT; goto done; } As I see it, we can do: 1) If group contains ghosts and memberuid, resolve memberuid I hope we can get rid of memberuid at some point. It was added as a performance improvement at a time where we didn't had the memcache and all request were handled by SSSD directly. Maybe this can be done together with the planned sysdb changes which will store all entries with a fully-qualified name. 2) For each ghost user either: a) Ignore b) Put the name in c) Try to find overrideObjectDN and apply possible name override, then put the name in I think b) should not be considered because it might return wrong results. The other two options are valid. Is it possible to have unresolved user but its override values stored in sssd cache with IPA views? no, the override data is always written to the cache after the user object. Then I think it will be best to keep it as is for IPA views, but resolve memberuid and put ghost in (1 and 2b) since it is not possible to have override object and unresolved user at the same time with local view. Do you agree? bye, Sumit HTH bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set
On Thu, Sep 17, 2015 at 12:44:37PM +0200, Pavel Březina wrote: > On 09/17/2015 12:37 PM, Sumit Bose wrote: > >On Thu, Sep 17, 2015 at 12:11:17PM +0200, Pavel Březina wrote: > >>https://fedorahosted.org/sssd/ticket/2790 > >> > >>This is just a partial fix. > >> > >>If a group contains member which doesn't have overrideDN specified, we fail > >>to resolve membership with LOCAL views. > >> > >>There is a bigger issue when group contains ghost members, we do not apply > >>overrides and just ignore the group. As code says, this is expected. Sumit, > >>do you remember why it is this way? > > > >If there is an IPA idview applied to a client all objects have to be > >completely resolved, i.e. the object must have been read from the server > >and it must have been checked if there is an override for the object, to > >return the right result to the caller. > > > >Since ghost entries are not resolved completely ENOENT is return to > >indicate to the caller to call the backend to resolve all needed > >objects completely, > > > >> > >>sysdb_getgrgid_with_views: > >>>if (el != NULL && el->num_values != 0) { > >>>DEBUG(SSSDBG_TRACE_ALL, > >>> "Group object [%s], contains ghost entries which must be > >>> " \ > >>> "resolved before overrides can be applied.\n", > >>> ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); > >>>ret = ENOENT; > >>>goto done; > >>>} > >> > >>As I see it, we can do: > >>1) If group contains ghosts and memberuid, resolve memberuid > > > >I hope we can get rid of memberuid at some point. It was added as a > >performance improvement at a time where we didn't had the memcache and > >all request were handled by SSSD directly. Maybe this can be done > >together with the planned sysdb changes which will store all entries > >with a fully-qualified name. > > > >>2) For each ghost user either: > >> a) Ignore > >> b) Put the name in > >> c) Try to find overrideObjectDN and apply possible name override, then > >> put > >>the name in > > > >I think b) should not be considered because it might return wrong > >results. The other two options are valid. > > Is it possible to have unresolved user but its override values stored in > sssd cache with IPA views? no, the override data is always written to the cache after the user object. bye, Sumit > > > > >HTH > > > >bye, > >Sumit > > > ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set
On 09/17/2015 01:26 PM, Pavel Březina wrote: On 09/17/2015 01:21 PM, Sumit Bose wrote: On Thu, Sep 17, 2015 at 12:44:37PM +0200, Pavel Březina wrote: On 09/17/2015 12:37 PM, Sumit Bose wrote: On Thu, Sep 17, 2015 at 12:11:17PM +0200, Pavel Březina wrote: https://fedorahosted.org/sssd/ticket/2790 This is just a partial fix. If a group contains member which doesn't have overrideDN specified, we fail to resolve membership with LOCAL views. There is a bigger issue when group contains ghost members, we do not apply overrides and just ignore the group. As code says, this is expected. Sumit, do you remember why it is this way? If there is an IPA idview applied to a client all objects have to be completely resolved, i.e. the object must have been read from the server and it must have been checked if there is an override for the object, to return the right result to the caller. Since ghost entries are not resolved completely ENOENT is return to indicate to the caller to call the backend to resolve all needed objects completely, sysdb_getgrgid_with_views: if (el != NULL && el->num_values != 0) { DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost entries which must be " \ "resolved before overrides can be applied.\n", ldb_dn_get_linearized(orig_obj->msgs[0]->dn)); ret = ENOENT; goto done; } As I see it, we can do: 1) If group contains ghosts and memberuid, resolve memberuid I hope we can get rid of memberuid at some point. It was added as a performance improvement at a time where we didn't had the memcache and all request were handled by SSSD directly. Maybe this can be done together with the planned sysdb changes which will store all entries with a fully-qualified name. 2) For each ghost user either: a) Ignore b) Put the name in c) Try to find overrideObjectDN and apply possible name override, then put the name in I think b) should not be considered because it might return wrong results. The other two options are valid. Is it possible to have unresolved user but its override values stored in sssd cache with IPA views? no, the override data is always written to the cache after the user object. Then I think it will be best to keep it as is for IPA views, but resolve memberuid and put ghost in (1 and 2b) since it is not possible to have override object and unresolved user at the same time with local view. Do you agree? As per meeting discussion I went ahead and wrote the patch. >From ce02f1cd128cb69443f79a03c7d8749c24b28ed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=Date: Thu, 17 Sep 2015 11:34:40 +0200 Subject: [PATCH 1/3] views: do not require overrideDN in grous when LOCAL view is set Resolves: https://fedorahosted.org/sssd/ticket/2790 --- src/db/sysdb_views.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/db/sysdb_views.c b/src/db/sysdb_views.c index 1db6c892de9e4764b673608166830800744b1148..82b1f187dbc6aab032403854714474301ae3324c 100644 --- a/src/db/sysdb_views.c +++ b/src/db/sysdb_views.c @@ -1337,6 +1337,12 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, override_dn_str = ldb_msg_find_attr_as_string(member_obj->msgs[0], SYSDB_OVERRIDE_DN, NULL); if (override_dn_str == NULL) { +if (is_local_view(domain->view_name)) { +/* LOCAL view doesn't have to have overrideDN specified. */ +ret = EOK; +goto done; +} + DEBUG(SSSDBG_CRIT_FAILURE, "Missing override DN for objext [%s].\n", ldb_dn_get_linearized(member_obj->msgs[0]->dn)); -- 1.9.3 >From 6f982918a6b87aa5190aa3d0d0d5bca7a8214abc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Thu, 17 Sep 2015 11:35:56 +0200 Subject: [PATCH 2/3] views: fix two typos in debug messages --- src/db/sysdb_views.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/db/sysdb_views.c b/src/db/sysdb_views.c index 82b1f187dbc6aab032403854714474301ae3324c..f0459fc174b94d7a735b7c416555eb5aaac42b7c 100644 --- a/src/db/sysdb_views.c +++ b/src/db/sysdb_views.c @@ -1193,7 +1193,7 @@ errno_t sysdb_add_overrides_to_object(struct sss_domain_info *domain, } DEBUG(SSSDBG_CRIT_FAILURE, - "Missing override DN for objext [%s].\n", + "Missing override DN for object [%s].\n", ldb_dn_get_linearized(obj->dn)); ret = ENOENT; @@ -1344,7 +1344,7 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain, } DEBUG(SSSDBG_CRIT_FAILURE, - "Missing override DN for objext [%s].\n", + "Missing override DN for object [%s].\n",