feat(queryapi): serve metric queries with server-enforced project isolation

.github/workflows/test.yml

@@ -8,7 +8,7 @@ on:
 
 jobs:
   test:
-    name: Test (${{ matrix.module }})
+    name: Test (${{ matrix.module }}${{ matrix.suite && format(' {0}', matrix.suite) || '' }})
     runs-on: ubuntu-latest
     strategy:
       matrix:
@@ -27,6 +27,9 @@ jobs:
             test-cmd: go test ./...
           - module: queryapi
             test-cmd: task test
+          - module: queryapi
+            suite: integration-prometheus
+            test-cmd: task test-integration-prometheus
           - module: cli
             test-cmd: go test -race ./...
     steps:

config/operator/iam/protected-resources/metrics.yaml

@@ -1,6 +1,5 @@
-# The metric query surface. queryapi's Prometheus-shaped routes return 501
-# today; declaring the permissions now means the endpoints cannot ship
-# ungated. See queryapi/README.md.
+# The metric query surface. queryapi's Prometheus-shaped routes are gated by
+# these permissions like the log routes. See queryapi/README.md.
 apiVersion: iam.miloapis.com/v1alpha1
 kind: ProtectedResource
 metadata:

config/queryapi/deployment.yaml

@@ -35,7 +35,10 @@ spec:
           # the rest of this list alone. Same reason activity's deployment is
           # written this way; see queryapi/README.md for a worked patch.
           args:
-            - --storage=$(STORAGE)
+            - --logs-storage=$(STORAGE)
+            - --metrics-storage=$(METRICS_STORAGE)
+            - --prometheus-url=$(PROMETHEUS_URL)
+            - --prometheus-tenancy=$(PROMETHEUS_TENANCY)
             - --secure-port=$(SECURE_PORT)
             - --tls-cert-file=$(TLS_CERT_FILE)
             - --tls-private-key-file=$(TLS_PRIVATE_KEY_FILE)
@@ -51,6 +54,16 @@ spec:
             # once it can.
             - name: STORAGE
               value: "fake"
+            # "prometheus" also needs PROMETHEUS_URL and PROMETHEUS_TENANCY.
+            # extra-label is the only tenancy mode, and it requires
+            # VictoriaMetrics: a backend that ignores extra_label applies no
+            # isolation.
+            - name: METRICS_STORAGE
+              value: "fake"
+            - name: PROMETHEUS_URL
+              value: ""
+            - name: PROMETHEUS_TENANCY
+              value: ""
             # TLS is the whole point of the identity chain, not a hardening
             # extra: only a TLS connection carries the front proxy's client
             # certificate, and only that certificate makes the forwarded

queryapi/README.md

@@ -39,12 +39,15 @@ permissions, not collections.
 
 ## Storage backends
 
-| `--storage` | Behaviour |
+| `--logs-storage` | Behaviour |
 | --- | --- |
 | `fake` (default) | Deterministic synthetic logs, a pure function of the query's time range. Same range always returns the same rows. |
 | `clickhouse` | Serves log queries and label/stream discovery against the `o11y.logs` schema via LogQL-to-SQL translation. `/readyz` pings the backend. |
 
-Metric (Prometheus-shaped) endpoints return 501 on both backends.
+| `--metrics-storage` | Behaviour |
+| --- | --- |
+| `fake` (default) | One synthetic series, `fake_up`, for the caller's project. |
+| `prometheus` | Any Prometheus query API backend, confined to the caller's project by `--prometheus-tenancy`. |
 
 ## Flags
 
@@ -71,7 +74,14 @@ them without restating the rest. See [Deployment](#deployment).
 | `--authorization-always-allow-paths` | `/healthz,/readyz,/livez,/metrics` |
 | `--authorization-webhook-cache-authorized-ttl` | `10s` |
 | `--authorization-webhook-cache-unauthorized-ttl` | `10s` |
-| `--storage` | `fake` |
+| `--logs-storage` | `fake` |
+| `--storage` | deprecated alias for `--logs-storage`; removed in the next release |
+| `--metrics-storage` | `fake` |
+| `--prometheus-url` | none; required with `--metrics-storage=prometheus` |
+| `--prometheus-tenancy` | none; required with `--metrics-storage=prometheus`. The only mode is `extra-label`, which requires VictoriaMetrics |
+| `--prometheus-project-label` | `resourcemanager_datumapis_com_project_name` |
+| `--max-response-bytes` | `33554432` (32 MiB) |
+| `--max-range` | `744h` |
 | `--fake-rate` | `2` (lines/sec) |
 | `--query-timeout` | `30s` |
 | `--default-limit` | `100` |
@@ -99,7 +109,7 @@ serving stack owns the `http.Server` and sets them (32s and 90s).
 
 ## ClickHouse environment
 
-Used only with `--storage=clickhouse`. Authentication is by client
+Used only with `--logs-storage=clickhouse`. Authentication is by client
 certificate, not password, matching `clickhouse-migrate`.
 
 | Env var | Default |
@@ -194,8 +204,8 @@ hostnames and customer identifiers. The `telemetry.miloapis.com-viewer` role
 ([`config/operator/iam/`](../config/operator/iam/)) grants both the queryapi
 route permissions and the proxy permissions (`logs/api.get`, `metrics/v1.get`
 and `metrics/v1.create`).
-The metrics routes return 501 today and are gated anyway, so they cannot ship
-unguarded.
+The metrics routes are gated the same way as the log routes, so they cannot
+ship unguarded.
 
 The split is only enforceable here, not at the aggregator, so a metadata-only
 role depends on queryapi's check holding rather than on Milo refusing the

queryapi/Taskfile.yaml

@@ -39,6 +39,33 @@ tasks:
         done
       - CLICKHOUSE_TEST_ADDR=127.0.0.1:{{.PORT}} CLICKHOUSE_TEST_PASSWORD={{.PASSWORD}} go test -count=1 -run TestStoreAgainstClickHouse ./internal/storage/clickhouse/
 
+  test-integration-prometheus:
+    desc: Run the Prometheus store's integration test against a throwaway VictoriaMetrics (needs docker)
+    vars:
+      IMAGE: victoriametrics/victoria-metrics:v1.114.0
+      CONTAINER: queryapi-vm-test
+      PORT: 18428
+    cmds:
+      - docker rm -f {{.CONTAINER}} >/dev/null 2>&1 || true
+      # latencyOffset=0: just-imported samples are queryable at once. The result
+      # cache stays on, as in production.
+      - >-
+        docker run -d --name {{.CONTAINER}} -p {{.PORT}}:8428 {{.IMAGE}}
+        -search.latencyOffset=0s -retentionPeriod=1d
+      - defer: docker rm -f {{.CONTAINER}} >/dev/null
+      - |
+        for i in $(seq 1 60); do
+          curl -fsS http://127.0.0.1:{{.PORT}}/-/healthy >/dev/null 2>&1 && break
+          sleep 1
+        done
+      # A renamed or skipped test must fail the task, not pass it: -run that
+      # matches nothing exits 0. grep without -q reads to EOF: -q would SIGPIPE tee.
+      - |
+        set -o pipefail
+        PROMETHEUS_TEST_URL=http://127.0.0.1:{{.PORT}} \
+          go test -count=1 -v -run '^TestStoreAgainstVictoriaMetrics$' ./internal/storage/prometheus/ \
+          | tee /dev/stderr | grep -- '--- PASS: TestStoreAgainstVictoriaMetrics' >/dev/null
+
   lint:
     desc: Run golangci-lint (assumes it's installed on PATH -- see CI's pinned version)
     cmds:

queryapi/apiserver.go

@@ -26,9 +26,10 @@ const kubeVersion = "1.34"
 
 // ExtraConfig is everything the generic apiserver knows nothing about.
 type ExtraConfig struct {
-	Logger *slog.Logger
-	Store  storage.LogStore
-	Query  Config
+	Logger      *slog.Logger
+	LogStore    storage.LogStore
+	MetricStore storage.MetricStore
+	Query       Config
 }
 
 // ServerConfig combines the generic apiserver configuration with queryapi's.
@@ -75,14 +76,17 @@ func NewGenericConfig() *genericapiserver.RecommendedConfig {
 // to stop the process, and there is no later point at which that is still a
 // startup failure rather than a caller's 403.
 func (c *ServerConfig) Complete() (CompletedServerConfig, error) {
-	if c.Extra.Store == nil {
+	if c.Extra.LogStore == nil {
 		return CompletedServerConfig{}, fmt.Errorf("a storage backend is required")
 	}
+	if c.Extra.MetricStore == nil {
+		return CompletedServerConfig{}, fmt.Errorf("no metric store configured")
+	}
 	if c.Extra.Logger == nil {
 		return CompletedServerConfig{}, fmt.Errorf("a logger is required")
 	}
 
-	router, err := newRouter(c.Extra.Logger, c.Extra.Store, c.Extra.Query)
+	router, err := newRouter(c.Extra.Logger, c.Extra.LogStore, c.Extra.MetricStore, c.Extra.Query)
 	if err != nil {
 		return CompletedServerConfig{}, err
 	}
@@ -122,8 +126,8 @@ func (c *ServerConfig) Complete() (CompletedServerConfig, error) {
 type Server struct {
 	GenericAPIServer *genericapiserver.GenericAPIServer
 
-	logger *slog.Logger
-	store  storage.LogStore
+	logger   *slog.Logger
+	logStore storage.LogStore
 }
 
 // New builds the server and installs everything queryapi adds to the generic
@@ -135,15 +139,18 @@ func (c CompletedServerConfig) New() (*Server, error) {
 		return nil, err
 	}
 
-	s := &Server{GenericAPIServer: generic, logger: c.extra.Logger, store: c.extra.Store}
+	s := &Server{GenericAPIServer: generic, logger: c.extra.Logger, logStore: c.extra.LogStore}
 	s.installDiscovery()
 	s.installRoutes(c.extra.Logger, c.router)
 
 	// The framework serves /readyz; this is the check that used to be a
 	// hand-registered route. /healthz and /livez stay independent of storage,
 	// or an outage that only makes queries fail would also restart the pod.
-	if err := generic.AddReadyzChecks(storageReady(c.extra.Store, c.extra.Query.QueryTimeout)); err != nil {
-		return nil, fmt.Errorf("register the storage readiness check: %w", err)
+	if err := generic.AddReadyzChecks(
+		storageReady("logs-storage", c.extra.LogStore, c.extra.Query.QueryTimeout),
+		storageReady("metrics-storage", c.extra.MetricStore, c.extra.Query.QueryTimeout),
+	); err != nil {
+		return nil, fmt.Errorf("register the storage readiness checks: %w", err)
 	}
 
 	return s, nil
@@ -208,7 +215,7 @@ func (s *Server) installRoutes(logger *slog.Logger, r *router) {
 // closing the storage backend.
 func (s *Server) Run(ctx context.Context) error {
 	defer func() {
-		closer, ok := s.store.(io.Closer)
+		closer, ok := s.logStore.(io.Closer)
 		if !ok {
 			return
 		}