feat(just): ship ujust report via common with Renovate tracking#382
feat(just): ship ujust report via common with Renovate tracking#382hanthor wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds the ChangesBonedigger System Report Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
system_files/shared/usr/share/ublue-os/just/60-bonedigger.just (1)
170-170: 💤 Low valueConsider tracking the OTEL collector image digest with Renovate.
The container image
docker.io/otel/opentelemetry-collector-contrib@sha256:a2a52e43c1a80aa94120ad78c2db68780eb90e6d11c8db5b3ce2f6a0cc6b5029is pinned by digest, which is excellent for reproducibility. However, this digest won't be automatically updated when new versions are released.Consider adding a Renovate custom manager rule to track this digest, similar to the pattern used for
BONEDIGGER_VERSION.🤖 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 `@system_files/shared/usr/share/ublue-os/just/60-bonedigger.just` at line 170, The OTEL collector image is pinned by digest (docker.io/otel/opentelemetry-collector-contrib@sha256:a2a52e43c1a80aa94120ad78c2db68780eb90e6d11c8db5b3ce2f6a0cc6b5029) but not tracked by Renovate; add a Renovate custom rule that matches this image (or a regex for otel/opentelemetry-collector-contrib digests) similar to the existing BONEDIGGER_VERSION rule so Renovate will open updates for new digests, specifying the package pattern/regex and update type (digest) and any grouping or schedule you use for BONEDIGGER_VERSION.system_files/shared/usr/share/ublue-os/otel/ujust-report-config.yaml (1)
156-158: 💤 Low valueConsider expanding username and email redaction patterns.
The current patterns handle common cases but may miss some contexts:
USER/LOGNAME (lines 156-157): Only match environment variable format (
USER=alice). Usernames in other contexts (e.g.,"user alice logged in","Owner: alice") won't be redacted.Email regex (line 158): The pattern
[a-zA-Z0-9._%+\\-]+@[a-zA-Z0-9.\\-]+\\.[a-zA-Z]{2,}catches most emails but might miss:
- Emails in quotes:
"user@example.com"(matches the email but not the quotes)- Emails in angle brackets:
<user@example.com>(matches the email but not the brackets)- Emails with uncommon TLDs less than 2 characters (rare)
These edge cases may or may not be relevant depending on what appears in journald logs. Consider whether additional patterns are needed based on actual log content.
🤖 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 `@system_files/shared/usr/share/ublue-os/otel/ujust-report-config.yaml` around lines 156 - 158, The redaction patterns using replace_pattern for "USER=[^\\s\"']+", "LOGNAME=[^\\s\"']+" and the email regex should be broadened: add additional replace_pattern rules that match common surrounding contexts (e.g., key/value and free-text forms such as "user[:=]\\s*[^\\s\"'<>:,]+" and "owner[:=]\\s*[^\\s\"'<>:,]+") in addition to the existing ENV-style patterns (refer to the existing replace_pattern calls for USER and LOGNAME), and add an email replace_pattern that captures surrounding delimiters (e.g., optional leading/trailing quotes or angle brackets) by making the delimiter part of the match so they are redacted together (update the existing email replace_pattern line to include optional surrounding ["'<>()\\s]* or add a separate pattern to cover those cases); ensure new patterns are conservative (use word boundaries or explicit separators) to avoid over-redacting.
🤖 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 `@system_files/shared/usr/share/ublue-os/otel/ujust-report-config.yaml`:
- Around line 154-155: The current replace_pattern calls (the two lines calling
replace_pattern with "/home/[^/\\s\"']+/" and "/var/home/[^/\\s\"']+/") only
match when a trailing slash follows the username; update those regexes so they
also match usernames at end-of-path or followed by whitespace (i.e., allow
either a trailing slash or a boundary/whitespace/end). Concretely, adjust the
two patterns used in replace_pattern (for "/home/..." and "/var/home/...") to
accept an optional trailing slash or use a lookahead for (/|\\b|\\s|$) so paths
like "/home/alice" and "/var/home/bob" are redacted as well.
---
Nitpick comments:
In `@system_files/shared/usr/share/ublue-os/just/60-bonedigger.just`:
- Line 170: The OTEL collector image is pinned by digest
(docker.io/otel/opentelemetry-collector-contrib@sha256:a2a52e43c1a80aa94120ad78c2db68780eb90e6d11c8db5b3ce2f6a0cc6b5029)
but not tracked by Renovate; add a Renovate custom rule that matches this image
(or a regex for otel/opentelemetry-collector-contrib digests) similar to the
existing BONEDIGGER_VERSION rule so Renovate will open updates for new digests,
specifying the package pattern/regex and update type (digest) and any grouping
or schedule you use for BONEDIGGER_VERSION.
In `@system_files/shared/usr/share/ublue-os/otel/ujust-report-config.yaml`:
- Around line 156-158: The redaction patterns using replace_pattern for
"USER=[^\\s\"']+", "LOGNAME=[^\\s\"']+" and the email regex should be broadened:
add additional replace_pattern rules that match common surrounding contexts
(e.g., key/value and free-text forms such as "user[:=]\\s*[^\\s\"'<>:,]+" and
"owner[:=]\\s*[^\\s\"'<>:,]+") in addition to the existing ENV-style patterns
(refer to the existing replace_pattern calls for USER and LOGNAME), and add an
email replace_pattern that captures surrounding delimiters (e.g., optional
leading/trailing quotes or angle brackets) by making the delimiter part of the
match so they are redacted together (update the existing email replace_pattern
line to include optional surrounding ["'<>()\\s]* or add a separate pattern to
cover those cases); ensure new patterns are conservative (use word boundaries or
explicit separators) to avoid over-redacting.
🪄 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: CHILL
Plan: Pro Plus
Run ID: c5794acd-e9bd-427f-9404-f4e25422e74b
📒 Files selected for processing (3)
.github/renovate.json5system_files/shared/usr/share/ublue-os/just/60-bonedigger.justsystem_files/shared/usr/share/ublue-os/otel/ujust-report-config.yaml
| - replace_pattern(body, "/home/[^/\\s\"']+/", "/home/[REDACTED]/") | ||
| - replace_pattern(body, "/var/home/[^/\\s\"']+/", "/var/home/[REDACTED]/") |
There was a problem hiding this comment.
Path redaction regex misses paths without trailing slashes.
The regex patterns require a trailing slash to match:
- Pattern:
/home/[^/\\s\"']+/matches/home/alice/Documents/but not/home/alice - Pattern:
/var/home/[^/\\s\"']+/matches/var/home/alice/file.txtbut not/var/home/alice
Log messages containing paths without trailing slashes will leak usernames. Examples:
"Permission denied: /home/alice"→ username not redacted"cd /home/alice"→ username not redacted"Failed to access /var/home/bob"→ username not redacted
🔒 Proposed fix to match paths with or without trailing slashes
- - replace_pattern(body, "/home/[^/\\s\"']+/", "/home/[REDACTED]/")
- - replace_pattern(body, "/var/home/[^/\\s\"']+/", "/var/home/[REDACTED]/")
+ - replace_pattern(body, "/home/([^/\\s\"']+)(/|\\s|\\b)", "/home/[REDACTED]$2")
+ - replace_pattern(body, "/var/home/([^/\\s\"']+)(/|\\s|\\b)", "/var/home/[REDACTED]$2")This matches usernames followed by either a slash, whitespace, or word boundary, preserving the delimiter while redacting the username.
📝 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.
| - replace_pattern(body, "/home/[^/\\s\"']+/", "/home/[REDACTED]/") | |
| - replace_pattern(body, "/var/home/[^/\\s\"']+/", "/var/home/[REDACTED]/") | |
| - replace_pattern(body, "/home/([^/\\s\"']+)(/|\\s|\\b)", "/home/[REDACTED]$2") | |
| - replace_pattern(body, "/var/home/([^/\\s\"']+)(/|\\s|\\b)", "/var/home/[REDACTED]$2") |
🤖 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 `@system_files/shared/usr/share/ublue-os/otel/ujust-report-config.yaml` around
lines 154 - 155, The current replace_pattern calls (the two lines calling
replace_pattern with "/home/[^/\\s\"']+/" and "/var/home/[^/\\s\"']+/") only
match when a trailing slash follows the username; update those regexes so they
also match usernames at end-of-path or followed by whitespace (i.e., allow
either a trailing slash or a boundary/whitespace/end). Concretely, adjust the
two patterns used in replace_pattern (for "/home/..." and "/var/home/...") to
accept an optional trailing slash or use a lookahead for (/|\\b|\\s|$) so paths
like "/home/alice" and "/var/home/bob" are redacted as well.
inffy
left a comment
There was a problem hiding this comment.
This seems to be very Bluefin specific. We can't use this on Aurora so this shouldn't be on the shared folder
|
Fixed — moved both |
castrojo
left a comment
There was a problem hiding this comment.
Reviewed. LGTM — clean implementation of ujust report via common.
60-bonedigger.just: well-structured; BONEDIGGER_VERSION sentinel is the right pattern for Renovate tracking; privacy redaction (home paths, GPU UUID/serial, hostname) is thoroughujust-report-config.yaml: OTel pipeline looks sound — memory_limiter first, resource/privacy + attributes/privacy_logs + transform/scrub_logs strip PII before exporting; two separate file exporters for metrics vs logs is correct per OTel spec- Renovate regex pattern correctly targets the sentinel in 60-bonedigger.just; github-releases datasource + projectbluefin/bonedigger depName is right
CI green. ✅ Approved.
ujust report and its OTEL config are Bluefin-specific and cannot be used on Aurora. Move both files from system_files/shared/ to system_files/bluefin/ so they only land on Bluefin images. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
e1a420c to
b5ce3a6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@system_files/bluefin/usr/share/ublue-os/otel/ujust-report-config.yaml`:
- Line 1: The header comment in ujust-report-config.yaml leaks a personal
username path; update the comment line that currently reads "# Canonical source:
/var/home/jorge/src/dakota/files/otel/ujust-report-config.yaml" to a
non-personal, generic reference (for example "# Canonical source:
/path/to/dakota/files/otel/ujust-report-config.yaml" or "# Canonical source:
repository/files/otel/ujust-report-config.yaml") so the maintainer username is
removed while preserving a usable canonical-source hint.
- Around line 64-70: Replace the hard-coded docker_stats.endpoint path so it
resolves to the runtime user's socket instead of /run/user/1000; update the OTEL
config template to use an environment-driven value (e.g.
${XDG_RUNTIME_DIR}/podman/podman.sock or a placeholder that expands to
/run/user/$(id -u)/podman/podman.sock) and ensure the recipe/just script
(60-bonedigger.just) injects or preserves that variable rather than only
replacing /output/; specifically change the docker_stats.endpoint entry and
adapt the templating/just patching logic so docker_stats.endpoint is rewritten
at build/runtime to the dynamic path.
🪄 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: CHILL
Plan: Pro Plus
Run ID: a960b7bb-dd9b-48d0-9eeb-9a08a8510b36
📒 Files selected for processing (3)
.github/renovate.json5system_files/bluefin/usr/share/ublue-os/just/60-bonedigger.justsystem_files/bluefin/usr/share/ublue-os/otel/ujust-report-config.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/renovate.json5
| @@ -0,0 +1,185 @@ | |||
| # Canonical source: /var/home/jorge/src/dakota/files/otel/ujust-report-config.yaml | |||
There was a problem hiding this comment.
Header comment leaks a maintainer username.
/var/home/jorge/src/dakota/... embeds a personal username/path in a file that ships to every Bluefin user and whose stated goal is privacy scrubbing. Generalize the canonical-source reference.
✂️ Suggested edit
-# Canonical source: /var/home/jorge/src/dakota/files/otel/ujust-report-config.yaml
+# Canonical source: projectbluefin/dakota — files/otel/ujust-report-config.yaml🤖 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 `@system_files/bluefin/usr/share/ublue-os/otel/ujust-report-config.yaml` at
line 1, The header comment in ujust-report-config.yaml leaks a personal username
path; update the comment line that currently reads "# Canonical source:
/var/home/jorge/src/dakota/files/otel/ujust-report-config.yaml" to a
non-personal, generic reference (for example "# Canonical source:
/path/to/dakota/files/otel/ujust-report-config.yaml" or "# Canonical source:
repository/files/otel/ujust-report-config.yaml") so the maintainer username is
removed while preserving a usable canonical-source hint.
| # docker_stats works against the Podman socket via Docker-compatible API. | ||
| # Socket path /run/user/1000/ assumes UID 1000; the recipe patches this at runtime. | ||
| # If the socket is absent the receiver fails silently (optional). | ||
| docker_stats: | ||
| endpoint: unix:///run/user/1000/podman/podman.sock | ||
| collection_interval: 10s | ||
| timeout: 5s |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the recipe only rewrites /output/ and never patches the podman socket UID.
fd -t f '60-bonedigger.just' --exec rg -nC3 -e 'podman\.sock' -e "content\.replace" -e '/run/user' {}Repository: projectbluefin/common
Length of output: 1202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant yaml snippet
echo "---- YAML: ujust-report-config.yaml (docker_stats) ----"
FILE="system_files/bluefin/usr/share/ublue-os/otel/ujust-report-config.yaml"
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '50,90p'
else
echo "Missing $FILE"
fi
echo "---- Recipe: 60-bonedigger.just occurrences ----"
# Search within recipe for run/user/1000 and podman sock usage + rewrite logic
fd -t f '60-bonedigger.just' | head -n 5 | while read -r R; do
echo "== $R =="
rg -n --hidden -S '/run/user/1000|podman\.sock|docker_stats|ujust-report-config\.yaml|replace\(' "$R" || true
echo "---- Context around OTEL_CONFIG_SOURCE and OTEL_CONFIG generation ----"
rg -n -S 'OTEL_CONFIG_SOURCE|mktemp.*otel-config|python3 -c "import sys; content' "$R" -n || true
rg -n -S 'content\.replace|PODMAN_SOCK|SOCKET_MOUNT|podman run --rm' "$R" -n || true
doneRepository: projectbluefin/common
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="system_files/bluefin/usr/share/ublue-os/otel/ujust-report-config.yaml"
echo "---- YAML file exists? ----"
ls -la "$FILE" || true
echo "---- YAML docker_stats section (lines 55-85) ----"
# Use cat -n (available) to show line numbers; limit output
cat -n "$FILE" | sed -n '55,85p' || true
echo "---- Find and inspect 60-bonedigger.just ----"
RFILE="$(fd -t f '60-bonedigger.just' . | head -n 1 || true)"
echo "Recipe: $RFILE"
if [ -z "$RFILE" ]; then
exit 0
fi
echo "---- Occurrences: /run/user/1000 ----"
rg -n --hidden -S '/run/user/1000' "$RFILE" || true
echo "---- Occurrences: docker_stats / podman socket ----"
rg -n --hidden -S 'docker_stats|podman\.sock|podman/podman\.sock|PODMAN_SOCK|SOCKET_MOUNT|unix:///run/user|/run/user' "$RFILE" || true
echo "---- Occurrences: content.replace (all calls) ----"
rg -n --hidden -S 'content\.replace\(' "$RFILE" || trueRepository: projectbluefin/common
Length of output: 2121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
RFILE="./system_files/bluefin/usr/share/ublue-os/just/60-bonedigger.just"
echo "---- Occurrences: 1000 ----"
rg -n --hidden -S '1000' "$RFILE" || true
echo "---- Occurrences: /run/user ----"
rg -n --hidden -S '/run/user' "$RFILE" || true
echo "---- Occurrences: sed/awk/envsubst/dedent/replace patterns ----"
rg -n --hidden -S 'sed |awk |envsubst|replace\(|/output/' "$RFILE" || trueRepository: projectbluefin/common
Length of output: 2463
Fix runtime Podman socket UID mismatch for docker_stats
system_files/bluefin/usr/share/ublue-os/otel/ujust-report-config.yaml hard-codes docker_stats.endpoint to unix:///run/user/1000/podman/podman.sock, but system_files/bluefin/usr/share/ublue-os/just/60-bonedigger.just only patches OTEL config by replacing /output/—it never rewrites /run/user/1000. The recipe mounts the real socket at /run/user/$(id -u)/..., so non-1000 users will point the receiver at a socket path that isn’t present.
Update the recipe/config templating so the docker_stats.endpoint matches the dynamically mounted /run/user/$(id -u)/podman/podman.sock (or use ${XDG_RUNTIME_DIR} / an env substitution passed into the collector).
🤖 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 `@system_files/bluefin/usr/share/ublue-os/otel/ujust-report-config.yaml` around
lines 64 - 70, Replace the hard-coded docker_stats.endpoint path so it resolves
to the runtime user's socket instead of /run/user/1000; update the OTEL config
template to use an environment-driven value (e.g.
${XDG_RUNTIME_DIR}/podman/podman.sock or a placeholder that expands to
/run/user/$(id -u)/podman/podman.sock) and ensure the recipe/just script
(60-bonedigger.just) injects or preserves that variable rather than only
replacing /output/; specifically change the docker_stats.endpoint entry and
adapt the templating/just patching logic so docker_stats.endpoint is rewritten
at build/runtime to the dynamic path.
Closes #373.
system_files/shared/usr/share/ublue-os/otel/ujust-report-config.yaml.reportrecipe tosystem_files/shared/usr/share/ublue-os/just/60-bonedigger.justwith custom overrides for Bluefin.projectbluefin/bonediggerand automatically update the version pin.Summary by CodeRabbit