Encapsulate groups config in the source package

12 files changed, 92 insertions, 63 deletions

diff --git a/app.go b/app.go
index c14d7bc2..7b3a191c 100644
--- a/app.go
+++ b/app.go
@@ -7,9 +7,7 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"strings"
 	"sync"
-	"time"
 
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/cors"
@@ -28,7 +26,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/logging"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/netutil"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
-	"gitlab.com/gitlab-org/gitlab-pages/internal/source/dirs"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/source"
 )
 
 const (
@@ -48,8 +46,7 @@ var (
 
 type theApp struct {
 	appConfig
-	dm             dirs.Map
-	lock           sync.RWMutex
+	domains        *source.Domains
 	Artifact       *artifact.Artifact
 	Auth           *auth.Auth
 	AcmeMiddleware *acme.Middleware
@@ -57,15 +54,7 @@ type theApp struct {
 }
 
 func (a *theApp) isReady() bool {
-	return a.dm != nil
-}
-
-func (a *theApp) domain(host string) *domain.Domain {
-	host = strings.ToLower(host)
-	a.lock.RLock()
-	defer a.lock.RUnlock()
-	domain, _ := a.dm[host]
-	return domain
+	return a.domains.Ready()
 }
 
 func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
@@ -73,7 +62,7 @@ func (a *theApp) ServeTLS(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
 		return nil, nil
 	}
 
-	if domain := a.domain(ch.ServerName); domain != nil {
+	if domain := a.domains.GetDomain(ch.ServerName); domain != nil {
 		tls, _ := domain.EnsureCertificate()
 		return tls, nil
 	}
@@ -107,6 +96,10 @@ func (a *theApp) getHostAndDomain(r *http.Request) (host string, domain *domain.
 	return host, a.domain(host)
 }
 
+func (a *theApp) domain(host string) *domain.Domain {
+	return a.domains.GetDomain(host)
+}
+
 func (a *theApp) checkAuthenticationIfNotExists(domain *domain.Domain, w http.ResponseWriter, r *http.Request) bool {
 	if domain == nil || !domain.HasProject(r) {
 
@@ -206,7 +199,7 @@ func (a *theApp) acmeMiddleware(handler http.Handler) http.Handler {
 // authMiddleware handles authentication requests
 func (a *theApp) authMiddleware(handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if a.Auth.TryAuthenticate(w, r, a.dm, &a.lock) {
+		if a.Auth.TryAuthenticate(w, r, a.domains) {
 			return
 		}
 
@@ -323,12 +316,6 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) {
 	return handler, nil
 }
 
-func (a *theApp) UpdateDomains(dm dirs.Map) {
-	a.lock.Lock()
-	defer a.lock.Unlock()
-	a.dm = dm
-}
-
 func (a *theApp) Run() {
 	var wg sync.WaitGroup
 
@@ -367,7 +354,7 @@ func (a *theApp) Run() {
 	a.listenAdminUnix(&wg)
 	a.listenAdminHTTPS(&wg)
 
-	go dirs.Watch(a.Domain, a.UpdateDomains, time.Second)
+	a.domains.Watch(a.Domain)
 
 	wg.Wait()
 }
@@ -474,7 +461,8 @@ func (a *theApp) listenAdminHTTPS(wg *sync.WaitGroup) {
 }
 
 func runApp(config appConfig) {
-	a := theApp{appConfig: config}
+	a := theApp{appConfig: config, domains: new(source.Domains)}
+
 	err := logging.ConfigureLogging(a.LogFormat, a.LogVerbose)
 	if err != nil {
 		log.WithError(err).Fatal("Failed to initialize logging")
diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index 154d86da..95a26250 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -11,7 +11,6 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/gorilla/securecookie"
@@ -22,7 +21,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httptransport"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
-	"gitlab.com/gitlab-org/gitlab-pages/internal/source/dirs"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/source"
 
 	"golang.org/x/crypto/hkdf"
 )
@@ -108,7 +107,7 @@ func (a *Auth) checkSession(w http.ResponseWriter, r *http.Request) (*sessions.S
 }
 
 // TryAuthenticate tries to authenticate user and fetch access token if request is a callback to auth
-func (a *Auth) TryAuthenticate(w http.ResponseWriter, r *http.Request, dm dirs.Map, lock *sync.RWMutex) bool {
+func (a *Auth) TryAuthenticate(w http.ResponseWriter, r *http.Request, domains *source.Domains) bool {
 
 	if a == nil {
 		return false
@@ -126,7 +125,7 @@ func (a *Auth) TryAuthenticate(w http.ResponseWriter, r *http.Request, dm dirs.M
 
 	logRequest(r).Info("Receive OAuth authentication callback")
 
-	if a.handleProxyingAuth(session, w, r, dm, lock) {
+	if a.handleProxyingAuth(session, w, r, domains) {
 		return true
 	}
 
@@ -200,16 +199,17 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res
 	http.Redirect(w, r, redirectURI, 302)
 }
 
-func (a *Auth) domainAllowed(domain string, dm dirs.Map, lock *sync.RWMutex) bool {
-	lock.RLock()
-	defer lock.RUnlock()
+func (a *Auth) domainAllowed(domain string, domains *source.Domains) bool {
+	domainConfigured := (domain == a.pagesDomain) || strings.HasSuffix("."+domain, a.pagesDomain)
 
-	domain = strings.ToLower(domain)
-	_, present := dm[domain]
-	return domain == a.pagesDomain || strings.HasSuffix("."+domain, a.pagesDomain) || present
+	if domainConfigured {
+		return true
+	}
+
+	return domains.HasDomain(domain)
 }
 
-func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWriter, r *http.Request, dm dirs.Map, lock *sync.RWMutex) bool {
+func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWriter, r *http.Request, domains *source.Domains) bool {
 	// If request is for authenticating via custom domain
 	if shouldProxyAuth(r) {
 		domain := r.URL.Query().Get("domain")
@@ -228,7 +228,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit
 			host = proxyurl.Host
 		}
 
-		if !a.domainAllowed(host, dm, lock) {
+		if !a.domainAllowed(host, domains) {
 			logRequest(r).WithField("domain", host).Warn("Domain is not configured")
 			httperrors.Serve401(w)
 			return true
diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go
index 8102a5d1..e8ff5e94 100644
--- a/internal/auth/auth_test.go
+++ b/internal/auth/auth_test.go
@@ -5,14 +5,13 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"sync"
 	"testing"
 
 	"github.com/gorilla/sessions"
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
-	"gitlab.com/gitlab-org/gitlab-pages/internal/source/dirs"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/source"
 )
 
 func createAuth(t *testing.T) *Auth {
@@ -55,7 +54,7 @@ func TestTryAuthenticate(t *testing.T) {
 	require.NoError(t, err)
 	r := request.WithHTTPSFlag(&http.Request{URL: reqURL}, true)
 
-	require.Equal(t, false, auth.TryAuthenticate(result, r, make(dirs.Map), &sync.RWMutex{}))
+	require.Equal(t, false, auth.TryAuthenticate(result, r, new(source.Domains)))
 }
 
 func TestTryAuthenticateWithError(t *testing.T) {
@@ -66,7 +65,7 @@ func TestTryAuthenticateWithError(t *testing.T) {
 	require.NoError(t, err)
 	r := request.WithHTTPSFlag(&http.Request{URL: reqURL}, true)
 
-	require.Equal(t, true, auth.TryAuthenticate(result, r, make(dirs.Map), &sync.RWMutex{}))
+	require.Equal(t, true, auth.TryAuthenticate(result, r, new(source.Domains)))
 	require.Equal(t, 401, result.Code)
 }
 
@@ -83,7 +82,7 @@ func TestTryAuthenticateWithCodeButInvalidState(t *testing.T) {
 	session.Values["state"] = "state"
 	session.Save(r, result)
 
-	require.Equal(t, true, auth.TryAuthenticate(result, r, make(dirs.Map), &sync.RWMutex{}))
+	require.Equal(t, true, auth.TryAuthenticate(result, r, new(source.Domains)))
 	require.Equal(t, 401, result.Code)
 }
 
@@ -123,7 +122,7 @@ func testTryAuthenticateWithCodeAndState(t *testing.T, https bool) {
 	})
 
 	result := httptest.NewRecorder()
-	require.Equal(t, true, auth.TryAuthenticate(result, r, make(dirs.Map), &sync.RWMutex{}))
+	require.Equal(t, true, auth.TryAuthenticate(result, r, new(source.Domains)))
 	require.Equal(t, 302, result.Code)
 	require.Equal(t, "https://pages.gitlab-example.com/project/", result.Header().Get("Location"))
 	require.Equal(t, 600, result.Result().Cookies()[0].MaxAge)
diff --git a/internal/source/domains.go b/internal/source/domains.go
new file mode 100644
index 00000000..5e7f113c
--- /dev/null
+++ b/internal/source/domains.go
@@ -0,0 +1,56 @@
+package source
+
+import (
+	"strings"
+	"sync"
+	"time"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/source/groups"
+)
+
+// Domains struct represents a map of all domains supported by pages. It is
+// currently reading them from disk.
+type Domains struct {
+	dm   groups.Map
+	lock sync.RWMutex
+}
+
+// GetDomain returns a domain from the domains map
+func (d *Domains) GetDomain(host string) *domain.Domain {
+	host = strings.ToLower(host)
+	d.lock.RLock()
+	defer d.lock.RUnlock()
+	domain, _ := d.dm[host]
+
+	return domain
+}
+
+// HasDomain checks for presence of a domain in the domains map
+func (d *Domains) HasDomain(host string) bool {
+	d.lock.RLock()
+	defer d.lock.RUnlock()
+
+	host = strings.ToLower(host)
+	_, isPresent := d.dm[host]
+
+	return isPresent
+}
+
+// Ready checks if the domains source is ready for work
+func (d *Domains) Ready() bool {
+	return d.dm != nil
+}
+
+// Watch starts the domain source, in this case it is reading domains from
+// groups on disk concurrently
+func (d *Domains) Watch(rootDomain string) {
+	go groups.Watch(rootDomain, d.updateDomains, time.Second)
+}
+
+func (d *Domains) updateDomains(dm groups.Map) {
+	d.lock.Lock()
+	defer d.lock.Unlock()
+
+	d.dm = dm
+}
diff --git a/internal/source/dirs/config.go b/internal/source/groups/config.go
index b26da8a6..88947259 100644
--- a/internal/source/dirs/config.go
+++ b/internal/source/groups/config.go
@@ -1,4 +1,4 @@
-package dirs
+package groups
 
 import (
 	"encoding/json"
diff --git a/internal/source/dirs/config_test.go b/internal/source/groups/config_test.go
index d2bef10c..ed563b87 100644
--- a/internal/source/dirs/config_test.go
+++ b/internal/source/groups/config_test.go
@@ -1,4 +1,4 @@
-package dirs
+package groups
 
 import (
 	"io/ioutil"
diff --git a/internal/source/dirs/group.go b/internal/source/groups/group.go
index a711221c..e5534cf6 100644
--- a/internal/source/dirs/group.go
+++ b/internal/source/groups/group.go
@@ -1,4 +1,4 @@
-package dirs
+package groups
 
 import (
 	"errors"
diff --git a/internal/source/dirs/group_domain_test.go b/internal/source/groups/group_domain_test.go
index 3c20549b..38225cf5 100644
--- a/internal/source/dirs/group_domain_test.go
+++ b/internal/source/groups/group_domain_test.go
@@ -1,4 +1,4 @@
-package dirs
+package groups
 
 import (
 	"compress/gzip"
diff --git a/internal/source/dirs/group_test.go b/internal/source/groups/group_test.go
index 8633e7e4..a8070c87 100644
--- a/internal/source/dirs/group_test.go
+++ b/internal/source/groups/group_test.go
@@ -1,4 +1,4 @@
-package dirs
+package groups
 
 import (
 	"strings"
diff --git a/internal/source/dirs/map.go b/internal/source/groups/map.go
index 849f9e14..c72b2fe7 100644
--- a/internal/source/dirs/map.go
+++ b/internal/source/groups/map.go
@@ -1,4 +1,4 @@
-package dirs
+package groups
 
 import (
 	"bytes"
diff --git a/internal/source/dirs/map_test.go b/internal/source/groups/map_test.go
index ac9b14a9..4f333c9a 100644
--- a/internal/source/dirs/map_test.go
+++ b/internal/source/groups/map_test.go
@@ -1,4 +1,4 @@
-package dirs
+package groups
 
 import (
 	"crypto/rand"
diff --git a/internal/source/source.go b/internal/source/source.go
deleted file mode 100644
index 23dc72fe..00000000
--- a/internal/source/source.go
+++ /dev/null
@@ -1,14 +0,0 @@
-package source
-
-import (
-	"net/http"
-
-	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
-)
-
-// Source represents a source of information about a domain. Whenever a request
-// appears a concret implementation of a Source should find a domain that is
-// needed to handle the request and serve pages
-type Source interface {
-	GetDomain(*http.Request) *domain.Domain
-}