Re: [SSSD] [PATCH] views: do not require overrideDN in groups when LOCAL view is set

2015-09-18 Thread Sumit Bose
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

2015-09-18 Thread Jakub Hrozek
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

2015-09-17 Thread Sumit Bose
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

2015-09-17 Thread Pavel Březina

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

2015-09-17 Thread Pavel Březina

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

2015-09-17 Thread Sumit Bose
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

2015-09-17 Thread Pavel Březina

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",