From f3a535db714d3944cab2bbfdd71e1896674a8afe Mon Sep 17 00:00:00 2001 From: Vimal Kumar Date: Tue, 25 Aug 2026 16:51:41 +0530 Subject: [PATCH] OLS-3990 restore OTEL Collector reconciliation and add service-ca trust for tracing Reverts the OLS-3737 change that disabled OTEL Collector reconciliation. Adds service-ca CA bundle mount to the collector deployment so the otlp/tracing exporter can verify TLS certificates signed by OpenShift's service-ca. Includes hack/deploy-jaeger-tls.sh for testing trace export. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Vimal Kumar --- .ai/spec/what/audit-logging.md | 20 +- .ai/spec/what/templog.md | 20 +- config/default/deployment-patch.yaml | 3 + hack/deploy-jaeger-tls.sh | 235 ++++++++++++++++++ .../controller/agenticintegration/assets.go | 15 +- .../agenticintegration/assets_test.go | 15 +- .../agenticintegration/reconciler.go | 17 +- .../agenticintegration/reconciler_test.go | 14 +- internal/controller/appserver/assets.go | 16 +- internal/controller/appserver/assets_test.go | 14 +- internal/controller/olsconfig_controller.go | 6 + internal/controller/otelcollector/assets.go | 1 + .../controller/otelcollector/assets_test.go | 1 + .../controller/otelcollector/deployment.go | 14 ++ .../otelcollector/deployment_test.go | 12 + internal/controller/utils/constants.go | 6 + related_images.json | 9 + 17 files changed, 350 insertions(+), 68 deletions(-) create mode 100755 hack/deploy-jaeger-tls.sh diff --git a/.ai/spec/what/audit-logging.md b/.ai/spec/what/audit-logging.md index 83eee64b7..6036a259c 100644 --- a/.ai/spec/what/audit-logging.md +++ b/.ai/spec/what/audit-logging.md @@ -6,14 +6,14 @@ Collector / Postgres storage and OTEL hub behavior: see `what/templog.md`. ## Architecture -Audit configuration is split between **service** (stdout JSON events) and **collector** (Postgres storage, optional external trace forwarding). The operator generates `olsconfig.yaml` for lightspeed-service from service fields only; `spec.audit` is collector-only. Service trace export to the in-cluster collector is currently **disabled** (OLS-3737). +Audit configuration is split between **service** (stdout JSON events, trace export to in-cluster collector) and **collector** (Postgres storage, optional external trace forwarding). The operator generates `olsconfig.yaml` for lightspeed-service from service fields only; `spec.audit` is collector-only. -```yaml +``` spec.ols.auditEventsEnabled → olsconfig.yaml audit.logging (Enabled|Disabled) → stdout compliance JSON -(disabled — OLS-3737) → olsconfig.yaml audit.otel.endpoint - → NOT injected until e2e coverage exists +always → olsconfig.yaml audit.otel.endpoint + → lightspeed-otel-collector..svc:4317 (TLS) spec.audit.logging → collector Postgres pipeline (OLS-3510+) spec.audit.tracingEndpoint → collector external trace export (OLS-3510+) @@ -47,15 +47,17 @@ spec: | olsconfig.yaml key | Source | Default | |---|---|---| | `audit.logging` | `spec.ols.auditEventsEnabled` | `Enabled` | +| `audit.otel.endpoint` | operator constant | `lightspeed-otel-collector..svc:4317` | +| `audit.otel.tls_mode` | operator constant | `Secure` | -7. **OLS-3737**: OTEL endpoint injection (`audit.otel.endpoint`, `audit.otel.tls_mode`) is **disabled** until e2e tests prove the collector pipeline works end-to-end. The service falls back to a no-op tracer when the otel section is absent. Re-enablement is tracked in OLS-3737 Phase 3. +7. Trace export to the in-cluster collector is **always** configured (gRPC OTLP). External backends are reached via the collector (`spec.audit.tracingEndpoint`), not by setting a service endpoint on the CR. 8. `spec.audit` MUST NOT affect generated `olsconfig.yaml` audit settings. 9. Changes to `spec.ols.auditEventsEnabled` MUST trigger reconciliation that regenerates `olsconfig.yaml` and rolls the app-server deployment. -10. The operator mounts the OpenShift service-ca bundle (`openshift-service-ca.crt`) at `/etc/certs/otel-collector-ca/service-ca.crt` in the app-server, adds it to `extra_ca`, and sets `OTEL_EXPORTER_OTLP_CERTIFICATE` to that path. These mounts are **retained** even while OTEL export is disabled (OLS-3737) to simplify Phase 3 re-enablement. See `tls.md`. +10. The operator mounts the OpenShift service-ca bundle (`openshift-service-ca.crt`) at `/etc/certs/otel-collector-ca/service-ca.crt` in the app-server, adds it to `extra_ca`, and sets `OTEL_EXPORTER_OTLP_CERTIFICATE` to that path for OTLP/gRPC trust. See `tls.md`. ### Reconciliation -11. The operator does not emit its own audit events. Its responsibilities are CRD schema and `olsconfig.yaml` generation for stdout audit config. OTEL Collector operand reconciliation (`OtelCollectorReady`) and in-cluster trace export are currently **disabled** (OLS-3737); see `reconciliation.md` and `templog.md`. +11. The operator does not emit its own audit events. Its responsibilities are CRD schema, `olsconfig.yaml` generation for stdout audit and in-cluster trace export, and OTEL Collector operand reconciliation (`OtelCollectorReady`). See `reconciliation.md` and `templog.md`. ## Migration (breaking change) @@ -64,8 +66,8 @@ The previous `spec.audit.logging` (`Enabled`/`Disabled`) and `spec.audit.otel` b | Previous | New | |---|---| | `spec.audit.logging: Enabled/Disabled` | `spec.ols.auditEventsEnabled: true/false` | -| `spec.audit.otel.endpoint` | removed — operator-injected endpoint (currently disabled, OLS-3737) | -| `spec.audit.otel.tlsMode: Insecure` | removed — TLS mode `Secure` when re-enabled (OLS-3737 Phase 3) | +| `spec.audit.otel.endpoint` | removed — service always exports to in-cluster collector | +| `spec.audit.otel.tlsMode: Insecure` | removed — service always uses `Secure` to collector | | (none) | `spec.audit.tracingEndpoint` — external trace export via collector | Existing CRs with the old `spec.audit` shape must be rewritten manually before upgrade. There is no conversion webhook. diff --git a/.ai/spec/what/templog.md b/.ai/spec/what/templog.md index c90da7698..50a6f1021 100644 --- a/.ai/spec/what/templog.md +++ b/.ai/spec/what/templog.md @@ -4,14 +4,14 @@ Implementation details for the lightspeed-operator's role in the templog / OTEL ## Architecture -The OTEL Collector is the in-cluster telemetry hub. Its reconciliation is currently **disabled** (OLS-3737) until e2e tests prove the pipeline works; it will not be deployed on new installs. Configuration is split between **service behavior** (stdout audit events) and **collector behavior** (Postgres storage, trace forwarding). +The OTEL Collector is the in-cluster telemetry hub. It is **always deployed** when Lightspeed is installed. Configuration is split between **service behavior** (stdout audit events, trace export to collector) and **collector behavior** (Postgres storage, trace forwarding). ```text lightspeed-service ├─ stdout JSON audit events ← spec.ols.auditEventsEnabled - └─ OTLP traces (gRPC :4317) ← disabled (OLS-3737); no-op tracer fallback + └─ OTLP traces (gRPC :4317) ← always → lightspeed-otel-collector Service -OTEL Collector (disabled — OLS-3737; re-enable in Phase 3) +OTEL Collector (always deployed) ├─ logs pipeline → Postgres ← spec.audit.logging (*bool, default true) │ (only service.name=lightspeed-agentic-sandbox) ├─ postgres_admin HTTPS :8080 ← always (templog cleanup / GET for agentic-operator) @@ -64,10 +64,12 @@ The operator generates service audit config independently of `spec.audit`: | olsconfig.yaml | Source | |----------------|--------| | `audit.logging` | `spec.ols.auditEventsEnabled` (default Enabled) | +| `audit.otel.endpoint` | Always `lightspeed-otel-collector..svc:4317` | +| `audit.otel.tls_mode` | Always `Secure` (OTLP/gRPC with TLS) | -**OLS-3737**: OTEL endpoint injection (`audit.otel.endpoint`, `audit.otel.tls_mode`) is **disabled** until e2e tests prove the collector pipeline works. The service falls back to a no-op tracer when the otel section is absent. Re-enablement tracked in OLS-3737 Phase 3. +The operator mounts the OpenShift service-ca bundle into the app-server at `/etc/certs/otel-collector-ca/service-ca.crt`, adds it to `extra_ca` in `olsconfig.yaml`, and sets `OTEL_EXPORTER_OTLP_CERTIFICATE` to that path (required for OTLP/gRPC; `extra_ca` alone is not used by the exporter). See `tls.md`. -The operator still mounts the OpenShift service-ca bundle into the app-server at `/etc/certs/otel-collector-ca/service-ca.crt`, adds it to `extra_ca` in `olsconfig.yaml`, and sets `OTEL_EXPORTER_OTLP_CERTIFICATE` to that path. These mounts are retained for Phase 3 re-enablement. See `tls.md`. +Service continues to use the existing gRPC OTLP trace exporter (`opentelemetry.exporter.otlp.proto.grpc`). ## Operator image flag ([OLS-3509](https://redhat.atlassian.net/browse/OLS-3509)) @@ -80,15 +82,13 @@ The operator still mounts the OpenShift service-ca bundle into the app-server at The Postgres bootstrap script creates only `quota` and `conversation_cache` schemas. It does **not** create the `templogs` schema or tables. -When the collector is enabled, it creates and manages the `templogs` schema, `logs` table, and indexes via the `postgres_admin` extension at collector startup (`postgres_admin` is always enabled for clients). `spec.audit.logging` only controls whether new OTLP logs are exported into that schema. The operator never drops this schema. The `logs` table uses `agentic_run_id` (AgenticRun UID, normalized to 32-char hex) and `phase` (audit phase name) as the primary query dimensions, with a composite index on `(agentic_run_id, phase)`. +The OTEL Collector always creates and manages the `templogs` schema, `logs` table, and indexes via the `postgres_admin` extension at collector startup (`postgres_admin` is always enabled for clients). `spec.audit.logging` only controls whether new OTLP logs are exported into that schema. The operator never drops this schema. The `logs` table uses `agentic_run_id` (AgenticRun UID, normalized to 32-char hex) and `phase` (audit phase name) as the primary query dimensions, with a composite index on `(agentic_run_id, phase)`. See `postgres.md` for Postgres bootstrap scope and `templog.md` (lightspeed-service repo) for table DDL semantics. ## Collector Operand ([OLS-3510](https://redhat.atlassian.net/browse/OLS-3510), [OLS-3513](https://redhat.atlassian.net/browse/OLS-3513), [OLS-3656](https://redhat.atlassian.net/browse/OLS-3656)) -> **OLS-3737**: Collector reconciliation is **disabled** — the rules below describe the target architecture when re-enabled in Phase 3. The collector code is commented out, not deleted. - -1. Deploy a single-replica Collector Deployment. Service exposes OTLP gRPC `:4317`, OTLP HTTP `:4318`, `postgres_admin` HTTPS `:8080`, and HTTPS Prometheus metrics `:8888`. Health check listens on `:13133` (pod-local; not on the Service). +1. **Always** deploy a single-replica Collector Deployment. Service exposes OTLP gRPC `:4317`, OTLP HTTP `:4318`, `postgres_admin` HTTPS `:8080`, and HTTPS Prometheus metrics `:8888`. Health check listens on `:13133` (pod-local; not on the Service). 2. Image from `GetOtelCollectorImage()`; pod scheduling from `spec.ols.deployment.otelCollector`. 3. ConfigMap pipelines driven by `spec.audit`: - `logging` true/absent → logs pipeline with `routing/logs` connector and `postgresexporter`; only OTLP logs where `service.name == "lightspeed-agentic-sandbox"` are stored in Postgres; unmatched logs go to `logs/unmatched` → `nop` @@ -116,7 +116,7 @@ Agentic-operator reads OTLP/admin endpoints from `lightspeed-agentic-configurati ## Constraints -1. Collector is a single replica (when enabled). +1. Collector is always a single replica. 2. Collector container image is operator-managed via `--otel-collector-image` (not user-supplied in CR). 3. `templogs` schema is created by the OTEL Collector, not Postgres bootstrap; the operator never drops it. 4. Only sandbox audit logs (`service.name=lightspeed-agentic-sandbox`) are routed to Postgres. diff --git a/config/default/deployment-patch.yaml b/config/default/deployment-patch.yaml index 131ebdd70..9ff0fc1ea 100644 --- a/config/default/deployment-patch.yaml +++ b/config/default/deployment-patch.yaml @@ -16,6 +16,9 @@ - op: add path: /spec/template/spec/containers/0/args/- value: --openshift-mcp-server-image=__REPLACE_OPENSHIFT_MCP_SERVER__ +- op: add + path: /spec/template/spec/containers/0/args/- + value: --otel-collector-image=__REPLACE_LIGHTSPEED_OTEL_COLLECTOR__ - op: add path: /spec/template/spec/containers/0/args/- value: --rhokp-image=__REPLACE_RHOKP__ diff --git a/hack/deploy-jaeger-tls.sh b/hack/deploy-jaeger-tls.sh new file mode 100755 index 000000000..2ac79d850 --- /dev/null +++ b/hack/deploy-jaeger-tls.sh @@ -0,0 +1,235 @@ +#!/usr/bin/env bash +# Deploy Jaeger all-in-one with TLS on OpenShift using service-ca certificates. +# The OTEL Collector's otlp/tracing exporter can then send traces securely. +# +# Usage: hack/deploy-jaeger-tls.sh [NAMESPACE] +# NAMESPACE defaults to "observability" +# +# To remove: hack/deploy-jaeger-tls.sh --delete [NAMESPACE] + +set -euo pipefail + +NAMESPACE="${2:-${1:-observability}}" +JAEGER_IMAGE="quay.io/jaegertracing/jaeger:latest" + +if [[ "${1:-}" == "--delete" ]]; then + echo "Removing Jaeger from namespace ${NAMESPACE}..." + oc delete route jaeger-query -n "${NAMESPACE}" --ignore-not-found + oc delete service jaeger-query jaeger-otlp-grpc -n "${NAMESPACE}" --ignore-not-found + oc delete deployment jaeger -n "${NAMESPACE}" --ignore-not-found + oc delete configmap jaeger-config -n "${NAMESPACE}" --ignore-not-found + oc delete serviceaccount jaeger -n "${NAMESPACE}" --ignore-not-found + echo "Done. Namespace ${NAMESPACE} left in place." + exit 0 +fi + +echo "Deploying Jaeger all-in-one with TLS in namespace: ${NAMESPACE}" + +# Create namespace if it doesn't exist +oc get namespace "${NAMESPACE}" &>/dev/null || oc create namespace "${NAMESPACE}" + +# ServiceAccount +oc apply -f - </dev/null; then + echo "TLS secret jaeger-otlp-tls is ready." + break + fi + if [[ $i -eq 30 ]]; then + echo "ERROR: Timed out waiting for jaeger-otlp-tls secret. Is the service-ca operator running?" + exit 1 + fi + sleep 2 +done + +# Jaeger v2 config — uses OTEL Collector format internally +oc apply -f - <