From 19086c1fb27f3a36399dd45d049799c4f40ac244 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Mon, 17 May 2021 16:48:01 +1000 Subject: [PATCH 1/2] Remove domain-config-source flag Changelog: removed --- app.go | 4 +- app_test.go | 8 ++- internal/config/config.go | 21 +++--- internal/config/flags.go | 3 +- internal/source/domains.go | 122 +++---------------------------- internal/source/domains_test.go | 124 ++------------------------------ 6 files changed, 34 insertions(+), 248 deletions(-) diff --git a/app.go b/app.go index 3b469b2d1..a8bc3eae7 100644 --- a/app.go +++ b/app.go @@ -412,8 +412,6 @@ func (a *theApp) Run() { a.listenMetricsFD(&wg, a.config.ListenMetrics) } - a.domains.Read(a.config.General.Domain) - wg.Wait() } @@ -498,7 +496,7 @@ func (a *theApp) listenMetricsFD(wg *sync.WaitGroup, fd uintptr) { } func runApp(config *cfg.Config) { - domains, err := source.NewDomains(config.General.DomainConfigurationSource, &config.GitLab) + domains, err := source.NewDomains(&config.GitLab) if err != nil { log.WithError(err).Fatal("could not create domains config source") } diff --git a/app_test.go b/app_test.go index 48a6013b3..d4855aa6a 100644 --- a/app_test.go +++ b/app_test.go @@ -10,9 +10,10 @@ import ( "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitlab-pages/internal/source" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/request" - "gitlab.com/gitlab-org/gitlab-pages/internal/source" ) func Test_setRequestScheme(t *testing.T) { @@ -88,7 +89,10 @@ func TestHealthCheckMiddleware(t *testing.T) { require.NoError(t, err) cfg.General.StatusPath = "/-/healthcheck" - domains, err := source.NewDomains("auto", &cfg.GitLab) + cfg.GitLab.InternalServer = "https://gitlab.example.com" + cfg.GitLab.APISecretKey = []byte("secret key") + + domains, err := source.NewDomains(&cfg.GitLab) require.NoError(t, err) app := theApp{ diff --git a/internal/config/config.go b/internal/config/config.go index bb9f43456..9059c3595 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -45,16 +45,15 @@ type Config struct { // General groups settings that are general to GitLab Pages and can not // be categorized under other head. type General struct { - Domain string - DomainConfigurationSource string - HTTP2 bool - MaxConns int - MetricsAddress string - RedirectHTTP bool - RootCertificate []byte - RootDir string - RootKey []byte - StatusPath string + Domain string + HTTP2 bool + MaxConns int + MetricsAddress string + RedirectHTTP bool + RootCertificate []byte + RootDir string + RootKey []byte + StatusPath string DisableCrossOriginRequests bool InsecureCiphers bool @@ -187,7 +186,6 @@ func loadConfig() (*Config, error) { config := &Config{ General: General{ Domain: strings.ToLower(*pagesDomain), - DomainConfigurationSource: *domainConfigSource, HTTP2: *useHTTP2, MaxConns: *maxConns, MetricsAddress: *metricsAddress, @@ -329,7 +327,6 @@ func LogConfig(config *Config) { "gitlab-server": config.GitLab.Server, "internal-gitlab-server": config.GitLab.InternalServer, "api-secret-key": *gitLabAPISecretKey, - "domain-config-source": config.General.DomainConfigurationSource, "enable-disk": config.GitLab.EnableDisk, "auth-redirect-uri": config.Authentication.RedirectURI, "auth-scope": config.Authentication.Scope, diff --git a/internal/config/flags.go b/internal/config/flags.go index 25bf37ac2..c6ab3a68b 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -40,8 +40,7 @@ var ( gitlabRetrievalInterval = flag.Duration("gitlab-retrieval-interval", time.Second, "The interval to wait before retrying to resolve a domain's configuration via the GitLab API") gitlabRetrievalRetries = flag.Int("gitlab-retrieval-retries", 3, "The maximum number of times to retry to resolve a domain's configuration via the API") - domainConfigSource = flag.String("domain-config-source", "auto", "Domain configuration source 'disk', 'auto' or 'gitlab' (default: 'auto'). DEPRECATED: gitlab-pages will use the API-based configuration starting from 14.0 see https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382") - enableDisk = flag.Bool("enable-disk", true, "Enable disk access, shall be disabled in environments where shared disk storage isn't available") + enableDisk = flag.Bool("enable-disk", true, "Enable disk access, shall be disabled in environments where shared disk storage isn't available") clientID = flag.String("auth-client-id", "", "GitLab application Client ID") clientSecret = flag.String("auth-client-secret", "", "GitLab application Client Secret") diff --git a/internal/source/domains.go b/internal/source/domains.go index 7507938a1..e60eeb2f7 100644 --- a/internal/source/domains.go +++ b/internal/source/domains.go @@ -1,14 +1,10 @@ package source import ( - "fmt" "regexp" - "gitlab.com/gitlab-org/labkit/log" - "gitlab.com/gitlab-org/gitlab-pages/internal/config" "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" ) @@ -16,83 +12,29 @@ var ( // serverlessDomainRegex is a regular expression we use to check if a domain // is a serverless domain, to short circuit gitlab source rollout. It can be // removed after the rollout is done + // TODO: remove serverless serverlessDomainRegex = regexp.MustCompile(`^[^.]+-[[:xdigit:]]{2}a1[[:xdigit:]]{10}f2[[:xdigit:]]{2}[[:xdigit:]]+-?.*`) ) -type configSource int - -const ( - sourceGitlab configSource = iota - // Disk source is deprecated and support will be removed in 14.0 - // https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382 - sourceDisk - sourceAuto -) - // Domains struct represents a map of all domains supported by pages. It is // currently using two sources during the transition to the new GitLab domains // source. type Domains struct { - configSource configSource - gitlab Source - disk *disk.Disk // legacy disk source + gitlab Source } // NewDomains is a factory method for domains initializing a mutex. It should // not initialize `dm` as we later check the readiness by comparing it with a // nil value. -func NewDomains(source string, cfg *config.GitLab) (*Domains, error) { - domains := &Domains{} - if err := domains.setConfigSource(source, cfg); err != nil { - return nil, err - } - - return domains, nil -} - -// setConfigSource and initialize gitlab source -// returns error if -domain-config-source is not valid -// returns error if -domain-config-source=gitlab and init fails -func (d *Domains) setConfigSource(source string, cfg *config.GitLab) error { - switch source { - case "gitlab": - d.configSource = sourceGitlab - return d.setGitLabClient(cfg) - case "auto": - d.configSource = sourceAuto - // enable disk for auto for now - d.disk = disk.New() - return d.setGitLabClient(cfg) - case "disk": - // TODO: disable domains.disk https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382 - d.configSource = sourceDisk - d.disk = disk.New() - default: - return fmt.Errorf("invalid option for -domain-config-source: %q", source) - } - - return nil -} - -// setGitLabClient when domain-config-source is `gitlab` or `auto`, only return error for `gitlab` source -func (d *Domains) setGitLabClient(cfg *config.GitLab) error { - // We want to notify users about any API issues - // Creating a glClient will start polling connectivity in the background - // and spam errors in log +func NewDomains(cfg *config.GitLab) (*Domains, error) { glClient, err := gitlab.New(cfg) if err != nil { - if d.configSource == sourceGitlab { - return err - } - - log.WithError(err).Warn("failed to initialize GitLab client for `-domain-config-source=auto`") - - return nil + return nil, err } - d.gitlab = glClient - - return nil + return &Domains{ + gitlab: glClient, + }, nil } // GetDomain retrieves a domain information from a source. We are using two @@ -100,59 +42,14 @@ func (d *Domains) setGitLabClient(cfg *config.GitLab) 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) -} - -// Read starts the disk domain source. It is DEPRECATED, because we want to -// remove it entirely when disk source gets removed. -func (d *Domains) Read(rootDomain string) { - // start disk.Read for sourceDisk and sourceAuto - if d.configSource != sourceGitlab { - d.disk.Read(rootDomain) - } + return d.gitlab.GetDomain(name) } // IsReady checks if the disk domain source managed to traverse entire pages // filesystem and is ready for use. It is DEPRECATED, because we want to remove // it entirely when disk source gets removed. func (d *Domains) IsReady() bool { - switch d.configSource { - case sourceGitlab: - return d.gitlab.IsReady() - case sourceDisk: - return d.disk.IsReady() - case sourceAuto: - // if gitlab is configured and is ready - if d.gitlab != nil && d.gitlab.IsReady() { - return true - } - - return d.disk.IsReady() - default: - return false - } -} - -func (d *Domains) source(domain string) Source { - // This check is only needed until we enable `d.gitlab` source in all - // environments (including on-premises installations) followed by removal of - // `d.disk` source. This can be safely removed afterwards. - if IsServerlessDomain(domain) { - return d.gitlab - } - - switch d.configSource { - case sourceDisk: - return d.disk - case sourceGitlab: - return d.gitlab - default: - if d.gitlab != nil && d.gitlab.IsReady() { - return d.gitlab - } - - return d.disk - } + return d.gitlab.IsReady() } // IsServerlessDomain checks if a domain requested is a serverless domain we @@ -161,6 +58,7 @@ func (d *Domains) source(domain string) Source { // Domain is a serverless domain when it matches `serverlessDomainRegex`. The // regular expression is also defined on the gitlab-rails side, see // https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/serverless/domain.rb#L7 +// TODO: remove serverless func IsServerlessDomain(domain string) bool { return serverlessDomainRegex.MatchString(domain) } diff --git a/internal/source/domains_test.go b/internal/source/domains_test.go index a9404eef3..9e96267c7 100644 --- a/internal/source/domains_test.go +++ b/internal/source/domains_test.go @@ -2,105 +2,12 @@ package source import ( "testing" - "time" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/domain" - "gitlab.com/gitlab-org/gitlab-pages/internal/source/disk" ) -func TestNewDomains(t *testing.T) { - validCfg := config.GitLab{ - InternalServer: "https://gitlab.com", - APISecretKey: []byte("abc"), - ClientHTTPTimeout: time.Second, - JWTTokenExpiration: time.Second, - } - - tests := []struct { - name string - source string - config config.GitLab - expectedErr string - expectGitlabNil bool - expectDiskNil bool - }{ - { - name: "no_source_config", - source: "", - expectedErr: "invalid option for -domain-config-source: \"\"", - }, - { - name: "invalid_source_config", - source: "invalid", - expectedErr: "invalid option for -domain-config-source: \"invalid\"", - }, - { - name: "disk_source", - source: "disk", - expectGitlabNil: true, - expectDiskNil: false, - }, - { - name: "auto_without_api_config", - source: "auto", - expectGitlabNil: true, - expectDiskNil: false, - }, - { - name: "auto_with_api_config", - source: "auto", - config: validCfg, - expectGitlabNil: false, - expectDiskNil: false, - }, - { - name: "gitlab_source_success", - source: "gitlab", - config: validCfg, - expectDiskNil: true, - }, - { - name: "gitlab_source_no_url", - source: "gitlab", - config: func() config.GitLab { - cfg := validCfg - cfg.InternalServer = "" - - return cfg - }(), - expectedErr: "GitLab API URL or API secret has not been provided", - }, - { - name: "gitlab_source_no_secret", - source: "gitlab", - config: func() config.GitLab { - cfg := validCfg - cfg.APISecretKey = []byte{} - - return cfg - }(), - expectedErr: "GitLab API URL or API secret has not been provided", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - domains, err := NewDomains(tt.source, &tt.config) - if tt.expectedErr != "" { - require.EqualError(t, err, tt.expectedErr) - return - } - require.NoError(t, err) - - require.Equal(t, tt.expectGitlabNil, domains.gitlab == nil, "mismatch gitlab nil") - require.Equal(t, tt.expectDiskNil, domains.disk == nil, "mismatch disk nil") - }) - } -} - func TestGetDomain(t *testing.T) { t.Run("when requesting an existing domain for gitlab source", func(t *testing.T) { testDomain := "new-source-test.gitlab.io" @@ -111,24 +18,7 @@ func TestGetDomain(t *testing.T) { Once() defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource, sourceGitlab) - - domain, err := domains.GetDomain(testDomain) - require.NoError(t, err) - require.NotNil(t, domain) - }) - - t.Run("when requesting an existing domain for auto source", func(t *testing.T) { - testDomain := "new-source-test.gitlab.io" - - newSource := NewMockSource() - newSource.On("GetDomain", testDomain). - Return(&domain.Domain{Name: testDomain}, nil). - Once() - newSource.On("IsReady").Return(true).Once() - defer newSource.AssertExpectations(t) - - domains := newTestDomains(t, newSource, sourceAuto) + domains := newTestDomains(t, newSource) domain, err := domains.GetDomain(testDomain) require.NoError(t, err) @@ -143,13 +33,14 @@ func TestGetDomain(t *testing.T) { defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource, sourceGitlab) + domains := newTestDomains(t, newSource) domain, err := domains.GetDomain("does-not-exist.test.io") require.NoError(t, err) require.Nil(t, domain) }) + // TODO: remove serverless t.Run("when requesting a serverless domain", func(t *testing.T) { testDomain := "func-aba1aabbccddeef2abaabbcc.serverless.gitlab.io" @@ -160,7 +51,7 @@ func TestGetDomain(t *testing.T) { defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource, sourceGitlab) + domains := newTestDomains(t, newSource) domain, err := domains.GetDomain(testDomain) require.NoError(t, err) @@ -168,6 +59,7 @@ func TestGetDomain(t *testing.T) { }) } +// TODO: remove serverless func TestIsServerlessDomain(t *testing.T) { t.Run("when a domain is serverless domain", func(t *testing.T) { require.True(t, IsServerlessDomain("some-function-aba1aabbccddeef2abaabbcc.serverless.gitlab.io")) @@ -182,12 +74,10 @@ func TestIsServerlessDomain(t *testing.T) { }) } -func newTestDomains(t *testing.T, gitlabSource *MockSource, config configSource) *Domains { +func newTestDomains(t *testing.T, gitlabSource *MockSource) *Domains { t.Helper() return &Domains{ - configSource: config, - gitlab: gitlabSource, - disk: disk.New(), + gitlab: gitlabSource, } } -- GitLab From ca65440d31d56bfb123ef69309b6e27d83c4b0a7 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Mon, 17 May 2021 17:11:51 +1000 Subject: [PATCH 2/2] Update acceptance tests and skip disabled ones --- test/acceptance/acceptance_test.go | 5 + test/acceptance/acme_test.go | 4 +- test/acceptance/artifacts_test.go | 4 +- test/acceptance/auth_test.go | 24 ++-- test/acceptance/config_test.go | 6 +- test/acceptance/encodings_test.go | 4 +- test/acceptance/helpers_test.go | 2 +- test/acceptance/proxyv2_test.go | 2 +- test/acceptance/redirects_test.go | 6 +- test/acceptance/serving_test.go | 142 ++++++-------------- test/acceptance/status_test.go | 6 +- test/acceptance/tls_test.go | 11 +- test/acceptance/unknown_http_method_test.go | 3 +- test/acceptance/zip_test.go | 6 +- 14 files changed, 89 insertions(+), 136 deletions(-) diff --git a/test/acceptance/acceptance_test.go b/test/acceptance/acceptance_test.go index becae7772..8c2c5fa40 100644 --- a/test/acceptance/acceptance_test.go +++ b/test/acceptance/acceptance_test.go @@ -12,6 +12,7 @@ import ( const ( objectStorageMockServer = "127.0.0.1:38001" + diskSourceTest = "disk-source-test" ) var ( @@ -78,6 +79,10 @@ func skipUnlessEnabled(t *testing.T, conditions ...string) { t.Log("Not supported with -daemon-inplace-chroot") t.SkipNow() } + case diskSourceTest: + // TODO: rework acceptance tests to use GitLab source instead + t.Log("Disk source is not supported anymore, please update test to use API-Based config") + t.SkipNow() default: t.Error("Unknown condition:", condition) t.FailNow() diff --git a/test/acceptance/acme_test.go b/test/acceptance/acme_test.go index eb8f2160e..55b96bb41 100644 --- a/test/acceptance/acme_test.go +++ b/test/acceptance/acme_test.go @@ -10,7 +10,7 @@ import ( ) func TestAcmeChallengesWhenItIsNotConfigured(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "") defer teardown() @@ -39,7 +39,7 @@ func TestAcmeChallengesWhenItIsNotConfigured(t *testing.T) { } func TestAcmeChallengesWhenItIsConfigured(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-gitlab-server=https://gitlab-acme.com") defer teardown() diff --git a/test/acceptance/artifacts_test.go b/test/acceptance/artifacts_test.go index b4dcdce47..31584a29e 100644 --- a/test/acceptance/artifacts_test.go +++ b/test/acceptance/artifacts_test.go @@ -15,7 +15,7 @@ import ( ) func TestArtifactProxyRequest(t *testing.T) { - skipUnlessEnabled(t, "not-inplace-chroot") + skipUnlessEnabled(t, "not-inplace-chroot", diskSourceTest) transport := (TestHTTPSClient.Transport).(*http.Transport) defer func(t time.Duration) { @@ -161,7 +161,7 @@ func TestArtifactProxyRequest(t *testing.T) { } func TestPrivateArtifactProxyRequest(t *testing.T) { - skipUnlessEnabled(t, "not-inplace-chroot") + skipUnlessEnabled(t, "not-inplace-chroot", diskSourceTest) setupTransport(t) diff --git a/test/acceptance/auth_test.go b/test/acceptance/auth_test.go index 980fe377e..6cea78f04 100644 --- a/test/acceptance/auth_test.go +++ b/test/acceptance/auth_test.go @@ -15,7 +15,7 @@ import ( ) func TestWhenAuthIsDisabledPrivateIsNotAccessible(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "") defer teardown() @@ -27,7 +27,7 @@ func TestWhenAuthIsDisabledPrivateIsNotAccessible(t *testing.T) { } func TestWhenAuthIsEnabledPrivateWillRedirectToAuthorize(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithAuth(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -59,7 +59,7 @@ func TestWhenAuthIsEnabledPrivateWillRedirectToAuthorize(t *testing.T) { } func TestWhenAuthDeniedWillCauseUnauthorized(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithAuth(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -71,7 +71,7 @@ func TestWhenAuthDeniedWillCauseUnauthorized(t *testing.T) { require.Equal(t, http.StatusUnauthorized, rsp.StatusCode) } func TestWhenLoginCallbackWithWrongStateShouldFail(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithAuth(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -90,7 +90,7 @@ func TestWhenLoginCallbackWithWrongStateShouldFail(t *testing.T) { } func TestWhenLoginCallbackWithUnencryptedCode(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithAuth(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -181,7 +181,7 @@ func sleepIfAuthorized(t *testing.T, authorization string, w http.ResponseWriter } func TestAccessControlUnderCustomDomain(t *testing.T) { - skipUnlessEnabled(t, "not-inplace-chroot") + skipUnlessEnabled(t, "not-inplace-chroot", diskSourceTest) testServer := makeGitLabPagesAccessStub(t) testServer.Start() @@ -263,7 +263,7 @@ func TestAccessControlUnderCustomDomain(t *testing.T) { } func TestCustomErrorPageWithAuth(t *testing.T) { - skipUnlessEnabled(t, "not-inplace-chroot") + skipUnlessEnabled(t, "not-inplace-chroot", diskSourceTest) testServer := makeGitLabPagesAccessStub(t) testServer.Start() defer testServer.Close() @@ -372,7 +372,7 @@ func TestCustomErrorPageWithAuth(t *testing.T) { } func TestAccessControlUnderCustomDomainWithHTTPSProxy(t *testing.T) { - skipUnlessEnabled(t, "not-inplace-chroot") + skipUnlessEnabled(t, "not-inplace-chroot", diskSourceTest) testServer := makeGitLabPagesAccessStub(t) testServer.Start() @@ -440,7 +440,7 @@ func TestAccessControlUnderCustomDomainWithHTTPSProxy(t *testing.T) { } func TestAccessControlGroupDomain404RedirectsAuth(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithAuth(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -455,7 +455,7 @@ func TestAccessControlGroupDomain404RedirectsAuth(t *testing.T) { require.Equal(t, "/auth", url.Path) } func TestAccessControlProject404DoesNotRedirect(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithAuth(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -476,7 +476,7 @@ func setupTransport(t *testing.T) { type runPagesFunc func(t *testing.T, pagesPath string, listeners []ListenSpec, promPort string, sslCertFile string, authServer string) func() func testAccessControl(t *testing.T, runPages runPagesFunc) { - skipUnlessEnabled(t, "not-inplace-chroot") + skipUnlessEnabled(t, "not-inplace-chroot", diskSourceTest) setupTransport(t) @@ -650,7 +650,7 @@ func TestAccessControlWithSSLCertDir(t *testing.T) { // Read the issue description if any changes to internal/auth/ break this test. // Related to https://tools.ietf.org/html/rfc6749#section-10.6. func TestHijackedCode(t *testing.T) { - skipUnlessEnabled(t, "not-inplace-chroot") + skipUnlessEnabled(t, "not-inplace-chroot", diskSourceTest) testServer := makeGitLabPagesAccessStub(t) testServer.Start() diff --git a/test/acceptance/config_test.go b/test/acceptance/config_test.go index 93e9aa22e..33fe18e1d 100644 --- a/test/acceptance/config_test.go +++ b/test/acceptance/config_test.go @@ -11,7 +11,7 @@ import ( ) func TestEnvironmentVariablesConfig(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) os.Setenv("LISTEN_HTTP", net.JoinHostPort(httpListener.Host, httpListener.Port)) defer func() { os.Unsetenv("LISTEN_HTTP") }() @@ -27,7 +27,7 @@ func TestEnvironmentVariablesConfig(t *testing.T) { } func TestMixedConfigSources(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) os.Setenv("LISTEN_HTTP", net.JoinHostPort(httpListener.Host, httpListener.Port)) defer func() { os.Unsetenv("LISTEN_HTTP") }() @@ -45,7 +45,7 @@ func TestMixedConfigSources(t *testing.T) { } func TestMultiFlagEnvironmentVariables(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) listenSpecs := []ListenSpec{{"http", "127.0.0.1", "37001"}, {"http", "127.0.0.1", "37002"}} envVarValue := fmt.Sprintf("%s,%s", net.JoinHostPort("127.0.0.1", "37001"), net.JoinHostPort("127.0.0.1", "37002")) diff --git a/test/acceptance/encodings_test.go b/test/acceptance/encodings_test.go index e58afd4f7..65e4ccae5 100644 --- a/test/acceptance/encodings_test.go +++ b/test/acceptance/encodings_test.go @@ -9,7 +9,7 @@ import ( ) func TestMIMETypes(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithoutWait(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -40,7 +40,7 @@ func TestMIMETypes(t *testing.T) { } func TestCompressedEncoding(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) tests := []struct { name string diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index ba5790a34..7aaf943d6 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -236,7 +236,7 @@ func RunPagesProcessWithStubGitLabServer(t *testing.T, wait bool, pagesBinary st source := NewGitlabDomainsSourceStub(t, &stubOpts{}) gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t) - pagesArgs := append([]string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab"}, extraArgs...) + pagesArgs := append([]string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey}, extraArgs...) logBuf, cleanup := runPagesProcess(t, wait, pagesBinary, listeners, promPort, envs, pagesArgs...) diff --git a/test/acceptance/proxyv2_test.go b/test/acceptance/proxyv2_test.go index 670f711c1..64aa44e5e 100644 --- a/test/acceptance/proxyv2_test.go +++ b/test/acceptance/proxyv2_test.go @@ -10,7 +10,7 @@ import ( ) func TestProxyv2(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) logBuf, teardown := RunPagesProcessWithOutput(t, *pagesBinary, supportedListeners(), "") defer teardown() diff --git a/test/acceptance/redirects_test.go b/test/acceptance/redirects_test.go index ed6ce1531..44f7a2139 100644 --- a/test/acceptance/redirects_test.go +++ b/test/acceptance/redirects_test.go @@ -10,7 +10,7 @@ import ( ) func TestDisabledRedirects(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, supportedListeners(), "", []string{"FF_ENABLE_REDIRECTS=false"}) defer teardown() @@ -31,7 +31,7 @@ func TestDisabledRedirects(t *testing.T) { } func TestRedirectStatusPage(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -48,7 +48,7 @@ func TestRedirectStatusPage(t *testing.T) { } func TestRedirect(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() diff --git a/test/acceptance/serving_test.go b/test/acceptance/serving_test.go index 45203c92e..096a2ba1a 100644 --- a/test/acceptance/serving_test.go +++ b/test/acceptance/serving_test.go @@ -15,7 +15,7 @@ import ( ) func TestUnknownHostReturnsNotFound(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -29,7 +29,7 @@ func TestUnknownHostReturnsNotFound(t *testing.T) { } func TestUnknownProjectReturnsNotFound(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -40,7 +40,7 @@ func TestUnknownProjectReturnsNotFound(t *testing.T) { } func TestGroupDomainReturns200(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -51,7 +51,7 @@ func TestGroupDomainReturns200(t *testing.T) { } func TestKnownHostReturns200(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -101,7 +101,7 @@ func TestKnownHostReturns200(t *testing.T) { } func TestNestedSubgroups(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) maxNestedSubgroup := 21 @@ -147,7 +147,7 @@ func TestNestedSubgroups(t *testing.T) { } func TestCustom404(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -209,7 +209,8 @@ func TestCustom404(t *testing.T) { } func TestCORSWhenDisabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-disable-cross-origin-requests") defer teardown() @@ -225,7 +226,7 @@ func TestCORSWhenDisabled(t *testing.T) { } func TestCORSAllowsGET(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -241,7 +242,7 @@ func TestCORSAllowsGET(t *testing.T) { } func TestCORSForbidsPOST(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -256,7 +257,7 @@ func TestCORSForbidsPOST(t *testing.T) { } func TestCustomHeaders(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-header", "X-Test1:Testing1", "-header", "X-Test2:Testing2") defer teardown() @@ -271,7 +272,7 @@ func TestCustomHeaders(t *testing.T) { } func TestKnownHostWithPortReturns200(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -286,7 +287,8 @@ func TestKnownHostWithPortReturns200(t *testing.T) { } func TestHttpToHttpsRedirectDisabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -302,7 +304,8 @@ func TestHttpToHttpsRedirectDisabled(t *testing.T) { } func TestHttpToHttpsRedirectEnabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-redirect-http=true") defer teardown() @@ -320,7 +323,8 @@ func TestHttpToHttpsRedirectEnabled(t *testing.T) { } func TestHttpsOnlyGroupEnabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -331,7 +335,8 @@ func TestHttpsOnlyGroupEnabled(t *testing.T) { } func TestHttpsOnlyGroupDisabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -342,7 +347,8 @@ func TestHttpsOnlyGroupDisabled(t *testing.T) { } func TestHttpsOnlyProjectEnabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -353,7 +359,8 @@ func TestHttpsOnlyProjectEnabled(t *testing.T) { } func TestHttpsOnlyProjectDisabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -364,7 +371,8 @@ func TestHttpsOnlyProjectDisabled(t *testing.T) { } func TestHttpsOnlyDomainDisabled(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -378,10 +386,9 @@ func TestDomainsSource(t *testing.T) { skipUnlessEnabled(t) type args struct { - configSource string - domain string - urlSuffix string - readyCount int + domain string + urlSuffix string + readyCount int } type want struct { statusCode int @@ -394,11 +401,10 @@ func TestDomainsSource(t *testing.T) { want want }{ { - name: "gitlab_source_domain_exists", + name: "domain_exists", args: args{ - configSource: "gitlab", - domain: "new-source-test.gitlab.io", - urlSuffix: "/my/pages/project/", + domain: "new-source-test.gitlab.io", + urlSuffix: "/my/pages/project/", }, want: want{ statusCode: http.StatusOK, @@ -407,77 +413,12 @@ func TestDomainsSource(t *testing.T) { }, }, { - name: "gitlab_source_domain_does_not_exist", - args: args{ - configSource: "gitlab", - domain: "non-existent-domain.gitlab.io", - }, - want: want{ - statusCode: http.StatusNotFound, - apiCalled: true, - }, - }, - { - name: "disk_source_domain_exists", + name: "domain_does_not_exist", args: args{ - configSource: "disk", - // test.domain.com sourced from disk configuration - domain: "test.domain.com", - urlSuffix: "/", - }, - want: want{ - statusCode: http.StatusOK, - content: "main-dir\n", - apiCalled: false, - }, - }, - { - name: "disk_source_domain_does_not_exist", - args: args{ - configSource: "disk", - domain: "non-existent-domain.gitlab.io", + domain: "non-existent-domain.gitlab.io", }, want: want{ statusCode: http.StatusNotFound, - apiCalled: false, - }, - }, - { - name: "disk_source_domain_should_not_exist_under_hashed_dir", - args: args{ - configSource: "disk", - domain: "hashed.com", - }, - want: want{ - statusCode: http.StatusNotFound, - apiCalled: false, - }, - }, - { - name: "auto_source_gitlab_is_not_ready", - args: args{ - configSource: "auto", - domain: "test.domain.com", - urlSuffix: "/", - readyCount: 100, // big number to ensure the API is in bad state for a while - }, - want: want{ - statusCode: http.StatusOK, - content: "main-dir\n", - apiCalled: false, - }, - }, - { - name: "auto_source_gitlab_is_ready", - args: args{ - configSource: "auto", - domain: "new-source-test.gitlab.io", - urlSuffix: "/my/pages/project/", - readyCount: 0, - }, - want: want{ - statusCode: http.StatusOK, - content: "New Pages GitLab Source TEST OK\n", apiCalled: true, }, }, @@ -495,7 +436,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} + pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey} teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, []ListenSpec{httpListener}, "", []string{}, pagesArgs...) defer teardown() @@ -520,6 +461,8 @@ func TestDomainsSource(t *testing.T) { // 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) { + skipUnlessEnabled(t, diskSourceTest) + opts := &stubOpts{ // edge case https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535 pagesStatusResponse: http.StatusUnauthorized, @@ -529,7 +472,7 @@ func TestGitLabSourceBecomesUnauthorized(t *testing.T) { gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t) - pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "auto"} + pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey} teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, []ListenSpec{httpListener}, "", []string{}, pagesArgs...) defer teardown() @@ -556,7 +499,7 @@ func TestGitLabSourceBecomesUnauthorized(t *testing.T) { } func TestKnownHostInReverseProxySetupReturns200(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) specs := []ListenSpec{ proxyListener, @@ -619,7 +562,7 @@ func TestDomainResolverError(t *testing.T) { defer source.Close() gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t) - pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab"} + pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey} if test.timeout != 0 { pagesArgs = append(pagesArgs, "-gitlab-client-http-timeout", test.timeout.String(), "-gitlab-retrieval-timeout", "200ms", "-gitlab-retrieval-interval", "200ms", "-gitlab-retrieval-retries", "1") @@ -668,7 +611,8 @@ func doCrossOriginRequest(t *testing.T, spec ListenSpec, method, reqMethod, url } func TestQueryStringPersistedInSlashRewrite(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -687,7 +631,7 @@ func TestQueryStringPersistedInSlashRewrite(t *testing.T) { } func TestServerRepliesWithHeaders(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) tests := map[string]struct { flags []string diff --git a/test/acceptance/status_test.go b/test/acceptance/status_test.go index 84bdf4766..cb8e0a03e 100644 --- a/test/acceptance/status_test.go +++ b/test/acceptance/status_test.go @@ -9,7 +9,7 @@ import ( ) func TestStatusPage(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-pages-status=/@statuscheck") defer teardown() @@ -20,7 +20,7 @@ func TestStatusPage(t *testing.T) { } func TestStatusNotYetReady(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithoutWait(t, *pagesBinary, supportedListeners(), "", "-pages-status=/@statuscheck", "-pages-root=../../shared/invalid-pages") defer teardown() @@ -32,7 +32,7 @@ func TestStatusNotYetReady(t *testing.T) { } func TestPageNotAvailableIfNotLoaded(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) teardown := RunPagesProcessWithoutWait(t, *pagesBinary, supportedListeners(), "", "-pages-root=../../shared/invalid-pages") defer teardown() waitForRoundtrips(t, supportedListeners(), 5*time.Second) diff --git a/test/acceptance/tls_test.go b/test/acceptance/tls_test.go index 8e887a837..a24a255e0 100644 --- a/test/acceptance/tls_test.go +++ b/test/acceptance/tls_test.go @@ -8,7 +8,8 @@ import ( ) func TestAcceptsSupportedCiphers(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -45,7 +46,8 @@ func tlsConfigWithInsecureCiphersOnly() *tls.Config { } func TestRejectsUnsupportedCiphers(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -63,7 +65,8 @@ func TestRejectsUnsupportedCiphers(t *testing.T) { } func TestEnableInsecureCiphers(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-insecure-ciphers") defer teardown() @@ -80,7 +83,7 @@ func TestEnableInsecureCiphers(t *testing.T) { } func TestTLSVersions(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) tests := map[string]struct { tlsMin string diff --git a/test/acceptance/unknown_http_method_test.go b/test/acceptance/unknown_http_method_test.go index 3aabf800e..422684ae9 100644 --- a/test/acceptance/unknown_http_method_test.go +++ b/test/acceptance/unknown_http_method_test.go @@ -8,7 +8,8 @@ import ( ) func TestUnknownHTTPMethod(t *testing.T) { - skipUnlessEnabled(t) + skipUnlessEnabled(t, diskSourceTest) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() diff --git a/test/acceptance/zip_test.go b/test/acceptance/zip_test.go index 212435e84..c49347c16 100644 --- a/test/acceptance/zip_test.go +++ b/test/acceptance/zip_test.go @@ -24,7 +24,7 @@ func TestZipServing(t *testing.T) { gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t) - pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab"} + pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey} teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, supportedListeners(), "", []string{}, pagesArgs...) defer teardown() @@ -124,7 +124,7 @@ func TestZipServingFromDisk(t *testing.T) { gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t) - pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab", "-pages-root", wd} + pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-pages-root", wd} teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, supportedListeners(), "", []string{}, pagesArgs...) defer teardown() @@ -214,7 +214,7 @@ func TestZipServingConfigShortTimeout(t *testing.T) { gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t) - pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab", + pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-zip-open-timeout=1ns"} // <- test purpose teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, supportedListeners(), "", []string{}, pagesArgs...) -- GitLab