Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions

2014-01-26 Thread Martin Kosek
On 01/24/2014 05:23 PM, Simo Sorce wrote:
 On Fri, 2014-01-24 at 17:17 +0100, Petr Viktorin wrote:
 On 01/24/2014 04:57 PM, Simo Sorce wrote:
 On Fri, 2014-01-24 at 16:48 +0100, Petr Viktorin wrote:
...
 Technically we could alias the name so the attribute can be called
 either way, but that is not necessarily a good option either.

 If breaking master is unacceptable, we can use the old name instead. 
 ipaPermIncludedAttr is more consistent but ipaPermAllowedAttr isn't 
 downright wrong.
 
 Ok, let's hear other opinions, I see a lot f value in consistent naming,
 and not breaking a developer build is not that strong of a reason to
 have substandard naming I guess. What do others think ?
 
 Simo.

Hmm, I obviously see things differently here. I would rather break the master
and let developers running on the git version to reinstall the servers
(including myself) than to have to live with suboptimal attribute name for ever
or by adding unnecessary cruft to the code...

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES] 225-230 Drop support for the legacy LDAP API

2014-01-26 Thread Jan Cholasta

On 24.1.2014 20:31, Petr Viktorin wrote:

On 01/22/2014 05:47 PM, Jan Cholasta wrote:

On 20.1.2014 12:23, Petr Viktorin wrote:

On 01/14/2014 11:31 AM, Jan Cholasta wrote:

On 10.1.2014 16:02, Petr Viktorin wrote:

On 01/07/2014 01:54 PM, Jan Cholasta wrote:

On 16.12.2013 14:45, Petr Viktorin wrote:

On 12/16/2013 10:22 AM, Jan Cholasta wrote:

On 13.12.2013 15:16, Petr Viktorin wrote:

On 12/10/2013 04:05 PM, Jan Cholasta wrote:

Hi,

I believe the time has come to drop support for the legacy (dn,
entry_attrs) tuple API and move to the new LDAPEntry API
exclusively.
The attached patches convert existing code which uses the old
API to
the
new API and remove backward compatibility code from the ipaldap
module.

Note that there are still some functions/methods which accept
separate
dn and entry_attrs arguments, they will be adapted to LDAPEntry
later.

Honza


The first N-1 patches can be tested,acked,pushed independently,
right?


Yes.


If that's the case, ACK for 225


Pushed that one to master, 5 more to go.
bc3f3381c6bf0b4941889b775025a60f56318551



226 needs a rebase.

227: in install/tools/ipa-adtrust-install:

+entry_attrs = conn.make_entry(
+dn,
+objectclass=['top', 'pkiuser', 'nscontainer'],
+usercertificate=cert)
+conn.add_entry(entry_attrs)

Shouldn't it be `usercertificate=[cert]` now?  Similarly in ra_cert,
and
in ipa-server-install with ipacertificatesubjectbase

Otherwise this looks good.

228: in ipaserver/install/plugins/update_idranges.py, again we should
use lists

Otherwise it looks good

229: ACK



Rebased and fixed everything, updated patches attached.


Here, patch 226 breaks tests for selinuxusermap_enable/disable, at
least. The EmptyModlist and AlreadyActive/AlreadyInactive error is no
longer raised, since the previous entry state is no longer retrieved.



Well, I forgot to test this patchset after patches for
https://fedorahosted.org/freeipa/ticket/3488 were pushed, sorry.

Added new patch 235 which makes LDAPUpdate get old entry state from
LDAP, it fixes most of the issues in *_mod commands.


If getting the entry is really always needed then I see no real reason
why it's not done at the beginning of execute() and made available
throughout the command's execution. It would be so much easier to do
non-trivial work in the callbacks. And more efficient, too, compared to
the current way of always asking LDAP when the original entry is needed.
Ah well. One day we can rewrite the whole thing :/


+1, I wish there was more time for this.


For now, ACK


Fixed the rest of the issues in patches 226-230 and rebased them on top
of patch 235.


ACK, pushed to master: 5737eaf1348ba101ae227fa79fb4451a2413fc84



Thanks for the review.

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel