From 1b1e62cf993b16386fd95b6c778cd886ee194182 Mon Sep 17 00:00:00 2001
From: Eric Wang <37554696+ericwang401@users.noreply.github.com>
Date: Mon, 4 Mar 2024 15:57:15 -0600
Subject: [PATCH] Scope VMID check to a node

---
 .../Admin/Servers/StoreServerRequest.php      |  1 +
 .../Eloquent/ServerRepository.php             | 23 +++++++++++++------
 .../Servers/ServerCreationService.php         | 10 ++++----
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/app/Http/Requests/Admin/Servers/StoreServerRequest.php b/app/Http/Requests/Admin/Servers/StoreServerRequest.php
index ed166ec01f7..c65f91bd06c 100644
--- a/app/Http/Requests/Admin/Servers/StoreServerRequest.php
+++ b/app/Http/Requests/Admin/Servers/StoreServerRequest.php
@@ -23,6 +23,7 @@ public function rules(): array
             'name' => $rules['name'],
             'user_id' => $rules['user_id'],
             'node_id' => $rules['node_id'],
+            // TODO: validation should be added for manually setting the vmid
             'vmid' => 'present|nullable|numeric|min:100|max:999999999',
             'hostname' => $rules['hostname'],
             'limits' => 'required|array',
diff --git a/app/Repositories/Eloquent/ServerRepository.php b/app/Repositories/Eloquent/ServerRepository.php
index cbf0ab03ba9..70ee2d374d0 100644
--- a/app/Repositories/Eloquent/ServerRepository.php
+++ b/app/Repositories/Eloquent/ServerRepository.php
@@ -2,11 +2,11 @@
 
 namespace Convoy\Repositories\Eloquent;
 
+use Convoy\Contracts\Repository\ServerRepositoryInterface;
+use Convoy\Exceptions\Repository\RecordNotFoundException;
 use Convoy\Models\Server;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
-use Convoy\Exceptions\Repository\RecordNotFoundException;
-use Convoy\Contracts\Repository\ServerRepositoryInterface;
 
 class ServerRepository extends EloquentRepository implements ServerRepositoryInterface
 {
@@ -15,12 +15,21 @@ public function model(): string
         return Server::class;
     }
 
+    public function isUniqueVmId(int $nodeId, int $vmid): bool
+    {
+        return !$this->getBuilder()
+                     ->where('vmid', '=', $vmid)
+                     ->where('node_id', '=', $nodeId)
+                     ->exists();
+    }
+
     /**
      * Check if a given UUID and UUID-Short string are unique to a server.
      */
     public function isUniqueUuidCombo(string $uuid, string $short): bool
     {
-        return ! $this->getBuilder()->where('uuid', '=', $uuid)->orWhere('uuid_short', '=', $short)->exists();
+        return !$this->getBuilder()->where('uuid', '=', $uuid)->orWhere('uuid_short', '=', $short)
+                     ->exists();
     }
 
     /**
@@ -33,10 +42,10 @@ public function getByUuid(string $uuid): Server
         try {
             /** @var Server $model */
             $model = $this->getBuilder()
-                ->where(function (Builder $query) use ($uuid) {
-                    $query->where('uuid_short', $uuid)->orWhere('uuid', $uuid);
-                })
-                ->firstOrFail($this->getColumns());
+                          ->where(function (Builder $query) use ($uuid) {
+                              $query->where('uuid_short', $uuid)->orWhere('uuid', $uuid);
+                          })
+                          ->firstOrFail($this->getColumns());
 
             return $model;
         } catch (ModelNotFoundException $exception) {
diff --git a/app/Services/Servers/ServerCreationService.php b/app/Services/Servers/ServerCreationService.php
index a9a4d161ebe..8c59d0743ba 100644
--- a/app/Services/Servers/ServerCreationService.php
+++ b/app/Services/Servers/ServerCreationService.php
@@ -42,14 +42,16 @@ public function handle(array $data)
             }
         }
 
+        $nodeId = Arr::get($data, 'node_id');
+
         $server = Server::create([
             'uuid' => $uuid,
             'uuid_short' => substr($uuid, 0, 8),
             'status' => $shouldCreateServer ? Status::INSTALLING->value : null,
             'name' => Arr::get($data, 'name'),
             'user_id' => Arr::get($data, 'user_id'),
-            'node_id' => Arr::get($data, 'node_id'),
-            'vmid' => Arr::get($data, 'vmid') ?? $this->generateUniqueVmId(),
+            'node_id' => $nodeId,
+            'vmid' => Arr::get($data, 'vmid') ?? $this->generateUniqueVmId($nodeId),
             'hostname' => Arr::get($data, 'hostname'),
             'cpu' => Arr::get($data, 'limits.cpu'),
             'memory' => Arr::get($data, 'limits.memory'),
@@ -78,12 +80,12 @@ public function handle(array $data)
         return $server;
     }
 
-    public function generateUniqueVmId(): int
+    public function generateUniqueVmId(int $nodeId): int
     {
         $vmid = random_int(100, 999999999);
         $attempts = 0;
 
-        while ($this->repository->getBuilder()->where('vmid', '=', $vmid)->exists()) {
+        while (!$this->repository->isUniqueVmId($nodeId, $vmid)) {
             $vmid = random_int(100, 999999999);
 
             if ($attempts++ > 10) {