Skip to content

Commit 7f63749

Browse files
pixie-agententlein
authored andcommitted
feat(pem): add authenticated direct-query gRPC endpoint
Exposes VizierService.ExecuteScript directly on the normal PEM over a dedicated port (:50305 default), so node-local clients can query Carnot without a broker hop or cloud dependency. The feature is opt-in (--direct_query_enabled=false by default) so existing PEM deployments are byte-for-byte unchanged until opted in. A compile-time kill switch (--//src/vizier/services/agent/pem:direct_query=disabled) removes the feature entirely from the binary. Changes: direct_query_server.{h,cc} (new) - DirectQueryServer implements api::vizierpb::VizierService::ExecuteScript against the live PEM Carnot (reusing its table_store + metadata callback). - AuthenticateRequest: HS256 JWT verifier via BoringSSL HMAC + rapidjson. Requires aud=vizier, iss=PL, Scopes=service, valid exp. Defends against alg:none, wrong-key, expired, tampered-payload attacks. - Fail-soft startup: init failure never brings the PEM data plane down. - ExecuteScript only; mutations return UNIMPLEMENTED. direct_query_server_test.cc (new) - In-process gRPC fixture exercises the full auth metadata flow. - Auth-negative cases: no token, wrong key, expired, alg:none, truncated, tampered header/payload/sig, wrong aud/iss/scope. - Execution cases: trivial query, projection, time-range, join, concurrent. BUILD.bazel (modified) - config_setting(:direct_query_disabled) for compile-time kill switch. - New deps on //src/carnot, @boringssl//:crypto, @rapidjson. - New pl_cc_test(:direct_query_server_test). pem_manager.{h,cc} (modified) - MaybeStartDirectQueryServer() builds and starts the gRPC server after broker registration; StopImpl tears it down. - Flags: --direct_query_enabled (default false), --direct_query_port (default 50305), --direct_query_jwt_signing_key. shared/manager/manager.cc (modified) - Empty-key guard at Manager::Init: refuses an empty PL_JWT_SIGNING_KEY rather than crashing mid-stream on the first service call. DIRECT_QUERY_CONTRACT.md, DIRECT_QUERY_SECURITY.md (new) - Behavioral spec, auth requirements, threat model, key-flow diagram, tampering test matrix, TLS transport details, disable instructions. Signed-off-by: entlein <einentlein@gmail.com>
1 parent 830ff2a commit 7f63749

12 files changed

Lines changed: 2146 additions & 1 deletion

File tree

src/carnot/BUILD.bazel

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ pl_cc_library(
4444
"carnot_executable.cc",
4545
],
4646
),
47+
visibility = [
48+
"//src/carnot:__subpackages__",
49+
"//src/experimental:__subpackages__",
50+
"//src/vizier/services/agent:__subpackages__",
51+
],
4752
deps = [
4853
"//src/carnot/exec:cc_library",
4954
"//src/carnot/exec/ml:cc_library",

src/carnot/exec/BUILD.bazel

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616

1717
load("//bazel:pl_build_system.bzl", "pl_cc_binary", "pl_cc_library", "pl_cc_test", "pl_cc_test_library")
1818

19-
package(default_visibility = ["//src/carnot:__subpackages__"])
19+
package(default_visibility = [
20+
"//src/carnot:__subpackages__",
21+
"//src/vizier/services/agent:__subpackages__",
22+
])
2023

2124
pl_cc_library(
2225
name = "cc_library",

src/carnot/udf/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ load("//bazel:pl_build_system.bzl", "pl_cc_binary", "pl_cc_library", "pl_cc_test
1919
package(default_visibility = [
2020
"//src/carnot:__subpackages__",
2121
"//src/vizier/funcs:__subpackages__",
22+
"//src/vizier/services/agent:__subpackages__",
2223
])
2324

2425
pl_cc_library(

src/vizier/services/agent/pem/BUILD.bazel

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,18 @@ load("//bazel:pl_build_system.bzl", "pl_cc_binary", "pl_cc_library", "pl_cc_test
2020

2121
package(default_visibility = ["//src/vizier:__subpackages__"])
2222

23+
# Compile-time kill switch for the direct-query endpoint.
24+
# Operators who do not want the feature available even as a disabled-by-default
25+
# option build with `bazel build … --//src/vizier/services/agent/pem:direct_query=disabled`
26+
# (or `--define=PX_PEM_DIRECT_QUERY=disabled`). This propagates
27+
# `-DPX_PEM_DIRECT_QUERY_DISABLED` into cc_library; the entire feature body
28+
# in direct_query_server.cc and pem_manager.cc is `#ifndef`'d out and
29+
# stub methods return UNIMPLEMENTED. See DIRECT_QUERY_SECURITY.md.
30+
config_setting(
31+
name = "direct_query_disabled",
32+
define_values = {"PX_PEM_DIRECT_QUERY": "disabled"},
33+
)
34+
2335
pl_cc_library(
2436
name = "cc_library",
2537
srcs = glob(
@@ -30,7 +42,27 @@ pl_cc_library(
3042
],
3143
),
3244
hdrs = glob(["*.h"]),
45+
defines = select({
46+
":direct_query_disabled": ["PX_PEM_DIRECT_QUERY_DISABLED"],
47+
"//conditions:default": [],
48+
}),
3349
deps = [
50+
# direct-query server deps (direct_query_server.{h,cc}):
51+
"//src/api/proto/vizierpb:vizier_pl_cc_proto",
52+
"//src/carnot",
53+
"//src/carnot:cc_library",
54+
"//src/carnot/carnotpb:carnot_pl_cc_proto",
55+
"//src/carnot/exec:cc_library",
56+
"//src/carnot/funcs:cc_library",
57+
"//src/carnot/planner/compiler:cc_library",
58+
"//src/carnot/planpb:plan_pl_cc_proto",
59+
"//src/carnot/udf:cc_library",
60+
# HS256 verify uses BoringSSL HMAC directly and parses claims via rapidjson.
61+
"@boringssl//:crypto",
62+
"@com_github_grpc_grpc//:grpc++",
63+
"@com_github_rlyeh_sole//:sole",
64+
"@com_github_tencent_rapidjson//:rapidjson",
65+
# existing PEM deps:
3466
"//src/carnot/planner/dynamic_tracing/ir/logicalpb:logical_pl_cc_proto",
3567
"//src/integrations/grpc_clocksync:cc_library",
3668
"//src/shared/tracepoint_translation:cc_library",
@@ -51,6 +83,26 @@ pl_cc_test(
5183
],
5284
)
5385

86+
# TDD contract for the PEM direct-query endpoint.
87+
# See DIRECT_QUERY_CONTRACT.md for the behavioral spec.
88+
pl_cc_test(
89+
name = "direct_query_server_test",
90+
srcs = ["direct_query_server_test.cc"],
91+
deps = [
92+
":cc_library",
93+
"//src/api/proto/vizierpb:vizier_pl_cc_proto",
94+
"//src/carnot",
95+
"//src/carnot:cc_library",
96+
"//src/carnot/exec:cc_library",
97+
"//src/carnot/funcs:cc_library",
98+
"//src/carnot/udf:cc_library",
99+
"//src/table_store:cc_library",
100+
"@com_github_arun11299_cpp_jwt//:cpp_jwt",
101+
"@com_github_grpc_grpc//:grpc++",
102+
"@com_github_rlyeh_sole//:sole",
103+
],
104+
)
105+
54106
pl_cc_binary(
55107
name = "pem",
56108
srcs = ["pem_main.cc"],
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# PEM direct-query gRPC endpoint — contract
2+
3+
## Why
4+
5+
Today in-cluster services read PEM data by querying the **vizier-query-broker**
6+
(the standard `ExecuteScript` path). That works and is the primary path. This
7+
feature adds the *node-local* alternative: make the normal `vizier-pem` itself
8+
serve `ExecuteScript` directly over gRPC, so an on-node client can query its
9+
node-local PEM with no broker hop and no cloud dependency.
10+
11+
This capability was already proved in the experimental `standalone_pem`
12+
(`src/experimental/standalone_pem/vizier_server.h``px::vizier::agent::VizierServer`
13+
implementing `api::vizierpb::VizierService::ExecuteScript` against a local Carnot).
14+
The two differences for the real PEM:
15+
16+
1. **Metadata-connected.** The normal PEM has the metadata service, so per-pod PxL
17+
filters (`df[df.ctx['pod'] == ...]`) resolve — the gap that made standalone_pem
18+
return empty per-pod results. Reuse the PEM's existing Carnot +
19+
table_store + metadata state; do **not** stand up a second Carnot.
20+
2. **Authenticated.** standalone_pem was insecure (`WithDirectCredsInsecure`). The
21+
real PEM is in `pl` and must require a **valid cluster service JWT** (the same
22+
`jwt-signing-key` used by kelvin, query-broker, and metadata-server).
23+
24+
## The endpoint
25+
26+
- Service: `px.api.vizierpb.VizierService` (the generated gRPC service).
27+
- Method implemented: **`ExecuteScript`** (server-streaming). Mutations/tracepoints
28+
are **out of scope** — return `UNIMPLEMENTED` for `req.mutation()==true`.
29+
(standalone_pem handles mutations; the read path never mutates.)
30+
- Transport: gRPC over TLS using the in-cluster self-signed CA
31+
(`SSL::DefaultGRPCServerCreds()`). Insecure fallback only when `PL_DISABLE_SSL=1`
32+
is explicitly set for a dev/soak cluster.
33+
34+
## Config (flags / env) — gated OFF by default
35+
36+
| flag / env | default | meaning |
37+
|-------------------------------------|---------|----------------------------------------------------|
38+
| `--direct_query_enabled` / `PL_PEM_DIRECT_QUERY_ENABLED` | `false` | master switch; when false the port is never opened |
39+
| `--direct_query_port` / `PL_PEM_DIRECT_QUERY_PORT` | `50305` | gRPC listen port for the direct-query service |
40+
| `--direct_query_jwt_signing_key` / `PL_JWT_SIGNING_KEY` | `""` | HMAC key the bearer JWT must verify against |
41+
42+
Default-off so existing PEM deployments are byte-for-byte unchanged until opted in.
43+
Opt out at runtime with `PL_PEM_DIRECT_QUERY_ENABLED=false`, or compile it out with
44+
`--//src/vizier/services/agent/pem:direct_query=disabled`.
45+
46+
## Auth contract
47+
48+
Every `ExecuteScript` call MUST present `authorization: Bearer <jwt>` metadata.
49+
The JWT is verified with `PL_JWT_SIGNING_KEY` and must:
50+
- have a valid signature (HS256) against the signing key,
51+
- be unexpired (`exp` in the future),
52+
- carry a service/audience claim acceptable to vizier
53+
(minted via `GenerateJWTForService` in `src/shared/services/utils`).
54+
55+
Missing/invalid/expired token → `grpc::StatusCode::UNAUTHENTICATED`. No token must
56+
ever fall through to query execution.
57+
58+
## Behavioral contract (the executable spec → `direct_query_server_test.cc`)
59+
60+
1. **flag-off → no listener.** With `direct_query_enabled=false`, nothing listens on
61+
the port; the PEM starts exactly as today.
62+
2. **flag-on → serves ExecuteScript.** With it enabled + a signing key set, a gRPC
63+
client with a valid bearer JWT gets a streamed response (status OK) for a trivial
64+
PxL (e.g. `import px; px.display(px.DataFrame('http_events'))`).
65+
3. **auth required.** Same call with (a) no token, (b) a token signed by the wrong
66+
key, (c) an expired token → each `UNAUTHENTICATED`, no rows.
67+
4. **metadata-connected filter.** A PxL with a per-pod filter returns only that pod's
68+
rows (proves the metadata gap from standalone_pem is closed on the real PEM). May
69+
be an integration test tagged `requires_metadata` if a unit Carnot fixture can't
70+
supply pod context.
71+
5. **mutation rejected.** `req.mutation()==true``UNIMPLEMENTED` (scope guard).
72+
6. **no regression.** The existing PEM agent registration / Carnot / Stirling path is
73+
unchanged when the flag is off (assert via the existing PEM smoke/unit tests).
74+
75+
## Client integration (informational)
76+
77+
The existing in-cluster clients already have the pieces:
78+
- **JWT mint**: `GenerateJWTForService` in `src/shared/services/utils/jwt.go`; mount
79+
the `pl-cluster-secrets/jwt-signing-key` via `secretKeyRef`.
80+
- **gRPC metadata**: attach `authorization: Bearer <jwt>` to each call.
81+
- **Address**: `<HOST_IP>:50305` (HOST_IP via downward API, or the pod's node IP).
82+
- **TLS**: use `WithDisableTLSVerification` against the cluster's self-signed CA,
83+
matching the broker path.
84+
85+
## Done =
86+
87+
`direct_query_server_test.cc` green under `bazel test`, PEM image builds via the
88+
vizier-release workflow, and a live cluster shows an on-node client ruling in an
89+
e2e scenario off the node-local PEM with the same verdict it gets via the broker.

0 commit comments

Comments
 (0)