New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subdomain inherit #247

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@mzidek-rh
Contributor

mzidek-rh commented Apr 25, 2017

I tested if the options that work in subdomain inherit also work in trusted domain section in sssd.conf. Most seem to work without any changes in the code except for two. With these two patches only one that does not work remains (I wanted to send patchset that adds all the options, but I got stuck on the option that sets the ldap principal, so I am sending this in the meantime).

mzidek-rh added some commits Apr 21, 2017

SUBDOMAINS: Configurable ignore_group_members
Allow ignore_group_members in the subdomain section in sssd.conf.

Resolves:
https://pagure.io/SSSD/sssd/issue/3337
MAN: Add options for subdomains
Add options supported in subdomain_inherit to the subdomain section
of sssd.conf.

Resolves:
https://pagure.io/SSSD/sssd/issue/3337
@lslebodn

This comment has been minimized.

Contributor

lslebodn commented Apr 25, 2017

This patch set modified the function check_subdom_config_file which is called from the function new_subdomain. And this function is already tested in unitests.

sh$ git grep new_subdomain -- src/tests/
src/tests/cmocka/test_fqnames.c:    /* just to make new_subdomain happy */
src/tests/cmocka/test_fqnames.c:    test_ctx->subdom = new_subdomain(dom, dom, SUBDOMNAME, NULL, SUBFLATNAME,
src/tests/cmocka/test_nss_srv.c:#include "db/sysdb_private.h"   /* new_subdomain() */
src/tests/cmocka/test_nss_srv.c:    subdomain = new_subdomain(nss_test_ctx, nss_test_ctx->tctx->dom,
src/tests/cmocka/test_simple_access.c:#include "db/sysdb_private.h"   /* new_subdomain() */
src/tests/sysdb-tests.c:    subdomain = new_subdomain(test_ctx, test_ctx->domain,
src/tests/sysdb-tests.c:    subdomain = new_subdomain(test_ctx, test_ctx->domain,
src/tests/sysdb-tests.c:    subdomain = new_subdomain(test_ctx, test_ctx->domain,
src/tests/sysdb-tests.c:    subdomain = new_subdomain(test_ctx, test_ctx->domain,

Please provide unit test for this change.

@mzidek-rh

This comment has been minimized.

Contributor

mzidek-rh commented Apr 27, 2017

I forgot to comment on this. In order to add tests for this I first need to fix the https://pagure.io/SSSD/sssd/issue/3338 otherwise it would be double work. I already started doing it, before I switched to the https://pagure.io/SSSD/sssd/issue/3381 , which should be fixed soon.

@lslebodn

This comment has been minimized.

Contributor

lslebodn commented Apr 28, 2017

I forgot to comment on this. In order to add tests for this I first need to fix the https://pagure.io/SSSD/sssd/issue/3338 otherwise it would be double work.

Ticket 3338 is about function ad_create_sdap_options in src/providers/ad/ad_common.c
This change is on sysdb level. They are two different things. We have quite high code coverage done by sysdb test. Therefore ticket 3338 is not a blocker for writing sysdb test.

I already started doing it, before I switched to the https://pagure.io/SSSD/sssd/issue/3381 , which should be fixed soon.

Sure

@lslebodn

This comment has been minimized.

Contributor

lslebodn commented Jun 6, 2017

@mzidek-rh Do we still need this patch or it should be closed as rejected?
Because requested changes were not done in 5.5 week.

@mzidek-rh

This comment has been minimized.

Contributor

mzidek-rh commented Jun 6, 2017

@lslebodn the downstream bugs were moved to later milestone, but we still need to do it... unfortunately something with higher priority always pops up... I hope to get back to this before next week

@lslebodn

This comment has been minimized.

Contributor

lslebodn commented Aug 11, 2017

Bump.

https://pagure.io/SSSD/sssd/issue/3337 is planed for 1.15.4.
So it would be good to work on this sooner then later.

@lslebodn

This comment has been minimized.

Contributor

lslebodn commented Aug 25, 2017

Bump

@lslebodn

This comment has been minimized.

Contributor

lslebodn commented Sep 5, 2017

Bump for 3rd time.
Requested unit test has not been written for long time therefore priority for related upstream ticket was changed from critical to blocker

@fidencio

This comment has been minimized.

Contributor

fidencio commented Oct 13, 2017

@mzidek-rh, what's the status of this PR? Would you mind giving us a update?

@jhrozek

This comment has been minimized.

Contributor

jhrozek commented May 21, 2018

Hi Michal,
since this PR appears quite stalled, I'm going to close it. I still think the work is something that SSSD would benefit from, but the work shouldn't be tracked in a PR, unless there is active interest in merging the code.

Please let me know if I can help with resolving whatever is blocked with the PR.

@jhrozek jhrozek closed this May 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment