fix: e2e_tests docker compose version #490
Conversation
Signed-off-by: Haoyu Sun <hasun@redhat.com>
WalkthroughUpdated the GitHub Actions e2e workflow to use Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/e2e_tests.yaml (3)
4-4: High risk:pull_request_target+ job-level secrets exposesOPENAI_API_KEYto untrusted forks.Untrusted PR code runs with repo secrets via the job-level
env. Switch topull_requestor gate secrets to trusted contexts and move secrets to step-level.-on: [push, pull_request_target] +on: [push, pull_request] @@ - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + # Avoid exposing secrets to forked PRs. Provide secrets only in guarded steps: + # (add per-step) + # if: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.fork == false }} + # env: + # OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}If you must keep
pull_request_target, run untrusted code without secrets and kick off a separate, trusted workflow viaworkflow_runfor secret-requiring steps.Also applies to: 10-11
235-244: Use explicit-fchecks instead ofls | headwhen verifying Compose fileThe suggested
ls … | headsnippet can mis-parse due to pipe and!precedence, causing false negatives. Replace it with a straightforward file-existence test:- name: Verify Docker Compose file exists run: | if [ ! -f docker-compose.yml ] && [ ! -f docker-compose.yaml ] && [ ! -f compose.yml ] && [ ! -f compose.yaml ]; then echo "ERROR: no Docker Compose file found in repo root"; exit 1 fi docker compose version docker compose up -dApply the same check before the log-tail step (lines 253-256).
260-267: EnsureuvCLI is installed beforeuv sync. Theuvbinary is missing and nopyproject.tomlorrequirements.txtexists in this repo, souv syncwill always fail.
🧹 Nitpick comments (2)
.github/workflows/e2e_tests.yaml (2)
242-242: Add a compatibility fallback for environments with legacydocker-compose.Guard against runners that still have the v1 binary only.
- docker compose version + DOCKER_COMPOSE="docker compose" + $DOCKER_COMPOSE version >/dev/null 2>&1 || DOCKER_COMPOSE="docker-compose" + $DOCKER_COMPOSE version
245-249: Replace fixed sleep with a readiness probe.Reduces flakiness and speeds up happy paths.
- echo "Waiting for services to be healthy..." - sleep 20 # adjust depending on boot time + echo "Waiting for service to be ready (max 120s)..." + for i in {1..24}; do + if curl -fsS http://localhost:8080/healthz >/dev/null; then + echo "Service is ready." + break + fi + sleep 5 + doneIf no health endpoint, consider checking
docker compose psforhealthystatus.
📜 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)
242-242: Switch todocker compose versionis correct.Matches Compose v2 CLI; avoids the
--versionflag quirk.
| echo "OPENAI_API_KEY is set: $([ -n "$OPENAI_API_KEY" ] && echo 'YES' || echo 'NO')" | ||
| echo "OPENAI_API_KEY length: ${#OPENAI_API_KEY}" | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not print presence/length of OPENAI_API_KEY.
Even metadata leaks aid exfiltration attempts.
- echo "OPENAI_API_KEY is set: $([ -n "$OPENAI_API_KEY" ] && echo 'YES' || echo 'NO')"
- echo "OPENAI_API_KEY length: ${#OPENAI_API_KEY}"
+ echo "OPENAI_API_KEY present: [redacted]"Or remove entirely.
📝 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.
| echo "OPENAI_API_KEY is set: $([ -n "$OPENAI_API_KEY" ] && echo 'YES' || echo 'NO')" | |
| echo "OPENAI_API_KEY length: ${#OPENAI_API_KEY}" | |
| echo "OPENAI_API_KEY present: [redacted]" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 241-241: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/e2e_tests.yaml around lines 239 to 241, remove the two echo
lines that print the presence and length of OPENAI_API_KEY; do not output any
metadata about secret values. Delete those echo statements (or replace them with
a non-sensitive, generic log such as a static "OPENAI_API_KEY configured"
message that does not reveal presence, length, or any secret-derived info) to
prevent metadata leakage.
|
/retest |
2 participants
Description
fix the error calling
docker compose --versionin e2e test scripts.Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit