Re: [Freeipa-devel] [PATCH] 874 suppress managed netgroups as indirect members of hosts

2011-09-15 Thread Martin Kosek
On Wed, 2011-09-14 at 16:39 -0400, Rob Crittenden wrote:
 Suppress managed netgroups as indirect members of hosts. This enhances a 
 previous patch that I did for hostgroups.
 
 rob

This works fine. I just one suggestion for the code - the function
suppress_netgroup_memberof() function was already implemented in the
last patch:

https://fedorahosted.org/freeipa/changeset/ca1ca17cb61516dff6933b1b0381b32e1e38d44c

for hostgroup. I suggest making this function more general and calling
it from both host and hostgroup objects.

Martin


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


Re: [Freeipa-devel] [PATCH] 25 Create Tool for Enabling Disabling Managed Entry

2011-09-15 Thread Martin Kosek
On Thu, 2011-09-15 at 00:47 +, JR Aquino wrote:
 On Jul 22, 2011, at 7:05 AM, Martin Kosek wrote:

  5) I was thinking if there is a better solution to enabling/disabling of
  the plugin. Likes setting something like managedEntryEnabled attribute
  to on/off as we do with compat plugin. Current concept with disabling
  the definition by damaging the originFilter and then restoring it from
  an LDIF seems a bit awkward to me.
 
 This has been completely changed:
 Instead of looking to ldif files, an ldap look up is now performed to 
 dynamically list the available managed entries.

Now we are talking :-) I like the new approach.

I have reviewed your patch, basic functionality looks good. But I still
have few (nitpicking) comments:

1) There are parts from the previous file that are no longer needed
since you switched to different approach:

+import os

+from ipaserver.install.ldapupdate import LDAPUpdate, BadSyntax

+import StringIO

+import ldif

+except BadSyntax, e:
+print There is a syntax error in this update file:
+print   %s % e
+sys.exit(1)


2) I saw few whitespace errors on following lines of the patch: 419, 433
and 453

3) Output of the --list method is confusing:

# ipa-managed-entries --list
Directory Manager password: 

Available Managed Entry Plugins:
cn=upg definition,cn=definitions,cn=managed
entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
cn=ngp definition,cn=definitions,cn=managed
entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com

You must specify a managed entry definition 
# echo $?
1

a) I shouldn't be asked to specify a managed entry definition for --list
b) The listing was successful, so we shouldn't return error code

4) Return code for disabling an already disabled entry should be 2
(according to man pages):

# ipa-managed-entries -e 'cn=upg definition,cn=definitions,cn=managed 
entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com' disable
Directory Manager password: 

Plugin already disabled
# echo $?
0

5) Strange is, that enabling a disabled plugin gives me return code 2:
# ipa-managed-entries -e 'cn=upg definition,cn=definitions,cn=managed 
entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com' enable
Directory Manager password: 

Enabling Plugin
# echo $?
2

Return codes for these actions should fit the man pages.

6) I would improve working with LDAP filters, current solution is error
prone. Try disablingenabling NGP Defition, we end up with this
originFilter:

originfilter: ((objectclass=ipahostgroup))

I think the cleanest solution would be to use ldap.make_filter and
ldap.combine_filters functions to play with these filter. You can
inspire yourself in this example I wrote for DNS plugin:

rev_zone_filter = ldap.make_filter(search_kw, rules=ldap.MATCH_NONE, 
exact=False,
trailing_wildcard=False)
filter = ldap.combine_filters((rev_zone_filter, filter), rules=ldap.MATCH_ALL)

7) Entering Directory Manager every time may be a bit tedious. Could we
use current Kerberos credentials and fall-back to asking Directory
Manager password if it doesn't work? Its already done this way in
ipa-replica-manage for example.

We could fix this, however, as an enhancement in another patch.

8) Man page - please use the new united FreeIPA man page header. Instead
of 

+.TH ipa-managed-entries 1 Sept 15 2011 freeipa 

use:

+.TH ipa-managed-entries 1 Sept 15 2011 FreeIPA FreeIPA Manual
Pages


9) Man page - comma is missing for --list option:

+\fB\-l\-\-list\fR


10) install/po/Makefile.in should be updated to: there is still
reference to ipa-host-net-manage and ipa-managed-entries reference is
missing


Martin

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


Re: [Freeipa-devel] [PATCH] 874 suppress managed netgroups as indirect members of hosts

2011-09-15 Thread Rob Crittenden

Martin Kosek wrote:

