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

2012-10-22 Thread Tomas Babej

On 10/18/2012 05:14 PM, Rob Crittenden wrote:

Tomas Babej wrote:

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


Can you add startup_timeout as well? This controls the amount of time 
we wait when starting a service. The default is 2 minutes.


And dogtag_version?

The changes you made look good.

rob



The descriptions for the requested options have been added:

+.B dogtag_version version
+Stores the version of Dogtag. Value 9 is assumed if not specified 
otherwise.


+.B startup_timeout time in seconds
+Controls the amount of time waited when starting a service. The default 
value is 120 seconds.


Tomas


From 0b6a755a50de7a632ce8ca0a84bdf9f564e472f8 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 'dogtag_version', 'startup_timeout', 'server',
'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 | 86 ---
 1 file changed, 49 insertions(+), 37 deletions(-)

diff --git a/ipa-client/man/default.conf.5 b/ipa-client/man/default.conf.5
index fb913e6cca79284fec9d2e3fab77fd47b4b8e48f..f779b51f53c1d181c954d90e686c945f62e5213a 100644
--- a/ipa-client/man/default.conf.5
+++ b/ipa-client/man/default.conf.5
@@ -71,21 +71,45 @@ 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 dogtag_version version
+Stores the version of Dogtag. Value 9 is assumed if not specified otherwise.
+.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 

Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name

2012-10-22 Thread Petr Spacek

On 10/19/2012 03:46 PM, Rob Crittenden wrote:

Petr Spacek wrote:

On 10/19/2012 03:10 PM, Rob Crittenden wrote:

Petr Spacek wrote:

On 10/19/2012 10:10 AM, Martin Kosek wrote:

On 10/18/2012 09:42 PM, Rob Crittenden wrote:

We were seeing a unicode failure when trying to request a certificate
with
subject alt names. This one-liner should fix it.

rob



Yup, this fixes it, works fine on --selfsign IPA CA too.

Just when testing your patch, I found out we don't treat some non-DNS
subject
alternative name well, e.g. email extension, an we try to match it
with our hosts:

Certificate Request:
...
 Attributes:
 Requested Extensions:
 X509v3 Subject Alternative Name:
 email:f...@testcert.example.com, DNS:web.example.com
...

cert-request result:

ipa: ERROR: no host record for subject alt name
f...@testcert.example.com in
certificate request


IMHO there should be a --force option. SAN can contain a lot of
different things. Also, we can't assume that we manage the whole world
... (now :-))



The intention was just to provide support for DNS alt names. I don't
think
requiring a host entry exist for any alt hosts is asking too much.

I think a new ticket should be opened to support non-DNS alt names.


IMHO SAN names usually contain a lot of virtual names like
www.shop1.com, ftp.shop1.com, etc.

These names are usually CNAMEs to real name like srv1.shop1.com. In
that case host object doesn't make sense. (But SAN is required for
proper certificate validation.)


The purpose is so we more tightly control was certificates are issued by our
CA because we automatically issue them.

rob


We discussed this check in Brno a bit and we found a problem:

1) Each SAN has to have corresponding host object (in current implementation)
2) Subject Alternative Name = other name for existing object
   E.g. Name www.shop1.com is CNAME for srv1.shop1.com
3) Host object makes sense for real host, not for alias like www
4) Implication from points 1-3 = Current check can't pass in reasonable case, 
because aliases do not have own host objects.


Reasonable check can be: There have to be host object for at least one name 
stated in certificate CN or SAN. (IMHO equivalence between CN and host object 
name would be probably better...)


Does it make sense?

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name

2012-10-22 Thread Martin Kosek
On 10/22/2012 12:40 PM, Petr Spacek wrote:
 On 10/19/2012 03:46 PM, Rob Crittenden wrote:
 Petr Spacek wrote:
 On 10/19/2012 03:10 PM, Rob Crittenden wrote:
 Petr Spacek wrote:
 On 10/19/2012 10:10 AM, Martin Kosek wrote:
 On 10/18/2012 09:42 PM, Rob Crittenden wrote:
 We were seeing a unicode failure when trying to request a certificate
 with
 subject alt names. This one-liner should fix it.

 rob


 Yup, this fixes it, works fine on --selfsign IPA CA too.

 Just when testing your patch, I found out we don't treat some non-DNS
 subject
 alternative name well, e.g. email extension, an we try to match it
 with our hosts:

 Certificate Request:
 ...
  Attributes:
  Requested Extensions:
  X509v3 Subject Alternative Name:
  email:f...@testcert.example.com, DNS:web.example.com
 ...

 cert-request result:

 ipa: ERROR: no host record for subject alt name
 f...@testcert.example.com in
 certificate request

 IMHO there should be a --force option. SAN can contain a lot of
 different things. Also, we can't assume that we manage the whole world
 ... (now :-))


 The intention was just to provide support for DNS alt names. I don't
 think
 requiring a host entry exist for any alt hosts is asking too much.

 I think a new ticket should be opened to support non-DNS alt names.

 IMHO SAN names usually contain a lot of virtual names like
 www.shop1.com, ftp.shop1.com, etc.

 These names are usually CNAMEs to real name like srv1.shop1.com. In
 that case host object doesn't make sense. (But SAN is required for
 proper certificate validation.)

 The purpose is so we more tightly control was certificates are issued by our
 CA because we automatically issue them.

 rob
 
 We discussed this check in Brno a bit and we found a problem:
 
 1) Each SAN has to have corresponding host object (in current implementation)
 2) Subject Alternative Name = other name for existing object
E.g. Name www.shop1.com is CNAME for srv1.shop1.com
 3) Host object makes sense for real host, not for alias like www
 4) Implication from points 1-3 = Current check can't pass in reasonable case,
 because aliases do not have own host objects.
 
 Reasonable check can be: There have to be host object for at least one name
 stated in certificate CN or SAN. (IMHO equivalence between CN and host object
 name would be probably better...)
 
 Does it make sense?
 

I am also not a big fan of current SAN check. Making sure that a host record
for the service host exists (it does, by design) is enough IMO. As Petr said,
SANs are often just aliases for the host, so no need for a host record.

But I think if we insist on these checks, we may add --force option to
workaround the check or display the validation errors only as warnings (when
the planned warning framework is ready - ticket #2732).

Martin

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


Re: [Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

2012-10-22 Thread Tomas Babej

On 10/19/2012 09:55 AM, Petr Viktorin wrote:

On 10/18/2012 08:01 PM, Rob Crittenden wrote:

Tomas Babej wrote:

On 10/02/2012 03:55 PM, Rob Crittenden wrote:

Tomas Babej wrote:

Hi,

When executing ipa-replica-manage connect to an unknown or irrelevant
master, we now print a sensible error message informing the user
about this possiblity as well.

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

Tomas


I put a whole bunch of code into a try/except and this may be catching
errors in unexpected ways.

I'm not entirely sure right now what we should do, but looking at the
code in the try:

repl1.conn.getEntry(master1_dn, ldap.SCOPE_BASE)
repl1.conn.getEntry(master2_dn, ldap.SCOPE_BASE)

We take in replica1 and replica1 as arguments (the default for
replica1 is the current host).

If either of these raise a NotFound it means there there is no master
of that name. Does that mean that the master was deleted? Well,
clearly not.

A lot has changed since I did this, I may have been relying on a
side-effect, or just hadn't tested well-enough.

I wonder if we need that message at all. Is foo is not an IPA server
good enough? It still might be confusing if someone didn't know that
foo was deleted and it was still running. We could probably verify
that it is at least an IPA server by doing similar checking in the
client, it all depends on how far we want to take it.

rob


I modified the patch. Now if the NotFound error is encountered, we try
to investigate whether we're trying to connect to an IPA server at all.
Please see if you have any suggestions.

Tomas


Getting there, the errors are a lot better.

Can you modify the 'Connection unsuccessful' message to include the
server we failed to connect to?

The logger isn't so nice either, I think I'd prefer it if we could
exclude the severity:

ipa: ERROR: LDAP Error: Connect error: TLS error -8172:Peer's
certificate issuer has been marked as not trusted by the user.
Connection unsuccessful.

So drop the 'ipa: ERROR: ' part for consistency. TI don't believe we
configure the root logger at all in this tool so it is using the 
defaults.


It's not very easy to find the right call to configure the logger to 
drop the ipa: ERROR: part:

standard_logging_setup(console_format='%(message)s')
The function is in ipapython.ipa_log_manager. Hopefully that helps.


Thanks!

I'd also replace the test for -4 with the constant
ipadiscovery.NOT_IPA_SERVER

rob




I implemented your suggestions. Please have a look at the new patch version.

Tomas
From 228840f76b94194ec1c38bbb55bdabbfe6e0d9ae Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 2 Oct 2012 09:15:33 -0400
Subject: [PATCH] IPA Server check in ipa-replica-manage

When executing ipa-replica-manage connect to an master that raises
NotFound error we now check if the master is at least IPA server.
If so, we inform the user that it is probably foreign or previously
deleted master. If not, we inform the user that the master is not
an IPA server at all.

https://fedorahosted.org/freeipa/ticket/3105
---
 install/tools/ipa-replica-manage | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 897d117681d3e1559d5710366101b50540b705c8..82740baff79048f3a18bfd3cbb78ddd2b52c0802 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -33,6 +33,7 @@ from ipalib import api, errors, util
 from ipapython.ipa_log_manager import *
 from ipapython.dn import DN
 from ipapython.config import IPAOptionParser
+from ipaclient import ipadiscovery
 
 CACERT = /etc/ipa/ca.crt
 
@@ -709,7 +710,21 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
 repl2.conn.getEntry(master2_dn, ldap.SCOPE_BASE)
 
 except errors.NotFound:
-sys.exit(You cannot connect to a previously deleted master)
+standard_logging_setup(console_format='%(message)s')
+
+ds = ipadiscovery.IPADiscovery()
+ret = ds.search(server=replica2)
+
+if ret == ipadiscovery.NOT_IPA_SERVER:
+sys.exit(Connection unsuccessful: %s is not an IPA Server. %
+replica2)
+elif ret == 0:  # success
+sys.exit(Connection unsuccessful: %s is an IPA Server, 
+but it might be unknown, foreign or previously deleted 
+one. % replica2)
+else:
+sys.exit(Connection to %s unsuccessful., replica2)
+
 repl1.setup_gssapi_replication(replica2, DN(('cn', 'Directory Manager')), dirman_passwd)
 print Connected '%s' to '%s' % (replica1, replica2)
 
-- 
1.7.11.7

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

Re: [Freeipa-devel] [PATCH 0018] Make service naming in ipa-server-install consistent

2012-10-22 Thread Tomas Babej

On 10/19/2012 03:16 PM, Martin Kosek wrote:

On 10/19/2012 02:49 PM, Tomas Babej wrote:

On 10/19/2012 01:44 PM, Martin Kosek wrote:

On 10/19/2012 01:26 PM, Tomas Babej wrote:

On 10/18/2012 11:27 AM, Martin Kosek wrote:

On 10/11/2012 05:11 PM, Tomas Babej wrote:

On 10/11/2012 12:32 PM, Martin Kosek wrote:

On 10/11/2012 12:26 PM, Tomas Babej wrote:

Hi,

This patch forces more consistency into ipa-server-install output. All
descriptions of services that are not instances of
SimpleServiceInstance are now in the following format:

Description (Service Name)

Furthermore, start_creation method has been modified to support
custom start and end messages.

Sample output produced by this patch attached.

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

Tomas


Just based on reading the patch, this does not look right:

-self.start_creation(Configuring certificate server, 210)
+self.start_creation(Configuring directory server for the CA,
+end_message=Done configuring directory server for the CA,
+show_service_name=True, runtime=210)

Martin


Thanks, glitch fixed.

Tomas

Ok, I managed to get the patch a proper review. I like the result, in the
console, but I still not entirely satisfied with the patch, looks
over-engineered to me + there is a lot of duplication with Configuring
%(service)s and Done configuring %(service)s messages.

These messages could be easily generated only based on name of a service
(self.service_name, we got that) and a new friendly service name or
description.

If we add description this way:

class KrbInstance(service.Service):
   def __init__(self, fstore=None):
   service.Service.__init__(self, krb5kdc, description=Kerberos
KDC)
...

the start_creation could be very simple:
...
self.start_creation(runtime=30)
...

All messages could be simply generated by the Service class just based on
self.service_name and self.description with having the same output as you do
now.

Martin

I simplified the approach as you suggested. However, I kept optional
start_message and end_message parameters in case we would want
to specify the messages instead of using the pre-generated ones.
This is used in baseupdate.py and upgradeinstance.py

More info in the documentation of start_creation() function.

Tomas

NACK. Tried running ./make-lint with your patch, got thousands of errors...

More like one error thousand times :) I somehow botched up rebasing
the patch, I swear that I corrected that indentation issue beforehand.
Nevermind, thanks for checking. ./make-lint runs clean now :)

But the approach looks OK, I am just thinking that default value for
show_service_name of start_creation() should be True as it is the most widely
used value - you would not have to specify it everytime you call
start_creation() in *instance.py files.

Martin

Default values switched.

Tomas

NACK. Server installation crashed with the first step:

# ipa-server-install

Configuring NTP daemon (ntpd)
   [1/4]: stopping ntpd
   [2/4]: writing configuration
   [3/4]: configuring ntpd to start on boot
   [4/4]: starting ntpd
Unexpected error - see /var/log/ipaserver-install.log for details:
TypeError: expected a character buffer object


ipaserver-install.log:

2012-10-19T12:59:37Z INFO   File
/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py,  line
614, in run_script
 return_value = main_function()

   File /sbin/ipa-server-install, line 904, in main
 ntp.create_instance()

   File /usr/lib/python2.7/site-packages/ipaserver/install/ntpinstance.py,
line 158, in create_instance
 self.start_creation()

   File /usr/lib/python2.7/site-packages/ipaserver/install/service.py, line
358, in start_creation
 self.print_msg(end_message)

   File /usr/lib/python2.7/site-packages/ipaserver/install/service.py, line
295, in print_msg
 print_msg(message, self.output_fd)

   File /usr/lib/python2.7/site-packages/ipaserver/install/service.py, line
60, in print_msg
 output_fd.write(message)

2012-10-19T12:59:37Z INFO The ipa-server-install command failed, exception:
TypeError: expected a   character buffer object


Martin
I also encountered this beforehand. Due to the unfortunate rebase, I 
retested

the whole patch once again, to make sure there are no such errors that I
considered fixed still lurking.
This should work now.

Tomas
From 60ecefdfb7acdad253deca1a8f747eb77c2c767f Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 11 Oct 2012 03:32:17 -0400
Subject: [PATCH] Make service naming in ipa-server-install consistent

Forces more consistency into ipa-server-install output. All
descriptions of services that are not instances of
SimpleServiceInstance are now in the following format:

Description (Service Name)

Furthermore, start_creation method has been modified to support
custom start and end messages. See documentation for more info.

https://fedorahosted.org/freeipa/ticket/3059
---
 ipaserver/install/adtrustinstance.py|  

Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name

2012-10-22 Thread Simo Sorce
On Mon, 2012-10-22 at 13:01 +0200, Martin Kosek wrote:
 On 10/22/2012 12:40 PM, Petr Spacek wrote:
  On 10/19/2012 03:46 PM, Rob Crittenden wrote:
  Petr Spacek wrote:
  On 10/19/2012 03:10 PM, Rob Crittenden wrote:
  Petr Spacek wrote:
  On 10/19/2012 10:10 AM, Martin Kosek wrote:
  On 10/18/2012 09:42 PM, Rob Crittenden wrote:
  We were seeing a unicode failure when trying to request a certificate
  with
  subject alt names. This one-liner should fix it.
 
  rob
 
 
  Yup, this fixes it, works fine on --selfsign IPA CA too.
 
  Just when testing your patch, I found out we don't treat some non-DNS
  subject
  alternative name well, e.g. email extension, an we try to match it
  with our hosts:
 
  Certificate Request:
  ...
   Attributes:
   Requested Extensions:
   X509v3 Subject Alternative Name:
   email:f...@testcert.example.com, DNS:web.example.com
  ...
 
  cert-request result:
 
  ipa: ERROR: no host record for subject alt name
  f...@testcert.example.com in
  certificate request
 
  IMHO there should be a --force option. SAN can contain a lot of
  different things. Also, we can't assume that we manage the whole world
  ... (now :-))
 
 
  The intention was just to provide support for DNS alt names. I don't
  think
  requiring a host entry exist for any alt hosts is asking too much.
 
  I think a new ticket should be opened to support non-DNS alt names.
 
  IMHO SAN names usually contain a lot of virtual names like
  www.shop1.com, ftp.shop1.com, etc.
 
  These names are usually CNAMEs to real name like srv1.shop1.com. In
  that case host object doesn't make sense. (But SAN is required for
  proper certificate validation.)
 
  The purpose is so we more tightly control was certificates are issued by 
  our
  CA because we automatically issue them.
 
  rob
  
  We discussed this check in Brno a bit and we found a problem:
  
  1) Each SAN has to have corresponding host object (in current 
  implementation)
  2) Subject Alternative Name = other name for existing object
 E.g. Name www.shop1.com is CNAME for srv1.shop1.com
  3) Host object makes sense for real host, not for alias like www
  4) Implication from points 1-3 = Current check can't pass in reasonable 
  case,
  because aliases do not have own host objects.
  
  Reasonable check can be: There have to be host object for at least one name
  stated in certificate CN or SAN. (IMHO equivalence between CN and host 
  object
  name would be probably better...)
  
  Does it make sense?
  
 
 I am also not a big fan of current SAN check. Making sure that a host record
 for the service host exists (it does, by design) is enough IMO. As Petr said,
 SANs are often just aliases for the host, so no need for a host record.
 
 But I think if we insist on these checks, we may add --force option to
 workaround the check or display the validation errors only as warnings (when
 the planned warning framework is ready - ticket #2732).

+1,
but I'd go further, I have found places where getting a DNS name
registered could take a couple of days (due to bad bureaucracy), in
those case you may not want to wait for that to happen, and keep going
(you can test with an entry in /etc/hosts in the meanwhile). So a
--force flag is something I think we really need.

I am all for checking and warning but outright preventing is wrong, the
admin is ultimately responsible and we shouldn't prevent admins from
doing operations unless they are fatally irreversible and would brick
the whole solution.

Simo.

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

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


[Freeipa-devel] What is platform services' is_installed() supposed to do?

2012-10-22 Thread Petr Viktorin
For example, ipapython.platform.systemd.SystemdService.is_installed(). 
What is it supposed to do?
Is it checking if the package is installed, or if IPΑ's instance of the 
service is configured and ready to use?


There's no documentation and from the implementation it's not clear 
either. Since people will hopefully want to port the platform code, it 
would be good to document this.



Also, I think the current implementation is wrong, but I can't know for 
sure.


--
Petr³

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

Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name

2012-10-22 Thread Petr Spacek

On 10/22/2012 02:40 PM, Simo Sorce wrote:

On Mon, 2012-10-22 at 13:01 +0200, Martin Kosek wrote:

On 10/22/2012 12:40 PM, Petr Spacek wrote:

On 10/19/2012 03:46 PM, Rob Crittenden wrote:

Petr Spacek wrote:

On 10/19/2012 03:10 PM, Rob Crittenden wrote:

Petr Spacek wrote:

On 10/19/2012 10:10 AM, Martin Kosek wrote:

On 10/18/2012 09:42 PM, Rob Crittenden wrote:

We were seeing a unicode failure when trying to request a certificate
with
subject alt names. This one-liner should fix it.

rob



Yup, this fixes it, works fine on --selfsign IPA CA too.

Just when testing your patch, I found out we don't treat some non-DNS
subject
alternative name well, e.g. email extension, an we try to match it
with our hosts:

Certificate Request:
...
  Attributes:
  Requested Extensions:
  X509v3 Subject Alternative Name:
  email:f...@testcert.example.com, DNS:web.example.com
...

cert-request result:

ipa: ERROR: no host record for subject alt name
f...@testcert.example.com in
certificate request


IMHO there should be a --force option. SAN can contain a lot of
different things. Also, we can't assume that we manage the whole world
... (now :-))



The intention was just to provide support for DNS alt names. I don't
think
requiring a host entry exist for any alt hosts is asking too much.

I think a new ticket should be opened to support non-DNS alt names.


IMHO SAN names usually contain a lot of virtual names like
www.shop1.com, ftp.shop1.com, etc.

These names are usually CNAMEs to real name like srv1.shop1.com. In
that case host object doesn't make sense. (But SAN is required for
proper certificate validation.)


The purpose is so we more tightly control was certificates are issued by our
CA because we automatically issue them.

rob


We discussed this check in Brno a bit and we found a problem:

1) Each SAN has to have corresponding host object (in current implementation)
2) Subject Alternative Name = other name for existing object
E.g. Name www.shop1.com is CNAME for srv1.shop1.com
3) Host object makes sense for real host, not for alias like www
4) Implication from points 1-3 = Current check can't pass in reasonable case,
because aliases do not have own host objects.

Reasonable check can be: There have to be host object for at least one name
stated in certificate CN or SAN. (IMHO equivalence between CN and host object
name would be probably better...)

Does it make sense?



I am also not a big fan of current SAN check. Making sure that a host record
for the service host exists (it does, by design) is enough IMO. As Petr said,
SANs are often just aliases for the host, so no need for a host record.

But I think if we insist on these checks, we may add --force option to
workaround the check or display the validation errors only as warnings (when
the planned warning framework is ready - ticket #2732).


+1,
but I'd go further, I have found places where getting a DNS name
registered could take a couple of days (due to bad bureaucracy), in
those case you may not want to wait for that to happen, and keep going
(you can test with an entry in /etc/hosts in the meanwhile). So a
--force flag is something I think we really need.

I am all for checking and warning but outright preventing is wrong, the
admin is ultimately responsible and we shouldn't prevent admins from
doing operations unless they are fatally irreversible and would brick
the whole solution.


+5 (because was admin 9 months ago :-)

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES] backport of Firefox extension to FreeIPA 2.2

2012-10-22 Thread Martin Kosek
On 10/17/2012 12:02 PM, Petr Vobornik wrote:
 On 10/16/2012 06:10 PM, Endi Sukma Dewata wrote:
 On 10/12/2012 5:55 AM, Petr Viktorin wrote:
 On 10/11/2012 02:55 PM, Petr Vobornik wrote:
 This bunch of patches is a backport of Firefox extension to FreeIPA 2.2.

 First apply pvoborni's patches then pviktori's.

 I tested several replication/upgrade scenarios. Upgrading from 2.2.0 and
 to master works fine. ACK for the Python part.

 ACK for the JavaScript part.
 
 Pushed to ipa-2-2

 Just a minor issue, there's a typo in ipa_init.json and internal.py, I
 think it's supposed to use a curly bracket:

http://${host]/ipa/config/unauthorized.html

 This problem exists on both pvoborni-219-1 and pvoborni-219-2-2 patches.

 Fixed, attaching updated and pushed patch for ipa-2-2. I will send patch for
 3.0 separately.
 

I started with IPA 2.2.1 release, but found out that the spec file is not right
and some files are missing:

Processing files: freeipa-debuginfo-2.2.1-0.fc17.x86_64
Checking for unpackaged file(s): /usr/lib/rpm/check-files
/root/freeipa-2-2-0/rpmbuild/BUILDROOT/freeipa-2.2.1-0.fc17.x86_64
error: Installed (but unpackaged) file(s) found:
   /etc/ipa/html/ffconfig.js
   /etc/ipa/html/ffconfig_page.js


RPM build errors:
Installed (but unpackaged) file(s) found:
   /etc/ipa/html/ffconfig.js
   /etc/ipa/html/ffconfig_page.js
make: *** [rpms] Error 1


Petr, can you please fix the spec file and check if the extension work is
otherwise ok?

Thanks,
Martin

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


Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf

2012-10-22 Thread Martin Kosek
On 10/08/2012 08:27 PM, Rob Crittenden wrote:
 Jakub Hrozek wrote:
 On Fri, Aug 17, 2012 at 12:20:27PM -0400, Simo Sorce wrote:


 - Original Message -
 Hi,

 the attached patches add the directory the SSSD writes domain-realm
 mappings as includedir to krb5.conf when installing the client.

 [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter for
 options
 ipachangeconf only allows one delimeter between keys and values. This
 patch adds the possibility of also specifying delim in the option
 dictionary to override the default delimeter.

 On a slightly-unrelated note, we really should think about adopting
 Augeas. Changing configuration with home-grown scripts is getting
 tricky.

 [PATCH 2/3] Specify includedir in krb5.conf on new installs
 This patch utilizes the new functionality from the previous patch to
 add
 the includedir on top of the krb5.conf file

 [PATCH 3/3] Add the includedir to krb5.conf on upgrades
 This patch is completely untested and I'm only posting it to get
 opinions. At first I was going to use an upgrade script in %post but
 then I thought it would be overengineering when all we want to do is
 prepend one line.. Would a simple munging like this be acceptable or
 shall I write a full script?

 NACK, using a scriptlet is fine, but not the way you did, as it has a huge
 race condition where krb5.conf exists and has only one line in it (the
 include line).

 You should first create the new file: echo include ...  
 /etc/krb.conf.ipanew
 Then cat the contents of the existing file in i:t cat /etc/krb.conf 
 /etc/krb.conf.ipanew
 And finally atomically rename it: mv /etc/krb.conf.ipanew /etc/krb.conf

 This method is also safe wrt something killing the yum process ...

 Simo.

 I'm attaching a new revision of the patches not even two months after
 the original nack.

 I also think it might be nice to have a more general way of upgrading
 the client config so I filed
 https://fedorahosted.org/freeipa/ticket/3149
 
 I don't think grepping for a string is an effective way to determine if the
 client has been configured. Someone could have removed that line.
 
 I'd prefer using /var/lib/ipa-client/sysrestore/sysrestore.index. If it exists
 and has more than 2 lines in it ([files] + one other file) then it is safe to
 say the client is configured, or at least partially configured.
 
 rob
 

I just found one more issue. What if ipa-client-install is run with --no-sssd
option? In that case I assume we should not include the SSSD's krb5.include.d,
right? The same would also appy for upgrades, we would need to check if client
was actually configured with SSSD before mangling their krb5.conf.

Martin

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


Re: [Freeipa-devel] [PATCHES] backport of Firefox extension to FreeIPA 2.2

2012-10-22 Thread Petr Vobornik

On 10/22/2012 05:01 PM, Martin Kosek wrote:

On 10/17/2012 12:02 PM, Petr Vobornik wrote:

On 10/16/2012 06:10 PM, Endi Sukma Dewata wrote:

On 10/12/2012 5:55 AM, Petr Viktorin wrote:

On 10/11/2012 02:55 PM, Petr Vobornik wrote:

This bunch of patches is a backport of Firefox extension to FreeIPA 2.2.

First apply pvoborni's patches then pviktori's.


I tested several replication/upgrade scenarios. Upgrading from 2.2.0 and
to master works fine. ACK for the Python part.


ACK for the JavaScript part.


Pushed to ipa-2-2


Just a minor issue, there's a typo in ipa_init.json and internal.py, I
think it's supposed to use a curly bracket:

http://${host]/ipa/config/unauthorized.html

This problem exists on both pvoborni-219-1 and pvoborni-219-2-2 patches.


Fixed, attaching updated and pushed patch for ipa-2-2. I will send patch for
3.0 separately.



I started with IPA 2.2.1 release, but found out that the spec file is not right
and some files are missing:

Processing files: freeipa-debuginfo-2.2.1-0.fc17.x86_64
Checking for unpackaged file(s): /usr/lib/rpm/check-files
/root/freeipa-2-2-0/rpmbuild/BUILDROOT/freeipa-2.2.1-0.fc17.x86_64
error: Installed (but unpackaged) file(s) found:
/etc/ipa/html/ffconfig.js
/etc/ipa/html/ffconfig_page.js


RPM build errors:
 Installed (but unpackaged) file(s) found:
/etc/ipa/html/ffconfig.js
/etc/ipa/html/ffconfig_page.js
make: *** [rpms] Error 1


Petr, can you please fix the spec file and check if the extension work is
otherwise ok?

Thanks,
Martin



Hm, interdiff of 219-2-2 and 219-2-2-1 says that I ommitted changes in 
freeipa.spec.in in the latter patch. Weird. Anyway shame on me that I 
pushed it without proper check. Attaching diff patch which should fix it.

--
Petr Vobornik
From 8258f7b53a18c23a660675cf3e0bbda24ecf609c Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 1 Oct 2012 17:36:42 +0200
Subject: [PATCH] RPM spec fix for ffconfig.js and ffconfig_page.js

Spec file is missing %files instructions for ffconfig.js and ffconfig_page.js

Ticket: https://fedorahosted.org/freeipa/ticket/3094
---
 freeipa.spec.in | 8 
 1 file changed, 8 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 38eeffd5e57700773f059bea0b259ca2f00856b2..3a6a14ed5022f4774023382d655e77c36c88d42c 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -341,6 +341,10 @@ rm %{buildroot}/%{_libdir}/krb5/plugins/kdb/ipadb.la
 mkdir -p %{buildroot}/%{_sysconfdir}/ipa/html
 mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysrestore
 mkdir %{buildroot}%{_usr}/share/ipa/html/
+ln -s ../../../..%{_sysconfdir}/ipa/html/ffconfig.js \
+%{buildroot}%{_usr}/share/ipa/html/ffconfig.js
+ln -s ../../../..%{_sysconfdir}/ipa/html/ffconfig_page.js \
+%{buildroot}%{_usr}/share/ipa/html/ffconfig_page.js
 ln -s ../../../..%{_sysconfdir}/ipa/html/ssbrowser.html \
 %{buildroot}%{_usr}/share/ipa/html/ssbrowser.html
 ln -s ../../../..%{_sysconfdir}/ipa/html/unauthorized.html \
@@ -560,6 +564,8 @@ fi
 %dir %{_usr}/share/ipa/ffextension/locale/en-US
 %{_usr}/share/ipa/ffextension/locale/en-US/kerberosauth.properties
 %dir %{_usr}/share/ipa/html
+%{_usr}/share/ipa/html/ffconfig.js
+%{_usr}/share/ipa/html/ffconfig_page.js
 %{_usr}/share/ipa/html/ssbrowser.html
 %{_usr}/share/ipa/html/browserconfig.html
 %{_usr}/share/ipa/html/unauthorized.html
@@ -587,6 +593,8 @@ fi
 %{_usr}/share/ipa/ui/images/*.gif
 %dir %{_sysconfdir}/ipa
 %dir %{_sysconfdir}/ipa/html
+%config(noreplace) %{_sysconfdir}/ipa/html/ffconfig.js
+%config(noreplace) %{_sysconfdir}/ipa/html/ffconfig_page.js
 %config(noreplace) %{_sysconfdir}/ipa/html/ssbrowser.html
 %config(noreplace) %{_sysconfdir}/ipa/html/ipa_error.css
 %config(noreplace) %{_sysconfdir}/ipa/html/unauthorized.html
-- 
1.7.11.7

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

Re: [Freeipa-devel] [PATCH] 0092 Make sure the CA is running when starting services

2012-10-22 Thread Petr Viktorin

On 10/19/2012 05:42 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

https://fedorahosted.org/freeipa/ticket/3084
See ticket  commit message.


Please tell me of a better way to extend the Services.


What's interesting is that usually the CA is running right after the
ports are opened, but if not, it takes *exactly* one minute between the
ports being open and the time I stop getting 503 Service Temporarily
Unavailable from ca/admin/ca/getStatus. Is there a sleep somewhere in
pki? or httpd? or IPΑ?


No sleep that I know of, and I'm not seeing that behavior. In my testing
I got 503 exactly once. Most of the time once the port(s) were open and
the request went through the status was that dogtag was up and ready.

Just a few minor requests.

Can you add a block comment to ca_status? I think particularly
explaining why port 443 and not a CA port directly (I assume so we test
the proxy).


Added.



I'm a little confused by the wait variable. It is a boolean in some
cases and a string in others (no-proxy)? Why not just pass in False?


Since the proxy is installed after the CA, we can't check the proxy in 
CA installation's restart step, but we do want to wait for the ports to 
open.
I agree the 'no-proxy' was a bad solution. The included patch just skips 
the check if the httpd service is not configured yet.


I found out that knownservices.httpd.is_installed() returns True even 
after an IPA server is uninstalled. I'm not sure if that's expected, 
I've sent a separate mail asking for clarification.
As a workaround I check for the /etc/httpd/conf.d/ipa.conf file, which 
we remove on uninstall.


Juggling the proxy configuration, httpd restart, and CA restart/wait to 
happen in the right order in all cases is not trivial (especially with 
the long testing times). If you can think of an exotic scenario, please 
test it.



The patch itself looks good. I'm having a replica install problem which
I'm guessing is unrelated.

The configure proxy step is failing to restart httpd. It is failing
because the default mod_nss port is 8443 which is also being used by
dogtag, so httpd fails to restart and the installation blows up.


Thanks for catching this. It was happening with --setup-ca. Fixed.

I think that ipa-replica-install --setup-ca should be changed to behave 
exactly like ipa-replica-install followed by ipa-ca-install. Same for 
the DNS installation. If there are no objections I'll include this in my 
installer framework effort (#2652).



--
Petr³

From 348e963028dcda6deb2c1eb9ba84f5182c4a Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 25 Sep 2012 09:57:03 -0400
Subject: [PATCH] Make sure the CA is running when starting services

- Provide a function for determinig the CA status using Dogtag 10's new
  getStatus endpoint.
  This must be done over HTTPS, but since our client certificate may not be set
  up yet, we need HTTPS without client authentication.
  Rather than copying from the existing http_request and https_request
  function, shared code is factored out to a common helper.
- Call the new function when restarting the CA service. Since our Service
  can only be extended in platform-specific code, do this for Fedora only.
  Also, the status is only checked with Dogtag 10+.
- When a restart call in cainstance failed, users were refered to the
  installation log, but no info was actually logged. Log the exception.

https://fedorahosted.org/freeipa/ticket/3084
---
 ipapython/dogtag.py | 164 +---
 ipapython/platform/fedora16.py  |  51 -
 ipaserver/install/cainstance.py |   8 ++
 3 files changed, 162 insertions(+), 61 deletions(-)

diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 907cebc613127288554378d516eb456f421455ba..5cf5a9df89fa70533d81a78fb18b0ee0f52d6f87 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -130,6 +130,15 @@ def configured_constants(api=None):
 return Dogtag9Constants
 
 
+def error_from_xml(doc, message_template):
+try:
+item_node = doc.getElementsByTagName(Error)
+reason = item_node[0].childNodes[0].data
+return errors.RemoteRetrieveError(reason=reason)
+except Exception, e:
+return errors.RemoteRetrieveError(reason=message_template % e)
+
+
 def get_ca_certchain(ca_host=None):
 
 Retrieve the CA Certificate chain from the configured Dogtag server.
@@ -151,48 +160,118 @@ def get_ca_certchain(ca_host=None):
 item_node = doc.getElementsByTagName(ChainBase64)
 chain = item_node[0].childNodes[0].data
 except IndexError:
-try:
-item_node = doc.getElementsByTagName(Error)
-reason = item_node[0].childNodes[0].data
-raise errors.RemoteRetrieveError(reason=reason)
-except Exception, e:
-raise errors.RemoteRetrieveError(
-reason=_(Retrieving CA 

[Freeipa-devel] One-minute wait before CA is available (Was: PATCH 0092 Make sure the CA is running when starting services)

2012-10-22 Thread Petr Viktorin

On 10/19/2012 05:42 PM, Rob Crittenden wrote:

Petr Viktorin wrote:


What's interesting is that usually the CA is running right after the
ports are opened, but if not, it takes *exactly* one minute between the
ports being open and the time I stop getting 503 Service Temporarily
Unavailable from ca/admin/ca/getStatus. Is there a sleep somewhere in
pki? or httpd? or IPΑ?


No sleep that I know of, and I'm not seeing that behavior. In my testing
I got 503 exactly once. Most of the time once the port(s) were open and
the request went through the status was that dogtag was up and ready.



I looked more closely.
This is happening on servers upgraded from Dogtag 9 to 10 (both still 
with split databases). Ade, do you recall a 60-second wait being added 
somewhere in Dogtag?


--
Petr³

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

Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name

2012-10-22 Thread Rob Crittenden

Petr Spacek wrote:

On 10/22/2012 02:40 PM, Simo Sorce wrote:

On Mon, 2012-10-22 at 13:01 +0200, Martin Kosek wrote:

On 10/22/2012 12:40 PM, Petr Spacek wrote:

On 10/19/2012 03:46 PM, Rob Crittenden wrote:

Petr Spacek wrote:

On 10/19/2012 03:10 PM, Rob Crittenden wrote:

Petr Spacek wrote:

On 10/19/2012 10:10 AM, Martin Kosek wrote:

On 10/18/2012 09:42 PM, Rob Crittenden wrote:

We were seeing a unicode failure when trying to request a
certificate
with
subject alt names. This one-liner should fix it.

rob



Yup, this fixes it, works fine on --selfsign IPA CA too.

Just when testing your patch, I found out we don't treat some
non-DNS
subject
alternative name well, e.g. email extension, an we try to match it
with our hosts:

Certificate Request:
...
  Attributes:
  Requested Extensions:
  X509v3 Subject Alternative Name:
  email:f...@testcert.example.com,
DNS:web.example.com
...

cert-request result:

ipa: ERROR: no host record for subject alt name
f...@testcert.example.com in
certificate request


IMHO there should be a --force option. SAN can contain a lot of
different things. Also, we can't assume that we manage the whole
world
... (now :-))



The intention was just to provide support for DNS alt names. I don't
think
requiring a host entry exist for any alt hosts is asking too much.

I think a new ticket should be opened to support non-DNS alt names.


IMHO SAN names usually contain a lot of virtual names like
www.shop1.com, ftp.shop1.com, etc.

These names are usually CNAMEs to real name like srv1.shop1.com. In
that case host object doesn't make sense. (But SAN is required for
proper certificate validation.)


The purpose is so we more tightly control was certificates are
issued by our
CA because we automatically issue them.

rob


We discussed this check in Brno a bit and we found a problem:

1) Each SAN has to have corresponding host object (in current
implementation)
2) Subject Alternative Name = other name for existing object
E.g. Name www.shop1.com is CNAME for srv1.shop1.com
3) Host object makes sense for real host, not for alias like www
4) Implication from points 1-3 = Current check can't pass in
reasonable case,
because aliases do not have own host objects.

Reasonable check can be: There have to be host object for at least
one name
stated in certificate CN or SAN. (IMHO equivalence between CN and
host object
name would be probably better...)

Does it make sense?



I am also not a big fan of current SAN check. Making sure that a host
record
for the service host exists (it does, by design) is enough IMO. As
Petr said,
SANs are often just aliases for the host, so no need for a host record.

But I think if we insist on these checks, we may add --force option to
workaround the check or display the validation errors only as
warnings (when
the planned warning framework is ready - ticket #2732).


+1,
but I'd go further, I have found places where getting a DNS name
registered could take a couple of days (due to bad bureaucracy), in
those case you may not want to wait for that to happen, and keep going
(you can test with an entry in /etc/hosts in the meanwhile). So a
--force flag is something I think we really need.

I am all for checking and warning but outright preventing is wrong, the
admin is ultimately responsible and we shouldn't prevent admins from
doing operations unless they are fatally irreversible and would brick
the whole solution.


+5 (because was admin 9 months ago :-)



I'm not sure what you mean, it's been like this since inception years ago.

Cert operations are not limited to admins which is why this check is in 
place. A host may request tickets on its own behalf or on behalf of its 
services. I didn't want a host requesting certs for any old host on the 
network via SAN.


I'm fine with relaxing the rules but I'd like to do it via another 
permission such that one set of permissions allows for any SAN and the 
default limits SAN to known hosts.


This isn't linked to DNS in any way.

rob

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


Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf

2012-10-22 Thread Simo Sorce
On Mon, 2012-10-22 at 17:15 +0200, Martin Kosek wrote:
 On 10/08/2012 08:27 PM, Rob Crittenden wrote:
  Jakub Hrozek wrote:
  On Fri, Aug 17, 2012 at 12:20:27PM -0400, Simo Sorce wrote:
 
 
  - Original Message -
  Hi,
 
  the attached patches add the directory the SSSD writes domain-realm
  mappings as includedir to krb5.conf when installing the client.
 
  [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter for
  options
  ipachangeconf only allows one delimeter between keys and values. This
  patch adds the possibility of also specifying delim in the option
  dictionary to override the default delimeter.
 
  On a slightly-unrelated note, we really should think about adopting
  Augeas. Changing configuration with home-grown scripts is getting
  tricky.
 
  [PATCH 2/3] Specify includedir in krb5.conf on new installs
  This patch utilizes the new functionality from the previous patch to
  add
  the includedir on top of the krb5.conf file
 
  [PATCH 3/3] Add the includedir to krb5.conf on upgrades
  This patch is completely untested and I'm only posting it to get
  opinions. At first I was going to use an upgrade script in %post but
  then I thought it would be overengineering when all we want to do is
  prepend one line.. Would a simple munging like this be acceptable or
  shall I write a full script?
 
  NACK, using a scriptlet is fine, but not the way you did, as it has a huge
  race condition where krb5.conf exists and has only one line in it (the
  include line).
 
  You should first create the new file: echo include ...  
  /etc/krb.conf.ipanew
  Then cat the contents of the existing file in i:t cat /etc/krb.conf 
  /etc/krb.conf.ipanew
  And finally atomically rename it: mv /etc/krb.conf.ipanew /etc/krb.conf
 
  This method is also safe wrt something killing the yum process ...
 
  Simo.
 
  I'm attaching a new revision of the patches not even two months after
  the original nack.
 
  I also think it might be nice to have a more general way of upgrading
  the client config so I filed
  https://fedorahosted.org/freeipa/ticket/3149
  
  I don't think grepping for a string is an effective way to determine if the
  client has been configured. Someone could have removed that line.
  
  I'd prefer using /var/lib/ipa-client/sysrestore/sysrestore.index. If it 
  exists
  and has more than 2 lines in it ([files] + one other file) then it is safe 
  to
  say the client is configured, or at least partially configured.
  
  rob
  
 
 I just found one more issue. What if ipa-client-install is run with --no-sssd
 option? In that case I assume we should not include the SSSD's krb5.include.d,
 right? The same would also appy for upgrades, we would need to check if client
 was actually configured with SSSD before mangling their krb5.conf.

Yeah that's right, we should have all sssd related changes under a
conditional that is true only when sssd is enabled.

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] What is platform services' is_installed() supposed to do?

2012-10-22 Thread Alexander Bokovoy

On Mon, 22 Oct 2012, Petr Viktorin wrote:
For example, 
ipapython.platform.systemd.SystemdService.is_installed(). What is it 
supposed to do?
Is it checking if the package is installed, or if IPΑ's instance of 
the service is configured and ready to use?


There's no documentation and from the implementation it's not clear 
either. Since people will hopefully want to port the platform code, 
it would be good to document this.



Also, I think the current implementation is wrong, but I can't know 
for sure.

is_installed() supposed to handle system-specific details of software
availability. We wanted to avoid embedding package manager-specific
knowledge which might not be possible to use during upgrades (to avoid
potential lock ups on parallel access to the same database in some
package managers).

So, for httpd it is correctly reporting that the service is installed.

--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd

2012-10-22 Thread Simo Sorce
On Fri, 2012-10-19 at 14:15 -0400, Rob Crittenden wrote:
 Simo Sorce wrote:
  On Fri, 2012-10-19 at 13:47 -0400, Rob Crittenden wrote:
  Simo Sorce wrote:
  On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote:
  Simo Sorce wrote:
  On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote:
  Simo Sorce wrote:
  Also improve shutdown reliability and restart behavior so we always 
  kill
  all the processes we started even if the list of processes to handle
  changed in LDAP.
 
  Fixes: https://fedorahosted.org/freeipa/ticket/2302
 
  Should this list be updated if we do a post-install of DNS or the CA? 
  It
  isn't now which would leave some services running.
 
  Yes we probably should,
  but I think it should be done as a separate ticket.
 
  Simo.
 
 
  What's the impact if we don't do this now?
 
  some services will not be turned off by ipactl, however this shouldn't
  impact much at shutdown, as systemd will shutdown left over stuff as it
  keeps track of all processes.
 
  uhmm however if someone does a ipactl restart those services will not be
  restarted the first time it is run (we will tell systemd to start them
  but it will do likely nothing as they are already started), after that
  they will be handled as they fact they are enabled will be stored in the
  file.
 
  Well, what is the downside of a call to refresh the list of running
  services that should be run at the end of setting up new services?
 
  Otherwise we could find outselves with a bunch of unreproducible cases
  where things weren't start/stopped properly.
 
  Yeah I have been thinking about that.
  I am thinking of changing the patch so that it's the service class
  itself that saves this data.
 
  That way saving is always performed no matter 'what' starts the service.
  Do you agree ?
 
 
 Yes, that sounds like it'll be very robust.

Ok I created 3 new patches that replace the previous one.

The first 2 patches lay groundwork to have the service class save in the
services.list file.
The last one changes ipactl to actually use the file.

Simo.


-- 
Simo Sorce * Red Hat, Inc * New York
From ef39621897d3bced7f4bf41ab9ac04a266bf9fa3 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Mon, 22 Oct 2012 16:59:43 -0400
Subject: [PATCH 1/3] Preserve original service_name in services

This is needed to be able to reference stuff always wth the same name.
The platform specific private name must be kept in a platform specific
variable.
In the case of systemd we store it in systemd_name
For the redhat platform wellknown names and service name are the same so
currently no special name is needed.
---
 ipapython/platform/fedora16.py | 11 +--
 ipapython/platform/systemd.py  | 19 ++-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/ipapython/platform/fedora16.py b/ipapython/platform/fedora16.py
index 794c39e2091f9402282e18fbe162d40892cb1e0d..ced85ce593dd31655dcbbd8a6d1900cee7b9e9ea 100644
--- a/ipapython/platform/fedora16.py
+++ b/ipapython/platform/fedora16.py
@@ -68,15 +68,14 @@ system_units['pki_tomcatd'] = system_units['pki-tomcatd']
 class Fedora16Service(systemd.SystemdService):
 def __init__(self, service_name):
 if service_name in system_units:
-service_name = system_units[service_name]
+systemd_name = system_units[service_name]
 else:
 if len(service_name.split('.')) == 1:
 # if service_name does not have a dot, it is not foo.service
 # and not a foo.target. Thus, not correct service name for
 # systemd, default to foo.service style then
-service_name = %s.service % (service_name)
-super(Fedora16Service, self).__init__(service_name)
-
+systemd_name = %s.service % (service_name)
+super(Fedora16Service, self).__init__(service_name, systemd_name)
 # Special handling of directory server service
 #
 # We need to explicitly enable instances to install proper symlinks as
@@ -104,8 +103,8 @@ class Fedora16DirectoryService(Fedora16Service):
 
 def restart(self, instance_name=, capture_output=True, wait=True):
 if len(instance_name)  0:
-elements = self.service_name.split(@)
-srv_etc = os.path.join(self.SYSTEMD_ETC_PATH, self.service_name)
+elements = self.systemd_name.split(@)
+srv_etc = os.path.join(self.SYSTEMD_ETC_PATH, self.systemd_name)
 srv_tgt = os.path.join(self.SYSTEMD_ETC_PATH, self.SYSTEMD_SRV_TARGET % (elements[0]))
 srv_lnk = os.path.join(srv_tgt, self.service_instance(instance_name))
 if not os.path.exists(srv_etc):
diff --git a/ipapython/platform/systemd.py b/ipapython/platform/systemd.py
index c174488c08a73ce02b5f568ddd24c98d8dab83d1..6c25a79b6ecdfbda1c85ada6642a656d704fdb2d 100644
--- a/ipapython/platform/systemd.py
+++ b/ipapython/platform/systemd.py
@@ -27,25 +27,26 @@ class SystemdService(base.PlatformService):
 

[Freeipa-devel] Experimental patchwork server

2012-10-22 Thread Simo Sorce
Hello fellow developers,

We have discussed for a while among us about how to improve patch
tracking for review purposes. Various method have been discussed for
quite some time now (including gerrit and review board) but for one
reason or another we haven't done much.

I have now set up a patchwork instance here:
https://patchwork.acksyn.org

Patchwork is a very lightweight system that doesn't take over our
current practices (although may require minor changes).

Most importantly it does not replace our mailing list with a new system
that pretends to take over the whole process.

With patchwork review happens on the mailing list as usual but the
server does automatic tracking of patches updating with comments sent to
the mailing list.

Feel free to get a login there, and start managing your own patches.
I will keep a look on the system and override patch status for those
that choose not to use it.

The server has just been installed and I am still configuring it.
If you have any issue please contact me privately or on this list so we
can try to address it.

I hope we will find the tool useful.

There is just one thing that patchwork does not handle well, and that is
multiple patches sent in the same mail as attachment.
See what happend here: https://patchwork.acksyn.org/patch/2 where I sent
three patches and only the last one was picked up by the system.

Patchwork[1] has been developed mostly in the kernel community and there
the rule is to send 1 patch per mail by using git send-mail.

I will switch to use git send-mail (and resend the above set as a test)
so that patchwork is happy, I hope you all can try to use it as well so
that we can try to use patchwork for all patches.
However I do not want to force people to use git send-mail.
If you are not going to use git send-mail however I would like to ask
you to not send more than one patch per mail message, and instead send
different patches in different messages.
Traditionally this is done by using a patchset header of [PATCH 0/5] and
then following one mail per patch [PATCH 1/5] and then PATCH[2/5]
etc ... the subject should stay the same for all patches in the same
patchset.

If you have questions or proposal please let me know.

Also patchwork is python+django so if you have an itch and want to
scratch it then feel free to send patches to me as well as upstream so
we can improve the tool.

Simo.

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

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


[Freeipa-devel] [PATCH 0/3] 500 Fix shutdown issues with systemd

2012-10-22 Thread Simo Sorce
From: Simo Sorce sso...@redhat.com

Fix shoutdonw issues with systemd and also improve shutdown reliability and
restart behavior so we always kill all the processes we started even if the
list of processes to handle changed in LDAP.

We do this by saving into a file, using json, which services have been
started in teh start() action of the serivce class. We read the list back
from the file when we need to stop the services then the file is deleted.
We always read the list from LDAP.

NOTE: This is the same patchset previously posted, just testing git send-mail 
for the
benefit of patchwork.

Simo Sorce (3):
  Preserve original service_name in services
  Save service name on service startup
  Get list of service from LDAP only at startup

 freeipa.spec.in|2 +
 init/systemd/ipa.conf.tmpfiles |1 +
 install/tools/ipactl   |  199 
 ipapython/platform/base.py |   23 +
 ipapython/platform/fedora16.py |   12 +-
 ipapython/platform/redhat.py   |1 +
 ipapython/platform/systemd.py  |   20 ++--
 ipapython/services.py.in   |4 +
 8 files changed, 188 insertions(+), 74 deletions(-)

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


[Freeipa-devel] [PATCH 2/3] Save service name on service startup

2012-10-22 Thread Simo Sorce
From: Simo Sorce sso...@redhat.com

This is done as a default action of the ancestor class so that no matter what
platform is currently used this code is always the same and the name is the
wellknown service name.
This information will be used by ipacl to stop only and all the services that
have been started by any ipa tool/install script
---
 ipapython/platform/base.py|   23 +++
 ipapython/platform/redhat.py  |1 +
 ipapython/platform/systemd.py |1 +
 ipapython/services.py.in  |4 
 4 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/ipapython/platform/base.py b/ipapython/platform/base.py
index 
2d39d216991c08c4d439a34af99b67b250058889..1101b5cdcf1d033f757598ad5b0fdc526ca8eefe
 100644
--- a/ipapython/platform/base.py
+++ b/ipapython/platform/base.py
@@ -17,6 +17,8 @@
 # along with this program.  If not, see http://www.gnu.org/licenses/.
 
 from ipalib.plugable import MagicDict
+import json
+import os
 
 # Canonical names of services as IPA wants to see them. As we need to have
 # *some* naming, set them as in Red Hat distributions. Actual implementation
@@ -40,6 +42,8 @@ wellknownports = {
 'pki-tomcatd': [8080, 8443],  # used if the incoming instance name is blank
 }
 
+SVC_LIST_FILE = /var/run/ipa/services.list
+
 class AuthConfig(object):
 
 AuthConfig class implements system-independent interface to configure
@@ -133,6 +137,25 @@ class PlatformService(object):
 self.service_name = service_name
 
 def start(self, instance_name=, capture_output=True, wait=True):
+
+When a service is started record the fact in a special file.
+This allows ipactl stop to always stop all services that have
+been started via ipa tools
+
+svc_list = []
+try:
+f = open(SVC_LIST_FILE, 'r')
+svc_list = json.load(f)
+except Exception:
+# not fatal, may be the first service
+pass
+
+svc_list.append(self.service_name)
+
+f = open(SVC_LIST_FILE, 'w')
+json.dump(svc_list, f)
+f.flush()
+f.close()
 return
 
 def stop(self, instance_name=, capture_output=True):
diff --git a/ipapython/platform/redhat.py b/ipapython/platform/redhat.py
index 
3551c28410ceeabfc1064ac79e86dc7ee40dd8c3..cd1277733e8c8798b5ef540d4b14c0b1b9d96088
 100644
--- a/ipapython/platform/redhat.py
+++ b/ipapython/platform/redhat.py
@@ -71,6 +71,7 @@ class RedHatService(base.PlatformService):
 ipautil.run([/sbin/service, self.service_name, start, 
instance_name], capture_output=capture_output)
 if wait and self.is_running(instance_name):
 self.__wait_for_open_ports(instance_name)
+super(RedHatService, self).start(instance_name)
 
 def restart(self, instance_name=, capture_output=True, wait=True):
 ipautil.run([/sbin/service, self.service_name, restart, 
instance_name], capture_output=capture_output)
diff --git a/ipapython/platform/systemd.py b/ipapython/platform/systemd.py
index 
6c25a79b6ecdfbda1c85ada6642a656d704fdb2d..63414058355e7697c04126ede7dbb77f73e94cab
 100644
--- a/ipapython/platform/systemd.py
+++ b/ipapython/platform/systemd.py
@@ -96,6 +96,7 @@ class SystemdService(base.PlatformService):
 ipautil.run([/bin/systemctl, start, 
self.service_instance(instance_name)], capture_output=capture_output)
 if wait and self.is_running(instance_name):
 self.__wait_for_open_ports(self.service_instance(instance_name))
+super(SystemdService, self).start(instance_name)
 
 def restart(self, instance_name=, capture_output=True, wait=True):
 # Restart command is broken before systemd-36-3.fc16
diff --git a/ipapython/services.py.in b/ipapython/services.py.in
index 
8fe663763d06d89c62604086a3eea7a9a983f49d..16b62ca8508d4078e896cd1da6fd664f52a3930e
 100644
--- a/ipapython/services.py.in
+++ b/ipapython/services.py.in
@@ -51,4 +51,8 @@ backup_and_replace_hostname = 
backup_and_replace_hostname_default
 def check_selinux_status():
 return
 
+from ipapython.platform.base import SVC_LIST_FILE
+def get_svc_list_file():
+return SVC_LIST_FILE
+
 from ipapython.platform.SUPPORTED_PLATFORM import *
-- 
1.7.1

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


[Freeipa-devel] [PATCH 1/3] Preserve original service_name in services

2012-10-22 Thread Simo Sorce
From: Simo Sorce sso...@redhat.com

This is needed to be able to reference stuff always wth the same name.
The platform specific private name must be kept in a platform specific
variable.
In the case of systemd we store it in systemd_name
For the redhat platform wellknown names and service name are the same so
currently no special name is needed.
---
 ipapython/platform/fedora16.py |   12 ++--
 ipapython/platform/systemd.py  |   19 ++-
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/ipapython/platform/fedora16.py b/ipapython/platform/fedora16.py
index 
794c39e2091f9402282e18fbe162d40892cb1e0d..45b4f6e28126caab20a6bce5b583cbd6a702fcd1
 100644
--- a/ipapython/platform/fedora16.py
+++ b/ipapython/platform/fedora16.py
@@ -67,16 +67,16 @@ system_units['pki_tomcatd'] = system_units['pki-tomcatd']
 
 class Fedora16Service(systemd.SystemdService):
 def __init__(self, service_name):
+systemd_name = service_name
 if service_name in system_units:
-service_name = system_units[service_name]
+systemd_name = system_units[service_name]
 else:
 if len(service_name.split('.')) == 1:
 # if service_name does not have a dot, it is not foo.service
 # and not a foo.target. Thus, not correct service name for
 # systemd, default to foo.service style then
-service_name = %s.service % (service_name)
-super(Fedora16Service, self).__init__(service_name)
-
+systemd_name = %s.service % (service_name)
+super(Fedora16Service, self).__init__(service_name, systemd_name)
 # Special handling of directory server service
 #
 # We need to explicitly enable instances to install proper symlinks as
@@ -104,8 +104,8 @@ class Fedora16DirectoryService(Fedora16Service):
 
 def restart(self, instance_name=, capture_output=True, wait=True):
 if len(instance_name)  0:
-elements = self.service_name.split(@)
-srv_etc = os.path.join(self.SYSTEMD_ETC_PATH, self.service_name)
+elements = self.systemd_name.split(@)
+srv_etc = os.path.join(self.SYSTEMD_ETC_PATH, self.systemd_name)
 srv_tgt = os.path.join(self.SYSTEMD_ETC_PATH, 
self.SYSTEMD_SRV_TARGET % (elements[0]))
 srv_lnk = os.path.join(srv_tgt, 
self.service_instance(instance_name))
 if not os.path.exists(srv_etc):
diff --git a/ipapython/platform/systemd.py b/ipapython/platform/systemd.py
index 
c174488c08a73ce02b5f568ddd24c98d8dab83d1..6c25a79b6ecdfbda1c85ada6642a656d704fdb2d
 100644
--- a/ipapython/platform/systemd.py
+++ b/ipapython/platform/systemd.py
@@ -27,25 +27,26 @@ class SystemdService(base.PlatformService):
 SYSTEMD_LIB_PATH = /lib/systemd/system/
 SYSTEMD_SRV_TARGET = %s.target.wants
 
-def __init__(self, service_name):
+def __init__(self, service_name, systemd_name):
 super(SystemdService, self).__init__(service_name)
-self.lib_path = os.path.join(self.SYSTEMD_LIB_PATH, self.service_name)
+self.systemd_name = systemd_name
+self.lib_path = os.path.join(self.SYSTEMD_LIB_PATH, self.systemd_name)
 self.lib_path_exists = None
 
 def service_instance(self, instance_name):
 if self.lib_path_exists is None:
 self.lib_path_exists = os.path.exists(self.lib_path)
 
-elements = self.service_name.split(@)
+elements = self.systemd_name.split(@)
 
 # Short-cut: if there is already exact service name, return it
 if self.lib_path_exists and len(instance_name) == 0:
 if len(elements) == 1:
 # service name is like pki-tomcatd.target or krb5kdc.service
-return self.service_name
+return self.systemd_name
 if len(elements)  1 and elements[1][0] != '.':
 # Service name is like pki-tomcatd@pki-tomcat.service and that 
file exists
-return self.service_name
+return self.systemd_name
 
 if len(elements)  1:
 # We have dynamic service
@@ -59,7 +60,7 @@ class SystemdService(base.PlatformService):
 if os.path.exists(srv_lib):
 return tgt_name
 
-return self.service_name
+return self.systemd_name
 
 def parse_variables(self, text, separator=None):
 
@@ -82,7 +83,7 @@ class SystemdService(base.PlatformService):
 if instance_name in base.wellknownports:
 ports = base.wellknownports[instance_name]
 else:
-elements = self.service_name.split(@)
+elements = self.systemd_name.split(@)
 if elements[0] in base.wellknownports:
 ports = base.wellknownports[elements[0]]
 if ports:
@@ -141,7 +142,7 @@ class SystemdService(base.PlatformService):
 def enable(self, instance_name=):
 if self.lib_path_exists is None:
 

[Freeipa-devel] [PATCH 3/3] Get list of service from LDAP only at startup

2012-10-22 Thread Simo Sorce
From: Simo Sorce sso...@redhat.com

We check (possibly different) data from LDAP only at (re)start.
This way we always shutdown exactly the services we started even if the list
changed in the meanwhile (we avoid leaving a service running even if it was
removed from LDAP as the admin decided it should not be started in future).

This should also fix a problematic deadlock with systemd when we try to read
the list of service from LDAP at shutdown.
---
 freeipa.spec.in|2 +
 init/systemd/ipa.conf.tmpfiles |1 +
 install/tools/ipactl   |  199 
 3 files changed, 143 insertions(+), 59 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
916630029f6dfac8ef32dabb00f338052cbbf08e..41745c318655fa3eb37a512aaf253016f1620581
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -446,6 +446,7 @@ install -m 0644 init/systemd/ipa.conf.tmpfiles 
%{buildroot}%{_sysconfdir}/tmpfil
 
 mkdir -p %{buildroot}%{_localstatedir}/run/
 install -d -m 0700 %{buildroot}%{_localstatedir}/run/ipa_memcached/
+install -d -m 0700 %{buildroot}%{_localstatedir}/run/ipa/
 
 mkdir -p %{buildroot}%{_libdir}/krb5/plugins/libkrb5
 touch %{buildroot}%{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so
@@ -623,6 +624,7 @@ fi
 %{_sysconfdir}/cron.d/ipa-compliance
 %config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
 %dir %attr(0700,apache,apache) %{_localstatedir}/run/ipa_memcached/
+%dir %attr(0700,root,root) %{_localstatedir}/run/ipa/
 %if 0%{?fedora} = 15
 %config %{_sysconfdir}/tmpfiles.d/ipa.conf
 %endif
diff --git a/init/systemd/ipa.conf.tmpfiles b/init/systemd/ipa.conf.tmpfiles
index 
e4b679a55d68a6b83991ac72dd520c32b2a0de50..1e7a896ed8df00c97f2d092504e2a65960bb341d
 100644
--- a/init/systemd/ipa.conf.tmpfiles
+++ b/init/systemd/ipa.conf.tmpfiles
@@ -1 +1,2 @@
 d /var/run/ipa_memcached 0700 apache apache
+d /var/run/ipa 0700 root root
diff --git a/install/tools/ipactl b/install/tools/ipactl
index 
d4b2c0878f2b62fd12198f76bef01ef70e9f3de1..9b151ab9f9bd10423d5145a1fcf028b6ddb65096
 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -34,6 +34,7 @@ try:
 import ldap.sasl
 import ldapurl
 import socket
+import json
 except ImportError:
 print  sys.stderr, \
 There was a problem importing one of the required Python modules. The
@@ -162,11 +163,30 @@ def get_config(dirsrv):
 for p in entry[1]['ipaConfigString']:
 if p.startswith('startOrder '):
 order = p.split()[1]
-svc_list.append((order, name))
+svc_list.append([order, name])
+
+ordered_list = []
+for (order, svc) in sorted(svc_list):
+ordered_list.append(service.SERVICE_LIST[svc][0])
+return ordered_list
+
+def get_config_from_file():
+
+svc_list = []
+
+try:
+f = open(ipaservices.get_svc_list_file(), 'r')
+svc_list = json.load(f)
+except Exception, e:
+raise IpactlError(Unknown error when retrieving list of services from 
file:  + str(e))
 
 return svc_list
 
 def ipa_start(options):
+
+if os.path.isfile(ipaservices.get_svc_list_file()):
+raise IpactlError(IPA service already started!)
+
 dirsrv = ipaservices.knownservices.dirsrv
 try:
 print Starting Directory Service
@@ -174,7 +194,7 @@ def ipa_start(options):
 except Exception, e:
 raise IpactlError(Failed to start Directory Service:  + str(e))
 
-svc_list = []
+ldap_list = []
 try:
 svc_list = get_config(dirsrv)
 except Exception, e:
@@ -191,21 +211,19 @@ def ipa_start(options):
 raise IpactlError()
 
 if len(svc_list) == 0:
-# no service to stop
+# no service to start
 return
 
-for (order, svc) in sorted(svc_list):
-svc_name = service.SERVICE_LIST[svc][0]
-svchandle = ipaservices.service(svc_name)
+for svc in svc_list:
+svchandle = ipaservices.service(svc)
 try:
 print Starting %s Service % svc
-svchandle.start(capture_output=get_capture_output(svc_name, 
options.debug))
+svchandle.start(capture_output=get_capture_output(svc, 
options.debug))
 except:
 emit_err(Failed to start %s Service % svc)
 emit_err(Shutting down)
-for (order, svc) in sorted(svc_list):
-svc_name = service.SERVICE_LIST[svc][0]
-svc_off = ipaservices.service(svc_name)
+for svc in svc_list:
+svc_off = ipaservices.service(svc)
 try:
 svc_off.stop(capture_output=False)
 except:
@@ -220,11 +238,10 @@ def ipa_stop(options):
 dirsrv = ipaservices.knownservices.dirsrv
 svc_list = []
 try:
-svc_list = get_config(dirsrv)
+svc_list = get_config_from_file()
 except Exception, e:
-# ok if dirsrv died this may fail, so let's try to quickly restart it
-# and see if we 

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

2012-10-22 Thread Rob Crittenden

Tomas Babej wrote:

On 10/18/2012 05:14 PM, Rob Crittenden wrote:

Tomas Babej wrote:

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


Can you add startup_timeout as well? This controls the amount of time
we wait when starting a service. The default is 2 minutes.

And dogtag_version?

The changes you made look good.

rob



The descriptions for the requested options have been added:

+.B dogtag_version version
+Stores the version of Dogtag. Value 9 is assumed if not specified
otherwise.

+.B startup_timeout time in seconds
+Controls the amount of time waited when starting a service. The default
value is 120 seconds.

Tomas




ack, pushed to master and ipa-3-0

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


Re: [Freeipa-devel] Experimental patchwork server

2012-10-22 Thread Simo Sorce
On Mon, 2012-10-22 at 18:03 -0400, Simo Sorce wrote:
 Hello fellow developers,
 
 We have discussed for a while among us about how to improve patch
 tracking for review purposes. Various method have been discussed for
 quite some time now (including gerrit and review board) but for one
 reason or another we haven't done much.
 
 I have now set up a patchwork instance here:
 https://patchwork.acksyn.org
 
 Patchwork is a very lightweight system that doesn't take over our
 current practices (although may require minor changes).
 
 Most importantly it does not replace our mailing list with a new system
 that pretends to take over the whole process.
 
 With patchwork review happens on the mailing list as usual but the
 server does automatic tracking of patches updating with comments sent to
 the mailing list.
 
 Feel free to get a login there, and start managing your own patches.
 I will keep a look on the system and override patch status for those
 that choose not to use it.
 
 The server has just been installed and I am still configuring it.
 If you have any issue please contact me privately or on this list so we
 can try to address it.
 
 I hope we will find the tool useful.
 
 There is just one thing that patchwork does not handle well, and that is
 multiple patches sent in the same mail as attachment.
 See what happend here: https://patchwork.acksyn.org/patch/2 where I sent
 three patches and only the last one was picked up by the system.
 
 Patchwork[1] has been developed mostly in the kernel community and there
 the rule is to send 1 patch per mail by using git send-mail.
 
 I will switch to use git send-mail (and resend the above set as a test)
 so that patchwork is happy, I hope you all can try to use it as well so
 that we can try to use patchwork for all patches.
 However I do not want to force people to use git send-mail.
 If you are not going to use git send-mail however I would like to ask
 you to not send more than one patch per mail message, and instead send
 different patches in different messages.
 Traditionally this is done by using a patchset header of [PATCH 0/5] and
 then following one mail per patch [PATCH 1/5] and then PATCH[2/5]
 etc ... the subject should stay the same for all patches in the same
 patchset.
 
 If you have questions or proposal please let me know.
 
 Also patchwork is python+django so if you have an itch and want to
 scratch it then feel free to send patches to me as well as upstream so
 we can improve the tool.


In case you wonder how to use git send-mail here are a couple of things
I do to make it easier.

1. I cloned my public review repo on the machine I use for email so I
can push directly from there even when the patches are built on my
development machines.

2. set the [sendemail] option for the outgoing smtp server in
~/.gitconfig

3. I create 2 aliases that make the process just s simple 2 commands:

alias prep-freeipa='rm -fr $HOME/git-send-mail  git format-patch -M -C 
--patience --full-index -n --cover-letter -o $HOME/git-send-mail'
alias send-freeipa='git send-email --no-chain-reply-to --to 
freeipa-devel@redhat.com --suppress-cc=all $HOME/git-send-mail/*patch'

This first alias is called like this:
$ prep-freeipa -3

It will create a bundle for the lst 3 patches in the tree and dump
patches as well as a standard cover letter in a directory called
$HOME/git-send-mail

I then vim $HOME/git-send-mail/-cover-letter.patch, where you need
to add a subject and fill in the body of the presentation email

Then call simply send-freeipa without any option.
It will ask a couple of questions to which you can normally just hit
return (defaults are usually ok).

HTH,
Simo.

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

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