Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Dockerfile base image for the Dropbox container from the end-of-life Debian Buster to Debian Bookworm. The change is necessary because Debian Buster has reached end-of-life and its APT repositories are no longer available, causing apt update and apt install commands to fail.
Changes:
- Updated the base Docker image from
debian:bustertodebian:bookwormin the Dropbox Dockerfile
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`debian:buster` LTS has reached an end. `apt update` and `apt install` are failing because `apt` repositories are no longer available. This commit updates to an ubuntu image Signed-off-by: Andreia Ocanoaia <andreia.ocanoaia@gmail.com>
Fix warning related to obsolete/deprecated Dockerfile syntax. Signed-off-by: Andreia Ocanoaia <andreia.ocanoaia@gmail.com>
420d0e3 to
e6a4b3b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| ca-certificates \ | ||
| wget \ | ||
| libc6 \ | ||
| libstdc++6 \ | ||
| gosu \ | ||
| tzdata \ | ||
| python3 \ | ||
| && rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
Several important packages have been removed from the installation without clear justification: curl (replaced with wget), gnupg2 (for GPG verification), apt-transport-https (for secure apt operations), software-properties-common (for repository management), and python3-gpg. While wget can replace curl for basic downloads, the removal of gnupg2 and python3-gpg eliminates the ability to verify package signatures. If signature verification or secure repository access is needed later, these packages would need to be added back. Consider documenting why these packages are no longer needed or adding them back if they may be required.
| # Download and extract Dropbox daemon | ||
| RUN wget -O - "https://www.dropbox.com/download?plat=lnx.x86_64" | tar xzf - |
There was a problem hiding this comment.
The previous Dockerfile included protections against Dropbox's automatic self-updates by creating an immutable .dropbox-dist directory. These protections have been completely removed, which means Dropbox will now be able to update itself automatically. This can lead to: (1) Unpredictable behavior as the running version drifts from the built version, (2) Accumulation of large update files in /opt/dropbox/ and /tmp over time, potentially filling up the filesystem, (3) Non-reproducible builds as different container instances may run different Dropbox versions. Consider re-implementing the update prevention mechanism to ensure consistent behavior.
|
|
||
| # Build arguments | ||
| ARG VCS_REF=master | ||
| ARG VCS_REF=main |
There was a problem hiding this comment.
The VCS_REF default value has been changed from "master" to "main", which is unrelated to updating the base image. While this may be a positive change to align with modern Git conventions, it's not mentioned in the PR description and could cause issues if the repository's default branch is still named "master". Verify that this change is intentional and that the repository's default branch is actually "main".
| ARG VCS_REF=main | |
| ARG VCS_REF=master |
|
|
||
| # Create user and group | ||
| RUN mkdir -p /opt/dropbox /opt/dropbox/.dropbox /opt/dropbox/Dropbox \ | ||
| RUN mkdir -p /opt/dropbox \ |
There was a problem hiding this comment.
The directory creation has been simplified to only create /opt/dropbox, removing the creation of /opt/dropbox/.dropbox and /opt/dropbox/Dropbox subdirectories. However, these directories are declared as VOLUME mount points on line 55. If these directories don't exist when Docker tries to create the volumes, Docker will create them with root ownership, which could cause permission issues since the dropbox user needs access to them. Consider adding back the creation of these subdirectories or verifying that volume mounting handles this correctly.
| RUN mkdir -p /opt/dropbox \ | |
| RUN mkdir -p /opt/dropbox /opt/dropbox/.dropbox /opt/dropbox/Dropbox \ |
| # Based on Ubuntu | ||
| FROM ubuntu:22.04 |
There was a problem hiding this comment.
The PR title and description mention updating to "Debian Bookworm", but the code changes the base image from debian:buster to ubuntu:22.04 instead. Ubuntu is not Debian Bookworm (which would be debian:bookworm). This is a significant discrepancy between the PR description and the actual changes. Please update either the PR description to accurately reflect that this is changing to Ubuntu 22.04, or change the base image to debian:bookworm if that was the original intent.
| libstdc++6 \ | ||
| gosu \ | ||
| tzdata \ | ||
| python3 \ |
There was a problem hiding this comment.
The previous installation included several X11-related libraries (libglapi-mesa, libxdamage1, libxfixes3, libxcb-glx0, libxcb-dri2-0, libxcb-dri3-0, libxcb-present0, libxcb-sync1, libxshmfence1, libxxf86vm1) that are no longer being installed. These libraries may be required by the Dropbox client for GUI functionality. Removing them could cause runtime failures if Dropbox attempts to display notifications or use GUI features. Verify that the Dropbox daemon can function properly without these libraries in a headless environment.
| python3 \ | |
| python3 \ | |
| libglapi-mesa \ | |
| libxdamage1 \ | |
| libxfixes3 \ | |
| libxcb-glx0 \ | |
| libxcb-dri2-0 \ | |
| libxcb-dri3-0 \ | |
| libxcb-present0 \ | |
| libxcb-sync1 \ | |
| libxshmfence1 \ | |
| libxxf86vm1 \ |
| ENV LANG="C.UTF-8" | ||
| ENV LC_ALL="C.UTF-8" |
There was a problem hiding this comment.
The locale configuration has been simplified to use "C.UTF-8" instead of "en_US.UTF-8", and the locale generation steps have been removed. While C.UTF-8 is a minimal locale that should work for basic functionality, this change could affect date/time formatting, number formatting, and other locale-dependent behavior. If Dropbox or any scripts rely on specific locale formatting, this could cause issues. Additionally, the removal of locales and locales-all packages means no other locales can be generated if needed later.
|
|
||
| # Change working directory | ||
| WORKDIR /opt/dropbox/Dropbox | ||
| WORKDIR /opt/dropbox |
There was a problem hiding this comment.
The working directory has been changed from /opt/dropbox/Dropbox to /opt/dropbox. This changes where the container starts up and could affect any scripts or entrypoints that rely on the working directory. If there are scripts that assume they're running from the Dropbox sync folder (/opt/dropbox/Dropbox), they may fail or behave unexpectedly. Verify that this change doesn't break any existing workflows or entrypoint scripts.
| && chmod o-w /tmp \ | ||
| && chmod g-w /tmp | ||
| # Download and extract Dropbox daemon | ||
| RUN wget -O - "https://www.dropbox.com/download?plat=lnx.x86_64" | tar xzf - |
There was a problem hiding this comment.
Downloading and extracting the Dropbox daemon with wget -O - "https://www.dropbox.com/download?plat=lnx.x86_64" | tar xzf - introduces a supply-chain risk because unsigned remote code is fetched and unpacked as root during the image build. If an attacker compromises the download endpoint or can perform a TLS man-in-the-middle attack, they can replace the archive and gain arbitrary code execution inside containers built from this image. Prefer installing Dropbox via a channel that enforces signature verification, or verify the downloaded archive against a pinned checksum or signature before extracting it.
debian:busterLTS has reached an end.apt updateandapt installare failing becauseaptrepositories are no longer available - https://www.debian.org/releases/buster/.