[PATCH] ldap_opts: Get rid on 389ds specific values in rfc2307bis schema

[Lukas Slebodnik]

ehlo,

There is problem with OpenLDAP server and dereferencing of attributes
that is not in the schema  of the server?

sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b 'dc=example,dc=com' \
                   -E 'deref=member:uid,dummy_attr' cn=ref_grp
Protocol error (2)
Additional information: Dereference control: attribute decoding error
sh-4.2$ echo $?
2

The attribute nsUniqueID is a 389-only, non-standard attribute.
It is an operational attribute that is not in the rfc2307bis nor inetOrgPerson
nor posixAccount schema.  OpenLDAP supports the standard entryUUID attribute,
which is basically the same (uniquely identifies an entry throughout
a replication topology), but uses the standard UUID format rather than
the non-standard format used by 389.

4x FIXME removed :-)

Any comments are welcomed.

LS

[Jakub Hrozek]

On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik wrote:
> ehlo,
>
> There is problem with OpenLDAP server and dereferencing of attributes
> that is not in the schema  of the server?
>
> sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b 'dc=example,dc=com' \
>                    -E 'deref=member:uid,dummy_attr' cn=ref_grp
> Protocol error (2)
> Additional information: Dereference control: attribute decoding error
> sh-4.2$ echo $?
> 2
>
> The attribute nsUniqueID is a 389-only, non-standard attribute.
> It is an operational attribute that is not in the rfc2307bis nor inetOrgPerson
> nor posixAccount schema.  OpenLDAP supports the standard entryUUID attribute,
> which is basically the same (uniquely identifies an entry throughout
> a replication topology), but uses the standard UUID format rather than
> the non-standard format used by 389.
>
> 4x FIXME removed :-)
>
> Any comments are welcomed.
>
> LS

Thanks for the detective work on finding the root cause of the problem!

I wonder if we could remove the attribute completely, though. It appears
to be completely unused now:

$ git grep -l SYSDB_UUID
src/db/sysdb.h:#define SYSDB_UUID "uniqueID"
src/providers/ad/ad_opts.h:    { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL },
src/providers/ad/ad_opts.h:    { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL },
src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid", "nsUniqueId", SYSDB_UUID, NULL },
src/providers/ipa/ipa_opts.h:    { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL },
src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid", "nsUniqueId", SYSDB_UUID, NULL },
src/providers/ipa/ipa_opts.h:    { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL },
src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
src/providers/ipa/ipa_opts.h:    { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL, SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", NULL, SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", "nsUniqueId", SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid", "nsUniqueId", SYSDB_UUID, NULL },

And according to "git log -S" it was actually never used.

So my proposal is to just remove the attribute along with SYSDB_UUID.

[Lukas Slebodnik]

On (18/07/14 16:34), Jakub Hrozek wrote:
> On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik wrote:
> > ehlo,
> >
> > There is problem with OpenLDAP server and dereferencing of attributes
> > that is not in the schema  of the server?
> >
> > sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b 'dc=example,dc=com' \
> >                    -E 'deref=member:uid,dummy_attr' cn=ref_grp
> > Protocol error (2)
> > Additional information: Dereference control: attribute decoding error
> > sh-4.2$ echo $?
> > 2
> >
> > The attribute nsUniqueID is a 389-only, non-standard attribute.
> > It is an operational attribute that is not in the rfc2307bis nor inetOrgPerson
> > nor posixAccount schema.  OpenLDAP supports the standard entryUUID attribute,
> > which is basically the same (uniquely identifies an entry throughout
> > a replication topology), but uses the standard UUID format rather than
> > the non-standard format used by 389.
> >
> > 4x FIXME removed :-)
> >
> > Any comments are welcomed.
> >
> > LS
>
> Thanks for the detective work on finding the root cause of the problem!
>
> I wonder if we could remove the attribute completely, though. It appears
> to be completely unused now:
>
> $ git grep -l SYSDB_UUID
> src/db/sysdb.h:#define SYSDB_UUID "uniqueID"
> src/providers/ad/ad_opts.h:    { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL },
> src/providers/ad/ad_opts.h:    { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL },
> src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> src/providers/ipa/ipa_opts.h:    { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> src/providers/ipa/ipa_opts.h:    { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL },
> src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
> src/providers/ipa/ipa_opts.h:    { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
> src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
> src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL, SYSDB_UUID, NULL },
> src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", NULL, SYSDB_UUID, NULL },
> src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL },
> src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL },
> src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid", "nsUniqueId", SYSDB_UUID, NULL },
>
> And according to "git log -S" it was actually never used.
>
> So my proposal is to just remove the attribute along with SYSDB_UUID.

