Skip to content

Restore ability to build Containerfile(s) on PR#645

Open
tarilabs wants to merge 13 commits intotrustyai-explainability:mainfrom
tarilabs:tarilabs-20260213-GHAbuildImages
Open

Restore ability to build Containerfile(s) on PR#645
tarilabs wants to merge 13 commits intotrustyai-explainability:mainfrom
tarilabs:tarilabs-20260213-GHAbuildImages

Conversation

@tarilabs
Copy link
Copy Markdown
Member

@tarilabs tarilabs commented Feb 13, 2026

Helps when developing on Mac and testing against an usual Amd64 cluster
Kept commit history so the operations to restore can be better identified

Will need that github repo contains:

  • secrets.QUAY_ROBOT_USERNAME
  • secrets.QUAY_ROBOT_SECRET

which appears to be still existing

Summary by Sourcery

Add a GitHub Actions workflow to build and push CI container images for pull requests based on labels and post the resulting image references as a PR comment.

Build:

  • Introduce a PR-triggered GitHub Actions workflow that conditionally builds and pushes operator, LMES, and orchestrator container images to Quay with short-lived CI tags.

CI:

  • Gate PR image builds on repository and label checks, and automatically comment on the PR with the built image locations.

Summary by CodeRabbit

  • Chores
    • Automated image build & publish on pull requests, posting build results and direct image links as PR comments for verification.
    • Conditional builds for additional components when corresponding PR flags/labels are present.
    • CI-tagged images now include a 7-day expiration.
    • Enhanced build logs and updated PR comments to show current image coordinates and status.

…bility#631)"

This reverts commit 67f38fc.

Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Feb 13, 2026

Reviewer's Guide

Adds a conditional GitHub Actions workflow that, when specific labels are present on a PR, builds and pushes CI-scoped container images for the operator, LMES driver/job, and guardrails orchestrator to Quay, tagging them with the PR head SHA and posting a status comment back to the PR.

Sequence diagram for PR-triggered CI image build and Quay publish workflow

sequenceDiagram
    actor Developer
    participant GitHub
    participant Workflow_build_images_on_PR
    participant DockerEngine
    participant Quay
    participant PR_comment

    Developer->>GitHub: Open PR or update PR
    Developer->>GitHub: Apply labels needs-build and ok-to-test or lgtm or approved

    GitHub-->>Workflow_build_images_on_PR: Trigger pull_request event
    Workflow_build_images_on_PR-->>Workflow_build_images_on_PR: Check repository and label conditions
    alt Conditions satisfied
        Workflow_build_images_on_PR->>Workflow_build_images_on_PR: Set default env variables
        opt LMES build requested (label needs-lmes-build)
            Workflow_build_images_on_PR->>Workflow_build_images_on_PR: Override LMES env vars
        end
        opt Orchestrator build requested (label needs-orchestrator-build)
            Workflow_build_images_on_PR->>Workflow_build_images_on_PR: Override orchestrator env vars
        end

        Workflow_build_images_on_PR->>Workflow_build_images_on_PR: Patch Dockerfile labels with expiry
        Workflow_build_images_on_PR->>DockerEngine: docker login to Quay

        Workflow_build_images_on_PR->>DockerEngine: docker build operator image (tag=PR head sha)
        DockerEngine->>Quay: Push operator image

        opt LMES build requested
            Workflow_build_images_on_PR->>DockerEngine: docker build LMES driver image
            DockerEngine->>Quay: Push LMES driver image
            Workflow_build_images_on_PR->>DockerEngine: docker build LMES job image
            DockerEngine->>Quay: Push LMES job image
        end

        opt Orchestrator build requested
            Workflow_build_images_on_PR->>DockerEngine: docker build orchestrator image
            DockerEngine->>Quay: Push orchestrator image
        end

        Workflow_build_images_on_PR->>PR_comment: Find existing status comment
        Workflow_build_images_on_PR->>PR_comment: Create or update comment with image links
    else Conditions not satisfied
        Workflow_build_images_on_PR-->>GitHub: Job skipped
    end
Loading

Flow diagram for conditional image builds based on PR labels

