On 10/21/2015 07:25 PM, Michal Židek wrote:
On 10/21/2015 07:19 PM, Michal Židek wrote:
On 10/21/2015 06:40 PM, Pavel Reichl wrote:
On 10/21/2015 06:28 PM, Michal Židek wrote:
+
+def test_wrong_LC_ALL(local_domain_only):
+ """
+ Regression test for ticket
+https://fedorahosted.org/sssd/ticket/2785
+
+ """
+ subprocess.call(["sss_useradd", "foo", "-M"])
+ pwd.getpwnam("foo")
+
+ # Change the LC_ALL variable to nonexistent locale
+ oldvalue = os.environ.get("LC_ALL", "")
+ os.environ["LC_ALL"] = "nonexistent_locale"
+
+ # sss_userdel must remove the user despite wrong LC_ALL
+ ret = subprocess.call(["sss_userdel", "foo", "-R"])
Please check ret value or use check_call method. Thanks!
+ assert_nonexistent_user("foo")
+ os.environ["LC_LOCAL"] = oldvalue
-- 2.1.0
Please run pep8, I think I saw missing line and missing space after #.
I haven't tested patches I just noticed this nitpicks.
Thanks!
Thank you Pavel, I fixed the nitpicks and
checked the code with pep8.
New patches are attached.
Michal
New patches attached. I had unused constant
in the code,
Michal
I added one change in the Makefile.am .
New patches attached.
Michal
>From a84b7cfc766e1125399a100f28f7565a532c3863 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Mon, 19 Oct 2015 15:38:08 +0200
Subject: [PATCH 1/4] util: Continue if setlocale fails
Fixes:
https://fedorahosted.org/sssd/ticket/2785
setlocale needs some environment variables
to be set in order to work. These variables
are not present in some special cases. We
should not fail completely in these cases
but continue with the compatible C locale.
---
src/sss_client/ssh/sss_ssh_client.c | 4 +++-
src/tools/tools_util.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/sss_client/ssh/sss_ssh_client.c b/src/sss_client/ssh/sss_ssh_client.c
index 0d206ef..a198039 100644
--- a/src/sss_client/ssh/sss_ssh_client.c
+++ b/src/sss_client/ssh/sss_ssh_client.c
@@ -50,7 +50,9 @@ int set_locale(void)
c = setlocale(LC_ALL, "");
if (c == NULL) {
- return EIO;
+ /* If setlocale fails, continue with the default
+ * locale. */
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to set locale\n");
}
errno = 0;
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 68f6588..f9dca72 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -259,7 +259,9 @@ int set_locale(void)
c = setlocale(LC_ALL, "");
if (c == NULL) {
- return EIO;
+ /* If setlocale fails, continue with the default
+ * locale. */
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to set locale\n");
}
errno = 0;
--
2.1.0
>From ea19184b2c1a6fe130b2346ec8504d181e1312e2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Mon, 19 Oct 2015 15:49:02 +0200
Subject: [PATCH 2/4] server_setup: Log failed attempt to set locale
Failed setlocale call could cause unexpected
behaviour. It is better to generate DEBUG
message if this happens.
---
src/util/server.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/util/server.c b/src/util/server.c
index 036dace..03796be 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -458,6 +458,7 @@ int server_setup(const char *name, int flags,
bool dm;
struct tevent_signal *tes;
struct logrotate_ctx *lctx;
+ char *locale;
ret = chown_debug_file(NULL, uid, gid);
if (ret != EOK) {
@@ -508,7 +509,12 @@ int server_setup(const char *name, int flags,
}
/* Set up locale */
- setlocale(LC_ALL, "");
+ locale = setlocale(LC_ALL, "");
+ if (locale == NULL) {
+ /* Just print debug message and continue */
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to set locale\n");
+ }
+
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
--
2.1.0
>From 2b15daa0871ac7f3abadbeb33f115146c5cd1859 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Tue, 20 Oct 2015 18:18:01 +0200
Subject: [PATCH 3/4] tests: Run intgcheck without libsemanage
For now the libsemanage can not be used inside
intgcheck tests.
---
Makefile.am | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile.am b/Makefile.am
index 15d99ce..77a325a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2647,6 +2647,7 @@ intgcheck:
--prefix="$$prefix" \
--with-ldb-lib-dir="$$prefix"/lib/ldb \
--enable-intgcheck-reqs \
+ --without-semanage \
$(INTGCHECK_CONFIGURE_FLAGS); \
$(MAKE) $(AM_MAKEFLAGS); \
: Force single-thread install to workaround concurrency issues; \
--
2.1.0
>From 7462496bf237dd067335ecf084b24bb1e353ea45 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Tue, 20 Oct 2015 15:03:22 +0200
Subject: [PATCH 4/4] tests: Regression test with wrong LC_ALL
Ticket:
https://fedorahosted.org/sssd/ticket/2785
Test local domain tool with wrong LC_ALL
environment variable value.
NOTE: The memory cache files are not deleted
properly in the test teardown to work around the
problem described in ticket
https://fedorahosted.org/sssd/ticket/2726
Once the ticket above is solved, the teardown
will be updated to remove the memory cache
files.
---
src/tests/intg/Makefile.am | 1 +
src/tests/intg/test_local_domain.py | 115 ++++++++++++++++++++++++++++++++++++
2 files changed, 116 insertions(+)
create mode 100644 src/tests/intg/test_local_domain.py
diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am
index 6819c2f..12a4fc2 100644
--- a/src/tests/intg/Makefile.am
+++ b/src/tests/intg/Makefile.am
@@ -7,6 +7,7 @@ dist_noinst_DATA = \
ent_test.py \
ldap_ent.py \
ldap_test.py \
+ test_local_domain.py \
util.py \
test_memory_cache.py \
$(NULL)
diff --git a/src/tests/intg/test_local_domain.py b/src/tests/intg/test_local_domain.py
new file mode 100644
index 0000000..d778412
--- /dev/null
+++ b/src/tests/intg/test_local_domain.py
@@ -0,0 +1,115 @@
+#
+# SSSD LOCAL domain tests
+#
+# Copyright (c) 2015 Red Hat, Inc.
+# Author: Michal Židek <mzi...@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 ent
+import grp
+import pwd
+import time
+import config
+import signal
+import subprocess
+import pytest
+import sssd_id
+from util import unindent
+
+
+def stop_sssd():
+ 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)
+
+
+def create_conf_fixture(request, contents):
+ """Generate sssd.conf and add teardown for removing it"""
+ conf = open(config.CONF_PATH, "w")
+ conf.write(contents)
+ conf.close()
+ os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+ request.addfinalizer(lambda: os.unlink(config.CONF_PATH))
+
+
+def create_sssd_fixture(request):
+ """Start sssd and add teardown for stopping it and removing state"""
+ if subprocess.call(["sssd", "-D", "-f"]) != 0:
+ raise Exception("sssd start failed")
+
+ def teardown():
+ try:
+ stop_sssd()
+ except:
+ pass
+ subprocess.call(["sss_cache", "-E"])
+ for path in os.listdir(config.DB_PATH):
+ os.unlink(config.DB_PATH + "/" + path)
+ # FIXME: Uncomment this when ticket #2726 is solved
+ # https://fedorahosted.org/sssd/ticket/2726
+ # for path in os.listdir(config.MCACHE_PATH):
+ # os.unlink(config.MCACHE_PATH + "/" + path)
+ request.addfinalizer(teardown)
+
+
+@pytest.fixture
+def local_domain_only(request):
+ conf = unindent("""\
+ [sssd]
+ domains = LOCAL
+ services = nss
+
+ [nss]
+ memcache_timeout = 0
+
+ [domain/LOCAL]
+ id_provider = local
+ min_id = 10000
+ max_id = 20000
+ """).format(**locals())
+ create_conf_fixture(request, conf)
+ create_sssd_fixture(request)
+ return None
+
+
+def assert_nonexistent_user(name):
+ with pytest.raises(KeyError):
+ pwd.getpwnam(name)
+
+
+def test_wrong_LC_ALL(local_domain_only):
+ """
+ Regression test for ticket
+ https://fedorahosted.org/sssd/ticket/2785
+
+ """
+ subprocess.check_call(["sss_useradd", "foo", "-M"])
+ pwd.getpwnam("foo")
+
+ # Change the LC_ALL variable to nonexistent locale
+ oldvalue = os.environ.get("LC_ALL", "")
+ os.environ["LC_ALL"] = "nonexistent_locale"
+
+ # sss_userdel must remove the user despite wrong LC_ALL
+ subprocess.check_call(["sss_userdel", "foo", "-R"])
+ assert_nonexistent_user("foo")
+ os.environ["LC_LOCAL"] = oldvalue
--
2.1.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
