-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Redirect to www. if 404 Not Found #3035
Conversation
@@ -17,7 +17,7 @@ cd /etc/nginx/ | |||
|
|||
replaceEnvVars() { | |||
echo "Updating $i" | |||
envsubst '$KUBE_NAMESPACE,$PROXY_DOMAIN_REGEX,$PROXY_DOMAIN_COOKIE,$NAMESERVER,$SERVER_PROXY_APIKEY' < $1 > /tmp/foo; | |||
envsubst '$KUBE_NAMESPACE,$PROXY_DOMAIN,$PROXY_DOMAIN_REGEX,$PROXY_DOMAIN_COOKIE,$NAMESERVER,$SERVER_PROXY_APIKEY' < $1 > /tmp/foo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
} | ||
|
||
location @error_404_dashboard { | ||
return 302 https://www.${PROXY_DOMAIN}$request_uri; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the assumption that there is a www.
subdomain is new - I bet not. Also, would be nice if this is configurable, as "landing page", e.g.
But I think this is fine for now, especially as we want to test if this breaks sth on staging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Let's see if this breaks anything, like redirects or similar.
BTW: Created https://github.com/gitpod-com/gitpod-dev/issues/45 so we can test things like this (redirects to landing page) in the future. |
@geropl's idea from #2659 (comment) written in code. 🙏🏻
The suggestion is to test this on staging and see if that fits.
I find it hard to test on dev, because there is nothing in the ingress proxy of the dev staging to handle the
www.
URLs. (Gero mentioned, we can add this. But it's also nothing we miss ATM.)Resolves #2659