repair the public/storage symlink shipped in the release tarball - #229
Conversation
The koel release archive contains public/storage as an absolute symlink into the path the release runner built at, so it resolves nowhere in the container and koel cannot read or write uploaded images — koel:doctor reports the image storage directory as unwritable. Laravel's storage:link cannot repair it either: file_exists() is false for a dangling link, so the command tries to create one and fails on the existing path. Replace it at build time with a relative link to the same target, and assert both it and the image directory in the goss suite.
📝 WalkthroughWalkthroughThe Docker image now replaces the release symlink with a relative ChangesPublic storage symlink
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 105-108: The Dockerfile source step must clone the Koel repository
at the declared KOEL_VERSION_REF instead of downloading a release tarball.
Update the initial application-fetch command to use ARG KOEL_VERSION_REF as the
git ref, while preserving the existing build layout and subsequent setup steps.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # The release tarball ships public/storage as an absolute symlink into the path the | ||
| # release runner built it at, which resolves nowhere here and leaves koel unable to read | ||
| # or write uploaded images. Replace it with a relative link to the same target. | ||
| && ln -sfn ../storage/app/public /var/www/html/public/storage \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use the required git clone source for KOEL_VERSION_REF.
The Dockerfile still downloads a release tarball at Line 8. It does not clone Koel at the ref declared by ARG KOEL_VERSION_REF.
Change the source step to clone KOEL_VERSION_REF, or update the guideline if release tarballs are intentional.
As per coding guidelines: “The Dockerfile must use ARG KOEL_VERSION_REF=... as the git ref to clone the koel app at the specified version.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 105 - 108, The Dockerfile source step must clone the
Koel repository at the declared KOEL_VERSION_REF instead of downloading a
release tarball. Update the initial application-fetch command to use ARG
KOEL_VERSION_REF as the git ref, while preserving the existing build layout and
subsequent setup steps.
Source: Coding guidelines
|
Correcting something I got wrong above. I wrote that if (file_exists($link) && ! $this->isRemovableSymlink($link, $this->option('force'))) {
// skipped for a dangling link, since file_exists() is false
}
if (is_link($link)) {
$this->laravel->make('files')->delete($link);
}So None of this changes the fix or the diagnosis of the underlying bug — the archive genuinely ships a link into the release runner's filesystem. I just described the repair path incorrectly. |
1 participant
Fixes the image storage failure reported in #224, at a different layer than that PR proposed.
The bug
koel's release archive ships
public/storageas an absolute symlink into the directory the release runner built in. Straight from the v9.11.1 tarball:That path exists on no machine but the GitHub Actions runner, so in the container the link dangles. koel resolves uploaded images through
public_path('storage/images'), so it can neither read nor write them, andkoel:doctorreports:Confirmed in the currently published image:
koel:initcannot repair it. Laravel'sstorage:linkchecksfile_exists(), which is false for a dangling symlink, so it attempts to create one and fails on the path that is already there.The fix
Replace the link at build time with a relative one to the same target, so it resolves wherever the tree is unpacked. The volume stays where the
VOLUMEdeclaration already points, so existing installations keep their images....and
public/storage/imagesis writable bywww-data, which is whatkoel:doctorchecks.The goss suite now asserts the symlink and its target directory, so a regression fails CI rather than reaching users.
On #224
That PR moved the volume mount to
public/storage/imagesindocker-compose.mysql.yml, which does make the path writable, but mounts over the symlink's location — leaving theVOLUMEdeclaration pointing at an orphaned path, moving existing installations' images to a different volume, and patching one of the three compose files. Fixing the link in the image covers every compose file and every hand-rolleddocker run.The root cause is in koel itself and is fixed separately in koel/koel#2644 —
koel:initnow creates a relative link, so future releases ship a portable archive. This change is still worth keeping: it repairs every release already published.Summary by CodeRabbit
Bug Fixes
Tests