Move zip flags to config

5 files changed, 46 insertions, 40 deletions

diff --git a/internal/config/config.go b/internal/config/config.go
index 3e0f1965..b5fd834b 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -2,24 +2,27 @@ package config
 
 import (
 	"time"
+
+	"github.com/namsral/flag"
 )
 
-// TODO: refactor config flags in main.go and find a better way to handle all settings
+// Default configuration that can be accessed by different packages
+var Default *Config
 
-type Config struct {
-	zip *ZipServing
+// TODO: move all flags to this package, including flag.Parse()
+func init() {
+	Default = &Config{
+		Zip: &ZipServing{},
+	}
+
+	flag.DurationVar(&Default.Zip.ExpirationInterval, "zip-cache-expiration", 60*time.Second, "Zip serving archive cache expiration interval")
+	flag.DurationVar(&Default.Zip.CleanupInterval, "zip-cache-cleanup", 30*time.Second, "Zip serving archive cache cleanup interval")
+	flag.DurationVar(&Default.Zip.RefreshInterval, "zip-cache-refresh", 30*time.Second, "Zip serving archive cache refresh interval")
+	flag.DurationVar(&Default.Zip.OpenTimeout, "zip-open-timeout", 30*time.Second, "Zip archive open timeout")
 }
 
-// DefaultConfig struct that can be accessed by different packages to share
-// configuration parameters
-var DefaultConfig = &Config{
-	// TODO: think of a way to not repeat these here and in main.go
-	zip: &ZipServing{
-		ExpirationInterval: time.Minute,
-		CleanupInterval:    time.Minute / 2,
-		RefreshInterval:    time.Minute / 2,
-		OpenTimeout:        time.Minute / 2,
-	},
+type Config struct {
+	Zip *ZipServing
 }
 
 // ZipServing stores all configuration values to be used by the zip VFS opening and
@@ -30,12 +33,3 @@ type ZipServing struct {
 	RefreshInterval    time.Duration
 	OpenTimeout        time.Duration
 }
-
-// SetZip config to the global config
-func (c *Config) SetZip(zip *ZipServing) {
-	c.zip = zip
-}
-
-func (c *Config) GetZip() *ZipServing {
-	return c.zip
-}
diff --git a/internal/serving/disk/zip/serving.go b/internal/serving/disk/zip/serving.go
index 53e66f13..b552050c 100644
--- a/internal/serving/disk/zip/serving.go
+++ b/internal/serving/disk/zip/serving.go
@@ -14,7 +14,7 @@ var instance serving.Serving
 // from a zip archives opened from a URL, most likely stored in object storage
 func Instance() serving.Serving {
 	if instance == nil {
-		instance = disk.New(vfs.Instrumented(zip.New(config.DefaultConfig.GetZip())))
+		instance = disk.New(vfs.Instrumented(zip.New(config.Default.Zip)))
 	}
 
 	return instance
diff --git a/internal/vfs/zip/archive_test.go b/internal/vfs/zip/archive_test.go
index 08aa22ff..3273edad 100644
--- a/internal/vfs/zip/archive_test.go
+++ b/internal/vfs/zip/archive_test.go
@@ -17,9 +17,10 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
 )
 
-var chdirSet = false
-
-var zipCfg = config.DefaultConfig.GetZip()
+var (
+	chdirSet = false
+	zipCfg   = config.Default.Zip
+)
 
 func TestOpen(t *testing.T) {
 	zip, cleanup := openZipArchive(t, nil)
diff --git a/main.go b/main.go
index 1b5109fa..95eae4ce 100644
--- a/main.go
+++ b/main.go
@@ -16,7 +16,6 @@ import (
 
 	"gitlab.com/gitlab-org/labkit/errortracking"
 
-	cfg "gitlab.com/gitlab-org/gitlab-pages/internal/config"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/logging"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/tlsconfig"
@@ -80,12 +79,6 @@ var (
 
 	disableCrossOriginRequests = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
 
-	// zip serving settings
-	zipCacheExpiration = flag.Duration("zip-cache-expiration", 60*time.Second, "Zip serving archive cache expiration interval (default: 60s)")
-	zipCacheCleanup    = flag.Duration("zip-cache-cleanup", 30*time.Second, "Zip serving archive cache cleanup interval (default: 30s)")
-	zipCacheRefresh    = flag.Duration("zip-cache-refresh", 30*time.Second, "Zip serving archive cache refresh interval (default: 30s)")
-	zipOpenTimeout     = flag.Duration("zip-open-timeout", 30*time.Second, "Zip archive open timeout (default: 30s)")
-
 	// See init()
 	listenHTTP         MultiStringFlag
 	listenHTTPS        MultiStringFlag
@@ -220,13 +213,6 @@ func configFromFlags() appConfig {
 
 	checkAuthenticationConfig(config)
 
-	cfg.DefaultConfig.SetZip(&cfg.ZipServing{
-		ExpirationInterval: *zipCacheExpiration,
-		CleanupInterval:    *zipCacheCleanup,
-		RefreshInterval:    *zipCacheRefresh,
-		OpenTimeout:        *zipOpenTimeout,
-	})
-
 	return config
 }
 
diff --git a/test/acceptance/zip_test.go b/test/acceptance/zip_test.go
index ea703ebe..3bfa21bf 100644
--- a/test/acceptance/zip_test.go
+++ b/test/acceptance/zip_test.go
@@ -104,6 +104,31 @@ func TestZipServing(t *testing.T) {
 	}
 }
 
+func TestZipServingConfigShortTimeout(t *testing.T) {
+	skipUnlessEnabled(t)
+
+	var apiCalled bool
+	source := NewGitlabDomainsSourceStub(t, &apiCalled)
+	defer source.Close()
+
+	gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t)
+
+	pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab",
+		"-zip-open-timeout=1ns"} // <- test purpose
+
+	teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, listeners, "", []string{}, pagesArgs...)
+	defer teardown()
+
+	_, cleanup := newZipFileServerURL(t, "../../shared/pages/group/zip.gitlab.io/public.zip")
+	defer cleanup()
+
+	response, err := GetPageFromListener(t, httpListener, "zip.gitlab.io", "/")
+	require.NoError(t, err)
+	defer response.Body.Close()
+
+	require.Equal(t, http.StatusInternalServerError, response.StatusCode, "should fail to serve")
+}
+
 func newZipFileServerURL(t *testing.T, zipFilePath string) (string, func()) {
 	t.Helper()