Merge branch 'remove/file-descriptors' into 'master'

refactor: stop passing file descriptors around and use net.Listen directly

See merge request gitlab-org/gitlab-pages!667

3 files changed, 41 insertions, 21 deletions

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{