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

[Pavel Březina]

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?

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

[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:
> 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.
> 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

[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:
> > 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.
> > 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

[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:
> > > 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.
> > > 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

[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:
> > > > 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.
> > > > 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

[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:
> > > > > 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.
> > > > > 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.

[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:
> > > > > > 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.
> > > > > > 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

[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