carverauto/serviceradar

Fork 0

missing deps #2434

Merged

mfreeman451 merged 2 commits from refs/pull/2434/head into main

2025-11-20 05:23:12 +00:00

mfreeman451 commented

2025-11-20 05:13:15 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1966
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1966
Original created: 2025-11-20T05:13:15Z
Original updated: 2025-11-20T05:23:36Z
Original head: carverauto/serviceradar:chore/fix_kong_rel
Original base: main
Original merged: 2025-11-20T05:23:12Z 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, Bug fix

Description

Add C compiler detection and installation logic to ensure build dependencies
Pass PATH and CC environment variables to Bazel build configuration
Support multiple package managers (apt-get, yum, dnf, apk) for gcc installation
Gracefully handle missing compiler with informative error message

Diagram Walkthrough

flowchart LR
  A["Build Script"] --> B["ensure_cc Function"]
  B --> C["Check CC Environment"]
  C --> D{Compiler Found?}
  D -->|Yes| E["Export CC Variable"]
  D -->|No| F["Detect Package Manager"]
  F --> G["Install gcc"]
  G --> E
  E --> H["Pass to Bazel via COMMON_FLAGS"]
  H --> I["Build Kong"]

File Walkthrough

Relevant files

Enhancement, error handling, dependencies

build-kong-vendor.sh

Add C compiler detection and environment setup

scripts/build-kong-vendor.sh

Added ensure_cc() function to detect and install C compiler with
support for gcc, clang, and cc
Integrated package manager detection (apt-get, yum, dnf, apk) for
automatic gcc installation
Added --repo_env=PATH=${PATH} flag to COMMON_FLAGS for Bazel
configuration
Modified main() to call ensure_cc() and conditionally pass CC
environment variable to Bazel
Added error handling with informative message when no C compiler is
available

+87/-0

Imported from GitHub pull request. Original GitHub pull request: #1966 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1966 Original created: 2025-11-20T05:13:15Z Original updated: 2025-11-20T05:23:36Z Original head: carverauto/serviceradar:chore/fix_kong_rel Original base: main Original merged: 2025-11-20T05:23:12Z 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, Bug fix ___ ### **Description** - Add C compiler detection and installation logic to ensure build dependencies - Pass PATH and CC environment variables to Bazel build configuration - Support multiple package managers (apt-get, yum, dnf, apk) for gcc installation - Gracefully handle missing compiler with informative error message ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Build Script"] --> B["ensure_cc Function"] B --> C["Check CC Environment"] C --> D{Compiler Found?} D -->|Yes| E["Export CC Variable"] D -->|No| F["Detect Package Manager"] F --> G["Install gcc"] G --> E E --> H["Pass to Bazel via COMMON_FLAGS"] H --> I["Build Kong"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Enhancement, error handling, dependencies</td><td><table> <tr> <td> <details> <summary>build-kong-vendor.sh<dd><code>Add C compiler detection and environment setup</code>                      </dd></summary> <hr> scripts/build-kong-vendor.sh <ul><li>Added <code>ensure_cc()</code> function to detect and install C compiler with support for gcc, clang, and cc <li> Integrated package manager detection (apt-get, yum, dnf, apk) for automatic gcc installation <li> Added <code>--repo_env=PATH=${PATH}</code> flag to COMMON_FLAGS for Bazel configuration <li> Modified <code>main()</code> to call <code>ensure_cc()</code> and conditionally pass <code>CC</code> environment variable to Bazel <li> Added error handling with informative message when no C compiler is available</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24">+87/-0</a>    </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-11-20 05:13:37 +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/1966#issuecomment-3555848574
Original created: 2025-11-20T05:13:37Z

PR Compliance Guide 🔍

(Compliance updated until commit `github.com/carverauto/serviceradar@f31201a702`)

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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation 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: Newly added build actions (e.g., setting PATH/CC and installing gcc) are only echoed via info without structured audit details like actor, timestamp, or outcome, making comprehensive auditing uncertain. Referred Code `if command -v clang >/dev/null 2>&1; then CC="$(command -v clang)" export CC info "Using clang at ${CC}" >&2 return fi if command -v cc >/dev/null 2>&1; then CC="$(command -v cc)" export CC info "Using cc at ${CC}" >&2 return fi if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get (build-essential)" >&2 if command -v sudo >/dev/null 2>&1; then sudo apt-get update -y >/dev/null sudo apt-get install -y build-essential >/dev/null else ... (clipped 46 lines)` Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

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

Previous compliance checks

Compliance check up to commit d213bdc

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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Limited Auditing: The script adds informational messages for compiler detection/installation but does not produce structured or comprehensive audit logs for critical actions like package installation or build configuration changes. Referred Code `ensure_cc() { if command -v "${CC:-}" >/dev/null 2>&1; then info "Using C compiler from CC=${CC}" >&2 return fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi if command -v clang >/dev/null 2>&1; then CC="$(command -v clang)" export CC info "Using clang at ${CC}" >&2 return fi if command -v cc >/dev/null 2>&1; then ... (clipped 59 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: Generic Failure: The compiler-missing path exits with a single generic message and does not include actionable context such as attempted package managers or exit codes from install attempts. Referred Code `echo "[kong] No C compiler found (gcc/clang/cc). Install one or set CC before running this script." >&2 exit 1 }` 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: Env Trust Assumption: The script passes PATH and CC into Bazel without validation or sanitization, which may propagate untrusted environment values to the build process. Referred Code `"--repo_env=PATH=${PATH}" )` Learn more about managing compliance generic rules or creating your own custom rules

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1966#issuecomment-3555848574 Original created: 2025-11-20T05:13:37Z --- ## PR Compliance Guide 🔍  #### (Compliance updated until commit https://github.com/carverauto/serviceradar/commit/f31201a7026fdfa794a4c076d060b53da2c28679) 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=5>🟢</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: Robust Error Handling and Edge Case Management</summary> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful degradation **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=1>⚪</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/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R72-R138'>Action logging</a>: Newly added build actions (e.g., setting PATH/CC and installing gcc) are only echoed via info without structured audit details like actor, timestamp, or outcome, making comprehensive auditing uncertain. <details open><summary>Referred Code</summary> ```shell if command -v clang >/dev/null 2>&1; then CC="$(command -v clang)" export CC info "Using clang at ${CC}" >&2 return fi if command -v cc >/dev/null 2>&1; then CC="$(command -v cc)" export CC info "Using cc at ${CC}" >&2 return fi if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get (build-essential)" >&2 if command -v sudo >/dev/null 2>&1; then sudo apt-get update -y >/dev/null sudo apt-get install -y build-essential >/dev/null else ... (clipped 46 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 align="center" colspan="2">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant 🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label </details> ___ #### Previous compliance checks <details> <summary>Compliance check up to commit <a href='https://github.com/carverauto/serviceradar/commit/d213bdc8c113fb260deca62e0e5648c28cff77c5'>d213bdc</a></summary> <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=3>🟢</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 rowspan=3>⚪</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/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R60-R139'>Limited Auditing</a>: The script adds informational messages for compiler detection/installation but does not produce structured or comprehensive audit logs for critical actions like package installation or build configuration changes. <details open><summary>Referred Code</summary> ```shell ensure_cc() { if command -v "${CC:-}" >/dev/null 2>&1; then info "Using C compiler from CC=${CC}" >&2 return fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi if command -v clang >/dev/null 2>&1; then CC="$(command -v clang)" export CC info "Using clang at ${CC}" >&2 return fi if command -v cc >/dev/null 2>&1; then ... (clipped 59 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/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R137-R139'>Generic Failure</a>: The compiler-missing path exits with a single generic message and does not include actionable context such as attempted package managers or exit codes from install attempts. <details open><summary>Referred Code</summary> ```shell echo "[kong] No C compiler found (gcc/clang/cc). Install one or set CC before running this script." >&2 exit 1 } ``` </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: Security-First Input Validation and Data Handling</summary> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities **Status:** <a href='https://github.com/carverauto/serviceradar/pull/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R22-R23'>Env Trust Assumption</a>: The script passes PATH and CC into Bazel without validation or sanitization, which may propagate untrusted environment values to the build process. <details open><summary>Referred Code</summary> ```shell "--repo_env=PATH=${PATH}" ) ``` </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">  </td></tr></tbody></table> </details>

