[Freeipa-devel] [PATCH] 0012 Modify existing SSSD configuration instead of dropping it

Alexander Bokovoy abokovoy at redhat.com
Wed Oct 12 16:24:08 UTC 2011


On Tue, 13 Sep 2011, Stephen Gallagher wrote:

> On Tue, 2011-09-13 at 16:33 +0300, Alexander Bokovoy wrote:
> > On Tue, 13 Sep 2011, Stephen Gallagher wrote:
> > > > >   File "/usr/lib/python2.7/site-packages/SSSDConfig.py", line 1207, in import_config
> > > > >     fd = open(configfile, 'r')
> > > > > IOError: [Errno 2] No such file or directory: '/etc/sssd/sssd.conf'
> > > > Right, we need to fallback to new sssd.conf in case of any exception, 
> > > > not only for ParsingError.
> > > Actually, that's not necessarily true. Do we want to fall back on
> > > permission error, for instance? This could result in clobbering an
> > > existing file (if for example the existing sssd.conf's SELinux context
> > > is wrong, preventing reading, but when we create a new one and save it
> > > in place later we have the right context and it replaces the old one).
> > Let's define what we want to see here.
> > 
> > 1. There is no sssd.conf -> create new one (unlikely for existing SSSD 
> > installation -- if we went to this path, we already found SSSD 
> > installed)
> 
> Actually, this is fairly likely in future releases. There's so much
> noise in the example configuration that I've decided that we're going to
> install it as %doc instead of %config. So a raw installation of the SSSD
> package will have no sssd.conf in place. We obviously need to consider
> this.
> 
> > 2. There is sssd.conf -> modify existing one
> >    2.1. Can't open for write -> report error
> 
> Agreed.
> 
> >    2.2. Can't open and read due to parsing error -> create new one
> > ...
> 
> There are two issues here. Failure to open for reading and ParseError on
> read. That said, I think they should be handled the same way (see
> below).
> 
> 
> Unfortunately, we have additional issues here... The SSSDConfig API is
> more strict with options than the SSSD itself is. Specifically, the SSSD
> will ignore unknown options entirely, but the SSSDConfig will through a
> ParseError on unknown options.
> 
> The long-term goal is for SSSD to be more strict about this, but we
> don't currently have a way to do this.
> 
> So as I see it, we have three choices for dealing with ParseError:
> 
> 1) Back up the existing sssd.conf and replace it with a completely new
> one for FreeIPA. Pros: sssd.conf is guaranteed to parse cleanly. FreeIPA
> client install completes successfully. Cons: existing configuration
> (which may have worked) is not preserved.
> 
> 2) Be restrictive on ParseError and throw an error telling them to fix
> their config file. Pros: we don't break an existing setup. Cons: FreeIPA
> installation has been broken.
> 
> 3) Default to one of the above but provide a command-line flag to behave
> the other way. This is probably our best bet. I'd suggest defaulting to
> replacing the config file on ParseError (with a loud message at the END
> of ipa-client-install pointing to the backed-up file).
Attached patch tries to implement these ideas. It is untested as I 
wanted to get feedback on the approach first.


-- 
/ Alexander Bokovoy
-------------- next part --------------
>From 459e77ebd6a6e0d6f1371d93fb1a16dad84ff1bf Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Wed, 12 Oct 2011 19:14:55 +0300
Subject: [PATCH] Refactor authconfig use in ipa-client-install

When certain features are being configured via authconfig, we need to remember
what was configured and what was the state before it so that during uninstall
we restore proper state of the services.

Mostly it affects sssd configuration with multiple domains but also pre-existing
LDAP and krb5 configurations.

This should fix following tickets:
https://fedorahosted.org/freeipa/ticket/1750
https://fedorahosted.org/freeipa/ticket/1769
---
 ipa-client/ipa-install/ipa-client-install |  104 +++++++++++++++++++++++------
 ipapython/sysrestore.py                   |   13 ++++
 2 files changed, 97 insertions(+), 20 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 969dc9b0faa5e131f1e9199325bdf2350157ab8a..1bc2d719c84c44a629041d0ec9609ab8c13b9cdd 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -108,6 +108,9 @@ def parse_options():
     sssd_group.add_option("-S", "--no-sssd", dest="sssd",
                       action="store_false", default=True,
                       help="Do not configure the client to use SSSD for authentication")
+    sssd_group.add_option("--preserve-sssd", dest="preserve_sssd",
+                      action="store_true", default=False,
+                      help="Preserve old SSSD configuration if possible")
     parser.add_option_group(sssd_group)
 
     uninstall_group = OptionGroup(parser, "uninstall options")
@@ -177,22 +180,33 @@ def uninstall(options, env, quiet=False):
         print "Refer to ipa-server-install for uninstallation."
         return CLIENT_NOT_CONFIGURED
 
-    sssdconfig = SSSDConfig.SSSDConfig()
-    sssdconfig.import_config()
-    domains = sssdconfig.list_active_domains()
-
     hostname = None
-    for name in domains:
-        domain = sssdconfig.get_domain(name)
-        try:
-            provider = domain.get_option('id_provider')
-        except SSSDConfig.NoOptionError:
-            continue
-        if provider == "ipa":
+    was_sssd_configured = False
+    try:
+        sssdconfig = SSSDConfig.SSSDConfig()
+        sssdconfig.import_config()
+        domains = sssdconfig.list_active_domains()
+        if len(domains) > 1:
+            # There was more than IPA domain configured
+            was_sssd_configured = True
+        for name in domains:
+            domain = sssdconfig.get_domain(name)
             try:
-                hostname = domain.get_option('ipa_hostname')
+                provider = domain.get_option('id_provider')
             except SSSDConfig.NoOptionError:
                 continue
+            if provider == "ipa":
+                try:
+                    hostname = domain.get_option('ipa_hostname')
+                except SSSDConfig.NoOptionError:
+                    continue
+    except Exception, e:
+        # We were unable to read existing SSSD config. This might mean few things:
+        # - sssd wasn't installed
+        # - sssd was removed after install and before uninstall
+        # - there are no active domains
+        # in both cases we cannot continue with SSSD
+        pass
 
     if hostname is None:
         hostname = socket.getfqdn()
@@ -266,14 +280,31 @@ def uninstall(options, env, quiet=False):
             logging.debug("Failed to remove Kerberos service principals: %s" % str(e))
 
     emit_quiet(quiet, "Disabling client Kerberos and LDAP configurations")
+    was_sssd_installed = False
+    if fstore.has_files():
+        was_sssd_installed = fstore.has_file("/etc/sssd/sssd.conf")
     try:
         auth_config = ipaservices.authconfig()
-        auth_config.disable("ldap").\
-                    disable("krb5").\
-                    disable("sssd").\
-                    disable("sssdauth").\
-                    disable("mkhomedir").\
-                    add_option("update")
+        if statestore.has_state('authconfig'):
+            # disable only those configurations that we enabled during install
+            for conf in ('ldap', 'krb5', 'sssd', 'sssdauth', 'mkhomedir'):
+                cnf = statestore.restore_state('authconfig', conf)
+                if cnf:
+                    auth_config.disable(conf)
+        else:
+            # There was no authconfig status store
+            # It means the code was upgraded after original install
+            # Fall back to old logic
+            auth_config.disable("ldap").\
+                        disable("krb5")
+            if not(was_sssd_installed and was_sssd_configured):
+                # assume there was sssd.conf before install and there were more than one domain in it
+                # In such case restoring sssd.conf will require us to keep SSSD running
+                auth_config.disable("sssd").\
+                            disable("sssdauth")
+            auth_config.disable("mkhomedir")
+
+        auth_config.add_option("update")
         auth_config.execute()
     except Exception, e:
         emit_quiet(quiet, "Failed to remove krb5/LDAP configuration. " +str(e))
@@ -345,6 +376,13 @@ def uninstall(options, env, quiet=False):
            if restored:
                ipaservices.knownservices.ntpd.restart()
 
+    if was_sssd_installed and was_sssd_configured:
+        # SSSD was installed before our installation, config now is restored, restart it
+        emit_quiet(quiet, "The original configuration of SSSD included other domains than IPA-based one.")
+        emit_quiet(quiet, "Original configuration file is restored, restarting SSSD service.")
+        sssd = ipaservices.service('sssd')
+        sssd.restart()
+
     if not options.unattended:
         emit_quiet(quiet, "The original nsswitch.conf configuration has been restored.")
         emit_quiet(quiet, "You may need to restart services or reboot the machine.")
@@ -618,8 +656,29 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options):
             print "%s request for host certificate failed" % (cmonger.service_name)
 
 def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options):
-    sssdconfig = SSSDConfig.SSSDConfig()
-    sssdconfig.new_config()
+    try:
+        sssdconfig = SSSDConfig.SSSDConfig()
+        sssdconfig.import_config()
+    except Exception, e:
+        if os.path.exists("/etc/sssd/sssd.conf") and options.preserve_sssd:
+            # SSSD config is in place but we are unable to read it
+            # In addition, we are instructed to preserve it
+            # This all means we can't use it and have to bail out
+            print "SSSD config exists but cannot be parsed: %s" % (str(e))
+            print "Correct errors in /etc/sssd/sssd.conf and re-run installation"
+            logging.error("Failed to parse SSSD configuration and was instructed to preserve existing SSSD config: %s" % (str(e)))
+            return 1
+
+        # SSSD configuration does not exist or we are not asked to preserve it, create new one
+        # We do make new SSSDConfig instance because IPAChangeConf-derived classes have no
+        # means to reset their state and ParseError exception could come due to parsing
+        # error from older version which cannot be upgraded anymore, leaving sssdconfig
+        # instance practically unusable
+        # Note that we already backed up sssd.conf before going into this routine
+        print "New SSSD config will be generated. The old one is backed up and can be restored during uninstall"
+        logging.error("Failed to parse SSSD configuration and will generate new one")
+        sssdconfig = SSSDConfig.SSSDConfig()
+        sssdconfig.new_config()
 
     domain = sssdconfig.new_domain(cli_domain)
     domain.add_provider('ipa', 'id')
@@ -1085,16 +1144,20 @@ def install(options, env, fstore, statestore):
     # Modify nsswitch/pam stack
     auth_config = ipaservices.authconfig()
     if options.sssd:
+        statestore.backup_state('authconfig', 'sssd', True)
+        statestore.backup_state('authconfig', 'sssdauth', True)
         auth_config.enable("sssd").\
                     enable("sssdauth")
         message = "SSSD enabled"
         conf = 'SSSD'
     else:
+        statestore.backup_state('authconfig', 'ldap', True)
         auth_config.enable("ldap").\
                     enable("forcelegacy")
         message = "LDAP enabled"
 
     if options.mkhomedir:
+        statestore.backup_state('authconfig', 'mkhomedir', True)
         auth_config.enable("mkhomedir")
 
     auth_config.add_option("update")
@@ -1104,6 +1167,7 @@ def install(options, env, fstore, statestore):
     if not options.sssd:
         #Modify pam to add pam_krb5 only when sssd is not in use
         auth_config.reset()
+        statestore.backup_state('authconfig', 'krb5', True)
         auth_config.enable("krb5").\
                     add_option("update").\
                     add_option("nostart")
diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
index 9b0e39fcb6b2b5035f1bd5012ac598309c83e9ae..e22b4d4fa617141c3546290d7f049f5037651ce1 100644
--- a/ipapython/sysrestore.py
+++ b/ipapython/sysrestore.py
@@ -130,6 +130,19 @@ class FileStore:
         self.files[filename] = string.join([str(stat.st_mode),str(stat.st_uid),str(stat.st_gid),path], ',')
         self.save()
 
+    def has_file(self, path):
+        """Checks whether file at @path was added to the file store
+
+        Returns #True if the file exists in the file store, #False otherwise
+        """
+        result = False
+        for (key, value) in self.files.items():
+            (mode,uid,gid,filepath) = string.split(value, ',', 3)
+            if (filepath == path):
+                result = True
+                break
+        return result
+
     def restore_file(self, path):
         """Restore the copy of a file at @path to its original
         location and delete the copy.
-- 
1.7.6.4



More information about the Freeipa-devel mailing list