AE control auth + CodeRabbit #53 followup

[entlein]

Summary: AE control surface gains bearer-JWT auth + TLS, the trigger gets a PollLimit-saturation escape, fork-cloud config restored after the main merge, and CodeRabbit Go-code findings on the umbrella PR #53 are all addressed (controller rollback, NaN-finite gate, MaxBytesReader, streaming retain-on-fail, etc.).

Test Plan: build + 14/14 AE go test packages green locally on cb81ecd / c2642da; CI-image lint (tools/linters/lint_like_ci.sh) reports 0 Errors 0 Warnings; vizier-release built aeprod22 (cb81ecd) successfully, aeprod23 (c2642da) in flight; trigger differential oracle added (oracle_test.go) pins watermark advancement + boundary dedup + PollLimit-at-capacity + the documented overflow trade-off.

Type of change: /kind feature

AE control auth + CodeRabbit #53 followup

Followup to #53 (merge #53 first — this stacks on ae-prod; I'll retarget to main once #53 lands). Captured all 48 inline + 8 review + 12 issue CodeRabbit comments to biz/PoC/log4j/ae_pr53_coderabbit_feedback/ (JSON + DIGEST.md) so nothing is lost.

✅ Done in this PR

• Bearer-JWT auth on the control surface — uses the same shared lib the broker/PEM use (px.dev/pixie/src/shared/services/utils); SetAuth verifies via jwtutils.ParseToken (signature + audience), middleware on Handler(), /healthz exempt. dx already mints this exact service JWT — it just attaches it. No new secret/crypto. Flag-gated (CONTROL_REQUIRE_AUTH, default off) for safe incremental rollout. Unit test mints a real JWT and asserts 401/pass/healthz. (CodeRabbit server.go: protect control endpoints.)
• Input validation — reject invalid t_end (≤0) and query windows (start≥end, non-positive). (server.go: reject invalid t_end/window.)

Companion change (separate, entlein/dx)

dx aeclient.go must attach Authorization: Bearer <minted JWT> on its AE control POSTs. Then flip CONTROL_REQUIRE_AUTH=true + mount PL_JWT_SIGNING_KEY on AE. (Tracked; small.)

Already resolved on ae-prod (verified — no action)

• http.Server timeouts (main.go) — already added (CodeRabbit r3379377432).
• ~9 harness Majors (ae_config/ae_vs_all/deploy_ae/exp_datavolume_extreme/exp_log4j_reps/… fail-fast) — those scripts were deleted in the harness consolidation; moot.
• The http_events=0 "bug" — self-retracted by the author (misdiagnosis).

Remaining AE-code Majors — triaged (to address as commits here)

Safe / clear:

• streaming/filter.go:196 — timer leak on deltaCh close.
• passthrough/passthrough.go:147 — per-table query/write timeouts.
• pixieapi/pixieapi.go:89 — direct-mode TLS-skip uses wrong option (→ WithDirectTLSSkipVerify).
• stats.py:57 — guard false "EXACT" when zero PASS rows.
• schema.sql:485 — retention on ae_reconcile (unbounded growth).
• controller.go:347 — don't fan out Pixie rows when attribution persistence fails.

Risky / needs careful testing before a presentation — defer with care:

• controller.go:255 — rehydrated active windows never restart fan-out (state-machine path).
• trigger/clickhouse.go:303 — inclusive-watermark paging (this is the F8 watermark area — change only with the F8 fixtures).
• schema.sql:430 — adaptive_attribution ORDER BY key width (schema change — migration risk).
• sink/clickhouse.go:433 — Record blocks hot path on CH (async refactor — perf-sensitive).

[entlein]

there are indications that it causes PEM to crash, needs RCA

[entlein]

Merged main in — conflicts resolved (now MERGEABLE). main landed #53 (production AE), which this branch builds on (+auth +TLS +#53-followups +#67). 9 files conflicted; all resolved keeping this branch's followup work on top of #53 (main's side was the pre-followup #53 in every case):

file	kept
cmd/main.go	bearer-auth block + CONTROL_TLS/ListenAndServeTLS
clickhouse/schema.sql	30d TTL on the debug-log table
control/BUILD.bazel	//src/shared/services/utils dep (auth)
control/server.go	SetAuth + auth middleware + input validation
control/server_test.go	TestControlAuth
controller/controller.go	re-arm fan-out on restart + skip-fan-out on attribution-persist-fail (CodeRabbit)
passthrough/passthrough.go	per-table context.WithTimeout (CodeRabbit)
trigger/clickhouse.go	#67 PollLimit watermark boundary-escape
trigger/clickhouse_internal_test.go	sync.Mutex race-fix on the test

Verified: 0 conflict markers, gofmt clean on all resolved .go, and signature coherence (control.New/SetAuth/jwtutils.ParseToken calls match defs). Bazel build is the CI gate (not run locally).

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/vizier/services/adaptive_export/internal/trigger/oracle_test.go`:
- Around line 351-365: The overflow test has two issues: the timeout branch in
the receive loop only breaks the select, so the outer loop can spin forever, and
the error returned by tr.Subscribe(ctx) is being ignored. Update the Subscribe
setup in oracle_test.go to check the error and fail fast with t.Fatalf, and
change the deadline case in the collected-events loop to explicitly fail the
test or exit the loop so the timeout path cannot hang.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 50bd8567-0709-4cfd-b548-c297ff735a59

📥 Commits

Reviewing files that changed from the base of the PR and between 0c65751 and 0fd9c3f.

📒 Files selected for processing (3)

• .github/workflows/vizier_release.yaml
• src/vizier/services/adaptive_export/internal/trigger/BUILD.bazel
• src/vizier/services/adaptive_export/internal/trigger/oracle_test.go

[coderabbitai]

Actionable comments posted: 17

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/vizier/services/adaptive_export/internal/trigger/clickhouse.go (1)

322-338: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift

Do not advance past the boundary timestamp without a total row order.

Line 401 only orders by normalized event_time. That means a full page of already-seen rows at the current watermark does not prove there are no unseen rows with the same timestamp after the LIMIT. Bumping the watermark by +1ns here can therefore drop real rows permanently whenever more than PollLimit rows share the boundary timestamp.

This needs a stable tie-breaker in the query/state (for example, page by event_time plus a secondary key/fingerprint) instead of skipping to the next timestamp.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vizier/services/adaptive_export/internal/trigger/clickhouse.go` around
lines 322 - 338, The paging escape in the ClickHouse trigger advances the
watermark past a boundary timestamp without a total row order, which can
permanently skip unseen rows. Update the polling logic in the trigger code path
around the boundary handling to avoid incrementing the watermark by 1ns when the
query is only ordered by normalized event_time. Instead, add a stable secondary
tie-breaker to the pagination/state in the query and cursor logic used by the
trigger so rows at the same timestamp can be paged deterministically without
dropping data.

src/vizier/services/adaptive_export/internal/chhttp/chhttp.go (1)

107-122: 🩺 Stability & Availability | 🟠 Major

Add a timeout to the boot-time Rehydrate call. cmd/main.go:443 passes the long-lived service context (context.WithCancel(context.Background())) into ctl.Rehydrate, so the new no-timeout QueryStream path can block startup indefinitely if ClickHouse stalls. Wrap that call in a deadline-bound context.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vizier/services/adaptive_export/internal/chhttp/chhttp.go` around lines
107 - 122, The boot-time ctl.Rehydrate call is using a long-lived service
context, so the new QueryStream path can block startup forever if ClickHouse
stalls. Update the startup flow around ctl.Rehydrate to create and pass a
deadline-bound context instead of the background-derived cancel-only context,
and ensure the timeout is scoped just to rehydration. Use the existing
ctl.Rehydrate call site and the QueryStream behavior in chhttp.Client to locate
the fix.

♻️ Duplicate comments (3)

src/vizier/services/adaptive_export/internal/trigger/oracle_test.go (1)

351-351: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Check the Subscribe setup error.

Line 351 still discards the error from tr.Subscribe(ctx), so a setup failure gets reported as a later collection timeout instead of failing fast at the source.

Suggested fix

-	ch, _ := tr.Subscribe(ctx)
+	ch, err := tr.Subscribe(ctx)
+	if err != nil {
+		t.Fatalf("Subscribe: %v", err)
+	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vizier/services/adaptive_export/internal/trigger/oracle_test.go` at line
351, The test setup in `Subscribe` is still ignoring the returned error, which
can hide a subscription failure until a later timeout. Update the
`tr.Subscribe(ctx)` call in `oracle_test` to handle the error immediately and
fail the test at the source, using the existing `tr` and `Subscribe` flow rather
than continuing with a potentially nil channel.

src/vizier/services/adaptive_export/internal/controller/controller.go (1)

344-382: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Do not publish unpersisted attribution state before Sink.Write succeeds.

The rollback happens after the failed write, but Lines 344-353 already mutate c.active; an existing pushPixieRows loop can re-snapshot that unpersisted extension while Sink.Write is still pending and fan out rows against a phantom attribution anchor. Stage the updated row in a local copy, write it first, then commit it to c.active under the mutex only after persistence succeeds.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vizier/services/adaptive_export/internal/controller/controller.go` around
lines 344 - 382, The attribution update in controller logic is mutating c.active
before Sink.Write succeeds, which can let pushPixieRows observe and fan out an
unpersisted snapshot. Fix the flow in the controller method that handles the row
update by staging the modified AttributionRow in a local copy, calling
c.sink.Write first, and only committing the new state into c.active under c.mu
after the write succeeds; keep the rollback path for failures, including the
inFlight bookkeeping, unchanged.

src/vizier/services/adaptive_export/internal/control/server_test.go (1)

181-189: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a non-positive window timestamp case.

These cases pin inverted and zero-length windows, but not the new Window[0] <= 0 || Window[1] <= 0 contract in handleQuery.

Proposed test addition

 	if r := do(t, srv, http.MethodPost, "/query",
 		`{"pod":"p","table":"http_events","query_id":"x","window":[5,5]}`); r.StatusCode != http.StatusBadRequest {
 		t.Fatalf("query zero-window = %d, want 400", r.StatusCode)
 	}
+	if r := do(t, srv, http.MethodPost, "/query",
+		`{"pod":"p","table":"http_events","query_id":"x","window":[0,5]}`); r.StatusCode != http.StatusBadRequest {
+		t.Fatalf("query non-positive-window = %d, want 400", r.StatusCode)
+	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vizier/services/adaptive_export/internal/control/server_test.go` around
lines 181 - 189, The query tests in server_test.go cover inverted and
zero-length windows, but they do not exercise the new non-positive timestamp
validation in handleQuery. Add a test case in the existing /query checks using
do(...) with a window where either Window[0] or Window[1] is non-positive, and
assert it returns http.StatusBadRequest; place it alongside the current window
validation cases so the handleQuery contract is pinned by the tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/mirror_demos.yaml:
- Line 12: The workflow job’s custom runner label was renamed to
oracle-vm-16cpu-64gb-x86-64, but actionlint will still fail until the shared
custom runner label list is updated to match. Update the actionlint
configuration that defines allowed self-hosted labels so it includes the new
label used by the workflow’s runs-on entry, keeping both references in sync.

In `@bazel/ui.bzl`:
- Around line 78-79: Guard the stamp export logic in the shell command built by
the `ui` rule so missing `STABLE_BUILD_TAG` or `BUILD_TIMESTAMP` entries do not
remove the leading command and break the `&&` chain before `yarn build_prod`.
Update the two `sed` expansions that reference `ctx.info_file.path` and
`ctx.version_file.path` to a no-op-safe form, such as wrapping each in `eval
"$(sed ...)"`, so the command still succeeds when a key is absent.

In `@cockpit/cloud_ingress.yaml`:
- Around line 7-16: The ingress route for cloud-proxy-service:443 defines the
backend transport but never applies it, so the serverName override is ignored.
Update cockpit/cloud_ingress.yaml to attach the Traefik service.serverstransport
annotation on the / backend route and point it to cloud-backend-transport, using
the existing ingress resource so the backend actually picks up the transport
settings.

In `@cockpit/plugin_db_updater_job.yaml`:
- Around line 15-90: Harden the Job spec for the updater workload by setting
restrictive pod/security defaults before it consumes PL_POSTGRES_PASSWORD and
PL_JWT_SIGNING_KEY. Update the Job template in plugin_db_updater_job.yaml to
disable root execution and privilege escalation, drop unnecessary Linux
capabilities, and prevent automatic service-account token mounting; apply these
settings at the pod-level and, if needed, reinforce them on the postgres-wait
and updater containers.

In `@cockpit/proxy_envoy.yaml`:
- Around line 43-50: The CORS configuration in the Envoy proxy is using a
suffix-based origin match, which is broader than the single approved host and
can match unintended subdomains. Update the cors allow_origin_string_match
setting to use an exact origin match for the same host configured in
cockpit/domain_config.yaml, and keep the rest of the cors settings unchanged so
credentialed requests are only allowed from that one origin.
- Around line 87-96: The UpstreamTlsContext for api_service only configures a
client certificate and does not verify the backend server identity. Update the
envoy transport socket configuration to add a validation_context with the
trusted CA for the upstream certificate chain and set the appropriate
sni/hostname so Envoy validates the server cert before proxying traffic. Keep
the fix localized to the UpstreamTlsContext/common_tls_context block so the
server authentication settings are applied alongside the existing
tls_certificates.

In `@src/vizier/services/adaptive_export/internal/control/server.go`:
- Around line 185-188: The decode helper currently accepts a valid first JSON
value and ignores any trailing bytes, so malformed control requests can slip
through. Update decode in server.go to use the existing json.Decoder on r.Body,
then verify there is no extra non-whitespace input remaining after the first
Decode before returning success. If any trailing junk is present, make the
request fail so the caller returns a 400.

In `@terraform/kubernetes/auth0/.terraform.lock.hcl`:
- Around line 4-6: The Auth0 provider is still allowed to resolve from a
too-broad version range, which can let lockfile regeneration pull in an untested
major. Update the required provider constraint in auth0_main.tf for auth0/auth0
to the validated series used by the lock entry, then regenerate the lockfile so
.terraform.lock.hcl stays aligned with that pinned range.

In `@terraform/kubernetes/auth0/auth0_main.tf`:
- Line 12: Disable the Auth0 provider debug logging in the committed Terraform
config by removing or setting the debug flag to false in the auth0 provider
block. Update the auth0_main.tf provider configuration so debug is not enabled
by default, keeping any temporary troubleshooting setting out of checked-in
code.
- Line 6: The Auth0 provider constraint is still too open-ended, so update the
provider version requirement in the auth0_main configuration to a bounded range
instead of only a minimum. Use the existing provider block that sets the
auth0/auth0 version and narrow it to a tested range such as a compatible patch
range or an explicit upper bound below the next major release.

In `@terraform/kubernetes/auth0/generated.tf`:
- Around line 1702-1715: The non_interactive Auth0 client is exposing
browser-based grant types that should not be enabled for this M2M app. Update
the grant_types configuration in the generated Auth0 client definition to keep
only client_credentials, and remove implicit, authorization_code, and
refresh_token unless there is a clear need for browser flows. Use the
non_interactive client block in the generated Terraform output to locate the
change.

In `@terraform/kubernetes/auth0/variables.tf`:
- Line 1: The cloud_domain variable is currently untyped and unvalidated, so
misconfiguration can slip through until runtime. Update the cloud_domain
variable in variables.tf to declare it as a string and add validation that
rejects empty values (and any other invalid format checks you deem appropriate
for callback URL usage). Keep the validation close to the variable definition so
the failure happens early when Terraform plans or applies.

In `@terraform/kubernetes/cloud_deps/base_cloud_deps.tf`:
- Around line 1-4: The kustomization_build data source for the elastic base
dependency is hardcoded to a checkout-specific relative path, which will break
when Terraform runs from a different working directory in CI. Update the elastic
data block to use a parameterized source path from a variable or shared local
used by the cloud_deps Terraform config, and keep the existing data
"kustomization_build" "elastic" identifier so the new path can be reused across
environments.

In `@terraform/kubernetes/cloud_deps/cert_manager.tf`:
- Around line 1-4: The local cert_config is indexing
var.cert_details[var.cloud_domain] too early, so a missing domain triggers an
Invalid index before the custom validation in the cert_manager logic can run.
Update the cert_manager locals/validation flow by using lookup or try when
setting cert_config, or otherwise defer the access into variable validation so
the existing guard for cloud_domain/cert_details can execute first.

In `@terraform/kubernetes/cloud_deps/core_resource_deps.tf`:
- Around line 7-110: The kubernetes_secret_v1 resources are storing sensitive
values in Terraform state via the data fields in cloud_auth, cloud_session,
db_secrets, and cloud_auth0. Refactor the secret generation and secret creation
flow so these values are managed outside Terraform state, either by moving them
to a dedicated secret manager or by provisioning the Kubernetes secrets
out-of-band and only referencing their names in this module; keep the focus on
the data.external sources and the kubernetes_secret_v1 resources involved.

In `@terraform/kubernetes/cloud_deps/env_cloud_deps.tf`:
- Around line 1-4: The kustomization_build data source in env_deps is using a
hardcoded relative path that only works for one local checkout layout. Update
the path resolution in env_cloud_deps.tf so it is derived from the
module/working context instead of a fixed "../../../k8s/cloud_deps/public"
relative path, and make sure the env_deps data block still points to the same
k8s/cloud_deps/public manifests across local runs and Terraform pipelines.

In `@terraform/kubernetes/cloud_deps/provider.tf`:
- Around line 14-22: The kubernetes and kustomization provider blocks are
hardcoded to a developer-specific kubeconfig and context, which prevents
portable auth. Update the provider configuration to use configurable inputs or
environment-based discovery instead of fixed values, and adjust the provider
definitions in provider.tf so they can work in CI and on other machines without
the cockpit-config path or default context being baked in.

---

Outside diff comments:
In `@src/vizier/services/adaptive_export/internal/chhttp/chhttp.go`:
- Around line 107-122: The boot-time ctl.Rehydrate call is using a long-lived
service context, so the new QueryStream path can block startup forever if
ClickHouse stalls. Update the startup flow around ctl.Rehydrate to create and
pass a deadline-bound context instead of the background-derived cancel-only
context, and ensure the timeout is scoped just to rehydration. Use the existing
ctl.Rehydrate call site and the QueryStream behavior in chhttp.Client to locate
the fix.

In `@src/vizier/services/adaptive_export/internal/trigger/clickhouse.go`:
- Around line 322-338: The paging escape in the ClickHouse trigger advances the
watermark past a boundary timestamp without a total row order, which can
permanently skip unseen rows. Update the polling logic in the trigger code path
around the boundary handling to avoid incrementing the watermark by 1ns when the
query is only ordered by normalized event_time. Instead, add a stable secondary
tie-breaker to the pagination/state in the query and cursor logic used by the
trigger so rows at the same timestamp can be paged deterministically without
dropping data.

---

Duplicate comments:
In `@src/vizier/services/adaptive_export/internal/control/server_test.go`:
- Around line 181-189: The query tests in server_test.go cover inverted and
zero-length windows, but they do not exercise the new non-positive timestamp
validation in handleQuery. Add a test case in the existing /query checks using
do(...) with a window where either Window[0] or Window[1] is non-positive, and
assert it returns http.StatusBadRequest; place it alongside the current window
validation cases so the handleQuery contract is pinned by the tests.

