From a47afc36423573d1cee156264123f216aa83168c Mon Sep 17 00:00:00 2001 From: Joana Carvalho Date: Sat, 3 Oct 2026 14:10:32 +0000 Subject: [PATCH] fix(collector): carry exporter auth, and stop --force from deleting it (#171) The agent and gateway templates share an exporters.otlp block with an endpoint and nothing else. Agent mode forwards "to a gateway or directly to a backend", and a backend rejects every unauthenticated batch; with retry and the sending queue on, the data sits in the queue and is then dropped. otelcol-contrib validate passes, so nothing flags it at generation time. Worse, /otel-collector --force had no preserve contract, and the receiver-auth guard only looked for `authenticator:`. A hand-added exporters.otlp.headers block was deleted on regeneration with no warning. - /otel-collector --force now reads the existing file first. Exporter headers/auth on an exporter the new config also has (otlp) are carried over verbatim; auth on an exporter the new config lacks stops the run unless --confirm-remove-exporter-auth is passed, the same shape as --confirm-remove-auth. A header value that is not an ${env:} reference is never printed. - New repeatable --exporter-header Name=${env:VAR} emits the headers block. Every value must be an environment reference; a literal is refused. The plugin does not hard-code header names per vendor: they are each backend's OTLP-ingest contract and are not verified in this repo. - Step 6 says when the generated exporter carries no credentials and what to do about it. - collector-topology gains an "Exporter auth (the outbound hop)" section. Adds a golden config with exporter headers, validated by tests/collector-validate.sh against otelcol-contrib 0.161.0 (with a check that every header value is an env reference, which fails on a literal), and tests/hooks/collector-exporter-auth.test.sh, wired into lint-hooks (10 of its 11 checks fail against the previous prose). Bumps to 0.8.17. --- .claude-plugin/plugin.json | 2 +- .github/workflows/ci.yml | 2 + ROADMAP.md | 2 +- commands/otel-collector.md | 46 +++++++++++++- skills/collector-topology/SKILL.md | 27 ++++++++ tests/collector-validate.sh | 14 +++++ tests/hooks/collector-exporter-auth.test.sh | 43 +++++++++++++ .../collector/otelcol-agent-headers.yaml.snap | 61 +++++++++++++++++++ 8 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 tests/hooks/collector-exporter-auth.test.sh create mode 100644 tests/snapshots/collector/otelcol-agent-headers.yaml.snap diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 5cb6ad6..0038983 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "otel-as-code", - "version": "0.8.16", + "version": "0.8.17", "description": "OpenTelemetry instrumentation and observability-as-code for any stack", "author": { "name": "radra.gwent", diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 22c4581..92d5fb7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,8 @@ jobs: run: bash tests/hooks/go-instrument-wiring.test.sh - name: Run PHP refusal test (detected, in scope, refused with a reason) run: bash tests/hooks/php-refusal.test.sh + - name: Run collector exporter-auth contract (--exporter-header; --force keeps exporter headers; #171) + run: bash tests/hooks/collector-exporter-auth.test.sh - name: Run force-scope contract (--force overwrites only marker-carrying files; #169) run: bash tests/hooks/force-scope.test.sh - name: Run Rust instrument-wiring contract (scanner gate, command, generator ↔ golden crate) diff --git a/ROADMAP.md b/ROADMAP.md index 886623c..072a847 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -8,7 +8,7 @@ > of the same shape, and every concrete line item that isn't done yet has a linked tracking issue — > comment there rather than only in this file if you want to weigh in. -Current release: **0.8.16**. Node.js, Python, Java, .NET, Go, Ruby, and Rust (axum) instrumentation, Terraform +Current release: **0.8.17**. Node.js, Python, Java, .NET, Go, Ruby, and Rust (axum) instrumentation, Terraform for Grafana, Datadog, New Relic, and Dash0, semconv pinned at 1.44.0. Since 0.2.0 the command set has grown past generation: `/otel-uninstrument` rolls instrumentation back behind an ownership marker, `/otel-remediate` proposes a deployment-config diff for services that have no application diff --git a/commands/otel-collector.md b/commands/otel-collector.md index 636dddb..c31ad87 100644 --- a/commands/otel-collector.md +++ b/commands/otel-collector.md @@ -1,6 +1,6 @@ --- description: Generate an otelcol-contrib config (agent or gateway mode) -argument-hint: "[agent|gateway] [--experimental] [--public] [--force] [--confirm-remove-auth] [--dry-run]" +argument-hint: "[agent|gateway] [--experimental] [--public] [--exporter-header Name=${env:VAR}] [--force] [--confirm-remove-auth] [--confirm-remove-exporter-auth] [--dry-run]" --- # /otel-collector [mode] [--experimental] [--public] @@ -18,9 +18,20 @@ Generate an otelcol-contrib config for this service's Collector setup. collector is a requirement for this to actually secure anything** — say so explicitly in Step 6. See the `collector-topology` skill's "Receiver auth" section for the exact shape and why this is opt-in rather than inferred. +- `--exporter-header ` — repeatable. Adds a header to the generated `otlp` exporter's + `headers:` block, for a Collector that exports straight to a backend or to an authenticated + gateway. **Every value must reference an environment variable (`${env:VAR}`), never a literal**: + refuse a value with no `${env:` in it and say why (a secret must not be committed, and even a + non-secret value such as a dataset name differs per environment). Example: + `--exporter-header 'Authorization=Bearer ${env:BACKEND_AUTH_TOKEN}'`. The plugin deliberately + does not pick header names per vendor — they are the vendor's OTLP-ingest contract, so take + them from the vendor's docs. Without this flag the exporter carries no credentials. - `--force` — overwrite an existing `otelcol-agent.yaml` / `otelcol-gateway.yaml`. Required if the write-guard hook blocks re-generation (it protects generated collector configs). If the existing file has receiver auth and `--public` is not also passed, `--force` alone is refused — see + `--confirm-remove-auth`. Exporter `headers:` / `auth:` already in the file are kept — see Step 3. +- `--confirm-remove-exporter-auth` — required alongside `--force` to regenerate over exporter + auth the new config cannot carry (see Step 3). An explicit, typed opt-out, like `--confirm-remove-auth`. - `--confirm-remove-auth` — required alongside `--force` (and without `--public`) to regenerate an auth-having config into an unauthenticated one. An explicit, typed opt-out for a downgrade that @@ -67,6 +78,24 @@ If the file already exists: and exit. Only proceed past this check if `--public` is set (auth stays) or the user passed `--confirm-remove-auth` alongside `--force` (an explicit, typed opt-out — never inferred, never silent). +- **With `--force`, existing file has `headers:` or `auth:` under any `exporters.`:** that + is the user's backend credential wiring (typically `Authorization: Bearer ${env:…}` or an API + key header), and the generator does not produce it, so a plain regeneration would delete it + with no warning and every export would then be rejected. Read it BEFORE generating and: + - **keep it** when the regenerated config has an exporter of the same name (the templates + define `otlp`): carry the `headers:` and `auth:` blocks over verbatim, then apply + `--exporter-header` entries on top (a flag entry replaces a kept header of the same name). + Say so: `↻ Kept exporter auth: exporters.otlp.headers (Authorization, Dash0-Dataset)`. + - **stop** when it sits on an exporter the new config does not have (e.g. `otlp/dash0`): there + is nowhere to put it. Print + ``` + ⚠ has exporter auth on exporters., which this regeneration does not produce — + it would be REMOVED. Keep that exporter by editing the file by hand, or add + --confirm-remove-exporter-auth to intentionally drop it. + ``` + and exit. Proceed only with the typed `--confirm-remove-exporter-auth`. + Never print a header VALUE that is not an `${env:…}` reference; if the existing file holds a + literal secret there, say "a literal credential" and recommend moving it to an env var. - **With `--force` otherwise:** print "↻ Overwriting `` (--force)." and authorize the write-guard by writing the absolute path of the file to the `.claude/.otel-force` sentinel. A slash-command flag cannot set an env var for the hook process, so the sentinel is how `--force` @@ -119,6 +148,11 @@ Apply `--public` (both modes — they share the `receivers.otlp` block): - If `--public` is NOT set, emit no `extensions:` block at all — the flag is the only signal for this (see the skill for why it's never inferred from `host`/deployment fields instead). +Apply exporter auth (both modes — they share the `exporters.otlp` block): add a `headers:` block +under `exporters.otlp` holding any kept headers (Step 3) and `--exporter-header` entries, each +value an `${env:VAR}` reference. Emit no `headers:` block when there are none. See the +`collector-topology` skill's "Exporter auth" section. + ## Step 5: Write the file and validate Write the YAML config file. If a `.claude/.otel-force` sentinel was created in Step 3, remove it @@ -142,6 +176,16 @@ To run a local Collector: Set in your app: OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317 ``` +If the generated exporter has no `headers:` or `auth:` block, append: +``` +Note: the exporter sends no credentials. That is right for a hop to an unauthenticated +collector on a private network; a backend or an authenticated gateway will reject every batch, +and with retry and the queue on, the data sits there and is then dropped. Add the backend's +header from an env var: re-run with --exporter-header 'Name=${env:VAR}' (header names are in +the vendor's OTLP docs), or for an authenticated gateway use the client-side bearertokenauth +shape shown below. +``` + If `--public` was **NOT** set, append one line — the flag exists precisely so this is a decision, not a silent default: ``` diff --git a/skills/collector-topology/SKILL.md b/skills/collector-topology/SKILL.md index 8cf007c..51803d3 100644 --- a/skills/collector-topology/SKILL.md +++ b/skills/collector-topology/SKILL.md @@ -242,6 +242,33 @@ exporters: queue_size: 1000 ``` +## Exporter auth (the outbound hop) + +The `exporters.otlp` block above sends no credentials. That is correct when it forwards to an +unauthenticated collector on a private network. Agent mode also "forwards to a gateway **or +directly to a backend**", and every backend's OTLP ingest requires an auth header: without one, +each batch is rejected, and because `retry_on_failure` and `sending_queue` are on, the data sits +in the queue and is then dropped (#171). The failure is silent at generation time and only +appears at export time; `otelcol-contrib validate` passes. + +Credentials go in `headers:` under the exporter, always as an environment reference, never a +literal: +```yaml +exporters: + otlp: + endpoint: ${env:OTEL_EXPORTER_OTLP_ENDPOINT} + headers: + Authorization: "Bearer ${env:BACKEND_AUTH_TOKEN}" +``` +`/otel-collector --exporter-header 'Name=${env:VAR}'` emits it. The header names are the +backend's contract (the vendor's OTLP-ingest docs), so this plugin does not hard-code them per +vendor; an authenticated gateway made by `--public` uses client-side `bearertokenauth` instead +(see `/otel-collector`'s Step 6). + +**Regeneration must not drop it.** Exporter `headers:` / `auth:` are user-owned wiring the +templates do not produce, so a `--force` regeneration keeps them on a same-named exporter and +stops, like the receiver-auth guard, if they sit on an exporter the new config lacks. + ## otelcol-contrib vs otelcol Always generate configs for `otelcol-contrib` (the distribution with all receivers, diff --git a/tests/collector-validate.sh b/tests/collector-validate.sh index f38b105..f9dcabc 100755 --- a/tests/collector-validate.sh +++ b/tests/collector-validate.sh @@ -14,6 +14,7 @@ CONFIG="tests/snapshots/collector/otelcol-agent.yaml.snap" # internet-exposed collector. NOT used by the e2e harness (its apps send no auth header) — this # golden exists solely so the --public shape is proven against a real otelcol-contrib. PUBLIC_CONFIG="tests/snapshots/collector/otelcol-agent-public.yaml.snap" +HEADERS_CONFIG="tests/snapshots/collector/otelcol-agent-headers.yaml.snap" # --------------------------------------------------------------------------- # Ordering guard: `otelcol validate` builds the component graph and checks @@ -217,3 +218,16 @@ echo "OK: every authenticator reference is defined under extensions: and listed OTEL_EXPORTER_OTLP_ENDPOINT="localhost:4317" COLLECTOR_AUTH_TOKEN="dummy" \ "$OTELCOL_BIN" validate --config="$PUBLIC_CONFIG" echo "OK: $PUBLIC_CONFIG validates against otelcol-contrib ${actual_version}" + +# Exporter auth variant (#171): the headers: block /otel-collector --exporter-header emits must be +# accepted by the real validator, with the env reference resolving. Also assert the golden never +# carries a literal credential: every header value must be an ${env:...} reference. +echo "Checking memory_limiter-first ordering in $HEADERS_CONFIG..." +check_processor_ordering "$HEADERS_CONFIG" +echo "Checking exporter header values are env references, not literals in $HEADERS_CONFIG..." +if awk '/^ headers:/{f=1;next} f&&/^ [A-Za-z]/{print} f&&!/^ /{f=0}' "$HEADERS_CONFIG" | grep -v '\${env:' | grep -q .; then + echo "FAIL: $HEADERS_CONFIG has an exporter header whose value is not an \${env:...} reference" >&2; exit 1 +fi +OTEL_EXPORTER_OTLP_ENDPOINT="localhost:4317" BACKEND_AUTH_TOKEN="dummy" BACKEND_DATASET="default" \ + "$OTELCOL_BIN" validate --config="$HEADERS_CONFIG" +echo "OK: $HEADERS_CONFIG validates against otelcol-contrib ${actual_version}" diff --git a/tests/hooks/collector-exporter-auth.test.sh b/tests/hooks/collector-exporter-auth.test.sh new file mode 100644 index 0000000..beb3ab4 --- /dev/null +++ b/tests/hooks/collector-exporter-auth.test.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +# tests/hooks/collector-exporter-auth.test.sh +# Locks the #171 contract. The agent/gateway exporter used to carry no credentials, and a --force +# regeneration deleted hand-added exporters.*.headers / auth with no warning (the receiver-auth +# guard only looked at `authenticator:`). Both fail silently: `otelcol-contrib validate` passes +# and the data is rejected at export time, then dropped from the queue. +set -uo pipefail +cd "$(dirname "$0")/../.." +pass=0; fail=0 +check() { if eval "$2"; then echo "PASS: $1"; pass=$((pass+1)); else echo "FAIL: $1"; fail=$((fail+1)); fi; } + +CMD="commands/otel-collector.md" +SKILL="skills/collector-topology/SKILL.md" +GOLD="tests/snapshots/collector/otelcol-agent-headers.yaml.snap" +STEP3=$(sed -n '/^## Step 3: Determine output path/,/^## Step 4/p' "$CMD") # captured once: grep -q + pipefail would race +has3() { grep -qF -- "$1" <<<"$STEP3"; } + +check "flag --exporter-header is in the argument hint and the flag list" \ + 'sed -n 1,4p "$CMD" | grep -qF -- "--exporter-header" && grep -qF -- "- \`--exporter-header \`" "$CMD"' +check "the flag refuses a literal value (every value must be an \${env:} reference)" \ + 'grep -qF "refuse a value with no \`\${env:\` in it" "$CMD"' +check "--force keeps exporter headers/auth on a same-named exporter" \ + 'has3 "headers:" && has3 "auth:" && has3 "carry the \`headers:\` and \`auth:\` blocks over verbatim"' +check "--force stops when exporter auth sits on an exporter the new config lacks" \ + 'has3 "does not produce" && has3 "--confirm-remove-exporter-auth"' +check "the typed opt-out is documented as a flag and in the argument hint" \ + 'grep -qF -- "- \`--confirm-remove-exporter-auth\`" "$CMD" && sed -n 1,4p "$CMD" | grep -qF -- "--confirm-remove-exporter-auth"' +check "a header value that is not an env reference is never printed" \ + 'has3 "Never print a header VALUE that is not an"' +check "generation emits the headers block from kept headers and flag entries" \ + 'grep -qF "Apply exporter auth (both modes" "$CMD"' +check "Step 6 warns when the exporter carries no credentials" \ + 'grep -qF "the exporter sends no credentials" "$CMD"' +check "topology skill documents Exporter auth and the no-literal rule" \ + 'grep -qF "## Exporter auth (the outbound hop)" "$SKILL" && grep -qF "never a" "$SKILL"' +check "topology skill states regeneration must not drop it" \ + 'grep -qF "Regeneration must not drop it." "$SKILL"' +check "golden with headers exists, uses env references only, and is validated by collector-validate.sh" \ + 'test -f "$GOLD" && grep -qF "\${env:BACKEND_AUTH_TOKEN}" "$GOLD" && grep -qF "HEADERS_CONFIG" tests/collector-validate.sh' + +echo "" +echo "Results: $pass passed, $fail failed" +[ "$fail" -eq 0 ] diff --git a/tests/snapshots/collector/otelcol-agent-headers.yaml.snap b/tests/snapshots/collector/otelcol-agent-headers.yaml.snap new file mode 100644 index 0000000..5cc46ce --- /dev/null +++ b/tests/snapshots/collector/otelcol-agent-headers.yaml.snap @@ -0,0 +1,61 @@ +# Golden agent Collector config WITH exporter auth (/otel-collector --exporter-header, #171). +# Same as otelcol-agent.yaml.snap plus exporters.otlp.headers. +# Validated standalone by tests/collector-validate.sh (#3). +receivers: + otlp: + protocols: + grpc: + endpoint: 0.0.0.0:4317 + http: + endpoint: 0.0.0.0:4318 + +processors: + memory_limiter: + check_interval: 1s + limit_mib: 256 + transform: + error_mode: ignore + trace_statements: + - context: span + statements: + - delete_key(attributes, "user.id") where IsString(attributes["user.id"]) + - delete_key(attributes, "session.id") where IsString(attributes["session.id"]) + - delete_key(attributes, "request.id") where IsString(attributes["request.id"]) + - delete_key(attributes, "order.id") where IsString(attributes["order.id"]) + batch: + timeout: 1s + send_batch_size: 1024 + +exporters: + otlp: + endpoint: ${env:OTEL_EXPORTER_OTLP_ENDPOINT} + tls: + insecure: true # local agent -> local sink; production terminates TLS (insecure: false) + headers: # backend credentials: always an ${env:...} reference (#171) + Authorization: "Bearer ${env:BACKEND_AUTH_TOKEN}" + Dash0-Dataset: "${env:BACKEND_DATASET}" + timeout: 30s + retry_on_failure: + enabled: true + initial_interval: 5s + max_interval: 30s + max_elapsed_time: 300s + sending_queue: + enabled: true + num_consumers: 10 + queue_size: 1000 + +service: + pipelines: + traces: + receivers: [otlp] + processors: [memory_limiter, transform, batch] + exporters: [otlp] + metrics: + receivers: [otlp] + processors: [memory_limiter, batch] + exporters: [otlp] + logs: + receivers: [otlp] + processors: [memory_limiter, batch] + exporters: [otlp]