I was thinking about this change but I thought that we need to deprecate
options before removing. I don't have a problem with removing them.
and it should not be a problem in this case.
I would like to know opinion of older developers (Sumit, Stephen, ...)
Why were these options introduced? Do they help with ldap search?

LS

[Jakub Hrozek]

On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> On (18/07/14 16:34), Jakub Hrozek wrote:
> > On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > There is problem with OpenLDAP server and dereferencing of attributes
> > > that is not in the schema  of the server?
> > >
> > > sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b 'dc=example,dc=com' \
> > >                    -E 'deref=member:uid,dummy_attr' cn=ref_grp
> > > Protocol error (2)
> > > Additional information: Dereference control: attribute decoding error
> > > sh-4.2$ echo $?
> > > 2
> > >
> > > The attribute nsUniqueID is a 389-only, non-standard attribute.
> > > It is an operational attribute that is not in the rfc2307bis nor inetOrgPerson
> > > nor posixAccount schema.  OpenLDAP supports the standard entryUUID attribute,
> > > which is basically the same (uniquely identifies an entry throughout
> > > a replication topology), but uses the standard UUID format rather than
> > > the non-standard format used by 389.
> > >
> > > 4x FIXME removed :-)
> > >
> > > Any comments are welcomed.
> > >
> > > LS
> >
> > Thanks for the detective work on finding the root cause of the problem!
> >
> > I wonder if we could remove the attribute completely, though. It appears
> > to be completely unused now:
> >
> > $ git grep -l SYSDB_UUID
> > src/db/sysdb.h:#define SYSDB_UUID "uniqueID"
> > src/providers/ad/ad_opts.h:    { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL },
> > src/providers/ad/ad_opts.h:    { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL },
> > src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> > src/providers/ipa/ipa_opts.h:    { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> > src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> > src/providers/ipa/ipa_opts.h:    { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL },
> > src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
> > src/providers/ipa/ipa_opts.h:    { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
> > src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid", "ipaUniqueID", SYSDB_UUID, NULL},
> > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL, SYSDB_UUID, NULL },
> > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", NULL, SYSDB_UUID, NULL },
> > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL },
> > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL },
> > src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid", "nsUniqueId", SYSDB_UUID, NULL },
> >
> > And according to "git log -S" it was actually never used.
> >
> > So my proposal is to just remove the attribute along with SYSDB_UUID.
>
> I was thinking about this change but I thought that we need to deprecate
> options before removing. I don't have a problem with removing them.
> and it should not be a problem in this case.
> I would like to know opinion of older developers (Sumit, Stephen, ...)
> Why were these options introduced? Do they help with ldap search?
>
> LS

Sure, being defensive when removing anything is a good idea. But since
the option doesn't do anything and never did, I think we're fine. You're
right that removing an option requires two acks, though. So far you have
mine :-)

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/20/2014 03:20 PM, Jakub Hrozek wrote:
> On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> > On (18/07/14 16:34), Jakub Hrozek wrote:
> > > On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik
> > > wrote:
> > > > ehlo,
> > > >
> > > > There is problem with OpenLDAP server and dereferencing of
> > > > attributes that is not in the schema  of the server?
> > > >
> > > > sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b
> > > > 'dc=example,dc=com' \ -E 'deref=member:uid,dummy_attr'
> > > > cn=ref_grp Protocol error (2) Additional information:
> > > > Dereference control: attribute decoding error sh-4.2$ echo
> > > > $? 2
> > > >
> > > > The attribute nsUniqueID is a 389-only, non-standard
> > > > attribute. It is an operational attribute that is not in the
> > > > rfc2307bis nor inetOrgPerson nor posixAccount schema.
> > > > OpenLDAP supports the standard entryUUID attribute, which is
> > > > basically the same (uniquely identifies an entry throughout a
> > > > replication topology), but uses the standard UUID format
> > > > rather than the non-standard format used by 389.
> > > >
> > > > 4x FIXME removed :-)
> > > >
> > > > Any comments are welcomed.
> > > >
> > > > LS
> > >
> > > Thanks for the detective work on finding the root cause of the
> > > problem!
> > >
> > > I wonder if we could remove the attribute completely, though.
> > > It appears to be completely unused now:
> > >
> > > $ git grep -l SYSDB_UUID src/db/sysdb.h:#define SYSDB_UUID
> > > "uniqueID" src/providers/ad/ad_opts.h:    { "ldap_user_uuid",
> > > "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.h:
> > > { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, 
> > > src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid",
> > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL }, 
> > > src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid",
> > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, 
> > > src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid",
> > > "ipaUniqueID", SYSDB_UUID, NULL}, src/providers/ipa/ipa_opts.h:
> > > { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid",
> > > "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL,
> > > SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.h:    {
> > > "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, 
> > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > "objectGUID", SYSDB_UUID, NULL }, 
> > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > "objectGUID", SYSDB_UUID, NULL }, 
> > > src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid",
> > > "nsUniqueId", SYSDB_UUID, NULL },
> > >
> > > And according to "git log -S" it was actually never used.
> > >
> > > So my proposal is to just remove the attribute along with
> > > SYSDB_UUID.
> >
> > I was thinking about this change but I thought that we need to
> > deprecate options before removing. I don't have a problem with
> > removing them. and it should not be a problem in this case. I
> > would like to know opinion of older developers (Sumit, Stephen,
> > ...) Why were these options introduced? Do they help with ldap
> > search?
> >
> > LS
>
> Sure, being defensive when removing anything is a good idea. But
> since the option doesn't do anything and never did, I think we're
> fine. You're right that removing an option requires two acks,
> though. So far you have mine :-)

I'll ack it as well. I suspect that this was planned for a future use
that we never ended up with. I don't remember any reason for it.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlPNLUkACgkQeiVVYja6o6MqVwCfXP/RfVbEL3ojKjBdEw3JLvXv
> NeAAn10YBMy2Q2J75KQxDS7JOSP+VnJA
> =2Gtf
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

On Mon, Jul 21, 2014 at 11:10:02AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/20/2014 03:20 PM, Jakub Hrozek wrote:
> > On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> > > On (18/07/14 16:34), Jakub Hrozek wrote:
> > > > On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik
> > > > wrote:
> > > > > ehlo,
> > > > >
> > > > > There is problem with OpenLDAP server and dereferencing of
> > > > > attributes that is not in the schema  of the server?
> > > > >
> > > > > sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b
> > > > > 'dc=example,dc=com' \ -E 'deref=member:uid,dummy_attr'
> > > > > cn=ref_grp Protocol error (2) Additional information:
> > > > > Dereference control: attribute decoding error sh-4.2$ echo
> > > > > $? 2
> > > > >
> > > > > The attribute nsUniqueID is a 389-only, non-standard
> > > > > attribute. It is an operational attribute that is not in the
> > > > > rfc2307bis nor inetOrgPerson nor posixAccount schema.
> > > > > OpenLDAP supports the standard entryUUID attribute, which is
> > > > > basically the same (uniquely identifies an entry throughout a
> > > > > replication topology), but uses the standard UUID format
> > > > > rather than the non-standard format used by 389.
> > > > >
> > > > > 4x FIXME removed :-)
> > > > >
> > > > > Any comments are welcomed.
> > > > >
> > > > > LS
> > > >
> > > > Thanks for the detective work on finding the root cause of the
> > > > problem!
> > > >
> > > > I wonder if we could remove the attribute completely, though.
> > > > It appears to be completely unused now:
> > > >
> > > > $ git grep -l SYSDB_UUID src/db/sysdb.h:#define SYSDB_UUID
> > > > "uniqueID" src/providers/ad/ad_opts.h:    { "ldap_user_uuid",
> > > > "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.h:
> > > > { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, 
> > > > src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, 
> > > > src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid",
> > > > "ipaUniqueID", SYSDB_UUID, NULL}, src/providers/ipa/ipa_opts.h:
> > > > { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid",
> > > > "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL,
> > > > SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.h:    {
> > > > "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL },
> > > >
> > > > And according to "git log -S" it was actually never used.
> > > >
> > > > So my proposal is to just remove the attribute along with
> > > > SYSDB_UUID.
> > >
> > > I was thinking about this change but I thought that we need to
> > > deprecate options before removing. I don't have a problem with
> > > removing them. and it should not be a problem in this case. I
> > > would like to know opinion of older developers (Sumit, Stephen,
> > > ...) Why were these options introduced? Do they help with ldap
> > > search?
> > >
> > > LS
> >
> > Sure, being defensive when removing anything is a good idea. But
> > since the option doesn't do anything and never did, I think we're
> > fine. You're right that removing an option requires two acks,
> > though. So far you have mine :-)
>
> I'll ack it as well. I suspect that this was planned for a future use
> that we never ended up with. I don't remember any reason for it.

Thanks for the second pair of eyes!

[Lukas Slebodnik]

On (21/07/14 11:10), Stephen Gallagher wrote:
> On 07/20/2014 03:20 PM, Jakub Hrozek wrote:
> > On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> > > On (18/07/14 16:34), Jakub Hrozek wrote:
> > > > On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik
> > > > wrote:
> > > > > ehlo,
> > > > >
> > > > > There is problem with OpenLDAP server and dereferencing of
> > > > > attributes that is not in the schema  of the server?
> > > > >
> > > > > sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b
> > > > > 'dc=example,dc=com' \ -E 'deref=member:uid,dummy_attr'
> > > > > cn=ref_grp Protocol error (2) Additional information:
> > > > > Dereference control: attribute decoding error sh-4.2$ echo
> > > > > $? 2
> > > > >
> > > > > The attribute nsUniqueID is a 389-only, non-standard
> > > > > attribute. It is an operational attribute that is not in the
> > > > > rfc2307bis nor inetOrgPerson nor posixAccount schema.
> > > > > OpenLDAP supports the standard entryUUID attribute, which is
> > > > > basically the same (uniquely identifies an entry throughout a
> > > > > replication topology), but uses the standard UUID format
> > > > > rather than the non-standard format used by 389.
> > > > >
> > > > > 4x FIXME removed :-)
> > > > >
> > > > > Any comments are welcomed.
> > > > >
> > > > > LS
> > > >
> > > > Thanks for the detective work on finding the root cause of the
> > > > problem!
> > > >
> > > > I wonder if we could remove the attribute completely, though.
> > > > It appears to be completely unused now:
> > > >
> > > > $ git grep -l SYSDB_UUID src/db/sysdb.h:#define SYSDB_UUID
> > > > "uniqueID" src/providers/ad/ad_opts.h:    { "ldap_user_uuid",
> > > > "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.h:
> > > > { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, 
> > > > src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, 
> > > > src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid",
> > > > "ipaUniqueID", SYSDB_UUID, NULL}, src/providers/ipa/ipa_opts.h:
> > > > { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid",
> > > > "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL,
> > > > SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.h:    {
> > > > "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid",
> > > > "nsUniqueId", SYSDB_UUID, NULL },
> > > >
> > > > And according to "git log -S" it was actually never used.
> > > >
> > > > So my proposal is to just remove the attribute along with
> > > > SYSDB_UUID.
> > >
> > > I was thinking about this change but I thought that we need to
> > > deprecate options before removing. I don't have a problem with
> > > removing them. and it should not be a problem in this case. I
> > > would like to know opinion of older developers (Sumit, Stephen,
> > > ...) Why were these options introduced? Do they help with ldap
> > > search?
> > >
> > > LS
> >
> > Sure, being defensive when removing anything is a good idea. But
> > since the option doesn't do anything and never did, I think we're
> > fine. You're right that removing an option requires two acks,
> > though. So far you have mine :-)
>
> I'll ack it as well. I suspect that this was planned for a future use
> that we never ended up with. I don't remember any reason for it.

I prepared separate patches for different *_uuid options.
I thought it would be simple change. There is small complication with
IPA_UNIQUE_ID. I am not sure why it was used for storing HBAC rules.
(0005-Remove-macro-IPA_UNIQUE_ID.patch). HBAC works with 5th patch.
I hope I tested all possible ways. If we agree with this version it can be
squashed to 4th patch (0004-LDAP-Remove-unused-option-ipa_hostgroup_uuid)

Any comments are welcomed.

LS

[Jakub Hrozek]

On Tue, Jul 22, 2014 at 01:28:17PM +0200, Lukas Slebodnik wrote:
> On (21/07/14 11:10), Stephen Gallagher wrote:
> > On 07/20/2014 03:20 PM, Jakub Hrozek wrote:
> > > On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> > > > On (18/07/14 16:34), Jakub Hrozek wrote:
> > > > > On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik
> > > > > wrote:
> > > > > > ehlo,
> > > > > >
> > > > > > There is problem with OpenLDAP server and dereferencing of
> > > > > > attributes that is not in the schema  of the server?
> > > > > >
> > > > > > sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b
> > > > > > 'dc=example,dc=com' \ -E 'deref=member:uid,dummy_attr'
> > > > > > cn=ref_grp Protocol error (2) Additional information:
> > > > > > Dereference control: attribute decoding error sh-4.2$ echo
> > > > > > $? 2
> > > > > >
> > > > > > The attribute nsUniqueID is a 389-only, non-standard
> > > > > > attribute. It is an operational attribute that is not in the
> > > > > > rfc2307bis nor inetOrgPerson nor posixAccount schema.
> > > > > > OpenLDAP supports the standard entryUUID attribute, which is
> > > > > > basically the same (uniquely identifies an entry throughout a
> > > > > > replication topology), but uses the standard UUID format
> > > > > > rather than the non-standard format used by 389.
> > > > > >
> > > > > > 4x FIXME removed :-)
> > > > > >
> > > > > > Any comments are welcomed.
> > > > > >
> > > > > > LS
> > > > >
> > > > > Thanks for the detective work on finding the root cause of the
> > > > > problem!
> > > > >
> > > > > I wonder if we could remove the attribute completely, though.
> > > > > It appears to be completely unused now:
> > > > >
> > > > > $ git grep -l SYSDB_UUID src/db/sysdb.h:#define SYSDB_UUID
> > > > > "uniqueID" src/providers/ad/ad_opts.h:    { "ldap_user_uuid",
> > > > > "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.h:
> > > > > { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, 
> > > > > src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid",
> > > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > > { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid",
> > > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > > { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, 
> > > > > src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid",
> > > > > "ipaUniqueID", SYSDB_UUID, NULL}, src/providers/ipa/ipa_opts.h:
> > > > > { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > > src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid",
> > > > > "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL,
> > > > > SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.h:    {
> > > > > "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, 
> > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > > src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid",
> > > > > "nsUniqueId", SYSDB_UUID, NULL },
> > > > >
> > > > > And according to "git log -S" it was actually never used.
> > > > >
> > > > > So my proposal is to just remove the attribute along with
> > > > > SYSDB_UUID.
> > > >
> > > > I was thinking about this change but I thought that we need to
> > > > deprecate options before removing. I don't have a problem with
> > > > removing them. and it should not be a problem in this case. I
> > > > would like to know opinion of older developers (Sumit, Stephen,
> > > > ...) Why were these options introduced? Do they help with ldap
> > > > search?
> > > >
> > > > LS
> > >
> > > Sure, being defensive when removing anything is a good idea. But
> > > since the option doesn't do anything and never did, I think we're
> > > fine. You're right that removing an option requires two acks,
> > > though. So far you have mine :-)
> >
> > I'll ack it as well. I suspect that this was planned for a future use
> > that we never ended up with. I don't remember any reason for it.
>
> I prepared separate patches for different *_uuid options.
> I thought it would be simple change. There is small complication with
> IPA_UNIQUE_ID. I am not sure why it was used for storing HBAC rules.
> (0005-Remove-macro-IPA_UNIQUE_ID.patch). HBAC works with 5th patch.
> I hope I tested all possible ways. If we agree with this version it can be
> squashed to 4th patch (0004-LDAP-Remove-unused-option-ipa_hostgroup_uuid)

I can't see a reason either. The only benefit of storing the uniqueID,
as far as I can see, is to be able to match objects with their server
side counterparts. I also tested some HBAC combinations, even offline
auth with cache using the old RDN, but with patched code -- everything
worked fine, because the code doesn't care about either the DN format or
the presence of UUID. We don't need to upgrade sysdb either.

I wonder if, with some very old IPA versions, the rules could have no
name? The following code in hbac_attrs_to_rule() seems to suggest it:

274     ret = sysdb_attrs_get_el(hbac_ctx->rules[idx],
275                              IPA_CN, &el);
276     if (ret != EOK || el->num_values == 0) {
277         DEBUG(SSSDBG_CONF_SETTINGS, "rule has no name, assuming '(none)'.\n");
278         new_rule->name = talloc_strdup(new_rule, "(none)");
279     } else {
280         new_rule->name = talloc_strndup(new_rule,
281                                         (const char*) el->values[0].data,
282                                         el->values[0].length);
283     }
284

Your patches make 'cn' mandatory because it's used as an RDN value,
so additional change to patch 5/7 should also change the code above to
require CN.  BTW I also tested that IPA wouldn't let you define two rules
with the same CN..

So I think we should push these patches to master, if we can't justify
the presence of some code, it should be removed..but at the same time,
sssd-1-11 should only contain minimal and safe changes at this point.

Can you also prepare patches #1, #2 so that they apply on sssd-1-11 and
send a version of #3 that only removes the LDAP part?

[Lukas Slebodnik]

On (25/07/14 10:40), Jakub Hrozek wrote:
> On Tue, Jul 22, 2014 at 01:28:17PM +0200, Lukas Slebodnik wrote:
> > On (21/07/14 11:10), Stephen Gallagher wrote:
> > > On 07/20/2014 03:20 PM, Jakub Hrozek wrote:
> > > > On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> > > > > On (18/07/14 16:34), Jakub Hrozek wrote:
> > > > > > On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik
> > > > > > wrote:
> > > > > > > ehlo,
> > > > > > > 
> > > > > > > There is problem with OpenLDAP server and dereferencing of
> > > > > > > attributes that is not in the schema  of the server?
> > > > > > > 
> > > > > > > sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b
> > > > > > > 'dc=example,dc=com' \ -E 'deref=member:uid,dummy_attr'
> > > > > > > cn=ref_grp Protocol error (2) Additional information:
> > > > > > > Dereference control: attribute decoding error sh-4.2$ echo
> > > > > > > $? 2
> > > > > > > 
> > > > > > > The attribute nsUniqueID is a 389-only, non-standard
> > > > > > > attribute. It is an operational attribute that is not in the
> > > > > > > rfc2307bis nor inetOrgPerson nor posixAccount schema.
> > > > > > > OpenLDAP supports the standard entryUUID attribute, which is
> > > > > > > basically the same (uniquely identifies an entry throughout a
> > > > > > > replication topology), but uses the standard UUID format
> > > > > > > rather than the non-standard format used by 389.
> > > > > > > 
> > > > > > > 4x FIXME removed :-)
> > > > > > > 
> > > > > > > 
> > > > > > > Any comments are welcomed.
> > > > > > > 
> > > > > > > LS
> > > > > >
> > > > > > Thanks for the detective work on finding the root cause of the
> > > > > > problem!
> > > > > >
> > > > > > I wonder if we could remove the attribute completely, though.
> > > > > > It appears to be completely unused now:
> > > > > >
> > > > > > $ git grep -l SYSDB_UUID src/db/sysdb.h:#define SYSDB_UUID
> > > > > > "uniqueID" src/providers/ad/ad_opts.h:    { "ldap_user_uuid",
> > > > > > "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.h:
> > > > > > { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, 
> > > > > > src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid",
> > > > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > > > { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > > src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid",
> > > > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > > > { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, 
> > > > > > src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid",
> > > > > > "ipaUniqueID", SYSDB_UUID, NULL}, src/providers/ipa/ipa_opts.h:
> > > > > > { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > > > src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid",
> > > > > > "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL,
> > > > > > SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.h:    {
> > > > > > "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, 
> > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid",
> > > > > > "nsUniqueId", SYSDB_UUID, NULL },
> > > > > >
> > > > > > And according to "git log -S" it was actually never used.
> > > > > >
> > > > > > So my proposal is to just remove the attribute along with
> > > > > > SYSDB_UUID.
> > > > >
> > > > > I was thinking about this change but I thought that we need to
> > > > > deprecate options before removing. I don't have a problem with
> > > > > removing them. and it should not be a problem in this case. I
> > > > > would like to know opinion of older developers (Sumit, Stephen,
> > > > > ...) Why were these options introduced? Do they help with ldap
> > > > > search?
> > > > >
> > > > > LS
> > > >
> > > > Sure, being defensive when removing anything is a good idea. But
> > > > since the option doesn't do anything and never did, I think we're
> > > > fine. You're right that removing an option requires two acks,
> > > > though. So far you have mine :-)
> > >
> > > I'll ack it as well. I suspect that this was planned for a future use
> > > that we never ended up with. I don't remember any reason for it.
> >
> > I prepared separate patches for different *_uuid options.
> > I thought it would be simple change. There is small complication with
> > IPA_UNIQUE_ID. I am not sure why it was used for storing HBAC rules.
> > (0005-Remove-macro-IPA_UNIQUE_ID.patch). HBAC works with 5th patch.
> > I hope I tested all possible ways. If we agree with this version it can be
> > squashed to 4th patch (0004-LDAP-Remove-unused-option-ipa_hostgroup_uuid)
>
> I can't see a reason either. The only benefit of storing the uniqueID,
> as far as I can see, is to be able to match objects with their server
> side counterparts. I also tested some HBAC combinations, even offline
> auth with cache using the old RDN, but with patched code -- everything
> worked fine, because the code doesn't care about either the DN format or
> the presence of UUID. We don't need to upgrade sysdb either.
>
> I wonder if, with some very old IPA versions, the rules could have no
> name? The following code in hbac_attrs_to_rule() seems to suggest it:
>
> 274     ret = sysdb_attrs_get_el(hbac_ctx->rules[idx],
> 275                              IPA_CN, &el);
> 276     if (ret != EOK || el->num_values == 0) {
> 277         DEBUG(SSSDBG_CONF_SETTINGS, "rule has no name, assuming '(none)'.\n");
> 278         new_rule->name = talloc_strdup(new_rule, "(none)");
> 279     } else {
> 280         new_rule->name = talloc_strndup(new_rule,
> 281                                         (const char*) el->values[0].data,
> 282                                         el->values[0].length);
> 283     }
> 284
>
> Your patches make 'cn' mandatory because it's used as an RDN value,
> so additional change to patch 5/7 should also change the code above to
> require CN.  BTW I also tested that IPA wouldn't let you define two rules
> with the same CN..
>
> So I think we should push these patches to master, if we can't justify
> the presence of some code, it should be removed..but at the same time,
> sssd-1-11 should only contain minimal and safe changes at this point.
>
> Can you also prepare patches #1, #2 so that they apply on sssd-1-11 and
> send a version of #3 that only removes the LDAP part?

Attached patches remove only options ldap_*_uuid. They can be applied on master
or sssd-1-11.

Now, you should decide which patch set you want to push to which branch :-)

LS

[Jakub Hrozek]

On Fri, Jul 25, 2014 at 02:33:24PM +0200, Lukas Slebodnik wrote:
> On (25/07/14 10:40), Jakub Hrozek wrote:
> > On Tue, Jul 22, 2014 at 01:28:17PM +0200, Lukas Slebodnik wrote:
> > > On (21/07/14 11:10), Stephen Gallagher wrote:
> > > > On 07/20/2014 03:20 PM, Jakub Hrozek wrote:
> > > > > On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> > > > > > On (18/07/14 16:34), Jakub Hrozek wrote:
> > > > > > > On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik
> > > > > > > wrote:
> > > > > > >> ehlo,
> > > > > > >> 
> > > > > > >> There is problem with OpenLDAP server and dereferencing of
> > > > > > >> attributes that is not in the schema  of the server?
> > > > > > >> 
> > > > > > >> sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b
> > > > > > >> 'dc=example,dc=com' \ -E 'deref=member:uid,dummy_attr'
> > > > > > >> cn=ref_grp Protocol error (2) Additional information:
> > > > > > >> Dereference control: attribute decoding error sh-4.2$ echo
> > > > > > >> $? 2
> > > > > > >> 
> > > > > > >> The attribute nsUniqueID is a 389-only, non-standard
> > > > > > >> attribute. It is an operational attribute that is not in the
> > > > > > >> rfc2307bis nor inetOrgPerson nor posixAccount schema.
> > > > > > >> OpenLDAP supports the standard entryUUID attribute, which is
> > > > > > >> basically the same (uniquely identifies an entry throughout a
> > > > > > >> replication topology), but uses the standard UUID format
> > > > > > >> rather than the non-standard format used by 389.
> > > > > > >> 
> > > > > > >> 4x FIXME removed :-)
> > > > > > >> 
> > > > > > >> 
> > > > > > >> Any comments are welcomed.
> > > > > > >> 
> > > > > > >> LS
> > > > > > > 
> > > > > > > Thanks for the detective work on finding the root cause of the
> > > > > > > problem!
> > > > > > > 
> > > > > > > I wonder if we could remove the attribute completely, though.
> > > > > > > It appears to be completely unused now:
> > > > > > > 
> > > > > > > $ git grep -l SYSDB_UUID src/db/sysdb.h:#define SYSDB_UUID
> > > > > > > "uniqueID" src/providers/ad/ad_opts.h:    { "ldap_user_uuid",
> > > > > > > "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.h:
> > > > > > > { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid",
> > > > > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > > > > { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid",
> > > > > > > "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> > > > > > > { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid",
> > > > > > > "ipaUniqueID", SYSDB_UUID, NULL}, src/providers/ipa/ipa_opts.h:
> > > > > > > { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > > > > src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid",
> > > > > > > "ipaUniqueID", SYSDB_UUID, NULL}, 
> > > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL,
> > > > > > > SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.h:    {
> > > > > > > "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > > > > "nsUniqueId", SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> > > > > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> > > > > > > "objectGUID", SYSDB_UUID, NULL }, 
> > > > > > > src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid",
> > > > > > > "nsUniqueId", SYSDB_UUID, NULL },
> > > > > > > 
> > > > > > > And according to "git log -S" it was actually never used.
> > > > > > > 
> > > > > > > So my proposal is to just remove the attribute along with
> > > > > > > SYSDB_UUID.
> > > > > > I was thinking about this change but I thought that we need to
> > > > > > deprecate options before removing. I don't have a problem with
> > > > > > removing them. and it should not be a problem in this case. I
> > > > > > would like to know opinion of older developers (Sumit, Stephen,
> > > > > > ...) Why were these options introduced? Do they help with ldap
> > > > > > search?
> > > > > >
> > > > > > LS
> > > > >
> > > > > Sure, being defensive when removing anything is a good idea. But
> > > > > since the option doesn't do anything and never did, I think we're
> > > > > fine. You're right that removing an option requires two acks,
> > > > > though. So far you have mine :-)
> > > >
> > > > I'll ack it as well. I suspect that this was planned for a future use
> > > > that we never ended up with. I don't remember any reason for it.
> > >
> > > I prepared separate patches for different *_uuid options.
> > > I thought it would be simple change. There is small complication with
> > > IPA_UNIQUE_ID. I am not sure why it was used for storing HBAC rules.
> > > (0005-Remove-macro-IPA_UNIQUE_ID.patch). HBAC works with 5th patch.
> > > I hope I tested all possible ways. If we agree with this version it can be
> > > squashed to 4th patch (0004-LDAP-Remove-unused-option-ipa_hostgroup_uuid)
> >
> > I can't see a reason either. The only benefit of storing the uniqueID,
> > as far as I can see, is to be able to match objects with their server
> > side counterparts. I also tested some HBAC combinations, even offline
> > auth with cache using the old RDN, but with patched code -- everything
> > worked fine, because the code doesn't care about either the DN format or
> > the presence of UUID. We don't need to upgrade sysdb either.
> >
> > I wonder if, with some very old IPA versions, the rules could have no
> > name? The following code in hbac_attrs_to_rule() seems to suggest it:
> >
> > 274     ret = sysdb_attrs_get_el(hbac_ctx->rules[idx],
> > 275                              IPA_CN, &el);
> > 276     if (ret != EOK || el->num_values == 0) {
> > 277         DEBUG(SSSDBG_CONF_SETTINGS, "rule has no name, assuming '(none)'.\n");
> > 278         new_rule->name = talloc_strdup(new_rule, "(none)");
> > 279     } else {
> > 280         new_rule->name = talloc_strndup(new_rule,
> > 281                                         (const char*) el->values[0].data,
> > 282                                         el->values[0].length);
> > 283     }
> > 284
> >
> > Your patches make 'cn' mandatory because it's used as an RDN value,
> > so additional change to patch 5/7 should also change the code above to
> > require CN.  BTW I also tested that IPA wouldn't let you define two rules
> > with the same CN..
> >
> > So I think we should push these patches to master, if we can't justify
> > the presence of some code, it should be removed..but at the same time,
> > sssd-1-11 should only contain minimal and safe changes at this point.
> >
> > Can you also prepare patches #1, #2 so that they apply on sssd-1-11 and
> > send a version of #3 that only removes the LDAP part?
>
> Attached patches remove only options ldap_*_uuid. They can be applied on master
> or sssd-1-11.
>
> Now, you should decide which patch set you want to push to which branch :-)
>
> LS

ACK to these three.

We can also push the IPA patches on top of these but it's OK to do it
after you're back from vacation, those patches do just cleanup, these
three fix a bug.

[Jakub Hrozek]

On Fri, Jul 25, 2014 at 02:55:51PM +0200, Jakub Hrozek wrote:
> ACK to these three.
>
> We can also push the IPA patches on top of these but it's OK to do it
> after you're back from vacation, those patches do just cleanup, these
> three fix a bug.

* master:
    dfb2960ab251f609466fa660449703835c97f99a
    b5242c146cc0ca96e2b898a74fb060efda15bc77
    87ff519b472568b19809963ca860d2182e874fcd
* sssd-1-11:
    842f9303f53b7d17214ed390203ed9e14203055a
    641585186fdd6401451ed06b971ec8e3cee4d610
    b7ce30e5889a42e64f1867db42400266ecaba9c9