URL: https://github.com/SSSD/sssd/pull/313 Author: mzidek-rh Title: #313: SDAP: Update parent sdap_list Action: opened
PR body: """ Update parent sdap_list with newly created subdomain sdap domain.
Preiously, we inherited the parent sdap_list and used it also in the subdomain's context (this was introduced recently with commit c4ddb9ccab670f9c0d0377680237b62f9f91c496), but it caused problems that were difficult to debug (we somewhere rewrite part of the list incorrectly).
This patch reverses to the previous bahavior, where every subdomain has it's own sdap_list, however this time the parrent domain's sdap_list is updated so that it has correct information about search bases of the child domains.
We should ideally have just one sdap_list to avoid the updating completely, but this would require more refactoring in the sdap code.
Resolves: https://pagure.io/SSSD/sssd/issue/3421
"""
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/313/head:pr313 git checkout pr313
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
lslebodn commented: """ Nice try :-) a) 1st patch is unrelated to ticket 3421 b) there is a very tiny issue :-) It does not compile with standard C compiler.
``` src/providers/ad/ad_subdomains.c: In function ‘ads_store_sdap_subdom’: src/providers/ad/ad_subdomains.c:677:13: error: ‘a’ undeclared (first use in this function) DEBUG(SSSDBG_CRIT_FAILURE, "YYY %d\n", a); ^~~~~ src/providers/ad/ad_subdomains.c:677:13: note: each undeclared identifier is reported only once for each function it appears in ``` """
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-309560107
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
mzidek-rh commented: """ Sorry I had uncommitted changes in the repo and force pushed the patch. Fixed now.
Yes the first one is unrelated. I even forgot I have it in this branch, but it should not slow down the review. """
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-309684597
URL: https://github.com/SSSD/sssd/pull/313 Author: mzidek-rh Title: #313: SDAP: Update parent sdap_list Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/313/head:pr313 git checkout pr313
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/313 Author: mzidek-rh Title: #313: SDAP: Update parent sdap_list Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/313/head:pr313 git checkout pr313
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
mzidek-rh commented: """ I hope I pushed it correctly without any garbage this time. """
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-309717017
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
jhrozek commented: """ FWIW, no regressions were found with downstream test runs. I will conclude the review tomorrow with verifying the fix. """
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-309871004
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
jhrozek commented: """ The patches work well. I will push them when CI and coverity finish.
But I have one request that can be done in a subsequent patch -- can we have a `sdap_domain_copy_search_bases` function instead of duplicating the code that sets the search bases and risking a bug later one when we add a new search base?
In general, i would like to make sdap_domain opaque, but I don't think that can be done simply.. """
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-310017277
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
jhrozek commented: """ CI: http://sssd-ci.duckdns.org/logs/job/71/60/summary.html """
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-310032125
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
jhrozek commented: """ I filed a ticket about the de-duplication of code: https://pagure.io/SSSD/sssd/issue/3435 """
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-310033073
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
jhrozek commented: """ * master: * 630aea13063c4b242b3433d16ca4346a1a38429b * b1d34059533eb50f6e5a4ac7b6fa1bb6fa60a445
"""
See the full comment at https://github.com/SSSD/sssd/pull/313#issuecomment-310033506
URL: https://github.com/SSSD/sssd/pull/313 Author: mzidek-rh Title: #313: SDAP: Update parent sdap_list Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/313/head:pr313 git checkout pr313
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/313 Title: #313: SDAP: Update parent sdap_list
Label: +Accepted
sssd-devel@lists.fedorahosted.org
-
jhrozek -
lslebodn
-
mzidek-rh