Update Dockerfile#921
Conversation
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
WalkthroughDockerfile refactors: ENV entries consolidated; package install/cleanup simplified; rosa/regctl/hub downloads switched to streamed curl/tar or direct curl into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Dockerfile (1)
61-61: Remove unnecessary verbose flag from curl commands.The
-vflag on curl outputs verbose logging to stderr. Since the RUN layer already usesset -x(line 60), debug output is enabled at the shell level, making curl's-vredundant and potentially noisy in build logs.Apply this diff to simplify:
- && curl -vL https://mirror.openshift.com/pub/openshift-v4/clients/rosa/latest/rosa-linux.tar.gz | tar -C $BIN_DIR -xzvf - rosa \ + && curl -L https://mirror.openshift.com/pub/openshift-v4/clients/rosa/latest/rosa-linux.tar.gz | tar -C $BIN_DIR -xzvf - rosa \ && chmod +x $BIN_DIR/rosa \ - && curl -vL https://github.com/regclient/regclient/releases/latest/download/regctl-linux-amd64 -o $BIN_DIR/regctl \ + && curl -L https://github.com/regclient/regclient/releases/latest/download/regctl-linux-amd64 -o $BIN_DIR/regctl \ && chmod +x $BIN_DIR/regctl \ - && curl -vL https://github.com/mislav/hub/releases/download/v2.14.2/hub-linux-amd64-2.14.2.tgz | tar --strip-components=2 -C $BIN_DIR -xzvf - '*/bin/hub' \ + && curl -L https://github.com/mislav/hub/releases/download/v2.14.2/hub-linux-amd64-2.14.2.tgz | tar --strip-components=2 -C $BIN_DIR -xzvf - '*/bin/hub' \ && chmod +x $BIN_DIR/hubAlso applies to: 63-63, 65-65
rabin-io
force-pushed
the
update-dockerfile
branch
from
e5d725e to
6080310
Compare
rabin-io
force-pushed
the
update-dockerfile
branch
from
6080310 to
1a8fb92
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Dockerfile (2)
61-67: Add error handling to curl and tar commands (set -e, --fail).The RUN layer uses
set -x(debugging) but notset -e(exit on error). If curl fails to download or tar fails to extract, the build layer may succeed with partial or missing binaries. Additionally, curl lacks the--failflag, which causes curl to silently pass HTTP errors (4xx/5xx) to tar or the output file.Update to use
set -exand add--failto all curl invocations:-RUN set -x \ +RUN set -ex \ && curl -vL https://mirror.openshift.com/pub/openshift-v4/clients/rosa/latest/rosa-linux.tar.gz | tar -C $BIN_DIR -xzvf - rosa \ && chmod +x $BIN_DIR/rosa \ - && curl -vL https://github.com/regclient/regclient/releases/latest/download/regctl-linux-amd64 >$BIN_DIR/regctl \ + && curl --fail -vL https://github.com/regclient/regclient/releases/latest/download/regctl-linux-amd64 -o $BIN_DIR/regctl \ && chmod +x $BIN_DIR/regctl \ - && curl -vL https://github.com/mislav/hub/releases/download/v2.14.2/hub-linux-amd64-2.14.2.tgz | tar --strip-components=2 -C $BIN_DIR -xzvf - '*/bin/hub' \ + && curl --fail -vL https://github.com/mislav/hub/releases/download/v2.14.2/hub-linux-amd64-2.14.2.tgz | tar --strip-components=2 -C $BIN_DIR -xzvf - '*/bin/hub' \ && chmod +x $BIN_DIR/hubKey improvements:
set -ex: Exits immediately on any error and echoes commands for debugging.--failon all curl commands: Causes curl to exit with non-zero status on HTTP 4xx/5xx errors.-oinstead of>for regctl: Ensures curl detects and reports download failures before writing to the file.
61-67: Clarify versioning strategy for reproducibility.The three tools use inconsistent versioning: rosa and regctl both use
latest, while hub is pinned tov2.14.2. This makes builds non-deterministic—rosa and regctl may pull different versions on subsequent builds while hub remains fixed.Choose a consistent strategy:
Option A: Pin all versions for full reproducibility
RUN set -ex \ && curl --fail -vL https://mirror.openshift.com/pub/openshift-v4/clients/rosa/v1.X.X/rosa-linux.tar.gz | tar -C $BIN_DIR -xzvf - rosa \ && chmod +x $BIN_DIR/rosa \ && curl --fail -vL https://github.com/regclient/regclient/releases/download/vX.X.X/regctl-linux-amd64 -o $BIN_DIR/regctl \ && chmod +x $BIN_DIR/regctl \ && curl --fail -vL https://github.com/mislav/hub/releases/download/v2.14.2/hub-linux-amd64-2.14.2.tgz | tar --strip-components=2 -C $BIN_DIR -xzvf - '*/bin/hub' \ && chmod +x $BIN_DIR/hubOption B: Use "latest" for all, with documented maintenance policy
Add a clear comment explaining the rationale and commit to periodic reviews/updates.
rabin-io
force-pushed
the
update-dockerfile
branch
from
1a8fb92 to
bf3b594
Compare
- Extract bin files directly from curl stream (no temp file needed) - Run curl with -vL Signed-off-by: Rabin Yasharzadehe <rabin@rabin.io>
rabin-io
force-pushed
the
update-dockerfile
branch
from
bf3b594 to
3b6100a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Dockerfile (1)
61-67: Versioning strategy remains inconsistent.Rosa and Regctl downloads use
latest, while Hub uses a pinned version (v2.14.2). This mix creates non-deterministic builds and makes version tracking difficult. This issue was raised in the previous review and remains unaddressed.Choose one strategy:
- Option A (Recommended): Pin all versions explicitly
- Option B: Document the "latest" approach with explicit maintenance cadence
🧹 Nitpick comments (1)
Dockerfile (1)
58-58: Non-deterministic uv image tag undermines build reproducibility.The COPY instruction uses
ghcr.io/astral-sh/uv:latest, which can pull different versions on subsequent builds. This conflicts with the goal of reproducible builds, especially when tools like rosa and hub have versioning considerations.Consider pinning to a specific version:
-COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx ${BIN_DIR}/ +COPY --from=ghcr.io/astral-sh/uv:0.5.0 /uv /uvx ${BIN_DIR}/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile(3 hunks)
🔇 Additional comments (5)
Dockerfile (5)
5-10: ENV variable ordering fixed correctly.The reordering now ensures
$USERNAMEis defined before$HOME_DIRand$BIN_DIR, resolving the earlier critical issue. Environment variable expansion should now work as intended.
14-34: Clean package installation consolidation.The consolidation of dnf commands with consistent flags (
--nodocs,install_weak_deps=False,--disable-repo) and extended cleanup (line 34) helps reduce image size and improves consistency. The cleanup targets appear appropriate for DNF/YUM artifact removal.
52-56: UV environment variables consolidated effectively.Grouping UV-related variables in a single ENV block improves readability and reduces layer count.
61-67: Curl error handling properly implemented.The use of
set -ex(line 61),--failon all curl commands (lines 62, 64, 66), and explicit-ofor the regctl download ensure proper error propagation. This addresses the previous critical issue about missing error handling in download operations.
73-75: New operational improvements appreciated.The addition of HEALTHCHECK (line 73) and explicit ENTRYPOINT (line 75) improve container observability and runtime behavior, making the image more production-ready.
|
/verified |
thanks to #921 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Python Version Requirements** * Updated Python version requirement to 3.13.x * **Chores** * Optimized Docker build configuration with consolidated environment settings * Enhanced package installation with improved dependency management * Improved binary download process with strengthened error handling * Extended development tooling support <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
3 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.