openfaas / of-watchdog

Reverse proxy for STDIO and HTTP microservices
MIT License
263 stars 116 forks source link

Original HTTP Host header is not passed to the handler. #22

Closed dmrub closed 5 years ago

dmrub commented 6 years ago

Expected Behaviour

Host header should be forwarded to the upstream function.

Current Behaviour

Since Host header is not a part of request.Header field it is not copied by the copyHeaders function.

Possible Solution

Do request.Host = r.Host before copyHeaders(request.Header, &r.Header) in http_runner.go

Steps to Reproduce (for bugs)

1. 2. 3. 4.

Context

We need to construct URI links that depends on the Host header. This information is lost when function is called. Also seems that the same modification need to be done for the gateway as well.

Your Environment

alexellis commented 6 years ago

Can you confirm whether this is the case with the original watchdog too?

alexellis commented 6 years ago

Please can you test your proposed change locally and then verify?

ivanayov commented 6 years ago

Derek add label: support

dmrub commented 6 years ago

I can confirm that original fwatchdog also does not propagate Host header to the handler. index.py:

import sys
import os

def handler(req):
    s = "Request: [\n"+req+"\n]\nHTTP Headers: [\n" \
        + '\n'.join([k + ': ' + v for k,v in os.environ.items() if k.startswith('Http_')]) \
        + '\n]\n'

    return s

def get_stdin():
    buf = ""
    for line in sys.stdin:
        buf = buf + line
    return buf

if __name__ == "__main__":
    st = get_stdin()
    ret = handler(st)
    if ret != None:
        print(ret)

Run fwatchdog with port=8081 fprocess="python index.py" ./fwatchdog Run curl -H "Host: foobar" -H "A: B" -v -d 'DATA' 'http://127.0.0.1:8081?a=b'

Output:

$ curl -H "Host: foobar"  -H "A: B" -v -d 'DATA' 'http://127.0.0.1:8081?a=b'
* Rebuilt URL to: http://127.0.0.1:8081/?a=b
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8081 (#0)
> POST /?a=b HTTP/1.1
> Host: foobar
> User-Agent: curl/7.51.0
> Accept: */*
> A: B
> Content-Length: 4
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 4 out of 4 bytes
< HTTP/1.1 200 OK
< Content-Type: application/x-www-form-urlencoded
< X-Duration-Seconds: 0.018921
< Date: Fri, 10 Aug 2018 13:04:36 GMT
< Content-Length: 238
< 
Request: [
DATA
]
HTTP Headers: [
Http_Method: POST
Http_User_Agent: curl/7.51.0
Http_Content_Type: application/x-www-form-urlencoded
Http_A: B
Http_Accept: */*
Http_Content_Length: 4
Http_Query: a=b
Http_ContentLength: 4
Http_Path: /
]

* Curl_http_done: called premature == 0
* Connection #0 to host 127.0.0.1 left intact
dmrub commented 6 years ago

In order to propagate Host header original fwatchdog need to be modified differently: https://github.com/dmrub/faas/commit/d1e2e4feba1bd34aa24f710f0f1eeb1f2dd423b8

dmrub commented 6 years ago

I created PR for this issue: https://github.com/openfaas-incubator/of-watchdog/pull/24

ivanayov commented 6 years ago

Thanks @dmrub Can you please open a PR in faas?

alexellis commented 6 years ago

@dmrub are you testing the watchdog/s in isolation or through the gateway? Is the gateway passing the host entry too?

dmrub commented 6 years ago

I am testing both, first watchdog in isolation then watchdog in combination with gateway in Kubernetes cluster. And even after current fix my function still reports wrong Host value: sl1.default:8080 sl1 is the name of the function, and default is Kubernetes namespace. Instead Host should contain a name and a port of the URI which I type in browser or curl. So I think there is the same problem when gateway creates HTTP request in gateway.

alexellis commented 6 years ago

This is why I was asking you whether you had tested end-to-end through the gateway or not.

The Host entry is technically correct because it passes through several hops.

IngressController or LoadBalancer (www.site.com/function) -> API Gateway (Host: www.site.com) -> http://function-name:8080 -> of-watchdog (Host: function-name).

alexellis commented 6 years ago

You will also need to raise another PR that adds the Host entry for asynchronous calls - i.e. http://gateway:8080/async-function/function-name. In this instance the Header is serialised on a queue and then rehydrated.

https://github.com/openfaas/faas/blob/master/gateway/queue/types.go#L10

https://github.com/openfaas/nats-queue-worker/blob/master/main.go#L82

dmrub commented 6 years ago

Yes, Host header is technically correct, but I need in the function functionality to generate URLs which are relative to the host used in the original request to the gateway. One option would be to put reverse proxy like nginx and generate X-Forward-* headers: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host , https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto . But, I assume that function should also behave as a part of gateway from HTTP point of view and receive all HTTP headers of the gateway's request unmodified.

alexellis commented 6 years ago

Where are we at with this now @dmrub ?

dmrub commented 6 years ago

When openfaas/nats-queue-worker#34 is merged, I think it should be propagated to the vendor directory of openfaas/faas repository. Then, I think, this issue can be closed.