[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ I will be running some more tests, but these are the ones that should cover the majority of what I've changed. Please review the patches again, thank you. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-338432384 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ OK, so one of the test failures was a bug in my patches, actually. This version passed all the tests I ran so far: * 2106158 - local provider * 2106157 - tests against openldap * 2106156 - multidomain tests * 2106155 - ldap id/auth tests """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-338432323 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ OK, first downstream tests passed. I'll just post job IDs here: * 2104793 - local provider tests * 2104801 - Tests-for-Multiple-Domains """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-338248214 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ New patches are pushed: * the UID and GID uniqueness is only enforces unless the domain had the local provider. This would prevent having to do a backwards-incompatible change. I would like to file a ticket to actually remove that check and break the compatibility, but pagure is giving me 500 server error now.. * I added a test for @pbrezina's suggestion. It turns out we've been handling this case well in the NSS initgroups code already * The request that turns by-group search into by-user search for MPG domains was called for by-GID searches only, which doesn't make sense. We should do that for all search keys. There is also a test added. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-338003510 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ btw downstream tests were rescheduled with the new code and I'll post the results (job IDs) here later, when the tests finish. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-338003698 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ The case @pbrezina mentioned works fine, but I found another issue resolving the automatic groups in case the user wasn't resolved first. Additionally, some of the downstream tests are failing, so I'm setting Changes Requested until I can figure out both issues. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-337873402 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ Good question, let's add a test! """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-337851092 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ btw I restarted all the downstram tests because a) in some tests I added the repo to the server, not the client and b) there was a mismatch between ldb used for build and for runtime, which was causing crashes """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-337851249 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups pbrezina commented: """ I have just one question -- what happens when gidNumber is present in the user entry and gidNumber == uidNumber and mpg is true? """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-337848867 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups fidencio commented: """ CI: http://vm-058-233.${abc}/logs/job/79/73/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-337344947 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ btw some downstream tests failed and some didn't pick up the correct sssd version and I'm not sure why. I will investigate further. But please don't add the accepted label or push the patches before the test issues are solved. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-337340994 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups fidencio commented: """ @jhrozek, patch set looks good to me, so ACK! I'd explicitly wait for @pbrezina's ACK and the result of downstream tests. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-337232931 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ About the downstream tests, no, but thanks for the reminder, I will run some now. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-336995356 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups jhrozek commented: """ @fidencio thanks, I fixed the nitpicks and added some more content to the commit message about tests. Let me know if more details need clarifying.. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-336982611 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups pbrezina commented: """ I can chime in as well. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-336861685 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups fidencio commented: """ And, last but not least, I really would feel more comfortable if we can have another reviewer working on this patch set instead of just me. """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-336806564 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups fidencio commented: """ So, @jhrozek, I didn't clearly/easily understand the changes done in the tests currently avaiable. Would you mind adding some details in the commit messages why those changes were needed? Also, did you run any downstream test with your patches in order to track down whether we may introduce a regression as we're breaking the backward-compat? """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-336806354 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#402][comment] LDAP: Allow autogenerating user-private groups
URL: https://github.com/SSSD/sssd/pull/402 Title: #402: LDAP: Allow autogenerating user-private groups fidencio commented: """ A few nitpicks: - **CONFIG: Add a new option auto_private_groups**: ``` diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index c20cb53ca..a02822481 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -938,7 +938,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, ret = get_entry_as_bool(res->msgs[0], &domain->mpg, CONFDB_DOMAIN_AUTO_UPG, 0); -if(ret != EOK) { +if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Invalid value for %s\n", CONFDB_DOMAIN_AUTO_UPG); goto done; ``` - **SDAP: Allow the mpg flag for the main domain**: ``` diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c index 34c0eabb0..7338b4a15 100644 --- a/src/providers/ldap/sdap_async_users.c +++ b/src/providers/ldap/sdap_async_users.c @@ -424,7 +424,7 @@ int sdap_save_user(TALLOC_CTX *memctx, "Missing GID, won't save the %s attribute\n", SYSDB_PRIMARY_GROUP_GIDNUM); -/* Store a the UID as GID (since we're in a MPG domain so that it doesn't +/* Store the UID as GID (since we're in a MPG domain so that it doesn't * get treated as a missing attribute and removed */ ret = sdap_replace_id(attrs, SYSDB_GIDNUM, uid); ``` - **LDAP: Turn by-GID request into by-UID request for MPG domains if needed**: ``` diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index bd988f0dd..9f0c762e9 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -1165,7 +1165,6 @@ static errno_t groups_get_handle_no_group(struct tevent_req *req) return ret; } break; - case BE_FILTER_IDNUM: gid = (gid_t) strtouint32(state->filter_value, &endptr, 10); if (errno || *endptr || (state->filter_value == endptr)) { @@ -1181,14 +1180,12 @@ static errno_t groups_get_handle_no_group(struct tevent_req *req) return ret; } break; - case BE_FILTER_SECID: case BE_FILTER_UUID: /* Since it is not clear if the SID/UUID belongs to a user or a * group we have nothing to do here. */ ret = EOK; break; - case BE_FILTER_WILDCARD: /* We can't know if all groups are up-to-date, especially in * a large environment. Do not delete any records, let the @@ -1196,7 +1193,6 @@ static errno_t groups_get_handle_no_group(struct tevent_req *req) */ ret = EOK; break; - default: ret = EINVAL; break; ``` """ See the full comment at https://github.com/SSSD/sssd/pull/402#issuecomment-336805797 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org