ehlo,
Attached is a sligtly modified version of Michal's patch. I fixed few coding style issues + added missing creation of directory + spec file change.
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 :-)
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 --version Print version number and exit
Help options: -?, --help Show this help message --usage Display brief usage message
LS
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 exitHelp options: -?, --help Show this help message --usage Display brief usage message
LS
From edd7ba4ad310cae986ca874312f1cd4ed3963f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confd: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@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/monitor/monitor.c | 6 ++++-- src/util/sss_ini.c | 54 +++++++++++++++++++++++++++++++++++++++++------ src/util/sss_ini.h | 3 ++- 8 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/Makefile.am b/Makefile.am index ee8f698d620ad778d4ee53eaeb4976b23e0129fd..53f95189c20a6171379e7aea19adf2458c13cae8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3643,6 +3643,7 @@ SSSD_USER_DIRS = \ $(DESTDIR)$(pubconfpath)/krb5.include.d \ $(DESTDIR)$(gpocachepath) \ $(DESTDIR)$(sssdconfdir) \
- $(DESTDIR)$(sssdconfdir)/conf.d \ $(DESTDIR)$(sssddefaultconfdir) \ $(DESTDIR)$(logpath) \ $(NULL)
@@ -3677,7 +3678,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.dif HAVE_DOXYGEN docs: diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 0f2bcdf6136d35e522302867786b97d28837d587..87363d7ed30ba22130cf7e90a7c152f68256563f 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -772,6 +772,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/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);diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 766a75ea6730b3a8f5fbec5f2086075fbcb2d81b..d39993cab457a9bf5bc596368783121fab24e83e 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,58 @@ 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;}
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));}
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
From edd7ba4ad310cae986ca874312f1cd4ed3963f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confd: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@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/monitor/monitor.c | 6 ++++-- src/util/sss_ini.c | 54 +++++++++++++++++++++++++++++++++++++++++------ src/util/sss_ini.h | 3 ++- 8 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/Makefile.am b/Makefile.am index ee8f698d620ad778d4ee53eaeb4976b23e0129fd..53f95189c20a6171379e7aea19adf2458c13cae8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3643,6 +3643,7 @@ SSSD_USER_DIRS = \ $(DESTDIR)$(pubconfpath)/krb5.include.d \ $(DESTDIR)$(gpocachepath) \ $(DESTDIR)$(sssdconfdir) \
- $(DESTDIR)$(sssdconfdir)/conf.d \ $(DESTDIR)$(sssddefaultconfdir) \ $(DESTDIR)$(logpath) \ $(NULL)
@@ -3677,7 +3678,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.dif HAVE_DOXYGEN docs: diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 0f2bcdf6136d35e522302867786b97d28837d587..87363d7ed30ba22130cf7e90a7c152f68256563f 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -772,6 +772,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/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);diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 766a75ea6730b3a8f5fbec5f2086075fbcb2d81b..d39993cab457a9bf5bc596368783121fab24e83e 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,58 @@ 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;}
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));}
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
On 06/20/2016 03:09 PM, 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.
I think as long as we keep the parsing time very short, this is probably not an issue. However, if we ever move SSSD to a socket-activated behavior, we will want this to be very fast (or otherwise skippable).
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..
I think we originally added this thinking of a future in which we might want to be able to run a live SSSD for automated tests, but we obviously went another route with it.
The only use I can think of for this would be if someone actually wanted to move their sssd.conf to a mounted read-only location (since IIRC we actually explicitly disallow sssd.conf as a symlink).
On (20/06/16 16:13), Stephen Gallagher wrote:
On 06/20/2016 03:09 PM, 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?
Just single patch.
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.
It were very small changes which does not deserve separate patch :-) I added my sign-off.
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.
I let Michal's version untouched with FIXME in code. Maybe, we will fix it sometimes.
And we can say that ticket https://fedorahosted.org/sssd/ticket/3020 is fixed :-)
I think as long as we keep the parsing time very short, this is probably not an issue. However, if we ever move SSSD to a socket-activated behavior, we will want this to be very fast (or otherwise skippable).
It might be more complicated with socket activation. ATM, the monitor is responsible for parsing sssd.conf and creating conf.db But there could be a race condition with many socket activated responders. Maybe we will switch to pure libini version of config (without confdb) But that's far future :-)
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..
I think we originally added this thinking of a future in which we might want to be able to run a live SSSD for automated tests, but we obviously went another route with it.
It might be handy when we will get rid of custom build of sssd in cwrap integration tests.
The only use I can think of for this would be if someone actually wanted to move their sssd.conf to a mounted read-only location (since IIRC we actually explicitly disallow sssd.conf as a symlink).
I will not add new option now.
It looks like there is an agreement that we needn't change anything in libini_config for this use-case. So I will focus on other ding-libs patches.
BTW I need to update manual pages to reflect this change.
LS
On Tue, Jun 21, 2016 at 10:25:41AM +0200, Lukas Slebodnik wrote:
It looks like there is an agreement that we needn't change anything in libini_config for this use-case. So I will focus on other ding-libs patches.
BTW I need to update manual pages to reflect this change.
Please file a ticket about the mtime changes so that we don't forget.
On Tue, Jun 21, 2016 at 10:57:02AM +0200, Jakub Hrozek wrote:
On Tue, Jun 21, 2016 at 10:25:41AM +0200, Lukas Slebodnik wrote:
It looks like there is an agreement that we needn't change anything in libini_config for this use-case. So I will focus on other ding-libs patches.
BTW I need to update manual pages to reflect this change.
Please file a ticket about the mtime changes so that we don't forget.
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 exitHelp 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
On (23/06/16 11:03), Lukas Slebodnik wrote:
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 exitHelp 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
BTW do not forget that SELinux might deny access to conf.d
type=AVC msg=audit(06/23/2016 10:44:57.486:910) : avc: denied { read } for pid=27671 comm=sssd name=conf.d dev="dm-1" ino=1871243 scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:object_r:sssd_conf_t:s0 tclass=dir permissive=0
LS
On Thu, Jun 23, 2016 at 11:46:33AM +0200, Lukas Slebodnik wrote:
On (23/06/16 11:03), Lukas Slebodnik wrote:
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 exitHelp 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
BTW do not forget that SELinux might deny access to conf.d
type=AVC msg=audit(06/23/2016 10:44:57.486:910) : avc: denied { read } for pid=27671 comm=sssd name=conf.d dev="dm-1" ino=1871243 scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:object_r:sssd_conf_t:s0 tclass=dir permissive=0
We need a downstream selinux-policy bug to amend the selinux-policy package, then. Thanks for the reminder.
On (23/06/16 11:51), Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:46:33AM +0200, Lukas Slebodnik wrote:
On (23/06/16 11:03), Lukas Slebodnik wrote:
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 exitHelp 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
BTW do not forget that SELinux might deny access to conf.d
type=AVC msg=audit(06/23/2016 10:44:57.486:910) : avc: denied { read } for pid=27671 comm=sssd name=conf.d dev="dm-1" ino=1871243 scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:object_r:sssd_conf_t:s0 tclass=dir permissive=0
We need a downstream selinux-policy bug to amend the selinux-policy package, then. Thanks for the reminder.
Updated patch also log the error from libini_config into sssd.log
LS
Lukas,
did you have some local patches related to this patch, that you did not send here?
Because I can not apply the patch on current master and the changes in the context of your patch seem like you did something with default config file path?
Slightly modified version that applies on current master is attached (the changes are the same, but the context differs).
Michal
On 06/23/2016 05:50 PM, Michal Židek wrote:
Lukas,
did you have some local patches related to this patch, that you did not send here?
Because I can not apply the patch on current master and the changes in the context of your patch seem like you did something with default config file path?
Slightly modified version that applies on current master is attached (the changes are the same, but the context differs).
Michal
I added the missing access check for snippets. If the snippets does not pass the access check it is skipped and a loud debug message is logged.
I attach it as separate patch for easier review, but it should be squashed before pushing. I also attach the first patch for convenience.
Michal
On (23/06/16 18:24), Michal Židek wrote:
On 06/23/2016 05:50 PM, Michal Židek wrote:
Lukas,
did you have some local patches related to this patch, that you did not send here?
Because I can not apply the patch on current master and the changes in the context of your patch seem like you did something with default config file path?
Slightly modified version that applies on current master is attached (the changes are the same, but the context differs).
Michal
I added the missing access check for snippets. If the snippets does not pass the access check it is skipped and a loud debug message is logged.
I attach it as separate patch for easier review, but it should be squashed before pushing. I also attach the first patch for convenience.
Michal
From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confdb: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@redhat.com
Makefile.am | 4 ++- 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 | 73 ++++++++++++++++++++++++++++++++++++++----- src/util/sss_ini.h | 3 +- 8 files changed, 96 insertions(+), 47 deletions(-)
diff --git a/Makefile.am b/Makefile.am index 7161bef..67fa639 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3566,6 +3566,7 @@ SSSD_USER_DIRS = \ $(DESTDIR)$(pubconfpath) \ $(DESTDIR)$(pubconfpath)/krb5.include.d \ $(DESTDIR)$(gpocachepath) \
- $(DESTDIR)$(sssdconfdir)/conf.d \ $(DESTDIR)$(sssdconfdir) \ $(DESTDIR)$(logpath) \ $(NULL)
@@ -3600,7 +3601,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.dif HAVE_DOXYGEN docs: diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index c478ef0..9d9a887 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -41,6 +41,7 @@ #define CONFDB_DEFAULT_CFG_FILE_VER 2 #define CONFDB_FILE "config.ldb" #define CONFDB_DEFAULT_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 694a7f0..3cfc8b0 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -125,14 +125,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 };
@@ -174,41 +174,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 2b8802f..f4f03b4 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 9e5c69f..a2bba42 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=1AC_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 ac3af28..e6bb624 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);diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 766a75e..3dad18b 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,23 @@ 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 };
+#ifdef HAVE_LIBINI_CONFIG_V1_3
- uint32_t i = 0;
- char *msg = NULL;
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
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 +236,73 @@ 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));- }
- while (ref_array_get(init_data->ra_error_list, i, &msg) != NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,"Config merge error: %s\n", msg);i++;- }
- i = 0;
- while (ref_array_get(init_data->ra_success_list, i, &msg) != NULL) {
DEBUG(SSSDBG_TRACE_FUNC,"Config merge success: %s\n", msg);i++;- }
+#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 3beaca1..f5b36de 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.5.0
From 97b8542bd8ad0603d97cf0b2201807d5a3073e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Thu, 23 Jun 2016 18:13:50 +0200 Subject: [PATCH] Squash me before pushing
Just add missing access check for snippets.
src/util/sss_ini.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 3dad18b..3c014b3 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -218,6 +218,14 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data, #ifdef HAVE_LIBINI_CONFIG_V1_3 uint32_t i = 0; char *msg = NULL;
- struct access_check snip_check;
- snip_check.flags = INI_ACCESS_CHECK_MODE | INI_ACCESS_CHECK_UID
| INI_ACCESS_CHECK_GID;- snip_check.uid = 0; /* owned by root */
- snip_check.gid = 0; /* owned by root */
- snip_check.mode = S_IRUSR; /* r**------ */
- snip_check.mask = ALLPERMS & ~(S_IWUSR|S_IXUSR);
#endif /* HAVE_LIBINI_CONFIG_V1_3 */
init_data->sssd_config = NULL;@@ -270,7 +278,7 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data, config_dir, patterns, sections,
NULL,
&snip_check,
LGTM
On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
On (23/06/16 18:24), Michal Židek wrote:
On 06/23/2016 05:50 PM, Michal Židek wrote:
Lukas,
did you have some local patches related to this patch, that you did not send here?
Because I can not apply the patch on current master and the changes in the context of your patch seem like you did something with default config file path?
Slightly modified version that applies on current master is attached (the changes are the same, but the context differs).
Michal
I added the missing access check for snippets. If the snippets does not pass the access check it is skipped and a loud debug message is logged.
I attach it as separate patch for easier review, but it should be squashed before pushing. I also attach the first patch for convenience.
Michal
From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confdb: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@redhat.com
Makefile.am | 4 ++- 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 | 73 ++++++++++++++++++++++++++++++++++++++----- src/util/sss_ini.h | 3 +- 8 files changed, 96 insertions(+), 47 deletions(-)
diff --git a/Makefile.am b/Makefile.am index 7161bef..67fa639 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3566,6 +3566,7 @@ SSSD_USER_DIRS = \ $(DESTDIR)$(pubconfpath) \ $(DESTDIR)$(pubconfpath)/krb5.include.d \ $(DESTDIR)$(gpocachepath) \
- $(DESTDIR)$(sssdconfdir)/conf.d \ $(DESTDIR)$(sssdconfdir) \ $(DESTDIR)$(logpath) \ $(NULL)
@@ -3600,7 +3601,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.dif HAVE_DOXYGEN docs: diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index c478ef0..9d9a887 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -41,6 +41,7 @@ #define CONFDB_DEFAULT_CFG_FILE_VER 2 #define CONFDB_FILE "config.ldb" #define CONFDB_DEFAULT_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 694a7f0..3cfc8b0 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -125,14 +125,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 };
@@ -174,41 +174,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 2b8802f..f4f03b4 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 9e5c69f..a2bba42 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=1AC_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 ac3af28..e6bb624 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);diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 766a75e..3dad18b 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,23 @@ 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 };
+#ifdef HAVE_LIBINI_CONFIG_V1_3
- uint32_t i = 0;
- char *msg = NULL;
+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
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 +236,73 @@ 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));- }
- while (ref_array_get(init_data->ra_error_list, i, &msg) != NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,"Config merge error: %s\n", msg);i++;- }
- i = 0;
- while (ref_array_get(init_data->ra_success_list, i, &msg) != NULL) {
DEBUG(SSSDBG_TRACE_FUNC,"Config merge success: %s\n", msg);i++;- }
+#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 3beaca1..f5b36de 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.5.0
From 97b8542bd8ad0603d97cf0b2201807d5a3073e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Thu, 23 Jun 2016 18:13:50 +0200 Subject: [PATCH] Squash me before pushing
Just add missing access check for snippets.
src/util/sss_ini.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 3dad18b..3c014b3 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -218,6 +218,14 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data, #ifdef HAVE_LIBINI_CONFIG_V1_3 uint32_t i = 0; char *msg = NULL;
- struct access_check snip_check;
- snip_check.flags = INI_ACCESS_CHECK_MODE | INI_ACCESS_CHECK_UID
| INI_ACCESS_CHECK_GID;- snip_check.uid = 0; /* owned by root */
- snip_check.gid = 0; /* owned by root */
- snip_check.mode = S_IRUSR; /* r**------ */
- snip_check.mask = ALLPERMS & ~(S_IWUSR|S_IXUSR);
#endif /* HAVE_LIBINI_CONFIG_V1_3 */
init_data->sssd_config = NULL;@@ -270,7 +278,7 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data, config_dir, patterns, sections,
NULL,
&snip_check,LGTM
As Lukas sugested off-list I am attaching the patched already squashed to the original one.
Michal
On (23/06/16 18:59), Michal Židek wrote:
On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
On (23/06/16 18:24), Michal Židek wrote:
On 06/23/2016 05:50 PM, Michal Židek wrote:
Lukas,
did you have some local patches related to this patch, that you did not send here?
Because I can not apply the patch on current master and the changes in the context of your patch seem like you did something with default config file path?
Slightly modified version that applies on current master is attached (the changes are the same, but the context differs).
Michal
I added the missing access check for snippets. If the snippets does not pass the access check it is skipped and a loud debug message is logged.
I attach it as separate patch for easier review, but it should be squashed before pushing. I also attach the first patch for convenience.
Michal
From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confdb: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@redhat.com
LGTM.
It isn't ACK because I was involved in development and I didn;t test last version but code is fine fore me.
LS
On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
On (23/06/16 18:59), Michal Židek wrote:
On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
On (23/06/16 18:24), Michal Židek wrote:
On 06/23/2016 05:50 PM, Michal Židek wrote:
Lukas,
did you have some local patches related to this patch, that you did not send here?
Because I can not apply the patch on current master and the changes in the context of your patch seem like you did something with default config file path?
Slightly modified version that applies on current master is attached (the changes are the same, but the context differs).
Michal
I added the missing access check for snippets. If the snippets does not pass the access check it is skipped and a loud debug message is logged.
I attach it as separate patch for easier review, but it should be squashed before pushing. I also attach the first patch for convenience.
Michal
From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confdb: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@redhat.com
LGTM.
It isn't ACK because I was involved in development and I didn;t test last version but code is fine fore me.
I just ran a quick test and the patch works for me and the code reads OK as well. I will push after CI finishes.
I also opened: https://fedorahosted.org/sssd/ticket/3062 because the patch is missing documentation and https://fedorahosted.org/sssd/ticket/3063 to add at least a simple integration test.
On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
On (23/06/16 18:59), Michal Židek wrote:
On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
On (23/06/16 18:24), Michal Židek wrote:
On 06/23/2016 05:50 PM, Michal Židek wrote:
Lukas,
did you have some local patches related to this patch, that you did not send here?
Because I can not apply the patch on current master and the changes in the context of your patch seem like you did something with default config file path?
Slightly modified version that applies on current master is attached (the changes are the same, but the context differs).
Michal
I added the missing access check for snippets. If the snippets does not pass the access check it is skipped and a loud debug message is logged.
I attach it as separate patch for easier review, but it should be squashed before pushing. I also attach the first patch for convenience.
Michal
From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confdb: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@redhat.com
LGTM.
It isn't ACK because I was involved in development and I didn;t test last version but code is fine fore me.
I just ran a quick test and the patch works for me and the code reads OK as well. I will push after CI finishes.
I also opened: https://fedorahosted.org/sssd/ticket/3062 because the patch is missing documentation and https://fedorahosted.org/sssd/ticket/3063 to add at least a simple integration test.
Actually, can we get rid of this warning?
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] # const char *patterns[] = { "^[^\.].*\.conf", NULL }; # ^ # 214| int ret; # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217| const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] # const char *sections[] = { ".*", NULL }; # ^ # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217|-> const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 # 219| uint32_t i = 0;
On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
On (23/06/16 18:59), Michal Židek wrote:
On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
On (23/06/16 18:24), Michal Židek wrote:
On 06/23/2016 05:50 PM, Michal Židek wrote: > Lukas, > > did you have some local patches related to > this patch, that you did not send here? > > Because I can not apply the patch on current > master and the changes in the context of > your patch seem like you did something with > default config file path? > > Slightly modified version that applies on > current master is attached (the changes > are the same, but the context differs). > > Michal >
I added the missing access check for snippets. If the snippets does not pass the access check it is skipped and a loud debug message is logged.
I attach it as separate patch for easier review, but it should be squashed before pushing. I also attach the first patch for convenience.
Michal
From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH] confdb: Make it possible to use config snippets
Resolves: https://fedorahosted.org/sssd/ticket/2247
Signed-off-by: Lukas Slebodnik lslebodn@redhat.com
LGTM.
It isn't ACK because I was involved in development and I didn;t test last version but code is fine fore me.
I just ran a quick test and the patch works for me and the code reads OK as well. I will push after CI finishes.
I also opened: https://fedorahosted.org/sssd/ticket/3062 because the patch is missing documentation and https://fedorahosted.org/sssd/ticket/3063 to add at least a simple integration test.
Actually, can we get rid of this warning?
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] # const char *patterns[] = { "^[^\.].*\.conf", NULL }; # ^ # 214| int ret; # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217| const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] # const char *sections[] = { ".*", NULL }; # ^ # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217|-> const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 # 219| uint32_t i = 0; _______________________________________________
New patch attached.
Michal
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
On (23/06/16 18:59), Michal Židek wrote:
On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
On (23/06/16 18:24), Michal Židek wrote: > On 06/23/2016 05:50 PM, Michal Židek wrote: > > Lukas, > > > > did you have some local patches related to > > this patch, that you did not send here? > > > > Because I can not apply the patch on current > > master and the changes in the context of > > your patch seem like you did something with > > default config file path? > > > > Slightly modified version that applies on > > current master is attached (the changes > > are the same, but the context differs). > > > > Michal > > > > I added the missing access check for > snippets. If the snippets does not > pass the access check it is skipped > and a loud debug message is logged. > > I attach it as separate patch for > easier review, but it should be squashed > before pushing. I also attach the first > patch for convenience. > > Michal
> From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com > Date: Tue, 22 Mar 2016 14:09:34 +0100 > Subject: [PATCH] confdb: Make it possible to use config snippets > > Resolves: > https://fedorahosted.org/sssd/ticket/2247 > > Signed-off-by: Lukas Slebodnik lslebodn@redhat.com > ---
LGTM.
It isn't ACK because I was involved in development and I didn;t test last version but code is fine fore me.
I just ran a quick test and the patch works for me and the code reads OK as well. I will push after CI finishes.
I also opened: https://fedorahosted.org/sssd/ticket/3062 because the patch is missing documentation and https://fedorahosted.org/sssd/ticket/3063 to add at least a simple integration test.
Actually, can we get rid of this warning?
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] # const char *patterns[] = { "^[^\.].*\.conf", NULL }; # ^ # 214| int ret; # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217| const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] # const char *sections[] = { ".*", NULL }; # ^ # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217|-> const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 # 219| uint32_t i = 0; _______________________________________________
New patch attached.
Michal
The patch is fine, (so ack also from me), but can someone check if the attached rebase on top of Pavel's sssctl is OK?
On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
On (23/06/16 18:59), Michal Židek wrote:
On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: > On (23/06/16 18:24), Michal Židek wrote: >> On 06/23/2016 05:50 PM, Michal Židek wrote: >>> Lukas, >>> >>> did you have some local patches related to >>> this patch, that you did not send here? >>> >>> Because I can not apply the patch on current >>> master and the changes in the context of >>> your patch seem like you did something with >>> default config file path? >>> >>> Slightly modified version that applies on >>> current master is attached (the changes >>> are the same, but the context differs). >>> >>> Michal >>> >> >> I added the missing access check for >> snippets. If the snippets does not >> pass the access check it is skipped >> and a loud debug message is logged. >> >> I attach it as separate patch for >> easier review, but it should be squashed >> before pushing. I also attach the first >> patch for convenience. >> >> Michal > >> From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com >> Date: Tue, 22 Mar 2016 14:09:34 +0100 >> Subject: [PATCH] confdb: Make it possible to use config snippets >> >> Resolves: >> https://fedorahosted.org/sssd/ticket/2247 >> >> Signed-off-by: Lukas Slebodnik lslebodn@redhat.com >> ---
LGTM.
It isn't ACK because I was involved in development and I didn;t test last version but code is fine fore me.
I just ran a quick test and the patch works for me and the code reads OK as well. I will push after CI finishes.
I also opened: https://fedorahosted.org/sssd/ticket/3062 because the patch is missing documentation and https://fedorahosted.org/sssd/ticket/3063 to add at least a simple integration test.
Actually, can we get rid of this warning?
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] # const char *patterns[] = { "^[^\.].*\.conf", NULL }; # ^ # 214| int ret; # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217| const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] # const char *sections[] = { ".*", NULL }; # ^ # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217|-> const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 # 219| uint32_t i = 0; _______________________________________________
New patch attached.
Michal
The patch is fine, (so ack also from me), but can someone check if the attached rebase on top of Pavel's sssctl is OK?
I did not check the differences between the rebased and non-rebased version, but I just tried branch with sssctl patches + this rebased patch + startup validation patches and it worked for me.
But the CI failed for me with the snippets in place. Will take a look at what is happening there.
Michal
On (24/06/16 10:29), Michal Židek wrote:
On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
On (23/06/16 18:59), Michal Židek wrote: > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: > > On (23/06/16 18:24), Michal Židek wrote: > > > On 06/23/2016 05:50 PM, Michal Židek wrote: > > > > Lukas, > > > > > > > > did you have some local patches related to > > > > this patch, that you did not send here? > > > > > > > > Because I can not apply the patch on current > > > > master and the changes in the context of > > > > your patch seem like you did something with > > > > default config file path? > > > > > > > > Slightly modified version that applies on > > > > current master is attached (the changes > > > > are the same, but the context differs). > > > > > > > > Michal > > > > > > > > > > I added the missing access check for > > > snippets. If the snippets does not > > > pass the access check it is skipped > > > and a loud debug message is logged. > > > > > > I attach it as separate patch for > > > easier review, but it should be squashed > > > before pushing. I also attach the first > > > patch for convenience. > > > > > > Michal > > > > > From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com > > > Date: Tue, 22 Mar 2016 14:09:34 +0100 > > > Subject: [PATCH] confdb: Make it possible to use config snippets > > > > > > Resolves: > > > https://fedorahosted.org/sssd/ticket/2247 > > > > > > Signed-off-by: Lukas Slebodnik lslebodn@redhat.com > > > --- LGTM.
It isn't ACK because I was involved in development and I didn;t test last version but code is fine fore me.
I just ran a quick test and the patch works for me and the code reads OK as well. I will push after CI finishes.
I also opened: https://fedorahosted.org/sssd/ticket/3062 because the patch is missing documentation and https://fedorahosted.org/sssd/ticket/3063 to add at least a simple integration test.
Actually, can we get rid of this warning?
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] # const char *patterns[] = { "^[^\.].*\.conf", NULL }; # ^ # 214| int ret; # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217| const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] # const char *sections[] = { ".*", NULL }; # ^ # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217|-> const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 # 219| uint32_t i = 0; _______________________________________________
New patch attached.
Michal
The patch is fine, (so ack also from me), but can someone check if the attached rebase on top of Pavel's sssctl is OK?
I did not check the differences between the rebased and non-rebased version, but I just tried branch with sssctl patches + this rebased patch + startup validation patches and it worked for me.
But the CI failed for me with the snippets in place. Will take a look at what is happening there.
It failed beccause it bas broken with libini_config 1.0 .. 1.2 and current CI machines still have libini_config 1.2 or 1.1 (el6)
Updated patch is attached.
LS
On (24/06/16 13:49), Lukas Slebodnik wrote:
On (24/06/16 10:29), Michal Židek wrote:
On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote: > On (23/06/16 18:59), Michal Židek wrote: > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: > > > On (23/06/16 18:24), Michal Židek wrote: > > > > On 06/23/2016 05:50 PM, Michal Židek wrote: > > > > > Lukas, > > > > > > > > > > did you have some local patches related to > > > > > this patch, that you did not send here? > > > > > > > > > > Because I can not apply the patch on current > > > > > master and the changes in the context of > > > > > your patch seem like you did something with > > > > > default config file path? > > > > > > > > > > Slightly modified version that applies on > > > > > current master is attached (the changes > > > > > are the same, but the context differs). > > > > > > > > > > Michal > > > > > > > > > > > > > I added the missing access check for > > > > snippets. If the snippets does not > > > > pass the access check it is skipped > > > > and a loud debug message is logged. > > > > > > > > I attach it as separate patch for > > > > easier review, but it should be squashed > > > > before pushing. I also attach the first > > > > patch for convenience. > > > > > > > > Michal > > > > > > > From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100 > > > > Subject: [PATCH] confdb: Make it possible to use config snippets > > > > > > > > Resolves: > > > > https://fedorahosted.org/sssd/ticket/2247 > > > > > > > > Signed-off-by: Lukas Slebodnik lslebodn@redhat.com > > > > --- > LGTM. > > It isn't ACK because I was involved in development > and I didn;t test last version but code is fine fore me.
I just ran a quick test and the patch works for me and the code reads OK as well. I will push after CI finishes.
I also opened: https://fedorahosted.org/sssd/ticket/3062 because the patch is missing documentation and https://fedorahosted.org/sssd/ticket/3063 to add at least a simple integration test.
Actually, can we get rid of this warning?
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] # const char *patterns[] = { "^[^\.].*\.conf", NULL }; # ^ # 214| int ret; # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217| const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] # const char *sections[] = { ".*", NULL }; # ^ # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217|-> const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 # 219| uint32_t i = 0; _______________________________________________
New patch attached.
Michal
The patch is fine, (so ack also from me), but can someone check if the attached rebase on top of Pavel's sssctl is OK?
I did not check the differences between the rebased and non-rebased version, but I just tried branch with sssctl patches + this rebased patch + startup validation patches and it worked for me.
But the CI failed for me with the snippets in place. Will take a look at what is happening there.
It failed beccause it bas broken with libini_config 1.0 .. 1.2 and current CI machines still have libini_config 1.2 or 1.1 (el6)
Updated patch is attached.
Ups,
ENOPATCH
fixed in this version :-)
LS
On (24/06/16 13:51), Lukas Slebodnik wrote:
On (24/06/16 13:49), Lukas Slebodnik wrote:
On (24/06/16 10:29), Michal Židek wrote:
On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote: > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote: > > On (23/06/16 18:59), Michal Židek wrote: > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: > > > > On (23/06/16 18:24), Michal Židek wrote: > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote: > > > > > > Lukas, > > > > > > > > > > > > did you have some local patches related to > > > > > > this patch, that you did not send here? > > > > > > > > > > > > Because I can not apply the patch on current > > > > > > master and the changes in the context of > > > > > > your patch seem like you did something with > > > > > > default config file path? > > > > > > > > > > > > Slightly modified version that applies on > > > > > > current master is attached (the changes > > > > > > are the same, but the context differs). > > > > > > > > > > > > Michal > > > > > > > > > > > > > > > > I added the missing access check for > > > > > snippets. If the snippets does not > > > > > pass the access check it is skipped > > > > > and a loud debug message is logged. > > > > > > > > > > I attach it as separate patch for > > > > > easier review, but it should be squashed > > > > > before pushing. I also attach the first > > > > > patch for convenience. > > > > > > > > > > Michal > > > > > > > > > From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100 > > > > > Subject: [PATCH] confdb: Make it possible to use config snippets > > > > > > > > > > Resolves: > > > > > https://fedorahosted.org/sssd/ticket/2247 > > > > > > > > > > Signed-off-by: Lukas Slebodnik lslebodn@redhat.com > > > > > --- > > LGTM. > > > > It isn't ACK because I was involved in development > > and I didn;t test last version but code is fine fore me. > > I just ran a quick test and the patch works for me and the code reads OK > as well. I will push after CI finishes. > > I also opened: > https://fedorahosted.org/sssd/ticket/3062 > because the patch is missing documentation and > https://fedorahosted.org/sssd/ticket/3063 > to add at least a simple integration test.
Actually, can we get rid of this warning?
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] # const char *patterns[] = { "^[^\.].*\.conf", NULL }; # ^ # 214| int ret; # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217| const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
Error: COMPILER_WARNING: sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] # const char *sections[] = { ".*", NULL }; # ^ # 215| #ifdef HAVE_LIBINI_CONFIG_V1 # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; # 217|-> const char *sections[] = { ".*", NULL }; # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 # 219| uint32_t i = 0; _______________________________________________
New patch attached.
Michal
The patch is fine, (so ack also from me), but can someone check if the attached rebase on top of Pavel's sssctl is OK?
I did not check the differences between the rebased and non-rebased version, but I just tried branch with sssctl patches + this rebased patch + startup validation patches and it worked for me.
But the CI failed for me with the snippets in place. Will take a look at what is happening there.
It failed beccause it bas broken with libini_config 1.0 .. 1.2 and current CI machines still have libini_config 1.2 or 1.1 (el6)
Updated patch is attached.
Ups,
ENOPATCH
fixed in this version :-)
Previous version had an issue with SELinux and blocking access to conf.d.
Hopefully last version is attached.
LS
On 06/24/2016 02:52 PM, Lukas Slebodnik wrote:
On (24/06/16 13:51), Lukas Slebodnik wrote:
On (24/06/16 13:49), Lukas Slebodnik wrote:
On (24/06/16 10:29), Michal Židek wrote:
On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
On 06/23/2016 09:40 PM, Jakub Hrozek wrote: > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote: >> On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote: >>> On (23/06/16 18:59), Michal Židek wrote: >>>> On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: >>>>> On (23/06/16 18:24), Michal Židek wrote: >>>>>> On 06/23/2016 05:50 PM, Michal Židek wrote: >>>>>>> Lukas, >>>>>>> >>>>>>> did you have some local patches related to >>>>>>> this patch, that you did not send here? >>>>>>> >>>>>>> Because I can not apply the patch on current >>>>>>> master and the changes in the context of >>>>>>> your patch seem like you did something with >>>>>>> default config file path? >>>>>>> >>>>>>> Slightly modified version that applies on >>>>>>> current master is attached (the changes >>>>>>> are the same, but the context differs). >>>>>>> >>>>>>> Michal >>>>>>> >>>>>> >>>>>> I added the missing access check for >>>>>> snippets. If the snippets does not >>>>>> pass the access check it is skipped >>>>>> and a loud debug message is logged. >>>>>> >>>>>> I attach it as separate patch for >>>>>> easier review, but it should be squashed >>>>>> before pushing. I also attach the first >>>>>> patch for convenience. >>>>>> >>>>>> Michal >>>>> >>>>>> From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 >>>>>> From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com >>>>>> Date: Tue, 22 Mar 2016 14:09:34 +0100 >>>>>> Subject: [PATCH] confdb: Make it possible to use config snippets >>>>>> >>>>>> Resolves: >>>>>> https://fedorahosted.org/sssd/ticket/2247 >>>>>> >>>>>> Signed-off-by: Lukas Slebodnik lslebodn@redhat.com >>>>>> --- >>> LGTM. >>> >>> It isn't ACK because I was involved in development >>> and I didn;t test last version but code is fine fore me. >> >> I just ran a quick test and the patch works for me and the code reads OK >> as well. I will push after CI finishes. >> >> I also opened: >> https://fedorahosted.org/sssd/ticket/3062 >> because the patch is missing documentation and >> https://fedorahosted.org/sssd/ticket/3063 >> to add at least a simple integration test. > > Actually, can we get rid of this warning? > > Error: COMPILER_WARNING: > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] > # const char *patterns[] = { "^[^\.].*\.conf", NULL }; > # ^ > # 214| int ret; > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 > # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; > # 217| const char *sections[] = { ".*", NULL }; > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 > > Error: COMPILER_WARNING: > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] > # const char *sections[] = { ".*", NULL }; > # ^ > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 > # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; > # 217|-> const char *sections[] = { ".*", NULL }; > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 > # 219| uint32_t i = 0; > _______________________________________________
New patch attached.
Michal
The patch is fine, (so ack also from me), but can someone check if the attached rebase on top of Pavel's sssctl is OK?
I did not check the differences between the rebased and non-rebased version, but I just tried branch with sssctl patches + this rebased patch + startup validation patches and it worked for me.
But the CI failed for me with the snippets in place. Will take a look at what is happening there.
It failed beccause it bas broken with libini_config 1.0 .. 1.2 and current CI machines still have libini_config 1.2 or 1.1 (el6)
Updated patch is attached.
Ups,
ENOPATCH
fixed in this version :-)
Previous version had an issue with SELinux and blocking access to conf.d.
Hopefully last version is attached.
LS
Thanks, this version works for all CI machines.
Here is result of the CI: http://sssd-ci.duckdns.org/logs/job/46/08/summary.html
There are two failures that are not related to these patches (One is "device is busy" error during mock and the other was that ldap server failed to start for one set of tests).
On Fri, Jun 24, 2016 at 03:55:14PM +0200, Michal Židek wrote:
On 06/24/2016 02:52 PM, Lukas Slebodnik wrote:
On (24/06/16 13:51), Lukas Slebodnik wrote:
On (24/06/16 13:49), Lukas Slebodnik wrote:
On (24/06/16 10:29), Michal Židek wrote:
On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote: > On 06/23/2016 09:40 PM, Jakub Hrozek wrote: > > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote: > > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote: > > > > On (23/06/16 18:59), Michal Židek wrote: > > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: > > > > > > On (23/06/16 18:24), Michal Židek wrote: > > > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote: > > > > > > > > Lukas, > > > > > > > > > > > > > > > > did you have some local patches related to > > > > > > > > this patch, that you did not send here? > > > > > > > > > > > > > > > > Because I can not apply the patch on current > > > > > > > > master and the changes in the context of > > > > > > > > your patch seem like you did something with > > > > > > > > default config file path? > > > > > > > > > > > > > > > > Slightly modified version that applies on > > > > > > > > current master is attached (the changes > > > > > > > > are the same, but the context differs). > > > > > > > > > > > > > > > > Michal > > > > > > > > > > > > > > > > > > > > > > I added the missing access check for > > > > > > > snippets. If the snippets does not > > > > > > > pass the access check it is skipped > > > > > > > and a loud debug message is logged. > > > > > > > > > > > > > > I attach it as separate patch for > > > > > > > easier review, but it should be squashed > > > > > > > before pushing. I also attach the first > > > > > > > patch for convenience. > > > > > > > > > > > > > > Michal > > > > > > > > > > > > > From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 > > > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com > > > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100 > > > > > > > Subject: [PATCH] confdb: Make it possible to use config snippets > > > > > > > > > > > > > > Resolves: > > > > > > > https://fedorahosted.org/sssd/ticket/2247 > > > > > > > > > > > > > > Signed-off-by: Lukas Slebodnik lslebodn@redhat.com > > > > > > > --- > > > > LGTM. > > > > > > > > It isn't ACK because I was involved in development > > > > and I didn;t test last version but code is fine fore me. > > > > > > I just ran a quick test and the patch works for me and the code reads OK > > > as well. I will push after CI finishes. > > > > > > I also opened: > > > https://fedorahosted.org/sssd/ticket/3062 > > > because the patch is missing documentation and > > > https://fedorahosted.org/sssd/ticket/3063 > > > to add at least a simple integration test. > > > > Actually, can we get rid of this warning? > > > > Error: COMPILER_WARNING: > > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] > > # const char *patterns[] = { "^[^\.].*\.conf", NULL }; > > # ^ > > # 214| int ret; > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 > > # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; > > # 217| const char *sections[] = { ".*", NULL }; > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 > > > > Error: COMPILER_WARNING: > > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' > > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] > > # const char *sections[] = { ".*", NULL }; > > # ^ > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 > > # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; > > # 217|-> const char *sections[] = { ".*", NULL }; > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 > > # 219| uint32_t i = 0; > > _______________________________________________ > > New patch attached. > > Michal >
The patch is fine, (so ack also from me), but can someone check if the attached rebase on top of Pavel's sssctl is OK?
I did not check the differences between the rebased and non-rebased version, but I just tried branch with sssctl patches + this rebased patch + startup validation patches and it worked for me.
But the CI failed for me with the snippets in place. Will take a look at what is happening there.
It failed beccause it bas broken with libini_config 1.0 .. 1.2 and current CI machines still have libini_config 1.2 or 1.1 (el6)
Updated patch is attached.
Ups,
ENOPATCH
fixed in this version :-)
Previous version had an issue with SELinux and blocking access to conf.d.
Hopefully last version is attached.
LS
Thanks, this version works for all CI machines.
Here is result of the CI: http://sssd-ci.duckdns.org/logs/job/46/08/summary.html
There are two failures that are not related to these patches (One is "device is busy" error during mock and the other was that ldap server failed to start for one set of tests).
I would like to push the sssctl patches first, can you rebase? (You can see what I changed in the previous mail to rebase the patch atop sssctl).
On (27/06/16 14:00), Jakub Hrozek wrote:
On Fri, Jun 24, 2016 at 03:55:14PM +0200, Michal Židek wrote:
On 06/24/2016 02:52 PM, Lukas Slebodnik wrote:
On (24/06/16 13:51), Lukas Slebodnik wrote:
On (24/06/16 13:49), Lukas Slebodnik wrote:
On (24/06/16 10:29), Michal Židek wrote:
On 06/24/2016 09:57 AM, Jakub Hrozek wrote: > On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote: > > On 06/23/2016 09:40 PM, Jakub Hrozek wrote: > > > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote: > > > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote: > > > > > On (23/06/16 18:59), Michal Židek wrote: > > > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: > > > > > > > On (23/06/16 18:24), Michal Židek wrote: > > > > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote: > > > > > > > > > Lukas, > > > > > > > > > > > > > > > > > > did you have some local patches related to > > > > > > > > > this patch, that you did not send here? > > > > > > > > > > > > > > > > > > Because I can not apply the patch on current > > > > > > > > > master and the changes in the context of > > > > > > > > > your patch seem like you did something with > > > > > > > > > default config file path? > > > > > > > > > > > > > > > > > > Slightly modified version that applies on > > > > > > > > > current master is attached (the changes > > > > > > > > > are the same, but the context differs). > > > > > > > > > > > > > > > > > > Michal > > > > > > > > > > > > > > > > > > > > > > > > > I added the missing access check for > > > > > > > > snippets. If the snippets does not > > > > > > > > pass the access check it is skipped > > > > > > > > and a loud debug message is logged. > > > > > > > > > > > > > > > > I attach it as separate patch for > > > > > > > > easier review, but it should be squashed > > > > > > > > before pushing. I also attach the first > > > > > > > > patch for convenience. > > > > > > > > > > > > > > > > Michal > > > > > > > > > > > > > > > From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001 > > > > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com > > > > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100 > > > > > > > > Subject: [PATCH] confdb: Make it possible to use config snippets > > > > > > > > > > > > > > > > Resolves: > > > > > > > > https://fedorahosted.org/sssd/ticket/2247 > > > > > > > > > > > > > > > > Signed-off-by: Lukas Slebodnik lslebodn@redhat.com > > > > > > > > --- > > > > > LGTM. > > > > > > > > > > It isn't ACK because I was involved in development > > > > > and I didn;t test last version but code is fine fore me. > > > > > > > > I just ran a quick test and the patch works for me and the code reads OK > > > > as well. I will push after CI finishes. > > > > > > > > I also opened: > > > > https://fedorahosted.org/sssd/ticket/3062 > > > > because the patch is missing documentation and > > > > https://fedorahosted.org/sssd/ticket/3063 > > > > to add at least a simple integration test. > > > > > > Actually, can we get rid of this warning? > > > > > > Error: COMPILER_WARNING: > > > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' [-Wunused-variable] > > > # const char *patterns[] = { "^[^\.].*\.conf", NULL }; > > > # ^ > > > # 214| int ret; > > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 > > > # 216|-> const char *patterns[] = { "^[^\.].*\.conf", NULL }; > > > # 217| const char *sections[] = { ".*", NULL }; > > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 > > > > > > Error: COMPILER_WARNING: > > > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config' > > > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' [-Wunused-variable] > > > # const char *sections[] = { ".*", NULL }; > > > # ^ > > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 > > > # 216| const char *patterns[] = { "^[^\.].*\.conf", NULL }; > > > # 217|-> const char *sections[] = { ".*", NULL }; > > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 > > > # 219| uint32_t i = 0; > > > _______________________________________________ > > > > New patch attached. > > > > Michal > > > > The patch is fine, (so ack also from me), but can someone check if the > attached rebase on top of Pavel's sssctl is OK? > >
I did not check the differences between the rebased and non-rebased version, but I just tried branch with sssctl patches + this rebased patch + startup validation patches and it worked for me.
But the CI failed for me with the snippets in place. Will take a look at what is happening there.
It failed beccause it bas broken with libini_config 1.0 .. 1.2 and current CI machines still have libini_config 1.2 or 1.1 (el6)
Updated patch is attached.
Ups,
ENOPATCH
fixed in this version :-)
Previous version had an issue with SELinux and blocking access to conf.d.
Hopefully last version is attached.
LS
Thanks, this version works for all CI machines.
Here is result of the CI: http://sssd-ci.duckdns.org/logs/job/46/08/summary.html
There are two failures that are not related to these patches (One is "device is busy" error during mock and the other was that ldap server failed to start for one set of tests).
I would like to push the sssctl patches first, can you rebase? (You can see what I changed in the previous mail to rebase the patch atop sssctl).
The patch was created on top of Pavel's patches.
It depends on which patches will be pushed to master. but I was able to apply patch from this thread with 3way merge on Pavel's sssctl branch from fedorapeople; there weren't any conflicts
LS
On Mon, Jun 27, 2016 at 03:05:54PM +0200, Lukas Slebodnik wrote:
It depends on which patches will be pushed to master. but I was able to apply patch from this thread with 3way merge on Pavel's sssctl branch from fedorapeople; there weren't any conflicts
The latest ones on the list. I will push them once CI finishes and then it will be clear what to rebase on.
On 06/27/2016 04:18 PM, Jakub Hrozek wrote:
On Mon, Jun 27, 2016 at 03:05:54PM +0200, Lukas Slebodnik wrote:
It depends on which patches will be pushed to master. but I was able to apply patch from this thread with 3way merge on Pavel's sssctl branch from fedorapeople; there weren't any conflicts
The latest ones on the list. I will push them once CI finishes and then it will be clear what to rebase on.
The attached patch can apply even without 3way. I tested this version too.
Michal
On Mon, Jun 27, 2016 at 05:58:49PM +0200, Michal Židek wrote:
On 06/27/2016 04:18 PM, Jakub Hrozek wrote:
On Mon, Jun 27, 2016 at 03:05:54PM +0200, Lukas Slebodnik wrote:
It depends on which patches will be pushed to master. but I was able to apply patch from this thread with 3way merge on Pavel's sssctl branch from fedorapeople; there weren't any conflicts
The latest ones on the list. I will push them once CI finishes and then it will be clear what to rebase on.
The attached patch can apply even without 3way. I tested this version too.
Michal
Thanks, I did a smoke test of this patch and it still seems to work fine. I'll push it when CI finishes.
On Mon, Jun 27, 2016 at 07:02:19PM +0200, Jakub Hrozek wrote:
On Mon, Jun 27, 2016 at 05:58:49PM +0200, Michal Židek wrote:
On 06/27/2016 04:18 PM, Jakub Hrozek wrote:
On Mon, Jun 27, 2016 at 03:05:54PM +0200, Lukas Slebodnik wrote:
It depends on which patches will be pushed to master. but I was able to apply patch from this thread with 3way merge on Pavel's sssctl branch from fedorapeople; there weren't any conflicts
The latest ones on the list. I will push them once CI finishes and then it will be clear what to rebase on.
The attached patch can apply even without 3way. I tested this version too.
Michal
Thanks, I did a smoke test of this patch and it still seems to work fine. I'll push it when CI finishes.
CI: http://sssd-ci.duckdns.org/logs/job/46/39/summary.html (our rawhide machine is sick)
master: cca497b4cbbbf05c4f9181b7d8113cde81754831
On Thu, Jun 23, 2016 at 11:46:33AM +0200, Lukas Slebodnik wrote:
On (23/06/16 11:03), Lukas Slebodnik wrote:
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 exitHelp 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
BTW do not forget that SELinux might deny access to conf.d
type=AVC msg=audit(06/23/2016 10:44:57.486:910) : avc: denied { read } for pid=27671 comm=sssd name=conf.d dev="dm-1" ino=1871243 scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:object_r:sssd_conf_t:s0 tclass=dir permissive=0
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Michal Židek -
Stephen Gallagher