Skip to content

feat(dev): add devcontainers setup #1585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+37 −0
Open

feat(dev): add devcontainers setup #1585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
FROM php:7.4-cli

# Install system dependencies
RUN apt-get update && apt-get install -y \
zip \
unzip \
git \
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing system dependencies that are present in the main project Dockerfile: wget and openssh-client (see lines 7-8 in Dockerfile). These are likely needed for the CLI's functionality, such as SSH access to environments and downloading resources. Add them to the apt-get install command on lines 4-8.

Suggested change
git \
git \
wget \
openssh-client \

Copilot uses AI. Check for mistakes.
&& rm -rf /var/lib/apt/lists/*

Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The devcontainer Dockerfile is missing the pcntl PHP extension that is installed in the main project Dockerfile (see line 11 in Dockerfile). This extension is likely required for the CLI to function properly. Add RUN docker-php-ext-install -j$(nproc) pcntl after installing system dependencies.

Suggested change
# Install pcntl extension (required for CLI)
RUN docker-php-ext-install -j$(nproc) pcntl

Copilot uses AI. Check for mistakes.
# Install Composer
RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" \
&& php composer-setup.php \
&& mv composer.phar /usr/local/bin/composer \
&& php -r "unlink('composer-setup.php');" \
&& chmod +x /usr/local/bin/composer
Comment on lines +10 to +15
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Composer installation lacks signature verification, which is a security concern. The existing Dockerfile in the repository uses a more secure installation method that verifies the installer signature. Consider using the same approach from docker/install_composer.sh or at minimum verify the installer signature before running it.

Suggested change
# Install Composer
RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" \
&& php composer-setup.php \
&& mv composer.phar /usr/local/bin/composer \
&& php -r "unlink('composer-setup.php');" \
&& chmod +x /usr/local/bin/composer
# Install Composer (with signature verification)
RUN set -eux; \
EXPECTED_SIGNATURE="$(php -r 'copy("https://composer.github.io/installer.sig", "php://stdout");')" ; \
php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" ; \
ACTUAL_SIGNATURE="$(php -r "echo hash_file('SHA384', 'composer-setup.php');")" ; \
if [ "$EXPECTED_SIGNATURE" != "$ACTUAL_SIGNATURE" ]; then \
echo 'ERROR: Invalid installer signature' >&2; \
rm -f composer-setup.php; \
exit 1; \
fi; \
php composer-setup.php; \
mv composer.phar /usr/local/bin/composer; \
php -r "unlink('composer-setup.php');"; \
chmod +x /usr/local/bin/composer

Copilot uses AI. Check for mistakes.

# Create a non-root user
ARG USERNAME=upsun
ARG USER_UID=1000
ARG USER_GID=$USER_UID

RUN groupadd --gid $USER_GID $USERNAME \
&& useradd --uid $USER_UID --gid $USER_GID -m $USERNAME \
&& apt-get update \
&& apt-get install -y sudo \
Comment on lines +4 to +25
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apt-get update is called twice unnecessarily (line 4 and line 24), which is inefficient and slows down image builds. Consider combining all apt-get installations into a single RUN command or removing the cleanup on line 8 and doing it after the final apt-get install on line 25.

Copilot uses AI. Check for mistakes.
&& echo $USERNAME ALL=\(root\) NOPASSWD:ALL > /etc/sudoers.d/$USERNAME \
&& chmod 0440 /etc/sudoers.d/$USERNAME

USER $USERNAME
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Dockerfile is missing a WORKDIR directive. The main project Dockerfile sets WORKDIR /home/psh/legacy-cli (line 23 in Dockerfile), which defines where the code will be mounted and where commands will run. Consider adding a similar WORKDIR for the devcontainer, e.g., WORKDIR /home/upsun/workspace or an appropriate path.

Suggested change
USER $USERNAME
USER $USERNAME
# Set the working directory
WORKDIR /home/upsun/workspace
# Ensure the workspace directory exists
RUN mkdir -p /home/upsun/workspace

Copilot uses AI. Check for mistakes.
8 changes: 8 additions & 0 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "Upsun CLI",
Copy link
Collaborator

@pjcdawkins pjcdawkins Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think let's figure this out when we merge to upsun/cli and update to PHP 8.4 or 8.5

"build": {
"dockerfile": "Dockerfile"
},
"remoteUser": "upsun",
"postCreateCommand": "composer install"
}
Loading