On Wed, Aug 28, 2013 at 04:58:07PM +0200, Sumit Bose wrote: > On Wed, Aug 28, 2013 at 04:17:15PM +0200, Jakub Hrozek wrote: > > On Tue, Aug 27, 2013 at 01:11:14PM +0200, Jakub Hrozek wrote: > > > On Tue, Aug 27, 2013 at 12:34:27PM +0200, Sumit Bose wrote: > > > > On Mon, Aug 26, 2013 at 10:16:34PM +0200, Jakub Hrozek wrote: > > > > > On Mon, Aug 26, 2013 at 04:02:59PM -0400, Simo Sorce wrote: > > > > > > On Mon, 2013-08-26 at 21:41 +0200, Jakub Hrozek wrote: > > > > > > > On Mon, Aug 26, 2013 at 03:35:09PM -0400, Simo Sorce wrote: > > > > > > > > On Mon, 2013-08-26 at 21:17 +0200, Jakub Hrozek wrote: > > > > > > > > > On Mon, Aug 26, 2013 at 06:18:05PM +0200, Sumit Bose wrote: > > > > > > > > > > On Mon, Aug 26, 2013 at 05:20:21PM +0200, Jakub Hrozek > > > > > > > > > > wrote: > > > > > > > > > > > On Fri, Aug 23, 2013 at 03:44:09PM +0200, Sumit Bose > > > > > > > > > > > wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > > > > currently in ipa-server-mode only the AD groups > > > > > > > > > > > > memberships are > > > > > > > > > > > > available. This patch adds the IPA group memberships to > > > > > > > > > > > > trusted AD > > > > > > > > > > > > users. > > > > > > > > > > > > > > > > > > > > > > > > This patch is missing some unit tests for some of the > > > > > > > > > > > > helper functions. > > > > > > > > > > > > I will send them later, but I didn't want to delay the > > > > > > > > > > > > next release. > > > > > > > > > > > > > > > > > > > > > > > > bye, > > > > > > > > > > > > Sumit > > > > > > > > > > > > > > > > > > > > > > I haven't done any testing yet but do we need the > > > > > > > > > > > timeout? Since the > > > > > > > > > > > initgroups is a rare operation and on logins we generally > > > > > > > > > > > want to have > > > > > > > > > > > the correct memberships, can we just rely on responder > > > > > > > > > > > caching? > > > > > > > > > > > > > > > > > > > > I was thinking of situations where multiple logins happen > > > > > > > > > > in a short > > > > > > > > > > time. Additionally I think even if group memberships of a > > > > > > > > > > user might > > > > > > > > > > change often the mapping of AD to IPA group memberships via > > > > > > > > > > the external > > > > > > > > > > groups will only change rarely. > > > > > > > > > > > > > > > > > > > > Maybe we can a cache time option to make it more flexible? > > > > > > > > > > > > > > > > > > > > bye, > > > > > > > > > > Sumit > > > > > > > > > > > > > > > > > > I was thinking about this more on my way home and I think > > > > > > > > > you're > > > > > > > > > right we need to optimize the ipa_server_mode. This could > > > > > > > > > cause the "8AM > > > > > > > > > login rush" to be a real bottleneck. > > > > > > > > > > > > > > > > > > But I think we can exploit the fact that we know the server > > > > > > > > > well during > > > > > > > > > the ipa_server_mode. What about this approach? > > > > > > > > > 1. on startup we download all external groups > > > > > > > > > > > > > > > > it could be a huge set, and would cause a huge load if someone > > > > > > > > runs a > > > > > > > > puppet script to reconfigure and restart a few 1000 machines > > > > > > > > with sssd. > > > > > > > > > > > > > > > > > > > > > > A huge set of external groups? > > > > > > > > > > > > Why not ? > > > > > > > > > > No technical reason. I just don't see that as a common use-case that's > > > > > all. > > > > > > > > > > > > > > > > > > > > 2. store the largest lastUSN to the server mode context > > > > > > > > > > > > > > > > we already have this afaik > > > > > > > > > > > > > > No we don't. This is new code just for the server mode. > > > > > > > > > > > > Ahh, that explains some things, sorry. > > > > > > > > > > No problem, lot of patches are flailing around, I know. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. on subsequent lookups, only download and store groups > > > > > > > > > with higher > > > > > > > > > lastUSN > > > > > > > > > > > > > > > > if you change server you are back to square zero though > > > > > > > > > > > > > > > > > > > > > > Which is never during in the server mode. (except for slapd > > > > > > > outages etc) > > > > > > > > > > > > > > > > 4. perform the lookup always. It's on the server after > > > > > > > > > all so > > > > > > > > > network LDAP search is quite cheap. > > > > > > > > > > > > > > > > not sure I understand what this means ? > > > > > > > > > > > > > > The current patch re-downloads external groups every 600 seconds > > > > > > > and if > > > > > > > 600 seconds hasn't passed, uses cache. I'm proposing we download > > > > > > > (&(objectClass=externalGroup)(lastUSN>=stored_last_usn)) on every > > > > > > > request because in the server mode the latency is not an issue. > > > > > > > > > > > > 600 is a lot :( > > > > > > > > > > yes, that's why I was trying to come up with something more > > > > > optimized.. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The point of checking initgroups at login is to assure the right > > > > > > > > memberships are in place, both for security reasons and to > > > > > > > > allow a user > > > > > > > > to logoff and login back again and make sure eh gets new group > > > > > > > > memberships if he has been granted any new ones. > > > > > > > > > > > > > > > > Loggin off then back on is the only way to gain system-wide the > > > > > > > > new > > > > > > > > memberships so that's what an administrator will tell a user to > > > > > > > > do if > > > > > > > > the user complains he can't access something. > > > > > > > > > > > > > > > > This means that caching can't last long as the side effects are > > > > > > > > severe. > > > > > > > > So anything more than a few seconds would probably be bad. A > > > > > > > > few seconds > > > > > > > > make total sense for load issues when someone is abusing pam > > > > > > > > atuh (for > > > > > > > > example someon eusing basic auth wired to pam auth for a a web > > > > > > > > server > > > > > > > > that will receive potentially many tens of authentications for > > > > > > > > the same > > > > > > > > user within a fraction of a second as each image and file is > > > > > > > > loaded in a > > > > > > > > new connection). > > > > > > > > > > > > > > > > In this case you really want to completely cache the whole > > > > > > > > operation and > > > > > > > > not touch the network for any reason, otherwise latency will > > > > > > > > make things > > > > > > > > unbearable. But that's the extent to which you want to go, a > > > > > > > > few seconds > > > > > > > > for auth bursts, nothing more. > > > > > > > > > > > > > > > > Simo. > > > > > > > > > > > > > > The latency in the server mode is really small, we should always > > > > > > > be > > > > > > > talking to the local server. > > > > > > > > > > > > I think we shouold defer this until the synrepl control is > > > > > > available in > > > > > > 389ds, then we can simply have a persistent search open using > > > > > > syncrepl > > > > > > and we'll get changes as needed and no more that that (including > > > > > > deletions). > > > > > > > > > > > > Simo. > > > > > > > > > > Yes, that sounds like a very good solution, I just think we also need > > > > > to > > > > > have a way to handle the external groups in time for 1.11/3.3. If we > > > > > can > > > > > leverage a 389DS control later, even better. > > > > > > > > What about using a shorter timeout, e.g. 10s, for 1.11.0 and > > > > optimizations in 1.11.x? > > > > > > > > bye, > > > > Sumit > > > > > > That sounds good to me. > > > > Here is a diff I would squash before pushing the patches: > > > > --- a/src/providers/ipa/ipa_subdomains_ext_groups.c > > +++ b/src/providers/ipa/ipa_subdomains_ext_groups.c > > @@ -606,7 +606,7 @@ static void ipa_get_ext_groups_done(struct tevent_req > > *subreq) > > > > state->server_mode->ext_groups->ext_groups = ext_group_hash; > > /* Do we have to make the update timeout configurable? */ > > - state->server_mode->ext_groups->next_update = time(NULL) + 600; > > + state->server_mode->ext_groups->next_update = time(NULL) + 10; > > > > ret = ipa_add_ext_groups_step(req); > > if (ret == EOK) { > > > > Also I filed https://fedorahosted.org/sssd/ticket/2062 to track using > > either persistent search or syncrepl > > ACK > > bye, > Sumit
Pushed to master. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel