Add ldap_autofs_map_master_name option so that the auto master map name change be changed from the default auto.master.
On Fri, May 24, 2013 at 04:41:13PM -0700, C. S. wrote:
Add ldap_autofs_map_master_name option so that the auto master map name change be changed from the default auto.master.
Hi Cove,
this quite a good patch, thank you very much for the contribution.
I haven't done much testing yet, but I only have a couple of small suggestions before we can apply the patch to master.
Here is a diff that summarizes them with comments inline:
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index 3e019d9..201dcbe 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -415,7 +415,6 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, /* automount master map name */ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); if (master_map == NULL) { - master_map = strdup(AUTOFS_MAP_MASTER_NAME);
You don't have to strdup the option value, the call to dp_opt_set_string() duplicates the value itself and maintains the memory hierarchy as it uses the talloc allocator.
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, master_map); if (ret != EOK) { diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index e56690a..7d2d3f5 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h
In the LDAP maps it's OK to add the new option to the "enum sdap_basic_opt". The reason is that SSSD maintains a couple of "option maps" for both the LDAP provider as a whole and for the individual object maps. The LDAP provider maps describe how to get the object from LDAP (for instance it contains the search bases, timeouts, schema etc.) and then the per-object maps describe how to get the individual objects (what attribute name describes which attribute, for instance "cn" usually stands for "name).
I was wondering for a short while whether the new attribute belongs to the generic LDAP map or the autofs map object map, but I think this solution is OK. It describes a mean to get the master map, not any master map attribute.
@@ -328,7 +328,6 @@ enum sdap_service_attrs { };
enum sdap_autofs_map_attrs { - SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_MAP, SDAP_AT_AUTOFS_MAP_NAME,
@@ -336,7 +335,6 @@ enum sdap_autofs_map_attrs { };
enum sdap_autofs_entry_attrs { - _SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_ENTRY, SDAP_AT_AUTOFS_ENTRY_KEY, SDAP_AT_AUTOFS_ENTRY_VALUE,
Thank you again for the patch!
Hi Jakub,
Thanks for the feedback! Very helpful. Looks like I dropped the LDAP map changes during a merge, that is now included. And I've removed the strdup(), whoops! ;-)
cs
From 33689c2565ec71d10eefe081fb3c502d1944d89e Mon Sep 17 00:00:00 2001
From: Cove Schneider cove@ilm.com Date: Tue, 28 May 2013 16:15:11 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
--- src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 3 +++ src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 5 +++++ src/providers/ldap/sdap.h | 3 +++ src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 59 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry> + <term>ldap_autofs_map_master_name (string)</term> + <listitem> + <para> + The name of the automount master map in LDAP. + </para> + <para> + Default: auto.master + </para> + </listitem> + </varlistentry> + </variablelist> + <variablelist> + <varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para> diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, + { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE }, diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
- /* If a request for auto.master comes in, the automounter deamon - * has been reloaded. Expire all autofs maps to force reload - */ - if (strcmp(be_autofs_req->mapname, "auto.master") == 0) { - be_autofs_req->invalidate = true; - } - be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) { diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..9a99dcdb782ea3cb070f877362a0ab2b7d80be5c 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, + { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, @@ -287,12 +288,14 @@ struct sdap_attr_map ipa_service_map[] = { };
struct sdap_attr_map ipa_autofs_mobject_map[] = { + { "ldap_autofs_map_master_name", "automountMapMasterName", SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_map_object_class", "automountMap", SYSDB_AUTOFS_MAP_OC, NULL }, { "ldap_autofs_map_name", "automountMapName", SYSDB_AUTOFS_MAP_NAME, NULL }, SDAP_ATTR_MAP_TERMINATOR };
struct sdap_attr_map ipa_autofs_entry_map[] = { + { "ldap_autofs_map_master_name", "automountMapMasterName", SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_entry_object_class", "automount", SYSDB_AUTOFS_ENTRY_OC, NULL }, { "ldap_autofs_entry_key", "automountKey", SYSDB_AUTOFS_ENTRY_KEY, NULL }, { "ldap_autofs_entry_value", "automountInformation", SYSDB_AUTOFS_ENTRY_VALUE, NULL }, diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..0c70d44b78eaa45f27e6ae0ee29c96404ca6df0b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base; + const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
+ /* automount master map name */ + master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); + if (master_map == NULL) { + ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, + AUTOFS_MAP_MASTER_NAME); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name" + " to default value\n")); + return ret; + } + master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); + } + DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map)); + /* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) { diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master" + /* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..f4b572e687ce0227f124217f9f2b43b1c801e227 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, + { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, @@ -330,12 +331,14 @@ struct sdap_attr_map service_map[] = { };
struct sdap_attr_map rfc2307_autofs_mobject_map[] = { + { "ldap_autofs_map_master_name", "automountMapMasterName", SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_map_object_class", "automountMap", SYSDB_AUTOFS_MAP_OC, NULL }, { "ldap_autofs_map_name", "ou", SYSDB_AUTOFS_MAP_NAME, NULL }, SDAP_ATTR_MAP_TERMINATOR };
struct sdap_attr_map rfc2307_autofs_entry_map[] = { + { "ldap_autofs_map_master_name", "automountMapMasterName", SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_entry_object_class", "automount", SYSDB_AUTOFS_ENTRY_OC, NULL }, { "ldap_autofs_entry_key", "cn", SYSDB_AUTOFS_ENTRY_KEY, NULL }, { "ldap_autofs_entry_value", "automountInformation", SYSDB_AUTOFS_ENTRY_VALUE, NULL }, @@ -343,12 +346,14 @@ struct sdap_attr_map rfc2307_autofs_entry_map[] = { };
struct sdap_attr_map rfc2307bis_autofs_mobject_map[] = { + { "ldap_autofs_map_master_name", "automountMapMasterName", SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_map_object_class", "automountMap", SYSDB_AUTOFS_MAP_OC, NULL }, { "ldap_autofs_map_name", "automountMapName", SYSDB_AUTOFS_MAP_NAME, NULL }, SDAP_ATTR_MAP_TERMINATOR };
struct sdap_attr_map rfc2307bis_autofs_entry_map[] = { + { "ldap_autofs_map_master_name", "automountMapMasterName", SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_entry_object_class", "automount", SYSDB_AUTOFS_ENTRY_OC, NULL }, { "ldap_autofs_entry_key", "automountKey", SYSDB_AUTOFS_ENTRY_KEY, NULL }, { "ldap_autofs_entry_value", "automountInformation", SYSDB_AUTOFS_ENTRY_VALUE, NULL }, diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..e56690a10907ede5205ddb463ff6d4fc226145b3 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE, + SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM, @@ -327,6 +328,7 @@ enum sdap_service_attrs { };
enum sdap_autofs_map_attrs { + SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_MAP, SDAP_AT_AUTOFS_MAP_NAME,
@@ -334,6 +336,7 @@ enum sdap_autofs_map_attrs { };
enum sdap_autofs_entry_attrs { + _SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_ENTRY, SDAP_AT_AUTOFS_ENTRY_KEY, SDAP_AT_AUTOFS_ENTRY_VALUE, diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req; + const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n")); @@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
+ if (autofs_req->mapname != NULL) { + master_map = dp_opt_get_string(id_ctx->opts->basic, + SDAP_AUTOFS_MAP_MASTER_NAME); + if (strcmp(master_map, autofs_req->mapname) == 0) { + autofs_req->invalidate = true; + DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n", + autofs_req->mapname)); + } + } + if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain); -- 1.8.1.4
On Mon, May 27, 2013 at 8:28 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Fri, May 24, 2013 at 04:41:13PM -0700, C. S. wrote:
Add ldap_autofs_map_master_name option so that the auto master map name change be changed from the default auto.master.
Hi Cove,
this quite a good patch, thank you very much for the contribution.
I haven't done much testing yet, but I only have a couple of small suggestions before we can apply the patch to master.
Here is a diff that summarizes them with comments inline:
diff --git a/src/providers/ldap/ldap_common.c
b/src/providers/ldap/ldap_common.c
index 3e019d9..201dcbe 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -415,7 +415,6 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, /* automount master map name */ master_map = dp_opt_get_string(opts->basic,
SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
master_map = strdup(AUTOFS_MAP_MASTER_NAME);You don't have to strdup the option value, the call to dp_opt_set_string() duplicates the value itself and maintains the memory hierarchy as it uses the talloc allocator.
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, master_map); if (ret != EOK) {diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index e56690a..7d2d3f5 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h
In the LDAP maps it's OK to add the new option to the "enum sdap_basic_opt". The reason is that SSSD maintains a couple of "option maps" for both the LDAP provider as a whole and for the individual object maps. The LDAP provider maps describe how to get the object from LDAP (for instance it contains the search bases, timeouts, schema etc.) and then the per-object maps describe how to get the individual objects (what attribute name describes which attribute, for instance "cn" usually stands for "name).
I was wondering for a short while whether the new attribute belongs to the generic LDAP map or the autofs map object map, but I think this solution is OK. It describes a mean to get the master map, not any master map attribute.
@@ -328,7 +328,6 @@ enum sdap_service_attrs { };
enum sdap_autofs_map_attrs {
- SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_MAP, SDAP_AT_AUTOFS_MAP_NAME,
@@ -336,7 +335,6 @@ enum sdap_autofs_map_attrs { };
enum sdap_autofs_entry_attrs {
- _SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_ENTRY, SDAP_AT_AUTOFS_ENTRY_KEY, SDAP_AT_AUTOFS_ENTRY_VALUE,
Thank you again for the patch! _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
(Including once more as an attachment, it looks like something may have wrapped the lines when I had the patch in the body of the message.)
On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote:
Hi Jakub,
Thanks for the feedback! Very helpful. Looks like I dropped the LDAP map changes during a merge, that is now included. And I've removed the strdup(), whoops! ;-)
cs
From 33689c2565ec71d10eefe081fb3c502d1944d89e Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Tue, 28 May 2013 16:15:11 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 3 +++ src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 5 +++++ src/providers/ldap/sdap.h | 3 +++ src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 59 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry>
<term>ldap_autofs_map_master_name (string)</term><listitem><para>The name of the automount master map in LDAP.</para><para>Default: auto.master</para></listitem></varlistentry></variablelist><variablelist><varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para>diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING,
NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE }, diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
/* If a request for auto.master comes in, the automounter deamon
* has been reloaded. Expire all autofs maps to force reload*/if (strcmp(be_autofs_req->mapname, "auto.master") == 0) {
be_autofs_req->invalidate = true;}
be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..9a99dcdb782ea3cb070f877362a0ab2b7d80be5c 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING,
NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, @@ -287,12 +288,14 @@ struct sdap_attr_map ipa_service_map[] = { };
struct sdap_attr_map ipa_autofs_mobject_map[] = {
- { "ldap_autofs_map_master_name", "automountMapMasterName",
SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_map_object_class", "automountMap", SYSDB_AUTOFS_MAP_OC, NULL }, { "ldap_autofs_map_name", "automountMapName", SYSDB_AUTOFS_MAP_NAME, NULL }, SDAP_ATTR_MAP_TERMINATOR };
struct sdap_attr_map ipa_autofs_entry_map[] = {
- { "ldap_autofs_map_master_name", "automountMapMasterName",
SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_entry_object_class", "automount", SYSDB_AUTOFS_ENTRY_OC, NULL }, { "ldap_autofs_entry_key", "automountKey", SYSDB_AUTOFS_ENTRY_KEY, NULL }, { "ldap_autofs_entry_value", "automountInformation", SYSDB_AUTOFS_ENTRY_VALUE, NULL }, diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..0c70d44b78eaa45f27e6ae0ee29c96404ca6df0b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base;
const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
/* automount master map name */
master_map = dp_opt_get_string(opts->basic,
SDAP_AUTOFS_MAP_MASTER_NAME);
- if (master_map == NULL) {
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME,AUTOFS_MAP_MASTER_NAME);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master mapname"
" to default value\n"));return ret;}master_map = dp_opt_get_string(opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);
- }
- DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to
%s\n", master_map));
- /* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master"
/* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..f4b572e687ce0227f124217f9f2b43b1c801e227 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING,
NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, @@ -330,12 +331,14 @@ struct sdap_attr_map service_map[] = { };
struct sdap_attr_map rfc2307_autofs_mobject_map[] = {
- { "ldap_autofs_map_master_name", "automountMapMasterName",
SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_map_object_class", "automountMap", SYSDB_AUTOFS_MAP_OC, NULL }, { "ldap_autofs_map_name", "ou", SYSDB_AUTOFS_MAP_NAME, NULL }, SDAP_ATTR_MAP_TERMINATOR };
struct sdap_attr_map rfc2307_autofs_entry_map[] = {
- { "ldap_autofs_map_master_name", "automountMapMasterName",
SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_entry_object_class", "automount", SYSDB_AUTOFS_ENTRY_OC, NULL }, { "ldap_autofs_entry_key", "cn", SYSDB_AUTOFS_ENTRY_KEY, NULL }, { "ldap_autofs_entry_value", "automountInformation", SYSDB_AUTOFS_ENTRY_VALUE, NULL }, @@ -343,12 +346,14 @@ struct sdap_attr_map rfc2307_autofs_entry_map[] = { };
struct sdap_attr_map rfc2307bis_autofs_mobject_map[] = {
- { "ldap_autofs_map_master_name", "automountMapMasterName",
SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_map_object_class", "automountMap", SYSDB_AUTOFS_MAP_OC, NULL }, { "ldap_autofs_map_name", "automountMapName", SYSDB_AUTOFS_MAP_NAME, NULL }, SDAP_ATTR_MAP_TERMINATOR };
struct sdap_attr_map rfc2307bis_autofs_entry_map[] = {
- { "ldap_autofs_map_master_name", "automountMapMasterName",
SYSDB_AUTOFS_MAP_MASTER_NAME, NULL }, { "ldap_autofs_entry_object_class", "automount", SYSDB_AUTOFS_ENTRY_OC, NULL }, { "ldap_autofs_entry_key", "automountKey", SYSDB_AUTOFS_ENTRY_KEY, NULL }, { "ldap_autofs_entry_value", "automountInformation", SYSDB_AUTOFS_ENTRY_VALUE, NULL }, diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..e56690a10907ede5205ddb463ff6d4fc226145b3 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE,
- SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM,
@@ -327,6 +328,7 @@ enum sdap_service_attrs { };
enum sdap_autofs_map_attrs {
- SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_MAP, SDAP_AT_AUTOFS_MAP_NAME,
@@ -334,6 +336,7 @@ enum sdap_autofs_map_attrs { };
enum sdap_autofs_entry_attrs {
- _SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_ENTRY, SDAP_AT_AUTOFS_ENTRY_KEY, SDAP_AT_AUTOFS_ENTRY_VALUE,
diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req;
const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n"));
@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
- if (autofs_req->mapname != NULL) {
master_map = dp_opt_get_string(id_ctx->opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);if (strcmp(master_map, autofs_req->mapname) == 0) {autofs_req->invalidate = true;DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master maptriggered: %s\n",
autofs_req->mapname));}- }
- if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain);
-- 1.8.1.4
On Mon, May 27, 2013 at 8:28 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Fri, May 24, 2013 at 04:41:13PM -0700, C. S. wrote:
Add ldap_autofs_map_master_name option so that the auto master map name change be changed from the default auto.master.
Hi Cove,
this quite a good patch, thank you very much for the contribution.
I haven't done much testing yet, but I only have a couple of small suggestions before we can apply the patch to master.
Here is a diff that summarizes them with comments inline:
diff --git a/src/providers/ldap/ldap_common.c
b/src/providers/ldap/ldap_common.c
index 3e019d9..201dcbe 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -415,7 +415,6 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, /* automount master map name */ master_map = dp_opt_get_string(opts->basic,
SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
master_map = strdup(AUTOFS_MAP_MASTER_NAME);You don't have to strdup the option value, the call to
dp_opt_set_string()
duplicates the value itself and maintains the memory hierarchy as it uses the talloc allocator.
ret = dp_opt_set_string(opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME,
master_map); if (ret != EOK) {diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index e56690a..7d2d3f5 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h
In the LDAP maps it's OK to add the new option to the "enum sdap_basic_opt". The reason is that SSSD maintains a couple of "option maps" for both the LDAP provider as a whole and for the individual object maps. The LDAP provider maps describe how to get the object from LDAP (for instance it contains the search bases, timeouts, schema etc.) and then the per-object maps describe how to get the individual objects (what attribute name describes which attribute, for instance "cn" usually stands for "name).
I was wondering for a short while whether the new attribute belongs to the generic LDAP map or the autofs map object map, but I think this solution is OK. It describes a mean to get the master map, not any master map attribute.
@@ -328,7 +328,6 @@ enum sdap_service_attrs { };
enum sdap_autofs_map_attrs {
- SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_MAP, SDAP_AT_AUTOFS_MAP_NAME,
@@ -336,7 +335,6 @@ enum sdap_autofs_map_attrs { };
enum sdap_autofs_entry_attrs {
- _SDAP_AT_AUTOFS_MAP_MASTER_NAME, SDAP_OC_AUTOFS_ENTRY, SDAP_AT_AUTOFS_ENTRY_KEY, SDAP_AT_AUTOFS_ENTRY_VALUE,
Thank you again for the patch! _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote:
(Including once more as an attachment, it looks like something may have wrapped the lines when I had the patch in the body of the message.)
On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote:
Hi Jakub,
Thanks for the feedback! Very helpful. Looks like I dropped the LDAP map changes during a merge, that is now included. And I've removed the strdup(), whoops! ;-)
cs
Hi Cove,
this patch is working for me with a couple of additonal changes that I attach as a mini-patch. If you like, you can squash it into yours and resend, then I think the patch would be good to go.
Basically I think it's enough to have the master map name defined in the LDAP options, not the map and entry option lists. Also the option lists need the objectclass to be the first entry in the map.
Sorry, I think I confused you with my previous e-mail.
With the attached patch on top of yours, I was able to run automounter with a custom master map (MASTER_MAP_NAME="auto.slave" in /etc/sysconfig/autofs) just fine.
Thanks again for the contribution.
Hi Jakub,
Enclosed is the squashed patch.
No worries, re-reading the thread I think I missed a few things early on. You were suggesting adding an attribute to handle an alias on the master map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map would needed to have been updated(?).
Also, automounter works fine with different master map names, we use it in production without issue at least. It was my understanding that sssd needs to know the master map name so that it can expire it from the cache properly on updates, is that correct? So in sssd.conf you also have ldap_autofs_map_master_name = auto.slave, right?
Thanks again for your patience, much appreciated. ;-)
On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote:
(Including once more as an attachment, it looks like something may have wrapped the lines when I had the patch in the body of the message.)
On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote:
Hi Jakub,
Thanks for the feedback! Very helpful. Looks like I dropped the LDAP
map
changes during a merge, that is now included. And I've removed the strdup(), whoops! ;-)
cs
Hi Cove,
this patch is working for me with a couple of additonal changes that I attach as a mini-patch. If you like, you can squash it into yours and resend, then I think the patch would be good to go.
Basically I think it's enough to have the master map name defined in the LDAP options, not the map and entry option lists. Also the option lists need the objectclass to be the first entry in the map.
Sorry, I think I confused you with my previous e-mail.
With the attached patch on top of yours, I was able to run automounter with a custom master map (MASTER_MAP_NAME="auto.slave" in /etc/sysconfig/autofs) just fine.
Thanks again for the contribution.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote:
Hi Jakub,
Enclosed is the squashed patch.
No worries, re-reading the thread I think I missed a few things early on. You were suggesting adding an attribute to handle an alias on the master map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map would needed to have been updated(?).
Sorry, I can be very confusing sometimes. I was just pondering two different options -- one was modifying the default_basic_opts[] array which you did, the other was modifying the mobject and entry maps, which I realized was the worse option.
In the latest patch, there are still additions to rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should go away. Can you test the attached patch? It is your patch with those two extra lines removed. If it works for you, I'll run the patch by another engineer to be sure (I won't be able to push patch with my additions myself then) and push.
The modified patch is attached.
Also, automounter works fine with different master map names, we use it in production without issue at least. It was my understanding that sssd needs to know the master map name so that it can expire it from the cache properly on updates, is that correct? So in sssd.conf you also have ldap_autofs_map_master_name = auto.slave, right?
Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to include the same master map called auto.slave.
Thanks again for your patience, much appreciated. ;-)
Thank you very much for the patch. Once the two additions to mobject and mentry are removed, I'll simply push the patch. I didn't mean to confuse you, sorry :)
On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote:
(Including once more as an attachment, it looks like something may have wrapped the lines when I had the patch in the body of the message.)
On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote:
Hi Jakub,
Thanks for the feedback! Very helpful. Looks like I dropped the LDAP
map
changes during a merge, that is now included. And I've removed the strdup(), whoops! ;-)
cs
Hi Cove,
this patch is working for me with a couple of additonal changes that I attach as a mini-patch. If you like, you can squash it into yours and resend, then I think the patch would be good to go.
Basically I think it's enough to have the master map name defined in the LDAP options, not the map and entry option lists. Also the option lists need the objectclass to be the first entry in the map.
Sorry, I think I confused you with my previous e-mail.
With the attached patch on top of yours, I was able to run automounter with a custom master map (MASTER_MAP_NAME="auto.slave" in /etc/sysconfig/autofs) just fine.
Thanks again for the contribution.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (03/06/13 22:41), Jakub Hrozek wrote:
On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote:
Hi Jakub,
Enclosed is the squashed patch.
No worries, re-reading the thread I think I missed a few things early on. You were suggesting adding an attribute to handle an alias on the master map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map would needed to have been updated(?).
Sorry, I can be very confusing sometimes. I was just pondering two different options -- one was modifying the default_basic_opts[] array which you did, the other was modifying the mobject and entry maps, which I realized was the worse option.
In the latest patch, there are still additions to rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should go away. Can you test the attached patch? It is your patch with those two extra lines removed. If it works for you, I'll run the patch by another engineer to be sure (I won't be able to push patch with my additions myself then) and push.
The modified patch is attached.
Also, automounter works fine with different master map names, we use it in production without issue at least. It was my understanding that sssd needs to know the master map name so that it can expire it from the cache properly on updates, is that correct? So in sssd.conf you also have ldap_autofs_map_master_name = auto.slave, right?
Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to include the same master map called auto.slave.
Thanks again for your patience, much appreciated. ;-)
Thank you very much for the patch. Once the two additions to mobject and mentry are removed, I'll simply push the patch. I didn't mean to confuse you, sorry :)
On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote:
(Including once more as an attachment, it looks like something may have wrapped the lines when I had the patch in the body of the message.)
On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote:
Hi Jakub,
Thanks for the feedback! Very helpful. Looks like I dropped the LDAP
map
changes during a merge, that is now included. And I've removed the strdup(), whoops! ;-)
cs
Hi Cove,
this patch is working for me with a couple of additonal changes that I attach as a mini-patch. If you like, you can squash it into yours and resend, then I think the patch would be good to go.
Basically I think it's enough to have the master map name defined in the LDAP options, not the map and entry option lists. Also the option lists need the objectclass to be the first entry in the map.
Sorry, I think I confused you with my previous e-mail.
With the attached patch on top of yours, I was able to run automounter with a custom master map (MASTER_MAP_NAME="auto.slave" in /etc/sysconfig/autofs) just fine.
Thanks again for the contribution.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I would like to apploligize for late answer, but, I had few problems with autofs configuration.
From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 1 + src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 1 + src/providers/ldap/sdap.h | 1 + src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry>
<term>ldap_autofs_map_master_name (string)</term><listitem><para>The name of the automount master map in LDAP.</para><para>Default: auto.master</para></listitem></varlistentry></variablelist><variablelist><varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para>diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
/* If a request for auto.master comes in, the automounter deamon
* has been reloaded. Expire all autofs maps to force reload*/if (strcmp(be_autofs_req->mapname, "auto.master") == 0) {
be_autofs_req->invalidate = true;}
be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base;
const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
/* automount master map name */
master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME,AUTOFS_MAP_MASTER_NAME);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name"" to default value\n"));return ret;}master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);}
DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map));
/* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master"
/* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE,
- SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM,
diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req;
const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n"));
@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
- if (autofs_req->mapname != NULL) {
master_map = dp_opt_get_string(id_ctx->opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);
^^^^^^^^^^ I think, that function dp_opt_get_string can return NULL. It would be not good idea to compare NULL with autofs_req->mapname.
if (strcmp(master_map, autofs_req->mapname) == 0) {autofs_req->invalidate = true;DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n",autofs_req->mapname));}- }
- if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain);
-- 1.8.2.1
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote:
On (03/06/13 22:41), Jakub Hrozek wrote:
On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote:
Hi Jakub,
Enclosed is the squashed patch.
No worries, re-reading the thread I think I missed a few things early on. You were suggesting adding an attribute to handle an alias on the master map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map would needed to have been updated(?).
Sorry, I can be very confusing sometimes. I was just pondering two different options -- one was modifying the default_basic_opts[] array which you did, the other was modifying the mobject and entry maps, which I realized was the worse option.
In the latest patch, there are still additions to rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should go away. Can you test the attached patch? It is your patch with those two extra lines removed. If it works for you, I'll run the patch by another engineer to be sure (I won't be able to push patch with my additions myself then) and push.
The modified patch is attached.
Also, automounter works fine with different master map names, we use it in production without issue at least. It was my understanding that sssd needs to know the master map name so that it can expire it from the cache properly on updates, is that correct? So in sssd.conf you also have ldap_autofs_map_master_name = auto.slave, right?
Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to include the same master map called auto.slave.
Thanks again for your patience, much appreciated. ;-)
Thank you very much for the patch. Once the two additions to mobject and mentry are removed, I'll simply push the patch. I didn't mean to confuse you, sorry :)
On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote:
(Including once more as an attachment, it looks like something may have wrapped the lines when I had the patch in the body of the message.)
On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote:
Hi Jakub,
Thanks for the feedback! Very helpful. Looks like I dropped the LDAP
map
changes during a merge, that is now included. And I've removed the strdup(), whoops! ;-)
cs
Hi Cove,
this patch is working for me with a couple of additonal changes that I attach as a mini-patch. If you like, you can squash it into yours and resend, then I think the patch would be good to go.
Basically I think it's enough to have the master map name defined in the LDAP options, not the map and entry option lists. Also the option lists need the objectclass to be the first entry in the map.
Sorry, I think I confused you with my previous e-mail.
With the attached patch on top of yours, I was able to run automounter with a custom master map (MASTER_MAP_NAME="auto.slave" in /etc/sysconfig/autofs) just fine.
Thanks again for the contribution.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I would like to apploligize for late answer, but, I had few problems with autofs configuration.
From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 1 + src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 1 + src/providers/ldap/sdap.h | 1 + src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry>
<term>ldap_autofs_map_master_name (string)</term><listitem><para>The name of the automount master map in LDAP.</para><para>Default: auto.master</para></listitem></varlistentry></variablelist><variablelist><varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para>diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
/* If a request for auto.master comes in, the automounter deamon
* has been reloaded. Expire all autofs maps to force reload*/if (strcmp(be_autofs_req->mapname, "auto.master") == 0) {
be_autofs_req->invalidate = true;}
be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base;
const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
/* automount master map name */
master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME,AUTOFS_MAP_MASTER_NAME);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name"" to default value\n"));return ret;}master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);}
DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map));
/* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master"
/* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE,
- SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM,
diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req;
const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n"));
@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
- if (autofs_req->mapname != NULL) {
master_map = dp_opt_get_string(id_ctx->opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);^^^^^^^^^^I think, that function dp_opt_get_string can return NULL. It would be not good idea to compare NULL with autofs_req->mapname.
It shouldn't happen but yeah, famous last words :-)
I agree with Lukas, let's be extra paranoid and also add a NULL check here. If we ever get a NULL string, we'll just use stale data, nothing terrible.
Thank you for testing!
if (strcmp(master_map, autofs_req->mapname) == 0) {autofs_req->invalidate = true;DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n",autofs_req->mapname));}- }
- if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain);
-- 1.8.2.1
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (17/06/13 19:47), Jakub Hrozek wrote:
On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote:
On (03/06/13 22:41), Jakub Hrozek wrote:
On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote:
Hi Jakub,
Enclosed is the squashed patch.
No worries, re-reading the thread I think I missed a few things early on. You were suggesting adding an attribute to handle an alias on the master map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map would needed to have been updated(?).
Sorry, I can be very confusing sometimes. I was just pondering two different options -- one was modifying the default_basic_opts[] array which you did, the other was modifying the mobject and entry maps, which I realized was the worse option.
In the latest patch, there are still additions to rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should go away. Can you test the attached patch? It is your patch with those two extra lines removed. If it works for you, I'll run the patch by another engineer to be sure (I won't be able to push patch with my additions myself then) and push.
The modified patch is attached.
Also, automounter works fine with different master map names, we use it in production without issue at least. It was my understanding that sssd needs to know the master map name so that it can expire it from the cache properly on updates, is that correct? So in sssd.conf you also have ldap_autofs_map_master_name = auto.slave, right?
Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to include the same master map called auto.slave.
Thanks again for your patience, much appreciated. ;-)
Thank you very much for the patch. Once the two additions to mobject and mentry are removed, I'll simply push the patch. I didn't mean to confuse you, sorry :)
On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote:
(Including once more as an attachment, it looks like something may have wrapped the lines when I had the patch in the body of the message.)
On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote:
> Hi Jakub, > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP
map
> changes during a merge, that is now included. And I've removed the > strdup(), whoops! ;-) > > cs >
Hi Cove,
this patch is working for me with a couple of additonal changes that I attach as a mini-patch. If you like, you can squash it into yours and resend, then I think the patch would be good to go.
Basically I think it's enough to have the master map name defined in the LDAP options, not the map and entry option lists. Also the option lists need the objectclass to be the first entry in the map.
Sorry, I think I confused you with my previous e-mail.
With the attached patch on top of yours, I was able to run automounter with a custom master map (MASTER_MAP_NAME="auto.slave" in /etc/sysconfig/autofs) just fine.
Thanks again for the contribution.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I would like to apploligize for late answer, but, I had few problems with autofs configuration.
From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 1 + src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 1 + src/providers/ldap/sdap.h | 1 + src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry>
<term>ldap_autofs_map_master_name (string)</term><listitem><para>The name of the automount master map in LDAP.</para><para>Default: auto.master</para></listitem></varlistentry></variablelist><variablelist><varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para>diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
/* If a request for auto.master comes in, the automounter deamon
* has been reloaded. Expire all autofs maps to force reload*/if (strcmp(be_autofs_req->mapname, "auto.master") == 0) {
be_autofs_req->invalidate = true;}
be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base;
const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
/* automount master map name */
master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME,AUTOFS_MAP_MASTER_NAME);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name"" to default value\n"));return ret;}master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);}
DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map));
/* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master"
/* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE,
- SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM,
diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req;
const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n"));
@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
- if (autofs_req->mapname != NULL) {
master_map = dp_opt_get_string(id_ctx->opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);^^^^^^^^^^I think, that function dp_opt_get_string can return NULL. It would be not good idea to compare NULL with autofs_req->mapname.
It shouldn't happen but yeah, famous last words :-)
I agree with Lukas, let's be extra paranoid and also add a NULL check here. If we ever get a NULL string, we'll just use stale data, nothing terrible.
It is not about to be extra paranoid. I hit this bug, but I forgot to attach back_trace in my previous mail.
Thank you for testing!
if (strcmp(master_map, autofs_req->mapname) == 0) {autofs_req->invalidate = true;DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n",autofs_req->mapname));}- }
- if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain);
-- 1.8.2.1
I attach back_trace for record. LS
On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote:
On (17/06/13 19:47), Jakub Hrozek wrote:
On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote:
On (03/06/13 22:41), Jakub Hrozek wrote:
On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote:
Hi Jakub,
Enclosed is the squashed patch.
No worries, re-reading the thread I think I missed a few things early on. You were suggesting adding an attribute to handle an alias on the master map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map would needed to have been updated(?).
Sorry, I can be very confusing sometimes. I was just pondering two different options -- one was modifying the default_basic_opts[] array which you did, the other was modifying the mobject and entry maps, which I realized was the worse option.
In the latest patch, there are still additions to rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should go away. Can you test the attached patch? It is your patch with those two extra lines removed. If it works for you, I'll run the patch by another engineer to be sure (I won't be able to push patch with my additions myself then) and push.
The modified patch is attached.
Also, automounter works fine with different master map names, we use it in production without issue at least. It was my understanding that sssd needs to know the master map name so that it can expire it from the cache properly on updates, is that correct? So in sssd.conf you also have ldap_autofs_map_master_name = auto.slave, right?
Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to include the same master map called auto.slave.
Thanks again for your patience, much appreciated. ;-)
Thank you very much for the patch. Once the two additions to mobject and mentry are removed, I'll simply push the patch. I didn't mean to confuse you, sorry :)
On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: > (Including once more as an attachment, it looks like something may have > wrapped the lines when I had the patch in the body of the message.) > > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: > > > Hi Jakub, > > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP map > > changes during a merge, that is now included. And I've removed the > > strdup(), whoops! ;-) > > > > cs > >
Hi Cove,
this patch is working for me with a couple of additonal changes that I attach as a mini-patch. If you like, you can squash it into yours and resend, then I think the patch would be good to go.
Basically I think it's enough to have the master map name defined in the LDAP options, not the map and entry option lists. Also the option lists need the objectclass to be the first entry in the map.
Sorry, I think I confused you with my previous e-mail.
With the attached patch on top of yours, I was able to run automounter with a custom master map (MASTER_MAP_NAME="auto.slave" in /etc/sysconfig/autofs) just fine.
Thanks again for the contribution.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I would like to apploligize for late answer, but, I had few problems with autofs configuration.
From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 1 + src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 1 + src/providers/ldap/sdap.h | 1 + src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry>
<term>ldap_autofs_map_master_name (string)</term><listitem><para>The name of the automount master map in LDAP.</para><para>Default: auto.master</para></listitem></varlistentry></variablelist><variablelist><varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para>diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
/* If a request for auto.master comes in, the automounter deamon
* has been reloaded. Expire all autofs maps to force reload*/if (strcmp(be_autofs_req->mapname, "auto.master") == 0) {
be_autofs_req->invalidate = true;}
be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base;
const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
/* automount master map name */
master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME,AUTOFS_MAP_MASTER_NAME);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name"" to default value\n"));return ret;}master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);}
DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map));
/* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master"
/* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE,
- SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM,
diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req;
const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n"));
@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
- if (autofs_req->mapname != NULL) {
master_map = dp_opt_get_string(id_ctx->opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);^^^^^^^^^^I think, that function dp_opt_get_string can return NULL. It would be not good idea to compare NULL with autofs_req->mapname.
It shouldn't happen but yeah, famous last words :-)
I agree with Lukas, let's be extra paranoid and also add a NULL check here. If we ever get a NULL string, we'll just use stale data, nothing terrible.
It is not about to be extra paranoid. I hit this bug, but I forgot to attach back_trace in my previous mail.
Thank you for testing!
if (strcmp(master_map, autofs_req->mapname) == 0) {autofs_req->invalidate = true;DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n",autofs_req->mapname));}- }
- if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain);
-- 1.8.2.1
I attach back_trace for record. LS
How did you get into this situation? I thought that the option defaulted to "auto.master" even in the map. If not, then I think it should.
On (18/06/13 11:15), Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote:
On (17/06/13 19:47), Jakub Hrozek wrote:
On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote:
On (03/06/13 22:41), Jakub Hrozek wrote:
On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote:
Hi Jakub,
Enclosed is the squashed patch.
No worries, re-reading the thread I think I missed a few things early on. You were suggesting adding an attribute to handle an alias on the master map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map would needed to have been updated(?).
Sorry, I can be very confusing sometimes. I was just pondering two different options -- one was modifying the default_basic_opts[] array which you did, the other was modifying the mobject and entry maps, which I realized was the worse option.
In the latest patch, there are still additions to rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should go away. Can you test the attached patch? It is your patch with those two extra lines removed. If it works for you, I'll run the patch by another engineer to be sure (I won't be able to push patch with my additions myself then) and push.
The modified patch is attached.
Also, automounter works fine with different master map names, we use it in production without issue at least. It was my understanding that sssd needs to know the master map name so that it can expire it from the cache properly on updates, is that correct? So in sssd.conf you also have ldap_autofs_map_master_name = auto.slave, right?
Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to include the same master map called auto.slave.
Thanks again for your patience, much appreciated. ;-)
Thank you very much for the patch. Once the two additions to mobject and mentry are removed, I'll simply push the patch. I didn't mean to confuse you, sorry :)
On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote:
> On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: > > (Including once more as an attachment, it looks like something may have > > wrapped the lines when I had the patch in the body of the message.) > > > > > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: > > > > > Hi Jakub, > > > > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP > map > > > changes during a merge, that is now included. And I've removed the > > > strdup(), whoops! ;-) > > > > > > cs > > > > > Hi Cove, > > this patch is working for me with a couple of additonal changes that I > attach as a mini-patch. If you like, you can squash it into yours and > resend, then I think the patch would be good to go. > > Basically I think it's enough to have the master map name defined in the > LDAP options, not the map and entry option lists. Also the option lists > need the objectclass to be the first entry in the map. > > Sorry, I think I confused you with my previous e-mail. > > With the attached patch on top of yours, I was able to run automounter > with a custom master map (MASTER_MAP_NAME="auto.slave" in > /etc/sysconfig/autofs) just fine. > > Thanks again for the contribution. > > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I would like to apploligize for late answer, but, I had few problems with autofs configuration.
From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 1 + src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 1 + src/providers/ldap/sdap.h | 1 + src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry>
<term>ldap_autofs_map_master_name (string)</term><listitem><para>The name of the automount master map in LDAP.</para><para>Default: auto.master</para></listitem></varlistentry></variablelist><variablelist><varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para>diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
/* If a request for auto.master comes in, the automounter deamon
* has been reloaded. Expire all autofs maps to force reload*/if (strcmp(be_autofs_req->mapname, "auto.master") == 0) {
be_autofs_req->invalidate = true;}
be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base;
const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
/* automount master map name */
master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME,AUTOFS_MAP_MASTER_NAME);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name"" to default value\n"));return ret;}master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);}
DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map));
/* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master"
/* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE,
- SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM,
diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req;
const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n"));
@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
- if (autofs_req->mapname != NULL) {
master_map = dp_opt_get_string(id_ctx->opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);^^^^^^^^^^I think, that function dp_opt_get_string can return NULL. It would be not good idea to compare NULL with autofs_req->mapname.
It shouldn't happen but yeah, famous last words :-)
I agree with Lukas, let's be extra paranoid and also add a NULL check here. If we ever get a NULL string, we'll just use stale data, nothing terrible.
It is not about to be extra paranoid. I hit this bug, but I forgot to attach back_trace in my previous mail.
Thank you for testing!
if (strcmp(master_map, autofs_req->mapname) == 0) {autofs_req->invalidate = true;DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n",autofs_req->mapname));}- }
- if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain);
-- 1.8.2.1
I attach back_trace for record. LS
How did you get into this situation? I thought that the option defaulted to "auto.master" even in the map. If not, then I think it should.
Reproducible: Always
Steps to reproduce: 1. service sssd stop 2. update sssd (sssd with patch) 3. remove cache and logs 4. service sssd start 5. servive autofs restart ^^^^ Result: sssd crashed.
Could be default value in "struct dp_option default_basic_opts[]" ?
Like { "ldap_default_authtok_type", DP_OPT_STRING, { "password" }, NULL_STRING},
LS
On Tue, Jun 18, 2013 at 05:00:17PM +0200, Lukas Slebodnik wrote:
On (18/06/13 11:15), Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote:
On (17/06/13 19:47), Jakub Hrozek wrote:
On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote:
On (03/06/13 22:41), Jakub Hrozek wrote:
On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote: > Hi Jakub, > > Enclosed is the squashed patch. > > No worries, re-reading the thread I think I missed a few things early on. > You were suggesting adding an attribute to handle an alias on the master > map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map > would needed to have been updated(?). >
Sorry, I can be very confusing sometimes. I was just pondering two different options -- one was modifying the default_basic_opts[] array which you did, the other was modifying the mobject and entry maps, which I realized was the worse option.
In the latest patch, there are still additions to rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should go away. Can you test the attached patch? It is your patch with those two extra lines removed. If it works for you, I'll run the patch by another engineer to be sure (I won't be able to push patch with my additions myself then) and push.
The modified patch is attached.
> Also, automounter works fine with different master map names, we use it in > production without issue at least. It was my understanding that sssd needs > to know the master map name so that it can expire it from the cache > properly on updates, is that correct? So in sssd.conf you also have > ldap_autofs_map_master_name = auto.slave, right? >
Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to include the same master map called auto.slave.
> Thanks again for your patience, much appreciated. ;-) >
Thank you very much for the patch. Once the two additions to mobject and mentry are removed, I'll simply push the patch. I didn't mean to confuse you, sorry :)
> > > > On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote: > > > On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: > > > (Including once more as an attachment, it looks like something may have > > > wrapped the lines when I had the patch in the body of the message.) > > > > > > > > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: > > > > > > > Hi Jakub, > > > > > > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP > > map > > > > changes during a merge, that is now included. And I've removed the > > > > strdup(), whoops! ;-) > > > > > > > > cs > > > > > > > > Hi Cove, > > > > this patch is working for me with a couple of additonal changes that I > > attach as a mini-patch. If you like, you can squash it into yours and > > resend, then I think the patch would be good to go. > > > > Basically I think it's enough to have the master map name defined in the > > LDAP options, not the map and entry option lists. Also the option lists > > need the objectclass to be the first entry in the map. > > > > Sorry, I think I confused you with my previous e-mail. > > > > With the attached patch on top of yours, I was able to run automounter > > with a custom master map (MASTER_MAP_NAME="auto.slave" in > > /etc/sysconfig/autofs) just fine. > > > > Thanks again for the contribution. > > > > _______________________________________________ > > sssd-devel mailing list > > sssd-devel@lists.fedorahosted.org > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > > > >
> _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I would like to apploligize for late answer, but, I had few problems with autofs configuration.
From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + src/db/sysdb_autofs.h | 5 +++-- src/man/sssd-ldap.5.xml | 13 +++++++++++++ src/providers/ad/ad_opts.h | 1 + src/providers/data_provider_be.c | 7 ------- src/providers/ipa/ipa_opts.h | 1 + src/providers/ldap/ldap_common.c | 15 +++++++++++++++ src/providers/ldap/ldap_common.h | 2 ++ src/providers/ldap/ldap_opts.h | 1 + src/providers/ldap/sdap.h | 1 + src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ 12 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false
[provider/ipa/autofs] ipa_automount_location = str, None, false +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false ldap_sudorule_order = str, None, false
[provider/ldap/autofs] +ldap_autofs_map_master_name = str, None, false ldap_autofs_map_object_class = str, None, false ldap_autofs_map_name = str, None, false ldap_autofs_entry_object_class = str, None, false diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -28,8 +28,9 @@ #define AUTOFS_MAP_SUBDIR "autofsmaps" #define AUTOFS_ENTRY_SUBDIR "autofsentries"
-#define SYSDB_AUTOFS_MAP_OC "automountMap" -#define SYSDB_AUTOFS_MAP_NAME "automountMapName" +#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" +#define SYSDB_AUTOFS_MAP_OC "automountMap" +#define SYSDB_AUTOFS_MAP_NAME "automountMapName"
#define SYSDB_AUTOFS_ENTRY_OC "automount" #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com <para> <variablelist> <varlistentry>
<term>ldap_autofs_map_master_name (string)</term><listitem><para>The name of the automount master map in LDAP.</para><para>Default: auto.master</para></listitem></varlistentry></variablelist><variablelist><varlistentry> <term>ldap_autofs_map_object_class (string)</term> <listitem> <para>diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 --- a/src/providers/ad/ad_opts.h +++ b/src/providers/ad/ad_opts.h @@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) goto done; }
/* If a request for auto.master comes in, the automounter deamon
* has been reloaded. Expire all autofs maps to force reload*/if (strcmp(be_autofs_req->mapname, "auto.master") == 0) {
be_autofs_req->invalidate = true;}
be_req->req_data = be_autofs_req;
if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 --- a/src/providers/ipa/ipa_opts.h +++ b/src/providers/ipa/ipa_opts.h @@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, struct sdap_options *opts) { const char *search_base;
const char *master_map; struct sdap_attr_map *default_entry_map; struct sdap_attr_map *default_mobject_map; int ret;
/* automount master map name */
master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);
if (master_map == NULL) {
ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME,AUTOFS_MAP_MASTER_NAME);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name"" to default value\n"));return ret;}master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME);}
DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map));
/* search base */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,6 +39,8 @@ #define LDAP_SSL_URI "ldaps://" #define LDAP_LDAPI_URI "ldapi://"
+#define AUTOFS_MAP_MASTER_NAME "auto.master"
/* a fd the child process would log into */ extern int ldap_child_debug_fd;
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 --- a/src/providers/ldap/ldap_opts.h +++ b/src/providers/ldap/ldap_opts.h @@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
- { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -164,6 +164,7 @@ enum sdap_basic_opt { SDAP_SUDO_INCLUDE_NETGROUPS, SDAP_SUDO_INCLUDE_REGEXP, SDAP_AUTOFS_SEARCH_BASE,
- SDAP_AUTOFS_MAP_MASTER_NAME, SDAP_SCHEMA, SDAP_OFFLINE_TIMEOUT, SDAP_FORCE_UPPER_CASE_REALM,
diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 --- a/src/providers/ldap/sdap_autofs.c +++ b/src/providers/ldap/sdap_autofs.c @@ -30,6 +30,7 @@ #include "providers/ldap/sdap.h" #include "providers/ldap/sdap_async.h" #include "providers/dp_backend.h" +#include "providers/data_provider.h" #include "db/sysdb_autofs.h" #include "util/util.h"
@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) struct sdap_id_ctx *id_ctx; struct be_autofs_req *autofs_req; struct tevent_req *req;
const char *master_map; int ret = EOK;
DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n"));
@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", autofs_req->mapname ? autofs_req->mapname : "<ALL>\n"));
- if (autofs_req->mapname != NULL) {
master_map = dp_opt_get_string(id_ctx->opts->basic,SDAP_AUTOFS_MAP_MASTER_NAME);^^^^^^^^^^I think, that function dp_opt_get_string can return NULL. It would be not good idea to compare NULL with autofs_req->mapname.
It shouldn't happen but yeah, famous last words :-)
I agree with Lukas, let's be extra paranoid and also add a NULL check here. If we ever get a NULL string, we'll just use stale data, nothing terrible.
It is not about to be extra paranoid. I hit this bug, but I forgot to attach back_trace in my previous mail.
Thank you for testing!
if (strcmp(master_map, autofs_req->mapname) == 0) {autofs_req->invalidate = true;DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n",autofs_req->mapname));}- }
- if (autofs_req->invalidate) { ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, id_ctx->be->domain);
-- 1.8.2.1
I attach back_trace for record. LS
How did you get into this situation? I thought that the option defaulted to "auto.master" even in the map. If not, then I think it should.
Reproducible: Always
Steps to reproduce:
- service sssd stop
- update sssd (sssd with patch)
- remove cache and logs
- service sssd start
- servive autofs restart ^^^^
Result: sssd crashed.
Could be default value in "struct dp_option default_basic_opts[]" ?
Like { "ldap_default_authtok_type", DP_OPT_STRING, { "password" }, NULL_STRING},
Yes, we should default to auto.master there.
Cove, would you like us to help you with the patch in any way?
On Wed, Jul 10, 2013 at 02:51:37PM +0200, Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 05:00:17PM +0200, Lukas Slebodnik wrote:
On (18/06/13 11:15), Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote:
On (17/06/13 19:47), Jakub Hrozek wrote:
On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote:
On (03/06/13 22:41), Jakub Hrozek wrote: >On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote: >> Hi Jakub, >> >> Enclosed is the squashed patch. >> >> No worries, re-reading the thread I think I missed a few things early on. >> You were suggesting adding an attribute to handle an alias on the master >> map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map >> would needed to have been updated(?). >> > >Sorry, I can be very confusing sometimes. I was just pondering two >different options -- one was modifying the default_basic_opts[] array >which you did, the other was modifying the mobject and entry maps, which >I realized was the worse option. > >In the latest patch, there are still additions to >rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should >go away. Can you test the attached patch? It is your patch with those >two extra lines removed. If it works for you, I'll run the patch by >another engineer to be sure (I won't be able to push patch with my >additions myself then) and push. > >The modified patch is attached. > >> Also, automounter works fine with different master map names, we use it in >> production without issue at least. It was my understanding that sssd needs >> to know the master map name so that it can expire it from the cache >> properly on updates, is that correct? So in sssd.conf you also have >> ldap_autofs_map_master_name = auto.slave, right? >> > >Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to >include the same master map called auto.slave. > >> Thanks again for your patience, much appreciated. ;-) >> > >Thank you very much for the patch. Once the two additions to mobject and >mentry are removed, I'll simply push the patch. I didn't mean to confuse >you, sorry :) > >> >> >> >> On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote: >> >> > On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: >> > > (Including once more as an attachment, it looks like something may have >> > > wrapped the lines when I had the patch in the body of the message.) >> > > >> > > >> > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: >> > > >> > > > Hi Jakub, >> > > > >> > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP >> > map >> > > > changes during a merge, that is now included. And I've removed the >> > > > strdup(), whoops! ;-) >> > > > >> > > > cs >> > > > >> > >> > Hi Cove, >> > >> > this patch is working for me with a couple of additonal changes that I >> > attach as a mini-patch. If you like, you can squash it into yours and >> > resend, then I think the patch would be good to go. >> > >> > Basically I think it's enough to have the master map name defined in the >> > LDAP options, not the map and entry option lists. Also the option lists >> > need the objectclass to be the first entry in the map. >> > >> > Sorry, I think I confused you with my previous e-mail. >> > >> > With the attached patch on top of yours, I was able to run automounter >> > with a custom master map (MASTER_MAP_NAME="auto.slave" in >> > /etc/sysconfig/autofs) just fine. >> > >> > Thanks again for the contribution. >> > >> > _______________________________________________ >> > sssd-devel mailing list >> > sssd-devel@lists.fedorahosted.org >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> > >> > > > >> _______________________________________________ >> sssd-devel mailing list >> sssd-devel@lists.fedorahosted.org >> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >
I would like to apploligize for late answer, but, I had few problems with autofs configuration.
>From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 >From: Cove Schneider cove@ilm.com >Date: Fri, 31 May 2013 23:56:48 -0700 >Subject: [PATCH] Add ldap_autofs_map_master_name option > >--- > src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + > src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + > src/db/sysdb_autofs.h | 5 +++-- > src/man/sssd-ldap.5.xml | 13 +++++++++++++ > src/providers/ad/ad_opts.h | 1 + > src/providers/data_provider_be.c | 7 ------- > src/providers/ipa/ipa_opts.h | 1 + > src/providers/ldap/ldap_common.c | 15 +++++++++++++++ > src/providers/ldap/ldap_common.h | 2 ++ > src/providers/ldap/ldap_opts.h | 1 + > src/providers/ldap/sdap.h | 1 + > src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ > 12 files changed, 51 insertions(+), 9 deletions(-) > >diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf >index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 >--- a/src/config/etc/sssd.api.d/sssd-ipa.conf >+++ b/src/config/etc/sssd.api.d/sssd-ipa.conf >@@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false > > [provider/ipa/autofs] > ipa_automount_location = str, None, false >+ldap_autofs_map_master_name = str, None, false > ldap_autofs_map_object_class = str, None, false > ldap_autofs_map_name = str, None, false > ldap_autofs_entry_object_class = str, None, false >diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf >index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 >--- a/src/config/etc/sssd.api.d/sssd-ldap.conf >+++ b/src/config/etc/sssd.api.d/sssd-ldap.conf >@@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false > ldap_sudorule_order = str, None, false > > [provider/ldap/autofs] >+ldap_autofs_map_master_name = str, None, false > ldap_autofs_map_object_class = str, None, false > ldap_autofs_map_name = str, None, false > ldap_autofs_entry_object_class = str, None, false >diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h >index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 >--- a/src/db/sysdb_autofs.h >+++ b/src/db/sysdb_autofs.h >@@ -28,8 +28,9 @@ > #define AUTOFS_MAP_SUBDIR "autofsmaps" > #define AUTOFS_ENTRY_SUBDIR "autofsentries" > >-#define SYSDB_AUTOFS_MAP_OC "automountMap" >-#define SYSDB_AUTOFS_MAP_NAME "automountMapName" >+#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" >+#define SYSDB_AUTOFS_MAP_OC "automountMap" >+#define SYSDB_AUTOFS_MAP_NAME "automountMapName" > > #define SYSDB_AUTOFS_ENTRY_OC "automount" > #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" >diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml >index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 >--- a/src/man/sssd-ldap.5.xml >+++ b/src/man/sssd-ldap.5.xml >@@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com > <para> > <variablelist> > <varlistentry> >+ <term>ldap_autofs_map_master_name (string)</term> >+ <listitem> >+ <para> >+ The name of the automount master map in LDAP. >+ </para> >+ <para> >+ Default: auto.master >+ </para> >+ </listitem> >+ </varlistentry> >+ </variablelist> >+ <variablelist> >+ <varlistentry> > <term>ldap_autofs_map_object_class (string)</term> > <listitem> > <para> >diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h >index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 >--- a/src/providers/ad/ad_opts.h >+++ b/src/providers/ad/ad_opts.h >@@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE }, >diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c >index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 >--- a/src/providers/data_provider_be.c >+++ b/src/providers/data_provider_be.c >@@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) > goto done; > } > >- /* If a request for auto.master comes in, the automounter deamon >- * has been reloaded. Expire all autofs maps to force reload >- */ >- if (strcmp(be_autofs_req->mapname, "auto.master") == 0) { >- be_autofs_req->invalidate = true; >- } >- > be_req->req_data = be_autofs_req; > > if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) { >diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h >index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 >--- a/src/providers/ipa/ipa_opts.h >+++ b/src/providers/ipa/ipa_opts.h >@@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c >index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 >--- a/src/providers/ldap/ldap_common.c >+++ b/src/providers/ldap/ldap_common.c >@@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, > struct sdap_options *opts) > { > const char *search_base; >+ const char *master_map; > struct sdap_attr_map *default_entry_map; > struct sdap_attr_map *default_mobject_map; > int ret; > >+ /* automount master map name */ >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); >+ if (master_map == NULL) { >+ ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, >+ AUTOFS_MAP_MASTER_NAME); >+ if (ret != EOK) { >+ DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name" >+ " to default value\n")); >+ return ret; >+ } >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); >+ } >+ DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map)); >+ > /* search base */ > search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); > if (search_base != NULL) { >diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h >index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 >--- a/src/providers/ldap/ldap_common.h >+++ b/src/providers/ldap/ldap_common.h >@@ -39,6 +39,8 @@ > #define LDAP_SSL_URI "ldaps://" > #define LDAP_LDAPI_URI "ldapi://" > >+#define AUTOFS_MAP_MASTER_NAME "auto.master" >+ > /* a fd the child process would log into */ > extern int ldap_child_debug_fd; > >diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h >index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 >--- a/src/providers/ldap/ldap_opts.h >+++ b/src/providers/ldap/ldap_opts.h >@@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, >diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h >index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 >--- a/src/providers/ldap/sdap.h >+++ b/src/providers/ldap/sdap.h >@@ -164,6 +164,7 @@ enum sdap_basic_opt { > SDAP_SUDO_INCLUDE_NETGROUPS, > SDAP_SUDO_INCLUDE_REGEXP, > SDAP_AUTOFS_SEARCH_BASE, >+ SDAP_AUTOFS_MAP_MASTER_NAME, > SDAP_SCHEMA, > SDAP_OFFLINE_TIMEOUT, > SDAP_FORCE_UPPER_CASE_REALM, >diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c >index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 >--- a/src/providers/ldap/sdap_autofs.c >+++ b/src/providers/ldap/sdap_autofs.c >@@ -30,6 +30,7 @@ > #include "providers/ldap/sdap.h" > #include "providers/ldap/sdap_async.h" > #include "providers/dp_backend.h" >+#include "providers/data_provider.h" > #include "db/sysdb_autofs.h" > #include "util/util.h" > >@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) > struct sdap_id_ctx *id_ctx; > struct be_autofs_req *autofs_req; > struct tevent_req *req; >+ const char *master_map; > int ret = EOK; > > DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n")); >@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) > DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", > autofs_req->mapname ? autofs_req->mapname : "<ALL>\n")); > >+ if (autofs_req->mapname != NULL) { >+ master_map = dp_opt_get_string(id_ctx->opts->basic, >+ SDAP_AUTOFS_MAP_MASTER_NAME); ^^^^^^^^^^ I think, that function dp_opt_get_string can return NULL. It would be not good idea to compare NULL with autofs_req->mapname.
It shouldn't happen but yeah, famous last words :-)
I agree with Lukas, let's be extra paranoid and also add a NULL check here. If we ever get a NULL string, we'll just use stale data, nothing terrible.
It is not about to be extra paranoid. I hit this bug, but I forgot to attach back_trace in my previous mail.
Thank you for testing!
>+ if (strcmp(master_map, autofs_req->mapname) == 0) { >+ autofs_req->invalidate = true; >+ DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n", >+ autofs_req->mapname)); >+ } >+ } >+ > if (autofs_req->invalidate) { > ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, > id_ctx->be->domain); >-- >1.8.2.1
I attach back_trace for record. LS
How did you get into this situation? I thought that the option defaulted to "auto.master" even in the map. If not, then I think it should.
Reproducible: Always
Steps to reproduce:
- service sssd stop
- update sssd (sssd with patch)
- remove cache and logs
- service sssd start
- servive autofs restart ^^^^
Result: sssd crashed.
Could be default value in "struct dp_option default_basic_opts[]" ?
Like { "ldap_default_authtok_type", DP_OPT_STRING, { "password" }, NULL_STRING},
Yes, we should default to auto.master there.
Cove, would you like us to help you with the patch in any way?
I'm cleaning up patchwork after last upstream release and attached is a patch with couple of coments I had (see 0001-review-fixes.patch) and also resulting patch with Cove's attribution.
On (04/11/13 12:11), Jakub Hrozek wrote:
On Wed, Jul 10, 2013 at 02:51:37PM +0200, Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 05:00:17PM +0200, Lukas Slebodnik wrote:
On (18/06/13 11:15), Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote:
On (17/06/13 19:47), Jakub Hrozek wrote:
On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote: > On (03/06/13 22:41), Jakub Hrozek wrote: > >On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote: > >> Hi Jakub, > >> > >> Enclosed is the squashed patch. > >> > >> No worries, re-reading the thread I think I missed a few things early on. > >> You were suggesting adding an attribute to handle an alias on the master > >> map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map > >> would needed to have been updated(?). > >> > > > >Sorry, I can be very confusing sometimes. I was just pondering two > >different options -- one was modifying the default_basic_opts[] array > >which you did, the other was modifying the mobject and entry maps, which > >I realized was the worse option. > > > >In the latest patch, there are still additions to > >rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should > >go away. Can you test the attached patch? It is your patch with those > >two extra lines removed. If it works for you, I'll run the patch by > >another engineer to be sure (I won't be able to push patch with my > >additions myself then) and push. > > > >The modified patch is attached. > > > >> Also, automounter works fine with different master map names, we use it in > >> production without issue at least. It was my understanding that sssd needs > >> to know the master map name so that it can expire it from the cache > >> properly on updates, is that correct? So in sssd.conf you also have > >> ldap_autofs_map_master_name = auto.slave, right? > >> > > > >Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to > >include the same master map called auto.slave. > > > >> Thanks again for your patience, much appreciated. ;-) > >> > > > >Thank you very much for the patch. Once the two additions to mobject and > >mentry are removed, I'll simply push the patch. I didn't mean to confuse > >you, sorry :) > > > >> > >> > >> > >> On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote: > >> > >> > On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: > >> > > (Including once more as an attachment, it looks like something may have > >> > > wrapped the lines when I had the patch in the body of the message.) > >> > > > >> > > > >> > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: > >> > > > >> > > > Hi Jakub, > >> > > > > >> > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP > >> > map > >> > > > changes during a merge, that is now included. And I've removed the > >> > > > strdup(), whoops! ;-) > >> > > > > >> > > > cs > >> > > > > >> > > >> > Hi Cove, > >> > > >> > this patch is working for me with a couple of additonal changes that I > >> > attach as a mini-patch. If you like, you can squash it into yours and > >> > resend, then I think the patch would be good to go. > >> > > >> > Basically I think it's enough to have the master map name defined in the > >> > LDAP options, not the map and entry option lists. Also the option lists > >> > need the objectclass to be the first entry in the map. > >> > > >> > Sorry, I think I confused you with my previous e-mail. > >> > > >> > With the attached patch on top of yours, I was able to run automounter > >> > with a custom master map (MASTER_MAP_NAME="auto.slave" in > >> > /etc/sysconfig/autofs) just fine. > >> > > >> > Thanks again for the contribution. > >> > > >> > _______________________________________________ > >> > sssd-devel mailing list > >> > sssd-devel@lists.fedorahosted.org > >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >> > > >> > > > > > > >> _______________________________________________ > >> sssd-devel mailing list > >> sssd-devel@lists.fedorahosted.org > >> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > > > > I would like to apploligize for late answer, but, I had few problems with > autofs configuration. > > >From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 > >From: Cove Schneider cove@ilm.com > >Date: Fri, 31 May 2013 23:56:48 -0700 > >Subject: [PATCH] Add ldap_autofs_map_master_name option > > > >--- > > src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + > > src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + > > src/db/sysdb_autofs.h | 5 +++-- > > src/man/sssd-ldap.5.xml | 13 +++++++++++++ > > src/providers/ad/ad_opts.h | 1 + > > src/providers/data_provider_be.c | 7 ------- > > src/providers/ipa/ipa_opts.h | 1 + > > src/providers/ldap/ldap_common.c | 15 +++++++++++++++ > > src/providers/ldap/ldap_common.h | 2 ++ > > src/providers/ldap/ldap_opts.h | 1 + > > src/providers/ldap/sdap.h | 1 + > > src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ > > 12 files changed, 51 insertions(+), 9 deletions(-) > > > >diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf > >index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 > >--- a/src/config/etc/sssd.api.d/sssd-ipa.conf > >+++ b/src/config/etc/sssd.api.d/sssd-ipa.conf > >@@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false > > > > [provider/ipa/autofs] > > ipa_automount_location = str, None, false > >+ldap_autofs_map_master_name = str, None, false > > ldap_autofs_map_object_class = str, None, false > > ldap_autofs_map_name = str, None, false > > ldap_autofs_entry_object_class = str, None, false > >diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf > >index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 > >--- a/src/config/etc/sssd.api.d/sssd-ldap.conf > >+++ b/src/config/etc/sssd.api.d/sssd-ldap.conf > >@@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false > > ldap_sudorule_order = str, None, false > > > > [provider/ldap/autofs] > >+ldap_autofs_map_master_name = str, None, false > > ldap_autofs_map_object_class = str, None, false > > ldap_autofs_map_name = str, None, false > > ldap_autofs_entry_object_class = str, None, false > >diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h > >index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 > >--- a/src/db/sysdb_autofs.h > >+++ b/src/db/sysdb_autofs.h > >@@ -28,8 +28,9 @@ > > #define AUTOFS_MAP_SUBDIR "autofsmaps" > > #define AUTOFS_ENTRY_SUBDIR "autofsentries" > > > >-#define SYSDB_AUTOFS_MAP_OC "automountMap" > >-#define SYSDB_AUTOFS_MAP_NAME "automountMapName" > >+#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" > >+#define SYSDB_AUTOFS_MAP_OC "automountMap" > >+#define SYSDB_AUTOFS_MAP_NAME "automountMapName" > > > > #define SYSDB_AUTOFS_ENTRY_OC "automount" > > #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" > >diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml > >index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 > >--- a/src/man/sssd-ldap.5.xml > >+++ b/src/man/sssd-ldap.5.xml > >@@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com > > <para> > > <variablelist> > > <varlistentry> > >+ <term>ldap_autofs_map_master_name (string)</term> > >+ <listitem> > >+ <para> > >+ The name of the automount master map in LDAP. > >+ </para> > >+ <para> > >+ Default: auto.master > >+ </para> > >+ </listitem> > >+ </varlistentry> > >+ </variablelist> > >+ <variablelist> > >+ <varlistentry> > > <term>ldap_autofs_map_object_class (string)</term> > > <listitem> > > <para> > >diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h > >index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 > >--- a/src/providers/ad/ad_opts.h > >+++ b/src/providers/ad/ad_opts.h > >@@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { > > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > > { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, > > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE }, > >diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c > >index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 > >--- a/src/providers/data_provider_be.c > >+++ b/src/providers/data_provider_be.c > >@@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) > > goto done; > > } > > > >- /* If a request for auto.master comes in, the automounter deamon > >- * has been reloaded. Expire all autofs maps to force reload > >- */ > >- if (strcmp(be_autofs_req->mapname, "auto.master") == 0) { > >- be_autofs_req->invalidate = true; > >- } > >- > > be_req->req_data = be_autofs_req; > > > > if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) { > >diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h > >index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 > >--- a/src/providers/ipa/ipa_opts.h > >+++ b/src/providers/ipa/ipa_opts.h > >@@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { > > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > > { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, > > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c > >index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 > >--- a/src/providers/ldap/ldap_common.c > >+++ b/src/providers/ldap/ldap_common.c > >@@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, > > struct sdap_options *opts) > > { > > const char *search_base; > >+ const char *master_map; > > struct sdap_attr_map *default_entry_map; > > struct sdap_attr_map *default_mobject_map; > > int ret; > > > >+ /* automount master map name */ > >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); > >+ if (master_map == NULL) { > >+ ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, > >+ AUTOFS_MAP_MASTER_NAME); > >+ if (ret != EOK) { > >+ DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name" > >+ " to default value\n")); > >+ return ret; > >+ } > >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); > >+ } > >+ DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map)); > >+ > > /* search base */ > > search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); > > if (search_base != NULL) { > >diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h > >index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 > >--- a/src/providers/ldap/ldap_common.h > >+++ b/src/providers/ldap/ldap_common.h > >@@ -39,6 +39,8 @@ > > #define LDAP_SSL_URI "ldaps://" > > #define LDAP_LDAPI_URI "ldapi://" > > > >+#define AUTOFS_MAP_MASTER_NAME "auto.master" > >+ > > /* a fd the child process would log into */ > > extern int ldap_child_debug_fd; > > > >diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h > >index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 > >--- a/src/providers/ldap/ldap_opts.h > >+++ b/src/providers/ldap/ldap_opts.h > >@@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { > > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > > { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, > > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, > >diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h > >index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 > >--- a/src/providers/ldap/sdap.h > >+++ b/src/providers/ldap/sdap.h > >@@ -164,6 +164,7 @@ enum sdap_basic_opt { > > SDAP_SUDO_INCLUDE_NETGROUPS, > > SDAP_SUDO_INCLUDE_REGEXP, > > SDAP_AUTOFS_SEARCH_BASE, > >+ SDAP_AUTOFS_MAP_MASTER_NAME, > > SDAP_SCHEMA, > > SDAP_OFFLINE_TIMEOUT, > > SDAP_FORCE_UPPER_CASE_REALM, > >diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c > >index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 > >--- a/src/providers/ldap/sdap_autofs.c > >+++ b/src/providers/ldap/sdap_autofs.c > >@@ -30,6 +30,7 @@ > > #include "providers/ldap/sdap.h" > > #include "providers/ldap/sdap_async.h" > > #include "providers/dp_backend.h" > >+#include "providers/data_provider.h" > > #include "db/sysdb_autofs.h" > > #include "util/util.h" > > > >@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) > > struct sdap_id_ctx *id_ctx; > > struct be_autofs_req *autofs_req; > > struct tevent_req *req; > >+ const char *master_map; > > int ret = EOK; > > > > DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n")); > >@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) > > DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", > > autofs_req->mapname ? autofs_req->mapname : "<ALL>\n")); > > > >+ if (autofs_req->mapname != NULL) { > >+ master_map = dp_opt_get_string(id_ctx->opts->basic, > >+ SDAP_AUTOFS_MAP_MASTER_NAME); > ^^^^^^^^^^ > I think, that function dp_opt_get_string can return NULL. > It would be not good idea to compare NULL with autofs_req->mapname.
It shouldn't happen but yeah, famous last words :-)
I agree with Lukas, let's be extra paranoid and also add a NULL check here. If we ever get a NULL string, we'll just use stale data, nothing terrible.
It is not about to be extra paranoid. I hit this bug, but I forgot to attach back_trace in my previous mail.
Thank you for testing!
> > >+ if (strcmp(master_map, autofs_req->mapname) == 0) { > >+ autofs_req->invalidate = true; > >+ DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n", > >+ autofs_req->mapname)); > >+ } > >+ } > >+ > > if (autofs_req->invalidate) { > > ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, > > id_ctx->be->domain); > >-- > >1.8.2.1
I attach back_trace for record. LS
How did you get into this situation? I thought that the option defaulted to "auto.master" even in the map. If not, then I think it should.
Reproducible: Always
Steps to reproduce:
- service sssd stop
- update sssd (sssd with patch)
- remove cache and logs
- service sssd start
- servive autofs restart ^^^^
Result: sssd crashed.
Could be default value in "struct dp_option default_basic_opts[]" ?
Like { "ldap_default_authtok_type", DP_OPT_STRING, { "password" }, NULL_STRING},
Yes, we should default to auto.master there.
Cove, would you like us to help you with the patch in any way?
I'm cleaning up patchwork after last upstream release and attached is a patch with couple of coments I had (see 0001-review-fixes.patch) and also resulting patch with Cove's attribution.
From 955f24c1ec3c690ea19483b52243287415d15112 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 4 Nov 2013 12:07:55 +0100 Subject: [PATCH] review fixes
I cannot apply this patch to master. There is a lot of conflicts with Cove's patch.
From ac674d2e4196da9ff749db8709e967289446e9cc Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
LS
On Tue, Nov 05, 2013 at 06:51:46PM +0100, Lukas Slebodnik wrote:
On (04/11/13 12:11), Jakub Hrozek wrote:
On Wed, Jul 10, 2013 at 02:51:37PM +0200, Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 05:00:17PM +0200, Lukas Slebodnik wrote:
On (18/06/13 11:15), Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote:
On (17/06/13 19:47), Jakub Hrozek wrote: >On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote: >> On (03/06/13 22:41), Jakub Hrozek wrote: >> >On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote: >> >> Hi Jakub, >> >> >> >> Enclosed is the squashed patch. >> >> >> >> No worries, re-reading the thread I think I missed a few things early on. >> >> You were suggesting adding an attribute to handle an alias on the master >> >> map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map >> >> would needed to have been updated(?). >> >> >> > >> >Sorry, I can be very confusing sometimes. I was just pondering two >> >different options -- one was modifying the default_basic_opts[] array >> >which you did, the other was modifying the mobject and entry maps, which >> >I realized was the worse option. >> > >> >In the latest patch, there are still additions to >> >rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should >> >go away. Can you test the attached patch? It is your patch with those >> >two extra lines removed. If it works for you, I'll run the patch by >> >another engineer to be sure (I won't be able to push patch with my >> >additions myself then) and push. >> > >> >The modified patch is attached. >> > >> >> Also, automounter works fine with different master map names, we use it in >> >> production without issue at least. It was my understanding that sssd needs >> >> to know the master map name so that it can expire it from the cache >> >> properly on updates, is that correct? So in sssd.conf you also have >> >> ldap_autofs_map_master_name = auto.slave, right? >> >> >> > >> >Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to >> >include the same master map called auto.slave. >> > >> >> Thanks again for your patience, much appreciated. ;-) >> >> >> > >> >Thank you very much for the patch. Once the two additions to mobject and >> >mentry are removed, I'll simply push the patch. I didn't mean to confuse >> >you, sorry :) >> > >> >> >> >> >> >> >> >> On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote: >> >> >> >> > On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: >> >> > > (Including once more as an attachment, it looks like something may have >> >> > > wrapped the lines when I had the patch in the body of the message.) >> >> > > >> >> > > >> >> > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: >> >> > > >> >> > > > Hi Jakub, >> >> > > > >> >> > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP >> >> > map >> >> > > > changes during a merge, that is now included. And I've removed the >> >> > > > strdup(), whoops! ;-) >> >> > > > >> >> > > > cs >> >> > > > >> >> > >> >> > Hi Cove, >> >> > >> >> > this patch is working for me with a couple of additonal changes that I >> >> > attach as a mini-patch. If you like, you can squash it into yours and >> >> > resend, then I think the patch would be good to go. >> >> > >> >> > Basically I think it's enough to have the master map name defined in the >> >> > LDAP options, not the map and entry option lists. Also the option lists >> >> > need the objectclass to be the first entry in the map. >> >> > >> >> > Sorry, I think I confused you with my previous e-mail. >> >> > >> >> > With the attached patch on top of yours, I was able to run automounter >> >> > with a custom master map (MASTER_MAP_NAME="auto.slave" in >> >> > /etc/sysconfig/autofs) just fine. >> >> > >> >> > Thanks again for the contribution. >> >> > >> >> > _______________________________________________ >> >> > sssd-devel mailing list >> >> > sssd-devel@lists.fedorahosted.org >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> >> > >> >> > >> > >> > >> >> _______________________________________________ >> >> sssd-devel mailing list >> >> sssd-devel@lists.fedorahosted.org >> >> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> > >> >> I would like to apploligize for late answer, but, I had few problems with >> autofs configuration. >> >> >From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 >> >From: Cove Schneider cove@ilm.com >> >Date: Fri, 31 May 2013 23:56:48 -0700 >> >Subject: [PATCH] Add ldap_autofs_map_master_name option >> > >> >--- >> > src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + >> > src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + >> > src/db/sysdb_autofs.h | 5 +++-- >> > src/man/sssd-ldap.5.xml | 13 +++++++++++++ >> > src/providers/ad/ad_opts.h | 1 + >> > src/providers/data_provider_be.c | 7 ------- >> > src/providers/ipa/ipa_opts.h | 1 + >> > src/providers/ldap/ldap_common.c | 15 +++++++++++++++ >> > src/providers/ldap/ldap_common.h | 2 ++ >> > src/providers/ldap/ldap_opts.h | 1 + >> > src/providers/ldap/sdap.h | 1 + >> > src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ >> > 12 files changed, 51 insertions(+), 9 deletions(-) >> > >> >diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf >> >index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 >> >--- a/src/config/etc/sssd.api.d/sssd-ipa.conf >> >+++ b/src/config/etc/sssd.api.d/sssd-ipa.conf >> >@@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false >> > >> > [provider/ipa/autofs] >> > ipa_automount_location = str, None, false >> >+ldap_autofs_map_master_name = str, None, false >> > ldap_autofs_map_object_class = str, None, false >> > ldap_autofs_map_name = str, None, false >> > ldap_autofs_entry_object_class = str, None, false >> >diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf >> >index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 >> >--- a/src/config/etc/sssd.api.d/sssd-ldap.conf >> >+++ b/src/config/etc/sssd.api.d/sssd-ldap.conf >> >@@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false >> > ldap_sudorule_order = str, None, false >> > >> > [provider/ldap/autofs] >> >+ldap_autofs_map_master_name = str, None, false >> > ldap_autofs_map_object_class = str, None, false >> > ldap_autofs_map_name = str, None, false >> > ldap_autofs_entry_object_class = str, None, false >> >diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h >> >index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 >> >--- a/src/db/sysdb_autofs.h >> >+++ b/src/db/sysdb_autofs.h >> >@@ -28,8 +28,9 @@ >> > #define AUTOFS_MAP_SUBDIR "autofsmaps" >> > #define AUTOFS_ENTRY_SUBDIR "autofsentries" >> > >> >-#define SYSDB_AUTOFS_MAP_OC "automountMap" >> >-#define SYSDB_AUTOFS_MAP_NAME "automountMapName" >> >+#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" >> >+#define SYSDB_AUTOFS_MAP_OC "automountMap" >> >+#define SYSDB_AUTOFS_MAP_NAME "automountMapName" >> > >> > #define SYSDB_AUTOFS_ENTRY_OC "automount" >> > #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" >> >diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml >> >index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 >> >--- a/src/man/sssd-ldap.5.xml >> >+++ b/src/man/sssd-ldap.5.xml >> >@@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com >> > <para> >> > <variablelist> >> > <varlistentry> >> >+ <term>ldap_autofs_map_master_name (string)</term> >> >+ <listitem> >> >+ <para> >> >+ The name of the automount master map in LDAP. >> >+ </para> >> >+ <para> >> >+ Default: auto.master >> >+ </para> >> >+ </listitem> >> >+ </varlistentry> >> >+ </variablelist> >> >+ <variablelist> >> >+ <varlistentry> >> > <term>ldap_autofs_map_object_class (string)</term> >> > <listitem> >> > <para> >> >diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h >> >index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 >> >--- a/src/providers/ad/ad_opts.h >> >+++ b/src/providers/ad/ad_opts.h >> >@@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> > { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE }, >> >diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c >> >index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 >> >--- a/src/providers/data_provider_be.c >> >+++ b/src/providers/data_provider_be.c >> >@@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) >> > goto done; >> > } >> > >> >- /* If a request for auto.master comes in, the automounter deamon >> >- * has been reloaded. Expire all autofs maps to force reload >> >- */ >> >- if (strcmp(be_autofs_req->mapname, "auto.master") == 0) { >> >- be_autofs_req->invalidate = true; >> >- } >> >- >> > be_req->req_data = be_autofs_req; >> > >> > if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) { >> >diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h >> >index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 >> >--- a/src/providers/ipa/ipa_opts.h >> >+++ b/src/providers/ipa/ipa_opts.h >> >@@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> > { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c >> >index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 >> >--- a/src/providers/ldap/ldap_common.c >> >+++ b/src/providers/ldap/ldap_common.c >> >@@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, >> > struct sdap_options *opts) >> > { >> > const char *search_base; >> >+ const char *master_map; >> > struct sdap_attr_map *default_entry_map; >> > struct sdap_attr_map *default_mobject_map; >> > int ret; >> > >> >+ /* automount master map name */ >> >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); >> >+ if (master_map == NULL) { >> >+ ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, >> >+ AUTOFS_MAP_MASTER_NAME); >> >+ if (ret != EOK) { >> >+ DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name" >> >+ " to default value\n")); >> >+ return ret; >> >+ } >> >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); >> >+ } >> >+ DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map)); >> >+ >> > /* search base */ >> > search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); >> > if (search_base != NULL) { >> >diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h >> >index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 >> >--- a/src/providers/ldap/ldap_common.h >> >+++ b/src/providers/ldap/ldap_common.h >> >@@ -39,6 +39,8 @@ >> > #define LDAP_SSL_URI "ldaps://" >> > #define LDAP_LDAPI_URI "ldapi://" >> > >> >+#define AUTOFS_MAP_MASTER_NAME "auto.master" >> >+ >> > /* a fd the child process would log into */ >> > extern int ldap_child_debug_fd; >> > >> >diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h >> >index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 >> >--- a/src/providers/ldap/ldap_opts.h >> >+++ b/src/providers/ldap/ldap_opts.h >> >@@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> > { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, >> >diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h >> >index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 >> >--- a/src/providers/ldap/sdap.h >> >+++ b/src/providers/ldap/sdap.h >> >@@ -164,6 +164,7 @@ enum sdap_basic_opt { >> > SDAP_SUDO_INCLUDE_NETGROUPS, >> > SDAP_SUDO_INCLUDE_REGEXP, >> > SDAP_AUTOFS_SEARCH_BASE, >> >+ SDAP_AUTOFS_MAP_MASTER_NAME, >> > SDAP_SCHEMA, >> > SDAP_OFFLINE_TIMEOUT, >> > SDAP_FORCE_UPPER_CASE_REALM, >> >diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c >> >index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 >> >--- a/src/providers/ldap/sdap_autofs.c >> >+++ b/src/providers/ldap/sdap_autofs.c >> >@@ -30,6 +30,7 @@ >> > #include "providers/ldap/sdap.h" >> > #include "providers/ldap/sdap_async.h" >> > #include "providers/dp_backend.h" >> >+#include "providers/data_provider.h" >> > #include "db/sysdb_autofs.h" >> > #include "util/util.h" >> > >> >@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) >> > struct sdap_id_ctx *id_ctx; >> > struct be_autofs_req *autofs_req; >> > struct tevent_req *req; >> >+ const char *master_map; >> > int ret = EOK; >> > >> > DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n")); >> >@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) >> > DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", >> > autofs_req->mapname ? autofs_req->mapname : "<ALL>\n")); >> > >> >+ if (autofs_req->mapname != NULL) { >> >+ master_map = dp_opt_get_string(id_ctx->opts->basic, >> >+ SDAP_AUTOFS_MAP_MASTER_NAME); >> ^^^^^^^^^^ >> I think, that function dp_opt_get_string can return NULL. >> It would be not good idea to compare NULL with autofs_req->mapname. > >It shouldn't happen but yeah, famous last words :-) > >I agree with Lukas, let's be extra paranoid and also add a NULL check >here. If we ever get a NULL string, we'll just use stale data, nothing >terrible. It is not about to be extra paranoid. I hit this bug, but I forgot to attach back_trace in my previous mail.
> >Thank you for testing! > >> >> >+ if (strcmp(master_map, autofs_req->mapname) == 0) { >> >+ autofs_req->invalidate = true; >> >+ DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n", >> >+ autofs_req->mapname)); >> >+ } >> >+ } >> >+ >> > if (autofs_req->invalidate) { >> > ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, >> > id_ctx->be->domain); >> >-- >> >1.8.2.1
I attach back_trace for record. LS
How did you get into this situation? I thought that the option defaulted to "auto.master" even in the map. If not, then I think it should.
Reproducible: Always
Steps to reproduce:
- service sssd stop
- update sssd (sssd with patch)
- remove cache and logs
- service sssd start
- servive autofs restart ^^^^
Result: sssd crashed.
Could be default value in "struct dp_option default_basic_opts[]" ?
Like { "ldap_default_authtok_type", DP_OPT_STRING, { "password" }, NULL_STRING},
Yes, we should default to auto.master there.
Cove, would you like us to help you with the patch in any way?
I'm cleaning up patchwork after last upstream release and attached is a patch with couple of coments I had (see 0001-review-fixes.patch) and also resulting patch with Cove's attribution.
From 955f24c1ec3c690ea19483b52243287415d15112 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 4 Nov 2013 12:07:55 +0100 Subject: [PATCH] review fixes
I cannot apply this patch to master. There is a lot of conflicts with Cove's patch.
Aah, sorry, this patch is not intended to be *applied*, it is just an interdiff between Cove's original patch and the one I sent..
From ac674d2e4196da9ff749db8709e967289446e9cc Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
This one should be applied.
On (06/11/13 13:46), Jakub Hrozek wrote:
On Tue, Nov 05, 2013 at 06:51:46PM +0100, Lukas Slebodnik wrote:
On (04/11/13 12:11), Jakub Hrozek wrote:
On Wed, Jul 10, 2013 at 02:51:37PM +0200, Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 05:00:17PM +0200, Lukas Slebodnik wrote:
On (18/06/13 11:15), Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote: > On (17/06/13 19:47), Jakub Hrozek wrote: > >On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote: > >> On (03/06/13 22:41), Jakub Hrozek wrote: > >> >On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote: > >> >> Hi Jakub, > >> >> > >> >> Enclosed is the squashed patch. > >> >> > >> >> No worries, re-reading the thread I think I missed a few things early on. > >> >> You were suggesting adding an attribute to handle an alias on the master > >> >> map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map > >> >> would needed to have been updated(?). > >> >> > >> > > >> >Sorry, I can be very confusing sometimes. I was just pondering two > >> >different options -- one was modifying the default_basic_opts[] array > >> >which you did, the other was modifying the mobject and entry maps, which > >> >I realized was the worse option. > >> > > >> >In the latest patch, there are still additions to > >> >rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should > >> >go away. Can you test the attached patch? It is your patch with those > >> >two extra lines removed. If it works for you, I'll run the patch by > >> >another engineer to be sure (I won't be able to push patch with my > >> >additions myself then) and push. > >> > > >> >The modified patch is attached. > >> > > >> >> Also, automounter works fine with different master map names, we use it in > >> >> production without issue at least. It was my understanding that sssd needs > >> >> to know the master map name so that it can expire it from the cache > >> >> properly on updates, is that correct? So in sssd.conf you also have > >> >> ldap_autofs_map_master_name = auto.slave, right? > >> >> > >> > > >> >Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to > >> >include the same master map called auto.slave. > >> > > >> >> Thanks again for your patience, much appreciated. ;-) > >> >> > >> > > >> >Thank you very much for the patch. Once the two additions to mobject and > >> >mentry are removed, I'll simply push the patch. I didn't mean to confuse > >> >you, sorry :) > >> > > >> >> > >> >> > >> >> > >> >> On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote: > >> >> > >> >> > On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: > >> >> > > (Including once more as an attachment, it looks like something may have > >> >> > > wrapped the lines when I had the patch in the body of the message.) > >> >> > > > >> >> > > > >> >> > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: > >> >> > > > >> >> > > > Hi Jakub, > >> >> > > > > >> >> > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP > >> >> > map > >> >> > > > changes during a merge, that is now included. And I've removed the > >> >> > > > strdup(), whoops! ;-) > >> >> > > > > >> >> > > > cs > >> >> > > > > >> >> > > >> >> > Hi Cove, > >> >> > > >> >> > this patch is working for me with a couple of additonal changes that I > >> >> > attach as a mini-patch. If you like, you can squash it into yours and > >> >> > resend, then I think the patch would be good to go. > >> >> > > >> >> > Basically I think it's enough to have the master map name defined in the > >> >> > LDAP options, not the map and entry option lists. Also the option lists > >> >> > need the objectclass to be the first entry in the map. > >> >> > > >> >> > Sorry, I think I confused you with my previous e-mail. > >> >> > > >> >> > With the attached patch on top of yours, I was able to run automounter > >> >> > with a custom master map (MASTER_MAP_NAME="auto.slave" in > >> >> > /etc/sysconfig/autofs) just fine. > >> >> > > >> >> > Thanks again for the contribution. > >> >> > > >> >> > _______________________________________________ > >> >> > sssd-devel mailing list > >> >> > sssd-devel@lists.fedorahosted.org > >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >> >> > > >> >> > > >> > > >> > > >> >> _______________________________________________ > >> >> sssd-devel mailing list > >> >> sssd-devel@lists.fedorahosted.org > >> >> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >> > > >> > >> I would like to apploligize for late answer, but, I had few problems with > >> autofs configuration. > >> > >> >From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 > >> >From: Cove Schneider cove@ilm.com > >> >Date: Fri, 31 May 2013 23:56:48 -0700 > >> >Subject: [PATCH] Add ldap_autofs_map_master_name option > >> > > >> >--- > >> > src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + > >> > src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + > >> > src/db/sysdb_autofs.h | 5 +++-- > >> > src/man/sssd-ldap.5.xml | 13 +++++++++++++ > >> > src/providers/ad/ad_opts.h | 1 + > >> > src/providers/data_provider_be.c | 7 ------- > >> > src/providers/ipa/ipa_opts.h | 1 + > >> > src/providers/ldap/ldap_common.c | 15 +++++++++++++++ > >> > src/providers/ldap/ldap_common.h | 2 ++ > >> > src/providers/ldap/ldap_opts.h | 1 + > >> > src/providers/ldap/sdap.h | 1 + > >> > src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ > >> > 12 files changed, 51 insertions(+), 9 deletions(-) > >> > > >> >diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf > >> >index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 > >> >--- a/src/config/etc/sssd.api.d/sssd-ipa.conf > >> >+++ b/src/config/etc/sssd.api.d/sssd-ipa.conf > >> >@@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false > >> > > >> > [provider/ipa/autofs] > >> > ipa_automount_location = str, None, false > >> >+ldap_autofs_map_master_name = str, None, false > >> > ldap_autofs_map_object_class = str, None, false > >> > ldap_autofs_map_name = str, None, false > >> > ldap_autofs_entry_object_class = str, None, false > >> >diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf > >> >index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 > >> >--- a/src/config/etc/sssd.api.d/sssd-ldap.conf > >> >+++ b/src/config/etc/sssd.api.d/sssd-ldap.conf > >> >@@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false > >> > ldap_sudorule_order = str, None, false > >> > > >> > [provider/ldap/autofs] > >> >+ldap_autofs_map_master_name = str, None, false > >> > ldap_autofs_map_object_class = str, None, false > >> > ldap_autofs_map_name = str, None, false > >> > ldap_autofs_entry_object_class = str, None, false > >> >diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h > >> >index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 > >> >--- a/src/db/sysdb_autofs.h > >> >+++ b/src/db/sysdb_autofs.h > >> >@@ -28,8 +28,9 @@ > >> > #define AUTOFS_MAP_SUBDIR "autofsmaps" > >> > #define AUTOFS_ENTRY_SUBDIR "autofsentries" > >> > > >> >-#define SYSDB_AUTOFS_MAP_OC "automountMap" > >> >-#define SYSDB_AUTOFS_MAP_NAME "automountMapName" > >> >+#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" > >> >+#define SYSDB_AUTOFS_MAP_OC "automountMap" > >> >+#define SYSDB_AUTOFS_MAP_NAME "automountMapName" > >> > > >> > #define SYSDB_AUTOFS_ENTRY_OC "automount" > >> > #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" > >> >diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml > >> >index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 > >> >--- a/src/man/sssd-ldap.5.xml > >> >+++ b/src/man/sssd-ldap.5.xml > >> >@@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com > >> > <para> > >> > <variablelist> > >> > <varlistentry> > >> >+ <term>ldap_autofs_map_master_name (string)</term> > >> >+ <listitem> > >> >+ <para> > >> >+ The name of the automount master map in LDAP. > >> >+ </para> > >> >+ <para> > >> >+ Default: auto.master > >> >+ </para> > >> >+ </listitem> > >> >+ </varlistentry> > >> >+ </variablelist> > >> >+ <variablelist> > >> >+ <varlistentry> > >> > <term>ldap_autofs_map_object_class (string)</term> > >> > <listitem> > >> > <para> > >> >diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h > >> >index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 > >> >--- a/src/providers/ad/ad_opts.h > >> >+++ b/src/providers/ad/ad_opts.h > >> >@@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { > >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >> > { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, > >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE }, > >> >diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c > >> >index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 > >> >--- a/src/providers/data_provider_be.c > >> >+++ b/src/providers/data_provider_be.c > >> >@@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) > >> > goto done; > >> > } > >> > > >> >- /* If a request for auto.master comes in, the automounter deamon > >> >- * has been reloaded. Expire all autofs maps to force reload > >> >- */ > >> >- if (strcmp(be_autofs_req->mapname, "auto.master") == 0) { > >> >- be_autofs_req->invalidate = true; > >> >- } > >> >- > >> > be_req->req_data = be_autofs_req; > >> > > >> > if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) { > >> >diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h > >> >index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 > >> >--- a/src/providers/ipa/ipa_opts.h > >> >+++ b/src/providers/ipa/ipa_opts.h > >> >@@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { > >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >> > { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, > >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >> >diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c > >> >index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 > >> >--- a/src/providers/ldap/ldap_common.c > >> >+++ b/src/providers/ldap/ldap_common.c > >> >@@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, > >> > struct sdap_options *opts) > >> > { > >> > const char *search_base; > >> >+ const char *master_map; > >> > struct sdap_attr_map *default_entry_map; > >> > struct sdap_attr_map *default_mobject_map; > >> > int ret; > >> > > >> >+ /* automount master map name */ > >> >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); > >> >+ if (master_map == NULL) { > >> >+ ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, > >> >+ AUTOFS_MAP_MASTER_NAME); > >> >+ if (ret != EOK) { > >> >+ DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name" > >> >+ " to default value\n")); > >> >+ return ret; > >> >+ } > >> >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); > >> >+ } > >> >+ DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map)); > >> >+ > >> > /* search base */ > >> > search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); > >> > if (search_base != NULL) { > >> >diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h > >> >index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 > >> >--- a/src/providers/ldap/ldap_common.h > >> >+++ b/src/providers/ldap/ldap_common.h > >> >@@ -39,6 +39,8 @@ > >> > #define LDAP_SSL_URI "ldaps://" > >> > #define LDAP_LDAPI_URI "ldapi://" > >> > > >> >+#define AUTOFS_MAP_MASTER_NAME "auto.master" > >> >+ > >> > /* a fd the child process would log into */ > >> > extern int ldap_child_debug_fd; > >> > > >> >diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h > >> >index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 > >> >--- a/src/providers/ldap/ldap_opts.h > >> >+++ b/src/providers/ldap/ldap_opts.h > >> >@@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { > >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, > >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, > >> > { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, > >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, > >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, > >> >diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h > >> >index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 > >> >--- a/src/providers/ldap/sdap.h > >> >+++ b/src/providers/ldap/sdap.h > >> >@@ -164,6 +164,7 @@ enum sdap_basic_opt { > >> > SDAP_SUDO_INCLUDE_NETGROUPS, > >> > SDAP_SUDO_INCLUDE_REGEXP, > >> > SDAP_AUTOFS_SEARCH_BASE, > >> >+ SDAP_AUTOFS_MAP_MASTER_NAME, > >> > SDAP_SCHEMA, > >> > SDAP_OFFLINE_TIMEOUT, > >> > SDAP_FORCE_UPPER_CASE_REALM, > >> >diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c > >> >index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 > >> >--- a/src/providers/ldap/sdap_autofs.c > >> >+++ b/src/providers/ldap/sdap_autofs.c > >> >@@ -30,6 +30,7 @@ > >> > #include "providers/ldap/sdap.h" > >> > #include "providers/ldap/sdap_async.h" > >> > #include "providers/dp_backend.h" > >> >+#include "providers/data_provider.h" > >> > #include "db/sysdb_autofs.h" > >> > #include "util/util.h" > >> > > >> >@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) > >> > struct sdap_id_ctx *id_ctx; > >> > struct be_autofs_req *autofs_req; > >> > struct tevent_req *req; > >> >+ const char *master_map; > >> > int ret = EOK; > >> > > >> > DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n")); > >> >@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) > >> > DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", > >> > autofs_req->mapname ? autofs_req->mapname : "<ALL>\n")); > >> > > >> >+ if (autofs_req->mapname != NULL) { > >> >+ master_map = dp_opt_get_string(id_ctx->opts->basic, > >> >+ SDAP_AUTOFS_MAP_MASTER_NAME); > >> ^^^^^^^^^^ > >> I think, that function dp_opt_get_string can return NULL. > >> It would be not good idea to compare NULL with autofs_req->mapname. > > > >It shouldn't happen but yeah, famous last words :-) > > > >I agree with Lukas, let's be extra paranoid and also add a NULL check > >here. If we ever get a NULL string, we'll just use stale data, nothing > >terrible. > It is not about to be extra paranoid. I hit this bug, but > I forgot to attach back_trace in my previous mail. > > > > >Thank you for testing! > > > >> > >> >+ if (strcmp(master_map, autofs_req->mapname) == 0) { > >> >+ autofs_req->invalidate = true; > >> >+ DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n", > >> >+ autofs_req->mapname)); > >> >+ } > >> >+ } > >> >+ > >> > if (autofs_req->invalidate) { > >> > ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, > >> > id_ctx->be->domain); > >> >-- > >> >1.8.2.1 > > I attach back_trace for record. > LS
How did you get into this situation? I thought that the option defaulted to "auto.master" even in the map. If not, then I think it should.
Reproducible: Always
Steps to reproduce:
- service sssd stop
- update sssd (sssd with patch)
- remove cache and logs
- service sssd start
- servive autofs restart ^^^^
Result: sssd crashed.
Could be default value in "struct dp_option default_basic_opts[]" ?
Like { "ldap_default_authtok_type", DP_OPT_STRING, { "password" }, NULL_STRING},
Yes, we should default to auto.master there.
Cove, would you like us to help you with the patch in any way?
I'm cleaning up patchwork after last upstream release and attached is a patch with couple of coments I had (see 0001-review-fixes.patch) and also resulting patch with Cove's attribution.
From 955f24c1ec3c690ea19483b52243287415d15112 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 4 Nov 2013 12:07:55 +0100 Subject: [PATCH] review fixes
I cannot apply this patch to master. There is a lot of conflicts with Cove's patch.
Aah, sorry, this patch is not intended to be *applied*, it is just an interdiff between Cove's original patch and the one I sent..
From ac674d2e4196da9ff749db8709e967289446e9cc Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
This one should be applied.
autofs works fine. There is no crash.
ACK
LS
On Mon, Nov 11, 2013 at 12:25:34PM +0100, Lukas Slebodnik wrote:
On (06/11/13 13:46), Jakub Hrozek wrote:
On Tue, Nov 05, 2013 at 06:51:46PM +0100, Lukas Slebodnik wrote:
On (04/11/13 12:11), Jakub Hrozek wrote:
On Wed, Jul 10, 2013 at 02:51:37PM +0200, Jakub Hrozek wrote:
On Tue, Jun 18, 2013 at 05:00:17PM +0200, Lukas Slebodnik wrote:
On (18/06/13 11:15), Jakub Hrozek wrote: >On Tue, Jun 18, 2013 at 10:29:36AM +0200, Lukas Slebodnik wrote: >> On (17/06/13 19:47), Jakub Hrozek wrote: >> >On Mon, Jun 17, 2013 at 05:28:17PM +0200, Lukas Slebodnik wrote: >> >> On (03/06/13 22:41), Jakub Hrozek wrote: >> >> >On Sat, Jun 01, 2013 at 12:30:47AM -0700, C. S. wrote: >> >> >> Hi Jakub, >> >> >> >> >> >> Enclosed is the squashed patch. >> >> >> >> >> >> No worries, re-reading the thread I think I missed a few things early on. >> >> >> You were suggesting adding an attribute to handle an alias on the master >> >> >> map, which is why rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map >> >> >> would needed to have been updated(?). >> >> >> >> >> > >> >> >Sorry, I can be very confusing sometimes. I was just pondering two >> >> >different options -- one was modifying the default_basic_opts[] array >> >> >which you did, the other was modifying the mobject and entry maps, which >> >> >I realized was the worse option. >> >> > >> >> >In the latest patch, there are still additions to >> >> >rfc2307_autofs_mobject_map and rfc2307bis_autofs_entry_map which should >> >> >go away. Can you test the attached patch? It is your patch with those >> >> >two extra lines removed. If it works for you, I'll run the patch by >> >> >another engineer to be sure (I won't be able to push patch with my >> >> >additions myself then) and push. >> >> > >> >> >The modified patch is attached. >> >> > >> >> >> Also, automounter works fine with different master map names, we use it in >> >> >> production without issue at least. It was my understanding that sssd needs >> >> >> to know the master map name so that it can expire it from the cache >> >> >> properly on updates, is that correct? So in sssd.conf you also have >> >> >> ldap_autofs_map_master_name = auto.slave, right? >> >> >> >> >> > >> >> >Yes, correct. I modified both /etc/sysconfig/autofs and sssd.conf to >> >> >include the same master map called auto.slave. >> >> > >> >> >> Thanks again for your patience, much appreciated. ;-) >> >> >> >> >> > >> >> >Thank you very much for the patch. Once the two additions to mobject and >> >> >mentry are removed, I'll simply push the patch. I didn't mean to confuse >> >> >you, sorry :) >> >> > >> >> >> >> >> >> >> >> >> >> >> >> On Thu, May 30, 2013 at 9:44 AM, Jakub Hrozek jhrozek@redhat.com wrote: >> >> >> >> >> >> > On Tue, May 28, 2013 at 04:32:00PM -0700, C. S. wrote: >> >> >> > > (Including once more as an attachment, it looks like something may have >> >> >> > > wrapped the lines when I had the patch in the body of the message.) >> >> >> > > >> >> >> > > >> >> >> > > On Tue, May 28, 2013 at 4:29 PM, C. S. csleclish@gmail.com wrote: >> >> >> > > >> >> >> > > > Hi Jakub, >> >> >> > > > >> >> >> > > > Thanks for the feedback! Very helpful. Looks like I dropped the LDAP >> >> >> > map >> >> >> > > > changes during a merge, that is now included. And I've removed the >> >> >> > > > strdup(), whoops! ;-) >> >> >> > > > >> >> >> > > > cs >> >> >> > > > >> >> >> > >> >> >> > Hi Cove, >> >> >> > >> >> >> > this patch is working for me with a couple of additonal changes that I >> >> >> > attach as a mini-patch. If you like, you can squash it into yours and >> >> >> > resend, then I think the patch would be good to go. >> >> >> > >> >> >> > Basically I think it's enough to have the master map name defined in the >> >> >> > LDAP options, not the map and entry option lists. Also the option lists >> >> >> > need the objectclass to be the first entry in the map. >> >> >> > >> >> >> > Sorry, I think I confused you with my previous e-mail. >> >> >> > >> >> >> > With the attached patch on top of yours, I was able to run automounter >> >> >> > with a custom master map (MASTER_MAP_NAME="auto.slave" in >> >> >> > /etc/sysconfig/autofs) just fine. >> >> >> > >> >> >> > Thanks again for the contribution. >> >> >> > >> >> >> > _______________________________________________ >> >> >> > sssd-devel mailing list >> >> >> > sssd-devel@lists.fedorahosted.org >> >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> >> >> > >> >> >> > >> >> > >> >> > >> >> >> _______________________________________________ >> >> >> sssd-devel mailing list >> >> >> sssd-devel@lists.fedorahosted.org >> >> >> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> >> > >> >> >> >> I would like to apploligize for late answer, but, I had few problems with >> >> autofs configuration. >> >> >> >> >From e36d14827fed18e6a633c004996606f6f0b01509 Mon Sep 17 00:00:00 2001 >> >> >From: Cove Schneider cove@ilm.com >> >> >Date: Fri, 31 May 2013 23:56:48 -0700 >> >> >Subject: [PATCH] Add ldap_autofs_map_master_name option >> >> > >> >> >--- >> >> > src/config/etc/sssd.api.d/sssd-ipa.conf | 1 + >> >> > src/config/etc/sssd.api.d/sssd-ldap.conf | 1 + >> >> > src/db/sysdb_autofs.h | 5 +++-- >> >> > src/man/sssd-ldap.5.xml | 13 +++++++++++++ >> >> > src/providers/ad/ad_opts.h | 1 + >> >> > src/providers/data_provider_be.c | 7 ------- >> >> > src/providers/ipa/ipa_opts.h | 1 + >> >> > src/providers/ldap/ldap_common.c | 15 +++++++++++++++ >> >> > src/providers/ldap/ldap_common.h | 2 ++ >> >> > src/providers/ldap/ldap_opts.h | 1 + >> >> > src/providers/ldap/sdap.h | 1 + >> >> > src/providers/ldap/sdap_autofs.c | 12 ++++++++++++ >> >> > 12 files changed, 51 insertions(+), 9 deletions(-) >> >> > >> >> >diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf >> >> >index e6f1bb0a88dd72a5f7bfcb269fa4736e7efd947c..5105a40a629d87a62a21cb6933a758943c1688cc 100644 >> >> >--- a/src/config/etc/sssd.api.d/sssd-ipa.conf >> >> >+++ b/src/config/etc/sssd.api.d/sssd-ipa.conf >> >> >@@ -162,6 +162,7 @@ ipa_hostgroup_uuid = str, None, false >> >> > >> >> > [provider/ipa/autofs] >> >> > ipa_automount_location = str, None, false >> >> >+ldap_autofs_map_master_name = str, None, false >> >> > ldap_autofs_map_object_class = str, None, false >> >> > ldap_autofs_map_name = str, None, false >> >> > ldap_autofs_entry_object_class = str, None, false >> >> >diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf >> >> >index 14e979da3413fe575e83b9a508c6cd8029d2bfba..aa521567f3f657885ba8978fdd118174b0ff9b4f 100644 >> >> >--- a/src/config/etc/sssd.api.d/sssd-ldap.conf >> >> >+++ b/src/config/etc/sssd.api.d/sssd-ldap.conf >> >> >@@ -153,6 +153,7 @@ ldap_sudorule_notafter = str, None, false >> >> > ldap_sudorule_order = str, None, false >> >> > >> >> > [provider/ldap/autofs] >> >> >+ldap_autofs_map_master_name = str, None, false >> >> > ldap_autofs_map_object_class = str, None, false >> >> > ldap_autofs_map_name = str, None, false >> >> > ldap_autofs_entry_object_class = str, None, false >> >> >diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h >> >> >index e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923..97edaea03179d89d37073df41ca95cd544a53a29 100644 >> >> >--- a/src/db/sysdb_autofs.h >> >> >+++ b/src/db/sysdb_autofs.h >> >> >@@ -28,8 +28,9 @@ >> >> > #define AUTOFS_MAP_SUBDIR "autofsmaps" >> >> > #define AUTOFS_ENTRY_SUBDIR "autofsentries" >> >> > >> >> >-#define SYSDB_AUTOFS_MAP_OC "automountMap" >> >> >-#define SYSDB_AUTOFS_MAP_NAME "automountMapName" >> >> >+#define SYSDB_AUTOFS_MAP_MASTER_NAME "automountMapMasterName" >> >> >+#define SYSDB_AUTOFS_MAP_OC "automountMap" >> >> >+#define SYSDB_AUTOFS_MAP_NAME "automountMapName" >> >> > >> >> > #define SYSDB_AUTOFS_ENTRY_OC "automount" >> >> > #define SYSDB_AUTOFS_ENTRY_KEY "automountKey" >> >> >diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml >> >> >index 37df5ec1b2e0b150b407f5e3e39db21860055580..b06286742577b84c347701ab0cd56c4e661a44c8 100644 >> >> >--- a/src/man/sssd-ldap.5.xml >> >> >+++ b/src/man/sssd-ldap.5.xml >> >> >@@ -2169,6 +2169,19 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com >> >> > <para> >> >> > <variablelist> >> >> > <varlistentry> >> >> >+ <term>ldap_autofs_map_master_name (string)</term> >> >> >+ <listitem> >> >> >+ <para> >> >> >+ The name of the automount master map in LDAP. >> >> >+ </para> >> >> >+ <para> >> >> >+ Default: auto.master >> >> >+ </para> >> >> >+ </listitem> >> >> >+ </varlistentry> >> >> >+ </variablelist> >> >> >+ <variablelist> >> >> >+ <varlistentry> >> >> > <term>ldap_autofs_map_object_class (string)</term> >> >> > <listitem> >> >> > <para> >> >> >diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h >> >> >index 218614dca82ab1b5a2d54291ead3d9719a4c7e2a..8ce0c475265a8602edea72ecdff18f77f8ecab75 100644 >> >> >--- a/src/providers/ad/ad_opts.h >> >> >+++ b/src/providers/ad/ad_opts.h >> >> >@@ -65,6 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { >> >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >> > { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING }, >> >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, >> >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE }, >> >> >diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c >> >> >index cd6715680eb4871806cc5148cecd2e118885a105..11e8979c2afe4bbcad19ef77b241843463e02c58 100644 >> >> >--- a/src/providers/data_provider_be.c >> >> >+++ b/src/providers/data_provider_be.c >> >> >@@ -1643,13 +1643,6 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn) >> >> > goto done; >> >> > } >> >> > >> >> >- /* If a request for auto.master comes in, the automounter deamon >> >> >- * has been reloaded. Expire all autofs maps to force reload >> >> >- */ >> >> >- if (strcmp(be_autofs_req->mapname, "auto.master") == 0) { >> >> >- be_autofs_req->invalidate = true; >> >> >- } >> >> >- >> >> > be_req->req_data = be_autofs_req; >> >> > >> >> > if (!be_cli->bectx->bet_info[BET_AUTOFS].bet_ops) { >> >> >diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h >> >> >index 4dfa72db430b1d3dfb528cc425c7156baf5ebe47..f8a3bdda0d8b2cc66d40b73dec2aab3b9e65b5d9 100644 >> >> >--- a/src/providers/ipa/ipa_opts.h >> >> >+++ b/src/providers/ipa/ipa_opts.h >> >> >@@ -89,6 +89,7 @@ struct dp_option ipa_def_ldap_opts[] = { >> >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >> > { "ldap_schema", DP_OPT_STRING, { "ipa_v1" }, NULL_STRING }, >> >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, >> >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >> >diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c >> >> >index acb24b190157efe06967963ad05014cf9017db7f..33faf232bdd13c79c90db76b90fea4770a458a6b 100644 >> >> >--- a/src/providers/ldap/ldap_common.c >> >> >+++ b/src/providers/ldap/ldap_common.c >> >> >@@ -407,10 +407,25 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx, >> >> > struct sdap_options *opts) >> >> > { >> >> > const char *search_base; >> >> >+ const char *master_map; >> >> > struct sdap_attr_map *default_entry_map; >> >> > struct sdap_attr_map *default_mobject_map; >> >> > int ret; >> >> > >> >> >+ /* automount master map name */ >> >> >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); >> >> >+ if (master_map == NULL) { >> >> >+ ret = dp_opt_set_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME, >> >> >+ AUTOFS_MAP_MASTER_NAME); >> >> >+ if (ret != EOK) { >> >> >+ DEBUG(SSSDBG_OP_FAILURE, ("Could not set autofs master map name" >> >> >+ " to default value\n")); >> >> >+ return ret; >> >> >+ } >> >> >+ master_map = dp_opt_get_string(opts->basic, SDAP_AUTOFS_MAP_MASTER_NAME); >> >> >+ } >> >> >+ DEBUG(SSSDBG_FUNC_DATA, ("Option ldap_autofs_map_master_name set to %s\n", master_map)); >> >> >+ >> >> > /* search base */ >> >> > search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); >> >> > if (search_base != NULL) { >> >> >diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h >> >> >index 2d17b755808e58831b458bcc86a50eb74b1f1057..ec48db68a457b9cf4e490b649eaefd0573ad8e71 100644 >> >> >--- a/src/providers/ldap/ldap_common.h >> >> >+++ b/src/providers/ldap/ldap_common.h >> >> >@@ -39,6 +39,8 @@ >> >> > #define LDAP_SSL_URI "ldaps://" >> >> > #define LDAP_LDAPI_URI "ldapi://" >> >> > >> >> >+#define AUTOFS_MAP_MASTER_NAME "auto.master" >> >> >+ >> >> > /* a fd the child process would log into */ >> >> > extern int ldap_child_debug_fd; >> >> > >> >> >diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h >> >> >index 807716c18244ae70759d9b2eff7e40fd3c634fb2..8d0a5a647f068a01c5170f30a7f6865f7ca5e0c7 100644 >> >> >--- a/src/providers/ldap/ldap_opts.h >> >> >+++ b/src/providers/ldap/ldap_opts.h >> >> >@@ -56,6 +56,7 @@ struct dp_option default_basic_opts[] = { >> >> > { "ldap_sudo_include_netgroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >> > { "ldap_sudo_include_regexp", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, >> >> > { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >> >+ { "ldap_autofs_map_master_name", DP_OPT_STRING, NULL_STRING, NULL_STRING }, >> >> > { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING }, >> >> > { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER }, >> >> > { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, >> >> >diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h >> >> >index 162250fff76295807cce0779bebdf88577ed755c..7d2d3f534c04f4a21a920286fa17f199277999e9 100644 >> >> >--- a/src/providers/ldap/sdap.h >> >> >+++ b/src/providers/ldap/sdap.h >> >> >@@ -164,6 +164,7 @@ enum sdap_basic_opt { >> >> > SDAP_SUDO_INCLUDE_NETGROUPS, >> >> > SDAP_SUDO_INCLUDE_REGEXP, >> >> > SDAP_AUTOFS_SEARCH_BASE, >> >> >+ SDAP_AUTOFS_MAP_MASTER_NAME, >> >> > SDAP_SCHEMA, >> >> > SDAP_OFFLINE_TIMEOUT, >> >> > SDAP_FORCE_UPPER_CASE_REALM, >> >> >diff --git a/src/providers/ldap/sdap_autofs.c b/src/providers/ldap/sdap_autofs.c >> >> >index 0bb211aa3e3ae9ec822e117f8febf8378bb09f79..e4ef7257bd251fdf7cc672c7f96bb67e3bc4ff32 100644 >> >> >--- a/src/providers/ldap/sdap_autofs.c >> >> >+++ b/src/providers/ldap/sdap_autofs.c >> >> >@@ -30,6 +30,7 @@ >> >> > #include "providers/ldap/sdap.h" >> >> > #include "providers/ldap/sdap_async.h" >> >> > #include "providers/dp_backend.h" >> >> >+#include "providers/data_provider.h" >> >> > #include "db/sysdb_autofs.h" >> >> > #include "util/util.h" >> >> > >> >> >@@ -82,6 +83,7 @@ void sdap_autofs_handler(struct be_req *be_req) >> >> > struct sdap_id_ctx *id_ctx; >> >> > struct be_autofs_req *autofs_req; >> >> > struct tevent_req *req; >> >> >+ const char *master_map; >> >> > int ret = EOK; >> >> > >> >> > DEBUG(SSSDBG_TRACE_INTERNAL, ("sdap autofs handler called\n")); >> >> >@@ -98,6 +100,16 @@ void sdap_autofs_handler(struct be_req *be_req) >> >> > DEBUG(SSSDBG_FUNC_DATA, ("Requested refresh for: %s\n", >> >> > autofs_req->mapname ? autofs_req->mapname : "<ALL>\n")); >> >> > >> >> >+ if (autofs_req->mapname != NULL) { >> >> >+ master_map = dp_opt_get_string(id_ctx->opts->basic, >> >> >+ SDAP_AUTOFS_MAP_MASTER_NAME); >> >> ^^^^^^^^^^ >> >> I think, that function dp_opt_get_string can return NULL. >> >> It would be not good idea to compare NULL with autofs_req->mapname. >> > >> >It shouldn't happen but yeah, famous last words :-) >> > >> >I agree with Lukas, let's be extra paranoid and also add a NULL check >> >here. If we ever get a NULL string, we'll just use stale data, nothing >> >terrible. >> It is not about to be extra paranoid. I hit this bug, but >> I forgot to attach back_trace in my previous mail. >> >> > >> >Thank you for testing! >> > >> >> >> >> >+ if (strcmp(master_map, autofs_req->mapname) == 0) { >> >> >+ autofs_req->invalidate = true; >> >> >+ DEBUG(SSSDBG_FUNC_DATA, ("Refresh of automount master map triggered: %s\n", >> >> >+ autofs_req->mapname)); >> >> >+ } >> >> >+ } >> >> >+ >> >> > if (autofs_req->invalidate) { >> >> > ret = sysdb_invalidate_autofs_maps(id_ctx->be->domain->sysdb, >> >> > id_ctx->be->domain); >> >> >-- >> >> >1.8.2.1 >> >> I attach back_trace for record. >> LS > >How did you get into this situation? I thought that the option defaulted >to "auto.master" even in the map. If not, then I think it should.
Reproducible: Always
Steps to reproduce:
- service sssd stop
- update sssd (sssd with patch)
- remove cache and logs
- service sssd start
- servive autofs restart ^^^^
Result: sssd crashed.
Could be default value in "struct dp_option default_basic_opts[]" ?
Like { "ldap_default_authtok_type", DP_OPT_STRING, { "password" }, NULL_STRING},
Yes, we should default to auto.master there.
Cove, would you like us to help you with the patch in any way?
I'm cleaning up patchwork after last upstream release and attached is a patch with couple of coments I had (see 0001-review-fixes.patch) and also resulting patch with Cove's attribution.
From 955f24c1ec3c690ea19483b52243287415d15112 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 4 Nov 2013 12:07:55 +0100 Subject: [PATCH] review fixes
I cannot apply this patch to master. There is a lot of conflicts with Cove's patch.
Aah, sorry, this patch is not intended to be *applied*, it is just an interdiff between Cove's original patch and the one I sent..
From ac674d2e4196da9ff749db8709e967289446e9cc Mon Sep 17 00:00:00 2001 From: Cove Schneider cove@ilm.com Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option
This one should be applied.
autofs works fine. There is no crash.
ACK
LS
Pushed to master.
sssd-devel@lists.fedorahosted.org
-
C. S. -
Jakub Hrozek -
Lukas Slebodnik