Alexander Bokovoy wrote:
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.


Well, in the "generate new file" option I think the output is a bit misleading.

+ print "New SSSD config will be generated. The old one is backed up and can be restored during uninstall"

There could have been no existing sssd.conf, right?

+ logging.error("Failed to parse SSSD configuration and will generate new one")

This could imply that an error occurred when in fact there just was no sssd.conf to import.

Otherwise the approach looks good.

rob

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to