From ab1e49567ed2bbd192fdd462ce52973c8b156d98 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Tue, 27 Dec 2022 19:02:48 -0800 Subject: [PATCH] gVisor: Add control command to get metric registration information. This metric registration information contains the metadata of all the metrics that the Sentry is expected to produce during its lifetime, including all possible field combinations, the types, distribution bucket boundaries, etc. This will be called during sandbox startup, before starting any container, in order to save this information in the metrics server so that it can verify the validity of instrumentation data from the Sentry once the container is started. This change has no tests, but coverage is provided in a later change that provides an end-to-end container tests that the metric server works and exports data faithfully. This change is part of a series of changes to support Prometheus-style metrics in `runsc`. Doing so requires making several seemingly-odd design decisions, due to the following architectural constraints: - Prometheus requires an HTTP server serving the `/metrics` endpoint. - For performance reasons, the `runsc boot` process cannot run the `netpoller` goroutine. - Since we don't want to write our own HTTP server implementation, this means the HTTP endpoint has to be served by a separate process that remains running during the lifetime of the container. - The `runsc boot` process is untrusted. - This means we cannot trust metrics data that comes out of the Sentry. Therefore, there needs to be an elaborate dance where we pre-register metric metadata before starting any untrusted workload. Then, the server relaying the metric data must verify the validity of metric values against this metric metadata. This avoids leaking metrics, cardinality blow-ups, and other such DoS vectors. - This feature needs to be easy-to-use in a typical Docker setting. - This means having the ability to just say `--metrics-server=localhost:1337` in the `runsc` runtime entry in `/etc/docker/daemon.json` and have that Just Work(TM), even when multiple containers are running. - Since only one process may listen on a port at a given time, this means the metric server needs to be able to multiplex requests out to multiple running sandboxes, and remain alive for the entire duration of either of these sandboxes. However, it should also die when there are no sandboxes, so that we don't end up with leftover metric servers lying around. - For this reason, the metrics server runs *outside* of the usual per-container cgroups. - This also saves system resources by not running one server per sandbox. - The metrics server must be exposed to the outside world, and cannot assume that its clients are trustworthy. - For this reason, a metrics server is bound to a runtime root directory, and double-checks all that the sandboxes it is asked to follow actually exist in this root directory. PiperOrigin-RevId: 498076197 --- pkg/metric/metric.go | 16 ++++++++++++++++ pkg/sentry/control/BUILD | 1 + pkg/sentry/control/metrics.go | 23 +++++++++++++++++++++++ runsc/boot/controller.go | 3 ++- 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/pkg/metric/metric.go b/pkg/metric/metric.go index 0fd68dddc..72e0d88cc 100644 --- a/pkg/metric/metric.go +++ b/pkg/metric/metric.go @@ -135,6 +135,7 @@ func Initialize() error { for _, s := range allStages { m.Stages = append(m.Stages, string(s)) } + allMetrics.registration = &m if err := eventchannel.Emit(&m); err != nil { return fmt.Errorf("unable to emit metric initialize event: %w", err) } @@ -143,6 +144,18 @@ func Initialize() error { return nil } +// GetMetricRegistration returns the metric registration data for all registered metrics. +// Must be called after Initialize(). +func GetMetricRegistration() (*pb.MetricRegistration, error) { + if !initialized { + return nil, errors.New("metric.GetMetricRegistration called before metric.Initialize") + } + if allMetrics.registration == nil { + return nil, errors.New("metrics are disabled") + } + return allMetrics.registration, nil +} + // Disable sends an empty metric registration event over the event channel, // disabling metric collection. // @@ -846,6 +859,9 @@ func (s stageTiming) inProgress() bool { // metricSet holds metric data. type metricSet struct { + // Metric registration data for all the metrics below. + registration *pb.MetricRegistration + // Map of uint64 metrics. uint64Metrics map[string]customUint64Metric diff --git a/pkg/sentry/control/BUILD b/pkg/sentry/control/BUILD index 430eabe4a..9a28b0b2e 100644 --- a/pkg/sentry/control/BUILD +++ b/pkg/sentry/control/BUILD @@ -38,6 +38,7 @@ go_library( "//pkg/fspath", "//pkg/log", "//pkg/metric", + "//pkg/metric:metric_go_proto", "//pkg/prometheus", "//pkg/sentry/fdimport", "//pkg/sentry/fsimpl/host", diff --git a/pkg/sentry/control/metrics.go b/pkg/sentry/control/metrics.go index 801e86a11..f7af83cee 100644 --- a/pkg/sentry/control/metrics.go +++ b/pkg/sentry/control/metrics.go @@ -16,12 +16,35 @@ package control import ( "gvisor.dev/gvisor/pkg/metric" + pb "gvisor.dev/gvisor/pkg/metric/metric_go_proto" "gvisor.dev/gvisor/pkg/prometheus" ) // Metrics includes metrics-related RPC stubs. type Metrics struct{} +// GetRegisteredMetricsOpts contains metric registration query options. +type GetRegisteredMetricsOpts struct{} + +// MetricsRegistrationResponse contains metric registration data. +type MetricsRegistrationResponse struct { + RegisteredMetrics *pb.MetricRegistration +} + +// GetRegisteredMetrics sets `out` to the metric registration information. +// Meant to be called over the control channel, with `out` as return value. +// This should be called during Sentry boot before any container starts. +// Metric registration data is used by the processes querying sandbox metrics +// to ensure the integrity of metrics exported from the untrusted sandbox. +func (u *Metrics) GetRegisteredMetrics(_ *GetRegisteredMetricsOpts, out *MetricsRegistrationResponse) error { + registration, err := metric.GetMetricRegistration() + if err != nil { + return err + } + out.RegisteredMetrics = registration + return nil +} + // MetricsExportOpts contains metric exporting options. type MetricsExportOpts struct{} diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index bc71e7be6..9ca112540 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -131,7 +131,8 @@ const ( // Metrics related commands (see metrics.go). const ( - MetricsExport = "Metrics.Export" + MetricsGetRegistered = "Metrics.GetRegisteredMetrics" + MetricsExport = "Metrics.Export" ) // Commands for interacting with cgroupfs within the sandbox.