Fix MongoDB mongosh installation for Docker builds#462
Fix MongoDB mongosh installation for Docker builds#462KavindaRajapaksha wants to merge 4 commits intoLDFLK:mainfrom
Conversation
Summary of ChangesHello @KavindaRajapaksha, 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 critical Docker build failures stemming from Debian's updated security policy, which now rejects SHA1 signatures in GPG keys. The changes primarily involve adjusting how MongoDB's Highlights
Changelog
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 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 counter productive. You can react with 👍 and 👎 on @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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses Docker build failures by changing the installation method for mongosh and improves build reliability by adding retry logic for package installations. The changes are generally positive, but the new mongosh installation method hardcodes the amd64 architecture, which breaks the previously supported multi-architecture builds (e.g., for arm64). I've added comments with suggestions to fix this by dynamically detecting the architecture.
opengin/core-api/docker/Dockerfile
Outdated
| wget https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | ||
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | ||
| rm mongodb-mongosh_2.1.1_amd64.deb && \ |
There was a problem hiding this comment.
The installation of mongosh now hardcodes the amd64 architecture. This is a regression from the previous approach which supported both amd64 and arm64, and will cause build failures on arm64 platforms. To maintain multi-architecture support, you should determine the architecture dynamically.
DEB_FILE="mongodb-mongosh_2.1.1_$(dpkg --print-architecture).deb" && \
wget "https://downloads.mongodb.com/compass/${DEB_FILE}" && \
(dpkg -i "${DEB_FILE}" || apt-get -f install -y) && rm "${DEB_FILE}" && \
| wget https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | ||
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | ||
| rm mongodb-mongosh_2.1.1_amd64.deb && \ |
There was a problem hiding this comment.
The installation of mongosh now hardcodes the amd64 architecture. This will cause build failures on arm64 platforms, which is explicitly mentioned as a target platform in this Dockerfile's comments. To maintain multi-architecture support, you should determine the architecture dynamically.
DEB_FILE="mongodb-mongosh_2.1.1_$(dpkg --print-architecture).deb" && \
wget "https://downloads.mongodb.com/compass/${DEB_FILE}" && \
(dpkg -i "${DEB_FILE}" || apt-get -f install -y) && rm "${DEB_FILE}" && \
There was a problem hiding this comment.
Pull request overview
This pull request addresses Docker build failures by changing how MongoDB mongosh is installed in Debian-based images and adding retry logic to Alpine package manager commands. The PR tackles issues related to Debian's rejection of SHA1 signatures in GPG keys by switching to direct .deb package downloads for mongosh. Additionally, it improves build reliability by adding retry logic to Alpine package installations and modernizes docker-compose files by removing the obsolete version field.
Changes:
- MongoDB mongosh installation changed from repository-based to direct .deb download to avoid SHA1 GPG key issues
- Added retry logic with 3 attempts and 5-second delays to Alpine package manager (apk) commands across multiple Dockerfiles
- Startup scripts now copied from builder stage with line ending fixes applied
- Removed obsolete
versionfield from docker-compose files
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| opengin/core-api/docker/Dockerfile | Changed mongosh installation to download .deb directly; updated startup script copy from builder stage with line ending fixes |
| opengin/core-api/docker/Dockerfile.choreo | Changed mongosh installation to download .deb directly; updated startup script copy from builder stage with line ending fixes |
| opengin/read-api/docker/Dockerfile | Added retry logic to apk package installation commands |
| opengin/read-api/docker/Dockerfile.choreo | Added retry logic to apk package installation commands in both builder and final stages |
| opengin/ingestion-api/docker/Dockerfile | Added retry logic to apk package installation commands |
| opengin/ingestion-api/docker/Dockerfile.choreo | Added retry logic to apk package installation commands in both builder and final stages |
| docker-compose.yml | Removed obsolete version field |
| docker-compose.override.yml | Removed obsolete version field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RUN for i in 1 2 3; do \ | ||
| apk add --no-cache ca-certificates netcat-openbsd dos2unix shadow \ | ||
| && break || sleep 5; \ | ||
| done && \ |
There was a problem hiding this comment.
The retry loop doesn't fail the build if all three attempts are unsuccessful. If the apk command fails after all retries, the RUN command will still succeed because the loop itself exits with code 0. Consider adding error handling after the loop to ensure the build fails if the package installation never succeeds, or add a final check like '|| exit 1' after the done statement.
opengin/read-api/docker/Dockerfile
Outdated
| RUN for i in 1 2 3; do \ | ||
| apk update && apk add --no-cache \ | ||
| curl gnupg wget net-tools nano \ | ||
| unzip netcat-openbsd iputils bind-tools \ | ||
| dos2unix \ | ||
| && break || sleep 5; \ | ||
| done \ |
There was a problem hiding this comment.
The PR description mentions fixing MongoDB mongosh installation due to Debian's SHA1 GPG key rejection, but this file uses Alpine Linux (apk package manager), not Debian. The retry logic being added here appears to be addressing transient network or repository failures, which is unrelated to the SHA1 GPG key issue described in the PR. Consider clarifying the PR description to mention both the mongosh GPG issue fix AND the addition of retry logic for improved build reliability.
| RUN for i in 1 2 3; do \ | ||
| apk update && apk add --no-cache \ | ||
| curl gnupg wget net-tools nano \ | ||
| unzip netcat-openbsd iputils bind-tools \ | ||
| dos2unix \ | ||
| && break || sleep 5; \ | ||
| done \ | ||
| && rm -rf /var/cache/apk/* |
There was a problem hiding this comment.
The retry loop doesn't fail the build if all three attempts are unsuccessful. If the apk command fails after all retries, the RUN command will still succeed because the loop itself exits with code 0. Consider adding error handling after the loop to ensure the build fails if the package installation never succeeds, or add a final check like '|| exit 1' after the done statement.
| RUN for i in 1 2 3; do \ | |
| apk update && apk add --no-cache \ | |
| curl gnupg wget net-tools nano \ | |
| unzip netcat-openbsd iputils bind-tools \ | |
| dos2unix \ | |
| && break || sleep 5; \ | |
| done \ | |
| && rm -rf /var/cache/apk/* | |
| RUN success=0; \ | |
| for i in 1 2 3; do \ | |
| if apk update && apk add --no-cache \ | |
| curl gnupg wget net-tools nano \ | |
| unzip netcat-openbsd iputils bind-tools \ | |
| dos2unix; then \ | |
| success=1; \ | |
| break; \ | |
| else \ | |
| sleep 5; \ | |
| fi; \ | |
| done; \ | |
| if [ "$success" -ne 1 ]; then \ | |
| echo "ERROR: Failed to install system packages with apk after 3 attempts." >&2; \ | |
| exit 1; \ | |
| fi; \ | |
| rm -rf /var/cache/apk/* |
| RUN for i in 1 2 3; do \ | ||
| apk add --no-cache shadow \ | ||
| && break || sleep 5; \ | ||
| done && \ |
There was a problem hiding this comment.
The retry loop doesn't fail the build if all three attempts are unsuccessful. If the apk command fails after all retries, the RUN command will still succeed because the loop itself exits with code 0. Consider adding error handling after the loop to ensure the build fails if the package installation never succeeds, or add a final check like '|| exit 1' after the done statement.
opengin/core-api/docker/Dockerfile
Outdated
| apt-get update && \ | ||
| apt-get install -y mongodb-mongosh && \ | ||
| # Download and install mongosh directly (avoid repository GPG key SHA1 issue) | ||
| wget https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ |
There was a problem hiding this comment.
MongoDB shell (mongosh) version 2.1.1 is hardcoded in the download URL. Consider using a variable or environment variable for the version to make it easier to update in the future. Additionally, verify that version 2.1.1 is the correct version to address the SHA1 GPG key issue mentioned in the PR description.
| apk update && apk add --no-cache \ | ||
| curl gnupg wget net-tools nano \ | ||
| unzip netcat-openbsd iputils bind-tools \ | ||
| dos2unix \ | ||
| && break || sleep 5; \ |
There was a problem hiding this comment.
The retry loop uses 'sleep 5' which will execute even on the first failure. Consider whether this delay is necessary for package repository failures. Additionally, the loop doesn't provide any logging about retries, which could make debugging build failures more difficult. Consider adding echo statements to indicate when retries are happening.
| apk update && apk add --no-cache \ | |
| curl gnupg wget net-tools nano \ | |
| unzip netcat-openbsd iputils bind-tools \ | |
| dos2unix \ | |
| && break || sleep 5; \ | |
| echo "Attempt ${i}: Installing system packages with apk..."; \ | |
| if apk update && apk add --no-cache \ | |
| curl gnupg wget net-tools nano \ | |
| unzip netcat-openbsd iputils bind-tools \ | |
| dos2unix; then \ | |
| echo "System packages installed successfully on attempt ${i}."; \ | |
| break; \ | |
| else \ | |
| if [ "${i}" -lt 3 ]; then \ | |
| echo "Attempt ${i} failed. Retrying in 5 seconds..."; \ | |
| sleep 5; \ | |
| else \ | |
| echo "Package installation failed after ${i} attempts. Exiting."; \ | |
| exit 1; \ | |
| fi; \ | |
| fi; \ |
| # Copy the startup script from builder stage | ||
| COPY --from=builder /app/opengin/core-api/docker/core_startup.sh /app/start.sh |
There was a problem hiding this comment.
The startup script is now being copied from the builder stage and has line ending conversion applied. The sed command to fix Windows line endings (CRLF to LF) is a good addition. However, verify that the script exists in the builder stage at the expected path. According to the Dockerfile comments, the build context is the repo root, so the script should be at /app/opengin/core-api/docker/core_startup.sh in the builder stage.
| # Copy the startup script from builder stage | |
| COPY --from=builder /app/opengin/core-api/docker/core_startup.sh /app/start.sh | |
| # Copy the startup script directly from the build context | |
| COPY opengin/core-api/docker/core_startup.sh /app/start.sh |
opengin/core-api/docker/Dockerfile
Outdated
| wget https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | ||
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | ||
| rm mongodb-mongosh_2.1.1_amd64.deb && \ | ||
| # Download and install Cypher Shell | ||
| wget https://dist.neo4j.org/cypher-shell/cypher-shell-5.26.5.zip && \ |
There was a problem hiding this comment.
The wget command doesn't include error handling or retry logic. If the download fails due to network issues, the build will fail. Consider adding wget options like --tries=3 --timeout=10 to make the download more reliable, or wrap the entire mongosh installation block in a retry loop similar to the pattern used for apk commands.
| wget https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | |
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | |
| rm mongodb-mongosh_2.1.1_amd64.deb && \ | |
| # Download and install Cypher Shell | |
| wget https://dist.neo4j.org/cypher-shell/cypher-shell-5.26.5.zip && \ | |
| wget --tries=3 --timeout=10 --waitretry=5 --retry-connrefused https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | |
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | |
| rm mongodb-mongosh_2.1.1_amd64.deb && \ | |
| # Download and install Cypher Shell | |
| wget --tries=3 --timeout=10 --waitretry=5 --retry-connrefused https://dist.neo4j.org/cypher-shell/cypher-shell-5.26.5.zip && \ |
| wget https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | ||
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | ||
| rm mongodb-mongosh_2.1.1_amd64.deb && \ | ||
| # Download and install Cypher Shell | ||
| wget https://dist.neo4j.org/cypher-shell/cypher-shell-5.26.5.zip && \ |
There was a problem hiding this comment.
The wget command doesn't include error handling or retry logic. If the download fails due to network issues, the build will fail. Consider adding wget options like --tries=3 --timeout=10 to make the download more reliable, or wrap the entire mongosh installation block in a retry loop similar to the pattern used for apk commands.
| wget https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | |
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | |
| rm mongodb-mongosh_2.1.1_amd64.deb && \ | |
| # Download and install Cypher Shell | |
| wget https://dist.neo4j.org/cypher-shell/cypher-shell-5.26.5.zip && \ | |
| wget --tries=3 --timeout=10 https://downloads.mongodb.com/compass/mongodb-mongosh_2.1.1_amd64.deb && \ | |
| dpkg -i mongodb-mongosh_2.1.1_amd64.deb || apt-get install -f -y && \ | |
| rm mongodb-mongosh_2.1.1_amd64.deb && \ | |
| # Download and install Cypher Shell | |
| wget --tries=3 --timeout=10 https://dist.neo4j.org/cypher-shell/cypher-shell-5.26.5.zip && \ |
| RUN for i in 1 2 3; do \ | ||
| apk add --no-cache ca-certificates netcat-openbsd dos2unix \ | ||
| && break || sleep 5; \ | ||
| done |
There was a problem hiding this comment.
The retry loop doesn't fail the build if all three attempts are unsuccessful. If the apk command fails after all retries, the RUN command will still succeed because the loop itself exits with code 0. Consider adding error handling after the loop to ensure the build fails if the package installation never succeeds, or add a final check like '|| exit 1' after the done statement.
|
Hi @KavindaRajapaksha, thanks for looking into this! We've been seeing the same Docker build issues locally. We have been testing a slightly more minimal fix that achieves the same result by pinning the base images to -bookworm to ensure consistent dependency resolution, which also allows for a cleaner OpenSSL upgrade path. It only touches these 2 files rather than the wider scope. Could you take a look at this patch and see if it works for your use case? If it does, maybe we can simplify the PR to just these changes! |
|
@sehansi-9 I added those changes to docker files now all are working perfectly .Please check them and need any modification let me know.Thank you! |
@KavindaRajapaksha, thank you for the quick update! We’ve reviewed the changes. We really appreciate the extra effort you put into the retry loops and the architecture specific logic.those are great improvements for the long term. However to keep this PR focused and ensure a quick merge, let's include only the core fixes for now (pinning to -bookworm and the simplified OpenSSL upgrade). Could you please revert the additional retry loops and the manual downloading logic, and keep just the patch for the base images and OpenSSL? We’d definitely encourage you to submit the other optimizations (like the retry logic) as a separate PR afterward so we can review them properly on their own. Thank you for understanding and for all your help on this! |
|
@sehansi-9 I revert those changes Please checkout them |
Fixes Docker build failures caused by Debian's February 2026 security policy update that rejects SHA1 signatures in GPG keys.