Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feed returns incorrect 301 Location header #192

Open
shin-nien opened this issue Dec 14, 2018 · 3 comments
Open

Feed returns incorrect 301 Location header #192

shin-nien opened this issue Dec 14, 2018 · 3 comments

Comments

@shin-nien
Copy link
Contributor

shin-nien commented Dec 14, 2018

For any ingress that uses a non / path e.g. /myapp, trying to hit http://<host>/myapp results in a 301 with an incorrect Location header. Hitting /myapp/ hits the backend as expected.

HTTP/1.1 301 Moved Permanently
Content-Type: text/html
Date: Fri, 14 Dec 2018 12:11:47 GMT
Location: http://shin-test-ingress.<domain>:8080/myapp/
Server: nginx
Content-Length: 178
Connection: keep-alive

The 8080 is the port nginx is configured to listen to rather than the listening port of the ELB.

Arguably, hitting /myapp shouldn't be issuing a redirect at all; it's the implementation of

location {{ if $location.Path }}{{ $location.Path }}{{ end }} {
that appends a trailing / presumably because prefix matching was preferred but then users of feed had to adhere to calling the endpoints with a trailing /.

My guess is that it's failing to find an exact match so it fallbacks to an approximate match, finds it but then thinks it should issue a 301 to the path it's found.

Historically, most endpoints in feed have been APIs and this was only noticed when someone tried to use a path with a website and a browser although in practise, we tend to just use / as the path if we're fronting Grafana, Jenkins etc.

@jsravn
Copy link
Contributor

jsravn commented Jan 7, 2019

Is it possible to override 8080 w/ the correct port?

@shin-nien
Copy link
Contributor Author

That could work. The commandline args would just need to change to match the ELB ports and to check if the ingress is wanting https or not.

listen {{ $portConf.Port }}{{- if eq $portConf.Name "https" }} ssl{{ end }}{{ if $proxyprotocol }} proxy_protocol{{ end }};

@jsravn
Copy link
Contributor

jsravn commented Jan 8, 2019

A little harder than that probably, since you won't be able to bind to 80/443 by default without setting capabilities.

alangibson01 pushed a commit that referenced this issue Mar 22, 2019
Goes some way to resolving #192.

Right now, if an ingress specifies a path of e.g. /my/test/path, this will be rendered in the location block as location /my/test/path/. If a browser then hits /my/test/path it will be redirected to /my/test/path/ with a 301 permanent redirect.

If /my/test/path is not a directory, but is instead an absolute path to a resource, then redirecting to /my/test/path/ simply doesn't make sense. This PR introduces the concept of "exact paths" by allowing ingresses to specify if all of their paths are absolute paths to resources. If so, we render the path as location = <raw path value>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants