From ef0c3bcf17882226fc66fe08caa71e0eb7517c3d Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Tue, 26 Jan 2021 15:13:53 +1100 Subject: Handle unauthorized error and fallback to disk source --- internal/source/domains.go | 11 +++++++++- internal/source/gitlab/client/client.go | 8 ++++++++ internal/source/gitlab/client/client_test.go | 2 +- test/acceptance/helpers_test.go | 16 +++++++++++---- test/acceptance/serving_test.go | 30 +++++++++++++++++++++------- 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/internal/source/domains.go b/internal/source/domains.go index cf81fab2..6365c2e8 100644 --- a/internal/source/domains.go +++ b/internal/source/domains.go @@ -1,6 +1,7 @@ package source import ( + "errors" "fmt" "regexp" @@ -9,6 +10,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/domain" "gitlab.com/gitlab-org/gitlab-pages/internal/source/disk" "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab" + "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client" ) var ( @@ -97,7 +99,14 @@ func (d *Domains) setGitLabClient(config Config) error { // for some subset of domains, to test / PoC the new GitLab Domains Source that // we plan to use to replace the disk source. func (d *Domains) GetDomain(name string) (*domain.Domain, error) { - return d.source(name).GetDomain(name) + resolvedDomain, err := d.source(name).GetDomain(name) + if errors.Is(err, client.ErrUnauthorizedAPI) && d.configSource == sourceAuto { + log.WithError(err).Warn("Pages cannot communicate with an instance of the GitLab API, please sync your gitlab-secrets.json file https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535#workaround ") + // temporary workaround for https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535 + return d.disk.GetDomain(name) + } + + return resolvedDomain, err } // Read starts the disk domain source. It is DEPRECATED, because we want to diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index 524eb8a4..c4e6ebf2 100644 --- a/internal/source/gitlab/client/client.go +++ b/internal/source/gitlab/client/client.go @@ -24,6 +24,12 @@ import ( // or a 401 given that the credentials used are wrong const ConnectionErrorMsg = "failed to connect to internal Pages API" +// ErrUnauthorizedAPI is returned when resolving a domain with the GitLab API +// returns a http.StatusUnauthorized. This is an edge case when `domain-config-source=auto` +// and there are multiple instances of GitLab Rails and GitLab Pages running. +// See https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535 for more info +var ErrUnauthorizedAPI = errors.New("pages endpoint unauthorized") + // Client is a HTTP client to access Pages internal API type Client struct { secretKey []byte @@ -161,6 +167,8 @@ func (gc *Client) get(ctx context.Context, path string, params url.Values) (*htt // StatusNoContent means that a domain does not exist, it is not an error if resp.StatusCode == http.StatusNoContent { return nil, nil + } else if resp.StatusCode == http.StatusUnauthorized { + return nil, ErrUnauthorizedAPI } return nil, fmt.Errorf("HTTP status: %d", resp.StatusCode) diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go index 153b9372..32fd8c18 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -138,7 +138,7 @@ func TestNewInvalidConfiguration(t *testing.T) { } func TestLookupForErrorResponses(t *testing.T) { tests := map[int]string{ - http.StatusUnauthorized: "HTTP status: 401", + http.StatusUnauthorized: ErrUnauthorizedAPI.Error(), http.StatusNotFound: "HTTP status: 404", } diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index 8d8ca8d6..fe43dfcc 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -536,10 +536,11 @@ func waitForRoundtrips(t *testing.T, listeners []ListenSpec, timeout time.Durati } type stubOpts struct { - apiCalled bool - statusReadyCount int - statusHandler http.HandlerFunc - pagesHandler http.HandlerFunc + apiCalled bool + statusReadyCount int + statusHandler http.HandlerFunc + pagesHandler http.HandlerFunc + pagesStatusResponse int } func NewGitlabDomainsSourceStub(t *testing.T, opts *stubOpts) *httptest.Server { @@ -553,6 +554,7 @@ func NewGitlabDomainsSourceStub(t *testing.T, opts *stubOpts) *httptest.Server { statusHandler := func(w http.ResponseWriter, r *http.Request) { if currentStatusCount < opts.statusReadyCount { w.WriteHeader(http.StatusBadGateway) + return } w.WriteHeader(http.StatusNoContent) @@ -566,6 +568,12 @@ func NewGitlabDomainsSourceStub(t *testing.T, opts *stubOpts) *httptest.Server { pagesHandler := func(w http.ResponseWriter, r *http.Request) { opts.apiCalled = true + + if opts.pagesStatusResponse != 0 { + w.WriteHeader(opts.pagesStatusResponse) + return + } + domain := r.URL.Query().Get("host") path := "../../shared/lookups/" + domain + ".json" diff --git a/test/acceptance/serving_test.go b/test/acceptance/serving_test.go index 6afa9560..e3f5cb3c 100644 --- a/test/acceptance/serving_test.go +++ b/test/acceptance/serving_test.go @@ -381,7 +381,7 @@ func TestDomainsSource(t *testing.T) { configSource string domain string urlSuffix string - readyCount int + stubOpts stubOpts } type want struct { statusCode int @@ -459,7 +459,9 @@ func TestDomainsSource(t *testing.T) { configSource: "auto", domain: "test.domain.com", urlSuffix: "/", - readyCount: 100, // big number to ensure the API is in bad state for a while + stubOpts: stubOpts{ + statusReadyCount: 100, // big number to ensure the API is in bad state for a while + }, }, want: want{ statusCode: http.StatusOK, @@ -473,7 +475,6 @@ func TestDomainsSource(t *testing.T) { configSource: "auto", domain: "new-source-test.gitlab.io", urlSuffix: "/my/pages/project/", - readyCount: 0, }, want: want{ statusCode: http.StatusOK, @@ -481,14 +482,29 @@ func TestDomainsSource(t *testing.T) { apiCalled: true, }, }, + { + name: "auto_source_gitlab_is_unauthorized_fallback_to_disk", + args: args{ + configSource: "auto", + domain: "test.domain.com", + urlSuffix: "/", + stubOpts: stubOpts{ + apiCalled: false, + // edge case https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535 + pagesStatusResponse: http.StatusUnauthorized, + }, + }, + want: want{ + statusCode: http.StatusOK, + content: "main-dir\n", + apiCalled: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - opts := &stubOpts{ - apiCalled: false, - statusReadyCount: tt.args.readyCount, - } + opts := &tt.args.stubOpts source := NewGitlabDomainsSourceStub(t, opts) defer source.Close() -- cgit v1.2.3