From 100570387dbf29f956880ff0e7b1eb29a604f873 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Wed, 15 Jan 2025 14:54:00 -0500 Subject: [PATCH 01/16] Add AWS S3 Adapter to Composer dependencies --- composer.json | 1 + composer.lock | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 6df571f69..f87f2fcc5 100644 --- a/composer.json +++ b/composer.json @@ -33,6 +33,7 @@ "laravel/socialite": "5.16.1", "laravel/ui": "4.6.0", "lcobucci/jwt": "5.4.2", + "league/flysystem-aws-s3-v3": "^3.29", "mll-lab/laravel-graphiql": "3.2.1", "nuwave/lighthouse": "6.47.0", "pear/archive_tar": "1.5.0", diff --git a/composer.lock b/composer.lock index 5f20ef99a..8af94ffed 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "3182e69a75a77c79a2211cc99d90e44c", + "content-hash": "1d62081b6e5763ae0d198716bea6edb1", "packages": [ { "name": "24slides/laravel-saml2", @@ -2947,6 +2947,61 @@ }, "time": "2024-10-08T08:58:34+00:00" }, + { + "name": "league/flysystem-aws-s3-v3", + "version": "3.29.0", + "source": { + "type": "git", + "url": "https://github.com/thephpleague/flysystem-aws-s3-v3.git", + "reference": "c6ff6d4606e48249b63f269eba7fabdb584e76a9" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/thephpleague/flysystem-aws-s3-v3/zipball/c6ff6d4606e48249b63f269eba7fabdb584e76a9", + "reference": "c6ff6d4606e48249b63f269eba7fabdb584e76a9", + "shasum": "" + }, + "require": { + "aws/aws-sdk-php": "^3.295.10", + "league/flysystem": "^3.10.0", + "league/mime-type-detection": "^1.0.0", + "php": "^8.0.2" + }, + "conflict": { + "guzzlehttp/guzzle": "<7.0", + "guzzlehttp/ringphp": "<1.1.1" + }, + "type": "library", + "autoload": { + "psr-4": { + "League\\Flysystem\\AwsS3V3\\": "" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Frank de Jonge", + "email": "info@frankdejonge.nl" + } + ], + "description": "AWS S3 filesystem adapter for Flysystem.", + "keywords": [ + "Flysystem", + "aws", + "file", + "files", + "filesystem", + "s3", + "storage" + ], + "support": { + "source": "https://github.com/thephpleague/flysystem-aws-s3-v3/tree/3.29.0" + }, + "time": "2024-08-17T13:10:48+00:00" + }, { "name": "league/flysystem-local", "version": "3.29.0", From 5f505736610c1ceb1db000ee6d4b62993f029674 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Wed, 15 Jan 2025 14:54:28 -0500 Subject: [PATCH 02/16] Update s3 filesystem config to match stock Laravel 10 --- config/filesystems.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/filesystems.php b/config/filesystems.php index 8e7200ffc..e2f83b199 100755 --- a/config/filesystems.php +++ b/config/filesystems.php @@ -60,6 +60,8 @@ 'region' => env('AWS_DEFAULT_REGION'), 'bucket' => env('AWS_BUCKET'), 'url' => env('AWS_URL'), + 'endpoint' => env('AWS_ENDPOINT'), + 'use_path_style_endpoint' => env('AWS_USE_PATH_STYLE_ENDPOINT', false), ], 'cdash' => [ From 279464f850765068594bff8e165f86410100abee Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Tue, 14 Jan 2025 15:10:23 -0500 Subject: [PATCH 03/16] Update submission process to better support remote filesystems Remove use of md5_file() and update call sites of Storage::path() --- .env.example | 47 +++++++++++++++---- .../RemoteProcessingController.php | 3 +- app/Http/Controllers/SubmissionController.php | 19 ++++---- app/Jobs/ProcessSubmission.php | 7 +-- app/Utils/SubmissionUtils.php | 9 ++++ app/Utils/UnparsedSubmissionProcessor.php | 2 +- config/filesystems.php | 9 +++- docs/config.md | 30 +++++++++++- tests/Feature/RemoteWorkers.php | 1 + 9 files changed, 100 insertions(+), 27 deletions(-) diff --git a/.env.example b/.env.example index d5848b55d..ab43017eb 100755 --- a/.env.example +++ b/.env.example @@ -18,6 +18,8 @@ DB_PASSWORD=secret #DB_PORT= #DB_USERNAME= +# cdash.php + # How long since the last submission before considering a project inactive. # Set to 0 to always show all projects on viewProjects.php. #ACTIVE_PROJECT_DAYS=7 @@ -77,6 +79,12 @@ DB_PASSWORD=secret # something other than an email address in LDAP. #LOGIN_FIELD=Email +# The maximum visibility level for user-created projects on this instance. +# Instance admins are able to override this setting and set project visibility +# to anything. Thus, this setting is only meaningful if USER_CREATE_PROJECTS=true. +# Options: PUBLIC, PROTECTED, PRIVATE +# MAX_PROJECT_VISIBILITY=PUBLIC + # Maximum per-project upload quota, in GB #MAX_UPLOAD_QUOTA=10 @@ -148,6 +156,9 @@ DB_PASSWORD=secret # VCS (eg. GitHub) API endpoints. #USE_VCS_API=true +# Should normal users be allowed to create projects +# USER_CREATE_PROJECTS = false + # logging.php #LOG_CHANNEL=stack @@ -172,6 +183,33 @@ QUEUE_CONNECTION=database # Number of minutes before and idle session is allowed to expire. #SESSION_LIFETIME=120 +# filesystem.php + +# Default filesystem driver for CDash to use. +# Supported options are 'local' and 's3'. +#FILESYSTEM_DRIVER=local + +# The following env vars are only relevant for S3 support. +# The name of the bucket that CDash where will store files. +# AWS_BUCKET=cdash + +# The AWS region where this S3 bucket is stored. +# Otherwise set this to 'local' if you're using MinIO. +# AWS_REGION= + +# Credentials for access to this S3 bucket. +#AWS_ACCESS_KEY_ID= +#AWS_SECRET_ACCESS_KEY= + +# Set this to true if you're using MinIO +#AWS_USE_PATH_STYLE_ENDPOINT=false + +# URL of your MinIO server (if you're using MinIO). Leave blank otherwise. +#AWS_ENDPOINT= + +# URL of the bucket on your MinIO server (if you're using MinIO). Leave blank otherwise. +#AWS_URL= + # mail.php #MAIL_MAILER=smtp #MAIL_HOST=smtp.mailgun.org @@ -258,12 +296,3 @@ MIX_PUSHER_APP_CLUSTER="${PUSHER_APP_CLUSTER}" # Whether or not to automatically register new users upon first login #SAML2_AUTO_REGISTER_NEW_USERS=false - -# Should normal users be allowed to create projects -# USER_CREATE_PROJECTS = false - -# The maximum visibility level for user-created projects on this instance. -# Instance admins are able to override this setting and set project visibility -# to anything. Thus, this setting is only meaningful if USER_CREATE_PROJECTS=true. -# Options: PUBLIC, PROTECTED, PRIVATE -# MAX_PROJECT_VISIBILITY=PUBLIC diff --git a/app/Http/Controllers/RemoteProcessingController.php b/app/Http/Controllers/RemoteProcessingController.php index 632f4f114..4f7613bab 100644 --- a/app/Http/Controllers/RemoteProcessingController.php +++ b/app/Http/Controllers/RemoteProcessingController.php @@ -99,6 +99,7 @@ public function requeueSubmissionFile(): Response $filename = request()->string('filename'); $buildid = request()->integer('buildid'); $projectid = request()->integer('projectid'); + $md5 = request()->string('md5'); if (!Storage::exists("inprogress/{$filename}")) { return response('File not found', Response::HTTP_NOT_FOUND); } @@ -112,7 +113,7 @@ public function requeueSubmissionFile(): Response // Requeue the file with exponential backoff. PendingSubmissions::IncrementForBuildId($buildid); $delay = ((int) config('cdash.retry_base')) ** $retry_handler->Retries; - ProcessSubmission::dispatch($filename, $projectid, $buildid, md5_file(Storage::path("inbox/{$filename}")))->delay(now()->addSeconds($delay)); + ProcessSubmission::dispatch($filename, $projectid, $buildid, $md5)->delay(now()->addSeconds($delay)); return response('OK', Response::HTTP_OK); } } diff --git a/app/Http/Controllers/SubmissionController.php b/app/Http/Controllers/SubmissionController.php index 64a09f67a..78714d75b 100644 --- a/app/Http/Controllers/SubmissionController.php +++ b/app/Http/Controllers/SubmissionController.php @@ -85,23 +85,22 @@ private function submitProcess(): Response $authtoken = AuthTokenUtil::getBearerToken(); $authtoken_hash = $authtoken === null || $authtoken === '' ? '' : AuthTokenUtil::hashToken($authtoken); - // Save the incoming file in the inbox directory. - $filename = "{$projectname}_-_{$authtoken_hash}_-_" . Str::uuid()->toString() . "_-_{$expected_md5}.xml"; - $fp = request()->getContent(true); - if (!Storage::put("inbox/{$filename}", $fp)) { - Log::error("Failed to save submission to inbox for $projectname (md5=$expected_md5)"); - abort(Response::HTTP_INTERNAL_SERVER_ERROR, 'Failed to save submission file.'); - } - // Check that the md5sum of the file matches what we were told to expect. + $fp = request()->getContent(true); if (strlen($expected_md5) > 0) { - $md5sum = md5_file(Storage::path("inbox/{$filename}")); + $md5sum = SubmissionUtils::md5FileHandle($fp); if ($md5sum != $expected_md5) { - Storage::delete("inbox/{$filename}"); abort(Response::HTTP_BAD_REQUEST, "md5 mismatch. expected: {$expected_md5}, received: {$md5sum}"); } } + // Save the incoming file in the inbox directory. + $filename = "{$projectname}_-_{$authtoken_hash}_-_" . Str::uuid()->toString() . "_-_{$expected_md5}.xml"; + if (!Storage::put("inbox/{$filename}", $fp)) { + Log::error("Failed to save submission to inbox for $projectname (md5=$expected_md5)"); + abort(Response::HTTP_INTERNAL_SERVER_ERROR, 'Failed to save submission file.'); + } + // Check if we can connect to the database before proceeding any further. try { DB::connection()->getPdo(); diff --git a/app/Jobs/ProcessSubmission.php b/app/Jobs/ProcessSubmission.php index 997100500..28c0a3452 100644 --- a/app/Jobs/ProcessSubmission.php +++ b/app/Jobs/ProcessSubmission.php @@ -99,6 +99,7 @@ private function requeueSubmissionFile($buildid): bool 'filename' => $this->filename, 'buildid' => $buildid, 'projectid' => $this->projectid, + 'md5' => $this->expected_md5, ]); if ($this->localFilename !== '') { unlink($this->localFilename); @@ -119,9 +120,9 @@ private function requeueSubmissionFile($buildid): bool if (config('queue.default') === 'sqs-fifo') { // Special handling for sqs-fifo, which does not support per-message delays. sleep(10); - self::dispatch($this->filename, $this->projectid, $buildid, md5_file(Storage::path("inbox/{$this->filename}"))); + self::dispatch($this->filename, $this->projectid, $buildid, $this->expected_md5); } else { - self::dispatch($this->filename, $this->projectid, $buildid, md5_file(Storage::path("inbox/{$this->filename}")))->delay(now()->addSeconds($delay)); + self::dispatch($this->filename, $this->projectid, $buildid, $this->expected_md5)->delay(now()->addSeconds($delay)); } return true; @@ -293,7 +294,7 @@ private function getSubmissionFileHandle($filename) if ((bool) config('cdash.remote_workers') && is_string($filename)) { return $this->getRemoteSubmissionFileHandle($filename); } elseif (Storage::exists($filename)) { - return fopen(Storage::path($filename), 'r'); + return Storage::readStream($filename); } else { \Log::error('Failed to get a file handle for submission (was type ' . gettype($filename) . ')'); return false; diff --git a/app/Utils/SubmissionUtils.php b/app/Utils/SubmissionUtils.php index 6ca95f6ff..69f46a231 100644 --- a/app/Utils/SubmissionUtils.php +++ b/app/Utils/SubmissionUtils.php @@ -300,4 +300,13 @@ public static function compute_error_difference($buildid, $previousbuildid, $war } $pdo->commit(); } + + /** Return the hash of an open file handle */ + public static function hashFileHandle(mixed $filehandle, string $algo): string + { + $hashContext = hash_init($algo); + hash_update_stream($hashContext, $filehandle); + rewind($filehandle); + return hash_final($hashContext); + } } diff --git a/app/Utils/UnparsedSubmissionProcessor.php b/app/Utils/UnparsedSubmissionProcessor.php index d33058aeb..f1d198114 100644 --- a/app/Utils/UnparsedSubmissionProcessor.php +++ b/app/Utils/UnparsedSubmissionProcessor.php @@ -321,7 +321,7 @@ public function populateBuildFileRow(): void } // Check that the md5sum of the file matches what we were expecting. - $md5sum = md5_file(Storage::path($this->inboxdatafilename)); + $md5sum = SubmissionUtils::md5FileHandle(Storage::readStream($this->inboxdatafilename)); if ($md5sum != $this->md5) { Storage::delete($this->inboxdatafilename); $buildfile->delete(); diff --git a/config/filesystems.php b/config/filesystems.php index e2f83b199..173257e05 100755 --- a/config/filesystems.php +++ b/config/filesystems.php @@ -58,10 +58,17 @@ 'key' => env('AWS_ACCESS_KEY_ID'), 'secret' => env('AWS_SECRET_ACCESS_KEY'), 'region' => env('AWS_DEFAULT_REGION'), - 'bucket' => env('AWS_BUCKET'), + 'bucket' => env('AWS_BUCKET', 'cdash'), 'url' => env('AWS_URL'), 'endpoint' => env('AWS_ENDPOINT'), 'use_path_style_endpoint' => env('AWS_USE_PATH_STYLE_ENDPOINT', false), + // N.B. Laravel downloads files from S3 to a temporary directory on the + // local file system before serving them to clients! + // Setting 'stream_reads=true' would allow us to stream from S3 directly, + // but we can't use it because of our various calls to rewind(). + // See https://github.com/laravel/framework/discussions/49232 + // for more details. + // 'stream_reads' => true, ], 'cdash' => [ diff --git a/docs/config.md b/docs/config.md index 6d79c08f2..41f30aa50 100644 --- a/docs/config.md +++ b/docs/config.md @@ -48,8 +48,6 @@ Please see that file for a full list of configuration options. | MAIL_USERNAME | The username used to connect to the SMTP server | '' | | MAIL_PASSWORD | The password of the SMTP user | '' | - - ## Authentication By default, CDash stores each user's email address and a hash of their password in the `user` table. This information is used to authenticate users as they log @@ -73,6 +71,34 @@ service. See the [Submissions guide](submissions.md) for more details. +## File Storage + +By default, CDash uses the local filesystem to store files it receives from CTest. +If you're happy with this default, no additional configuration is necessary. + +CDash can be configured to use S3 instead, either hosted by AWS or a MinIO server. + +The following environment variables are common for both S3 implementations: +``` +FILESYSTEM_DRIVER=s3 +AWS_ACCESS_KEY_ID= +AWS_SECRET_ACCESS_KEY= +AWS_BUCKET= (defaults to 'cdash') +``` + +For AWS S3, you will also need to set: +``` +AWS_REGION= +``` + +For MinIO, you should set the following environment variables: +``` +AWS_REGION=local +AWS_USE_PATH_STYLE_ENDPOINT=true +AWS_ENDPOINT= (e.g. http://127.0.0.1:9001) +AWS_URL= (e.g. http://127.0.0.1:9001/cdash/) +``` + ## Other settings | Variable | Description | Default | | --------- | ----------- | ------- | diff --git a/tests/Feature/RemoteWorkers.php b/tests/Feature/RemoteWorkers.php index 9fe67d4ae..32b737190 100644 --- a/tests/Feature/RemoteWorkers.php +++ b/tests/Feature/RemoteWorkers.php @@ -17,6 +17,7 @@ protected function setUp(): void URL::forceRootUrl('http://localhost'); Config::set('cdash.remote_workers', true); Config::set('cdash.backup_timeframe', 0); + Config::set('filesystem.default', 'local'); } public function testRemoteWorkerAPIAccess(): void From 8a2782bbb498d5832f5eb5ce418a6c3cf59bea23 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Wed, 15 Jan 2025 16:31:14 -0500 Subject: [PATCH 04/16] Update XML validation to support remote filesystems --- app/Http/Controllers/SubmissionController.php | 4 +-- .../xml_handlers/abstract_xml_handler.php | 29 +++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/SubmissionController.php b/app/Http/Controllers/SubmissionController.php index 78714d75b..44d761465 100644 --- a/app/Http/Controllers/SubmissionController.php +++ b/app/Http/Controllers/SubmissionController.php @@ -137,7 +137,7 @@ private function submitProcess(): Response $stored_filename = 'inbox/' . $filename; $xml_info = []; try { - $xml_info = SubmissionUtils::get_xml_type(fopen(Storage::path($stored_filename), 'r'), $stored_filename); + $xml_info = SubmissionUtils::get_xml_type(Storage::readStream($stored_filename), $stored_filename); } catch (BadSubmissionException $e) { $xml_info['xml_handler'] = ''; $message = "Could not determine submission file type for: '{$stored_filename}'"; @@ -148,7 +148,7 @@ private function submitProcess(): Response } if ($xml_info['xml_handler'] !== '') { // If validation is enabled and if this file has a corresponding schema, validate it - $validation_errors = $xml_info['xml_handler']::validate(storage_path('app/' . $stored_filename)); + $validation_errors = $xml_info['xml_handler']::validate($stored_filename); if (count($validation_errors) > 0) { $error_string = implode(PHP_EOL, $validation_errors); diff --git a/app/cdash/xml_handlers/abstract_xml_handler.php b/app/cdash/xml_handlers/abstract_xml_handler.php index 8bcd1a9ac..486f4f638 100644 --- a/app/cdash/xml_handlers/abstract_xml_handler.php +++ b/app/cdash/xml_handlers/abstract_xml_handler.php @@ -20,6 +20,7 @@ use CDash\Model\Build; use CDash\Model\Project; use CDash\ServiceContainer; +use Illuminate\Support\Facades\Storage; abstract class AbstractXmlHandler extends AbstractSubmissionHandler { @@ -50,16 +51,36 @@ public static function validate(string $path): array return []; } - $errors = []; // let us control the failures so we can continue // parsing files instead of crashing midway libxml_use_internal_errors(true); // load the input file to be validated + $local_path = ''; $xml = new DOMDocument(); - $xml->load($path, LIBXML_PARSEHUGE); + if (file_exists($path)) { + $xml->load($path, LIBXML_PARSEHUGE); + } else { + if (!Storage::exists($path)) { + return ["ERROR: could not find {$path} for validation"]; + } + if (config('filesystem.default') === 'local') { + $xml->load(Storage::path($path), LIBXML_PARSEHUGE); + } else { + // Temporarily download the file because DOMDocument->load takes a path, + // not a stream... + $fp = Storage::readStream($path); + if ($fp === null) { + return ["ERROR: could not find {$path} for validation"]; + } + $local_path = 'tmp/' . basename($path); + Storage::disk('local')->put($local_path, $fp); + $xml->load(Storage::disk('local')->path($local_path), LIBXML_PARSEHUGE); + } + } // run the validator and collect errors if there are any. + $errors = []; if (!$xml->schemaValidate(base_path(static::$schema_file))) { $validation_errors = libxml_get_errors(); foreach ($validation_errors as $error) { @@ -70,6 +91,10 @@ public static function validate(string $path): array libxml_clear_errors(); } + if (config('filesystem.default') !== 'local') { + Storage::disk('local')->delete($local_path); + } + return $errors; } From d5dec8380eaca231ac276fc30db30798d0bb5398 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Thu, 16 Jan 2025 15:10:47 -0500 Subject: [PATCH 05/16] Update uploaded files to support remote filesystems --- app/Http/Controllers/BuildController.php | 20 +++++++++++++++---- app/Http/Controllers/SubmissionController.php | 14 +++++++++---- app/cdash/tests/test_uploadfile.php | 9 +++++---- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/app/Http/Controllers/BuildController.php b/app/Http/Controllers/BuildController.php index 7814e3396..73cf50200 100644 --- a/app/Http/Controllers/BuildController.php +++ b/app/Http/Controllers/BuildController.php @@ -29,7 +29,7 @@ use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Illuminate\View\View; -use Symfony\Component\HttpFoundation\BinaryFileResponse; +use Symfony\Component\HttpFoundation\StreamedResponse; require_once 'include/api_common.php'; @@ -829,7 +829,7 @@ public function files(int $build_id): View ->with('urls', $urls); } - public function build_file(int $build_id, int $file_id): BinaryFileResponse + public function build_file(int $build_id, int $file_id): StreamedResponse { $this->setBuildById($build_id); @@ -841,10 +841,22 @@ public function build_file(int $build_id, int $file_id): BinaryFileResponse $uploadFile = new UploadFile(); $uploadFile->Id = $file_id; $uploadFile->Fill(); - return response()->file(Storage::path("upload/{$uploadFile->Sha1Sum}"), [ + + $fp = Storage::readStream("upload/{$uploadFile->Sha1Sum}"); + if ($fp === null) { + abort(404, 'File not found'); + } + $filename = $uploadFile->Filename; + $headers = [ 'Content-Type' => 'text/plain', 'Content-Disposition' => "inline/attachment; filename={$uploadFile->Filename}", - ]); + ]; + return response()->streamDownload(function () use ($fp) { + while (!feof($fp)) { + echo fread($fp, 1024); + } + fclose($fp); + }, $filename, $headers, 'inline'); } public function ajaxBuildNote(): View diff --git a/app/Http/Controllers/SubmissionController.php b/app/Http/Controllers/SubmissionController.php index 44d761465..3f2e110e2 100644 --- a/app/Http/Controllers/SubmissionController.php +++ b/app/Http/Controllers/SubmissionController.php @@ -88,7 +88,7 @@ private function submitProcess(): Response // Check that the md5sum of the file matches what we were told to expect. $fp = request()->getContent(true); if (strlen($expected_md5) > 0) { - $md5sum = SubmissionUtils::md5FileHandle($fp); + $md5sum = SubmissionUtils::hashFileHandle($fp, 'md5'); if ($md5sum != $expected_md5) { abort(Response::HTTP_BAD_REQUEST, "md5 mismatch. expected: {$expected_md5}, received: {$md5sum}"); } @@ -232,18 +232,24 @@ public function storeUploadedFile(Request $request): Response } try { - $sha1sum = decrypt($request->input('sha1sum')); + $expected_sha1sum = decrypt($request->input('sha1sum')); } catch (DecryptException $e) { return response('This feature is disabled', Response::HTTP_CONFLICT); } $uploaded_file = array_values(request()->allFiles())[0]; - $stored_path = $uploaded_file->storeAs('upload', $sha1sum); + $stored_path = $uploaded_file->storeAs('upload', $expected_sha1sum); if ($stored_path === false) { abort(Response::HTTP_INTERNAL_SERVER_ERROR, 'Failed to store uploaded file'); } - if (sha1_file(Storage::path($stored_path)) !== $sha1sum) { + $fp = Storage::readStream($stored_path); + if ($fp === null) { + abort(Response::HTTP_INTERNAL_SERVER_ERROR, 'Failed to store uploaded file'); + } + + $found_sha1sum = SubmissionUtils::hashFileHandle($fp, 'sha1'); + if ($found_sha1sum !== $expected_sha1sum) { Storage::delete($stored_path); return response('Uploaded file does not match expected sha1sum', Response::HTTP_BAD_REQUEST); } diff --git a/app/cdash/tests/test_uploadfile.php b/app/cdash/tests/test_uploadfile.php index cda9e37c0..36a55974e 100644 --- a/app/cdash/tests/test_uploadfile.php +++ b/app/cdash/tests/test_uploadfile.php @@ -6,6 +6,7 @@ // require_once dirname(__FILE__) . '/cdash_test_case.php'; +use App\Utils\SubmissionUtils; use Illuminate\Support\Facades\Storage; class UploadFileTestCase extends KWWebTestCase @@ -63,8 +64,8 @@ public function testVerifyFileSubmission() $this->FileId = $query[0]['id']; $this->Sha1Sum = $query[0]['sha1sum']; - $uploaded_filepath = Storage::path('upload') . "/{$this->Sha1Sum}"; - if (!file_exists($uploaded_filepath)) { + $uploaded_filepath = "upload/{$this->Sha1Sum}"; + if (!Storage::exists($uploaded_filepath)) { $this->fail("File was not written to $uploaded_filepath"); return; } @@ -79,10 +80,10 @@ public function testVerifyFileSubmission() $body = $response->getBody(); file_put_contents($tmp_file, $body); - if (filesize($tmp_file) !== filesize($uploaded_filepath)) { + if (filesize($tmp_file) !== Storage::size($uploaded_filepath)) { $this->fail('filesize mismatch for downloaded file'); } - if (sha1_file($tmp_file) !== sha1_file($uploaded_filepath)) { + if (md5_file($tmp_file) !== SubmissionUtils::hashFileHandle(Storage::readStream($uploaded_filepath), 'md5')) { $this->fail("hash mismatch for downloaded file ($tmp_file) vs ($uploaded_filepath)"); } } From e2486bea7580dc15b5741f88f2ef550eb092ad60 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Wed, 22 Jan 2025 10:55:21 -0500 Subject: [PATCH 06/16] Update non-XML handlers to support remote filesystems --- app/Jobs/ProcessSubmission.php | 9 +++- app/Utils/UnparsedSubmissionProcessor.php | 2 +- app/cdash/include/common.php | 45 ++++++++++++++++--- app/cdash/include/ctestparser.php | 42 +++++------------ app/cdash/tests/test_extracttar.php | 4 +- app/cdash/xml_handlers/BazelJSON_handler.php | 5 ++- .../BuildPropertiesJSON_handler.php | 9 +++- app/cdash/xml_handlers/GcovTar_handler.php | 9 +--- app/cdash/xml_handlers/JSCoverTar_handler.php | 9 +--- .../xml_handlers/JavaJSONTar_handler.php | 9 +--- .../xml_handlers/OpenCoverTar_handler.php | 15 +++---- .../SubProjectDirectories_handler.php | 12 ++++- 12 files changed, 92 insertions(+), 78 deletions(-) diff --git a/app/Jobs/ProcessSubmission.php b/app/Jobs/ProcessSubmission.php index 28c0a3452..1967723df 100644 --- a/app/Jobs/ProcessSubmission.php +++ b/app/Jobs/ProcessSubmission.php @@ -216,8 +216,13 @@ private function doSubmit($filename, $projectid, $buildid = null, $expected_md5 return $handler; } - // Parse the XML file - $handler = ctest_parse($filehandle, $filename, $projectid, $expected_md5, $buildid); + // Special handling for unparsed (non-XML) submissions. + $handler = parse_put_submission($filename, $projectid, $expected_md5, $buildid); + if ($handler === false) { + // Otherwise, parse this submission as CTest XML. + $handler = ctest_parse($filehandle, $filename, $projectid, $expected_md5, $buildid); + } + fclose($filehandle); unset($filehandle); diff --git a/app/Utils/UnparsedSubmissionProcessor.php b/app/Utils/UnparsedSubmissionProcessor.php index f1d198114..91372a4c0 100644 --- a/app/Utils/UnparsedSubmissionProcessor.php +++ b/app/Utils/UnparsedSubmissionProcessor.php @@ -321,7 +321,7 @@ public function populateBuildFileRow(): void } // Check that the md5sum of the file matches what we were expecting. - $md5sum = SubmissionUtils::md5FileHandle(Storage::readStream($this->inboxdatafilename)); + $md5sum = SubmissionUtils::hashFileHandle(Storage::readStream($this->inboxdatafilename), 'md5'); if ($md5sum != $this->md5) { Storage::delete($this->inboxdatafilename); $buildfile->delete(); diff --git a/app/cdash/include/common.php b/app/cdash/include/common.php index 46af2c030..19d169f1d 100644 --- a/app/cdash/include/common.php +++ b/app/cdash/include/common.php @@ -914,19 +914,54 @@ function create_aggregate_build($build, $siteid = null): Build } /** - * Extract a tarball to a given directory. + * Extract a tarball within the local storage directory. */ -function extract_tar(string $filename, string $dirName): bool +function extract_tar(string $stored_filepath): string { + if (!Storage::exists($stored_filepath)) { + Log::error("{$stored_filepath} does not exist", [ + 'function' => 'extract_tar', + ]); + return ''; + } + + // Create a new directory where we can extract the tarball. + $localTmpDirPath = 'tmp' . DIRECTORY_SEPARATOR . pathinfo($stored_filepath, PATHINFO_FILENAME); + Storage::disk('local')->makeDirectory($localTmpDirPath); + $dirName = Storage::disk('local')->path($localTmpDirPath); + + if (config('filesystem.default') !== 'local') { + // Download this file to the local Storage tmp dir. + $remote_stored_filepath = $stored_filepath; + $stored_filepath = 'tmp/' . basename($stored_filepath); + $fp = Storage::readStream($remote_stored_filepath); + if ($fp === null) { + return ''; + } + Storage::disk('local')->put($stored_filepath, $fp); + } + try { - $tar = new Archive_Tar($filename); + $tar = new Archive_Tar(Storage::disk('local')->path($stored_filepath)); $tar->setErrorHandling(PEAR_ERROR_CALLBACK, function ($pear_error) { throw new PEAR_Exception($pear_error->getMessage()); }); - return $tar->extract($dirName); + $tar_extract_result = $tar->extract($dirName); + if (config('filesystem.default') !== 'local') { + Storage::disk('local')->delete($stored_filepath); + } + if ($tar_extract_result === false) { + Storage::disk('local')->deleteDirectory($localTmpDirPath); + return ''; + } + return $dirName; } catch (PEAR_Exception $e) { + if (config('filesystem.default') !== 'local') { + Storage::disk('local')->delete($stored_filepath); + } + Storage::disk('local')->deleteDirectory($localTmpDirPath); report($e); - return false; + return ''; } } diff --git a/app/cdash/include/ctestparser.php b/app/cdash/include/ctestparser.php index 7c3ef8bef..8c45a50dd 100644 --- a/app/cdash/include/ctestparser.php +++ b/app/cdash/include/ctestparser.php @@ -24,10 +24,6 @@ use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; -class CDashParseException extends RuntimeException -{ -} - /** Determine the descriptive filename for a submission file. */ function generateBackupFileName($projectname, $subprojectname, $buildname, $sitename, $stamp, $fileNameWithExt) @@ -65,11 +61,11 @@ function generateBackupFileName($projectname, $subprojectname, $buildname, } /** Function to handle new style submissions via HTTP PUT */ -function parse_put_submission($filehandler, $projectid, $expected_md5, ?int $buildid): AbstractSubmissionHandler|false +function parse_put_submission(string $filename, int $projectid, ?string $expected_md5, ?int $buildid): AbstractSubmissionHandler|false { $db = Database::getInstance(); - if (!$expected_md5) { + if ($expected_md5 === null) { return false; } @@ -103,10 +99,6 @@ function parse_put_submission($filehandler, $projectid, $expected_md5, ?int $bui } $sitename = $row['name']; - // Work directly off the open file handle. - $meta_data = stream_get_meta_data($filehandler); - $filename = $meta_data['uri']; - // Include the handler file for this type of submission. $include_file = 'xml_handlers/' . $buildfile->type . '_handler.php'; $valid_types = [ @@ -137,17 +129,16 @@ function parse_put_submission($filehandler, $projectid, $expected_md5, ?int $bui $build->Id = $buildfile->buildid; $handler = new $className($build); - // Parse the file. - if (file_exists($filename)) { - $filepath = $filename; - } elseif (Storage::exists($filename)) { - $filepath = Storage::path($filename); - } else { - throw new CDashParseException('Failed to locate file ' . $filename); + // Make sure the file exists. + if (!Storage::exists($filename)) { + Log::error("Failed to locate file {$filename}"); + return false; } - if ($handler->Parse($filepath) === false) { - throw new CDashParseException('Failed to parse file ' . $filename); + // Parse the file. + if ($handler->Parse($filename) === false) { + Log::error("Failed to parse file {$filename}"); + return false; } $buildfile->delete(); @@ -162,19 +153,6 @@ function parse_put_submission($filehandler, $projectid, $expected_md5, ?int $bui */ function ctest_parse($filehandle, string $filename, $projectid, $expected_md5 = '', ?int $buildid = null): AbstractSubmissionHandler|false { - // Check if this is a new style PUT submission. - try { - $handler = parse_put_submission($filehandle, $projectid, $expected_md5, $buildid); - if ($handler !== false) { - return $handler; - } - } catch (CDashParseException $e) { - Log::error($e->getMessage(), [ - 'function' => 'ctest_parse', - ]); - return false; - } - // Try to get the IP of the build. $ip = null; if (array_key_exists('REMOTE_ADDR', $_SERVER)) { diff --git a/app/cdash/tests/test_extracttar.php b/app/cdash/tests/test_extracttar.php index f1270f04c..858d53e00 100644 --- a/app/cdash/tests/test_extracttar.php +++ b/app/cdash/tests/test_extracttar.php @@ -9,9 +9,9 @@ public function __construct() public function testExtractTarArchiveTarWithInvalidFile() { - $result = extract_tar(dirname(__FILE__) . '/../config/config.php', 'foo'); + $result = extract_tar('this_file_does_not_exist'); - $this->assertFalse($result); + $this->assertTrue($result === ''); $this->assertTrue(is_readable($this->logfilename)); $logfileContents = file_get_contents($this->logfilename); diff --git a/app/cdash/xml_handlers/BazelJSON_handler.php b/app/cdash/xml_handlers/BazelJSON_handler.php index df3255a2c..3c3d1eddc 100644 --- a/app/cdash/xml_handlers/BazelJSON_handler.php +++ b/app/cdash/xml_handlers/BazelJSON_handler.php @@ -25,6 +25,7 @@ use CDash\Model\BuildErrorFilter; use CDash\Model\Project; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Storage; class BazelJSONHandler extends AbstractSubmissionHandler { @@ -90,8 +91,8 @@ public function __construct(Build $build) **/ public function Parse($filename) { - $handle = fopen($filename, 'r'); - if (!$handle) { + $handle = Storage::readStream($filename); + if ($handle === null) { Log::error("Could not open $filename for parsing", [ 'function' => 'BazelJSONHandler::Parse', ]); diff --git a/app/cdash/xml_handlers/BuildPropertiesJSON_handler.php b/app/cdash/xml_handlers/BuildPropertiesJSON_handler.php index fa3c71e1d..e48a4f057 100644 --- a/app/cdash/xml_handlers/BuildPropertiesJSON_handler.php +++ b/app/cdash/xml_handlers/BuildPropertiesJSON_handler.php @@ -32,7 +32,14 @@ public function __construct(Build $build) public function Parse($filename) { // Test that this file contains valid JSON that PHP can decode. - $json_obj = json_decode(file_get_contents($filename), true); + $json_str = Storage::get($filename); + if ($json_str === null) { + Log::error("Failed to retrieve $filename from Storage", [ + 'function' => 'BuildPropertiesJSONHandler::Parse', + ]); + return false; + } + $json_obj = json_decode($json_str, true); if ($json_obj === null) { $err = json_last_error_msg(); Log::error("Failed to parse $filename: $err", [ diff --git a/app/cdash/xml_handlers/GcovTar_handler.php b/app/cdash/xml_handlers/GcovTar_handler.php index b536024b3..a6de75341 100644 --- a/app/cdash/xml_handlers/GcovTar_handler.php +++ b/app/cdash/xml_handlers/GcovTar_handler.php @@ -24,7 +24,6 @@ use CDash\Model\Label; use CDash\Model\SubProject; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; class GcovTarHandler extends AbstractSubmissionHandler { @@ -67,13 +66,9 @@ public function __construct(Build $build) **/ public function Parse($filename) { - // Create a new directory where we can extract our tarball. - $dirName = Storage::path('parsed') . DIRECTORY_SEPARATOR . pathinfo($filename, PATHINFO_FILENAME); - mkdir($dirName); - // Extract the tarball. - $result = extract_tar($filename, $dirName); - if ($result === false) { + $dirName = extract_tar($filename); + if ($dirName === '') { Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ 'function' => 'GcovTarHandler::Parse', ]); diff --git a/app/cdash/xml_handlers/JSCoverTar_handler.php b/app/cdash/xml_handlers/JSCoverTar_handler.php index 5ad16cd1e..73a7d993e 100644 --- a/app/cdash/xml_handlers/JSCoverTar_handler.php +++ b/app/cdash/xml_handlers/JSCoverTar_handler.php @@ -22,7 +22,6 @@ use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; class JSCoverTarHandler extends AbstractSubmissionHandler { @@ -47,13 +46,9 @@ public function __construct(Build $build) **/ public function Parse($filename) { - // Create a new directory where we can extract our tarball. - $dirName = Storage::path('parsed') . DIRECTORY_SEPARATOR . pathinfo($filename, PATHINFO_FILENAME); - mkdir($dirName); - // Extract the tarball. - $result = extract_tar($filename, $dirName); - if ($result === false) { + $dirName = extract_tar($filename); + if ($dirName === '') { Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ 'function' => 'JSCoverTarHandler::Parse', ]); diff --git a/app/cdash/xml_handlers/JavaJSONTar_handler.php b/app/cdash/xml_handlers/JavaJSONTar_handler.php index 2aa0fc256..90d26770c 100644 --- a/app/cdash/xml_handlers/JavaJSONTar_handler.php +++ b/app/cdash/xml_handlers/JavaJSONTar_handler.php @@ -22,7 +22,6 @@ use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; class JavaJSONTarHandler extends AbstractSubmissionHandler { @@ -43,13 +42,9 @@ public function __construct(Build $init) **/ public function Parse($filename) { - // Create a new directory where we can extract our tarball. - $dirName = Storage::path('parsed') . DIRECTORY_SEPARATOR . pathinfo($filename, PATHINFO_FILENAME); - mkdir($dirName); - // Extract the tarball. - $result = extract_tar($filename, $dirName); - if ($result === false) { + $dirName = extract_tar($filename); + if ($dirName === '') { Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ 'function' => 'JavaJSONTarHandler::Parse', ]); diff --git a/app/cdash/xml_handlers/OpenCoverTar_handler.php b/app/cdash/xml_handlers/OpenCoverTar_handler.php index 1d1d95974..825d5ccb6 100644 --- a/app/cdash/xml_handlers/OpenCoverTar_handler.php +++ b/app/cdash/xml_handlers/OpenCoverTar_handler.php @@ -21,7 +21,6 @@ use CDash\Model\CoverageFileLog; use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; class OpenCoverTarHandler extends AbstractXmlHandler { @@ -139,13 +138,9 @@ public function text($parser, $data) **/ public function Parse($filename) { - // Create a new directory where we can extract our tarball. - $dirName = Storage::path('parsed') . DIRECTORY_SEPARATOR . pathinfo($filename, PATHINFO_FILENAME); - mkdir($dirName); - $this->tarDir = $dirName; - $result = extract_tar($filename, $dirName); - if ($result === false) { - Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ + $this->tarDir = extract_tar($filename); + if ($this->tarDir === '') { + Log::error("Could not extract {$filename}", [ 'function' => 'OpenCoverTarHandler::Parse', ]); return false; @@ -153,7 +148,7 @@ public function Parse($filename) // Search for data.json $iterator = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($dirName), + new RecursiveDirectoryIterator($this->tarDir), RecursiveIteratorIterator::CHILD_FIRST); foreach ($iterator as $fileinfo) { if ($fileinfo->getFilename() == 'data.json') { @@ -225,7 +220,7 @@ public function Parse($filename) } // Delete the directory when we're done. - DeleteDirectory($dirName); + DeleteDirectory($this->tarDir); return true; } diff --git a/app/cdash/xml_handlers/SubProjectDirectories_handler.php b/app/cdash/xml_handlers/SubProjectDirectories_handler.php index cb6f3261d..614d9d5b1 100644 --- a/app/cdash/xml_handlers/SubProjectDirectories_handler.php +++ b/app/cdash/xml_handlers/SubProjectDirectories_handler.php @@ -36,13 +36,21 @@ public function __construct(Build|Project $init) **/ public function Parse($filename) { - $open_list = file($filename, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); - if ($open_list === false) { + $open_list = []; + $fp = Storage::readStream($filename); + if ($fp === null) { Log::error("Could not open $filename for parsing", [ 'function' => 'SubProjectDirectoriesHandler::Parse', ]); return false; } + while (($line = fgets($fp)) !== false) { + $line = trim($line); + if ($line === '') { + continue; + } + $open_list[] = $line; + } // Record the order that the packages were listed in. $order = 1; From a9439476d2c16fcb5cf4e50c40e0c28c1205f55e Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Wed, 22 Jan 2025 13:33:32 -0500 Subject: [PATCH 07/16] Update Done.xml handlers to support remote filesystems --- app/Jobs/ProcessSubmission.php | 2 +- app/cdash/tests/test_donehandler.php | 10 +++++++--- app/cdash/xml_handlers/retry_handler.php | 11 ++++++++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/Jobs/ProcessSubmission.php b/app/Jobs/ProcessSubmission.php index 1967723df..6abdb5ee2 100644 --- a/app/Jobs/ProcessSubmission.php +++ b/app/Jobs/ProcessSubmission.php @@ -108,7 +108,7 @@ private function requeueSubmissionFile($buildid): bool return $response->ok(); } else { // Increment retry count. - $retry_handler = new RetryHandler(Storage::path("inprogress/{$this->filename}")); + $retry_handler = new RetryHandler("inprogress/{$this->filename}"); $retry_handler->increment(); // Move file back to inbox. diff --git a/app/cdash/tests/test_donehandler.php b/app/cdash/tests/test_donehandler.php index 43a8a6c6e..51b6b66cd 100644 --- a/app/cdash/tests/test_donehandler.php +++ b/app/cdash/tests/test_donehandler.php @@ -31,6 +31,11 @@ public function testDoneHandlerLocal() public function testDoneHandlerRemote() { + if (config('filesystem.default') !== 'local') { + // Skip this test case if we're already testing remote storage. + return; + } + $this->ConfigFile = dirname(__FILE__) . '/../../../.env'; $this->Original = file_get_contents($this->ConfigFile); @@ -66,7 +71,7 @@ private function performTest($remote = false) $this->assertTrue($build->Id > 0); // Generate a Done.xml file and submit it. - $tmpfname = tempnam(Storage::path('inbox'), 'Done'); + $tmpfname = tempnam(Storage::disk('local')->path('tmp'), 'Done'); $handle = fopen($tmpfname, 'w'); fwrite($handle, "$build->Id"); fclose($handle); @@ -103,8 +108,7 @@ private function performTest($remote = false) } } - $files = Storage::files('parsed'); - $contents = file_get_contents(Storage::path($files[0])); + $contents = Storage::get(Storage::files('parsed')[0]); $this->assertTrue(str_contains($contents, 'Done retries="5"')); unlink($tmpfname); diff --git a/app/cdash/xml_handlers/retry_handler.php b/app/cdash/xml_handlers/retry_handler.php index 77f9fc140..07f83569b 100644 --- a/app/cdash/xml_handlers/retry_handler.php +++ b/app/cdash/xml_handlers/retry_handler.php @@ -33,11 +33,16 @@ public function __construct($filename) **/ public function increment() { - if (!file_exists($this->FileName)) { + if (!Storage::exists($this->FileName)) { return false; } - $xml = simplexml_load_file($this->FileName); + $contents = Storage::get($this->FileName); + if ($contents === null) { + return false; + } + + $xml = simplexml_load_string($contents); $attributes = $xml->attributes(); if (isset($attributes['retries'])) { $this->Retries = intval($attributes['retries']) + 1; @@ -46,6 +51,6 @@ public function increment() } $xml['retries'] = $this->Retries; - return $xml->asXML($this->FileName); + return Storage::put($this->FileName, $xml->asXML()); } } From b626caed566ffb72aadc168285cd663f93bdfdca Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Thu, 23 Jan 2025 11:09:16 -0500 Subject: [PATCH 08/16] Update docker compose to test remote storage --- .env.dev | 3 +++ docker/docker-compose.minio.yml | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 docker/docker-compose.minio.yml diff --git a/.env.dev b/.env.dev index 7d9a0c82c..42b4915d5 100755 --- a/.env.dev +++ b/.env.dev @@ -19,3 +19,6 @@ SLOW_PAGE_TIME=1000000 MAIL_MAILER=smtp MAIL_HOST=mailpit MAIL_PORT=1025 + +AWS_ACCESS_KEY_ID=minioadmin +AWS_SECRET_ACCESS_KEY=minioadmin diff --git a/docker/docker-compose.minio.yml b/docker/docker-compose.minio.yml new file mode 100644 index 000000000..ddc1da7a1 --- /dev/null +++ b/docker/docker-compose.minio.yml @@ -0,0 +1,23 @@ +services: + cdash: + environment: + FILESYSTEM_DRIVER: s3 + AWS_BUCKET: cdash + AWS_REGION: local + AWS_ACCESS_KEY_ID: ${AWS_ACCESS_KEY_ID} + AWS_SECRET_ACCESS_KEY: ${AWS_SECRET_ACCESS_KEY} + AWS_USE_PATH_STYLE_ENDPOINT: true + AWS_URL: http://minio:9000/cdash/ + AWS_ENDPOINT: http://minio:9000 + minio: + image: bitnami/minio:latest + ports: + - "9000:9000" + environment: + MINIO_DEFAULT_BUCKETS: "cdash" + MINIO_ROOT_USER: ${AWS_ACCESS_KEY_ID} + MINIO_ROOT_PASSWORD: ${AWS_SECRET_ACCESS_KEY} + volumes: + - miniodata:/bitnami/minio/data +volumes: + miniodata: From 501647e56066e1b6ffba5cd0215002bab099c4cd Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Thu, 23 Jan 2025 14:56:08 -0500 Subject: [PATCH 09/16] Update GitHub Actions to test remote storage --- .github/workflows/ci.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7fa864911..06be03dea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,16 +20,25 @@ jobs: matrix: database: ['mysql', 'postgres'] base-image: ['debian', 'ubi'] + storage: ['local', 'minio'] + exclude: + - storage: minio + base-image: ubi + - storage: minio + database: mysql steps: - uses: actions/checkout@v4 - uses: docker/setup-buildx-action@v3 - name: Build images shell: bash run: | + if [ "${{matrix.storage}}" == "minio" ]; then + extra_args="-f docker/docker-compose.minio.yml" + fi docker compose \ -f docker/docker-compose.yml \ -f docker/docker-compose.dev.yml \ - -f "docker/docker-compose.${{matrix.database}}.yml" \ + -f "docker/docker-compose.${{matrix.database}}.yml" ${extra_args} \ --env-file .env.dev up -d \ --build \ --wait From 7ad6cddf9b0cd407fbb3a13f50cc0988eb22bf8a Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Fri, 24 Jan 2025 12:09:45 -0500 Subject: [PATCH 10/16] extract_tar: throw exception rather than returning empty string --- app/cdash/include/common.php | 16 +++++++------- app/cdash/tests/test_extracttar.php | 21 +++++++++++-------- app/cdash/xml_handlers/GcovTar_handler.php | 11 ++++++++-- app/cdash/xml_handlers/JSCoverTar_handler.php | 11 ++++++++-- .../xml_handlers/JavaJSONTar_handler.php | 11 ++++++++-- .../xml_handlers/OpenCoverTar_handler.php | 11 ++++++++-- 6 files changed, 57 insertions(+), 24 deletions(-) diff --git a/app/cdash/include/common.php b/app/cdash/include/common.php index 19d169f1d..b7001e9dc 100644 --- a/app/cdash/include/common.php +++ b/app/cdash/include/common.php @@ -27,6 +27,8 @@ use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; function xslt_process(XSLTProcessor $xsltproc, $xml_arg, @@ -915,14 +917,14 @@ function create_aggregate_build($build, $siteid = null): Build /** * Extract a tarball within the local storage directory. + * + * @throws FileNotFoundException + * @throws UnableToReadFile */ function extract_tar(string $stored_filepath): string { if (!Storage::exists($stored_filepath)) { - Log::error("{$stored_filepath} does not exist", [ - 'function' => 'extract_tar', - ]); - return ''; + throw new FileNotFoundException($stored_filepath); } // Create a new directory where we can extract the tarball. @@ -936,7 +938,7 @@ function extract_tar(string $stored_filepath): string $stored_filepath = 'tmp/' . basename($stored_filepath); $fp = Storage::readStream($remote_stored_filepath); if ($fp === null) { - return ''; + throw UnableToReadFile::fromLocation($remote_stored_filepath); } Storage::disk('local')->put($stored_filepath, $fp); } @@ -952,7 +954,7 @@ function extract_tar(string $stored_filepath): string } if ($tar_extract_result === false) { Storage::disk('local')->deleteDirectory($localTmpDirPath); - return ''; + throw new RuntimeException("Unable to extract {$stored_filepath}"); } return $dirName; } catch (PEAR_Exception $e) { @@ -961,8 +963,8 @@ function extract_tar(string $stored_filepath): string } Storage::disk('local')->deleteDirectory($localTmpDirPath); report($e); - return ''; } + return ''; } /** diff --git a/app/cdash/tests/test_extracttar.php b/app/cdash/tests/test_extracttar.php index 858d53e00..8271702f7 100644 --- a/app/cdash/tests/test_extracttar.php +++ b/app/cdash/tests/test_extracttar.php @@ -1,5 +1,8 @@ assertTrue($result === ''); - $this->assertTrue(is_readable($this->logfilename)); - - $logfileContents = file_get_contents($this->logfilename); - - $this->assertTrue($this->findString($logfileContents, 'ERROR')); - $this->assertTrue($this->findString($logfileContents, 'extract_tar')); + $exception_thrown = false; + try { + extract_tar('this_file_does_not_exist'); + } catch (FileNotFoundException|UnableToReadFile $e) { + $exception_thrown = true; + } + if (!$exception_thrown) { + $this->fail('No Exception thrown when expected'); + } } } diff --git a/app/cdash/xml_handlers/GcovTar_handler.php b/app/cdash/xml_handlers/GcovTar_handler.php index a6de75341..121869cfc 100644 --- a/app/cdash/xml_handlers/GcovTar_handler.php +++ b/app/cdash/xml_handlers/GcovTar_handler.php @@ -24,6 +24,8 @@ use CDash\Model\Label; use CDash\Model\SubProject; use Illuminate\Support\Facades\Log; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; class GcovTarHandler extends AbstractSubmissionHandler { @@ -64,10 +66,15 @@ public function __construct(Build $build) /** * Parse a tarball of .gcov files. **/ - public function Parse($filename) + public function Parse(string $filename): bool { // Extract the tarball. - $dirName = extract_tar($filename); + try { + $dirName = extract_tar($filename); + } catch (FileNotFoundException|UnableToReadFile $e) { + report($e); + return false; + } if ($dirName === '') { Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ 'function' => 'GcovTarHandler::Parse', diff --git a/app/cdash/xml_handlers/JSCoverTar_handler.php b/app/cdash/xml_handlers/JSCoverTar_handler.php index 73a7d993e..1f036a979 100644 --- a/app/cdash/xml_handlers/JSCoverTar_handler.php +++ b/app/cdash/xml_handlers/JSCoverTar_handler.php @@ -22,6 +22,8 @@ use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; class JSCoverTarHandler extends AbstractSubmissionHandler { @@ -44,10 +46,15 @@ public function __construct(Build $build) /** * Parse a tarball of JSON files. **/ - public function Parse($filename) + public function Parse(string $filename): bool { // Extract the tarball. - $dirName = extract_tar($filename); + try { + $dirName = extract_tar($filename); + } catch (FileNotFoundException|UnableToReadFile $e) { + report($e); + return false; + } if ($dirName === '') { Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ 'function' => 'JSCoverTarHandler::Parse', diff --git a/app/cdash/xml_handlers/JavaJSONTar_handler.php b/app/cdash/xml_handlers/JavaJSONTar_handler.php index 90d26770c..30aa66677 100644 --- a/app/cdash/xml_handlers/JavaJSONTar_handler.php +++ b/app/cdash/xml_handlers/JavaJSONTar_handler.php @@ -22,6 +22,8 @@ use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; class JavaJSONTarHandler extends AbstractSubmissionHandler { @@ -40,10 +42,15 @@ public function __construct(Build $init) /** * Parse a tarball of JSON files. **/ - public function Parse($filename) + public function Parse(string $filename): bool { // Extract the tarball. - $dirName = extract_tar($filename); + try { + $dirName = extract_tar($filename); + } catch (FileNotFoundException|UnableToReadFile $e) { + report($e); + return false; + } if ($dirName === '') { Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ 'function' => 'JavaJSONTarHandler::Parse', diff --git a/app/cdash/xml_handlers/OpenCoverTar_handler.php b/app/cdash/xml_handlers/OpenCoverTar_handler.php index 825d5ccb6..25d342ab8 100644 --- a/app/cdash/xml_handlers/OpenCoverTar_handler.php +++ b/app/cdash/xml_handlers/OpenCoverTar_handler.php @@ -21,6 +21,8 @@ use CDash\Model\CoverageFileLog; use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\Log; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; class OpenCoverTarHandler extends AbstractXmlHandler { @@ -136,9 +138,14 @@ public function text($parser, $data) /** * Parse a tarball of JSON files. **/ - public function Parse($filename) + public function Parse(string $filename): bool { - $this->tarDir = extract_tar($filename); + try { + $this->tarDir = extract_tar($filename); + } catch (FileNotFoundException|UnableToReadFile $e) { + report($e); + return false; + } if ($this->tarDir === '') { Log::error("Could not extract {$filename}", [ 'function' => 'OpenCoverTarHandler::Parse', From 014e230d9eb44bdc8d9ed0fdd82f0d1f9275f745 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Fri, 24 Jan 2025 12:10:03 -0500 Subject: [PATCH 11/16] Delete tmp file in test_uploadfile --- app/cdash/tests/test_uploadfile.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/cdash/tests/test_uploadfile.php b/app/cdash/tests/test_uploadfile.php index 36a55974e..34a74066c 100644 --- a/app/cdash/tests/test_uploadfile.php +++ b/app/cdash/tests/test_uploadfile.php @@ -86,6 +86,8 @@ public function testVerifyFileSubmission() if (md5_file($tmp_file) !== SubmissionUtils::hashFileHandle(Storage::readStream($uploaded_filepath), 'md5')) { $this->fail("hash mismatch for downloaded file ($tmp_file) vs ($uploaded_filepath)"); } + + unlink($tmp_file); } // Make sure the build label has been set From 1e90dbd5e322509db4c24e2e5b61068370c9fea1 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Fri, 24 Jan 2025 12:13:25 -0500 Subject: [PATCH 12/16] Add comment defining required behavior for build_file() --- app/Http/Controllers/BuildController.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/Http/Controllers/BuildController.php b/app/Http/Controllers/BuildController.php index 73cf50200..acb40b2d0 100644 --- a/app/Http/Controllers/BuildController.php +++ b/app/Http/Controllers/BuildController.php @@ -842,6 +842,10 @@ public function build_file(int $build_id, int $file_id): StreamedResponse $uploadFile->Id = $file_id; $uploadFile->Fill(); + // The code below satisfies the following requirements: + // 1) Render text and images in browser (as opposed to forcing a download). + // 2) Download other files to the proper filename (not a numeric identifier). + // 3) Support downloading files that are larger than the PHP memory_limit. $fp = Storage::readStream("upload/{$uploadFile->Sha1Sum}"); if ($fp === null) { abort(404, 'File not found'); From 38ffcfef7facdba60c53af645d3efdb6d807b2ac Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Fri, 24 Jan 2025 12:32:06 -0500 Subject: [PATCH 13/16] Throw exceptions rather than return errors in AbstractXmlHandler::validate --- app/Console/Commands/ValidateXml.php | 5 +++++ app/Http/Controllers/SubmissionController.php | 12 +++++++++++- app/cdash/xml_handlers/abstract_xml_handler.php | 9 +++++++-- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/app/Console/Commands/ValidateXml.php b/app/Console/Commands/ValidateXml.php index ee84d2b03..0123630fc 100644 --- a/app/Console/Commands/ValidateXml.php +++ b/app/Console/Commands/ValidateXml.php @@ -6,6 +6,8 @@ use App\Utils\SubmissionUtils; use BadMethodCallException; use Illuminate\Console\Command; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; class ValidateXml extends Command { @@ -69,6 +71,9 @@ public function handle(): int $this->warn("WARNING: Skipped input file '{$input_xml_file}' as validation" . ' of this file format is currently not supported.'); $has_skipped = true; + } catch (FileNotFoundException|UnableToReadFile $e) { + $this->error($e->getMessage()); + $has_skipped = true; } } diff --git a/app/Http/Controllers/SubmissionController.php b/app/Http/Controllers/SubmissionController.php index 3f2e110e2..7c39c0c56 100644 --- a/app/Http/Controllers/SubmissionController.php +++ b/app/Http/Controllers/SubmissionController.php @@ -21,6 +21,8 @@ use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; use Symfony\Component\HttpKernel\Exception\HttpException; final class SubmissionController extends AbstractProjectController @@ -148,7 +150,15 @@ private function submitProcess(): Response } if ($xml_info['xml_handler'] !== '') { // If validation is enabled and if this file has a corresponding schema, validate it - $validation_errors = $xml_info['xml_handler']::validate($stored_filename); + $validation_errors = []; + try { + $validation_errors = $xml_info['xml_handler']::validate($stored_filename); + } catch (FileNotFoundException|UnableToReadFile $e) { + Log::warning($e->getMessage()); + if ((bool) config('cdash.validate_xml_submissions') === true) { + abort(400, "XML validation failed for $filename:" . PHP_EOL . $e->getMessage()); + } + } if (count($validation_errors) > 0) { $error_string = implode(PHP_EOL, $validation_errors); diff --git a/app/cdash/xml_handlers/abstract_xml_handler.php b/app/cdash/xml_handlers/abstract_xml_handler.php index 486f4f638..ae636a446 100644 --- a/app/cdash/xml_handlers/abstract_xml_handler.php +++ b/app/cdash/xml_handlers/abstract_xml_handler.php @@ -21,6 +21,8 @@ use CDash\Model\Project; use CDash\ServiceContainer; use Illuminate\Support\Facades\Storage; +use League\Flysystem\UnableToReadFile; +use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; abstract class AbstractXmlHandler extends AbstractSubmissionHandler { @@ -44,6 +46,9 @@ public function __construct(Build|Project $init) * Validate the given XML file based on its type * * @return array + * + * @throws FileNotFoundException + * @throws UnableToReadFile */ public static function validate(string $path): array { @@ -62,7 +67,7 @@ public static function validate(string $path): array $xml->load($path, LIBXML_PARSEHUGE); } else { if (!Storage::exists($path)) { - return ["ERROR: could not find {$path} for validation"]; + throw new FileNotFoundException($path); } if (config('filesystem.default') === 'local') { $xml->load(Storage::path($path), LIBXML_PARSEHUGE); @@ -71,7 +76,7 @@ public static function validate(string $path): array // not a stream... $fp = Storage::readStream($path); if ($fp === null) { - return ["ERROR: could not find {$path} for validation"]; + throw UnableToReadFile::fromLocation($path); } $local_path = 'tmp/' . basename($path); Storage::disk('local')->put($local_path, $fp); From 6613cae28dd17cf20a10c369accd997c76a9998a Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Fri, 24 Jan 2025 14:03:34 -0500 Subject: [PATCH 14/16] Regenerate phpstan-baseline.neon --- phpstan-baseline.neon | 92 ++++++------------------------------------- 1 file changed, 11 insertions(+), 81 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 001da5dc4..a32ba3a2a 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -5372,6 +5372,11 @@ parameters: - message: "#^Parameter \\#1 \\$stream of function rewind expects resource, mixed given\\.$#" + count: 2 + path: app/Utils/SubmissionUtils.php + + - + message: "#^Parameter \\#2 \\$stream of function hash_update_stream expects resource, mixed given\\.$#" count: 1 path: app/Utils/SubmissionUtils.php @@ -13456,6 +13461,11 @@ parameters: count: 2 path: app/cdash/include/common.php + - + message: "#^Dynamic call to static method Illuminate\\\\Filesystem\\\\FilesystemAdapter\\:\\:path\\(\\)\\.$#" + count: 2 + path: app/cdash/include/common.php + - message: "#^Function DeleteDirectory\\(\\) throws checked exception UnexpectedValueException but it's missing from the PHPDoc @throws tag\\.$#" count: 1 @@ -13977,26 +13987,6 @@ parameters: count: 1 path: app/cdash/include/ctestparser.php - - - message: "#^Function parse_put_submission\\(\\) has parameter \\$expected_md5 with no type specified\\.$#" - count: 1 - path: app/cdash/include/ctestparser.php - - - - message: "#^Function parse_put_submission\\(\\) has parameter \\$filehandler with no type specified\\.$#" - count: 1 - path: app/cdash/include/ctestparser.php - - - - message: "#^Function parse_put_submission\\(\\) has parameter \\$projectid with no type specified\\.$#" - count: 1 - path: app/cdash/include/ctestparser.php - - - - message: "#^Function parse_put_submission\\(\\) throws checked exception CDashParseException but it's missing from the PHPDoc @throws tag\\.$#" - count: 2 - path: app/cdash/include/ctestparser.php - - message: "#^Loose comparison via \"\\!\\=\" is not allowed\\.$#" count: 2 @@ -14007,11 +13997,6 @@ parameters: count: 1 path: app/cdash/include/ctestparser.php - - - message: "#^Offset 'uri' might not exist on array\\{timed_out\\: bool, blocked\\: bool, eof\\: bool, unread_bytes\\: int, stream_type\\: string, wrapper_type\\: string, wrapper_data\\: mixed, mode\\: string, \\.\\.\\.\\}\\.$#" - count: 1 - path: app/cdash/include/ctestparser.php - - message: "#^Parameter \\#2 \\$data of function xml_parse expects string, string\\|false given\\.$#" count: 2 @@ -22669,7 +22654,7 @@ parameters: path: app/cdash/tests/test_donehandler.php - - message: "#^Parameter \\#1 \\$haystack of function str_contains expects string, string\\|false given\\.$#" + message: "#^Parameter \\#1 \\$haystack of function str_contains expects string, string\\|null given\\.$#" count: 1 path: app/cdash/tests/test_donehandler.php @@ -23413,11 +23398,6 @@ parameters: count: 1 path: app/cdash/tests/test_extracttar.php - - - message: "#^Parameter \\#1 \\$mystring of method KWWebTestCase\\:\\:findString\\(\\) expects string, string\\|false given\\.$#" - count: 2 - path: app/cdash/tests/test_extracttar.php - - message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#" count: 1 @@ -28808,11 +28788,6 @@ parameters: count: 1 path: app/cdash/xml_handlers/BazelJSON_handler.php - - - message: "#^Only booleans are allowed in a negated boolean, resource\\|false given\\.$#" - count: 1 - path: app/cdash/xml_handlers/BazelJSON_handler.php - - message: "#^Only booleans are allowed in an if condition, int\\|false given\\.$#" count: 1 @@ -28978,11 +28953,6 @@ parameters: count: 1 path: app/cdash/xml_handlers/BuildPropertiesJSON_handler.php - - - message: "#^Parameter \\#1 \\$json of function json_decode expects string, string\\|false given\\.$#" - count: 1 - path: app/cdash/xml_handlers/BuildPropertiesJSON_handler.php - - message: "#^Property CDash\\\\Model\\\\BuildProperties\\:\\:\\$Properties \\(array\\) does not accept mixed\\.$#" count: 1 @@ -29028,16 +28998,6 @@ parameters: count: 3 path: app/cdash/xml_handlers/GcovTar_handler.php - - - message: "#^Method GcovTarHandler\\:\\:Parse\\(\\) has no return type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/GcovTar_handler.php - - - - message: "#^Method GcovTarHandler\\:\\:Parse\\(\\) has parameter \\$filename with no type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/GcovTar_handler.php - - message: "#^Method GcovTarHandler\\:\\:Parse\\(\\) throws checked exception UnexpectedValueException but it's missing from the PHPDoc @throws tag\\.$#" count: 2 @@ -29218,16 +29178,6 @@ parameters: count: 2 path: app/cdash/xml_handlers/JSCoverTar_handler.php - - - message: "#^Method JSCoverTarHandler\\:\\:Parse\\(\\) has no return type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/JSCoverTar_handler.php - - - - message: "#^Method JSCoverTarHandler\\:\\:Parse\\(\\) has parameter \\$filename with no type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/JSCoverTar_handler.php - - message: "#^Method JSCoverTarHandler\\:\\:Parse\\(\\) throws checked exception UnexpectedValueException but it's missing from the PHPDoc @throws tag\\.$#" count: 1 @@ -29293,16 +29243,6 @@ parameters: count: 2 path: app/cdash/xml_handlers/JavaJSONTar_handler.php - - - message: "#^Method JavaJSONTarHandler\\:\\:Parse\\(\\) has no return type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/JavaJSONTar_handler.php - - - - message: "#^Method JavaJSONTarHandler\\:\\:Parse\\(\\) has parameter \\$filename with no type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/JavaJSONTar_handler.php - - message: "#^Method JavaJSONTarHandler\\:\\:Parse\\(\\) throws checked exception UnexpectedValueException but it's missing from the PHPDoc @throws tag\\.$#" count: 1 @@ -29428,16 +29368,6 @@ parameters: count: 6 path: app/cdash/xml_handlers/OpenCoverTar_handler.php - - - message: "#^Method OpenCoverTarHandler\\:\\:Parse\\(\\) has no return type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/OpenCoverTar_handler.php - - - - message: "#^Method OpenCoverTarHandler\\:\\:Parse\\(\\) has parameter \\$filename with no type specified\\.$#" - count: 1 - path: app/cdash/xml_handlers/OpenCoverTar_handler.php - - message: "#^Method OpenCoverTarHandler\\:\\:Parse\\(\\) throws checked exception UnexpectedValueException but it's missing from the PHPDoc @throws tag\\.$#" count: 1 From a86ae2373ce97f6569cf4fa34917608795d5ede9 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Mon, 27 Jan 2025 10:01:22 -0500 Subject: [PATCH 15/16] Throw exception for invalid extraction path in extract_tar() --- app/cdash/include/common.php | 3 ++- app/cdash/xml_handlers/GcovTar_handler.php | 9 +-------- app/cdash/xml_handlers/JSCoverTar_handler.php | 9 +-------- app/cdash/xml_handlers/JavaJSONTar_handler.php | 9 +-------- app/cdash/xml_handlers/OpenCoverTar_handler.php | 9 +-------- 5 files changed, 6 insertions(+), 33 deletions(-) diff --git a/app/cdash/include/common.php b/app/cdash/include/common.php index b7001e9dc..91d225685 100644 --- a/app/cdash/include/common.php +++ b/app/cdash/include/common.php @@ -919,6 +919,7 @@ function create_aggregate_build($build, $siteid = null): Build * Extract a tarball within the local storage directory. * * @throws FileNotFoundException + * @throws RuntimeException * @throws UnableToReadFile */ function extract_tar(string $stored_filepath): string @@ -964,7 +965,7 @@ function extract_tar(string $stored_filepath): string Storage::disk('local')->deleteDirectory($localTmpDirPath); report($e); } - return ''; + throw new RuntimeException('Unable to determine valid extraction directory'); } /** diff --git a/app/cdash/xml_handlers/GcovTar_handler.php b/app/cdash/xml_handlers/GcovTar_handler.php index 121869cfc..43b833d42 100644 --- a/app/cdash/xml_handlers/GcovTar_handler.php +++ b/app/cdash/xml_handlers/GcovTar_handler.php @@ -23,7 +23,6 @@ use CDash\Model\CoverageSummary; use CDash\Model\Label; use CDash\Model\SubProject; -use Illuminate\Support\Facades\Log; use League\Flysystem\UnableToReadFile; use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; @@ -71,16 +70,10 @@ public function Parse(string $filename): bool // Extract the tarball. try { $dirName = extract_tar($filename); - } catch (FileNotFoundException|UnableToReadFile $e) { + } catch (FileNotFoundException|UnableToReadFile|RuntimeException $e) { report($e); return false; } - if ($dirName === '') { - Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ - 'function' => 'GcovTarHandler::Parse', - ]); - return false; - } // Find the data.json file and extract the source directory from it. $iterator = new RecursiveIteratorIterator( diff --git a/app/cdash/xml_handlers/JSCoverTar_handler.php b/app/cdash/xml_handlers/JSCoverTar_handler.php index 1f036a979..5af38038b 100644 --- a/app/cdash/xml_handlers/JSCoverTar_handler.php +++ b/app/cdash/xml_handlers/JSCoverTar_handler.php @@ -21,7 +21,6 @@ use CDash\Model\CoverageFileLog; use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Log; use League\Flysystem\UnableToReadFile; use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; @@ -51,16 +50,10 @@ public function Parse(string $filename): bool // Extract the tarball. try { $dirName = extract_tar($filename); - } catch (FileNotFoundException|UnableToReadFile $e) { + } catch (FileNotFoundException|UnableToReadFile|RuntimeException $e) { report($e); return false; } - if ($dirName === '') { - Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ - 'function' => 'JSCoverTarHandler::Parse', - ]); - return false; - } // Recursively search for .json files and parse them. $iterator = new RecursiveIteratorIterator( diff --git a/app/cdash/xml_handlers/JavaJSONTar_handler.php b/app/cdash/xml_handlers/JavaJSONTar_handler.php index 30aa66677..4e911fba1 100644 --- a/app/cdash/xml_handlers/JavaJSONTar_handler.php +++ b/app/cdash/xml_handlers/JavaJSONTar_handler.php @@ -21,7 +21,6 @@ use CDash\Model\CoverageFileLog; use CDash\Model\CoverageSummary; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Log; use League\Flysystem\UnableToReadFile; use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; @@ -47,16 +46,10 @@ public function Parse(string $filename): bool // Extract the tarball. try { $dirName = extract_tar($filename); - } catch (FileNotFoundException|UnableToReadFile $e) { + } catch (FileNotFoundException|UnableToReadFile|RuntimeException $e) { report($e); return false; } - if ($dirName === '') { - Log::error('Could not extract ' . $filename . ' into ' . $dirName, [ - 'function' => 'JavaJSONTarHandler::Parse', - ]); - return false; - } // Check if this submission included a package_map.json file. // This tells us how Java packages correspond to CDash subprojects. diff --git a/app/cdash/xml_handlers/OpenCoverTar_handler.php b/app/cdash/xml_handlers/OpenCoverTar_handler.php index 25d342ab8..4682aee47 100644 --- a/app/cdash/xml_handlers/OpenCoverTar_handler.php +++ b/app/cdash/xml_handlers/OpenCoverTar_handler.php @@ -20,7 +20,6 @@ use CDash\Model\CoverageFile; use CDash\Model\CoverageFileLog; use CDash\Model\CoverageSummary; -use Illuminate\Support\Facades\Log; use League\Flysystem\UnableToReadFile; use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException; @@ -142,16 +141,10 @@ public function Parse(string $filename): bool { try { $this->tarDir = extract_tar($filename); - } catch (FileNotFoundException|UnableToReadFile $e) { + } catch (FileNotFoundException|UnableToReadFile|RuntimeException $e) { report($e); return false; } - if ($this->tarDir === '') { - Log::error("Could not extract {$filename}", [ - 'function' => 'OpenCoverTarHandler::Parse', - ]); - return false; - } // Search for data.json $iterator = new RecursiveIteratorIterator( From 82f2b2d5eb7d01c84e8044b7c2ddb9716762bf97 Mon Sep 17 00:00:00 2001 From: Zack Galbreath Date: Mon, 27 Jan 2025 13:14:28 -0500 Subject: [PATCH 16/16] Disable RemoteWorkers test when using s3 storage --- .github/workflows/ci.yml | 1 + .github/workflows/ctest_driver_script.cmake | 1 + .github/workflows/submit.sh | 3 +++ CMakeLists.txt | 3 +++ app/cdash/tests/CMakeLists.txt | 5 ++++- 5 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06be03dea..1d2d1046e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,6 +14,7 @@ jobs: env: SITENAME: GitHub Actions BASE_IMAGE: ${{matrix.base-image}} + STORAGE_TYPE: ${{matrix.storage}} runs-on: ubuntu-latest strategy: fail-fast: false diff --git a/.github/workflows/ctest_driver_script.cmake b/.github/workflows/ctest_driver_script.cmake index 2678dedd6..b1f808445 100644 --- a/.github/workflows/ctest_driver_script.cmake +++ b/.github/workflows/ctest_driver_script.cmake @@ -18,6 +18,7 @@ ctest_empty_binary_directory("${CTEST_BINARY_DIRECTORY}") set(cfg_options "-DCDASH_DIR_NAME=" "-DCDASH_SERVER=localhost:8080" + "-DCDASH_STORAGE_TYPE=${STORAGE_TYPE}" ) # Backup .env file diff --git a/.github/workflows/submit.sh b/.github/workflows/submit.sh index e5e7d72b7..755e01058 100644 --- a/.github/workflows/submit.sh +++ b/.github/workflows/submit.sh @@ -14,6 +14,8 @@ submit_type="${submit_type:-Experimental}" site="${SITENAME:-$(hostname)}" +storage_type="${STORAGE_TYPE:-local}" + echo "site=$site" echo "database=$database" echo "ctest_driver=$ctest_driver" @@ -40,6 +42,7 @@ docker exec cdash bash -c "\ --schedule-random \ -DSITENAME=\"${site}\" \ -DDATABASE=\"${database}\" \ + -DSTORAGE_TYPE=\"${storage_type}\" \ -DSUBMIT_TYPE=\"${submit_type}\" \ -S \"${ctest_driver}\" \ " diff --git a/CMakeLists.txt b/CMakeLists.txt index 527863051..129d2cdca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,6 +10,9 @@ configure_file( # to configure the testing install set(CDASH_SERVER localhost CACHE STRING "CDash testing server") set(CDASH_IMAGE "$ENV{BASE_IMAGE}" CACHE STRING "Docker image name") +if(NOT DEFINED CDASH_STORAGE_TYPE) + set(CDASH_STORAGE_TYPE "local") +endif() get_filename_component(CDASH_DIR_NAME_DEFAULT ${CDash_SOURCE_DIR} NAME) set(CDASH_DIR_NAME "${CDASH_DIR_NAME_DEFAULT}" CACHE STRING "URL suffix. Ie 'http:///'") diff --git a/app/cdash/tests/CMakeLists.txt b/app/cdash/tests/CMakeLists.txt index 19f8eb795..44b5f9254 100644 --- a/app/cdash/tests/CMakeLists.txt +++ b/app/cdash/tests/CMakeLists.txt @@ -261,7 +261,10 @@ add_unit_test(/CDash/Messaging/Topic/TopicDecorator) set_tests_properties(/CDash/Messaging/Topic/TopicDecorator PROPERTIES DEPENDS /CDash/XmlHandler/UpdateHandler) add_laravel_test(/Feature/RemoteWorkers) -set_tests_properties(/Feature/RemoteWorkers PROPERTIES DEPENDS /CDash/XmlHandler/UpdateHandler) +set_tests_properties(/Feature/RemoteWorkers PROPERTIES + DEPENDS /CDash/XmlHandler/UpdateHandler + DISABLED "$>" +) add_laravel_test(/Feature/Jobs/NotifyExpiringAuthTokensTest) set_tests_properties(/Feature/Jobs/NotifyExpiringAuthTokensTest PROPERTIES RUN_SERIAL TRUE)