Add integrator-dist Dockerfiles for alpine, ubuntu, and rocky#411
Add integrator-dist Dockerfiles for alpine, ubuntu, and rocky#411chiranSachintha wants to merge 1 commit intowso2:masterfrom
Conversation
WalkthroughThree new Dockerfile and README pairs are added for WSO2 Integrator distribution across Alpine, Rocky, and Ubuntu base images. Each Dockerfile performs OS setup, downloads and verifies Temurin OpenJDK 21, extracts the Integrator distribution, configures Ballerina, and establishes a non-root user runtime environment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
dockerfiles/rocky/integrator-dist/Dockerfile (1)
14-31: Missing s390x architecture support compared to Ubuntu Dockerfile.Ubuntu Dockerfile supports
s390x|s390:64-bitarchitecture but Rocky does not. If s390x support is needed for Rocky, add the corresponding case. If intentionally omitted (e.g., Rocky doesn't support s390x), consider adding a comment explaining the difference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dockerfiles/rocky/integrator-dist/Dockerfile` around lines 14 - 31, The Rocky Dockerfile's architecture case for ${ARCH} is missing the s390x entry present in the Ubuntu Dockerfile; add a case branch for s390x (e.g., pattern like s390x|s390:64-bit) that sets ESUM and BINARY_URL to the same values used in the Ubuntu Dockerfile's s390x case (so the OpenJDK tarball and checksum match), or if s390x support is intentionally not provided for Rocky, add a clear comment above the case block explaining why s390x was omitted; update the case handling around the existing ESUM and BINARY_URL assignments accordingly.dockerfiles/rocky/integrator-dist/README.md (1)
1-1: Consider expanding documentation.The single-line README is acceptable for an initial addition, but consider adding build instructions, required build arguments (e.g.,
WSO2_SERVER_DIST_URL), and usage examples in a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dockerfiles/rocky/integrator-dist/README.md` at line 1, The README currently contains only a title; expand it to include concise build and usage documentation: add a "Build" section detailing required build arguments (at minimum document WSO2_SERVER_DIST_URL and any other Docker build-arg names used in the Dockerfile), example docker build command with the build-args, a "Run/Usage" section showing example docker run commands and exposed ports, and a "Notes" or "Troubleshooting" section for common pitfalls; reference the README's title line so the maintainer knows to update that file and ensure the documented build-arg names exactly match the ARG names used in the Dockerfile(s).dockerfiles/alpine/integrator-dist/Dockerfile (1)
53-53: Placeholder URL will cause build failure if not overridden.The default value
<INTEGRATOR_DIST_URL>will causewgetto fail with a confusing error if the build argument isn't provided. Consider adding validation or documenting the required build argument prominently.💡 Optional: Add validation before download
+RUN if [ "${WSO2_SERVER_DIST_URL}" = "<INTEGRATOR_DIST_URL>" ]; then \ + echo "Error: WSO2_SERVER_DIST_URL build argument must be provided"; \ + exit 1; \ + fi + RUN wget -O ${WSO2_SERVER}.zip "${WSO2_SERVER_DIST_URL}" \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dockerfiles/alpine/integrator-dist/Dockerfile` at line 53, The Dockerfile currently sets ARG WSO2_SERVER_DIST_URL=<INTEGRATOR_DIST_URL>, which will cause wget to fail if the build arg is not provided; update the Dockerfile to avoid a confusing failure by removing the placeholder default or setting it empty and add an early validation step that checks WSO2_SERVER_DIST_URL before attempting the download (e.g., a RUN that tests if the variable is non-empty and prints a clear error and exits if not), and ensure the wget/download step references the same ARG/ENV (WSO2_SERVER_DIST_URL) so the build fails fast with a clear message when the build-arg is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dockerfiles/rocky/integrator-dist/Dockerfile`:
- Line 1: The Dockerfile's base image tag "FROM rockylinux:9.5" is invalid and
causes builds to fail; update that FROM line to use a valid tag such as
"rockylinux:9.3" or the generic "rockylinux:9" so the image exists on Docker
Hub, i.e., replace the "FROM rockylinux:9.5" entry in the Dockerfile with one of
those valid tags.
- Around line 12-13: The Dockerfile's ARCH detection relies on objdump (see
ARCH="$(objdump="$(command -v objdump)" ...")") but binutils may not be
installed, causing a "command not found" failure; update the Dockerfile to
either install binutils (so objdump exists) in the package install step or
replace the objdump-based detection with a portable command like uname -m to set
ARCH, and ensure the change references the ARCH variable and the objdump
invocation so you modify the correct logic.
---
Nitpick comments:
In `@dockerfiles/alpine/integrator-dist/Dockerfile`:
- Line 53: The Dockerfile currently sets ARG
WSO2_SERVER_DIST_URL=<INTEGRATOR_DIST_URL>, which will cause wget to fail if the
build arg is not provided; update the Dockerfile to avoid a confusing failure by
removing the placeholder default or setting it empty and add an early validation
step that checks WSO2_SERVER_DIST_URL before attempting the download (e.g., a
RUN that tests if the variable is non-empty and prints a clear error and exits
if not), and ensure the wget/download step references the same ARG/ENV
(WSO2_SERVER_DIST_URL) so the build fails fast with a clear message when the
build-arg is missing.
In `@dockerfiles/rocky/integrator-dist/Dockerfile`:
- Around line 14-31: The Rocky Dockerfile's architecture case for ${ARCH} is
missing the s390x entry present in the Ubuntu Dockerfile; add a case branch for
s390x (e.g., pattern like s390x|s390:64-bit) that sets ESUM and BINARY_URL to
the same values used in the Ubuntu Dockerfile's s390x case (so the OpenJDK
tarball and checksum match), or if s390x support is intentionally not provided
for Rocky, add a clear comment above the case block explaining why s390x was
omitted; update the case handling around the existing ESUM and BINARY_URL
assignments accordingly.
In `@dockerfiles/rocky/integrator-dist/README.md`:
- Line 1: The README currently contains only a title; expand it to include
concise build and usage documentation: add a "Build" section detailing required
build arguments (at minimum document WSO2_SERVER_DIST_URL and any other Docker
build-arg names used in the Dockerfile), example docker build command with the
build-args, a "Run/Usage" section showing example docker run commands and
exposed ports, and a "Notes" or "Troubleshooting" section for common pitfalls;
reference the README's title line so the maintainer knows to update that file
and ensure the documented build-arg names exactly match the ARG names used in
the Dockerfile(s).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49a37aac-701b-441a-b8e2-5416db237ceb
📒 Files selected for processing (6)
dockerfiles/alpine/integrator-dist/Dockerfiledockerfiles/alpine/integrator-dist/README.mddockerfiles/rocky/integrator-dist/Dockerfiledockerfiles/rocky/integrator-dist/README.mddockerfiles/ubuntu/integrator-dist/Dockerfiledockerfiles/ubuntu/integrator-dist/README.md
| @@ -0,0 +1,69 @@ | |||
| FROM rockylinux:9.5 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check available Rocky Linux tags on Docker Hub
curl -s "https://hub.docker.com/v2/repositories/library/rockylinux/tags?page_size=20" | jq -r '.results[].name' | head -20Repository: wso2/docker-ei
Length of output: 281
Update base image to an available Rocky Linux version; rockylinux:9.5 does not exist.
The Dockerfile references rockylinux:9.5, but this tag is not available on Docker Hub. The highest patch version available for Rocky 9 is 9.3. Update to either rockylinux:9.3 or the generic rockylinux:9 tag. This will cause the build to fail immediately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dockerfiles/rocky/integrator-dist/Dockerfile` at line 1, The Dockerfile's
base image tag "FROM rockylinux:9.5" is invalid and causes builds to fail;
update that FROM line to use a valid tag such as "rockylinux:9.3" or the generic
"rockylinux:9" so the image exists on Docker Hub, i.e., replace the "FROM
rockylinux:9.5" entry in the Dockerfile with one of those valid tags.
| RUN set -eux; \ | ||
| ARCH="$(objdump="$(command -v objdump)" && objdump --file-headers "$objdump" | awk -F '[:,]+[[:space:]]+' '$1 == "architecture" { print $2 }')"; \ |
There was a problem hiding this comment.
Architecture detection depends on objdump which may not be installed.
The objdump command requires binutils which is not installed in line 6. This will cause the build to fail with "command not found".
🐛 Proposed fix: Add binutils to installed packages
RUN dnf update -y \
- && dnf install -y ca-certificates curl glibc-langpack-en unzip wget \
+ && dnf install -y binutils ca-certificates curl glibc-langpack-en unzip wget \
&& dnf clean allAlternatively, consider using uname -m for architecture detection, which is more portable:
RUN set -eux; \
- ARCH="$(objdump="$(command -v objdump)" && objdump --file-headers "$objdump" | awk -F '[:,]+[[:space:]]+' '$1 == "architecture" { print $2 }')"; \
+ ARCH="$(uname -m)"; \
case "${ARCH}" in \
- aarch64|arm64) \
+ aarch64) \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dockerfiles/rocky/integrator-dist/Dockerfile` around lines 12 - 13, The
Dockerfile's ARCH detection relies on objdump (see ARCH="$(objdump="$(command -v
objdump)" ...")") but binutils may not be installed, causing a "command not
found" failure; update the Dockerfile to either install binutils (so objdump
exists) in the package install step or replace the objdump-based detection with
a portable command like uname -m to set ARCH, and ensure the change references
the ARCH variable and the objdump invocation so you modify the correct logic.
Purpose
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit