Closed remche closed 2 years ago
Good catch, I'm using a reverse proxy. I will give 1.17.0-pre1 a try and keep you informed.
Tried with 1.17.0-pre1, and playing with traefik to preserve source host, but it's always throwing exceptions...
See this comment: he tweaks the nginx so it doesn't close the connection. https://github.com/sscarduzio/elasticsearch-readonlyrest-plugin/pull/407#discussion_r254771720
I saw it, and tried with ingress.kubernetes.io/custom-request-headers: Connection:keep-alive
with no more success...
Printing remoteHost returns me internal k8s traefik IP or Cerebro IP... Note that it seems to work correctly, but output is dirty... Tell me if you want me further debug.
we could make the extraction of the OA a lazy var and if you don't use it it won't even try.
@coutoPL is the hostname extraction lazy in the new core?
@sscarduzio the RequestInfo class didn't change. Could you explain me what is the problem with current solution, because I don't think I get it?
So, apparently under certain conditions (i.e. reverse proxy closes the connection too early) the source IP address can't be retrieved from the connection. Now, because the we eagerly extract all the data from the RequestInfo in order to compose a RequestContext, even if your ACL doesn't have the hosts rule, you will get the NullPointerException.
The proposal is to mitigate this exception making the RequestContext#getRemoteAddress
method lazy.
After consideration, I'm not sure it's the reverse proxy that causes the excetpion. It might be the kubernetes readinessProbe. The effect and the solution are the same, though.
Don't you think this solution just move the problem a little further? Users which doesn't use hosts rule, won't be affected, but the one who uses it will have the same problem. Maybe we should make the remote address optional? Hosts rule is going to return NO_MATCH when remote address is none.
move the problem a little further
Yes it does move a problem we can't fix as far as possible especially from users that are not concerned with remote addresses at all. I think this should be done.
Hosts rule is going to return NO_MATCH when remote address is none
This would be very confusing, as the remote address in a connection should be always known. So I agree. Yes: we should return NO_MATCH; and yes we could print out very explicit warnings instead of a wild NPE.
ok, piece of cake
I've changed it in new core. See: https://github.com/sscarduzio/elasticsearch-readonlyrest-plugin/pull/412/commits/f80dcdfe9d359ca41f6feae3cf6bbc97ae0953fd
This should be warn (and exclamation mark too! :P )
ror version : 1.16.34 es version : 6.6.0 configuration : 3 nodes cluster
I have a three nodes cluster, running in kubernetes. When I run an image with ror installed, a NullPointerException is throwed by nodes, and the cluster is instable.
I tried with a minimal configuration
readonlyrest.yml :
stack trace :