Repository navigation
fix(e2e-sandbox): use curl -fsSL when downloading mc - #2690
Conversation
The minio client was downloaded without -L, so when dl.min.io served
a 302 redirect, curl wrote the HTML body ("<a href=...>Found</a>") to
/usr/local/bin/mc, chmod +x'd it, and shipped that as the binary. When
the bucket E2E test ran "mc alias set ...", the shell tried to interpret
the HTML as a script and errored with:
/usr/local/bin/mc: 1: cannot open a: No such file
(dash reading "<a" as an input redirect from a file named "a").
The image is rebuilt per CI run from cached layers; the broken mc layer
was cached and reused. Adding -fsSL fixes the download and the textual
change busts the cache so the next build pulls a real ELF binary.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Signed-off-by: Myasnikov Daniil <[email protected]>
github-actions
Bot
added
area/uncategorized
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a failure in the e2e-sandbox image build process caused by an improperly configured curl command. By adding flags to follow redirects and fail on server errors, the change ensures that the correct ELF binary is downloaded instead of an HTML redirect response, thereby preventing runtime errors during E2E testing. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughThe e2e-sandbox Dockerfile's MinIO client installation is updated from a basic curl invocation to use fail-fast flags ( ChangesMinIO Client Installation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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 docstrings
🧪 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 |
dosubot
Bot
added
area/build
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the e2e-sandbox Dockerfile to improve the reliability of the MinIO client download by adding standard curl flags and fixing minor formatting. Feedback indicates that the PR does not comply with repository standards: the PR body is missing the required release-note block, and the PR title uses an invalid scope that does not match the project's defined scopes.
| RUN curl -sSL "https://github.com/cozystack/cozyhr/raw/refs/heads/main/hack/install.sh" | sh -s -- -v "${COZYHR_VERSION}" | ||
| RUN curl https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc --create-dirs -o /usr/local/bin/mc \ | ||
| && chmod +x /usr/local/bin/mc | ||
| RUN curl -fsSL "https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc" --create-dirs -o /usr/local/bin/mc \ |
There was a problem hiding this comment.
The PR body is missing the required release-note block. According to the repository style guide, every pull request must include a release note in the following format:
```release-note
type(scope): human-readable changelog entry
<details>
<summary>References</summary>
1. PR body must contain a release note block. <sup>([link](https://github.com/cozystack/cozystack/blob/main/.gemini/styleguide.md))</sup>
</details>
There was a problem hiding this comment.
Added a release-note block to the PR description.
| RUN curl -sSL "https://github.com/cozystack/cozyhr/raw/refs/heads/main/hack/install.sh" | sh -s -- -v "${COZYHR_VERSION}" | ||
| RUN curl https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc --create-dirs -o /usr/local/bin/mc \ | ||
| && chmod +x /usr/local/bin/mc | ||
| RUN curl -fsSL "https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc" --create-dirs -o /usr/local/bin/mc \ |
There was a problem hiding this comment.
The PR title uses an invalid scope e2e-sandbox. According to the style guide, the scope should match a package directory under packages/ (e.g., testing) or be one of the predefined meta scopes (e.g., tests).
References
- Valid scopes must follow the Conventional Commits format and match the project's defined scopes. (link)
There was a problem hiding this comment.
e2e-sandbox matches the package directory packages/core/testing/images/e2e-sandbox/ and is the scope the earlier commit on this branch (cc37fec) used. docs/agents/contributing.md lists scopes as examples that can be extended for a specific area, so keeping it as is.
There was a problem hiding this comment.
Code Review
This pull request modifies the Dockerfile for the e2e-sandbox image, specifically updating the curl command for the MinIO client to include the -fsSL flags and proper quoting. The review feedback identifies a missing release-note block in the pull request description, which is a requirement of the repository style guide, and recommends pinning the MinIO client to a specific version to improve build reproducibility.
| RUN curl -sSL "https://github.com/cozystack/cozyhr/raw/refs/heads/main/hack/install.sh" | sh -s -- -v "${COZYHR_VERSION}" | ||
| RUN curl https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc --create-dirs -o /usr/local/bin/mc \ | ||
| && chmod +x /usr/local/bin/mc | ||
| RUN curl -fsSL "https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc" --create-dirs -o /usr/local/bin/mc \ |
There was a problem hiding this comment.
The pull request body is missing the required release-note block. According to the repository style guide, every PR must include a block in the following format:
```release-note
type(scope): human-readable changelog entry
Please add this block to the PR description to satisfy the project requirements.
<details>
<summary>References</summary>
1. PR body must contain a release note block in a specific format. Flag any PR whose body has no release-note block. <sup>([link](https://github.com/cozystack/cozystack/blob/main/.gemini/styleguide.md))</sup>
</details>
There was a problem hiding this comment.
Added the release-note block to the PR description.
| RUN curl -sSL "https://github.com/cozystack/cozyhr/raw/refs/heads/main/hack/install.sh" | sh -s -- -v "${COZYHR_VERSION}" | ||
| RUN curl https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc --create-dirs -o /usr/local/bin/mc \ | ||
| && chmod +x /usr/local/bin/mc | ||
| RUN curl -fsSL "https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc" --create-dirs -o /usr/local/bin/mc \ |
There was a problem hiding this comment.
To ensure build reproducibility and avoid non-deterministic behavior, consider pinning the mc binary to a specific version rather than downloading the latest release from the release/ path. Other tools in this Dockerfile (such as kubectl, talosctl, and helm) already follow this pattern using versioned ARGs.
There was a problem hiding this comment.
Leaving the version pin out of scope for this redirect fix. The Dockerfile already mixes pinning styles — kubectl/talosctl/helm use ARGs, yq is hardcoded in the URL, and the fluxcd.io and cozyhr install scripts pull whatever those installers resolve. Adding a single ARG for mc doesn't meaningfully improve reproducibility on its own; happy to do a follow-up that pins everything consistently if there's appetite.
There was a problem hiding this comment.
Code Review
This pull request updates the curl command used to download the MinIO client in the e2e-sandbox Dockerfile, adding standard flags and improving formatting. Feedback was provided regarding compliance with the repository's style guide, specifically the requirement for a release-note block in the PR description and the use of the correct commit scope. Additionally, it was suggested to use a specific version for the mc binary to ensure build reproducibility.
| RUN curl -sSL "https://github.com/cozystack/cozyhr/raw/refs/heads/main/hack/install.sh" | sh -s -- -v "${COZYHR_VERSION}" | ||
| RUN curl https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc --create-dirs -o /usr/local/bin/mc \ | ||
| && chmod +x /usr/local/bin/mc | ||
| RUN curl -fsSL "https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc" --create-dirs -o /usr/local/bin/mc \ |
There was a problem hiding this comment.
The pull request body is missing the required release-note block. Please add it to the description as per the repository style guide. Additionally, the commit scope e2e-sandbox should ideally be testing to match the package directory name under packages/core/, as specified in rule 45.
References
- PR body must contain a release note block and commit scopes should match package names. (link)
There was a problem hiding this comment.
Added the release-note block to the PR description. On the scope: e2e-sandbox matches the package directory packages/core/testing/images/e2e-sandbox/ and docs/agents/contributing.md lists scopes as examples that can be extended for a specific area, so keeping it as e2e-sandbox.
| RUN curl -sSL "https://github.com/cozystack/cozyhr/raw/refs/heads/main/hack/install.sh" | sh -s -- -v "${COZYHR_VERSION}" | ||
| RUN curl https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc --create-dirs -o /usr/local/bin/mc \ | ||
| && chmod +x /usr/local/bin/mc | ||
| RUN curl -fsSL "https://dl.min.io/client/mc/release/${TARGETOS}-${TARGETARCH}/mc" --create-dirs -o /usr/local/bin/mc \ |
There was a problem hiding this comment.
While updating this line, consider using a specific version for the mc binary (e.g., via an ARG) instead of always pulling the latest release. This would improve build reproducibility and maintain consistency with other tools in this Dockerfile (like kubectl or talosctl) which are explicitly versioned.
There was a problem hiding this comment.
See the reply on the parallel thread — leaving the version pin out of scope for this redirect fix. The Dockerfile already mixes pinned and unpinned downloads, so adding a single ARG for mc doesn't move the reproducibility needle on its own.
2 participants
Summary
RUN curl … mcinpackages/core/testing/images/e2e-sandbox/Dockerfileran without-L, so whendl.min.ioreturned a 302, the HTML redirect body (<a href=...>Found</a>) was written to/usr/local/bin/mcand chmod'd executable.mc alias set …, the shell tried to interpret the HTML as a script and errored with/usr/local/bin/mc: 1: cannot open a: No such file(dash reading<aas an input redirect).curl -fsSLfollows the redirect and fails fast on HTTP errors. The textual change to theRUNinstruction also busts the cached layer so the next build pulls a real ELF binary.Test plan
e2e-sandboximage with the new layerbucketbats test runsmc alias setwithout thecannot open aerrorfile /usr/local/bin/mcinside the rebuilt image reports an ELF binary, not ASCII textRelease note
Summary by CodeRabbit