Re: [SSSD] [PATCH] sss_override: add -find and -show

2015-10-23 Thread Pavel Reichl 

Patch won't apply. Please rebase on top of 'sss_override: Remove unused 
parameter tool_ctx' (c12cd2d95d08c9316bc358c2f7707d92551b6909).
Thanks.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_override: Add restart requirements to man page

2015-10-23 Thread Lukas Slebodnik 
On (23/10/15 09:54), Pavel Reichl wrote:
>
>
>On 10/23/2015 09:10 AM, Lukas Slebodnik wrote:
>>On (22/10/15 13:35), Pavel Reichl wrote:
>>>On 10/22/2015 12:27 PM, Pavel Březina wrote:
On 10/22/2015 11:23 AM, Pavel Reichl wrote:
>
>
>On 10/21/2015 12:44 PM, Pavel Březina wrote:
>>On 10/21/2015 12:39 PM, Pavel Reichl wrote:
>>>Hello,
>>>
>>>while reviewing integration tests for sss_override Nick noticed that man
>>>page does not mention restart requirements. Attached patch (attempts to)
>>>address that.
>>>
>>>Thanks!
>>
>>LGTM
>
>Pavel it seems to me that after calling user-del SSSD must be also
>restarted, right?

No. But you may be hitting memory cache.
>>>
>>>Oh, thanks. Please see updated patches.
>>>
>>>
>>
>>>From 073155493b06f3805f282843143a4a020e63bfbf Mon Sep 17 00:00:00 2001
>>>From: Pavel Reichl 
>>>Date: Wed, 21 Oct 2015 12:28:37 +0200
>>>Subject: [PATCH 1/2] sss_override: Add restart requirements to man page
>>>
>>>---
>>>src/man/sss_override.8.xml | 11 +--
>>>1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>>diff --git a/src/man/sss_override.8.xml b/src/man/sss_override.8.xml
>>>index 
>>>24c38936984946b3284d1523a7321a7e7f3d7982..5be4295183a78a9e6625b6689cb1003ce1c01541
>>> 100644
>>>--- a/src/man/sss_override.8.xml
>>>+++ b/src/man/sss_override.8.xml
>>>@@ -34,8 +34,15 @@
>>> and groups. This change takes effect only on local machine.
>>> 
>>> 
>>>-Overrides data are stored in SSSD cache. If the cache is deleted
>>>-all local overrides are lost.
>>>+Overrides data are stored in the SSSD cache. If the cache is 
>>>deleted,
>>>+all local overrides are lost. Please note that after the first
>>>+override is created using any of the following
>>>+user-add, group-add,
>>>+user-import or
>>>+group-import command. SSSD needs to be
>>>+restarted to take effect.
>>>+sss_override prints message when a restart 
>>>is
>>>+required.
>>> 
>>> 
>>The patch is different iw we compare to the initial version.
>>I think Dad deserves a credit so the author should be Dan.
>>Will you prepare new patch or shoudl we change it before pushing.
>
>Change the author before pushing then.
>
>BTW do I deserve something for my effort? :-)
I filed many tickets for regressions and it's not tracked anywhere.
So, do I deserve something for my effort? :-)

>BTW2: second patch was not acked at all, first should be reacked.
>
I'm not the best person for review of man page changes.
So you might find reviewer somewhere else.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] intg: Fix all PEP8 issues

2015-10-23 Thread Michal Židek 

On 10/23/2015 08:55 AM, Lukas Slebodnik wrote:

On (22/10/15 19:52), Nikolai Kondrashov wrote:

On 10/22/2015 05:55 PM, Michal Židek wrote:

On 10/22/2015 04:06 PM, Nikolai Kondrashov wrote:

Hi everyone,

The attached patch fixes all PEP8 issues in src/tests/intg directory.
I.e. the
integration tests. It was extracted from the "intg: Add more LDAP tests"
thread.

Nick




Thank you Nick.
Now there are just the
"E126 continuation line over-indented for hanging indent"
warnings, but we relaxed the condition on that one.

No we didn't.


Yes, this was my mistake as a reviewer. Sorry for wasting
everyone's time. I though the warning means something
different (stupid me!!).




Argh, this should have fixed everything. I'll take a look.


I would prefer if you could spend time with something more useful.
e.g. preparing samba dc in cwrap envirment.


It was me who asked Nick to put this to separate thread,
because more people are working on CI tests and I wanted
to avoid as many conflicts as possible by having at least
some parts sooner in master.



So I decided to save you some time and attacehd patch fixes
remain pep8 issues.

LS


I actually send patch with the rest of the fixes
to Nick and he ACKed it off-list (because I did not
CC devel list with the original mail).

Since the patch was the same as the one you attached
with the exception of 2 lines I think the ACK is
transitional, but I sent it to CI again to see results
of this version. Will post the results when they arrive.

Please put Nick as a reviewer of the patch as well since
he also did the review off-list.

Michal

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_override: add -find and -show

2015-10-23 Thread Pavel Reichl 

Thanks for rebase.

I'm having little trouble testing now, it seems to me that user-show does not 
work, but it's Friday evening so the problem might be between between keyboard 
and chair :-)

$ getent passwd john
john:*:1234:1:John Doe:/home/john:/bin/bash

sudo sss_override user-add john --name jon
SSSD needs to be restarted for the changes to take effect.

$ sudo systemctl restart sssd


$ sudo sss_override user-find
john@ol:jon:

sudo sss_override user-show john