qodo-code-review[bot] commented

2025-11-20 05:14:34 +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/1966#issuecomment-3555851622
Original created: 2025-11-20T05:14:34Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Avoid automatic system package installation Remove the logic that automatically detects the package manager and installs `gcc`. Instead, the script should only check for a C compiler and exit with an informative error message if one is not found. Examples: scripts/build-kong-vendor.sh [87-135] `if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get (build-essential)" >&2 if command -v sudo >/dev/null 2>&1; then sudo apt-get update -y >/dev/null sudo apt-get install -y build-essential >/dev/null else apt-get update -y >/dev/null apt-get install -y build-essential >/dev/null fi if command -v gcc >/dev/null 2>&1; then ... (clipped 39 lines)` Solution Walkthrough: Before: `ensure_cc() { # Check for existing compilers (CC, gcc, clang, cc) if command -v gcc >/dev/null 2>&1; then ... return fi ... # Try to install gcc using various package managers if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get..." # (sudo) apt-get install ... return fi if command -v yum >/dev/null 2>&1; then # yum install ... return fi # ... similar for dnf, apk echo "[kong] No C compiler found..." >&2 exit 1 }` After: `ensure_cc() { # Check for existing compilers (CC, gcc, clang, cc) if command -v "${CC:-}" >/dev/null 2>&1; then ... return fi if command -v gcc >/dev/null 2>&1; then ... return fi # ... similar for clang, cc # If no compiler is found, exit with a helpful error message echo "[kong] No C compiler found (gcc/clang/cc)." >&2 echo "Please install a C compiler (e.g., 'sudo apt-get install build-essential') or set the CC environment variable." >&2 exit 1 }` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that automatically installing system packages within a build script is an anti-pattern that can cause permission issues and have unintended side effects on the host system.	High
Possible issue	Use sudo for package installation Add `sudo` checks for `yum` and `dnf` commands to allow package installation by non-root users, mirroring the existing logic for `apt-get`. scripts/build-kong-vendor.sh [104-124] if command -v yum >/dev/null 2>&1; then info "Installing gcc via yum" >&2 - yum install -y gcc >/dev/null + if command -v sudo >/dev/null 2>&1; then + sudo yum install -y gcc >/dev/null + else + yum install -y gcc >/dev/null + fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi fi if command -v dnf >/dev/null 2>&1; then info "Installing gcc via dnf" >&2 - dnf install -y gcc >/dev/null + if command -v sudo >/dev/null 2>&1; then + sudo dnf install -y gcc >/dev/null + else + dnf install -y gcc >/dev/null + fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi fi Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a functional bug where `yum` and `dnf` commands would fail for non-root users, and proposes a fix that makes them consistent with the `apt-get` implementation.	Medium
General	Avoid suppressing package installation errors Remove the redirection to `/dev/null` from package manager `install` commands to ensure installation errors are visible to the user for easier debugging. scripts/build-kong-vendor.sh [87-102] if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get (build-essential)" >&2 if command -v sudo >/dev/null 2>&1; then sudo apt-get update -y >/dev/null - sudo apt-get install -y build-essential >/dev/null + sudo apt-get install -y build-essential else apt-get update -y >/dev/null - apt-get install -y build-essential >/dev/null + apt-get install -y build-essential fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi fi Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that suppressing error output from package managers hinders debugging. Showing these errors would significantly improve the script's usability when installations fail.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1966#issuecomment-3555851622 Original created: 2025-11-20T05:14:34Z --- ## 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>Avoid automatic system package installation</summary> ___ **Remove the logic that automatically detects the package manager and installs <code>gcc</code>. Instead, the script should only check for a C compiler and exit with an informative error message if one is not found.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R87-R135">scripts/build-kong-vendor.sh [87-135]</a> </summary> ```shell if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get (build-essential)" >&2 if command -v sudo >/dev/null 2>&1; then sudo apt-get update -y >/dev/null sudo apt-get install -y build-essential >/dev/null else apt-get update -y >/dev/null apt-get install -y build-essential >/dev/null fi if command -v gcc >/dev/null 2>&1; then ... (clipped 39 lines) ``` </details> ### Solution Walkthrough: #### Before: ```shell ensure_cc() { # Check for existing compilers (CC, gcc, clang, cc) if command -v gcc >/dev/null 2>&1; then ... return fi ... # Try to install gcc using various package managers if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get..." # (sudo) apt-get install ... return fi if command -v yum >/dev/null 2>&1; then # yum install ... return fi # ... similar for dnf, apk echo "[kong] No C compiler found..." >&2 exit 1 } ``` #### After: ```shell ensure_cc() { # Check for existing compilers (CC, gcc, clang, cc) if command -v "${CC:-}" >/dev/null 2>&1; then ... return fi if command -v gcc >/dev/null 2>&1; then ... return fi # ... similar for clang, cc # If no compiler is found, exit with a helpful error message echo "[kong] No C compiler found (gcc/clang/cc)." >&2 echo "Please install a C compiler (e.g., 'sudo apt-get install build-essential') or set the CC environment variable." >&2 exit 1 } ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies that automatically installing system packages within a build script is an anti-pattern that can cause permission issues and have unintended side effects on the host system. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Use sudo for package installation</summary> ___ **Add <code>sudo</code> checks for <code>yum</code> and <code>dnf</code> commands to allow package installation by non-root users, mirroring the existing logic for <code>apt-get</code>.** [scripts/build-kong-vendor.sh [104-124]](https://github.com/carverauto/serviceradar/pull/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R104-R124) ```diff if command -v yum >/dev/null 2>&1; then info "Installing gcc via yum" >&2 - yum install -y gcc >/dev/null + if command -v sudo >/dev/null 2>&1; then + sudo yum install -y gcc >/dev/null + else + yum install -y gcc >/dev/null + fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi fi if command -v dnf >/dev/null 2>&1; then info "Installing gcc via dnf" >&2 - dnf install -y gcc >/dev/null + if command -v sudo >/dev/null 2>&1; then + sudo dnf install -y gcc >/dev/null + else + dnf install -y gcc >/dev/null + fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi fi ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a functional bug where `yum` and `dnf` commands would fail for non-root users, and proposes a fix that makes them consistent with the `apt-get` implementation. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Avoid suppressing package installation errors</summary> ___ **Remove the redirection to <code>/dev/null</code> from package manager <code>install</code> commands to ensure installation errors are visible to the user for easier debugging.** [scripts/build-kong-vendor.sh [87-102]](https://github.com/carverauto/serviceradar/pull/1966/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R87-R102) ```diff if command -v apt-get >/dev/null 2>&1; then info "Installing gcc via apt-get (build-essential)" >&2 if command -v sudo >/dev/null 2>&1; then sudo apt-get update -y >/dev/null - sudo apt-get install -y build-essential >/dev/null + sudo apt-get install -y build-essential else apt-get update -y >/dev/null - apt-get install -y build-essential >/dev/null + apt-get install -y build-essential fi if command -v gcc >/dev/null 2>&1; then CC="$(command -v gcc)" export CC info "Using gcc at ${CC}" >&2 return fi fi ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly points out that suppressing error output from package managers hinders debugging. Showing these errors would significantly improve the script's usability when installations fail. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>