Re: [Freeipa-devel] [PATCH 0020] Refactoring of default.conf man page
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
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
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
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
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
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?
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
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
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
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
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
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)
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
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
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?
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
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
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
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
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
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
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
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
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