Re: [SSSD] I think #2637 is out of scope of 1.13

2015-09-02 Thread Sumit Bose 
On Tue, Sep 01, 2015 at 08:19:06PM +0200, Jakub Hrozek wrote:
> Hi,
> 
> ssia...
> 
> I was working on implementing https://fedorahosted.org/sssd/ticket/2637
> and while for most occurences it was quite easy to set domain status
> instead of back end status, I hit the sdap_id_op module..
> 
> The sdap_id_op operation sets the offline status directly, in a bit of
> a hackish way by accessing the be_ctx through sdap_id_ctx. It's not
> trivial to change the request to operate on domains, except changing
> sdap_id_op_create() to also accept domain as a parameter..but we already
> have 37 calls to sdap_id_op_create().
> 
> A better way would be to change the whole sdap_id_op request to return an
> error code and don't touch the backend status, but that's even more work
> -- we need to do that and it's something we need to do in the next weeks
> anyway, I just think it's not something that should land in 1.13..
> 
> Thoughts? Should I go ahead and change sdap_id_op_create() to accept
> domain in 1.13 or do refactor the sdap_id_op for 1.14? I would
> personally prefer the latter..

I agree that sdap_id_op should not change the status on its own and that
it should be fixed with the refactoring planned for 1.14.

Nevertheless I wonder if you already have a patch which covers all the
other cases if it wouldn't be an improvement to the current state to add
it?

I would assume that most issue are caused by error during the DNS
lookup, initial connect or authentication which iirc are not handled by
sdap_id_op. I would assume that error during sdap_id_op, e.g. a server
shutdown, are more rare and it would be acceptable to let those errors
switch the whole backend to offline until the refactoring is done.

bye,
Sumit

> 
> btw my work in progress is here:
> https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/log/?h=subdomfo
> ___
> 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 v3] Remove trailing whitespace

2015-09-02 Thread Lukas Slebodnik 
On (01/09/15 15:36), Nikolai Kondrashov wrote:
>On 08/31/2015 09:59 AM, Lukas Slebodnik wrote:
>>diff --git a/src/tests/whitespace_test b/src/tests/whitespace_test
>>new file mode 100755
>>index 
>>..508fe2d532cc6a3967af8eaa5c7dc4dccdc653f3
>>--- /dev/null
>>+++ b/src/tests/whitespace_test
>>@@ -0,0 +1,32 @@
>>+#!/bin/bash
>>+
>>+set -e -u -o pipefail
>>+
>>+# An AWK regex matching tracked file paths to be excluded from the search.
>>+# Example: '.*\.po|README'
>>+PATH_EXCLUDE_REGEX='.*\.po|.*\.patch|.*\.diff'
>
>I suggest we also add '|/debian/.*' here to keep the downstream happy.
>Although it theoretically should be done in their own repo, we can make life
>simpler to them.
>
Even though there isn't big problem with other files in debian
Timo (debian maintainer) prefer the same.

>>+
>>+export GIT_DIR="$ABS_TOP_SRCDIR/.git"
>>+export GIT_WORK_TREE="$ABS_TOP_SRCDIR"
>>+
>>+if [ ! -d "$GIT_DIR" ]; then
>>+echo "Git repository is required for this test!"
>>+exit 77
>>+fi
>
>Strictly speaking, this message should be output to stderr, but that doesn't
>make much difference here.
>
No problem.
Updated.

BTW test passed(was not skipped) even if builddir was outside of GIT_WORK_TREE.

Nikolai,
Do you have any other ideas dow to improve the test?
Or is there a use case which is covered only in your version?

LS
>From 4f364e091abfbcfa68c0323dc05806a7b45477ac Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Tue, 11 Aug 2015 04:54:50 -0400
Subject: [PATCH 1/2] Remove trailing whitespace

---
 README  | 2 +-
 src/conf_macros.m4  | 4 ++--
 src/external/glib.m4| 4 ++--
 src/external/pkg.m4 | 6 +++---
 src/man/sssd.conf.5.xml | 8 
 src/sss_client/ssh/sss_ssh_client.c | 6 +++---
 src/tests/pyhbac-test.py| 1 -
 src/util/dlinklist.h| 8 
 src/util/signal.c   | 8 
 9 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/README b/README
index 
ed08970cee4652626067b628b3a17f3370a73e56..189f66fe5b20c85728ac2640d734e3b490e23817
 100644
--- a/README
+++ b/README
@@ -9,7 +9,7 @@
   an NSS and PAM interface toward the system and a pluggable backend system
   to connect to multiple different account sources.
 
-  More information about SSSD can be found on its project page - 
+  More information about SSSD can be found on its project page -
   
 
   Building and installation
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
index 
c6ce00059094aa0359f6af9efefe7c7438c1e28f..cd731e605b153f69931d3cb0764f10801a596f96
 100644
--- a/src/conf_macros.m4
+++ b/src/conf_macros.m4
@@ -596,11 +596,11 @@ AC_DEFUN([WITH_UNICODE_LIB],
 if test x"$with_unicode_lib" != x; then
 unicode_lib=$with_unicode_lib
 fi
-
+
 if test x"$unicode_lib" != x"libunistring" -a x"$unicode_lib" != x"glib2"; 
then
AC_MSG_ERROR([Unsupported unicode library])
 fi
-
+
 AM_CONDITIONAL([WITH_LIBUNISTRING], test x"$unicode_lib" = x"libunistring")
 AM_CONDITIONAL([WITH_GLIB], test x"$unicode_lib" = x"glib2")
   ])
diff --git a/src/external/glib.m4 b/src/external/glib.m4
index 
8cec763263cd86d4ff1c45a7a3e2afe12954fe1c..3db25136b685bb6be05d1e9b648f032eaeaa1ec5
 100644
--- a/src/external/glib.m4
+++ b/src/external/glib.m4
@@ -3,9 +3,9 @@ PKG_CHECK_MODULES([GLIB2],[glib-2.0])
 if test x$has_glib2 != xno; then
 SAFE_LIBS="$LIBS"
 LIBS="$GLIB2_LIBS"
-
+
 AC_CHECK_FUNC([g_utf8_validate],
   AC_DEFINE([HAVE_G_UTF8_VALIDATE], [1],
 [Define if g_utf8_validate exists]))
 LIBS="$SAFE_LIBS"
-fi
\ No newline at end of file
+fi
diff --git a/src/external/pkg.m4 b/src/external/pkg.m4
index 
a8b3d06c8192229aab14cdc643a8be8f789376fd..568127f104de92fc81fa9b9ccf2fbd2d470840a1
 100644
--- a/src/external/pkg.m4
+++ b/src/external/pkg.m4
@@ -1,5 +1,5 @@
 # pkg.m4 - Macros to locate and utilise pkg-config.-*- Autoconf -*-
-# 
+#
 # Copyright © 2004 Scott James Remnant .
 #
 # This program is free software; you can redistribute it and/or modify
@@ -38,7 +38,7 @@ if test -n "$PKG_CONFIG"; then
AC_MSG_RESULT([no])
PKG_CONFIG=""
fi
-   
+
 fi[]dnl
 ])# PKG_PROG_PKG_CONFIG
 
@@ -119,7 +119,7 @@ if test $pkg_failed = yes; then
 _PKG_SHORT_ERRORS_SUPPORTED
 if test $_pkg_short_errors_supported = yes; then
$1[]_PKG_ERRORS=`$PKG_CONFIG --short-errors --errors-to-stdout 
--print-errors "$2"`
-else 
+else
$1[]_PKG_ERRORS=`$PKG_CONFIG --errors-to-stdout --print-errors 
"$2"`
 fi
# Put the nasty error message in config.log where it belongs
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index

Re: [SSSD] pep8 warnings in python code

2015-09-02 Thread Lukas Slebodnik 
On (01/09/15 14:37), Pavel Reichl wrote:
>This patch fixes some of the pep8 warnings (number of issues reported by pep8
>utility changed from 204 to 127).
>Should we file a ticket to reduce pep8 warning as much as possible? I suppose
>we could even add a test that would fail if new pep8 warning is introduced,
>but I'm little worried that it might prove to be too difficult to write such
>a test in quality that it will be accepted by all SSSD developers. What do
>you think?
>
The quite recent test src/tests/intg/test_memory_cache.py does not have any
pep8 warnings or errors. However there is old python code with many warnings
which would prevent us from wrinting such test.

But there is still way how to test pep8 warnings in new python code.

sh$ pep8 --help
//snip
  --diff   report only lines changed according to the unified diff
   received on STDIN

If we fix/suppress all pep8 warnings then we can write test.

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

[SSSD] Fix #2275 nested netgroups do not work in IPA provider

2015-09-02 Thread Petr Cech 

Hi,

reverting this commit "5e9bc89b28f1ac3ce573ecdece74fe9623580c28" fixed 
the problem for me. So is the original commit no longer valid?


Regards,

Petr
>From 3a161789fc8ef82f4636e55369f4c5b04985f7c2 Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 2 Sep 2015 11:51:12 -0400
Subject: [PATCH] Revert "netgroup: resolve hostgroup membership correctly"

This reverts commit 5e9bc89b28f1ac3ce573ecdece74fe9623580c28.

Ticket: https://fedorahosted.org/sssd/ticket/2275
---
 src/providers/ipa/ipa_netgroups.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_netgroups.c b/src/providers/ipa/ipa_netgroups.c
index db29d29ee8f18d3d963402c4811bdef43bae63dc..8a68ae41311a95d7489868c7d21b739886cf4eea 100644
--- a/src/providers/ipa/ipa_netgroups.c
+++ b/src/providers/ipa/ipa_netgroups.c
@@ -715,7 +715,7 @@ static bool extract_entities(hash_entry_t *entry, void *pvt)
 state = talloc_get_type(pvt, struct extract_state);
 member = talloc_get_type(entry->value.ptr, struct sysdb_attrs);
 
-ret = sysdb_attrs_get_el(member, SYSDB_ORIG_MEMBEROF, );
+ret = sysdb_attrs_get_el(member, SYSDB_MEMBEROF, );
 if (ret != EOK) return false;
 
 ret = sysdb_attrs_get_el(member, SYSDB_NAME, _el);
-- 
2.4.3

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

Re: [SSSD] [WIKI] Contribute and DevelTips are duplicate

2015-09-02 Thread Michal Židek 

On 08/17/2015 02:21 PM, Petr Cech wrote:

On 07/17/2015 01:26 PM, Petr Cech wrote:

Hi,

I have read the wiki pages. And I have the edited version. It would be
difficult to send the diff, so I started a new pages where you can
view the result.

Original pages:
[ 1] https://fedorahosted.org/sssd/wiki/Contribute
[ 2] https://fedorahosted.org/sssd/wiki/DevelTips
[ 3] https://fedorahosted.org/sssd/wiki/DevelTutorials
[ 4] https://fedorahosted.org/sssd/wiki/Reporting_sssd_bugs
[ 5] https://fedorahosted.org/sssd/wiki/BugLifecycle
[ 6] https://fedorahosted.org/sssd/wiki/Repositories

Content of [3] has been divided between [1] and [3], content of [5]
has been divided between [1] and [4]. Then [3,5,6] will be deleted.

Test of new pages:
[ 7] https://fedorahosted.org/sssd/wiki/pcech_test_contribute
[ 8] https://fedorahosted.org/sssd/wiki/pcech_test_devel_tips
[ 9] https://fedorahosted.org/sssd/wiki/pcech_test_reporting_sssd_bugs

Note that the links lead to the original pages.
At [7] you can find "COPR Repository" section, but I am not sure with
text here. Please look at it.
I did not pass the whole wiki. I think there might be a link from [8]
(perhaps [9]) on Troubleshooting.

I look forward to your comments, I need the opinions of another persons.

Petr


Hi,

a did some little edits according to talk with Jakub:
   * deleting Code Submission Process in Contribute
   * simplifying the structure of the headings in Contribute
   * adding link to tevent documentation in Devel tips
   * merging SSSD bug report
and we would like to move link to COPR repo to the homepage (and add
note about Ubuntu package, is it right?)

So new version (without homepage and link to Ubuntu repo) is on the same
place:
[ 7] https://fedorahosted.org/sssd/wiki/pcech_test_contribute
[ 8] https://fedorahosted.org/sssd/wiki/pcech_test_devel_tips
[ 9] https://fedorahosted.org/sssd/wiki/pcech_test_reporting_sssd_bugs

Petr


Hi!

I think that Petr's changes to Wiki are improvement over the
current state. He removes a lot of duplicated and outdated
info. So if nobody objects I would like Petr to go ahead
and replace the current pages with the new ones.

I have one comment: Does somebody know how to move the
table of contents to the left? Currently it is in the upper
right corner and I think (especially on bigger monitors)
it is really not easy to spot. The table is IMO very important
and gives good outline of what to expect from the page
so I would really like to have it on the left nice and
visible.

Also I like the idea of revisiting the wiki pages regularly
in order to further improve them and keep them up-to-date.
I think the overall navigation on our wiki has room for
improvement, but we do not need to do everything at once.

Michal

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

Re: [SSSD] [WIKI] Contribute and DevelTips are duplicate

2015-09-02 Thread Jakub Hrozek 
On Wed, Sep 02, 2015 at 05:18:24PM +0200, Michal Židek wrote:
> On 08/17/2015 02:21 PM, Petr Cech wrote:
> >On 07/17/2015 01:26 PM, Petr Cech wrote:
> >>Hi,
> >>
> >>I have read the wiki pages. And I have the edited version. It would be
> >>difficult to send the diff, so I started a new pages where you can
> >>view the result.
> >>
> >>Original pages:
> >>[ 1] https://fedorahosted.org/sssd/wiki/Contribute
> >>[ 2] https://fedorahosted.org/sssd/wiki/DevelTips
> >>[ 3] https://fedorahosted.org/sssd/wiki/DevelTutorials
> >>[ 4] https://fedorahosted.org/sssd/wiki/Reporting_sssd_bugs
> >>[ 5] https://fedorahosted.org/sssd/wiki/BugLifecycle
> >>[ 6] https://fedorahosted.org/sssd/wiki/Repositories
> >>
> >>Content of [3] has been divided between [1] and [3], content of [5]
> >>has been divided between [1] and [4]. Then [3,5,6] will be deleted.
> >>
> >>Test of new pages:
> >>[ 7] https://fedorahosted.org/sssd/wiki/pcech_test_contribute
> >>[ 8] https://fedorahosted.org/sssd/wiki/pcech_test_devel_tips
> >>[ 9] https://fedorahosted.org/sssd/wiki/pcech_test_reporting_sssd_bugs
> >>
> >>Note that the links lead to the original pages.
> >>At [7] you can find "COPR Repository" section, but I am not sure with
> >>text here. Please look at it.
> >>I did not pass the whole wiki. I think there might be a link from [8]
> >>(perhaps [9]) on Troubleshooting.
> >>
> >>I look forward to your comments, I need the opinions of another persons.
> >>
> >>Petr
> >
> >Hi,
> >
> >a did some little edits according to talk with Jakub:
> >   * deleting Code Submission Process in Contribute
> >   * simplifying the structure of the headings in Contribute
> >   * adding link to tevent documentation in Devel tips
> >   * merging SSSD bug report
> >and we would like to move link to COPR repo to the homepage (and add
> >note about Ubuntu package, is it right?)
> >
> >So new version (without homepage and link to Ubuntu repo) is on the same
> >place:
> >[ 7] https://fedorahosted.org/sssd/wiki/pcech_test_contribute
> >[ 8] https://fedorahosted.org/sssd/wiki/pcech_test_devel_tips
> >[ 9] https://fedorahosted.org/sssd/wiki/pcech_test_reporting_sssd_bugs
> >
> >Petr
> 
> Hi!
> 
> I think that Petr's changes to Wiki are improvement over the
> current state. He removes a lot of duplicated and outdated
> info. So if nobody objects I would like Petr to go ahead
> and replace the current pages with the new ones.

