OCPBUGS-99935, OCPBUGS-100030: fix: replace opentelemetry-jaeger+thrift with opentelemetry-otlp - #1075
Conversation
WalkthroughThe tracing stack now uses OpenTelemetry 0.30 with OTLP HTTP export instead of Jaeger. Span attributes use ChangesOpenTelemetry OTLP migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Service
participant CommonsTracing
participant OpenTelemetrySDK
participant OTLPHTTPExporter
Service->>CommonsTracing: initialize tracing
CommonsTracing->>OpenTelemetrySDK: configure provider and cincinnati tracer
OpenTelemetrySDK->>OTLPHTTPExporter: configure OTLP HTTP export
Service->>CommonsTracing: create spans with parent context
CommonsTracing->>OTLPHTTPExporter: export spans
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@tmshort: This pull request references Jira Issue OCPBUGS-99935, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-100030, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@tmshort: This pull request references Jira Issue OCPBUGS-99935, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
This pull request references Jira Issue OCPBUGS-100030, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@tmshort: This pull request references Jira Issue OCPBUGS-99935, which is valid. 3 validation(s) were run on this bug
This pull request references Jira Issue OCPBUGS-100030, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@commons/Cargo.toml`:
- Around line 22-24: Pin all OpenTelemetry dependency requirements to the exact
approved 0.32 release using exact-version syntax in commons/Cargo.toml lines
22-24, cincinnati/Cargo.toml line 37, graph-builder/Cargo.toml line 40,
metadata-helper/Cargo.toml line 22, and policy-engine/Cargo.toml line 32; then
regenerate Cargo.lock and verify supported dependency hashes.
In `@commons/src/tracing.rs`:
- Around line 120-124: Update set_span_tags to stop exporting arbitrary inbound
headers; either remove the header iteration entirely or restrict it to a small
reviewed allow-list of safe header names. For any allowed header, replace
the_str().unwrap() with fallible handling that skips values rejected by
to_str(), while preserving the path attribute.
- Around line 37-45: Update the SdkTracerProvider builder to use
with_batch_exporter(exporter) instead of with_simple_exporter(exporter), keeping
the existing sampler, resource, and exporter configuration unchanged so span
exports do not block request processing.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: eaacb635-832c-46e9-9367-528c13e52e82
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
cincinnati/Cargo.tomlcincinnati/src/plugins/internal/cincinnati_graph_fetch.rscommons/Cargo.tomlcommons/src/tracing.rsgraph-builder/Cargo.tomlgraph-builder/src/config/settings.rsgraph-builder/src/main.rsmetadata-helper/Cargo.tomlmetadata-helper/src/config/settings.rspolicy-engine/Cargo.tomlpolicy-engine/src/config/settings.rs
| opentelemetry = "0.32" | ||
| opentelemetry_sdk = { version = "0.32", features = ["trace"] } | ||
| opentelemetry-otlp = { version = "0.32", features = ["http-proto", "reqwest-client"] } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin the OpenTelemetry dependencies exactly.
These "0.32" requirements permit later 0.32.x releases during a future lockfile update. Pin the reviewed release with an exact requirement and regenerate Cargo.lock.
commons/Cargo.toml#L22-L24: Change each OpenTelemetry requirement to an exact approved version.cincinnati/Cargo.toml#L37-L37: Change the OpenTelemetry requirement to an exact approved version.graph-builder/Cargo.toml#L40-L40: Change the OpenTelemetry requirement to an exact approved version.metadata-helper/Cargo.toml#L22-L22: Change the OpenTelemetry requirement to an exact approved version.policy-engine/Cargo.toml#L32-L32: Change the OpenTelemetry requirement to an exact approved version.
As per path instructions, “Pin exact versions; verify hashes where supported.”
📍 Affects 5 files
commons/Cargo.toml#L22-L24(this comment)cincinnati/Cargo.toml#L37-L37graph-builder/Cargo.toml#L40-L40metadata-helper/Cargo.toml#L22-L22policy-engine/Cargo.toml#L32-L32
🤖 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 `@commons/Cargo.toml` around lines 22 - 24, Pin all OpenTelemetry dependency
requirements to the exact approved 0.32 release using exact-version syntax in
commons/Cargo.toml lines 22-24, cincinnati/Cargo.toml line 37,
graph-builder/Cargo.toml line 40, metadata-helper/Cargo.toml line 22, and
policy-engine/Cargo.toml line 32; then regenerate Cargo.lock and verify
supported dependency hashes.
Source: Path instructions
| let provider = SdkTracerProvider::builder() | ||
| .with_sampler(Sampler::AlwaysOn) | ||
| .with_resource( | ||
| Resource::builder_empty() | ||
| .with_attribute(KeyValue::new("service.name", name)) | ||
| .build(), | ||
| ) | ||
| .with_simple_exporter(exporter) | ||
| .with_config(Config { | ||
| sampler: Box::new(Sampler::AlwaysOn), | ||
| ..Default::default() | ||
| }) | ||
| .build(); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate file ---'
wc -l commons/src/tracing.rs
ast-grep outline commons/src/tracing.rs
sed -n '1,120p' commons/src/tracing.rs
printf '%s\n' '--- relevant dependency declarations ---'
rg -n -C 3 'opentelemetry|opentelemetry-otlp|tracing-opentelemetry|actix|graph-builder' --glob 'Cargo.toml' --glob 'Cargo.lock' .
printf '%s\n' '--- tracing setup and span call sites ---'
rg -n -C 4 'init_tracer|SdkTracerProvider|SimpleSpanProcessor|BatchSpanProcessor|with_simple_exporter|with_batch_exporter|tracing_opentelemetry|TracerProvider|span!|instrument' --glob '*.rs' .Repository: openshift/cincinnati
Length of output: 27029
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- graph-builder middleware ---'
sed -n '1,180p' graph-builder/src/main.rs
printf '%s\n' '--- policy-engine and metadata-helper middleware ---'
sed -n '90,190p' policy-engine/src/main.rs
sed -n '100,190p' metadata-helper/src/main.rs
printf '%s\n' '--- all tracing span construction and completion paths ---'
rg -n -C 6 'get_tracer|span\(|start\(|end\(|FutureExt|mark_span_as_active|with_context|set_span_tags' --glob '*.rs' graph-builder policy-engine metadata-helper commonsRepository: openshift/cincinnati
Length of output: 36832
🌐 Web query:
OpenTelemetry Rust opentelemetry_sdk 0.32 SimpleSpanProcessor on_end exporter export synchronous BatchSpanProcessor with_http reqwest-client implementation
💡 Result:
In OpenTelemetry Rust 0.32, span processors are responsible for handling spans after they are finished [1]. The two primary built-in processors, SimpleSpanProcessor and BatchSpanProcessor, handle export synchronization differently [2][3]. SimpleSpanProcessor: This processor calls the configured SpanExporter's export method synchronously on the same thread that ends the span [2]. It does not perform any batching [2]. Because it is synchronous, it is primarily intended for debugging, testing, or low-throughput scenarios where immediate export is required [2][1]. If using an async exporter like one configured with reqwest-client, you must ensure the span is ended on a thread compatible with the runtime [2][1]. BatchSpanProcessor: This processor buffers finished spans in a memory queue and exports them in batches [4][3]. It operates asynchronously, typically using a background thread to manage the queue and perform exports, which prevents the application's main execution flow from blocking [5][3]. It is recommended for high-throughput environments [2][3]. Reqwest Client Integration: When using the OTLP exporter, the reqwest-client feature enables the use of the asynchronous Reqwest HTTP client [6][7]. The exporter supports automatic selection based on enabled features, with a priority order that favors reqwest-client over hyper-client and reqwest-blocking-client if multiple are enabled [8][9]. When using reqwest-client (async): - The TracerProvider can be created anywhere [2]. - Spans must be emitted (ended) from a thread where the tokio runtime is active, as the exporter uses async network calls [2][1]. - If you use SimpleSpanProcessor, the synchronous call to the exporter will execute on the thread ending the span; if that thread is not part of the tokio runtime, the async exporter operation may fail [2][1]. - BatchSpanProcessor handles this more gracefully by managing the export in a dedicated background worker [3]. To implement, enable the reqwest-client feature in your Cargo.toml for the opentelemetry-otlp crate [6][7]. Ensure that your exporter configuration utilizes the async client, which will be auto-selected if the feature is enabled and no other client is manually specified [8][9].
Citations:
- 1: https://www.mintlify.com/open-telemetry/opentelemetry-rust/traces/span-processors
- 2: https://docs.rs/opentelemetry_sdk/latest/opentelemetry_sdk/trace/struct.SimpleSpanProcessor.html
- 3: https://docs.rs/opentelemetry_sdk/latest/opentelemetry_sdk/trace/struct.BatchSpanProcessor.html
- 4: https://github.com/open-telemetry/opentelemetry-specification/blob/de3a38878e8535edc5a7acbd8b713be4f4a14031/specification/sdk-span-processors.md
- 5: https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-sdk/src/trace/span_processor.rs
- 6: https://docs.rs/crate/opentelemetry-otlp/latest/source/Cargo.toml
- 7: https://open-telemetry-opentelemetry-rust.mintlify.app/api/opentelemetry-otlp
- 8: https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/CHANGELOG.md
- 9: https://github.com/open-telemetry/opentelemetry-rust/blob/69ab1c4e/opentelemetry-otlp/src/exporter/http/mod.rs
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from urllib.request import Request, urlopen
from html import unescape
import re
urls = {
"span_processor": "https://docs.rs/opentelemetry_sdk/0.32.1/src/opentelemetry_sdk/trace/span_processor.rs.html",
"builder": "https://docs.rs/opentelemetry_sdk/0.32.1/src/opentelemetry_sdk/trace/provider.rs.html",
"otlp_http": "https://docs.rs/opentelemetry-otlp/0.32.0/src/opentelemetry_otlp/exporter/http/mod.rs.html",
"otlp_manifest": "https://docs.rs/crate/opentelemetry-otlp/0.32.0/source/Cargo.toml",
}
for label, url in urls.items():
print(f"--- {label}: {url} ---")
try:
req = Request(url, headers={"User-Agent": "review-readonly-probe"})
text = unescape(urlopen(req, timeout=15).read().decode("utf-8", "replace"))
text = re.sub(r"<[^>]+>", "", text)
lines = [line.strip() for line in text.splitlines() if line.strip()]
needles = ("SimpleSpanProcessor", "BatchSpanProcessor", "with_batch_exporter",
"with_simple_exporter", "export(", "reqwest-client", "runtime")
hits = [line for line in lines if any(n in line for n in needles)]
for line in hits[:100]:
print(line[:300])
except Exception as exc:
print(type(exc).__name__, str(exc))
PYRepository: openshift/cincinnati
Length of output: 1152
🌐 Web query:
site:docs.rs/opentelemetry_sdk/0.32.1 SdkTracerProvider with_batch_exporter BatchSpanProcessor opentelemetry_sdk 0.32.1
💡 Result:
For opentelemetry_sdk 0.32.1, configure batching directly on SdkTracerProvider:
use opentelemetry_sdk::trace::SdkTracerProvider;
let provider = SdkTracerProvider::builder()
.with_batch_exporter(exporter)
.build();with_batch_exporter adds a BatchSpanProcessor using the supplied SpanExporter.[1]
For custom batch settings:
use opentelemetry_sdk::trace::{
BatchSpanProcessor, SdkTracerProvider, SpanProcessor,
};
let processor = BatchSpanProcessor::builder(exporter).build();
let provider = SdkTracerProvider::builder()
.with_span_processor(processor)
.build();The processor exports when the batch is full or after the scheduled delay; call provider.shutdown() before exit to flush buffered spans.[2] The default batch processor is thread-based; async-runtime batching requires the experimental_trace_batch_span_processor_with_async_runtime feature.[3]
🌐 Web query:
site:docs.rs/opentelemetry-otlp/0.32.0 SpanExporter builder with_http reqwest-client BatchSpanProcessor
💡 Result:
In the Rust OpenTelemetry ecosystem, exporters are generally provided by specific crates rather than a generic SpanExporter builder that takes a reqwest client directly [1][2]. To export trace data, you typically use an exporter crate such as opentelemetry-otlp [1][2]. If your goal is to use a specific HTTP client like reqwest, you typically configure the exporter's client implementation via the options provided by the exporter's builder API, if supported by that specific exporter [1][2]. The BatchSpanProcessor is used to process spans in batches before sending them to an exporter [3]. You configure it by passing an exporter instance to its constructor: let exporter = opentelemetry_otlp::new_exporter...build_span_exporter?; let batch_processor = opentelemetry_sdk::trace::BatchSpanProcessor::builder(exporter, runtime).build; For outbound HTTP instrumentation, crates like telemetry-rust offer helpers to instrument reqwest clients directly, which is separate from the span export process [4]. The opentelemetry-sdk provides the core infrastructure for trace processing, while specific exporters handle the transmission of data to your telemetry backend [1][2].
Citations:
- 1: https://docs.rs/opentelemetry/latest/opentelemetry/
- 2: https://docs.rs/crate/opentelemetry/^0.25
- 3: https://docs.rs/crate/opentelemetry/0.22.0
- 4: https://docs.rs/telemetry-rust/latest/telemetry_rust/index.html
Use with_batch_exporter for request tracing.
SimpleSpanProcessor exports each finished span on the request-processing thread. A slow OTLP collector can delay request completion. Replace .with_simple_exporter(exporter) with .with_batch_exporter(exporter); the existing reqwest-client feature supports this exporter.
🤖 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 `@commons/src/tracing.rs` around lines 37 - 45, Update the SdkTracerProvider
builder to use with_batch_exporter(exporter) instead of
with_simple_exporter(exporter), keeping the existing sampler, resource, and
exporter configuration unchanged so span exports do not block request
processing.
| pub fn set_span_tags<S: Span>(req_path: &str, headers: &HttpHeaderMap, span: &mut S) { | ||
| span.set_attribute(KeyValue::new("path", req_path.to_string())); | ||
| headers.iter().for_each(|(k, v)| { | ||
| let value = v.to_str().unwrap().to_string(); | ||
| span.set_attribute(Key::new(format!("header.{}", k)).string(value)) | ||
| span.set_attribute(KeyValue::new(format!("header.{}", k), value)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not export all inbound request headers.
headers contains client-controlled values. Lines 122-124 export credentials, cookies, and user identifiers if clients send them. to_str().unwrap() also panics when a header value is not valid text. Remove raw header attributes, or use a small reviewed allow-list and skip values that to_str() rejects.
Proposed fix
-pub fn set_span_tags<S: Span>(req_path: &str, headers: &HttpHeaderMap, span: &mut S) {
+pub fn set_span_tags<S: Span>(req_path: &str, _headers: &HttpHeaderMap, span: &mut S) {
span.set_attribute(KeyValue::new("path", req_path.to_string()));
- headers.iter().for_each(|(k, v)| {
- let value = v.to_str().unwrap().to_string();
- span.set_attribute(KeyValue::new(format!("header.{}", k), value))
- });
}As per path instructions, validate at trust boundaries with allow-lists, not deny-lists.
📝 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.
| pub fn set_span_tags<S: Span>(req_path: &str, headers: &HttpHeaderMap, span: &mut S) { | |
| span.set_attribute(KeyValue::new("path", req_path.to_string())); | |
| headers.iter().for_each(|(k, v)| { | |
| let value = v.to_str().unwrap().to_string(); | |
| span.set_attribute(Key::new(format!("header.{}", k)).string(value)) | |
| span.set_attribute(KeyValue::new(format!("header.{}", k), value)) | |
| pub fn set_span_tags<S: Span>(req_path: &str, _headers: &HttpHeaderMap, span: &mut S) { | |
| span.set_attribute(KeyValue::new("path", req_path.to_string())); | |
| } |
🤖 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 `@commons/src/tracing.rs` around lines 120 - 124, Update set_span_tags to stop
exporting arbitrary inbound headers; either remove the header iteration entirely
or restrict it to a small reviewed allow-list of safe header names. For any
allowed header, replace the_str().unwrap() with fallible handling that skips
values rejected by to_str(), while preserving the path attribute.
Source: Path instructions
Addresses CVE-2026-55969 and CVE-2026-43871, both Apache Thrift vulnerabilities fixed in Thrift >= 0.24.0, tracked in: OCPBUGS-99935 (integer overflow/wraparound, CVE-2026-55969) OCPBUGS-100030 (infinite loop, CVE-2026-43871) Root cause ---------- commons/Cargo.toml had two paths pulling in old thrift versions: - direct: thrift = "0.17" (resolves to thrift 0.17.0) - indirect: opentelemetry-jaeger = "0.13.0" -> thrift 0.13.0 Every released version of opentelemetry-jaeger (0.13-0.22) pins thrift ^0.17.0 (<0.18.0), so thrift 0.24.0 is unreachable through that dependency chain. A simple version bump is not possible. Fix --- Replace opentelemetry-jaeger with opentelemetry-otlp, which has no Thrift dependency. opentelemetry-jaeger is officially deprecated upstream; OTLP is the recommended successor. Jaeger natively supports OTLP since v1.35 (2022), so no observability capability is lost. This requires upgrading opentelemetry 0.14 -> 0.30 and adopting the opentelemetry_sdk crate (split from the main crate in opentelemetry 0.20). opentelemetry-otlp 0.30 is used rather than the latest 0.32 because 0.31+ pulls in prost 0.14 (MSRV rustc 1.85) while CI runs rustc 1.84.1; 0.30 uses prost 0.13 and reqwest 0.12, both compatible. Code changes ------------ commons/Cargo.toml: - Remove opentelemetry-jaeger, thrift - Add opentelemetry_sdk 0.30, opentelemetry-otlp 0.30 - opentelemetry 0.14 -> 0.30 commons/src/tracing.rs: - Replace opentelemetry_jaeger pipeline with opentelemetry_otlp HTTP SpanExporter + SdkTracerProvider - Move TraceContextPropagator import to opentelemetry_sdk (SDK was split into its own crate in opentelemetry 0.20) - set_span_tags: dyn Span -> generic <S: Span> (Span is no longer object-safe in 0.32 due to generic methods) - Key::new(...).string/bool() -> KeyValue::new(...) (Key value builder methods removed in newer opentelemetry) cincinnati/src/plugins/internal/cincinnati_graph_fetch.rs: - Key::new("cached").bool(...) -> KeyValue::new("cached", ...) graph-builder/src/main.rs: - start_with_context(name, cx) -> start_with_context(name, &cx) (parameter now takes &Context instead of Context) {graph-builder,metadata-helper,policy-engine}/Cargo.toml: - opentelemetry 0.14.0 -> 0.30 {graph-builder,metadata-helper,policy-engine}/src/config/settings.rs: - Update tracing_endpoint doc comment: "Jaeger host and port" -> "OTLP HTTP endpoint (e.g. http://host:4318)" Deployment note --------------- The --service.tracing_endpoint flag now expects an OTLP HTTP URL (e.g. http://jaeger-collector:4318) instead of a Jaeger UDP agent address (e.g. jaeger-agent:6831). Deployments with tracing enabled will need this flag updated. Deployments without tracing (endpoint not set, which is the default) are unaffected. The cincinnati-operator does not configure this flag and requires no changes. Signed-off-by: Todd Short <tshort@redhat.com>
8e1a464 to
827f020
Compare
There was a problem hiding this comment.
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 `@commons/Cargo.toml`:
- Around line 22-24: Update the direct OpenTelemetry dependencies at
commons/Cargo.toml:22-24, graph-builder/Cargo.toml:40,
metadata-helper/Cargo.toml:22, and policy-engine/Cargo.toml:32 to exact patched
versions, then regenerate Cargo.lock and resolve all vulnerable transitive
crates, preserving checksums and avoiding invalid thrift or pre-release wasi
entries. Run complete-graph vulnerability checks and add release-process steps
for SBOM generation, provenance attestations, and Sigstore/cosign signing.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5505b26b-1177-4f9e-932b-4116f023fad4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
cincinnati/Cargo.tomlcincinnati/src/plugins/internal/cincinnati_graph_fetch.rscommons/Cargo.tomlcommons/src/tracing.rsgraph-builder/Cargo.tomlgraph-builder/src/config/settings.rsgraph-builder/src/main.rsmetadata-helper/Cargo.tomlmetadata-helper/src/config/settings.rspolicy-engine/Cargo.tomlpolicy-engine/src/config/settings.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- graph-builder/src/config/settings.rs
- cincinnati/Cargo.toml
- policy-engine/src/config/settings.rs
- cincinnati/src/plugins/internal/cincinnati_graph_fetch.rs
- metadata-helper/src/config/settings.rs
- graph-builder/src/main.rs
- commons/src/tracing.rs
| opentelemetry = "0.30" | ||
| opentelemetry_sdk = { version = "0.30", features = ["trace"] } | ||
| opentelemetry-otlp = { version = "0.30", features = ["http-proto", "reqwest-client"] } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
cargo metadata --locked --format-version 1 > /tmp/cargo-metadata.json
jq -r '
.packages[]
| select(.name | IN("opentelemetry", "opentelemetry_sdk", "opentelemetry-otlp", "thrift"))
| [.name, .version, (.license // "unknown")]
| `@tsv`
' /tmp/cargo-metadata.json
if jq -e '.packages[] | select(.name == "thrift")' /tmp/cargo-metadata.json >/dev/null; then
echo "thrift is still present in the resolved graph" >&2
exit 1
fi
jq -r '.packages[] | [.name, .version] | `@tsv`' /tmp/cargo-metadata.json |
while IFS=$'\t' read -r name version; do
curl -sSf https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "$(jq -nc --arg name "$name" --arg version "$version" \
'{package:{ecosystem:"crates.io",name:$name},version:$version}')"
done
rg -n -i 'sbom|provenance|attest|sigstore|cosign' . || trueRepository: openshift/cincinnati
Length of output: 405
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- manifests ---'
for f in commons/Cargo.toml graph-builder/Cargo.toml metadata-helper/Cargo.toml policy-engine/Cargo.toml; do
if [ -f "$f" ]; then
echo "[$f]"
cat -n "$f" | sed -n '1,90p'
fi
done
printf '%s\n' '--- lockfiles and workflows ---'
git ls-files '*Cargo.lock' '.github/workflows/*' '.gitlab-ci.yml' 'Makefile' 'Justfile' 'README*' |
sort
printf '%s\n' '--- dependency and supply-chain references ---'
rg -n -i 'opentelemetry|opentelemetry_sdk|opentelemetry-otlp|thrift|sbom|spdx|cyclonedx|provenance|attest|sigstore|cosign|cargo-audit|cargo-deny|cargo-vet|cargo-about|cargo\s+vendor|checksum' \
--glob '!target/**' --glob '!node_modules/**' . || true
printf '%s\n' '--- repository status and top-level files ---'
git status --short
git ls-files | sed -n '1,160p'Repository: openshift/cincinnati
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Cargo.lock package entries ---'
if [ -f Cargo.lock ]; then
awk '
/^\[\[package\]\]/ { block=""; next }
/^name = / { name=$0 }
/^version = / { version=$0 }
/^source = / { source=$0 }
/^checksum = / { checksum=$0 }
/^$/ {
if (name ~ /opentelemetry|thrift/) print name "\n" version "\n" source "\n" checksum "\n"
name=""; version=""; source=""; checksum=""
}
' Cargo.lock
else
echo 'Cargo.lock is absent'
fi
printf '%s\n' '--- lockfile dependency graph references ---'
if [ -f Cargo.lock ]; then
rg -n -C 3 'name = "(opentelemetry|opentelemetry_sdk|opentelemetry-otlp|thrift)"|opentelemetry|thrift' Cargo.lock || true
fi
printf '%s\n' '--- workflow files ---'
for f in $(git ls-files '.github/workflows/*' '.gitlab-ci.yml' 2>/dev/null); do
echo "[$f]"
cat -n "$f"
doneRepository: openshift/cincinnati
Length of output: 3696
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- dependency manifests and lockfile inventory ---'
git ls-files | rg '(^|/)(Cargo\.toml|Cargo\.lock|.*\.ya?ml|.*\.yaml|.*\.yml|Makefile|Justfile|README[^/]*|deny\.toml|audit\.toml|about\.toml|vet\.toml)$' | sort
printf '%s\n' '--- OpenTelemetry declarations across manifests ---'
rg -n -C 2 '^\s*(opentelemetry|opentelemetry_sdk|opentelemetry-otlp)\s*=' --glob 'Cargo.toml' .
printf '%s\n' '--- exact package metadata from Cargo.lock ---'
python3 - <<'PY'
from pathlib import Path
p = Path("Cargo.lock")
if not p.exists():
print("Cargo.lock is absent")
raise SystemExit
blocks = p.read_text().split("[[package]]")
for block in blocks:
lines = block.splitlines()
name = next((x.split("=",1)[1].strip().strip('"') for x in lines if x.startswith("name = ")), None)
if name and (name.startswith("opentelemetry") or name == "thrift"):
print("[[package]]")
print("\n".join(lines[:12]))
PY
printf '%s\n' '--- supply-chain configuration references ---'
rg -n -i 'sbom|spdx|cyclonedx|provenance|attest|sigstore|cosign|cargo-audit|cargo-deny|cargo-vet|cargo-about|checksum|license' \
--glob '!target/**' --glob '!node_modules/**' . || trueRepository: openshift/cincinnati
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import json
import subprocess
import urllib.request
import urllib.error
import tomllib
from pathlib import Path
from concurrent.futures import ThreadPoolExecutor, as_completed
lock = tomllib.loads(Path("Cargo.lock").read_text())
packages = [
(p["name"], p["version"])
for p in lock.get("package", [])
if p.get("source", "").startswith("registry+")
]
print(f"locked_registry_packages={len(packages)}")
print("target_packages:")
for p in lock.get("package", []):
if p["name"].startswith("opentelemetry") or p["name"] == "thrift":
print(json.dumps({k: p.get(k) for k in ("name", "version", "checksum", "source")}))
def post_osv(batch):
data = json.dumps({"queries": [
{"package": {"ecosystem": "crates.io", "name": n}, "version": v}
for n, v in batch
]}).encode()
req = urllib.request.Request(
"https://api.osv.dev/v1/querybatch",
data=data,
headers={"content-type": "application/json"},
)
with urllib.request.urlopen(req, timeout=30) as r:
return json.load(r)
# Query in batches so the result stays concise and includes all locked registry crates.
osv_results = []
for i in range(0, len(packages), 100):
try:
response = post_osv(packages[i:i+100])
for (name, version), result in zip(packages[i:i+100], response.get("results", [])):
for vuln in result.get("vulns", []):
osv_results.append((name, version, vuln.get("id"), vuln.get("summary", "")))
except Exception as exc:
print(f"osv_error_batch={i//100}: {exc}")
if osv_results:
print("osv_findings:")
for row in osv_results:
print("\t".join(row))
else:
print("osv_findings=none")
def crate_version_status(item):
name, version = item
url = f"https://crates.io/api/v1/crates/{name}/{version}"
try:
with urllib.request.urlopen(url, timeout=30) as r:
data = json.load(r)
v = data.get("version", {})
return name, version, v.get("yanked"), v.get("license"), None
except Exception as exc:
return name, version, None, None, str(exc)
# Check yanked status and declared license for every registry package.
status = []
with ThreadPoolExecutor(max_workers=12) as pool:
futures = [pool.submit(crate_version_status, item) for item in packages]
for future in as_completed(futures):
status.append(future.result())
errors = [x for x in status if x[4]]
yanked = [x for x in status if x[2] is True]
missing_license = [x for x in status if not x[3] and not x[4]]
print(f"crates_io_errors={len(errors)}")
if errors:
for x in sorted(errors):
print("crates_io_error", x[0], x[1], x[4])
print(f"yanked_packages={len(yanked)}")
for x in sorted(yanked):
print("yanked", x[0], x[1])
print(f"packages_without_declared_license={len(missing_license)}")
for x in sorted(missing_license):
print("missing_license", x[0], x[1])
print("target_license_status:")
for x in sorted(status):
if x[0].startswith("opentelemetry") or x[0] == "thrift":
print("\t".join("" if v is None else str(v) for v in x[:4]))
PYRepository: openshift/cincinnati
Length of output: 278
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- dependency rationale and runtime use ---'
for f in commons/src/tracing.rs graph-builder/src/main.rs graph-builder/src/graph.rs metadata-helper/src/main.rs metadata-helper/src/signatures.rs policy-engine/src/main.rs policy-engine/src/graph.rs cincinnati/Cargo.toml Cargo.toml Justfile dist/vendor.sh; do
if [ -f "$f" ]; then
echo "[$f]"
rg -n -C 3 'opentelemetry|tracer|tracing|vendor|build|release|package|image|sign' "$f" || true
fi
done
printf '%s\n' '--- all CI and release-like files ---'
git ls-files | rg -i '(^|/)(\.github|\.gitlab|ci|cd|release|build|package|image|container|docker|workflow|pipeline|attest|sbom|provenance|sigstore|cosign)(/|\.|$)' | sort || true
printf '%s\n' '--- exact dependency syntax for OpenTelemetry ---'
rg -n '^\s*opentelemetry(-sdk|-otlp|_sdk)?\s*=' --glob 'Cargo.toml' .Repository: openshift/cincinnati
Length of output: 24458
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import json
import re
import urllib.request
from concurrent.futures import ThreadPoolExecutor, as_completed
from pathlib import Path
text = Path("Cargo.lock").read_text()
packages = []
for block in text.split("[[package]]")[1:]:
name = re.search(r'^name = "([^"]+)"$', block, re.M)
version = re.search(r'^version = "([^"]+)"$', block, re.M)
source = re.search(r'^source = "([^"]+)"$', block, re.M)
checksum = re.search(r'^checksum = "([^"]+)"$', block, re.M)
if name and version and source and source.group(1).startswith("registry+"):
packages.append((name.group(1), version.group(1), checksum.group(1) if checksum else None))
print(f"locked_registry_packages={len(packages)}")
print("target_packages:")
for item in packages:
if item[0].startswith("opentelemetry") or item[0] == "thrift":
print("\t".join("" if x is None else x for x in item))
# Check the complete locked graph for OSV findings.
def query_osv(batch):
body = json.dumps({"queries": [
{"package": {"ecosystem": "crates.io", "name": n}, "version": v}
for n, v, _ in batch
]}).encode()
req = urllib.request.Request(
"https://api.osv.dev/v1/querybatch",
data=body,
headers={"content-type": "application/json"},
)
with urllib.request.urlopen(req, timeout=60) as response:
return json.load(response)
osv_findings = []
osv_errors = []
for offset in range(0, len(packages), 100):
batch = packages[offset:offset + 100]
try:
result = query_osv(batch)
for item, match in zip(batch, result.get("results", [])):
for vuln in match.get("vulns", []):
osv_findings.append((item[0], item[1], vuln.get("id", ""), vuln.get("summary", "")))
except Exception as exc:
osv_errors.append((offset, str(exc)))
print(f"osv_errors={len(osv_errors)}")
for item in osv_errors:
print("osv_error", *item)
print(f"osv_findings={len(osv_findings)}")
for item in osv_findings:
print("osv", *item, sep="\t")
# Check crates.io yanked state and declared license for every locked registry package.
def crate_status(item):
name, version, _ = item
try:
url = f"https://crates.io/api/v1/crates/{name}/{version}"
with urllib.request.urlopen(url, timeout=60) as response:
version_data = json.load(response).get("version", {})
return item, version_data.get("yanked"), version_data.get("license"), None
except Exception as exc:
return item, None, None, str(exc)
statuses = []
with ThreadPoolExecutor(max_workers=16) as executor:
futures = [executor.submit(crate_status, item) for item in packages]
for future in as_completed(futures):
statuses.append(future.result())
errors = [x for x in statuses if x[3]]
yanked = [x for x in statuses if x[1] is True]
missing_license = [x for x in statuses if not x[2] and not x[3]]
print(f"crates_io_errors={len(errors)}")
for item in sorted(errors):
print("crates_io_error", item[0][0], item[0][1], item[3])
print(f"yanked_packages={len(yanked)}")
for item in sorted(yanked):
print("yanked", item[0][0], item[0][1])
print(f"packages_without_declared_license={len(missing_license)}")
for item in sorted(missing_license):
print("missing_license", item[0][0], item[0][1])
print("target_license_status:")
for item, is_yanked, license_name, error in sorted(statuses):
if item[0].startswith("opentelemetry") or item[0] == "thrift":
print(item[0], item[1], f"yanked={is_yanked}", f"license={license_name}", f"error={error}", sep="\t")
# Detect prerelease versions in the lockfile without relying on package metadata.
prereleases = [(n, v) for n, v, _ in packages if "-" in v]
print(f"prerelease_locked_packages={len(prereleases)}")
for item in prereleases:
print("prerelease", *item, sep="\t")
PYRepository: openshift/cincinnati
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import json
import re
import subprocess
from pathlib import Path
text = Path("Cargo.lock").read_text()
packages = []
for block in text.split("[[package]]")[1:]:
n = re.search(r'^name = "([^"]+)"$', block, re.M)
v = re.search(r'^version = "([^"]+)"$', block, re.M)
s = re.search(r'^source = "([^"]+)"$', block, re.M)
c = re.search(r'^checksum = "([^"]+)"$', block, re.M)
if n and v and s and s.group(1).startswith("registry+"):
packages.append((n.group(1), v.group(1), c.group(1) if c else None))
print(f"registry_packages={len(packages)}")
print(f"registry_packages_without_checksum={sum(c is None for _, _, c in packages)}")
print(f"thrift_entries={sum(n == 'thrift' for n, _, _ in packages)}")
print(f"locked_prerelease_entries={sum('-' in v for _, v, _ in packages)}")
print("opentelemetry_entries:")
for item in packages:
if item[0].startswith("opentelemetry"):
print("\t".join(item))
Path("/tmp/locked-crates.json").write_text(json.dumps([
{"name": n, "version": v} for n, v, _ in packages
]))
PY
python3 - <<'PY'
import json
import subprocess
from pathlib import Path
packages = json.loads(Path("/tmp/locked-crates.json").read_text())
findings = []
errors = []
for offset in range(0, len(packages), 100):
batch = packages[offset:offset + 100]
payload = json.dumps({"queries": [
{"package": {"ecosystem": "crates.io", "name": p["name"]}, "version": p["version"]}
for p in batch
]})
proc = subprocess.run(
["curl", "-k", "-sS", "https://api.osv.dev/v1/querybatch",
"-H", "content-type: application/json", "--data-binary", payload],
text=True, capture_output=True,
)
if proc.returncode:
errors.append((offset, proc.stderr.strip()))
continue
try:
response = json.loads(proc.stdout)
for package, result in zip(batch, response.get("results", [])):
for vuln in result.get("vulns", []):
findings.append((package["name"], package["version"],
vuln.get("id", ""), vuln.get("summary", "")))
except Exception as exc:
errors.append((offset, str(exc)))
print(f"osv_errors={len(errors)}")
for item in errors:
print("osv_error", *item, sep="\t")
print(f"osv_findings={len(findings)}")
for item in findings:
print("osv", *item, sep="\t")
PY
for crate in opentelemetry opentelemetry-http opentelemetry-otlp opentelemetry-proto opentelemetry_sdk; do
curl -k -sSf "https://crates.io/api/v1/crates/${crate}/0.30.0" |
jq -c --arg crate "$crate" '{name:$crate, version:.version.num, yanked:.version.yanked, license:.version.license}'
doneRepository: openshift/cincinnati
Length of output: 3347
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- selected OSV advisory details ---'
for id in \
GHSA-w9wp-h8wv-79jx \
GHSA-8v2v-wjwg-vx6r \
GHSA-gcqf-3g44-vc9p \
GHSA-xhj4-vrgc-hr34 \
GHSA-434x-w66g-qw3r \
GHSA-j39j-6gw9-jw6h \
GHSA-4fcv-w3qc-ppgg \
GHSA-3pv8-6f4r-ffg2 \
GHSA-rr8g-9fpq-6wmg
do
curl -k -sSf "https://api.osv.dev/v1/vulns/${id}" |
jq -c '{id,summary,details,severity,affected:[.affected[] | {package:.package, ranges, versions, database_specific}]}'
done
printf '%s\n' '--- OpenTelemetry crates.io status ---'
for spec in \
'opentelemetry 0.30.0' \
'opentelemetry-http 0.30.0' \
'opentelemetry-otlp 0.30.0' \
'opentelemetry-proto 0.30.0' \
'opentelemetry_sdk 0.30.0'
do
set -- $spec
curl -k -sSf "https://crates.io/api/v1/crates/$1/$2" |
jq -c --arg expected_name "$1" --arg expected_version "$2" \
'{name:.crate.name,version:.version.num,yanked:.version.yanked,license:.version.license,expected_name:$expected_name,expected_version:$expected_version}'
doneRepository: openshift/cincinnati
Length of output: 21008
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import re
from collections import deque
from pathlib import Path
text = Path("Cargo.lock").read_text()
packages = {}
for block in text.split("[[package]]")[1:]:
name_m = re.search(r'^name = "([^"]+)"$', block, re.M)
if not name_m:
continue
name = name_m.group(1)
deps_m = re.search(r'^dependencies = \[\n(.*?)^\]$', block, re.M | re.S)
deps = []
if deps_m:
deps = re.findall(r'^ "([^"]+)"', deps_m.group(1), re.M)
deps = [d.split(" ")[0] for d in deps]
packages[name] = deps
roots = [
"cincinnati", "commons", "graph-builder", "metadata-helper",
"policy-engine", "prometheus-query", "quay", "e2e",
"rh-manifest-generator",
]
targets = [
"opentelemetry_sdk", "actix-files", "actix-http", "bytes", "git2",
"openssl", "tar", "tokio", "rustls-webpki", "anyhow",
]
for target in targets:
paths = []
for root in roots:
if root not in packages:
continue
q = deque([(root, [root])])
seen = {root}
while q:
node, path = q.popleft()
if node == target:
paths.append(path)
break
for dep in packages.get(node, []):
if dep not in seen and dep in packages:
seen.add(dep)
q.append((dep, path + [dep]))
print(f"{target}:")
for path in paths:
print(" " + " -> ".join(path))
if not paths:
print(" no workspace path found")
PYRepository: openshift/cincinnati
Length of output: 3012
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
text = Path("Cargo.lock").read_text()
for block in text.split("[[package]]")[1:]:
name = re.search(r'^name = "([^"]+)"$', block, re.M)
version = re.search(r'^version = "([^"]+)"$', block, re.M)
source = re.search(r'^source = "([^"]+)"$', block, re.M)
if name and version and source and source.group(1).startswith("registry+") and "-" in version.group(1):
print(name.group(1), version.group(1))
PYRepository: openshift/cincinnati
Length of output: 216
Resolve the vulnerable dependency graph before merge.
opentelemetry_sdk 0.30.0 has a network-triggerable resource-exhaustion advisory. The lockfile has 62 OSV findings, including vulnerable actix-files, actix-http, bytes, git2, openssl, tar, and tokio paths.
Upgrade affected crates to patched versions, pin direct dependencies exactly, and rerun the complete-graph checks. Cargo.lock has checksums, thrift is absent, and the wasi entries are build-metadata versions, not pre-releases.
Add SBOM generation, provenance attestations, and Sigstore or cosign signing to the release process.
📍 Affects 4 files
commons/Cargo.toml#L22-L24(this comment)graph-builder/Cargo.toml#L40-L40metadata-helper/Cargo.toml#L22-L22policy-engine/Cargo.toml#L32-L32
🤖 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 `@commons/Cargo.toml` around lines 22 - 24, Update the direct OpenTelemetry
dependencies at commons/Cargo.toml:22-24, graph-builder/Cargo.toml:40,
metadata-helper/Cargo.toml:22, and policy-engine/Cargo.toml:32 to exact patched
versions, then regenerate Cargo.lock and resolve all vulnerable transitive
crates, preserving checksums and avoiding invalid thrift or pre-release wasi
entries. Run complete-graph vulnerability checks and add release-process steps
for SBOM generation, provenance attestations, and Sigstore/cosign signing.
Source: Path instructions
|
@tmshort: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Closing as obsoleted by #1076, but re-open if I'm misunderstanding. |
|
@tmshort: This pull request references Jira Issue OCPBUGS-99935. The bug has been updated to no longer refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-100030. The bug has been updated to no longer refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Summary
Fixes OCPBUGS-99935 (CVE-2026-55969) and OCPBUGS-100030 (CVE-2026-43871) — both Apache Thrift vulnerabilities fixed in Thrift ≥ 0.24.0.
commons/Cargo.tomlhad two paths pulling in oldthriftversions:thrift = "0.17"opentelemetry-jaeger = "0.13.0"→thrift 0.13.0Every released version of
opentelemetry-jaeger(0.13–0.22) pinsthrift ^0.17.0(<0.18.0), making thrift 0.24.0 unreachable. A simple bump is not sufficient.Fix: replace
opentelemetry-jaegerwithopentelemetry-otlp, which has no Thrift dependency.opentelemetry-jaegeris officially deprecated upstream; OTLP is the recommended successor. Jaeger natively supports OTLP since v1.35 (2022), so no observability capability is lost.This requires upgrading
opentelemetry0.14 → 0.30 and adopting theopentelemetry_sdkcrate (split from the main crate in opentelemetry 0.20).opentelemetry-otlp 0.30is used rather than the latest 0.32 because 0.31+ pulls inprost 0.14(MSRV rustc 1.85) while CI runs rustc 1.84.1; 0.30 usesprost 0.13andreqwest 0.12, both compatible.Changes
commons/Cargo.toml: removeopentelemetry-jaeger,thrift; addopentelemetry_sdk 0.30,opentelemetry-otlp 0.30; bumpopentelemetryto 0.30commons/src/tracing.rs: replace Jaeger pipeline with OTLP HTTP exporter; fix API changes (dyn Span→ generic,Key::new(...).string/bool()→KeyValue::new(...),TraceContextPropagatorimport moved toopentelemetry_sdk)cincinnati/src/plugins/internal/cincinnati_graph_fetch.rs:Key::new("cached").bool(...)→KeyValue::new("cached", ...)graph-builder/src/main.rs:start_with_context(name, cx)→start_with_context(name, &cx)(&Contextin 0.30){graph-builder,metadata-helper,policy-engine}/Cargo.toml:opentelemetry0.14.0 → 0.30{graph-builder,metadata-helper,policy-engine}/src/config/settings.rs: updatetracing_endpointdoc comment to reflect OTLPDeployment note
The
--service.tracing_endpointflag now expects an OTLP HTTP URL (e.g.http://jaeger-collector:4318) instead of a Jaeger UDP agent address (e.g.jaeger-agent:6831). Deployments with tracing enabled will need this flag updated. Deployments without tracing (endpoint not set, which is the default) are unaffected. Thecincinnati-operatordoes not configure this flag and requires no changes.Test plan
cargo build— clean, no errorscargo test— all pre-existing tests pass; 5 failures incincinnati_graph_fetchare pre-existing onmaster(confirmed by running against unmodified tree)thriftabsent fromCargo.lockafter changeSummary by CodeRabbit
Enhancements
Documentation