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
diff options
context:
space:
mode:
-rw-r--r--app.go7
-rw-r--r--internal/domain/domain.go35
-rw-r--r--internal/serving/disk/reader.go1
-rw-r--r--internal/source/disk/custom.go4
-rw-r--r--internal/source/domains.go6
-rw-r--r--internal/source/gitlab/cache/retriever.go13
-rw-r--r--internal/source/gitlab/client/client.go49
-rw-r--r--internal/source/gitlab/client/client_test.go4
-rw-r--r--internal/source/gitlab/gitlab.go5
9 files changed, 52 insertions, 72 deletions
diff --git a/app.go b/app.go
index 1352b630..ec0c36a5 100644
--- a/app.go
+++ b/app.go
@@ -2,6 +2,7 @@ package main
import (
"crypto/tls"
+ "errors"
"fmt"
"net"
"net/http"
@@ -150,8 +151,8 @@ func (a *theApp) routingMiddleware(handler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// if we could not retrieve a domain from domains source we break the
// middleware chain and simply respond with 502 after logging this
- host, domain, err := a.getHostAndDomain(r)
- if err != nil {
+ host, d, err := a.getHostAndDomain(r)
+ if err != nil && !errors.Is(err, domain.ErrDomainDoesNotExist) {
metrics.DomainsSourceFailures.Inc()
log.WithError(err).Error("could not fetch domain information from a source")
@@ -159,7 +160,7 @@ func (a *theApp) routingMiddleware(handler http.Handler) http.Handler {
return
}
- r = request.WithHostAndDomain(r, host, domain)
+ r = request.WithHostAndDomain(r, host, d)
handler.ServeHTTP(w, r)
})
diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index 33828616..f70c4fea 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
@@ -51,13 +54,10 @@ func (d *Domain) isUnconfigured() bool {
}
func (d *Domain) resolve(r *http.Request) (*serving.Request, error) {
if d.Resolver == nil {
- return nil, errors.New("no resolver")
- }
- req, err := d.Resolver.Resolve(r)
- if err != nil {
- panic("WTFFFFFF- " + err.Error())
+ return nil, ErrDomainDoesNotExist
}
- return req, nil
+
+ return d.Resolver.Resolve(r)
}
// GetLookupPath returns a project details based on the request. It returns nil
@@ -144,14 +144,8 @@ 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 {
request, err := d.resolve(r)
- if request == nil {
- // TODO this seems wrong
- httperrors.Serve404(w)
- return true
- }
-
if err != nil {
- if errors.Is(err, client.ErrDomainDoesNotExist) {
+ if errors.Is(err, ErrDomainDoesNotExist) {
// serve generic 404
httperrors.Serve404(w)
return true
@@ -162,15 +156,19 @@ 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)
}
// ServeNotFoundHTTP serves the not found pages from the projects.
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
@@ -181,6 +179,11 @@ func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
return
}
+ if request.LookupPath == nil {
+ httperrors.Serve404(w)
+ return
+ }
+
request.ServeNotFoundHTTP(w, r)
}
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go
index 12223bad..df6b6d94 100644
--- a/internal/serving/disk/reader.go
+++ b/internal/serving/disk/reader.go
@@ -11,6 +11,7 @@ import (
"time"
"github.com/prometheus/client_golang/prometheus"
+
"gitlab.com/gitlab-org/labkit/errortracking"
"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
diff --git a/internal/source/disk/custom.go b/internal/source/disk/custom.go
index da30c3d5..037abfee 100644
--- a/internal/source/disk/custom.go
+++ b/internal/source/disk/custom.go
@@ -1,9 +1,9 @@
package disk
import (
- "errors"
"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"
)
@@ -16,7 +16,7 @@ type customProjectResolver struct {
func (p *customProjectResolver) Resolve(r *http.Request) (*serving.Request, error) {
if p.config == nil {
- return nil, errors.New("domain not found")
+ return nil, domain.ErrDomainDoesNotExist
}
lookupPath := &serving.LookupPath{
diff --git a/internal/source/domains.go b/internal/source/domains.go
index cbec8d94..cf81fab2 100644
--- a/internal/source/domains.go
+++ b/internal/source/domains.go
@@ -97,11 +97,7 @@ func (d *Domains) setGitLabClient(config Config) error {
// for some subset of domains, to test / PoC the new GitLab Domains Source that
// we plan to use to replace the disk source.
func (d *Domains) GetDomain(name string) (*domain.Domain, error) {
- ddd, err := d.source(name).GetDomain(name)
- if err != nil {
- fmt.Printf("name: %q - d.configSource: %d\n", name, d.configSource)
- }
- return ddd, err
+ return d.source(name).GetDomain(name)
}
// Read starts the disk domain source. It is DEPRECATED, because we want to
diff --git a/internal/source/gitlab/cache/retriever.go b/internal/source/gitlab/cache/retriever.go
index 136fc206..8b3e4723 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
@@ -47,20 +47,19 @@ func (r *Retriever) Retrieve(domain string) (lookup api.Lookup) {
return lookup
}
-func (r *Retriever) resolveWithBackoff(ctx context.Context, domain string) <-chan api.Lookup {
+func (r *Retriever) resolveWithBackoff(ctx context.Context, domainName string) <-chan api.Lookup {
response := make(chan api.Lookup)
go func() {
var lookup api.Lookup
for i := 1; i <= r.maxRetrievalRetries; i++ {
- lookup = r.client.GetLookup(ctx, domain)
-
- if lookup.Error != nil && !errors.Is(lookup.Error, client.ErrDomainDoesNotExist) {
+ lookup = r.client.GetLookup(ctx, domainName)
+ if lookup.Error != nil && !errors.Is(lookup.Error, domain.ErrDomainDoesNotExist) {
time.Sleep(r.maxRetrievalInterval)
- } else {
- break
+ continue
}
+ break
}
response <- lookup
diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go
index 2042b8e3..524eb8a4 100644
--- a/internal/source/gitlab/client/client.go
+++ b/internal/source/gitlab/client/client.go
@@ -8,12 +8,12 @@ import (
"io"
"io/ioutil"
"net/http"
- "net/http/httputil"
"net/url"
"time"
"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"
@@ -24,10 +24,6 @@ import (
// or a 401 given that the credentials used are wrong
const ConnectionErrorMsg = "failed to connect to internal Pages API"
-// ErrDomainDoesNotExist should be returned when we get a 204 from the API when
-// GetLookup is called
-var ErrDomainDoesNotExist = errors.New("domain does not exist")
-
// Client is a HTTP client to access Pages internal API
type Client struct {
secretKey []byte
@@ -97,8 +93,10 @@ func (gc *Client) GetLookup(ctx context.Context, host string) api.Lookup {
return api.Lookup{Name: host, Error: err}
}
- dres, err := httputil.DumpResponse(resp, true)
- fmt.Printf("dres: %s\n%+v\n", dres, err)
+ if resp == nil {
+ return api.Lookup{Name: host, Error: domain.ErrDomainDoesNotExist}
+ }
+
// ensure that entire response body has been read and close it, to make it
// possible to reuse HTTP connection. In case of a JSON being invalid and
// larger than 512 bytes, the response body will not be closed properly, thus
@@ -109,10 +107,6 @@ func (gc *Client) GetLookup(ctx context.Context, host string) api.Lookup {
}()
lookup := api.Lookup{Name: host}
- if resp.StatusCode == http.StatusNoContent {
- lookup.Error = ErrDomainDoesNotExist
- }
-
lookup.Error = json.NewDecoder(resp.Body).Decode(&lookup.Domain)
return lookup
@@ -155,17 +149,21 @@ func (gc *Client) get(ctx context.Context, path string, params url.Values) (*htt
}
// StatusOK means we should return the API response
- if resp.StatusCode == http.StatusOK || resp.StatusCode == http.StatusNoContent {
+ if resp.StatusCode == http.StatusOK {
return resp, nil
}
- var apiErr *Err
- if err := json.NewDecoder(resp.Body).Decode(&apiErr); err != nil {
- return nil, err
+ // nolint: errcheck
+ // best effort to discard and close the response body
+ io.Copy(ioutil.Discard, resp.Body)
+ resp.Body.Close()
+
+ // StatusNoContent means that a domain does not exist, it is not an error
+ if resp.StatusCode == http.StatusNoContent {
+ return nil, nil
}
- // return nil, fmt.Errorf("HTTP status: %d", resp.StatusCode)
- return nil, apiErr
+ return nil, fmt.Errorf("HTTP status: %d", resp.StatusCode)
}
func (gc *Client) endpoint(path string, params url.Values) (*url.URL, error) {
@@ -193,8 +191,6 @@ func (gc *Client) request(ctx context.Context, method string, endpoint *url.URL)
}
req.Header.Set("Gitlab-Pages-Api-Request", token)
- dreq, err := httputil.DumpRequestOut(req, true)
- fmt.Printf("dreq: %s\n%+v\n", dreq, err)
return req, nil
}
@@ -211,18 +207,3 @@ func (gc *Client) token() (string, error) {
return token, nil
}
-
-// Err describes any error response received from the API
-type Err struct {
- Status int
- Message string `json:"message"`
- Err string `json:"error"`
-}
-
-func (e *Err) Error() string {
- if e.Err != "" {
- return fmt.Sprintf("status: %d error: %s", e.Status, e.Err)
- }
-
- return fmt.Sprintf("status: %d error: %s", e.Status, e.Message)
-}
diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go
index 6d4ce814..153b9372 100644
--- a/internal/source/gitlab/client/client_test.go
+++ b/internal/source/gitlab/client/client_test.go
@@ -3,6 +3,7 @@ package client
import (
"context"
"encoding/base64"
+ "errors"
"fmt"
"net/http"
"net/http/httptest"
@@ -13,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"
)
@@ -176,7 +178,7 @@ func TestMissingDomain(t *testing.T) {
lookup := client.GetLookup(context.Background(), "group.gitlab.io")
- require.NoError(t, lookup.Error)
+ 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 28d9cac8..e7cd76e7 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -18,9 +18,6 @@ import (
"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client"
)
-// ErrDomainDoesNotExist is returned by
-var ErrDomainDoesNotExist = errors.New("domain does not exist")
-
// Gitlab source represent a new domains configuration source. We fetch all the
// information about domains from GitLab instance.
type Gitlab struct {
@@ -58,7 +55,7 @@ func (g *Gitlab) GetDomain(name string) (*domain.Domain, error) {
// Domain does not exist
if lookup.Domain == nil {
- return nil, ErrDomainDoesNotExist
+ return nil, domain.ErrDomainDoesNotExist
}
// TODO introduce a second-level cache for domains, invalidate using etags