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?= <mzidek(a)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
>
>
> +(a)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?= <mzidek(a)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