On Wed, 2011-09-14 at 16:39 -0400, Rob Crittenden wrote:

Suppress managed netgroups as indirect members of hosts. This enhances a
previous patch that I did for hostgroups.

rob


This works fine. I just one suggestion for the code - the function
suppress_netgroup_memberof() function was already implemented in the
last patch:

https://fedorahosted.org/freeipa/changeset/ca1ca17cb61516dff6933b1b0381b32e1e38d44c

for hostgroup. I suggest making this function more general and calling
it from both host and hostgroup objects.

Martin




I looked at that. For the hostgroup once you find your own entry you can 
exit, for hosts you have to look at all netgroups. The dn comparison is 
also very different. These could be handled as arguments but I think the 
code would be less clear so I chose quasi-duplication.


rob

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


Re: [Freeipa-devel] [PATCH] 270 Fixed posix group checkbox.

2011-09-15 Thread Petr Vobornik

On 09/15/2011 02:06 AM, Endi Sukma Dewata wrote:

In the adder dialog for groups the checkbox has been modified to use
the correct field name nonposix and be checked by default.

Note: This is a temporary fix to minimize the changes due to release
schedule. Eventually the field label will be changed into Non-POSIX
group and the checkbox will be unchecked by default, which is more
consistent with CLI.

Ticket #1799


The temporary workaround approach is good but there might be a minor 
issue. One test for this patch fails. It is because current 
implementation sets checked attribute to false for all values except 
'TRUE'. Previous implementation set checked to true if there was any 
value except 'FALSE'. I tried to search for all usages of checkbox and 
found that this behaviour is not a problem right now, but I'm not sure 
if it won't be in the future (but there would have to be changes in save 
method). So if its OK, after a test correction I would consider it ACKed.



--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 25 Create Tool for Enabling Disabling Managed Entry

2011-09-15 Thread JR Aquino
On Sep 15, 2011, at 1:47 AM, Martin Kosek wrote:

 On Thu, 2011-09-15 at 00:47 +, JR Aquino wrote:
 On Jul 22, 2011, at 7:05 AM, Martin Kosek wrote:
 
 5) I was thinking if there is a better solution to enabling/disabling of
 the plugin. Likes setting something like managedEntryEnabled attribute
 to on/off as we do with compat plugin. Current concept with disabling
 the definition by damaging the originFilter and then restoring it from
 an LDIF seems a bit awkward to me.
 
 This has been completely changed:
 Instead of looking to ldif files, an ldap look up is now performed to 
 dynamically list the available managed entries.
 
 Now we are talking :-) I like the new approach.

high five

 
 I have reviewed your patch, basic functionality looks good. But I still
 have few (nitpicking) comments:
 
 1) There are parts from the previous file that are no longer needed
 since you switched to different approach:
 
 +import os
 
 +from ipaserver.install.ldapupdate import LDAPUpdate, BadSyntax
 
 +import StringIO
 
 +import ldif
 
 +except BadSyntax, e:
 +print There is a syntax error in this update file:
 +print   %s % e
 +sys.exit(1)

Removed

 
 2) I saw few whitespace errors on following lines of the patch: 419, 433
 and 453

Fixed whitespace errors

 
 3) Output of the --list method is confusing:
 
 # ipa-managed-entries --list
 Directory Manager password: 
 
 Available Managed Entry Plugins:
 cn=upg definition,cn=definitions,cn=managed
 entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
 cn=ngp definition,cn=definitions,cn=managed
 entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
 
 You must specify a managed entry definition 
 # echo $?
 1
 
 a) I shouldn't be asked to specify a managed entry definition for --list

Fixed

 b) The listing was successful, so we shouldn't return error code

Corrected error code

 
 4) Return code for disabling an already disabled entry should be 2
 (according to man pages):
 
 # ipa-managed-entries -e 'cn=upg definition,cn=definitions,cn=managed 
 entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com' disable
 Directory Manager password: 
 
 Plugin already disabled
 # echo $?
 0

Fixed error code

 
 5) Strange is, that enabling a disabled plugin gives me return code 2:
 # ipa-managed-entries -e 'cn=upg definition,cn=definitions,cn=managed 
 entries,cn=etc,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com' enable
 Directory Manager password: 
 
 Enabling Plugin
 # echo $?
 2
 
 Return codes for these actions should fit the man pages.

