Log serving type used for requestslog-serving-type

This extends our structured logging
with information about how the given
request was served

7 files changed, 90 insertions, 50 deletions

diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index d3daa143..deff2cc5 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -53,6 +53,10 @@ func (d *Domain) resolve(r *http.Request) *serving.Request {
 // GetLookupPath returns a project details based on the request. It returns nil
 // if project does not exist.
 func (d *Domain) GetLookupPath(r *http.Request) *serving.LookupPath {
+	if d.isUnconfigured() {
+		return nil
+	}
+
 	request := d.resolve(r)
 
 	if request == nil {
@@ -65,10 +69,6 @@ func (d *Domain) GetLookupPath(r *http.Request) *serving.LookupPath {
 // IsHTTPSOnly figures out if the request should be handled with HTTPS
 // only by looking at group and project level config.
 func (d *Domain) IsHTTPSOnly(r *http.Request) bool {
-	if d.isUnconfigured() {
-		return false
-	}
-
 	if lookupPath := d.GetLookupPath(r); lookupPath != nil {
 		return lookupPath.IsHTTPSOnly
 	}
@@ -78,10 +78,6 @@ func (d *Domain) IsHTTPSOnly(r *http.Request) bool {
 
 // IsAccessControlEnabled figures out if the request is to a project that has access control enabled
 func (d *Domain) IsAccessControlEnabled(r *http.Request) bool {
-	if d.isUnconfigured() {
-		return false
-	}
-
 	if lookupPath := d.GetLookupPath(r); lookupPath != nil {
 		return lookupPath.HasAccessControl
 	}
@@ -91,10 +87,6 @@ func (d *Domain) IsAccessControlEnabled(r *http.Request) bool {
 
 // IsNamespaceProject figures out if the request is to a namespace project
 func (d *Domain) IsNamespaceProject(r *http.Request) bool {
-	if d.isUnconfigured() {
-		return false
-	}
-
 	if lookupPath := d.GetLookupPath(r); lookupPath != nil {
 		return lookupPath.IsNamespaceProject
 	}
@@ -104,10 +96,6 @@ func (d *Domain) IsNamespaceProject(r *http.Request) bool {
 
 // GetProjectID figures out what is the ID of the project user tries to access
 func (d *Domain) GetProjectID(r *http.Request) uint64 {
-	if d.isUnconfigured() {
-		return 0
-	}
-
 	if lookupPath := d.GetLookupPath(r); lookupPath != nil {
 		return lookupPath.ProjectID
 	}
@@ -117,10 +105,6 @@ func (d *Domain) GetProjectID(r *http.Request) uint64 {
 
 // HasLookupPath figures out if the project exists that the user tries to access
 func (d *Domain) HasLookupPath(r *http.Request) bool {
-	if d.isUnconfigured() {
-		return false
-	}
-
 	return d.GetLookupPath(r) != nil
 }
 
@@ -146,7 +130,7 @@ func (d *Domain) EnsureCertificate() (*tls.Certificate, error) {
 
 // ServeFileHTTP returns true if something was served, false if not.
 func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
-	if d.isUnconfigured() || !d.HasLookupPath(r) {
+	if !d.HasLookupPath(r) {
 		// TODO: this seems to be wrong: as we should rather return false, and
 		// fallback to `ServeNotFoundHTTP` to handle this case
 		httperrors.Serve404(w)
@@ -160,7 +144,7 @@ func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
 
 // ServeNotFoundHTTP serves the not found pages from the projects.
 func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
-	if d.isUnconfigured() || !d.HasLookupPath(r) {
+	if !d.HasLookupPath(r) {
 		httperrors.Serve404(w)
 		return
 	}
@@ -196,7 +180,7 @@ func (d *Domain) serveNamespaceNotFound(w http.ResponseWriter, r *http.Request)
 // ServeNotFoundAuthFailed handler to be called when auth failed so the correct custom
 // 404 page is served.
 func (d *Domain) ServeNotFoundAuthFailed(w http.ResponseWriter, r *http.Request) {
-	if d.isUnconfigured() || !d.HasLookupPath(r) {
+	if !d.HasLookupPath(r) {
 		httperrors.Serve404(w)
 		return
 	}
diff --git a/internal/logging/logging.go b/internal/logging/logging.go
index 3432cdf7..6643e169 100644
--- a/internal/logging/logging.go
+++ b/internal/logging/logging.go
@@ -53,16 +53,20 @@ func getAccessLogger(format string) (*logrus.Logger, error) {
 // getExtraLogFields is used to inject additional fields into the
 // HTTP access logger middleware.
 func getExtraLogFields(r *http.Request) log.Fields {
-	var projectID uint64
-	if d := request.GetDomain(r); d != nil {
-		projectID = d.GetProjectID(r)
+	logFields := log.Fields{
+		"pages_https": request.IsHTTPS(r),
+		"pages_host":  request.GetHost(r),
 	}
 
-	return log.Fields{
-		"pages_https":      request.IsHTTPS(r),
-		"pages_host":       request.GetHost(r),
-		"pages_project_id": projectID,
+	if d := request.GetDomain(r); d != nil {
+		if lp := d.GetLookupPath(r); lp != nil {
+			logFields["pages_project_serving_type"] = lp.ServingType
+			logFields["pages_project_prefix"] = lp.Prefix
+			logFields["pages_project_id"] = lp.ProjectID
+		}
 	}
+
+	return logFields
 }
 
 // BasicAccessLogger configures the GitLab pages basic HTTP access logger middleware
diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go
index ec8837b6..e87a8c0d 100644
--- a/internal/logging/logging_test.go
+++ b/internal/logging/logging_test.go
@@ -8,32 +8,78 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
 )
 
+type lookupPathFunc func(*http.Request) *serving.LookupPath
+
+func (f lookupPathFunc) Resolve(r *http.Request) (*serving.Request, error) {
+	return &serving.Request{LookupPath: f(r)}, nil
+}
+
 func TestGetExtraLogFields(t *testing.T) {
+	domainWithResolver := &domain.Domain{
+		Resolver: lookupPathFunc(func(*http.Request) *serving.LookupPath {
+			return &serving.LookupPath{
+				ServingType: "file",
+				ProjectID:   100,
+				Prefix:      "/prefix",
+			}
+		}),
+	}
+
 	tests := []struct {
-		name   string
-		scheme string
-		host   string
-		domain *domain.Domain
+		name                  string
+		scheme                string
+		host                  string
+		domain                *domain.Domain
+		expectedHTTPS         interface{}
+		expectedHost          interface{}
+		expectedProjectID     interface{}
+		expectedProjectPrefix interface{}
+		expectedServingType   interface{}
 	}{
 		{
-			name:   "https",
-			scheme: request.SchemeHTTPS,
-			host:   "githost.io",
-			domain: &domain.Domain{},
+			name:                  "https",
+			scheme:                request.SchemeHTTPS,
+			host:                  "githost.io",
+			domain:                domainWithResolver,
+			expectedHTTPS:         true,
+			expectedHost:          "githost.io",
+			expectedProjectID:     uint64(100),
+			expectedProjectPrefix: "/prefix",
+			expectedServingType:   "file",
+		},
+		{
+			name:                  "http",
+			scheme:                request.SchemeHTTP,
+			host:                  "githost.io",
+			domain:                domainWithResolver,
+			expectedHTTPS:         false,
+			expectedHost:          "githost.io",
+			expectedProjectID:     uint64(100),
+			expectedProjectPrefix: "/prefix",
+			expectedServingType:   "file",
 		},
 		{
-			name:   "http",
-			scheme: request.SchemeHTTP,
-			host:   "githost.io",
-			domain: &domain.Domain{},
+			name:                "domain_without_resolved",
+			scheme:              request.SchemeHTTP,
+			host:                "githost.io",
+			domain:              &domain.Domain{},
+			expectedHTTPS:       false,
+			expectedHost:        "githost.io",
+			expectedProjectID:   nil,
+			expectedServingType: nil,
 		},
 		{
-			name:   "no_domain",
-			scheme: request.SchemeHTTP,
-			host:   "githost.io",
-			domain: nil,
+			name:                "no_domain",
+			scheme:              request.SchemeHTTP,
+			host:                "githost.io",
+			domain:              nil,
+			expectedHTTPS:       false,
+			expectedHost:        "githost.io",
+			expectedProjectID:   nil,
+			expectedServingType: nil,
 		},
 	}
 
@@ -46,9 +92,11 @@ func TestGetExtraLogFields(t *testing.T) {
 			req = request.WithHostAndDomain(req, tt.host, tt.domain)
 
 			got := getExtraLogFields(req)
-			require.Equal(t, got["pages_https"], tt.scheme == request.SchemeHTTPS)
-			require.Equal(t, got["pages_host"], tt.host)
-			require.Equal(t, got["pages_project_id"], uint64(0x0))
+			require.Equal(t, tt.expectedHTTPS, got["pages_https"])
+			require.Equal(t, tt.expectedHost, got["pages_host"])
+			require.Equal(t, tt.expectedProjectID, got["pages_project_id"])
+			require.Equal(t, tt.expectedProjectPrefix, got["pages_project_prefix"])
+			require.Equal(t, tt.expectedServingType, got["pages_project_serving_type"])
 		})
 	}
 }
diff --git a/internal/serving/lookup_path.go b/internal/serving/lookup_path.go
index 4360358b..1aefe1b8 100644
--- a/internal/serving/lookup_path.go
+++ b/internal/serving/lookup_path.go
@@ -2,6 +2,7 @@ package serving
 
 // LookupPath holds a domain project configuration needed to handle a request
 type LookupPath struct {
+	ServingType        string // Serving type being used, like `zip`
 	Prefix             string // Project prefix, for example, /my/project in group.gitlab.io/my/project/index.html
 	Path               string // Path is an internal and serving-specific location of a document
 	IsNamespaceProject bool   // IsNamespaceProject is DEPRECATED, see https://gitlab.com/gitlab-org/gitlab-pages/issues/272
diff --git a/internal/source/disk/custom.go b/internal/source/disk/custom.go
index f120cb65..fb2aafcd 100644
--- a/internal/source/disk/custom.go
+++ b/internal/source/disk/custom.go
@@ -15,6 +15,7 @@ type customProjectResolver struct {
 
 func (p *customProjectResolver) Resolve(r *http.Request) (*serving.Request, error) {
 	lookupPath := &serving.LookupPath{
+		ServingType:        "file",
 		Prefix:             "/",
 		Path:               p.path,
 		IsNamespaceProject: false,
diff --git a/internal/source/disk/group.go b/internal/source/disk/group.go
index 161ffa02..b12727aa 100644
--- a/internal/source/disk/group.go
+++ b/internal/source/disk/group.go
@@ -88,6 +88,7 @@ func (g *Group) Resolve(r *http.Request) (*serving.Request, error) {
 	}
 
 	lookupPath := &serving.LookupPath{
+		ServingType:        "file",
 		Prefix:             prefix,
 		Path:               filepath.Join(g.name, projectPath, "public") + "/",
 		IsNamespaceProject: projectConfig.NamespaceProject,
diff --git a/internal/source/gitlab/factory.go b/internal/source/gitlab/factory.go
index 39193a16..41f7ea56 100644
--- a/internal/source/gitlab/factory.go
+++ b/internal/source/gitlab/factory.go
@@ -15,8 +15,9 @@ import (
 // https://gitlab.com/gitlab-org/gitlab-pages/issues/272
 func fabricateLookupPath(size int, lookup api.LookupPath) *serving.LookupPath {
 	return &serving.LookupPath{
-		Prefix:             lookup.Prefix,
+		ServingType:        lookup.Source.Type,
 		Path:               lookup.Source.Path,
+		Prefix:             lookup.Prefix,
 		IsNamespaceProject: (lookup.Prefix == "/" && size > 1),
 		IsHTTPSOnly:        lookup.HTTPSOnly,
 		HasAccessControl:   lookup.AccessControl,