diff options
-rw-r--r-- | .golangci.yml | 9 | ||||
-rw-r--r-- | .tool-versions | 2 | ||||
-rw-r--r-- | CHANGELOG.md | 11 | ||||
-rw-r--r-- | README.md | 2 | ||||
-rw-r--r-- | VERSION | 2 | ||||
-rw-r--r-- | app.go | 63 | ||||
-rw-r--r-- | app_test.go | 3 | ||||
-rw-r--r-- | helpers.go | 27 | ||||
-rw-r--r-- | internal/config/config.go | 20 | ||||
-rw-r--r-- | internal/config/validate.go | 24 | ||||
-rw-r--r-- | internal/config/validate_test.go | 18 | ||||
-rw-r--r-- | internal/domain/logging.go | 31 | ||||
-rw-r--r-- | internal/domain/logging_test.go | 71 | ||||
-rw-r--r-- | internal/logging/logging.go | 24 | ||||
-rw-r--r-- | internal/logging/logging_test.go | 87 | ||||
-rw-r--r-- | internal/vfs/serving/serving.go | 11 | ||||
-rw-r--r-- | internal/vfs/zip/vfs_test.go | 2 | ||||
-rw-r--r-- | main.go | 97 | ||||
-rw-r--r-- | server.go | 42 | ||||
-rw-r--r-- | test/acceptance/acme_test.go | 4 |
20 files changed, 118 insertions, 432 deletions
diff --git a/.golangci.yml b/.golangci.yml index 33467c49..c9fc0444 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,6 +37,12 @@ linters-settings: min-occurrences: 3 goimports: local-prefixes: gitlab.com/gitlab-org/gitlab-pages + staticcheck: + # Select the Go version to target. + # Default: 1.13 + go: "1.16" + # https://staticcheck.io/docs/options#checks + checks: [ "all" ] linters: disable-all: true @@ -49,12 +55,13 @@ linters: - goconst - gocyclo - goimports - - golint - gosimple - govet - gosec - ineffassign - misspell + - revive + - staticcheck - structcheck - typecheck - unconvert diff --git a/.tool-versions b/.tool-versions index bf23029f..29cc9a03 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1 +1 @@ -golang 1.16.12 +golang 1.17.6 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b8a9b2d..56dabab5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +## 1.53.0 (2022-02-01) + +### Fixed (2 changes) + +- [fix: Fix 500 errors when clients disconnect](gitlab-org/gitlab-pages@1b50e38f3959c784f44c280720ed3249802d2622) ([merge request](gitlab-org/gitlab-pages!681)) +- [fix: fix metrics and logs not including domain resolution time](gitlab-org/gitlab-pages@adc0b9233fa4a0b2b449e27336d9ae39c75819ba) ([merge request](gitlab-org/gitlab-pages!674)) + +### Changed (1 change) + +- [refactor: stop passing file descriptors around and use net.Listen](gitlab-org/gitlab-pages@052a7fb36f4605634385f54c833db21a9edc6d67) by @feistel ([merge request](gitlab-org/gitlab-pages!667)) + ## 1.52.0 (2022-01-31) ### Added (1 change) @@ -4,7 +4,7 @@ [![coverage report](https://gitlab.com/gitlab-org/gitlab-pages/badges/master/coverage.svg)](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.16.12. +CNAMEs and SNI using HTTP/HTTP2. The minimum supported Go version is v1.16.9. ### How it generates routes @@ -1 +1 @@ -1.52.0 +1.53.0 @@ -138,7 +138,7 @@ func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, ht } // healthCheckMiddleware is serving the application status check -func (a *theApp) healthCheckMiddleware(handler http.Handler) (http.Handler, error) { +func (a *theApp) healthCheckMiddleware(handler http.Handler) http.Handler { healthCheck := http.HandlerFunc(func(w http.ResponseWriter, _r *http.Request) { if a.isReady() { w.Write([]byte("success\n")) @@ -147,19 +147,14 @@ func (a *theApp) healthCheckMiddleware(handler http.Handler) (http.Handler, erro } }) - loggedHealthCheck, err := logging.BasicAccessLogger(healthCheck, a.config.Log.Format, nil) - if err != nil { - return nil, err - } - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.RequestURI == a.config.General.StatusPath { - loggedHealthCheck.ServeHTTP(w, r) + healthCheck.ServeHTTP(w, r) return } handler.ServeHTTP(w, r) - }), nil + }) } // auxiliaryMiddleware will handle status updates, not-ready requests and other @@ -234,24 +229,13 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) { handler = a.auxiliaryMiddleware(handler) handler = a.Auth.AuthenticationMiddleware(handler, a.source) handler = a.AcmeMiddleware.AcmeMiddleware(handler) - handler, err := logging.BasicAccessLogger(handler, a.config.Log.Format, domain.LogFields) - if err != nil { - return nil, err - } - - // Metrics - metricsMiddleware := labmetrics.NewHandlerFactory(labmetrics.WithNamespace("gitlab_pages")) - handler = metricsMiddleware(handler) handler = routing.NewMiddleware(handler, a.source) handler = handlers.Ratelimiter(handler, &a.config.RateLimit) // Health Check - handler, err = a.healthCheckMiddleware(handler) - if err != nil { - return nil, err - } + handler = a.healthCheckMiddleware(handler) // Custom response headers handler = customheaders.NewMiddleware(handler, a.CustomHeaders) @@ -262,6 +246,15 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) { correlationOpts = append(correlationOpts, correlation.WithPropagation()) } handler = handlePanicMiddleware(handler) + + // Access logs and metrics + handler, err := logging.BasicAccessLogger(handler, a.config.Log.Format) + if err != nil { + return nil, err + } + metricsMiddleware := labmetrics.NewHandlerFactory(labmetrics.WithNamespace("gitlab_pages")) + handler = metricsMiddleware(handler) + handler = correlation.InjectCorrelationID(handler, correlationOpts...) // These middlewares MUST be added in the end. @@ -301,9 +294,9 @@ func (a *theApp) Run() { var servers []*http.Server // Listen for HTTP - for _, fd := range a.config.Listeners.HTTP { + for _, addr := range a.config.ListenHTTPStrings.Split() { s := a.listen( - fd, + addr, httpHandler, errortracking.WithField("listener", request.SchemeHTTP), withLimiter(limiter), @@ -312,14 +305,14 @@ func (a *theApp) Run() { } // Listen for HTTPS - for _, fd := range a.config.Listeners.HTTPS { + for _, addr := range a.config.ListenHTTPSStrings.Split() { tlsConfig, err := a.TLSConfig() if err != nil { log.WithError(err).Fatal("Unable to retrieve tls config") } s := a.listen( - fd, + addr, httpHandler, errortracking.WithField("listener", request.SchemeHTTPS), withLimiter(limiter), @@ -329,9 +322,9 @@ func (a *theApp) Run() { } // Listen for HTTP proxy requests - for _, fd := range a.config.Listeners.Proxy { + for _, addr := range a.config.ListenProxyStrings.Split() { s := a.listen( - fd, + addr, proxyHandler, errortracking.WithField("listener", "http proxy"), withLimiter(limiter), @@ -340,14 +333,14 @@ func (a *theApp) Run() { } // Listen for HTTPS PROXYv2 requests - for _, fd := range a.config.Listeners.HTTPSProxyv2 { + for _, addr := range a.config.ListenHTTPSProxyv2Strings.Split() { tlsConfig, err := a.TLSConfig() if err != nil { log.WithError(err).Fatal("Unable to retrieve tls config") } s := a.listen( - fd, + addr, httpHandler, errortracking.WithField("listener", "https proxy"), withLimiter(limiter), @@ -358,8 +351,8 @@ func (a *theApp) Run() { } // Serve metrics for Prometheus - if a.config.ListenMetrics != 0 { - a.listenMetricsFD(&wg, a.config.ListenMetrics) + if a.config.General.MetricsAddress != "" { + a.listenMetrics(&wg, a.config.General.MetricsAddress) } sigChan := make(chan os.Signal, 1) @@ -385,10 +378,10 @@ func (a *theApp) Run() { } } -func (a *theApp) listen(fd uintptr, h http.Handler, errTrackingOpt errortracking.CaptureOption, opts ...option) *http.Server { +func (a *theApp) listen(addr string, h http.Handler, errTrackingOpt errortracking.CaptureOption, opts ...option) *http.Server { server := &http.Server{} go func() { - if err := a.listenAndServe(server, fd, h, opts...); err != nil && !errors.Is(err, http.ErrServerClosed) { + if err := a.listenAndServe(server, addr, h, opts...); err != nil && !errors.Is(err, http.ErrServerClosed) { capturingFatal(err, errTrackingOpt) } }() @@ -396,14 +389,14 @@ func (a *theApp) listen(fd uintptr, h http.Handler, errTrackingOpt errortracking return server } -func (a *theApp) listenMetricsFD(wg *sync.WaitGroup, fd uintptr) { +func (a *theApp) listenMetrics(wg *sync.WaitGroup, addr string) { wg.Add(1) go func() { defer wg.Done() - l, err := net.FileListener(os.NewFile(fd, "[socket]")) + l, err := net.Listen("tcp", addr) if err != nil { - capturingFatal(fmt.Errorf("failed to listen on FD %d: %w", fd, err), errortracking.WithField("listener", "metrics")) + capturingFatal(fmt.Errorf("failed to listen on addr %s: %w", addr, err), errortracking.WithField("listener", "metrics")) } monitoringOpts := []monitoring.Option{ diff --git a/app_test.go b/app_test.go index db4d9d67..3b1d6697 100644 --- a/app_test.go +++ b/app_test.go @@ -118,8 +118,7 @@ func TestHealthCheckMiddleware(t *testing.T) { r := httptest.NewRequest("GET", tc.path, nil) rr := httptest.NewRecorder() - middleware, err := app.healthCheckMiddleware(handler) - require.NoError(t, err) + middleware := app.healthCheckMiddleware(handler) middleware.ServeHTTP(rr, r) require.Equal(t, tc.status, rr.Code) @@ -1,36 +1,9 @@ package main import ( - "net" - "os" - "gitlab.com/gitlab-org/labkit/errortracking" ) -// Be careful: if you let either of the return values get garbage -// collected by Go they will be closed automatically. -func createSocket(addr string) (net.Listener, *os.File) { - l, err := net.Listen("tcp", addr) - if err != nil { - fatal(err, "could not create socket") - } - - return l, fileForListener(l) -} - -func fileForListener(l net.Listener) *os.File { - type filer interface { - File() (*os.File, error) - } - - f, err := l.(filer).File() - if err != nil { - fatal(err, "could not find file for listener") - } - - return f -} - func capturingFatal(err error, fields ...errortracking.CaptureOption) { fields = append(fields, errortracking.WithStackTrace()) errortracking.Capture(err, fields...) diff --git a/internal/config/config.go b/internal/config/config.go index c29fa65d..3bb7b126 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -20,22 +20,14 @@ type Config struct { ArtifactsServer ArtifactsServer Authentication Auth GitLab GitLab - Listeners Listeners Log Log Sentry Sentry TLS TLS Zip ZipServing - // Fields used to share information between files. These are not directly - // set by command line flags, but rather populated based on info from them. - // ListenMetrics points to a file descriptor of a socket, whose address is - // specified by `Config.General.MetricsAddress`. - ListenMetrics uintptr - // These fields contain the raw strings passed for listen-http, // listen-https, listen-proxy and listen-https-proxyv2 settings. It is used - // by appmain() to create listeners, and the pointers to these listeners - // gets assigned to Config.Listeners.* fields + // by appmain() to create listeners. ListenHTTPStrings MultiStringFlag ListenHTTPSStrings MultiStringFlag ListenProxyStrings MultiStringFlag @@ -111,15 +103,6 @@ type GitLab struct { EnableDisk bool } -// Listeners groups settings related to configuring various listeners -// (HTTP, HTTPS, Proxy, HTTPSProxyv2) -type Listeners struct { - HTTP []uintptr - HTTPS []uintptr - Proxy []uintptr - HTTPSProxyv2 []uintptr -} - // Log groups settings related to configuring logging type Log struct { Format string @@ -251,7 +234,6 @@ func loadConfig() (*Config, error) { ListenHTTPSStrings: listenHTTPS, ListenProxyStrings: listenProxy, ListenHTTPSProxyv2Strings: listenHTTPSProxyv2, - Listeners: Listeners{}, } var err error diff --git a/internal/config/validate.go b/internal/config/validate.go index 3039d00a..a3dbcc3b 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -2,6 +2,7 @@ package config import ( "errors" + "fmt" "net/url" "github.com/hashicorp/go-multierror" @@ -18,6 +19,7 @@ var ( errAuthNoRedirect = errors.New("auth-redirect-uri must be defined if authentication is supported") errArtifactsServerUnsupportedScheme = errors.New("artifacts-server scheme must be either http:// or https://") errArtifactsServerInvalidTimeout = errors.New("artifacts-server-timeout must be greater than or equal to 1") + errEmptyListener = errors.New("listener must not be empty") ) // Validate values populated in Config @@ -42,7 +44,27 @@ func validateListeners(config *Config) error { return errNoListener } - return nil + var result *multierror.Error + + result = multierror.Append(result, + validateListenerAddr(config.ListenHTTPStrings, "http"), + validateListenerAddr(config.ListenHTTPSStrings, "https"), + validateListenerAddr(config.ListenHTTPSProxyv2Strings, "proxyv2"), + validateListenerAddr(config.ListenProxyStrings, "proxy"), + ) + + return result.ErrorOrNil() +} + +func validateListenerAddr(listeners MultiStringFlag, name string) error { + var result *multierror.Error + for i, s := range listeners.Split() { + if s == "" { + result = multierror.Append(result, fmt.Errorf("empty %s listener at index %d: %w", name, i, errEmptyListener)) + } + } + + return result.ErrorOrNil() } func validateAuthConfig(config *Config) error { diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go index 87eb26b3..60e37732 100644 --- a/internal/config/validate_test.go +++ b/internal/config/validate_test.go @@ -61,6 +61,11 @@ func TestConfigValidate(t *testing.T) { cfg: artifactsInvalidTimeout, expectedErr: errArtifactsServerInvalidTimeout, }, + { + name: "empty_listener", + cfg: emptyListeners, + expectedErr: errEmptyListener, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -77,6 +82,13 @@ func TestConfigValidate(t *testing.T) { } } +func emptyListeners(cfg *Config) { + cfg.ListenHTTPSStrings = MultiStringFlag{ + value: []string{"127.0.0.1:8080", "", ":8081"}, + separator: ",", + } +} + func noListeners(cfg *Config) { cfg.ListenHTTPStrings = MultiStringFlag{separator: ","} cfg.ListenHTTPSStrings = MultiStringFlag{separator: ","} @@ -123,7 +135,11 @@ func artifactsInvalidTimeout(cfg *Config) { func validConfig() Config { cfg := Config{ ListenHTTPStrings: MultiStringFlag{ - value: []string{"127.0.0.1:80"}, + value: []string{"127.0.0.1:80", ":8081"}, + separator: ",", + }, + ListenHTTPSStrings: MultiStringFlag{ + value: []string{"127.0.0.1:", ":8082"}, separator: ",", }, ArtifactsServer: ArtifactsServer{ diff --git a/internal/domain/logging.go b/internal/domain/logging.go deleted file mode 100644 index 2ecddf6c..00000000 --- a/internal/domain/logging.go +++ /dev/null @@ -1,31 +0,0 @@ -package domain - -import ( - "net/http" - - "gitlab.com/gitlab-org/labkit/log" -) - -func LogFields(r *http.Request) log.Fields { - logFields := log.Fields{ - "pages_https": r.URL.Scheme == "https", - "pages_host": GetHost(r), - } - - d := FromRequest(r) - if d == nil { - return logFields - } - - lp, err := d.GetLookupPath(r) - if err != nil { - logFields["error"] = err.Error() - return logFields - } - - logFields["pages_project_serving_type"] = lp.ServingType - logFields["pages_project_prefix"] = lp.Prefix - logFields["pages_project_id"] = lp.ProjectID - - return logFields -} diff --git a/internal/domain/logging_test.go b/internal/domain/logging_test.go deleted file mode 100644 index b736758c..00000000 --- a/internal/domain/logging_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package domain - -import ( - "net/http" - "testing" - - log "github.com/sirupsen/logrus" - "github.com/stretchr/testify/require" - - "gitlab.com/gitlab-org/gitlab-pages/internal/serving" -) - -type resolver struct { - err error - f func(*http.Request) *serving.LookupPath -} - -func (r *resolver) Resolve(req *http.Request) (*serving.Request, error) { - if r.f != nil { - return &serving.Request{LookupPath: r.f(req)}, nil - } - - return nil, r.err -} - -func TestDomainLogFields(t *testing.T) { - domainWithResolver := New("", "", "", &resolver{f: func(*http.Request) *serving.LookupPath { - return &serving.LookupPath{ - ServingType: "file", - ProjectID: 100, - Prefix: "/prefix", - } - }}) - - tests := map[string]struct { - domain *Domain - host string - expectedFields log.Fields - }{ - "nil_domain_returns_empty_fields": { - domain: nil, - host: "gitlab.io", - expectedFields: log.Fields{"pages_https": false, "pages_host": "gitlab.io"}, - }, - "unresolved_domain_returns_error": { - domain: New("githost.io", "", "", &resolver{err: ErrDomainDoesNotExist}), - host: "gitlab.io", - expectedFields: log.Fields{"error": ErrDomainDoesNotExist.Error(), "pages_https": false, "pages_host": "gitlab.io"}, - }, - "domain_with_fields": { - domain: domainWithResolver, - host: "gitlab.io", - expectedFields: log.Fields{ - "pages_https": false, - "pages_host": "gitlab.io", - "pages_project_id": uint64(100), - "pages_project_prefix": "/prefix", - "pages_project_serving_type": "file", - }, - }, - } - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - r, err := http.NewRequest("GET", "/", nil) - require.NoError(t, err) - - r = ReqWithHostAndDomain(r, tt.host, tt.domain) - require.Equal(t, tt.expectedFields, LogFields(r)) - }) - } -} diff --git a/internal/logging/logging.go b/internal/logging/logging.go index edc670d6..6e8533d8 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -52,34 +52,24 @@ func getAccessLogger(format string) (*logrus.Logger, error) { } // BasicAccessLogger configures the GitLab pages basic HTTP access logger middleware -func BasicAccessLogger(handler http.Handler, format string, extraFields log.ExtraFieldsGeneratorFunc) (http.Handler, error) { +func BasicAccessLogger(handler http.Handler, format string) (http.Handler, error) { accessLogger, err := getAccessLogger(format) if err != nil { return nil, err } return log.AccessLogger(handler, - log.WithExtraFields(enrichExtraFields(extraFields)), + log.WithExtraFields(extraFields), log.WithAccessLogger(accessLogger), log.WithXFFAllowed(func(sip string) bool { return false }), ), nil } -func enrichExtraFields(extraFields log.ExtraFieldsGeneratorFunc) log.ExtraFieldsGeneratorFunc { - return func(r *http.Request) log.Fields { - enrichedFields := log.Fields{ - "correlation_id": correlation.ExtractFromContext(r.Context()), - "pages_https": request.IsHTTPS(r), - "pages_host": r.Host, - } - - if extraFields != nil { - for field, value := range extraFields(r) { - enrichedFields[field] = value - } - } - - return enrichedFields +func extraFields(r *http.Request) log.Fields { + return log.Fields{ + "correlation_id": correlation.ExtractFromContext(r.Context()), + "pages_https": request.IsHTTPS(r), + "pages_host": r.Host, } } diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go deleted file mode 100644 index d56f6777..00000000 --- a/internal/logging/logging_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package logging - -import ( - "net/http" - "testing" - - "github.com/stretchr/testify/require" - - "gitlab.com/gitlab-org/gitlab-pages/internal/domain" - "gitlab.com/gitlab-org/gitlab-pages/internal/request" - "gitlab.com/gitlab-org/gitlab-pages/internal/serving" -) - -type resolver struct { - err error - f func(*http.Request) *serving.LookupPath -} - -func (r *resolver) Resolve(req *http.Request) (*serving.Request, error) { - if r.f != nil { - return &serving.Request{LookupPath: r.f(req)}, nil - } - - return nil, r.err -} - -func TestGetExtraLogFields(t *testing.T) { - domainWithResolver := domain.New("", "", "", &resolver{f: func(*http.Request) *serving.LookupPath { - return &serving.LookupPath{ - ServingType: "file", - ProjectID: 100, - Prefix: "/prefix", - SHA256: "foo", - } - }}) - - tests := []struct { - name string - scheme string - host string - expectedHTTPS interface{} - expectedHost interface{} - expectedProjectID interface{} - expectedProjectPrefix interface{} - expectedServingType interface{} - expectedErrMsg interface{} - }{ - { - name: "https", - scheme: request.SchemeHTTPS, - host: "githost.io", - expectedHTTPS: true, - expectedHost: "githost.io", - expectedProjectID: uint64(100), - expectedProjectPrefix: "/prefix", - expectedServingType: "file", - }, - { - name: "http", - scheme: request.SchemeHTTP, - host: "githost.io", - expectedHTTPS: false, - expectedHost: "githost.io", - expectedProjectID: uint64(100), - expectedProjectPrefix: "/prefix", - expectedServingType: "file", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - req, err := http.NewRequest("GET", "/", nil) - require.NoError(t, err) - - req.URL.Scheme = tt.scheme - req = domain.ReqWithHostAndDomain(req, tt.host, domainWithResolver) - - got := domain.LogFields(req) - require.Equal(t, tt.expectedHTTPS, got["pages_https"]) - require.Equal(t, tt.expectedHost, got["pages_host"]) - require.Equal(t, tt.expectedProjectID, got["pages_project_id"]) - require.Equal(t, tt.expectedProjectPrefix, got["pages_project_prefix"]) - require.Equal(t, tt.expectedServingType, got["pages_project_serving_type"]) - require.Equal(t, tt.expectedErrMsg, got["error"]) - }) - } -} diff --git a/internal/vfs/serving/serving.go b/internal/vfs/serving/serving.go index 57b0228f..6f45e1d5 100644 --- a/internal/vfs/serving/serving.go +++ b/internal/vfs/serving/serving.go @@ -47,17 +47,10 @@ func serveContent(w http.ResponseWriter, r *http.Request, modtime time.Time, con return } - if r.Method == http.MethodHead { - w.WriteHeader(code) - return - } + w.WriteHeader(code) if r.Method != http.MethodHead { - if _, err := io.Copy(w, content); err != nil { - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) - logging.LogRequest(r).WithError(err).Error("error serving content") - httperrors.Serve500(w) - } + io.Copy(w, content) } } diff --git a/internal/vfs/zip/vfs_test.go b/internal/vfs/zip/vfs_test.go index 4de0240a..12e82c1c 100644 --- a/internal/vfs/zip/vfs_test.go +++ b/internal/vfs/zip/vfs_test.go @@ -169,7 +169,7 @@ func TestVFSFindOrOpenArchiveRefresh(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { withExpectedArchiveCount(t, 1, func(t *testing.T) { - cfg := *&zipCfg + cfg := zipCfg cfg.ExpirationInterval = test.expirationInterval cfg.RefreshInterval = test.refreshInterval @@ -2,7 +2,6 @@ package main import ( "fmt" - "io" "math/rand" "os" "time" @@ -75,105 +74,9 @@ func appMain() { fatal(err, "could not change directory into pagesRoot") } - for _, cs := range [][]io.Closer{ - createAppListeners(config), - createMetricsListener(config), - } { - defer closeAll(cs) - } - runApp(config) } -func closeAll(cs []io.Closer) { - for _, c := range cs { - c.Close() - } -} - -// createAppListeners returns net.Listener and *os.File instances. The -// caller must ensure they don't get closed or garbage-collected (which -// implies closing) too soon. -func createAppListeners(config *cfg.Config) []io.Closer { - var closers []io.Closer - var httpListeners []uintptr - var httpsListeners []uintptr - var proxyListeners []uintptr - var httpsProxyv2Listeners []uintptr - - for _, addr := range config.ListenHTTPStrings.Split() { - l, f := createSocket(addr) - closers = append(closers, l, f) - - log.WithFields(log.Fields{ - "listener": addr, - }).Debug("Set up HTTP listener") - - httpListeners = append(httpListeners, f.Fd()) - } - - for _, addr := range config.ListenHTTPSStrings.Split() { - l, f := createSocket(addr) - closers = append(closers, l, f) - - log.WithFields(log.Fields{ - "listener": addr, - }).Debug("Set up HTTPS listener") - - httpsListeners = append(httpsListeners, f.Fd()) - } - - for _, addr := range config.ListenProxyStrings.Split() { - l, f := createSocket(addr) - closers = append(closers, l, f) - - log.WithFields(log.Fields{ - "listener": addr, - }).Debug("Set up proxy listener") - - proxyListeners = append(proxyListeners, f.Fd()) - } - - for _, addr := range config.ListenHTTPSProxyv2Strings.Split() { - l, f := createSocket(addr) - closers = append(closers, l, f) - - log.WithFields(log.Fields{ - "listener": addr, - }).Debug("Set up https proxyv2 listener") - - httpsProxyv2Listeners = append(httpsProxyv2Listeners, f.Fd()) - } - - config.Listeners = cfg.Listeners{ - HTTP: httpListeners, - HTTPS: httpsListeners, - Proxy: proxyListeners, - HTTPSProxyv2: httpsProxyv2Listeners, - } - - return closers -} - -// createMetricsListener returns net.Listener and *os.File instances. The -// caller must ensure they don't get closed or garbage-collected (which -// implies closing) too soon. -func createMetricsListener(config *cfg.Config) []io.Closer { - addr := config.General.MetricsAddress - if addr == "" { - return nil - } - - l, f := createSocket(addr) - config.ListenMetrics = f.Fd() - - log.WithFields(log.Fields{ - "listener": addr, - }).Debug("Set up metrics listener") - - return []io.Closer{l, f} -} - func printVersion(showVersion bool, version string) { if showVersion { fmt.Fprintf(os.Stdout, "%s\n", version) @@ -1,47 +1,26 @@ package main import ( + "context" "crypto/tls" "fmt" "net" "net/http" - "os" "time" proxyproto "github.com/pires/go-proxyproto" + "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/gitlab-pages/internal/netutil" ) -type keepAliveListener struct { - net.Listener -} - -type keepAliveSetter interface { - SetKeepAlive(bool) error - SetKeepAlivePeriod(time.Duration) error -} - type listenerConfig struct { isProxyV2 bool tlsConfig *tls.Config limiter *netutil.Limiter } -func (ln *keepAliveListener) Accept() (net.Conn, error) { - conn, err := ln.Listener.Accept() - if err != nil { - return nil, err - } - - kc := conn.(keepAliveSetter) - kc.SetKeepAlive(true) - kc.SetKeepAlivePeriod(3 * time.Minute) - - return conn, nil -} - -func (a *theApp) listenAndServe(server *http.Server, fd uintptr, h http.Handler, opts ...option) error { +func (a *theApp) listenAndServe(server *http.Server, addr string, h http.Handler, opts ...option) error { config := &listenerConfig{} for _, opt := range opts { @@ -58,17 +37,19 @@ func (a *theApp) listenAndServe(server *http.Server, fd uintptr, h http.Handler, server.TLSConfig.NextProtos = append(server.TLSConfig.NextProtos, "h2") } - l, err := net.FileListener(os.NewFile(fd, "[socket]")) + lc := net.ListenConfig{ + KeepAlive: 3 * time.Minute, + } + + l, err := lc.Listen(context.Background(), "tcp", addr) if err != nil { - return fmt.Errorf("failed to listen on FD %d: %w", fd, err) + return fmt.Errorf("failed to listen on addr %s: %w", addr, err) } if config.limiter != nil { l = netutil.SharedLimitListener(l, config.limiter) } - l = &keepAliveListener{l} - if config.isProxyV2 { l = &proxyproto.Listener{ Listener: l, @@ -82,6 +63,11 @@ func (a *theApp) listenAndServe(server *http.Server, fd uintptr, h http.Handler, l = tls.NewListener(l, server.TLSConfig) } + log.WithFields(log.Fields{ + "config_addr": addr, + "listen_addr": l.Addr(), + }).Infof("server listening on: %s", l.Addr()) + return server.Serve(l) } diff --git a/test/acceptance/acme_test.go b/test/acceptance/acme_test.go index f0259e05..77c4d6c0 100644 --- a/test/acceptance/acme_test.go +++ b/test/acceptance/acme_test.go @@ -41,8 +41,8 @@ func TestAcmeChallengesWhenItIsNotConfigured(t *testing.T) { rsp, err := GetRedirectPage(t, httpListener, "withacmechallenge.domain.com", test.token) - defer rsp.Body.Close() require.NoError(t, err) + defer rsp.Body.Close() require.Equal(t, test.expectedStatus, rsp.StatusCode) body, err := io.ReadAll(rsp.Body) require.NoError(t, err) @@ -82,8 +82,8 @@ func TestAcmeChallengesWhenItIsConfigured(t *testing.T) { rsp, err := GetRedirectPage(t, httpListener, "withacmechallenge.domain.com", test.token) - defer rsp.Body.Close() require.NoError(t, err) + defer rsp.Body.Close() require.Equal(t, test.expectedStatus, rsp.StatusCode) body, err := io.ReadAll(rsp.Body) require.NoError(t, err) |