[PATCH] Add ldap_autofs_map_master_name option - sssd-devel - Fedora Mailing-Lists

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

[PATCH] Add ldap_autofs_map_master_name option

[PATCH] build: fix ordering of...

[PATCH] Replace prog_DEPENDENCIES...

C. S.

Friday, 24 May 2013 Fri, 24 May '13

6:41 p.m.

Add ldap_autofs_map_master_name option so that the auto master map name change be changed from the default auto.master.

Attachments:

attachment.html (text/html — 171 bytes)
0001-Add-ldap_autofs_map_master_name-option.patch (application/octet-stream — 11.0 KB)

Reply

Show replies by thread

Jakub Hrozek

Monday, 27 May Mon, 27 May

10:28 a.m.

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!

Reply

C. S.

Tuesday, 28 May Tue, 28 May

6:29 p.m.

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(a)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(a)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(a)lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply

C. S.

6:32 p.m.

(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(a)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(a)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(a)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(a)lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply

Jakub Hrozek

Thursday, 30 May Thu, 30 May

11:44 a.m.

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(a)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.

Reply

C. S.

Saturday, 1 June Sat, 1 Jun

2:30 a.m.

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(a)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(a)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(a)lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply

Jakub Hrozek

Monday, 3 June Mon, 3 Jun

3:41 p.m.

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(a)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(a)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(a)lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >

_______________________________________________ sssd-devel mailing list sssd-devel(a)lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply

Lukas Slebodnik

Monday, 17 June Mon, 17 Jun

10:28 a.m.

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(a)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(a)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(a)lists.fedorahosted.org > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > > > > > _______________________________________________ > sssd-devel mailing list > sssd-devel(a)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(a)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(a)lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply

Jakub Hrozek

12:47 p.m.

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(a)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(a)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(a)lists.fedorahosted.org >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> > >> > > > >> _______________________________________________ >> sssd-devel mailing list >> sssd-devel(a)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(a)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(a)lists.fedorahosted.org >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel _______________________________________________ sssd-devel mailing list sssd-devel(a)lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply

Lukas Slebodnik

Tuesday, 18 June Tue, 18 Jun

3:29 a.m.

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(a)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(a)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(a)lists.fedorahosted.org > >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >> > > >> > > > > > > >> _______________________________________________ > >> sssd-devel mailing list > >> sssd-devel(a)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(a)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

Reply

Jakub Hrozek

4:15 a.m.

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(a)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(a)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(a)lists.fedorahosted.org >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> >> > >> >> > >> > >> > >> >> _______________________________________________ >> >> sssd-devel mailing list >> >> sssd-devel(a)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(a)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.

Reply

Lukas Slebodnik

10 a.m.

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(a)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(a)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(a)lists.fedorahosted.org > >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >> >> > > >> >> > > >> > > >> > > >> >> _______________________________________________ > >> >> sssd-devel mailing list > >> >> sssd-devel(a)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(a)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

Reply

Jakub Hrozek

Wednesday, 10 July Wed, 10 Jul

7:51 a.m.

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(a)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(a)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(a)lists.fedorahosted.org >> >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> >> >> > >> >> >> > >> >> > >> >> > >> >> >> _______________________________________________ >> >> >> sssd-devel mailing list >> >> >> sssd-devel(a)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(a)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},

Yes, we should default to auto.master there. Cove, would you like us to help you with the patch in any way?

Reply

Jakub Hrozek

Monday, 4 November Mon, 4 Nov

5:11 a.m.

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(a)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(a)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(a)lists.fedorahosted.org > >> >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >> >> >> > > >> >> >> > > >> >> > > >> >> > > >> >> >> _______________________________________________ > >> >> >> sssd-devel mailing list > >> >> >> sssd-devel(a)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(a)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}, 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.

Reply

Lukas Slebodnik

Tuesday, 5 November Tue, 5 Nov

11:51 a.m.

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(a)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(a)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(a)lists.fedorahosted.org > > >> >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > > >> >> >> > > > >> >> >> > > > >> >> > > > >> >> > > > >> >> >> _______________________________________________ > > >> >> >> sssd-devel mailing list > > >> >> >> sssd-devel(a)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(a)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}, > > 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(a)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(a)ilm.com> Date: Fri, 31 May 2013 23:56:48 -0700 Subject: [PATCH] Add ldap_autofs_map_master_name option

LS

Reply

Jakub Hrozek

Wednesday, 6 November Wed, 6 Nov

6:46 a.m.

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(a)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(a)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(a)lists.fedorahosted.org >> > >> >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> > >> >> >> > >> > >> >> >> > >> > >> >> > >> > >> >> > >> > >> >> >> _______________________________________________ >> > >> >> >> sssd-devel mailing list >> > >> >> >> sssd-devel(a)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(a)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}, >> >> 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(a)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(a)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.

Reply

Lukas Slebodnik

Monday, 11 November Mon, 11 Nov

5:25 a.m.

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(a)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(a)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(a)lists.fedorahosted.org > >> > >> >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >> > >> >> >> > > >> > >> >> >> > > >> > >> >> > > >> > >> >> > > >> > >> >> >> _______________________________________________ > >> > >> >> >> sssd-devel mailing list > >> > >> >> >> sssd-devel(a)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(a)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}, > >> > >> 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(a)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(a)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

Reply

Jakub Hrozek

Tuesday, 12 November Tue, 12 Nov

3:57 a.m.

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(a)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(a)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(a)lists.fedorahosted.org >> >> > >> >> >> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >> >> > >> >> >> > >> >> > >> >> >> > >> >> > >> >> > >> >> > >> >> > >> >> > >> >> >> _______________________________________________ >> >> > >> >> >> sssd-devel mailing list >> >> > >> >> >> sssd-devel(a)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(a)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}, >> >> >> >> 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(a)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(a)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.

Reply

3829

days inactive

4001

days old

sssd-devel@lists.fedorahosted.org

Manage subscription

17 comments

3 participants

tags (0)

participants (3)

C. S.
Jakub Hrozek
Lukas Slebodnik