This is what override looks for:
(&(objectClass=userOverride)(overrideAnchorUUID=:LOCAL:name\\5c3Djohn\\5c,cn\\5c3Dusers\\5c,cn\\5c3Dol\\5c,cn\\5c3Dsysd\220)

This is content of cache:
ldbsearch -H cache_ol.ldb '(objectClass=userOverride)' overrideAnchorUUID
asq: Unable to register control with rootdse!
# record 1
dn: 
overrideAnchorUUID=:LOCAL:name\3Djohn\,cn\3Dusers\,cn\3Dol\,cn\3Dsysdb,cn=LOCAL,cn=views,cn=sysdb
overrideAnchorUUID:: OkxPQ0FMOm5hbWVcM0Rqb2huXCxjblwzRHVzZXJzXCxjblwzRG9sXCxjb
 lwzRHN5c2Ri
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_override: add -find and -show

2015-10-23 Thread Pavel Březina 

On 10/23/2015 05:41 PM, Pavel Reichl wrote:

Thanks for rebase.

I'm having little trouble testing now, it seems to me that user-show
does not work, but it's Friday evening so the problem might be between
between keyboard and chair :-)

$ getent passwd john
john:*:1234:1:John Doe:/home/john:/bin/bash

sudo sss_override user-add john --name jon
SSSD needs to be restarted for the changes to take effect.

$ sudo systemctl restart sssd


$ sudo sss_override user-find
john@ol:jon:

sudo sss_override user-show john


This is what override looks for:
(&(objectClass=userOverride)(overrideAnchorUUID=:LOCAL:name\\5c3Djohn\\5c,cn\\5c3Dusers\\5c,cn\\5c3Dol\\5c,cn\\5c3Dsysd\220)


This is content of cache:
ldbsearch -H cache_ol.ldb '(objectClass=userOverride)' overrideAnchorUUID
asq: Unable to register control with rootdse!
# record 1
dn:
overrideAnchorUUID=:LOCAL:name\3Djohn\,cn\3Dusers\,cn\3Dol\,cn\3Dsysdb,cn=LOCAL,cn=views,cn=sysdb

overrideAnchorUUID::
OkxPQ0FMOm5hbWVcM0Rqb2huXCxjblwzRHVzZXJzXCxjblwzRG9sXCxjb
  lwzRHN5c2Ri


Works for me, we'll debug it on Monday on site.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH v2] intg: Add more LDAP tests

2015-10-23 Thread Michal Židek 

Hi!

There is one new pep8 error in the code:
../src/tests/intg/ldap_test.py:819:37: E126 continuation line 
over-indented for hanging indent



For the memcache workaroud please do this change in the
code:
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index 8263d88..eb466ab 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -194,8 +194,10 @@ def cleanup_sssd_process():
 subprocess.call(["sss_cache", "-E"])
 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)
+# 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)


See some comments bellow...

On 10/12/2015 03:31 PM, Nikolai Kondrashov wrote:


0002-intg-Add-more-LDAP-tests.patch


 From a2c75e045f8f341402a5c771489b462f18e27a39 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov
Date: Tue, 29 Sep 2015 21:18:18 +0300
Subject: [PATCH 2/3] intg: Add more LDAP tests

Add a bunch of LDAP tests.

 * Adding/removing a user/group/membership with rfc2307(bis) schema.
 * Filtering users/groups with rfc2307(bis) schema.
 * The effect of override_homedir option.
 * The effect of fallback_homedir option.
 * The effect of override_shell option.
 * The effect of shell_fallback option.
 * The effect of default_shell option.
 * The effect of vetoed_shells option.
---
  src/tests/intg/ldap_test.py | 534 
  1 file changed, 534 insertions(+)

diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index 6a09b37..cca9431 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -125,6 +125,23 @@ def format_basic_conf(ldap_conn, schema, enum):
  """).format(**locals())


+def format_interactive_conf(ldap_conn, schema):
+"""Format an SSSD configuration with all caches refreshing in 4 seconds"""
+return \
+format_basic_conf(ldap_conn, schema, enum=True) + \
+unindent("""
+[nss]
+memcache_timeout= 4


It is better to set memcache timeout to zero outside tests
that are not dedicated to memcache. This will also probably
solve the membership tests failure that you saw when using the
workaround for the memcache tests failure.


+enum_cache_timeout  = 4
+entry_negative_timeout  = 4


I would set negative cache timeout to zero as well,
see comments below.


