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

Reply via email to