Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
Jakub Hrozek wrote: On Mon, Nov 12, 2012 at 05:42:21PM +0100, Jan Cholasta wrote: On 9.11.2012 17:58, Jakub Hrozek wrote: On Wed, Nov 07, 2012 at 05:20:30PM +0100, Martin Kosek wrote: On 11/07/2012 12:53 AM, Jakub Hrozek wrote: On Wed, Oct 31, 2012 at 01:22:52PM +0100, Martin Kosek wrote: On 10/31/2012 11:00 AM, Jakub Hrozek wrote: On Mon, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: 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. OK, new patches are attached. In new installs, the includedir is only added when options.sssd is true. During upgrades, I checked for sssd.conf's existence, I'm not sure if there's any other way to check if the client was configured with sssd? Hello Jakub, thanks for these patches. I think that checking if /etc/sssd.conf exists as actually not so bad way to test if it was configured. Anyway, I did few tests on server and client but I still see few issues: 1) SELinux context of krb5.conf is not as expected (but I am not sure what real issue could that cause): # restorecon -FvvR /etc/krb5.conf restorecon reset /etc/krb5.conf context unconfined_u:object_r:etc_t:s0-system_u:object_r:krb5_conf_t:s0 Fixed. Thanks, I shouldn have noticed that doing mv would just replace the original context. 2) I can no longer kinit on IPA server after applying your patch # rpm -q sssd sssd-1.9.90-0.20121030T1436Zgitf46bf56.fc17.x86_64 # rpm -Uvh --force freeipa-*.rpm # head -n 5 /etc/krb5.conf includedir /var/lib/sss/pubconf/krb5.include.d/ [logging] default = FILE:/var/log/krb5libs.log kdc = FILE:/var/log/krb5kdc.log admin_server = FILE:/var/log/kadmind.log # KRB5_TRACE=/dev/stdout kinit admin [21059] 1351684052.658548: Getting initial credentials for ad...@idm.lab.bos.redhat.com [21059] 1351684052.665269: Sending request (200 bytes) to IDM.LAB.BOS.REDHAT.COM [21059] 1351684052.665989: Resolving hostname vm-044.idm.lab.bos.redhat.com [21059] 1351684052.667511: Sending initial UDP request to dgram 10.16.78.44:88 [21059] 1351684052.672514: Received answer from dgram 10.16.78.44:88 [21059] 1351684052.672653: Response was from master KDC [21059] 1351684052.672751: Received error from KDC: -1765328370/KDC has no support for encryption type kinit: KDC has no
Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
On Mon, Nov 12, 2012 at 05:42:21PM +0100, Jan Cholasta wrote: On 9.11.2012 17:58, Jakub Hrozek wrote: On Wed, Nov 07, 2012 at 05:20:30PM +0100, Martin Kosek wrote: On 11/07/2012 12:53 AM, Jakub Hrozek wrote: On Wed, Oct 31, 2012 at 01:22:52PM +0100, Martin Kosek wrote: On 10/31/2012 11:00 AM, Jakub Hrozek wrote: On Mon, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: 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. OK, new patches are attached. In new installs, the includedir is only added when options.sssd is true. During upgrades, I checked for sssd.conf's existence, I'm not sure if there's any other way to check if the client was configured with sssd? Hello Jakub, thanks for these patches. I think that checking if /etc/sssd.conf exists as actually not so bad way to test if it was configured. Anyway, I did few tests on server and client but I still see few issues: 1) SELinux context of krb5.conf is not as expected (but I am not sure what real issue could that cause): # restorecon -FvvR /etc/krb5.conf restorecon reset /etc/krb5.conf context unconfined_u:object_r:etc_t:s0-system_u:object_r:krb5_conf_t:s0 Fixed. Thanks, I shouldn have noticed that doing mv would just replace the original context. 2) I can no longer kinit on IPA server after applying your patch # rpm -q sssd sssd-1.9.90-0.20121030T1436Zgitf46bf56.fc17.x86_64 # rpm -Uvh --force freeipa-*.rpm # head -n 5 /etc/krb5.conf includedir /var/lib/sss/pubconf/krb5.include.d/ [logging] default = FILE:/var/log/krb5libs.log kdc = FILE:/var/log/krb5kdc.log admin_server = FILE:/var/log/kadmind.log # KRB5_TRACE=/dev/stdout kinit admin [21059] 1351684052.658548: Getting initial credentials for ad...@idm.lab.bos.redhat.com [21059] 1351684052.665269: Sending request (200 bytes) to IDM.LAB.BOS.REDHAT.COM [21059] 1351684052.665989: Resolving hostname vm-044.idm.lab.bos.redhat.com [21059] 1351684052.667511: Sending initial UDP request to dgram 10.16.78.44:88 [21059] 1351684052.672514: Received answer from dgram 10.16.78.44:88 [21059] 1351684052.672653: Response was from master KDC
Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
On Wed, Nov 07, 2012 at 05:20:30PM +0100, Martin Kosek wrote: On 11/07/2012 12:53 AM, Jakub Hrozek wrote: On Wed, Oct 31, 2012 at 01:22:52PM +0100, Martin Kosek wrote: On 10/31/2012 11:00 AM, Jakub Hrozek wrote: On Mon, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: 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. OK, new patches are attached. In new installs, the includedir is only added when options.sssd is true. During upgrades, I checked for sssd.conf's existence, I'm not sure if there's any other way to check if the client was configured with sssd? Hello Jakub, thanks for these patches. I think that checking if /etc/sssd.conf exists as actually not so bad way to test if it was configured. Anyway, I did few tests on server and client but I still see few issues: 1) SELinux context of krb5.conf is not as expected (but I am not sure what real issue could that cause): # restorecon -FvvR /etc/krb5.conf restorecon reset /etc/krb5.conf context unconfined_u:object_r:etc_t:s0-system_u:object_r:krb5_conf_t:s0 Fixed. Thanks, I shouldn have noticed that doing mv would just replace the original context. 2) I can no longer kinit on IPA server after applying your patch # rpm -q sssd sssd-1.9.90-0.20121030T1436Zgitf46bf56.fc17.x86_64 # rpm -Uvh --force freeipa-*.rpm # head -n 5 /etc/krb5.conf includedir /var/lib/sss/pubconf/krb5.include.d/ [logging] default = FILE:/var/log/krb5libs.log kdc = FILE:/var/log/krb5kdc.log admin_server = FILE:/var/log/kadmind.log # KRB5_TRACE=/dev/stdout kinit admin [21059] 1351684052.658548: Getting initial credentials for ad...@idm.lab.bos.redhat.com [21059] 1351684052.665269: Sending request (200 bytes) to IDM.LAB.BOS.REDHAT.COM [21059] 1351684052.665989: Resolving hostname vm-044.idm.lab.bos.redhat.com [21059] 1351684052.667511: Sending initial UDP request to dgram 10.16.78.44:88 [21059] 1351684052.672514: Received answer from dgram 10.16.78.44:88 [21059] 1351684052.672653: Response was from master KDC [21059]
Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
On 11/07/2012 12:53 AM, Jakub Hrozek wrote: On Wed, Oct 31, 2012 at 01:22:52PM +0100, Martin Kosek wrote: On 10/31/2012 11:00 AM, Jakub Hrozek wrote: On Mon, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: 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. OK, new patches are attached. In new installs, the includedir is only added when options.sssd is true. During upgrades, I checked for sssd.conf's existence, I'm not sure if there's any other way to check if the client was configured with sssd? Hello Jakub, thanks for these patches. I think that checking if /etc/sssd.conf exists as actually not so bad way to test if it was configured. Anyway, I did few tests on server and client but I still see few issues: 1) SELinux context of krb5.conf is not as expected (but I am not sure what real issue could that cause): # restorecon -FvvR /etc/krb5.conf restorecon reset /etc/krb5.conf context unconfined_u:object_r:etc_t:s0-system_u:object_r:krb5_conf_t:s0 Fixed. Thanks, I shouldn have noticed that doing mv would just replace the original context. 2) I can no longer kinit on IPA server after applying your patch # rpm -q sssd sssd-1.9.90-0.20121030T1436Zgitf46bf56.fc17.x86_64 # rpm -Uvh --force freeipa-*.rpm # head -n 5 /etc/krb5.conf includedir /var/lib/sss/pubconf/krb5.include.d/ [logging] default = FILE:/var/log/krb5libs.log kdc = FILE:/var/log/krb5kdc.log admin_server = FILE:/var/log/kadmind.log # KRB5_TRACE=/dev/stdout kinit admin [21059] 1351684052.658548: Getting initial credentials for ad...@idm.lab.bos.redhat.com [21059] 1351684052.665269: Sending request (200 bytes) to IDM.LAB.BOS.REDHAT.COM [21059] 1351684052.665989: Resolving hostname vm-044.idm.lab.bos.redhat.com [21059] 1351684052.667511: Sending initial UDP request to dgram 10.16.78.44:88 [21059] 1351684052.672514: Received answer from dgram 10.16.78.44:88 [21059] 1351684052.672653: Response was from master KDC [21059] 1351684052.672751: Received error from KDC: -1765328370/KDC has no support for encryption type kinit: KDC has no support for encryption type while getting initial credentials Now when I comment includedir:
Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
On Wed, Oct 31, 2012 at 01:22:52PM +0100, Martin Kosek wrote: On 10/31/2012 11:00 AM, Jakub Hrozek wrote: On Mon, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: 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. OK, new patches are attached. In new installs, the includedir is only added when options.sssd is true. During upgrades, I checked for sssd.conf's existence, I'm not sure if there's any other way to check if the client was configured with sssd? Hello Jakub, thanks for these patches. I think that checking if /etc/sssd.conf exists as actually not so bad way to test if it was configured. Anyway, I did few tests on server and client but I still see few issues: 1) SELinux context of krb5.conf is not as expected (but I am not sure what real issue could that cause): # restorecon -FvvR /etc/krb5.conf restorecon reset /etc/krb5.conf context unconfined_u:object_r:etc_t:s0-system_u:object_r:krb5_conf_t:s0 Fixed. Thanks, I shouldn have noticed that doing mv would just replace the original context. 2) I can no longer kinit on IPA server after applying your patch # rpm -q sssd sssd-1.9.90-0.20121030T1436Zgitf46bf56.fc17.x86_64 # rpm -Uvh --force freeipa-*.rpm # head -n 5 /etc/krb5.conf includedir /var/lib/sss/pubconf/krb5.include.d/ [logging] default = FILE:/var/log/krb5libs.log kdc = FILE:/var/log/krb5kdc.log admin_server = FILE:/var/log/kadmind.log # KRB5_TRACE=/dev/stdout kinit admin [21059] 1351684052.658548: Getting initial credentials for ad...@idm.lab.bos.redhat.com [21059] 1351684052.665269: Sending request (200 bytes) to IDM.LAB.BOS.REDHAT.COM [21059] 1351684052.665989: Resolving hostname vm-044.idm.lab.bos.redhat.com [21059] 1351684052.667511: Sending initial UDP request to dgram 10.16.78.44:88 [21059] 1351684052.672514: Received answer from dgram 10.16.78.44:88 [21059] 1351684052.672653: Response was from master KDC [21059] 1351684052.672751: Received error from KDC: -1765328370/KDC has no support for encryption type kinit: KDC has no support for encryption type while getting initial
Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
On Tue, Nov 06, 2012 at 08:47:04AM +0100, Martin Kosek wrote: Once updated we get a slew of AVCs: Yeah, we will need to have those fixed before inclusion in IPA... Did you file any bugzilla or should I do it? Yes: https://bugzilla.redhat.com/show_bug.cgi?id=873429 ___ 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, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: 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. OK, new patches are attached. In new installs, the includedir is only added when options.sssd is true. During upgrades, I checked for sssd.conf's existence, I'm not sure if there's any other way to check if the client was configured with sssd? From 184e44e5209e2cdbe03446e3e00754bfc5b115df Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhro...@redhat.com Date: Fri, 17 Aug 2012 11:19:03 +0200 Subject: [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter for options --- ipa-client/ipaclient/ipachangeconf.py | 35 +++ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py index 6cf47b807957c245fe03ff4259e35526c49175a9..bdc5579fccd82193e837b5e86cbc694c2619317c 100644 --- a/ipa-client/ipaclient/ipachangeconf.py +++ b/ipa-client/ipaclient/ipachangeconf.py @@ -174,9 +174,12 @@ class IPAChangeConf: self.subsectdel[1])) continue if o['type'] == option: +delim = o.get('delim', self.dassign) +if delim not in self.assign: +raise ValueError('Unknown delim %s must be one of %s' % (delim, .join([d for d in self.assign]))) output.append(self._dump_line(self.indent[level], o['name'], - self.dassign, + delim, o['value'])) continue if o['type'] == comment: @@ -200,13 +203,21 @@ class IPAChangeConf: 'type': 'comment', 'value': value.rstrip()} # pylint: disable=E1103 +o = dict() parts = line.split(self.dassign, 1) if len(parts) 2: -
Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
On 10/31/2012 11:00 AM, Jakub Hrozek wrote: On Mon, Oct 22, 2012 at 02:14:00PM -0400, Simo Sorce wrote: 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. OK, new patches are attached. In new installs, the includedir is only added when options.sssd is true. During upgrades, I checked for sssd.conf's existence, I'm not sure if there's any other way to check if the client was configured with sssd? Hello Jakub, thanks for these patches. I think that checking if /etc/sssd.conf exists as actually not so bad way to test if it was configured. Anyway, I did few tests on server and client but I still see few issues: 1) SELinux context of krb5.conf is not as expected (but I am not sure what real issue could that cause): # restorecon -FvvR /etc/krb5.conf restorecon reset /etc/krb5.conf context unconfined_u:object_r:etc_t:s0-system_u:object_r:krb5_conf_t:s0 2) I can no longer kinit on IPA server after applying your patch # rpm -q sssd sssd-1.9.90-0.20121030T1436Zgitf46bf56.fc17.x86_64 # rpm -Uvh --force freeipa-*.rpm # head -n 5 /etc/krb5.conf includedir /var/lib/sss/pubconf/krb5.include.d/ [logging] default = FILE:/var/log/krb5libs.log kdc = FILE:/var/log/krb5kdc.log admin_server = FILE:/var/log/kadmind.log # KRB5_TRACE=/dev/stdout kinit admin [21059] 1351684052.658548: Getting initial credentials for ad...@idm.lab.bos.redhat.com [21059] 1351684052.665269: Sending request (200 bytes) to IDM.LAB.BOS.REDHAT.COM [21059] 1351684052.665989: Resolving hostname vm-044.idm.lab.bos.redhat.com [21059] 1351684052.667511: Sending initial UDP request to dgram 10.16.78.44:88 [21059] 1351684052.672514: Received answer from dgram 10.16.78.44:88 [21059] 1351684052.672653: Response was from master KDC [21059] 1351684052.672751: Received error from KDC: -1765328370/KDC has no support for encryption type kinit: KDC has no support for encryption type while getting initial credentials Now when I comment includedir: # head -n 5 /etc/krb5.conf # kinit admin Password for ad...@idm.lab.bos.redhat.com: # echo $? 0 When I upgraded client machine (without krb5kdc), kinit worked fine. Does that mean that krb5.conf can only be changed on client machines? 3) We
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] [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] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
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 From f93e181d4812dd66eda7cbc2cd9fc8ccc603e0c5 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhro...@redhat.com Date: Fri, 17 Aug 2012 11:19:03 +0200 Subject: [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter for options --- ipa-client/ipaclient/ipachangeconf.py | 37 +++ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py index 6cf47b807957c245fe03ff4259e35526c49175a9..087e096920854bdd822d83182102723a2082af2a 100644 --- a/ipa-client/ipaclient/ipachangeconf.py +++ b/ipa-client/ipaclient/ipachangeconf.py @@ -174,9 +174,13 @@ class IPAChangeConf: self.subsectdel[1])) continue if o['type'] == option: +delim = o.get('delim', self.dassign) +if delim not in self.assign: +raise ValueError('Unknown delim %s must be one of %s' % (delim, .join([d for d in self.assign]))) +output += self.indent[level]+o['name']+delim+o['value']+self.deol output.append(self._dump_line(self.indent[level], o['name'], - self.dassign, + delim, o['value'])) continue if o['type'] == comment: @@ -200,13 +204,21 @@ class IPAChangeConf: 'type': 'comment', 'value': value.rstrip()} # pylint: disable=E1103 +o = dict() parts = line.split(self.dassign, 1) if len(parts) 2: -raise SyntaxError('Syntax Error: Unknown line format') +# The default assign didn't match, try the non-default +for d in self.assign[1:]: +parts = line.split(d, 1) +if len(parts) = 2: +o['delim'] = d +break -return {'name': parts[0].strip(), -'type': 'option', -'value': parts[1].rstrip()} +if 'delim' not in o: +raise SyntaxError, 'Syntax Error: Unknown line format' + +o.update({'name':parts[0].strip(), 'type':'option', 'value':parts[1].rstrip()}) +return o def findOpts(self, opts, type, name, exclude_sections=False): @@ -256,13 +268,14 @@ class IPAChangeConf: 'value': val}) continue if o['type'] == 'option': -val = self._dump_line(self.indent[level], - o['name'], - self.dassign, - o['value']) -opts.append({'name': 'comment', - 'type': 'comment', - 'value': val}) +delim = o.get('delim', self.dassign) +
Re: [Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
On Mon, 2012-10-08 at 18:17 +0200, 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 ACK 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] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf
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? From 507dad241486258348153bedb06011e0f884c88f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhro...@redhat.com Date: Fri, 17 Aug 2012 11:19:03 +0200 Subject: [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter for options --- ipa-client/ipaclient/ipachangeconf.py | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py index f6288062be5c5d1b29341ed90814e1fa1431019c..f5ec227fee6a46eb666a1bfe9a3349e40f80b7e9 100644 --- a/ipa-client/ipaclient/ipachangeconf.py +++ b/ipa-client/ipaclient/ipachangeconf.py @@ -161,7 +161,10 @@ class IPAChangeConf: output += self.indent[level]+self.subsectdel[1]+self.deol continue if o['type'] == option: -output += self.indent[level]+o['name']+self.dassign+o['value']+self.deol +delim = o.get('delim', self.dassign) +if delim not in self.assign: +raise ValueError('Unknown delim %s must be one of %s' % (delim, .join([d for d in self.assign]))) +output += self.indent[level]+o['name']+delim+o['value']+self.deol continue if o['type'] == comment: output += self.dcomment+o['value']+self.deol @@ -182,11 +185,21 @@ class IPAChangeConf: if value: return {'name':'comment', 'type':'comment', 'value':value.rstrip()} #pylint: disable=E1103 +o = dict() parts = line.split(self.dassign, 1) if len(parts) 2: -raise SyntaxError, 'Syntax Error: Unknown line format' +# The default assign didn't match, try the non-default +for d in self.assign[1:]: +parts = line.split(d, 1) +if len(parts) = 2: +o['delim'] = d +break -return {'name':parts[0].strip(), 'type':'option', 'value':parts[1].rstrip()} +if 'delim' not in o: +raise SyntaxError, 'Syntax Error: Unknown line format' + +o.update({'name':parts[0].strip(), 'type':'option', 'value':parts[1].rstrip()}) +return o def findOpts(self, opts, type, name, exclude_sections=False): @@ -224,7 +237,9 @@ class IPAChangeConf: opts.append({'name':'comment', 'type':'comment', 'value':val}) continue if o['type'] == 'option': -val = self.indent[level]+o['name']+self.dassign+o['value'] +delim = o.get('delim', self.dassign) +if delim not in self.assign: +val = self.indent[level]+o['name']+delim+o['value'] opts.append({'name':'comment', 'type':'comment', 'value':val}) continue if o['type'] == 'comment': -- 1.7.11.2 From 74091875ec511b1b271f723eb17918544258a957 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhro...@redhat.com Date: Sun, 5 Aug 2012 20:47:12 +0200 Subject: [PATCH 2/3] Specify includedir in krb5.conf on new installs --- ipa-client/ipa-install/ipa-client-install | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 2e65921e8de2dfe68443f5b5875954d71dd48ed2..a4c1b6fbdfefc26f9be8297b6e60ac4d0da42876 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -642,7 +642,7 @@ def hardcode_ldap_server(cli_server): def configure_krb5_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, cli_kdc, dnsok, options, filename, client_domain): krbconf = ipaclient.ipachangeconf.IPAChangeConf(IPA Installer) -krbconf.setOptionAssignment( = ) +krbconf.setOptionAssignment(( = , )) krbconf.setSectionNameDelimiters(([,]))