tus / tusd

Reference server implementation in Go of tus: the open protocol for resumable file uploads
https://tus.github.io/tusd
MIT License
3.09k stars 482 forks source link

Example haproxy config #1156

Open edwh opened 4 months ago

edwh commented 4 months ago

Question @Acconut You mentioned that you're using haproxy in live deployments. I'm having trouble getting that working. Would you be able to share your haproxy config?

edwh commented 4 months ago

Here's what I have. This does manage to upload, but it was a bit of a struggle and I had to add various CORS headers which I don't think I should need to:

frontend tusd_frontend
  bind *:8080 ssl crt /etc/haproxy/<whatever>.pem
  redirect scheme https if !{ ssl_fc }
  http-response add-header Access-Control-Allow-Origin *
  http-response add-header Access-Control-Allow-Headers 'Authorization, Origin, X-Requested-With, X-Request-ID, X-HTTP-Method-Override, Content-Type, Upload-Length, Upload-Offset, Tus-Resumable, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Complete, Upload-Draft-Interop-Version, x-token'
  http-response add-header Access-Control-Allow-Methods 'POST, HEAD, PATCH, OPTIONS, GET, DELETE'
  http-response add-header Access-Control-Expose-Headers 'Location,Upload-Offset'
  no option http-buffer-request

  default_backend tusd_backend

backend tusd_backend
  mode http
  http-request set-header X-Forwarded-Proto "https"
  http-request add-header X-Forwarded-For %[src]
  server app1 <ip>:8080 check

And this is what I was running:

./tusd -upload-dir=./files -disable-cors -behind-proxy -base-path /

Acconut commented 4 months ago

It appears odd to me that you disable the CORS headers from tusd and then set them manually in HAProxy. Is there a specific reason why you do this? Normally the CORS handling from tusd is sufficient to get tus uploads working.

What errors were you running into?

edwh commented 4 months ago

@Acconut I found that tusd wasn't sending any of the CORS headers, no matter which option I chose. So I turned that off and bodged it inside haproxy.

I think this may have been because there was no Origin header on the incoming request - unrouted_handler.go has some code along those lines, though I may not be looking in the right place.

I'm not sure whether that's deliberate/correct - I'm not sure when Origin is/should be present.

Acconut commented 4 months ago

The Origin header should be present in an CORS-related request (preflight requests and main request): https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS Unless some other intermediary is removing this header, it should be there. Did you inspect the incoming requests and saw that the header was missing?

edwh commented 4 months ago

Yes, it was missing.

This sounds like it's specific to my environment. But would there be any harm in returning the CORS headers anyway?

Acconut commented 4 months ago

A CORS request must include the Origin header or otherwise it's not a valid CORS request as far as I know. Without the Origin header, the server can also not validate whether it should allow the cross-origin request or not.

What error message did you get from the browser? What do the request/response look like that were sent/received?

edwh commented 4 months ago

I agree a CORS request should include those. My point is: would it do any harm to include the CORS headers on a response whether or not the Origin header is present on the request?

I can't see that it would. I think I've set up lots of configs in the past (e.g. in nginx) where that would occur. If it would do no harm, then always adding them would also solve this.

I'll try to repro this later, but the browser was logging console errors about CORS once it tried to PATCH the uploaded file.

edwh commented 4 months ago

Ok, some more experimentation. As you were too polite to say, CORS is probably a red herring or a consequence of some other config I had in there while experimenting.

Here's an haproxy config that works.

frontend tusd_frontend
  bind *:8080 ssl crt /etc/haproxy/<whatever>.pem
  redirect scheme https if !{ ssl_fc }
  mode http
  no option http-buffer-request
  default_backend tusd_backend

backend tusd_backend
  mode http
  balance leastconn
  no option http-buffer-request
  stick-table type ip size 200k expire 30m
  stick on src
  option redispatch
  http-request set-header X-Forwarded-Proto "https"
  http-request add-header X-Forwarded-For %[src]
  server app1 <whatever>:8080 check
  server app4 <whatever>:8080 check backup

I think the key option is no option http-buffer-request. Without this, I get empty requests back on the PATCH.

The docs allude to needing to configure haproxy in this kind of way, but I think it would be super helpful to have a sample config that you know works, as you have for nginx.

Given that you are using haproxy in production, you probably have a config that you are happy with, and it might easily include some more/better configuration that I've not figured out yet. Could you add that to the docs?

Acconut commented 4 months ago

I am glad to read that you figured it out and I also agree that having a working HAProxy config as an example would be helpful for users. Unfortunately, our HAProxy configuration is a lot more involved, so I cannot easily share it. However, I can tell that we do not use no option http-buffer-request and don't have any upload issues. As far as I understand, request buffering is not enabled by default, so this configuration directive is only needed if it's enabled somewhere else. Maybe that was the case for you?

Besides that, our configuration includes logic to set the X-Forwarded-Host property.

edwh commented 4 months ago

Maybe. Feel free to close this one, but I'll just also mention (for anyone else who finds it in future) that I hit problems because Sentry adds some headers to the request. So I had to reinstate this into my haproxy config (the last two headers are the ones it adds):

http-response add-header Access-Control-Allow-Headers 'Authorization, Origin, X-Requested-With, X-Request-ID, X-HTTP-Method-Override, Content-Type, Upload-Length, Upload-Offset, Tus-Resumable, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Complete, Upload-Draft-Interop-Version, x-token, baggage, sentry-trace'

Acconut commented 4 months ago

You can also configure tusd's CORS setup to allow/expose additional headers: https://tus.github.io/tusd/getting-started/configuration/#cross-origin-resource-sharing-cors Might be helpful for you :)