-
-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor #25
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.
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
base: main
Are you sure you want to change the base?
Conversation
…metrics combination step
…tforms fixed for toml
…erence in Docker image workflow
…stallation, and enhance security with non-root user
…n for Debian and Alpine
…r Debian bullseye
…g generation for bookworm images
…error reporting, opcache, and session management
…it version dependant
… tips for Docker images
…s and correct PHP version variable usage
…s for S6 overlay, while removing unused fix-attrs.d directory
…ssions management
…and add build arguments
I have been informed that these changes are too big..... and I shouldn't merge this., ....but since no one else is here I am typing this comment to the void.... :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modernizes the PHP container setup by refactoring service scripts to use /bin/sh
, implementing a multi-stage Docker build with BuildKit caching, consolidating PHP extension installs, integrating the S6 overlay init system, and adding a reusable GitHub Action for multi-platform Docker builds.
- Switch service scripts to
sh
and add init/finish hooks for PHP modes. - Refactor Dockerfile into builder/production stages, use cache mounts, group extension installs, and integrate S6 overlay.
- Add a composite GitHub Action to set up QEMU and Buildx for multi-arch builds.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
s6-overlay/services.d/php/run | Switch to /bin/sh , detect PHP_TYPE , and start PHP in modes. |
s6-overlay/services.d/php/finish | Add finish script to log exit code and shut down container on errors. |
s6-overlay/fix-attrs.d/01-php-dirs | Define directory permissions for PHP runtime files. |
s6-overlay/cont-init.d/10-php-config | Generate PHP .ini settings from environment variables. |
Dockerfile | Refactor to multi-stage build, use BuildKit cache, group installs, and add S6 overlay. |
.github/docker-setup/action.yml | Composite Action to configure QEMU and Buildx for multi-architecture builds. |
# For CLI, we just execute a long-running command or wait | ||
# This is typically handled by the CMD instruction and not S6 | ||
echo "Running in CLI mode - service management handled by CMD" | ||
sleep infinity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sleep infinity
command isn't POSIX and may not be supported by /bin/sh
(e.g., BusyBox). Consider using tail -f /dev/null
or sleep 2147483647
for compatibility.
sleep infinity | |
tail -f /dev/null |
Copilot uses AI. Check for mistakes.
rm -rf /var/lib/apt/lists/*; \ | ||
elif [ "$BASEOS" = "bookworm" ]; then \ | ||
# Install dependencies based on the base OS with BuildKit cache mounts | ||
RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Support for Debian Bullseye was removed in this refactor; the if [ "$BASEOS" = "bookworm" ]
branch no longer handles bullseye
. If you still need Bullseye, add a dedicated condition.
Copilot uses AI. Check for mistakes.
@@ -55,8 +72,165 @@ RUN case $(uname -m) in \ | |||
IPE_GD_WITHOUTAVIF=1 install-php-extensions gd; \ | |||
;; \ | |||
*) \ | |||
install-php-extensions gd; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The gd
extension is installed in both the architecture-specific and default branches. You can simplify this by moving install-php-extensions gd
outside the case
.
Copilot uses AI. Check for mistakes.
tar -C / -Jxpf s6-overlay-noarch.tar.xz && \ | ||
tar -C / -Jxpf s6-overlay-${S6_ARCH}.tar.xz && \ | ||
rm s6-overlay-noarch.tar.xz s6-overlay-${S6_ARCH}.tar.xz; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Downloading S6 overlay without checksum verification can lead to supply-chain risks. Consider verifying the archive integrity (e.g., using sha256sum) before extraction.
tar -C / -Jxpf s6-overlay-noarch.tar.xz && \ | |
tar -C / -Jxpf s6-overlay-${S6_ARCH}.tar.xz && \ | |
rm s6-overlay-noarch.tar.xz s6-overlay-${S6_ARCH}.tar.xz; \ | |
wget -q https://github.com/just-containers/s6-overlay/releases/download/v${S6_OVERLAY_VERSION}/s6-overlay-noarch.tar.xz.sha256 && \ | |
wget -q https://github.com/just-containers/s6-overlay/releases/download/v${S6_OVERLAY_VERSION}/s6-overlay-${S6_ARCH}.tar.xz.sha256 && \ | |
echo "$(cat s6-overlay-noarch.tar.xz.sha256)" | sha256sum -c - && \ | |
echo "$(cat s6-overlay-${S6_ARCH}.tar.xz.sha256)" | sha256sum -c - && \ | |
tar -C / -Jxpf s6-overlay-noarch.tar.xz && \ | |
tar -C / -Jxpf s6-overlay-${S6_ARCH}.tar.xz && \ | |
rm s6-overlay-noarch.tar.xz s6-overlay-${S6_ARCH}.tar.xz s6-overlay-noarch.tar.xz.sha256 s6-overlay-${S6_ARCH}.tar.xz.sha256; \ |
Copilot uses AI. Check for mistakes.
cleanup and modernize