feat(preview): add configurable PR suffix toggle for volumes
Add `is_preview_suffix_enabled` flag to `local_file_volumes` and `local_persistent_volumes` tables, allowing per-volume control over whether a `-pr-N` suffix is appended during preview deployments. Defaults to `true` to preserve existing behavior. Users can disable it for volumes containing shared config or repository scripts that should not be isolated per PR.
This commit is contained in:
parent
1b484a56b0
commit
add16853a8
10 changed files with 172 additions and 20 deletions
|
|
@ -2745,7 +2745,8 @@ private function generate_local_persistent_volumes()
|
|||
} else {
|
||||
$volume_name = $persistentStorage->name;
|
||||
}
|
||||
if ($this->pull_request_id !== 0) {
|
||||
$isPreviewSuffixEnabled = (bool) data_get($persistentStorage, 'is_preview_suffix_enabled', true);
|
||||
if ($this->pull_request_id !== 0 && $isPreviewSuffixEnabled) {
|
||||
$volume_name = addPreviewDeploymentSuffix($volume_name, $this->pull_request_id);
|
||||
}
|
||||
$local_persistent_volumes[] = $volume_name.':'.$persistentStorage->mount_path;
|
||||
|
|
@ -2763,7 +2764,8 @@ private function generate_local_persistent_volumes_only_volume_names()
|
|||
}
|
||||
$name = $persistentStorage->name;
|
||||
|
||||
if ($this->pull_request_id !== 0) {
|
||||
$isPreviewSuffixEnabled = (bool) data_get($persistentStorage, 'is_preview_suffix_enabled', true);
|
||||
if ($this->pull_request_id !== 0 && $isPreviewSuffixEnabled) {
|
||||
$name = addPreviewDeploymentSuffix($name, $this->pull_request_id);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -40,12 +40,16 @@ class FileStorage extends Component
|
|||
#[Validate(['required', 'boolean'])]
|
||||
public bool $isBasedOnGit = false;
|
||||
|
||||
#[Validate(['required', 'boolean'])]
|
||||
public bool $isPreviewSuffixEnabled = true;
|
||||
|
||||
protected $rules = [
|
||||
'fileStorage.is_directory' => 'required',
|
||||
'fileStorage.fs_path' => 'required',
|
||||
'fileStorage.mount_path' => 'required',
|
||||
'content' => 'nullable',
|
||||
'isBasedOnGit' => 'required|boolean',
|
||||
'isPreviewSuffixEnabled' => 'required|boolean',
|
||||
];
|
||||
|
||||
public function mount()
|
||||
|
|
@ -71,12 +75,14 @@ public function syncData(bool $toModel = false): void
|
|||
// Sync to model
|
||||
$this->fileStorage->content = $this->content;
|
||||
$this->fileStorage->is_based_on_git = $this->isBasedOnGit;
|
||||
$this->fileStorage->is_preview_suffix_enabled = $this->isPreviewSuffixEnabled;
|
||||
|
||||
$this->fileStorage->save();
|
||||
} else {
|
||||
// Sync from model
|
||||
$this->content = $this->fileStorage->content;
|
||||
$this->isBasedOnGit = $this->fileStorage->is_based_on_git;
|
||||
$this->isPreviewSuffixEnabled = $this->fileStorage->is_preview_suffix_enabled ?? true;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -29,10 +29,13 @@ class Show extends Component
|
|||
|
||||
public ?string $hostPath = null;
|
||||
|
||||
public bool $isPreviewSuffixEnabled = true;
|
||||
|
||||
protected $rules = [
|
||||
'name' => 'required|string',
|
||||
'mountPath' => 'required|string',
|
||||
'hostPath' => 'string|nullable',
|
||||
'isPreviewSuffixEnabled' => 'required|boolean',
|
||||
];
|
||||
|
||||
protected $validationAttributes = [
|
||||
|
|
@ -53,11 +56,13 @@ private function syncData(bool $toModel = false): void
|
|||
$this->storage->name = $this->name;
|
||||
$this->storage->mount_path = $this->mountPath;
|
||||
$this->storage->host_path = $this->hostPath;
|
||||
$this->storage->is_preview_suffix_enabled = $this->isPreviewSuffixEnabled;
|
||||
} else {
|
||||
// Sync FROM model (on load/refresh)
|
||||
$this->name = $this->storage->name;
|
||||
$this->mountPath = $this->storage->mount_path;
|
||||
$this->hostPath = $this->storage->host_path;
|
||||
$this->isPreviewSuffixEnabled = $this->storage->is_preview_suffix_enabled ?? true;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -67,6 +72,15 @@ public function mount()
|
|||
$this->isReadOnly = $this->storage->shouldBeReadOnlyInUI();
|
||||
}
|
||||
|
||||
public function instantSave()
|
||||
{
|
||||
$this->authorize('update', $this->resource);
|
||||
|
||||
$this->syncData(true);
|
||||
$this->storage->save();
|
||||
$this->dispatch('success', 'Storage updated successfully');
|
||||
}
|
||||
|
||||
public function submit()
|
||||
{
|
||||
$this->authorize('update', $this->resource);
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ class LocalFileVolume extends BaseModel
|
|||
// 'mount_path' => 'encrypted',
|
||||
'content' => 'encrypted',
|
||||
'is_directory' => 'boolean',
|
||||
'is_preview_suffix_enabled' => 'boolean',
|
||||
];
|
||||
|
||||
use HasFactory;
|
||||
|
|
|
|||
|
|
@ -10,6 +10,10 @@ class LocalPersistentVolume extends Model
|
|||
{
|
||||
protected $guarded = [];
|
||||
|
||||
protected $casts = [
|
||||
'is_preview_suffix_enabled' => 'boolean',
|
||||
];
|
||||
|
||||
public function resource()
|
||||
{
|
||||
return $this->morphTo('resource');
|
||||
|
|
|
|||
|
|
@ -789,6 +789,12 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
$mainDirectory = str(base_configuration_dir().'/applications/'.$uuid);
|
||||
}
|
||||
$source = replaceLocalSource($source, $mainDirectory);
|
||||
$isPreviewSuffixEnabled = $foundConfig
|
||||
? (bool) data_get($foundConfig, 'is_preview_suffix_enabled', true)
|
||||
: true;
|
||||
if ($isPullRequest && $isPreviewSuffixEnabled) {
|
||||
$source = addPreviewDeploymentSuffix($source, $pull_request_id);
|
||||
}
|
||||
LocalFileVolume::updateOrCreate(
|
||||
[
|
||||
'mount_path' => $target,
|
||||
|
|
@ -1312,19 +1318,19 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
}
|
||||
if (! $isDatabase && $fqdns instanceof Collection && $fqdns->count() > 0) {
|
||||
$shouldGenerateLabelsExactly = $resource->destination->server->settings->generate_exact_labels;
|
||||
$uuid = $resource->uuid;
|
||||
$network = data_get($resource, 'destination.network');
|
||||
$labelUuid = $resource->uuid;
|
||||
$labelNetwork = data_get($resource, 'destination.network');
|
||||
if ($isPullRequest) {
|
||||
$uuid = "{$resource->uuid}-{$pullRequestId}";
|
||||
$labelUuid = "{$resource->uuid}-{$pullRequestId}";
|
||||
}
|
||||
if ($isPullRequest) {
|
||||
$network = "{$resource->destination->network}-{$pullRequestId}";
|
||||
$labelNetwork = "{$resource->destination->network}-{$pullRequestId}";
|
||||
}
|
||||
if ($shouldGenerateLabelsExactly) {
|
||||
switch ($server->proxyType()) {
|
||||
case ProxyTypes::TRAEFIK->value:
|
||||
$serviceLabels = $serviceLabels->merge(fqdnLabelsForTraefik(
|
||||
uuid: $uuid,
|
||||
uuid: $labelUuid,
|
||||
domains: $fqdns,
|
||||
is_force_https_enabled: $originalResource->isForceHttpsEnabled(),
|
||||
serviceLabels: $serviceLabels,
|
||||
|
|
@ -1336,8 +1342,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
break;
|
||||
case ProxyTypes::CADDY->value:
|
||||
$serviceLabels = $serviceLabels->merge(fqdnLabelsForCaddy(
|
||||
network: $network,
|
||||
uuid: $uuid,
|
||||
network: $labelNetwork,
|
||||
uuid: $labelUuid,
|
||||
domains: $fqdns,
|
||||
is_force_https_enabled: $originalResource->isForceHttpsEnabled(),
|
||||
serviceLabels: $serviceLabels,
|
||||
|
|
@ -1351,7 +1357,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
}
|
||||
} else {
|
||||
$serviceLabels = $serviceLabels->merge(fqdnLabelsForTraefik(
|
||||
uuid: $uuid,
|
||||
uuid: $labelUuid,
|
||||
domains: $fqdns,
|
||||
is_force_https_enabled: $originalResource->isForceHttpsEnabled(),
|
||||
serviceLabels: $serviceLabels,
|
||||
|
|
@ -1361,8 +1367,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
image: $image
|
||||
));
|
||||
$serviceLabels = $serviceLabels->merge(fqdnLabelsForCaddy(
|
||||
network: $network,
|
||||
uuid: $uuid,
|
||||
network: $labelNetwork,
|
||||
uuid: $labelUuid,
|
||||
domains: $fqdns,
|
||||
is_force_https_enabled: $originalResource->isForceHttpsEnabled(),
|
||||
serviceLabels: $serviceLabels,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,30 @@
|
|||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Database\Schema\Blueprint;
|
||||
use Illuminate\Support\Facades\Schema;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
public function up(): void
|
||||
{
|
||||
Schema::table('local_file_volumes', function (Blueprint $table) {
|
||||
$table->boolean('is_preview_suffix_enabled')->default(true)->after('is_based_on_git');
|
||||
});
|
||||
|
||||
Schema::table('local_persistent_volumes', function (Blueprint $table) {
|
||||
$table->boolean('is_preview_suffix_enabled')->default(true)->after('host_path');
|
||||
});
|
||||
}
|
||||
|
||||
public function down(): void
|
||||
{
|
||||
Schema::table('local_file_volumes', function (Blueprint $table) {
|
||||
$table->dropColumn('is_preview_suffix_enabled');
|
||||
});
|
||||
|
||||
Schema::table('local_persistent_volumes', function (Blueprint $table) {
|
||||
$table->dropColumn('is_preview_suffix_enabled');
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
@ -15,6 +15,13 @@
|
|||
<x-forms.input label="Destination Path" :value="$fileStorage->mount_path" readonly />
|
||||
</div>
|
||||
</div>
|
||||
@can('update', $resource)
|
||||
<div class="w-96">
|
||||
<x-forms.checkbox instantSave label="Add suffix for PR deployments"
|
||||
id="isPreviewSuffixEnabled"
|
||||
helper="When enabled, a -pr-N suffix is added to this volume's path for preview deployments (e.g. ./scripts becomes ./scripts-pr-1). Disable this for volumes that contain shared config or scripts from your repository."></x-forms.checkbox>
|
||||
</div>
|
||||
@endcan
|
||||
<form wire:submit='submit' class="flex flex-col gap-2">
|
||||
@if (!$isReadOnly)
|
||||
@can('update', $resource)
|
||||
|
|
|
|||
|
|
@ -38,6 +38,13 @@
|
|||
<x-forms.input id="mountPath" required readonly />
|
||||
</div>
|
||||
@endif
|
||||
@can('update', $resource)
|
||||
<div class="w-96">
|
||||
<x-forms.checkbox instantSave label="Add suffix for PR deployments"
|
||||
id="isPreviewSuffixEnabled"
|
||||
helper="When enabled, a -pr-N suffix is added to this volume's name for preview deployments (e.g. myvolume becomes myvolume-pr-1). Disable this for volumes that should be shared between the main and preview deployments."></x-forms.checkbox>
|
||||
</div>
|
||||
@endcan
|
||||
@else
|
||||
@can('update', $resource)
|
||||
@if ($isFirst)
|
||||
|
|
@ -54,6 +61,13 @@
|
|||
<x-forms.input id="mountPath" required />
|
||||
</div>
|
||||
@endif
|
||||
@if (data_get($resource, 'settings.is_preview_deployments_enabled'))
|
||||
<div class="w-96">
|
||||
<x-forms.checkbox instantSave label="Add suffix for PR deployments"
|
||||
id="isPreviewSuffixEnabled"
|
||||
helper="When enabled, a -pr-N suffix is added to this volume's name for preview deployments (e.g. myvolume becomes myvolume-pr-1). Disable this for volumes that should be shared between the main and preview deployments."></x-forms.checkbox>
|
||||
</div>
|
||||
@endif
|
||||
<div class="flex gap-2">
|
||||
<x-forms.button type="submit">
|
||||
Update
|
||||
|
|
|
|||
|
|
@ -3,12 +3,13 @@
|
|||
/**
|
||||
* Tests for GitHub issue #7802: volume mappings from repo content in Preview Deployments.
|
||||
*
|
||||
* Bind mount volumes (e.g., ./scripts:/scripts:ro) should NOT get a -pr-N suffix
|
||||
* during preview deployments, because the repo files exist at the original path.
|
||||
* Only named Docker volumes need the suffix for isolation between PRs.
|
||||
* Bind mount volumes use a per-volume `is_preview_suffix_enabled` setting to control
|
||||
* whether the -pr-N suffix is applied during preview deployments.
|
||||
* When enabled (default), the suffix is applied for data isolation.
|
||||
* When disabled, the volume path is shared with the main deployment.
|
||||
* Named Docker volumes also respect this setting.
|
||||
*/
|
||||
it('does not apply preview deployment suffix to bind mount source paths', function () {
|
||||
// Read the applicationParser volume handling in parsers.php
|
||||
it('uses is_preview_suffix_enabled setting for bind mount suffix in preview deployments', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Find the bind mount handling block (type === 'bind')
|
||||
|
|
@ -16,12 +17,14 @@
|
|||
$volumeBlockStart = strpos($parsersFile, "} elseif (\$type->value() === 'volume')");
|
||||
$bindBlock = substr($parsersFile, $bindBlockStart, $volumeBlockStart - $bindBlockStart);
|
||||
|
||||
// Bind mount paths should NOT be suffixed with -pr-N
|
||||
expect($bindBlock)->not->toContain('addPreviewDeploymentSuffix');
|
||||
// Bind mount block should check is_preview_suffix_enabled before applying suffix
|
||||
expect($bindBlock)
|
||||
->toContain('$isPreviewSuffixEnabled')
|
||||
->toContain('is_preview_suffix_enabled')
|
||||
->toContain('addPreviewDeploymentSuffix');
|
||||
});
|
||||
|
||||
it('still applies preview deployment suffix to named volume paths', function () {
|
||||
// Read the applicationParser volume handling in parsers.php
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Find the named volume handling block (type === 'volume')
|
||||
|
|
@ -39,3 +42,68 @@
|
|||
$result = addPreviewDeploymentSuffix('myvolume', 0);
|
||||
expect($result)->toBe('myvolume');
|
||||
});
|
||||
|
||||
/**
|
||||
* Tests for GitHub issue #7343: $uuid mutation in label generation leaks into
|
||||
* subsequent services' volume paths during preview deployments.
|
||||
*
|
||||
* The label generation block must use a local variable ($labelUuid) instead of
|
||||
* mutating the shared $uuid variable, which is used for volume base paths.
|
||||
*/
|
||||
it('does not mutate shared uuid variable during label generation', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Find the FQDN label generation block
|
||||
$labelBlockStart = strpos($parsersFile, '$shouldGenerateLabelsExactly = $resource->destination->server->settings->generate_exact_labels;');
|
||||
$labelBlock = substr($parsersFile, $labelBlockStart, 300);
|
||||
|
||||
// Should use $labelUuid, not mutate $uuid
|
||||
expect($labelBlock)
|
||||
->toContain('$labelUuid = $resource->uuid')
|
||||
->not->toContain('$uuid = $resource->uuid')
|
||||
->not->toContain("\$uuid = \"{$resource->uuid}");
|
||||
});
|
||||
|
||||
it('uses labelUuid in all proxy label generation calls', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Find the FQDN label generation block (from shouldGenerateLabelsExactly to the closing brace)
|
||||
$labelBlockStart = strpos($parsersFile, '$shouldGenerateLabelsExactly');
|
||||
$labelBlockEnd = strpos($parsersFile, "data_forget(\$service, 'volumes.*.content')");
|
||||
$labelBlock = substr($parsersFile, $labelBlockStart, $labelBlockEnd - $labelBlockStart);
|
||||
|
||||
// All uuid references in label functions should use $labelUuid
|
||||
expect($labelBlock)
|
||||
->toContain('uuid: $labelUuid')
|
||||
->not->toContain('uuid: $uuid');
|
||||
});
|
||||
|
||||
it('checks is_preview_suffix_enabled in deployment job for persistent volumes', function () {
|
||||
$deploymentJobFile = file_get_contents(__DIR__.'/../../app/Jobs/ApplicationDeploymentJob.php');
|
||||
|
||||
// Find the generate_local_persistent_volumes method
|
||||
$methodStart = strpos($deploymentJobFile, 'function generate_local_persistent_volumes()');
|
||||
$methodEnd = strpos($deploymentJobFile, 'function generate_local_persistent_volumes_only_volume_names()');
|
||||
$methodBlock = substr($deploymentJobFile, $methodStart, $methodEnd - $methodStart);
|
||||
|
||||
// Should check is_preview_suffix_enabled before applying suffix
|
||||
expect($methodBlock)
|
||||
->toContain('is_preview_suffix_enabled')
|
||||
->toContain('$isPreviewSuffixEnabled')
|
||||
->toContain('addPreviewDeploymentSuffix');
|
||||
});
|
||||
|
||||
it('checks is_preview_suffix_enabled in deployment job for volume names', function () {
|
||||
$deploymentJobFile = file_get_contents(__DIR__.'/../../app/Jobs/ApplicationDeploymentJob.php');
|
||||
|
||||
// Find the generate_local_persistent_volumes_only_volume_names method
|
||||
$methodStart = strpos($deploymentJobFile, 'function generate_local_persistent_volumes_only_volume_names()');
|
||||
$methodEnd = strpos($deploymentJobFile, 'function generate_healthcheck_commands()');
|
||||
$methodBlock = substr($deploymentJobFile, $methodStart, $methodEnd - $methodStart);
|
||||
|
||||
// Should check is_preview_suffix_enabled before applying suffix
|
||||
expect($methodBlock)
|
||||
->toContain('is_preview_suffix_enabled')
|
||||
->toContain('$isPreviewSuffixEnabled')
|
||||
->toContain('addPreviewDeploymentSuffix');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue