carverauto/serviceradar

Fork 0

Update/rust edge onboarding #2537

Merged

mfreeman451 merged 5 commits from refs/pull/2537/head into main

2025-12-10 14:59:57 +00:00

mfreeman451 commented

2025-12-10 05:44:29 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2096
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2096
Original created: 2025-12-10T05:44:29Z
Original updated: 2025-12-10T15:00:01Z
Original head: carverauto/serviceradar:update/rust_edge_onboarding
Original base: main
Original merged: 2025-12-10T14:59:57Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Enhancement, Tests

Description

Add EL9 (RHEL 9) build support with ZFS-enabled sysmon binary
- New --config=el9 Bazel configuration targeting Oracle Linux 9
- Separate ZFS and non-ZFS sysmon binaries for compatibility
- Custom RBE executor image with ZFS libraries installed
Implement version management system via Bazel module extension
- New build/version.bzl reads VERSION file at load time
- Exposes version/release as build variables for RPM packaging
Enhance RPM packaging with dynamic version/release substitution
- Add package_file_name parameter to pkg_rpm rules
- Support custom spec templates to disable AutoReq for optional dependencies
Migrate GitHub Actions to self-hosted runners (arc-runner-set)
- Update multiple workflows for improved build performance
Add manifest flag to release publisher for flexible artifact selection

Diagram Walkthrough

flowchart LR
  A["VERSION file"] -->|"version_ext module"| B["serviceradar_version repo"]
  B -->|"VERSION/RELEASE vars"| C["pkg_rpm rules"]
  C -->|"dynamic naming"| D["RPM packages"]
  E["--config=el9"] -->|"rbe_linux_el9 platform"| F["ZFS builder"]
  F -->|"libzfs support"| G["sysmon_zfs binary"]
  H["musl builder"] -->|"static linking"| I["sysmon non-ZFS binary"]
  G -->|"EL9 packages"| J["Release artifacts"]
  I -->|"Standard packages"| J

File Walkthrough

Relevant files

Enhancement

15 files

version.bzl `New Bazel module extension for version management`	+83/-0
BUILD.bazel `Add ZFS constraint and EL9 RBE platform definition`	+40/-0
package_rules.bzl `Add dynamic RPM naming and spec template support`	+11/-2
packages.bzl `Split sysmon into ZFS and non-ZFS variants with custom spec`	+8/-1
release_targets.bzl `Separate EL9 packages from standard release artifacts`	+54/-2
BUILD.bazel `Export custom RPM spec template file`	+3/-0
template.spec.tpl `New custom RPM spec template disabling AutoReq`	+32/-0
BUILD.bazel `Add ZFS-enabled library and binary targets with constraints`	+43/-0
publish_packages.go `Add manifest flag for flexible artifact selection`	+3/-2
BUILD.bazel `Add separate publish_packages_el9 binary target`	+15/-0
build-rbe-image-el9.yml `New workflow to build EL9 RBE executor Docker image`	+67/-0
release.yml `Add EL9 package publishing step with ZFS support`	+58/-0
MODULE.bazel `Add version_ext module extension and libzetta crate dependency`	+10/-0
Dockerfile.rbe-ora9 `Add ZFS development libraries and update library paths`	+8/-3
Dockerfile.rpm.sysmon `Refactor to separate musl and ZFS builder stages`

Configuration changes

7 files

.bazelrc `Add el9 build configuration with EL9 platform settings`	+36/-0
deploy-pages.yml `Migrate to self-hosted arc-runner-set runners`	+2/-2
golangci-lint.yml `Migrate to self-hosted arc-runner-set runners`	+1/-1
sbom-images.yml `Migrate to self-hosted arc-runner-set runners`	+2/-2
sbom-syft.yml `Migrate to self-hosted arc-runner-set runners`	+1/-1
tests-golang.yml `Migrate to self-hosted arc-runner-set runners`	+1/-1
web-lint.yml `Migrate to self-hosted arc-runner-set runners`	+1/-1

Dependencies

1 files

build-rbe-image.yml
Update checkout action version from v6 to v4 +1/-1

Additional files

1 files

Dockerfile.rpm.sysmon

+65/-32

Imported from GitHub pull request. Original GitHub pull request: #2096 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2096 Original created: 2025-12-10T05:44:29Z Original updated: 2025-12-10T15:00:01Z Original head: carverauto/serviceradar:update/rust_edge_onboarding Original base: main Original merged: 2025-12-10T14:59:57Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Enhancement, Tests ___ ### **Description** - Add EL9 (RHEL 9) build support with ZFS-enabled sysmon binary - New `--config=el9` Bazel configuration targeting Oracle Linux 9 - Separate ZFS and non-ZFS sysmon binaries for compatibility - Custom RBE executor image with ZFS libraries installed - Implement version management system via Bazel module extension - New `build/version.bzl` reads VERSION file at load time - Exposes version/release as build variables for RPM packaging - Enhance RPM packaging with dynamic version/release substitution - Add `package_file_name` parameter to `pkg_rpm` rules - Support custom spec templates to disable AutoReq for optional dependencies - Migrate GitHub Actions to self-hosted runners (arc-runner-set) - Update multiple workflows for improved build performance - Add manifest flag to release publisher for flexible artifact selection ___ ### Diagram Walkthrough ```mermaid flowchart LR A["VERSION file"] -->|"version_ext module"| B["serviceradar_version repo"] B -->|"VERSION/RELEASE vars"| C["pkg_rpm rules"] C -->|"dynamic naming"| D["RPM packages"] E["--config=el9"] -->|"rbe_linux_el9 platform"| F["ZFS builder"] F -->|"libzfs support"| G["sysmon_zfs binary"] H["musl builder"] -->|"static linking"| I["sysmon non-ZFS binary"] G -->|"EL9 packages"| J["Release artifacts"] I -->|"Standard packages"| J ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Enhancement</td><td><details><summary>15 files</summary><table> <tr> <td>version.bzl<dd><code>New Bazel module extension for version management</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-cd08dc840f591559a86fe7cd77002afcbd521f4c9e5b328eeabee2da3a6c618b">+83/-0</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Add ZFS constraint and EL9 RBE platform definition</code>              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-d7da264d8f13c39aafc9e2343c3f9649ee1b143f653edda46521f21378a8467e">+40/-0</a>    </td> </tr> <tr> <td>package_rules.bzl<dd><code>Add dynamic RPM naming and spec template support</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-3876e101ca949a98325d3dea0371c361af54a507bc700eca353418d22f2d029d">+11/-2</a>    </td> </tr> <tr> <td>packages.bzl<dd><code>Split sysmon into ZFS and non-ZFS variants with custom spec</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-9bfe2a5141a9e402bb5a5a8fca53b9eea64396ec18108c535392e1054c90b913">+8/-1</a>      </td> </tr> <tr> <td>release_targets.bzl<dd><code>Separate EL9 packages from standard release artifacts</code>        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-4b654ab00b665505b39b6622594edce8a637859c5d1c075803ef714b2c082d6e">+54/-2</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Export custom RPM spec template file</code>                                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-841e48997b26f0364ca621dd13d18f7f6b9ac0611c7cf73be0660a3dfe430ff4">+3/-0</a>      </td> </tr> <tr> <td>template.spec.tpl<dd><code>New custom RPM spec template disabling AutoReq</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-8b4c436f776366797a5b7ac06a048ff72b2a5b8c2354ea66066bf4804abb7880">+32/-0</a>    </td> </tr> <tr> <td>BUILD.bazel<dd><code>Add ZFS-enabled library and binary targets with constraints</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-6ab6e69dfb3cd621d100077fa496690634adb5fcd88806f891575024f1835480">+43/-0</a>    </td> </tr> <tr> <td>publish_packages.go<dd><code>Add manifest flag for flexible artifact selection</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-638cfa7da020be995c5e05ba794edd1080d194845870a0061688e0b9afbcadc7">+3/-2</a>      </td> </tr> <tr> <td>BUILD.bazel<dd><code>Add separate publish_packages_el9 binary target</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-fb0e62a1ff706ec8f72e2a1bc655729d1d4fea31e015702ac01e032f67aaf708">+15/-0</a>    </td> </tr> <tr> <td>build-rbe-image-el9.yml<dd><code>New workflow to build EL9 RBE executor Docker image</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-5fbf1cb04c63cec170ffaea9b822106c30b08d20d3426bb39635e923345afdb5">+67/-0</a>    </td> </tr> <tr> <td>release.yml<dd><code>Add EL9 package publishing step with ZFS support</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34">+58/-0</a>    </td> </tr> <tr> <td>MODULE.bazel<dd><code>Add version_ext module extension and libzetta crate dependency</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-6136fc12446089c3db7360e923203dd114b6a1466252e71667c6791c20fe6bdc">+10/-0</a>    </td> </tr> <tr> <td>Dockerfile.rbe-ora9<dd><code>Add ZFS development libraries and update library paths</code>      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-c4a73a282f345855c6c173679a1f3dcf26b8cc70e3a0f026bdb0ab0a40b6834d">+8/-3</a>      </td> </tr> <tr> <td>Dockerfile.rpm.sysmon<dd><code>Refactor to separate musl and ZFS builder stages</code>                  </dd></td> <td><a href=""></a></td> </tr> </table></details></td></tr><tr><td>Configuration changes</td><td><details><summary>7 files</summary><table> <tr> <td>.bazelrc<dd><code>Add el9 build configuration with EL9 platform settings</code>      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-544556920c45b42cbfe40159b082ce8af6bd929e492d076769226265f215832f">+36/-0</a>    </td> </tr> <tr> <td>deploy-pages.yml<dd><code>Migrate to self-hosted arc-runner-set runners</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-0f132b0962009071404990a6789b3466dbd8723d1f7b744f6aca0312396a7900">+2/-2</a>      </td> </tr> <tr> <td>golangci-lint.yml<dd><code>Migrate to self-hosted arc-runner-set runners</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-d9a956120ac84289d2650137f64bd8f0893331de05e44cc41899dd984c9e0d48">+1/-1</a>      </td> </tr> <tr> <td>sbom-images.yml<dd><code>Migrate to self-hosted arc-runner-set runners</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-306f4aa8e8e286f727246a7517eecd45f3535fd99a644f60d635b9fa39875f54">+2/-2</a>      </td> </tr> <tr> <td>sbom-syft.yml<dd><code>Migrate to self-hosted arc-runner-set runners</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-f79636c7ac6e657fd98f9ead92fd4ffe68b8306a9a329bb602e3d1520b8a8017">+1/-1</a>      </td> </tr> <tr> <td>tests-golang.yml<dd><code>Migrate to self-hosted arc-runner-set runners</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-07d64b93f6e82cf63364ba9ed8e6b02148c6b817eca20ac588da5516a3f38523">+1/-1</a>      </td> </tr> <tr> <td>web-lint.yml<dd><code>Migrate to self-hosted arc-runner-set runners</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-9d090859e31fc574efb47cacf534d619a3e83d55e59da3be2484999b9055b1b2">+1/-1</a>      </td> </tr> </table></details></td></tr><tr><td>Dependencies</td><td><details><summary>1 files</summary><table> <tr> <td>build-rbe-image.yml<dd><code>Update checkout action version from v6 to v4</code>                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-267c8680ad3f655b43d9bc8d7c55af4cef338efacdc9c90b3264ef9856a22493">+1/-1</a>      </td> </tr> </table></details></td></tr><tr><td>Additional files</td><td><details><summary>1 files</summary><table> <tr> <td>Dockerfile.rpm.sysmon</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-7227b4c80aa4ab7d44c3ce924facce7189dc752319462f3829b11070d3c4ac61">+65/-32</a>  </td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-12-10 05:45:09 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2096#issuecomment-3635492179
Original created: 2025-12-10T05:45:09Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Action Logging: The new publishing step executes critical release actions without adding explicit audit logs that include actor identity and outcomes, making it unclear if all critical actions are recorded. Referred Code - name: Publish EL9 packages (sysmon with ZFS) env: RELEASE_TAG: ${{ steps.release.outputs.tag }} NOTES_FILE: ${{ steps.notes.outputs.path }} DRY_RUN: ${{ steps.release.outputs.dry_run }} APPEND_NOTES: ${{ steps.release.outputs.append_notes }} PRERELEASE: ${{ steps.release.outputs.prerelease }} DRAFT: ${{ steps.release.outputs.draft }} OVERWRITE_ASSETS: ${{ steps.release.outputs.overwrite_assets }} run: \| set -euo pipefail if [[ -z "$RELEASE_TAG" ]]; then echo "Release tag is empty" >&2 exit 1 fi declare -a args args+=("--tag" "$RELEASE_TAG") args+=("--manifest" "release/package_manifest_el9.txt") if [[ -n "$NOTES_FILE" ]]; then ... (clipped 36 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Error Handling: The EL9 publish step exits on failure without retry or detailed diagnostic logging beyond a generic message, which may not provide actionable context for failures. Referred Code `run_el9_local() { echo "EL9 packages require remote execution with --config=el9" >&2 echo "Cannot build EL9 packages locally (requires ZFS libraries)" >&2 exit 1 } if ! run_el9_remote; then echo "EL9 packaging failed" >&2 run_el9_local` Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2096#issuecomment-3635492179 Original created: 2025-12-10T05:45:09Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR: <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td>🟢</td><td><details><summary>No security concerns identified</summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'>Ticket Compliance</td></tr> <tr><td>⚪</td><td><details><summary>🎫 No ticket provided </summary> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'>Codebase Duplication Compliance</td></tr> <tr><td>⚪</td><td><details><summary>Codebase context is not defined </summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'>Custom Compliance</td></tr> <tr><td rowspan=4>🟢</td><td> <details><summary>Generic: Meaningful Naming and Self-Documenting Code</summary> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code self-documenting **Status:** Passed > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary>Generic: Secure Error Handling</summary> **Objective:** To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. **Status:** Passed > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary>Generic: Secure Logging Practices</summary> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. **Status:** Passed > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary>Generic: Security-First Input Validation and Data Handling</summary> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities **Status:** Passed > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=2>⚪</td> <td><details> <summary>Generic: Comprehensive Audit Trails</summary> **Objective:** To create a detailed and reliable record of critical system actions for security analysis and compliance. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2096/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R281-R337'>Action Logging</a>: The new publishing step executes critical release actions without adding explicit audit logs that include actor identity and outcomes, making it unclear if all critical actions are recorded. <details open><summary>Referred Code</summary> ```yaml - name: Publish EL9 packages (sysmon with ZFS) env: RELEASE_TAG: ${{ steps.release.outputs.tag }} NOTES_FILE: ${{ steps.notes.outputs.path }} DRY_RUN: ${{ steps.release.outputs.dry_run }} APPEND_NOTES: ${{ steps.release.outputs.append_notes }} PRERELEASE: ${{ steps.release.outputs.prerelease }} DRAFT: ${{ steps.release.outputs.draft }} OVERWRITE_ASSETS: ${{ steps.release.outputs.overwrite_assets }} run: | set -euo pipefail if [[ -z "$RELEASE_TAG" ]]; then echo "Release tag is empty" >&2 exit 1 fi declare -a args args+=("--tag" "$RELEASE_TAG") args+=("--manifest" "release/package_manifest_el9.txt") if [[ -n "$NOTES_FILE" ]]; then ... (clipped 36 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary>Generic: Robust Error Handling and Edge Case Management</summary> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful degradation **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2096/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R328-R336'>Error Handling</a>: The EL9 publish step exits on failure without retry or detailed diagnostic logging beyond a generic message, which may not provide actionable context for failures. <details open><summary>Referred Code</summary> ```yaml run_el9_local() { echo "EL9 packages require remote execution with --config=el9" >&2 echo "Cannot build EL9 packages locally (requires ZFS libraries)" >&2 exit 1 } if ! run_el9_remote; then echo "EL9 packaging failed" >&2 run_el9_local ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant 🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label </details>

qodo-code-review[bot] commented

2025-12-10 05:46:21 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2096#issuecomment-3635494895
Original created: 2025-12-10T05:46:21Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate build logic into Bazel Eliminate the redundant `cargo` build process within the `Dockerfile.rpm.sysmon` file. Instead, leverage the existing Bazel build system to produce the binaries and use those artifacts directly for RPM packaging. Examples: docker/rpm/Dockerfile.rpm.sysmon [1-76] `# Build stage for non-ZFS binary using musl for universal compatibility # This produces a fully static binary that works on any Linux distro FROM rust:1.81-alpine AS musl-builder RUN apk add --no-cache musl-dev protobuf-dev protoc WORKDIR /src # Copy workspace dependencies COPY rust/config-bootstrap ../rust/config-bootstrap/ ... (clipped 66 lines)` cmd/checkers/sysmon/BUILD.bazel [38-89] `rust_binary( name = "sysmon", srcs = ["src/main.rs"], edition = "2021", deps = [ ":sysmon_lib", "//rust/config-bootstrap:config_bootstrap", "//rust/edge-onboarding:edge_onboarding", ] + all_crate_deps(normal = True), ) ... (clipped 42 lines)` Solution Walkthrough: Before: # docker/rpm/Dockerfile.rpm.sysmon # Stage 1: Build non-ZFS binary from source FROM rust:1.81-alpine AS musl-builder WORKDIR /src COPY source_code . RUN cargo build --release # Stage 2: Build ZFS binary from source FROM rockylinux:9 AS zfs-builder RUN dnf install -y libzfs-devel WORKDIR /src COPY source_code . RUN cargo build --release --features zfs # Stage 3: Package binaries into RPM FROM rockylinux:9 AS rpm-builder COPY --from=musl-builder ... /binary-non-zfs COPY --from=zfs-builder ... /binary-zfs RUN rpmbuild ... After: `# packaging/packages.bzl # Define package using Bazel-built artifacts serviceradar_package( name = "sysmon", ... binary = { "target": "//cmd/checkers/sysmon:sysmon", # Bazel target for non-ZFS "dest": "/path/in/rpm/binary-non-zfs", }, files = [ { "target": "//cmd/checkers/sysmon:sysmon_zfs", # Bazel target for ZFS "dest": "/path/in/rpm/binary-zfs", }, ... ], ... ) # docker/rpm/Dockerfile.rpm.sysmon is removed or simplified, # as pkg_rpm now uses the pre-built binaries from Bazel.` Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a major architectural flaw where a parallel build system in Docker undermines the purpose of using Bazel, leading to maintenance overhead and potential inconsistencies.	High
Possible issue	Avoid hardcoding the Docker image tag Avoid hardcoding the Docker image tag `v1.0.23` in the `build-rbe-image-el9.yml` workflow. Instead, adopt a versioning strategy based on git tags to ensure image versions are explicit and immutable. .github/workflows/build-rbe-image-el9.yml [41-51] `- name: Extract metadata id: meta uses: docker/metadata-action@v5 with: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} tags: \| type=ref,event=branch type=ref,event=pr type=sha,format=short,prefix=sha- - type=raw,value=v1.0.23 + type=semver,pattern={{version}} type=raw,value=latest,enable={{is_default_branch}}` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that hardcoding the Docker image tag `v1.0.23` is risky, as it can lead to overwriting a stable image and breaking remote builds. Proposing a git-tag-based versioning strategy is a significant improvement for maintainability and build stability.	Medium
General	Make manifest flag a required argument Make the `-manifest` flag a required argument in `release/publish_packages.go`. Removing the default value forces the caller to explicitly provide the manifest path, which improves robustness and clarity since the tool is used with different manifests. release/publish_packages.go [91] `-manifestFlag := flag.String("manifest", defaultManifestRunfile, "Path to the package manifest runfile") +manifestFlag := flag.String("manifest", "", "Path to the package manifest runfile (required)")` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that having a default manifest path is brittle, given that the tool is now used in two different contexts with different manifests. Making the `-manifest` flag required forces explicit configuration, making the build definitions more robust and less ambiguous.	Medium
	Optimize Rust installation in Dockerfile Optimize the Rust installation in `docker/rpm/Dockerfile.rpm.sysmon` by using the `minimal` profile with `rustup`. This will create a smaller, more efficient build stage by installing only the necessary compiler and Cargo components. docker/rpm/Dockerfile.rpm.sysmon [43-45] `# Install Rust -RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs \| sh -s -- -y --default-toolchain 1.81.0 +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs \| sh -s -- -y --profile minimal --default-toolchain 1.81.0 ENV PATH="/root/.cargo/bin:${PATH}"` Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion provides a valid optimization by using the `minimal` profile for the `rustup` installation, which reduces Docker image size and build time by avoiding unnecessary components like documentation.	Low
	Improve handling of boolean script argument Refactor the handling of the `OVERWRITE_ASSETS` variable in the release workflow script. Explicitly pass `--overwrite_assets=true` or `--overwrite_assets=false` to make the script's behavior clearer and more robust. .github/workflows/release.yml [315-317] `-if [[ "$OVERWRITE_ASSETS" != "true" ]]; then - args+=("--overwrite_assets=${OVERWRITE_ASSETS}") +if [[ "$OVERWRITE_ASSETS" == "true" ]]; then + args+=("--overwrite_assets=true") +else + args+=("--overwrite_assets=false") fi` Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that the shell logic for the `OVERWRITE_ASSETS` flag is brittle. The proposed change makes the argument passing more explicit and robust by always providing a clear `true` or `false` value, improving the script's reliability.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2096#issuecomment-3635494895 Original created: 2025-12-10T05:46:21Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Consolidate build logic into Bazel</summary> ___ **Eliminate the redundant <code>cargo</code> build process within the <code>Dockerfile.rpm.sysmon</code> file. Instead, leverage the existing Bazel build system to produce the binaries and use those artifacts directly for RPM packaging.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-7227b4c80aa4ab7d44c3ce924facce7189dc752319462f3829b11070d3c4ac61R1-R76">docker/rpm/Dockerfile.rpm.sysmon [1-76]</a> </summary> ```dockerfile # Build stage for non-ZFS binary using musl for universal compatibility # This produces a fully static binary that works on any Linux distro FROM rust:1.81-alpine AS musl-builder RUN apk add --no-cache musl-dev protobuf-dev protoc WORKDIR /src # Copy workspace dependencies COPY rust/config-bootstrap ../rust/config-bootstrap/ ... (clipped 66 lines) ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2096/files#diff-6ab6e69dfb3cd621d100077fa496690634adb5fcd88806f891575024f1835480R38-R89">cmd/checkers/sysmon/BUILD.bazel [38-89]</a> </summary> ```starlark rust_binary( name = "sysmon", srcs = ["src/main.rs"], edition = "2021", deps = [ ":sysmon_lib", "//rust/config-bootstrap:config_bootstrap", "//rust/edge-onboarding:edge_onboarding", ] + all_crate_deps(normal = True), ) ... (clipped 42 lines) ``` </details> ### Solution Walkthrough: #### Before: ```starlark # docker/rpm/Dockerfile.rpm.sysmon # Stage 1: Build non-ZFS binary from source FROM rust:1.81-alpine AS musl-builder WORKDIR /src COPY source_code . RUN cargo build --release # Stage 2: Build ZFS binary from source FROM rockylinux:9 AS zfs-builder RUN dnf install -y libzfs-devel WORKDIR /src COPY source_code . RUN cargo build --release --features zfs # Stage 3: Package binaries into RPM FROM rockylinux:9 AS rpm-builder COPY --from=musl-builder ... /binary-non-zfs COPY --from=zfs-builder ... /binary-zfs RUN rpmbuild ... ``` #### After: ```starlark # packaging/packages.bzl # Define package using Bazel-built artifacts serviceradar_package( name = "sysmon", ... binary = { "target": "//cmd/checkers/sysmon:sysmon", # Bazel target for non-ZFS "dest": "/path/in/rpm/binary-non-zfs", }, files = [ { "target": "//cmd/checkers/sysmon:sysmon_zfs", # Bazel target for ZFS "dest": "/path/in/rpm/binary-zfs", }, ... ], ... ) # docker/rpm/Dockerfile.rpm.sysmon is removed or simplified, # as pkg_rpm now uses the pre-built binaries from Bazel. ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion correctly identifies a major architectural flaw where a parallel build system in Docker undermines the purpose of using Bazel, leading to maintenance overhead and potential inconsistencies. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Avoid hardcoding the Docker image tag</summary> ___ **Avoid hardcoding the Docker image tag <code>v1.0.23</code> in the <code>build-rbe-image-el9.yml</code> workflow. Instead, adopt a versioning strategy based on git tags to ensure image versions are explicit and immutable.** [.github/workflows/build-rbe-image-el9.yml [41-51]](https://github.com/carverauto/serviceradar/pull/2096/files#diff-5fbf1cb04c63cec170ffaea9b822106c30b08d20d3426bb39635e923345afdb5R41-R51) ```diff - name: Extract metadata id: meta uses: docker/metadata-action@v5 with: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} tags: | type=ref,event=branch type=ref,event=pr type=sha,format=short,prefix=sha- - type=raw,value=v1.0.23 + type=semver,pattern={{version}} type=raw,value=latest,enable={{is_default_branch}} ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that hardcoding the Docker image tag `v1.0.23` is risky, as it can lead to overwriting a stable image and breaking remote builds. Proposing a git-tag-based versioning strategy is a significant improvement for maintainability and build stability. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=3>General</td> <td> <details><summary>Make manifest flag a required argument</summary> ___ **Make the <code>-manifest</code> flag a required argument in <code>release/publish_packages.go</code>. Removing the default value forces the caller to explicitly provide the manifest path, which improves robustness and clarity since the tool is used with different manifests.** [release/publish_packages.go [91]](https://github.com/carverauto/serviceradar/pull/2096/files#diff-638cfa7da020be995c5e05ba794edd1080d194845870a0061688e0b9afbcadc7R91-R91) ```diff -manifestFlag := flag.String("manifest", defaultManifestRunfile, "Path to the package manifest runfile") +manifestFlag := flag.String("manifest", "", "Path to the package manifest runfile (required)") ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that having a default manifest path is brittle, given that the tool is now used in two different contexts with different manifests. Making the `-manifest` flag required forces explicit configuration, making the build definitions more robust and less ambiguous. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Optimize Rust installation in Dockerfile</summary> ___ **Optimize the Rust installation in <code>docker/rpm/Dockerfile.rpm.sysmon</code> by using the <code>minimal</code> profile with <code>rustup</code>. This will create a smaller, more efficient build stage by installing only the necessary compiler and Cargo components.** [docker/rpm/Dockerfile.rpm.sysmon [43-45]](https://github.com/carverauto/serviceradar/pull/2096/files#diff-7227b4c80aa4ab7d44c3ce924facce7189dc752319462f3829b11070d3c4ac61R43-R45) ```diff # Install Rust -RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain 1.81.0 +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal --default-toolchain 1.81.0 ENV PATH="/root/.cargo/bin:${PATH}" ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion provides a valid optimization by using the `minimal` profile for the `rustup` installation, which reduces Docker image size and build time by avoiding unnecessary components like documentation. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Improve handling of boolean script argument</summary> ___ **Refactor the handling of the <code>OVERWRITE_ASSETS</code> variable in the release workflow script. Explicitly pass <code>--overwrite_assets=true</code> or <code>--overwrite_assets=false</code> to make the script's behavior clearer and more robust.** [.github/workflows/release.yml [315-317]](https://github.com/carverauto/serviceradar/pull/2096/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R315-R317) ```diff -if [[ "$OVERWRITE_ASSETS" != "true" ]]; then - args+=("--overwrite_assets=${OVERWRITE_ASSETS}") +if [[ "$OVERWRITE_ASSETS" == "true" ]]; then + args+=("--overwrite_assets=true") +else + args+=("--overwrite_assets=false") fi ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly points out that the shell logic for the `OVERWRITE_ASSETS` flag is brittle. The proposed change makes the argument passing more explicit and robust by always providing a clear `true` or `false` value, improving the script's reliability. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>