Merge branch '520-add-flags-to-pages' into 'master'

Add GitLab API cache config flags

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

7 files changed, 59 insertions, 30 deletions

diff --git a/internal/config/config.go b/internal/config/config.go
index 18ec7d32..0842a195 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -209,20 +209,20 @@ func (config Config) InternalGitLabServerURL() string {
 	return config.GitLab.InternalServer
 }
 
-// GitlabClientSecret returns GitLab server access token.
-func (config Config) GitlabAPISecret() []byte {
+// GitlabAPISecret returns GitLab server access token.
+func (config *Config) GitlabAPISecret() []byte {
 	return config.GitLab.APISecretKey
 }
 
-func (config Config) GitlabClientConnectionTimeout() time.Duration {
+func (config *Config) GitlabClientConnectionTimeout() time.Duration {
 	return config.GitLab.ClientHTTPTimeout
 }
 
-func (config Config) GitlabJWTTokenExpiry() time.Duration {
+func (config *Config) GitlabJWTTokenExpiry() time.Duration {
 	return config.GitLab.JWTTokenExpiration
 }
 
-func (config Config) DomainConfigSource() string {
+func (config *Config) DomainConfigSource() string {
 	if config.General.UseLegacyStorage {
 		return "disk"
 	}
@@ -230,9 +230,8 @@ func (config Config) DomainConfigSource() string {
 	return config.General.DomainConfigurationSource
 }
 
-func (config Config) Cache() *Cache {
-	// TODO: return values from flags https://gitlab.com/gitlab-org/gitlab-pages/-/issues/520#implementation
-	return nil
+func (config *Config) Cache() *Cache {
+	return &config.GitLab.Cache
 }
 
 func loadConfig() *Config {
@@ -256,7 +255,14 @@ func loadConfig() *Config {
 		GitLab: GitLab{
 			ClientHTTPTimeout:  *gitlabClientHTTPTimeout,
 			JWTTokenExpiration: *gitlabClientJWTExpiry,
-			// TODO: assign values from flags https://gitlab.com/gitlab-org/gitlab-pages/-/issues/520#implementation
+			Cache: Cache{
+				CacheExpiry:          *gitlabCacheExpiry,
+				CacheCleanupInterval: *gitlabCacheCleanup,
+				EntryRefreshTimeout:  *gitlabCacheRefresh,
+				RetrievalTimeout:     *gitlabRetrievalTimeout,
+				MaxRetrievalInterval: *gitlabRetrievalInterval,
+				MaxRetrievalRetries:  *gitlabRetrievalRetries,
+			},
 		},
 		ArtifactsServer: ArtifactsServer{
 			TimeoutSeconds: *artifactsServerTimeout,
diff --git a/internal/config/flags.go b/internal/config/flags.go
index 30fa58ec..84eef0f6 100644
--- a/internal/config/flags.go
+++ b/internal/config/flags.go
@@ -39,7 +39,14 @@ var (
 	gitLabAPISecretKey      = flag.String("api-secret-key", "", "File with secret key used to authenticate with the GitLab API")
 	gitlabClientHTTPTimeout = flag.Duration("gitlab-client-http-timeout", 10*time.Second, "GitLab API HTTP client connection timeout in seconds (default: 10s)")
 	gitlabClientJWTExpiry   = flag.Duration("gitlab-client-jwt-expiry", 30*time.Second, "JWT Token expiry time in seconds (default: 30s)")
-	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")
+	gitlabCacheExpiry       = flag.Duration("gitlab-cache-expiry", 10*time.Minute, "The maximum time a domain's configuration is stored in the cache")
+	gitlabCacheRefresh      = flag.Duration("gitlab-cache-refresh", time.Minute, "The interval at which a domain's configuration is set to be due to refresh")
+	gitlabCacheCleanup      = flag.Duration("gitlab-cache-cleanup", time.Minute, "The interval at which expired items are removed from the cache")
+	gitlabRetrievalTimeout  = flag.Duration("gitlab-retrieval-timeout", 30*time.Second, "The maximum time to wait for a response from the GitLab API per request")
+	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")
 	// TODO: remove this flag https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6009
 	useLegacyStorage = flag.Bool("use-legacy-storage", false, "Temporary flag that enables legacy serving from disk/NFS. API-Based configuration and object storage are preferred https://docs.gitlab.com/ee/administration/pages/ and will be the only available solution starting from 14.4")
 
diff --git a/internal/source/domains_test.go b/internal/source/domains_test.go
index 5da5354c..db9c2c23 100644
--- a/internal/source/domains_test.go
+++ b/internal/source/domains_test.go
@@ -36,7 +36,14 @@ func (c sourceConfig) DomainConfigSource() string {
 	return c.domainSource
 }
 func (c sourceConfig) Cache() *config.Cache {
-	return nil
+	return &config.Cache{
+		CacheExpiry:          10 * time.Minute,
+		CacheCleanupInterval: time.Minute,
+		EntryRefreshTimeout:  60 * time.Second,
+		RetrievalTimeout:     30 * time.Second,
+		MaxRetrievalInterval: time.Second,
+		MaxRetrievalRetries:  3,
+	}
 }
 
 func TestNewDomains(t *testing.T) {
diff --git a/internal/source/gitlab/cache/cache.go b/internal/source/gitlab/cache/cache.go
index d9a49e84..77291033 100644
--- a/internal/source/gitlab/cache/cache.go
+++ b/internal/source/gitlab/cache/cache.go
@@ -2,22 +2,12 @@ package cache
 
 import (
 	"context"
-	"time"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/config"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
 
-var defaultCacheConfig = config.Cache{
-	CacheExpiry:          10 * time.Minute,
-	CacheCleanupInterval: time.Minute,
-	EntryRefreshTimeout:  60 * time.Second,
-	RetrievalTimeout:     30 * time.Second,
-	MaxRetrievalInterval: time.Second,
-	MaxRetrievalRetries:  3,
-}
-
 // Cache is a short and long caching mechanism for GitLab source
 type Cache struct {
 	client api.Client
@@ -26,10 +16,6 @@ type Cache struct {
 
 // NewCache creates a new instance of Cache.
 func NewCache(client api.Client, cc *config.Cache) *Cache {
-	if cc == nil {
-		cc = &defaultCacheConfig
-	}
-
 	return &Cache{
 		client: client,
 		store:  newMemStore(client, cc),
diff --git a/internal/source/gitlab/cache/cache_test.go b/internal/source/gitlab/cache/cache_test.go
index 1819a486..2d3ada76 100644
--- a/internal/source/gitlab/cache/cache_test.go
+++ b/internal/source/gitlab/cache/cache_test.go
@@ -14,6 +14,15 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
 )
 
+var testCacheConfig = config.Cache{
+	CacheExpiry:          time.Second,
+	CacheCleanupInterval: time.Second / 2,
+	EntryRefreshTimeout:  time.Second / 2,
+	RetrievalTimeout:     time.Second,
+	MaxRetrievalInterval: time.Second / 3,
+	MaxRetrievalRetries:  3,
+}
+
 type clientMock struct {
 	counter uint64
 	lookups chan uint64
@@ -52,6 +61,9 @@ func withTestCache(config resolverConfig, cacheConfig *config.Cache, block func(
 		lookups: make(chan uint64, 100),
 		failure: config.failure,
 	}
+	if cacheConfig == nil {
+		cacheConfig = &testCacheConfig
+	}
 
 	cache := NewCache(resolver, cacheConfig)
 
@@ -189,7 +201,7 @@ func TestResolve(t *testing.T) {
 	})
 
 	t.Run("when retrieval failed with an error", func(t *testing.T) {
-		cc := defaultCacheConfig
+		cc := testCacheConfig
 		cc.MaxRetrievalInterval = 0
 		err := errors.New("500 error")
 
@@ -202,7 +214,7 @@ func TestResolve(t *testing.T) {
 	})
 
 	t.Run("when retrieval failed because of an internal retriever context timeout", func(t *testing.T) {
-		cc := defaultCacheConfig
+		cc := testCacheConfig
 		cc.RetrievalTimeout = 0
 
 		withTestCache(resolverConfig{}, &cc, func(cache *Cache, resolver *clientMock) {
diff --git a/internal/source/gitlab/cache/entry_test.go b/internal/source/gitlab/cache/entry_test.go
index e7b7bceb..36aee522 100644
--- a/internal/source/gitlab/cache/entry_test.go
+++ b/internal/source/gitlab/cache/entry_test.go
@@ -49,7 +49,7 @@ func TestIsUpToDateAndNeedsRefresh(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			entry := newCacheEntry("my.gitlab.com", defaultCacheConfig.EntryRefreshTimeout, nil)
+			entry := newCacheEntry("my.gitlab.com", testCacheConfig.EntryRefreshTimeout, nil)
 			if tt.resolved {
 				entry.response = &api.Lookup{}
 			}
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index 2f74ae65..644ee1ec 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -19,6 +19,8 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client"
 )
 
+var errCacheNotConfigured = errors.New("cache not configured")
+
 // Gitlab source represent a new domains configuration source. We fetch all the
 // information about domains from GitLab instance.
 type Gitlab struct {
@@ -34,14 +36,23 @@ func New(config client.Config) (*Gitlab, error) {
 		return nil, err
 	}
 
+	cc := config.Cache()
+	if cc == nil {
+		return nil, errCacheNotConfigured
+	}
+
+	cachedClient := cache.NewCache(client, cc)
+	if err != nil {
+		return nil, err
+	}
+
 	g := &Gitlab{
-		client: cache.NewCache(client, config.Cache()),
+		client: cachedClient,
 		mu:     &sync.RWMutex{},
 	}
 
 	go g.poll(backoff.DefaultInitialInterval, maxPollingTime)
 
-	// using nil for cache config will use the default values specified in internal/source/gitlab/cache/cache.go#12
 	return g, nil
 }