On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
-    } else if (version < CONFDB_VERSION_INT) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-                "Config file is an old version. "
-                 "Please run configuration upgrade script.\n");
-        ret = EINVAL;
-        goto done;
-    } else if (version > CONFDB_VERSION_INT) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-                "Config file version is newer than confdb\n");
-        ret = EINVAL;
-        goto done;
+        /* No known version. Use default. */
+        DEBUG(SSSDBG_CONF_SETTINGS,
+              "Value of config_file_version option not found. "
+              "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
+    } else {
+        version = sss_ini_get_int_config_value(init_data,
+                                               CONFDB_DEFAULT_CFG_FILE_VER,
+                                               -1, &ret);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_FATAL_FAILURE,
+                    "Config file version could not be determined\n");
                       ^^
I do not prefer nested "if"s. If you decided to do it in this way then

Me neither, but sss_ini_get_int_config_value() has to be
skipped conditionally. It is just call to the function
plus error checking that is nested. I think it is not
too bad in this case.

you shoudl have proper indentation.


Fixed in the new version.

It works because integration tests passed.
http://sssd-ci.duckdns.org/logs/job/20/68/summary.html
But ...

I tested new version with ipa-client-install
and "config_file_version = 2" is still added to sssd.conf
even though it is a default value.

ipa-client-install uses our python API (python-sssdconfig)
and it does not try to add this option itself.

I often change version of SSSD with git checkout sssd<version>.
If I generate the config with realmd or ipa-client-install
with latest version then the config_version_file
would need to be added manually (and I am pretty
sure it would be after I looked into logs to see why sssd
is not starting). I know this will probably only hit
testers/developers, but I would prefer not to add unnecessary
little inconveniences.


Please also remove config_file_version from test_memory_cache.py


We do not necessary need to have all tests without
the option to test if the default works. But I removed
it from test_memory_cache.py anyway.

Michal

LS
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


>From 2168de02177ffbd22281484aa8a0b974caca21c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <[email protected]>
Date: Tue, 7 Jul 2015 15:15:32 +0200
Subject: [PATCH] CONFDB: Assume config file version 2 if missing

Default to config file version 2 if the version
is not specified explicitly.

Ticket:
https://fedorahosted.org/sssd/ticket/2688
---
 src/confdb/confdb.h                          |  1 +
 src/confdb/confdb_setup.c                    | 48 ++++++++++++++--------------
 src/config/SSSDConfig/sssd_upgrade_config.py |  3 +-
 src/tests/intg/ldap_test.py                  |  2 --
 src/tests/intg/test_memory_cache.py          |  3 --
 5 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index df45433..dece899 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -38,6 +38,7 @@
  * @{
  */
 
+#define CONFDB_DEFAULT_CFG_FILE_VER 2
 #define CONFDB_FILE "config.ldb"
 #define CONFDB_DEFAULT_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf"
 #define SSSD_MIN_ID 1
diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c
index 93a1a1b..694a7f0 100644
--- a/src/confdb/confdb_setup.c
+++ b/src/confdb/confdb_setup.c
@@ -224,30 +224,30 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
 
     ret = sss_ini_check_config_obj(init_data);
     if (ret != EOK) {
-        /* No known version. Assumed to be version 1 */
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              "Config file is an old version. "
-               "Please run configuration upgrade script.\n");
-        ret = EINVAL;
-        goto done;
-    }
-
-    version = sss_ini_get_int_config_value(init_data, 1, -1, &ret);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-                "Config file version could not be determined\n");
-        goto done;
-    } else if (version < CONFDB_VERSION_INT) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-                "Config file is an old version. "
-                 "Please run configuration upgrade script.\n");
-        ret = EINVAL;
-        goto done;
-    } else if (version > CONFDB_VERSION_INT) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-                "Config file version is newer than confdb\n");
-        ret = EINVAL;
-        goto done;
+        /* No known version. Use default. */
+        DEBUG(SSSDBG_CONF_SETTINGS,
+              "Value of config_file_version option not found. "
+              "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
+    } else {
+        version = sss_ini_get_int_config_value(init_data,
+                                               CONFDB_DEFAULT_CFG_FILE_VER,
+                                               -1, &ret);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_FATAL_FAILURE,
+                  "Config file version could not be determined\n");
+            goto done;
+        } else if (version < CONFDB_VERSION_INT) {
+            DEBUG(SSSDBG_FATAL_FAILURE,
+                  "Config file is an old version. "
+                  "Please run configuration upgrade script.\n");
+            ret = EINVAL;
+            goto done;
+        } else if (version > CONFDB_VERSION_INT) {
+            DEBUG(SSSDBG_FATAL_FAILURE,
+                  "Config file version is newer than confdb\n");
+            ret = EINVAL;
+            goto done;
+        }
     }
 
     /* Set up a transaction to replace the configuration */
diff --git a/src/config/SSSDConfig/sssd_upgrade_config.py b/src/config/SSSDConfig/sssd_upgrade_config.py
index 282d6c4..767d06d 100644
--- a/src/config/SSSDConfig/sssd_upgrade_config.py
+++ b/src/config/SSSDConfig/sssd_upgrade_config.py
@@ -47,7 +47,8 @@ class SSSDConfigFile(SSSDChangeConf):
     def get_version(self):
         ver = self.get_option_index('sssd', 'config_file_version')[1]
         if not ver:
-            return 1
+            # config_file_version not found -> default to version 2
+            return 2
         try:
             return int(ver['value'])
         except ValueError:
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index bfe4e65..d01ebc7 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -116,7 +116,6 @@ def sanity_rfc2307(request, ldap_conn):
     conf = unindent("""\
         [sssd]
         debug_level         = 0xffff
-        config_file_version = 2
         domains             = LDAP
         services            = nss, pam
 
@@ -174,7 +173,6 @@ def sanity_rfc2307_bis(request, ldap_conn):
     conf = unindent("""\
         [sssd]
         debug_level             = 0xffff
-        config_file_version     = 2
         domains                 = LDAP
         services                = nss, pam
 
diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py
index 1fd577e..9e76a4a 100644
--- a/src/tests/intg/test_memory_cache.py
+++ b/src/tests/intg/test_memory_cache.py
@@ -136,7 +136,6 @@ def sanity_rfc2307(request, ldap_conn):
 
     conf = unindent("""\
         [sssd]
-        config_file_version = 2
         domains             = LDAP
         services            = nss
 
@@ -162,7 +161,6 @@ def fqname_rfc2307(request, ldap_conn):
 
     conf = unindent("""\
         [sssd]
-        config_file_version = 2
         domains             = LDAP
         services            = nss
 
@@ -189,7 +187,6 @@ def fqname_case_insensitive_rfc2307(request, ldap_conn):
 
     conf = unindent("""\
         [sssd]
-        config_file_version = 2
         domains             = LDAP
         services            = nss
 
-- 
2.1.0

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to