Merge branch '535-handle-unauthorized-err' into 'master'

Handle unauthorized error and fallback to disk source

Closes #535

See merge request gitlab-org/gitlab-pages!424

2 files changed, 59 insertions, 6 deletions

diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go
index 8d8ca8d6..5c380938 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)
@@ -565,8 +567,20 @@ func NewGitlabDomainsSourceStub(t *testing.T, opts *stubOpts) *httptest.Server {
 	mux.HandleFunc("/api/v4/internal/pages/status", statusHandler)
 
 	pagesHandler := func(w http.ResponseWriter, r *http.Request) {
-		opts.apiCalled = true
 		domain := r.URL.Query().Get("host")
+		if domain == "127.0.0.1" {
+			// shortcut for healthy checkup done by WaitUntilRequestSucceeds
+			w.WriteHeader(http.StatusNoContent)
+			return
+		}
+
+		opts.apiCalled = true
+
+		if opts.pagesStatusResponse != 0 {
+			w.WriteHeader(opts.pagesStatusResponse)
+			return
+		}
+
 		path := "../../shared/lookups/" + domain + ".json"
 
 		fixture, err := os.Open(path)
diff --git a/test/acceptance/serving_test.go b/test/acceptance/serving_test.go
index 6cc41eff..19476830 100644
--- a/test/acceptance/serving_test.go
+++ b/test/acceptance/serving_test.go
@@ -496,7 +496,7 @@ func TestDomainsSource(t *testing.T) {
 			gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t)
 
 			pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", tt.args.configSource}
-			teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, listeners, "", []string{}, pagesArgs...)
+			teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, []ListenSpec{httpListener}, "", []string{}, pagesArgs...)
 			defer teardown()
 
 			response, err := GetPageFromListener(t, httpListener, tt.args.domain, tt.args.urlSuffix)
@@ -516,6 +516,45 @@ func TestDomainsSource(t *testing.T) {
 	}
 }
 
+// TestGitLabSourceBecomesUnauthorized proves workaround for https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535
+// The first request will fail and display an error but subsequent requests will
+// serve from disk source when `domain-config-source=auto`
+func TestGitLabSourceBecomesUnauthorized(t *testing.T) {
+	opts := &stubOpts{
+		// edge case https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535
+		pagesStatusResponse: http.StatusUnauthorized,
+	}
+	source := NewGitlabDomainsSourceStub(t, opts)
+	defer source.Close()
+
+	gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t)
+
+	pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "auto"}
+	teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, []ListenSpec{httpListener}, "", []string{}, pagesArgs...)
+	defer teardown()
+
+	domain := "test.domain.com"
+	failedResponse, err := GetPageFromListener(t, httpListener, domain, "/")
+	require.NoError(t, err)
+
+	require.True(t, opts.apiCalled, "API should be called")
+	require.Equal(t, http.StatusBadGateway, failedResponse.StatusCode, "first response should fail with 502")
+
+	// make request again
+	opts.apiCalled = false
+
+	response, err := GetPageFromListener(t, httpListener, domain, "/")
+	require.NoError(t, err)
+	defer response.Body.Close()
+
+	require.False(t, opts.apiCalled, "API should not be called after the first failure")
+	require.Equal(t, http.StatusOK, response.StatusCode, "second response should succeed")
+
+	body, err := ioutil.ReadAll(response.Body)
+	require.NoError(t, err)
+	require.Equal(t, "main-dir\n", string(body), "content mismatch")
+}
+
 func TestKnownHostInReverseProxySetupReturns200(t *testing.T) {
 	skipUnlessEnabled(t)