fix(nginx.tmpl): port_in_redirect off

[jsravn]

For locations with a trailing prefix like /foo/, nginx will redirect
any requests /foo to /foo/. By default, it redirects to the internal
listen port which likely will not match the port the external client
uses. Therefore, we should always preserve the external port the client
uses to access ingress.

[aecay]

I'm not 100% convinced that this is the correct fix for the problem seen. This fixes a problem in our case (where feed is listening on a non-default port, but is fronted by other loadbalancers that are listening on the default port). But this fix would break if the outer edge of the LB stack was configured to listen on a non-default port (since it would redirect to the default port). AFAICS, that's an equally valid configuration for feed the OSS product, albeit not one we have opted for at Sky.

The alternative fix would be something like absolute_redirect off to prevent redirects from affecting the port (or host) at all. But I haven't studied the options well enough to know whether (1) that's an equivalent fix to the problem you saw or (2) whether it's the optimal fix.

As we discussed earlier, due to constraints on Sky as an engineering organization and feed's place in the (internal and external) software ecosystem, I don't believe we should be spending time on code fixes to feed where said fixes don't contribute to fixing a problem we actually face (and as discussed this fix does not do that, since beyond the redirect pointing to an invalid port you are also facing other issues with the redirect manipulating a trailing slash -- so you are forced to implement a solution that avoids this redirect entirely). So rather than an invitation to explore alternatives, you should read what I have written here as an explanation of why I don't want us to press forward with this (or any) change in this area.

PS If we were to accept any PR to fix bugs, it should come with regression tests to guard against that bug reemerging. I don't fully understand the structure/purpose of the unit tests in the nginx module, but I see lots of tests that (apparently) probe plenty of details of the nginx config syntax, so something along those lines would be inventable in principle for this change as well.