+
+[domain/LDAP]
+ldap_enumeration_refresh_timeout= 4
+ldap_purge_cache_timeout= 1
+entry_cache_timeout = 4
+""")
+
+
  def create_conf_file(contents):
  """Create sssd.conf with specified contents"""
  conf = open(config.CONF_PATH, "w")
@@ -387,3 +404,520 @@ def test_refresh_after_cleanup_task(ldap_conn, 
refresh_after_cleanup_task):
  ent.assert_group_by_name(
  "group2",
  dict(mem=ent.contains_only("user1")))
+
+
+@pytest.fixture
+def blank_rfc2307(request, ldap_conn):
+"""Create blank RFC2307 directory fixture with interactive SSSD conf"""
+create_ldap_cleanup(request, ldap_conn)
+create_conf_fixture(request,
+format_interactive_conf(ldap_conn, SCHEMA_RFC2307))
+create_sssd_fixture(request)
+
+
+@pytest.fixture
+def blank_rfc2307_bis(request, ldap_conn):
+"""Create blank RFC2307bis directory fixture with interactive SSSD conf"""
+create_ldap_cleanup(request, ldap_conn)
+create_conf_fixture(request,
+format_interactive_conf(ldap_conn, SCHEMA_RFC2307_BIS))
+create_sssd_fixture(request)
+
+
+@pytest.fixture
+def user_and_group_rfc2307(request, ldap_conn):
+"""
+Create an RFC2307 directory fixture with interactive SSSD conf,
+one user and one group
+"""
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+ent_list.add_user("user", 1001, 2000)
+ent_list.add_group("group", 2001)
+create_ldap_fixture(request, ldap_conn, ent_list)
+create_conf_fixture(request,
+format_interactive_conf(ldap_conn, SCHEMA_RFC2307))
+create_sssd_fixture(request)
+return None
+
+
+@pytest.fixture
+def user_and_groups_rfc2307_bis(request, ldap_conn):
+"""
+Create an RFC2307bis directory fixture with interactive SSSD conf,
+one user and two groups
+"""
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+ent_list.add_user("user", 1001, 2000)
+ent_list.add_group_bis("group1", 2001)
+ent_list.add_group_bis("group2", 2002)
+create_ldap_fixture(request, ldap_conn, ent_list)
+create_conf_fixture(request,
+

Re: [SSSD] [PATCHES] BUILD: Remove unused variables

2015-10-23 Thread Jakub Hrozek 
On Mon, Oct 19, 2015 at 09:16:40AM +0200, Lukas Slebodnik wrote:
> ehlo,
> 
> attached are few simple patches which simplify Makefile.am in src/test/cwrap
> 
> LS

The patches look good to me and CI passed:
http://sssd-ci.duckdns.org/logs/job/31/21/summary.html  

   

ACK
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] intg: Fix all PEP8 issues

2015-10-23 Thread Michal Židek 

On 10/23/2015 12:26 PM, Michal Židek wrote:

On 10/23/2015 08:55 AM, Lukas Slebodnik wrote:

On (22/10/15 19:52), Nikolai Kondrashov wrote:

On 10/22/2015 05:55 PM, Michal Židek wrote:

On 10/22/2015 04:06 PM, Nikolai Kondrashov wrote:

Hi everyone,

The attached patch fixes all PEP8 issues in src/tests/intg directory.
I.e. the
integration tests. It was extracted from the "intg: Add more LDAP
tests"
thread.

Nick




Thank you Nick.
Now there are just the
"E126 continuation line over-indented for hanging indent"
warnings, but we relaxed the condition on that one.

No we didn't.


Yes, this was my mistake as a reviewer. Sorry for wasting
everyone's time. I though the warning means something
different (stupid me!!).




Argh, this should have fixed everything. I'll take a look.


I would prefer if you could spend time with something more useful.
e.g. preparing samba dc in cwrap envirment.


It was me who asked Nick to put this to separate thread,
because more people are working on CI tests and I wanted
to avoid as many conflicts as possible by having at least
some parts sooner in master.



So I decided to save you some time and attacehd patch fixes
remain pep8 issues.

LS


I actually send patch with the rest of the fixes
to Nick and he ACKed it off-list (because I did not
CC devel list with the original mail).

Since the patch was the same as the one you attached
with the exception of 2 lines I think the ACK is
transitional, but I sent it to CI again to see results
of this version. Will post the results when they arrive.

Please put Nick as a reviewer of the patch as well since
he also did the review off-list.



ACK, CI link: http://sssd-ci.duckdns.org/logs/job/31/24/summary.html

Michal

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] Running tests with different environment

2015-10-23 Thread Lukas Slebodnik 
On (21/10/15 09:20), Sumit Bose wrote:
>On Tue, Oct 20, 2015 at 10:15:06PM +0200, Jakub Hrozek wrote:
>> On Tue, Oct 20, 2015 at 05:17:49PM +0300, Nikolai Kondrashov wrote:
>> > Hi Jakub,
>> > 
>> > On 10/19/2015 09:43 PM, Jakub Hrozek wrote:
>> > >I'm working on pam_sss.so tests[1] and I ran into a problem that I don't
>> > >know how to solve best.
>> > >
>> > >tl;dr, I would like to set different environment variables for different
>> > >tests in order to set up cwrap libraries differently per-test.
>> > >
>> > >I can't use setenv() in the test itself, because that's too late, I need
>> > >the variables to be set when __attribute__(constructor) is run, so 
>> > >normally
>> > >at program startup, when the libraries are loaded.
>> > >
>> > >With cmake it's easy, use set(TEST_ENVIRONMENT). But with autotools, I
>> > >only found two ways:
>> > > - TESTS_ENVIRONMENT - this is fine, but it's per Makefile.am. So I
>> > >   would have to split the tests more, into pam_wrapper tests that 
>> > > also
>> > >   require uid_wrapper, tests that only require pam_wrapper, ...
>> > > - LOG_COMPILER - this allows to run a wrapper script before a test
>> > >   that receives the test name as argv. So this is pretty much what I
>> > >   want except this is a feature new to automake 1.12, which would
>> > >   rule out both RHEL-6 and Ubuntu Trusty (which is used by Travis)
>> > >
>> > >So I'm really leaning towards creating a src/tests/cwrap/pwrap/Makefile.am
>> > >and src/tests/cwrap/pwrap_root/Makefile.am. The downside of multiple
>> > >Makefile.am files is that there is some code duplication and the build
>> > >takes longer. But I still think there is enough interest (from us and from
>> > >our users) to support git master on old platforms. I can file a ticket to
>> > >remove this and use LOG_COMPILER when we drop support for RHEL-6 and old
>> > >Ubuntu versions...
>> > >
>> > >If you disagree, please reply, otherwise I'm going to send a patch with
>> > >per-test Makefile...
>> > 
>> > Ah, so these are unit tests, not integration tests?
>> 
>> I'm working on both, actually. The first part is more or less an
>> isolated unit test of all the options that pam_sss supports. The reason
>> is that some options (2FA, smart cards, ...) are not really easily
>> testable without a mock back end, at the moment we only have openldap in
>> the integration tests.
>> 
>> The next step I will start right after I finish this part is integration
>> tests that will exercise LDAP authentication, password change and maybe
>> authorisation if there's time left.
>> 
>> > 
>> > I'm not sure I understood everything right, sorry, but perhaps you can find
>> > something useful in contrib/ci/run, contrib/ci/make-check-wrap and
>> > contrib/ci/valgrind-condense where CI matches and handles particular tests
>> > differently regardless of whether LOG_COMPILER is supported or not.  Ping 
>> > me
>> > if you need help figuring out what's going on there.
>> 
>> So more or less I wanted to have two tests and wanted to run the first
>> as (simplified):
>> PAM_WRAPPER=1 ./src/tests/cwrap/pam_sss_wrapper-tests
>> and other as:
>> PAM_WRAPPER=1 UID_WRAPPER=1 ./src/tests/cwrap/pam_sss_wrapper-root--tests
>> 
>> but it occured to me that I can always start with UID wrapper, just drop
>> privileges if I need a strictly non-root test. It's a bit of a hack :-)
>> but since the root is fake anyway, I think it's acceptable.
>> 
>> > 
>> > Granted this is from outside the build, but maybe you can concoct something
>> > from inside as well.
>> 
>> I think this might work as well; thank you!
>
>I had a short look at libtool. Since we use it during 'make check' not
>the actual binaries are called but a libtool generated wrapper script.
>If it would be possible to set the environment variables here they would
>be visible for the binary at startup.
>
>Libtool has the concept of 'executable wrappers' to support cygwin and
>similar environments but I didn't found an easy way to add own wrapper
>here.
>
>Adding the variables directly in the generated wrapper scripts would be
>quite a hack. But maybe it would be possible if we add our own version
>of build/ltmain.sh where the wrapper scripts are generated in
>func_emit_wrapper()?
>
>So, I'm afraid this is not a direct answer to your question but maybe
>libtool might be useful here.
>

I read this thread after are came up with almost similar solution
as Sumit. With a small difference. I did not decide to inject env
variables to generated script but I decided to write yet another
wrapper on top of and set env variable there.

Here is a POC version. What do you think about such solution.

LS
>From 79e8f0e1cdd09ca791755bb03c1e8f38b8078dc3 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Fri, 23 Oct 2015 13:06:00 +0200
Subject: [PATCH] temp

---
 src/tests/cwrap/Makefile.am   | 8 ++--
 src/tests/cwrap/become_user-tests.sh  | 5 +

Re: [SSSD] [PATCH] sss_override: add -find and -show

2015-10-23 Thread Pavel Březina 

On 10/23/2015 10:59 AM, Pavel Reichl wrote:

Patch won't apply. Please rebase on top of 'sss_override: Remove unused
parameter tool_ctx' (c12cd2d95d08c9316bc358c2f7707d92551b6909).
Thanks.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


>From 3fdba1bfdb5b11a0143373b94ae8086a48dbf68b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Tue, 20 Oct 2015 12:22:23 +0200
Subject: [PATCH 1/6] sss_tools: always show common and help options

popt don't handle merging NULL option table, thus common and help
options were not displayed when command doesn't have any options.
---
 src/tools/common/sss_tools.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c
index c0b52bb86941d3245569e13ff8b830d861ba..abb9dbace3c622df84350cfc0b7a6f42c1a5e469 100644
--- a/src/tools/common/sss_tools.c
+++ b/src/tools/common/sss_tools.c
@@ -262,6 +262,19 @@ int sss_tool_route(int argc, const char **argv,
 return sss_tool_usage(argv[0], commands);
 }
 
+static struct poptOption *nonnull_popt_table(struct poptOption *options)
+{
+static struct poptOption empty[] = {
+POPT_TABLEEND
+};
+
+if (options == NULL) {
+return empty;
+}
+
+return options;
+}
+
 int sss_tool_popt_ex(struct sss_cmdline *cmdline,
  struct poptOption *options,
  enum sss_tool_opt require_option,
@@ -272,7 +285,7 @@ int sss_tool_popt_ex(struct sss_cmdline *cmdline,
  const char **_fopt)
 {
 struct poptOption opts_table[] = {
-{NULL, '\0', POPT_ARG_INCLUDE_TABLE, options, \
+{NULL, '\0', POPT_ARG_INCLUDE_TABLE, nonnull_popt_table(options), \
  0, _("Command options:"), NULL },
 {NULL, '\0', POPT_ARG_INCLUDE_TABLE, sss_tool_common_opts_table(), \
  0, _("Common options:"), NULL },
-- 
1.9.3

>From 338dcb0516af1124639f6320129080448c29c05c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Tue, 20 Oct 2015 11:18:31 +0200
Subject: [PATCH 2/6] sss_override: fix exporting multiple domains

There was a mistake in the code which resulted in exporting one
domain several times if multiple domain were configured.
---
 src/tools/sss_override.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c
index 041c2a10617c98bac584b9058fe0050286f71249..d0bf38729519e785aeff8e06e6e7b4e8710e0946 100644
--- a/src/tools/sss_override.c
+++ b/src/tools/sss_override.c
@@ -1249,7 +1249,7 @@ static int override_user_export(struct sss_cmdline *cmdline,
 
 dom = tool_ctx->domains;
 do {
-objs = list_user_overrides(tool_ctx, tool_ctx->domains);
+objs = list_user_overrides(tool_ctx, dom);
 if (objs == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get override objects\n");
 exit = EXIT_FAILURE;
@@ -1454,7 +1454,7 @@ static int override_group_export(struct sss_cmdline *cmdline,
 
 dom = tool_ctx->domains;
 do {
-objs = list_group_overrides(tool_ctx, tool_ctx->domains);
+objs = list_group_overrides(tool_ctx, dom);
 if (objs == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get override objects\n");
 exit = EXIT_FAILURE;
-- 
1.9.3

>From 396ea7f7108362daa76305a6c571c480d4e562a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Fri, 23 Oct 2015 13:30:08 +0200
Subject: [PATCH 3/6] sss_override: add user-find

Resolves:
https://fedorahosted.org/sssd/ticket/2736
---
 src/man/sss_override.8.xml |  11 +++
 src/tools/sss_override.c   | 194 -
 2 files changed, 152 insertions(+), 53 deletions(-)

diff --git a/src/man/sss_override.8.xml b/src/man/sss_override.8.xml
index 24c38936984946b3284d1523a7321a7e7f3d7982..c5cc32e1999a10e610c34ea00d90a90efdd96a30 100644
--- a/src/man/sss_override.8.xml
+++ b/src/man/sss_override.8.xml
@@ -79,6 +79,17 @@
 
 
 
+user-find
+-d,--domain DOMAIN
+
+
+
+List user overrides.
+
+
+
+
+
 user-import
 FILE
 
diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c
index d0bf38729519e785aeff8e06e6e7b4e8710e0946..f438f92d6849402818000a33916ab184713b1400 100644
--- a/src/tools/sss_override.c
+++ b/src/tools/sss_override.c
@@ -135,6 +135,43 @@ static int parse_cmdline_group_del(struct sss_cmdline *cmdline,
  >orig_name, >domain);
 }
 
+static int parse_cmdline_find(struct sss_cmdline

Re: [SSSD] [PATCH] TEST: recent_valid filter testing

2015-10-23 Thread Jakub Hrozek 
On Fri, Oct 02, 2015 at 01:55:29PM +0200, Petr Cech wrote:
> Hi,
> 
> there is WiP attached. I removed some tests like this one some time ago.
> They fail really often and we decided that the test logic was corrupted. Now
> I am trying get it back to the codebase.
> 
> There is some kind of cmocka magic around data provider. I think it creates
> test_user_1 during creation of filter.
> 
> In case of this type of tests, we need two users, one stored before filter
> request and one stored after filter request. There is a special type of
> filter which has time parameter which it search from. So the filter returns
> only one user.
> 
> If this concept is right, I will send whole patch.
> 
> Regards
> 
> Petr
> 
> PS: I applied my patch after 000*-cache_req_*. Those patches are on list.

> From aa0b0ab7c0a95ff47d5003907730c5432ff7bb85 Mon Sep 17 00:00:00 2001
> From: Petr Cech 
> Date: Fri, 2 Oct 2015 07:34:08 -0400
> Subject: [PATCH] TEST: recent_valid filter testing
> 
> Some tests were removed in past. This is only WiP, not regular patch.
> I rewrote one of the removed test. Is it this right way?
> 
> We speak about RECENT filter. It returns only records which
> have been wrote or updated after filter was created (or another given
> time). Some notes are written in comments of this patch.

Thank you, I think your approach is correct. Your test essentially tests
that testuser2 was on the server but was removed, so only testuser1 is
returned.

It's correct, but because the interface is able to return more users, I
would prefer if we tested that as well.

I have one more minor remark inline, but in general, please go
ahead and add back the other tests..

> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/2730
> ---
>  src/tests/cmocka/test_responder_cache_req.c | 60 
> -
>  1 file changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/src/tests/cmocka/test_responder_cache_req.c 
> b/src/tests/cmocka/test_responder_cache_req.c
> index 
> bb79fd10eefd7186f17a1f9306b57ddca2e3279f..c01d92fd9f3f078d853da1642e63cdbc3a1aed7b
>  100644
> --- a/src/tests/cmocka/test_responder_cache_req.c
> +++ b/src/tests/cmocka/test_responder_cache_req.c
> @@ -1239,6 +1239,58 @@ static void cache_req_user_by_filter_test_done(struct 
> tevent_req *req)
>  ctx->tctx->done = true;
>  }
>  
> +/* NOTE better name is filter_recent_valid */
> +void test_users_by_filter_valid(void **state)
> +{
> +struct cache_req_test_ctx *test_ctx = NULL;
> +TALLOC_CTX *req_mem_ctx = NULL;
> +struct tevent_req *req = NULL;
> +const char *ldbname = NULL;
> +errno_t ret;
> +
> +test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
> +test_ctx->create_user = true;
> +
> +/* NOTE This user (#2) is stored before filter creation. */
> +ret = sysdb_store_user(test_ctx->tctx->dom, TEST_USER_NAME2, "pwd", 
> 1001, 1001,
> +   NULL, NULL, NULL, "cn="TEST_USER_NAME2",dc=test", 
> NULL,
> +   NULL, 1000, time(NULL));
> +assert_int_equal(ret, EOK);
> +
> +/* NOTE To make sure that the times of user/filter creation will vary.*/
> +sleep(1);
> +
> +req_mem_ctx = talloc_new(global_talloc_context);

Please either allocate the context on test_ctx or just use test_ctx..

> +check_leaks_push(req_mem_ctx);
> +
> +/* Filters always go to DP */
> +will_return(__wrap_sss_dp_get_account_send, test_ctx);
> +mock_account_recv_simple();
> +
> +/* NOTE During this call the TEST_USER_NAME (#1) will be stored. */
> +req = cache_req_user_by_filter_send(req_mem_ctx, test_ctx->tctx->ev,
> +test_ctx->rctx,
> +test_ctx->tctx->dom->name,
> +"test*");
> +assert_non_null(req);
> +
> +tevent_req_set_callback(req, cache_req_user_by_filter_test_done, 
> test_ctx);
> +
> +ret = test_ev_loop(test_ctx->tctx);
> +assert_int_equal(ret, ERR_OK);
> +assert_true(check_leaks_pop(req_mem_ctx));
> +
> +/* NOTE We receive only user #1, because #2 was stored before filter was 
> created. */
> +assert_non_null(test_ctx->result);
> +assert_int_equal(test_ctx->result->count, 1);
> +
> +ldbname = ldb_msg_find_attr_as_string(test_ctx->result->msgs[0],
> +  SYSDB_NAME, NULL);
> +assert_non_null(ldbname);
> +assert_string_equal(ldbname, TEST_USER_NAME);
> +}
> +
> +
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

2015-10-23 Thread Petr Cech 

How to reproduce:

Sumit wrote explanation to ticket comment. Better is if .log files 
missing. And you need run SSSD logging only to journal. Lukas wrote in 
soe mail above in thread, how to enable it.


Thanks to all.

Petr
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] util: Continue if setlocale fails

2015-10-23 Thread Michal Židek 

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?= 
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?= 
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?= 
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?= 
Date: Tue, 20 Oct 2015 15:03:22 +0200
Subject: [PATCH 4/4] tests: Regression test with wrong LC_ALL

Ticket:

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

2015-10-23 Thread Petr Cech 

On 10/23/2015 02:18 PM, Petr Cech wrote:

How to reproduce:

Sumit wrote explanation to ticket comment. Better is if .log files
missing. And you need run SSSD logging only to journal. Lukas wrote in
soe mail above in thread, how to enable it.

Thanks to all.

Petr


# sudo bash
# systemctl stop sssd
# vim /etc/systemd/system/sssd.service.d/journal.conf
# rm /var/log/sssd/*.log
# systemctl daemon-reload
# systemctl start sssd
# journalctl -r | grep 'chown failed'
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] intg: Fix all PEP8 issues

2015-10-23 Thread Lukas Slebodnik 
On (22/10/15 19:52), Nikolai Kondrashov wrote:
>On 10/22/2015 05:55 PM, Michal Židek wrote:
>>On 10/22/2015 04:06 PM, Nikolai Kondrashov wrote:
>>>Hi everyone,
>>>
>>>The attached patch fixes all PEP8 issues in src/tests/intg directory.
>>>I.e. the
>>>integration tests. It was extracted from the "intg: Add more LDAP tests"
>>>thread.
>>>
>>>Nick
>>>
>>>
>>
>>Thank you Nick.
>>Now there are just the
>>"E126 continuation line over-indented for hanging indent"
>>warnings, but we relaxed the condition on that one.
No we didn't.

>Argh, this should have fixed everything. I'll take a look.
>
I would prefer if you could spend time with something more useful.
e.g. preparing samba dc in cwrap envirment.

So I decided to save you some time and attacehd patch fixes
remain pep8 issues.

LS
>From 88122162235e5e40afc26161f80544427f6ca50a Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Fri, 23 Oct 2015 08:52:49 +0200
Subject: [PATCH] intg_tests: Fix PEP8 warnings

---
 src/tests/intg/ent.py   | 28 ++--
 src/tests/intg/ent_test.py  | 14 +++---
 src/tests/intg/ldap_test.py |  6 --
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/src/tests/intg/ent.py b/src/tests/intg/ent.py
index 
0fe08dcbd8195af7dcdeedc5b31dbe5d0d85ff08..2d3d02a539945b00d81f158383ba82b3de8155d3
 100644
--- a/src/tests/intg/ent.py
+++ b/src/tests/intg/ent.py
@@ -129,7 +129,7 @@ def _diff(ent, pattern, desc_map={}):
 items = _get_desc(desc_map, None)[0] + "s"
 if len(unmatched_pattern) > 0:
 return "\nexpected " + items + " not found:\n" + \
-pformat(unmatched_pattern)
+pformat(unmatched_pattern)
 elif isinstance(pattern, list):
 if not isinstance(ent, list):
 return "not a list, " + str(type(ent))
@@ -153,10 +153,10 @@ def _diff(ent, pattern, desc_map={}):
 d = ""
 if len(unmatched_pattern) > 0:
 d += "\nexpected " + items + " not found:\n" + \
-pformat(unmatched_pattern)
+pformat(unmatched_pattern)
 if len(unmatched_ent) != 0:
 d += "\nunexpected " + items + " found:\n" + \
-pformat(unmatched_ent)
+pformat(unmatched_ent)
 if len(d) > 0:
 return d
 else:
@@ -187,13 +187,13 @@ def _convert_passwd(passwd):
 Convert a passwd entry returned by pwd module to an entry dictionary.
 """
 return dict(
-name=passwd.pw_name,
-passwd=passwd.pw_passwd,
-uid=passwd.pw_uid,
-gid=passwd.pw_gid,
-gecos=passwd.pw_gecos,
-dir=passwd.pw_dir,
-shell=passwd.pw_shell
+name=passwd.pw_name,
+passwd=passwd.pw_passwd,
+uid=passwd.pw_uid,
+gid=passwd.pw_gid,
+gecos=passwd.pw_gecos,
+dir=passwd.pw_dir,
+shell=passwd.pw_shell
 )
 
 
@@ -350,10 +350,10 @@ def _convert_group(group):
 Convert a group entry returned by grp module to an entry dictionary.
 """
 return dict(
-name=group.gr_name,
-passwd=group.gr_passwd,
-gid=group.gr_gid,
-mem=group.gr_mem
+name=group.gr_name,
+passwd=group.gr_passwd,
+gid=group.gr_gid,
+mem=group.gr_mem
 )
 
 
diff --git a/src/tests/intg/ent_test.py b/src/tests/intg/ent_test.py
index 
abba94a24f81cb3c099eef396eee57bee7ed0c9b..598930324a41ad9ff473e94e7f2deb302e1bc942
 100644
--- a/src/tests/intg/ent_test.py
+++ b/src/tests/intg/ent_test.py
@@ -169,7 +169,7 @@ def test_assert_each_passwd_by_name(users_and_groups):
 assert False
 except AssertionError as e:
 assert str(e) == \
-   "user 'user1' mismatch: 'name' mismatch: 'user2' != 'user1'"
+"user 'user1' mismatch: 'name' mismatch: 'user2' != 'user1'"
 
 
 def test_assert_each_passwd_by_uid(users_and_groups):
@@ -186,7 +186,7 @@ def test_assert_each_passwd_by_uid(users_and_groups):
 assert False
 except AssertionError as e:
 assert str(e) == \
-   "user 1001 mismatch: 'uid' mismatch: 1002 != 1001"
+"user 1001 mismatch: 'uid' mismatch: 1002 != 1001"
 
 
 def test_assert_each_passwd_with_name(users_and_groups):
@@ -203,7 +203,7 @@ def test_assert_each_passwd_with_name(users_and_groups):
 assert False
 except AssertionError as e:
 assert str(e) == \
-   "user 'user1' mismatch: 'uid' mismatch: 1002 != 1001"
+"user 'user1' mismatch: 'uid' mismatch: 1002 != 1001"
 
 
 def test_assert_each_passwd_with_uid(users_and_groups):
@@ -220,7 +220,7 @@ def test_assert_each_passwd_with_uid(users_and_groups):
 assert False
 except AssertionError as e:
 assert str(e) == \
-   "user 1001 mismatch: 'name' mismatch: 'user2' != 'user1'"
+"user 1001 mismatch: 'name' mismatch: 'user2' != 'user1'"
 
 
 def

Re: [SSSD] [PATCH] sss_override: Add restart requirements to man page

2015-10-23 Thread Lukas Slebodnik 
On (22/10/15 13:35), Pavel Reichl wrote:
>On 10/22/2015 12:27 PM, Pavel Březina wrote:
>>On 10/22/2015 11:23 AM, Pavel Reichl wrote:
>>>
>>>
>>>On 10/21/2015 12:44 PM, Pavel Březina wrote:
On 10/21/2015 12:39 PM, Pavel Reichl wrote:
>Hello,
>
>while reviewing integration tests for sss_override Nick noticed that man
>page does not mention restart requirements. Attached patch (attempts to)
>address that.
>
>Thanks!

LGTM
>>>
>>>Pavel it seems to me that after calling user-del SSSD must be also
>>>restarted, right?
>>
>>No. But you may be hitting memory cache.
>
>Oh, thanks. Please see updated patches.
>
>

>From 073155493b06f3805f282843143a4a020e63bfbf Mon Sep 17 00:00:00 2001
>From: Pavel Reichl 
>Date: Wed, 21 Oct 2015 12:28:37 +0200
>Subject: [PATCH 1/2] sss_override: Add restart requirements to man page
>
>---
> src/man/sss_override.8.xml | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/src/man/sss_override.8.xml b/src/man/sss_override.8.xml
>index 
>24c38936984946b3284d1523a7321a7e7f3d7982..5be4295183a78a9e6625b6689cb1003ce1c01541
> 100644
>--- a/src/man/sss_override.8.xml
>+++ b/src/man/sss_override.8.xml
>@@ -34,8 +34,15 @@
> and groups. This change takes effect only on local machine.
> 
> 
>-Overrides data are stored in SSSD cache. If the cache is deleted
>-all local overrides are lost.
>+Overrides data are stored in the SSSD cache. If the cache is 
>deleted,
>+all local overrides are lost. Please note that after the first
>+override is created using any of the following
>+user-add, group-add,
>+user-import or
>+group-import command. SSSD needs to be
>+restarted to take effect.
>+sss_override prints message when a restart is
>+required.
> 
> 
The patch is different iw we compare to the initial version.
I think Dad deserves a credit so the author should be Dan.
Will you prepare new patch or shoudl we change it before pushing.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] CI: Add --enable-silent-rules flag

2015-10-23 Thread Lukas Slebodnik 
On (22/10/15 16:53), Michal Židek wrote:
>On 10/22/2015 04:43 PM, Jakub Hrozek wrote:
>>On Thu, Oct 22, 2015 at 02:13:11PM +0200, Michal Židek wrote:
>>>On 10/22/2015 02:01 PM, Jakub Hrozek wrote:
On Thu, Oct 22, 2015 at 01:56:23PM +0200, Michal Židek wrote:
>Hi,
>
>I think it is better to enable silent-rules by default
>in CI. See the attached simple patch.
>
>Michal

Why is it better? If the build fails, then chances are we need to see what
is the reason..
>>>
>>>I thought that silent rules do not hide errors. If they do then
>>>of course they should not be enabled.
>>
>>Yes, warnings and errors are displayed, but what I was trying to say is
>>that if there's something wrong with linking binaries for example, then
>>we sometimes need to know exactly what objects were linked with what
>>libraries and the silent rules only show "CC" or "LD" there, especially
>>if all you get is a log.
>>
>>btw that's the reason why Fedora (and RHEL) prefer that silent rules
>>should not be used for builds...
>>
>>>
>>>I see now in the docs:
>>>"Note that silent rules are disabled by default; the user must enable them
>>>explicitly at either configure run time or at make run time. We think that
>>>this is a good policy, since it provides the casual user with enough
>>>information to prepare a good bug report in case anything breaks. "
>>>
>>>So yes, this patch is probably wrong. Sorry.
>>
>>I'm fine with the patch for local CI runs, but not the Jenkins runs..
>
>Thanks for explanation. I think the patch is not needed then.
>I can push the changes just to my local git repo. No need
>for master cahnges.
>
+1
If somene really need silent rule then he can do the local change.

Currently there is not a way how use different options
in local execution and in Jenkins.

I marked the patch in patchwork as rejected.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_override: Add restart requirements to man page

2015-10-23 Thread Pavel Reichl 



On 10/23/2015 09:10 AM, Lukas Slebodnik wrote:

On (22/10/15 13:35), Pavel Reichl wrote:

On 10/22/2015 12:27 PM, Pavel Březina wrote:

On 10/22/2015 11:23 AM, Pavel Reichl wrote:



On 10/21/2015 12:44 PM, Pavel Březina wrote:

On 10/21/2015 12:39 PM, Pavel Reichl wrote:

Hello,

while reviewing integration tests for sss_override Nick noticed that man
page does not mention restart requirements. Attached patch (attempts to)
address that.

Thanks!


LGTM


Pavel it seems to me that after calling user-del SSSD must be also
restarted, right?


No. But you may be hitting memory cache.


Oh, thanks. Please see updated patches.





From 073155493b06f3805f282843143a4a020e63bfbf Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Wed, 21 Oct 2015 12:28:37 +0200
Subject: [PATCH 1/2] sss_override: Add restart requirements to man page

---
src/man/sss_override.8.xml | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/man/sss_override.8.xml b/src/man/sss_override.8.xml
index 
24c38936984946b3284d1523a7321a7e7f3d7982..5be4295183a78a9e6625b6689cb1003ce1c01541
 100644
--- a/src/man/sss_override.8.xml
+++ b/src/man/sss_override.8.xml
@@ -34,8 +34,15 @@
 and groups. This change takes effect only on local machine.
 
 
-Overrides data are stored in SSSD cache. If the cache is deleted
-all local overrides are lost.
+Overrides data are stored in the SSSD cache. If the cache is 
deleted,
+all local overrides are lost. Please note that after the first
+override is created using any of the following
+user-add, group-add,
+user-import or
+group-import command. SSSD needs to be
+restarted to take effect.
+sss_override prints message when a restart is
+required.
 
 

The patch is different iw we compare to the initial version.
I think Dad deserves a credit so the author should be Dan.
Will you prepare new patch or shoudl we change it before pushing.


Change the author before pushing then.

BTW do I deserve something for my effort? :-)
BTW2: second patch was not acked at all, first should be reacked.



LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Use refcount to keep track of server structures returned from failover

2015-10-23 Thread Jakub Hrozek 
On Wed, Oct 21, 2015 at 11:50:07AM +0200, Pavel Březina wrote:
> On 10/11/2015 10:00 PM, Jakub Hrozek wrote:
> >Hi,
> >
> >the attached patches are my proposal to fix
> >https://fedorahosted.org/sssd/ticket/2829
> >
> >I haven't tested them past make check yet, because I'm not sure I like
> >them myself :) but at the same time I can't see a better way to keep
> >track of the servers and let callers set state of servers.
> >
> >The most ugly thing so far IMO is the fo_internal_owner member. I would
> >prefer to instead have a fo_server_wrap structure that would be used for
> >the server_list, but I didn't want to do a large change before we agree
> >the refcount is a good idea at all.
> 
> It is not a good idea, but it is a way to go as we agreed on the call.
> 
> >The other ugly side-effect is that we need to be sure that nobody calls
> >talloc_free on the fo_server structure. Instead, only the parent context
> >can be freed (that's also what the first patch is about).
> 
> Another reason to do failover refactoring soon.
> 
> Ack to the patches.

Thank you, I squashed the patch that reproduces the error to the patch
that fixes the bug, because otherwise the test would fail in CI and
pushed the patches to master:
* 10c07e188323a2f9824b5e34379f3b1a9b37759e
* 4a4af8e1b6a9bab7c7a34d86055a400376e3829e
* 63af9215ea9114062fd87003161e6b5982bf9b1f
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] util: Include disabled domains in link_forest_roots

2015-10-23 Thread Jakub Hrozek 
On Thu, Oct 22, 2015 at 01:00:36PM +0200, Michal Židek wrote:
> On 10/22/2015 11:12 AM, Jakub Hrozek wrote:
> >On Wed, Oct 21, 2015 at 04:27:36PM +0200, Michal Židek wrote:
> >>If you agree that these Coverity complains are false positives
> >>it would be good to push the patches. So far they still apply.
> >>
> >>Michal
> >
> >The code change we did yesterday silenced the Coverity warning, so if
> >you can update the patches also in the autofs responder and re-send I'll
> >ACK and push.
> 
> Thanks Jakub!
> 
> I added new patch to silence the Coverity warnings.
> Other patches are unchanged.
> 
> Michal
> 

ACK to all.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] util: Include disabled domains in link_forest_roots

2015-10-23 Thread Jakub Hrozek 
On Fri, Oct 23, 2015 at 10:32:38AM +0200, Jakub Hrozek wrote:
> On Thu, Oct 22, 2015 at 01:00:36PM +0200, Michal Židek wrote:
> > On 10/22/2015 11:12 AM, Jakub Hrozek wrote:
> > >On Wed, Oct 21, 2015 at 04:27:36PM +0200, Michal Židek wrote:
> > >>If you agree that these Coverity complains are false positives
> > >>it would be good to push the patches. So far they still apply.
> > >>
> > >>Michal
> > >
> > >The code change we did yesterday silenced the Coverity warning, so if
> > >you can update the patches also in the autofs responder and re-send I'll
> > >ACK and push.
> > 
> > Thanks Jakub!
> > 
> > I added new patch to silence the Coverity warnings.
> > Other patches are unchanged.
> > 
> > Michal
> > 
> 
> ACK to all.

master:
 * e563de9203be581acc30c7794f568ae40d22bee0
 * 2bbc9d6f8d5f2c1b07fd6968314b7f530b7f3a4d
 * f191a6f9f3313df88eaf3debf52eebfe5d3dee59
 * c84dcaa58449c53cf038311ce63bb2c304081b9d
 * 877b92e80bde510d5cd9f03dbf01e2bcf73ab072 

Even though the ticket was originally in 1.13.2, I moved it to 1.14
alpha and only pushed the patches to master. We can always backport them
later if we see conflicts, but in general I would prefer to push few
patches to sssd-1-13..
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel