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.