Skip to content

Commit

Permalink
Fix 500 on duplicate ports (#861)
Browse files Browse the repository at this point in the history 
* Fix 500 on duplicate ports

This should also address N+1 issues from the last PR

* Combine into one method

* Pint

* Add missing type

* Add 0.0.0.0

* Add notifications to help the user

* Pint

* Too verbose

* Show notification here

* Simplify code

* Reset the ports if the ip changes

* Don’t limit these anymore

---------

Co-authored-by: Lance Pioch <[email protected]>
  • Loading branch information
@notAreYouScared @lancepioch
notAreYouScared and lancepioch authored Jan 5, 2025
1 parent 168d37b commit 7cc4358
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 176 deletions.
56 changes: 8 additions & 48 deletions app/Filament/Admin/Resources/NodeResource/RelationManagers/AllocationsRelationManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

namespace App\Filament\Admin\Resources\NodeResource\RelationManagers;

use App\Filament\Admin\Resources\ServerResource\Pages\CreateServer;
use App\Models\Allocation;
use App\Models\Node;
use App\Services\Allocations\AssignmentService;
use Filament\Forms\Components\Select;
use Filament\Forms\Components\TagsInput;
use Filament\Forms\Components\TextInput;
use Filament\Forms\Form;
use Filament\Forms\Get;
use Filament\Forms\Set;
use Filament\Resources\RelationManagers\RelationManager;
use Filament\Tables;
Expand Down Expand Up @@ -80,6 +82,8 @@ public function table(Table $table): Table
->inlineLabel()
->ipv4()
->helperText("Usually your machine's public IP unless you are port forwarding.")
->afterStateUpdated(fn (Set $set) => $set('allocation_ports', []))
->live()
->required(),
TextInput::make('allocation_alias')
->label('Alias')
Expand All @@ -97,54 +101,10 @@ public function table(Table $table): Table
->label('Ports')
->inlineLabel()
->live()
->afterStateUpdated(function ($state, Set $set) {
$ports = collect();
$update = false;
foreach ($state as $portEntry) {
if (!str_contains($portEntry, '-')) {
if (is_numeric($portEntry)) {
$ports->push((int) $portEntry);

continue;
}

// Do not add non numerical ports
$update = true;

continue;
}

$update = true;
[$start, $end] = explode('-', $portEntry);
if (!is_numeric($start) || !is_numeric($end)) {
continue;
}

$start = max((int) $start, 0);
$end = min((int) $end, 2 ** 16 - 1);
foreach (range($start, $end) as $i) {
$ports->push($i);
}
}

$uniquePorts = $ports->unique()->values();
if ($ports->count() > $uniquePorts->count()) {
$update = true;
$ports = $uniquePorts;
}

$sortedPorts = $ports->sort()->values();
if ($sortedPorts->all() !== $ports->all()) {
$update = true;
$ports = $sortedPorts;
}

$ports = $ports->filter(fn ($port) => $port > 1024 && $port < 65535)->values();

if ($update) {
$set('allocation_ports', $ports->all());
}
})
->disabled(fn (Get $get) => empty($get('allocation_ip')))
->afterStateUpdated(fn ($state, Set $set, Get $get) => $set('allocation_ports',
CreateServer::retrieveValidPorts($this->getOwnerRecord(), $state, $get('allocation_ip')))
)
->splitKeys(['Tab', ' ', ','])
->required(),
])
Expand Down
205 changes: 125 additions & 80 deletions app/Filament/Admin/Resources/ServerResource/Pages/CreateServer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,86 +198,47 @@ public function form(Form $form): Form
->where('node_id', $get('node_id'))
->whereNull('server_id'),
)
->createOptionForm(fn (Get $get) => [
Select::make('allocation_ip')
->options(collect(Node::find($get('node_id'))?->ipAddresses())->mapWithKeys(fn (string $ip) => [$ip => $ip]))
->label('IP Address')
->inlineLabel()
->ipv4()
->helperText("Usually your machine's public IP unless you are port forwarding.")
->required(),
TextInput::make('allocation_alias')
->label('Alias')
->inlineLabel()
->default(null)
->datalist([
$get('name'),
Egg::find($get('egg_id'))?->name,
])
->helperText('Optional display name to help you remember what these are.')
->required(false),
TagsInput::make('allocation_ports')
->placeholder('Examples: 27015, 27017-27019')
->helperText(new HtmlString('
These are the ports that users can connect to this Server through.
<br />
You would have to port forward these on your home network.
'))
->label('Ports')
->inlineLabel()
->live()
->afterStateUpdated(function ($state, Set $set) {
$ports = collect();
$update = false;
foreach ($state as $portEntry) {
if (!str_contains($portEntry, '-')) {
if (is_numeric($portEntry)) {
$ports->push((int) $portEntry);

continue;
}

// Do not add non-numerical ports
$update = true;

continue;
}

$update = true;
[$start, $end] = explode('-', $portEntry);
if (!is_numeric($start) || !is_numeric($end)) {
continue;
}

$start = max((int) $start, 0);
$end = min((int) $end, 2 ** 16 - 1);
$range = $start <= $end ? range($start, $end) : range($end, $start);
foreach ($range as $i) {
if ($i > 1024 && $i <= 65535) {
$ports->push($i);
}
}
}

$uniquePorts = $ports->unique()->values();
if ($ports->count() > $uniquePorts->count()) {
$update = true;
$ports = $uniquePorts;
}

$sortedPorts = $ports->sort()->values();
if ($sortedPorts->all() !== $ports->all()) {
$update = true;
$ports = $sortedPorts;
}

if ($update) {
$set('allocation_ports', $ports->all());
}
})
->splitKeys(['Tab', ' ', ','])
->required(),
])
->createOptionForm(function (Get $get) {
$getPage = $get;

return [
Select::make('allocation_ip')
->options(collect(Node::find($get('node_id'))?->ipAddresses())->mapWithKeys(fn (string $ip) => [$ip => $ip]))
->label('IP Address')
->helperText("Usually your machine's public IP unless you are port forwarding.")
->afterStateUpdated(fn (Set $set) => $set('allocation_ports', []))
->inlineLabel()
->ipv4()
->live()
->required(),
TextInput::make('allocation_alias')
->label('Alias')
->inlineLabel()
->default(null)
->datalist([
$get('name'),
Egg::find($get('egg_id'))?->name,
])
->helperText('Optional display name to help you remember what these are.')
->required(false),
TagsInput::make('allocation_ports')
->placeholder('Examples: 27015, 27017-27019')
->helperText(new HtmlString('
These are the ports that users can connect to this Server through.
<br />
You would have to port forward these on your home network.
'))
->label('Ports')
->inlineLabel()
->live()
->disabled(fn (Get $get) => empty($get('allocation_ip')))
->afterStateUpdated(fn ($state, Set $set, Get $get) => $set('allocation_ports',
CreateServer::retrieveValidPorts(Node::find($getPage('node_id')), $state, $get('allocation_ip')))
)
->splitKeys(['Tab', ' ', ','])
->required(),
];
})
->createOptionUsing(function (array $data, Get $get, AssignmentService $assignmentService): int {
return collect(
$assignmentService->handle(Node::find($get('node_id')), $data)
Expand Down Expand Up @@ -922,4 +883,88 @@ private function getSelectOptionsFromRules(Get $get): array
->mapWithKeys(fn ($value) => [$value => $value])
->all();
}

public static function retrieveValidPorts(Node $node, array $portEntries, string $ip): array
{
$portRangeLimit = AssignmentService::PORT_RANGE_LIMIT;
$portFloor = AssignmentService::PORT_FLOOR;
$portCeil = AssignmentService::PORT_CEIL;

$ports = collect();

$existingPorts = $node
->allocations()
->where('ip', $ip)
->pluck('port')
->all();

foreach ($portEntries as $portEntry) {
$start = $end = $portEntry;
if (str_contains($portEntry, '-')) {
[$start, $end] = explode('-', $portEntry);
}

if (!is_numeric($start) || !is_numeric($end)) {
Notification::make()
->title('Invalid Port Range')
->danger()
->body("Your port range are not valid integers: $portEntry")
->send();

continue;
}

$start = (int) $start;
$end = (int) $end;
$range = $start <= $end ? range($start, $end) : range($end, $start);

if (count($range) > $portRangeLimit) {
Notification::make()
->title('Too many ports at one time!')
->danger()
->body("The current limit is $portRangeLimit number of ports at one time.")
->send();

continue;
}

foreach ($range as $i) {
// Invalid port number
if ($i <= $portFloor || $i > $portCeil) {
Notification::make()
->title('Port not in valid range')
->danger()
->body("$i is not in the valid port range between $portFloor-$portCeil")
->send();

continue;
}

// Already exists
if (in_array($i, $existingPorts)) {
Notification::make()
->title('Port already in use')
->danger()
->body("$i is already with an allocation")
->send();

continue;
}

$ports->push($i);
}
}

$uniquePorts = $ports->unique()->values();
if ($ports->count() > $uniquePorts->count()) {
$ports = $uniquePorts;
}

$sortedPorts = $ports->sort()->values();
if ($sortedPorts->all() !== $ports->all()) {
$ports = $sortedPorts;
}

return $ports->all();
}
}
54 changes: 6 additions & 48 deletions app/Filament/Admin/Resources/ServerResource/RelationManagers/AllocationsRelationManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

namespace App\Filament\Admin\Resources\ServerResource\RelationManagers;

use App\Filament\Admin\Resources\ServerResource\Pages\CreateServer;
use App\Models\Allocation;
use App\Models\Server;
use App\Services\Allocations\AssignmentService;
use Filament\Forms\Components\Select;
use Filament\Forms\Components\TagsInput;
use Filament\Forms\Components\TextInput;
use Filament\Forms\Form;
use Filament\Forms\Get;
use Filament\Forms\Set;
use Filament\Resources\RelationManagers\RelationManager;
use Filament\Tables;
Expand Down Expand Up @@ -78,6 +80,7 @@ public function table(Table $table): Table
->inlineLabel()
->ipv4()
->helperText("Usually your machine's public IP unless you are port forwarding.")
->afterStateUpdated(fn (Set $set) => $set('allocation_ports', []))
->required(),
TextInput::make('allocation_alias')
->label('Alias')
Expand All @@ -95,54 +98,9 @@ public function table(Table $table): Table
->label('Ports')
->inlineLabel()
->live()
->afterStateUpdated(function ($state, Set $set) {
$ports = collect();
$update = false;
foreach ($state as $portEntry) {
if (!str_contains($portEntry, '-')) {
if (is_numeric($portEntry)) {
$ports->push((int) $portEntry);

continue;
}

// Do not add non numerical ports
$update = true;

continue;
}

$update = true;
[$start, $end] = explode('-', $portEntry);
if (!is_numeric($start) || !is_numeric($end)) {
continue;
}

$start = max((int) $start, 0);
$end = min((int) $end, 2 ** 16 - 1);
foreach (range($start, $end) as $i) {
$ports->push($i);
}
}

$uniquePorts = $ports->unique()->values();
if ($ports->count() > $uniquePorts->count()) {
$update = true;
$ports = $uniquePorts;
}

$sortedPorts = $ports->sort()->values();
if ($sortedPorts->all() !== $ports->all()) {
$update = true;
$ports = $sortedPorts;
}

$ports = $ports->filter(fn ($port) => $port > 1024 && $port < 65535)->values();

if ($update) {
$set('allocation_ports', $ports->all());
}
})
->afterStateUpdated(fn ($state, Set $set, Get $get) => $set('allocation_ports',
CreateServer::retrieveValidPorts($this->getOwnerRecord()->node, $state, $get('allocation_ip')))
)
->splitKeys(['Tab', ' ', ','])
->required(),
])
Expand Down
2 changes: 2 additions & 0 deletions app/Models/Node.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ public function ipAddresses(): array
// pass
}

$ips->push('0.0.0.0');

// Only IPV4
$ips = $ips->filter(fn (string $ip) => filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false);

Expand Down

0 comments on commit 7cc4358

Please sign in to comment.