Multiple Container Fixes #2063
Multiple Container Fixes #2063
Conversation
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.
📝 WalkthroughWalkthroughThe changes update Docker image configurations and entrypoint script to add missing dependencies (yarn, compression utilities) for plugin installation, establish symlinks for persistent storage of icons and plugins, and fix environment variable parsing to handle spaces in values. Changes
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docker/entrypoint.sh (1)
6-6: Consider a more robust environment loading approach.The
xargs -d '\n'fix handles spaces in values, but theexport $(...)pattern remains fragile with special characters (quotes, backticks, etc.). Consider using:- export $(grep -v '^#' .env | xargs -d '\n') # Fix is from https://github.com/pelican-dev/panel/pull/2045 + set -a + . .env + set +aThis approach sources the file directly with automatic exporting enabled, avoiding word-splitting issues entirely.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
DockerfileDockerfile.devdocker/entrypoint.sh
🧰 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:58.780Z
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.
📚 Learning: 2025-12-24T09:22:18.560Z
Learnt from: QuintenQVD0
Repo: pelican-dev/panel PR: 2034
File: Dockerfile.base:8-8
Timestamp: 2025-12-24T09:22:18.560Z
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:
DockerfileDockerfile.dev
📚 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: Ensure that Dockerfiles install git at runtime where required for plugin system dependency installation. Update Dockerfile.base and any other Dockerfiles that build runtime images to include git in the runtime dependencies (not just for build-time), so plugin installation steps function correctly in the container.
Applied to files:
Dockerfile.dev
🪛 Shellcheck (0.11.0)
docker/entrypoint.sh
[warning] 6-6: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (4)
docker/entrypoint.sh (1)
28-28: LGTM!The addition of
/pelican-data/storage/iconscorrectly ensures the directory exists for the new icons symlink, supporting the objective to persist icons across container restarts.Dockerfile.dev (2)
70-74: Verify git is installed for plugin system.The added packages (zip, unzip, 7zip, bzip2-dev, yarn) correctly support plugin installation per the PR objectives. However, based on learnings, git is required at runtime for the plugin system's dependency installation. Ensure git is included in the runtime packages.
🔎 Proposed fix to add git
RUN apk add --no-cache \ # packages for running the panel caddy ca-certificates supervisor supercronic fcgi coreutils \ # required for installing plugins. Pulled from https://github.com/pelican-dev/panel/pull/2034 - zip unzip 7zip bzip2-dev yarn + zip unzip 7zip bzip2-dev yarn gitBased on learnings, git is required at runtime for plugin system dependency installation.
93-93: LGTM!The icons symlink correctly ensures icon persistence across container restarts, directly addressing issue #2054.
Dockerfile (1)
88-88: LGTM!The icons symlink correctly ensures persistence across container restarts, addressing issue #2054.
| RUN apk add --no-cache \ | ||
| caddy ca-certificates supervisor supercronic fcgi | ||
| # packages for running the panel | ||
| caddy ca-certificates supervisor supercronic fcgi \ | ||
| # required for installing plugins. Pulled from https://github.com/pelican-dev/panel/pull/2034 | ||
| zip unzip 7zip bzip2-dev yarn |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all Dockerfile-related files
find . -name "Dockerfile*" -type f | head -20Repository: pelican-dev/panel
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Check if Dockerfile.base exists and examine its contents
if [ -f "Dockerfile.base" ]; then
echo "=== Dockerfile.base ==="
cat -n Dockerfile.base
else
echo "Dockerfile.base not found in root"
# Search for it
find . -name "Dockerfile.base" -type f 2>/dev/null
fiRepository: pelican-dev/panel
Length of output: 595
🏁 Script executed:
#!/bin/bash
# Search for git references in Docker-related files
rg -l "git" --type dockerfile . 2>/dev/null | head -10Repository: pelican-dev/panel
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the main Dockerfile to see base image reference
if [ -f "Dockerfile" ]; then
echo "=== Checking Dockerfile base image ==="
head -20 Dockerfile
fiRepository: pelican-dev/panel
Length of output: 714
🏁 Script executed:
#!/bin/bash
# Check lines 65-69 in Dockerfile
sed -n '60,75p' DockerfileRepository: pelican-dev/panel
Length of output: 700
Add git to the runtime packages.
Git is required at runtime for the plugin system's dependency installation, but it's not included in the base image (Dockerfile.base) or in the packages being installed here. Add git to the RUN apk add command:
RUN apk add --no-cache \
# packages for running the panel
caddy ca-certificates git supervisor supercronic fcgi \
# required for installing plugins. Pulled from https://github.com/pelican-dev/panel/pull/2034
zip unzip 7zip bzip2-dev yarn
Alternatively, add it to Dockerfile.base if git should be part of the base image.
🤖 Prompt for AI Agents
In Dockerfile around lines 65 to 69, the runtime apk package list is missing git
which the plugin system needs for dependency installation; update the RUN apk
add --no-cache command to include git among the runtime packages (or add git to
Dockerfile.base if you prefer it in the base image) so git is available at
container runtime for plugin installs.
| && rmdir /var/www/html/plugins \ | ||
| # fix from https://github.com/pelican-dev/panel/pull/2051 | ||
| && ln -s /pelican-data/plugins /var/www/html \ |
There was a problem hiding this comment.
Ensure rmdir handles missing or non-empty directories.
The rmdir command will fail if /var/www/html/plugins doesn't exist or is not empty, causing the Docker build to fail. Use a more resilient approach.
🔎 Proposed fix
- && rmdir /var/www/html/plugins \
# fix from https://github.com/pelican-dev/panel/pull/2051
+ && rm -rf /var/www/html/plugins \
&& ln -s /pelican-data/plugins /var/www/htmlAlternatively, if you want to preserve the safety of rmdir (ensuring it's empty):
- && rmdir /var/www/html/plugins \
+ && (rmdir /var/www/html/plugins 2>/dev/null || true) \
# fix from https://github.com/pelican-dev/panel/pull/2051
&& ln -s /pelican-data/plugins /var/www/html📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| && rmdir /var/www/html/plugins \ | |
| # fix from https://github.com/pelican-dev/panel/pull/2051 | |
| && ln -s /pelican-data/plugins /var/www/html \ | |
| && rm -rf /var/www/html/plugins \ | |
| # fix from https://github.com/pelican-dev/panel/pull/2051 | |
| && ln -s /pelican-data/plugins /var/www/html |
🤖 Prompt for AI Agents
In Dockerfile around lines 89-91, the rmdir command can fail if
/var/www/html/plugins is missing or non-empty; replace it with a resilient
removal sequence that first checks/removes any existing path and does not fail
the build (e.g., test -e and remove recursively or use rm -rf safely), and
ensure any existing /var/www/html/plugins or /var/www/html/plugins symlink is
removed before creating the new symlink to /pelican-data/plugins so ln -s cannot
fail.
| && rmdir /var/www/html/plugins \ | ||
| # fix from https://github.com/pelican-dev/panel/pull/2051 | ||
| && ln -s /pelican-data/plugins /var/www/html \ |
There was a problem hiding this comment.
Ensure rmdir handles missing or non-empty directories.
The rmdir command will fail if /var/www/html/plugins doesn't exist or is not empty, causing the Docker build to fail. Use a more resilient approach.
🔎 Proposed fix
- && rmdir /var/www/html/plugins \
# fix from https://github.com/pelican-dev/panel/pull/2051
+ && rm -rf /var/www/html/plugins \
&& ln -s /pelican-data/plugins /var/www/htmlAlternatively, if you want to preserve the safety of rmdir (ensuring it's empty):
- && rmdir /var/www/html/plugins \
+ && (rmdir /var/www/html/plugins 2>/dev/null || true) \
# fix from https://github.com/pelican-dev/panel/pull/2051
&& ln -s /pelican-data/plugins /var/www/html📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| && rmdir /var/www/html/plugins \ | |
| # fix from https://github.com/pelican-dev/panel/pull/2051 | |
| && ln -s /pelican-data/plugins /var/www/html \ | |
| && rm -rf /var/www/html/plugins \ | |
| # fix from https://github.com/pelican-dev/panel/pull/2051 | |
| && ln -s /pelican-data/plugins /var/www/html \ |
🤖 Prompt for AI Agents
Dockerfile.dev lines 94-96: the use of plain `rmdir` will fail the build if
/var/www/html/plugins is missing or not empty; change the step to a resilient
removal that only attempts to remove if the directory exists and otherwise is a
no-op (or use a safe recursive-force removal when you intend to delete
contents), then create the symlink—i.e., replace the `rmdir` command with a
check-and-remove or `rm -rf` style approach so the Docker build does not fail
when the directory is absent or non-empty.
| && rmdir /var/www/html/plugins \ | ||
| # fix from https://github.com/pelican-dev/panel/pull/2051 | ||
| && ln -s /pelican-data/plugins /var/www/html \ |
There was a problem hiding this comment.
| && rmdir /var/www/html/plugins \ | |
| # fix from https://github.com/pelican-dev/panel/pull/2051 | |
| && ln -s /pelican-data/plugins /var/www/html \ | |
| && ln -sf /pelican-data/plugins /var/www/html \ |
Why don't use the same as above ?
(Ran on the php:8.4-fpm-alpine image)
| && rmdir /var/www/html/plugins \ | ||
| # fix from https://github.com/pelican-dev/panel/pull/2051 | ||
| && ln -s /pelican-data/plugins /var/www/html \ |
There was a problem hiding this comment.
| && rmdir /var/www/html/plugins \ | |
| # fix from https://github.com/pelican-dev/panel/pull/2051 | |
| && ln -s /pelican-data/plugins /var/www/html \ | |
| && ln -sf /pelican-data/plugins /var/www/html \ |
Why don't use the same as above ?
(Ran on the php:8.4-fpm-alpine image)
There was a problem hiding this comment.
Other then the ln situation LGTM
Also it doesn't actually solves #2056 im still able to reproduce inside & outside of docker.
3 participants
Fixes being able to install plugins
Resolves #2024
Resolves #2025
Fixes symlink for plugins
Adds symlink for server icons
Resolves #2054
Fixes loading environment variables
Resolves #2037
Integrates #2051 #2034 #2045 into a single PR.