search

Graylog2 / graylog-docker

Official Graylog Docker image
https://hub.docker.com/r/graylog/graylog/
Apache License 2.0
367 stars 133 forks source link

Healthcheck protocol fix #111

Closed structurefall closed 4 years ago

structurefall commented 4 years ago

This improves the logic for parsing the http_publish_uri variable. The existing logic doesn't account for a situation in which a reverse proxy is used to present the publish URI as HTTPS while Graylog has TLS turned off.

jalogisch commented 4 years ago

he @structurefall

thank you for your contribution. The http_publish_uri needs to be unique for each node and it needs to be the URL that is used to communicate with other nodes. Means that this is used for inter-cluster communication.

The usecase you describe would be to set the http_external_uri to https via proxy.

In addition to that it would be more failsafe to check with [:alpha:] and not [:alnum:] as the only valid option would be http or https. What does not contain any numbers.

Maybe you want to adjust and rethink your PR and add some modification.

thx Jan

structurefall commented 4 years ago

@jalogisch sorry for the delayed response- in my particular case, I have individual Graylog nodes which are in fact communicating with each other via HTTPS through a per-node NGINX proxy, in addition to having a single http_external_uri for the cluster. I realize this is a little unusual but it's a requirement for our environment. You're right about the [:alpha:] thing however, I'll update that.

malcyon commented 4 years ago

@jalogisch Is this good to be merged?

jalogisch commented 4 years ago

I'm unsure @juju2112 if this makes sense at all. you have to explizit enable https in Graylog with a flag and with that setting it will get more complicated (IMHO). So that is the reason I did not merged until now.

The per node nginx proxy making https termination is not very common setup. In fact I can count on one hand over the last year that I have seen this ...

So my personal decision would not to merge this, but I'm happy to discuss.

malcyon commented 4 years ago

Alright, closing this out, then. If there is any additional discussion, we can reopen it.