On 08/10/2016 08:36 PM, Lukas Slebodnik wrote:
On (10/08/16 17:41), Michal Židek wrote:
Hi,
see the attached patch.
I modified the detection of duplicates when
extending the maps (sysdb_attr:ldap_attr).
When we try to add entry to the map
that already exists in the map, then
without this patch we will fail.
With this patch, we only fail if the
newly added extension would redefine
already existing entry in the map.
Otherwise it is just skipped without
a failure (we just skip adding what
is already there).
I created simple CI test for this (first
patch).
Michal
From 932745aaadd7c2a73e817bfd685fbe1aaa462cf1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Wed, 10 Aug 2016 16:40:45 +0200
Subject: [PATCH 1/2] CI: Test extra attributes duplicate
Regresion test for ticket #3120
Resolves:
https://fedorahosted.org/sssd/ticket/3120
---
src/tests/intg/ldap_test.py | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index 84f7f2d..b8315e5 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -292,6 +292,28 @@ def sanity_rfc2307_bis(request, ldap_conn):
return None
+@pytest.fixture
+def just_start_ldap(request, ldap_conn):
+ ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+ create_ldap_fixture(request, ldap_conn, ent_list)
+
+ conf = \
+ format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \
+ unindent("""\
+ [domain/LDAP]
+ ldap_user_extra_attrs = mail
+ """).format(**locals())
+
+ create_conf_fixture(request, conf)
+ create_sssd_cleanup(request)
+ return None
+
+
+def test_extra_attribute_already_exists(ldap_conn, just_start_ldap):
+ if subprocess.call(["sssd", "-D", "-f"]) != 0:
+ raise Exception("sssd start failed")
+
+
def test_regression_ticket2163(ldap_conn, simple_rfc2307):
ent.assert_passwd_by_name(
'usr\\001',
--
2.5.0
From 5a2ef2a98e483701603a42bc50e9a11d8ee651ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Wed, 10 Aug 2016 15:41:34 +0200
Subject: [PATCH 2/2] sdap: Skip exact duplicates when extending maps
When extending map with entry that already
exists in the map in the exacty same form,
then there is no need to fail.
We should only fail if we try to
change purpose of already used sysdb
attribute.
Resolves:
https://fedorahosted.org/sssd/ticket/3120
---
src/providers/ldap/sdap.c | 41 +++++++++++++++++++++++++++++++++++------
1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 97b8f12..e1cf70f 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -122,19 +122,39 @@ static errno_t split_extra_attr(TALLOC_CTX *mem_ctx,
return EOK;
}
-static bool is_sysdb_duplicate(struct sdap_attr_map *map,
- int num_entries,
- const char *sysdb_attr)
+/* _already_in_map is set to true if the attribute
+ * already exists in the map and is used for the same
+ * LDAP attribute.
+ *
+ * _conflicts_with_map is set to true if the attribute
+ * already exists in map, but is used for different
+ * LDAP attribute.
+ * */
+static void check_duplicate(struct sdap_attr_map *map,
+ int num_entries,
+ const char *sysdb_attr,
+ const char *ldap_attr,
+ bool *_already_in_map,
+ bool *_conflicts_with_map)
{
This function has 3 output boolean argumets:
It would be better to return enum instead of
adding new parametrs.
LS
Ok, attached is version with enum.
Michal
>From 932745aaadd7c2a73e817bfd685fbe1aaa462cf1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Wed, 10 Aug 2016 16:40:45 +0200
Subject: [PATCH 1/2] CI: Test extra attributes duplicate
Regresion test for ticket #3120
Resolves:
https://fedorahosted.org/sssd/ticket/3120
---
src/tests/intg/ldap_test.py | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index 84f7f2d..b8315e5 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -292,6 +292,28 @@ def sanity_rfc2307_bis(request, ldap_conn):
return None
+@pytest.fixture
+def just_start_ldap(request, ldap_conn):
+ ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+ create_ldap_fixture(request, ldap_conn, ent_list)
+
+ conf = \
+ format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \
+ unindent("""\
+ [domain/LDAP]
+ ldap_user_extra_attrs = mail
+ """).format(**locals())
+
+ create_conf_fixture(request, conf)
+ create_sssd_cleanup(request)
+ return None
+
+
+def test_extra_attribute_already_exists(ldap_conn, just_start_ldap):
+ if subprocess.call(["sssd", "-D", "-f"]) != 0:
+ raise Exception("sssd start failed")
+
+
def test_regression_ticket2163(ldap_conn, simple_rfc2307):
ent.assert_passwd_by_name(
'usr\\001',
--
2.5.0
>From 90bc2074a0c262bc83d5f1da8a7f31142505b935 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Wed, 10 Aug 2016 15:41:34 +0200
Subject: [PATCH 2/2] sdap: Skip exact duplicates when extending maps
When extending map with entry that already
exists in the map in the exacty same form,
then there is no need to fail.
We should only fail if we try to
change purpose of already used sysdb
attribute.
Resolves:
https://fedorahosted.org/sssd/ticket/3120
---
src/providers/ldap/sdap.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 97b8f12..3518342 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -122,19 +122,30 @@ static errno_t split_extra_attr(TALLOC_CTX *mem_ctx,
return EOK;
}
-static bool is_sysdb_duplicate(struct sdap_attr_map *map,
- int num_entries,
- const char *sysdb_attr)
+enum duplicate_t {
+ NOT_FOUND = 0,
+ ALREADY_IN_MAP, /* nothing to add */
+ CONFLICT_WITH_MAP /* attempt to redefine attribute */
+};
+
+static enum duplicate_t check_duplicate(struct sdap_attr_map *map,
+ int num_entries,
+ const char *sysdb_attr,
+ const char *ldap_attr)
{
int i;
for (i = 0; i < num_entries; i++) {
if (strcmp(map[i].sys_name, sysdb_attr) == 0) {
- return true;
+ if (strcmp(map[i].name, ldap_attr) == 0) {
+ return ALREADY_IN_MAP;
+ } else {
+ return CONFLICT_WITH_MAP;
+ }
}
}
- return false;
+ return NOT_FOUND;
}
int sdap_extend_map(TALLOC_CTX *memctx,
@@ -174,7 +185,13 @@ int sdap_extend_map(TALLOC_CTX *memctx,
continue;
}
- if (is_sysdb_duplicate(map, num_entries, sysdb_attr)) {
+ ret = check_duplicate(map, num_entries, sysdb_attr, ldap_attr);
+ if (ret == ALREADY_IN_MAP) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Attribute %s (%s in LDAP) is already in map.\n",
+ sysdb_attr, ldap_attr);
+ continue;
+ } else if (ret == CONFLICT_WITH_MAP) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Attribute %s (%s in LDAP) is already used by SSSD, please "
"choose a different cache name\n", sysdb_attr, ldap_attr);
--
2.5.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
