diff options
author | Vladimir Shushlin <vshushlin@gitlab.com> | 2022-06-03 13:25:31 +0300 |
---|---|---|
committer | Vladimir Shushlin <vshushlin@gitlab.com> | 2022-06-03 13:25:31 +0300 |
commit | fd62cfc0771c627bd1bda001fd1fa71178dd447b (patch) | |
tree | dc70546496ad01228802f6fa6298c7ae7f7827ea | |
parent | 3c89945269c643b8b3b55d1d967c19922101c48c (diff) | |
parent | 1ddad1bd6ab55120e7862c61e033632b20e66d6f (diff) |
Merge branch 'tls-metrics' into 'master'
Add TLS for metrics
See merge request gitlab-org/gitlab-pages!772
-rw-r--r-- | README.md | 2 | ||||
-rw-r--r-- | app.go | 14 | ||||
-rw-r--r-- | internal/config/config.go | 55 | ||||
-rw-r--r-- | internal/config/config_test.go | 85 | ||||
-rw-r--r-- | internal/config/flags.go | 2 | ||||
-rw-r--r-- | test/acceptance/metrics_test.go | 25 |
6 files changed, 176 insertions, 7 deletions
@@ -166,6 +166,8 @@ $ make $ ./gitlab-pages -listen-http ":8090" -metrics-address ":9235" -pages-root path/to/gitlab/shared/pages -pages-domain example.com ``` +Passing the `-metrics-certificate` and `-metrics-key` flags along with `-metrics-address` flag would add TLS to the metrics. + ### Structured logging You can use the `-log-format json` option to make GitLab Pages output @@ -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,17 @@ 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) + } + + if config.TLSConfig != nil { + l = cryptotls.NewListener(l, config.TLSConfig) } monitoringOpts := []monitoring.Option{ diff --git a/internal/config/config.go b/internal/config/config.go index f3650ffb..18fe33eb 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,16 @@ type Server struct { ListenKeepAlive time.Duration } +type Metrics struct { + Address string + TLSConfig *tls.Config +} + +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 @@ -186,13 +198,45 @@ 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 + } + + cert, err := tls.LoadX509KeyPair(*metricsCertificate, *metricsKey) + if err != nil { + return metrics, err + } + + metrics.TLSConfig = &tls.Config{ + Certificates: []tls.Certificate{cert}, + MinVersion: tls.VersionTLS12, + } + + 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, @@ -276,6 +320,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 @@ -319,6 +368,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/config_test.go b/internal/config/config_test.go new file mode 100644 index 00000000..f120d17d --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,85 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-pages/internal/fixture" +) + +func Test_loadMetricsConfig(t *testing.T) { + defaultMetricsAdress := ":9325" + defaultDir, defaultMetricsKey, defaultMetricsCertificate := setupHTTPSFixture(t) + + tests := map[string]struct { + metricsAddress string + metricsCertificate string + metricsKey string + expectedError error + }{ + "no metrics": {}, + "http metrics": { + metricsAddress: defaultMetricsAdress, + }, + "https metrics": { + metricsAddress: defaultMetricsAdress, + metricsCertificate: defaultMetricsCertificate, + metricsKey: defaultMetricsKey, + }, + "https metrics no certificate": { + metricsAddress: defaultMetricsAdress, + metricsKey: defaultMetricsKey, + expectedError: errMetricsNoCertificate, + }, + "https metrics no key": { + metricsAddress: defaultMetricsAdress, + metricsCertificate: defaultMetricsCertificate, + expectedError: errMetricsNoKey, + }, + "https metrics invalid certificate path": { + metricsAddress: defaultMetricsAdress, + metricsCertificate: filepath.Join(defaultDir, "domain.certificate.missing"), + metricsKey: defaultMetricsKey, + expectedError: os.ErrNotExist, + }, + "https metrics invalid key path": { + metricsAddress: defaultMetricsAdress, + metricsCertificate: defaultMetricsCertificate, + metricsKey: filepath.Join(defaultDir, "domain.key.missing"), + expectedError: os.ErrNotExist, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + metricsAddress = &tc.metricsAddress + metricsCertificate = &tc.metricsCertificate + metricsKey = &tc.metricsKey + _, err := loadMetricsConfig() + require.ErrorIs(t, err, tc.expectedError) + }) + } +} + +func setupHTTPSFixture(t *testing.T) (dir string, key string, cert string) { + t.Helper() + + tmpDir := t.TempDir() + + keyfile, err := os.CreateTemp(tmpDir, "https-fixture") + require.NoError(t, err) + key = keyfile.Name() + keyfile.Close() + + certfile, err := os.CreateTemp(tmpDir, "https-fixture") + require.NoError(t, err) + cert = certfile.Name() + certfile.Close() + + require.NoError(t, os.WriteFile(key, []byte(fixture.Key), 0644)) + require.NoError(t, os.WriteFile(cert, []byte(fixture.Certificate), 0644)) + + return tmpDir, keyfile.Name(), certfile.Name() +} 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/metrics_test.go b/test/acceptance/metrics_test.go index d1e8a1b6..193d21e7 100644 --- a/test/acceptance/metrics_test.go +++ b/test/acceptance/metrics_test.go @@ -1,6 +1,7 @@ package acceptance_test import ( + "crypto/tls" "io" "net/http" "testing" @@ -58,3 +59,27 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) { require.Contains(t, string(body), "gitlab_pages_limit_listener_concurrent_conns") require.Contains(t, string(body), "gitlab_pages_limit_listener_waiting_conns") } + +func TestMetricsHTTPSConnection(t *testing.T) { + keyFile, certFile := CreateHTTPSFixtureFiles(t) + + RunPagesProcess(t, + withExtraArgument("metrics-address", ":42345"), + withExtraArgument("metrics-certificate", certFile), + withExtraArgument("metrics-key", keyFile), + ) + + 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) +} |