Fixed error code

 
 6) I would improve working with LDAP filters, current solution is error
 prone. Try disablingenabling NGP Defition, we end up with this
 originFilter:
 
 originfilter: ((objectclass=ipahostgroup))
 
 I think the cleanest solution would be to use ldap.make_filter and
 ldap.combine_filters functions to play with these filter. You can
 inspire yourself in this example I wrote for DNS plugin:
 
 rev_zone_filter = ldap.make_filter(search_kw, rules=ldap.MATCH_NONE, 
 exact=False,
trailing_wildcard=False)
 filter = ldap.combine_filters((rev_zone_filter, filter), rules=ldap.MATCH_ALL)

Rob and you addressed this in the mailing list.
For the record, I do agree that we are lacking a method for reading and 
modifying existing ldap filters.
We will continue with the simple string method here for this iteration.

 
 7) Entering Directory Manager every time may be a bit tedious. Could we
 use current Kerberos credentials and fall-back to asking Directory
 Manager password if it doesn't work? Its already done this way in
 ipa-replica-manage for example.
 
 We could fix this, however, as an enhancement in another patch.

Fixed. We now will use gssapi if available, and prompt for password if there is 
no ticket.

 
 8) Man page - please use the new united FreeIPA man page header. Instead
 of 
 
 +.TH ipa-managed-entries 1 Sept 15 2011 freeipa 
 
 use:
 
 +.TH ipa-managed-entries 1 Sept 15 2011 FreeIPA FreeIPA Manual
 Pages

Fixed

 
 
 9) Man page - comma is missing for --list option:
 
 +\fB\-l\-\-list\fR
 

Fixed

 
 10) install/po/Makefile.in should be updated to: there is still
 reference to ipa-host-net-manage and ipa-managed-entries reference is
 missing

Fixed



bin1u2MkpIsph.bin
Description: freeipa-jraquino-0025-Create-Tool-for-Enabling-Disabling-Managed-Entries.patch

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

Re: [Freeipa-devel] [PATCH] 45 Check that install hostname matches the server hostname

2011-09-15 Thread Rob Crittenden

Alexander Bokovoy wrote:

On Mon, 12 Sep 2011, Jan Cholasta wrote:

We can't dictate which interface matches the hostname. At most we can
warn about this, but not fail to install.

rob


Changed to print a warning message instead of raising an error.

ACK.



pushed to master and ipa-2-1

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


[Freeipa-devel] [PATCH] 875 fix rpm installation ordering

2011-09-15 Thread Rob Crittenden
freeipa-server-selinux was getting installed before freeipa-server which 
caused some SELinux contexts to not be set properly.


The existing Requires we had used to fix things, perhaps a change in rpm 
caused this to break, who knows. I'm not even sure when this stopped 
working.


I added an extra postun rule so that the server-selinux package is 
removed as a dependency when you do a yum erase freeipa-python.


rob
From f17f96510a07b360e189b305b01536d3bfe21ee7 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 15 Sep 2011 15:57:41 -0400
Subject: [PATCH] Change the Requires for the server and server-selinux for proper order

The server package needs to be installed before the server-selinux
package otherwise the SELinux contexts won't get set properly.

The (postun) is so you can continue to do yum erase freeipa-python
and it will pick up everything else.

https://fedorahosted.org/freeipa/ticket/1779
---
 freeipa.spec.in |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 940b009..d4f3ac6 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -84,7 +84,7 @@ Group: System Environment/Base
 Requires: %{name}-python = %{version}-%{release}
 Requires: %{name}-client = %{version}-%{release}
 Requires: %{name}-admintools = %{version}-%{release}
-Requires(post): %{name}-server-selinux = %{version}-%{release}
+Requires: %{name}-server-selinux = %{version}-%{release}
 Requires(pre): 389-ds-base = 1.2.9.7-1
 Requires: openldap-clients
 Requires: nss
@@ -144,7 +144,8 @@ this package).
 %package server-selinux
 Summary: SELinux rules for freeipa-server daemons
 Group: System Environment/Base
-Requires: %{name}-server = %{version}-%{release}
+Requires(post): %{name}-server = %{version}-%{release}
+Requires(postun): %{name}-server = %{version}-%{release}
 Requires(pre): policycoreutils = %{POLICYCOREUTILSVER}
 
 Obsoletes: ipa-server-selinux = 1.0
-- 
1.7.4

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

Re: [Freeipa-devel] Upgrading a machine to use the proxy.

2011-09-15 Thread Adam Young
OK,  here's something closer to releasable and written in Perl.  This 
script will upgrade the proxy ports  to 9444 by default, or allow you to 
override by setting the first parameter.


enable_proxy_dogtag.pl
Description: Perl program
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel