On 08/27/2012 01:42 PM, Martin Kosek wrote: > On 08/17/2012 03:04 PM, Tomas Babej wrote: >> Hi, >> >> The sssd.conf file is no longer left behind in case sssd was not >> configured before the installation. >> >> https://fedorahosted.org/freeipa/ticket/2740 >> >> Tomas >> > > I found few issues with this approach: > > 1) (major) We do not want to delete sssd.conf when there were more domain's > that just the IPA one configured (was_sssd_configured variable). > > I would consider changing it like that: > > ... > if was_sssd_installed and was_sssd_configured: > # SSSD was installed before our installation, config now is restored, > restart it > ... > elif not was_sssd_configured: > - remove sssd.conf file (or move it to sssd.conf.deleted as Stephen > suggested if we want to be more defensive) > ... > > If we choose the rename operation over delete operation, we should inform a > user about the rename as well. > > 2) (minor) As I mentioned earlier, as a rule of thumb it is better to catch > more specific exceptions that just bare "except" clause, PEP8 should contain > more info to this cause. In this case, I would catch just for OSError > exception. > > Martin
I had a short discussion with Tomas and current state of handling of sssd.conf during uninstall looks weird. Uninstall code do check if there is some non-IPA domain configured in current sssd.conf, but if it is, we still rather restore old sssd.conf which was backed up during ipa-client-install instead of modifying current one. I think this may cause user authentication issues if he configured non-IPA domains after ipa-client-install. I think that the right behavior of SSSD conf uninstall should be the following: * sssd.conf existed before IPA install + non-IPA domains in sssd.conf found: - move backed conf up sssd.conf.bkp (and inform the user) - use SSSDConfig delete_domain function to remove ipa domain from sssd.conf - restart sssd afterwards * sssd.conf did not exist before IPA install + non-IPA domains in sssd.conf found: - use SSSDConfig delete_domain function to remove ipa domain from sssd.conf - restart sssd afterwards * sssd.conf did not exist before IPA install + no other domains in sssd.conf: - remove sssd.conf or rename it to sssd.conf.deleted Jakub, any recommendations? Thanks, Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel