[do not merge] test trigger #496
Conversation
Signed-off-by: Haoyu Sun <hasun@redhat.com>
WalkthroughUpdates the .github/workflows/e2e_tests.yaml so that the file-action generates lightspeed-stack.yaml with name: "foo bar baz 1" instead of "foo bar baz". No other fields, workflow steps, environment, or control flow are changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/e2e_tests.yaml (4)
4-4: High risk: pull_request_target runs untrusted PR code with repo secrets.This workflow checks out and executes the PR head under pull_request_target while exposing secrets (see OPENAI_API_KEY below). This is a known exfiltration vector. Use pull_request for untrusted PRs.
Apply this diff:
-on: [push, pull_request_target] +on: [push, pull_request]
9-11: Don’t expose OPENAI_API_KEY at job scope or to third‑party actions.
- Job-level env exposes the secret to every step (including debug and list/cat).
- Passing secrets to 1arp/create-a-file-action is unnecessary and increases supply-chain risk.
Restrict the secret to only the step that actually needs it, and gate that step to trusted contexts.
Apply this diff:
jobs: e2e_tests: runs-on: ubuntu-latest - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} @@ - - uses: 1arp/create-a-file-action@0.4.5 - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + - uses: 1arp/create-a-file-action@0.4.5 with: @@ - - uses: 1arp/create-a-file-action@0.4.5 - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + - uses: 1arp/create-a-file-action@0.4.5 with: @@ - - name: Run service manually - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + - name: Run service manually + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false) }} + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}Also applies to: 61-63, 95-97
6-8: Tighten default token permissions.Minimize the GITHUB_TOKEN scope to reduce blast radius.
Apply this diff:
jobs: e2e_tests: runs-on: ubuntu-latest + permissions: + contents: read
234-244: Gate heavy/secret-dependent steps for forks.Avoid running docker/compose and tests on forked PRs that won’t have secrets anyway.
Apply this diff:
- - name: Run service manually + - name: Run service manually + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false) }} @@ - - name: Run e2e tests + - name: Run e2e tests + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false) }}Also applies to: 259-267
🧹 Nitpick comments (2)
.github/workflows/e2e_tests.yaml (2)
26-46: Debug-only step looks repo/branch-specific; gate or remove.Prevents noisy logs in unrelated PRs.
Apply this diff:
- - name: Debug checkout for umago/lightspeed-stack setup-metrics branch - run: | + - name: Debug checkout (internal only) + if: ${{ github.repository == 'umago/lightspeed-stack' }} + run: | echo "=== GitHub Event Information ===" ...
245-249: Stabilize readiness and connectivity checks.Replace fixed sleep and add curl retries/timeouts to reduce flakes.
Apply this diff:
- - name: Wait for services - run: | - echo "Waiting for services to be healthy..." - sleep 20 # adjust depending on boot time + - name: Wait for services + run: | + echo "Waiting for services to be healthy..." + for i in {1..30}; do + if curl -sf http://localhost:8080/v1/models >/dev/null; then + echo "Service is up"; exit 0 + fi + sleep 2 + done + echo "Service did not become ready in time"; exit 1 @@ - - name: Quick connectivity test + - name: Quick connectivity test run: | echo "Testing basic connectivity before full test suite..." - curl -f http://localhost:8080/v1/models || { + curl -f --retry 5 --retry-connrefused --max-time 10 http://localhost:8080/v1/models || { echo "❌ Basic connectivity failed - showing logs before running full tests" docker compose logs --tail=30 exit 1 }Also applies to: 251-257
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/e2e_tests.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
🔇 Additional comments (1)
.github/workflows/e2e_tests.yaml (1)
68-68: Change to generated config name is fine.No functional impact beyond the literal content. Safe.
1 participant
do not merge
Summary by CodeRabbit
Chores
Tests