-
Notifications
You must be signed in to change notification settings - Fork 593
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
feat(fiat) - Cache fetched LDAP roles to speed up role syncs. #1066
Conversation
* BaseUserRolesProvider: New class to enable caching layer for concrete UserRolesProvider implementations. * LdapUserRolesProvider: Update class to consume caching layer.
FWIW we've been running this for ~2 years at Salesforce. |
@Mergifyio update |
✅ Branch has been successfully updated |
I don't use the LDAP provider, but I'm curious about the side effects of this. If user A is added to a new AD group with permissions on an application, does this mean they have to wait until cache expiry to start using it? Alternatively, if a user is removed from an AD group, will they still have access until cache expiry? |
@mattgogerly - No. Since the entry of user A is not present in the cache, the request goes to LDAP. Expiry is at the entry level.
Yes, this is possible as with any caching mechanism (unless there is a way to invalidate explicitly). |
#804 MIGHT have been of interest FYI Ignoring that... this is minor and not requesting changes, just more of. "hey here's another custom cache system." Is there a reason NOT to use standard spring cache system? Just using |
@Mergifyio update |
✅ Branch has been successfully updated |
You're right @jasonmcintosh. No doubt our ignorance when we implemented this. Since we've been running this code for awhile and have good confidence in it, I'm gonna go ahead and merge it and treat using something more standard as a future improvement. |
…ker#1066) * BaseUserRolesProvider: New class to enable caching layer for concrete UserRolesProvider implementations. * LdapUserRolesProvider: Update class to consume caching layer. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Cached entries expire after
expireAfterWriteSeconds
seconds. This is an optional configuration whose default value is 600(i..e, 1hour).Addressed the spinnaker issue: Fiat - local cache for ldap spinnaker#6845