-
-
Notifications
You must be signed in to change notification settings - Fork 253
feat: Add runtime plugin dependencies and composer binary to Dockerfile #2034
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
feat: Add runtime plugin dependencies and composer binary to Dockerfile #2034
Conversation
📝 WalkthroughWalkthroughFinal runtime image now includes Node/Yarn and plugin runtime tooling and copies the Composer binary from the composer build stage; base and dev images enable PHP bz2 support and install Git in build stages. Dockerfile and Dockerfile.dev differ slightly in which runtime packages (bash present in dev only). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin as Admin UI
participant App as Panel (PHP app)
participant Container as Runtime container
participant Composer as Composer binary
participant Yarn as Yarn binary
participant Registry as Package Registry
Note over Container: Runtime image now includes Composer, Node/Yarn, and archivers (p7zip, bz2 dev)
Admin->>App: "Install Plugin" request
App->>Container: spawn install process (shell)
alt Plugin requires PHP deps
Container->>Composer: run composer require / install
Composer->>Registry: fetch PHP packages
Registry-->>Composer: return packages
Composer-->>App: install result
end
alt Plugin requires JS deps
Container->>Yarn: run yarn install
Yarn->>Registry: fetch JS packages
Registry-->>Yarn: return packages
Yarn-->>App: install result
end
App-->>Admin: report success / errors
Pre-merge checks✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-12-24T09:21:58.780ZApplied to files:
📚 Learning: 2025-12-24T09:22:18.560ZApplied to files:
🔇 Additional comments (2)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile.dev (1)
65-71: Critical: Runtime plugin dependencies missing from final stage OR production Dockerfile not included in review.The PR objectives (issues #2024, #2025) require nodejs, npm, yarn, and composer in the production image to fix plugin installation failures. However, the final stage (lines 65-115) only installs
caddy ca-certificates supervisor supercronic fcgi coreutilson lines 70-71.The AI summary states: "Final image Dockerfile now installs nodejs, npm, yarn, bash and copies the Composer binary into /usr/local/bin" — but these changes are not visible in either Dockerfile.dev or Dockerfile.base provided for review.
Action required:
- If there's a separate production
Dockerfile(not.devor.base), it must be included in this review to verify the runtime dependencies are properly added.- If this
Dockerfile.devis intended for production use, the final stage needs to install nodejs, npm, yarn, and copy the composer binary to allow plugins to install their dependencies at runtime.Expected additions to final stage
The final stage should include something like:
# Install additional required libraries RUN apk add --no-cache \ - caddy ca-certificates supervisor supercronic fcgi coreutils + caddy ca-certificates supervisor supercronic fcgi coreutils \ + nodejs npm yarn bash + +# Copy composer binary for runtime plugin dependency installation +COPY --from=composer:latest /usr/bin/composer /usr/local/bin/composer
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile.baseDockerfile.dev
🔇 Additional comments (2)
Dockerfile.base (1)
8-8: LGTM! PHP bz2 extension added.The addition of the bz2 PHP extension is correct and aligns with the commit message noting this dependency may be required.
Dockerfile.dev (1)
8-8: LGTM! PHP bz2 extension added.The addition of the bz2 PHP extension is correct and consistent with Dockerfile.base.
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.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile.dev (1)
67-74: CRITICAL: Composer binary is not copied to the final runtime image.The PR objectives explicitly state that composer must be added to the runtime image to fix issue #2025 ("composer: not found"). While nodejs, npm, and yarn are correctly added at line 74, the composer binary from the
composerbuild stage (line 21) is never copied to the final stage.Without this, composer-based plugin dependencies will still fail at runtime, leaving issue #2025 unresolved.
🔎 Proposed fix to copy composer binary to final image
Add this line after line 77 to copy the composer binary to the runtime image:
COPY --chown=root:www-data --chmod=640 --from=composerbuild /build . COPY --chown=root:www-data --chmod=640 --from=yarnbuild /build/public ./public +COPY --from=composer:latest /usr/bin/composer /usr/local/bin/composerNote: Ensure the composer binary has appropriate permissions. You may need to adjust
--chmodto make it executable (e.g.,--chmod=755).
♻️ Duplicate comments (1)
Dockerfile.dev (1)
8-12: Same comments as Dockerfile.base apply here.See the review comments on Dockerfile.base regarding bz2 and git.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
DockerfileDockerfile.baseDockerfile.dev
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile.dev (1)
67-77: Add composer binary copy to dev final stage for runtime plugin management.The production Dockerfile copies the composer binary to the final stage for runtime plugin dependency management. Dockerfile.dev is missing this instruction, but the application code (PluginService.php, lines 193 and 206) executes
composer removeandcomposer requirecommands at runtime when plugins are installed or removed. Without the composer binary in the dev environment, any attempt to manage plugin dependencies will fail with "composer: not found" errors.Add the following line to the final stage after line 77:
COPY --from=composer /usr/local/bin/composer /usr/local/bin/composer
♻️ Duplicate comments (1)
Dockerfile.dev (1)
73-74: Runtime dependencies approved per maintainer directive; note coreutils addition in dev.The runtime dependencies (nodejs, npm, yarn, bash, 7zip, bzip2-dev) match the production Dockerfile and align with the maintainer's confirmation that official plugins require these packages to work out of the box.
Note: The dev image includes
coreutilswhich is not present in the production Dockerfile. This appears intentional for development tooling, but ensure it's actually needed for dev workflows.Regarding the past review comment suggesting removal of bash and 7zip: this suggestion was contradicted by maintainer Boy132's approval that these packages must be included for official plugin support.
Regarding the Alpine version mismatch concern raised in past comments:
The Alpine version pinning issue (node:20-alpine uses Alpine 3.23 while php:8.4-fpm-alpine may use an earlier version) was already flagged in past review comments. This remains a valid concern for ensuring consistency between build and runtime environments.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
DockerfileDockerfile.dev
🔇 Additional comments (4)
Dockerfile (2)
70-72: LGTM! Composer binary correctly added for runtime plugin management.This implementation directly resolves the "composer: not found" errors reported in issue #2025. The COPY instruction properly reuses the composer binary from the composer stage (line 15) as suggested in past review comments, enabling plugins to install composer dependencies at runtime.
65-68: Approve runtime dependencies for plugin system.The addition of nodejs, npm, yarn, and bash aligns with the PR objectives to fix plugin installation failures and provide a complete runtime environment. The 7zip package supports the existing plugin system. The bzip2-dev package is justified by the application's explicit support for tar.bz2 file compression, as referenced in the file handling system (MIME type registration, compression UI options, and API validation for tar.bz2 extensions).
Dockerfile.dev (2)
8-8: LGTM! bz2 PHP extension approved for official plugin support.The addition of the bz2 extension aligns with the maintainer's directive (Boy132 in past comments) that official plugins should work out of the box, and specifically confirmed that the bz2 PHP extension needs to be included.
Based on past review comments from project maintainers.
12-13: The assumption that git is needed for plugin Composer dependencies is not supported by the codebase. Composer.json and composer.lock contain no VCS-type repositories, repository definitions, or git-based package sources.Likely an incorrect or invalid review comment.
Dockerfile
Outdated
| RUN apk add --no-cache \ | ||
| caddy ca-certificates supervisor supercronic fcgi | ||
| caddy ca-certificates supervisor supercronic fcgi \ | ||
| nodejs npm yarn bash 7zip bzip2-dev |
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.
| nodejs npm yarn bash 7zip bzip2-dev | |
| nodejs npm yarn bash p7zip bzip2-dev |
As far as I know this Image is based of of Alphine and the 7zip package is p7zip
https://pkgs.alpinelinux.org/package/v3.16/main/armv7/p7zip
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.
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.
I see alpine 3.16. That is dead old.
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.
We are on alpine 3.23.0 in the php:8.4-fpm-alpine container. They have the package 7zip
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.
I searched in the container using apk search
|
Didn't know that It will commit my changes and I can't review myself. Can someone confirm this #2034 (comment) |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
DockerfileDockerfile.dev
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile.dev
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: QuintenQVD0
Repo: pelican-dev/panel PR: 2034
File: Dockerfile.base:12-12
Timestamp: 2025-12-24T09:21:52.260Z
Learning: Git is required at runtime in the Docker image for the plugin system's dependency installation, even though standard Packagist packages don't require it. This is needed in Dockerfile.base and other Dockerfiles.
Learnt from: QuintenQVD0
Repo: pelican-dev/panel PR: 2034
File: Dockerfile.base:8-8
Timestamp: 2025-12-24T09:22:11.913Z
Learning: In the pelican-dev/panel project, the bz2 PHP extension is required in Dockerfile.base because a plugin in the plugin system depends on it for bzip2 compression functionality.
📚 Learning: 2025-12-24T09:21:52.260Z
Learnt from: QuintenQVD0
Repo: pelican-dev/panel PR: 2034
File: Dockerfile.base:12-12
Timestamp: 2025-12-24T09:21:52.260Z
Learning: Git is required at runtime in the Docker image for the plugin system's dependency installation, even though standard Packagist packages don't require it. This is needed in Dockerfile.base and other Dockerfiles.
Applied to files:
Dockerfile
📚 Learning: 2025-12-24T09:22:11.913Z
Learnt from: QuintenQVD0
Repo: pelican-dev/panel PR: 2034
File: Dockerfile.base:8-8
Timestamp: 2025-12-24T09:22:11.913Z
Learning: In the pelican-dev/panel project, the bz2 PHP extension is required in Dockerfile.base because a plugin in the plugin system depends on it for bzip2 compression functionality.
Applied to files:
Dockerfile
🔇 Additional comments (1)
Dockerfile (1)
70-72: LGTM! Composer binary correctly copied for runtime plugin management.The implementation correctly reuses the composer binary from the composer stage, enabling plugins to install their dependencies at runtime as intended by the PR objectives.
Dockerfile
Outdated
| RUN apk add --no-cache \ | ||
| caddy ca-certificates supervisor supercronic fcgi | ||
| caddy ca-certificates supervisor supercronic fcgi \ | ||
| nodejs npm yarn bash p7zip bzip2-dev |
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.
Why add bash? We don't use it anywhere.
Fixes being able to install plugins Resolves pelican-dev#2024 Resolves pelican-dev#2025 Fixes symlink for plugins Adds symlink for server icons Resolves pelican-dev#2054 Fixes loading environment variables Resolves pelican-dev#2056 Integrates pelican-dev#2051 pelican-dev#2034 pelican-dev#2045 into a single PR.
|
Thank you very much for your contributions @PalmarHealer Please see this pull request this is currently being tested and supersedes this one: #2063 |
Closes #2024
Closes #2025
Fixes plugin installation failures in Docker deployments by adding required runtime dependencies (composer, nodejs, npm, yarn) to the production image.