Hello list,
there is attached patch set for intg. testing of ldap nested netgroups.
I used last version of Lukas patch 'sssd_netgroup.py: Resolve nested netgroups'. I don't know if it is on list.
It is still WIP. It is in state that it is possible to run it. But there are comments in code what is needed to fix.
If I remove some netgroups (in test), it is updated on LDAP and in cache, but sssd_netgroup.get_sssd_netgroups() returns nothing.
Please, see test_reproducer_t2841() in the second patch for more details. I debug it with run_shell() and there are some notes from LDAP server and LDB cache attached [1].
[1] ldap_ldb_content.txt
Regards
On (17/08/16 14:32), Petr Cech wrote:
Hello list,
there is attached patch set for intg. testing of ldap nested netgroups.
I used last version of Lukas patch 'sssd_netgroup.py: Resolve nested netgroups'. I don't know if it is on list.
It is still WIP. It is in state that it is possible to run it. But there are comments in code what is needed to fix.
If I remove some netgroups (in test), it is updated on LDAP and in cache, but sssd_netgroup.get_sssd_netgroups() returns nothing.
Yes, I was too strict regarding to failures in resolving nested netgroups. glibc(getent netgroup) does not fail if there is non-existing nested netgroup.
There is bunch of pep8 warnings. Please fix them. sh$ pep8 src/tests/intg/test_netgroup.py | wc -l 29
Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 2/2] WIP: INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 487 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 488 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index b8cc5c006845f911d8518df815925455482e9f6d..b3c553539d9e74dae986fa6551041544dd687c11 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -14,6 +14,7 @@ dist_noinst_DATA = \ util.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..4be2b1c7048f3d8dc4797557d6decf1367eea36d --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,487 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
//snip
+@pytest.fixture +def reproducer_t2841(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=True)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return (ldap_conn, ent_list)
+def test_reproducer_t2841(reproducer_t2841):
- """
- Adding two nested netgroup.
- """
- ldap_conn = reproducer_t2841[0]
- ent_list = reproducer_t2841[1]
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2'),
('host1', 'user1', 'domain1')]
- # removing of t2841_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # FIX: This should be SUCCES
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
Please never write test which always passes. Test must cover the test case and not be green in 100% cases. Otherwise we cousl have "assert True" everywhere :-)
- # point A
- # run_shell()
- # removing of t2841_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # FIX: This should be SUCCES
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # point B
- # run_shell()
I fixed the expected results(@see patch) and the test passes with latest version of sssd_netgroup.py and git master. There are two possible explanations a) we need to close ticket 2841 as works for me b) the test test_reproducer_t2841 is testing something different.
BTW I tested the same test-case outside of cwrap with simple shell script.
sh# ldapadd -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 \ -f netgroups.ldif adding new entry "ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com"
sh# systemctl restart sssd
sh# systemctl restart sssd sh# getent netgroup t2841_netgroup1 || echo FAILED t2841_netgroup1 (host1,user1,domain1) sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2) (host1,user1,domain1)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED FAILED sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3
Could you explain it?
LS
On 08/18/2016 12:39 PM, Lukas Slebodnik wrote:
On (17/08/16 14:32), Petr Cech wrote:
Hello list,
there is attached patch set for intg. testing of ldap nested netgroups.
I used last version of Lukas patch 'sssd_netgroup.py: Resolve nested netgroups'. I don't know if it is on list.
It is still WIP. It is in state that it is possible to run it. But there are comments in code what is needed to fix.
If I remove some netgroups (in test), it is updated on LDAP and in cache, but sssd_netgroup.get_sssd_netgroups() returns nothing.
Yes, I was too strict regarding to failures in resolving nested netgroups. glibc(getent netgroup) does not fail if there is non-existing nested netgroup.
There is bunch of pep8 warnings. Please fix them. sh$ pep8 src/tests/intg/test_netgroup.py | wc -l 29
Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 2/2] WIP: INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 487 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 488 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index b8cc5c006845f911d8518df815925455482e9f6d..b3c553539d9e74dae986fa6551041544dd687c11 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -14,6 +14,7 @@ dist_noinst_DATA = \ util.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..4be2b1c7048f3d8dc4797557d6decf1367eea36d --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,487 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
//snip
+@pytest.fixture +def reproducer_t2841(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=True)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return (ldap_conn, ent_list)
+def test_reproducer_t2841(reproducer_t2841):
- """
- Adding two nested netgroup.
- """
- ldap_conn = reproducer_t2841[0]
- ent_list = reproducer_t2841[1]
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2'),
('host1', 'user1', 'domain1')]
- # removing of t2841_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # FIX: This should be SUCCES
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
Please never write test which always passes. Test must cover the test case and not be green in 100% cases. Otherwise we cousl have "assert True" everywhere :-)
- # point A
- # run_shell()
- # removing of t2841_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # FIX: This should be SUCCES
- res, errno, netgroups = sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # point B
- # run_shell()
I fixed the expected results(@see patch) and the test passes with latest version of sssd_netgroup.py and git master. There are two possible explanations a) we need to close ticket 2841 as works for me b) the test test_reproducer_t2841 is testing something different.
BTW I tested the same test-case outside of cwrap with simple shell script.
sh# ldapadd -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 \ -f netgroups.ldif adding new entry "ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com"
sh# systemctl restart sssd
sh# systemctl restart sssd sh# getent netgroup t2841_netgroup1 || echo FAILED t2841_netgroup1 (host1,user1,domain1) sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2) (host1,user1,domain1)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED FAILED sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3
Could you explain it?
The situation is: netgroups: A, B, C B and C in A
I remove B and C, but the right way is remove B, C from A, not B, C themself.
LS
Hello,
there is whole patch set for https://fedorahosted.org/sssd/ticket/2841 attached.
The first patch fixes the issue, the second patch adds support for netgroups to INTG tests, the third adds tests.
This patch set is applicable after Lukas's patch sssd_netgroup.py: Resolve nested netgroups
Regards
On 08/19/2016 01:00 PM, Petr Cech wrote:
On 08/18/2016 12:39 PM, Lukas Slebodnik wrote:
On (17/08/16 14:32), Petr Cech wrote:
Hello list,
there is attached patch set for intg. testing of ldap nested netgroups.
I used last version of Lukas patch 'sssd_netgroup.py: Resolve nested netgroups'. I don't know if it is on list.
It is still WIP. It is in state that it is possible to run it. But there are comments in code what is needed to fix.
If I remove some netgroups (in test), it is updated on LDAP and in cache, but sssd_netgroup.get_sssd_netgroups() returns nothing.
Yes, I was too strict regarding to failures in resolving nested netgroups. glibc(getent netgroup) does not fail if there is non-existing nested netgroup.
There is bunch of pep8 warnings. Please fix them. sh$ pep8 src/tests/intg/test_netgroup.py | wc -l 29
Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 2/2] WIP: INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 487 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 488 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index b8cc5c006845f911d8518df815925455482e9f6d..b3c553539d9e74dae986fa6551041544dd687c11 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -14,6 +14,7 @@ dist_noinst_DATA = \ util.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..4be2b1c7048f3d8dc4797557d6decf1367eea36d
--- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,487 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
//snip
+@pytest.fixture +def reproducer_t2841(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=True)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return (ldap_conn, ent_list)
+def test_reproducer_t2841(reproducer_t2841):
- """
- Adding two nested netgroup.
- """
- ldap_conn = reproducer_t2841[0]
- ent_list = reproducer_t2841[1]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2'),
('host1', 'user1', 'domain1')]
- # removing of t2841_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # FIX: This should be SUCCES
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
Please never write test which always passes. Test must cover the test case and not be green in 100% cases. Otherwise we cousl have "assert True" everywhere :-)
- # point A
- # run_shell()
- # removing of t2841_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # FIX: This should be SUCCES
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # point B
- # run_shell()
I fixed the expected results(@see patch) and the test passes with latest version of sssd_netgroup.py and git master. There are two possible explanations a) we need to close ticket 2841 as works for me b) the test test_reproducer_t2841 is testing something different.
BTW I tested the same test-case outside of cwrap with simple shell script.
sh# ldapadd -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 \ -f netgroups.ldif adding new entry "ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com"
sh# systemctl restart sssd
sh# systemctl restart sssd sh# getent netgroup t2841_netgroup1 || echo FAILED t2841_netgroup1 (host1,user1,domain1) sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2) (host1,user1,domain1)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED FAILED sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3
This is only the attempt to change Subject.
Important part from previous mail:
Could you explain it?
The situation is: netgroups: A, B, C B and C in A
I remove B and C, but the right way is remove B, C from A, not B, C themself.
LS
Hello,
there is whole patch set for https://fedorahosted.org/sssd/ticket/2841 attached.
The first patch fixes the issue, the second patch adds support for netgroups to INTG tests, the third adds tests.
This patch set is applicable after Lukas's patch sssd_netgroup.py: Resolve nested netgroups
On (19/08/16 13:35), Petr Cech wrote:
On 08/19/2016 01:00 PM, Petr Cech wrote:
On 08/18/2016 12:39 PM, Lukas Slebodnik wrote:
On (17/08/16 14:32), Petr Cech wrote:
Hello list,
there is attached patch set for intg. testing of ldap nested netgroups.
I used last version of Lukas patch 'sssd_netgroup.py: Resolve nested netgroups'. I don't know if it is on list.
It is still WIP. It is in state that it is possible to run it. But there are comments in code what is needed to fix.
If I remove some netgroups (in test), it is updated on LDAP and in cache, but sssd_netgroup.get_sssd_netgroups() returns nothing.
Yes, I was too strict regarding to failures in resolving nested netgroups. glibc(getent netgroup) does not fail if there is non-existing nested netgroup.
There is bunch of pep8 warnings. Please fix them. sh$ pep8 src/tests/intg/test_netgroup.py | wc -l 29
Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 2/2] WIP: INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 487 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 488 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index b8cc5c006845f911d8518df815925455482e9f6d..b3c553539d9e74dae986fa6551041544dd687c11 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -14,6 +14,7 @@ dist_noinst_DATA = \ util.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..4be2b1c7048f3d8dc4797557d6decf1367eea36d
--- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,487 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
//snip
+@pytest.fixture +def reproducer_t2841(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=True)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return (ldap_conn, ent_list)
+def test_reproducer_t2841(reproducer_t2841):
- """
- Adding two nested netgroup.
- """
- ldap_conn = reproducer_t2841[0]
- ent_list = reproducer_t2841[1]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2'),
('host1', 'user1', 'domain1')]
- # removing of t2841_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # FIX: This should be SUCCES
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
Please never write test which always passes. Test must cover the test case and not be green in 100% cases. Otherwise we cousl have "assert True" everywhere :-)
- # point A
- # run_shell()
- # removing of t2841_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # FIX: This should be SUCCES
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # point B
- # run_shell()
I fixed the expected results(@see patch) and the test passes with latest version of sssd_netgroup.py and git master. There are two possible explanations a) we need to close ticket 2841 as works for me b) the test test_reproducer_t2841 is testing something different.
BTW I tested the same test-case outside of cwrap with simple shell script.
sh# ldapadd -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 \ -f netgroups.ldif adding new entry "ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com"
sh# systemctl restart sssd
sh# systemctl restart sssd sh# getent netgroup t2841_netgroup1 || echo FAILED t2841_netgroup1 (host1,user1,domain1) sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2) (host1,user1,domain1)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED FAILED sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3
This is only the attempt to change Subject.
Important part from previous mail:
Could you explain it?
The situation is: netgroups: A, B, C B and C in A
I remove B and C, but the right way is remove B, C from A, not B, C themself.
LS
Hello,
there is whole patch set for https://fedorahosted.org/sssd/ticket/2841 attached.
The first patch fixes the issue, the second patch adds support for netgroups to INTG tests, the third adds tests.
This patch set is applicable after Lukas's patch sssd_netgroup.py: Resolve nested netgroups
-- Petr^4 Čech
From 94d790db56243419fc0629291bc2b19a773aa851 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Fri, 22 Jul 2016 14:28:54 +0200 Subject: [PATCH 1/3] LDAP: Fixing of removing netgroup from cache
There were problem with local key which wasn't properly removed. This patch fixes it.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/providers/ldap/sdap_async_netgroups.c | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d956df70cfcb5f68bd2afc9e2a23c50c3bb..cf7d7b12361f8cc578b891961c0c5566442f1b4e 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -38,6 +38,35 @@ bool is_dn(const char *str) return (ret == LDAP_SUCCESS ? true : false); }
+static errno_t add_to_missing_attrs(TALLOC_CTX * mem_ctx,
struct sysdb_attrs *attrs,
const char *ext_key,
char ***_missing)
+{
- bool is_present = false;
- size_t size = 0;
- size_t ret;
- for (int i = 0; i < attrs->num; i++) {
if (strcmp(ext_key, attrs->a[i].name) == 0) {
is_present = true;
}
size++;
- }
- if (is_present == false) {
ret = add_string_to_list(attrs, ext_key, _missing);
if (ret != EOK) {
goto done;
}
- }
- ret = EOK;
+done:
- return ret;
+}
static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, struct sss_domain_info *dom, struct sdap_options *opts, @@ -138,6 +167,17 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
- /* Prepare SYSDB_NETGROUP_MEMBER removing
* if not present in netgroup_attrs
*/
- ret = add_to_missing_attrs(attrs, netgroup_attrs, SYSDB_NETGROUP_MEMBER,
&missing);
- if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add [%s] to missing attributes\n",
SYSDB_NETGROUP_MEMBER);
goto fail;
- }
Thank you for test but I would appraciate a little bit simpler solution. "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) Your patch will append "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) to missing attributes if it is not in netgroup_attrs.
However "memberNisNetgroup" and "originalMemberNisNetgroup" are tightly coupled attributes in sysdb. * the sysdb attributes "originalMemberNisNetgroup" contain the original value from LDAP which can be name of netgroup of dn. * the sysdb attributes "memberNisNetgroup" always contain shortname used by sssd. They are even the same if memberNisNetgroup in LDAP does not contain dn. One is generated from other.
And function list_missing_attrs already find that "originalMemberNisNetgroup" is a missing attribute. Therefore if "originalMemberNisNetgroup" is in missing attibutes list then we have to add there also "memberNisNetgroup".
e.g. diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d9..1fe40f5 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,14 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
+ if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) { + ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n"); + goto fail; + } + } + ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
+ some optional nice explanation/comment why :-)
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
BTW the same bug is also ipa_save_netgroup in "src/providers/ipa/ipa_netgroups.c" But sysdb_add_netgroup is called wit NULL for missing attrinbutes :-)
Anyway sdap_save_netgroup and ipa_save_netgroup do almost the same They just use different maps. You touched the netgroup related code. So it would be good to do small refactoring and reuse ldap sdap_save_netgroup in ipa_save_netgroup. So it would be good If you could fix ticket #3117 in recent future. Because you still remember the netgroup related code in LDAP. (It's not a blocker for #2841) It is just a recomendation :-)
From b284c65bb6651cb32d09b3b35599127ce995cc4f Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 14:01:09 +0200 Subject: [PATCH 2/3] INTG: Adding support for netgroups to ldap_ent
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/ldap_ent.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index f8f2f7fe6977aec6fd704ad1c78a476a163a16f1..f7835a5a9c0a0f50a91c0085f02964cfafcfeebf 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -87,6 +87,21 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
+def netgroup(base_dn, cn, triples=[], members=[]):
IIRC [] is a aangerous default value as argument and we should use (). You might ask freeIPA/python developers why
- """
- Generate an RFC2307bis netgroup add-modlist for passing to ldap.add*.
- """
- attr_list = [
('objectClass', ['top', 'nisNetgroup'])
- ]
- member_list = []
this variable is unused
- if len(triples) > 0:
attr_list.append(('nisNetgroupTriple', triples))
- if len(members) > 0:
attr_list.append(('memberNisNetgroup', members))
- return ("cn=" + cn + ",ou=Netgroups," + base_dn, attr_list)
class List(list): """LDAP add-modlist list"""
@@ -124,3 +139,8 @@ class List(list): self.append(group_bis(base_dn or self.base_dn, cn, gidNumber, member_uids, member_gids))
- def add_netgroup(self, cn, triples=[], members=[], base_dn=None):
^^ the same case as in previous function
"""Add an RFC2307bis netgroup add-modlist."""
self.append(netgroup(base_dn or self.base_dn,
cn, triples, members))
-- 2.7.4
From 483b2bf279f47772fddaf4aa7087f6336ddf78b9 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 3/3] INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 490 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 491 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index d73e4216310ccd1c90e6b7eb0a0e60068fc45bd5..75422a4417046116bec11a8a680fe2248e3afb69 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -15,6 +15,7 @@ dist_noinst_DATA = \ test_ldap.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..d004faf2c2f3ee01c1fb41e36820709427c5cbd9 --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,490 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
+import os +import stat +import sys +import pwd +import grp +import ent
Previous 4 imports are unused
+import config +import signal +import subprocess +import time +import ldap +import ldap.modlist +import pytest +import ds_openldap +import ldap_ent +import sssd_netgroup as sssd_ntg +from util import *
It would be good to aoin wildcard import and import just required names (in our case just unindent) It might be a leftover after debugginf with run_shell
It would be also good to separate system modules with internam modules empty line might be enough e.g. import os import stat import signal import subprocess import time import ldap import ldap.modlist import pytest
import config import ds_openldap import ldap_ent ...
+LDAP_BASE_DN = "dc=example,dc=com" +INTERACTIVE_TIMEOUT = 4
Unused
+@pytest.fixture(scope="module") +def ds_inst(request):
- """LDAP server instance fixture"""
- ds_inst = ds_openldap.DSOpenLDAP(
config.PREFIX, 10389, LDAP_BASE_DN,
"cn=admin", "Secret123"
- )
- try:
ds_inst.setup()
- except:
ds_inst.teardown()
raise
- request.addfinalizer(lambda: ds_inst.teardown())
- return ds_inst
+@pytest.fixture(scope="module") +def ldap_conn(request, ds_inst):
- """LDAP server connection fixture"""
- ldap_conn = ds_inst.bind()
- ldap_conn.ds_inst = ds_inst
- request.addfinalizer(lambda: ldap_conn.unbind_s())
- return ldap_conn
+def create_ldap_entries(ldap_conn, ent_list=None):
- """Add LDAP entries from ent_list"""
- if ent_list is not None:
for entry in ent_list:
ldap_conn.add_s(entry[0], entry[1])
+def cleanup_ldap_entries(ldap_conn, ent_list=None):
- """Remove LDAP entries added by create_ldap_entries"""
- if ent_list is None:
for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):
for entry in ldap_conn.search_s("ou=" + ou + "," +
ldap_conn.ds_inst.base_dn,
ldap.SCOPE_ONELEVEL,
attrlist=[]):
ldap_conn.delete_s(entry[0])
- else:
for entry in ent_list:
ldap_conn.delete_s(entry[0])
+def create_ldap_cleanup(request, ldap_conn, ent_list=None):
- """Add teardown for removing all user/group LDAP entries"""
- request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+def create_ldap_fixture(request, ldap_conn, ent_list=None):
- """Add LDAP entries and add teardown for removing them"""
- create_ldap_entries(ldap_conn, ent_list)
- create_ldap_cleanup(request, ldap_conn, ent_list)
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+def format_basic_conf(ldap_conn, schema, enum):
- """Format a basic SSSD configuration"""
- schema_conf = "ldap_schema = " + schema + "\n"
- schema_conf += "ldap_group_object_class = groupOfNames\n"
- return unindent("""\
[sssd]
debug_level = 0xffff
domains = LDAP
services = nss, pam
pam responder is not used; it needn't run
[nss]
debug_level = 0xffff
memcache_timeout = 0
netgroup is not cached in memory cache so you can remove the line
[pam]
debug_level = 0xffff
[domain/LDAP]
ldap_auth_disable_tls_never_use_in_production = true
PLease remove this line because you are not testing authentication
debug_level = 0xffff
enumerate = {enum}
It would be good to do not test with enumeration and remove argument enum from the function format_basic_conf If you want to test netgroups with enumeration then please add separate tests into test_enumeration
{schema_conf}
id_provider = ldap
auth_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
ldap_netgroup_search_base = ou=Netgroups,{ldap_conn.ds_inst.base_dn}
Do we need enbale debug_level? IMHO it would be needed just for froubleshooting
- """).format(**locals())
+def create_conf_file(contents):
- """Create sssd.conf with specified contents"""
- conf = open(config.CONF_PATH, "w")
- conf.write(contents)
- conf.close()
- os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+def cleanup_conf_file():
- """Remove sssd.conf, if it exists"""
- if os.path.lexists(config.CONF_PATH):
os.unlink(config.CONF_PATH)
+def create_conf_cleanup(request):
- """Add teardown for removing sssd.conf"""
- request.addfinalizer(cleanup_conf_file)
+def create_conf_fixture(request, contents):
- """
- Create sssd.conf with specified contents and add teardown for removing it
- """
- create_conf_file(contents)
- create_conf_cleanup(request)
+def create_sssd_process():
- """Start the SSSD process"""
- if subprocess.call(["sssd", "-D", "-f"]) != 0:
raise Exception("sssd start failed")
+def cleanup_sssd_process():
- """Stop the SSSD process and remove its state"""
- try:
pid_file = open(config.PIDFILE_PATH, "r")
pid = int(pid_file.read())
os.kill(pid, signal.SIGTERM)
while True:
try:
os.kill(pid, signal.SIGCONT)
except:
break
time.sleep(1)
- except:
pass
- for path in os.listdir(config.DB_PATH):
os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
+def create_sssd_cleanup(request):
- """Add teardown for stopping SSSD and removing its state"""
- request.addfinalizer(cleanup_sssd_process)
+def create_sssd_fixture(request):
- """Start SSSD and add teardown for stopping it and removing its state"""
- create_sssd_process()
- create_sssd_cleanup(request)
+@pytest.fixture +def add_nested_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("nested_netgroup")
Is it an appropriate name? I would say it is a empty netgroup The same applies to the name of fixture and name of following test.
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_nested_netgroup(add_nested_netgroup):
- """
- Adding nested netgroup.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("nested_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+@pytest.fixture +def add_two_nested_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("nested_netgroup1")
- ent_list.add_netgroup("nested_netgroup2")
The same here name of netgroups are confusing Or did you forgot to use nested netgroups here?
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_nested_netgroup(add_two_nested_netgroup):
- """
- Adding two nested netgroup.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("nested_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("nested_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+# TODO: Test for removing one by one
Is there a reason for todo? or is this line just a leftover?
+@pytest.fixture +def add_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup", ["(host,user,domain)"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_tripled_netgroup(add_tripled_netgroup):
- """
- Adding netgroup with trinity.
^^^^^^^^ I think that netgroups terminology would use triplet.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host", "user", "domain")]
+@pytest.fixture +def add_advanced_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("adv_tripled_netgroup", ["(host1,user1,domain1)",
"(host2,user2,domain2)"])
I would recomment to merge this fixture ( + related test) with previous one. It woul a little bit reduce cuplicated code.
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_advanced_tripled_netgroup(add_advanced_tripled_netgroup):
- """
- Adding netgroup with two trinities.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("adv_tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2")])
+@pytest.fixture +def add_two_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("tripled_netgroup2", ["(host2,user2,domain2)"])
the same as in previous case. The name of netgroups are not conflicting therefore it could be tested in common test (test_simple_netgroups) becasue you are testing just simple netgroups just with triplets and not nested netgroups.
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_tripled_netgroup(add_two_tripled_netgroup):
- """
- Adding two netgroups, each with trinity.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host2", "user2", "domain2")]
+# TODO: Test for removing one by one
Is there a reason for todo? or is it an leftover?
+@pytest.fixture +def add_mixed_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("mixed_netgroup1", [], [])
- ent_list.add_netgroup("mixed_netgroup2", [], ["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup3", ["(host1,user1,domain1)"], [])
- ent_list.add_netgroup("mixed_netgroup4",
["(host2,user2,domain2)", "(host3,user3,domain3)"],
[])
- ent_list.add_netgroup("mixed_netgroup5",
["(host4,user4,domain4)"],
["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup6",
["(host5,user5,domain5)"],
["mixed_netgroup2"])
- ent_list.add_netgroup("mixed_netgroup7", [], ["mixed_netgroup3"])
- ent_list.add_netgroup("mixed_netgroup8", [],
["mixed_netgroup3", "mixed_netgroup4"])
- ent_list.add_netgroup("mixed_netgroup9",
["(host6,user6,domain6)"],
["mixed_netgroup3", "mixed_netgroup4"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_mixed_netgroup(add_mixed_netgroup):
- """
- Adding many netgroups of different type.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup4")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup5")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host4", "user4", "domain4")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup6")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host5", "user5", "domain5")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup7")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup8")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup9")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3"),
("host6", "user6", "domain6")])
+@pytest.fixture +def reproducer_t2841(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_reproducer_t2841(reproducer_t2841, ldap_conn):
It would be good to use more desciptive name. So it is immediately visible in pytest output what you want to test. The pytest output is very reduced for green test. and reproducer_t2841 is not very descriptive.
What about test_removing_nested_netgroups?
- """
- Regression test for ticket 2841.
- https://fedorahosted.org/sssd/ticket/2841
- """
- t2841_netgroup3_dn = 'cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com'
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of t2841_netgroup1 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup1", "t2841_netgroup2"]}
- new = {'memberNisNetgroup': ["t2841_netgroup2"]}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(t2841_netgroup3_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of t2841_netgroup2 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup2"]}
- new = {'memberNisNetgroup': []}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(t2841_netgroup3_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
BTW in all cases you are tesing positive results So the errno variabel is never used. It make sense to check errno just for failures. So you can replace errno with _ which means taht you are intentionaly ingoring the value.
Than you very much for test. I really appreciate it. Now wi will be sure that there woudl not be a regression for the bug and we can refactor code without any worries :-) I had a many comments but they were mostly nitpicking.
One more time thank you very much and good job.
LS
On 08/19/2016 05:18 PM, Lukas Slebodnik wrote:
On (19/08/16 13:35), Petr Cech wrote:
On 08/19/2016 01:00 PM, Petr Cech wrote:
On 08/18/2016 12:39 PM, Lukas Slebodnik wrote:
On (17/08/16 14:32), Petr Cech wrote:
Hello list,
there is attached patch set for intg. testing of ldap nested netgroups.
I used last version of Lukas patch 'sssd_netgroup.py: Resolve nested netgroups'. I don't know if it is on list.
It is still WIP. It is in state that it is possible to run it. But there are comments in code what is needed to fix.
If I remove some netgroups (in test), it is updated on LDAP and in cache, but sssd_netgroup.get_sssd_netgroups() returns nothing.
Yes, I was too strict regarding to failures in resolving nested netgroups. glibc(getent netgroup) does not fail if there is non-existing nested netgroup.
There is bunch of pep8 warnings. Please fix them. sh$ pep8 src/tests/intg/test_netgroup.py | wc -l 29
Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 2/2] WIP: INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 487 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 488 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index b8cc5c006845f911d8518df815925455482e9f6d..b3c553539d9e74dae986fa6551041544dd687c11 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -14,6 +14,7 @@ dist_noinst_DATA = \ util.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..4be2b1c7048f3d8dc4797557d6decf1367eea36d
--- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,487 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
//snip
+@pytest.fixture +def reproducer_t2841(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=True)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return (ldap_conn, ent_list)
+def test_reproducer_t2841(reproducer_t2841):
- """
- Adding two nested netgroup.
- """
- ldap_conn = reproducer_t2841[0]
- ent_list = reproducer_t2841[1]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2'),
('host1', 'user1', 'domain1')]
- # removing of t2841_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # FIX: This should be SUCCES
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
Please never write test which always passes. Test must cover the test case and not be green in 100% cases. Otherwise we cousl have "assert True" everywhere :-)
- # point A
- # run_shell()
- # removing of t2841_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # FIX: This should be SUCCES
- res, errno, netgroups =
sssd_netgroup.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_netgroup.NssReturnCode.NOTFOUND
- assert netgroups == []
- # point B
- # run_shell()
I fixed the expected results(@see patch) and the test passes with latest version of sssd_netgroup.py and git master. There are two possible explanations a) we need to close ticket 2841 as works for me b) the test test_reproducer_t2841 is testing something different.
BTW I tested the same test-case outside of cwrap with simple shell script.
sh# ldapadd -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 \ -f netgroups.ldif adding new entry "ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com"
adding new entry "cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com"
sh# systemctl restart sssd
sh# systemctl restart sssd sh# getent netgroup t2841_netgroup1 || echo FAILED t2841_netgroup1 (host1,user1,domain1) sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2) (host1,user1,domain1)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup1,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED t2841_netgroup2 (host2,user2,domain2) sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3 (host2,user2,domain2)
sh# ldapdelete -x -w Secret123 -D 'cn=admin,dc=example,dc=com' -h localhost:10389 cn=t2841_netgroup2,ou=Netgroups,dc=example,dc=com sh# sss_cache -N
sh# getent netgroup t2841_netgroup1 || echo FAILED FAILED sh# getent netgroup t2841_netgroup2 || echo FAILED FAILED sh# getent netgroup t2841_netgroup3 || echo FAILED t2841_netgroup3
This is only the attempt to change Subject.
Important part from previous mail:
Could you explain it?
The situation is: netgroups: A, B, C B and C in A
I remove B and C, but the right way is remove B, C from A, not B, C themself.
LS
Hello,
there is whole patch set for https://fedorahosted.org/sssd/ticket/2841 attached.
The first patch fixes the issue, the second patch adds support for netgroups to INTG tests, the third adds tests.
This patch set is applicable after Lukas's patch sssd_netgroup.py: Resolve nested netgroups
-- Petr^4 Čech
From 94d790db56243419fc0629291bc2b19a773aa851 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Fri, 22 Jul 2016 14:28:54 +0200 Subject: [PATCH 1/3] LDAP: Fixing of removing netgroup from cache
There were problem with local key which wasn't properly removed. This patch fixes it.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/providers/ldap/sdap_async_netgroups.c | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d956df70cfcb5f68bd2afc9e2a23c50c3bb..cf7d7b12361f8cc578b891961c0c5566442f1b4e 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -38,6 +38,35 @@ bool is_dn(const char *str) return (ret == LDAP_SUCCESS ? true : false); }
+static errno_t add_to_missing_attrs(TALLOC_CTX * mem_ctx,
struct sysdb_attrs *attrs,
const char *ext_key,
char ***_missing)
+{
- bool is_present = false;
- size_t size = 0;
- size_t ret;
- for (int i = 0; i < attrs->num; i++) {
if (strcmp(ext_key, attrs->a[i].name) == 0) {
is_present = true;
}
size++;
- }
- if (is_present == false) {
ret = add_string_to_list(attrs, ext_key, _missing);
if (ret != EOK) {
goto done;
}
- }
- ret = EOK;
+done:
- return ret;
+}
static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, struct sss_domain_info *dom, struct sdap_options *opts, @@ -138,6 +167,17 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
- /* Prepare SYSDB_NETGROUP_MEMBER removing
* if not present in netgroup_attrs
*/
- ret = add_to_missing_attrs(attrs, netgroup_attrs, SYSDB_NETGROUP_MEMBER,
&missing);
- if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add [%s] to missing attributes\n",
SYSDB_NETGROUP_MEMBER);
goto fail;
- }
Thank you for test but I would appraciate a little bit simpler solution. "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) Your patch will append "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) to missing attributes if it is not in netgroup_attrs.
However "memberNisNetgroup" and "originalMemberNisNetgroup" are tightly coupled attributes in sysdb.
- the sysdb attributes "originalMemberNisNetgroup" contain the original value from LDAP which can be name of netgroup of dn.
- the sysdb attributes "memberNisNetgroup" always contain shortname used by sssd.
They are even the same if memberNisNetgroup in LDAP does not contain dn. One is generated from other.
And function list_missing_attrs already find that "originalMemberNisNetgroup" is a missing attribute. Therefore if "originalMemberNisNetgroup" is in missing attibutes list then we have to add there also "memberNisNetgroup".
e.g. diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d9..1fe40f5 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,14 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
}
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
some optional nice explanation/comment why :-)
Hi Lukas,
thanks. I tested your solution, it is valid. I have to agree it is simpler. I fixed my patch in this way.
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
BTW the same bug is also ipa_save_netgroup in "src/providers/ipa/ipa_netgroups.c" But sysdb_add_netgroup is called wit NULL for missing attrinbutes :-)
Anyway sdap_save_netgroup and ipa_save_netgroup do almost the same They just use different maps. You touched the netgroup related code. So it would be good to do small refactoring and reuse ldap sdap_save_netgroup in ipa_save_netgroup. So it would be good If you could fix ticket #3117 in recent future. Because you still remember the netgroup related code in LDAP. (It's not a blocker for #2841) It is just a recomendation :-)
Right, I will take a look.
From b284c65bb6651cb32d09b3b35599127ce995cc4f Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 14:01:09 +0200 Subject: [PATCH 2/3] INTG: Adding support for netgroups to ldap_ent
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/ldap_ent.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index f8f2f7fe6977aec6fd704ad1c78a476a163a16f1..f7835a5a9c0a0f50a91c0085f02964cfafcfeebf 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -87,6 +87,21 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
+def netgroup(base_dn, cn, triples=[], members=[]):
IIRC [] is a aangerous default value as argument and we should use (). You might ask freeIPA/python developers why
I asked. Reason is that () is immutable and it is best practise. Good to know. :-)
Addressed.
- """
- Generate an RFC2307bis netgroup add-modlist for passing to ldap.add*.
- """
- attr_list = [
('objectClass', ['top', 'nisNetgroup'])
- ]
- member_list = []
this variable is unused
- if len(triples) > 0:
attr_list.append(('nisNetgroupTriple', triples))
- if len(members) > 0:
attr_list.append(('memberNisNetgroup', members))
- return ("cn=" + cn + ",ou=Netgroups," + base_dn, attr_list)
class List(list): """LDAP add-modlist list"""
@@ -124,3 +139,8 @@ class List(list): self.append(group_bis(base_dn or self.base_dn, cn, gidNumber, member_uids, member_gids))
- def add_netgroup(self, cn, triples=[], members=[], base_dn=None):
^^ the same case as in previous function
Addressed.
"""Add an RFC2307bis netgroup add-modlist."""
self.append(netgroup(base_dn or self.base_dn,
cn, triples, members))
-- 2.7.4
From 483b2bf279f47772fddaf4aa7087f6336ddf78b9 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 3/3] INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 490 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 491 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index d73e4216310ccd1c90e6b7eb0a0e60068fc45bd5..75422a4417046116bec11a8a680fe2248e3afb69 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -15,6 +15,7 @@ dist_noinst_DATA = \ test_ldap.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..d004faf2c2f3ee01c1fb41e36820709427c5cbd9 --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,490 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
+import os +import stat +import sys +import pwd +import grp +import ent
Previous 4 imports are unused
Thanks. Addressed.
Lukas, do you have any method how to find such situations? I would like improve my developing process.
+import config +import signal +import subprocess +import time +import ldap +import ldap.modlist +import pytest +import ds_openldap +import ldap_ent +import sssd_netgroup as sssd_ntg +from util import *
It would be good to aoin wildcard import and import just required names (in our case just unindent) It might be a leftover after debugginf with run_shell
It would be also good to separate system modules with internam modules empty line might be enough e.g. import os import stat import signal import subprocess import time import ldap import ldap.modlist import pytest
import config import ds_openldap import ldap_ent ...
+LDAP_BASE_DN = "dc=example,dc=com" +INTERACTIVE_TIMEOUT = 4
Unused
+@pytest.fixture(scope="module") +def ds_inst(request):
- """LDAP server instance fixture"""
- ds_inst = ds_openldap.DSOpenLDAP(
config.PREFIX, 10389, LDAP_BASE_DN,
"cn=admin", "Secret123"
- )
- try:
ds_inst.setup()
- except:
ds_inst.teardown()
raise
- request.addfinalizer(lambda: ds_inst.teardown())
- return ds_inst
+@pytest.fixture(scope="module") +def ldap_conn(request, ds_inst):
- """LDAP server connection fixture"""
- ldap_conn = ds_inst.bind()
- ldap_conn.ds_inst = ds_inst
- request.addfinalizer(lambda: ldap_conn.unbind_s())
- return ldap_conn
+def create_ldap_entries(ldap_conn, ent_list=None):
- """Add LDAP entries from ent_list"""
- if ent_list is not None:
for entry in ent_list:
ldap_conn.add_s(entry[0], entry[1])
+def cleanup_ldap_entries(ldap_conn, ent_list=None):
- """Remove LDAP entries added by create_ldap_entries"""
- if ent_list is None:
for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):
for entry in ldap_conn.search_s("ou=" + ou + "," +
ldap_conn.ds_inst.base_dn,
ldap.SCOPE_ONELEVEL,
attrlist=[]):
ldap_conn.delete_s(entry[0])
- else:
for entry in ent_list:
ldap_conn.delete_s(entry[0])
+def create_ldap_cleanup(request, ldap_conn, ent_list=None):
- """Add teardown for removing all user/group LDAP entries"""
- request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+def create_ldap_fixture(request, ldap_conn, ent_list=None):
- """Add LDAP entries and add teardown for removing them"""
- create_ldap_entries(ldap_conn, ent_list)
- create_ldap_cleanup(request, ldap_conn, ent_list)
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+def format_basic_conf(ldap_conn, schema, enum):
- """Format a basic SSSD configuration"""
- schema_conf = "ldap_schema = " + schema + "\n"
- schema_conf += "ldap_group_object_class = groupOfNames\n"
- return unindent("""\
[sssd]
debug_level = 0xffff
domains = LDAP
services = nss, pam
pam responder is not used; it needn't run
Addressed.
[nss]
debug_level = 0xffff
memcache_timeout = 0
netgroup is not cached in memory cache so you can remove the line
Addressed.
[pam]
debug_level = 0xffff
[domain/LDAP]
ldap_auth_disable_tls_never_use_in_production = true
PLease remove this line because you are not testing authentication
Addressed.
debug_level = 0xffff
enumerate = {enum}
It would be good to do not test with enumeration and remove argument enum from the function format_basic_conf If you want to test netgroups with enumeration then please add separate tests into test_enumeration
Addressed.
{schema_conf}
id_provider = ldap
auth_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
ldap_netgroup_search_base = ou=Netgroups,{ldap_conn.ds_inst.base_dn}
Do we need enbale debug_level? IMHO it would be needed just for froubleshooting
No, we needn't. Addressed.
- """).format(**locals())
+def create_conf_file(contents):
- """Create sssd.conf with specified contents"""
- conf = open(config.CONF_PATH, "w")
- conf.write(contents)
- conf.close()
- os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+def cleanup_conf_file():
- """Remove sssd.conf, if it exists"""
- if os.path.lexists(config.CONF_PATH):
os.unlink(config.CONF_PATH)
+def create_conf_cleanup(request):
- """Add teardown for removing sssd.conf"""
- request.addfinalizer(cleanup_conf_file)
+def create_conf_fixture(request, contents):
- """
- Create sssd.conf with specified contents and add teardown for removing it
- """
- create_conf_file(contents)
- create_conf_cleanup(request)
+def create_sssd_process():
- """Start the SSSD process"""
- if subprocess.call(["sssd", "-D", "-f"]) != 0:
raise Exception("sssd start failed")
+def cleanup_sssd_process():
- """Stop the SSSD process and remove its state"""
- try:
pid_file = open(config.PIDFILE_PATH, "r")
pid = int(pid_file.read())
os.kill(pid, signal.SIGTERM)
while True:
try:
os.kill(pid, signal.SIGCONT)
except:
break
time.sleep(1)
- except:
pass
- for path in os.listdir(config.DB_PATH):
os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
+def create_sssd_cleanup(request):
- """Add teardown for stopping SSSD and removing its state"""
- request.addfinalizer(cleanup_sssd_process)
+def create_sssd_fixture(request):
- """Start SSSD and add teardown for stopping it and removing its state"""
- create_sssd_process()
- create_sssd_cleanup(request)
+@pytest.fixture +def add_nested_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("nested_netgroup")
Is it an appropriate name? I would say it is a empty netgroup The same applies to the name of fixture and name of following test.
Addressed.
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_nested_netgroup(add_nested_netgroup):
- """
- Adding nested netgroup.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("nested_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+@pytest.fixture +def add_two_nested_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("nested_netgroup1")
- ent_list.add_netgroup("nested_netgroup2")
The same here name of netgroups are confusing Or did you forgot to use nested netgroups here?
I really meant empty groups. Addressed.
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_nested_netgroup(add_two_nested_netgroup):
- """
- Adding two nested netgroup.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("nested_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("nested_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+# TODO: Test for removing one by one
Is there a reason for todo? or is this line just a leftover?
It is not conneted to the ticket. But remove tests are missing. I added test_remove_step_by_step().
+@pytest.fixture +def add_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup", ["(host,user,domain)"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_tripled_netgroup(add_tripled_netgroup):
- """
- Adding netgroup with trinity.
^^^^^^^^ I think that netgroups terminology would use triplet.
Triplet sounds better in this context. Addressed.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host", "user", "domain")]
+@pytest.fixture +def add_advanced_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("adv_tripled_netgroup", ["(host1,user1,domain1)",
"(host2,user2,domain2)"])
I would recomment to merge this fixture ( + related test) with previous one. It woul a little bit reduce cuplicated code.
Addressed.
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_advanced_tripled_netgroup(add_advanced_tripled_netgroup):
- """
- Adding netgroup with two trinities.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("adv_tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2")])
+@pytest.fixture +def add_two_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("tripled_netgroup2", ["(host2,user2,domain2)"])
the same as in previous case. The name of netgroups are not conflicting therefore it could be tested in common test (test_simple_netgroups) becasue you are testing just simple netgroups just with triplets and not nested netgroups.
I think this test is redudant after merging previous two.
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_tripled_netgroup(add_two_tripled_netgroup):
- """
- Adding two netgroups, each with trinity.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host2", "user2", "domain2")]
+# TODO: Test for removing one by one
Is there a reason for todo? or is it an leftover?
The same as above.
+@pytest.fixture +def add_mixed_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("mixed_netgroup1", [], [])
- ent_list.add_netgroup("mixed_netgroup2", [], ["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup3", ["(host1,user1,domain1)"], [])
- ent_list.add_netgroup("mixed_netgroup4",
["(host2,user2,domain2)", "(host3,user3,domain3)"],
[])
- ent_list.add_netgroup("mixed_netgroup5",
["(host4,user4,domain4)"],
["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup6",
["(host5,user5,domain5)"],
["mixed_netgroup2"])
- ent_list.add_netgroup("mixed_netgroup7", [], ["mixed_netgroup3"])
- ent_list.add_netgroup("mixed_netgroup8", [],
["mixed_netgroup3", "mixed_netgroup4"])
- ent_list.add_netgroup("mixed_netgroup9",
["(host6,user6,domain6)"],
["mixed_netgroup3", "mixed_netgroup4"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_mixed_netgroup(add_mixed_netgroup):
- """
- Adding many netgroups of different type.
- """
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup4")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup5")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host4", "user4", "domain4")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup6")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host5", "user5", "domain5")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup7")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup8")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup9")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3"),
("host6", "user6", "domain6")])
+@pytest.fixture +def reproducer_t2841(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=False)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_reproducer_t2841(reproducer_t2841, ldap_conn):
It would be good to use more desciptive name. So it is immediately visible in pytest output what you want to test. The pytest output is very reduced for green test. and reproducer_t2841 is not very descriptive.
What about test_removing_nested_netgroups?
Righ, it sounds better. Addressed.
- """
- Regression test for ticket 2841.
- https://fedorahosted.org/sssd/ticket/2841
- """
- t2841_netgroup3_dn = 'cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com'
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of t2841_netgroup1 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup1", "t2841_netgroup2"]}
- new = {'memberNisNetgroup': ["t2841_netgroup2"]}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(t2841_netgroup3_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of t2841_netgroup2 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup2"]}
- new = {'memberNisNetgroup': []}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(t2841_netgroup3_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, errno, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
BTW in all cases you are tesing positive results So the errno variabel is never used. It make sense to check errno just for failures. So you can replace errno with _ which means taht you are intentionaly ingoring the value.
Addressed.
Than you very much for test. I really appreciate it. Now wi will be sure that there woudl not be a regression for the bug and we can refactor code without any worries :-) I had a many comments but they were mostly nitpicking.
One more time thank you very much and good job.
LS
Thanks Lukas for code review. New patch set is attached.
Regards
On (22/08/16 16:39), Petr Cech wrote:
On 08/19/2016 05:18 PM, Lukas Slebodnik wrote:
Thank you for test but I would appraciate a little bit simpler solution. "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) Your patch will append "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) to missing attributes if it is not in netgroup_attrs.
However "memberNisNetgroup" and "originalMemberNisNetgroup" are tightly coupled attributes in sysdb.
- the sysdb attributes "originalMemberNisNetgroup" contain the original value from LDAP which can be name of netgroup of dn.
- the sysdb attributes "memberNisNetgroup" always contain shortname used by sssd.
They are even the same if memberNisNetgroup in LDAP does not contain dn. One is generated from other.
And function list_missing_attrs already find that "originalMemberNisNetgroup" is a missing attribute. Therefore if "originalMemberNisNetgroup" is in missing attibutes list then we have to add there also "memberNisNetgroup".
e.g. diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d9..1fe40f5 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,14 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
}
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
some optional nice explanation/comment why :-)
Hi Lukas,
thanks. I tested your solution, it is valid. I have to agree it is simpler. I fixed my patch in this way.
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
BTW the same bug is also ipa_save_netgroup in "src/providers/ipa/ipa_netgroups.c" But sysdb_add_netgroup is called wit NULL for missing attrinbutes :-)
Anyway sdap_save_netgroup and ipa_save_netgroup do almost the same They just use different maps. You touched the netgroup related code. So it would be good to do small refactoring and reuse ldap sdap_save_netgroup in ipa_save_netgroup. So it would be good If you could fix ticket #3117 in recent future. Because you still remember the netgroup related code in LDAP. (It's not a blocker for #2841) It is just a recomendation :-)
Right, I will take a look.
Thank you in advance.
From 25be35537e0d91af6939c3400340fe01cfb32ea7 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Fri, 22 Jul 2016 14:28:54 +0200 Subject: [PATCH 1/3] LDAP: Fixing of removing netgroup from cache
There were problem with local key which wasn't properly removed. This patch fixes it.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/providers/ldap/sdap_async_netgroups.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d956df70cfcb5f68bd2afc9e2a23c50c3bb..f36ea030414638ae2c778bc532fa09a0b69ed7de 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,18 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
- /* We can get SYSDB_ORIG_NETGROUP_MEMBER, but not SYSDB_NETGROUP_MEMBER
* from LDAP. Thus we add SYSDB_NETGROUP_MEMBER to missing
* if SYSDB_ORIG_NETGROUP_MEMBER is in.
*/
SYSDB_ORIG_NETGROUP_MEMBER is originalMemberNisNetgroup and SYSDB_NETGROUP_MEMBER is memberNisNetgroup
And we get memberNisNetgroup from LDAP but the vale us stored in sysdb attribute originalMemberNisNetgroup. But it may contain simple name or DN. That's the reason why we always translate/generate simple name and store it in SYSDB_NETGROUP_MEMBER(memberNisNetgroup) in sysdb which is internaly used for searching netgropus.
So the comment is a little bit confusing. Because we cannot get originalMemberNisNetgroup from LDAP.
- if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
- }
- ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
From 4ff31b600f01c810367756165500cfdb95f2be2b Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 14:01:09 +0200 Subject: [PATCH 2/3] INTG: Adding support for netgroups to ldap_ent
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/ldap_ent.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index f8f2f7fe6977aec6fd704ad1c78a476a163a16f1..2bb93f8bf605876acfb273088944dcb121e67241 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -87,6 +87,21 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
+def netgroup(base_dn, cn, triples=(), members=()):
- """
- Generate an RFC2307bis netgroup add-modlist for passing to ldap.add*.
- """
- attr_list = [
('objectClass', ['top', 'nisNetgroup'])
- ]
- member_list = []
You forgot to remove this unused aegument.
- if len(triples) > 0:
attr_list.append(('nisNetgroupTriple', triples))
- if len(members) > 0:
attr_list.append(('memberNisNetgroup', members))
When I was updating sssd_netgroup.py I found out there is a simpler way for testing non-empty array :-) if members: attr_list.append(('memberNisNetgroup', members))
- return ("cn=" + cn + ",ou=Netgroups," + base_dn, attr_list)
class List(list): """LDAP add-modlist list"""
@@ -124,3 +139,8 @@ class List(list): self.append(group_bis(base_dn or self.base_dn, cn, gidNumber, member_uids, member_gids))
- def add_netgroup(self, cn, triples=(), members=(), base_dn=None):
"""Add an RFC2307bis netgroup add-modlist."""
self.append(netgroup(base_dn or self.base_dn,
cn, triples, members))
-- 2.7.4
From ccb7aa90a1a63db0b13b60b92afeffc2325af50c Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 3/3] INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 488 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 489 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index d73e4216310ccd1c90e6b7eb0a0e60068fc45bd5..75422a4417046116bec11a8a680fe2248e3afb69 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -15,6 +15,7 @@ dist_noinst_DATA = \ test_ldap.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..3c5dc6f3634ece0669f1748ccbd49b514eeb874f --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,488 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
+import os +import stat +import signal +import subprocess +import time +import ldap +import ldap.modlist +import pytest
+import config +import ds_openldap +import ldap_ent +from util import unindent +import sssd_netgroup as sssd_ntg
+LDAP_BASE_DN = "dc=example,dc=com"
+@pytest.fixture(scope="module") +def ds_inst(request):
- """LDAP server instance fixture"""
- ds_inst = ds_openldap.DSOpenLDAP(
config.PREFIX, 10389, LDAP_BASE_DN,
"cn=admin", "Secret123"
- )
- try:
ds_inst.setup()
- except:
ds_inst.teardown()
raise
- request.addfinalizer(lambda: ds_inst.teardown())
You needn't use lambda if you do not plan to use functions without arguments. - request.addfinalizer(lambda: ds_inst.teardown()) + request.addfinalizer(ds_inst.teardown)
- return ds_inst
+@pytest.fixture(scope="module") +def ldap_conn(request, ds_inst):
- """LDAP server connection fixture"""
- ldap_conn = ds_inst.bind()
- ldap_conn.ds_inst = ds_inst
- request.addfinalizer(lambda: ldap_conn.unbind_s())
The same case here
- return ldap_conn
+def create_ldap_entries(ldap_conn, ent_list=None):
- """Add LDAP entries from ent_list"""
- if ent_list is not None:
for entry in ent_list:
ldap_conn.add_s(entry[0], entry[1])
+def cleanup_ldap_entries(ldap_conn, ent_list=None):
- """Remove LDAP entries added by create_ldap_entries"""
- if ent_list is None:
for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):
for entry in ldap_conn.search_s("ou=" + ou + "," +
ldap_conn.ds_inst.base_dn,
ldap.SCOPE_ONELEVEL,
attrlist=[]):
ldap_conn.delete_s(entry[0])
- else:
for entry in ent_list:
ldap_conn.delete_s(entry[0])
+def create_ldap_cleanup(request, ldap_conn, ent_list=None):
- """Add teardown for removing all user/group LDAP entries"""
- request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+def create_ldap_fixture(request, ldap_conn, ent_list=None):
- """Add LDAP entries and add teardown for removing them"""
- create_ldap_entries(ldap_conn, ent_list)
- create_ldap_cleanup(request, ldap_conn, ent_list)
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+def format_basic_conf(ldap_conn, schema):
- """Format a basic SSSD configuration"""
- schema_conf = "ldap_schema = " + schema + "\n"
- schema_conf += "ldap_group_object_class = groupOfNames\n"
- return unindent("""\
[sssd]
domains = LDAP
services = nss
[domain/LDAP]
{schema_conf}
id_provider = ldap
auth_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
ldap_netgroup_search_base = ou=Netgroups,{ldap_conn.ds_inst.base_dn}
- """).format(**locals())
+def create_conf_file(contents):
- """Create sssd.conf with specified contents"""
- conf = open(config.CONF_PATH, "w")
- conf.write(contents)
- conf.close()
- os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+def cleanup_conf_file():
- """Remove sssd.conf, if it exists"""
- if os.path.lexists(config.CONF_PATH):
os.unlink(config.CONF_PATH)
+def create_conf_cleanup(request):
- """Add teardown for removing sssd.conf"""
- request.addfinalizer(cleanup_conf_file)
+def create_conf_fixture(request, contents):
- """
- Create sssd.conf with specified contents and add teardown for removing it
- """
- create_conf_file(contents)
- create_conf_cleanup(request)
+def create_sssd_process():
- """Start the SSSD process"""
- if subprocess.call(["sssd", "-D", "-f"]) != 0:
raise Exception("sssd start failed")
+def cleanup_sssd_process():
- """Stop the SSSD process and remove its state"""
- try:
pid_file = open(config.PIDFILE_PATH, "r")
pid = int(pid_file.read())
os.kill(pid, signal.SIGTERM)
while True:
try:
os.kill(pid, signal.SIGCONT)
except:
break
time.sleep(1)
- except:
pass
- for path in os.listdir(config.DB_PATH):
os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
+def create_sssd_cleanup(request):
- """Add teardown for stopping SSSD and removing its state"""
- request.addfinalizer(cleanup_sssd_process)
+def create_sssd_fixture(request):
- """Start SSSD and add teardown for stopping it and removing its state"""
- create_sssd_process()
- create_sssd_cleanup(request)
+@pytest.fixture +def add_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_empty_netgroup(add_empty_netgroup):
- """
- Adding empty netgroup.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+@pytest.fixture +def add_two_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup1")
- ent_list.add_netgroup("empty_netgroup2")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_empty_netgroup(add_two_empty_netgroup):
- """
- Adding two empty netgroups.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
If you wanted to test nested empty netgroups then it is already covered in mixed_netgroup (by mixed_netgroup1 and mixed_netgroup2)
Otherwise this test-case is covered by previous test test_add_empty_netgroup
+@pytest.fixture +def add_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup", ["(host,user,domain)"])
- ent_list.add_netgroup("adv_tripled_netgroup", ["(host1,user1,domain1)",
"(host2,user2,domain2)"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_tripled_netgroup(add_tripled_netgroup):
- """
- Adding netgroup with triplet.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host", "user", "domain")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("adv_tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2")])
+@pytest.fixture +def add_mixed_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("mixed_netgroup1", [], [])
- ent_list.add_netgroup("mixed_netgroup2", [], ["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup3", ["(host1,user1,domain1)"], [])
- ent_list.add_netgroup("mixed_netgroup4",
["(host2,user2,domain2)", "(host3,user3,domain3)"],
[])
You can remove empty list from previous 4 function invocations. We can rely on default arguments. The same as in test_add_empty_netgroup
- ent_list.add_netgroup("mixed_netgroup5",
["(host4,user4,domain4)"],
["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup6",
["(host5,user5,domain5)"],
["mixed_netgroup2"])
- ent_list.add_netgroup("mixed_netgroup7", [], ["mixed_netgroup3"])
- ent_list.add_netgroup("mixed_netgroup8", [],
["mixed_netgroup3", "mixed_netgroup4"])
- ent_list.add_netgroup("mixed_netgroup9",
["(host6,user6,domain6)"],
["mixed_netgroup3", "mixed_netgroup4"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_mixed_netgroup(add_mixed_netgroup):
- """
- Adding many netgroups of different type.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup4")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup5")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host4", "user4", "domain4")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup6")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host5", "user5", "domain5")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup7")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup8")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup9")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3"),
("host6", "user6", "domain6")])
+@pytest.fixture +def remove_step_by_step(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("rm_empty_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("rm_empty_netgroup2",
["(host2,user2,domain2)"],
["rm_empty_netgroup1"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return ent_list
+def test_remove_step_by_step(remove_step_by_step, ldap_conn):
- """
- Removing netgroups step by step.
- """
- ent_list = remove_step_by_step
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of rm_empty_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of rm_empty_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
+@pytest.fixture +def removing_nested_netgroups(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_removing_nested_netgroups(removing_nested_netgroups, ldap_conn):
- """
- Regression test for ticket 2841.
- https://fedorahosted.org/sssd/ticket/2841
- """
- netgroup_dn = 'cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com'
^^^^^^^^^^^^^^^^^ It would be good to use ldap_conn.ds_inst.base_dn So after changind base_dn test will not fail
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of t2841_netgroup1 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup1", "t2841_netgroup2"]}
- new = {'memberNisNetgroup': ["t2841_netgroup2"]}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of t2841_netgroup2 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup2"]}
- new = {'memberNisNetgroup': []}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
LS
On 08/24/2016 09:15 AM, Lukas Slebodnik wrote:
On (22/08/16 16:39), Petr Cech wrote:
On 08/19/2016 05:18 PM, Lukas Slebodnik wrote:
Thank you for test but I would appraciate a little bit simpler solution. "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) Your patch will append "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) to missing attributes if it is not in netgroup_attrs.
However "memberNisNetgroup" and "originalMemberNisNetgroup" are tightly coupled attributes in sysdb.
- the sysdb attributes "originalMemberNisNetgroup" contain the original value from LDAP which can be name of netgroup of dn.
- the sysdb attributes "memberNisNetgroup" always contain shortname used by sssd.
They are even the same if memberNisNetgroup in LDAP does not contain dn. One is generated from other.
And function list_missing_attrs already find that "originalMemberNisNetgroup" is a missing attribute. Therefore if "originalMemberNisNetgroup" is in missing attibutes list then we have to add there also "memberNisNetgroup".
e.g. diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d9..1fe40f5 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,14 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
}
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
some optional nice explanation/comment why :-)
Hi Lukas,
thanks. I tested your solution, it is valid. I have to agree it is simpler. I fixed my patch in this way.
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
BTW the same bug is also ipa_save_netgroup in "src/providers/ipa/ipa_netgroups.c" But sysdb_add_netgroup is called wit NULL for missing attrinbutes :-)
Anyway sdap_save_netgroup and ipa_save_netgroup do almost the same They just use different maps. You touched the netgroup related code. So it would be good to do small refactoring and reuse ldap sdap_save_netgroup in ipa_save_netgroup. So it would be good If you could fix ticket #3117 in recent future. Because you still remember the netgroup related code in LDAP. (It's not a blocker for #2841) It is just a recomendation :-)
Right, I will take a look.
Thank you in advance.
From 25be35537e0d91af6939c3400340fe01cfb32ea7 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Fri, 22 Jul 2016 14:28:54 +0200 Subject: [PATCH 1/3] LDAP: Fixing of removing netgroup from cache
There were problem with local key which wasn't properly removed. This patch fixes it.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/providers/ldap/sdap_async_netgroups.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d956df70cfcb5f68bd2afc9e2a23c50c3bb..f36ea030414638ae2c778bc532fa09a0b69ed7de 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,18 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
- /* We can get SYSDB_ORIG_NETGROUP_MEMBER, but not SYSDB_NETGROUP_MEMBER
* from LDAP. Thus we add SYSDB_NETGROUP_MEMBER to missing
* if SYSDB_ORIG_NETGROUP_MEMBER is in.
*/
SYSDB_ORIG_NETGROUP_MEMBER is originalMemberNisNetgroup and SYSDB_NETGROUP_MEMBER is memberNisNetgroup
And we get memberNisNetgroup from LDAP but the vale us stored in sysdb attribute originalMemberNisNetgroup. But it may contain simple name or DN. That's the reason why we always translate/generate simple name and store it in SYSDB_NETGROUP_MEMBER(memberNisNetgroup) in sysdb which is internaly used for searching netgropus.
So the comment is a little bit confusing. Because we cannot get originalMemberNisNetgroup from LDAP.
- if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
- }
- ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
From 4ff31b600f01c810367756165500cfdb95f2be2b Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 14:01:09 +0200 Subject: [PATCH 2/3] INTG: Adding support for netgroups to ldap_ent
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/ldap_ent.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index f8f2f7fe6977aec6fd704ad1c78a476a163a16f1..2bb93f8bf605876acfb273088944dcb121e67241 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -87,6 +87,21 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
+def netgroup(base_dn, cn, triples=(), members=()):
- """
- Generate an RFC2307bis netgroup add-modlist for passing to ldap.add*.
- """
- attr_list = [
('objectClass', ['top', 'nisNetgroup'])
- ]
- member_list = []
You forgot to remove this unused aegument.
- if len(triples) > 0:
attr_list.append(('nisNetgroupTriple', triples))
- if len(members) > 0:
attr_list.append(('memberNisNetgroup', members))
When I was updating sssd_netgroup.py I found out there is a simpler way for testing non-empty array :-) if members: attr_list.append(('memberNisNetgroup', members))
- return ("cn=" + cn + ",ou=Netgroups," + base_dn, attr_list)
class List(list): """LDAP add-modlist list"""
@@ -124,3 +139,8 @@ class List(list): self.append(group_bis(base_dn or self.base_dn, cn, gidNumber, member_uids, member_gids))
- def add_netgroup(self, cn, triples=(), members=(), base_dn=None):
"""Add an RFC2307bis netgroup add-modlist."""
self.append(netgroup(base_dn or self.base_dn,
cn, triples, members))
-- 2.7.4
From ccb7aa90a1a63db0b13b60b92afeffc2325af50c Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 3/3] INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 488 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 489 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index d73e4216310ccd1c90e6b7eb0a0e60068fc45bd5..75422a4417046116bec11a8a680fe2248e3afb69 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -15,6 +15,7 @@ dist_noinst_DATA = \ test_ldap.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..3c5dc6f3634ece0669f1748ccbd49b514eeb874f --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,488 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
+import os +import stat +import signal +import subprocess +import time +import ldap +import ldap.modlist +import pytest
+import config +import ds_openldap +import ldap_ent +from util import unindent +import sssd_netgroup as sssd_ntg
+LDAP_BASE_DN = "dc=example,dc=com"
+@pytest.fixture(scope="module") +def ds_inst(request):
- """LDAP server instance fixture"""
- ds_inst = ds_openldap.DSOpenLDAP(
config.PREFIX, 10389, LDAP_BASE_DN,
"cn=admin", "Secret123"
- )
- try:
ds_inst.setup()
- except:
ds_inst.teardown()
raise
- request.addfinalizer(lambda: ds_inst.teardown())
You needn't use lambda if you do not plan to use functions without arguments.
- request.addfinalizer(lambda: ds_inst.teardown())
- request.addfinalizer(ds_inst.teardown)
- return ds_inst
+@pytest.fixture(scope="module") +def ldap_conn(request, ds_inst):
- """LDAP server connection fixture"""
- ldap_conn = ds_inst.bind()
- ldap_conn.ds_inst = ds_inst
- request.addfinalizer(lambda: ldap_conn.unbind_s())
The same case here
- return ldap_conn
+def create_ldap_entries(ldap_conn, ent_list=None):
- """Add LDAP entries from ent_list"""
- if ent_list is not None:
for entry in ent_list:
ldap_conn.add_s(entry[0], entry[1])
+def cleanup_ldap_entries(ldap_conn, ent_list=None):
- """Remove LDAP entries added by create_ldap_entries"""
- if ent_list is None:
for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):
for entry in ldap_conn.search_s("ou=" + ou + "," +
ldap_conn.ds_inst.base_dn,
ldap.SCOPE_ONELEVEL,
attrlist=[]):
ldap_conn.delete_s(entry[0])
- else:
for entry in ent_list:
ldap_conn.delete_s(entry[0])
+def create_ldap_cleanup(request, ldap_conn, ent_list=None):
- """Add teardown for removing all user/group LDAP entries"""
- request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+def create_ldap_fixture(request, ldap_conn, ent_list=None):
- """Add LDAP entries and add teardown for removing them"""
- create_ldap_entries(ldap_conn, ent_list)
- create_ldap_cleanup(request, ldap_conn, ent_list)
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+def format_basic_conf(ldap_conn, schema):
- """Format a basic SSSD configuration"""
- schema_conf = "ldap_schema = " + schema + "\n"
- schema_conf += "ldap_group_object_class = groupOfNames\n"
- return unindent("""\
[sssd]
domains = LDAP
services = nss
[domain/LDAP]
{schema_conf}
id_provider = ldap
auth_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
ldap_netgroup_search_base = ou=Netgroups,{ldap_conn.ds_inst.base_dn}
- """).format(**locals())
+def create_conf_file(contents):
- """Create sssd.conf with specified contents"""
- conf = open(config.CONF_PATH, "w")
- conf.write(contents)
- conf.close()
- os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+def cleanup_conf_file():
- """Remove sssd.conf, if it exists"""
- if os.path.lexists(config.CONF_PATH):
os.unlink(config.CONF_PATH)
+def create_conf_cleanup(request):
- """Add teardown for removing sssd.conf"""
- request.addfinalizer(cleanup_conf_file)
+def create_conf_fixture(request, contents):
- """
- Create sssd.conf with specified contents and add teardown for removing it
- """
- create_conf_file(contents)
- create_conf_cleanup(request)
+def create_sssd_process():
- """Start the SSSD process"""
- if subprocess.call(["sssd", "-D", "-f"]) != 0:
raise Exception("sssd start failed")
+def cleanup_sssd_process():
- """Stop the SSSD process and remove its state"""
- try:
pid_file = open(config.PIDFILE_PATH, "r")
pid = int(pid_file.read())
os.kill(pid, signal.SIGTERM)
while True:
try:
os.kill(pid, signal.SIGCONT)
except:
break
time.sleep(1)
- except:
pass
- for path in os.listdir(config.DB_PATH):
os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
+def create_sssd_cleanup(request):
- """Add teardown for stopping SSSD and removing its state"""
- request.addfinalizer(cleanup_sssd_process)
+def create_sssd_fixture(request):
- """Start SSSD and add teardown for stopping it and removing its state"""
- create_sssd_process()
- create_sssd_cleanup(request)
+@pytest.fixture +def add_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_empty_netgroup(add_empty_netgroup):
- """
- Adding empty netgroup.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+@pytest.fixture +def add_two_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup1")
- ent_list.add_netgroup("empty_netgroup2")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_empty_netgroup(add_two_empty_netgroup):
- """
- Adding two empty netgroups.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
If you wanted to test nested empty netgroups then it is already covered in mixed_netgroup (by mixed_netgroup1 and mixed_netgroup2)
Otherwise this test-case is covered by previous test test_add_empty_netgroup
+@pytest.fixture +def add_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup", ["(host,user,domain)"])
- ent_list.add_netgroup("adv_tripled_netgroup", ["(host1,user1,domain1)",
"(host2,user2,domain2)"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_tripled_netgroup(add_tripled_netgroup):
- """
- Adding netgroup with triplet.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host", "user", "domain")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("adv_tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2")])
+@pytest.fixture +def add_mixed_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("mixed_netgroup1", [], [])
- ent_list.add_netgroup("mixed_netgroup2", [], ["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup3", ["(host1,user1,domain1)"], [])
- ent_list.add_netgroup("mixed_netgroup4",
["(host2,user2,domain2)", "(host3,user3,domain3)"],
[])
You can remove empty list from previous 4 function invocations. We can rely on default arguments. The same as in test_add_empty_netgroup
- ent_list.add_netgroup("mixed_netgroup5",
["(host4,user4,domain4)"],
["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup6",
["(host5,user5,domain5)"],
["mixed_netgroup2"])
- ent_list.add_netgroup("mixed_netgroup7", [], ["mixed_netgroup3"])
- ent_list.add_netgroup("mixed_netgroup8", [],
["mixed_netgroup3", "mixed_netgroup4"])
- ent_list.add_netgroup("mixed_netgroup9",
["(host6,user6,domain6)"],
["mixed_netgroup3", "mixed_netgroup4"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_mixed_netgroup(add_mixed_netgroup):
- """
- Adding many netgroups of different type.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup4")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup5")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host4", "user4", "domain4")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup6")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host5", "user5", "domain5")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup7")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup8")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup9")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3"),
("host6", "user6", "domain6")])
+@pytest.fixture +def remove_step_by_step(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("rm_empty_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("rm_empty_netgroup2",
["(host2,user2,domain2)"],
["rm_empty_netgroup1"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return ent_list
+def test_remove_step_by_step(remove_step_by_step, ldap_conn):
- """
- Removing netgroups step by step.
- """
- ent_list = remove_step_by_step
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of rm_empty_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of rm_empty_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
+@pytest.fixture +def removing_nested_netgroups(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_removing_nested_netgroups(removing_nested_netgroups, ldap_conn):
- """
- Regression test for ticket 2841.
- https://fedorahosted.org/sssd/ticket/2841
- """
- netgroup_dn = 'cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com'
^^^^^^^^^^^^^^^^^ It would be good to use ldap_conn.ds_inst.base_dn So after changind base_dn test will not fail
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of t2841_netgroup1 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup1", "t2841_netgroup2"]}
- new = {'memberNisNetgroup': ["t2841_netgroup2"]}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of t2841_netgroup2 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup2"]}
- new = {'memberNisNetgroup': []}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
LS
Hi Lukas,
thanks for review. I hope that I addressed all notes. New version is attached.
Regards
On (24/08/16 10:53), Petr Cech wrote:
On 08/24/2016 09:15 AM, Lukas Slebodnik wrote:
On (22/08/16 16:39), Petr Cech wrote:
On 08/19/2016 05:18 PM, Lukas Slebodnik wrote:
Thank you for test but I would appraciate a little bit simpler solution. "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) Your patch will append "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) to missing attributes if it is not in netgroup_attrs.
However "memberNisNetgroup" and "originalMemberNisNetgroup" are tightly coupled attributes in sysdb.
- the sysdb attributes "originalMemberNisNetgroup" contain the original value from LDAP which can be name of netgroup of dn.
- the sysdb attributes "memberNisNetgroup" always contain shortname used by sssd.
They are even the same if memberNisNetgroup in LDAP does not contain dn. One is generated from other.
And function list_missing_attrs already find that "originalMemberNisNetgroup" is a missing attribute. Therefore if "originalMemberNisNetgroup" is in missing attibutes list then we have to add there also "memberNisNetgroup".
e.g. diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d9..1fe40f5 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,14 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
}
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
some optional nice explanation/comment why :-)
Hi Lukas,
thanks. I tested your solution, it is valid. I have to agree it is simpler. I fixed my patch in this way.
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
BTW the same bug is also ipa_save_netgroup in "src/providers/ipa/ipa_netgroups.c" But sysdb_add_netgroup is called wit NULL for missing attrinbutes :-)
Anyway sdap_save_netgroup and ipa_save_netgroup do almost the same They just use different maps. You touched the netgroup related code. So it would be good to do small refactoring and reuse ldap sdap_save_netgroup in ipa_save_netgroup. So it would be good If you could fix ticket #3117 in recent future. Because you still remember the netgroup related code in LDAP. (It's not a blocker for #2841) It is just a recomendation :-)
Right, I will take a look.
Thank you in advance.
From 25be35537e0d91af6939c3400340fe01cfb32ea7 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Fri, 22 Jul 2016 14:28:54 +0200 Subject: [PATCH 1/3] LDAP: Fixing of removing netgroup from cache
There were problem with local key which wasn't properly removed. This patch fixes it.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/providers/ldap/sdap_async_netgroups.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d956df70cfcb5f68bd2afc9e2a23c50c3bb..f36ea030414638ae2c778bc532fa09a0b69ed7de 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,18 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
- /* We can get SYSDB_ORIG_NETGROUP_MEMBER, but not SYSDB_NETGROUP_MEMBER
* from LDAP. Thus we add SYSDB_NETGROUP_MEMBER to missing
* if SYSDB_ORIG_NETGROUP_MEMBER is in.
*/
SYSDB_ORIG_NETGROUP_MEMBER is originalMemberNisNetgroup and SYSDB_NETGROUP_MEMBER is memberNisNetgroup
And we get memberNisNetgroup from LDAP but the vale us stored in sysdb attribute originalMemberNisNetgroup. But it may contain simple name or DN. That's the reason why we always translate/generate simple name and store it in SYSDB_NETGROUP_MEMBER(memberNisNetgroup) in sysdb which is internaly used for searching netgropus.
So the comment is a little bit confusing. Because we cannot get originalMemberNisNetgroup from LDAP.
- if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
- }
- ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
From 4ff31b600f01c810367756165500cfdb95f2be2b Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 14:01:09 +0200 Subject: [PATCH 2/3] INTG: Adding support for netgroups to ldap_ent
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/ldap_ent.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index f8f2f7fe6977aec6fd704ad1c78a476a163a16f1..2bb93f8bf605876acfb273088944dcb121e67241 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -87,6 +87,21 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
+def netgroup(base_dn, cn, triples=(), members=()):
- """
- Generate an RFC2307bis netgroup add-modlist for passing to ldap.add*.
- """
- attr_list = [
('objectClass', ['top', 'nisNetgroup'])
- ]
- member_list = []
You forgot to remove this unused aegument.
- if len(triples) > 0:
attr_list.append(('nisNetgroupTriple', triples))
- if len(members) > 0:
attr_list.append(('memberNisNetgroup', members))
When I was updating sssd_netgroup.py I found out there is a simpler way for testing non-empty array :-) if members: attr_list.append(('memberNisNetgroup', members))
- return ("cn=" + cn + ",ou=Netgroups," + base_dn, attr_list)
class List(list): """LDAP add-modlist list"""
@@ -124,3 +139,8 @@ class List(list): self.append(group_bis(base_dn or self.base_dn, cn, gidNumber, member_uids, member_gids))
- def add_netgroup(self, cn, triples=(), members=(), base_dn=None):
"""Add an RFC2307bis netgroup add-modlist."""
self.append(netgroup(base_dn or self.base_dn,
cn, triples, members))
-- 2.7.4
From ccb7aa90a1a63db0b13b60b92afeffc2325af50c Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 3/3] INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 488 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 489 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index d73e4216310ccd1c90e6b7eb0a0e60068fc45bd5..75422a4417046116bec11a8a680fe2248e3afb69 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -15,6 +15,7 @@ dist_noinst_DATA = \ test_ldap.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..3c5dc6f3634ece0669f1748ccbd49b514eeb874f --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,488 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
+import os +import stat +import signal +import subprocess +import time +import ldap +import ldap.modlist +import pytest
+import config +import ds_openldap +import ldap_ent +from util import unindent +import sssd_netgroup as sssd_ntg
+LDAP_BASE_DN = "dc=example,dc=com"
+@pytest.fixture(scope="module") +def ds_inst(request):
- """LDAP server instance fixture"""
- ds_inst = ds_openldap.DSOpenLDAP(
config.PREFIX, 10389, LDAP_BASE_DN,
"cn=admin", "Secret123"
- )
- try:
ds_inst.setup()
- except:
ds_inst.teardown()
raise
- request.addfinalizer(lambda: ds_inst.teardown())
You needn't use lambda if you do not plan to use functions without arguments.
- request.addfinalizer(lambda: ds_inst.teardown())
- request.addfinalizer(ds_inst.teardown)
- return ds_inst
+@pytest.fixture(scope="module") +def ldap_conn(request, ds_inst):
- """LDAP server connection fixture"""
- ldap_conn = ds_inst.bind()
- ldap_conn.ds_inst = ds_inst
- request.addfinalizer(lambda: ldap_conn.unbind_s())
The same case here
- return ldap_conn
+def create_ldap_entries(ldap_conn, ent_list=None):
- """Add LDAP entries from ent_list"""
- if ent_list is not None:
for entry in ent_list:
ldap_conn.add_s(entry[0], entry[1])
+def cleanup_ldap_entries(ldap_conn, ent_list=None):
- """Remove LDAP entries added by create_ldap_entries"""
- if ent_list is None:
for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):
for entry in ldap_conn.search_s("ou=" + ou + "," +
ldap_conn.ds_inst.base_dn,
ldap.SCOPE_ONELEVEL,
attrlist=[]):
ldap_conn.delete_s(entry[0])
- else:
for entry in ent_list:
ldap_conn.delete_s(entry[0])
+def create_ldap_cleanup(request, ldap_conn, ent_list=None):
- """Add teardown for removing all user/group LDAP entries"""
- request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+def create_ldap_fixture(request, ldap_conn, ent_list=None):
- """Add LDAP entries and add teardown for removing them"""
- create_ldap_entries(ldap_conn, ent_list)
- create_ldap_cleanup(request, ldap_conn, ent_list)
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+def format_basic_conf(ldap_conn, schema):
- """Format a basic SSSD configuration"""
- schema_conf = "ldap_schema = " + schema + "\n"
- schema_conf += "ldap_group_object_class = groupOfNames\n"
- return unindent("""\
[sssd]
domains = LDAP
services = nss
[domain/LDAP]
{schema_conf}
id_provider = ldap
auth_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
ldap_netgroup_search_base = ou=Netgroups,{ldap_conn.ds_inst.base_dn}
- """).format(**locals())
+def create_conf_file(contents):
- """Create sssd.conf with specified contents"""
- conf = open(config.CONF_PATH, "w")
- conf.write(contents)
- conf.close()
- os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+def cleanup_conf_file():
- """Remove sssd.conf, if it exists"""
- if os.path.lexists(config.CONF_PATH):
os.unlink(config.CONF_PATH)
+def create_conf_cleanup(request):
- """Add teardown for removing sssd.conf"""
- request.addfinalizer(cleanup_conf_file)
+def create_conf_fixture(request, contents):
- """
- Create sssd.conf with specified contents and add teardown for removing it
- """
- create_conf_file(contents)
- create_conf_cleanup(request)
+def create_sssd_process():
- """Start the SSSD process"""
- if subprocess.call(["sssd", "-D", "-f"]) != 0:
raise Exception("sssd start failed")
+def cleanup_sssd_process():
- """Stop the SSSD process and remove its state"""
- try:
pid_file = open(config.PIDFILE_PATH, "r")
pid = int(pid_file.read())
os.kill(pid, signal.SIGTERM)
while True:
try:
os.kill(pid, signal.SIGCONT)
except:
break
time.sleep(1)
- except:
pass
- for path in os.listdir(config.DB_PATH):
os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
+def create_sssd_cleanup(request):
- """Add teardown for stopping SSSD and removing its state"""
- request.addfinalizer(cleanup_sssd_process)
+def create_sssd_fixture(request):
- """Start SSSD and add teardown for stopping it and removing its state"""
- create_sssd_process()
- create_sssd_cleanup(request)
+@pytest.fixture +def add_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_empty_netgroup(add_empty_netgroup):
- """
- Adding empty netgroup.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+@pytest.fixture +def add_two_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup1")
- ent_list.add_netgroup("empty_netgroup2")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_empty_netgroup(add_two_empty_netgroup):
- """
- Adding two empty netgroups.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
If you wanted to test nested empty netgroups then it is already covered in mixed_netgroup (by mixed_netgroup1 and mixed_netgroup2)
Otherwise this test-case is covered by previous test test_add_empty_netgroup
+@pytest.fixture +def add_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup", ["(host,user,domain)"])
- ent_list.add_netgroup("adv_tripled_netgroup", ["(host1,user1,domain1)",
"(host2,user2,domain2)"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_tripled_netgroup(add_tripled_netgroup):
- """
- Adding netgroup with triplet.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host", "user", "domain")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("adv_tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2")])
+@pytest.fixture +def add_mixed_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("mixed_netgroup1", [], [])
- ent_list.add_netgroup("mixed_netgroup2", [], ["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup3", ["(host1,user1,domain1)"], [])
- ent_list.add_netgroup("mixed_netgroup4",
["(host2,user2,domain2)", "(host3,user3,domain3)"],
[])
You can remove empty list from previous 4 function invocations. We can rely on default arguments. The same as in test_add_empty_netgroup
- ent_list.add_netgroup("mixed_netgroup5",
["(host4,user4,domain4)"],
["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup6",
["(host5,user5,domain5)"],
["mixed_netgroup2"])
- ent_list.add_netgroup("mixed_netgroup7", [], ["mixed_netgroup3"])
- ent_list.add_netgroup("mixed_netgroup8", [],
["mixed_netgroup3", "mixed_netgroup4"])
- ent_list.add_netgroup("mixed_netgroup9",
["(host6,user6,domain6)"],
["mixed_netgroup3", "mixed_netgroup4"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_mixed_netgroup(add_mixed_netgroup):
- """
- Adding many netgroups of different type.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup4")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup5")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host4", "user4", "domain4")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup6")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host5", "user5", "domain5")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup7")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup8")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup9")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3"),
("host6", "user6", "domain6")])
+@pytest.fixture +def remove_step_by_step(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("rm_empty_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("rm_empty_netgroup2",
["(host2,user2,domain2)"],
["rm_empty_netgroup1"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return ent_list
+def test_remove_step_by_step(remove_step_by_step, ldap_conn):
- """
- Removing netgroups step by step.
- """
- ent_list = remove_step_by_step
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of rm_empty_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of rm_empty_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
+@pytest.fixture +def removing_nested_netgroups(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_removing_nested_netgroups(removing_nested_netgroups, ldap_conn):
- """
- Regression test for ticket 2841.
- https://fedorahosted.org/sssd/ticket/2841
- """
- netgroup_dn = 'cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com'
^^^^^^^^^^^^^^^^^ It would be good to use ldap_conn.ds_inst.base_dn So after changind base_dn test will not fail
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of t2841_netgroup1 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup1", "t2841_netgroup2"]}
- new = {'memberNisNetgroup': ["t2841_netgroup2"]}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of t2841_netgroup2 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup2"]}
- new = {'memberNisNetgroup': []}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
LS
Hi Lukas,
thanks for review. I hope that I addressed all notes. New version is attached.
ACK
http://sssd-ci.duckdns.org/logs/job/52/28/summary.html
LS
On (24/08/16 13:46), Lukas Slebodnik wrote:
On (24/08/16 10:53), Petr Cech wrote:
On 08/24/2016 09:15 AM, Lukas Slebodnik wrote:
On (22/08/16 16:39), Petr Cech wrote:
On 08/19/2016 05:18 PM, Lukas Slebodnik wrote:
Thank you for test but I would appraciate a little bit simpler solution. "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) Your patch will append "memberNisNetgroup"(SYSDB_NETGROUP_MEMBER) to missing attributes if it is not in netgroup_attrs.
However "memberNisNetgroup" and "originalMemberNisNetgroup" are tightly coupled attributes in sysdb.
- the sysdb attributes "originalMemberNisNetgroup" contain the original value from LDAP which can be name of netgroup of dn.
- the sysdb attributes "memberNisNetgroup" always contain shortname used by sssd.
They are even the same if memberNisNetgroup in LDAP does not contain dn. One is generated from other.
And function list_missing_attrs already find that "originalMemberNisNetgroup" is a missing attribute. Therefore if "originalMemberNisNetgroup" is in missing attibutes list then we have to add there also "memberNisNetgroup".
e.g. diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d9..1fe40f5 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,14 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
}
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
some optional nice explanation/comment why :-)
Hi Lukas,
thanks. I tested your solution, it is valid. I have to agree it is simpler. I fixed my patch in this way.
ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
BTW the same bug is also ipa_save_netgroup in "src/providers/ipa/ipa_netgroups.c" But sysdb_add_netgroup is called wit NULL for missing attrinbutes :-)
Anyway sdap_save_netgroup and ipa_save_netgroup do almost the same They just use different maps. You touched the netgroup related code. So it would be good to do small refactoring and reuse ldap sdap_save_netgroup in ipa_save_netgroup. So it would be good If you could fix ticket #3117 in recent future. Because you still remember the netgroup related code in LDAP. (It's not a blocker for #2841) It is just a recomendation :-)
Right, I will take a look.
Thank you in advance.
From 25be35537e0d91af6939c3400340fe01cfb32ea7 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Fri, 22 Jul 2016 14:28:54 +0200 Subject: [PATCH 1/3] LDAP: Fixing of removing netgroup from cache
There were problem with local key which wasn't properly removed. This patch fixes it.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/providers/ldap/sdap_async_netgroups.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/src/providers/ldap/sdap_async_netgroups.c b/src/providers/ldap/sdap_async_netgroups.c index df233d956df70cfcb5f68bd2afc9e2a23c50c3bb..f36ea030414638ae2c778bc532fa09a0b69ed7de 100644 --- a/src/providers/ldap/sdap_async_netgroups.c +++ b/src/providers/ldap/sdap_async_netgroups.c @@ -138,6 +138,18 @@ static errno_t sdap_save_netgroup(TALLOC_CTX *memctx, goto fail; }
- /* We can get SYSDB_ORIG_NETGROUP_MEMBER, but not SYSDB_NETGROUP_MEMBER
* from LDAP. Thus we add SYSDB_NETGROUP_MEMBER to missing
* if SYSDB_ORIG_NETGROUP_MEMBER is in.
*/
SYSDB_ORIG_NETGROUP_MEMBER is originalMemberNisNetgroup and SYSDB_NETGROUP_MEMBER is memberNisNetgroup
And we get memberNisNetgroup from LDAP but the vale us stored in sysdb attribute originalMemberNisNetgroup. But it may contain simple name or DN. That's the reason why we always translate/generate simple name and store it in SYSDB_NETGROUP_MEMBER(memberNisNetgroup) in sysdb which is internaly used for searching netgropus.
So the comment is a little bit confusing. Because we cannot get originalMemberNisNetgroup from LDAP.
- if (string_in_list(SYSDB_ORIG_NETGROUP_MEMBER, missing, false)) {
ret = add_string_to_list(attrs, SYSDB_NETGROUP_MEMBER, &missing);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add string into list\n");
goto fail;
}
- }
- ret = sysdb_add_netgroup(dom, name, NULL, netgroup_attrs, missing, dom->netgroup_timeout, now); if (ret) goto fail;
-- 2.7.4
From 4ff31b600f01c810367756165500cfdb95f2be2b Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 14:01:09 +0200 Subject: [PATCH 2/3] INTG: Adding support for netgroups to ldap_ent
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/ldap_ent.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index f8f2f7fe6977aec6fd704ad1c78a476a163a16f1..2bb93f8bf605876acfb273088944dcb121e67241 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -87,6 +87,21 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
+def netgroup(base_dn, cn, triples=(), members=()):
- """
- Generate an RFC2307bis netgroup add-modlist for passing to ldap.add*.
- """
- attr_list = [
('objectClass', ['top', 'nisNetgroup'])
- ]
- member_list = []
You forgot to remove this unused aegument.
- if len(triples) > 0:
attr_list.append(('nisNetgroupTriple', triples))
- if len(members) > 0:
attr_list.append(('memberNisNetgroup', members))
When I was updating sssd_netgroup.py I found out there is a simpler way for testing non-empty array :-) if members: attr_list.append(('memberNisNetgroup', members))
- return ("cn=" + cn + ",ou=Netgroups," + base_dn, attr_list)
class List(list): """LDAP add-modlist list"""
@@ -124,3 +139,8 @@ class List(list): self.append(group_bis(base_dn or self.base_dn, cn, gidNumber, member_uids, member_gids))
- def add_netgroup(self, cn, triples=(), members=(), base_dn=None):
"""Add an RFC2307bis netgroup add-modlist."""
self.append(netgroup(base_dn or self.base_dn,
cn, triples, members))
-- 2.7.4
From ccb7aa90a1a63db0b13b60b92afeffc2325af50c Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Wed, 17 Aug 2016 13:58:30 +0200 Subject: [PATCH 3/3] INTG: Tests for ldap nested netgroups
This patch adds tests on reproducer of t2841.
Resolves: https://fedorahosted.org/sssd/ticket/2841
src/tests/intg/Makefile.am | 1 + src/tests/intg/test_netgroup.py | 488 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 489 insertions(+) create mode 100644 src/tests/intg/test_netgroup.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index d73e4216310ccd1c90e6b7eb0a0e60068fc45bd5..75422a4417046116bec11a8a680fe2248e3afb69 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -15,6 +15,7 @@ dist_noinst_DATA = \ test_ldap.py \ test_memory_cache.py \ test_ts_cache.py \
- test_netgroup.py \ $(NULL)
config.py: config.py.m4 diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py new file mode 100644 index 0000000000000000000000000000000000000000..3c5dc6f3634ece0669f1748ccbd49b514eeb874f --- /dev/null +++ b/src/tests/intg/test_netgroup.py @@ -0,0 +1,488 @@ +# +# Netgroup integration test +# +# Copyright (c) 2016 Red Hat, Inc. +# Author: Petr Cech pcech@redhat.com +# +# This is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 only +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +#
+import os +import stat +import signal +import subprocess +import time +import ldap +import ldap.modlist +import pytest
+import config +import ds_openldap +import ldap_ent +from util import unindent +import sssd_netgroup as sssd_ntg
+LDAP_BASE_DN = "dc=example,dc=com"
+@pytest.fixture(scope="module") +def ds_inst(request):
- """LDAP server instance fixture"""
- ds_inst = ds_openldap.DSOpenLDAP(
config.PREFIX, 10389, LDAP_BASE_DN,
"cn=admin", "Secret123"
- )
- try:
ds_inst.setup()
- except:
ds_inst.teardown()
raise
- request.addfinalizer(lambda: ds_inst.teardown())
You needn't use lambda if you do not plan to use functions without arguments.
- request.addfinalizer(lambda: ds_inst.teardown())
- request.addfinalizer(ds_inst.teardown)
- return ds_inst
+@pytest.fixture(scope="module") +def ldap_conn(request, ds_inst):
- """LDAP server connection fixture"""
- ldap_conn = ds_inst.bind()
- ldap_conn.ds_inst = ds_inst
- request.addfinalizer(lambda: ldap_conn.unbind_s())
The same case here
- return ldap_conn
+def create_ldap_entries(ldap_conn, ent_list=None):
- """Add LDAP entries from ent_list"""
- if ent_list is not None:
for entry in ent_list:
ldap_conn.add_s(entry[0], entry[1])
+def cleanup_ldap_entries(ldap_conn, ent_list=None):
- """Remove LDAP entries added by create_ldap_entries"""
- if ent_list is None:
for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):
for entry in ldap_conn.search_s("ou=" + ou + "," +
ldap_conn.ds_inst.base_dn,
ldap.SCOPE_ONELEVEL,
attrlist=[]):
ldap_conn.delete_s(entry[0])
- else:
for entry in ent_list:
ldap_conn.delete_s(entry[0])
+def create_ldap_cleanup(request, ldap_conn, ent_list=None):
- """Add teardown for removing all user/group LDAP entries"""
- request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+def create_ldap_fixture(request, ldap_conn, ent_list=None):
- """Add LDAP entries and add teardown for removing them"""
- create_ldap_entries(ldap_conn, ent_list)
- create_ldap_cleanup(request, ldap_conn, ent_list)
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+def format_basic_conf(ldap_conn, schema):
- """Format a basic SSSD configuration"""
- schema_conf = "ldap_schema = " + schema + "\n"
- schema_conf += "ldap_group_object_class = groupOfNames\n"
- return unindent("""\
[sssd]
domains = LDAP
services = nss
[domain/LDAP]
{schema_conf}
id_provider = ldap
auth_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
ldap_netgroup_search_base = ou=Netgroups,{ldap_conn.ds_inst.base_dn}
- """).format(**locals())
+def create_conf_file(contents):
- """Create sssd.conf with specified contents"""
- conf = open(config.CONF_PATH, "w")
- conf.write(contents)
- conf.close()
- os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+def cleanup_conf_file():
- """Remove sssd.conf, if it exists"""
- if os.path.lexists(config.CONF_PATH):
os.unlink(config.CONF_PATH)
+def create_conf_cleanup(request):
- """Add teardown for removing sssd.conf"""
- request.addfinalizer(cleanup_conf_file)
+def create_conf_fixture(request, contents):
- """
- Create sssd.conf with specified contents and add teardown for removing it
- """
- create_conf_file(contents)
- create_conf_cleanup(request)
+def create_sssd_process():
- """Start the SSSD process"""
- if subprocess.call(["sssd", "-D", "-f"]) != 0:
raise Exception("sssd start failed")
+def cleanup_sssd_process():
- """Stop the SSSD process and remove its state"""
- try:
pid_file = open(config.PIDFILE_PATH, "r")
pid = int(pid_file.read())
os.kill(pid, signal.SIGTERM)
while True:
try:
os.kill(pid, signal.SIGCONT)
except:
break
time.sleep(1)
- except:
pass
- for path in os.listdir(config.DB_PATH):
os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
+def create_sssd_cleanup(request):
- """Add teardown for stopping SSSD and removing its state"""
- request.addfinalizer(cleanup_sssd_process)
+def create_sssd_fixture(request):
- """Start SSSD and add teardown for stopping it and removing its state"""
- create_sssd_process()
- create_sssd_cleanup(request)
+@pytest.fixture +def add_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_empty_netgroup(add_empty_netgroup):
- """
- Adding empty netgroup.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
+@pytest.fixture +def add_two_empty_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("empty_netgroup1")
- ent_list.add_netgroup("empty_netgroup2")
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_two_empty_netgroup(add_two_empty_netgroup):
- """
- Adding two empty netgroups.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
If you wanted to test nested empty netgroups then it is already covered in mixed_netgroup (by mixed_netgroup1 and mixed_netgroup2)
Otherwise this test-case is covered by previous test test_add_empty_netgroup
+@pytest.fixture +def add_tripled_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("tripled_netgroup", ["(host,user,domain)"])
- ent_list.add_netgroup("adv_tripled_netgroup", ["(host1,user1,domain1)",
"(host2,user2,domain2)"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_tripled_netgroup(add_tripled_netgroup):
- """
- Adding netgroup with triplet.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host", "user", "domain")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("adv_tripled_netgroup")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2")])
+@pytest.fixture +def add_mixed_netgroup(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("mixed_netgroup1", [], [])
- ent_list.add_netgroup("mixed_netgroup2", [], ["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup3", ["(host1,user1,domain1)"], [])
- ent_list.add_netgroup("mixed_netgroup4",
["(host2,user2,domain2)", "(host3,user3,domain3)"],
[])
You can remove empty list from previous 4 function invocations. We can rely on default arguments. The same as in test_add_empty_netgroup
- ent_list.add_netgroup("mixed_netgroup5",
["(host4,user4,domain4)"],
["mixed_netgroup1"])
- ent_list.add_netgroup("mixed_netgroup6",
["(host5,user5,domain5)"],
["mixed_netgroup2"])
- ent_list.add_netgroup("mixed_netgroup7", [], ["mixed_netgroup3"])
- ent_list.add_netgroup("mixed_netgroup8", [],
["mixed_netgroup3", "mixed_netgroup4"])
- ent_list.add_netgroup("mixed_netgroup9",
["(host6,user6,domain6)"],
["mixed_netgroup3", "mixed_netgroup4"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_add_mixed_netgroup(add_mixed_netgroup):
- """
- Adding many netgroups of different type.
- """
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup4")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup5")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host4", "user4", "domain4")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup6")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host5", "user5", "domain5")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup7")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [("host1", "user1", "domain1")]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup8")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3")])
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("mixed_netgroup9")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([("host1", "user1", "domain1"),
("host2", "user2", "domain2"),
("host3", "user3", "domain3"),
("host6", "user6", "domain6")])
+@pytest.fixture +def remove_step_by_step(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("rm_empty_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("rm_empty_netgroup2",
["(host2,user2,domain2)"],
["rm_empty_netgroup1"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return ent_list
+def test_remove_step_by_step(remove_step_by_step, ldap_conn):
- """
- Removing netgroups step by step.
- """
- ent_list = remove_step_by_step
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of rm_empty_netgroup1
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of rm_empty_netgroup2
- ldap_conn.delete_s(ent_list[0][0])
- ent_list.remove(ent_list[0])
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup1")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("rm_empty_netgroup2")
- assert res == sssd_ntg.NssReturnCode.NOTFOUND
- assert netgroups == []
+@pytest.fixture +def removing_nested_netgroups(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_netgroup("t2841_netgroup1", ["(host1,user1,domain1)"])
- ent_list.add_netgroup("t2841_netgroup2", ["(host2,user2,domain2)"])
- ent_list.add_netgroup("t2841_netgroup3", [],
["t2841_netgroup1", "t2841_netgroup2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_removing_nested_netgroups(removing_nested_netgroups, ldap_conn):
- """
- Regression test for ticket 2841.
- https://fedorahosted.org/sssd/ticket/2841
- """
- netgroup_dn = 'cn=t2841_netgroup3,ou=Netgroups,dc=example,dc=com'
^^^^^^^^^^^^^^^^^ It would be good to use ldap_conn.ds_inst.base_dn So after changind base_dn test will not fail
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert sorted(netgroups) == sorted([('host1', 'user1', 'domain1'),
('host2', 'user2', 'domain2')])
- # removing of t2841_netgroup1 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup1", "t2841_netgroup2"]}
- new = {'memberNisNetgroup': ["t2841_netgroup2"]}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- # removing of t2841_netgroup2 from t2841_netgroup3
- old = {'memberNisNetgroup': ["t2841_netgroup2"]}
- new = {'memberNisNetgroup': []}
- ldif = ldap.modlist.modifyModlist(old, new)
- ldap_conn.modify_s(netgroup_dn, ldif)
- if subprocess.call(["sss_cache", "-N"]) != 0:
raise Exception("sssd_cache failed")
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup1")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host1', 'user1', 'domain1')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup2")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == [('host2', 'user2', 'domain2')]
- res, _, netgroups = sssd_ntg.get_sssd_netgroups("t2841_netgroup3")
- assert res == sssd_ntg.NssReturnCode.SUCCESS
- assert netgroups == []
LS
Hi Lukas,
thanks for review. I hope that I addressed all notes. New version is attached.
ACK
Thank you for patience with review.
master: * 05457ed0e399aaacc919b7aacee5d8210e1c1072 * 1cba321946084231c220e9561487555671b944c3 * bf141e052a81b28ee0ad2f61ff8b4879e4faa13b
LS
sssd-devel@lists.fedorahosted.org