fix: Detect read-only Docker volumes with long-form syntax and enable refresh
- Fixed isReadOnlyVolume() to detect both short-form (:ro) and long-form (read_only: true) Docker Compose volume syntax - Fixed path matching to use mount_path only (fs_path is transformed during parsing from ./file to absolute path) - Added "Load from server" button for read-only volumes to allow users to refresh content - Changed loadStorageOnServer() authorization from 'update' to 'view' since loading is a read operation - Added helper text to Content field warning users that content may be outdated - Applied fixes to both LocalFileVolume and LocalPersistentVolume models 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
07153de68d
commit
f152ec00ad
5 changed files with 258 additions and 5 deletions
|
|
@ -104,7 +104,8 @@ public function convertToDirectory()
|
|||
public function loadStorageOnServer()
|
||||
{
|
||||
try {
|
||||
$this->authorize('update', $this->resource);
|
||||
// Loading content is a read operation, so we use 'view' permission
|
||||
$this->authorize('view', $this->resource);
|
||||
|
||||
$this->fileStorage->loadStorageOnServer();
|
||||
$this->syncData();
|
||||
|
|
|
|||
|
|
@ -239,22 +239,32 @@ public function isReadOnlyVolume(): bool
|
|||
$volumes = $compose['services'][$serviceName]['volumes'];
|
||||
|
||||
// Check each volume to find a match
|
||||
// Note: We match on mount_path (container path) only, since fs_path gets transformed
|
||||
// from relative (./file) to absolute (/data/coolify/services/uuid/file) during parsing
|
||||
foreach ($volumes as $volume) {
|
||||
// Volume can be string like "host:container:ro" or "host:container"
|
||||
if (is_string($volume)) {
|
||||
$parts = explode(':', $volume);
|
||||
|
||||
// Check if this volume matches our fs_path and mount_path
|
||||
// Check if this volume matches our mount_path
|
||||
if (count($parts) >= 2) {
|
||||
$hostPath = $parts[0];
|
||||
$containerPath = $parts[1];
|
||||
$options = $parts[2] ?? null;
|
||||
|
||||
// Match based on fs_path and mount_path
|
||||
if ($hostPath === $this->fs_path && $containerPath === $this->mount_path) {
|
||||
// Match based on mount_path (container path)
|
||||
if ($containerPath === $this->mount_path) {
|
||||
return $options === 'ro';
|
||||
}
|
||||
}
|
||||
} elseif (is_array($volume)) {
|
||||
// Long-form syntax: { type: bind, source: ..., target: ..., read_only: true }
|
||||
$containerPath = data_get($volume, 'target');
|
||||
$readOnly = data_get($volume, 'read_only', false);
|
||||
|
||||
// Match based on mount_path (container path)
|
||||
if ($containerPath === $this->mount_path) {
|
||||
return $readOnly === true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -85,6 +85,7 @@ public function isReadOnlyVolume(): bool
|
|||
$volumes = $compose['services'][$serviceName]['volumes'];
|
||||
|
||||
// Check each volume to find a match
|
||||
// Note: We match on mount_path (container path) only, since host paths get transformed
|
||||
foreach ($volumes as $volume) {
|
||||
// Volume can be string like "host:container:ro" or "host:container"
|
||||
if (is_string($volume)) {
|
||||
|
|
@ -104,6 +105,19 @@ public function isReadOnlyVolume(): bool
|
|||
return $options === 'ro';
|
||||
}
|
||||
}
|
||||
} elseif (is_array($volume)) {
|
||||
// Long-form syntax: { type: bind/volume, source: ..., target: ..., read_only: true }
|
||||
$containerPath = data_get($volume, 'target');
|
||||
$readOnly = data_get($volume, 'read_only', false);
|
||||
|
||||
// Match based on mount_path
|
||||
// Remove leading slash from mount_path if present for comparison
|
||||
$mountPath = str($this->mount_path)->ltrim('/')->toString();
|
||||
$containerPathClean = str($containerPath)->ltrim('/')->toString();
|
||||
|
||||
if ($mountPath === $containerPathClean || $this->mount_path === $containerPath) {
|
||||
return $readOnly === true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -65,6 +65,7 @@
|
|||
@endif
|
||||
<x-forms.textarea
|
||||
label="{{ $fileStorage->is_based_on_git ? 'Content (refreshed after a successful deployment)' : 'Content' }}"
|
||||
helper="The content shown may be outdated. Click 'Load from server' to fetch the latest version."
|
||||
rows="20" id="content"
|
||||
readonly="{{ $fileStorage->is_based_on_git || $fileStorage->is_binary }}"></x-forms.textarea>
|
||||
@if (!$fileStorage->is_based_on_git && !$fileStorage->is_binary)
|
||||
|
|
@ -79,12 +80,19 @@
|
|||
@endif
|
||||
<x-forms.textarea
|
||||
label="{{ $fileStorage->is_based_on_git ? 'Content (refreshed after a successful deployment)' : 'Content' }}"
|
||||
helper="The content shown may be outdated. Click 'Load from server' to fetch the latest version."
|
||||
rows="20" id="content" disabled></x-forms.textarea>
|
||||
@endcan
|
||||
@endif
|
||||
@else
|
||||
{{-- Read-only view --}}
|
||||
@if (!$fileStorage->is_directory)
|
||||
@can('view', $resource)
|
||||
<div class="flex gap-2">
|
||||
<x-forms.button type="button" wire:click="loadStorageOnServer">Load from
|
||||
server</x-forms.button>
|
||||
</div>
|
||||
@endcan
|
||||
@if (data_get($resource, 'settings.is_preserve_repository_enabled'))
|
||||
<div class="w-96">
|
||||
<x-forms.checkbox disabled label="Is this based on the Git repository?"
|
||||
|
|
@ -93,6 +101,7 @@
|
|||
@endif
|
||||
<x-forms.textarea
|
||||
label="{{ $fileStorage->is_based_on_git ? 'Content (refreshed after a successful deployment)' : 'Content' }}"
|
||||
helper="The content shown may be outdated. Click 'Load from server' to fetch the latest version."
|
||||
rows="20" id="content" disabled></x-forms.textarea>
|
||||
@endif
|
||||
@endif
|
||||
|
|
|
|||
219
tests/Unit/LocalFileVolumeReadOnlyTest.php
Normal file
219
tests/Unit/LocalFileVolumeReadOnlyTest.php
Normal file
|
|
@ -0,0 +1,219 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Unit tests to verify LocalFileVolume::isReadOnlyVolume() correctly detects
|
||||
* read-only volumes in both short-form and long-form Docker Compose syntax.
|
||||
*
|
||||
* Related Issue: Volumes with read_only: true in long-form syntax were not
|
||||
* being detected as read-only, allowing UI edits on files that should be protected.
|
||||
*
|
||||
* Related Files:
|
||||
* - app/Models/LocalFileVolume.php
|
||||
* - app/Livewire/Project/Service/FileStorage.php
|
||||
*/
|
||||
|
||||
use Symfony\Component\Yaml\Yaml;
|
||||
|
||||
/**
|
||||
* Helper function to parse volumes and detect read-only status.
|
||||
* This mirrors the logic in LocalFileVolume::isReadOnlyVolume()
|
||||
*
|
||||
* Note: We match on mount_path (container path) only, since fs_path gets transformed
|
||||
* from relative (./file) to absolute (/data/coolify/services/uuid/file) during parsing
|
||||
*/
|
||||
function isVolumeReadOnly(string $dockerComposeRaw, string $serviceName, string $mountPath): bool
|
||||
{
|
||||
$compose = Yaml::parse($dockerComposeRaw);
|
||||
|
||||
if (! isset($compose['services'][$serviceName]['volumes'])) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$volumes = $compose['services'][$serviceName]['volumes'];
|
||||
|
||||
foreach ($volumes as $volume) {
|
||||
// Volume can be string like "host:container:ro" or "host:container"
|
||||
if (is_string($volume)) {
|
||||
$parts = explode(':', $volume);
|
||||
|
||||
if (count($parts) >= 2) {
|
||||
$containerPath = $parts[1];
|
||||
$options = $parts[2] ?? null;
|
||||
|
||||
if ($containerPath === $mountPath) {
|
||||
return $options === 'ro';
|
||||
}
|
||||
}
|
||||
} elseif (is_array($volume)) {
|
||||
// Long-form syntax: { type: bind, source: ..., target: ..., read_only: true }
|
||||
$containerPath = data_get($volume, 'target');
|
||||
$readOnly = data_get($volume, 'read_only', false);
|
||||
|
||||
if ($containerPath === $mountPath) {
|
||||
return $readOnly === true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
test('detects read-only with short-form syntax using :ro', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- ./config.toml:/etc/config.toml:ro
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/config.toml'))->toBeTrue();
|
||||
});
|
||||
|
||||
test('detects writable with short-form syntax without :ro', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- ./config.toml:/etc/config.toml
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/config.toml'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('detects read-only with long-form syntax and read_only: true', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- type: bind
|
||||
source: ./garage.toml
|
||||
target: /etc/garage.toml
|
||||
read_only: true
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/garage.toml'))->toBeTrue();
|
||||
});
|
||||
|
||||
test('detects writable with long-form syntax and read_only: false', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- type: bind
|
||||
source: ./garage.toml
|
||||
target: /etc/garage.toml
|
||||
read_only: false
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/garage.toml'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('detects writable with long-form syntax without read_only key', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- type: bind
|
||||
source: ./garage.toml
|
||||
target: /etc/garage.toml
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/garage.toml'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('handles mixed short-form and long-form volumes in same service', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- ./data:/var/data
|
||||
- type: bind
|
||||
source: ./config.toml
|
||||
target: /etc/config.toml
|
||||
read_only: true
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/var/data'))->toBeFalse();
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/config.toml'))->toBeTrue();
|
||||
});
|
||||
|
||||
test('handles same file mounted in multiple services with different read_only settings', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/garage
|
||||
volumes:
|
||||
- type: bind
|
||||
source: ./garage.toml
|
||||
target: /etc/garage.toml
|
||||
garage-webui:
|
||||
image: example/webui
|
||||
volumes:
|
||||
- type: bind
|
||||
source: ./garage.toml
|
||||
target: /etc/garage.toml
|
||||
read_only: true
|
||||
YAML;
|
||||
|
||||
// Same file, different services, different read_only status
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/garage.toml'))->toBeFalse();
|
||||
expect(isVolumeReadOnly($compose, 'garage-webui', '/etc/garage.toml'))->toBeTrue();
|
||||
});
|
||||
|
||||
test('handles volume mount type', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
app:
|
||||
image: example/app
|
||||
volumes:
|
||||
- type: volume
|
||||
source: mydata
|
||||
target: /data
|
||||
read_only: true
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'app', '/data'))->toBeTrue();
|
||||
});
|
||||
|
||||
test('returns false when service has no volumes', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/config.toml'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('returns false when service does not exist', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- ./config.toml:/etc/config.toml:ro
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'nonexistent', '/etc/config.toml'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('returns false when mount path does not match', function () {
|
||||
$compose = <<<'YAML'
|
||||
services:
|
||||
garage:
|
||||
image: example/image
|
||||
volumes:
|
||||
- type: bind
|
||||
source: ./other.toml
|
||||
target: /etc/other.toml
|
||||
read_only: true
|
||||
YAML;
|
||||
|
||||
expect(isVolumeReadOnly($compose, 'garage', '/etc/config.toml'))->toBeFalse();
|
||||
});
|
||||
Loading…
Reference in a new issue