fix(router): Handle comma delimited X-Forwarded-For values

[krancour]

Fixes #167 (at least partially; we cannot necessarily fix idiosyncrasies of GCP)

To test this, you'd be best off using GKE. The TCP load balancer that k8s provisions for you when you install workflow would need to be replaced with an HTTP load balancer.

At that point, the router will receive inbound requests with multi-valued X-Forwarded-For headers.

Router should have it's router.deis.io/nginx.proxyRealIpCidr configures as follows:

router.deis.io/nginx.proxyRealIpCidr: 10.0.0.0/8,<load balancer public IP>/32

Requests to any all applications should yield access log messages form the router that contain your public IP.

See #167 for more information.

[arschles]

are you at all concerned with newlines in the config file here?

[krancour]

Ah... I am. (Not that I need to be.) What I was trying to do was avoid an unnecessary newline either before or after all the set_real_ip_from, but the consequence of that might have been that I don't get newlines between them either. I know you said this is non-blocking, but I do want to give this a look before I consider merging it.

[krancour]

Looks like some nicer formatting options made their way into templates last fall and I hadn't noticed since then:

golang/go@e6ee26a

[arschles]

yep, leading and trailing - chars would help here. official docs are under Text and Spaces at https://godoc.org/text/template

[arschles]

@krancour code LGTM. #168 (comment) is not blocking, just wanted to point it out. Added LGTM1 and removed in progress

[bacongobbler]

@krancour do you have an HTTP load balancer setup handy for testing this? How did you set yourself up on GKE?

[bacongobbler]

my comment triggered jenkins. Sorry about that!

[krancour]

my comment triggered jenkins. Sorry about that!

@bacongobbler: No worries. It happens. fwiw, I actually opened an issue against the Jenkins Github Pull Request Builder plugin with relevant logs. I've heard nothing back so far.

Re: Testing this in GKE, it's quite a pain to set up. I'll set it up and point you in its direction.

[krancour]

@bacongobbler... an easier way for you to test this might be as follows:

1. Deploy locally on vagrant. (This is so you're operating somewhere where there's no load balancer between you and the router. You will play the role of the LB. 😄)

2. Deploy deis-router from this branch: $ make deploy

3. Add the following annotation to the deis-router RC. This will relax some constraints to allow IP spoofing:

   router.deis.io/nginx.proxyRealIpCidrs: 0.0.0.0/0
   

4. curl an app, whilst explicitly setting the X-Forwarded-For-Header:

   $ curl -H "X-Forwarded-For: 1.2.3.4,5.6.7.8" <any valid app URL; event the controller>
   

5. Check router's access logs. If working, requests should appear to have originated from 1.2.3.4.

   $ kubectl logs <router pod> --namespace=deis
   

[bacongobbler]

LGTM:

><> kd annotate rc deis-router router.deis.io/nginx.proxyRealIpCidrs="0.0.0.0/0"
replicationcontroller "deis-router" annotated
><> kd delete po deis-router-99ovj
pod "deis-router-99ovj" deleted
><> curl -H "X-Forwarded-For: 1.2.3.4,5.6.7.8" deis.10.245.1.3.xip.io
<h1>Not Found</h1><p>The requested URL / was not found on this server.</p>
><> kd logs deis-router-6acz9
2016/05/02 18:12:59 INFO: Starting nginx...
2016/05/02 18:12:59 INFO: nginx started.
2016/05/02 18:12:59 INFO: Router configuration has changed in k8s.
2016/05/02 18:12:59 INFO: Reloading nginx...
2016/05/02 18:12:59 INFO: nginx reloaded.
[02/May/2016:18:13:39 +0000] - 1.2.3.4 - - - 404 - "GET / HTTP/1.1" - 415 - "-" - "curl/7.38.0" - "~^deis\x5C.(?<domain>.+)$" - 10.247.51.129:80 - deis.10.245.1.3.xip.io - 0.006 - 0.006

[krancour]

@bacongobbler thanks for taking the time. fwiw, no need to delete the router pod to apply configs in the future. Router is always watching its own annotations and applying new config when it spots diffs.

[bacongobbler]

I figured that the pod annotations are separate from the rc annotations so I just wanted to make sure. Thanks for the tip!