In `@src/vizier/services/adaptive_export/internal/controller/controller.go`:
- Around line 344-382: The attribution update in controller logic is mutating
c.active before Sink.Write succeeds, which can let pushPixieRows observe and fan
out an unpersisted snapshot. Fix the flow in the controller method that handles
the row update by staging the modified AttributionRow in a local copy, calling
c.sink.Write first, and only committing the new state into c.active under c.mu
after the write succeeds; keep the rollback path for failures, including the
inFlight bookkeeping, unchanged.

In `@src/vizier/services/adaptive_export/internal/trigger/oracle_test.go`:
- Line 351: The test setup in `Subscribe` is still ignoring the returned error,
which can hide a subscription failure until a later timeout. Update the
`tr.Subscribe(ctx)` call in `oracle_test` to handle the error immediately and
fail the test at the source, using the existing `tr` and `Subscribe` flow rather
than continuing with a potentially nil channel.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ffd11bf6-28d9-4236-ba9a-43653a5d3192

📥 Commits

Reviewing files that changed from the base of the PR and between 0fd9c3f and d94624a.

⛔ Files ignored due to path filters (1)

• terraform/kubernetes/cloud_deps/terraform.tfstate is excluded by !**/*.tfstate

📒 Files selected for processing (49)

• .github/workflows/cli_release.yaml
• .github/workflows/cloud_release.yaml
• .github/workflows/mirror_demos.yaml
• .github/workflows/mirror_deps.yaml
• .github/workflows/mirror_releases.yaml
• .github/workflows/operator_release.yaml
• .github/workflows/perf_common.yaml
• .sops.yaml
• bazel/ui.bzl
• cockpit/artifact_config.yaml
• cockpit/auth_deployment.yaml
• cockpit/cloud_ingress.yaml
• cockpit/domain_config.yaml
• cockpit/kustomization.yaml
• cockpit/oauth_config.yaml
• cockpit/plugin_db_updater_job.yaml
• cockpit/proxy_envoy.yaml
• cockpit/script_bundles_config.yaml
• cockpit/servers_transport.yaml
• skaffold/skaffold_cloud.yaml
• src/vizier/services/adaptive_export/cmd/main.go
• src/vizier/services/adaptive_export/internal/chhttp/chhttp.go
• src/vizier/services/adaptive_export/internal/control/server.go
• src/vizier/services/adaptive_export/internal/control/server_test.go
• src/vizier/services/adaptive_export/internal/controller/controller.go
• src/vizier/services/adaptive_export/internal/controller/controller_test.go
• src/vizier/services/adaptive_export/internal/passthrough/passthrough.go
• src/vizier/services/adaptive_export/internal/pixieapi/pixieapi.go
• src/vizier/services/adaptive_export/internal/sink/clickhouse.go
• src/vizier/services/adaptive_export/internal/sink/fastencode.go
• src/vizier/services/adaptive_export/internal/sink/integration_test.go
• src/vizier/services/adaptive_export/internal/streaming/writer.go
• src/vizier/services/adaptive_export/internal/trigger/clickhouse.go
• src/vizier/services/adaptive_export/internal/trigger/oracle_test.go
• terraform/credentials/cockpit/auth0_config.yaml
• terraform/kubernetes/auth0/.terraform.lock.hcl
• terraform/kubernetes/auth0/auth0_import.tf
• terraform/kubernetes/auth0/auth0_main.tf
• terraform/kubernetes/auth0/generated.tf
• terraform/kubernetes/auth0/terraform.tfvars
• terraform/kubernetes/auth0/variables.tf
• terraform/kubernetes/cloud_deps/.terraform.lock.hcl
• terraform/kubernetes/cloud_deps/base_cloud_deps.tf
• terraform/kubernetes/cloud_deps/cert_manager.tf
• terraform/kubernetes/cloud_deps/core_resource_deps.tf
• terraform/kubernetes/cloud_deps/env_cloud_deps.tf
• terraform/kubernetes/cloud_deps/provider.tf
• terraform/kubernetes/cloud_deps/variables.tf
• terraform/kubernetes/scripts/create_random_bytes.sh

[coderabbitai]

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "### transport definition"
sed -n '1,120p' cockpit/servers_transport.yaml

echo "\n### ingress annotations and backends"
sed -n '1,160p' cockpit/cloud_ingress.yaml

echo "\n### repo-wide transport references"
rg -n 'cloud-backend-transport|serverstransport|serversscheme|ServersTransport' cockpit

Repository: k8sstormcenter/pixie

Length of output: 2593

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "### broad repo references to cloud-backend-transport"
rg -n 'cloud-backend-transport|ServersTransport|serverstransport|serversTransport|transport' .

echo "\n### candidate service definition"
sed -n '1,220p' cockpit/cloud_proxy_service.yaml 2>/dev/null || true

echo "\n### kustomization context"
sed -n '1,120p' cockpit/kustomization.yaml

Repository: k8sstormcenter/pixie

Length of output: 50377

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "### broad repo references to cloud-backend-transport"
rg -n 'cloud-backend-transport|ServersTransport|serverstransport|serversTransport|transport' .

echo "\n### candidate service definition"
sed -n '1,220p' cockpit/cloud_proxy_service.yaml 2>/dev/null || true

echo "\n### kustomization context"
sed -n '1,120p' cockpit/kustomization.yaml

Repository: k8sstormcenter/pixie

Length of output: 50377

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' cockpit/cloud_proxy_service.yaml 2>/dev/null || true
printf '\n### traefik annotations in cockpit manifests\n'
rg -n 'traefik\.ingress\.kubernetes\.io|serverstransport|ServersTransport' cockpit/*.yaml cockpit/*.yml

Repository: k8sstormcenter/pixie

Length of output: 371

---

Attach cloud-backend-transport to the / backend

cockpit/cloud_ingress.yaml defines the transport, but the Ingress never references it. Add the Traefik service.serverstransport annotation to the cloud-proxy-service:443 route, otherwise the serverName override is ignored.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cockpit/cloud_ingress.yaml` around lines 7 - 16, The ingress route for
cloud-proxy-service:443 defines the backend transport but never applies it, so
the serverName override is ignored. Update cockpit/cloud_ingress.yaml to attach
the Traefik service.serverstransport annotation on the / backend route and point
it to cloud-backend-transport, using the existing ingress resource so the
backend actually picks up the transport settings.

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Harden this Job before mounting DB and JWT credentials into it.

Both containers inherit the runtime defaults here, so root execution, privilege escalation, and service-account token mounting stay enabled unless the cluster overrides them. That is a poor default for a pod that consumes PL_POSTGRES_PASSWORD and PL_JWT_SIGNING_KEY.

Suggested hardening

 spec:
   ttlSecondsAfterFinished: 10
   template:
     metadata:
       name: plugin-db-updater-job
       labels:
         jobgroup: plugin-db-updater
     spec:
+      automountServiceAccountToken: false
       initContainers:
       - name: postgres-wait
+        securityContext:
+          allowPrivilegeEscalation: false
+          capabilities:
+            drop: ["ALL"]
+          runAsNonRoot: true
+          seccompProfile:
+            type: RuntimeDefault
         image: ghcr.io/pixie-io/postgres:14-alpine-pl1@sha256:237c5fcf79b230979e12fe02f46e0ad29565b4ecb7cb15047197cbb9a6549e8d
         command: ['sh', '-c',
                   'until pg_isready -h ${PL_POSTGRES_HOSTNAME} -p ${PL_POSTGRES_PORT}; do
                   echo "waiting for postgres";
                   sleep 2;
                   done;']
         envFrom:
         - configMapRef:
             name: pl-db-config
       containers:
       - name: updater
+        securityContext:
+          allowPrivilegeEscalation: false
+          capabilities:
+            drop: ["ALL"]
+          runAsNonRoot: true
+          seccompProfile:
+            type: RuntimeDefault
         image: cloud-plugin-load_db:latest

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	spec:
	initContainers:
	- name: postgres-wait
	# TODO(ddelnano): This image was rebuilt from https://github.com/docker-library/postgres 14-alpine
	# to remediate a critical vulnerability. Switch back to upstream once fixed.
	# yamllint disable-line rule:line-length
	image: ghcr.io/pixie-io/postgres:14-alpine-pl1@sha256:237c5fcf79b230979e12fe02f46e0ad29565b4ecb7cb15047197cbb9a6549e8d
	command: ['sh', '-c',
	'until pg_isready -h ${PL_POSTGRES_HOSTNAME} -p ${PL_POSTGRES_PORT}; do
	echo "waiting for postgres";
	sleep 2;
	done;']
	envFrom:
	- configMapRef:
	name: pl-db-config
	containers:
	- name: updater
	image: cloud-plugin-load_db:latest
	envFrom:
	- configMapRef:
	name: pl-db-config
	- configMapRef:
	name: pl-domain-config
	- configMapRef:
	name: pl-tls-config
	volumeMounts:
	- name: certs
	mountPath: /certs
	env:
	- name: PL_POSTGRES_USERNAME
	valueFrom:
	secretKeyRef:
	name: pl-db-secrets
	key: PL_POSTGRES_USERNAME
	- name: PL_POSTGRES_PASSWORD
	valueFrom:
	secretKeyRef:
	name: pl-db-secrets
	key: PL_POSTGRES_PASSWORD
	- name: PL_JWT_SIGNING_KEY
	valueFrom:
	secretKeyRef:
	name: cloud-auth-secrets
	key: jwt-signing-key
	- name: PL_PLUGIN_SERVICE
	valueFrom:
	configMapKeyRef:
	name: pl-service-config
	key: PL_PLUGIN_SERVICE
	- name: PL_PLUGIN_REPO
	value: "k8sstormcenter/pixie-plugin"
	restartPolicy: Never
	volumes:
	- name: pl-db-secrets
	secret:
	secretName: pl-db-secrets
	- name: certs
	projected:
	sources:
	- secret:
	name: service-tls-server-certs
	items:
	- key: tls.crt
	path: server.crt
	- key: tls.key
	path: server.key
	- key: ca.crt
	path: ca.crt
	- secret:
	name: service-tls-client-certs
	items:
	- key: tls.crt
	path: client.crt
	- key: tls.key
	path: client.key
	backoffLimit: 1
	spec:
	automountServiceAccountToken: false
	initContainers:
	- name: postgres-wait
	securityContext:
	allowPrivilegeEscalation: false
	capabilities:
	drop: ["ALL"]
	runAsNonRoot: true
	seccompProfile:
	type: RuntimeDefault
	# TODO(ddelnano): This image was rebuilt from https://github.com/docker-library/postgres 14-alpine
	# to remediate a critical vulnerability. Switch back to upstream once fixed.
	# yamllint disable-line rule:line-length
	image: ghcr.io/pixie-io/postgres:14-alpine-pl1@sha256:237c5fcf79b230979e12fe02f46e0ad29565b4ecb7cb15047197cbb9a6549e8d
	command: ['sh', '-c',
	'until pg_isready -h ${PL_POSTGRES_HOSTNAME} -p ${PL_POSTGRES_PORT}; do
	echo "waiting for postgres";
	sleep 2;
	done;']
	envFrom:
	- configMapRef:
	name: pl-db-config
	containers:
	- name: updater
	securityContext:
	allowPrivilegeEscalation: false
	capabilities:
	drop: ["ALL"]
	runAsNonRoot: true
	seccompProfile:
	type: RuntimeDefault
	image: cloud-plugin-load_db:latest
	envFrom:
	- configMapRef:
	name: pl-db-config
	- configMapRef:
	name: pl-domain-config
	- configMapRef:
	name: pl-tls-config
	volumeMounts:
	- name: certs
	mountPath: /certs
	env:
	- name: PL_POSTGRES_USERNAME
	valueFrom:
	secretKeyRef:
	name: pl-db-secrets
	key: PL_POSTGRES_USERNAME
	- name: PL_POSTGRES_PASSWORD
	valueFrom:
	secretKeyRef:
	name: pl-db-secrets
	key: PL_POSTGRES_PASSWORD
	- name: PL_JWT_SIGNING_KEY
	valueFrom:
	secretKeyRef:
	name: cloud-auth-secrets
	key: jwt-signing-key
	- name: PL_PLUGIN_SERVICE
	valueFrom:
	configMapKeyRef:
	name: pl-service-config
	key: PL_PLUGIN_SERVICE
	- name: PL_PLUGIN_REPO
	value: "k8sstormcenter/pixie-plugin"
	restartPolicy: Never
	volumes:
	- name: pl-db-secrets
	secret:
	secretName: pl-db-secrets
	- name: certs
	projected:
	sources:
	- secret:
	name: service-tls-server-certs
	items:
	- key: tls.crt
	path: server.crt
	- key: tls.key
	path: server.key
	- key: ca.crt
	path: ca.crt
	- secret:
	name: service-tls-client-certs
	items:
	- key: tls.crt
	path: client.crt
	- key: tls.key
	path: client.key
	backoffLimit: 1

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cockpit/plugin_db_updater_job.yaml` around lines 15 - 90, Harden the Job spec
for the updater workload by setting restrictive pod/security defaults before it
consumes PL_POSTGRES_PASSWORD and PL_JWT_SIGNING_KEY. Update the Job template in
plugin_db_updater_job.yaml to disable root execution and privilege escalation,
drop unnecessary Linux capabilities, and prevent automatic service-account token
mounting; apply these settings at the pod-level and, if needed, reinforce them
on the postgres-wait and updater containers.

Source: Linters/SAST tools

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Use an exact CORS origin match here.

suffix: "test.austrianopencloudcommunity.org" is wider than the single host configured in cockpit/domain_config.yaml and will also match origins like https://eviltest.austrianopencloudcommunity.org. With allow_credentials: true, that broadens who can make credentialed browser requests.

Suggested fix

                   cors:
                     allow_origin_string_match:
-                    - suffix: "test.austrianopencloudcommunity.org"
+                    - exact: "https://test.austrianopencloudcommunity.org"
                     allow_methods: GET, PUT, DELETE, POST, OPTIONS

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	cors:
	allow_origin_string_match:
	- suffix: "test.austrianopencloudcommunity.org"
	allow_methods: GET, PUT, DELETE, POST, OPTIONS
	allow_headers: content-type,x-grpc-web,x-user-agent
	max_age: "1728000"
	expose_headers: grpc-status,grpc-message,grpc-timeout
	allow_credentials: true
	cors:
	allow_origin_string_match:
	- exact: "https://test.austrianopencloudcommunity.org"
	allow_methods: GET, PUT, DELETE, POST, OPTIONS
	allow_headers: content-type,x-grpc-web,x-user-agent
	max_age: "1728000"
	expose_headers: grpc-status,grpc-message,grpc-timeout
	allow_credentials: true

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cockpit/proxy_envoy.yaml` around lines 43 - 50, The CORS configuration in the
Envoy proxy is using a suffix-based origin match, which is broader than the
single approved host and can match unintended subdomains. Update the cors
allow_origin_string_match setting to use an exact origin match for the same host
configured in cockpit/domain_config.yaml, and keep the rest of the cors settings
unchanged so credentialed requests are only allowed from that one origin.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Parameterize the kustomize source path before this is used in CI.

This only works from the current checkout layout. The TODO already calls out the Azure pipeline case; if Terraform runs from a different working directory, kustomization_build will fail before any base deps are applied.

Suggested fix

 data "kustomization_build" "elastic" {
-  # TODO(ddelnano): This will need to be updated for the terraform Azure pipeline
-  path = "../../../k8s/cloud_deps/base/elastic/operator"
+  path = abspath("${path.module}/../../../k8s/cloud_deps/base/elastic/operator")
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	data "kustomization_build" "elastic" {
	# TODO(ddelnano): This will need to be updated for the terraform Azure pipeline
	path = "../../../k8s/cloud_deps/base/elastic/operator"
	}
	data "kustomization_build" "elastic" {
	path = abspath("${path.module}/../../../k8s/cloud_deps/base/elastic/operator")
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/kubernetes/cloud_deps/base_cloud_deps.tf` around lines 1 - 4, The
kustomization_build data source for the elastic base dependency is hardcoded to
a checkout-specific relative path, which will break when Terraform runs from a
different working directory in CI. Update the elastic data block to use a
parameterized source path from a variable or shared local used by the cloud_deps
Terraform config, and keep the existing data "kustomization_build" "elastic"
identifier so the new path can be reused across environments.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The cert_details guard never gets a chance to run.

Line 3 indexes var.cert_details[var.cloud_domain] while evaluating locals, so a missing domain fails with an Invalid index before Line 49's custom error can fire. Use lookup/try in the local, or move this to variable validation.

Suggested fix

 locals {
   cert_subdomain = var.cloud_domain
-  cert_config    = var.cert_details[var.cloud_domain]
+  cert_config    = lookup(var.cert_details, var.cloud_domain, null)
 }
@@
   lifecycle {
     precondition {
-      condition     = contains(keys(var.cert_details), var.cloud_domain)
+      condition     = local.cert_config != null
       error_message = "cloud_domain '${var.cloud_domain}' must have a corresponding entry in cert_details."
     }
   }
 }

Also applies to: 48-53

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/kubernetes/cloud_deps/cert_manager.tf` around lines 1 - 4, The
local cert_config is indexing var.cert_details[var.cloud_domain] too early, so a
missing domain triggers an Invalid index before the custom validation in the
cert_manager logic can run. Update the cert_manager locals/validation flow by
using lookup or try when setting cert_config, or otherwise defer the access into
variable validation so the existing guard for cloud_domain/cert_details can
execute first.

[entlein]

claude, can we assume that CONTROL_TLS is true and only implement a fallback if it is not. But it should NOT boot if TLS is off

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR expands adaptive export’s ClickHouse schema, control-surface security, trigger polling, persistence handling, streaming behavior, and live Go load-test suite. It also mounts TLS certificates, adds a direct TLS client option, updates runner labels, and excludes an unconverted load generator from Bazel.

Changes

Adaptive export runtime and validation

Layer / File(s)	Summary
ClickHouse schema and trigger polling src/vizier/services/adaptive_export/internal/clickhouse/schema.sql, src/vizier/services/adaptive_export/internal/trigger/*	Adds forensic storage objects, normalized event-time query coverage, boundary paging, and oracle tests for polling behavior.
Control surface and deployment security src/api/go/pxapi/opts.go, src/vizier/services/adaptive_export/cmd/main.go, src/vizier/services/adaptive_export/internal/control/*, k8s/vizier/bootstrap/adaptive_export_deployment.yaml	Adds direct TLS skip verification, optional JWT/TLS control serving, request limits and validation, control tests, resource limits, and certificate mounts.
Runtime persistence and streaming reliability src/vizier/services/adaptive_export/internal/controller/*, internal/passthrough/*, internal/chhttp/*, internal/streaming/*, internal/sink/*, internal/pixieapi/*	Adds attribution rollback and rehydration fan-out, bounded pulls, timeout-free streaming clients, retained failed batches, non-finite number rejection, and logging/test updates.
Live adaptive-export load-test suite src/e2e_test/adaptive_export_loadtest/*	Replaces the shell-oriented documentation with a Go module containing live environment helpers, ClickHouse injection and queries, deterministic fixtures, KPI assertions, and staged tests.
CI and repository infrastructure .bazelignore, .github/workflows/*	Excludes the Docker-built load generator from Bazel and changes release, mirror, and performance jobs to Oracle VM runners.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• k8sstormcenter/pixie#49 — Related direct node-local TLS and JWT client behavior.
• k8sstormcenter/pixie#53 — Related adaptive-export TLS, normalized trigger watermarks, and reconciliation schema changes.
• k8sstormcenter/pixie#64 — Related GitHub Actions Oracle runner label updates.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.45% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and reflects the main change: AE control auth with broader follow-up work.
Description check	✅ Passed	The description matches the changeset and summarizes AE control auth, trigger, controller, and related follow-up fixes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ae-followup-auth

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}