Thank you very much for review, they looked good to me as well when we
discussed the changes in person last time.

Petr, please move the pages and then send a mail to the list about the
update, we can always change more stuff or even roll back.

> 
> I have one comment: Does somebody know how to move the
> table of contents to the left? Currently it is in the upper
> right corner and I think (especially on bigger monitors)
> it is really not easy to spot. The table is IMO very important
> and gives good outline of what to expect from the page
> so I would really like to have it on the left nice and
> visible.

I only found http://trac.edgewall.org/wiki/PageOutline about the macro.

> 
> Also I like the idea of revisiting the wiki pages regularly
> in order to further improve them and keep them up-to-date.
> I think the overall navigation on our wiki has room for
> improvement, but we do not need to do everything at once.


Hmm I guess I missed that how exactly are we going to update them
regularly? (I agree we should, I'm just interested in the mechanics)
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [WIKI] Contribute and DevelTips are duplicate

2015-09-02 Thread Michal Židek 

On 09/02/2015 05:47 PM, Jakub Hrozek wrote:

On Wed, Sep 02, 2015 at 05:18:24PM +0200, Michal Židek wrote:

On 08/17/2015 02:21 PM, Petr Cech wrote:

On 07/17/2015 01:26 PM, Petr Cech wrote:

Hi,

I have read the wiki pages. And I have the edited version. It would be
difficult to send the diff, so I started a new pages where you can
view the result.

Original pages:
[ 1] https://fedorahosted.org/sssd/wiki/Contribute
[ 2] https://fedorahosted.org/sssd/wiki/DevelTips
[ 3] https://fedorahosted.org/sssd/wiki/DevelTutorials
[ 4] https://fedorahosted.org/sssd/wiki/Reporting_sssd_bugs
[ 5] https://fedorahosted.org/sssd/wiki/BugLifecycle
[ 6] https://fedorahosted.org/sssd/wiki/Repositories

Content of [3] has been divided between [1] and [3], content of [5]
has been divided between [1] and [4]. Then [3,5,6] will be deleted.

Test of new pages:
[ 7] https://fedorahosted.org/sssd/wiki/pcech_test_contribute
[ 8] https://fedorahosted.org/sssd/wiki/pcech_test_devel_tips
[ 9] https://fedorahosted.org/sssd/wiki/pcech_test_reporting_sssd_bugs

Note that the links lead to the original pages.
At [7] you can find "COPR Repository" section, but I am not sure with
text here. Please look at it.
I did not pass the whole wiki. I think there might be a link from [8]
(perhaps [9]) on Troubleshooting.

I look forward to your comments, I need the opinions of another persons.

Petr


Hi,

a did some little edits according to talk with Jakub:
   * deleting Code Submission Process in Contribute
   * simplifying the structure of the headings in Contribute
   * adding link to tevent documentation in Devel tips
   * merging SSSD bug report
and we would like to move link to COPR repo to the homepage (and add
note about Ubuntu package, is it right?)

So new version (without homepage and link to Ubuntu repo) is on the same
place:
[ 7] https://fedorahosted.org/sssd/wiki/pcech_test_contribute
[ 8] https://fedorahosted.org/sssd/wiki/pcech_test_devel_tips
[ 9] https://fedorahosted.org/sssd/wiki/pcech_test_reporting_sssd_bugs

Petr


Hi!

I think that Petr's changes to Wiki are improvement over the
current state. He removes a lot of duplicated and outdated
info. So if nobody objects I would like Petr to go ahead
and replace the current pages with the new ones.


Thank you very much for review, they looked good to me as well when we
discussed the changes in person last time.

Petr, please move the pages and then send a mail to the list about the
update, we can always change more stuff or even roll back.



I have one comment: Does somebody know how to move the
table of contents to the left? Currently it is in the upper
right corner and I think (especially on bigger monitors)
it is really not easy to spot. The table is IMO very important
and gives good outline of what to expect from the page
so I would really like to have it on the left nice and
visible.


I only found http://trac.edgewall.org/wiki/PageOutline about the macro.



Also I like the idea of revisiting the wiki pages regularly
in order to further improve them and keep them up-to-date.
I think the overall navigation on our wiki has room for
improvement, but we do not need to do everything at once.



Hmm I guess I missed that how exactly are we going to update them
regularly? (I agree we should, I'm just interested in the mechanics)


We did not agree on the mechanics yet. We could make it part
of release cycle. I would also not be against filing tickets
with titles like: "Review and update wiki pages for 1.14.1".
This would force us to look at the wiki at least once per
release (or every major version release). I am not sure if
this is good idea, but we can change the mechanics any time.

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

Re: [SSSD] Fix #2275 nested netgroups do not work in IPA provider

2015-09-02 Thread Lukas Slebodnik 
On (02/09/15 18:06), Petr Cech wrote:
>Hi,
>
>reverting this commit "5e9bc89b28f1ac3ce573ecdece74fe9623580c28" fixed the
>problem for me. So is the original commit no longer valid?
>
I'm little bit worried about reverting this patch.
Did you test the bug which was fixed by this commit.
@see https://fedorahosted.org/sssd/ticket/1519

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

Re: [SSSD] [PATCH] SYSDB: Index the objectSIDString attribute

2015-09-02 Thread Lukas Slebodnik 
On (19/08/15 18:15), Jakub Hrozek wrote:
>On Tue, Aug 18, 2015 at 05:31:43PM +0200, Michal Židek wrote:
>> On 08/17/2015 10:35 PM, Jakub Hrozek wrote:
>> >Hi,
>> >
>> >the attached patch was confirmed to work, so the code review should be
>> >easy. But because it adds an index to objectSID, which all AD objects
>> >have, there are two catches:
>> > 1) How log the upgrade takes
>> 
>> My main concern was the heartbeat interval. But I just tested it by
>> adding sleep(50) into your newly added upgrade function and it
>> work fine with default heartbeat (10 seconds). So I guess monitor
>> starts pinging the domain backend after it is fully initialized
>> and not sooner.
>> 
>> > 2) How much the database grows
>> >
>> >To test, I created an AD instance with 10.000 users and 10.000 groups
>> >(adcli is great for this type of testing btw). Fetched all groups and
>> >users with the old db, then ran the upgrade.
>> >
>> >On a VM running on my local laptop (granted, it has an SSD drive), the
>> >upgrade takes 10 seconds. The default systemd startup timeout
>> >(DefaultTimeoutStartSec) seems to be 90 seconds, which sounds OK to me.
>> >
>> >The size, though, has nearly doubled. This is backup before upgrade:
>> > [root@adclient ~]# du -sh /root/cache_win.trust.test.ldb
>> > 47M /root/cache_win.trust.test.ldb
>> >And this is the cache after I upgraded:
>> > [root@adclient ~]# du -sh /var/lib/sss/db/cache_win.trust.test.ldb
>> > 97M /var/lib/sss/db/cache_win.trust.test.ldb
>> >
>> >Unfortunately, I don't see us having another option than doing the
>> >upgrade. For attributes that are indexed, an index miss means that ldb
>> >is not going to search sequentially, but just return not found -- so
>> >adding indexes for newly added entries is not possible.
>> >
>> >I would like to document that for huge databases (tens of thousands of
>> >cached entries), the timeout might need to be raised during the upgrade
>> >and than for deployments whose cache resides in tmpfs, the cache size
>> >might grow.
>> >
>> >Is everyone OK with this?
>> >
>> 
>> It should be in realease notes, but I am not sure if that
>> is enough visible place, so I would suggest putting it to Wiki
>> as well (https://fedorahosted.org/sssd/wiki/Troubleshooting)
>> for the case when someone hits the systemd timeout.
>> 
>> ACK to the patch.
>
>* master: e61b0e41cb44004d2b260ad9d05802995f7bcb2e
Shall we push this performance enhancement to stable branch(1.12) as well?

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

Re: [SSSD] [PATCH v3] Remove trailing whitespace

2015-09-02 Thread Nikolai Kondrashov 

On 09/02/2015 10:05 AM, Lukas Slebodnik wrote:

On (01/09/15 15:36), Nikolai Kondrashov wrote:

On 08/31/2015 09:59 AM, Lukas Slebodnik wrote:

diff --git a/src/tests/whitespace_test b/src/tests/whitespace_test
new file mode 100755
index 
..508fe2d532cc6a3967af8eaa5c7dc4dccdc653f3
--- /dev/null
+++ b/src/tests/whitespace_test
@@ -0,0 +1,32 @@
+#!/bin/bash
+
+set -e -u -o pipefail
+
+# An AWK regex matching tracked file paths to be excluded from the search.
+# Example: '.*\.po|README'
+PATH_EXCLUDE_REGEX='.*\.po|.*\.patch|.*\.diff'


I suggest we also add '|/debian/.*' here to keep the downstream happy.
Although it theoretically should be done in their own repo, we can make life
simpler to them.


Even though there isn't big problem with other files in debian
Timo (debian maintainer) prefer the same.


+
+export GIT_DIR="$ABS_TOP_SRCDIR/.git"
+export GIT_WORK_TREE="$ABS_TOP_SRCDIR"
+
+if [ ! -d "$GIT_DIR" ]; then
+echo "Git repository is required for this test!"
+exit 77
+fi


Strictly speaking, this message should be output to stderr, but that doesn't
make much difference here.


No problem.
Updated.

BTW test passed(was not skipped) even if builddir was outside of GIT_WORK_TREE.


Sure. I was concerned with the other git uses, i.e. the rpm* targets.


Nikolai,
Do you have any other ideas dow to improve the test?
Or is there a use case which is covered only in your version?


Hmm, no, I don't think there is anything else to do here.

I haven't tested it, otherwise ACK.

Thank you, Lukas.

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

Re: [SSSD] [PATCH v3] Remove trailing whitespace

2015-09-02 Thread Lukas Slebodnik 
On (02/09/15 14:39), Nikolai Kondrashov wrote:
>On 09/02/2015 10:05 AM, Lukas Slebodnik wrote:
>>On (01/09/15 15:36), Nikolai Kondrashov wrote:
>>>On 08/31/2015 09:59 AM, Lukas Slebodnik wrote:
diff --git a/src/tests/whitespace_test b/src/tests/whitespace_test
new file mode 100755
index 
..508fe2d532cc6a3967af8eaa5c7dc4dccdc653f3
--- /dev/null
+++ b/src/tests/whitespace_test
@@ -0,0 +1,32 @@
+#!/bin/bash
+
+set -e -u -o pipefail
+
+# An AWK regex matching tracked file paths to be excluded from the search.
+# Example: '.*\.po|README'
+PATH_EXCLUDE_REGEX='.*\.po|.*\.patch|.*\.diff'
>>>
>>>I suggest we also add '|/debian/.*' here to keep the downstream happy.
>>>Although it theoretically should be done in their own repo, we can make life
>>>simpler to them.
>>>
>>Even though there isn't big problem with other files in debian
>>Timo (debian maintainer) prefer the same.
>>
+
+export GIT_DIR="$ABS_TOP_SRCDIR/.git"
+export GIT_WORK_TREE="$ABS_TOP_SRCDIR"
+
+if [ ! -d "$GIT_DIR" ]; then
+echo "Git repository is required for this test!"
+exit 77
+fi
>>>
>>>Strictly speaking, this message should be output to stderr, but that doesn't
>>>make much difference here.
>>>
>>No problem.
>>Updated.
>>
>>BTW test passed(was not skipped) even if builddir was outside of 
>>GIT_WORK_TREE.
>
>Sure. I was concerned with the other git uses, i.e. the rpm* targets.
>
In case of "make rpms" or "make prerelease-rpms", we run rpmbuild
for building packages. And rpmbuild uses tarball as a source so
ABS_TOP_SRCDIR would not be the same as GIT_WORK_TREE.

>>Nikolai,
>>Do you have any other ideas dow to improve the test?
>>Or is there a use case which is covered only in your version?
>
>Hmm, no, I don't think there is anything else to do here.
>
>I haven't tested it, otherwise ACK.
>
Thank you for review. But It would be good if 3rd person
formally ACK the patch because we(both) are authors of 2nd patch :-)

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

Re: [SSSD] [PATCH] NSS: Don't ignore backslash in usernames with ldap provider

2015-09-02 Thread Lukas Slebodnik 
On (31/08/15 21:01), Sumit Bose wrote:
>On Fri, Aug 28, 2015 at 07:23:29AM +0200, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> please review attached patch for regression #2772
>> 
>> LS
>
>Hi Lukas,
>
>thank you for taking care of the issue.
>
>The patch is working as expected without breaking the SID lookups and
>passes the CI http://sssd-ci.duckdns.org/logs/job/23/92/summary.html so
>ACK.
>
>I just want to mention that in the old version it was possible to change
>global_names pattern by setting re_expression in the [sssd] section of
>sssd.conf. But given the current usage for global_names I can hardly
>imagine any reason why the pattern should be changed.
>
I'm little bit confused.

Should we implement overriding global_names with re_expression
in the [sssd] section? Is there a valid use-case?

OR

Is better that users cannot override it?

I think it's late for downstream but we can file at least upstream ticket.

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

Re: [SSSD] [PATCH v3] Remove trailing whitespace

2015-09-02 Thread Pavel Reichl 

On 09/02/2015 02:25 PM, Jakub Hrozek wrote:

On Wed, Sep 02, 2015 at 01:52:42PM +0200, Lukas Slebodnik wrote:

On (02/09/15 14:39), Nikolai Kondrashov wrote:

I haven't tested it, otherwise ACK.


Thank you for review. But It would be good if 3rd person
formally ACK the patch because we(both) are authors of 2nd patch :-)


Pavel, you were involved in the original thread, please help out Lukas
and Nikolai here.



LGTM, ci passed: http://sssd-ci.duckdns.org/logs/job/24/27/summary.html

I added whitespace to source files - it was detected.
I added whitespace to po files  - it was ignored.

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

Re: [SSSD] I think #2637 is out of scope of 1.13

2015-09-02 Thread Jakub Hrozek 
On Wed, Sep 02, 2015 at 10:11:46AM +0200, Sumit Bose wrote:
> On Tue, Sep 01, 2015 at 08:19:06PM +0200, Jakub Hrozek wrote:
> > Hi,
> > 
> > ssia...
> > 
> > I was working on implementing https://fedorahosted.org/sssd/ticket/2637
> > and while for most occurences it was quite easy to set domain status
> > instead of back end status, I hit the sdap_id_op module..
> > 
> > The sdap_id_op operation sets the offline status directly, in a bit of
> > a hackish way by accessing the be_ctx through sdap_id_ctx. It's not
> > trivial to change the request to operate on domains, except changing
> > sdap_id_op_create() to also accept domain as a parameter..but we already
> > have 37 calls to sdap_id_op_create().
> > 
> > A better way would be to change the whole sdap_id_op request to return an
> > error code and don't touch the backend status, but that's even more work
> > -- we need to do that and it's something we need to do in the next weeks
> > anyway, I just think it's not something that should land in 1.13..
> > 
> > Thoughts? Should I go ahead and change sdap_id_op_create() to accept
> > domain in 1.13 or do refactor the sdap_id_op for 1.14? I would
> > personally prefer the latter..
> 
> I agree that sdap_id_op should not change the status on its own and that
> it should be fixed with the refactoring planned for 1.14.
> 
> Nevertheless I wonder if you already have a patch which covers all the
> other cases if it wouldn't be an improvement to the current state to add
> it?
> 
> I would assume that most issue are caused by error during the DNS
> lookup, initial connect or authentication which iirc are not handled by
> sdap_id_op.

The LDAP connection and the failover during the connection is handled by
sdap_id_op. That's the crux of the problem I didn't realize when doing
the initial design (frankly I didn't think the sdap operation was in the
business of changing failover status and offline status for the backend..)

> I would assume that error during sdap_id_op, e.g. a server
> shutdown, are more rare and it would be acceptable to let those errors
> switch the whole backend to offline until the refactoring is done.

Hmm perhaps you're right that we can try solve at least the use-cases
while we have the code in mind. I think we mostly care about:
- don't bring the whole AD backend offline if there are connection
  problems with discovering subdomains when AD client is connected
  to a subdomain
- don't bring the whole IPA backend offline if there are connection
  problems with AD server in server mode
- don't bring the while IPA backend offline if there are connection
  problems with AD server with IPA client

I think these are solvable without major hacks. I'll see about fixing
these use-cases.

The rest needs to be fixed when I work on the LDAP code and Pavel
Brezina works on the failover changes.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] NSS: Don't ignore backslash in usernames with ldap provider

2015-09-02 Thread Sumit Bose 
On Wed, Sep 02, 2015 at 01:23:11PM +0200, Lukas Slebodnik wrote:
> On (31/08/15 21:01), Sumit Bose wrote:
> >On Fri, Aug 28, 2015 at 07:23:29AM +0200, Lukas Slebodnik wrote:
> >> ehlo,
> >> 
> >> please review attached patch for regression #2772
> >> 
> >> LS
> >
> >Hi Lukas,
> >
> >thank you for taking care of the issue.
> >
> >The patch is working as expected without breaking the SID lookups and
> >passes the CI http://sssd-ci.duckdns.org/logs/job/23/92/summary.html so
> >ACK.
> >
> >I just want to mention that in the old version it was possible to change
> >global_names pattern by setting re_expression in the [sssd] section of
> >sssd.conf. But given the current usage for global_names I can hardly
> >imagine any reason why the pattern should be changed.
> >
> I'm little bit confused.
> 
> Should we implement overriding global_names with re_expression
> in the [sssd] section? Is there a valid use-case?
> 
> OR
> 
> Is better that users cannot override it?
> 
> I think it's late for downstream but we can file at least upstream ticket.

no, the current state is fine. As said I cannot think of a use-case, if
there is one coming up later we can add an option to change global_names
then.

bye,
Sumit

> 
> 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 v3] Remove trailing whitespace

2015-09-02 Thread Jakub Hrozek 
On Wed, Sep 02, 2015 at 01:52:42PM +0200, Lukas Slebodnik wrote:
> On (02/09/15 14:39), Nikolai Kondrashov wrote:
> >I haven't tested it, otherwise ACK.
> >
> Thank you for review. But It would be good if 3rd person
> formally ACK the patch because we(both) are authors of 2nd patch :-)

Pavel, you were involved in the original thread, please help out Lukas
and Nikolai here.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing

2015-09-02 Thread Lukas Slebodnik 
On (01/09/15 12:55), Michal Židek wrote:
>On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
>>On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
>>>On (01/09/15 10:51), Jakub Hrozek wrote:
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
>On (10/08/15 17:18), Michal Židek wrote:
>>On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
>>>On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
-} else if (version < CONFDB_VERSION_INT) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-"Config file is an old version. "
- "Please run configuration upgrade script.\n");
-ret = EINVAL;
-goto done;
-} else if (version > CONFDB_VERSION_INT) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-"Config file version is newer than confdb\n");
-ret = EINVAL;
-goto done;
+/* No known version. Use default. */
+DEBUG(SSSDBG_CONF_SETTINGS,
+  "Value of config_file_version option not found. "
+  "Assumed to be version %d.\n", 
CONFDB_DEFAULT_CFG_FILE_VER);
+} else {
+version = sss_ini_get_int_config_value(init_data,
+   
CONFDB_DEFAULT_CFG_FILE_VER,
+   -1, );
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+"Config file version could not be 
determined\n");
>   ^^
>I do not prefer nested "if"s. If you decided to do it in this way then

Me neither, but sss_ini_get_int_config_value() has to be
skipped conditionally. It is just call to the function
plus error checking that is nested. I think it is not
too bad in this case.

>you shoudl have proper indentation.
>

Fixed in the new version.

>>>It works because integration tests passed.
>>>http://sssd-ci.duckdns.org/logs/job/20/68/summary.html
>>>But ...
>>>
>>>I tested new version with ipa-client-install
>>>and "config_file_version = 2" is still added to sssd.conf
>>>even though it is a default value.
>>>
>>>ipa-client-install uses our python API (python-sssdconfig)
>>>and it does not try to add this option itself.
>>
>>I often change version of SSSD with git checkout sssd.
>>If I generate the config with realmd or ipa-client-install
>realmd do not use python module SSSDConfig.
>
>>with latest version then the config_version_file
>>would need to be added manually (and I am pretty
>>sure it would be after I looked into logs to see why sssd
>>is not starting). I know this will probably only hit
>>testers/developers, but I would prefer not to add unnecessary
>just developers and developers useually join machine
>to AD or IPA just once.
>
>>little inconveniences.
>>
>I do not try old sssd version very often. Just in case of bisect.
>and ther is nice/simple workarount for "little inconvenience"
>
>Just manually add "config_version_file = 2" to sssd.conf
>
>The main point of this ticket is to simplify sssd.conf
>and our python sssdconfig API should do the same.
>Otherwise we do not need to do such change.
>
>I still see "config_file_version = 2" in sssd.conf
>It was generated by python module SSSDConfig (via ipa-client-install)
>
>LS

This thread has stalled, let's try to restart it.

What versions are normally still being worked on by developers? I
suspect it's master and sssd-1-12. What about pushing the patch to
sssd-1-12 as well?
>>>I'm fine with sssd-1-12.
>>>The python sssdconfig need to be updated.
>>>
>>>LS
>>
>>Michal, if you agree, please update the patches so that we can push them
>>in time for sssd-1.13.1
>
>New patch attached. I tested ipa-client-install and the
>config_file_version is no longer added.
>
>Michal
>

>From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>Date: Tue, 7 Jul 2015 15:15:32 +0200
>Subject: [PATCH] CONFDB: Assume config file version 2 if missing
>
>Default to config file version 2 if the version
>is not specified explicitly.
>
>Ticket:
>https://fedorahosted.org/sssd/ticket/2688
>---
* integration tests passed
* ipa-client generated config without config_file_version and sssd worked.

ACK

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

Re: [SSSD] [PATCH] NSS: Don't ignore backslash in usernames with ldap provider

2015-09-02 Thread Lukas Slebodnik 
On (02/09/15 14:10), Sumit Bose wrote:
>On Wed, Sep 02, 2015 at 01:23:11PM +0200, Lukas Slebodnik wrote:
>> On (31/08/15 21:01), Sumit Bose wrote:
>> >On Fri, Aug 28, 2015 at 07:23:29AM +0200, Lukas Slebodnik wrote:
>> >> ehlo,
>> >> 
>> >> please review attached patch for regression #2772
>> >> 
>> >> LS
>> >
>> >Hi Lukas,
>> >
>> >thank you for taking care of the issue.
>> >
>> >The patch is working as expected without breaking the SID lookups and
>> >passes the CI http://sssd-ci.duckdns.org/logs/job/23/92/summary.html so
>> >ACK.
>> >
>> >I just want to mention that in the old version it was possible to change
>> >global_names pattern by setting re_expression in the [sssd] section of
>> >sssd.conf. But given the current usage for global_names I can hardly
>> >imagine any reason why the pattern should be changed.
>> >
>> I'm little bit confused.
>> 
>> Should we implement overriding global_names with re_expression
>> in the [sssd] section? Is there a valid use-case?
>> 
>> OR
>> 
>> Is better that users cannot override it?
>> 
>> I think it's late for downstream but we can file at least upstream ticket.
>
>no, the current state is fine. As said I cannot think of a use-case, if
>there is one coming up later we can add an option to change global_names
>then.
>
Thank you very much for clarification.

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

Re: [SSSD] [PATCH] SDAP: Remove unused function

2015-09-02 Thread Pavel Reichl 


On 09/02/2015 04:16 PM, Jakub Hrozek wrote:

Unused code is broken code, remove it :)



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

Should we improve comment in remove_ldap_connection_callbacks() which 
mentions sdap_mark_offline()?

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

[SSSD] [PATCH] SDAP: Remove unused function

2015-09-02 Thread Jakub Hrozek 
Unused code is broken code, remove it :)
>From e3d8eda8dcf32adbdd001915d0308f16f6e45dfb Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 1 Sep 2015 17:50:32 +0200
Subject: [PATCH] SDAP: Remove unused function

---
 src/providers/ldap/ldap_common.c | 5 -
 src/providers/ldap/ldap_common.h | 2 --
 2 files changed, 7 deletions(-)

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 
840a0987180935b16c774b8ea8dc0aaca0f846b1..aa4c6cb851a5735e051ef2c024ca0171a4f61148
 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -44,11 +44,6 @@ void sdap_handler_done(struct be_req *req, int dp_err,
 return be_req_terminate(req, dp_err, error, errstr);
 }
 
-void sdap_mark_offline(struct sdap_id_ctx *ctx)
-{
-be_mark_offline(ctx->be);
-}
-
 int ldap_id_setup_tasks(struct sdap_id_ctx *ctx)
 {
 return sdap_id_setup_tasks(ctx->be, ctx, ctx->opts->sdom,
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index 
8294d1db23bdca8d94a098533d93405c4d55226b..716aaa08ef83b8fca3779b830fdef20bbdb14937
 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -210,8 +210,6 @@ errno_t ldap_setup_cleanup(struct sdap_id_ctx *id_ctx,
 errno_t ldap_id_cleanup(struct sdap_options *opts,
 struct sdap_domain *sdom);
 
-void sdap_mark_offline(struct sdap_id_ctx *ctx);
-
 struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct sdap_id_ctx *ctx,
-- 
2.4.3

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