[SSSD] [PATCHES] Convert the SSSD to config_file_version 2

Sumit Bose sbose at redhat.com
Fri Sep 25 08:48:11 UTC 2009


Hi,

here are my comments:

On Thu, Sep 24, 2009 at 09:32:27PM -0400, Stephen Gallagher wrote:
> Patch 0001 (sgallagh): Convert the existing options to the new names.

--- a/server/providers/krb5/krb5_auth.c
+++ b/server/providers/krb5/krb5_auth.c
@@ -895,7 +895,7 @@ int sssm_krb5_auth_init(struct be_ctx *bectx,
     ctx->realm = value;
 
     ret = confdb_get_string(bectx->cdb, ctx, bectx->conf_path,
-                            CONFDB_KRB5_REALM, "/tmp", &value);
+                            CONFDB_KRB5_CCACHEDIR, "/tmp", &value);
     if (ret != EOK) goto fail;
     ret = lstat(value, &stat_buf);
     if (ret != EOK) {





> Patch 0002 (jhrozek): Update the manpages to reflect the new names.

- there is too much whitespace at line 771 of the patch
- in the example the sssd-krb5 the '[domains] section' is referred to and
  the example uses [domains/FOO] instead of [domain/FOO]. Maybe it
  should be underlined that this example is incomplete (missing
  id_proiver)


> Patch 0003 (jhrozek+sgallagh): Create a python script to convert
> existing config files to the new format and run it automatically during
> %post in the sssd.spec
> 

- is it planned to use upgrade_config.py for future upgrade, too, or is
  it planned to have separate tools for each upgrade. In the latter case
  I would call to something like upgrade_config_v1_v2.py
- when using '-f /somewhere/my.conf' the backup is created in /somewhere
  but the new config is written to /etc/sss/sssd.conf. My expectation was
  that the conversion would be done in-place. If you prefer it this way
  '--help' should say that the default output file is
  /etc/sss/sssd.conf

- upgrade_config.py has problems with '%' used in the
  krb5_ccname_template:

ERROR: '%' must be followed by '%' or '(', found: '%h/krb5cc_%u_%U_%p_%r_%P_%%_XXXXXX'
Traceback (most recent call last):
  File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 327, in main
    config.upgrade_v2(options.outfile, options.backup)
  File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 274, in upgrade_v2
    self._migrate_domains()
  File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 210, in _migrate_domains
    self._migrate_domain(domain)
  File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 200, in _migrate_domain
    self._migrate_kw(new_domsec, old_domsec, krb5_kw)
  File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 101, in _migrate_kw
    self._migrate_if_exists(to_section, new, from_section, old)
  File "/tmp/sssd_test/libexec/sssd/upgrade_config.py", line 93, in _migrate_if_exists
    self._config.get(from_section, from_option))
  File "/usr/lib/python2.6/ConfigParser.py", line 545, in get
    return self._interpolate(section, option, value, d)
  File "/usr/lib/python2.6/ConfigParser.py", line 613, in _interpolate
    self._interpolate_some(option, L, rawval, section, vars, 1)
  File "/usr/lib/python2.6/ConfigParser.py", line 655, in _interpolate_some
    "'%%' must be followed by '%%' or '(', found: %r" % (rest,))
InterpolationSyntaxError: '%' must be followed by '%' or '(', found: '%h/krb5cc_%u_%U_%p_%r_%P_%%_XXXXXX'


bye,
Sumit

> Jakub has reviewed patch 0001 and I have reviewed Patch 0002 and 0003
> (though I made some additional fixes to his version of 0003). I'm
> looking for one additional reviewer in order to push these.




More information about the sssd-devel mailing list