Skip to content

fix(ci): skip OTel export job on fork PRs - #239

Merged
wrn14897 merged 1 commit into
mainfrom
warren/fix-otel-export-fork-prs
Jun 30, 2026
Merged

fix(ci): skip OTel export job on fork PRs#239
wrn14897 merged 1 commit into
mainfrom
warren/fix-otel-export-fork-prs

Conversation

@wrn14897

Copy link
Copy Markdown
Collaborator

Fixes the spurious red OpenTelemetry Export Trace job failure that appears on every external-contributor (fork) PR.

Problem

chart-test.yml runs on pull_request. GitHub withholds repository secrets from fork PRs, so secrets.OTLP_ENDPOINT is empty. The workflow builds the endpoint as ${{ secrets.OTLP_ENDPOINT }}/v1/traces, which collapses to just /v1/traces (no host). The corentinmusard/otel-cicd-action then falls into its gRPC exporter branch and fails:

Create tracer provider for /v1/traces
Error: [{"code":14,"details":"Name resolution failed for target dns:v1","metadata":{}}]

Example failing run (fork PR #234, head repo mfroembgen/ClickStack-helm-charts):
https://github.com/ClickHouse/ClickStack-helm-charts/actions/runs/28361825424/job/84189297741

Updating the secret value does not help — no secret reaches a fork PR run.

Fix

  • Job-level guard: only run the export job for same-repo events (push, internal PRs, schedule, workflow_dispatch); skip it on fork PRs via github.event.pull_request.head.repo.full_name == github.repository. always() is preserved so failure traces still export on internal runs.
  • Defense-in-depth: lift OTLP_ENDPOINT into a job env and gate the step on env.OTLP_ENDPOINT != , so an empty endpoint never reaches the action even on a same-repo run where the secret happens to be unset. (secrets.* is not allowed in a step-level if, hence the env indirection.)

Impact

This job is not a required status check, so this change only removes the noisy ❌ on external-contributor PRs — it does not alter merge gating. Internal runs continue exporting CI traces unchanged.

Testing

  • python3 -c "import yaml; yaml.safe_load(...)" — YAML parses.
  • Expression contexts verified manually against GitHub Actions rules (github.* in job if, env.* in step if).

External-contributor (fork) PRs do not receive repository secrets, so
secrets.OTLP_ENDPOINT is empty. The otel-cicd-action then builds the
endpoint as '/v1/traces' (no host), falls into the gRPC exporter path,
and fails with 'Name resolution failed for target dns:v1'.

Guard the OpenTelemetry Export Trace job so it only runs for same-repo
events (push, internal PRs, schedule) and skips on fork PRs. Add a
step-level env guard as defense-in-depth so an empty endpoint never
reaches the action even on same-repo runs where the secret is unset.

This job is not a required check, so this only removes the spurious red
status on external-contributor PRs; it does not change merge gating.
@wrn14897
wrn14897 requested a review from a team as a code owner June 30, 2026 01:19
@changeset-bot

changeset-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 36851c4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

No critical issues found. The job-level guard correctly evaluates for every trigger — push/schedule/workflow_dispatch (first operand true), internal PRs (head.repo.full_name == github.repository true), and fork PRs (both operands false → skipped). always() && (…) preserves failure-trace export on same-repo runs, env.* is valid in a step-level if, and lifting the secret into a job env introduces no leak since the workflow uses the pull_request (not pull_request_target) trigger and the job performs no checkout. The fix does what it claims.

🟡 P2 — recommended

  • .github/workflows/chart-test.yml:112 — The new step-level if: env.OTLP_ENDPOINT != '' converts a previously-noisy failure into a silent skip: on a same-repo run (push/schedule/dispatch) where the secret is unset or accidentally cleared, the only step is skipped, the job reports green, and trace export stops with no signal.
    • Fix: Add a preceding step that fails loudly when github.event_name != 'pull_request' && env.OTLP_ENDPOINT == '', so an unset endpoint on a same-repo run surfaces instead of passing silently.
    • reliability
🔵 P3 nitpicks (3)
  • .github/workflows/chart-test.yml:115otlpEndpoint reads secrets.OTLP_ENDPOINT directly while the step guard reads env.OTLP_ENDPOINT, leaving two independent references to the same secret; behavior is identical today but a future secret rename could update one and not the other.
    • Fix: Reference env.OTLP_ENDPOINT in otlpEndpoint so the job env block is the single source of truth.
  • .github/workflows/chart-test.yml:113 — Pre-existing: the third-party corentinmusard/otel-cicd-action@v4 is pinned to a mutable tag while receiving the OTLP_HEADERS auth token; a compromised tag could exfiltrate the header.
    • Fix: Pin the action to a full commit SHA (out of scope for this PR; track separately).
  • .github/workflows/chart-test.yml:104 — Pre-existing: the export job/step has no timeout-minutes, so a hung OTLP endpoint blocks on the default 6-hour job timeout.
    • Fix: Add timeout-minutes to the job or the export step (out of scope for this PR).

Reviewers (6): correctness, security, reliability, maintainability, testing, project-standards.

Testing gaps: Workflow conditional logic is only observable via live runs — no fork-PR run was shown confirming the job reports skipped (not failed); the silent-skip path on a same-repo run with an empty endpoint produces a green job indistinguishable from a successful export, so a stopped trace pipeline would go unnoticed without an external "traces arriving" check.

@wrn14897
wrn14897 merged commit 6592cae into main Jun 30, 2026
7 checks passed
@wrn14897
wrn14897 deleted the warren/fix-otel-export-fork-prs branch June 30, 2026 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants