Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-pages.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
Diffstat (limited to 'internal')
-rw-r--r--internal/domain/domain.go54
-rw-r--r--internal/domain/domain_test.go20
-rw-r--r--internal/logging/logging.go1
-rw-r--r--internal/logging/logging_test.go23
-rw-r--r--internal/source/disk/custom.go4
-rw-r--r--internal/source/disk/domain_test.go6
-rw-r--r--internal/source/disk/group.go5
-rw-r--r--internal/source/gitlab/cache/cache_test.go28
-rw-r--r--internal/source/gitlab/cache/retriever.go4
-rw-r--r--internal/source/gitlab/client/client.go7
-rw-r--r--internal/source/gitlab/client/client_test.go3
-rw-r--r--internal/source/gitlab/gitlab.go11
12 files changed, 68 insertions, 98 deletions
diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index 5f32df0b..636b1bbd 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -11,9 +11,12 @@ import (
"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
- "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client"
)
+// ErrDomainDoesNotExist returned when a domain is not found or when a lookup path
+// for a domain could not be resolved
+var ErrDomainDoesNotExist = errors.New("domain does not exist")
+
// Domain is a domain that gitlab-pages can serve.
type Domain struct {
Name string
@@ -42,16 +45,9 @@ func (d *Domain) String() string {
return d.Name
}
-func (d *Domain) isUnconfigured() bool {
- if d == nil {
- return true
- }
-
- return d.Resolver == nil
-}
func (d *Domain) resolve(r *http.Request) (*serving.Request, error) {
- if d.Resolver == nil {
- return nil, client.ErrDomainDoesNotExist
+ if d == nil {
+ return nil, ErrDomainDoesNotExist
}
return d.Resolver.Resolve(r)
@@ -60,10 +56,6 @@ func (d *Domain) resolve(r *http.Request) (*serving.Request, error) {
// 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, error) {
- if d.isUnconfigured() {
- return nil, errors.New("not configured")
- }
-
servingReq, err := d.resolve(r)
if err != nil {
return nil, err
@@ -111,16 +103,17 @@ 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 _, err := d.GetLookupPath(r); err != nil {
+ if d == nil {
return false
}
+ _, err := d.GetLookupPath(r)
- return true
+ return err == nil
}
// EnsureCertificate parses the PEM-encoded certificate for the domain
func (d *Domain) EnsureCertificate() (*tls.Certificate, error) {
- if d.isUnconfigured() || len(d.CertificateKey) == 0 || len(d.CertificateCert) == 0 {
+ if d == nil || len(d.CertificateKey) == 0 || len(d.CertificateCert) == 0 {
return nil, errors.New("tls certificates can be loaded only for pages with configuration")
}
@@ -142,7 +135,7 @@ func (d *Domain) EnsureCertificate() (*tls.Certificate, error) {
func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
request, err := d.resolve(r)
if err != nil {
- if errors.Is(err, client.ErrDomainDoesNotExist) {
+ if errors.Is(err, ErrDomainDoesNotExist) {
// serve generic 404
httperrors.Serve404(w)
return true
@@ -153,11 +146,6 @@ func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
return true
}
- if request.LookupPath == nil {
- // return and let ServeNotFoundHTTP serve the 404 page
- return false
- }
-
return request.ServeFileHTTP(w, r)
}
@@ -165,7 +153,7 @@ func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
request, err := d.resolve(r)
if err != nil {
- if errors.Is(err, client.ErrDomainDoesNotExist) {
+ if errors.Is(err, ErrDomainDoesNotExist) {
// serve generic 404
httperrors.Serve404(w)
return
@@ -176,11 +164,6 @@ func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
return
}
- if request.LookupPath == nil {
- httperrors.Serve404(w)
- return
- }
-
request.ServeNotFoundHTTP(w, r)
}
@@ -194,16 +177,17 @@ func (d *Domain) serveNamespaceNotFound(w http.ResponseWriter, r *http.Request)
namespaceDomain, err := d.Resolver.Resolve(clonedReq)
if err != nil {
+ if errors.Is(err, ErrDomainDoesNotExist) {
+ // serve generic 404
+ httperrors.Serve404(w)
+ return
+ }
+
errortracking.Capture(err, errortracking.WithRequest(r))
httperrors.Serve503(w)
return
}
- if namespaceDomain.LookupPath == nil {
- httperrors.Serve404(w)
- return
- }
-
// for namespace domains that have no access control enabled
if !namespaceDomain.LookupPath.HasAccessControl {
namespaceDomain.ServeNotFoundHTTP(w, r)
@@ -222,7 +206,7 @@ func (d *Domain) ServeNotFoundAuthFailed(w http.ResponseWriter, r *http.Request)
return
}
- if d.IsNamespaceProject(r) && lookupPath.HasAccessControl {
+ if d.IsNamespaceProject(r) && !lookupPath.HasAccessControl {
d.ServeNotFoundHTTP(w, r)
return
}
diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go
index 2df1d992..f053001a 100644
--- a/internal/domain/domain_test.go
+++ b/internal/domain/domain_test.go
@@ -46,33 +46,31 @@ func TestIsHTTPSOnly(t *testing.T) {
}{
{
name: "Custom domain with HTTPS-only enabled",
- domain: &Domain{
- Resolver: &stubbedResolver{
+ domain: New("custom-domain", "", "",
+ &stubbedResolver{
project: &serving.LookupPath{
Path: "group/project/public",
IsHTTPSOnly: true,
},
- },
- },
+ }),
url: "http://custom-domain",
expected: true,
},
{
name: "Custom domain with HTTPS-only disabled",
- domain: &Domain{
- Resolver: &stubbedResolver{
+ domain: New("custom-domain", "", "",
+ &stubbedResolver{
project: &serving.LookupPath{
Path: "group/project/public",
IsHTTPSOnly: false,
},
- },
- },
+ }),
url: "http://custom-domain",
expected: false,
},
{
name: "Unknown project",
- domain: &Domain{},
+ domain: New("", "", "", &stubbedResolver{err: ErrDomainDoesNotExist}),
url: "http://test-domain/project",
expected: false,
},
@@ -90,7 +88,7 @@ func TestPredefined404ServeHTTP(t *testing.T) {
cleanup := setUpTests(t)
defer cleanup()
- testDomain := &Domain{}
+ testDomain := New("", "", "", &stubbedResolver{err: ErrDomainDoesNotExist})
testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.test.io/not-existing-file", nil, "The page you're looking for could not be found")
}
@@ -192,7 +190,7 @@ func TestServeNamespaceNotFound(t *testing.T) {
domain: "group.404.gitlab-example.com",
path: "/unknown",
resolver: &stubbedResolver{
- project: nil,
+ err: ErrDomainDoesNotExist,
subpath: "/",
},
expectedResponse: "The page you're looking for could not be found.",
diff --git a/internal/logging/logging.go b/internal/logging/logging.go
index 43fe65e6..4dcf66d2 100644
--- a/internal/logging/logging.go
+++ b/internal/logging/logging.go
@@ -61,7 +61,6 @@ func getExtraLogFields(r *http.Request) log.Fields {
if d := request.GetDomain(r); d != nil {
lp, err := d.GetLookupPath(r)
if err != nil {
- logFields["error"] = err.Error()
return logFields
}
diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go
index 2ed2b6ec..9079cc9d 100644
--- a/internal/logging/logging_test.go
+++ b/internal/logging/logging_test.go
@@ -18,13 +18,15 @@ func (f lookupPathFunc) Resolve(r *http.Request) (*serving.Request, error) {
}
func TestGetExtraLogFields(t *testing.T) {
- domainWithResolver := domain.New("", "", "", lookupPathFunc(func(*http.Request) *serving.LookupPath {
- return &serving.LookupPath{
- ServingType: "file",
- ProjectID: 100,
- Prefix: "/prefix",
- }
- }))
+ domainWithResolver := &domain.Domain{
+ Resolver: lookupPathFunc(func(*http.Request) *serving.LookupPath {
+ return &serving.LookupPath{
+ ServingType: "file",
+ ProjectID: 100,
+ Prefix: "/prefix",
+ }
+ }),
+ }
tests := []struct {
name string
@@ -36,7 +38,6 @@ func TestGetExtraLogFields(t *testing.T) {
expectedProjectID interface{}
expectedProjectPrefix interface{}
expectedServingType interface{}
- expectedErrMsg interface{}
}{
{
name: "https",
@@ -61,15 +62,14 @@ func TestGetExtraLogFields(t *testing.T) {
expectedServingType: "file",
},
{
- name: "domain_without_resolver",
+ name: "domain_without_resolved",
scheme: request.SchemeHTTP,
host: "githost.io",
- domain: &domain.Domain{},
+ domain: nil,
expectedHTTPS: false,
expectedHost: "githost.io",
expectedProjectID: nil,
expectedServingType: nil,
- expectedErrMsg: "not configured",
},
{
name: "no_domain",
@@ -97,7 +97,6 @@ func TestGetExtraLogFields(t *testing.T) {
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"])
- require.Equal(t, tt.expectedErrMsg, got["error"])
})
}
}
diff --git a/internal/source/disk/custom.go b/internal/source/disk/custom.go
index 93717028..037abfee 100644
--- a/internal/source/disk/custom.go
+++ b/internal/source/disk/custom.go
@@ -3,9 +3,9 @@ package disk
import (
"net/http"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/domain"
"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/local"
- "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client"
)
type customProjectResolver struct {
@@ -16,7 +16,7 @@ type customProjectResolver struct {
func (p *customProjectResolver) Resolve(r *http.Request) (*serving.Request, error) {
if p.config == nil {
- return nil, client.ErrDomainDoesNotExist
+ return nil, domain.ErrDomainDoesNotExist
}
lookupPath := &serving.LookupPath{
diff --git a/internal/source/disk/domain_test.go b/internal/source/disk/domain_test.go
index 323b45e5..abffddb4 100644
--- a/internal/source/disk/domain_test.go
+++ b/internal/source/disk/domain_test.go
@@ -162,8 +162,10 @@ func TestIsHTTPSOnly(t *testing.T) {
expected: false,
},
{
- name: "Unknown project",
- domain: &domain.Domain{},
+ name: "Unknown project",
+ domain: &domain.Domain{
+ Resolver: &Group{},
+ },
url: "http://test-domain/project",
expected: false,
},
diff --git a/internal/source/disk/group.go b/internal/source/disk/group.go
index b12727aa..9499df46 100644
--- a/internal/source/disk/group.go
+++ b/internal/source/disk/group.go
@@ -6,6 +6,7 @@ import (
"path/filepath"
"strings"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/domain"
"gitlab.com/gitlab-org/gitlab-pages/internal/host"
"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/local"
@@ -82,9 +83,7 @@ func (g *Group) Resolve(r *http.Request) (*serving.Request, error) {
projectConfig, prefix, projectPath, subPath := g.getProjectConfigWithSubpath(r)
if projectConfig == nil {
- // it is not an error when project does not exist, in that case
- // serving.Request.LookupPath is nil.
- return &serving.Request{Serving: local.Instance()}, nil
+ return nil, domain.ErrDomainDoesNotExist
}
lookupPath := &serving.LookupPath{
diff --git a/internal/source/gitlab/cache/cache_test.go b/internal/source/gitlab/cache/cache_test.go
index 757adb2b..7ed56f5a 100644
--- a/internal/source/gitlab/cache/cache_test.go
+++ b/internal/source/gitlab/cache/cache_test.go
@@ -13,14 +13,14 @@ import (
"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
)
-type clientMock struct {
+type client struct {
counter uint64
lookups chan uint64
domain chan string
failure error
}
-func (c *clientMock) GetLookup(ctx context.Context, _ string) api.Lookup {
+func (c *client) GetLookup(ctx context.Context, _ string) api.Lookup {
lookup := api.Lookup{}
if c.failure == nil {
lookup.Name = <-c.domain
@@ -33,11 +33,11 @@ func (c *clientMock) GetLookup(ctx context.Context, _ string) api.Lookup {
return lookup
}
-func (c *clientMock) Status() error {
+func (c *client) Status() error {
return nil
}
-func withTestCache(config resolverConfig, cacheConfig *cacheConfig, block func(*Cache, *clientMock)) {
+func withTestCache(config resolverConfig, cacheConfig *cacheConfig, block func(*Cache, *client)) {
var chanSize int
if config.buffered {
@@ -46,7 +46,7 @@ func withTestCache(config resolverConfig, cacheConfig *cacheConfig, block func(*
chanSize = 0
}
- resolver := &clientMock{
+ resolver := &client{
domain: make(chan string, chanSize),
lookups: make(chan uint64, 100),
failure: config.failure,
@@ -90,7 +90,7 @@ type entryConfig struct {
func TestResolve(t *testing.T) {
t.Run("when item is not cached", func(t *testing.T) {
- withTestCache(resolverConfig{buffered: true}, nil, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{buffered: true}, nil, func(cache *Cache, resolver *client) {
require.Equal(t, 0, len(resolver.lookups))
resolver.domain <- "my.gitlab.com"
@@ -103,7 +103,7 @@ func TestResolve(t *testing.T) {
})
t.Run("when item is not cached and accessed multiple times", func(t *testing.T) {
- withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *client) {
wg := &sync.WaitGroup{}
ctx := context.Background()
@@ -127,7 +127,7 @@ func TestResolve(t *testing.T) {
})
t.Run("when item is in short cache", func(t *testing.T) {
- withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *client) {
cache.withTestEntry(entryConfig{expired: false, retrieved: true}, func(*Entry) {
lookup := cache.Resolve(context.Background(), "my.gitlab.com")
@@ -138,7 +138,7 @@ func TestResolve(t *testing.T) {
})
t.Run("when a non-retrieved new item is in short cache", func(t *testing.T) {
- withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *client) {
cache.withTestEntry(entryConfig{expired: false, retrieved: false}, func(*Entry) {
lookup := make(chan *api.Lookup, 1)
@@ -157,7 +157,7 @@ func TestResolve(t *testing.T) {
})
t.Run("when item is in long cache only", func(t *testing.T) {
- withTestCache(resolverConfig{buffered: false}, nil, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{buffered: false}, nil, func(cache *Cache, resolver *client) {
cache.withTestEntry(entryConfig{expired: true, retrieved: true}, func(*Entry) {
lookup := cache.Resolve(context.Background(), "my.gitlab.com")
@@ -172,7 +172,7 @@ func TestResolve(t *testing.T) {
})
t.Run("when item in long cache is requested multiple times", func(t *testing.T) {
- withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *client) {
cache.withTestEntry(entryConfig{expired: true, retrieved: true}, func(*Entry) {
cache.Resolve(context.Background(), "my.gitlab.com")
cache.Resolve(context.Background(), "my.gitlab.com")
@@ -192,7 +192,7 @@ func TestResolve(t *testing.T) {
cc.maxRetrievalInterval = 0
err := errors.New("500 error")
- withTestCache(resolverConfig{failure: err}, &cc, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{failure: err}, &cc, func(cache *Cache, resolver *client) {
lookup := cache.Resolve(context.Background(), "my.gitlab.com")
require.Equal(t, 3, len(resolver.lookups))
@@ -204,7 +204,7 @@ func TestResolve(t *testing.T) {
cc := defaultCacheConfig
cc.retrievalTimeout = 0
- withTestCache(resolverConfig{}, &cc, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{}, &cc, func(cache *Cache, resolver *client) {
lookup := cache.Resolve(context.Background(), "my.gitlab.com")
require.Equal(t, 0, len(resolver.lookups))
@@ -213,7 +213,7 @@ func TestResolve(t *testing.T) {
})
t.Run("when retrieval failed because of resolution context being canceled", func(t *testing.T) {
- withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *clientMock) {
+ withTestCache(resolverConfig{}, nil, func(cache *Cache, resolver *client) {
cache.withTestEntry(entryConfig{expired: false, retrieved: false}, func(entry *Entry) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
diff --git a/internal/source/gitlab/cache/retriever.go b/internal/source/gitlab/cache/retriever.go
index 953c42e9..eddb02cb 100644
--- a/internal/source/gitlab/cache/retriever.go
+++ b/internal/source/gitlab/cache/retriever.go
@@ -7,8 +7,8 @@ import (
log "github.com/sirupsen/logrus"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/domain"
"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
- "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client"
)
// Retriever is an utility type that performs an HTTP request with backoff in
@@ -55,7 +55,7 @@ func (r *Retriever) resolveWithBackoff(ctx context.Context, domainName string) <
for i := 1; i <= r.maxRetrievalRetries; i++ {
lookup = r.client.GetLookup(ctx, domainName)
- if lookup.Error == nil || errors.Is(lookup.Error, client.ErrDomainDoesNotExist) {
+ if lookup.Error == nil || errors.Is(lookup.Error, domain.ErrDomainDoesNotExist) {
// do not retry if the domain does not exist
break
}
diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go
index eb956d33..524eb8a4 100644
--- a/internal/source/gitlab/client/client.go
+++ b/internal/source/gitlab/client/client.go
@@ -13,6 +13,7 @@ import (
"github.com/dgrijalva/jwt-go"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/domain"
"gitlab.com/gitlab-org/gitlab-pages/internal/httptransport"
"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
"gitlab.com/gitlab-org/gitlab-pages/metrics"
@@ -23,10 +24,6 @@ import (
// or a 401 given that the credentials used are wrong
const ConnectionErrorMsg = "failed to connect to internal Pages API"
-// ErrDomainDoesNotExist returned when a domain is not found or when a lookup path
-// for a domain could not be resolved
-var ErrDomainDoesNotExist = errors.New("domain does not exist")
-
// Client is a HTTP client to access Pages internal API
type Client struct {
secretKey []byte
@@ -97,7 +94,7 @@ func (gc *Client) GetLookup(ctx context.Context, host string) api.Lookup {
}
if resp == nil {
- return api.Lookup{Name: host, Error: ErrDomainDoesNotExist}
+ return api.Lookup{Name: host, Error: domain.ErrDomainDoesNotExist}
}
// ensure that entire response body has been read and close it, to make it
diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go
index ee5a5ab3..153b9372 100644
--- a/internal/source/gitlab/client/client_test.go
+++ b/internal/source/gitlab/client/client_test.go
@@ -14,6 +14,7 @@ import (
"github.com/dgrijalva/jwt-go"
"github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/domain"
"gitlab.com/gitlab-org/gitlab-pages/internal/fixture"
)
@@ -177,7 +178,7 @@ func TestMissingDomain(t *testing.T) {
lookup := client.GetLookup(context.Background(), "group.gitlab.io")
- require.True(t, errors.Is(lookup.Error, ErrDomainDoesNotExist))
+ require.True(t, errors.Is(lookup.Error, domain.ErrDomainDoesNotExist))
require.Nil(t, lookup.Domain)
}
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index c31d9466..d164460a 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -2,7 +2,6 @@ package gitlab
import (
"context"
- "errors"
"net/http"
"path"
"strings"
@@ -53,11 +52,6 @@ func (g *Gitlab) GetDomain(name string) (*domain.Domain, error) {
return nil, lookup.Error
}
- // Domain does not exist
- if lookup.Domain == nil {
- return nil, client.ErrDomainDoesNotExist
- }
-
// TODO introduce a second-level cache for domains, invalidate using etags
// from first-level cache
d := domain.New(name, lookup.Domain.Certificate, lookup.Domain.Key, g)
@@ -96,10 +90,7 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) {
}
}
- // TODO improve code around default serving, when `disk` serving gets removed
- // https://gitlab.com/gitlab-org/gitlab-pages/issues/353
- return &serving.Request{Serving: defaultServing()},
- errors.New("could not match lookup path")
+ return nil, domain.ErrDomainDoesNotExist
}
// IsReady returns the value of Gitlab `isReady` which is updated by `Poll`.
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-12 05:39:39 +0300