From 8488ef56611256c1761f93de5f8df23e07b86af4 Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Thu, 26 May 2022 14:10:09 +0530 Subject: Add support for tls for metrics Changelog: added --- app.go | 19 +++++++++++---- internal/config/config.go | 53 ++++++++++++++++++++++++++++++++++++++++-- internal/config/flags.go | 2 ++ test/acceptance/config_test.go | 27 +++++++++++++++++++++ 4 files changed, 94 insertions(+), 7 deletions(-) diff --git a/app.go b/app.go index 760a8bfb..7937e3fd 100644 --- a/app.go +++ b/app.go @@ -277,8 +277,8 @@ func (a *theApp) Run() error { } // Serve metrics for Prometheus - if a.config.General.MetricsAddress != "" { - s := a.listenMetrics(eg, a.config.General.MetricsAddress) + if a.config.Metrics.Address != "" { + s := a.listenMetrics(eg, a.config.Metrics) servers = append(servers, s) } @@ -322,13 +322,22 @@ func (a *theApp) listen(eg *errgroup.Group, addr string, h http.Handler, errTrac return server } -func (a *theApp) listenMetrics(eg *errgroup.Group, addr string) *http.Server { +func (a *theApp) listenMetrics(eg *errgroup.Group, config cfg.Metrics) *http.Server { server := &http.Server{} eg.Go(func() error { - l, err := net.Listen("tcp", addr) + l, err := net.Listen("tcp", config.Address) if err != nil { errortracking.CaptureErrWithStackTrace(err, errortracking.WithField("listener", "metrics")) - return fmt.Errorf("failed to listen on addr %s: %w", addr, err) + return fmt.Errorf("failed to listen on addr %s: %w", config.Address, err) + } + + metricsTLSConfig := &cryptotls.Config{ + Certificates: []cryptotls.Certificate{config.TLSCertificate}, + MinVersion: cryptotls.VersionTLS12, + } + + if config.IsHTTPS { + l = cryptotls.NewListener(l, metricsTLSConfig) } monitoringOpts := []monitoring.Option{ diff --git a/internal/config/config.go b/internal/config/config.go index 06e0520a..b26642d1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,9 @@ package config import ( + "crypto/tls" "encoding/base64" + "errors" "fmt" "os" "strings" @@ -23,6 +25,7 @@ type Config struct { Server Server TLS TLS Zip ZipServing + Metrics Metrics // These fields contain the raw strings passed for listen-http, // listen-https, listen-proxy and listen-https-proxyv2 settings. It is used @@ -39,7 +42,6 @@ type General struct { Domain string MaxConns int MaxURILength int - MetricsAddress string RedirectHTTP bool RootCertificate []byte RootDir string @@ -146,6 +148,17 @@ type Server struct { ListenKeepAlive time.Duration } +type Metrics struct { + Address string + IsHTTPS bool + TLSCertificate tls.Certificate +} + +var ( + errMetricsNoCertificate = errors.New("metrics certificate path must not be empty") + errMetricsNoKey = errors.New("metrics private key path must not be empty") +) + func internalGitlabServerFromFlags() string { if *internalGitLabServer != "" { return *internalGitLabServer @@ -178,13 +191,42 @@ func setGitLabAPISecretKey(secretFile string, config *Config) error { return nil } +func loadMetricsConfig() (metrics Metrics, err error) { + // don't validate anything if metrics are disabled + if *metricsAddress == "" { + return metrics, nil + } + metrics.Address = *metricsAddress + + // no error when using HTTP + if *metricsCertificate == "" && *metricsKey == "" { + return metrics, nil + } + + if *metricsCertificate == "" { + return metrics, errMetricsNoCertificate + } + + if *metricsKey == "" { + return metrics, errMetricsNoKey + } + + metrics.TLSCertificate, err = tls.LoadX509KeyPair(*metricsCertificate, *metricsKey) + if err != nil { + return metrics, err + } + + metrics.IsHTTPS = true + + return metrics, nil +} + func loadConfig() (*Config, error) { config := &Config{ General: General{ Domain: strings.ToLower(*pagesDomain), MaxConns: *maxConns, MaxURILength: *maxURILength, - MetricsAddress: *metricsAddress, RedirectHTTP: *redirectHTTP, RootDir: *pagesRoot, StatusPath: *pagesStatus, @@ -268,6 +310,11 @@ func loadConfig() (*Config, error) { var err error + // Validating and populating Metrics config + if config.Metrics, err = loadMetricsConfig(); err != nil { + return nil, err + } + // Populating remaining General settings for _, file := range []struct { contents *[]byte @@ -309,6 +356,8 @@ func LogConfig(config *Config) { "listen-https-proxyv2": listenHTTPSProxyv2, "log-format": *logFormat, "metrics-address": *metricsAddress, + "metrics-certificate": *metricsCertificate, + "metrics-key": *metricsKey, "pages-domain": *pagesDomain, "pages-root": *pagesRoot, "pages-status": *pagesStatus, diff --git a/internal/config/flags.go b/internal/config/flags.go index 23da47ed..88ebbb15 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -40,6 +40,8 @@ var ( artifactsServerTimeout = flag.Int("artifacts-server-timeout", 10, "Timeout (in seconds) for a proxied request to the artifacts server") pagesStatus = flag.String("pages-status", "", "The url path for a status page, e.g., /@status") metricsAddress = flag.String("metrics-address", "", "The address to listen on for metrics requests") + metricsCertificate = flag.String("metrics-certificate", "", "The default path to file certificate to serve metrics requests") + metricsKey = flag.String("metrics-key", "", "The default path to file private key to serve metrics requests") sentryDSN = flag.String("sentry-dsn", "", "The address for sending sentry crash reporting to") sentryEnvironment = flag.String("sentry-environment", "", "The environment for sentry crash reporting") propagateCorrelationID = flag.Bool("propagate-correlation-id", false, "Reuse existing Correlation-ID from the incoming request header `X-Request-ID` if present") diff --git a/test/acceptance/config_test.go b/test/acceptance/config_test.go index 95be6e17..011b4c73 100644 --- a/test/acceptance/config_test.go +++ b/test/acceptance/config_test.go @@ -1,6 +1,7 @@ package acceptance_test import ( + "crypto/tls" "fmt" "net" "net/http" @@ -8,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) func TestEnvironmentVariablesConfig(t *testing.T) { @@ -86,3 +88,28 @@ func TestUnixSocketListener(t *testing.T) { rsp.Body.Close() require.Equal(t, http.StatusOK, rsp.StatusCode) } + +func TestMetricsHTTPSConfig(t *testing.T) { + keyFile, certFile := CreateHTTPSFixtureFiles(t) + + RunPagesProcess(t, + withExtraArgument("metrics-address", ":42345"), + withExtraArgument("metrics-certificate", certFile), + withExtraArgument("metrics-key", keyFile), + ) + require.NoError(t, httpsListener.WaitUntilRequestSucceeds(nil)) + + tlsConfig := &tls.Config{InsecureSkipVerify: true} + transport := &http.Transport{TLSClientConfig: tlsConfig} + client := &http.Client{Transport: transport} + + res, err := client.Get("https://127.0.0.1:42345/metrics") + require.NoError(t, err) + testhelpers.Close(t, res.Body) + require.Equal(t, http.StatusOK, res.StatusCode) + + res, err = client.Get("http://127.0.0.1:42345/metrics") + require.NoError(t, err) + testhelpers.Close(t, res.Body) + require.Equal(t, http.StatusBadRequest, res.StatusCode) +} -- cgit v1.2.3