flowchart TD
    A[PR event\nopened, labeled, synchronize, reopened] --> B{Repository is trustyai-service-operator?}
    B -- No --> Z[Skip workflow]
    B -- Yes --> C{PR has label needs-build?}
    C -- No --> Z
    C -- Yes --> D{PR has label ok-to-test or lgtm or approved?}
    D -- No --> Z
    D -- Yes --> E[Set default operator, LMES, orchestrator env vars]

    E --> F{Label needs-lmes-build present?}
    F -- Yes --> G[Override LMES tags and image names for CI]
    F -- No --> H

    G --> H{Label needs-orchestrator-build present?}
    E --> H

    H -- Yes --> I[Override orchestrator tag and image name for CI]
    H -- No --> J

    I --> J[Build and push operator image]
    J --> K{Label needs-lmes-build present?}

    K -- Yes --> L[Build and push LMES driver image]
    L --> M[Build and push LMES job image]
    M --> N
    K -- No --> N[Build and push orchestrator image if requested]

    N --> O[Create or update PR comment with image references]
Loading

File-Level Changes

Change Details Files
Introduce a label-gated PR workflow to build and push CI images to Quay for the operator, LMES components, and orchestrator, and then comment results on the PR.
  • Create a pull_request-triggered workflow with path filters and label-based conditions requiring needs-build plus one of ok-to-test, lgtm, or approved, and restricting execution to the main upstream repo.
  • Set default image names and tags via environment variables, overriding tags and repositories for LMES and orchestrator builds when corresponding needs-* labels are present.
  • Log key reference and image variables for traceability and debugging, using the PR head SHA explicitly to avoid merge-commit ambiguity.
  • Add temporary expiry metadata to CI-tagged images by modifying Dockerfiles before build, and use docker login/build/push steps to publish operator, LMES driver/job, and orchestrator images to Quay when enabled.
  • Use peter-evans/find-comment and create-or-update-comment actions to upsert a PR comment summarizing successful image build and providing direct image references for each component.
.github/workflows/build-images-on-PR.yaml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Feb 13, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 13, 2026

Warning

Rate limit exceeded

@tarilabs has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 24 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Adds a new GitHub Actions workflow to build and push operator, LMES driver/job, and orchestrator container images on pull request events, with label gating, conditional builds, Quay authentication, and PR comments containing image links. (49 words)

Changes

Cohort / File(s) Summary
New CI workflow
.github/workflows/build-images-on-pr.yaml
Introduces a workflow triggered on PRs that enforces labels, sets image names/tags (operator tag from PR head SHA), conditionally builds LMES and orchestrator images, authenticates to Quay, pushes images, and comments/updates PR with build results and image links.
Container image metadata
Dockerfile
Adds a 7-day expiration / tag metadata for CI-tagged images.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer
    participant GH as GitHub (PR)
    participant Actions as GitHub Actions Runner
    participant Quay as Quay Registry
    participant API as GitHub API (PR comments)

    Dev->>GH: Open/Update PR (with labels)
    GH->>Actions: Trigger workflow (build-images-on-pr)
    Actions->>Actions: Determine build scope, set image tags (operator SHA), set env
    Actions->>Quay: Authenticate
    Actions->>Quay: Build & push operator image
    alt needs-lmes-build
        Actions->>Quay: Build & push LMES driver & job images
    end
    alt needs-orchestrator-build
        Actions->>Quay: Build & push orchestrator image
    end
    Actions->>API: Create or update PR comment with image links
    API->>GH: Display comment on PR
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

kind/enhancement, lgtm

Suggested reviewers

  • RobGeada

Poem

🐰 I hopped to the CI, with a carrot and cheer,
Built images on PRs for all to revere,
Tags from the SHA, pushed off to Quay,
A comment appears — hooray, hooray! 🥕🎉

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Restore ability to build Containerfile(s) on PR' directly and clearly summarizes the main change: restoring a GitHub Actions workflow to enable container image builds on pull requests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 4 issues, and left some high level feedback:

  • The environment variable used for the image name is inconsistent (you set IMAGE_NAME but later reference OPERATOR_IMAGE_NAME in logs and build/push steps), which will result in empty or incorrect tags; align these to a single variable.
  • Several docker-related steps (login/build/push) are not guarded by if: env.BUILD_CONTEXT == 'ci', so they may execute with unset environment variables when BUILD_CONTEXT is not defined; consider adding the same condition or explicitly initializing BUILD_CONTEXT.
  • The LMES/orchestrator steps check for env.BUILD_CONTEXT == 'main' or 'tag', but these contexts are never set in this workflow, and the PR comment uses https://${{ env.* }} URLs that won’t produce valid links for quay images; you may want to simplify the conditions and adjust the links to plain text or correct quay repository URLs.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The environment variable used for the image name is inconsistent (you set `IMAGE_NAME` but later reference `OPERATOR_IMAGE_NAME` in logs and build/push steps), which will result in empty or incorrect tags; align these to a single variable.
- Several docker-related steps (login/build/push) are not guarded by `if: env.BUILD_CONTEXT == 'ci'`, so they may execute with unset environment variables when `BUILD_CONTEXT` is not defined; consider adding the same condition or explicitly initializing `BUILD_CONTEXT`.
- The LMES/orchestrator steps check for `env.BUILD_CONTEXT == 'main'` or `'tag'`, but these contexts are never set in this workflow, and the PR comment uses `https://${{ env.* }}` URLs that won’t produce valid links for quay images; you may want to simplify the conditions and adjust the links to plain text or correct quay repository URLs.

## Individual Comments

### Comment 1
<location> `.github/workflows/build-images-on-PR.yaml:42` </location>
<code_context>
+        if: env.BUILD_CONTEXT == 'ci'
+        run: |
+          echo "OPERATOR_TAG=${{ github.event.pull_request.head.sha }}" >> $GITHUB_ENV
+          echo "IMAGE_NAME=quay.io/trustyai/trustyai-service-operator-ci" >> $GITHUB_ENV
+
+      - name: Set CI environment - LMES
</code_context>

<issue_to_address>
**issue (bug_risk):** Environment variable name used for the operator image is inconsistent and will result in an empty tag reference.

The workflow sets `IMAGE_NAME`, but later steps reference `OPERATOR_IMAGE_NAME` (e.g., in the log and `docker build -t ${{ env.OPERATOR_IMAGE_NAME }}:${{ env.OPERATOR_TAG }}`). Because `OPERATOR_IMAGE_NAME` is never defined, those steps will use an empty image name and fail. Align the variable usage by either exporting `OPERATOR_IMAGE_NAME` here or updating later steps to use `IMAGE_NAME` consistently.
</issue_to_address>

### Comment 2
<location> `.github/workflows/build-images-on-PR.yaml:74-75` </location>
<code_context>
+      - name: Put expiry date on CI-tagged image
+        if: env.BUILD_CONTEXT == 'ci'
+        run: sed -i 's#summary="odh-trustyai-service-operator\"#summary="odh-trustyai-service-operator" \\ \n      quay.expires-after=7d#' Dockerfile
+      - name: Log in to Quay
+        run: docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} -p ${{ secrets.QUAY_ROBOT_SECRET }} quay.io
+      - name: Build main image
+        run: docker build -t ${{ env.OPERATOR_IMAGE_NAME }}:${{ env.OPERATOR_TAG }} .
</code_context>

<issue_to_address>
**issue (bug_risk):** Docker login and image build/push steps should be gated on the CI context to avoid failures on PRs without secrets.

The Quay login and main image build/push steps are not conditioned on `env.BUILD_CONTEXT == 'ci'`, so they can still run on PRs without the necessary secrets or env vars and cause CI failures. Please gate these steps with the same CI-context condition (or equivalent) so they only execute when the required secrets are available.
</issue_to_address>

### Comment 3
<location> `.github/workflows/build-images-on-PR.yaml:124-130` </location>
<code_context>
+          body: |
+            PR image build and manifest generation completed successfully!
+
+            📦 [PR image](https://${{ env.OPERATOR_IMAGE_NAME }}:${{ env.OPERATOR_TAG }}): `${{ env.OPERATOR_IMAGE_NAME }}:${{ env.OPERATOR_TAG }}`
+
+            📦 [LMES driver image](https://${{ env.LMES_DRIVER_IMAGE_NAME }}:${{ env.LMES_DRIVER_TAG }}): `${{ env.LMES_DRIVER_IMAGE_NAME }}:${{ env.LMES_DRIVER_TAG }}`
+
+            📦 [LMES job image](https://${{ env.LMES_JOB_IMAGE_NAME }}:${{ env.LMES_JOB_TAG }}): `${{ env.LMES_JOB_IMAGE_NAME }}:${{ env.LMES_JOB_TAG }}`
+
+            📦 [Guardrails orchestrator image](https://${{ env.ORCH_IMAGE_NAME }}:${{ env.ORCH_TAG }}): `${{ env.ORCH_IMAGE_NAME }}:${{ env.ORCH_TAG }}`
</code_context>

<issue_to_address>
**suggestion (bug_risk):** The Quay image links use an invalid URL format and likely won’t be clickable.

These URLs use the `image:tag` form as a hostname and will not resolve. Keep `image:tag` in the inline code, but point the markdown links to real Quay web URLs instead (for example, `https://quay.io/repository/<org>/<repo>?tab=tags` or a tag-specific page) while still rendering `${{ env.* }}:${{ env.* }}` as the visible text.
</issue_to_address>

### Comment 4
<location> `.github/workflows/build-images-on-PR.yaml:82-91` </location>
<code_context>
+      - name: Build LMES driver image
</code_context>

<issue_to_address>
**suggestion:** Conditions referencing `BUILD_CONTEXT == 'main'` and `'tag'` appear unused in this PR-focused workflow.

In this workflow `BUILD_CONTEXT` is only ever set to `'ci'`, so any `if:` conditions checking for `'main'` or `'tag'` will never be true and act as dead code. If main/tag builds are handled elsewhere, consider dropping those checks and relying on the label-based conditions. If this workflow should handle main/tag as well, make sure `BUILD_CONTEXT` is actually set to those values somewhere in this file.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread .github/workflows/build-images-on-PR.yaml Outdated
Comment thread .github/workflows/build-images-on-PR.yaml
Comment thread .github/workflows/build-images-on-PR.yaml
Comment thread .github/workflows/build-images-on-PR.yaml Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @.github/workflows/build-images-on-PR.yaml:
- Around line 121-130: The PR comment always lists all four images causing
broken links when LMES/orchestrator weren't built; update the workflow to only
emit the LMES and orchestrator lines when their corresponding build labels are
present by checking the same conditions used to set the env vars (e.g.,
needs-lmes-build, needs-orchestrator-build) or by guarding the comment
generation step with conditional logic; specifically, modify the step that
constructs the comment body to omit the LMES lines referencing
LMES_DRIVER_IMAGE_NAME and LMES_JOB_IMAGE_NAME unless needs-lmes-build is true,
and omit the Guardrails line referencing ORCH_IMAGE_NAME unless
needs-orchestrator-build is true (leave the OPERATOR_IMAGE_NAME line unchanged).
- Around line 38-42: The workflow sets IMAGE_NAME but later references
OPERATOR_IMAGE_NAME, causing an undefined env var; update the "Set CI
environment" step to export OPERATOR_IMAGE_NAME instead (or export both
IMAGE_NAME and OPERATOR_IMAGE_NAME) so the referenced env.OPERATOR_IMAGE_NAME
used by steps referencing OPERATOR_IMAGE_NAME (seen in subsequent build/push
steps) is defined and contains the same
quay.io/trustyai/trustyai-service-operator-ci value (and keep
OPERATOR_TAG/OPERATOR_TAG usage unchanged).
- Around line 59-68: The "Log reference variables" step is injecting
github.head_ref directly into the inline run script (github.head_ref) which is
attacker-controlled; change the step to pass github.head_ref via an environment
variable (e.g., HEAD_REF: ${{ github.head_ref }}) or set it with echoing from
the env and then reference the env var inside run (e.g., echo "$HEAD_REF")
instead of interpolating ${{ github.head_ref }} directly; update the step that
logs variables (step name "Log reference variables") to use the environment
variable for github.head_ref and any other user-controllable refs before
echoing.
🧹 Nitpick comments (3)
.github/workflows/build-images-on-PR.yaml (3)

74-79: Build/push steps for the main operator image lack a BUILD_CONTEXT guard.

Lines 74–79 run unconditionally (no if), unlike the checkout and env-setting steps. Per the comment on Line 19, the intent is to guard against future on: additions—but these steps would run with empty OPERATOR_IMAGE_NAME/OPERATOR_TAG if BUILD_CONTEXT is not set. Consider adding the same if: env.BUILD_CONTEXT == 'ci' guard for consistency.


81-103: Dead conditions: BUILD_CONTEXT == 'main' and BUILD_CONTEXT == 'tag' are unreachable.

Since this workflow only triggers on pull_request events and BUILD_CONTEXT is only ever set to 'ci', the env.BUILD_CONTEXT == 'main' and env.BUILD_CONTEXT == 'tag' branches in these conditionals will never be true. This looks like a leftover from the original multi-trigger workflow. Consider removing these dead branches to avoid confusion.


74-75: Use --password-stdin for docker login instead of passing the password via CLI argument.

Passing the password with -p can expose it in process listings. Prefer piping it via stdin.

🔒 Proposed fix
       - name: Log in to Quay
-        run: docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} -p ${{ secrets.QUAY_ROBOT_SECRET }} quay.io
+        run: echo "${{ secrets.QUAY_ROBOT_SECRET }}" | docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} --password-stdin quay.io

Comment thread .github/workflows/build-images-on-PR.yaml Outdated
Comment thread .github/workflows/build-images-on-PR.yaml
Comment thread .github/workflows/build-images-on-PR.yaml
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Comment thread .github/workflows/build-images-on-PR.yaml
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
@tarilabs tarilabs added the needs-build Pull Request that need a new build of the TrustyAI Service Operator label Feb 13, 2026
@tarilabs tarilabs marked this pull request as draft February 13, 2026 23:18
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
@tarilabs tarilabs marked this pull request as ready for review February 13, 2026 23:35
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • The sed replacement used to inject quay.expires-after=7d into Dockerfile relies on a very specific summary line format and may be brittle to future edits; consider using a dedicated LABEL line or build-arg/ENV to add the expiry label in a less fragile way.
  • The PR comment builds image links as https://${{ env.OPERATOR_IMAGE_NAME }}:${{ env.OPERATOR_TAG }} which is not a valid Quay URL; it would be more useful to construct proper repository URLs (e.g. https://quay.io/repository/...) so they are clickable and point to the image page.
  • For docker login, using --password-stdin (e.g. echo ${{ secrets.QUAY_ROBOT_SECRET }} | docker login -u ... --password-stdin) is generally safer and avoids putting the password directly on the command line.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `sed` replacement used to inject `quay.expires-after=7d` into `Dockerfile` relies on a very specific `summary` line format and may be brittle to future edits; consider using a dedicated `LABEL` line or build-arg/ENV to add the expiry label in a less fragile way.
- The PR comment builds image links as `https://${{ env.OPERATOR_IMAGE_NAME }}:${{ env.OPERATOR_TAG }}` which is not a valid Quay URL; it would be more useful to construct proper repository URLs (e.g. `https://quay.io/repository/...`) so they are clickable and point to the image page.
- For `docker login`, using `--password-stdin` (e.g. `echo ${{ secrets.QUAY_ROBOT_SECRET }} | docker login -u ... --password-stdin`) is generally safer and avoids putting the password directly on the command line.

## Individual Comments

### Comment 1
<location> `.github/workflows/build-images-on-PR.yaml:69-73` </location>
<code_context>
+          echo "GUARDRAILS ORCH IMAGE AT: ${{ env.ORCH_IMAGE_NAME }}:${{ env.ORCH_TAG }}"
+
+      # Run docker commands
+      - name: Put expiry date on CI-tagged image
+        run: sed -i 's#summary="odh-trustyai-service-operator\"#summary="odh-trustyai-service-operator" \\ \n      quay.expires-after=7d#' Dockerfile
+      - name: Log in to Quay
+        run: docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} -p ${{ secrets.QUAY_ROBOT_SECRET }} quay.io
</code_context>

<issue_to_address>
**suggestion (bug_risk):** The `sed` replacement is brittle and may be error-prone across Dockerfile changes.

This command depends on an exact string match (including the escaped quote) and injects a newline and extra content into that same line. Small Dockerfile changes (whitespace, label order, quote style) will cause it to do nothing without failing, so the expiry label may be missing. Consider either adding a separate `LABEL quay.expires-after=7d` via a small patch/append step, or matching on the label key instead of the full value to make this more resilient to future edits.

```suggestion
      # Run docker commands
      - name: Put expiry date on CI-tagged image
        run: echo 'LABEL quay.expires-after=7d' >> Dockerfile
      - name: Log in to Quay
        run: docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} -p ${{ secrets.QUAY_ROBOT_SECRET }} quay.io
```
</issue_to_address>

### Comment 2
<location> `.github/workflows/build-images-on-PR.yaml:72-73` </location>
<code_context>
+      # Run docker commands
+      - name: Put expiry date on CI-tagged image
+        run: sed -i 's#summary="odh-trustyai-service-operator\"#summary="odh-trustyai-service-operator" \\ \n      quay.expires-after=7d#' Dockerfile
+      - name: Log in to Quay
+        run: docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} -p ${{ secrets.QUAY_ROBOT_SECRET }} quay.io
+      - name: Build main image
+        run: docker build -t ${{ env.OPERATOR_IMAGE_NAME }}:${{ env.OPERATOR_TAG }} .
</code_context>

