[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

jhrozek commented:
"""
master:
e0903f41922721edf292a9f7e6605a4519db53a1
eaf44bc07dda469a20be07d46737d93f518e2047
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253887578
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
So, ldap patch passed intg. tests locally.

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253782152
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

jhrozek commented:
"""
yes, if we don't know how to trigger the code, we shouldn't push it.

OK to push the two commits now?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253781462
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

lslebodn commented:
"""
On (14/10/16 04:48), celestian wrote:
>I did manual testing with reproducer above. And I ran chmake (it is without 
>intg., isn't it).
>Now I check ldap patch with intg.
>
Then the question is why manual testing is different than newly added
integration tests.

BTW It is possible that patch in memberof plugin can safe some unnecessary
ldb operations and can be considered as perfomance enhancement.
But it's impossible to say that without proper integration test.

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253780298
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
I did manual testing with reproducer above. And I ran chmake (it is without 
intg., isn't it).
Now I check ldap patch with intg.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253779213
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

lslebodn commented:
"""
On (14/10/16 04:33), Jakub Hrozek wrote:
>btw since the memberof patch we couldn't test is gone, I'm fine with pushing 
>these two.
>
I would like to know Petr's explanation of his comment before pushing the
patches.

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253776910
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

jhrozek commented:
"""
btw since the memberof patch we couldn't test is gone, I'm fine with pushing 
these two.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253776046
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

lslebodn commented:
"""
> Great, the result is the same. Both patches could fix it. So we can use just 
> ldap patch.

What do you mean by both patches could fix it?

I aplied just a memboerof patch + test
```
git log --oneline -3
1cde694 TESTS: Adding intg. tests on nested groups
5a9c686 MEMBEROF: Don't resolve members if they are removed
761515e sssctl: call service with absolute path
```

and test failed
```
make -C src/tests/intg intgcheck-installed INTGCHECK_PYTEST_ARGS=" -k 
test_ldap.py"
//snip
test_ldap.py::test_user_2307bis_nested_groups PASSED
test_ldap.py::test_special_characters_in_names PASSED
test_ldap.py::test_extra_attribute_already_exists PASSED
test_ldap.py::test_add_user_to_group PASSED
test_ldap.py::test_remove_user_from_group FAILED
test_ldap.py::test_remove_user_from_nested_group FAILED

 FAILURES 

__ test_remove_user_from_group 
___
Traceback (most recent call last):
  File "src/tests/intg/test_ldap.py", line 877, in test_remove_user_from_group
ent.assert_group_by_name("group1", dict(mem=ent.contains_only()))
  File "src/tests/intg/ent.py", line 377, in assert_group_by_name
assert not d, d
AssertionError: member list mismatch: 
unexpected members found:
['user1']
___ test_remove_user_from_nested_group 
___
Traceback (most recent call last):
  File "src/tests/intg/test_ldap.py", line 953, in 
test_remove_user_from_nested_group
dict(mem=ent.contains_only()))
  File "src/tests/intg/ent.py", line 377, in assert_group_by_name
assert not d, d
AssertionError: member list mismatch: 
unexpected members found:
['user1']
 102 tests deselected by '-ktest_ldap.py' 

== 2 failed, 15 passed, 102 deselected in 46.02 seconds 
==
Makefile:728: recipe for target 'intgcheck-installed' failed
make: *** [intgcheck-installed] Error 1
make: Leaving directory 'intg/bld/src/tests/intg'
```
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253772653
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

lslebodn commented:
"""
> Great, the result is the same. Both patches could fix it. So we can use just 
> ldap patch.
What do you mean by both patches could fix it?

I aplied just a memboerof patch + test
```
git log --oneline -3
1cde694 TESTS: Adding intg. tests on nested groups
5a9c686 MEMBEROF: Don't resolve members if they are removed
761515e sssctl: call service with absolute path
```

and test failed
```
make -C src/tests/intg intgcheck-installed INTGCHECK_PYTEST_ARGS=" -k 
test_ldap.py"
//snip
test_ldap.py::test_user_2307bis_nested_groups PASSED
test_ldap.py::test_special_characters_in_names PASSED
test_ldap.py::test_extra_attribute_already_exists PASSED
test_ldap.py::test_add_user_to_group PASSED
test_ldap.py::test_remove_user_from_group FAILED
test_ldap.py::test_remove_user_from_nested_group FAILED

 FAILURES 

__ test_remove_user_from_group 
___
Traceback (most recent call last):
  File "src/tests/intg/test_ldap.py", line 877, in test_remove_user_from_group
ent.assert_group_by_name("group1", dict(mem=ent.contains_only()))
  File "src/tests/intg/ent.py", line 377, in assert_group_by_name
assert not d, d
AssertionError: member list mismatch: 
unexpected members found:
['user1']
___ test_remove_user_from_nested_group 
___
Traceback (most recent call last):
  File "src/tests/intg/test_ldap.py", line 953, in 
test_remove_user_from_nested_group
dict(mem=ent.contains_only()))
  File "src/tests/intg/ent.py", line 377, in assert_group_by_name
assert not d, d
AssertionError: member list mismatch: 
unexpected members found:
['user1']
 102 tests deselected by '-ktest_ldap.py' 

== 2 failed, 15 passed, 102 deselected in 46.02 seconds 
==
Makefile:728: recipe for target 'intgcheck-installed' failed
make: *** [intgcheck-installed] Error 1
make: Leaving directory 'intg/bld/src/tests/intg'
```
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253772653
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
Great, the result is the same. Both patches could fix it. So we can use just 
ldap patch.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253736716
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
Oh no, I tested it for ldap, not for ipa provider. Wait a minute a will test it 
again. :-(
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253735227
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
I changed the author of ldap patch to @sumit-bose.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253734041
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

fidencio commented:
"""
On Fri, Oct 14, 2016 at 9:46 AM, celestian  wrote:

> So, I looked this patch set again. Both, ldap and memberof patch, can fix
> the issue itself. I removed memberof patch.
>
> Original author of ldap patch is @sumit-bose
>  -- do you agree with applying? I am
> sorry, I don't know how to write co-author to the patch.
>

If you changed something in the patch, just add this* to the end of the
commit message.

*: Co-Author: Petr Čech 

Best Regards,

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253732823
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
So, I looked this patch set again. Both, ldap and memberof patch, can fix the 
issue itself. I removed memberof patch.

Original author of ldap patch is @sumit-bose -- do you agree with applying? I 
am sorry, I don't know how to write co-author to the patch.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253732056
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

jhrozek commented:
"""
So I wanted to write a test for the memberof patch, but I couldn't reproduce 
the ticket #2940 even without the memberof patch. I tried exactly the steps in 
the ticket and it seemed to me that just applying the ldap provider patch 
solves the issue.

@celestian can you give me steps to reproduce the bug that the patch is 
solving? Then I can write a test..
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-253536311
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

jhrozek commented:
"""
I think :@lslebodn is right, because I removed just the memberof patch. Now I 
have:
```
ab6dee1 TESTS: Adding intg. tests on nested groups
257b66a LDAP: Removing of member link from group
```
and make intgcheck happily passes.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-252289165
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
At first I would like to inform that ticket: 
https://fedorahosted.org/sssd/ticket/3186 wasn't bug. This patch is valid for 
ldap provider too. (I tested again manually with reproducer.)

In my opinion, the test remove_user_from_nested_group() is variation on 
reproducer provided in this PR. So it tests our case.

If I run intg. tests without this PR:
```
test_ldap.py::test_remove_user_from_group FAILED
test_ldap.py::test_remove_user_from_nested_group FAILED
```
If I run intg. tests with this PR: they passed.



"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-251113687
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

lslebodn commented:
"""
Downstream related tests passed.
So +1

However, it seems that patch
"MEMBEROF: Don't resolve members if they are removed"
is not covered by integration test in 3rd patch.

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-249677204
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

jhrozek commented:
"""
I think all review requests were addressed, the code now looks good to me and 
just reverting the non-code changes make the tests fail.

So I'm adding the Accepted label, just will wait a bit if others have some 
comments..
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-248556748
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#13][comment] MEMBEROF: Don't resolve members if they are removed

[celestian]

  URL: https://github.com/SSSD/sssd/pull/13
Title: #13: MEMBEROF: Don't resolve members if they are removed

celestian commented:
"""
Hello,
I pushed new version with tests.

Thanks @sumit-bose with help with the second patch. By the way is there better 
way how to write co-authors? Please tell me.

This patch works for ipa provider, not for ldap provider. (For ldap provider we 
have https://fedorahosted.org/sssd/ticket/3186)

Reproducer:
```bash
# !/bin/bash

# prepare
ipa user-add --first=Adam --last=Adam --email=a...@persei.cz adam
ipa group-add group_1
ipa group-add-member --users=adam group_1
ipa group-add group_2

# reproducer

systemctl daemon-reload
sudo su -c "truncate -s0 /var/log/sssd/*.log"
sudo su -c "rm -f /var/lib/sss/db/*" 
sudo su -c "rm -f /var/lib/sss/mc/*"
sudo systemctl restart sssd.service

ipa group-add-member --groups=group_1 group_2
sss_cache -UG
sudo su -c "truncate -s0 /var/log/sssd/*.log"
getent group group_2

ipa group-remove-member --groups=group_1 group_2
sss_cache -UG
sudo su -c "truncate -s0 /var/log/sssd/*.log"
getent group group_2

# clean

ipa group-del group_2
ipa group_del group_1
ipa user-del adam
```
Configuration:
```bash
# cat /etc/sssd/sssd.conf 

[domain/ipa.beta]
cache_credentials = True
krb5_store_password_if_offline = True
ipa_domain = beta
id_provider = ipa
auth_provider = ipa
access_provider = ipa
ipa_hostname = mirach.beta
chpass_provider = ipa
dyndns_update = True
ipa_server = _srv_, algol.beta
dyndns_iface = ens3
ldap_tls_cacert = /etc/ipa/ca.crt
entry_cache_timeout = 30
debug_level = 0x0

[sssd]
services = nss, sudo, pam, ssh
domains = ipa.beta
debug_level = 0xFF0

[nss]
homedir_substring = /home
```
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/13#issuecomment-248002682
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org