Re: [Freeipa-devel] [PATCH] 1063 Allow no reverse domain

2012-10-17 Thread Martin Kosek
On 10/16/2012 07:27 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 On 10/16/2012 05:21 PM, Rob Crittenden wrote:
 A reverse zone is always created unless --no-reverse is passed.

 rob


 Yeah, this is much better. I would just unify our summary printed before
 installation. Now when running ipa-server-install with --no-reverse:

 ...
 BIND DNS server will be configured to serve IPA domain with:
 Forwarders:IP_ADDRESS
 Reverse zone:  No reverse zone   

 Continue to configure the system with these values? [no]:
 ...


 But if you answer no for THE question, then we have a bit different 
 wording:

 BIND DNS server will be configured to serve IPA domain with:
 Forwarders:10.16.255.2
 Reverse zone:  None 

 Continue to configure the system with these values? [no]: y
 ...

 If you fix this (it is a one-liner), then ACK.

 Martin

 
 Updated patch.
 
 rob

ACK. Pushed to master, ipa-3-0.

Martin

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


Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Sumit Bose
On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:
 On Wed, 10 Oct 2012, Sumit Bose wrote:
 On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:
 
 Warn about manual DNA plugin configuration when working with local ID ranges
 since we currently do not support automatic pick up of the changed
 settings for local ID ranges by the DNA plugin.
 https://fedorahosted.org/freeipa/ticket/3116
 
 
 --
 / Alexander Bokovoy
 
  )
 
 I wonder if we should add a sentence like See section 'Managing Unique
 UID and GID Number Assignments' in the FreeIPA Documentation for
 details' to point the admin to the right directory? Or replace the last
 sentence with something more explicit like 'The dnaNextRange attribute
 of 'cn=Posix IDs,cn=Distributed Numeric Assignment
 Plugin,cn=plugins,cn=config' has to be modified to match the new range'?
 Updated the patch, also adding the same warning to the 'idrange-add'
 help.
 
 -- 
 / Alexander Bokovoy

ACK.

If there is an easy way to avoid the duplication it would be nice if you
can modify the patch accordingly.

bye,
Sumit

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


[Freeipa-devel] [PATCH] 222 Fixed incorrect link to browser config after session expiration

2012-10-17 Thread Petr Vobornik

Fixed typo in message placeholder.

https://fedorahosted.org/freeipa/ticket/3187
--
Petr Vobornik
From 7c87acf4de28a80a9bd3a3c050aebd40917a8091 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 17 Oct 2012 10:14:20 +0200
Subject: [PATCH] Fixed incorrect link to browser config after session
 expiration

Fixed typo in message placeholder.

https://fedorahosted.org/freeipa/ticket/3187
---
 install/ui/test/data/ipa_init.json | 2 +-
 ipalib/plugins/internal.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 8a1c9a1b104b3e4b6e624eb4034b243572a841e5..5a88ebb2a4f8d3ef139330758d8d501b6ccd8e38 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -134,7 +134,7 @@
 login: {
 form_auth: To login with username and password, enter them in the fields below then click Login.,
 header: Logged In As,
-krb_auth_msg: To login with Kerberos, please make sure you have valid tickets (obtainable via kinit) and a href='http://${host]/ipa/config/unauthorized.html'configured/a the browser correctly, then click Login.,
+krb_auth_msg: To login with Kerberos, please make sure you have valid tickets (obtainable via kinit) and a href='http://${host}/ipa/config/unauthorized.html'configured/a the browser correctly, then click Login.,
 login: Login,
 logout: Logout,
 logout_error: Logout error,
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 1e21e81161b54d3a2c49476db87093cfb60a0d7e..7b56bbbc438f8b47d002784343aeca439159c9d8 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -269,7 +269,7 @@ class i18n_messages(Command):
 login: {
 form_auth: _(To login with username and password, enter them in the fields below then click Login.),
 header: _(Logged In As),
-krb_auth_msg: _(To login with Kerberos, please make sure you have valid tickets (obtainable via kinit) and a href='http://${host]/ipa/config/unauthorized.html'configured/a the browser correctly, then click Login.),
+krb_auth_msg: _(To login with Kerberos, please make sure you have valid tickets (obtainable via kinit) and a href='http://${host}/ipa/config/unauthorized.html'configured/a the browser correctly, then click Login.),
 login: _(Login),
 logout: _(Logout),
 logout_error: _(Logout error),
-- 
1.7.11.4

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

Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Martin Kosek
On 10/17/2012 11:43 AM, Sumit Bose wrote:
 On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:
 On Wed, 10 Oct 2012, Sumit Bose wrote:
 On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:

 Warn about manual DNA plugin configuration when working with local ID 
 ranges
 since we currently do not support automatic pick up of the changed
 settings for local ID ranges by the DNA plugin.
 https://fedorahosted.org/freeipa/ticket/3116


 --
 / Alexander Bokovoy

 )

 I wonder if we should add a sentence like See section 'Managing Unique
 UID and GID Number Assignments' in the FreeIPA Documentation for
 details' to point the admin to the right directory? Or replace the last
 sentence with something more explicit like 'The dnaNextRange attribute
 of 'cn=Posix IDs,cn=Distributed Numeric Assignment
 Plugin,cn=plugins,cn=config' has to be modified to match the new range'?
 Updated the patch, also adding the same warning to the 'idrange-add'
 help.

 -- 
 / Alexander Bokovoy
 
 ACK.
 
 If there is an easy way to avoid the duplication it would be nice if you
 can modify the patch accordingly.
 
 bye,
 Sumit
 

Pushed to master, ipa-3-0.

Martin

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


Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Alexander Bokovoy

On Wed, 17 Oct 2012, Sumit Bose wrote:

On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:

On Wed, 10 Oct 2012, Sumit Bose wrote:
On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:

Warn about manual DNA plugin configuration when working with local ID ranges
since we currently do not support automatic pick up of the changed
settings for local ID ranges by the DNA plugin.
https://fedorahosted.org/freeipa/ticket/3116


--
/ Alexander Bokovoy

 )

I wonder if we should add a sentence like See section 'Managing Unique
UID and GID Number Assignments' in the FreeIPA Documentation for
details' to point the admin to the right directory? Or replace the last
sentence with something more explicit like 'The dnaNextRange attribute
of 'cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config' has to be modified to match the new range'?
Updated the patch, also adding the same warning to the 'idrange-add'
help.

--
/ Alexander Bokovoy


ACK.

If there is an easy way to avoid the duplication it would be nice if you
can modify the patch accordingly.

Docstring is a string literal only:
s=text
   ... first
   ... second
def f():
   ...   another text
   ...  first
   ...  second+s
   ...   return
   ... 
print f.__doc__

   None
def y():
   ...   Doctstring for y()
   ...   return
   ...
print y.__doc__
   Doctstring for y()
   

Though we could play the game and do explicit 
	f.__doc__ = s

this would work but...

Any preference from others?.
--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Petr Viktorin

On 10/17/2012 12:10 PM, Alexander Bokovoy wrote:

On Wed, 17 Oct 2012, Sumit Bose wrote:

On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:

On Wed, 10 Oct 2012, Sumit Bose wrote:
On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:

Warn about manual DNA plugin configuration when working with local
ID ranges
since we currently do not support automatic pick up of the changed
settings for local ID ranges by the DNA plugin.
https://fedorahosted.org/freeipa/ticket/3116


--
/ Alexander Bokovoy

 )

I wonder if we should add a sentence like See section 'Managing Unique
UID and GID Number Assignments' in the FreeIPA Documentation for
details' to point the admin to the right directory? Or replace the last
sentence with something more explicit like 'The dnaNextRange attribute
of 'cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config' has to be modified to match the new
range'?
Updated the patch, also adding the same warning to the 'idrange-add'
help.

--
/ Alexander Bokovoy


ACK.

If there is an easy way to avoid the duplication it would be nice if you
can modify the patch accordingly.

Docstring is a string literal only:
 s=text
... first
... second
 def f():
...   another text
...  first
...  second+s
...   return
... print f.__doc__
None
 def y():
...   Doctstring for y()
...   return
...
 print y.__doc__
Doctstring for y()


Though we could play the game and do explicit f.__doc__ = s
this would work but...

Any preference from others?.


In the code you changed, we already play that game.

--
Petr³

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


Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Alexander Bokovoy

On Wed, 17 Oct 2012, Martin Kosek wrote:

On 10/17/2012 12:14 PM, Petr Viktorin wrote:

On 10/17/2012 12:10 PM, Alexander Bokovoy wrote:

On Wed, 17 Oct 2012, Sumit Bose wrote:

On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:

On Wed, 10 Oct 2012, Sumit Bose wrote:
On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:

Warn about manual DNA plugin configuration when working with local
ID ranges
since we currently do not support automatic pick up of the changed
settings for local ID ranges by the DNA plugin.
https://fedorahosted.org/freeipa/ticket/3116


--
/ Alexander Bokovoy

 )

I wonder if we should add a sentence like See section 'Managing Unique
UID and GID Number Assignments' in the FreeIPA Documentation for
details' to point the admin to the right directory? Or replace the last
sentence with something more explicit like 'The dnaNextRange attribute
of 'cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config' has to be modified to match the new
range'?
Updated the patch, also adding the same warning to the 'idrange-add'
help.

--
/ Alexander Bokovoy


ACK.

If there is an easy way to avoid the duplication it would be nice if you
can modify the patch accordingly.

Docstring is a string literal only:
 s=text
... first
... second
 def f():
...   another text
...  first
...  second+s
...   return
... print f.__doc__
None
 def y():
...   Doctstring for y()
...   return
...
 print y.__doc__
Doctstring for y()


Though we could play the game and do explicit f.__doc__ = s
this would work but...

Any preference from others?.


In the code you changed, we already play that game.



Ok, it seems I pushed the patch way too early. If Alexander wants to, he can
prepare a patch to fix the duplication and we I can push the change then.

See my another email. We have more fundamental issues that prevent this
kind of message reuse. You pushed the right patch, I believe.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Martin Kosek
On 10/17/2012 12:42 PM, Alexander Bokovoy wrote:
 On Wed, 17 Oct 2012, Petr Viktorin wrote:
 On 10/17/2012 12:10 PM, Alexander Bokovoy wrote:
 On Wed, 17 Oct 2012, Sumit Bose wrote:
 On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:
 On Wed, 10 Oct 2012, Sumit Bose wrote:
 On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:

 Warn about manual DNA plugin configuration when working with local
 ID ranges
 since we currently do not support automatic pick up of the changed
 settings for local ID ranges by the DNA plugin.
 https://fedorahosted.org/freeipa/ticket/3116


 -- 
 / Alexander Bokovoy

 )

 I wonder if we should add a sentence like See section 'Managing Unique
 UID and GID Number Assignments' in the FreeIPA Documentation for
 details' to point the admin to the right directory? Or replace the last
 sentence with something more explicit like 'The dnaNextRange attribute
 of 'cn=Posix IDs,cn=Distributed Numeric Assignment
 Plugin,cn=plugins,cn=config' has to be modified to match the new
 range'?
 Updated the patch, also adding the same warning to the 'idrange-add'
 help.

 -- 
 / Alexander Bokovoy

 ACK.

 If there is an easy way to avoid the duplication it would be nice if you
 can modify the patch accordingly.
 Docstring is a string literal only:
 s=text
... first
... second
 def f():
...   another text
...  first
...  second+s
...   return
... print f.__doc__
None
 def y():
...   Doctstring for y()
...   return
...
 print y.__doc__
Doctstring for y()


 Though we could play the game and do explicit f.__doc__ = s
 this would work but...

 Any preference from others?.

 In the code you changed, we already play that game.
 No, we don't. We do explicit string literals wrapped into Gettext()
 calls but it does not change the fact that they are still literals and
 xgettext does not support arbitrary expressions in Python code.
 
 I tried two different approaches:
 1.   a = _(one text)
   __doc__ = _(another text) + a
 
 2.   a = one text
   __doc__ = _(another text + a)
 
 Both don't work. First is because Gettext() class does not support
 concatenation. Second one is because xgettext does not work with such
 form and only takes another text.
 

I think I still do not get it, what's wrong with my example?

# python
 from ipalib import _
 a = Bonus text
 __doc__ = _(Another Text + a)
 __doc__
Gettext('Another TextBonus text', domain='ipa', localedir=None)
 unicode(__doc__)
u'Another TextBonus text'

Am I missing something?

Martin

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


Re: [Freeipa-devel] [PATCH] 0089 Clarify trust-add help regarding multiple runs against the same domain

2012-10-17 Thread Sumit Bose
On Wed, Oct 10, 2012 at 06:05:02PM +0300, Alexander Bokovoy wrote:
 Hi,
 
 this patch originated from off-list discussion regarding multiple runs
 of ipa trust-add against the same domain.
 
 Since trust-add re-establishes the trust every time it is run and all
 the other information fetched from the remote domain controller stays
 the same, it can be run multiple times. The only change would occur is
 update of trust relationship credentials -- they are supposed to be
 updated periodically by underlying infrastructure anyway.
 
 So the patch adds some clarity to the help and changes summary message
 when trust was re-established instead of created.
 -- 
 / Alexander Bokovoy

ACK

Btw, another useful feature of allowing to run trust-add multiple times
is to re-established the trust if it was deleted only on one side, AD or
IPA. Having a separate command for this would make no sense because it
would be basically be an alias to trust-add.

bye,
Sumit

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


[Freeipa-devel] [PATCH] 223 Simpler instructions to generate certificate

2012-10-17 Thread Petr Vobornik

Instructions to generate certificate were simplified.

New instructions:

 1) Create a certificate database or use an existing one. To create a 
new database:

# certutil -N -d database path
 2) Create a CSR with subject CN=hostname,O=realm, for example:
# certutil -R -d database path -a -g key size -s 
'CN=dev.example.com,O=DEV.EXAMPLE.COM'
 3) Copy and paste the CSR (from -BEGIN NEW CERTIFICATE 
REQUEST- to -END NEW CERTIFICATE REQUEST-) into the text 
area below:


https://fedorahosted.org/freeipa/ticket/3056
--
Petr Vobornik
From 0c883b427796b52cd6fed1f4b497d20e2e9c84d9 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 17 Oct 2012 12:49:34 +0200
Subject: [PATCH] Simpler instructions to generate certificate

Instructions to generate certificate were simplified.

New instructions:

 1) Create a certificate database or use an existing one. To create a new database:
# certutil -N -d database path
 2) Create a CSR with subject CN=hostname,O=realm, for example:
# certutil -R -d database path -a -g key size -s 'CN=dev.example.com,O=DEV.EXAMPLE.COM'
 3) Copy and paste the CSR (from -BEGIN NEW CERTIFICATE REQUEST- to -END NEW CERTIFICATE REQUEST-) into the text area below:

https://fedorahosted.org/freeipa/ticket/3056
---
 install/ui/test/data/ipa_init.json | 2 +-
 ipalib/plugins/internal.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 8a1c9a1b104b3e4b6e624eb4034b243572a841e5..c9bf2a2759fa6af71d95ac92c56cae88e9cc189f 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -195,7 +195,7 @@
 privilege_withdrawn: Privilege Withdrawn,
 reason: Reason for Revocation,
 remove_from_crl: Remove from CRL,
-request_message: olliExamples uses NSS database located in current directory. Replace \-d  .\ in example with \-d /path/to/database\ if NSS database is located elsewhere. If you don't have a NSS database you can create one in current directory by \certutil -N -d .\ /liliCreate a CSR with \CN=${hostname},O=${realm}\, for example:br/# certutil -R -d . -a em title=\key size in bits\-g 2048/em -s 'CN=${hostname},O=${realm}'/liliCopy and paste the CSR (the text block which starts with \-BEGIN NEW CERTIFICATE REQUEST-\ and ends with \-END NEW CERTIFICATE REQUEST-\) below:/li/ol,
+request_message: ol liCreate a certificate database or use an existing one. To create a new database:br/ code# certutil -N -d lt;database pathgt;/code /li liCreate a CSR with subject emCN=lt;hostnamegt;,O=lt;realmgt;/em, for example:br/ code# certutil -R -d lt;database pathgt; -a -g lt;key sizegt; -s 'CN=${hostname},O=${realm}'/code /li li Copy and paste the CSR (from em-BEGIN NEW CERTIFICATE REQUEST-/em to em-END NEW CERTIFICATE REQUEST-/em) into the text area below: /li /ol,
 requested: Certificate requested,
 restore_certificate: Restore Certificate for ${entity} ${primary_key},
 restore_confirmation: To confirm your intention to restore this certificate, click the \Restore\ button.,
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 1e21e81161b54d3a2c49476db87093cfb60a0d7e..0be9f4352b2bc3d44e5fe91e44c4e105feee5a50 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -331,7 +331,7 @@ class i18n_messages(Command):
 privilege_withdrawn: _(Privilege Withdrawn),
 reason: _(Reason for Revocation),
 remove_from_crl: _(Remove from CRL),
-request_message: _(olliExamples uses NSS database located in current directory. Replace \-d  .\ in example with \-d /path/to/database\ if NSS database is located elsewhere. If you don't have a NSS database you can create one in current directory by \certutil -N -d .\ /liliCreate a CSR with \CN=${hostname},O=${realm}\, for example:br/# certutil -R -d . -a em title=\key size in bits\-g 2048/em -s 'CN=${hostname},O=${realm}'/liliCopy and paste the CSR (the text block which starts with \-BEGIN NEW CERTIFICATE REQUEST-\ and ends with \-END NEW CERTIFICATE REQUEST-\) below:/li/ol),
+request_message: _(ol liCreate a certificate database or use an existing one. To create a new database:br/ code# certutil -N -d lt;database pathgt;/code /li liCreate a CSR with subject emCN=lt;hostnamegt;,O=lt;realmgt;/em, for example:br/ code# certutil -R -d lt;database pathgt; -a -g lt;key sizegt; -s 'CN=${hostname},O=${realm}'/code /li li Copy and paste the CSR (from em-BEGIN NEW CERTIFICATE REQUEST-/em to em-END NEW CERTIFICATE REQUEST-/em) into the text area below: /li /ol),
 requested: 

Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Alexander Bokovoy

On Wed, 17 Oct 2012, Martin Kosek wrote:

On 10/17/2012 12:42 PM, Alexander Bokovoy wrote:

On Wed, 17 Oct 2012, Petr Viktorin wrote:

On 10/17/2012 12:10 PM, Alexander Bokovoy wrote:

On Wed, 17 Oct 2012, Sumit Bose wrote:

On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:

On Wed, 10 Oct 2012, Sumit Bose wrote:

On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:


Warn about manual DNA plugin configuration when working with local

ID ranges

since we currently do not support automatic pick up of the changed
settings for local ID ranges by the DNA plugin.
https://fedorahosted.org/freeipa/ticket/3116


--
/ Alexander Bokovoy



)


I wonder if we should add a sentence like See section 'Managing Unique
UID and GID Number Assignments' in the FreeIPA Documentation for
details' to point the admin to the right directory? Or replace the last
sentence with something more explicit like 'The dnaNextRange attribute
of 'cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config' has to be modified to match the new

range'?
Updated the patch, also adding the same warning to the 'idrange-add'
help.

--
/ Alexander Bokovoy


ACK.

If there is an easy way to avoid the duplication it would be nice if you
can modify the patch accordingly.

Docstring is a string literal only:
s=text
   ... first
   ... second
def f():
   ...   another text
   ...  first
   ...  second+s
   ...   return
   ... print f.__doc__
   None
def y():
   ...   Doctstring for y()
   ...   return
   ...
print y.__doc__
   Doctstring for y()
   

Though we could play the game and do explicit f.__doc__ = s
this would work but...

Any preference from others?.


In the code you changed, we already play that game.

No, we don't. We do explicit string literals wrapped into Gettext()
calls but it does not change the fact that they are still literals and
xgettext does not support arbitrary expressions in Python code.

I tried two different approaches:
1.   a = _(one text)
  __doc__ = _(another text) + a

2.   a = one text
  __doc__ = _(another text + a)

Both don't work. First is because Gettext() class does not support
concatenation. Second one is because xgettext does not work with such
form and only takes another text.



I think I still do not get it, what's wrong with my example?

# python

from ipalib import _
a = Bonus text
__doc__ = _(Another Text + a)
__doc__

Gettext('Another TextBonus text', domain='ipa', localedir=None)

unicode(__doc__)

u'Another TextBonus text'

Am I missing something?

Yes. You are missing xgettext utility. It does extract strings for
translation. In your example it would extract Another Text only,
because it does not support evaluation of expressions.

More to that, the actual code would ask then for translated string with
msgid Another TextBonus text which does not exist in the
translation catalog at all. This means whatever was translated for
Another Text will never be shown.


We need to make Gettext() class additive, so that when two Gettext()
instances are added, they accumulate msgid(s) and then do concatenation
of translated entries on __str__.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 0089 Clarify trust-add help regarding multiple runs against the same domain

2012-10-17 Thread Martin Kosek
On 10/17/2012 12:52 PM, Sumit Bose wrote:
 On Wed, Oct 10, 2012 at 06:05:02PM +0300, Alexander Bokovoy wrote:
 Hi,

 this patch originated from off-list discussion regarding multiple runs
 of ipa trust-add against the same domain.

 Since trust-add re-establishes the trust every time it is run and all
 the other information fetched from the remote domain controller stays
 the same, it can be run multiple times. The only change would occur is
 update of trust relationship credentials -- they are supposed to be
 updated periodically by underlying infrastructure anyway.

 So the patch adds some clarity to the help and changes summary message
 when trust was re-established instead of created.
 -- 
 / Alexander Bokovoy
 
 ACK
 
 Btw, another useful feature of allowing to run trust-add multiple times
 is to re-established the trust if it was deleted only on one side, AD or
 IPA. Having a separate command for this would make no sense because it
 would be basically be an alias to trust-add.
 
 bye,
 Sumit
 

I am still a bit worried about our consistency with IPA command help
indentation. You have it indented with trust-add command:

# ipa help trust-add
Purpose: Add new trust to use.

This command establishes trust relationship to another domain
which becomes 'trusted'. As result, users of the trusted domain
may access resources of this domain.
...


But other commands don't, e.g.:

# ipa help passwd
Set a user's password

If someone other than a user changes that user's password (e.g., Helpdesk
resets it) then the p...

# ipa help ping
Ping the remote IPA server to ensure it is running.

The ping command sends an echo request to an IPA server. The server
...

Martin

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


Re: [Freeipa-devel] [PATCH 0019] Forbid overlapping primary and secondary rid ranges

2012-10-17 Thread Tomas Babej

On 10/17/2012 11:14 AM, Sumit Bose wrote:

On Tue, Oct 16, 2012 at 02:26:24PM +0200, Tomas Babej wrote:

Hi,

commands ipa idrange-add / idrange-mod no longer allows the user
to enter primary or secondary rid range such that has non-zero
intersection with primary or secondary rid range of another
existing id range, as this could cause collision.

Unit tests added to test_range_plugin.py

https://fedorahosted.org/freeipa/ticket/3086

Tomas

Thank you for the patch, comments are in-line.

bye,
Sumit


From a46a8d0aa4e64e105a53a177b6a12cf28e56620e Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 15 Oct 2012 06:28:16 -0400
Subject: [PATCH] Forbid overlapping primary and secondary rid ranges

Commands ipa idrange-add / idrange-mod no longer allows the user
to enter primary or secondary rid range such that has non-zero
intersection with primary or secondary rid range of another
existing id range, as this could cause collision.

Unit tests added to test_range_plugin.py

https://fedorahosted.org/freeipa/ticket/3086
---
  .../ipa-range-check/ipa_range_check.c  |  93 +---
  tests/test_xmlrpc/test_range_plugin.py | 120 +++--
  2 files changed, 191 insertions(+), 22 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c 
b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
index 
499e54a9c4a4c9134a231c0cd09e700390565a14..4f9f7437d11d2bc33238b14f5099a42b4c5463d2
 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
@@ -132,24 +132,67 @@ done:
  return ret;
  }
  
-#define IN_RANGE(x,base,size) ( (x) = (base)  ((x) - (base))  (size) )

-static bool ranges_overlap(struct range_info *r1, struct range_info *r2)
+#define IN_RANGE(x,base,size) ( (x) = (base)  (x)  (size+base) )

Would you mind to use the original definition of IN_RANGE? x-base looks
a bit odd, but I made it on purpose to avoid overruns. Since we already
know that x=base we can be sure that there will be no underrun.


