Merge branch '571-refactoring-acceptnace-part-5' into 'master'

Update more acceptance tests

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

7 files changed, 102 insertions, 89 deletions

diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go
index 11c0f381..a627cc00 100644
--- a/test/acceptance/helpers_test.go
+++ b/test/acceptance/helpers_test.go
@@ -211,11 +211,6 @@ func RunPagesProcess(t *testing.T, pagesBinary string, listeners []ListenSpec, p
 	return cleanup
 }
 
-func RunPagesProcessWithoutWait(t *testing.T, pagesBinary string, listeners []ListenSpec, promPort string, extraArgs ...string) (teardown func()) {
-	_, cleanup := runPagesProcess(t, false, pagesBinary, listeners, promPort, nil, extraArgs...)
-	return cleanup
-}
-
 func RunPagesProcessWithEnvs(t *testing.T, wait bool, pagesBinary string, listeners []ListenSpec, promPort string, envs []string, extraArgs ...string) (teardown func()) {
 	_, cleanup := runPagesProcess(t, wait, pagesBinary, listeners, promPort, envs, extraArgs...)
 	return cleanup
diff --git a/test/acceptance/serving_test.go b/test/acceptance/serving_test.go
index 049396e6..91e1181b 100644
--- a/test/acceptance/serving_test.go
+++ b/test/acceptance/serving_test.go
@@ -106,7 +106,7 @@ func TestKnownHostReturns200(t *testing.T) {
 	}
 }
 
-// TODO: remove along with https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382
+// TODO: remove along with support for disk configuration https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382
 func TestNestedSubgroups(t *testing.T) {
 	maxNestedSubgroup := 21
 
@@ -314,61 +314,53 @@ func TestHTTPSRedirect(t *testing.T) {
 	)
 
 	tests := map[string]struct {
+		domain         string
 		path           string
 		expectedStatus int
 	}{
 		"domain_https_only_true": {
+			domain:         "group.https-only.gitlab-example.com",
 			path:           "project1/",
 			expectedStatus: http.StatusMovedPermanently,
 		},
 		"domain_https_only_false": {
+			domain:         "group.https-only.gitlab-example.com",
 			path:           "project2/",
 			expectedStatus: http.StatusOK,
 		},
+		"custom_domain_https_enabled": {
+			domain:         "test.my-domain.com",
+			path:           "/index.html",
+			expectedStatus: http.StatusMovedPermanently,
+		},
+		"custom_domain_https_disabled": {
+			domain:         "test2.my-domain.com",
+			path:           "/",
+			expectedStatus: http.StatusOK,
+		},
+		"custom_domain_https_enabled_with_bad_certificate_is_still_redirected": {
+			domain:         "no.cert.com",
+			path:           "/",
+			expectedStatus: http.StatusMovedPermanently,
+		},
 	}
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			// see testdata/api_responses.go for per prefix configuration response from the API
-			rsp, err := GetRedirectPage(t, httpListener, "group.https-only.gitlab-example.com", test.path)
+			rsp, err := GetRedirectPage(t, httpListener, test.domain, test.path)
 			require.NoError(t, err)
-			defer rsp.Body.Close()
+
+			t.Cleanup(func() {
+				rsp.Body.Close()
+			})
 
 			require.Equal(t, test.expectedStatus, rsp.StatusCode)
 		})
 	}
 }
 
-func TestHttpsOnlyProjectEnabled(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "")
-	defer teardown()
-
-	rsp, err := GetRedirectPage(t, httpListener, "test.my-domain.com", "/index.html")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	require.Equal(t, http.StatusMovedPermanently, rsp.StatusCode)
-}
-
-func TestHttpsOnlyProjectDisabled(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "")
-	defer teardown()
-
-	rsp, err := GetPageFromListener(t, httpListener, "test2.my-domain.com", "/")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	require.Equal(t, http.StatusOK, rsp.StatusCode)
-}
-
-func TestHttpsOnlyDomainDisabled(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "")
-	defer teardown()
-
-	rsp, err := GetPageFromListener(t, httpListener, "no.cert.com", "/")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	require.Equal(t, http.StatusOK, rsp.StatusCode)
-}
-
+// TODO: remove with domain-source-config https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382
 func TestDomainsSource(t *testing.T) {
 	type args struct {
 		configSource string
@@ -512,6 +504,7 @@ 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`
+// TODO: remove with domain-source-config https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382
 func TestGitLabSourceBecomesUnauthorized(t *testing.T) {
 	opts := &stubOpts{
 		// edge case https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535
diff --git a/test/acceptance/status_test.go b/test/acceptance/status_test.go
index 13aa9534..6fb2efe0 100644
--- a/test/acceptance/status_test.go
+++ b/test/acceptance/status_test.go
@@ -9,8 +9,10 @@ import (
 )
 
 func TestStatusPage(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-pages-status=/@statuscheck")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t,
+		withListeners([]ListenSpec{httpListener}),
+		withExtraArgument("pages-status", "/@statuscheck"),
+	)
 
 	rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "@statuscheck")
 	require.NoError(t, err)
@@ -19,23 +21,29 @@ func TestStatusPage(t *testing.T) {
 }
 
 func TestStatusNotYetReady(t *testing.T) {
-	teardown := RunPagesProcessWithoutWait(t, *pagesBinary, supportedListeners(), "", "-pages-status=/@statuscheck", "-pages-root=../../shared/invalid-pages")
-	defer teardown()
-
-	waitForRoundtrips(t, supportedListeners(), 5*time.Second)
-	rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "@statuscheck")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	require.Equal(t, http.StatusServiceUnavailable, rsp.StatusCode)
-}
-
-func TestPageNotAvailableIfNotLoaded(t *testing.T) {
-	teardown := RunPagesProcessWithoutWait(t, *pagesBinary, supportedListeners(), "", "-pages-root=../../shared/invalid-pages")
-	defer teardown()
-	waitForRoundtrips(t, supportedListeners(), 5*time.Second)
-
-	rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "index.html")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	require.Equal(t, http.StatusServiceUnavailable, rsp.StatusCode)
+	listeners := supportedListeners()
+
+	RunPagesProcessWithStubGitLabServer(t,
+		withoutWait,
+		withExtraArgument("pages-status", "/@statuscheck"),
+		withExtraArgument("pages-root", "../../shared/invalid-pages"),
+		withStubOptions(&stubOpts{
+			statusReadyCount: 100,
+		}),
+	)
+
+	waitForRoundtrips(t, listeners, time.Duration(len(listeners))*time.Second)
+
+	// test status on all supported listeners
+	for _, spec := range listeners {
+		rsp, err := GetPageFromListener(t, spec, "group.gitlab-example.com", "@statuscheck")
+		require.NoError(t, err)
+		defer rsp.Body.Close()
+		require.Equal(t, http.StatusServiceUnavailable, rsp.StatusCode)
+
+		rsp2, err2 := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "index.html")
+		require.NoError(t, err2)
+		defer rsp2.Body.Close()
+		require.Equal(t, http.StatusServiceUnavailable, rsp2.StatusCode, "page should not be served")
+	}
 }
diff --git a/test/acceptance/stub_test.go b/test/acceptance/stub_test.go
index 6346fe4e..a22f798a 100644
--- a/test/acceptance/stub_test.go
+++ b/test/acceptance/stub_test.go
@@ -59,6 +59,12 @@ func withArguments(args []string) processOption {
 	}
 }
 
+func withStubOptions(opts *stubOpts) processOption {
+	return func(config *processConfig) {
+		config.gitlabStubOpts = opts
+	}
+}
+
 // makeGitLabPagesAccessStub provides a stub *httptest.Server to check pages_access API call.
 // the result is based on the project id.
 //
diff --git a/test/acceptance/testdata/api_responses.go b/test/acceptance/testdata/api_responses.go
index 227a72fd..b36563bb 100644
--- a/test/acceptance/testdata/api_responses.go
+++ b/test/acceptance/testdata/api_responses.go
@@ -51,6 +51,21 @@ var DomainResponses = map[string]responseFn{
 		projectID:  1001,
 		pathOnDisk: "group.redirects/custom-domain",
 	}),
+	"test.my-domain.com": customDomain(projectConfig{
+		projectID:  1002,
+		https:      true,
+		pathOnDisk: "group.https-only/project3",
+	}),
+	"test2.my-domain.com": customDomain(projectConfig{
+		projectID:  1003,
+		https:      false,
+		pathOnDisk: "group.https-only/project4",
+	}),
+	"no.cert.com": customDomain(projectConfig{
+		projectID:  1004,
+		https:      true,
+		pathOnDisk: "group.https-only/project5",
+	}),
 	// NOTE: before adding more domains here, generate the zip archive by running (per project)
 	// make zip PROJECT_SUBDIR=group/serving
 	// make zip PROJECT_SUBDIR=group/project2
diff --git a/test/acceptance/tls_test.go b/test/acceptance/tls_test.go
index a332e407..17e5263d 100644
--- a/test/acceptance/tls_test.go
+++ b/test/acceptance/tls_test.go
@@ -8,8 +8,9 @@ import (
 )
 
 func TestAcceptsSupportedCiphers(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t,
+		withListeners([]ListenSpec{httpsListener}),
+	)
 
 	tlsConfig := &tls.Config{
 		CipherSuites: []uint16{
@@ -25,12 +26,11 @@ func TestAcceptsSupportedCiphers(t *testing.T) {
 	defer cleanup()
 
 	rsp, err := client.Get(httpsListener.URL("/"))
+	require.NoError(t, err)
 
-	if rsp != nil {
+	t.Cleanup(func() {
 		rsp.Body.Close()
-	}
-
-	require.NoError(t, err)
+	})
 }
 
 func tlsConfigWithInsecureCiphersOnly() *tls.Config {
@@ -44,36 +44,32 @@ func tlsConfigWithInsecureCiphersOnly() *tls.Config {
 }
 
 func TestRejectsUnsupportedCiphers(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t,
+		withListeners([]ListenSpec{httpsListener}),
+	)
 
 	client, cleanup := ClientWithConfig(tlsConfigWithInsecureCiphersOnly())
 	defer cleanup()
 
 	rsp, err := client.Get(httpsListener.URL("/"))
-
-	if rsp != nil {
-		rsp.Body.Close()
-	}
-
-	require.Error(t, err)
 	require.Nil(t, rsp)
+	require.Error(t, err)
 }
 
 func TestEnableInsecureCiphers(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-insecure-ciphers")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t,
+		withListeners([]ListenSpec{httpsListener}),
+		withExtraArgument("-insecure-ciphers", "true"),
+	)
 
 	client, cleanup := ClientWithConfig(tlsConfigWithInsecureCiphersOnly())
 	defer cleanup()
 
 	rsp, err := client.Get(httpsListener.URL("/"))
-
-	if rsp != nil {
-		rsp.Body.Close()
-	}
-
 	require.NoError(t, err)
+	t.Cleanup(func() {
+		rsp.Body.Close()
+	})
 }
 
 func TestTLSVersions(t *testing.T) {
@@ -90,7 +86,7 @@ func TestTLSVersions(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			args := []string{}
+			var args []string
 			if tc.tlsMin != "" {
 				args = append(args, "-tls-min-version", tc.tlsMin)
 			}
@@ -98,8 +94,10 @@ func TestTLSVersions(t *testing.T) {
 				args = append(args, "-tls-max-version", tc.tlsMax)
 			}
 
-			teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", args...)
-			defer teardown()
+			RunPagesProcessWithStubGitLabServer(t,
+				withListeners([]ListenSpec{httpsListener}),
+				withArguments(args),
+			)
 
 			tlsConfig := &tls.Config{}
 			if tc.tlsClient != 0 {
@@ -111,14 +109,11 @@ func TestTLSVersions(t *testing.T) {
 
 			rsp, err := client.Get(httpsListener.URL("/"))
 
-			if rsp != nil {
-				rsp.Body.Close()
-			}
-
 			if tc.expectError {
 				require.Error(t, err)
 			} else {
 				require.NoError(t, err)
+				rsp.Body.Close()
 			}
 		})
 	}
diff --git a/test/acceptance/unknown_http_method_test.go b/test/acceptance/unknown_http_method_test.go
index ffa07153..c06dbed8 100644
--- a/test/acceptance/unknown_http_method_test.go
+++ b/test/acceptance/unknown_http_method_test.go
@@ -8,8 +8,9 @@ import (
 )
 
 func TestUnknownHTTPMethod(t *testing.T) {
-	teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t,
+		withListeners([]ListenSpec{httpListener}),
+	)
 
 	req, err := http.NewRequest("UNKNOWN", httpListener.URL(""), nil)
 	require.NoError(t, err)