<issue_to_address>
**🚨 suggestion (security):** Consider using `--password-stdin` for `docker login` to reduce secret exposure risk.

Using `-p` exposes the password on the process command line, which can be visible via process inspection or logs. Prefer piping the secret instead, e.g.: `echo "$QUAY_ROBOT_SECRET" | docker login -u "$QUAY_ROBOT_USERNAME" --password-stdin quay.io`, which matches Docker’s recommended practice and keeps the secret out of the argument list.

```suggestion
      - name: Log in to Quay
        run: echo "${{ secrets.QUAY_ROBOT_SECRET }}" | docker login -u "${{ secrets.QUAY_ROBOT_USERNAME }}" --password-stdin quay.io
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread .github/workflows/build-images-on-PR.yaml
Comment thread .github/workflows/build-images-on-PR.yaml
Signed-off-by: tarilabs <matteo.mortari@gmail.com>

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.github/workflows/build-images-on-PR.yaml:
- Around line 126-135: The PR comment links use the invalid "https://...:tag"
form; update the notification body (the `body` template) to build Quay web URLs
using the repository path with a query tag parameter instead: for each image
replace `https://${{ env.<IMAGE_NAME> }}:${{ env.<TAG> }}` with
`https://quay.io/repository/${{ env.<IMAGE_NAME> }}?tag=${{ env.<TAG> }}` (apply
for OPERATOR_IMAGE_NAME/OPERATOR_TAG, LMES_DRIVER_IMAGE_NAME/LMES_DRIVER_TAG,
LMES_JOB_IMAGE_NAME/LMES_JOB_TAG, ORCH_IMAGE_NAME/ORCH_TAG) while keeping the
displayed inline text `${{ env.<IMAGE_NAME> }}:${{ env.<TAG> }}` unchanged.
Ensure the substitutions are made in the `body` block where the 📦 links are
defined.
🧹 Nitpick comments (2)
.github/workflows/build-images-on-PR.yaml (2)

72-73: Use --password-stdin for docker login to avoid exposing the secret in the process argument list.

Even though GitHub Actions masks secrets in logs, passing -p exposes the value in /proc/<pid>/cmdline on the runner.

🔒 Proposed fix
       - name: Log in to Quay
-        run: docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} -p ${{ secrets.QUAY_ROBOT_SECRET }} quay.io
+        run: echo "${{ secrets.QUAY_ROBOT_ROBOT_SECRET }}" | docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} --password-stdin quay.io

Note: double-check the secret name — I kept the original QUAY_ROBOT_SECRET.

Corrected version:

       - name: Log in to Quay
-        run: docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} -p ${{ secrets.QUAY_ROBOT_SECRET }} quay.io
+        run: echo "${{ secrets.QUAY_ROBOT_SECRET }}" | docker login -u ${{ secrets.QUAY_ROBOT_USERNAME }} --password-stdin quay.io

70-71: Fragile sed pattern — silently no-ops if the Dockerfile LABEL line changes.

The match string is tightly coupled to the exact quoting and whitespace in the Dockerfile. If the LABEL line is reformatted, the expiry annotation won't be injected and the CI image will persist indefinitely. Consider appending the label instead (like the LMES/orchestrator steps do with echo >> Dockerfile) or at minimum adding a guard that fails if the substitution didn't apply.

💡 Alternative: append the label, consistent with the other Dockerfiles
       - name: Put expiry date on CI-tagged image
-        run: sed -i 's#summary="odh-trustyai-service-operator\"#summary="odh-trustyai-service-operator" \\ \n      quay.expires-after=7d#' Dockerfile
+        run: echo 'LABEL quay.expires-after=7d' >> Dockerfile

This mirrors lines 82, 93, and 104 and is immune to upstream Dockerfile formatting changes.

Comment thread .github/workflows/build-images-on-PR.yaml
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Feb 16, 2026

@tarilabs: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/trustyai-service-operator-e2e 1d86e01 link true /test trustyai-service-operator-e2e

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress needs-build Pull Request that need a new build of the TrustyAI Service Operator ok-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants