On (20/06/16 21:09), Jakub Hrozek wrote: >On Mon, Jun 20, 2016 at 08:54:18PM +0200, Lukas Slebodnik wrote: >> ehlo, >> >> Attached is a sligtly modified version of Michal's patch. > >The same patch is attached twice. Was it by accident or did you mean to >send two patches? > >> I fixed few coding style issues + added missing creation of directory >> + spec file change. > >You should have sent fixups to get credit, but meh :) Thanks for doing >the work nonetheless. > >> >> You might notice that Michal removed detection of sssd.conf modified time. >> It is because mtime could be obtiained from sssd.conf before parsing. >> However, snippets files are open after parsing sssd.conf and mtime >> of snippet files is ignored in the process. >> >> We have few options. >> * check mtime directly in sssd >> * add new function to libini_config to get latest mtime before parsing >> (max_mtime(main.conf + alowed snippet files) >> // it's little bit a complication for user of libini_config >> // because user will need to paste regex for allowed snippets twice >> // 1st time in new function for checking mtime and 2nd time in function >> // ini_config_augment >> * modify libini_config to set max mtime while parsing snippet files >> // but we will need to parse files anyway. So I'm not sure what will be >> // benefit of cehcking mtime after parsing. >> * last option is to ignore mtime. (Michal's current version) >> // and remove FIXME :-) > >Is there actually any downside to /always/ reading the config file and >always creating the confdb from scratch? I would say that sssd restarts >are a rare operation and the parsing and writes are not too big to slow >down the startup significantly. > >I think the whole mtime logic was there only to allow online config >changes, which is something we tried in the past, but could never code >it up properly. > > >> >> The main purpose of this mail is to decide wheteer we want change in >> ding-libs >> or no. >> >> BTW. We cannot change directory for snippet files from command line. >> Do we want such feature? >> [root@graviton ~]# /usr/sbin/sssd --help >> Usage: sssd [OPTION...] >> -d, --debug-level=INT Debug level >> -f, --debug-to-files Send the debug output to files instead of >> stderr >> --debug-timestamps=INT Add debug timestamps >> --debug-microseconds=INT Show timestamps with microseconds >> -D, --daemon Become a daemon (default) >> -i, --interactive Run interactive (not a daemon) >> -c, --config=STRING Specify a non-default config file > >Can you think of any use for this option? There can be only one sssd on >the system, so I actually wonder if we can remove it.. > >> --version Print version number and exit >> >> Help options: >> -?, --help Show this help message >> --usage Display brief usage message >> >> LS >
Updated patch is attached which fixes compilation with libini_config 1.1 (el6) Config snippets will not be available there LS
>From b51a60345f6d25c023a07f2a2dded88736061406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH 1/3] confd: Make it possible to use config snippets Resolves: https://fedorahosted.org/sssd/ticket/2247 Signed-off-by: Lukas Slebodnik <lsleb...@redhat.com> --- Makefile.am | 4 +++- contrib/sssd.spec.in | 1 + src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 41 +++++-------------------------- src/confdb/confdb_setup.h | 3 ++- src/external/libini_config.m4 | 12 ++++++++++ src/monitor/monitor.c | 6 +++-- src/util/sss_ini.c | 56 +++++++++++++++++++++++++++++++++++++------ src/util/sss_ini.h | 3 ++- 9 files changed, 80 insertions(+), 47 deletions(-) diff --git a/Makefile.am b/Makefile.am index ba4b0b47cbfd01987344da9be4b4003fd5e02317..0d11fb8562bad966245af82a4c293d386ea6db9e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3802,6 +3802,7 @@ SSSD_USER_DIRS = \ $(DESTDIR)$(pubconfpath)/krb5.include.d \ $(DESTDIR)$(gpocachepath) \ $(DESTDIR)$(sssdconfdir) \ + $(DESTDIR)$(sssdconfdir)/conf.d \ $(DESTDIR)$(sssddefaultconfdir) \ $(DESTDIR)$(logpath) \ $(NULL) @@ -3836,7 +3837,8 @@ endif $(INSTALL) -d -m 0755 $(DESTDIR)$(mcpath) $(DESTDIR)$(pipepath) \ $(DESTDIR)$(pubconfpath) \ $(DESTDIR)$(pubconfpath)/krb5.include.d $(DESTDIR)$(gpocachepath) - $(INSTALL) -d -m 0711 $(DESTDIR)$(sssdconfdir) + $(INSTALL) -d -m 0711 $(DESTDIR)$(sssdconfdir) \ + $(DESTDIR)$(sssdconfdir)/conf.d if HAVE_DOXYGEN docs: diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 68dc0afd95f6aadce9ad88ef54ea99a3c8ce7723..37d5acea79ea16502ff0e0118c2543bccd410bf5 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -781,6 +781,7 @@ done %attr(755,sssd,sssd) %dir %{gpocachepath} %attr(750,sssd,sssd) %dir %{_var}/log/%{name} %attr(711,sssd,sssd) %dir %{_sysconfdir}/sssd +%attr(711,sssd,sssd) %dir %{_sysconfdir}/sssd/conf.d %ghost %attr(0600,sssd,sssd) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf %if (0%{?use_systemd} == 1) %attr(755,root,root) %dir %{_sysconfdir}/systemd/system/sssd.service.d diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 0a3d6a035ca53e7e5563138b5d8daf5a0770a693..2cd75b9e8b7d81261774303ad48fcec4112e3ae4 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -42,6 +42,7 @@ #define CONFDB_FILE "config.ldb" #define SSSD_DEFAULT_CONFIG_FILE SSSD_DEFAULT_CONF_DIR"/sssd.conf" #define SSSD_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf" +#define CONFDB_DEFAULT_CONFIG_DIR SSSD_CONF_DIR"/conf.d" #define SSSD_MIN_ID 1 #define SSSD_LOCAL_MINID 1000 #define CONFDB_DEFAULT_SHELL_FALLBACK "/bin/sh" diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index dfdcae56697123c414968cfaaabe3e1cd68ca21f..b17a34b1213b0ebeeea5719c78ea1db8d5fabfd6 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -127,14 +127,14 @@ int confdb_create_base(struct confdb_ctx *cdb) return EOK; } -int confdb_init_db(const char *config_file, struct confdb_ctx *cdb) +int confdb_init_db(const char *config_file, const char *config_dir, + struct confdb_ctx *cdb) { TALLOC_CTX *tmp_ctx; int ret; int sret = EOK; int version; char timestr[21]; - char *lasttimestr; bool in_transaction = false; const char *config_ldif; const char *vals[2] = { timestr, NULL }; @@ -206,41 +206,12 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb) goto done; } - /* Determine if the conf file has changed since we last updated - * the confdb + /* FIXME: Determine if the conf file or any snippet has changed + * since we last updated the confdb or if some snippet was + * added or removed. */ - ret = sss_ini_get_stat(init_data); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Status check on config file failed.\n"); - ret = errno; - goto done; - } - errno = 0; - - ret = sss_ini_get_mtime(init_data, sizeof(timestr), timestr); - if (ret <= 0 || ret >= sizeof(timestr)) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Failed to convert time_t to string ??\n"); - ret = errno ? errno : EFAULT; - } - ret = confdb_get_string(cdb, tmp_ctx, "config", "lastUpdate", - NULL, &lasttimestr); - if (ret == EOK) { - - /* check if we lastUpdate and last file modification change differ*/ - if ((lasttimestr != NULL) && (strcmp(lasttimestr, timestr) == 0)) { - /* not changed, get out, nothing more to do */ - ret = EOK; - goto done; - } - } else { - DEBUG(SSSDBG_FATAL_FAILURE, "Failed to get lastUpdate attribute.\n"); - goto done; - } - - ret = sss_ini_get_config(init_data, config_file); + ret = sss_ini_get_config(init_data, config_file, config_dir); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to load configuration\n"); goto done; diff --git a/src/confdb/confdb_setup.h b/src/confdb/confdb_setup.h index 2b8802f6fd56ebf84379b46080e3563104598271..f4f03b4878aea5ac8e50d178d45a292d6ef99051 100644 --- a/src/confdb/confdb_setup.h +++ b/src/confdb/confdb_setup.h @@ -47,6 +47,7 @@ int confdb_create_base(struct confdb_ctx *cdb); int confdb_test(struct confdb_ctx *cdb); -int confdb_init_db(const char *config_file, struct confdb_ctx *cdb); +int confdb_init_db(const char *config_file, const char *config_dir, + struct confdb_ctx *cdb); #endif /* CONFDB_SETUP_H_ */ diff --git a/src/external/libini_config.m4 b/src/external/libini_config.m4 index 9e5c69fbde6f64ddfdffd276fb9c5a21d96f6a01..a2bba4243b86996bdfc81e894ca8bdc489a9c4e6 100644 --- a/src/external/libini_config.m4 +++ b/src/external/libini_config.m4 @@ -19,6 +19,18 @@ PKG_CHECK_MODULES(INI_CONFIG_V0, [ INI_CONFIG_LIBS="$INI_CONFIG_V1_1_LIBS" HAVE_LIBINI_CONFIG_V1_1=1 AC_DEFINE_UNQUOTED(HAVE_LIBINI_CONFIG_V1_1, 1, [libini_config version 1.1.0 or greater]) + PKG_CHECK_MODULES(INI_CONFIG_V1_3, [ + ini_config >= 1.3.0], [ + + INI_CONFIG_CFLAGS="$INI_CONFIG_V1_3_CFLAGS" + INI_CONFIG_LIBS="$INI_CONFIG_V1_3_LIBS" + HAVE_LIBINI_CONFIG_V1_3=1 + AC_DEFINE_UNQUOTED(HAVE_LIBINI_CONFIG_V1_3, 1, + [libini_config version 1.3.0 or greater]) + ], [ + AC_MSG_WARN([libini_config-devel >= 1.3.0 not available, using older version]) + ] + ) ], [ AC_MSG_WARN([libini_config-devel >= 1.1.0 not available, using older version]) ] diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 6b2cb55d68429d4a8776642e84a5dc1dcd7205f3..69daf1c0fccda62d0c89dc924fa7176baf5e67ea 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1872,6 +1872,7 @@ static int monitor_ctx_destructor(void *mem) */ errno_t load_configuration(TALLOC_CTX *mem_ctx, const char *config_file, + const char *config_dir, struct mt_ctx **monitor) { errno_t ret; @@ -1934,7 +1935,7 @@ errno_t load_configuration(TALLOC_CTX *mem_ctx, goto done; } - ret = confdb_init_db(config_file, ctx->cdb); + ret = confdb_init_db(config_file, config_dir, ctx->cdb); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "ConfDB initialization has failed [%s]\n", sss_strerror(ret)); @@ -3189,7 +3190,8 @@ int main(int argc, const char *argv[]) } /* Parse config file, fail if cannot be done */ - ret = load_configuration(tmp_ctx, config_file, &monitor); + ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR, + &monitor); if (ret != EOK) { switch (ret) { case ERR_MISSING_CONF: diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 766a75ea6730b3a8f5fbec5f2086075fbcb2d81b..2d786df94fe09601bd4b1d3d1fa145739f30ef39 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -46,7 +46,10 @@ struct sss_ini_initdata { char **error_list; + struct ref_array **ra_success_list; + struct ref_array **ra_error_list; struct ini_cfgobj *sssd_config; + struct ini_cfgobj *orig_sssd_config; struct value_obj *obj; const struct stat *cstat; struct ini_cfgfile *file; @@ -205,13 +208,19 @@ void sss_ini_config_print_errors(char **error_list) /* Load configuration */ int sss_ini_get_config(struct sss_ini_initdata *init_data, - const char *config_file) + const char *config_file, + const char *config_dir) { int ret; #ifdef HAVE_LIBINI_CONFIG_V1 + const char *patterns[] = { "^[^\\.].*\\.conf", NULL }; + const char *sections[] = { ".*", NULL }; + + init_data->sssd_config = NULL; + init_data->ra_success_list = NULL; /* Create config object */ - ret = ini_config_create(&(init_data->sssd_config)); + ret = ini_config_create(&(init_data->orig_sssd_config)); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to create config object. Error %d.\n", ret); @@ -223,27 +232,60 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data, INI_STOP_ON_ANY, INI_MV1S_OVERWRITE, INI_PARSE_NOWRAP, - init_data->sssd_config); + init_data->orig_sssd_config); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to parse configuration. Error %d.\n", ret); - if (ini_config_error_count(init_data->sssd_config)) { + if (ini_config_error_count(init_data->orig_sssd_config)) { DEBUG(SSSDBG_FATAL_FAILURE, "Errors detected while parsing: %s\n", ini_config_get_filename(init_data->file)); - ini_config_get_errors(init_data->sssd_config, + ini_config_get_errors(init_data->orig_sssd_config, &init_data->error_list); sss_ini_config_print_errors(init_data->error_list); ini_config_free_errors(init_data->error_list); } - ini_config_destroy(init_data->sssd_config); - init_data->sssd_config = NULL; + ini_config_destroy(init_data->orig_sssd_config); + init_data->orig_sssd_config = NULL; return ret; } + /* Create config object */ + ret = ini_config_create(&(init_data->sssd_config)); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Failed to create config object. Error %d.\n", ret); + return ret; + } + +#ifdef HAVE_LIBINI_CONFIG_V1_3 + ret = ini_config_augment(init_data->orig_sssd_config, + config_dir, + patterns, + sections, + NULL, + INI_STOP_ON_ANY, + INI_MV1S_OVERWRITE, + INI_PARSE_NOWRAP, + INI_MV2S_OVERWRITE, + &init_data->sssd_config, + init_data->ra_error_list, + init_data->ra_success_list); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to augment configuration [%d]: %s", + ret, sss_strerror(ret)); + } +#endif + + if (init_data->sssd_config == NULL) { + /* Fall back to original configuration */ + init_data->sssd_config = init_data->orig_sssd_config; + } + return ret; #else diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h index 3beaca15ba87ad12a016a3295371a56239165683..f5b36deb9cacfecbd68dd2a4d37a4398ce280c3c 100644 --- a/src/util/sss_ini.h +++ b/src/util/sss_ini.h @@ -58,7 +58,8 @@ int sss_ini_get_mtime(struct sss_ini_initdata *init_data, /* Load configuration */ int sss_ini_get_config(struct sss_ini_initdata *init_data, - const char *config_file); + const char *config_file, + const char *config_dir); /* Get configuration object */ int sss_ini_get_cfgobj(struct sss_ini_initdata *init_data, const char *section, const char *name); -- 2.7.4
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
