[fournos-launcher] Test v0.4.z#53
Conversation
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates runtime code to read Fournos workload namespace from ChangesNamespace Environment Variable Migration
CI Image Build & Push
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
|
/test fournos skeleton quick_test |
|
🔴 Test of 'fournos_launcher submit' failed after 00 hours 01 minutes 11 seconds 🔴 • Link to the test results. • No reports generated... Test configuration: • Failure indicator: Empty. |
|
/test fournos skeleton quick_test |
|
🟢 Test of 'skeleton test' succeeded after 00 hours 00 minutes 10 seconds 🟢 • Link to the test results. • Generated 2 Caliper report(s):
Test configuration: |
|
🟢 Test of 'fournos_launcher submit' succeeded after 00 hours 01 minutes 28 seconds 🟢 • Link to the test results. • No reports generated... Test configuration: |
|
merging this to get the forge image in quay.io |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/image-push.yaml (1)
13-13: ⚡ Quick winPin GitHub Actions to commit SHAs for supply-chain hardening.
Lines 13, 29, and 38 use floating major tags (
@v4,@v2). GitHub recommends pinning to immutable commit SHAs; tags can be moved or deleted and are not the only way to ensure an action is truly immutable.Suggested fix pattern
- - uses: actions/checkout@v4 + - uses: actions/checkout@<commit-sha> - uses: redhat-actions/buildah-build@v2 + uses: redhat-actions/buildah-build@<commit-sha> - uses: redhat-actions/push-to-registry@v2 + uses: redhat-actions/push-to-registry@<commit-sha>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/image-push.yaml at line 13, Replace floating action tags with immutable commit SHAs: locate the three "uses:" entries (e.g., "uses: actions/checkout@v4", "uses: actions/setup-node@v2", "uses: docker/build-push-action@v2") and update each tag to the corresponding full commit SHA from the action's upstream GitHub repo (replace `@vX` with @<commit-sha>). Fetch the appropriate SHA by opening the action repository and copying the commit hash for the release/tag you want to pin, then commit those SHA-pinned references so the workflow is immutably pinned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/image-push.yaml:
- Around line 10-11: The image-push job can race and overwrite the `latest` tag;
add GitHub Actions concurrency to the job to prevent stale pushes by defining a
concurrency group and enabling cancel-in-progress for the image-push job (use
the job name image-push as reference), e.g., set concurrency.group to a stable
identifier per branch or ref (such as github.ref or github.ref_name) and
concurrency.cancel-in-progress to true so an older run is cancelled when a newer
run for the same branch starts.
- Around line 22-24: The workflow currently appends
github.event.release.tag_name directly to TAGS which can include invalid
characters (e.g. '/') for OCI/Docker image tags; compute a sanitized tag (e.g.
CLEAN_TAG) from github.event.release.tag_name by replacing slashes with hyphens
and stripping any characters not in the allowed set [A-Za-z0-9_.-], then append
CLEAN_TAG to TAGS instead of the raw github.event.release.tag_name; update the
block that sets TAGS (the lines referencing TAGS and
github.event.release.tag_name) to use the sanitized variable.
---
Nitpick comments:
In @.github/workflows/image-push.yaml:
- Line 13: Replace floating action tags with immutable commit SHAs: locate the
three "uses:" entries (e.g., "uses: actions/checkout@v4", "uses:
actions/setup-node@v2", "uses: docker/build-push-action@v2") and update each tag
to the corresponding full commit SHA from the action's upstream GitHub repo
(replace `@vX` with @<commit-sha>). Fetch the appropriate SHA by opening the
action repository and copying the commit hash for the release/tag you want to
pin, then commit those SHA-pinned references so the workflow is immutably
pinned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c47d0ffb-1277-4742-9812-3aeb9e1a34a0
📒 Files selected for processing (3)
.github/workflows/image-push.yamlprojects/core/ci_entrypoint/fournos_resolve.pyprojects/core/library/export.py
🚧 Files skipped from review as they are similar to previous changes (2)
- projects/core/library/export.py
- projects/core/ci_entrypoint/fournos_resolve.py
| image-push: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Add concurrency control to prevent stale latest pushes.
On Line 10, runs are unconstrained; two close pushes to main can race and an older run may finish later and overwrite latest.
Suggested fix
name: Image Push
+concurrency:
+ group: image-push-${{ github.ref }}
+ cancel-in-progress: true
on:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image-push: | |
| runs-on: ubuntu-latest | |
| name: Image Push | |
| concurrency: | |
| group: image-push-${{ github.ref }} | |
| cancel-in-progress: true | |
| on: | |
| ... |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/image-push.yaml around lines 10 - 11, The image-push job
can race and overwrite the `latest` tag; add GitHub Actions concurrency to the
job to prevent stale pushes by defining a concurrency group and enabling
cancel-in-progress for the image-push job (use the job name image-push as
reference), e.g., set concurrency.group to a stable identifier per branch or ref
(such as github.ref or github.ref_name) and concurrency.cancel-in-progress to
true so an older run is cancelled when a newer run for the same branch starts.
| if [[ "${{ github.event_name }}" == "release" ]]; then | ||
| TAGS="stable ${TAGS} ${{ github.event.release.tag_name }}" | ||
| fi |
There was a problem hiding this comment.
Sanitize release tag before using it as an image tag.
On Line 23, ${{ github.event.release.tag_name }} is used directly; release tags may contain invalid container-tag chars (like /), causing build/push failures.
Suggested fix
- name: Determine tags
id: tags
run: |
TAGS="$(git rev-parse --short HEAD)"
if [[ "${{ github.event_name }}" == "push" ]]; then
TAGS="latest ${TAGS}"
fi
if [[ "${{ github.event_name }}" == "release" ]]; then
- TAGS="stable ${TAGS} ${{ github.event.release.tag_name }}"
+ RELEASE_TAG="${{ github.event.release.tag_name }}"
+ SAFE_RELEASE_TAG="$(echo "${RELEASE_TAG}" | tr '/:@ ' '-' | tr -cd '[:alnum:]_.-')"
+ TAGS="stable ${TAGS} ${SAFE_RELEASE_TAG}"
fi
echo "tags=${TAGS}" >> "$GITHUB_OUTPUT"What characters are allowed in OCI/Docker image tags, and is `/` allowed in a tag value?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/image-push.yaml around lines 22 - 24, The workflow
currently appends github.event.release.tag_name directly to TAGS which can
include invalid characters (e.g. '/') for OCI/Docker image tags; compute a
sanitized tag (e.g. CLEAN_TAG) from github.event.release.tag_name by replacing
slashes with hyphens and stripping any characters not in the allowed set
[A-Za-z0-9_.-], then append CLEAN_TAG to TAGS instead of the raw
github.event.release.tag_name; update the block that sets TAGS (the lines
referencing TAGS and github.event.release.tag_name) to use the sanitized
variable.
Summary by CodeRabbit