WIP: AMQ-9627 Don't fail on start if cachedLDAPAuthorizationMap is used and the LDAP server is not available. #1358
Conversation
| try { | ||
| query(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error updating authorization map. Partial policy may be applied until the next successful update.", e); |
There was a problem hiding this comment.
I copied the existing error message from https://github.com/apache/activemq/blob/main/activemq-broker/src/main/java/org/apache/activemq/security/SimpleCachedLDAPAuthorizationMap.java#L679
please let me know what you think
There was a problem hiding this comment.
I would also include a string that suggest ways for the user to mitigate it. Something like "The LDAP server might not be reachable, check ..." even tho the root cause can be many reason. Because "authorization map" is an internal concept, user might not get it and they don't know how to get themselves unstuck. However, It needs to be phrase in such a way that this is one possible root cause, but not necessarily THE root cause.
There was a problem hiding this comment.
the error message will look something like:
ERROR | Error updating authorization map. Partial policy may be applied until the next successful update.
javax.naming.CommunicationException: localhost:1024
which is exactly the same as the error users will see when the map is trying to update the cached values, but the ldap server is unreachable. in my opinion it has enough details about what went wrong(the communication exception).
my concern is more about Partial policy may be applied until the next successful update, because technically, there is no data, so nothing will be applied until the next successful update
…d the LDAP server is not available.
Nikita-Shupletsov
force-pushed
the
ldap_map_fix
branch
from
d9d1d5f to
0ef9542
Compare
|
Question @Nikita-Shupletsov, once the user fixed the issue with the configuration or their LDAP server, does the user need to restart the server? In another word, is this code part of the code path that get triggered everytime the broker performs AuthN/AuthZ? If that latter case is true, the activemq.log be flooded with error message. How do we make it less noisy? |
No, they will not need to. afaik, that's the whole idea of this Cached Map is to be able to withstand connection failures. it will try to fetch data on every request until is succeeds |
mattrpav
changed the title
What problems does this PR solve?
it fixes a crash on start if cachedLDAPAuthorizationMap is used and the LDAP server is not available.
Why is it beneficial to merge into ActiveMQ?
it's rather inconvenient, as cachedLDAPAuthorizationMap is usually used to avoid issues with the LDAP server being not always available
How do you make sure this PR is well tested?
I added a test that reproduces the error without the change.