diff options
57 files changed, 573 insertions, 801 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 497e3d51..aeb1c16a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -19,7 +19,7 @@ include: - local: .gitlab/ci/test.yml default: - image: golang:1.13 + image: golang:1.14 tags: - gitlab-org diff --git a/.gitlab/ci/test.yml b/.gitlab/ci/test.yml index f6ca3e08..f0e484b9 100644 --- a/.gitlab/ci/test.yml +++ b/.gitlab/ci/test.yml @@ -24,14 +24,6 @@ - TEST_DAEMONIZE=inplace make acceptance - make junit-report -test:1.13: - extends: .tests-unit - image: golang:1.13 - -test-acceptance:1.13: - extends: .tests-acceptance-deamon - image: golang:1.13 - test:1.14: extends: .tests-unit image: golang:1.14 diff --git a/.gitlab/issue_templates/release.md b/.gitlab/issue_templates/release.md index 9bfd7c7f..930508c8 100644 --- a/.gitlab/issue_templates/release.md +++ b/.gitlab/issue_templates/release.md @@ -1,20 +1,12 @@ - [ ] Set the milestone on this issue -- [ ] Review the list of changes since the last release and fill below: - - [ ] **In the changelog** - - [ ] **Not in the changelog** - - Hint: - ``` - git --no-pager log --merges --pretty=oneline master...vX.Y.Z - ``` - Decide on the version number by reference to the [Versioning](https://gitlab.com/gitlab-org/gitlab-pages/blob/master/PROCESS.md#versioning) * Typically if you want to release code from current `master` branch you will update `MINOR` version, e.g. `1.12.0` -> `1.13.0`. In that case you **don't** need to create stable branch * If you want to backport some bug fix or security fix you will need to update stable branch `X-Y-stable` - [ ] Create an MR for [gitlab-pages project](https://gitlab.com/gitlab-org/gitlab-pages). You can use [this MR](https://gitlab.com/gitlab-org/gitlab-pages/merge_requests/217) as an example. - - [ ] Update `VERSION` - - [ ] Update `CHANGELOG`, you can run `make changelog` once you have pushed your branch to the remote server + - [ ] Update `VERSION`, and push your branch + - [ ] Update `CHANGELOG` by running `GITLAB_PRIVATE_TOKEN= make changelog`, note that you need to create a personal access token - [ ] Assign to reviewer - [ ] Once `gitlab-pages` is merged create a signed+annotated tag pointing to the **merge commit** on the **stable branch** In case of `master` branch: @@ -47,18 +39,7 @@ - [ ] Create an MR for [gitlab project](https://gitlab.com/gitlab-org/gitlab). You can use [this MR](https://gitlab.com/gitlab-org/gitlab/merge_requests/23023) as an example. - [ ] Update `GITLAB_PAGES_VERSION` - - [ ] Create a changelog entry + - [ ] Added `Changelog: added` footer to your commit - [ ] Assign to a reviewer -### In the changelog -``` -- some change -- some change -``` -### Not in the changelog -``` -- some change -- some change -``` - /label ~backend ~"Category:Pages" ~"devops::release" ~"group::release" ~"feature::maintenance" diff --git a/.gitlab/merge_request_templates/Security Release.md b/.gitlab/merge_request_templates/Security Release.md index df832c7d..ea892588 100644 --- a/.gitlab/merge_request_templates/Security Release.md +++ b/.gitlab/merge_request_templates/Security Release.md @@ -19,7 +19,7 @@ When submitting a merge request for gitlab-pages, CE and EE merge requests for u - [ ] MR targets `master`, or `X-Y-stable` for backports - [ ] Milestone is set for the version this MR applies to - [ ] Title of this MR is the same as for all backports -- [ ] A CHANGELOG entry is added +- [ ] A [CHANGELOG entry] has been included, with `Changelog` trailer set to `security`. - [ ] Add a link to this MR in the `links` section of related issue - [ ] Create a merge request in [GitLab Security](https://gitlab.com/gitlab-org/security/gitlab) bumping GitLab pages version: MR_LINK_HERE - [ ] Assign to a Pages maintainer for review and merge @@ -30,4 +30,6 @@ When submitting a merge request for gitlab-pages, CE and EE merge requests for u - [ ] Merge this merge request - [ ] Create corresponding tag and push it to https://gitlab.com/gitlab-org/security/gitlab-pages +[CHANGELOG entry]: https://docs.gitlab.com/ee/development/changelog.html#overview + /label ~security diff --git a/CHANGELOG.md b/CHANGELOG.md index 99b0b9f1..46866e9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,18 @@ +## 1.40.0 (2021-06-09) + +### Changed (1 change) + +- [Use API source by default](gitlab-org/gitlab-pages@9a30f40a785ffe586a79a75ebebabda5f513b76f) ([merge request](gitlab-org/gitlab-pages!491)) + +### Removed (2 changes) + +- [Drop support for Go 1.13](gitlab-org/gitlab-pages@5abbb3dd66270f0124fc5c7fe4523f237337c890) ([merge request](gitlab-org/gitlab-pages!501)) +- [Remove gitlab-server fallback to auth-server and artifacts server](gitlab-org/gitlab-pages@6f806a34631581486ca12e2fe3addd3f722b6477) ([merge request](gitlab-org/gitlab-pages!472)) + +### Other (1 change) + +- [Sort lookup paths by prefix length](gitlab-org/gitlab-pages@b9ba78cf43a14a4ca1c4893c7115eed56ce1b115) ([merge request](gitlab-org/gitlab-pages!496)) + ## 1.39.0 (2021-05-14) ### Added (1 change) diff --git a/Makefile.util.mk b/Makefile.util.mk index 7037f2ce..3d803945 100644 --- a/Makefile.util.mk +++ b/Makefile.util.mk @@ -60,3 +60,9 @@ changelog: ifndef GITLAB_PRIVATE_TOKEN $(error GITLAB_PRIVATE_TOKEN is undefined) endif + +.PHONY: zip + +zip: + cd $(PWD)/shared/pages/$(PROJECT_SUBDIR)/ && \ + zip -r public.zip public/ @@ -4,7 +4,7 @@ [](https://gitlab.com/gitlab-org/gitlab-pages/commits/master) This is a simple HTTP server written in Go, made to serve GitLab Pages with -CNAMEs and SNI using HTTP/HTTP2. The minimum supported Go version is v1.13. +CNAMEs and SNI using HTTP/HTTP2. The minimum supported Go version is v1.14. ### How it generates routes @@ -92,7 +92,7 @@ as. The daemon starts listening on ports and reads certificates as root, then re-executes itself as the specified user. When re-executing it creates a chroot jail -containing a copy of its own binary, `/etc/hosts`, `/etc/resolv.conf`, and a bind mount of `pages-root`. +containing a copy of its own binary, `/etc/hosts`, `/etc/nsswitch.conf`, `/etc/resolv.conf`, and a bind mount of `pages-root`. When `-artifacts-server` points to an HTTPS URL we also need a list of certificates for the trusted Certification Authorities to copy inside the jail. @@ -110,7 +110,7 @@ $ sudo ./gitlab-pages -listen-http ":80" -pages-root path/to/gitlab/shared/pages #### Caveats -The `/etc/hosts` and `/etc/resolv.conf` files, and any file pointed to by the `SSL_CERT_FILE` +The `/etc/hosts`, `/etc/resolv.conf` and `/etc/nsswitch.conf` files, and any file pointed to by the `SSL_CERT_FILE` environment variable, will be copied into the jail. As a result, changes to these files will not be reflected in Pages until it's restarted. @@ -1 +1 @@ -1.39.0 +1.40.0 @@ -421,8 +421,7 @@ func (a *theApp) listenHTTPFD(wg *sync.WaitGroup, fd uintptr, httpHandler http.H wg.Add(1) go func() { defer wg.Done() - err := listenAndServe(fd, httpHandler, a.config.General.HTTP2, nil, limiter, false) - if err != nil { + if err := a.listenAndServe(listenerConfig{fd: fd, handler: httpHandler, limiter: limiter}); err != nil { capturingFatal(err, errortracking.WithField("listener", request.SchemeHTTP)) } }() @@ -437,8 +436,7 @@ func (a *theApp) listenHTTPSFD(wg *sync.WaitGroup, fd uintptr, httpHandler http. capturingFatal(err, errortracking.WithField("listener", request.SchemeHTTPS)) } - err = listenAndServe(fd, httpHandler, a.config.General.HTTP2, tlsConfig, limiter, false) - if err != nil { + if err := a.listenAndServe(listenerConfig{fd: fd, handler: httpHandler, limiter: limiter, tlsConfig: tlsConfig}); err != nil { capturingFatal(err, errortracking.WithField("listener", request.SchemeHTTPS)) } }() @@ -450,8 +448,7 @@ func (a *theApp) listenProxyFD(wg *sync.WaitGroup, fd uintptr, proxyHandler http wg.Add(1) go func(fd uintptr) { defer wg.Done() - err := listenAndServe(fd, proxyHandler, a.config.General.HTTP2, nil, limiter, false) - if err != nil { + if err := a.listenAndServe(listenerConfig{fd: fd, handler: proxyHandler, limiter: limiter}); err != nil { capturingFatal(err, errortracking.WithField("listener", "http proxy")) } }(fd) @@ -468,8 +465,7 @@ func (a *theApp) ListenHTTPSProxyv2FD(wg *sync.WaitGroup, fd uintptr, httpHandle capturingFatal(err, errortracking.WithField("listener", request.SchemeHTTPS)) } - err = listenAndServe(fd, httpHandler, a.config.General.HTTP2, tlsConfig, limiter, true) - if err != nil { + if err := a.listenAndServe(listenerConfig{fd: fd, handler: httpHandler, limiter: limiter, tlsConfig: tlsConfig, isProxyV2: true}); err != nil { capturingFatal(err, errortracking.WithField("listener", request.SchemeHTTPS)) } }() @@ -518,6 +514,9 @@ func runApp(config *cfg.Config) { a.Handlers = handlers.New(a.Auth, a.Artifact) + // TODO: This if was introduced when `gitlab-server` wasn't a required parameter + // once we completely remove support for legacy architecture and make it required + // we can just remove this if statement https://gitlab.com/gitlab-org/gitlab-pages/-/issues/581 if config.GitLab.Server != "" { a.AcmeMiddleware = &acme.Middleware{GitlabURL: config.GitLab.Server} } @@ -549,6 +548,7 @@ func (a *theApp) setAuth(config *cfg.Config) { } var err error + // TODO: use config.GitLab.InternalServer https://gitlab.com/gitlab-org/gitlab-pages/-/issues/581 a.Auth, err = auth.New(config.General.Domain, config.Authentication.Secret, config.Authentication.ClientID, config.Authentication.ClientSecret, config.Authentication.RedirectURI, config.GitLab.Server, config.Authentication.Scope) if err != nil { @@ -228,7 +228,7 @@ func jailCreate(cmd *exec.Cmd) (*jail.Jail, error) { return nil, err } - // Add /etc/resolv.conf and /etc/hosts inside the jail + // Add /etc/resolv.conf, /etc/hosts and /etc/nsswitch.conf inside the jail cage.MkDir("/etc", 0755) err = cage.Copy("/etc/resolv.conf") if err != nil { @@ -239,6 +239,13 @@ func jailCreate(cmd *exec.Cmd) (*jail.Jail, error) { return nil, err } + // When cgo is disabled, Go does not read `/etc/hosts` unless `/etc/nsswitch.conf` exists + // https://github.com/golang/go/issues/22846 + err = cage.Copy("/etc/nsswitch.conf") + if err != nil { + log.WithError(err).Warn("/etc/nsswitch.conf couldn't be copied to the jail, /etc/hosts might not be applicable") + } + // Add certificates inside the jail err = jailDaemonCerts(cmd, cage) if err != nil { @@ -1,9 +1,9 @@ module gitlab.com/gitlab-org/gitlab-pages -go 1.13 +go 1.14 require ( - github.com/andybalholm/brotli v1.0.1 + github.com/andybalholm/brotli v1.0.3 github.com/cenkalti/backoff/v4 v4.0.2 github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/golang/mock v1.3.1 @@ -26,7 +26,6 @@ require ( github.com/stretchr/testify v1.6.1 github.com/tj/assert v0.0.3 // indirect github.com/tj/go-redirects v0.0.0-20180508180010-5c02ead0bbc5 - github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce gitlab.com/gitlab-org/go-mimedb v1.45.0 gitlab.com/gitlab-org/labkit v1.3.0 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 @@ -36,8 +36,8 @@ github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuy github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= -github.com/andybalholm/brotli v1.0.1 h1:KqhlKozYbRtJvsPrrEeXcO+N2l6NYT5A2QAFmSULpEc= -github.com/andybalholm/brotli v1.0.1/go.mod h1:loMXtMfwqflxFJPmdbJO0a3KNoPuLBgiu3qAvBg8x/Y= +github.com/andybalholm/brotli v1.0.3 h1:fpcw+r1N1h0Poc1F/pHbW40cUm/lMEQslZtCkBQ0UnM= +github.com/andybalholm/brotli v1.0.3/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= @@ -463,8 +463,6 @@ github.com/tj/assert v0.0.3/go.mod h1:Ne6X72Q+TB1AteidzQncjw9PabbMp4PBMZ1k+vd1Pv github.com/tj/go-redirects v0.0.0-20180508180010-5c02ead0bbc5 h1:1gWKekoYJSrFfE3r+Q4kfV+DkOWpwH+DHTFZvbbaelQ= github.com/tj/go-redirects v0.0.0-20180508180010-5c02ead0bbc5/go.mod h1:E0E2H2gQA+uoi27VCSU+a/BULPtadQA78q3cpTjZbZw= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= -github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce h1:fb190+cK2Xz/dvi9Hv8eCYJYvIGUTN2/KLq1pT6CjEc= -github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce/go.mod h1:o8v6yHRoik09Xen7gje4m9ERNah1d1PPsVq1VEx9vE4= github.com/tommy-muehle/go-mnd v1.3.1-0.20200224220436-e6f9a994e8fa h1:RC4maTWLKKwb7p1cnoygsbKIgNlJqSYBeAFON3Ar8As= github.com/tommy-muehle/go-mnd v1.3.1-0.20200224220436-e6f9a994e8fa/go.mod h1:dSUh0FtTP8VhvkL1S+gUR1OKd9ZnSaozuI6r3m6wOig= github.com/uber-go/atomic v1.3.2/go.mod h1:/Ct5t2lcmbJ4OSe/waGBoaVvVqtO0bmtfVNex1PFV8g= diff --git a/internal/config/flags.go b/internal/config/flags.go index 25bf37ac..6617a1ee 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -25,7 +25,7 @@ var ( daemonGID = flag.Uint("daemon-gid", 0, "Drop privileges to this group") daemonInplaceChroot = flag.Bool("daemon-inplace-chroot", false, "Fall back to a non-bind-mount chroot of -pages-root when daemonizing") propagateCorrelationID = flag.Bool("propagate-correlation-id", false, "Reuse existing Correlation-ID from the incoming request header `X-Request-ID` if present") - logFormat = flag.String("log-format", "text", "The log output format: 'text' or 'json'") + logFormat = flag.String("log-format", "json", "The log output format: 'text' or 'json'") logVerbose = flag.Bool("log-verbose", false, "Verbose logging") secret = flag.String("auth-secret", "", "Cookie store hash key, should be at least 32 bytes long") gitLabServer = flag.String("gitlab-server", "", "GitLab server, for example https://www.gitlab.com") @@ -40,7 +40,7 @@ var ( 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") + domainConfigSource = flag.String("domain-config-source", "gitlab", "Domain configuration source 'disk', 'auto' or 'gitlab' (default: 'gitlab'). DEPRECATED: gitlab-pages will use the API-based configuration starting from 14.3 see https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/5993") enableDisk = flag.Bool("enable-disk", true, "Enable disk access, shall be disabled in environments where shared disk storage isn't available") clientID = flag.String("auth-client-id", "", "GitLab application Client ID") diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 769856a2..10705a9a 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -16,7 +16,7 @@ func ConfigureLogging(format string, verbose bool) error { var levelOption log.LoggerOption if format == "" { - format = "text" + format = "json" } if verbose { diff --git a/internal/serving/serverless/certs.go b/internal/serving/serverless/certs.go deleted file mode 100644 index 674e8b25..00000000 --- a/internal/serving/serverless/certs.go +++ /dev/null @@ -1,26 +0,0 @@ -package serverless - -import ( - "crypto/tls" - "crypto/x509" -) - -// Certs holds definition of certificates we use to perform mTLS -// handshake with a cluster -type Certs struct { - RootCerts *x509.CertPool - Certificate tls.Certificate -} - -// NewClusterCerts creates a new cluster configuration from cert / key pair -func NewClusterCerts(clientCert, clientKey string) (*Certs, error) { - cert, err := tls.X509KeyPair([]byte(clientCert), []byte(clientKey)) - if err != nil { - return nil, err - } - - caCertPool := x509.NewCertPool() - caCertPool.AppendCertsFromPEM([]byte(clientCert)) - - return &Certs{RootCerts: caCertPool, Certificate: cert}, nil -} diff --git a/internal/serving/serverless/cluster.go b/internal/serving/serverless/cluster.go deleted file mode 100644 index 6bdd51da..00000000 --- a/internal/serving/serverless/cluster.go +++ /dev/null @@ -1,28 +0,0 @@ -package serverless - -import ( - "crypto/tls" - "strings" -) - -// Cluster represent a Knative cluster that we want to proxy requests to -type Cluster struct { - Address string // Address is a real IP address of a cluster ingress - Port string // Port is a real port of HTTP TLS service - Name string // Name is a cluster name, used in cluster certificates - Certs *Certs -} - -// Host returns a real cluster location based on IP address and port -func (c Cluster) Host() string { - return strings.Join([]string{c.Address, c.Port}, ":") -} - -// TLSConfig builds a new tls.Config and returns a pointer to it -func (c Cluster) TLSConfig() *tls.Config { - return &tls.Config{ - Certificates: []tls.Certificate{c.Certs.Certificate}, - RootCAs: c.Certs.RootCerts, - ServerName: c.Name, - } -} diff --git a/internal/serving/serverless/director.go b/internal/serving/serverless/director.go deleted file mode 100644 index 3f1bc99a..00000000 --- a/internal/serving/serverless/director.go +++ /dev/null @@ -1,20 +0,0 @@ -package serverless - -import ( - "net/http" - - "github.com/tomasen/realip" -) - -// NewDirectorFunc returns a director function capable of configuring a proxy -// request -func NewDirectorFunc(service string) func(*http.Request) { - return func(request *http.Request) { - request.Host = service - request.URL.Host = service - request.URL.Scheme = "https" - request.Header.Set("User-Agent", "GitLab Pages Daemon") - request.Header.Set("X-Forwarded-For", realip.FromRequest(request)) - request.Header.Set("X-Forwarded-Proto", "https") - } -} diff --git a/internal/serving/serverless/errors.go b/internal/serving/serverless/errors.go deleted file mode 100644 index d208a11d..00000000 --- a/internal/serving/serverless/errors.go +++ /dev/null @@ -1,26 +0,0 @@ -package serverless - -import ( - "encoding/json" - "net/http" -) - -// NewErrorHandler returns a func(http.ResponseWriter, *http.Request, error) -// responsible for handling proxy errors -func NewErrorHandler() func(http.ResponseWriter, *http.Request, error) { - return func(w http.ResponseWriter, r *http.Request, err error) { - w.WriteHeader(http.StatusInternalServerError) - - message := "cluster error: " + err.Error() - msgmap := map[string]string{"error": message} - - json, err := json.Marshal(msgmap) - if err != nil { - w.Write([]byte(message)) - return - } - - w.Header().Set("Content-Type", "application/json") - w.Write(json) - } -} diff --git a/internal/serving/serverless/serverless.go b/internal/serving/serverless/serverless.go deleted file mode 100644 index f8bd4e87..00000000 --- a/internal/serving/serverless/serverless.go +++ /dev/null @@ -1,73 +0,0 @@ -package serverless - -import ( - "errors" - "net/http/httputil" - - "gitlab.com/gitlab-org/gitlab-pages/internal/config" - "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/api" - "gitlab.com/gitlab-org/gitlab-pages/metrics" -) - -// Serverless is a servering used to proxy requests between a client and -// Knative cluster. -type Serverless struct { - proxy *httputil.ReverseProxy -} - -// NewFromAPISource returns a serverless serving instance built from GitLab API -// response -func NewFromAPISource(config api.Serverless) (serving.Serving, error) { - if len(config.Service) == 0 { - return nil, errors.New("incomplete serverless serving config") - } - - certs, err := NewClusterCerts( - config.Cluster.CertificateCert, - config.Cluster.CertificateKey, - ) - if err != nil { - return nil, err - } - - cluster := Cluster{ - Name: config.Cluster.Hostname, - Address: config.Cluster.Address, - Port: config.Cluster.Port, - Certs: certs, - } - - return New(config.Service, cluster), nil -} - -// New returns a new serving instance -func New(service string, cluster Cluster) serving.Serving { - proxy := httputil.ReverseProxy{ - Director: NewDirectorFunc(service), - Transport: NewTransport(cluster), - ErrorHandler: NewErrorHandler(), - } - - return &Serverless{proxy: &proxy} -} - -// ServeFileHTTP handle an incoming request and proxies it to Knative cluster -func (s *Serverless) ServeFileHTTP(h serving.Handler) bool { - metrics.ServerlessRequests.Inc() - - s.proxy.ServeHTTP(h.Writer, h.Request) - - return true -} - -// ServeNotFoundHTTP responds with 404 -func (s *Serverless) ServeNotFoundHTTP(h serving.Handler) { - httperrors.Serve404(h.Writer) -} - -// Reconfigure noop -func (s *Serverless) Reconfigure(*config.Config) error { - return nil -} diff --git a/internal/serving/serverless/serverless_test.go b/internal/serving/serverless/serverless_test.go deleted file mode 100644 index ebd14343..00000000 --- a/internal/serving/serverless/serverless_test.go +++ /dev/null @@ -1,165 +0,0 @@ -package serverless - -import ( - "crypto/tls" - "io/ioutil" - "net/http" - "net/http/httptest" - "net/url" - "testing" - - "github.com/stretchr/testify/require" - - "gitlab.com/gitlab-org/gitlab-pages/internal/fixture" - "gitlab.com/gitlab-org/gitlab-pages/internal/serving" -) - -func withTestCluster(t *testing.T, cert, key string, block func(*http.ServeMux, *url.URL, *Certs)) { - mux := http.NewServeMux() - cluster := httptest.NewUnstartedServer(mux) - - certs, err := NewClusterCerts(fixture.Certificate, fixture.Key) - require.NoError(t, err) - - cluster.TLS = &tls.Config{ - Certificates: []tls.Certificate{certs.Certificate}, - RootCAs: certs.RootCerts, - } - - cluster.StartTLS() - defer cluster.Close() - - address, err := url.Parse(cluster.URL) - require.NoError(t, err) - - block(mux, address, certs) -} - -func TestServeFileHTTP(t *testing.T) { - t.Run("when proxying simple request to a cluster", func(t *testing.T) { - withTestCluster(t, fixture.Certificate, fixture.Key, func(mux *http.ServeMux, server *url.URL, certs *Certs) { - serverless := New( - "my-func.my-namespace-123.knative.example.com", - Cluster{ - Name: "knative.gitlab-example.com", - Address: server.Hostname(), - Port: server.Port(), - Certs: certs, - }, - ) - - writer := httptest.NewRecorder() - request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil) - handler := serving.Handler{Writer: writer, Request: request} - request.Header.Set("X-Real-IP", "127.0.0.105") - - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, "my-func.my-namespace-123.knative.example.com", r.Host) - require.Equal(t, "GitLab Pages Daemon", r.Header.Get("User-Agent")) - require.Equal(t, "https", r.Header.Get("X-Forwarded-Proto")) - require.Contains(t, r.Header.Get("X-Forwarded-For"), "127.0.0.105") - }) - - served := serverless.ServeFileHTTP(handler) - result := writer.Result() - - require.True(t, served) - require.Equal(t, http.StatusOK, result.StatusCode) - }) - }) - - t.Run("when proxying request with invalid hostname", func(t *testing.T) { - withTestCluster(t, fixture.Certificate, fixture.Key, func(mux *http.ServeMux, server *url.URL, certs *Certs) { - serverless := New( - "my-func.my-namespace-123.knative.example.com", - Cluster{ - Name: "knative.invalid-gitlab-example.com", - Address: server.Hostname(), - Port: server.Port(), - Certs: certs, - }, - ) - - writer := httptest.NewRecorder() - request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil) - handler := serving.Handler{Writer: writer, Request: request} - - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusBadRequest) - }) - - served := serverless.ServeFileHTTP(handler) - result := writer.Result() - body, err := ioutil.ReadAll(writer.Body) - require.NoError(t, err) - - require.True(t, served) - require.Equal(t, http.StatusInternalServerError, result.StatusCode) - require.Contains(t, string(body), "cluster error: x509: certificate") - }) - }) - - t.Run("when a cluster responds with an error", func(t *testing.T) { - withTestCluster(t, fixture.Certificate, fixture.Key, func(mux *http.ServeMux, server *url.URL, certs *Certs) { - serverless := New( - "my-func.my-namespace-123.knative.example.com", - Cluster{ - Name: "knative.gitlab-example.com", - Address: server.Hostname(), - Port: server.Port(), - Certs: certs, - }, - ) - - writer := httptest.NewRecorder() - request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil) - handler := serving.Handler{Writer: writer, Request: request} - - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - w.Write([]byte("sorry, service unavailable")) - }) - - served := serverless.ServeFileHTTP(handler) - result := writer.Result() - body, err := ioutil.ReadAll(writer.Body) - require.NoError(t, err) - - require.True(t, served) - require.Equal(t, http.StatusServiceUnavailable, result.StatusCode) - require.Contains(t, string(body), "sorry, service unavailable") - }) - }) - - t.Run("when a cluster responds correctly", func(t *testing.T) { - withTestCluster(t, fixture.Certificate, fixture.Key, func(mux *http.ServeMux, server *url.URL, certs *Certs) { - serverless := New( - "my-func.my-namespace-123.knative.example.com", - Cluster{ - Name: "knative.gitlab-example.com", - Address: server.Hostname(), - Port: server.Port(), - Certs: certs, - }, - ) - - writer := httptest.NewRecorder() - request := httptest.NewRequest("GET", "http://example.gitlab.com/", nil) - handler := serving.Handler{Writer: writer, Request: request} - - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write([]byte("OK")) - }) - - served := serverless.ServeFileHTTP(handler) - result := writer.Result() - body, err := ioutil.ReadAll(writer.Body) - require.NoError(t, err) - - require.True(t, served) - require.Equal(t, http.StatusOK, result.StatusCode) - require.Contains(t, string(body), "OK") - }) - }) -} diff --git a/internal/serving/serverless/transport.go b/internal/serving/serverless/transport.go deleted file mode 100644 index b7fabb13..00000000 --- a/internal/serving/serverless/transport.go +++ /dev/null @@ -1,51 +0,0 @@ -package serverless - -import ( - "context" - "net" - "net/http" - "time" - - "gitlab.com/gitlab-org/gitlab-pages/metrics" -) - -// Transport is a struct that handle the proxy connection round trip to Knative -// cluster -type Transport struct { - cluster Cluster - transport *http.Transport -} - -// NewTransport fabricates as new transport type -func NewTransport(cluster Cluster) *Transport { - dialer := net.Dialer{ - Timeout: 4 * time.Minute, - KeepAlive: 6 * time.Minute, - } - - dialContext := func(ctx context.Context, network, address string) (net.Conn, error) { - address = cluster.Host() - - return dialer.DialContext(ctx, network, address) - } - - return &Transport{ - cluster: cluster, - transport: &http.Transport{ - DialContext: dialContext, - TLSHandshakeTimeout: 5 * time.Second, - TLSClientConfig: cluster.TLSConfig(), - }, - } -} - -// RoundTrip performs a connection to a Knative cluster and returns a response -func (t *Transport) RoundTrip(request *http.Request) (*http.Response, error) { - start := time.Now() - - response, err := t.transport.RoundTrip(request) - - metrics.ServerlessLatency.Observe(time.Since(start).Seconds()) - - return response, err -} diff --git a/internal/source/domains.go b/internal/source/domains.go index 7507938a..30feca32 100644 --- a/internal/source/domains.go +++ b/internal/source/domains.go @@ -2,7 +2,6 @@ package source import ( "fmt" - "regexp" "gitlab.com/gitlab-org/labkit/log" @@ -12,13 +11,6 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab" ) -var ( - // serverlessDomainRegex is a regular expression we use to check if a domain - // is a serverless domain, to short circuit gitlab source rollout. It can be - // removed after the rollout is done - serverlessDomainRegex = regexp.MustCompile(`^[^.]+-[[:xdigit:]]{2}a1[[:xdigit:]]{10}f2[[:xdigit:]]{2}[[:xdigit:]]+-?.*`) -) - type configSource int const ( @@ -134,13 +126,6 @@ func (d *Domains) IsReady() bool { } func (d *Domains) source(domain string) Source { - // This check is only needed until we enable `d.gitlab` source in all - // environments (including on-premises installations) followed by removal of - // `d.disk` source. This can be safely removed afterwards. - if IsServerlessDomain(domain) { - return d.gitlab - } - switch d.configSource { case sourceDisk: return d.disk @@ -154,13 +139,3 @@ func (d *Domains) source(domain string) Source { return d.disk } } - -// IsServerlessDomain checks if a domain requested is a serverless domain we -// need to handle differently. -// -// Domain is a serverless domain when it matches `serverlessDomainRegex`. The -// regular expression is also defined on the gitlab-rails side, see -// https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/serverless/domain.rb#L7 -func IsServerlessDomain(domain string) bool { - return serverlessDomainRegex.MatchString(domain) -} diff --git a/internal/source/domains_test.go b/internal/source/domains_test.go index a9404eef..7c351718 100644 --- a/internal/source/domains_test.go +++ b/internal/source/domains_test.go @@ -149,37 +149,6 @@ func TestGetDomain(t *testing.T) { require.NoError(t, err) require.Nil(t, domain) }) - - t.Run("when requesting a serverless domain", func(t *testing.T) { - testDomain := "func-aba1aabbccddeef2abaabbcc.serverless.gitlab.io" - - newSource := NewMockSource() - newSource.On("GetDomain", testDomain). - Return(&domain.Domain{Name: testDomain}, nil). - Once() - - defer newSource.AssertExpectations(t) - - domains := newTestDomains(t, newSource, sourceGitlab) - - domain, err := domains.GetDomain(testDomain) - require.NoError(t, err) - require.NotNil(t, domain) - }) -} - -func TestIsServerlessDomain(t *testing.T) { - t.Run("when a domain is serverless domain", func(t *testing.T) { - require.True(t, IsServerlessDomain("some-function-aba1aabbccddeef2abaabbcc.serverless.gitlab.io")) - }) - - t.Run("when a domain is serverless domain with environment", func(t *testing.T) { - require.True(t, IsServerlessDomain("some-function-aba1aabbccddeef2abaabbcc-testing.serverless.gitlab.io")) - }) - - t.Run("when a domain is not a serverless domain", func(t *testing.T) { - require.False(t, IsServerlessDomain("somedomain.gitlab.io")) - }) } func newTestDomains(t *testing.T, gitlabSource *MockSource, config configSource) *Domains { diff --git a/internal/source/gitlab/api/lookup_path.go b/internal/source/gitlab/api/lookup_path.go index 77b264ff..0a765217 100644 --- a/internal/source/gitlab/api/lookup_path.go +++ b/internal/source/gitlab/api/lookup_path.go @@ -11,22 +11,6 @@ type LookupPath struct { // Source describes GitLab Page serving variant type Source struct { - Type string `json:"type,omitempty"` - Path string `json:"path,omitempty"` - Serverless Serverless `json:"serverless,omitempty"` -} - -// Serverless describes serverless serving configuration -type Serverless struct { - Service string `json:"service,omitempty"` - Cluster Cluster `json:"cluster,omitempty"` -} - -// Cluster describes serverless cluster configuration -type Cluster struct { - Address string `json:"address,omitempty"` - Port string `json:"port,omitempty"` - Hostname string `json:"hostname,omitempty"` - CertificateCert string `json:"cert,omitempty"` - CertificateKey string `json:"key,omitempty"` + Type string `json:"type,omitempty"` + Path string `json:"path,omitempty"` } diff --git a/internal/source/gitlab/cache/retriever.go b/internal/source/gitlab/cache/retriever.go index 865bf88b..656ccce6 100644 --- a/internal/source/gitlab/cache/retriever.go +++ b/internal/source/gitlab/cache/retriever.go @@ -42,7 +42,11 @@ func (r *Retriever) Retrieve(domain string) (lookup api.Lookup) { log.Debug("retrieval context done") lookup = api.Lookup{Error: errors.New("retrieval context done")} case lookup = <-r.resolveWithBackoff(ctx, domain): - log.Debug("retrieval response sent") + log.WithFields(log.Fields{ + "lookup_name": lookup.Name, + "lookup_paths": lookup.Domain, + "lookup_error": lookup.Error, + }).Debug("retrieval response sent") } return lookup diff --git a/internal/source/gitlab/client/testdata/group-first.gitlab.io.json b/internal/source/gitlab/client/testdata/group-first.gitlab.io.json new file mode 100644 index 00000000..2f055654 --- /dev/null +++ b/internal/source/gitlab/client/testdata/group-first.gitlab.io.json @@ -0,0 +1,26 @@ +{ + "certificate": "some--cert", + "key": "some--key", + "lookup_paths": [ + { + "access_control": false, + "https_only": true, + "prefix": "/", + "project_id": 1, + "source": { + "path": "some/path/group/", + "type": "file" + } + }, + { + "access_control": false, + "https_only": true, + "prefix": "/my/second-project/", + "project_id": 2, + "source": { + "path": "some/path/to/project-2/", + "type": "file" + } + } + ] +} diff --git a/internal/source/gitlab/factory.go b/internal/source/gitlab/factory.go index 700d3b8e..027150fe 100644 --- a/internal/source/gitlab/factory.go +++ b/internal/source/gitlab/factory.go @@ -45,20 +45,6 @@ func (g *Gitlab) fabricateServing(lookup api.LookupPath) (serving.Serving, error return local.Instance(), nil case "zip": return zip.Instance(), nil - case "serverless": - log.Errorf("attempted to fabricate serverless serving for project %d", lookup.ProjectID) - - // This feature has been disalbed, for more details see - // https://gitlab.com/gitlab-org/gitlab-pages/-/issues/467 - // - // serving, err := serverless.NewFromAPISource(source.Serverless) - // if err != nil { - // log.WithError(err).Errorf("could not fabricate serving for project %d", lookup.ProjectID) - // - // break - // } - // - // return serving } return nil, fmt.Errorf("gitlab: unknown serving source type: %q", source.Type) diff --git a/internal/source/gitlab/factory_test.go b/internal/source/gitlab/factory_test.go index 54a46c38..705fdc47 100644 --- a/internal/source/gitlab/factory_test.go +++ b/internal/source/gitlab/factory_test.go @@ -1,12 +1,10 @@ package gitlab import ( - "fmt" "testing" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitlab-pages/internal/fixture" "gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk" "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api" ) @@ -59,32 +57,4 @@ func TestFabricateServing(t *testing.T) { require.EqualError(t, err, ErrDiskDisabled.Error()) require.Nil(t, srv) }) - - t.Run("when lookup path requires serverless serving", func(t *testing.T) { - g := Gitlab{} - - lookup := api.LookupPath{ - Prefix: "/", - Source: api.Source{ - Type: "serverless", - Serverless: api.Serverless{ - Service: "my-func.knative.example.com", - Cluster: api.Cluster{ - Address: "127.0.0.10", - Port: "443", - Hostname: "my-cluster.example.com", - CertificateCert: fixture.Certificate, - CertificateKey: fixture.Key, - }, - }, - }, - } - - srv, err := g.fabricateServing(lookup) - require.EqualError(t, err, fmt.Sprintf("gitlab: unknown serving source type: %q", lookup.Source.Type)) - - // Serverless serving has been deprecated. - // require.IsType(t, &serverless.Serverless{}, fabricateServing(lookup)) - require.Nil(t, srv) - }) } diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go index 88b2b312..e52a34f0 100644 --- a/internal/source/gitlab/gitlab.go +++ b/internal/source/gitlab/gitlab.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "path" + "sort" "strings" "sync" @@ -84,6 +85,8 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) { urlPath := path.Clean(r.URL.Path) size := len(response.Domain.LookupPaths) + sortLookupsByPrefixLengthDesc(response.Domain.LookupPaths) + for _, lookup := range response.Domain.LookupPaths { isSubPath := strings.HasPrefix(urlPath, lookup.Prefix) isRootPath := urlPath == path.Clean(lookup.Prefix) @@ -109,6 +112,15 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) { return nil, domain.ErrDomainDoesNotExist } +// Ensure lookupPaths are sorted by prefix length to ensure the group level +// domain with prefix "/" is the last one to be checked. +// See https://gitlab.com/gitlab-org/gitlab-pages/-/issues/576 +func sortLookupsByPrefixLengthDesc(lookups []api.LookupPath) { + sort.SliceStable(lookups, func(i, j int) bool { + return len(lookups[i].Prefix) > len(lookups[j].Prefix) + }) +} + // IsReady returns the value of Gitlab `isReady` which is updated by `Poll`. func (g *Gitlab) IsReady() bool { g.mu.RLock() diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go index 4a5f1a7b..0848af88 100644 --- a/internal/source/gitlab/gitlab_test.go +++ b/internal/source/gitlab/gitlab_test.go @@ -108,3 +108,46 @@ func TestResolve(t *testing.T) { require.Equal(t, "index.html", response.SubPath) }) } + +// Test proves fix for https://gitlab.com/gitlab-org/gitlab-pages/-/issues/576 +func TestResolveLookupPathsOrderDoesNotMatter(t *testing.T) { + client := client.StubClient{File: "client/testdata/group-first.gitlab.io.json"} + source := Gitlab{client: client, enableDisk: true} + + tests := map[string]struct { + target string + expectedPrefix string + expectedPath string + expectedSubPath string + expectedIsNamespace bool + }{ + "when requesting the group root project with root path": { + target: "https://group-first.gitlab.io:443/", + expectedPrefix: "/", + expectedPath: "some/path/group/", + expectedSubPath: "", + expectedIsNamespace: true, + }, + "when requesting another project with path": { + target: "https://group-first.gitlab.io:443/my/second-project/index.html", + expectedPrefix: "/my/second-project/", + expectedPath: "some/path/to/project-2/", + expectedSubPath: "index.html", + expectedIsNamespace: false, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + request := httptest.NewRequest("GET", test.target, nil) + + response, err := source.Resolve(request) + require.NoError(t, err) + + require.Equal(t, test.expectedPrefix, response.LookupPath.Prefix) + require.Equal(t, test.expectedPath, response.LookupPath.Path) + require.Equal(t, test.expectedSubPath, response.SubPath) + require.Equal(t, test.expectedIsNamespace, response.LookupPath.IsNamespaceProject) + }) + } +} diff --git a/metrics/metrics.go b/metrics/metrics.go index f4e27f9b..0ae9bc48 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -54,18 +54,6 @@ var ( Help: "The number of GitLab API calls that failed", }) - // ServerlessRequests measures the amount of serverless invocations - ServerlessRequests = prometheus.NewCounter(prometheus.CounterOpts{ - Name: "gitlab_pages_serverless_requests", - Help: "The number of total GitLab Serverless requests served", - }) - - // ServerlessLatency records serverless serving roundtrip duration - ServerlessLatency = prometheus.NewHistogram(prometheus.HistogramOpts{ - Name: "gitlab_pages_serverless_latency", - Help: "Serverless serving roundtrip duration", - }) - // DomainsSourceAPIReqTotal is the number of calls made to the GitLab API that returned a 4XX error DomainsSourceAPIReqTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "gitlab_pages_domains_source_api_requests_total", @@ -243,8 +231,6 @@ func MustRegister() { DomainsSourceAPICallDuration, DomainsSourceAPITraceDuration, DomainsSourceFailures, - ServerlessRequests, - ServerlessLatency, DiskServingFileSize, ServingTime, VFSOperations, @@ -24,6 +24,14 @@ type keepAliveSetter interface { SetKeepAlivePeriod(time.Duration) error } +type listenerConfig struct { + fd uintptr + isProxyV2 bool + tlsConfig *tls.Config + limiter *netutil.Limiter + handler http.Handler +} + func (ln *keepAliveListener) Accept() (net.Conn, error) { conn, err := ln.Listener.Accept() if err != nil { @@ -37,29 +45,29 @@ func (ln *keepAliveListener) Accept() (net.Conn, error) { return conn, nil } -func listenAndServe(fd uintptr, handler http.Handler, useHTTP2 bool, tlsConfig *tls.Config, limiter *netutil.Limiter, proxyv2 bool) error { +func (a *theApp) listenAndServe(config listenerConfig) error { // create server - server := &http.Server{Handler: context.ClearHandler(handler), TLSConfig: tlsConfig} + server := &http.Server{Handler: context.ClearHandler(config.handler), TLSConfig: config.tlsConfig} - if useHTTP2 { + if a.config.General.HTTP2 { err := http2.ConfigureServer(server, &http2.Server{}) if err != nil { return err } } - l, err := net.FileListener(os.NewFile(fd, "[socket]")) + l, err := net.FileListener(os.NewFile(config.fd, "[socket]")) if err != nil { - return fmt.Errorf("failed to listen on FD %d: %v", fd, err) + return fmt.Errorf("failed to listen on FD %d: %v", config.fd, err) } - if limiter != nil { - l = netutil.SharedLimitListener(l, limiter) + if config.limiter != nil { + l = netutil.SharedLimitListener(l, config.limiter) } l = &keepAliveListener{l} - if proxyv2 { + if config.isProxyV2 { l = &proxyproto.Listener{ Listener: l, Policy: func(upstream net.Addr) (proxyproto.Policy, error) { @@ -68,7 +76,7 @@ func listenAndServe(fd uintptr, handler http.Handler, useHTTP2 bool, tlsConfig * } } - if tlsConfig != nil { + if config.tlsConfig != nil { l = tls.NewListener(l, server.TLSConfig) } diff --git a/shared/pages/@hashed/67/06/670671cd97404156226e507973f2ab8330d3022ca96e0c93bdbdb320c41adcaf/pages_deployments/01/artifacts.zip b/shared/pages/@hashed/zip-from-disk.gitlab.io/public.zip Binary files differindex f1278bce..f1278bce 100644 --- a/shared/pages/@hashed/67/06/670671cd97404156226e507973f2ab8330d3022ca96e0c93bdbdb320c41adcaf/pages_deployments/01/artifacts.zip +++ b/shared/pages/@hashed/zip-from-disk.gitlab.io/public.zip diff --git a/shared/pages/CapitalGroup/CapitalProject/public.zip b/shared/pages/CapitalGroup/CapitalProject/public.zip Binary files differnew file mode 100644 index 00000000..40dec60f --- /dev/null +++ b/shared/pages/CapitalGroup/CapitalProject/public.zip diff --git a/shared/pages/CapitalGroup/project/public.zip b/shared/pages/CapitalGroup/project/public.zip Binary files differnew file mode 100644 index 00000000..a575b782 --- /dev/null +++ b/shared/pages/CapitalGroup/project/public.zip diff --git a/shared/pages/group.404/domain.404/public.zip b/shared/pages/group.404/domain.404/public.zip Binary files differnew file mode 100644 index 00000000..2e72c112 --- /dev/null +++ b/shared/pages/group.404/domain.404/public.zip diff --git a/shared/pages/group.404/group.404.gitlab-example.com/public.zip b/shared/pages/group.404/group.404.gitlab-example.com/public.zip Binary files differnew file mode 100644 index 00000000..66d27243 --- /dev/null +++ b/shared/pages/group.404/group.404.gitlab-example.com/public.zip diff --git a/shared/pages/group.404/project.404/public.zip b/shared/pages/group.404/project.404/public.zip Binary files differnew file mode 100644 index 00000000..7f291f05 --- /dev/null +++ b/shared/pages/group.404/project.404/public.zip diff --git a/shared/pages/group.404/project.no.404/public.zip b/shared/pages/group.404/project.no.404/public.zip Binary files differnew file mode 100644 index 00000000..ae3b8b76 --- /dev/null +++ b/shared/pages/group.404/project.no.404/public.zip diff --git a/shared/pages/group.acme/with.acme.challenge/public.zip b/shared/pages/group.acme/with.acme.challenge/public.zip Binary files differnew file mode 100644 index 00000000..1eadb231 --- /dev/null +++ b/shared/pages/group.acme/with.acme.challenge/public.zip diff --git a/shared/pages/group.https-only/project1/public.zip b/shared/pages/group.https-only/project1/public.zip Binary files differnew file mode 100644 index 00000000..ac74f320 --- /dev/null +++ b/shared/pages/group.https-only/project1/public.zip diff --git a/shared/pages/group.https-only/project2/public.zip b/shared/pages/group.https-only/project2/public.zip Binary files differnew file mode 100644 index 00000000..ead779f5 --- /dev/null +++ b/shared/pages/group.https-only/project2/public.zip diff --git a/shared/pages/group/CapitalProject/public.zip b/shared/pages/group/CapitalProject/public.zip Binary files differnew file mode 100644 index 00000000..70d45619 --- /dev/null +++ b/shared/pages/group/CapitalProject/public.zip diff --git a/shared/pages/group/group.gitlab-example.com/public.zip b/shared/pages/group/group.gitlab-example.com/public.zip Binary files differnew file mode 100644 index 00000000..397e5563 --- /dev/null +++ b/shared/pages/group/group.gitlab-example.com/public.zip diff --git a/shared/pages/group/new-source-test.gitlab.io/public.zip b/shared/pages/group/new-source-test.gitlab.io/public.zip Binary files differnew file mode 100644 index 00000000..01984f66 --- /dev/null +++ b/shared/pages/group/new-source-test.gitlab.io/public.zip diff --git a/shared/pages/group/project/public.zip b/shared/pages/group/project/public.zip Binary files differnew file mode 100644 index 00000000..d9cadcae --- /dev/null +++ b/shared/pages/group/project/public.zip diff --git a/shared/pages/group/project2/public.zip b/shared/pages/group/project2/public.zip Binary files differnew file mode 100644 index 00000000..fee628f7 --- /dev/null +++ b/shared/pages/group/project2/public.zip diff --git a/shared/pages/group/serving/public.zip b/shared/pages/group/serving/public.zip Binary files differnew file mode 100644 index 00000000..a36adb45 --- /dev/null +++ b/shared/pages/group/serving/public.zip diff --git a/shared/pages/group/subgroup/project/public.zip b/shared/pages/group/subgroup/project/public.zip Binary files differnew file mode 100644 index 00000000..ce310c44 --- /dev/null +++ b/shared/pages/group/subgroup/project/public.zip diff --git a/test/acceptance/acme_test.go b/test/acceptance/acme_test.go index eb8f2160..c7a33796 100644 --- a/test/acceptance/acme_test.go +++ b/test/acceptance/acme_test.go @@ -9,65 +9,95 @@ import ( "github.com/stretchr/testify/require" ) +const ( + existingAcmeTokenPath = "/.well-known/acme-challenge/existingtoken" + notExistingAcmeTokenPath = "/.well-known/acme-challenge/notexistingtoken" +) + func TestAcmeChallengesWhenItIsNotConfigured(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "") - defer teardown() - - t.Run("When domain folder contains requested acme challenge it responds with it", func(t *testing.T) { - rsp, err := GetRedirectPage(t, httpListener, "withacmechallenge.domain.com", - existingAcmeTokenPath) + RunPagesProcessWithStubGitLabServer(t, + withListeners([]ListenSpec{httpListener}), + ) - defer rsp.Body.Close() - require.NoError(t, err) - require.Equal(t, http.StatusOK, rsp.StatusCode) - body, _ := ioutil.ReadAll(rsp.Body) - require.Equal(t, "this is token\n", string(body)) - }) + tests := map[string]struct { + token string + expectedStatus int + expectedContent string + }{ + "When domain folder contains requested acme challenge it responds with it": { + token: existingAcmeTokenPath, + expectedStatus: http.StatusOK, + expectedContent: "this is token\n", + }, + "When domain folder does not contain requested acme challenge it returns 404": { + token: notExistingAcmeTokenPath, + expectedStatus: http.StatusNotFound, + expectedContent: "The page you're looking for could not be found.", + }, + } - t.Run("When domain folder doesn't contains requested acme challenge it returns 404", - func(t *testing.T) { + for name, test := range tests { + t.Run(name, func(t *testing.T) { rsp, err := GetRedirectPage(t, httpListener, "withacmechallenge.domain.com", - notExistingAcmeTokenPath) + test.token) defer rsp.Body.Close() require.NoError(t, err) - require.Equal(t, http.StatusNotFound, rsp.StatusCode) - }, - ) + require.Equal(t, test.expectedStatus, rsp.StatusCode) + body, err := ioutil.ReadAll(rsp.Body) + require.NoError(t, err) + + require.Contains(t, string(body), test.expectedContent) + }) + } } func TestAcmeChallengesWhenItIsConfigured(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-gitlab-server=https://gitlab-acme.com") - defer teardown() - - t.Run("When domain folder contains requested acme challenge it responds with it", func(t *testing.T) { - rsp, err := GetRedirectPage(t, httpListener, "withacmechallenge.domain.com", - existingAcmeTokenPath) + RunPagesProcessWithStubGitLabServer(t, + withListeners([]ListenSpec{httpListener}), + withExtraArgument("gitlab-server", "https://gitlab-acme.com"), + ) - defer rsp.Body.Close() - require.NoError(t, err) - require.Equal(t, http.StatusOK, rsp.StatusCode) - body, _ := ioutil.ReadAll(rsp.Body) - require.Equal(t, "this is token\n", string(body)) - }) + tests := map[string]struct { + token string + expectedStatus int + expectedContent string + expectedLocation string + }{ + "When domain folder contains requested acme challenge it responds with it": { + token: existingAcmeTokenPath, + expectedStatus: http.StatusOK, + expectedContent: "this is token\n", + }, + "When domain folder doesn't contains requested acme challenge it redirects to GitLab": { + token: notExistingAcmeTokenPath, + expectedStatus: http.StatusTemporaryRedirect, + expectedContent: "", + expectedLocation: "https://gitlab-acme.com/-/acme-challenge?domain=withacmechallenge.domain.com&token=notexistingtoken", + }, + } - t.Run("When domain folder doesn't contains requested acme challenge it redirects to GitLab", - func(t *testing.T) { + for name, test := range tests { + t.Run(name, func(t *testing.T) { rsp, err := GetRedirectPage(t, httpListener, "withacmechallenge.domain.com", - notExistingAcmeTokenPath) + test.token) defer rsp.Body.Close() require.NoError(t, err) - require.Equal(t, http.StatusTemporaryRedirect, rsp.StatusCode) + require.Equal(t, test.expectedStatus, rsp.StatusCode) + body, err := ioutil.ReadAll(rsp.Body) + require.NoError(t, err) + + require.Contains(t, string(body), test.expectedContent) - url, err := url.Parse(rsp.Header.Get("Location")) + redirectURL, err := url.Parse(rsp.Header.Get("Location")) require.NoError(t, err) - require.Equal(t, url.String(), "https://gitlab-acme.com/-/acme-challenge?domain=withacmechallenge.domain.com&token=notexistingtoken") - }, - ) + require.Equal(t, redirectURL.String(), test.expectedLocation) + }) + } } diff --git a/test/acceptance/config_test.go b/test/acceptance/config_test.go index 93e9aa22..aa568969 100644 --- a/test/acceptance/config_test.go +++ b/test/acceptance/config_test.go @@ -4,7 +4,6 @@ import ( "fmt" "net" "net/http" - "os" "testing" "github.com/stretchr/testify/require" @@ -12,11 +11,14 @@ import ( func TestEnvironmentVariablesConfig(t *testing.T) { skipUnlessEnabled(t) - os.Setenv("LISTEN_HTTP", net.JoinHostPort(httpListener.Host, httpListener.Port)) - defer func() { os.Unsetenv("LISTEN_HTTP") }() - teardown := RunPagesProcessWithoutWait(t, *pagesBinary, []ListenSpec{}, "") - defer teardown() + envVarValue := "LISTEN_HTTP=" + net.JoinHostPort(httpListener.Host, httpListener.Port) + + RunPagesProcessWithStubGitLabServer(t, + withoutWait, + withListeners([]ListenSpec{}), // explicitly disable listeners for this test + withEnv([]string{envVarValue}), + ) require.NoError(t, httpListener.WaitUntilRequestSucceeds(nil)) rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com:", "project/") @@ -28,11 +30,13 @@ func TestEnvironmentVariablesConfig(t *testing.T) { func TestMixedConfigSources(t *testing.T) { skipUnlessEnabled(t) - os.Setenv("LISTEN_HTTP", net.JoinHostPort(httpListener.Host, httpListener.Port)) - defer func() { os.Unsetenv("LISTEN_HTTP") }() + envVarValue := "LISTEN_HTTP=" + net.JoinHostPort(httpListener.Host, httpListener.Port) - teardown := RunPagesProcessWithoutWait(t, *pagesBinary, []ListenSpec{httpsListener}, "") - defer teardown() + RunPagesProcessWithStubGitLabServer(t, + withoutWait, + withListeners([]ListenSpec{httpsListener}), + withEnv([]string{envVarValue}), + ) for _, listener := range []ListenSpec{httpListener, httpsListener} { require.NoError(t, listener.WaitUntilRequestSucceeds(nil)) @@ -44,16 +48,17 @@ func TestMixedConfigSources(t *testing.T) { } } -func TestMultiFlagEnvironmentVariables(t *testing.T) { +func TestMultipleListenersFromEnvironmentVariables(t *testing.T) { skipUnlessEnabled(t) - listenSpecs := []ListenSpec{{"http", "127.0.0.1", "37001"}, {"http", "127.0.0.1", "37002"}} - envVarValue := fmt.Sprintf("%s,%s", net.JoinHostPort("127.0.0.1", "37001"), net.JoinHostPort("127.0.0.1", "37002")) - os.Setenv("LISTEN_HTTP", envVarValue) - defer func() { os.Unsetenv("LISTEN_HTTP") }() + listenSpecs := []ListenSpec{{"http", "127.0.0.1", "37001"}, {"http", "127.0.0.1", "37002"}} + envVarValue := fmt.Sprintf("LISTEN_HTTP=%s,%s", net.JoinHostPort("127.0.0.1", "37001"), net.JoinHostPort("127.0.0.1", "37002")) - teardown := RunPagesProcess(t, *pagesBinary, []ListenSpec{}, "") - defer teardown() + RunPagesProcessWithStubGitLabServer(t, + withoutWait, + withListeners([]ListenSpec{}), // explicitly disable listeners for this test + withEnv([]string{envVarValue}), + ) for _, listener := range listenSpecs { require.NoError(t, listener.WaitUntilRequestSucceeds(nil)) diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index ba5790a3..2b95cf4b 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -25,6 +25,7 @@ import ( "golang.org/x/net/nettest" "gitlab.com/gitlab-org/gitlab-pages/internal/request" + "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" "gitlab.com/gitlab-org/gitlab-pages/test/acceptance/testdata" ) @@ -90,9 +91,6 @@ var ( return conn, err } - - existingAcmeTokenPath = "/.well-known/acme-challenge/existingtoken" - notExistingAcmeTokenPath = "/.well-known/acme-challenge/notexistingtoken" ) type tWriter struct { @@ -232,18 +230,37 @@ func RunPagesProcessWithOutput(t *testing.T, pagesBinary string, listeners []Lis return runPagesProcess(t, true, pagesBinary, listeners, promPort, nil, extraArgs...) } -func RunPagesProcessWithStubGitLabServer(t *testing.T, wait bool, pagesBinary string, listeners []ListenSpec, promPort string, envs []string, extraArgs ...string) (*LogCaptureBuffer, func()) { - source := NewGitlabDomainsSourceStub(t, &stubOpts{}) +func RunPagesProcessWithStubGitLabServer(t *testing.T, opts ...processOption) *LogCaptureBuffer { + chdir := false + chdirCleanup := testhelpers.ChdirInPath(t, "../../shared/pages", &chdir) + + wd, err := os.Getwd() + require.NoError(t, err) + + processCfg := defaultProcessConfig + + for _, opt := range opts { + opt(&processCfg) + } + + if processCfg.gitlabStubOpts.pagesRoot == "" { + processCfg.gitlabStubOpts.pagesRoot = wd + } + + source := NewGitlabDomainsSourceStub(t, processCfg.gitlabStubOpts) gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t) - pagesArgs := append([]string{"-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab"}, extraArgs...) + processCfg.extraArgs = append(processCfg.extraArgs, "-pages-root", wd, "-internal-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab") - logBuf, cleanup := runPagesProcess(t, wait, pagesBinary, listeners, promPort, envs, pagesArgs...) + logBuf, cleanup := runPagesProcess(t, processCfg.wait, processCfg.pagesBinary, processCfg.listeners, "", processCfg.envs, processCfg.extraArgs...) - return logBuf, func() { + t.Cleanup(func() { source.Close() + chdirCleanup() cleanup() - } + }) + + return logBuf } func RunPagesProcessWithAuth(t *testing.T, pagesBinary string, listeners []ListenSpec, promPort string) func() { @@ -373,6 +390,14 @@ func getPagesArgs(t *testing.T, listeners []ListenSpec, promPort string, extraAr args = append(args, "-metrics-address", promPort) } + // most of our acceptance tests still work only with disk source + // TODO: remove this with -domain-config-source flag itself: + // https://gitlab.com/gitlab-org/gitlab-pages/-/issues/571 + // https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382 + if !contains(extraArgs, "-domain-config-source") { + args = append(args, "-domain-config-source", "disk") + } + args = append(args, getPagesDaemonArgs(t)...) args = append(args, extraArgs...) @@ -645,7 +670,7 @@ func defaultAPIHandler(t *testing.T, opts *stubOpts) http.HandlerFunc { // check if predefined response exists if responseFn, ok := testdata.DomainResponses[domain]; ok { - err := json.NewEncoder(w).Encode(responseFn(opts.pagesRoot)) + err := json.NewEncoder(w).Encode(responseFn(t, opts.pagesRoot)) require.NoError(t, err) return } diff --git a/test/acceptance/metrics_test.go b/test/acceptance/metrics_test.go index f8d7f276..6ba147aa 100644 --- a/test/acceptance/metrics_test.go +++ b/test/acceptance/metrics_test.go @@ -14,8 +14,10 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) { _, cleanup := newZipFileServerURL(t, "../../shared/pages/group/zip.gitlab.io/public.zip") defer cleanup() - _, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), ":42345", []string{}, "-max-conns=10") - defer teardown() + RunPagesProcessWithStubGitLabServer(t, + withExtraArgument("max-conns", "10"), + withExtraArgument("metrics-address", ":42345"), + ) // need to call an actual resource to populate certain metrics e.g. gitlab_pages_domains_source_api_requests_total res, err := GetPageFromListener(t, httpListener, "zip.gitlab.io", @@ -41,8 +43,6 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) { require.Contains(t, string(body), "gitlab_pages_domains_source_cache_hit") require.Contains(t, string(body), "gitlab_pages_domains_source_cache_miss") require.Contains(t, string(body), "gitlab_pages_domains_source_failures_total") - require.Contains(t, string(body), "gitlab_pages_serverless_requests 0") - require.Contains(t, string(body), "gitlab_pages_serverless_latency_sum 0") require.Contains(t, string(body), "gitlab_pages_disk_serving_file_size_bytes_sum") require.Contains(t, string(body), "gitlab_pages_serving_time_seconds_sum") require.Contains(t, string(body), `gitlab_pages_domains_source_api_requests_total{status_code="200"}`) diff --git a/test/acceptance/proxyv2_test.go b/test/acceptance/proxyv2_test.go index 670f711c..095007ba 100644 --- a/test/acceptance/proxyv2_test.go +++ b/test/acceptance/proxyv2_test.go @@ -21,14 +21,14 @@ func TestProxyv2(t *testing.T) { urlSuffix string expectedStatusCode int expectedContent string - expectedLog string + expectedLog []string }{ "basic_proxyv2_request": { host: "group.gitlab-example.com", urlSuffix: "project/", expectedStatusCode: http.StatusOK, expectedContent: "project-subdir\n", - expectedLog: "group.gitlab-example.com 10.1.1.1", + expectedLog: []string{"\"host\":\"group.gitlab-example.com\"", "\"remote_ip\":\"10.1.1.1\""}, }, } @@ -49,7 +49,9 @@ func TestProxyv2(t *testing.T) { // give the process enough time to write the log message require.Eventually(t, func() bool { - require.Contains(t, logBuf.String(), tt.expectedLog, "log mismatch") + for _, e := range tt.expectedLog { + require.Contains(t, logBuf.String(), e, "log mismatch") + } return true }, time.Second, time.Millisecond) }) diff --git a/test/acceptance/serving_test.go b/test/acceptance/serving_test.go index 45203c92..32291e52 100644 --- a/test/acceptance/serving_test.go +++ b/test/acceptance/serving_test.go @@ -4,7 +4,6 @@ import ( "fmt" "io/ioutil" "net/http" - "net/textproto" "os" "path" "strings" @@ -16,8 +15,8 @@ import ( func TestUnknownHostReturnsNotFound(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) for _, spec := range supportedListeners() { rsp, err := GetPageFromListener(t, spec, "invalid.invalid", "") @@ -30,8 +29,8 @@ func TestUnknownHostReturnsNotFound(t *testing.T) { func TestUnknownProjectReturnsNotFound(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "/nonexistent/") require.NoError(t, err) @@ -41,49 +40,60 @@ func TestUnknownProjectReturnsNotFound(t *testing.T) { func TestGroupDomainReturns200(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "/") require.NoError(t, err) defer rsp.Body.Close() require.Equal(t, http.StatusOK, rsp.StatusCode) + + body, err := ioutil.ReadAll(rsp.Body) + require.NoError(t, err) + + require.Equal(t, string(body), "OK\n") } func TestKnownHostReturns200(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) tests := []struct { - name string - host string - path string + name string + host string + path string + content string }{ { - name: "lower case", - host: "group.gitlab-example.com", - path: "project/", + name: "lower case", + host: "group.gitlab-example.com", + path: "project/", + content: "project-subdir\n", }, { - name: "capital project", - host: "group.gitlab-example.com", - path: "CapitalProject/", + name: "capital project", + host: "group.gitlab-example.com", + path: "CapitalProject/", + content: "Capital Project\n", }, { - name: "capital group", - host: "CapitalGroup.gitlab-example.com", - path: "project/", + name: "capital group", + host: "CapitalGroup.gitlab-example.com", + path: "project/", + content: "Capital Group\n", }, { - name: "capital group and project", - host: "CapitalGroup.gitlab-example.com", - path: "CapitalProject/", + name: "capital group and project", + host: "CapitalGroup.gitlab-example.com", + path: "CapitalProject/", + content: "Capital Group & Project\n", }, { - name: "subgroup", - host: "group.gitlab-example.com", - path: "subgroup/project/", + name: "subgroup", + host: "group.gitlab-example.com", + path: "subgroup/project/", + content: "A subgroup project\n", }, } @@ -91,15 +101,20 @@ func TestKnownHostReturns200(t *testing.T) { t.Run(tt.name, func(t *testing.T) { for _, spec := range supportedListeners() { rsp, err := GetPageFromListener(t, spec, tt.host, tt.path) + require.NoError(t, err) + require.Equal(t, http.StatusOK, rsp.StatusCode) + body, err := ioutil.ReadAll(rsp.Body) require.NoError(t, err) + require.Equal(t, tt.content, string(body)) + rsp.Body.Close() - require.Equal(t, http.StatusOK, rsp.StatusCode) } }) } } +// TODO: remove along with https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382 func TestNestedSubgroups(t *testing.T) { skipUnlessEnabled(t) @@ -148,8 +163,8 @@ func TestNestedSubgroups(t *testing.T) { func TestCustom404(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) tests := []struct { host string @@ -210,8 +225,8 @@ func TestCustom404(t *testing.T) { func TestCORSWhenDisabled(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-disable-cross-origin-requests") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t, withExtraArgument("disable-cross-origin-requests", "true")) for _, spec := range supportedListeners() { for _, method := range []string{"GET", "OPTIONS"} { @@ -226,8 +241,8 @@ func TestCORSWhenDisabled(t *testing.T) { func TestCORSAllowsGET(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) for _, spec := range supportedListeners() { for _, method := range []string{"GET", "OPTIONS"} { @@ -243,8 +258,7 @@ func TestCORSAllowsGET(t *testing.T) { func TestCORSForbidsPOST(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + RunPagesProcessWithStubGitLabServer(t) for _, spec := range supportedListeners() { rsp := doCrossOriginRequest(t, spec, "OPTIONS", "POST", spec.URL("project/")) @@ -258,8 +272,10 @@ func TestCORSForbidsPOST(t *testing.T) { func TestCustomHeaders(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-header", "X-Test1:Testing1", "-header", "X-Test2:Testing2") - defer teardown() + RunPagesProcessWithStubGitLabServer(t, + withExtraArgument("header", "X-Test1:Testing1"), + withExtraArgument("header", "X-Test2:Testing2"), + ) for _, spec := range supportedListeners() { rsp, err := GetPageFromListener(t, spec, "group.gitlab-example.com:", "project/") @@ -273,8 +289,7 @@ func TestCustomHeaders(t *testing.T) { func TestKnownHostWithPortReturns200(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + RunPagesProcessWithStubGitLabServer(t) for _, spec := range supportedListeners() { rsp, err := GetPageFromListener(t, spec, "group.gitlab-example.com:"+spec.Port, "project/") @@ -287,8 +302,8 @@ func TestKnownHostWithPortReturns200(t *testing.T) { func TestHttpToHttpsRedirectDisabled(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/") require.NoError(t, err) @@ -303,8 +318,8 @@ func TestHttpToHttpsRedirectDisabled(t *testing.T) { func TestHttpToHttpsRedirectEnabled(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "", "-redirect-http=true") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t, withExtraArgument("redirect-http", "true")) rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/") require.NoError(t, err) @@ -319,30 +334,40 @@ func TestHttpToHttpsRedirectEnabled(t *testing.T) { require.Equal(t, http.StatusOK, rsp.StatusCode) } -func TestHttpsOnlyGroupEnabled(t *testing.T) { - skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() +func TestHTTPSRedirect(t *testing.T) { + RunPagesProcessWithStubGitLabServer(t, + withListeners([]ListenSpec{httpListener}), + ) - rsp, err := GetRedirectPage(t, httpListener, "group.https-only.gitlab-example.com", "project1/") - require.NoError(t, err) - defer rsp.Body.Close() - require.Equal(t, http.StatusMovedPermanently, rsp.StatusCode) -} + tests := map[string]struct { + path string + expectedStatus int + }{ + "domain_https_only_true": { + path: "project1/", + expectedStatus: http.StatusMovedPermanently, + }, + "domain_https_only_false": { + path: "project2/", + expectedStatus: http.StatusOK, + }, + } -func TestHttpsOnlyGroupDisabled(t *testing.T) { - skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // see testdata/api_responses.go for per prefix configuration response from the API + rsp, err := GetRedirectPage(t, httpListener, "group.https-only.gitlab-example.com", test.path) + require.NoError(t, err) + defer rsp.Body.Close() - rsp, err := GetPageFromListener(t, httpListener, "group.https-only.gitlab-example.com", "project2/") - require.NoError(t, err) - defer rsp.Body.Close() - require.Equal(t, http.StatusOK, rsp.StatusCode) + require.Equal(t, test.expectedStatus, rsp.StatusCode) + }) + } } func TestHttpsOnlyProjectEnabled(t *testing.T) { skipUnlessEnabled(t) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -354,6 +379,7 @@ func TestHttpsOnlyProjectEnabled(t *testing.T) { func TestHttpsOnlyProjectDisabled(t *testing.T) { skipUnlessEnabled(t) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -365,6 +391,7 @@ func TestHttpsOnlyProjectDisabled(t *testing.T) { func TestHttpsOnlyDomainDisabled(t *testing.T) { skipUnlessEnabled(t) + teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") defer teardown() @@ -558,19 +585,13 @@ func TestGitLabSourceBecomesUnauthorized(t *testing.T) { func TestKnownHostInReverseProxySetupReturns200(t *testing.T) { skipUnlessEnabled(t) - specs := []ListenSpec{ - proxyListener, - } - teardown := RunPagesProcess(t, *pagesBinary, specs, "") - defer teardown() + RunPagesProcessWithStubGitLabServer(t) - for _, spec := range specs { - rsp, err := GetProxiedPageFromListener(t, spec, "localhost", "group.gitlab-example.com", "project/") + rsp, err := GetProxiedPageFromListener(t, proxyListener, "localhost", "group.gitlab-example.com", "project/") - require.NoError(t, err) - rsp.Body.Close() - require.Equal(t, http.StatusOK, rsp.StatusCode) - } + require.NoError(t, err) + rsp.Body.Close() + require.Equal(t, http.StatusOK, rsp.StatusCode) } func TestDomainResolverError(t *testing.T) { @@ -669,8 +690,8 @@ func doCrossOriginRequest(t *testing.T, spec ListenSpec, method, reqMethod, url func TestQueryStringPersistedInSlashRewrite(t *testing.T) { skipUnlessEnabled(t) - teardown := RunPagesProcess(t, *pagesBinary, supportedListeners(), "") - defer teardown() + + RunPagesProcessWithStubGitLabServer(t) rsp, err := GetRedirectPage(t, httpsListener, "group.gitlab-example.com", "project?q=test") require.NoError(t, err) @@ -706,9 +727,11 @@ func TestServerRepliesWithHeaders(t *testing.T) { for name, test := range tests { testFn := func(envArgs, headerArgs []string) func(*testing.T) { return func(t *testing.T) { - teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, []ListenSpec{httpListener}, "", envArgs, headerArgs...) - - defer teardown() + RunPagesProcessWithStubGitLabServer(t, + withListeners([]ListenSpec{httpListener}), + withEnv(envArgs), + withArguments(headerArgs), + ) rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "/") require.NoError(t, err) @@ -717,7 +740,7 @@ func TestServerRepliesWithHeaders(t *testing.T) { require.Equal(t, http.StatusOK, rsp.StatusCode) for key, value := range test.expectedHeaders { - got := headerValues(rsp.Header, key) + got := rsp.Header.Values(key) require.Equal(t, value, got) } } @@ -750,19 +773,12 @@ func TestServerRepliesWithHeaders(t *testing.T) { } } -func headerValues(header http.Header, key string) []string { - h := textproto.MIMEHeader(header) - - // NOTE: cannot use header.Values() in Go 1.13 or lower, this is the implementation - // from Go 1.15 https://github.com/golang/go/blob/release-branch.go1.15/src/net/textproto/header.go#L46 - return h[textproto.CanonicalMIMEHeaderKey(key)] -} - func TestDiskDisabledFailsToServeFileAndLocalContent(t *testing.T) { skipUnlessEnabled(t) - logBuf, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, []ListenSpec{httpListener}, "", nil, "-enable-disk=false") - defer teardown() + logBuf := RunPagesProcessWithStubGitLabServer(t, + withExtraArgument("enable-disk", "false"), + ) for host, suffix := range map[string]string{ // API serves "source": { "type": "local" } diff --git a/test/acceptance/stub_test.go b/test/acceptance/stub_test.go index 8f52ec37..3fcb179d 100644 --- a/test/acceptance/stub_test.go +++ b/test/acceptance/stub_test.go @@ -12,6 +12,53 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/fixture" ) +var defaultProcessConfig = processConfig{ + wait: true, + pagesBinary: *pagesBinary, + listeners: supportedListeners(), + envs: []string{}, + extraArgs: []string{}, + gitlabStubOpts: &stubOpts{}, +} + +type processConfig struct { + wait bool + pagesBinary string + listeners []ListenSpec + envs []string + extraArgs []string + gitlabStubOpts *stubOpts +} + +type processOption func(*processConfig) + +func withoutWait(config *processConfig) { + config.wait = false +} + +func withListeners(listeners []ListenSpec) processOption { + return func(config *processConfig) { + config.listeners = listeners + } +} + +func withEnv(envs []string) processOption { + return func(config *processConfig) { + config.envs = append(config.envs, envs...) + } +} + +func withExtraArgument(key, value string) processOption { + return func(config *processConfig) { + config.extraArgs = append(config.extraArgs, fmt.Sprintf("-%s=%s", key, value)) + } +} +func withArguments(args []string) processOption { + return func(config *processConfig) { + config.extraArgs = append(config.extraArgs, args...) + } +} + // makeGitLabPagesAccessStub provides a stub *httptest.Server to check pages_access API call. // the result is based on the project id. // diff --git a/test/acceptance/testdata/api_responses.go b/test/acceptance/testdata/api_responses.go index a79d2130..07349466 100644 --- a/test/acceptance/testdata/api_responses.go +++ b/test/acceptance/testdata/api_responses.go @@ -2,80 +2,153 @@ package testdata import ( "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api" ) -type responseFn func(string) api.VirtualDomain +type responseFn func(*testing.T, string) api.VirtualDomain // DomainResponses holds the predefined API responses for certain domains // that can be used with the GitLab API stub in acceptance tests +// Assume the working dir is inside shared/pages/ var DomainResponses = map[string]responseFn{ - "zip-from-disk.gitlab.io": ZipFromFile, - "zip-from-disk-not-found.gitlab.io": ZipFromFileNotFound, - "zip-not-allowed-path.gitlab.io": ZipFromNotAllowedPath, + "zip-from-disk.gitlab.io": customDomain(projectConfig{ + projectID: 123, + pathOnDisk: "@hashed/zip-from-disk.gitlab.io", + }), + "zip-from-disk-not-found.gitlab.io": customDomain(projectConfig{}), + // outside of working dir + "zip-not-allowed-path.gitlab.io": customDomain(projectConfig{pathOnDisk: "../../../../"}), + "group.gitlab-example.com": generateVirtualDomainFromDir("group", "group.gitlab-example.com", nil), + "CapitalGroup.gitlab-example.com": generateVirtualDomainFromDir("CapitalGroup", "CapitalGroup.gitlab-example.com", nil), + "group.404.gitlab-example.com": generateVirtualDomainFromDir("group.404", "group.404.gitlab-example.com", nil), + "group.https-only.gitlab-example.com": generateVirtualDomainFromDir("group.https-only", "group.https-only.gitlab-example.com", map[string]projectConfig{ + "/project1": { + projectID: 1000, + https: true, + }, + "/project2": { + projectID: 1100, + https: false, + }, + }), + "domain.404.com": customDomain(projectConfig{ + projectID: 1000, + pathOnDisk: "group.404/domain.404", + }), + "withacmechallenge.domain.com": customDomain(projectConfig{ + projectID: 1234, + pathOnDisk: "group.acme/with.acme.challenge", + }), + // NOTE: before adding more domains here, generate the zip archive by running (per project) + // make zip PROJECT_SUBDIR=group/serving + // make zip PROJECT_SUBDIR=group/project2 } -// ZipFromFile response for zip.gitlab.io -func ZipFromFile(wd string) api.VirtualDomain { - return api.VirtualDomain{ - Certificate: "", - Key: "", - LookupPaths: []api.LookupPath{ - { - ProjectID: 123, - AccessControl: false, - HTTPSOnly: false, - Prefix: "/", +// generateVirtualDomainFromDir walks the subdirectory inside of shared/pages/ to find any zip archives. +// It works for subdomains of pages-domain but not for custom domains (yet) +func generateVirtualDomainFromDir(dir, rootDomain string, perPrefixConfig map[string]projectConfig) responseFn { + return func(t *testing.T, wd string) api.VirtualDomain { + t.Helper() + + var foundZips []string + + // walk over dir and save any paths containing a `.zip` file + // $(GITLAB_PAGES_DIR)/shared/pages + "/" + group + + cleanDir := filepath.Join(wd, dir) + + // make sure resolved path inside dir is under wd to avoid https://securego.io/docs/rules/g304.html + require.Truef(t, strings.HasPrefix(cleanDir, wd), "path %q outside of wd %q", cleanDir, wd) + + filepath.Walk(cleanDir, func(path string, info os.FileInfo, err error) error { + require.NoError(t, err) + + if strings.HasSuffix(info.Name(), ".zip") { + project := strings.TrimPrefix(path, wd+"/"+dir) + foundZips = append(foundZips, project) + } + + return nil + }) + + lookupPaths := make([]api.LookupPath, 0, len(foundZips)) + // generate lookup paths + for _, project := range foundZips { + // if project = "group/subgroup/project/public.zip + // trim prefix group and suffix /public.zip + // so prefix = "/subgroup/project" + prefix := strings.TrimPrefix(project, dir) + prefix = strings.TrimSuffix(prefix, "/"+filepath.Base(project)) + + // use / as prefix when the current prefix matches the rootDomain, e.g. + // if request is group.gitlab-example.com/ and group/group.gitlab-example.com/public.zip exists + if prefix == "/"+rootDomain { + prefix = "/" + } + + cfg, ok := perPrefixConfig[prefix] + if !ok { + cfg = projectConfig{} + } + + lookupPath := api.LookupPath{ + ProjectID: cfg.projectID, + AccessControl: cfg.accessControl, + HTTPSOnly: cfg.https, + Prefix: prefix, Source: api.Source{ - Type: "zip", - Path: fmt.Sprintf("file://%s/@hashed/67/06/670671cd97404156226e507973f2ab8330d3022ca96e0c93bdbdb320c41adcaf/pages_deployments/01/artifacts.zip", wd), - Serverless: api.Serverless{}, + Type: "zip", + Path: fmt.Sprintf("file://%s", wd+"/"+dir+project), }, - }, - }, + } + + lookupPaths = append(lookupPaths, lookupPath) + } + + return api.VirtualDomain{ + LookupPaths: lookupPaths, + } } } -// ZipFromFileNotFound response for zip-from-disk-not-found.gitlab.io -func ZipFromFileNotFound(wd string) api.VirtualDomain { - return api.VirtualDomain{ - Certificate: "", - Key: "", - LookupPaths: []api.LookupPath{ - { - ProjectID: 123, - AccessControl: false, - HTTPSOnly: false, - Prefix: "/", - Source: api.Source{ - Type: "zip", - Path: fmt.Sprintf("file://%s/@hashed/67/06/670671cd97404156226e507973f2ab8330d3022ca96e0c93bdbdb320c41adcaf/pages_deployments/01/unknown.zip", wd), - Serverless: api.Serverless{}, - }, - }, - }, - } +type projectConfig struct { + // refer to makeGitLabPagesAccessStub for custom HTTP responses per projectID + projectID int + accessControl bool + https bool + pathOnDisk string } -// ZipFromNotAllowedPath response for zip-not-allowed-path.gitlab.io -func ZipFromNotAllowedPath(wd string) api.VirtualDomain { - return api.VirtualDomain{ - Certificate: "", - Key: "", - LookupPaths: []api.LookupPath{ - { - ProjectID: 123, - AccessControl: false, - HTTPSOnly: false, - Prefix: "/", - Source: api.Source{ - Type: "zip", - // path outside of `pages-root` - Path: "file:///some/random/path/public.zip", - Serverless: api.Serverless{}, +// customDomain with per project config +func customDomain(config projectConfig) responseFn { + return func(t *testing.T, wd string) api.VirtualDomain { + t.Helper() + + return api.VirtualDomain{ + Certificate: "", + Key: "", + LookupPaths: []api.LookupPath{ + { + ProjectID: config.projectID, + AccessControl: config.accessControl, + HTTPSOnly: config.https, + // prefix should always be `/` for custom domains, otherwise `resolvePath` will try + // to look for files under public/prefix/ when serving content instead of just public/ + // see internal/serving/disk/ for details + Prefix: "/", + Source: api.Source{ + Type: "zip", + Path: fmt.Sprintf("file://%s/%s/public.zip", wd, config.pathOnDisk), + }, }, }, - }, + } } } |