+static bool intervals_overlap(uint32_t x, uint32_t base, uint32_t x_size, 
uint32_t base_size)
  {
-if (r1-name != NULL  r2-name != NULL 
-strcasecmp(r1-name, r2-name) == 0) {
-return false;
-}
-
-if (IN_RANGE(r1-base_id, r2-base_id, r2-id_range_size) ||
-IN_RANGE((r1-base_id + r1-id_range_size - 1), r2-base_id, 
r2-id_range_size) ||
-IN_RANGE(r2-base_id, r1-base_id, r1-id_range_size) ||
-IN_RANGE((r2-base_id + r2-id_range_size - 1), r1-base_id, 
r1-id_range_size)) {
+if (IN_RANGE(x, base, base_size) ||
+IN_RANGE((x + x_size - 1), base, base_size) ||
+IN_RANGE(base, x, x_size) ||
+IN_RANGE((base + base_size - 1), x, x_size)) {
  return true;
  }
  
  return false;

  }
  
+//returns 0 if there is no overlap

+//connected ranges must not overlap:
+//  existing range:  base  rid  sec_rid
+//| |  \  / |
+//| |   \/  |
+//| |   /\  |
+//| |  /  \ |
+//  new range:   base  rid  sec_rid

I think we currently do not use C++ style comments in freeipa C code.
Can you switch to /* */ comments?


+static int ranges_overlap(struct range_info *r1, struct range_info *r2)
+{
+if (r1-name != NULL  r2-name != NULL 
+strcasecmp(r1-name, r2-name) == 0) {
+return 0;
+}
+
+//check if base range overlaps with existing base range
+if (intervals_overlap(r1-base_id, r2-base_id,
+r1-id_range_size, r2-id_range_size)){
+return 1;
+}
+
+//if both base_rid and secondary_base_rid are 0, the rid range is not set
+//in that case we skip the primary/secondary rid range overlap test
+if((r1-base_rid!=0 || r1-secondary_base_rid!=0) 
+   (r2-base_rid!=0 || r2-secondary_base_rid!=0)){

can you add spaces around '!=' ?


+
+//check if rid range overlaps with existing rid range
+if (intervals_overlap(r1-base_rid, r2-base_rid,
+r1-id_range_size, r2-id_range_size))
+return 2;
+
+//check if secondary rid range overlaps with existing secondary rid 
range
+if (intervals_overlap(r1-secondary_base_rid, r2-secondary_base_rid,
+r1-id_range_size, r2-id_range_size))
+return 3;
+
+//check if rid range overlaps with existing secondary rid range
+if (intervals_overlap(r1-base_rid, r2-secondary_base_rid,
+r1-id_range_size, r2-id_range_size))
+return 4;
+
+//check if secondary rid range overlaps with existing rid range
+if (intervals_overlap(r1-secondary_base_rid, r2-base_rid,
+r1-id_range_size, r2-id_range_size))
+return 5;
+}

Return code is missing if one of the ranges does not have the rid ranges
set. Can you add a test case for this condition as well?


+}
+
  static 

[Freeipa-devel] Hide private symbols in the bind-dyndb-ldap

2012-10-17 Thread Adam Tkac
Hello,

attached patch hides all symbols except dynamic_driver_{init,destroy}. Feedback
is appreciated.

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.
From 126929489baf4f69fe0444860776f7e76c1411f2 Mon Sep 17 00:00:00 2001
From: Adam Tkac von...@gmail.com
Date: Wed, 17 Oct 2012 13:00:31 +0200
Subject: [PATCH] Hide all symbols except dynamic_driver_{init,destroy}

Signed-off-by: Adam Tkac von...@gmail.com
---
 configure.ac  | 15 +++
 src/ldap_driver.c | 10 --
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index cff5526..30a7374 100644
--- a/configure.ac
+++ b/configure.ac
@@ -30,6 +30,21 @@ AC_TYPE_SIZE_T
 # Checks for library functions.
 AC_CHECK_FUNCS([memset strcasecmp strncasecmp])
 
+# Check if build chain supports symbol visibility
+AC_MSG_CHECKING([for -fvisibility=hidden compiler flag])
+SAVED_CFLAGS=$CFLAGS
+CFLAGS=$CFLAGS -fvisibility=hidden
+AC_TRY_COMPILE([
+   extern __attribute__((__visibility__(hidden))) int hidden;
+   extern __attribute__((__visibility__(default))) int def;
+   extern __attribute__((__visibility__(hidden))) int fhidden(void);
+   extern __attribute__((__visibility__(default))) int fdef(void);
+],[],
+[AC_MSG_RESULT([yes])
+ AC_DEFINE([HAVE_VISIBILITY], 1, [Define if compiler supports -fvisibility])],
+[CFLAGS=$SAVED_CFLAGS
+ AC_MSG_RESULT([no])])
+
 # Check version of libdns
 AC_MSG_CHECKING([libdns version])
 AC_TRY_RUN([
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 3a80223..99a8421 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -52,6 +52,12 @@
 #include util.h
 #include zone_manager.h
 
+#ifdef HAVE_VISIBILITY
+#define VISIBLE __attribute__((__visibility__(default)))
+#else
+#define VISIBLE
+#endif
+
 #define LDAPDB_MAGIC   ISC_MAGIC('L', 'D', 'P', 'D')
 #define VALID_LDAPDB(ldapdb) \
((ldapdb) != NULL  (ldapdb)-common.impmagic == LDAPDB_MAGIC)
@@ -1315,7 +1321,7 @@ static dns_dbimplementation_t *ldapdb_imp;
 const char *ldapdb_impname = dynamic-ldap;
 
 
-isc_result_t
+VISIBLE isc_result_t
 dynamic_driver_init(isc_mem_t *mctx, const char *name, const char * const 
*argv,
dns_dyndb_arguments_t *dyndb_args)
 {
@@ -1360,7 +1366,7 @@ dynamic_driver_init(isc_mem_t *mctx, const char *name, 
const char * const *argv,
return result;
 }
 
-void
+VISIBLE void
 dynamic_driver_destroy(void)
 {
/* Only unregister the implementation if it was registered by us. */
-- 
1.7.11.7

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

Re: [Freeipa-devel] [PATCH] 0087 Warn about DNA plugin configuration when working with local ID ranges

2012-10-17 Thread Petr Viktorin

On 10/17/2012 12:42 PM, Alexander Bokovoy wrote:

On Wed, 17 Oct 2012, Petr Viktorin wrote:

On 10/17/2012 12:10 PM, Alexander Bokovoy wrote:

On Wed, 17 Oct 2012, Sumit Bose wrote:

On Wed, Oct 10, 2012 at 12:59:53PM +0300, Alexander Bokovoy wrote:

On Wed, 10 Oct 2012, Sumit Bose wrote:

On Wed, Oct 10, 2012 at 10:51:11AM +0300, Alexander Bokovoy wrote:


Warn about manual DNA plugin configuration when working with local

ID ranges

since we currently do not support automatic pick up of the changed
settings for local ID ranges by the DNA plugin.
https://fedorahosted.org/freeipa/ticket/3116


--
/ Alexander Bokovoy



)


I wonder if we should add a sentence like See section 'Managing
Unique
UID and GID Number Assignments' in the FreeIPA Documentation for
details' to point the admin to the right directory? Or replace the
last
sentence with something more explicit like 'The dnaNextRange
attribute
of 'cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config' has to be modified to match the new

range'?
Updated the patch, also adding the same warning to the 'idrange-add'
help.

--
/ Alexander Bokovoy


ACK.

If there is an easy way to avoid the duplication it would be nice if
you
can modify the patch accordingly.

Docstring is a string literal only:
s=text
   ... first
   ... second
def f():
   ...   another text
   ...  first
   ...  second+s
   ...   return
   ... print f.__doc__
   None
def y():
   ...   Doctstring for y()
   ...   return
   ...
print y.__doc__
   Doctstring for y()
   

Though we could play the game and do explicit f.__doc__ = s
this would work but...

Any preference from others?.


In the code you changed, we already play that game.

No, we don't. We do explicit string literals wrapped into Gettext()
calls but it does not change the fact that they are still literals and
xgettext does not support arbitrary expressions in Python code.

I tried two different approaches:
1.   a = _(one text)
   __doc__ = _(another text) + a

2.   a = one text
   __doc__ = _(another text + a)

Both don't work. First is because Gettext() class does not support
concatenation. Second one is because xgettext does not work with such
form and only takes another text.



Ah, you're right. A similar complaint from another POV came from a 
translator not long ago. I've filed a ticket, 
https://fedorahosted.org/freeipa/ticket/3188


--
Petr³

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


[Freeipa-devel] [PATCH] 323 Report ipa-upgradeconfig errors during RPM upgrade

2012-10-17 Thread Martin Kosek
Report errors just like with ipa-ldap-updater. These messages should warn
user that some parts of the upgrades may have not been successful and
he should follow up on them. Otherwise, user may not notice them at all.

ipa-upgradeconfig logging has been made consistent with ipa-ldap-updater
logging - only error level or higher severity log messages are printed
to stderr, with the same console format. A new debug log that an upgrade
script has started was added to make log investigation easier.

https://fedorahosted.org/freeipa/ticket/3157

---

Reproducer is in Bugzilla attached to the ticket. With this patch, RPM update
will report errors to user:

# rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-*
Preparing...### [100%]
   1:freeipa-python ### [ 14%]
   2:freeipa-client ### [ 29%]
   3:freeipa-admintools ### [ 43%]
   4:freeipa-server ### [ 57%]
ERROR: Cannot update connections in /etc/named.conf: [Errno 13] Permission
denied: '/etc/named.conf'
   5:freeipa-server-selinux ### [ 71%]
   6:freeipa-server-trust-ad### [ 86%]
   7:freeipa-debuginfo  ### [100%]

Martin
From bcc791c27a55a98d051d6920a8fbb2ad9a4f1d10 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Wed, 17 Oct 2012 13:05:24 +0200
Subject: [PATCH] Report ipa-upgradeconfig errors during RPM upgrade

Report errors just like with ipa-ldap-updater. These messages should warn
user that some parts of the upgrades may have not been successful and
he should follow up on them. Otherwise, user may not notice them at all.

ipa-upgradeconfig logging has been made consistent with ipa-ldap-updater
logging - only error level or higher severity log messages are printed
to stderr, with the same console format. A new debug log that an upgrade
script has started was added to make log investigation easier.

https://fedorahosted.org/freeipa/ticket/3157
---
 freeipa.spec.in | 5 -
 install/tools/ipa-upgradeconfig | 5 +++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 060f09b12e6891defc8cb00d4f52a0d019198a70..4a0190cf5f98afbd61237e414b105d74d9489932 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -493,7 +493,7 @@ if [ $1 = 1 ]; then
 fi
 %endif
 if [ $1 -gt 1 ] ; then
-/usr/sbin/ipa-upgradeconfig /dev/null 21 || :
+/usr/sbin/ipa-upgradeconfig /dev/null || :
 fi
 
 %posttrans server
@@ -815,6 +815,9 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
 
 %changelog
+* Wed Oct 17 2012 Martin Kosek mko...@redhat.com - 2.99.0-51
+- Print ipa-upgradeconfig errors during RPM update
+
 * Wed Oct 10 2012 Alexander Bokovoy aboko...@redhat.com - 2.99.0-50
 - Make sure server-trust-ad subpackage alternates winbind_krb5_locator.so
   plugin to /dev/null since they cannot be used when trusts are configured
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 38426149864017789614095445f947a997d63b3c..cae0de8e15baaadf3e0973370356ab4e0ebc5420 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -577,9 +577,10 @@ def main():
 
 safe_options, options = parse_options()
 
-standard_logging_setup('/var/log/ipaupgrade.log', verbose=True,
-debug=options.debug, console_format='%(message)s',
+standard_logging_setup('/var/log/ipaupgrade.log', debug=options.debug,
+console_format='%(levelname)s: %(message)s',
 filemode='a')
+root_logger.debug('%s was invoked with options: %s' % (sys.argv[0], safe_options))
 
 fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore')
 
-- 
1.7.11.7

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

Re: [Freeipa-devel] [PATCH] support AES for cross-realm TGTs

2012-10-17 Thread Sumit Bose
On Wed, Sep 26, 2012 at 06:36:40PM -0400, Simo Sorce wrote:
 
 This patch allows Windows to send us TGTs using AES.
 
 Simo.
 
 -- 
 Simo Sorce * Red Hat, Inc. * New York

(sorry for the long delay)

ACK, patch is working as expected with w2k8r2.

bye,
Sumit

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


Re: [Freeipa-devel] [PATCH] support AES for cross-realm TGTs

2012-10-17 Thread Martin Kosek
On 10/17/2012 01:29 PM, Sumit Bose wrote:
 On Wed, Sep 26, 2012 at 06:36:40PM -0400, Simo Sorce wrote:

 This patch allows Windows to send us TGTs using AES.

 Simo.

 -- 
 Simo Sorce * Red Hat, Inc. * New York
 
 (sorry for the long delay)
 
 ACK, patch is working as expected with w2k8r2.
 
 bye,
 Sumit

Pushed to master, ipa-3-0.

Martin

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


Re: [Freeipa-devel] [PATCH] Fix various issues found by Coverity

2012-10-17 Thread Alexander Bokovoy

On Tue, 02 Oct 2012, Sumit Bose wrote:

Hi,

this patch fixes a couple of resource leaks and unchecked return and an
uninitialised value found by Coverity.

ACK.


--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 87 extdom: handle INP_POSIX_UID and INP_POSIX_GID requests

2012-10-17 Thread Alexander Bokovoy

On Thu, 11 Oct 2012, Sumit Bose wrote:

Hi,

I found this issue while working on a related sssd bug
https://fedorahosted.org/sssd/ticket/1561 .

This patch allows the clients to send a request map a UID or GID for a
trusted user to the name of the user. To achieve this the Posix ID is
mapped to the corresponding SID and then the SID is looked up.

FreeIPA ticket is https://fedorahosted.org/freeipa/ticket/3166 .

ACK.


--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH 0019] Forbid overlapping primary and secondary rid ranges

2012-10-17 Thread Sumit Bose
On Wed, Oct 17, 2012 at 12:59:52PM +0200, Tomas Babej wrote:
 On 10/17/2012 11:14 AM, Sumit Bose wrote:
 On Tue, Oct 16, 2012 at 02:26:24PM +0200, Tomas Babej wrote:
 Hi,
 
 commands ipa idrange-add / idrange-mod no longer allows the user
 to enter primary or secondary rid range such that has non-zero
 intersection with primary or secondary rid range of another
 existing id range, as this could cause collision.
 
 Unit tests added to test_range_plugin.py
 
 https://fedorahosted.org/freeipa/ticket/3086
 
 Tomas
 Thank you for the patch, comments are in-line.
 
 bye,
 Sumit
 

 Thank you for your suggestions. New version of the patch attached.
 
 Tomas

Thank you for addressing my comments. I just realized that the check is
too strict.

The ranges of the Posix IDs [base_id - base_id+id_range_size) may not
overlap for any existing range because those IDs belong to the single
Posix ID namespace of the IPA domain. I.e each user, local or from a
trusted domain, must have a unique Posix ID.

The RID ranges [base_rid, base_rid+id_range_size) and
[secondary_base_rid, secondary_base_rid+id_range_size) may not overlap
with RID ranges from the same domain. So the RID ranges for the local
domain may not overlap and the RID ranges for any specific trusted
domain may not overlap. It is allowed that there is a range form the
local domain may have base_rid=1000 and a range from a trusted domain as
well. This is ok because the RID is only part of the identifier, each
domain has a unique domain SID which is used together with the RID to
identify e.g. a user.

I would suggest to look for the ipaNTTrustedDomainSID attribute in
slapi_entry_to_range_info() too and add it to struct range_info. In
ranges_overlap() you can then check the Posix ID range for all ranges
but do the RID checks only when the domain identifiers are either both
NULL (local IPA domain) or are the same strings.

Sorry for not seeing this earlier.

bye,
Sumit

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


Re: [Freeipa-devel] [PATCH] Fix various issues found by Coverity

2012-10-17 Thread Martin Kosek
On 10/17/2012 02:14 PM, Alexander Bokovoy wrote:
 On Tue, 02 Oct 2012, Sumit Bose wrote:
 Hi,

 this patch fixes a couple of resource leaks and unchecked return and an
 uninitialised value found by Coverity.
 ACK.
 

Pushed to master, ipa-3-0.

Martin

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


Re: [Freeipa-devel] [PATCH 0019] Forbid overlapping primary and secondary rid ranges

2012-10-17 Thread Tomas Babej

On 10/17/2012 02:34 PM, Sumit Bose wrote:

On Wed, Oct 17, 2012 at 12:59:52PM +0200, Tomas Babej wrote:

On 10/17/2012 11:14 AM, Sumit Bose wrote:

On Tue, Oct 16, 2012 at 02:26:24PM +0200, Tomas Babej wrote:

Hi,

commands ipa idrange-add / idrange-mod no longer allows the user
to enter primary or secondary rid range such that has non-zero
intersection with primary or secondary rid range of another
existing id range, as this could cause collision.

Unit tests added to test_range_plugin.py

https://fedorahosted.org/freeipa/ticket/3086

Tomas

Thank you for the patch, comments are in-line.

bye,
Sumit




Thank you for your suggestions. New version of the patch attached.

Tomas

Thank you for addressing my comments. I just realized that the check is
too strict.

The ranges of the Posix IDs [base_id - base_id+id_range_size) may not
overlap for any existing range because those IDs belong to the single
Posix ID namespace of the IPA domain. I.e each user, local or from a
trusted domain, must have a unique Posix ID.

The RID ranges [base_rid, base_rid+id_range_size) and
[secondary_base_rid, secondary_base_rid+id_range_size) may not overlap
with RID ranges from the same domain. So the RID ranges for the local
domain may not overlap and the RID ranges for any specific trusted
domain may not overlap. It is allowed that there is a range form the
local domain may have base_rid=1000 and a range from a trusted domain as
well. This is ok because the RID is only part of the identifier, each
domain has a unique domain SID which is used together with the RID to
identify e.g. a user.

I would suggest to look for the ipaNTTrustedDomainSID attribute in
slapi_entry_to_range_info() too and add it to struct range_info. In
ranges_overlap() you can then check the Posix ID range for all ranges
but do the RID checks only when the domain identifiers are either both
NULL (local IPA domain) or are the same strings.

Sorry for not seeing this earlier.

bye,
Sumit


Thanks for catching this issue. It is solved in the newest revision
of the patch.

Tomas
From dab63f5d42e53218a0611c82a1cb0768ad4be17f Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 15 Oct 2012 06:28:16 -0400
Subject: [PATCH] Forbid overlapping primary and secondary rid ranges

Commands ipa idrange-add / idrange-mod no longer allows the user
to enter primary or secondary rid range such that has non-zero
intersection with primary or secondary rid range of another
existing id range, as this could cause collision.

Unit tests added to test_range_plugin.py

https://fedorahosted.org/freeipa/ticket/3086
---
 .../ipa-range-check/ipa_range_check.c  | 114 +---
 tests/test_xmlrpc/test_range_plugin.py | 120 +++--
 2 files changed, 212 insertions(+), 22 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
index 499e54a9c4a4c9134a231c0cd09e700390565a14..b866259134658da77aff3760b872acfe4ed5a5fe 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
@@ -49,6 +49,7 @@
 #define IPA_ID_RANGE_SIZE ipaIDRangeSize
 #define IPA_BASE_RID ipaBaseRID
 #define IPA_SECONDARY_BASE_RID ipaSecondaryBaseRID
+#define IPA_DOMAIN_ID ipaNTTrustedDomainSID
 #define RANGES_FILTER objectclass=ipaIDRange
 
 #define IPA_PLUGIN_NAME ipa-range-check
@@ -70,6 +71,7 @@ struct ipa_range_check_ctx {
 
 struct range_info {
 char *name;
+char *domain_id;
 uint32_t base_id;
 uint32_t id_range_size;
 uint32_t base_rid;
@@ -93,6 +95,8 @@ static int slapi_entry_to_range_info(struct slapi_entry *entry,
 return EINVAL;
 }
 
+range-domain_id = slapi_entry_attr_get_charptr(entry, IPA_DOMAIN_ID);
+
 ul_val = slapi_entry_attr_get_ulong(entry, IPA_BASE_ID);
 if (ul_val == 0 || ul_val = UINT32_MAX) {
 ret = ERANGE;
@@ -132,24 +136,81 @@ done:
 return ret;
 }
 
-#define IN_RANGE(x,base,size) ( (x) = (base)  ((x) - (base))  (size) )
-static bool ranges_overlap(struct range_info *r1, struct range_info *r2)
+#define IN_RANGE(x,base,size) ( (x) = (base)  ((x) - (base)  (size)) )
+static bool intervals_overlap(uint32_t x, uint32_t base, uint32_t x_size, uint32_t base_size)
 {
-if (r1-name != NULL  r2-name != NULL 
-strcasecmp(r1-name, r2-name) == 0) {
-return false;
-}
-
-if (IN_RANGE(r1-base_id, r2-base_id, r2-id_range_size) ||
-IN_RANGE((r1-base_id + r1-id_range_size - 1), r2-base_id, r2-id_range_size) ||
-IN_RANGE(r2-base_id, r1-base_id, r1-id_range_size) ||
-IN_RANGE((r2-base_id + r2-id_range_size - 1), r1-base_id, r1-id_range_size)) {
+if (IN_RANGE(x, base, base_size) ||
+IN_RANGE((x + x_size - 1), base, base_size) ||
+IN_RANGE(base, x, x_size) ||
+IN_RANGE((base + base_size - 1), x, x_size)) {
 return true;
 }
 
 return 

Re: [Freeipa-devel] [PATCH] convert the base platform modules into packages

2012-10-17 Thread Petr Viktorin

On 09/21/2012 04:57 PM, Timo Aaltonen wrote:

Ok, so this is the first step before we can start to rewrite bits from
ipaserver/install to make them support other distros. There are no real
functional changes yet.

had some dependency issues installing the resulting rpm's, so didn't
test the install scripts but they should work :)




Hello,

I recommend giving the -M flag to git format-patch, so it's easier to 
see changes in the patch.



Your split of the fedora16 code into two modules is unfortunate: each 
tries to import the other one, and one is the other's parent. This would 
need special care to get working correctly.


The best option here would probably be to put restore_context  
check_selinux_status into a separate submodule, so you don't need to 
import fedora16 from services.


Furthermore, in fedora16/__init__.py, you have:
from ipapython.platform.fedora16.service import *
This imports everything from that module, including e.g. redhat or os.
Please avoid star imports. List all the imported names explicitly, or 
import the module and then use qualified names.



Other than that, after a trivial rebase the patch seems to work fine on 
Fedora. Thanks!



--
Petr³

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


Re: [Freeipa-devel] Hide private symbols in the bind-dyndb-ldap

2012-10-17 Thread Simo Sorce
On Wed, 2012-10-17 at 13:04 +0200, Adam Tkac wrote:
 Hello,
 
 attached patch hides all symbols except dynamic_driver_{init,destroy}. 
 Feedback
 is appreciated.

Any reason not to use a simple export file ?
Anyway strong ACK, keeping private symbols private is good hygiene.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

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


Re: [Freeipa-devel] Hide private symbols in the bind-dyndb-ldap

2012-10-17 Thread Adam Tkac
On Wed, Oct 17, 2012 at 09:58:36AM -0400, Simo Sorce wrote:
 On Wed, 2012-10-17 at 13:04 +0200, Adam Tkac wrote:
  Hello,
  
  attached patch hides all symbols except dynamic_driver_{init,destroy}. 
  Feedback
  is appreciated.
 
 Any reason not to use a simple export file ?

This is also possible solution. However if I understand GNU build chain
correctly, using export file only affects linker and doesn't allow compiler to
perform more aggressive optimisations (i.e. inline hidden functions etc), does
it?

 Anyway strong ACK, keeping private symbols private is good hygiene.

Thanks, pushed to master.

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.

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


[Freeipa-devel] [PATCH 0020] Refactoring of default.conf man page

2012-10-17 Thread Tomas Babej

Hi,

Description for the 'server' and 'wait_for_attr' option has been
added. Option 'server' has been marked as deprecated, as it is not
used anywhere in IPA code. All the options have been sorted
lexicographically.

Please provide feedback for added descriptions:

+.TP
+.B server hostname
+Specifies the IPA Server hostname. This option is deprecated.

+.TP
+.B wait_for_attr boolean
+Debug option. Waits for asynchronous execution of 389-ds
postoperation plugins before returning data to the client,
therefore data added by postoperation plugins is included
in the result. Increases execution time.

The rest of the patch is just sorting options lexicographically.

Tomas
From 0ad81fd6cfca017631c705465f940a9b461a52ce Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 17 Oct 2012 08:27:26 -0400
Subject: [PATCH] Refactoring of default.conf man page

Description for the 'server' and 'wait_for_attr' option has been
added. Option 'server' has been marked as deprecated, as it is not
used anywhere in IPA code. All the options have been sorted
lexicographically.

https://fedorahosted.org/freeipa/ticket/3071
---
 ipa-client/man/default.conf.5 | 80 +++
 1 file changed, 43 insertions(+), 37 deletions(-)

diff --git a/ipa-client/man/default.conf.5 b/ipa-client/man/default.conf.5
index fb913e6cca79284fec9d2e3fab77fd47b4b8e48f..37e6cd2dc33e4ae47ab91334280480f5e5e7d3fa 100644
--- a/ipa-client/man/default.conf.5
+++ b/ipa-client/man/default.conf.5
@@ -71,21 +71,42 @@ Specifies the secure CA agent port. The default is 9443 for Dogtag 9, and 8443 f
 .B ca_ee_port port
 Specifies the secure CA end user port. The default is 9444 for Dogtag 9, and 8443 for Dogtag 10.
 .TP
-.B ca_port port
-Specifies the insecure CA end user port. The default is 9180 for Dogtag 9, and 8080 for Dogtag 10.
-.TP
 .B ca_host hostname
 Specifies the hostname of the dogtag CA server. The default is the hostname of the IPA server.
 .TP
+.B ca_port port
+Specifies the insecure CA end user port. The default is 9180 for Dogtag 9, and 8080 for Dogtag 10.
+.TP
 .B context context
 Specifies the context that IPA is being executed in. IPA may operate differently depending on the context. The current defined contexts are cli and server. Additionally this value is used to load /etc/ipa/\fBcontext\fR.conf to provide context\-specific configuration. For example, if you want to always perform client requests in verbose mode but do not want to have verbose enabled on the server, add the verbose option to \fI/etc/ipa/cli.conf\fR.
 .TP
-.B verbose boolean
-When True provides more information. Specifically this sets the global log level to info.
-.TP
 .B debug boolean
 When True provides detailed information. Specifically this set the global log level to debug. Default is False.
 .TP
+.B domain domain
+The domain of the IPA server e.g. example.com.
+.TP
+.B enable_ra boolean
+Specifies whether the CA is acting as an RA agent, such as when dogtag is being used as the Certificate Authority. This setting only applies to the IPA server configuration.
+.TP
+.B fallback boolean
+Specifies whether an IPA client should attempt to fall back and try other services if the first connection fails.
+.TP
+.B host hostname
+Specifies the hostname of the IPA server. This value is used to construct URL values on the client and server.
+.TP
+.B in_server boolean
+Specifies whether requests should be forwarded to an IPA server or handled locally. This is used internally by IPA in a similar way as context. The same IPA framework is used by the ipa command\-line tool and the server. This setting tells the framework whether it should execute the command as if on the server or forward it via XML\-RPC to a remote server.
+.TP
+.B in_tree  boolean
+This is used in development and is generally a detected value. It means that the code is being executed within a source tree.
+.TP
+.B interactive boolean
+Specifies whether values should be prompted for or not. The default is True.
+.TP
+.B ldap_uri URI
+Specifies the URI of the IPA LDAP server to connect to. The URI scheme may be one of \fBldap\fR or \fBldapi\fR. The default is to use ldapi, e.g. ldapi://%2fvar%2frun%2fslapd\-EXAMPLE\-COM.socket
+.TP
 .B log_logger_XXX comma separated list of regexps
 loggers matching regexp will be assigned XXX level.
 .IP
@@ -118,32 +139,8 @@ expression metacharacter (matches any character) therefore you
 will usually need to escape the dot in the logger names by
 preceeding it with a backslash.
 .TP
-.B domain domain
-The domain of the IPA server e.g. example.com.
-.TP
-.B enable_ra boolean
-Specifies whether the CA is acting as an RA agent, such as when dogtag is being used as the Certificate Authority. This setting only applies to the IPA server configuration.
-.TP
-.B fallback boolean
-Specifies whether an IPA client should attempt to fall back and try other services if the first connection fails.
-.TP
-.B host hostname
-Specifies the hostname of the 

Re: [Freeipa-devel] Hide private symbols in the bind-dyndb-ldap

2012-10-17 Thread Simo Sorce
On Wed, 2012-10-17 at 17:06 +0200, Adam Tkac wrote:
 On Wed, Oct 17, 2012 at 09:58:36AM -0400, Simo Sorce wrote:
  On Wed, 2012-10-17 at 13:04 +0200, Adam Tkac wrote:
   Hello,
   
   attached patch hides all symbols except dynamic_driver_{init,destroy}. 
   Feedback
   is appreciated.
  
  Any reason not to use a simple export file ?
 
 This is also possible solution. However if I understand GNU build chain
 correctly, using export file only affects linker and doesn't allow compiler to
 perform more aggressive optimisations (i.e. inline hidden functions etc), does
 it?

It's linker only indeed, good point, I'll keep that in mind.

  Anyway strong ACK, keeping private symbols private is good hygiene.
 
 Thanks, pushed to master.
 
 Regards, Adam
 


-- 
Simo Sorce * Red Hat, Inc * New York

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


Re: [Freeipa-devel] Hide private symbols in the bind-dyndb-ldap

2012-10-17 Thread Petr Spacek

On 10/17/2012 05:16 PM, Simo Sorce wrote:

On Wed, 2012-10-17 at 17:06 +0200, Adam Tkac wrote:

On Wed, Oct 17, 2012 at 09:58:36AM -0400, Simo Sorce wrote:

On Wed, 2012-10-17 at 13:04 +0200, Adam Tkac wrote:

Hello,

attached patch hides all symbols except dynamic_driver_{init,destroy}. Feedback
is appreciated.


I'm perfectly okay with that, but I haven't time for exploring it deeply today.

Petr^2 Spacek



Any reason not to use a simple export file ?


This is also possible solution. However if I understand GNU build chain
correctly, using export file only affects linker and doesn't allow compiler to
perform more aggressive optimisations (i.e. inline hidden functions etc), does
it?


It's linker only indeed, good point, I'll keep that in mind.


Anyway strong ACK, keeping private symbols private is good hygiene.


Thanks, pushed to master.

Regards, Adam


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


[Freeipa-devel] [PATCH] 1064 Improve error messages during ipa-replica-manage

2012-10-17 Thread Rob Crittenden

Make some of the errors in ipa-replica-manage clearer.

See ticket for more reproduction details.

rob
From f676e25754b3194fc03b9404239c0a51094b44d1 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Wed, 17 Oct 2012 11:54:14 -0400
Subject: [PATCH] Improve error messages in ipa-replica-manage.

Correctly handle case where we bind using GSSAPI with an unauthorized user.

Remove extraneous except clause. We now have handle for LDAP errors.

Make it explicit in a few places what server we can't connect to.

When the remote replica is down and we are forcing its removal, remove
a duplicate entry from the list of servers to remove.

https://fedorahosted.org/freeipa/ticket/2871
---
 install/tools/ipa-replica-manage | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 897d117681d3e1559d5710366101b50540b705c8..a62974a0f909cb6d45943dc42e4e446d8ee2b207 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -129,6 +129,10 @@ def test_connection(realm, host):
 return True
 except ldap.LOCAL_ERROR:
 return False
+except errors.NotFound:
+# We do a search in cn=config. NotFound in this case means no
+# permission
+return False
 
 def list_replicas(realm, host, replica, dirman_passwd, verbose):
 
@@ -267,11 +271,6 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
 repl2.delete_agreement(replica1)
 repl2.delete_referral(replica1)
 repl2.set_readonly(readonly=False)
-except ldap.LDAPError, e:
-desc = e.args[0]['desc'].strip()
-info = e.args[0].get('info', '').strip()
-print Unable to remove agreement on %s: %s: %s % (replica2, desc, info)
-failed = True
 except Exception, e:
 print Unable to remove agreement on %s: %s % (replica2, convert_error(e))
 failed = True
@@ -547,11 +546,12 @@ def del_master(realm, hostname, options):
 try:
 delrepl = replication.ReplicationManager(realm, hostname, options.dirman_passwd)
 except Exception, e:
+print Connection to '%s' failed: %s % (hostname, convert_error(e))
 if not options.force:
-print Unable to delete replica %s: %s % (hostname, convert_error(e))
+print Unable to delete replica '%s' % hostname
 sys.exit(1)
 else:
-print Unable to connect to replica %s, forcing removal % hostname
+print Forcing removal of %s % hostname
 force_del = True
 
 if force_del:
@@ -560,6 +560,12 @@ def del_master(realm, hostname, options):
 replica_names = []
 for entry in entries:
 replica_names.append(entry.getValue('cn'))
+# The host we're removing gets included in this list, remove it.
+# Otherwise we try to delete an agreement from the host to itself.
+try:
+replica_names.remove(hostname)
+except ValueError:
+pass
 else:
 # Get list of agreements.
 replica_names = delrepl.find_ipa_replication_agreements()
@@ -611,7 +617,7 @@ def del_master(realm, hostname, options):
 if not del_link(realm, r, hostname, options.dirman_passwd, force=True):
 print Unable to remove replication agreement for %s from %s. % (hostname, r)
 except Exception, e:
-print There were issues removing a connection: %s % convert_error(e)
+print There were issues removing a connection for %s from %s: %s % (hostname, r, convert_error(e))
 
 # 5. Clean RUV for the deleted master
 if repltype == replication.IPA_REPLICA:
-- 
1.7.12.1

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

Re: [Freeipa-devel] [PATCH 0019] Forbid overlapping primary and secondary rid ranges

2012-10-17 Thread Sumit Bose
On Wed, Oct 17, 2012 at 03:29:11PM +0200, Tomas Babej wrote:
 On 10/17/2012 02:34 PM, Sumit Bose wrote:
 On Wed, Oct 17, 2012 at 12:59:52PM +0200, Tomas Babej wrote:
 On 10/17/2012 11:14 AM, Sumit Bose wrote:
 On Tue, Oct 16, 2012 at 02:26:24PM +0200, Tomas Babej wrote:
 Hi,
 
 commands ipa idrange-add / idrange-mod no longer allows the user
 to enter primary or secondary rid range such that has non-zero
 intersection with primary or secondary rid range of another
 existing id range, as this could cause collision.
 
 Unit tests added to test_range_plugin.py
 
 https://fedorahosted.org/freeipa/ticket/3086
 
 Tomas
 Thank you for the patch, comments are in-line.
 
 bye,
 Sumit
 
 
 Thank you for your suggestions. New version of the patch attached.
 
 Tomas
 Thank you for addressing my comments. I just realized that the check is
 too strict.
 
 The ranges of the Posix IDs [base_id - base_id+id_range_size) may not
 overlap for any existing range because those IDs belong to the single
 Posix ID namespace of the IPA domain. I.e each user, local or from a
 trusted domain, must have a unique Posix ID.
 
 The RID ranges [base_rid, base_rid+id_range_size) and
 [secondary_base_rid, secondary_base_rid+id_range_size) may not overlap
 with RID ranges from the same domain. So the RID ranges for the local
 domain may not overlap and the RID ranges for any specific trusted
 domain may not overlap. It is allowed that there is a range form the
 local domain may have base_rid=1000 and a range from a trusted domain as
 well. This is ok because the RID is only part of the identifier, each
 domain has a unique domain SID which is used together with the RID to
 identify e.g. a user.
 
 I would suggest to look for the ipaNTTrustedDomainSID attribute in
 slapi_entry_to_range_info() too and add it to struct range_info. In
 ranges_overlap() you can then check the Posix ID range for all ranges
 but do the RID checks only when the domain identifiers are either both
 NULL (local IPA domain) or are the same strings.
 
 Sorry for not seeing this earlier.
 
 bye,
 Sumit
 
 Thanks for catching this issue. It is solved in the newest revision
 of the patch.
 
 Tomas

sorry, found another one ...

...
 +static int ranges_overlap(struct range_info *r1, struct range_info *r2)
 +{
 +if (r1-name != NULL  r2-name != NULL 
 +strcasecmp(r1-name, r2-name) == 0) {
 +return 0;
 +}
 +
 +/* check if base range overlaps with existing base range */
 +if (intervals_overlap(r1-base_id, r2-base_id,
 +r1-id_range_size, r2-id_range_size)){
 +return 1;
 +}
 +
 +/* if both base_rid and secondary_base_rid = 0, the rid range is not set 
 */
 +bool rid_ranges_set = (r1-base_rid != 0 || r1-secondary_base_rid != 0) 
 
 +  (r2-base_rid != 0 || r2-secondary_base_rid != 0);
 +
 +bool ranges_from_same_domain =
 + (r1-domain_id == NULL  r2-domain_id == NULL) ||
 + (strcasecmp(r1-domain_id, r2-domain_id) == 0);
 +

you have to check that both domain_id are not NULL before calling
strcasecmp.

bye,
Sumit

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


Re: [Freeipa-devel] [PATCH 75] log dogtag errors

2012-10-17 Thread John Dennis

On 10/12/2012 04:35 AM, Petr Viktorin wrote:

On 10/11/2012 06:53 PM, John Dennis wrote:

On 04/28/2012 09:50 AM, John Dennis wrote:

On 04/27/2012 04:45 AM, Petr Viktorin wrote:

On 04/20/2012 08:07 PM, John Dennis wrote:

Ticket #2622

If we get an error from dogtag we always did raise a
CertificateOperationError exception with a message describing the
problem. Unfortuanately that error message did not go into the log,
just sent back to the caller. The fix is to format the error message
and send the same message to both the log and use it to initialize the
CertificateOperationError exception.



The patch contains five hunks with almost exactly the same code,
applying the same changes in each case.
Wouldn't it make sense to move the _sslget call, parsing, and error
handling to a common method?



Yes it would and ordinarily I would have taken that approach. However on
IRC (or phone?) with Rob we decided not to perturb the code too much for
this particular issue because we intend to refactor the code later. This
was one of the last patches destined for 2.2 which is why we took the
more conservative approach.



I went back and looked at this. It's not practical to collapse
everything into a common subroutine unless you paramaterize the heck out
of a common subroutine. That's because all the patched locations have
subtly different things going on, different parameters to sslget
followed by different result parsing and handling. In retrospect I think
it's clearer to keep things separate rather than one subroutine that
needs a lot of parameters and/or convoluted logic to handle each unique
case.


I don't agree that it's clearer, but I guess that's debatable.


Part of the problem is the dogtag interface. Every command has the
potential to behave differently making it difficult to work with. I
wrote this code originally and got it reduced to as many common parts as
I could. At some point soon we'll be switching to a new dogtag REST
interface which hopefully will allow for greater commonality due to
interface consistency.

In summary: I still stand by the original patch.



However, I see no reason to not use a method such as:

def raise_certop_error(self, method_name, error=None, detail=None):
  Log and raise a CertificateOperationError

  :param method_name: Name of the method in which the error occured
  :param error: Error string. If None, Unable to communicate with
  CMS is used.
  :param detail: Detailed or low-level information. Will be put in
  parentheses.
  

to at least get rid of the repetition this patch is adding - almost the
same format+log+raise sequence is used twice in each of the five hunks.



Added a utility function as suggested. Revised patch attached.
--
John Dennis jden...@redhat.com

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
From 48f1560745cfa0281a387aacf0737841bb78b065 Mon Sep 17 00:00:00 2001
From: John Dennis jden...@redhat.com
Date: Wed, 17 Oct 2012 13:29:13 -0400
Subject: [PATCH 75-1] log dogtag errors
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

If we get an error from dogtag we always did raise a
CertificateOperationError exception with a message describing the
problem. Unfortuanately that error message did not go into the log,
just sent back to the caller. The fix is to format the error message
and send the same message to both the log and use it to initialize the
CertificateOperationError exception. This is done in the utility
method raise_certificate_operation_error().

https://fedorahosted.org/freeipa/ticket/2622
---
 ipaserver/plugins/dogtag.py | 68 -
 1 file changed, 48 insertions(+), 20 deletions(-)

diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index baa41ad..66fef57 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1233,6 +1233,27 @@ class ra(rabase.rabase):
 self.password = ''
 super(ra, self).__init__()
 
+def raise_certificate_operation_error(self, func_name, err_msg=None, detail=None):
+
+:param func_name: function name where error occurred
+
+:param err_msg:   diagnostic error message, if not supplied it will be
+  'Unable to communicate with CMS'
+:param detail:extra information that will be appended to err_msg
+  inside a parenthesis
+
+Raise a CertificateOperationError and log the error message.
+
+
+if err_msg is None:
+err_msg = _('Unable to communicate with CMS')
+
+if detail is not None:
+err_msg += ' (%s)' % detail
+
+self.error('%s.%s(): %s', self.fullname, func_name, err_msg)
+raise CertificateOperationError(error=err_msg)
+
 def _host_has_service(self, host, service='CA'):
 
 :param host: A host which might be a master for a service.
@@ -1376,14 +1397,15 @@ class 

Re: [Freeipa-devel] Unit tests failing on F18

2012-10-17 Thread Rob Crittenden

Martin Kosek wrote:

Hello,

I was investigating global unit test failure on Fedora 18 for most of today, I
would like to share results I found so far.

Unit test and its related scripts on F18 now reports NSS BUSY exception, just
like this one:

# ./make-testcert
Traceback (most recent call last):
   File ./make-testcert, line 134, in module
 sys.exit(makecert(reqdir))
   File ./make-testcert, line 111, in makecert
 add=True)
   File ./make-testcert, line 68, in run
 result = self.execute(method, *args, **options)
   File /root/freeipa-master2/ipalib/backend.py, line 146, in execute
 raise error #pylint: disable=E0702
ipalib.errors.NetworkError: cannot connect to
'http://vm-042.idm.lab.bos.redhat.com/ipa/session/xml': [Errno -8053]
(SEC_ERROR_BUSY) NSS could not shutdown. Objects are still in use.

Something In F18 must have changed, this worked before... But leaked
NSSConnection objects without proper close() now ends with the exception above.

In case of make-testcert script, the exception is raised because the script
does the following procedure:

1) connect, do one command
2) disconnect
3) connect, do second command

However, during disconnect, NSSConnection is leaked which makes NSS very
uncomfortable during second connection atempt (and nss_shutdown()). I managed
to fix this issue with attached patch. ./make-testcert or ./make-test
tests/test_xmlrpc/test_group_plugin.py works fine now.

But global ./make-test still fails, I think there is some remaining
NSSConnection leak, I suspect there is something wrong with how we use our
context (threading.local object). It looses a connection or some other thread
invoked in ldap2 module may be kicking in, here is my debug output:

CONTEXT[xmlclient] = ipalib.request.Connection object at 0x9a1f5ec

Test a simple LDAP bind using ldap2 ... SKIP: No directory manager password in
/root/.ipa/.dmpw
Test the `ipaserver.rpcserver.jsonserver.unmarshal` method. ... ok
tests.test_ipaserver.test_rpcserver.test_session.test_mount ... CONTEXT
150714476: GET languages

CONTEXT[xmlclient] = None

The connection is in the context, but then something happens and it is gone.
Then, unit tests try to connect again and NSS fails.

I would be really glad if somebody with a knowledge of NSS or how threads in
Python/IPA work could give me some advice...

Thanks!
Martin


I built upon your patch and have something that seems to work at least 
somewhat. I'm getting some unexpected test failures when running the 
entire suite but no NSS shutdown errors. I haven't had a chance to 
really investigate everything yet, sending this out as a 
work-in-progress in case you want to take a look.


rob

From ad3f420395c8e4fc24d9ab3aa6f53641f188efc6 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Wed, 17 Oct 2012 16:58:54 -0400
Subject: [PATCH] candidate for fixing test execution against httpd

---
 ipalib/rpc.py   | 22 +++---
 ipapython/nsslib.py |  6 ++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index e97536d..6de45cf 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -257,6 +257,9 @@ class SSLTransport(LanguageAwareTransport):
 # If we an existing connection exists using the same NSS database
 # there is no need to re-initialize. Pass thsi into the NSS
 # connection creator.
+if self._connection and host == self._connection[0]:
+return self._connection[1]
+
 dbdir = '/etc/pki/nssdb'
 no_init = self.__nss_initialized(dbdir)
 (major, minor, micro, releaselevel, serial) = sys.version_info
@@ -265,8 +268,10 @@ class SSLTransport(LanguageAwareTransport):
 else:
 conn = NSSConnection(host, 443, dbdir=dbdir, no_init=no_init)
 self.dbdir=dbdir
+
 conn.connect()
-return conn
+self._connection = host, conn
+return self._connection[1]
 
 
 class KerbTransport(SSLTransport):
@@ -331,6 +336,13 @@ class KerbTransport(SSLTransport):
 
 return (host, extra_headers, x509)
 
+
+def single_request(self, host, handler, request_body, verbose=0):
+try:
+return SSLTransport.single_request(self, host, handler, request_body, verbose)
+finally:
+self.close()
+
 def parse_response(self, response):
 session_cookie = response.getheader('Set-Cookie')
 if session_cookie:
@@ -371,7 +383,8 @@ class xmlclient(Connectible):
 
 if not hasattr(self.conn, '_ServerProxy__transport'):
 return None
-if type(self.conn._ServerProxy__transport) in (KerbTransport, DelegatedKerbTransport):
+if (isinstance(self.conn._ServerProxy__transport, KerbTransport) or
+isinstance(self.conn._ServerProxy__transport, DelegatedKerbTransport)):
 scheme = https
 else:
 scheme = http
@@ -493,7 +506,10 @@ class xmlclient(Connectible):
 return serverproxy
 

[Freeipa-devel] using 389-ds-base with betxn plugins enabled

2012-10-17 Thread Rich Megginson
I'm testing with f18, freeipa-server 3.0.0, 389-ds-base-1.3.0.a1, with 
betxn manually enabled in all plugins in 389.  I did an ipa-server-install.


I have ipa user-add --all --raw working - it returns the mep and 
memberof attributes immediately.  I had to do something like this:


diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 5d667dc..5a490bb 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -568,6 +568,11 @@ class user_add(LDAPCreate):
 newentry = wait_for_value(ldap, dn, 'objectclass', 
'mepOriginEntry')

 entry_from_entry(entry_attrs, newentry)

+if not self.api.env.wait_for_attr:
+# have to update memberof, mep data in entry to return
+(newdn, newentry) = ldap.get_entry(dn, ['*'])
+entry_attrs.update(newentry)
+
 if options.get('random', False):
 try:
 entry_attrs['randompassword'] = 
unicode(getattr(context, 'randompassword'))


That is, after user_add.post_callback adds the user to the group, it 
needs to get the updated memberof attribute from the user entry, as well 
as the mep data.  I think there are several other places in the code 
where wait_for_attr and wait_for_attr_memberof are used, that will have 
to change in a similar manner.  I don't know if this patch is the best 
way to solve the problem - I suppose it would be better to update only 
the memberof and objectclass and mepmanagedentry attributes in 
entry_attrs, but I'm not sure how to do that.


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