Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Port check when creating allocations #661

Closed
wants to merge 2 commits into from
Closed

Add Port check when creating allocations #661

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d966d8
Port check
notAreYouScared Oct 24, 2024
ca97cee
Update app/Filament/Resources/NodeResource/RelationManagers/Allocatio…
notAreYouScared Oct 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions app/Filament/Resources/NodeResource/RelationManagers/AllocationsRelationManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
use Filament\Forms\Components\TagsInput;
use Filament\Forms\Components\TextInput;
use Filament\Forms\Form;
use Filament\Forms\Get;
use Filament\Forms\Set;
use Filament\Notifications\Notification;
use Filament\Resources\RelationManagers\RelationManager;
use Filament\Tables;
use Filament\Tables\Actions\BulkActionGroup;
Expand Down Expand Up @@ -97,18 +99,26 @@ public function table(Table $table): Table
->label('Ports')
->inlineLabel()
->live()
->afterStateUpdated(function ($state, Set $set) {
->afterStateUpdated(function ($state, Set $set, Get $get) {
$ports = collect();
$update = false;
foreach ($state as $portEntry) {
if (!str_contains($portEntry, '-')) {
if (is_numeric($portEntry)) {
$ports->push((int) $portEntry);

continue;
if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to happen after the foreach loop because of the N+1 problem!

Notification::make()
->title('Port Already Exists')
->danger()
->body('Port ' . $portEntry . ' already exists.')
->send();
} else {
$ports->push((int) $portEntry);

continue;
}
}

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

continue;
Expand All @@ -122,8 +132,19 @@ public function table(Table $table): Table

$start = max((int) $start, 0);
$end = min((int) $end, 2 ** 16 - 1);
foreach (range($start, $end) as $i) {
$ports->push($i);
$range = $start <= $end ? range($start, $end) : range($end, $start);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the first port number is greater than the second port number, then no numbers should be added.

foreach ($range as $i) {
if ($i > 1024 && $i <= 65535) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invert the if statement and continue early!

if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to happen after the foreach loop because of the N+1 problem!

Notification::make()
->title('Port Already Exists')
->danger()
->body('Port ' . $portEntry . ' already exists.')
notAreYouScared marked this conversation as resolved.
Show resolved Hide resolved
->send();
} else {
$ports->push($i);
}
}
}
}

Expand All @@ -139,8 +160,6 @@ public function table(Table $table): Table
$ports = $sortedPorts;
}

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

if ($update) {
$set('allocation_ports', $ports->all());
}
Expand Down
27 changes: 22 additions & 5 deletions app/Filament/Resources/ServerResource/Pages/CreateServer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use Filament\Forms\Form;
use Filament\Forms\Get;
use Filament\Forms\Set;
use Filament\Notifications\Notification;
use Filament\Resources\Pages\CreateRecord;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;
Expand Down Expand Up @@ -219,15 +220,23 @@ public function form(Form $form): Form
->label('Ports')
->inlineLabel()
->live()
->afterStateUpdated(function ($state, Set $set) {
->afterStateUpdated(function ($state, Set $set, Get $get) {
$ports = collect();
$update = false;
foreach ($state as $portEntry) {
if (!str_contains($portEntry, '-')) {
if (is_numeric($portEntry)) {
$ports->push((int) $portEntry);

continue;
if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) {
Notification::make()
->title('Port Already Exists')
->danger()
->body('Port ' . $portEntry . ' already exists.')
->send();
} else {
$ports->push((int) $portEntry);

continue;
}
}

// Do not add non-numerical ports
Expand All @@ -247,7 +256,15 @@ public function form(Form $form): Form
$range = $start <= $end ? range($start, $end) : range($end, $start);
foreach ($range as $i) {
if ($i > 1024 && $i <= 65535) {
$ports->push($i);
if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to happen after the foreach loop because of the N+1 problem!

Notification::make()
->title('Port Already Exists')
->danger()
->body('Port ' . $portEntry . ' already exists.')
->send();
} else {
$ports->push($i);
}
}
}
}
Expand Down
35 changes: 27 additions & 8 deletions app/Filament/Resources/ServerResource/RelationManagers/AllocationsRelationManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
use Filament\Forms\Components\TagsInput;
use Filament\Forms\Components\TextInput;
use Filament\Forms\Form;
use Filament\Forms\Get;
use Filament\Forms\Set;
use Filament\Notifications\Notification;
use Filament\Resources\RelationManagers\RelationManager;
use Filament\Tables;
use Filament\Tables\Actions\Action;
Expand Down Expand Up @@ -93,18 +95,26 @@ public function table(Table $table): Table
->label('Ports')
->inlineLabel()
->live()
->afterStateUpdated(function ($state, Set $set) {
->afterStateUpdated(function ($state, Set $set, Get $get) {
$ports = collect();
$update = false;
foreach ($state as $portEntry) {
if (!str_contains($portEntry, '-')) {
if (is_numeric($portEntry)) {
$ports->push((int) $portEntry);
if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to happen after the foreach loop because of the N+1 problem!

Notification::make()
->title('Port Already Exists')
->danger()
->body('Port ' . $portEntry . ' already exists.')
->send();
} else {
$ports->push((int) $portEntry);

continue;
continue;
}
}

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

continue;
Expand All @@ -118,8 +128,19 @@ public function table(Table $table): Table

$start = max((int) $start, 0);
$end = min((int) $end, 2 ** 16 - 1);
foreach (range($start, $end) as $i) {
$ports->push($i);
$range = $start <= $end ? range($start, $end) : range($end, $start);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing as before.

foreach ($range as $i) {
if ($i > 1024 && $i <= 65535) {
if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to happen after the foreach loop because of the N+1 problem!

Notification::make()
->title('Port Already Exists')
->danger()
->body('Port ' . $portEntry . ' already exists.')
->send();
} else {
$ports->push($i);
}
}
}
}

Expand All @@ -135,8 +156,6 @@ public function table(Table $table): Table
$ports = $sortedPorts;
}

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

if ($update) {
$set('allocation_ports', $ports->all());
}
Expand Down
Loading