ci: MinIO + ROCKETRIDE_TEST_S3_* for S3Store integration tests (Linux)#1035
ci: MinIO + ROCKETRIDE_TEST_S3_* for S3Store integration tests (Linux)#1035kwit75 wants to merge 1 commit into
Conversation
Wires a local MinIO into the Linux build's Test step so the S3Store integration tests (gated on boto3 + ROCKETRIDE_TEST_S3_*) run in CI. Ubuntu-only (service containers aren't available on win/macOS), and continue-on-error + late var-export keep a MinIO hiccup from ever blocking a CI/release build (tests skip cleanly). Preinstalls boto3 (an optional extra) so the suite actually activates. Tests themselves land via Stepan's feat/deploy-v2 (commit 027bb64).
📝 WalkthroughWalkthroughThe workflow now provides local S3 testing infrastructure by launching MinIO on Ubuntu runners, verifying service health, creating a test bucket with smoke tests, and exporting S3 credentials. The pytest invocation is updated to preinstall boto3, enabling live S3 integration tests that use the MinIO endpoint. ChangesS3 Integration Testing Infrastructure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
No description provided. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/_build.yaml:
- Around line 221-225: The workflow currently passes an unpinned package via the
run command that includes --pytest-preinstall="boto3", which allows varying
boto3/botocore pairs to be installed; update that invocation (the run: ${{
matrix.builder_cmd }} test --verbose -s --pytest-preinstall="boto3") to pin
boto3 to a known-good version or range (for example boto3==1.26.XX or a
compatible range) so CI installs a reproducible boto3/botocore pair; replace the
unpinned "boto3" token with the chosen pinned spec (e.g. "boto3==<version>" or
"boto3>=<min>,<max>") in that run line.
- Around line 159-162: The workflow currently uses continue-on-error for the
MinIO/bootstrap steps while only exporting ROCKETRIDE_TEST_S3_* environment vars
on success, which lets S3 tests silently skip; change the behavior so the job
fails when S3 integration is required: remove continue-on-error from the
MinIO/bootstrap steps (the steps that set up MinIO and create the bucket), or
guard them behind a clear feature flag (e.g., ENFORCE_S3_INTEGRATION) and make
that flag default to false but set true for branches/environments where S3 must
be enforced (so when ENFORCE_S3_INTEGRATION==true the step must not use
continue-on-error); ensure the exporting of ROCKETRIDE_TEST_S3_* is performed
only after a successful MinIO step and that failure in those steps causes the
job to fail when enforcement is enabled (apply the same change to the other
affected blocks referenced around lines 168 and 189-196).
- Around line 183-188: Add a readback verification after uploading
/tmp/minio-smoke.txt: use aws --endpoint-url http://localhost:9000 s3 cp
s3://rocketride-test/smoke.txt /tmp/minio-smoke-out.txt (or aws s3api
get-object) and compare its contents to the original (e.g., diff
/tmp/minio-smoke.txt /tmp/minio-smoke-out.txt or test "$(cat
/tmp/minio-smoke-out.txt)" = "minio smoke"); if the comparison fails, print an
error and exit non‑zero so MinIO read-path regressions fail the workflow.
- Around line 163-165: The workflow currently starts MinIO using the mutable
image tag "minio/minio:latest" in the docker run invocation; replace that with
an immutable pinned image (either a specific digest like
"minio/minio@sha256:..." or a project-controlled mirror tag) so CI is
reproducible and immune to upstream changes, and keep or update the existing
comment about avoiding stale RELEASE tags to explain the choice; ensure the
docker run command and any corresponding environment/health-check steps
reference the new pinned image string.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 17d9a331-01c7-437c-a6fb-8f4ce846a7bc
📒 Files selected for processing (1)
.github/workflows/_build.yaml
| # continue-on-error + writing the test vars to $GITHUB_ENV ONLY after | ||
| # MinIO is confirmed healthy and the bucket exists: if anything here | ||
| # fails the vars stay unset → the S3 tests skip → a MinIO hiccup can | ||
| # never block a CI or release build. |
There was a problem hiding this comment.
Don't leave the S3 integration lane fail-open.
Line 168's continue-on-error combined with only exporting ROCKETRIDE_TEST_S3_* on success means any MinIO/bootstrap regression turns this into a green build with the live S3 suite silently skipped. That defeats the coverage once the packages/ai/tests/ai/account/test_store.py cases are expected to run on develop. Consider making the soft-fail behavior explicitly temporary, then fail the job whenever the S3 lane is supposed to be enforced.
Also applies to: 168-168, 189-196
🤖 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/_build.yaml around lines 159 - 162, The workflow currently
uses continue-on-error for the MinIO/bootstrap steps while only exporting
ROCKETRIDE_TEST_S3_* environment vars on success, which lets S3 tests silently
skip; change the behavior so the job fails when S3 integration is required:
remove continue-on-error from the MinIO/bootstrap steps (the steps that set up
MinIO and create the bucket), or guard them behind a clear feature flag (e.g.,
ENFORCE_S3_INTEGRATION) and make that flag default to false but set true for
branches/environments where S3 must be enforced (so when
ENFORCE_S3_INTEGRATION==true the step must not use continue-on-error); ensure
the exporting of ROCKETRIDE_TEST_S3_* is performed only after a successful MinIO
step and that failure in those steps causes the job to fail when enforcement is
enabled (apply the same change to the other affected blocks referenced around
lines 168 and 189-196).
| # (minio:latest is intentional — a stale pinned RELEASE tag would 404 | ||
| # and silently skip the suite; revisit pinning once we mirror the image.) | ||
| # ----------------------------------------------------------------------- |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
echo "---- locating file ----"
find . -path '*_build.yaml' -o -path './.github/workflows/_build.yaml' | sed -n '1,20p'
echo "---- show relevant lines ----"
python3 - <<'PY'
import itertools,sys,os
path=".github/workflows/_build.yaml"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i in range(140,210):
if i-1 < len(lines):
print(f"{i:4d}:{lines[i-1].rstrip()}")
PYRepository: rocketride-org/rocketride-server
Length of output: 7915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- show relevant lines from .github/workflows/_build.yaml ----"
python3 - <<'PY'
path=".github/workflows/_build.yaml"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
start, end = 140, 210
for i in range(start, end+1):
if 1 <= i <= len(lines):
print(f"{i:4d}:{lines[i-1].rstrip()}")
PYRepository: rocketride-org/rocketride-server
Length of output: 4674
Pin the MinIO image to an immutable digest (not minio/minio:latest).
.github/workflows/_build.yaml starts MinIO with docker run ... minio/minio:latest ... (lines ~175-177), so upstream changes can make CI non-reproducible and alter test behavior. Switch to a pinned digest or a mirrored immutable image tag (keeping the existing “avoid 404s” intent).
🤖 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/_build.yaml around lines 163 - 165, The workflow currently
starts MinIO using the mutable image tag "minio/minio:latest" in the docker run
invocation; replace that with an immutable pinned image (either a specific
digest like "minio/minio@sha256:..." or a project-controlled mirror tag) so CI
is reproducible and immune to upstream changes, and keep or update the existing
comment about avoiding stale RELEASE tags to explain the choice; ensure the
docker run command and any corresponding environment/health-check steps
reference the new pinned image string.
| # Create the bucket + smoke a put/get so the backend is proven even | ||
| # before the S3Store tests land on develop (they live on feat/deploy-v2). | ||
| aws --endpoint-url http://localhost:9000 s3 mb s3://rocketride-test | ||
| printf 'minio smoke\n' > /tmp/minio-smoke.txt | ||
| aws --endpoint-url http://localhost:9000 s3 cp /tmp/minio-smoke.txt s3://rocketride-test/smoke.txt | ||
| aws --endpoint-url http://localhost:9000 s3 ls s3://rocketride-test/ |
There was a problem hiding this comment.
The smoke test never validates object reads.
This step proves bucket creation, upload, and listing, but the downstream S3Store path also does get_object in packages/ai/tests/ai/account/test_store.py:339-366. Add a readback here so MinIO read-path regressions fail before pytest.
Suggested readback check
aws --endpoint-url http://localhost:9000 s3 mb s3://rocketride-test
printf 'minio smoke\n' > /tmp/minio-smoke.txt
aws --endpoint-url http://localhost:9000 s3 cp /tmp/minio-smoke.txt s3://rocketride-test/smoke.txt
aws --endpoint-url http://localhost:9000 s3 ls s3://rocketride-test/
+ aws --endpoint-url http://localhost:9000 s3 cp s3://rocketride-test/smoke.txt /tmp/minio-smoke-read.txt
+ cmp /tmp/minio-smoke.txt /tmp/minio-smoke-read.txt📝 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.
| # Create the bucket + smoke a put/get so the backend is proven even | |
| # before the S3Store tests land on develop (they live on feat/deploy-v2). | |
| aws --endpoint-url http://localhost:9000 s3 mb s3://rocketride-test | |
| printf 'minio smoke\n' > /tmp/minio-smoke.txt | |
| aws --endpoint-url http://localhost:9000 s3 cp /tmp/minio-smoke.txt s3://rocketride-test/smoke.txt | |
| aws --endpoint-url http://localhost:9000 s3 ls s3://rocketride-test/ | |
| # Create the bucket + smoke a put/get so the backend is proven even | |
| # before the S3Store tests land on develop (they live on feat/deploy-v2). | |
| aws --endpoint-url http://localhost:9000 s3 mb s3://rocketride-test | |
| printf 'minio smoke\n' > /tmp/minio-smoke.txt | |
| aws --endpoint-url http://localhost:9000 s3 cp /tmp/minio-smoke.txt s3://rocketride-test/smoke.txt | |
| aws --endpoint-url http://localhost:9000 s3 ls s3://rocketride-test/ | |
| aws --endpoint-url http://localhost:9000 s3 cp s3://rocketride-test/smoke.txt /tmp/minio-smoke-read.txt | |
| cmp /tmp/minio-smoke.txt /tmp/minio-smoke-read.txt |
🤖 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/_build.yaml around lines 183 - 188, Add a readback
verification after uploading /tmp/minio-smoke.txt: use aws --endpoint-url
http://localhost:9000 s3 cp s3://rocketride-test/smoke.txt
/tmp/minio-smoke-out.txt (or aws s3api get-object) and compare its contents to
the original (e.g., diff /tmp/minio-smoke.txt /tmp/minio-smoke-out.txt or test
"$(cat /tmp/minio-smoke-out.txt)" = "minio smoke"); if the comparison fails,
print an error and exit non‑zero so MinIO read-path regressions fail the
workflow.
| # --pytest-preinstall=boto3: boto3 is an optional extra (commented out in | ||
| # account/requirements.txt), so without this the S3Store tests' importorskip | ||
| # would skip the live-S3 cases even with MinIO up. Harmless on win/macOS | ||
| # (those skip anyway — no ROCKETRIDE_TEST_S3_* set there). | ||
| run: ${{ matrix.builder_cmd }} test --verbose -s --pytest-preinstall="boto3" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Pin the boto3 version used by CI.
--pytest-preinstall="boto3" resolves whatever latest boto3/botocore pair exists on the day the workflow runs, so this test lane can start failing without any repo change. Use a known-good version or range to keep the S3 integration tests reproducible.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 225-225: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/_build.yaml around lines 221 - 225, The workflow currently
passes an unpinned package via the run command that includes
--pytest-preinstall="boto3", which allows varying boto3/botocore pairs to be
installed; update that invocation (the run: ${{ matrix.builder_cmd }} test
--verbose -s --pytest-preinstall="boto3") to pin boto3 to a known-good version
or range (for example boto3==1.26.XX or a compatible range) so CI installs a
reproducible boto3/botocore pair; replace the unpinned "boto3" token with the
chosen pinned spec (e.g. "boto3==<version>" or "boto3>=<min>,<max>") in that run
line.
|
minio setup change included into #1044 |
2 participants
What
Wires a local MinIO into the Linux leg of
_build.yaml's Test step so the S3Store integration tests run in CI.The tests (
packages/ai/tests/ai/account/test_store.py) gate the live-S3 cases onpytest.importorskip('boto3')+ROCKETRIDE_TEST_S3_ACCESS_KEY_ID, so they skip unless a real S3 endpoint is provided. This:minioadmin/minioadmin) on:9000, waits for health, creates bucketrocketride-test, and smoke-tests a put/get.ROCKETRIDE_TEST_S3_*(endpoint/region/keys/bucket) — the exact contracttest_store.pyreads.--pytest-preinstall=boto3so the optionalboto3extra (commented out inaccount/requirements.txt) is present and the suite activates.Safety
matrix.platform == 'ubuntu') — service containers/docker aren't on the win/macOS runners; tests skip cleanly there.continue-on-error+ late var-export: theROCKETRIDE_TEST_S3_*vars are written to$GITHUB_ENVonly after MinIO is healthy and the bucket exists. If MinIO ever fails to come up, the vars stay unset → tests skip → a MinIO hiccup can never block a CI or release build (_build.yamlis shared with the release path).Sequencing
The S3Store tests +
s3.pyendpoint support live on @stepmikhaylov'sfeat/deploy-v2(commit027bb64), not yet on develop. This PR is the CI infra only; once those tests reach develop they light up on this MinIO. The smoke step proves the backend works in this PR's CI in the meantime.cc @stepmikhaylov
Summary by CodeRabbit