carverauto/serviceradar
1
1
Fork 0
Code Issues 190 Pull requests 16 Projects Releases 173 Packages Wiki Activity Actions

Updates/k8s buildbuddy #2442

Merged
mfreeman451 merged 7 commits from refs/pull/2442/head into main 2025-11-21 05:02:47 +00:00
Conversation 2 Commits 7 Files changed 5 +28 -9
mfreeman451 commented 2025-11-21 03:58:25 +00:00 (Migrated from github.com)
Owner
Copy link

Imported from GitHub pull request.

Original GitHub pull request: #1974
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1974
Original created: 2025-11-21T03:58:25Z
Original updated: 2025-11-21T05:02:51Z
Original head: carverauto/serviceradar:updates/k8s_buildbuddy
Original base: main
Original merged: 2025-11-21T05:02:47Z 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

  • Pass GITHUB_TOKEN into RBE image build for authentication

  • Make CNPG image preload optional with fallback logic

  • Add error handling for missing credentials or failed pulls

  • Support authenticated and unauthenticated image pull scenarios

Diagram Walkthrough

flowchart LR
  A["RBE Build Workflow"] -->|"Pass GITHUB_TOKEN"| B["Docker Build"]
  B -->|"Build Args & Secrets"| C["Dockerfile.rbe"]
  C -->|"Check Token & Username"| D{"Credentials Available?"}
  D -->|"Yes"| E["Pull with Auth"]
  D -->|"No"| F["Try Unauthenticated Pull"]
  F -->|"Success"| G["Load CNPG Image"]
  F -->|"Fail"| H["Skip Preload with Warning"]
  E --> G

File Walkthrough
Relevant files
Configuration changes
build-rbe-image.yml
Pass GitHub token and actor to Docker build                           

.github/workflows/build-rbe-image.yml

  • Add build-args to pass GitHub actor username to Docker build
  • Add secrets to pass GITHUB_TOKEN to Docker build context
  • Enable authentication for private image registry access
 +4/-0     
Enhancement
Dockerfile.rbe
Make CNPG preload optional with fallback logic                     

docker/Dockerfile.rbe

  • Make ghcr_token secret optional instead of required
  • Add conditional logic to skip CNPG preload if image not set
  • Implement fallback to unauthenticated pull if credentials missing
  • Add warning message when CNPG preload is skipped
  • Improve error handling with graceful degradation
 +15/-4   
Imported from GitHub pull request. Original GitHub pull request: #1974 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1974 Original created: 2025-11-21T03:58:25Z Original updated: 2025-11-21T05:02:51Z Original head: carverauto/serviceradar:updates/k8s_buildbuddy Original base: main Original merged: 2025-11-21T05:02:47Z 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** - Pass GITHUB_TOKEN into RBE image build for authentication - Make CNPG image preload optional with fallback logic - Add error handling for missing credentials or failed pulls - Support authenticated and unauthenticated image pull scenarios ___ ### Diagram Walkthrough ```mermaid flowchart LR A["RBE Build Workflow"] -->|"Pass GITHUB_TOKEN"| B["Docker Build"] B -->|"Build Args & Secrets"| C["Dockerfile.rbe"] C -->|"Check Token & Username"| D{"Credentials Available?"} D -->|"Yes"| E["Pull with Auth"] D -->|"No"| F["Try Unauthenticated Pull"] F -->|"Success"| G["Load CNPG Image"] F -->|"Fail"| H["Skip Preload with Warning"] E --> G ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>build-rbe-image.yml</strong><dd><code>Pass GitHub token and actor to Docker build</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> .github/workflows/build-rbe-image.yml <ul><li>Add <code>build-args</code> to pass GitHub actor username to Docker build<br> <li> Add <code>secrets</code> to pass GITHUB_TOKEN to Docker build context<br> <li> Enable authentication for private image registry access</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1974/files#diff-267c8680ad3f655b43d9bc8d7c55af4cef338efacdc9c90b3264ef9856a22493">+4/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>Dockerfile.rbe</strong><dd><code>Make CNPG preload optional with fallback logic</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> docker/Dockerfile.rbe <ul><li>Make <code>ghcr_token</code> secret optional instead of required<br> <li> Add conditional logic to skip CNPG preload if image not set<br> <li> Implement fallback to unauthenticated pull if credentials missing<br> <li> Add warning message when CNPG preload is skipped<br> <li> Improve error handling with graceful degradation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1974/files#diff-40936cbae5822a0a5fa8016befa08eb3a7836c93328e8043dcdfb3885a6201b2">+15/-4</a>&nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2025-11-21 03:58:45 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1974#issuecomment-3561295136
Original created: 2025-11-21T03:58:45Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪
Unauthenticated image pull

Description: The Docker build now conditionally pulls a container image without authentication, which
can lead to non-reproducible builds and potential supply-chain risks if the image tag is
mutable or the registry is untrusted; authenticated pulls should be enforced for private
or security-sensitive images, or the image digest should be pinned.
Dockerfile.rbe [80-94]

Referred Code 
RUN --mount=type=secret,id=ghcr_token <<'EOF'
set -euo pipefail
TOKEN_FILE="/run/secrets/ghcr_token"
if [[ -z "${GHCR_CNPG_IMAGE:-}" ]]; then
  echo "CNPG image not set; skipping preload"
  exit 0
fi
if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then
  TOKEN_VALUE=$(cat "$TOKEN_FILE")
  skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"
elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then
  echo "Pulled CNPG image without auth"
else
  echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2
fi
Overprivileged token scope

Description: Passing the GitHub token to docker/build-push-action as a build secret without restricting
permissions or scope may overprivilege the build context; ensure the token has
least-privilege (e.g., ephemeral GITHUB_TOKEN with minimal scopes) to avoid credential
exposure via downstream tooling or image layer leaks.
build-rbe-image.yml [62-65]

Referred Code 
build-args: |
  GHCR_USERNAME=${{ github.actor }}
secrets: |
  ghcr_token=${{ secrets.GITHUB_TOKEN }}
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:
Missing audit logs: The new logic performs authenticated/unauthenticated image pulls and fallback handling
without emitting structured audit logs of the actions or outcomes, which may limit
traceability of critical build-time actions.

Referred Code 
RUN --mount=type=secret,id=ghcr_token <<'EOF'
set -euo pipefail
TOKEN_FILE="/run/secrets/ghcr_token"
if [[ -z "${GHCR_CNPG_IMAGE:-}" ]]; then
  echo "CNPG image not set; skipping preload"
  exit 0
fi
if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then
  TOKEN_VALUE=$(cat "$TOKEN_FILE")
  skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"
elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then
  echo "Pulled CNPG image without auth"
else
  echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2
fi

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/1974#issuecomment-3561295136 Original created: 2025-11-21T03:58:45Z --- ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/3e243743d6e2c280e858d27836383867f2d840bc --> Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td rowspan=2>⚪</td> <td><details><summary><strong>Unauthenticated image pull </strong></summary><br> <b>Description:</b> The Docker build now conditionally pulls a container image without authentication, which <br>can lead to non-reproducible builds and potential supply-chain risks if the image tag is <br>mutable or the registry is untrusted; authenticated pulls should be enforced for private <br>or security-sensitive images, or the image digest should be pinned.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1974/files#diff-40936cbae5822a0a5fa8016befa08eb3a7836c93328e8043dcdfb3885a6201b2R80-R94'>Dockerfile.rbe [80-94]</a></strong><br> <details open><summary>Referred Code</summary> ```txt RUN --mount=type=secret,id=ghcr_token <<'EOF' set -euo pipefail TOKEN_FILE="/run/secrets/ghcr_token" if [[ -z "${GHCR_CNPG_IMAGE:-}" ]]; then echo "CNPG image not set; skipping preload" exit 0 fi if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then TOKEN_VALUE=$(cat "$TOKEN_FILE") skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE" elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then echo "Pulled CNPG image without auth" else echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2 fi ``` </details></details></td></tr> <tr><td><details><summary><strong>Overprivileged token scope </strong></summary><br> <b>Description:</b> Passing the GitHub token to docker/build-push-action as a build secret without restricting <br>permissions or scope may overprivilege the build context; ensure the token has <br>least-privilege (e.g., ephemeral GITHUB_TOKEN with minimal scopes) to avoid credential <br>exposure via downstream tooling or image layer leaks.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1974/files#diff-267c8680ad3f655b43d9bc8d7c55af4cef338efacdc9c90b3264ef9856a22493R62-R65'>build-rbe-image.yml [62-65]</a></strong><br> <details open><summary>Referred Code</summary> ```yaml build-args: | GHCR_USERNAME=${{ github.actor }} secrets: | ghcr_token=${{ secrets.GITHUB_TOKEN }} ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></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'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=5>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > 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><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** Passed<br> > 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><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > 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><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** Passed<br> > 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><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > 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><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1974/files#diff-40936cbae5822a0a5fa8016befa08eb3a7836c93328e8043dcdfb3885a6201b2R80-R94'><strong>Missing audit logs</strong></a>: The new logic performs authenticated/unauthenticated image pulls and fallback handling <br>without emitting structured audit logs of the actions or outcomes, which may limit <br>traceability of critical build-time actions.<br> <details open><summary>Referred Code</summary> ```txt RUN --mount=type=secret,id=ghcr_token <<'EOF' set -euo pipefail TOKEN_FILE="/run/secrets/ghcr_token" if [[ -z "${GHCR_CNPG_IMAGE:-}" ]]; then echo "CNPG image not set; skipping preload" exit 0 fi if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then TOKEN_VALUE=$(cat "$TOKEN_FILE") skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE" elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then echo "Pulled CNPG image without auth" else echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2 fi ``` </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 <!-- /compliance --update_compliance=true --> </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>
qodo-code-review[bot] commented 2025-11-21 03:59:49 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1974#issuecomment-3561296889
Original created: 2025-11-21T03:59:49Z

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Dockerfile logic creates non-deterministic builds

The Dockerfile's conditional logic results in non-deterministic builds, as the
final image contents depend on the build environment. To ensure predictability,
the build should fail explicitly if the image cannot be preloaded, or
alternative Dockerfiles should be used for different contexts.

Examples:

docker/Dockerfile.rbe [80-95] 
RUN --mount=type=secret,id=ghcr_token <<'EOF'
set -euo pipefail
TOKEN_FILE="/run/secrets/ghcr_token"
if [[ -z "${GHCR_CNPG_IMAGE:-}" ]]; then
  echo "CNPG image not set; skipping preload"
  exit 0
fi
if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then
  TOKEN_VALUE=$(cat "$TOKEN_FILE")
  skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"

 ... (clipped 6 lines)

Solution Walkthrough:

Before:

# docker/Dockerfile.rbe
RUN --mount=type=secret,id=ghcr_token <<'EOF'
set -euo pipefail
...
if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then
  # Attempt authenticated pull
  skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" ...
elif skopeo copy "docker://$GHCR_CNPG_IMAGE" ...; then
  # Attempt unauthenticated pull
  echo "Pulled CNPG image without auth"
else
  # If both fail, warn and continue, creating a different image
  echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2
fi
EOF

After:

# docker/Dockerfile.rbe
RUN --mount=type=secret,id=ghcr_token <<'EOF'
set -euo pipefail
...
# Define a function to attempt the pull
pull_image() {
    if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then
        skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" ...
    else
        skopeo copy "docker://$GHCR_CNPG_IMAGE" ...
    fi
}

# Execute the pull and fail the build if it's unsuccessful
if ! pull_image; then
    echo "Error: Failed to preload CNPG image. Build failed." >&2
    exit 1
fi
EOF
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical design flaw where the conditional logic in the Dockerfile leads to non-deterministic builds, which undermines the reliability of the build system and can cause hard-to-debug downstream failures.

High
Possible issue
Fix broken fallback logic for pull

Fix the shell script's fallback logic by handling the potential failure of the
authenticated skopeo copy command. The current implementation with set -e would
cause the script to exit prematurely instead of attempting an unauthenticated
pull.

docker/Dockerfile.rbe [87-94]

 if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then
   TOKEN_VALUE=$(cat "$TOKEN_FILE")
-  skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"
+  if skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then
+    echo "Pulled CNPG image with auth"
+  elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then
+    echo "Pulled CNPG image without auth (authenticated pull failed)"
+  else
+    echo "Warning: CNPG preload skipped (pull failed)" >&2
+  fi
 elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then
   echo "Pulled CNPG image without auth"
 else
-  echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2
+  echo "Warning: CNPG preload skipped (missing credentials and pull failed)" >&2
 fi
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a critical bug where set -e would cause the script to exit on a failed authenticated pull, preventing the intended fallback to an unauthenticated pull. This fix is crucial for the reliability of the image build process.

