feat(appliance): implement re-direct on all unknown endpoints#64059
Conversation
craigfurman
left a comment
There was a problem hiding this comment.
This doesn't appear to work, but I wasn't able to get it working either 🤔
My progress is on top of this branch, in https://github.com/sourcegraph/sourcegraph/commit/469da79a5d89d39989c5ab55d7924d9224c7acdd. Please see commit message too.
I was testing this with:
docker rmi appliance-frontend:candidate
sg bazel run //docker-images/appliance-frontend:image_tarball
docker run -e API_ENDPOINT=http://localhost:8888 --platform linux/amd64 --rm -p 80:80 appliance-frontend:candidate
And navigating to localhost in my browser.
|
new hash, who dis: https://github.com/sourcegraph/sourcegraph/commit/2ce7687062bf6d8e113548c95a5d6317d68c7b68 @jdpleiness and I were pairing and managed to get this working 😱 🎉 🙌 🍻 I reckon we can start merging this 3-stack (2 PRs and my branch, which maybe we merge into here), and roll forward with any bug fixes. It's hard to test this e2e because the base of the basest branch is so old, that the frontend and backend have changed a lot. |
- Disable access logs. AFAICT these must be files, rather than references to stdout/stderr (I couldn't get that working). Since we'll be running in Kubernetes, we'd have to add a mechanism to pipe that file to stdout so that it shows up in logs, which doesn't seem worth it. Or bind-mount the file to a shared volume and use fluentd or similar to pipe it out, which also doesn't seem worth it. - Change error log to stderr. Kubernetes containers won't have permission to open /dev/console, and neither did my local docker setup. Using stderr means that these logs will be visible in kubectl-logs too. - Fix typo in "maintenance" for redirect - Use 302 instead of 301. The browser 301 cache needs to be specifically cleared, which makes iteration tricky. We are not bottlenecked on the latency of the user's 301 cache anyway :) - Add $server_port to redirect. Indeed, if we bind port 80 this is a no-op, but it might avoid confusing bugs in the future. - I think we had a redirect loop, which I've tried to fix with some ugly nginx logic (turns out it doesn't have and/or, and nested ifs cause warnings in linters I found). I managed to stop the loop, and I think the redirect logic works now...
718c054 to
6ba2a4d
Compare
craigfurman
left a comment
There was a problem hiding this comment.
@Chickensoupwithrice apologies for appropriating this PR, @jdpleiness and I are hacking in an earlier time zone. Holler if our changes are bogus and we can roll forward with fixes together
Resolves request 4 of [REL-78](https://linear.app/sourcegraph/issue/REL-78/when-sourcegraph-frontend-is-down-a-user-trying-to-access-sourcegraph#comment-ae562c6d) Co-authored-by: Craig Furman <craig.furman@sourcegraph.com>
Resolves request 4 of REL-78
Test plan
Hopes and Prayers 🙏
Changelog