mesosphere / marathon-lb

Marathon-lb is a service discovery & load balancing tool for DC/OS
Apache License 2.0
449 stars 300 forks source link

Invalid HAProxy config when using HAPROXY_0_HTTP_BACKEND_REVPROXY_PATH #471

Closed philwinder closed 5 years ago

philwinder commented 7 years ago

Hi guys, I have a few issues when using the HAPROXY_0_HTTP_BACKEND_REVPROXY_PATH.

  1. There isn't enough documentation regarding what to set the value as for typical use cases. I had to experiment with the setting to establish that the resultant config is:

rspirep "^Location: (https?://example.com(:[0-9]+)?)?(/.*)" "Location: /your/path if hdr_location"

So in reality, people probably want something more like:

HAPROXY_0_HTTP_BACKEND_REVPROXY_PATH = /your/path\3

I would propose that a typical use case is to reverse proxy a backend service to a path. E.g. I want to use grafana under the host.com/grafana path.

  1. The if hdr_location doesn't appear to be escaped properly. If you try the documented example, e.g. /my/path the actual call results in /my/path%20if%20hdr_location. Note the inclusion of the if statement.

This seems to be due to the use of quotes. I thought I could do something like /your/path" ", but you can't because that results in an invalid config because there is an extra space before the if (i.e. something like/your/path\ " if hdr_location").

Thanks.

rernst64 commented 7 years ago

Hi Mesosphere,

Phil is right. We experience the same bug with marathon-lb 1.7 and 1.8 with DC/OS 1.9.x in the REVPROXY template too.

o Wrong: … "Location: {rootpath} if hdr_location" o Right: … "Location: {rootpath}" if hdr_location

Only known solution for setting up marathon-lb as a reverse Proxy right now is to overwrite the template with the ACL Labels.

"labels": { "HAPROXY_GROUP": "external", "HAPROXY_0_HTTP_FRONTEND_ACL_WITHPATH": " acl host{cleanedUpHostname} hdr(host) -i {hostname}\n acl path_{backend}_tmp path_beg {path}\n acl hdrreferer{backend} hdr(Referer) -m found\n acl hdrreferer{backend}_inclpath hdr(Referer) -m sub {path}\n http-request set-path {path}%[path] if !path{backend}_tmp hdrreferer{backend} hdrreferer{backend}_inclpath\n acl path{backend}_final path_beg {path}\n usebackend {backend} if host{cleanedUpHostname} path_{backend}_final\n", "HAPROXY_0_HTTP_BACKEND_REVPROXY_PATH": "/test", "HAPROXY_0_HTTP_BACKEND_PROXYPASS_PATH": "/test", "HAPROXY_0_ENABLED": "true", "HAPROXY_0_PATH": "/test", "HAPROXY_0_VHOST": "example.com", "HAPROXY_0_HTTP_BACKEND_REVPROXY_GLUE": " acl hdr_location res.hdr(Location) -m found\n acl hdr_location_incl_prxypth res.hdr(Location) -m sub {rootpath}\n rspirep \"^Location:\ (https?://{hostname}(:[0-9]+)?)?(/.*)\" \"Location:\ {rootpath}\3\" if hdr_location !hdr_location_incl_prxypth\n" },

When is this going to be fixed as the workaround really isn't that nice?

jkoelker commented 5 years ago

This will be fixed in the next release: https://github.com/mesosphere/marathon-lb/pull/501.