Prevent Workhorse panics when Geo proxy URL is unset
What does this MR do and why?
When the Geo Rails API intends to disable the proxying functionality, like for example not being a secondary anymore, or the Rails feature flag is disabled, an empty URL will be returned.
This prevents the empty, invalid URL from getting parsed and resulting in panics because of it.
Screenshots or screen recordings
Logs, before:
2021-09-28_09:36:55.92250 gitlab-workhorse : {"level":"info","msg":"Geo Proxy: URL changed","newGeoProxyURL":{"Scheme":"http","Opaque":"","User":null,"Host":"gdk.test:3000","Path":"/","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"oldGeoProxyURL":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"time":"2021-09-28T10:36:55+01:00"}
2021-09-28_09:37:16.09171 gitlab-workhorse : {"level":"info","msg":"Geo Proxy: URL changed","newGeoProxyURL":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"oldGeoProxyURL":{"Scheme":"http","Opaque":"","User":null,"Host":"gdk.test:3000","Path":"/","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"time":"2021-09-28T10:37:16+01:00"}
2021-09-28_09:37:16.09386 gitlab-workhorse : panic: could not parse host:port from address ":" and scheme ""
2021-09-28_09:37:16.09387 gitlab-workhorse :
2021-09-28_09:37:16.09388 gitlab-workhorse : goroutine 424 [running]:
2021-09-28_09:37:16.09388 gitlab-workhorse : gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream/roundtripper.mustParseAddress(0x14687e0, 0x1, 0x0, 0x0, 0x0, 0x0)
2021-09-28_09:37:16.09388 gitlab-workhorse : /home/catalin/gdk-geo/gitlab/workhorse/internal/upstream/roundtripper/roundtripper.go:26 +0x28f
2021-09-28_09:37:16.09388 gitlab-workhorse : gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream/roundtripper.newBackendRoundTripper(0xc000b807e0, 0x0, 0x0, 0x45d964b800, 0x143d401, 0x0, 0xc000cade70, 0x5504ec)
2021-09-28_09:37:16.09388 gitlab-workhorse : /home/catalin/gdk-geo/gitlab/workhorse/internal/upstream/roundtripper/roundtripper.go:41 +0x3cc
2021-09-28_09:37:16.09388 gitlab-workhorse : gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream/roundtripper.NewBackendRoundTripper(...)
2021-09-28_09:37:16.09389 gitlab-workhorse : /home/catalin/gdk-geo/gitlab/workhorse/internal/upstream/roundtripper/roundtripper.go:31
2021-09-28_09:37:16.09389 gitlab-workhorse : gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream.(*upstream).updateGeoProxyFields(0xc00022ca00, 0xc000b807e0)
2021-09-28_09:37:16.09389 gitlab-workhorse : /home/catalin/gdk-geo/gitlab/workhorse/internal/upstream/upstream.go:237 +0xdb
2021-09-28_09:37:16.09390 gitlab-workhorse : gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream.(*upstream).callGeoProxyAPI(0xc00022ca00)
2021-09-28_09:37:16.09390 gitlab-workhorse : /home/catalin/gdk-geo/gitlab/workhorse/internal/upstream/upstream.go:228 +0x365
2021-09-28_09:37:16.09390 gitlab-workhorse : gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream.(*upstream).pollGeoProxyAPI(0xc00022ca00)
2021-09-28_09:37:16.09390 gitlab-workhorse : /home/catalin/gdk-geo/gitlab/workhorse/internal/upstream/upstream.go:207 +0x3d
2021-09-28_09:37:16.09390 gitlab-workhorse : created by gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream.newUpstream
2021-09-28_09:37:16.09391 gitlab-workhorse : /home/catalin/gdk-geo/gitlab/workhorse/internal/upstream/upstream.go:92 +0x6b5
2021-09-28_09:37:16.12248 gitlab-workhorse : {"build_time":"20210928.093541","level":"info","msg":"Starting","time":"2021-09-28T10:37:16+01:00","version":"11-10-0cfa69752d8-74ffd66ae-ee-124108-g817cb54a7d1"}
After:
2021-09-28_09:42:39.89558 gitlab-workhorse : {"level":"info","msg":"Geo Proxy: URL changed","newGeoProxyURL":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"oldGeoProxyURL":{"Scheme":"http","Opaque":"","User":null,"Host":"gdk.test:3000","Path":"/","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"time":"2021-09-28T10:42:49+01:00"}
2021-09-28_09:42:49.96587 gitlab-workhorse : {"level":"info","msg":"Geo Proxy: URL changed","newGeoProxyURL":{"Scheme":"http","Opaque":"","User":null,"Host":"gdk.test:3000","Path":"/","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"oldGeoProxyURL":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"time":"2021-09-28T10:43:10+01:00"}
2021-09-28_09:43:10.09871 gitlab-workhorse : {"level":"info","msg":"Geo Proxy: URL changed","newGeoProxyURL":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"oldGeoProxyURL":{"Scheme":"http","Opaque":"","User":null,"Host":"gdk.test:3000","Path":"/","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"time":"2021-09-28T10:43:40+01:00"}
How to set up and validate locally
Using the steps from !67700 (merged):
- Checkout this branch on your local Geo secondary site
cd <path to gdk-geo directory>
- Edit
Procfile
- Find the line that begins
gitlab-workhorse: exec /usr/bin/env PATH=
- Add
GEO_SECONDARY_PROXY=1
so the line looks likegitlab-workhorse: exec /usr/bin/env GEO_SECONDARY_PROXY=1 PATH=
make gitlab-workhorse-update && gdk restart gitlab-workhorse && gdk tail gitlab-workhorse
Testing the panic doesn't happen anymore, and flipping enabled/disabled works:
- In another Terminal:
cd <path to gdk directory (the primary)>/gitlab
bin/rails runner "Feature.enable(:geo_secondary_proxy)"
- Double check that requests get proxied to the primary (you should not see the "secondary is read-only" banner)
bin/rails runner "Feature.disable(:geo_secondary_proxy)"
- Double check with
gdk status gitlab-workhorse
and the logs that Workhorse didn't restart / there was no panic, and requests don't get proxied anymore
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.