From 5a3669cb26e3df26f1e32e741b8280ad23858c61 Mon Sep 17 00:00:00 2001 From: Lance Pioch Date: Tue, 8 Sep 2026 22:33:50 -0400 Subject: [PATCH 1/2] Add Application API tests for nodes, servers, mounts, allocations, and database hosts Each controller gets index, view, store, update, delete, validation error, and permission denial coverage, mirroring the existing user and egg tests. Wings calls are mocked through DaemonServerRepository and the database host connection check through a proxied partial of the database manager. The new tests surfaced two API bugs. StoreMountRequest declared no rules, so every mount store request validated nothing and creation always failed with a 500; it now applies the model rules the same way UpdateMountRequest already does. The mounts table also has no column defaults for read_only and user_mountable, so an API create without those optional booleans failed the insert; the model now supplies false for both. Also un-skips the three bare markTestSkipped tests: the two database management tests run against a mocked remote connection (the cleanup test now asserts the transaction rollback, since the drop-on-failure logic it referenced no longer exists), and the build modification test passes as written. Adds an installer redirect test guarding RedirectIfNotInstalled. --- .../Application/Mounts/StoreMountRequest.php | 18 ++ app/Models/Mount.php | 11 ++ tests/Feature/InstallerRedirectTest.php | 22 +++ .../Application/AllocationControllerTest.php | 80 +++++++++ .../DatabaseHostControllerTest.php | 148 +++++++++++++++++ .../Api/Application/MountControllerTest.php | 145 ++++++++++++++++ .../Api/Application/NodeControllerTest.php | 133 +++++++++++++++ .../Api/Application/ServerControllerTest.php | 156 ++++++++++++++++++ .../DatabaseManagementServiceTest.php | 69 ++++---- .../Servers/BuildModificationServiceTest.php | 2 - 10 files changed, 752 insertions(+), 32 deletions(-) create mode 100644 tests/Feature/InstallerRedirectTest.php create mode 100644 tests/Integration/Api/Application/AllocationControllerTest.php create mode 100644 tests/Integration/Api/Application/DatabaseHostControllerTest.php create mode 100644 tests/Integration/Api/Application/MountControllerTest.php create mode 100644 tests/Integration/Api/Application/NodeControllerTest.php create mode 100644 tests/Integration/Api/Application/ServerControllerTest.php diff --git a/app/Http/Requests/Api/Application/Mounts/StoreMountRequest.php b/app/Http/Requests/Api/Application/Mounts/StoreMountRequest.php index 8de69c1eba..563a2c6934 100644 --- a/app/Http/Requests/Api/Application/Mounts/StoreMountRequest.php +++ b/app/Http/Requests/Api/Application/Mounts/StoreMountRequest.php @@ -5,10 +5,28 @@ use App\Http\Requests\Api\Application\ApplicationApiRequest; use App\Models\Mount; use App\Services\Acl\Api\AdminAcl; +use Dedoc\Scramble\Attributes\BodyParameter; +use Illuminate\Contracts\Validation\ValidationRule; +// The rules come straight off the model, so there is no rules array to hang comments on. +// Attributes are not inherited, so UpdateMountRequest repeats them against the same descriptions. +#[BodyParameter('name', description: 'Name the mount is shown under in the Panel.')] +#[BodyParameter('description', description: 'Free form text describing the mount.')] +#[BodyParameter('source', description: 'Absolute path on the node that is mounted into the server.')] +#[BodyParameter('target', description: 'Absolute path inside the server container the source appears at.')] +#[BodyParameter('read_only', description: 'Mount the source read only so servers cannot write to it.')] +#[BodyParameter('user_mountable', description: 'Whether server owners may attach this mount themselves.')] class StoreMountRequest extends ApplicationApiRequest { protected ?string $resource = Mount::RESOURCE_NAME; protected int $permission = AdminAcl::WRITE; + + /** + * @return array> + */ + public function rules(): array + { + return Mount::getRules(); + } } diff --git a/app/Models/Mount.php b/app/Models/Mount.php index 2b38ece77a..fbcffb4b65 100644 --- a/app/Models/Mount.php +++ b/app/Models/Mount.php @@ -53,6 +53,17 @@ class Mount extends Model implements Validatable */ protected $guarded = ['id']; + /** + * The database columns carry no defaults, so creating a mount without + * these booleans (the API marks them 'sometimes') would fail the insert. + * + * @var array + */ + protected $attributes = [ + 'read_only' => false, + 'user_mountable' => false, + ]; + /** * Rules verifying that the data being stored matches the expectations of the database. * diff --git a/tests/Feature/InstallerRedirectTest.php b/tests/Feature/InstallerRedirectTest.php new file mode 100644 index 0000000000..c826d09fb6 --- /dev/null +++ b/tests/Feature/InstallerRedirectTest.php @@ -0,0 +1,22 @@ + false]); + + $this->get('/admin')->assertRedirect(route('installer')); +}); + +it('does not redirect to the installer when installed', function () { + config(['app.installed' => true]); + + // A guest hits the auth layer instead of being sent to the installer. + $response = $this->get('/admin'); + + expect($response->headers->get('Location'))->not->toBe(route('installer')); +}); + +it('allows the installer route itself when not installed', function () { + config(['app.installed' => false]); + + $this->get(route('installer'))->assertOk(); +}); diff --git a/tests/Integration/Api/Application/AllocationControllerTest.php b/tests/Integration/Api/Application/AllocationControllerTest.php new file mode 100644 index 0000000000..251a93fa72 --- /dev/null +++ b/tests/Integration/Api/Application/AllocationControllerTest.php @@ -0,0 +1,80 @@ +node = Node::factory()->create(); + } + + public function test_list_node_allocations(): void + { + $allocations = Allocation::factory(2)->create(['node_id' => $this->node->id]); + + $response = $this->getJson("/api/application/nodes/{$this->node->id}/allocations"); + $response->assertStatus(Response::HTTP_OK); + $response->assertJsonCount(2, 'data'); + + foreach ($allocations as $allocation) { + $response->assertJsonFragment(['id' => $allocation->id, 'port' => $allocation->port]); + } + } + + public function test_create_allocations_for_single_ports_and_ranges(): void + { + $this->postJson("/api/application/nodes/{$this->node->id}/allocations", [ + 'ip' => '10.0.0.1', + 'ports' => ['25565', '25570-25572'], + ])->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertEquals(4, Allocation::query()->where('node_id', $this->node->id)->count()); + $this->assertDatabaseHas('allocations', ['node_id' => $this->node->id, 'ip' => '10.0.0.1', 'port' => 25565]); + $this->assertDatabaseHas('allocations', ['node_id' => $this->node->id, 'ip' => '10.0.0.1', 'port' => 25572]); + } + + public function test_create_allocations_requires_ports(): void + { + $this->postJson("/api/application/nodes/{$this->node->id}/allocations", [ + 'ip' => '10.0.0.1', + ])->assertUnprocessable(); + } + + public function test_delete_unassigned_allocation(): void + { + $allocation = Allocation::factory()->create(['node_id' => $this->node->id, 'server_id' => null]); + + $this->deleteJson("/api/application/nodes/{$this->node->id}/allocations/{$allocation->id}") + ->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseMissing('allocations', ['id' => $allocation->id]); + } + + public function test_assigned_allocation_cannot_be_deleted(): void + { + $server = $this->createServerModel(['node_id' => $this->node->id]); + + $this->deleteJson("/api/application/nodes/{$this->node->id}/allocations/{$server->allocation_id}") + ->assertStatus(Response::HTTP_BAD_REQUEST); + + $this->assertDatabaseHas('allocations', ['id' => $server->allocation_id]); + } + + public function test_error_returned_if_no_permission(): void + { + $this->createNewDefaultApiKey($this->getApiUser(), [Allocation::RESOURCE_NAME => AdminAcl::NONE]); + + $response = $this->getJson("/api/application/nodes/{$this->node->id}/allocations"); + $this->assertAccessDeniedJson($response); + } +} diff --git a/tests/Integration/Api/Application/DatabaseHostControllerTest.php b/tests/Integration/Api/Application/DatabaseHostControllerTest.php new file mode 100644 index 0000000000..26fc66c7b3 --- /dev/null +++ b/tests/Integration/Api/Application/DatabaseHostControllerTest.php @@ -0,0 +1,148 @@ +buildConnection()->getPdo(), + * so fake the remote connection while leaving the panel's own connection real. + */ + private function fakeRemoteDatabaseConnection(?Exception $exception = null): void + { + $connection = Mockery::mock(Connection::class); + + if ($exception) { + $connection->shouldReceive('getPdo')->andThrow($exception); + } else { + $connection->shouldReceive('getPdo')->andReturn(Mockery::mock(PDO::class)); + } + + $manager = Mockery::mock(app('db')); + $manager->shouldReceive('build')->andReturn($connection); + DB::swap($manager); + $this->app->instance('db', $manager); + } + + public function test_list_all_database_hosts(): void + { + $host = DatabaseHost::factory()->create(); + + $response = $this->getJson('/api/application/database-hosts'); + $response->assertStatus(Response::HTTP_OK); + $response->assertJsonCount(1, 'data'); + $response->assertJsonFragment(['id' => $host->id]); + } + + public function test_return_single_database_host(): void + { + $host = DatabaseHost::factory()->create(); + + $response = $this->getJson('/api/application/database-hosts/' . $host->id); + $response->assertStatus(Response::HTTP_OK); + $response->assertJson([ + 'object' => 'database_host', + 'attributes' => $this->getExpectedData(DatabaseHostData::class, $host), + ], true); + } + + public function test_create_database_host(): void + { + $this->fakeRemoteDatabaseConnection(); + + $response = $this->postJson('/api/application/database-hosts', [ + 'name' => 'api-db-host', + 'host' => '127.0.0.1', + 'port' => 3306, + 'username' => 'pelicanuser', + 'password' => 'secret1234', + ]); + + $response->assertStatus(Response::HTTP_CREATED); + $this->assertDatabaseHas('database_hosts', ['name' => 'api-db-host', 'host' => '127.0.0.1']); + } + + public function test_database_host_is_not_saved_when_connection_fails(): void + { + $this->fakeRemoteDatabaseConnection(new PDOException('Connection refused')); + + $this->postJson('/api/application/database-hosts', [ + 'name' => 'unreachable-host', + 'host' => '10.0.0.99', + 'port' => 3306, + 'username' => 'pelicanuser', + 'password' => 'secret1234', + ])->assertServerError(); + + $this->assertDatabaseMissing('database_hosts', ['name' => 'unreachable-host']); + } + + public function test_create_database_host_requires_host(): void + { + $this->postJson('/api/application/database-hosts', [ + 'name' => 'api-db-host', + 'port' => 3306, + 'username' => 'pelicanuser', + ])->assertUnprocessable(); + } + + public function test_update_database_host(): void + { + $this->fakeRemoteDatabaseConnection(); + + $host = DatabaseHost::factory()->create(); + + $response = $this->patchJson('/api/application/database-hosts/' . $host->id, [ + 'name' => 'renamed-db-host', + 'host' => $host->host, + 'port' => $host->port, + 'username' => $host->username, + ]); + + $response->assertStatus(Response::HTTP_OK); + $this->assertDatabaseHas('database_hosts', ['id' => $host->id, 'name' => 'renamed-db-host']); + } + + public function test_delete_database_host(): void + { + $host = DatabaseHost::factory()->create(); + + $this->deleteJson('/api/application/database-hosts/' . $host->id) + ->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseMissing('database_hosts', ['id' => $host->id]); + } + + public function test_error_returned_if_no_permission(): void + { + $host = DatabaseHost::factory()->create(); + $this->createNewDefaultApiKey($this->getApiUser(), [DatabaseHost::RESOURCE_NAME => AdminAcl::NONE]); + + $response = $this->getJson('/api/application/database-hosts/' . $host->id); + $this->assertAccessDeniedJson($response); + } + + public function test_api_key_without_write_permissions_cannot_create(): void + { + $this->createNewDefaultApiKey($this->getApiUser(), [DatabaseHost::RESOURCE_NAME => AdminAcl::READ]); + + $response = $this->postJson('/api/application/database-hosts', [ + 'name' => 'api-db-host', + 'host' => '127.0.0.1', + 'port' => 3306, + 'username' => 'pelicanuser', + ]); + $this->assertAccessDeniedJson($response); + } +} diff --git a/tests/Integration/Api/Application/MountControllerTest.php b/tests/Integration/Api/Application/MountControllerTest.php new file mode 100644 index 0000000000..29b8d56833 --- /dev/null +++ b/tests/Integration/Api/Application/MountControllerTest.php @@ -0,0 +1,145 @@ +create([ + 'uuid' => Str::uuid()->toString(), + 'name' => 'Mount ' . Str::random(8), + 'source' => '/mnt/' . Str::random(8), + 'target' => '/srv/' . Str::random(8), + 'read_only' => false, + 'user_mountable' => false, + ]); + } + + public function test_list_all_mounts(): void + { + $mount = $this->createMount(); + + $response = $this->getJson('/api/application/mounts'); + $response->assertStatus(Response::HTTP_OK); + $response->assertJsonCount(1, 'data'); + $response->assertJsonFragment(['id' => $mount->id]); + } + + public function test_return_single_mount(): void + { + $mount = $this->createMount(); + + $response = $this->getJson('/api/application/mounts/' . $mount->id); + $response->assertStatus(Response::HTTP_OK); + $response->assertJson([ + 'object' => 'mount', + 'attributes' => $this->getExpectedData(MountData::class, $mount), + ], true); + } + + public function test_create_mount(): void + { + $response = $this->postJson('/api/application/mounts', [ + 'name' => 'api-mount', + 'source' => '/mnt/api-source', + 'target' => '/srv/api-target', + ]); + + $response->assertStatus(Response::HTTP_CREATED); + $this->assertDatabaseHas('mounts', [ + 'name' => 'api-mount', + 'source' => '/mnt/api-source', + 'target' => '/srv/api-target', + ]); + } + + public function test_create_mount_requires_source(): void + { + $this->postJson('/api/application/mounts', [ + 'name' => 'api-mount', + 'target' => '/srv/api-target', + ])->assertUnprocessable(); + + $this->assertDatabaseMissing('mounts', ['name' => 'api-mount']); + } + + public function test_update_mount(): void + { + $mount = $this->createMount(); + + $response = $this->patchJson('/api/application/mounts/' . $mount->id, [ + 'name' => 'renamed-mount', + 'source' => $mount->source, + 'target' => $mount->target, + ]); + + $response->assertStatus(Response::HTTP_OK); + $this->assertDatabaseHas('mounts', ['id' => $mount->id, 'name' => 'renamed-mount']); + } + + public function test_mount_egg_and_node_relations_can_be_managed(): void + { + $mount = $this->createMount(); + $egg = Egg::query()->firstOrFail(); + $node = Node::factory()->create(); + + $eggRow = ['mount_id' => $mount->id, 'mountable_type' => 'egg', 'mountable_id' => $egg->id]; + $nodeRow = ['mount_id' => $mount->id, 'mountable_type' => 'node', 'mountable_id' => $node->id]; + + $this->postJson("/api/application/mounts/{$mount->id}/eggs", ['eggs' => [$egg->id]]) + ->assertSuccessful(); + $this->assertDatabaseHas('mountables', $eggRow); + + $this->postJson("/api/application/mounts/{$mount->id}/nodes", ['nodes' => [$node->id]]) + ->assertSuccessful(); + $this->assertDatabaseHas('mountables', $nodeRow); + + $this->deleteJson("/api/application/mounts/{$mount->id}/eggs/{$egg->id}") + ->assertStatus(Response::HTTP_NO_CONTENT); + $this->assertDatabaseMissing('mountables', $eggRow); + + $this->deleteJson("/api/application/mounts/{$mount->id}/nodes/{$node->id}") + ->assertStatus(Response::HTTP_NO_CONTENT); + $this->assertDatabaseMissing('mountables', $nodeRow); + } + + public function test_delete_mount(): void + { + $mount = $this->createMount(); + + $this->deleteJson('/api/application/mounts/' . $mount->id) + ->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseMissing('mounts', ['id' => $mount->id]); + } + + public function test_error_returned_if_no_permission(): void + { + $mount = $this->createMount(); + $this->createNewDefaultApiKey($this->getApiUser(), [Mount::RESOURCE_NAME => AdminAcl::NONE]); + + $response = $this->getJson('/api/application/mounts/' . $mount->id); + $this->assertAccessDeniedJson($response); + } + + public function test_api_key_without_write_permissions_cannot_create(): void + { + $this->createNewDefaultApiKey($this->getApiUser(), [Mount::RESOURCE_NAME => AdminAcl::READ]); + + $response = $this->postJson('/api/application/mounts', [ + 'name' => 'api-mount', + 'source' => '/mnt/api-source', + 'target' => '/srv/api-target', + ]); + $this->assertAccessDeniedJson($response); + } +} diff --git a/tests/Integration/Api/Application/NodeControllerTest.php b/tests/Integration/Api/Application/NodeControllerTest.php new file mode 100644 index 0000000000..efbc68c715 --- /dev/null +++ b/tests/Integration/Api/Application/NodeControllerTest.php @@ -0,0 +1,133 @@ + */ + private array $storePayload = [ + 'name' => 'api-node', + 'fqdn' => '10.0.0.1', + 'scheme' => 'http', + 'memory' => 0, + 'memory_overallocate' => 0, + 'disk' => 0, + 'disk_overallocate' => 0, + 'cpu' => 0, + 'cpu_overallocate' => 0, + 'daemon_sftp' => 2022, + 'daemon_listen' => 8080, + 'daemon_connect' => 8080, + 'upload_size' => 256, + ]; + + public function test_list_all_nodes(): void + { + $nodes = Node::factory(2)->create(); + + $response = $this->getJson('/api/application/nodes'); + $response->assertStatus(Response::HTTP_OK); + $response->assertJsonCount(2, 'data'); + + foreach ($nodes as $node) { + $response->assertJsonFragment(['id' => $node->id]); + } + } + + public function test_return_single_node(): void + { + $node = Node::factory()->create(); + + $response = $this->getJson('/api/application/nodes/' . $node->id); + $response->assertStatus(Response::HTTP_OK); + // Not strict: the response carries null fields the Data object omits. + $response->assertJson([ + 'object' => 'node', + 'attributes' => $this->getExpectedData(NodeData::class, $node), + ]); + } + + public function test_create_node(): void + { + $response = $this->postJson('/api/application/nodes', $this->storePayload); + + $response->assertStatus(Response::HTTP_CREATED); + $this->assertDatabaseHas('nodes', ['name' => 'api-node', 'fqdn' => '10.0.0.1']); + + $node = Node::query()->where('name', 'api-node')->firstOrFail(); + // The creation response omits attributes that are null, so compare without them. + $response->assertJson([ + 'object' => 'node', + 'attributes' => array_filter($this->getExpectedData(NodeData::class, $node), fn ($value) => $value !== null), + 'meta' => ['resource' => route('api.application.nodes.view', $node->id)], + ]); + } + + public function test_create_node_requires_fqdn(): void + { + $payload = $this->storePayload; + unset($payload['fqdn']); + + $this->postJson('/api/application/nodes', $payload) + ->assertUnprocessable() + ->assertJsonPath('errors.0.meta.source_field', 'fqdn'); + } + + public function test_update_node(): void + { + // The update service pings Wings about the config change but tolerates failure. + Http::fake(); + + $node = Node::factory()->create(); + + $response = $this->patchJson('/api/application/nodes/' . $node->id, array_merge($this->storePayload, [ + 'name' => 'renamed-node', + ])); + + $response->assertStatus(Response::HTTP_OK); + $this->assertDatabaseHas('nodes', ['id' => $node->id, 'name' => 'renamed-node']); + } + + public function test_delete_node(): void + { + $node = Node::factory()->create(); + + $this->deleteJson('/api/application/nodes/' . $node->id) + ->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseMissing('nodes', ['id' => $node->id]); + } + + public function test_node_with_servers_cannot_be_deleted(): void + { + $server = $this->createServerModel(); + + $this->deleteJson('/api/application/nodes/' . $server->node_id) + ->assertStatus(Response::HTTP_BAD_REQUEST); + + $this->assertDatabaseHas('nodes', ['id' => $server->node_id]); + } + + public function test_error_returned_if_no_permission(): void + { + $node = Node::factory()->create(); + $this->createNewDefaultApiKey($this->getApiUser(), [Node::RESOURCE_NAME => AdminAcl::NONE]); + + $response = $this->getJson('/api/application/nodes/' . $node->id); + $this->assertAccessDeniedJson($response); + } + + public function test_api_key_without_write_permissions_cannot_create(): void + { + $this->createNewDefaultApiKey($this->getApiUser(), [Node::RESOURCE_NAME => AdminAcl::READ]); + + $response = $this->postJson('/api/application/nodes', $this->storePayload); + $this->assertAccessDeniedJson($response); + } +} diff --git a/tests/Integration/Api/Application/ServerControllerTest.php b/tests/Integration/Api/Application/ServerControllerTest.php new file mode 100644 index 0000000000..2e94267e75 --- /dev/null +++ b/tests/Integration/Api/Application/ServerControllerTest.php @@ -0,0 +1,156 @@ +daemonServerRepository = Mockery::mock(DaemonServerRepository::class); + $this->swap(DaemonServerRepository::class, $this->daemonServerRepository); + } + + /** @return array */ + private function storePayload(): array + { + $node = Node::factory()->create(); + $allocation = Allocation::factory()->create(['node_id' => $node->id, 'server_id' => null]); + // A factory egg has no variables, so no required environment values get in the way. + $egg = Egg::factory()->create(); + + return [ + 'name' => 'api-server', + 'user' => $this->getApiUser()->id, + 'egg' => $egg->id, + 'docker_image' => 'ghcr.io/pelican-eggs/yolks:java_21', + 'startup' => 'java -jar test.jar', + 'environment' => [], + 'limits' => ['memory' => 0, 'swap' => 0, 'disk' => 0, 'io' => 500, 'cpu' => 0], + 'feature_limits' => ['databases' => 0, 'allocations' => 0, 'backups' => 0], + 'allocation' => ['default' => $allocation->id], + 'start_on_completion' => false, + ]; + } + + public function test_list_all_servers(): void + { + $server = $this->createServerModel(); + + $response = $this->getJson('/api/application/servers'); + $response->assertStatus(Response::HTTP_OK); + $response->assertJsonCount(1, 'data'); + $response->assertJsonFragment(['id' => $server->id, 'uuid' => $server->uuid]); + } + + public function test_return_single_server(): void + { + $server = $this->createServerModel(); + + $response = $this->getJson('/api/application/servers/' . $server->id); + $response->assertStatus(Response::HTTP_OK); + $response->assertJson([ + 'object' => 'server', + 'attributes' => $this->getExpectedData(ServerData::class, $server), + ], true); + } + + public function test_return_server_by_external_id(): void + { + $server = $this->createServerModel(['external_id' => 'ext-123']); + + $response = $this->getJson('/api/application/servers/external/ext-123'); + $response->assertStatus(Response::HTTP_OK); + $response->assertJsonFragment(['id' => $server->id]); + } + + public function test_create_server(): void + { + $this->daemonServerRepository->expects('setServer')->andReturnSelf(); + $this->daemonServerRepository->expects('create')->andReturnUndefined(); + + $response = $this->postJson('/api/application/servers', $this->storePayload()); + + $response->assertStatus(Response::HTTP_CREATED); + $this->assertDatabaseHas('servers', ['name' => 'api-server']); + } + + public function test_server_is_removed_when_daemon_creation_fails(): void + { + // The failed creation triggers a force delete, which talks to the daemon again. + $this->daemonServerRepository->shouldReceive('setServer')->andReturnSelf(); + $this->daemonServerRepository->expects('create')->andThrow(new ConnectionException()); + $this->daemonServerRepository->shouldReceive('delete'); + + $this->postJson('/api/application/servers', $this->storePayload()) + ->assertServerError(); + + $this->assertDatabaseMissing('servers', ['name' => 'api-server']); + } + + public function test_create_server_requires_limits(): void + { + $payload = $this->storePayload(); + unset($payload['limits']); + + $this->postJson('/api/application/servers', $payload)->assertUnprocessable(); + } + + public function test_delete_server(): void + { + $this->daemonServerRepository->expects('setServer')->andReturnSelf(); + $this->daemonServerRepository->expects('delete')->andReturnUndefined(); + + $server = $this->createServerModel(); + + $this->deleteJson('/api/application/servers/' . $server->id) + ->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseMissing('servers', ['id' => $server->id]); + } + + public function test_force_delete_server_survives_daemon_failure(): void + { + $this->daemonServerRepository->expects('setServer')->andReturnSelf(); + $this->daemonServerRepository->expects('delete')->andThrow(new ConnectionException()); + + $server = $this->createServerModel(); + + $this->deleteJson('/api/application/servers/' . $server->id . '/force') + ->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseMissing('servers', ['id' => $server->id]); + } + + public function test_error_returned_if_no_permission(): void + { + $server = $this->createServerModel(); + $this->createNewDefaultApiKey($this->getApiUser(), [Server::RESOURCE_NAME => AdminAcl::NONE]); + + $response = $this->getJson('/api/application/servers/' . $server->id); + $this->assertAccessDeniedJson($response); + } + + public function test_api_key_without_write_permissions_cannot_create(): void + { + $this->createNewDefaultApiKey($this->getApiUser(), [Server::RESOURCE_NAME => AdminAcl::READ]); + + $response = $this->postJson('/api/application/servers', $this->storePayload()); + $this->assertAccessDeniedJson($response); + } +} diff --git a/tests/Integration/Services/Databases/DatabaseManagementServiceTest.php b/tests/Integration/Services/Databases/DatabaseManagementServiceTest.php index d5a5f0e205..ede495f9da 100644 --- a/tests/Integration/Services/Databases/DatabaseManagementServiceTest.php +++ b/tests/Integration/Services/Databases/DatabaseManagementServiceTest.php @@ -9,6 +9,9 @@ use App\Models\DatabaseHost; use App\Services\Databases\DatabaseManagementService; use App\Tests\Integration\IntegrationTestCase; +use Illuminate\Database\Connection; +use Illuminate\Support\Facades\DB; +use Mockery\MockInterface; use PHPUnit\Framework\Attributes\DataProvider; class DatabaseManagementServiceTest extends IntegrationTestCase @@ -110,18 +113,14 @@ public function test_creating_database_with_identical_name_triggers_an_exception */ public function test_server_database_can_be_created(): void { - $this->markTestSkipped(); - /* TODO: The exception is because the transaction is closed - because the database create closes it early */ - $server = $this->createServerModel(); $name = DatabaseManagementService::generateUniqueDatabaseName('something', $server->id); $host = DatabaseHost::factory()->recycle($server->node)->create(); - $username = null; - $secondUsername = null; - $password = null; + // CREATE DATABASE, CREATE USER, GRANT, FLUSH PRIVILEGES + $this->fakeRemoteDatabaseConnection() + ->shouldReceive('statement')->times(4)->andReturnTrue(); $response = $this->getService()->create($server, [ 'remote' => '%', @@ -131,46 +130,56 @@ public function test_server_database_can_be_created(): void $this->assertInstanceOf(Database::class, $response); $this->assertSame($response->server_id, $server->id); - $this->assertMatchesRegularExpression('/^(u\d+_)(\w){10}$/', $username); - $this->assertSame($username, $secondUsername); - $this->assertSame(24, strlen($password)); + $this->assertMatchesRegularExpression('/^(u\d+_)(\w){10}$/', $response->username); + $this->assertSame(24, strlen($response->password)); $this->assertDatabaseHas('databases', ['server_id' => $server->id, 'id' => $response->id]); } /** - * Test that an exception encountered while creating the database leads to the cleanup code - * being called and any exceptions encountered while cleaning up go unreported. + * Test that an exception encountered while creating the database on the remote host + * rolls the panel-side record back. */ - public function test_exception_encountered_while_creating_database_attempts_to_cleanup(): void + public function test_exception_encountered_while_creating_database_rolls_back(): void { - $this->markTestSkipped(); - - /* TODO: I think this is useful logic to be tested, - but this is a very hacky way of going about it. - The exception is because the transaction is closed - because the database create closes it early */ - $server = $this->createServerModel(); $name = DatabaseManagementService::generateUniqueDatabaseName('something', $server->id); $host = DatabaseHost::factory()->recycle($server->node)->create(); - $this->repository->expects('createDatabase')->with($name)->andThrows(new \BadMethodCallException()); - $this->repository->expects('dropDatabase')->with($name); - $this->repository->expects('dropUser')->withAnyArgs()->andThrows(new \InvalidArgumentException()); - - $this->expectException(\BadMethodCallException::class); + $this->fakeRemoteDatabaseConnection() + ->shouldReceive('statement')->andThrow(new \BadMethodCallException()); - $this->getService()->create($server, [ - 'remote' => '%', - 'database' => $name, - 'database_host_id' => $host->id, - ]); + try { + $this->getService()->create($server, [ + 'remote' => '%', + 'database' => $name, + 'database_host_id' => $host->id, + ]); + $this->fail('Expected the remote statement exception to bubble up.'); + } catch (\BadMethodCallException) { + // Expected. + } $this->assertDatabaseMissing('databases', ['server_id' => $server->id]); } + /** + * The database services confirm access via DatabaseHost->buildConnection(), so fake the + * remote connection while leaving the panel's own connection real. + */ + private function fakeRemoteDatabaseConnection(): MockInterface + { + $connection = \Mockery::mock(Connection::class); + + $manager = \Mockery::mock(app('db')); + $manager->shouldReceive('build')->andReturn($connection); + DB::swap($manager); + $this->app->instance('db', $manager); + + return $connection; + } + public static function invalidDataDataProvider(): array { return [ diff --git a/tests/Integration/Services/Servers/BuildModificationServiceTest.php b/tests/Integration/Services/Servers/BuildModificationServiceTest.php index b66a6f97f1..306707b52b 100644 --- a/tests/Integration/Services/Servers/BuildModificationServiceTest.php +++ b/tests/Integration/Services/Servers/BuildModificationServiceTest.php @@ -154,8 +154,6 @@ public function test_server_build_data_is_properly_updated_ondaemon(): void */ public function test_connection_exception_is_ignored_when_updating_server_settings(): void { - $this->markTestSkipped(); - $server = $this->createServerModel(); $this->daemonServerRepository->expects('setServer->sync')->andThrows(new ConnectionException()); From c44f155264e6cf985f21ae17e989c6bf5607f96b Mon Sep 17 00:00:00 2001 From: Lance Pioch Date: Tue, 8 Sep 2026 22:55:23 -0400 Subject: [PATCH 2/2] Tighten test assertions per review Assert every port from the allocation range, require exactly one daemon delete call in the failed-creation cleanup test, and pin the 401 auth response for the installed-state installer test. --- tests/Feature/InstallerRedirectTest.php | 3 ++- .../Api/Application/AllocationControllerTest.php | 7 ++++--- tests/Integration/Api/Application/ServerControllerTest.php | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/Feature/InstallerRedirectTest.php b/tests/Feature/InstallerRedirectTest.php index c826d09fb6..a331317dfe 100644 --- a/tests/Feature/InstallerRedirectTest.php +++ b/tests/Feature/InstallerRedirectTest.php @@ -9,9 +9,10 @@ it('does not redirect to the installer when installed', function () { config(['app.installed' => true]); - // A guest hits the auth layer instead of being sent to the installer. + // A guest hits the auth layer (401) instead of being sent to the installer. $response = $this->get('/admin'); + $response->assertUnauthorized(); expect($response->headers->get('Location'))->not->toBe(route('installer')); }); diff --git a/tests/Integration/Api/Application/AllocationControllerTest.php b/tests/Integration/Api/Application/AllocationControllerTest.php index 251a93fa72..37f4043977 100644 --- a/tests/Integration/Api/Application/AllocationControllerTest.php +++ b/tests/Integration/Api/Application/AllocationControllerTest.php @@ -38,9 +38,10 @@ public function test_create_allocations_for_single_ports_and_ranges(): void 'ports' => ['25565', '25570-25572'], ])->assertStatus(Response::HTTP_NO_CONTENT); - $this->assertEquals(4, Allocation::query()->where('node_id', $this->node->id)->count()); - $this->assertDatabaseHas('allocations', ['node_id' => $this->node->id, 'ip' => '10.0.0.1', 'port' => 25565]); - $this->assertDatabaseHas('allocations', ['node_id' => $this->node->id, 'ip' => '10.0.0.1', 'port' => 25572]); + $this->assertEqualsCanonicalizing( + [25565, 25570, 25571, 25572], + Allocation::query()->where('node_id', $this->node->id)->where('ip', '10.0.0.1')->pluck('port')->all(), + ); } public function test_create_allocations_requires_ports(): void diff --git a/tests/Integration/Api/Application/ServerControllerTest.php b/tests/Integration/Api/Application/ServerControllerTest.php index 2e94267e75..b3d00526e5 100644 --- a/tests/Integration/Api/Application/ServerControllerTest.php +++ b/tests/Integration/Api/Application/ServerControllerTest.php @@ -95,7 +95,7 @@ public function test_server_is_removed_when_daemon_creation_fails(): void // The failed creation triggers a force delete, which talks to the daemon again. $this->daemonServerRepository->shouldReceive('setServer')->andReturnSelf(); $this->daemonServerRepository->expects('create')->andThrow(new ConnectionException()); - $this->daemonServerRepository->shouldReceive('delete'); + $this->daemonServerRepository->expects('delete')->andReturnUndefined(); $this->postJson('/api/application/servers', $this->storePayload()) ->assertServerError();