Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 04/29/2013 10:28 PM, Tomas Babej wrote: On 04/29/2013 08:13 PM, Rob Crittenden wrote: Tomas Babej wrote: On 04/25/2013 12:42 PM, Martin Kosek wrote: On 04/25/2013 12:29 PM, Jan Cholasta wrote: On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them is already configured or none at all. 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? This would still break an existing user configuration, we would just tell user what we broke :-) Martin Honza The following version of the patch configures (BASE, URI, TLS_CACERT) attributes if they are not set. However, to preserve user-friendliness, our suggested option is added as an comment. See commit message for details. Tomas Ok, this works as advertised, I just have a general question. This could enable a mix of IPA and non-IPA settings. In my case I left BASE configured and only URI and TLS_CACERT got set. This could cause some unexpected results I think, depending on what got changed. Do we rather want to punt on all of them if any of them can't be updated? This would require a bit more code, and wouldn't be as generic. I just wonder if this would cause subtle failures. rob After IRC conversation with Rob, we decided to keep the behaviour, while having it explicitly mentioned in the ldap.conf file. For illustration, the ldap.conf file could look like this: [/etc/openldap/ldap.conf] # File modified by ipa-client-install # We do not want to break your existing configuration, hence: # URI, BASE and TLS_CACERT have been added if they were not set. # In case any of them were set, a comment with trailing note # # modified by IPA note has been inserted. # To use IPA server with openLDAP tools, please comment out your # existing configuration for these options and uncomment the # corresponding lines generated by IPA. # # LDAP Defaults # # See ldap.conf(5) for details # This file should be world readable but not world writable. #BASE dc=ipa,dc=example,dc=com # modified by IPA BASEdc=example,dc=com #URI ldaps://ipa.example.com # modified by IPA URI ldap://ldap.example.com #SIZELIMIT 12 #TIMELIMIT 15 #DEREF never TLS_CACERTDIR /etc/openldap/cacerts TLS_CACERT /etc/ipa/ca.crt Tomas [...] +root_logger.info(Could not parse {path}.format(path=target_fname)) +root_logger.debug(error_msg.format(path=target_fname, err=str(e))) To my (limited) knowledge, this would be the first Python 2.6+ feature used in the client code. Is it OK? Normally we use multiple arguments for logging: root_logger.info(Could not parse %s, target_fname)
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
Tomas Babej wrote: On 04/29/2013 08:13 PM, Rob Crittenden wrote: Tomas Babej wrote: On 04/25/2013 12:42 PM, Martin Kosek wrote: On 04/25/2013 12:29 PM, Jan Cholasta wrote: On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them is already configured or none at all. 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? This would still break an existing user configuration, we would just tell user what we broke :-) Martin Honza The following version of the patch configures (BASE, URI, TLS_CACERT) attributes if they are not set. However, to preserve user-friendliness, our suggested option is added as an comment. See commit message for details. Tomas Ok, this works as advertised, I just have a general question. This could enable a mix of IPA and non-IPA settings. In my case I left BASE configured and only URI and TLS_CACERT got set. This could cause some unexpected results I think, depending on what got changed. Do we rather want to punt on all of them if any of them can't be updated? This would require a bit more code, and wouldn't be as generic. I just wonder if this would cause subtle failures. rob After IRC conversation with Rob, we decided to keep the behaviour, while having it explicitly mentioned in the ldap.conf file. For illustration, the ldap.conf file could look like this: [/etc/openldap/ldap.conf] # File modified by ipa-client-install # We do not want to break your existing configuration, hence: # URI, BASE and TLS_CACERT have been added if they were not set. # In case any of them were set, a comment with trailing note # # modified by IPA note has been inserted. # To use IPA server with openLDAP tools, please comment out your # existing configuration for these options and uncomment the # corresponding lines generated by IPA. # # LDAP Defaults # # See ldap.conf(5) for details # This file should be world readable but not world writable. #BASE dc=ipa,dc=example,dc=com # modified by IPA BASEdc=example,dc=com #URI ldaps://ipa.example.com # modified by IPA URI ldap://ldap.example.com #SIZELIMIT 12 #TIMELIMIT 15 #DEREF never TLS_CACERTDIR /etc/openldap/cacerts TLS_CACERT /etc/ipa/ca.crt Tomas ACK, pushed to master rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
Petr Viktorin wrote: On 04/29/2013 10:28 PM, Tomas Babej wrote: On 04/29/2013 08:13 PM, Rob Crittenden wrote: Tomas Babej wrote: On 04/25/2013 12:42 PM, Martin Kosek wrote: On 04/25/2013 12:29 PM, Jan Cholasta wrote: On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them is already configured or none at all. 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? This would still break an existing user configuration, we would just tell user what we broke :-) Martin Honza The following version of the patch configures (BASE, URI, TLS_CACERT) attributes if they are not set. However, to preserve user-friendliness, our suggested option is added as an comment. See commit message for details. Tomas Ok, this works as advertised, I just have a general question. This could enable a mix of IPA and non-IPA settings. In my case I left BASE configured and only URI and TLS_CACERT got set. This could cause some unexpected results I think, depending on what got changed. Do we rather want to punt on all of them if any of them can't be updated? This would require a bit more code, and wouldn't be as generic. I just wonder if this would cause subtle failures. rob After IRC conversation with Rob, we decided to keep the behaviour, while having it explicitly mentioned in the ldap.conf file. For illustration, the ldap.conf file could look like this: [/etc/openldap/ldap.conf] # File modified by ipa-client-install # We do not want to break your existing configuration, hence: # URI, BASE and TLS_CACERT have been added if they were not set. # In case any of them were set, a comment with trailing note # # modified by IPA note has been inserted. # To use IPA server with openLDAP tools, please comment out your # existing configuration for these options and uncomment the # corresponding lines generated by IPA. # # LDAP Defaults # # See ldap.conf(5) for details # This file should be world readable but not world writable. #BASE dc=ipa,dc=example,dc=com # modified by IPA BASEdc=example,dc=com #URI ldaps://ipa.example.com # modified by IPA URI ldap://ldap.example.com #SIZELIMIT 12 #TIMELIMIT 15 #DEREF never TLS_CACERTDIR /etc/openldap/cacerts TLS_CACERT /etc/ipa/ca.crt Tomas [...] +root_logger.info(Could not parse {path}.format(path=target_fname)) +root_logger.debug(error_msg.format(path=target_fname, err=str(e))) To my (limited) knowledge, this would be the first Python 2.6+ feature used in the client code. Is it OK? Yes, we have no plans to rebase 3.x back to an older distro. It may make
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 04/25/2013 12:42 PM, Martin Kosek wrote: On 04/25/2013 12:29 PM, Jan Cholasta wrote: On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them is already configured or none at all. 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? This would still break an existing user configuration, we would just tell user what we broke :-) Martin Honza The following version of the patch configures (BASE, URI, TLS_CACERT) attributes if they are not set. However, to preserve user-friendliness, our suggested option is added as an comment. See commit message for details. Tomas From 80e70a0a68d64eb9826a8ff4c20330f49aa0d362 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Mon, 22 Apr 2013 12:55:38 +0200 Subject: [PATCH] Preserve already configured options in openldap conf We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options only if they are not configured. In the case they are, our suggested configuration is inserted as a comment. Also adds tab as a delimeter character in /etc/openldap/ldap.conf https://fedorahosted.org/freeipa/ticket/3582 --- ipa-client/ipa-install/ipa-client-install | 41 +++ ipa-client/ipaclient/ipachangeconf.py | 12 +++-- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 29adc93f3bcb3ccc81c31237af314af0ba61b8c9..c71c5e9b152dca2f66ffb26c3fa6241371666035 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -815,19 +815,38 @@ def configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, def configure_openldap_conf(fstore, cli_basedn, cli_server): ldapconf = ipaclient.ipachangeconf.IPAChangeConf(IPA Installer) -ldapconf.setOptionAssignment( ) +ldapconf.setOptionAssignment(( , \t)) -opts = [{'name':'comment', 'type':'comment', 'value':'File modified by ipa-client-install'}, -{'name':'empty', 'type':'empty'}, -{'name':'URI', 'type':'option', 'value':'ldaps://'+ cli_server[0]}, -{'name':'BASE', 'type':'option', 'value':cli_basedn}, -{'name':'TLS_CACERT', 'type':'option', 'value':CACERT}, -{'name':'empty', 'type':'empty'}] +opts = [{'name':'comment', 'type':'comment', +'value':'File modified by ipa-client-install'}, +
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
Tomas Babej wrote: On 04/25/2013 12:42 PM, Martin Kosek wrote: On 04/25/2013 12:29 PM, Jan Cholasta wrote: On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them is already configured or none at all. 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? This would still break an existing user configuration, we would just tell user what we broke :-) Martin Honza The following version of the patch configures (BASE, URI, TLS_CACERT) attributes if they are not set. However, to preserve user-friendliness, our suggested option is added as an comment. See commit message for details. Tomas Ok, this works as advertised, I just have a general question. This could enable a mix of IPA and non-IPA settings. In my case I left BASE configured and only URI and TLS_CACERT got set. This could cause some unexpected results I think, depending on what got changed. Do we rather want to punt on all of them if any of them can't be updated? This would require a bit more code, and wouldn't be as generic. I just wonder if this would cause subtle failures. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 04/29/2013 08:13 PM, Rob Crittenden wrote: Tomas Babej wrote: On 04/25/2013 12:42 PM, Martin Kosek wrote: On 04/25/2013 12:29 PM, Jan Cholasta wrote: On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them is already configured or none at all. 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? This would still break an existing user configuration, we would just tell user what we broke :-) Martin Honza The following version of the patch configures (BASE, URI, TLS_CACERT) attributes if they are not set. However, to preserve user-friendliness, our suggested option is added as an comment. See commit message for details. Tomas Ok, this works as advertised, I just have a general question. This could enable a mix of IPA and non-IPA settings. In my case I left BASE configured and only URI and TLS_CACERT got set. This could cause some unexpected results I think, depending on what got changed. Do we rather want to punt on all of them if any of them can't be updated? This would require a bit more code, and wouldn't be as generic. I just wonder if this would cause subtle failures. rob After IRC conversation with Rob, we decided to keep the behaviour, while having it explicitly mentioned in the ldap.conf file. For illustration, the ldap.conf file could look like this: [/etc/openldap/ldap.conf] # File modified by ipa-client-install # We do not want to break your existing configuration, hence: # URI, BASE and TLS_CACERT have been added if they were not set. # In case any of them were set, a comment with trailing note # # modified by IPA note has been inserted. # To use IPA server with openLDAP tools, please comment out your # existing configuration for these options and uncomment the # corresponding lines generated by IPA. # # LDAP Defaults # # See ldap.conf(5) for details # This file should be world readable but not world writable. #BASE dc=ipa,dc=example,dc=com # modified by IPA BASEdc=example,dc=com #URI ldaps://ipa.example.com # modified by IPA URI ldap://ldap.example.com #SIZELIMIT 12 #TIMELIMIT 15 #DEREF never TLS_CACERTDIR /etc/openldap/cacerts TLS_CACERT /etc/ipa/ca.crt Tomas From 9a41e0d875517be599d6393090b856cd98d8602c Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Mon, 22 Apr 2013 12:55:38 +0200 Subject: [PATCH] Preserve already configured options in openldap conf We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 04/25/2013 12:29 PM, Jan Cholasta wrote: On 25.4.2013 08:51, Martin Kosek wrote: On 04/24/2013 08:02 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob Yeah, I also agree that 1) is enough. It will not add any more options and will let us be more gentle and respectful to already existent custom user settings in ldap.conf. So Tomas, this seems like the way to go :-) Martin I don't see the point of updating only some of these values. What about Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them is already configured or none at all. 4) Update BASE and URI and TLS_CACERT, comment any old settings out. ? This would still break an existing user configuration, we would just tell user what we broke :-) Martin Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf
Jan Cholasta wrote: On 24.4.2013 14:54, Martin Kosek wrote: On 04/24/2013 02:51 PM, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 23.4.2013 12:28, Tomas Babej wrote: Hi, We should respect already configured options present in /etc/openldap/ldap.conf when generating our own configuration. With this patch, we only rewrite URI, BASE and TLS_CACERT options. https://fedorahosted.org/freeipa/ticket/3582 the changeConf call will fail when the file does not exist, we might want to handle that gracefully. Honza We also need to handle the case where these items are already defined. I'm honestly not sure what the behavior should be: overwrite, warn and overwrite, fail. rob I am also thinking that we may want to be more cautious before updating this file. AFAIK, we do not need the updated file for our function, its only updated for user convenience so that he can run ldapsearches more easily. I see several options here that could help this goal: 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these options are not set. If the options are already set, we could just print a note that we skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has these options commented out, so it should be possible to implement this check. 2) Do ldap.conf changes only if a new special option is passe (e.g. --configure-ldap-cong) 3) Do not update ldap.conf when a new special option is not passed (e.g. --no-ldap-conf Martin If we don't need the file for our function, we can just not configure it at all IMO. We can document how to configure it for users who want it. It was an RFE that we create this file. It is handy to have pre-configured, I like having it actually. We just need to try to have a gentler touch than my first crack at it, which overwrote it completely. I think #1 is probably enough for now. I'm not sure I want to add two new options this late in the game, and the client already has a lot of knobs. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel