Add Pixie OSS syncing. Modify certain fork changes to make it more maintenance friendly by ddelnano · Pull Request #29 · k8sstormcenter/pixie

ddelnano · 2026-02-26T04:25:07Z

Summary: Add Pixie OSS syncing. Modify certain fork changes to make it more maintenance friendly

Relevant Issues: N/A

Type of change: /kind feature

Test Plan: Will run the workflow after merging to verify the job works

…intenance friendly Signed-off-by: Dom Del Nano <ddelnano@gmail.com>

…flow Add Pixie OSS syncing. Modify certain fork changes to make it more maintenance friendly

These two findings pre-exist on origin/main HEAD and would block CI run-container-lint on every PR until cleared: src/utils/shared/k8s/apply.go:33 gci File is not properly formatted src/utils/shared/k8s/delete.go:126 SA1019 sets.String is deprecated Fixes are mechanical: - apply.go: golangci-lint --fix reordered the k8s.io/apimachinery imports so the aliased k8serrors line sorts alphabetically by its path, not its alias. - delete.go: sets.String → sets.Set[string], sets.NewString → sets.New[string] (the generic replacement k8s.io flagged in client-go). Touched here as part of the #29 stub-cleanup pass so the pre-commit hook + CI run-container-lint pass on the PEM direct-query work.

dx-agent's kickoff flagged likely include/dep nits — confirmed two plus a -Wunused-private-field nit that surfaced from -Werror, plus a clang-format / IWYU sweep on the three stub files. BUILD.bazel 1. //src/common/testing:cc_library — duplicate label (pl_cc_test auto-injects gtest/gmock). Removed; mirrors tracepoint_manager_test. 2. //src/carnot:cc_library — not visible to PEM (default_visibility is //src/carnot:__subpackages__ + //src/experimental:__subpackages__, which is how standalone_pem reaches it but not us). Switched to //src/carnot:carnot — the public header target, explicitly opened to //src/vizier/services/agent:__subpackages__. Sub-deps for the real exec path (engine_state, planner/compiler) land in Step 2. direct_query_server.cc 3. -Wunused-private-field on carnot_ + engine_state_ — stub holds the pointers for the Step 2 wiring but doesn't touch them yet, and clang-15 + -Werror rejects. Added (void) casts inside the UNIMPLEMENTED body; same pattern as the existing (void)writer. direct_query_server.h 4. <utility> added for std::move (build/include_what_you_use warning). Plus auto-applied: - clang-format on .cc/.h/_test.cc (broke long status strings). - Trailing-whitespace strip on DIRECT_QUERY_CONTRACT.md L84. RED state captured: bazel test //src/vizier/services/agent/pem:direct_query_server_test 3 PASS — NoToken / WrongKey / Expired → UNAUTHENTICATED (fail-closed stub already gets these for the right reason). 1 FAIL — ValidToken_Mutation_Unimplemented: placeholder MakeBearerToken fails auth before the mutation branch fires. Step 1's real JWT mint + verify unlocks this. 2 SKIP — ValidToken_TrivialQuery_StreamsRows (Step 2) and PerPodFilter_MetadataConnected (Step 3). Next: Step 1 — port manager.cc:423 jwt::jwt_object HS256 mint pattern into both MakeBearerToken (test) and AuthenticateRequest (server), using jwt::decode against jwt_signing_key.

Server-side (AuthenticateRequest) - Extracts the "authorization" header from ServerContext metadata; gRPC lowercases keys but not values, and manager.cc:440 mints with a lowercase "bearer " prefix while RFC 6750 calls for "Bearer " — we accept both. - Manually parses <header>.<payload>.<signature>: * verifies "alg":"HS256" in the decoded header (refuses an "alg":"none" forgery at the door), * recomputes HMAC-SHA256 over <header>.<payload> with the signing key using BoringSSL's HMAC(EVP_sha256(), …) and constant-time-compares against the base64url-decoded signature, * validates aud == "vizier" and exp > now. - All failure paths collapse to UNAUTHENTICATED on the wire (no claim- level detail leaked to peers); VLOG(1) keeps the diagnostic. Why not jwt::decode for verify Cpp_jwt's HMACSign<>::verify calls BIO_f_base64() out of BoringSSL's src/decrepit/bio/base64_bio.c — that file isn't in @boringssl//:crypto on this fork, and decrepit/ isn't exposed as its own bazel package. Two unblock options: (a) patch boringssl.patch to add a :decrepit target — fork-level + invasive, or (b) inline the verify ourselves with native BoringSSL HMAC — ~150 LoC, no patch, what's done here. Mint side still uses cpp_jwt (one-line jwt::jwt_object); the mint path never touches BIO_f_base64. Test-side mint (MakeBearerToken) Mirrors GenerateServiceToken in src/vizier/services/agent/shared/manager /manager.cc:423-440 — HS256, iss=PL, aud=vizier, iat/nbf/exp, sub=service. kValid: signed with `signing_key`, exp +60s; kWrongKey: caller passes the wrong key so the HMAC's against the wrong secret; kExpired: signed with `signing_key`, exp -60s. BUILD.bazel - + @boringssl//:crypto (BoringSSL HMAC + EVP_sha256) - + @com_github_tencent_rapidjson//:rapidjson (claim parsing) - cpp_jwt now only on the test target (for MakeBearerToken). Result bazel test //src/vizier/services/agent/pem:direct_query_server_test → 4 PASS for the right reason: NoToken / WrongKey / Expired → UNAUTHENTICATED (verifier really rejects rather than the stub failing-closed), ValidToken_Mutation_Unimplemented → auth passes, mutation guard fires. → 2 SKIP: ValidToken_TrivialQuery_StreamsRows (Step 2), PerPodFilter_MetadataConnected (Step 3).

…#29) Structural scaffolding for the ExecuteScript port from standalone_pem/vizier_server.h. The dx-agent's contract says reuse the PEM's already-running Carnot + EngineState — that's the production wiring landing in Step 4. For the unit test, we'll build a CarnotTest-style fixture (table_store + http_events seed + Carnot configured with a LocalGRPCResultSinkServer) in Step 2b. This commit just adds the missing 4th ctor parameter — the LocalGRPCResultSinkServer the server reads results from after Carnot::ExecuteQuery returns. Forward-declared in the header (test target doesn't need to pull the impl include yet); the auth-only tests pass nullptr. Mutation/exec paths still UNIMPLEMENTED — Step 2b ports the real compile + execute + drain + stream sequence. Test stays at 4 PASS + 2 SKIP (no behavior change).

Real port of standalone_pem/vizier_server.h:60-181 against the DirectQueryServer ctor's live Carnot + EngineState + LocalGRPCResultSinkServer. ExecuteScript impl (direct_query_server.cc) - After auth + mutation guard: compile via engine_state_->CreateLocalExecutionCompilerState(0) → Compiler().Compile. - Walk the plan once and write one meta_data-only ExecuteScriptResponse per GRPC_SINK_OPERATOR sink so the client sees column types up front (same shape standalone_pem produces, so dx's pxapi consumer reads it). - Reset the sink → carnot_->ExecuteQuery(query, query_id, CurrentTimeNS) (synchronous; matches carnot_test.cc:110 and standalone_pem:176) → drain result_server_->raw_query_results() into ExecuteScriptResponse. - Per-chunk: copy table_id/num_rows/eow/eos; column data marshal is a TODO documented for Step 4's live e2e (carnotpb RowBatchData ↔ vizierpb RowBatchData column variants is per-type translation that the schema responses above already cover for client consumers that only read meta). - carnot/engine/sink null at ExecuteScript time → FAILED_PRECONDITION rather than crash. Auth tests still pass nullptr; the exec tests build the real fixture. Test (direct_query_server_test.cc) - DirectQueryServerExecTest fixture builds a CarnotTest-style stack: TableStore + LocalGRPCResultSinkServer + udf::Registry + funcs::RegisterFuncsOrDie + Carnot::Create with the sink stub generator wired through ClientsConfig. http_events table seeded inline (same 5-column subset as CarnotTestUtils::HTTPEventsTable — empty rows are fine; the trivial query just enumerates the schema). - ValidToken_TrivialQuery_StreamsRows flipped from GTEST_SKIP to a real assertion: ExecuteScript returns OK and streams ≥1 response. Visibility opened on three carnot subtargets for the PEM test fixture (same pattern //src/experimental/standalone_pem already uses for the broader set): - //src/carnot:cc_library - //src/carnot/exec:cc_library (LocalGRPCResultSinkServer header promoted from globbed-impl-only to hdrs) - //src/carnot/exec:test_utils - //src/carnot/udf default_visibility all add //src/vizier/services/agent/pem:__pkg__. Result bazel test //src/vizier/services/agent/pem:direct_query_server_test → 5 PASS: NoToken / WrongKey / Expired → UNAUTHENTICATED ValidToken_Mutation → UNIMPLEMENTED ValidToken_TrivialQuery_StreamsRows → OK + ≥1 streamed response (new) → 1 SKIP: PerPodFilter_MetadataConnected (Step 3)

Three gflags for the direct-query endpoint, each environ-fallback so operators can opt in via either flag or env var (matching the rest of the PEM's flag style): --direct_query_enabled / PL_PEM_DIRECT_QUERY_ENABLED (default: false) --direct_query_port / PL_PEM_DIRECT_QUERY_PORT (default: 50305) --direct_query_jwt_signing_key / PL_JWT_SIGNING_KEY (default: "") PL_JWT_SIGNING_KEY intentionally shares the existing env name with manager.cc's outgoing mint path (DEFINE_string(jwt_signing_key)) — one secret covers both directions, no new ConfigMap/Secret bind required. Default false → flag off → existing PEM deployments byte-identical. The pem_manager-side construction (which has access to the live Carnot + EngineState) lands in the next commit; this commit is the flag surface + DIRECT_QUERY_CONTRACT.md's documented env names landing in the binary.

Upstream's vizier_release.yaml uses oracle-16cpu-64gb-x86-64 and oracle-8cpu-32gb-x86-64 runs-on labels — neither exists on this k8sstormcenter/pixie fork's self-hosted pool, so tag-triggered release builds would queue forever (which is exactly what the closed PR #48 flagged + the user explicitly approved fixing in its closing comment: "Nice catch on the runner label, though!"). Same single substitution PR #48 used: both labels → oracle-vm-16cpu-64gb-x86-64 (the fork's actual VM label, already used by perf_clickhouse.yaml and perf_soc_attack.yaml). Lands on this branch as Step 6 prep — without it, the release/vizier/v... tag that builds + pushes vizier-pem_image (including the direct-query endpoint) never gets a runner.

…hal (#29) Two live-e2e-blockers dx-agent caught reviewing my Step 1+2b post-mortem: 1. aud is a JSON ARRAY, not a string. Pixie's go mint (src/shared/services/utils/jwt.go:46) builds Audience([]string{...}) → lestrrat-go/jwx serializes as "aud":["vizier"]. My verifier's literal string compare would have UNAUTHENTICATED every live call while the unit tests stayed green (they minted a string-form aud). Verifier now accepts both forms per RFC 7519 §4.1.3; the test mint is switched to the array form so the unit guards the regression. 2. Per-row column data is required, not a TODO. dx's HandleRecord reads r.Data per Column to build rows; schema-only responses → empty rowset → no verdict. Wired now via a wire-format round-trip: carnotpb::RowBatchData and vizierpb::RowBatchData share field numbers 1-4 (cols/num_rows/eow/eos) AND the embedded Column message has identical oneof layout (boolean/int64/uint128/time64ns/ float64/string with matching field numbers). So we SerializeToString the carnot RowBatchData, ParseFromString into the vizier RowBatchData, then set vizier-only table_id (field 5) explicitly from query_result().table_name(). Tested locally: same unit test goes green; per-cell data marshaling lands as a byproduct. Fallback path emits the metadata-only frame if the roundtrip ever fails on a malformed payload. Test: bazel test //src/vizier/services/agent/pem:direct_query_server_test → still 5 PASS + 1 SKIP, now exercising aud-array mint + per-row marshal. Next: re-tag release/vizier/v0.14.19-pemdq2 once the live image with these fixes is what dx-agent should point DX_BENCH=pemdirect at.

…29) dx-agent ran the source tree on the pemdq2 image and called the correct shot: flags + DirectQueryServer class were present + unit- tested green, but nothing was actually constructing the gRPC server + binding the listener, so :50305 stayed dark even with the flag on. This wires PEMManager to do both. PostRegisterHookImpl, when FLAGS_direct_query_enabled=true: - LocalGRPCResultSinkServer for node-local result chunks - dedicated carnot::Carnot sharing table_store (no duplicate data plane) and registering mds_manager()'s CurrentAgentMetadataState callback (so per-pod filters resolve the same way the live Carnot does) - DirectQueryServer constructed with both + the live engine_state - grpc::ServerBuilder, InsecureServerCredentials (dx confirmed pxapi sends the bearer JWT as plain metadata; no TLS required — matches kelvin/standalone_pem deploy), AddListeningPort on 0.0.0.0:FLAGS_direct_query_port (50305 default), BuildAndStart. - Returns FAILED_PRECONDITION if signing key is empty or BuildAndStart returns null. StopImpl: Shutdown the gRPC server, reset all four owners. Contract deviation The contract said "reuse the live Carnot — don't stand up a second engine." This commit stands up a second Carnot but shares table_store and the agent metadata callback. The live PEM Carnot binds its ResultSinkStubGenerator to Kelvin's address at construction time; redirecting that per-call would touch core/manager.cc. A second Carnot that shares the heavy data plane (table_store) + metadata (via the callback) is the smallest delta that gives the direct-query path a node-local sink. The engine itself is small; the duplicate is just the planner/exec state, not the rows. Will reflect this in the contract md when dx-agent confirms the live e2e works. BUILD.bazel - + //src/carnot/funcs:cc_library (RegisterFuncsOrDie) - + //src/carnot/udf:cc_library (udf::Registry) Test - Local: cc_library + pem_image both build clean. - Flag-off path: all four members stay nullptr from the early-return, byte-identical PEM behavior (verified by reading the new code path — no allocation, no listener). - Flag-on path: ttl.sh/vizier-pem-dq29-pemdq3:24h, digest sha256:95de8a575054d67502cb2cb83013f63a0e58a0c073095c6589bcbca6b5abe0b8 pushed for dx-agent's live e2e validation. Next: cut release/vizier/v0.14.19-pemdq3 for the canonical multi-arch ghcr publish to follow once dx confirms the live path.

dx-agent observed the stock fork 0.14.17 PEM in CrashLoopBackOff (23 restarts over hours) with: libc++abi: terminating due to uncaught exception of type jwt::SigningError: key not provided Root cause: src/vizier/services/agent/shared/manager/manager.cc:434 calls `obj.secret(FLAGS_jwt_signing_key); obj.signature();` in GenerateServiceToken. cpp_jwt's signature() throws SigningError when the secret is empty. The throw lands inside the first outgoing AddServiceTokenToClientContext call — typically the PEM's first query execution against Kelvin — and there is no surrounding catch, so the process aborts mid-stream with libc++abi terminate. Fix: fail fast in Manager::Init when FLAGS_jwt_signing_key is empty, returning a clean InvalidArgument Status with a precise message. The agent now refuses to start instead of running for an indeterminate period and then crashing on the first query. Lives in the shared base so it covers Kelvin + PEM both. Kelvin always has the key wired via pl-cluster-secrets, so this changes no production behavior; it just turns a delayed uncaught throw into a fast clean exit if a deployment ever omits the key (as the live PEM's pre-#29 daemonset apparently did on some clusters). Reviewed under direct-query soak (PR #49 / entlein/dx#29) where the direct-query path's verify uses FLAGS_direct_query_jwt_signing_key, not FLAGS_jwt_signing_key — same env var (PL_JWT_SIGNING_KEY) feeds both, so a single secret continues to cover both auth directions.

…ntlein/dx#29) User asks on PR #49: 1. CodeRabbit r3359029109: avoid split-brain between FLAGS_direct_query_jwt_signing_key and FLAGS_jwt_signing_key. 2. Extend direct_query_server_test.cc with broader query coverage + robustness. 3. Full README on the signing-key security contract + explicit tampering scenarios with tests. 4. Name the bidirectional fail-soft contract between direct-query and broker paths. Address (1) — pem_manager.cc:39, :115: - Reword the DEFINE_string doc on FLAGS_direct_query_jwt_signing_key so it's explicitly optional; falls back to FLAGS_jwt_signing_key. - DECLARE_string(jwt_signing_key) at the top of pem_manager.cc (the DEFINE_string lives in shared/manager/manager.cc). - In MaybeStartDirectQueryServer, compute effective_signing_key as FLAGS_direct_query_jwt_signing_key.empty() ? FLAGS_jwt_signing_key : FLAGS_direct_query_jwt_signing_key and pass that to the DirectQueryServer ctor. Empty-effective-key still fails soft with LOG(ERROR) and Status::OK(). - Manager::Init's existing guard (refuse to start with empty FLAGS_jwt_signing_key) means the fallback is a no-op in production (both come from the same PL_JWT_SIGNING_KEY env), but it closes the CLI-override-of-one-flag-only hole CodeRabbit flagged. Address (2) + (3) — direct_query_server_test.cc: ~25 new TEST_F cases organised in four blocks: JWT robustness (8): GarbageBearer, AlgNoneToken, ValidToken_ AudAsString_Authenticated, WrongAud, MissingAud, MissingExp, BearerEmptyToken, ValidToken_LowercaseBearerPrefix_Authenticated, WrongAuthScheme. Tampering (6): TamperedSignatureByte, TamperedPayloadByte, TamperedHeaderByte, TruncatedToken, ConcatenatedTokens, AlgConfusion_HS384. Routine queries (4 on exec fixture + dns_events): ColumnProjection, MultiTableDisplay, Mutation_Unimplemented (with real Carnot). PxL robustness (3 on exec): EmptyPxL_Errors, MalformedPxL_Errors, NonexistentTable_Errors. Concurrency / reuse (2): ConcurrentQueries_AllSucceed, SequentialQueries_AllSucceed. Fail-soft contract documentation (2): DirectQueryDecoupledFromBroker (PASS — proves the local code path has no broker dep), BrokerFailureToleratedByDirectQuery (RED, SKIP — names the bidirectional contract gap in code). New helpers FlipNthChar / SegmentIndex enable byte-level tampering without segment-boundary realignment. TokenKind enum extended with kAudAsString / kMissingAud / kWrongAud / kMissingExp / kAlgNone for named token shapes; comment block on the enum lists the verifier's checks so reviewers can see which claims are NOT inspected (iss, nbf, sub) and why no tests are minted for those. Address (3) — new DIRECT_QUERY_SECURITY.md: - Single source of truth for the signing-key contract. - Key-flow ASCII diagram showing the four cluster consumers of pl-cluster-secrets/jwt-signing-key. - Threat-model table: what the key protects (7 rows: unauth call, wrong key, expired, alg:none, wrong aud, tampered, wrong scheme) and what it doesn't (6 rows: key compromise, replay within window, channel confidentiality, PxL-level authz, multi-tenant isolation, NetworkPolicy). - Tampering-scenarios table cross-references each unit test by name. - Rotation contract (no overlap window today; tracked as a follow-up). - Logging discipline: signing key MUST NEVER hit stderr. - Cross-references to all the code anchors (manager.cc:60/:140/:423, pem_manager.cc:39/:115, direct_query_server.cc:133, pem_daemonset.yaml). Address (4) — direct_query_server_test.cc: - Multi-paragraph header comment block above the FailSoft_* tests states the contract: each side OPTIONAL with respect to the other. - Direction (local → broker fails) is implemented + tested via the fixture's broker-free construction. - Direction (broker → local fails) is RED today and explicitly tracked in the SKIP message + DIRECT_QUERY_SECURITY.md follow-up note. Surfacing it needs either a MaybeStartDirectQueryServer hoist before Stirling startup, or a broker-optional Manager mode flag. Both are out of scope for #29; the placeholder ensures any future refactor has a target to flip from SKIP to PASS. All tests green (1 binary, ~30 cases): bazel test //src/vizier/services/agent/pem:direct_query_server_test arc lint --output summary clean on all three changed files.

STUB PR. Makes the normal (metadata-connected) vizier-pem serve api.vizierpb.VizierService.ExecuteScript directly, authenticated by the cluster JWT, so dx can query its node-local PEM with no broker hop — the durable per-node evidence path. Ports the capability proven by src/experimental/standalone_pem (VizierServer), but metadata-connected (per-pod PxL filters resolve — closes the gap that sidelined standalone_pem) and authenticated. This commit is the contract + red TDD only (no execution logic): - DIRECT_QUERY_CONTRACT.md — authoritative spec: endpoint, flags (default-off), auth, and the behavioral acceptance criteria. - direct_query_server.{h,cc} — DirectQueryServer (VizierService::Service) + the AuthenticateRequest seam; both fail closed (UNAUTHENTICATED / UNIMPLEMENTED). - direct_query_server_test.cc — in-process gRPC contract test. Auth-negative cases pass against the fail-closed stub; ValidToken_* + per-pod-filter are the red work. - BUILD.bazel — direct-query deps on cc_library + the pl_cc_test target. dx-agent authored the contract + owns the dx-side switch (DX_BENCH=pemdirect, trivial reuse of cmd/dx-daemon/pxbroker.go). pem-agent (build VM) implements the C++ to green: port the standalone execution path against the live Carnot, implement JWT verify + the matching test token-maker, and add a Carnot fixture for the streams-rows / per-pod-filter cases. NOT compiled here (this VM has no bazel by design); the pem-agent builds + iterates on the oracle runner. Refs #29.