fixed proton packaging for rpm #2250
No reviewers
Labels
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
carverauto/serviceradar!2250
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refs/pull/2250/head"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Imported from GitHub pull request.
Original GitHub pull request: #1670
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1670
Original created: 2025-09-29T15:20:48Z
Original updated: 2025-09-29T15:24:04Z
Original head: carverauto/serviceradar:packaging/proton_rpm_broken
Original base: main
Original merged: 2025-09-29T15:24:01Z by @mfreeman451
PR Type
Bug fix
Description
Fix runtime directory paths from
/var/runto/runAdd compatibility symlinks for proton service scripts
Correct binary ownership and permissions setup
Add proper systemd runtime directory management
Diagram Walkthrough
File Walkthrough
postinstall.sh
Update runtime paths and binary ownershippackaging/proton/scripts/postinstall.sh
/var/run/proton-serverto/run/proton-serverin directorycreation
root:rootfor/usr/bin/proton/runinstead of/var/runserviceradar-proton.service
Fix systemd service runtime directory configurationpackaging/proton/systemd/serviceradar-proton.service
/var/runto/runRuntimeDirectoryandRuntimeDirectoryModedirectives/runpathserviceradar-proton.spec
Add symlinks and fix RPM spec configurationpackaging/specs/serviceradar-proton.spec
proton-server,proton-client,proton-local%filessection/run/proton-server%postsectionImported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1670#issuecomment-3347621691
Original created: 2025-09-29T15:21:29Z
PR Reviewer Guide 🔍
Here are some key observations to aid the review process:
Ownership Scope
Changing ownership of /usr/bin/proton from proton:proton to root:root is likely correct, but verify that no parts of the system or scripts expect the binary to be owned by the proton user (e.g., self-upgrade or write attempts). Also confirm that capabilities remain effective with root ownership.
ExecStop Robustness
The ExecStop command depends on a PID file existing at shutdown. Validate behavior if the PID file is missing or stale. Consider using systemd PIDFile tracking with KillMode= or a more resilient ExecStop to avoid stop failures.
Symlink Packaging
Symlinks for proton-server/client/local are added as regular files in %files without %attr or %ghost. Ensure they resolve correctly (relative vs absolute) and verify alternatives/conflicts with any existing packages providing the same names.
Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1670#issuecomment-3347626016
Original created: 2025-09-29T15:22:36Z
PR Code Suggestions ✨
Explore these optional code suggestions:
Simplify packaging by removing redundant scripts
Remove the redundant manual creation of the
/run/proton-serverdirectory fromthe
postinstall.shand.specfiles, as this is now handled by theRuntimeDirectorysystemd directive. Also, remove the unnecessarychowncommandfor
/usr/bin/protonin the.specfile's%postsection, since ownership isalready set in the
%filessection.Examples:
packaging/specs/serviceradar-proton.spec [76-82]
packaging/proton/scripts/postinstall.sh [114]
Solution Walkthrough:
Before:
After:
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies redundant logic in packaging scripts (
postinstall.sh,.spec) that is now handled bysystemdandrpm, leading to simpler and more robust code.Remove redundant runtime directory creation
Remove the redundant creation of the
/run/proton-serverdirectory from the%postscript, as it is already handled by the
RuntimeDirectorydirective in thesystemd service file.
packaging/specs/serviceradar-proton.spec [76]
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies that creating the
/run/proton-serverdirectory is redundant, assystemd'sRuntimeDirectorydirective already handles this task.Add explicit symlink attributes
Add explicit
%attrattributes for the new symlinks in the%filessection of theRPM spec to ensure correct ownership and permissions.
packaging/specs/serviceradar-proton.spec [43-45]
Suggestion importance[1-10]: 6
__
Why: This is a good packaging practice that makes the RPM spec file more robust and explicit about file attributes, preventing potential permission issues.
Imported GitHub PR comment.
Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1670#issuecomment-3347626593
Original created: 2025-09-29T15:22:44Z
CI Feedback 🧐
A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
Action: build
Failed stage: Test [❌]
Failed test name: ""
Failure summary:
The action failed because Bazel could not analyze targets due to an opam toolchain setup failure.
Specifically:
- While evaluating module extension
@@tools_opam+//extensions:opam.bzl%opam, creatingthe opam switch
5.2.0failed with exit code 31.- Error details (external file
/home/runner/.bazel/external/tools_opam+/extensions/opam/opam_toolchain_xdg.bzl, line 242, column17):
-
fail("opam switch create failed; cmd=[... "opam", "switch", "create", "5.2.0", ...] rc=31...)- opam error report shows
ERROR while compiling ocaml-base-compiler.5.2.0during./configure...- Output includes
bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted, indicatingbubblewrap sandbox/network namespace operation not permitted in the CI environment.
- As a result,
many targets showed
WARNING: errors encountered while analyzing target ... error evaluating moduleextension @@tools_opam+//extensions:opam.bzl%opam.- Bazel concluded:
ERROR: command succeeded, butnot all targets were analyzedand laterERROR: No test targets were found, yet testing wasrequested.- The build and tests did not complete successfully, causing the job to exit with code 1.
Relevant error logs: