From 99d5aa9585dafeb934161ad2d94a2a3ccc05e605 Mon Sep 17 00:00:00 2001 From: Robert Deutz Date: Thu, 8 May 2025 17:50:00 +0200 Subject: [PATCH 1/6] add loop detection #25 --- app/Console/Commands/PerformUpdate.php | 9 +++++++++ config/autoupdates.php | 1 + 2 files changed, 10 insertions(+) diff --git a/app/Console/Commands/PerformUpdate.php b/app/Console/Commands/PerformUpdate.php index 59b7b95..2eb3416 100644 --- a/app/Console/Commands/PerformUpdate.php +++ b/app/Console/Commands/PerformUpdate.php @@ -6,6 +6,7 @@ use App\Models\Site; use Illuminate\Console\Command; use App\Console\Traits\RequestTargetVersion; +use Illuminate\Support\Facades\Log; class PerformUpdate extends Command { @@ -35,6 +36,14 @@ public function handle(): int /** @var Site $site */ $site = Site::findOrFail($this->input->getArgument('siteId')); + $updateCount = $site->updates()->where('new_version', $targetVersion)->count(); + + if ($updateCount >= config('autoupdates.max_update_tries')) { + Log::info("Update Loop detected for Site: " . $site->id . '; TargetVersion: ' . $targetVersion); + + return Command::SUCCESS; + } + UpdateSite::dispatchSync( $site, $targetVersion diff --git a/config/autoupdates.php b/config/autoupdates.php index 9f60fd5..0f215b9 100644 --- a/config/autoupdates.php +++ b/config/autoupdates.php @@ -4,4 +4,5 @@ 'healthcheck_interval' => env('HEALTH_CHECK_INTERVAL', 24), 'cleanup_site_delay' => env('CLEANUP_SITE_DELAY', 7), 'tuf_repo_cachetime' => env('TUF_REPO_CACHETIME', 5), + 'max_update_tries' => env('MAX_UPDATE_TRIES', 5), ]; From 894e95a46fd2b1b956110c1a81e16775ef4785ff Mon Sep 17 00:00:00 2001 From: Robert Deutz Date: Thu, 8 May 2025 18:19:26 +0200 Subject: [PATCH 2/6] move stuff and add tests --- app/Console/Commands/PerformUpdate.php | 8 -------- app/Jobs/UpdateSite.php | 9 +++++++++ app/Models/Site.php | 5 +++++ tests/Unit/Jobs/UpdateSiteTest.php | 21 ++++++++++++++++++++- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/app/Console/Commands/PerformUpdate.php b/app/Console/Commands/PerformUpdate.php index 2eb3416..b01a12a 100644 --- a/app/Console/Commands/PerformUpdate.php +++ b/app/Console/Commands/PerformUpdate.php @@ -36,14 +36,6 @@ public function handle(): int /** @var Site $site */ $site = Site::findOrFail($this->input->getArgument('siteId')); - $updateCount = $site->updates()->where('new_version', $targetVersion)->count(); - - if ($updateCount >= config('autoupdates.max_update_tries')) { - Log::info("Update Loop detected for Site: " . $site->id . '; TargetVersion: ' . $targetVersion); - - return Command::SUCCESS; - } - UpdateSite::dispatchSync( $site, $targetVersion diff --git a/app/Jobs/UpdateSite.php b/app/Jobs/UpdateSite.php index e11286b..9c71d72 100644 --- a/app/Jobs/UpdateSite.php +++ b/app/Jobs/UpdateSite.php @@ -9,6 +9,7 @@ use App\Models\Update; use App\RemoteSite\Connection; use App\RemoteSite\Responses\PrepareUpdate; +use Illuminate\Console\Command; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Facades\App; @@ -31,6 +32,14 @@ public function __construct(protected readonly Site $site, protected string $tar */ public function handle(): void { + $updateCount = $this->site->getUpdateCount($this->targetVersion); + + if ($updateCount >= config('autoupdates.max_update_tries')) { + Log::info("Update Loop detected for Site: " . $this->site->id . '; TargetVersion: ' . $this->targetVersion); + + return; + } + /** @var Connection $connection */ $connection = $this->site->connection; diff --git a/app/Models/Site.php b/app/Models/Site.php index 9df3285..66a4fcf 100644 --- a/app/Models/Site.php +++ b/app/Models/Site.php @@ -60,6 +60,11 @@ public function getFrontendStatus(): int return $httpClient->get($this->url)->getStatusCode(); } + public function getUpdateCount(string $targetVersion): int + { + return $this->updates()->where('new_version', $targetVersion)->count(); + } + /** * @return HasMany */ diff --git a/tests/Unit/Jobs/UpdateSiteTest.php b/tests/Unit/Jobs/UpdateSiteTest.php index 75c49a3..e22c4c9 100644 --- a/tests/Unit/Jobs/UpdateSiteTest.php +++ b/tests/Unit/Jobs/UpdateSiteTest.php @@ -58,6 +58,24 @@ public function testJobQuitsIfNoUpdateIsAvailable() $this->assertTrue(true); } + public function testJobQuitsIfWeDetectALoopForAVersion() + { + $site = $this->getSiteMock([], null, 6); + + Log::spy(); + + $object = new UpdateSite($site, "1.0.1"); + $object->handle(); + + Log::shouldHaveReceived('info') + ->once() + ->withArgs(function ($message) { + return str_contains($message, 'Update Loop detected for Site'); + }); + + $this->assertTrue(true); + } + public function testJobQuitsIfAvailableUpdateDoesNotMatchTargetVersion() { $site = $this->getSiteMock( @@ -144,7 +162,7 @@ public function testJobWritesSuccessLogForSuccessfulJobs() $object->handle(); } - protected function getSiteMock(array $responses, array $expectedLogRow = null) + protected function getSiteMock(array $responses, array $expectedLogRow = null, int $updateCount = 0) { $connectionMock = $this->getMockBuilder(Connection::class) ->disableOriginalConstructor() @@ -183,6 +201,7 @@ function ($method) use ($responses) { $siteMock->method('updates')->willReturn($updateMock); $siteMock->method('getConnectionAttribute')->willReturn($connectionMock); $siteMock->method('getFrontendStatus')->willReturn(200); + $siteMock->method('getUpdateCount')->willReturn($updateCount); $siteMock->id = 1; $siteMock->url = "http://example.org"; $siteMock->cms_version = "1.0.0"; From 113115ca23f5ca0183d6593e5fb2615d37f42c2e Mon Sep 17 00:00:00 2001 From: Robert Deutz Date: Thu, 8 May 2025 18:00:46 +0200 Subject: [PATCH 3/6] Implement major version check (#28) * implement major version check * add test --- app/Jobs/UpdateSite.php | 10 ++++++++++ tests/Unit/Jobs/UpdateSiteTest.php | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/app/Jobs/UpdateSite.php b/app/Jobs/UpdateSite.php index 9c71d72..5798d75 100644 --- a/app/Jobs/UpdateSite.php +++ b/app/Jobs/UpdateSite.php @@ -53,6 +53,16 @@ public function handle(): void return; } + // Do not make a major version update + $majorVersionCms = (int) $healthResult->cms_version; + $majorTargetVersion = (int) $this->targetVersion; + + if ($majorVersionCms <> $majorTargetVersion) { + Log::info("No major update for Site: " . $this->site->id); + + return; + } + // Store pre-update response code $this->preUpdateCode = $this->site->getFrontendStatus(); diff --git a/tests/Unit/Jobs/UpdateSiteTest.php b/tests/Unit/Jobs/UpdateSiteTest.php index e22c4c9..88a68e0 100644 --- a/tests/Unit/Jobs/UpdateSiteTest.php +++ b/tests/Unit/Jobs/UpdateSiteTest.php @@ -76,6 +76,28 @@ public function testJobQuitsIfWeDetectALoopForAVersion() $this->assertTrue(true); } +public function testJobQuitsIfAvailableUpdateWouldBeAMajorUpdate() + { + $site = $this->getSiteMock( + [ + 'checkHealth' => $this->getHealthCheckMock(), + 'getUpdate' => $this->getGetUpdateMock("2.0.0") + ] + ); + + Log::spy(); + + $object = new UpdateSite($site, "2.0.0"); + $object->handle(); + + Log::shouldHaveReceived('info') + ->once() + ->withArgs(function ($message) { + return str_contains($message, 'No major update for Site'); + }); + + $this->assertTrue(true); + } public function testJobQuitsIfAvailableUpdateDoesNotMatchTargetVersion() { $site = $this->getSiteMock( From 9071bbcf6dd2a6da2f69fc6e7e2e6f7f5b6dea94 Mon Sep 17 00:00:00 2001 From: Robert Deutz Date: Thu, 8 May 2025 18:19:26 +0200 Subject: [PATCH 4/6] move stuff and add tests From 5866f03f80dd806ec715c825ecd76bdcde38830b Mon Sep 17 00:00:00 2001 From: Robert Deutz Date: Thu, 8 May 2025 18:21:19 +0200 Subject: [PATCH 5/6] mock --- tests/Unit/Jobs/UpdateSiteTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Jobs/UpdateSiteTest.php b/tests/Unit/Jobs/UpdateSiteTest.php index 88a68e0..5098085 100644 --- a/tests/Unit/Jobs/UpdateSiteTest.php +++ b/tests/Unit/Jobs/UpdateSiteTest.php @@ -217,7 +217,7 @@ function ($method) use ($responses) { } $siteMock = $this->getMockBuilder(Site::class) - ->onlyMethods(['getConnectionAttribute', 'getFrontendStatus', 'updates']) + ->onlyMethods(['getConnectionAttribute', 'getFrontendStatus', 'getUpdateCount', 'updates']) ->getMock(); $siteMock->method('updates')->willReturn($updateMock); From 4c77b52618c800bce970169c7ef6b9d323f922d1 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Thu, 8 May 2025 18:31:11 +0200 Subject: [PATCH 6/6] cs fix --- app/Console/Commands/PerformUpdate.php | 1 - app/Jobs/UpdateSite.php | 2 -- 2 files changed, 3 deletions(-) diff --git a/app/Console/Commands/PerformUpdate.php b/app/Console/Commands/PerformUpdate.php index b01a12a..59b7b95 100644 --- a/app/Console/Commands/PerformUpdate.php +++ b/app/Console/Commands/PerformUpdate.php @@ -6,7 +6,6 @@ use App\Models\Site; use Illuminate\Console\Command; use App\Console\Traits\RequestTargetVersion; -use Illuminate\Support\Facades\Log; class PerformUpdate extends Command { diff --git a/app/Jobs/UpdateSite.php b/app/Jobs/UpdateSite.php index 5798d75..bff698b 100644 --- a/app/Jobs/UpdateSite.php +++ b/app/Jobs/UpdateSite.php @@ -6,10 +6,8 @@ use App\Exceptions\UpdateException; use App\Models\Site; -use App\Models\Update; use App\RemoteSite\Connection; use App\RemoteSite\Responses\PrepareUpdate; -use Illuminate\Console\Command; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Facades\App;