Medium
  • Update
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1974#issuecomment-3561296889 Original created: 2025-11-21T03:59:49Z --- ## PR Code Suggestions ✨ <!-- 3e24374 --> Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Dockerfile logic creates non-deterministic builds</summary> ___ **The Dockerfile's conditional logic results in non-deterministic builds, as the <br>final image contents depend on the build environment. To ensure predictability, <br>the build should fail explicitly if the image cannot be preloaded, or <br>alternative Dockerfiles should be used for different contexts.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1974/files#diff-40936cbae5822a0a5fa8016befa08eb3a7836c93328e8043dcdfb3885a6201b2R80-R95">docker/Dockerfile.rbe [80-95]</a> </summary> ```dockerfile RUN --mount=type=secret,id=ghcr_token <<'EOF' set -euo pipefail TOKEN_FILE="/run/secrets/ghcr_token" if [[ -z "${GHCR_CNPG_IMAGE:-}" ]]; then echo "CNPG image not set; skipping preload" exit 0 fi if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then TOKEN_VALUE=$(cat "$TOKEN_FILE") skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE" ... (clipped 6 lines) ``` </details> ### Solution Walkthrough: #### Before: ```dockerfile # docker/Dockerfile.rbe RUN --mount=type=secret,id=ghcr_token <<'EOF' set -euo pipefail ... if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then # Attempt authenticated pull skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" ... elif skopeo copy "docker://$GHCR_CNPG_IMAGE" ...; then # Attempt unauthenticated pull echo "Pulled CNPG image without auth" else # If both fail, warn and continue, creating a different image echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2 fi EOF ``` #### After: ```dockerfile # docker/Dockerfile.rbe RUN --mount=type=secret,id=ghcr_token <<'EOF' set -euo pipefail ... # Define a function to attempt the pull pull_image() { if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" ... else skopeo copy "docker://$GHCR_CNPG_IMAGE" ... fi } # Execute the pull and fail the build if it's unsuccessful if ! pull_image; then echo "Error: Failed to preload CNPG image. Build failed." >&2 exit 1 fi EOF ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a critical design flaw where the conditional logic in the `Dockerfile` leads to non-deterministic builds, which undermines the reliability of the build system and can cause hard-to-debug downstream failures. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Fix broken fallback logic for pull</summary> ___ **Fix the shell script's fallback logic by handling the potential failure of the <br>authenticated <code>skopeo copy</code> command. The current implementation with <code>set -e</code> would <br>cause the script to exit prematurely instead of attempting an unauthenticated <br>pull.** [docker/Dockerfile.rbe [87-94]](https://github.com/carverauto/serviceradar/pull/1974/files#diff-40936cbae5822a0a5fa8016befa08eb3a7836c93328e8043dcdfb3885a6201b2R87-R94) ```diff if [[ -f "$TOKEN_FILE" && -s "$TOKEN_FILE" && -n "${GHCR_USERNAME:-}" ]]; then TOKEN_VALUE=$(cat "$TOKEN_FILE") - skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE" + if skopeo copy --src-creds "$GHCR_USERNAME:$TOKEN_VALUE" "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then + echo "Pulled CNPG image with auth" + elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then + echo "Pulled CNPG image without auth (authenticated pull failed)" + else + echo "Warning: CNPG preload skipped (pull failed)" >&2 + fi elif skopeo copy "docker://$GHCR_CNPG_IMAGE" "docker-archive:/opt/cnpg_image.tar:$GHCR_CNPG_IMAGE"; then echo "Pulled CNPG image without auth" else - echo "Warning: CNPG preload skipped (missing credentials or pull failed)" >&2 + echo "Warning: CNPG preload skipped (missing credentials and pull failed)" >&2 fi ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a critical bug where `set -e` would cause the script to exit on a failed authenticated pull, preventing the intended fallback to an unauthenticated pull. This fix is crucial for the reliability of the image build process. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update <!-- /improve_multi --more_suggestions=true --> </td><td></td></tr></tbody></table>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
1week

   
2weeks

   
Failed compliance check

   
IP cameras

   
NATS

NATS JetStream

  
Possible security concern

   
Review effort 1/5

   
Review effort 2/5

   
Review effort 3/5

   
Review effort 4/5

   
Review effort 5/5

   
UI

   
aardvark

   
accessibility

   
amd64

   
api

   
arm64

   
auth

   
back-end

   
bgp

   
blog

   
bug

Something isn't working

  
build

   
checkers

   
ci-cd

continuous integration-continuous deployments

  
cleanup

   
cnpg

cloud-native postgres

  
codex

   
core

core service

  
dependencies

Pull requests that update a dependency file

  
device-management

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
dusk

   
ebpf

   
enhancement

New feature or request

  
eta 1d

   
eta 1hr

   
eta 3d

   
eta 3hr

   
feature

   
fieldsurvey

   
github_actions

Pull requests that update GitHub Actions code

  
go

Pull requests that update Go code

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update Javascript code

  
k8s

   
log-collector

   
mapper

   
mtr

multi traceroute

  
needs-triage

   
netflow

   
network-sweep

   
observability

   
oracle

Oracle Linux related issues

  
otel

opentelemetry logs, traces, metrics

  
plug-in

   
proton

timeplus proton streaming database

  
python

   
question

Further information is requested

  
reddit

   
redhat

   
research

   
rperf

   
rperf-checker

   
rust

Pull requests that update rust code

  
sdk

   
security

   
serviceradar-agent

   
serviceradar-agent-gateway

   
serviceradar-web

   
serviceradar-web-ng

   
siem

   
snmp

   
sysmon

   
topology

   
ubiquiti

   
wasm

   
wontfix

This will not be worked on

  
zen-engine
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!2442
No description provided.