carverauto/serviceradar

Fork 0

re-use bazel #2437

Merged

mfreeman451 merged 1 commit from refs/pull/2437/head into main

2025-11-20 06:14:16 +00:00

mfreeman451 commented

2025-11-20 06:13:49 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1969
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1969
Original created: 2025-11-20T06:13:49Z
Original updated: 2025-11-20T06:14:58Z
Original head: carverauto/serviceradar:update/kong_missing_bzel
Original base: main
Original merged: 2025-11-20T06:14:16Z 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

Description

Improve bazel provisioning with multiple fallback strategies
Check for existing bazelisk/bazel in PATH before downloading
Add retry logic with exponential backoff for downloads
Refactor download logic into reusable function with error handling

Diagram Walkthrough

flowchart LR
  A["Check existing bazel wrapper"] --> B{Found?}
  B -->|Yes| C["Use existing wrapper"]
  B -->|No| D{bazelisk in PATH?}
  D -->|Yes| E["Symlink bazelisk"]
  D -->|No| F{bazel in PATH?}
  F -->|Yes| G["Symlink bazel"]
  F -->|No| H{make available?}
  H -->|Yes| I["Try make check-bazel"]
  I --> J{Success?}
  J -->|Yes| K["Done"]
  J -->|No| L["Download bazelisk with retry"]
  H -->|No| L
  L --> M["Return bazel path"]

File Walkthrough

Relevant files

Enhancement

build-kong-vendor.sh

Refactor bazel provisioning with PATH lookup and retries

scripts/build-kong-vendor.sh

Reorder bazel provisioning logic to check PATH for existing
bazelisk/bazel before downloading
Extract download logic into separate download_bazelisk() function with
retry mechanism
Add exponential backoff retry logic (up to 4 attempts with 2s delays)
for download failures
Make make check-bazel a fallback option with error handling instead of
primary method

+42/-9

Imported from GitHub pull request. Original GitHub pull request: #1969 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1969 Original created: 2025-11-20T06:13:49Z Original updated: 2025-11-20T06:14:58Z Original head: carverauto/serviceradar:update/kong_missing_bzel Original base: main Original merged: 2025-11-20T06:14:16Z 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 ___ ### **Description** - Improve bazel provisioning with multiple fallback strategies - Check for existing bazelisk/bazel in PATH before downloading - Add retry logic with exponential backoff for downloads - Refactor download logic into reusable function with error handling ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Check existing bazel wrapper"] --> B{Found?} B -->|Yes| C["Use existing wrapper"] B -->|No| D{bazelisk in PATH?} D -->|Yes| E["Symlink bazelisk"] D -->|No| F{bazel in PATH?} F -->|Yes| G["Symlink bazel"] F -->|No| H{make available?} H -->|Yes| I["Try make check-bazel"] I --> J{Success?} J -->|Yes| K["Done"] J -->|No| L["Download bazelisk with retry"] H -->|No| L L --> M["Return bazel path"] ``` <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><table> <tr> <td> <details> <summary>build-kong-vendor.sh<dd><code>Refactor bazel provisioning with PATH lookup and retries</code>  </dd></summary> <hr> scripts/build-kong-vendor.sh <ul><li>Reorder bazel provisioning logic to check PATH for existing bazelisk/bazel before downloading <li> Extract download logic into separate <code>download_bazelisk()</code> function with retry mechanism <li> Add exponential backoff retry logic (up to 4 attempts with 2s delays) for download failures <li> Make <code>make check-bazel</code> a fallback option with error handling instead of primary method</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24">+42/-9</a>    </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-11-20 06:14:08 +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/1969#issuecomment-3556079174
Original created: 2025-11-20T06:14:08Z

PR Compliance Guide 🔍

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

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Untrusted PATH executable Description: Symlinking `bazelisk` or `bazel` from PATH into `${KONG_CLONE_DIR}/bin/bazel` without validating the target path or ownership can be abused if PATH is user-controlled or contains a malicious executable, leading to execution of an attacker-supplied binary. build-kong-vendor.sh [218-228] Referred Code `ln -sf "$(command -v bazelisk)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return`
	Unsigned binary download Description: Downloading `bazelisk` over HTTPS without verifying the checksum or signature (only relying on curl success) risks supply-chain attacks or tampering if the release asset is compromised or MITM occurs. build-kong-vendor.sh [243-251] Referred Code `for attempt in {1..4}; do info "Downloading bazelisk v${version} for ${os}/${machine} (attempt ${attempt})" >&2 if curl -sSfL "${url}" -o "${dest}"; then chmod +x "${dest}" return 0 fi sleep $((attempt * 2)) done return 1`
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: Missing auditing: The new provisioning logic (checking PATH, symlinking, downloading with retries) performs critical environment changes without emitting structured audit logs linking actions to a user or outcome details. Referred Code if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazelisk)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi download_bazelisk() { local dest="$1" local os machine version url attempt os=$(uname \| tr '[:upper:]' '[:lower:]') machine=$(uname -m) ... (clipped 33 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: Retry fallback gaps: Download failures are retried but final failure after all attempts lacks contextual error details (e.g., HTTP status) and make fallback errors are not surfaced, which may hinder diagnostics. Referred Code for attempt in {1..4}; do info "Downloading bazelisk v${version} for ${os}/${machine} (attempt ${attempt})" >&2 if curl -sSfL "${url}" -o "${dest}"; then chmod +x "${dest}" return 0 fi sleep $((attempt * 2)) done return 1 } if command -v make >/dev/null 2>&1; then info "Installing bazelisk wrapper via make check-bazel" >&2 if (cd "${KONG_CLONE_DIR}" && make check-bazel >/dev/null); then : else info "make check-bazel failed, falling back to direct bazelisk download" >&2 download_bazelisk "${bazel_bin}" fi else info "Installing bazelisk (make not available)" >&2 ... (clipped 5 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 b4482b9

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: No audit logs: The added provisioning logic performs system actions (downloads, symlinks) without emitting structured audit entries identifying actor, timestamp, action, and outcome, which may be required for critical operations in some environments. Referred Code if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazelisk)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi download_bazelisk() { local dest="$1" local os machine version url attempt os=$(uname \| tr '[:upper:]' '[:lower:]') machine=$(uname -m) ... (clipped 33 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: Retry limits result: Download retries with exponential backoff are added but failures from curl are not logged with error detail and the function’s non-zero return is not explicitly checked before proceeding to final executable check, making diagnosis of failure reasons harder. Referred Code for attempt in {1..4}; do info "Downloading bazelisk v${version} for ${os}/${machine} (attempt ${attempt})" >&2 if curl -sSfL "${url}" -o "${dest}"; then chmod +x "${dest}" return 0 fi sleep $((attempt * 2)) done return 1 } if command -v make >/dev/null 2>&1; then info "Installing bazelisk wrapper via make check-bazel" >&2 if (cd "${KONG_CLONE_DIR}" && make check-bazel >/dev/null); then : else info "make check-bazel failed, falling back to direct bazelisk download" >&2 download_bazelisk "${bazel_bin}" fi else info "Installing bazelisk (make not available)" >&2 ... (clipped 3 lines) 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/1969#issuecomment-3556079174 Original created: 2025-11-20T06:14:08Z --- ## PR Compliance Guide 🔍  #### (Compliance updated until commit https://github.com/carverauto/serviceradar/commit/b4482b9a37da785b3620667a0b3326b4fb5d14d0) Below is a summary of compliance checks for this PR: <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td rowspan=2>⚪</td> <td><details><summary>Untrusted PATH executable </summary> Description: Symlinking <code>bazelisk</code> or <code>bazel</code> from PATH into <code>${KONG_CLONE_DIR}/bin/bazel</code> without validating the target path or ownership can be abused if PATH is user-controlled or contains a malicious executable, leading to execution of an attacker-supplied binary. <a href='https://github.com/carverauto/serviceradar/pull/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R218-R228'>build-kong-vendor.sh [218-228]</a> <details open><summary>Referred Code</summary> ```shell ln -sf "$(command -v bazelisk)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return ``` </details></details></td></tr> <tr><td><details><summary>Unsigned binary download </summary> Description: Downloading <code>bazelisk</code> over HTTPS without verifying the checksum or signature (only relying on curl success) risks supply-chain attacks or tampering if the release asset is compromised or MITM occurs. <a href='https://github.com/carverauto/serviceradar/pull/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R243-R251'>build-kong-vendor.sh [243-251]</a> <details open><summary>Referred Code</summary> ```shell for attempt in {1..4}; do info "Downloading bazelisk v${version} for ${os}/${machine} (attempt ${attempt})" >&2 if curl -sSfL "${url}" -o "${dest}"; then chmod +x "${dest}" return 0 fi sleep $((attempt * 2)) done return 1 ``` </details></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/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R215-R268'>Missing auditing</a>: The new provisioning logic (checking PATH, symlinking, downloading with retries) performs critical environment changes without emitting structured audit logs linking actions to a user or outcome details. <details open><summary>Referred Code</summary> ```shell if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazelisk)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi download_bazelisk() { local dest="$1" local os machine version url attempt os=$(uname | tr '[:upper:]' '[:lower:]') machine=$(uname -m) ... (clipped 33 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/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R243-R268'>Retry fallback gaps</a>: Download failures are retried but final failure after all attempts lacks contextual error details (e.g., HTTP status) and make fallback errors are not surfaced, which may hinder diagnostics. <details open><summary>Referred Code</summary> ```shell for attempt in {1..4}; do info "Downloading bazelisk v${version} for ${os}/${machine} (attempt ${attempt})" >&2 if curl -sSfL "${url}" -o "${dest}"; then chmod +x "${dest}" return 0 fi sleep $((attempt * 2)) done return 1 } if command -v make >/dev/null 2>&1; then info "Installing bazelisk wrapper via make check-bazel" >&2 if (cd "${KONG_CLONE_DIR}" && make check-bazel >/dev/null); then : else info "make check-bazel failed, falling back to direct bazelisk download" >&2 download_bazelisk "${bazel_bin}" fi else info "Installing bazelisk (make not available)" >&2 ... (clipped 5 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/b4482b9a37da785b3620667a0b3326b4fb5d14d0'>b4482b9</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=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/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R215-R268'>No audit logs</a>: The added provisioning logic performs system actions (downloads, symlinks) without emitting structured audit entries identifying actor, timestamp, action, and outcome, which may be required for critical operations in some environments. <details open><summary>Referred Code</summary> ```shell if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazelisk)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi download_bazelisk() { local dest="$1" local os machine version url attempt os=$(uname | tr '[:upper:]' '[:lower:]') machine=$(uname -m) ... (clipped 33 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/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R243-R266'>Retry limits result</a>: Download retries with exponential backoff are added but failures from curl are not logged with error detail and the function’s non-zero return is not explicitly checked before proceeding to final executable check, making diagnosis of failure reasons harder. <details open><summary>Referred Code</summary> ```shell for attempt in {1..4}; do info "Downloading bazelisk v${version} for ${os}/${machine} (attempt ${attempt})" >&2 if curl -sSfL "${url}" -o "${dest}"; then chmod +x "${dest}" return 0 fi sleep $((attempt * 2)) done return 1 } if command -v make >/dev/null 2>&1; then info "Installing bazelisk wrapper via make check-bazel" >&2 if (cd "${KONG_CLONE_DIR}" && make check-bazel >/dev/null); then : else info "make check-bazel failed, falling back to direct bazelisk download" >&2 download_bazelisk "${bazel_bin}" fi else info "Installing bazelisk (make not available)" >&2 ... (clipped 3 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>

qodo-code-review[bot] commented

2025-11-20 06:14:58 +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/1969#issuecomment-3556081375
Original created: 2025-11-20T06:14:58Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Avoid using system `bazel` directly The logic that symlinks to a system-installed `bazel` should be removed. This change bypasses `bazelisk`, which is used to enforce a specific Bazel version and ensure reproducible builds. Examples: scripts/build-kong-vendor.sh [223-229] `if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi` Solution Walkthrough: Before: `... if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 ln -sf "$(command -v bazelisk)" "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 ln -sf "$(command -v bazel)" "${bazel_bin}" return fi # ... fallback to make or download ...` After: `... if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 ln -sf "$(command -v bazelisk)" "${bazel_bin}" return fi # The check for system 'bazel' is removed to ensure # bazelisk is always used to manage the bazel version. if command -v make >/dev/null 2>&1; then # ... fallback to make or download ...` Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical design flaw that undermines build reproducibility by bypassing the `bazelisk` version-enforcement mechanism, which is a core reason for using `bazelisk`.	High
Possible issue	Check function return value to fail-fast Check the return value of the `download_bazelisk` function and exit immediately on failure to provide clearer error context and fail faster. scripts/build-kong-vendor.sh [258-265] `else info "make check-bazel failed, falling back to direct bazelisk download" >&2 - download_bazelisk "${bazel_bin}" + if ! download_bazelisk "${bazel_bin}"; then + # The final check will catch this, but exiting here provides a better error context. + exit 1 + fi fi else info "Installing bazelisk (make not available)" >&2 - download_bazelisk "${bazel_bin}" + if ! download_bazelisk "${bazel_bin}"; then + exit 1 + fi fi` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the return code of the `download_bazelisk` function is ignored. Checking the return code and exiting on failure is a good practice that improves script robustness and provides clearer error reporting.	Medium
General	Refactor duplicate code into a single block Refactor the duplicated code blocks that check for `bazelisk` and `bazel` in the `PATH` into a single, more concise block to improve maintainability. scripts/build-kong-vendor.sh [215-229] - if command -v bazelisk >/dev/null 2>&1; then - info "Using bazelisk from PATH" >&2 + local bazel_cmd + bazel_cmd=$(command -v bazelisk \|\| command -v bazel) + if [[ -n "${bazel_cmd}" ]]; then + info "Using ${bazel_cmd} from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" - ln -sf "$(command -v bazelisk)" "${bazel_bin}" + ln -sf "${bazel_cmd}" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi - if command -v bazel >/dev/null 2>&1; then - info "Using bazel from PATH" >&2 - mkdir -p "$(dirname "${bazel_bin}")" - ln -sf "$(command -v bazel)" "${bazel_bin}" - printf '%s\n' "${bazel_bin}" - return - fi - Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies duplicated code for handling `bazelisk` and `bazel` from `PATH` and proposes a valid refactoring that improves code conciseness and maintainability.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1969#issuecomment-3556081375 Original created: 2025-11-20T06:14:58Z --- ## 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 using system <code>bazel</code> directly</summary> ___ **The logic that symlinks to a system-installed <code>bazel</code> should be removed. This change bypasses <code>bazelisk</code>, which is used to enforce a specific Bazel version and ensure reproducible builds.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R223-R229">scripts/build-kong-vendor.sh [223-229]</a> </summary> ```shell if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" ln -sf "$(command -v bazel)" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi ``` </details> ### Solution Walkthrough: #### Before: ```shell ... if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 ln -sf "$(command -v bazelisk)" "${bazel_bin}" return fi if command -v bazel >/dev/null 2>&1; then info "Using bazel from PATH" >&2 ln -sf "$(command -v bazel)" "${bazel_bin}" return fi # ... fallback to make or download ... ``` #### After: ```shell ... if command -v bazelisk >/dev/null 2>&1; then info "Using bazelisk from PATH" >&2 ln -sf "$(command -v bazelisk)" "${bazel_bin}" return fi # The check for system 'bazel' is removed to ensure # bazelisk is always used to manage the bazel version. if command -v make >/dev/null 2>&1; then # ... fallback to make or download ... ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion correctly identifies a critical design flaw that undermines build reproducibility by bypassing the `bazelisk` version-enforcement mechanism, which is a core reason for using `bazelisk`. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Check function return value to fail-fast</summary> ___ **Check the return value of the <code>download_bazelisk</code> function and exit immediately on failure to provide clearer error context and fail faster.** [scripts/build-kong-vendor.sh [258-265]](https://github.com/carverauto/serviceradar/pull/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R258-R265) ```diff else info "make check-bazel failed, falling back to direct bazelisk download" >&2 - download_bazelisk "${bazel_bin}" + if ! download_bazelisk "${bazel_bin}"; then + # The final check will catch this, but exiting here provides a better error context. + exit 1 + fi fi else info "Installing bazelisk (make not available)" >&2 - download_bazelisk "${bazel_bin}" + if ! download_bazelisk "${bazel_bin}"; then + exit 1 + fi fi ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that the return code of the `download_bazelisk` function is ignored. Checking the return code and exiting on failure is a good practice that improves script robustness and provides clearer error reporting. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Refactor duplicate code into a single block</summary> ___ **Refactor the duplicated code blocks that check for <code>bazelisk</code> and <code>bazel</code> in the <code>PATH</code> into a single, more concise block to improve maintainability.** [scripts/build-kong-vendor.sh [215-229]](https://github.com/carverauto/serviceradar/pull/1969/files#diff-60c9831d4f024788268c9fa56e16e212061b7b55939899f04579d8445036df24R215-R229) ```diff - if command -v bazelisk >/dev/null 2>&1; then - info "Using bazelisk from PATH" >&2 + local bazel_cmd + bazel_cmd=$(command -v bazelisk || command -v bazel) + if [[ -n "${bazel_cmd}" ]]; then + info "Using ${bazel_cmd} from PATH" >&2 mkdir -p "$(dirname "${bazel_bin}")" - ln -sf "$(command -v bazelisk)" "${bazel_bin}" + ln -sf "${bazel_cmd}" "${bazel_bin}" printf '%s\n' "${bazel_bin}" return fi - if command -v bazel >/dev/null 2>&1; then - info "Using bazel from PATH" >&2 - mkdir -p "$(dirname "${bazel_bin}")" - ln -sf "$(command -v bazel)" "${bazel_bin}" - printf '%s\n' "${bazel_bin}" - return - fi - ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly identifies duplicated code for handling `bazelisk` and `bazel` from `PATH` and proposes a valid refactoring that improves code conciseness and maintainability. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>