From 51a7e9761978354ebe8e7a8f2714e1e0af246504 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 5 Nov 2026 10:36:21 +0100 Subject: [PATCH] fix(healthcheck): import Dockerfile HEALTHCHECK options like Docker Read compound and fractional durations (1m30s, 1.5m), quoted values, only the flags before CMD, the escape parser directive, and NONE with flags. Zero and sub-second values no longer become 0 seconds. Closes #12455 Co-Authored-By: Claude Opus 5.5 --- app/Models/Application.php | 27 ++---- bootstrap/helpers/shared.php | 97 +++++++++++++------ .../ApplicationHealthcheckRemovalTest.php | 69 +++++++++++++ 3 files changed, 142 insertions(+), 51 deletions(-) diff --git a/app/Models/Application.php b/app/Models/Application.php index 6d6cacf5fd..23ca1881fe 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -2901,27 +2901,14 @@ class Application extends BaseModel } if ($hasHealthcheck && ($this->isHealthcheckDisabled() || $isInit)) { - $interval = str($healthcheckCommand)->match('/--interval=([0-9]+[a-zµ]*)/'); - $timeout = str($healthcheckCommand)->match('/--timeout=([0-9]+[a-zµ]*)/'); - $start_period = str($healthcheckCommand)->match('/--start-period=([0-9]+[a-zµ]*)/'); - $retries = str($healthcheckCommand)->match('/--retries=(\d+)/'); + $options = dockerfileHealthcheckOptions($healthcheckCommand); - if ($interval->isNotEmpty()) { - $this->health_check_interval = parseDockerfileInterval($interval); - } - if ($timeout->isNotEmpty()) { - $this->health_check_timeout = parseDockerfileInterval($timeout); - } - if ($start_period->isNotEmpty()) { - $this->health_check_start_period = parseDockerfileInterval($start_period); - } - if ($retries->isNotEmpty()) { - $this->health_check_retries = $retries->toInteger(); - } - if ($interval || $timeout || $start_period || $retries) { - $this->custom_healthcheck_found = true; - $this->save(); - } + $this->health_check_interval = $options['interval'] ?? $this->health_check_interval; + $this->health_check_timeout = $options['timeout'] ?? $this->health_check_timeout; + $this->health_check_start_period = $options['start_period'] ?? $this->health_check_start_period; + $this->health_check_retries = $options['retries'] ?? $this->health_check_retries; + $this->custom_healthcheck_found = true; + $this->save(); } } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 310d6d794c..5afb07319a 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -5138,15 +5138,20 @@ function getJobStatus(?string $jobId = null) */ function dockerfileHealthcheckInstruction(?string $dockerfile, ?string $target = null): ?string { + $dockerfile = (string) $dockerfile; + // The "# escape=`" parser directive (Windows Dockerfiles) is valid only in the comment lines at the top of the file. + $escape = preg_match('/\A(?:[ \t]*#[ \t]*[a-z]+[ \t]*=[^\r\n]*\R)*?[ \t]*#[ \t]*escape[ \t]*=[ \t]*([\\\\`])[ \t]*$/im', $dockerfile, $directive) + ? $directive[1] + : '\\'; $instructions = []; $current = ''; - foreach (preg_split('/\R/', (string) $dockerfile) as $line) { + foreach (preg_split('/\R/', $dockerfile) as $line) { $line = trim($line); if ($line === '' || str_starts_with($line, '#')) { continue; } - $continues = str_ends_with($line, '\\'); - $current = trim($current.' '.rtrim($line, '\\ ')); + $continues = str_ends_with($line, $escape); + $current = trim($current.' '.($continues ? substr($line, 0, -1) : $line)); if (! $continues) { $instructions[] = $current; $current = ''; @@ -5184,7 +5189,7 @@ function dockerfileHealthcheckInstruction(?string $dockerfile, ?string $target = while ($index !== null) { $stage = $stages[$index]; if ($stage['healthcheck'] !== null) { - return preg_match('/^HEALTHCHECK\s+NONE$/i', $stage['healthcheck']) ? null : $stage['healthcheck']; + return preg_match('/^HEALTHCHECK(?:\s+--\S+)*\s+NONE$/i', $stage['healthcheck']) ? null : $stage['healthcheck']; } $parentIndex = null; foreach ($stages as $i => $candidate) { @@ -5198,36 +5203,66 @@ function dockerfileHealthcheckInstruction(?string $dockerfile, ?string $target = return null; } -function parseDockerfileInterval(string $something) +/** + * Read the options of a HEALTHCHECK instruction like Docker does: only the --name=value flags before CMD count. + * An option is null when it is missing or not valid. Docker uses its default for a zero interval, timeout, or retries + * (30s, 30s, 3), so those are returned instead of 0. + * + * @return array{interval: ?int, timeout: ?int, start_period: ?int, retries: ?int} + */ +function dockerfileHealthcheckOptions(string $instruction): array { - $value = preg_replace('/[^0-9]/', '', $something); - $unit = preg_replace('/[0-9]/', '', $something); - - // Default to seconds if no unit specified - $unit = $unit ?: 's'; - - // Convert to seconds based on unit - $seconds = (int) $value; - switch ($unit) { - case 'ns': - $seconds = (int) ($value / 1000000000); - break; - case 'us': - case 'µs': - $seconds = (int) ($value / 1000000); - break; - case 'ms': - $seconds = (int) ($value / 1000); - break; - case 'm': - $seconds = (int) ($value * 60); - break; - case 'h': - $seconds = (int) ($value * 3600); - break; + preg_match('/^HEALTHCHECK((?:\s+--[a-z-]+=(?:"[^"]*"|\'[^\']*\'|\S+))*)/i', $instruction, $flagsPart); + preg_match_all('/--([a-z-]+)=("[^"]*"|\'[^\']*\'|\S+)/i', $flagsPart[1] ?? '', $flags, PREG_SET_ORDER); + $values = []; + foreach ($flags as [, $name, $value]) { + $values[$name] = preg_replace('/^(["\'])(.*)\1$/', '$2', $value); } - return $seconds; + $interval = parseDockerfileInterval($values['interval'] ?? ''); + $timeout = parseDockerfileInterval($values['timeout'] ?? ''); + $retries = preg_match('/^\+?\d{1,9}$/', $values['retries'] ?? '') ? (int) $values['retries'] : null; + + return [ + 'interval' => $interval === 0 ? 30 : $interval, + 'timeout' => $timeout === 0 ? 30 : $timeout, + 'start_period' => parseDockerfileInterval($values['start-period'] ?? ''), + 'retries' => $retries === 0 ? 3 : $retries, + ]; +} + +/** + * Convert a Dockerfile HEALTHCHECK duration (Go duration syntax, for example "1m30s" or "1.5m") to whole seconds. + * A value without a unit is read as seconds. A positive value below one second becomes 1, so it does not read as zero. + * Returns null when the value is not a valid duration or does not fit the integer column. + */ +function parseDockerfileInterval(string $duration): ?int +{ + if (str_starts_with($duration, '+')) { + $duration = substr($duration, 1); + } + if (preg_match('/^\d+$/', $duration)) { + return (int) $duration <= 2147483647 ? (int) $duration : null; + } + + $unitSeconds = ['ns' => 1e-9, 'us' => 1e-6, 'µs' => 1e-6, 'μs' => 1e-6, 'ms' => 1e-3, 's' => 1, 'm' => 60, 'h' => 3600]; + preg_match_all('/(\d+(?:\.\d*)?|\.\d+)(ns|us|µs|μs|ms|s|m|h)/u', $duration, $parts, PREG_SET_ORDER); + if ($parts === [] || implode('', array_column($parts, 0)) !== $duration) { + return null; + } + + $seconds = 0.0; + foreach ($parts as [, $value, $unit]) { + $seconds += (float) $value * $unitSeconds[$unit]; + } + if ($seconds > 0 && $seconds < 1) { + return 1; + } + + // Round away float noise (0.3m = 17.999...s), then drop the fraction: the columns store whole seconds. + $seconds = floor(round($seconds, 6)); + + return $seconds <= 2147483647 ? (int) $seconds : null; } function addPreviewDeploymentSuffix(string $name, int $pull_request_id = 0): string diff --git a/tests/Unit/ApplicationHealthcheckRemovalTest.php b/tests/Unit/ApplicationHealthcheckRemovalTest.php index 9dc5d53f0b..64ba0712f2 100644 --- a/tests/Unit/ApplicationHealthcheckRemovalTest.php +++ b/tests/Unit/ApplicationHealthcheckRemovalTest.php @@ -97,3 +97,72 @@ it('finds the HEALTHCHECK that applies to the built image', function (string $do 'comment mentions HEALTHCHECK' => ["FROM node\n# HEALTHCHECK is not needed\nCMD run", null, null], 'empty Dockerfile' => ['', null, null], ]); + +it('reads compound and fractional HEALTHCHECK durations like Docker', function () { + $application = healthcheckApplication(); + + $application->parseHealthcheckFromDockerfile("FROM scratch\nHEALTHCHECK --interval=1m30s --timeout=1.5m --start-period=2m15s --retries=4 CMD [\"/probe\"]"); + + expect($application->health_check_interval)->toBe(90) + ->and($application->health_check_timeout)->toBe(90) + ->and($application->health_check_start_period)->toBe(135) + ->and($application->health_check_retries)->toBe(4); +}); + +it('keeps the current value when a HEALTHCHECK duration is not valid', function () { + $application = healthcheckApplication(); + + $application->parseHealthcheckFromDockerfile("FROM scratch\nHEALTHCHECK --interval=1x --timeout=3000000h --retries=2 CMD [\"/probe\"]"); + + expect($application->health_check_interval)->toBe(5) + ->and($application->health_check_timeout)->toBe(5) + ->and($application->health_check_retries)->toBe(2); +}); + +it('converts a Docker duration to whole seconds', function (string $duration, ?int $expected) { + expect(parseDockerfileInterval($duration))->toBe($expected); +})->with([ + 'seconds' => ['30s', 30], + 'no unit' => ['30', 30], + 'minutes' => ['2m', 120], + 'compound' => ['1h2m3s', 3723], + 'fractional' => ['1.5m', 90], + 'fraction without leading digit' => ['.5m', 30], + 'fractional compound' => ['0.3m1.2s', 19], + 'milliseconds' => ['1500ms', 1], + 'microseconds' => ['2000000µs', 2], + 'nanoseconds' => ['3000000000ns', 3], + 'leading plus' => ['+30s', 30], + 'zero' => ['0s', 0], + 'below one second' => ['500ms', 1], + 'negative' => ['-30s', null], + 'unknown unit' => ['5x', null], + 'trailing text' => ['5sec', null], + 'empty' => ['', null], + 'out of range' => ['3000000h', null], +]); + +it('reads only the HEALTHCHECK flags before CMD, like Docker', function (string $instruction, array $expected) { + expect(dockerfileHealthcheckOptions($instruction))->toBe($expected); +})->with([ + 'all options' => ['HEALTHCHECK --interval=10s --timeout=3s --start-period=1m --retries=7 CMD ["/probe"]', ['interval' => 10, 'timeout' => 3, 'start_period' => 60, 'retries' => 7]], + 'no options' => ['HEALTHCHECK CMD ["/probe"]', ['interval' => null, 'timeout' => null, 'start_period' => null, 'retries' => null]], + 'quoted values' => ['HEALTHCHECK --interval="18s" --timeout=\'4s\' CMD /probe', ['interval' => 18, 'timeout' => 4, 'start_period' => null, 'retries' => null]], + 'flags of the command are not options' => ['HEALTHCHECK --interval=19s CMD /probe --timeout=99s --retries=9', ['interval' => 19, 'timeout' => null, 'start_period' => null, 'retries' => null]], + 'zero uses the Docker defaults' => ['HEALTHCHECK --interval=0s --timeout=0 --start-period=0s --retries=0 CMD /probe', ['interval' => 30, 'timeout' => 30, 'start_period' => 0, 'retries' => 3]], + 'start-interval is not interval' => ['HEALTHCHECK --start-interval=46s CMD /probe', ['interval' => null, 'timeout' => null, 'start_period' => null, 'retries' => null]], + 'retries with plus sign' => ['HEALTHCHECK --retries=+4 CMD /probe', ['interval' => null, 'timeout' => null, 'start_period' => null, 'retries' => 4]], + 'retries not an integer' => ['HEALTHCHECK --retries=1.5 CMD /probe', ['interval' => null, 'timeout' => null, 'start_period' => null, 'retries' => null]], +]); + +it('reads Dockerfile syntax around HEALTHCHECK like Docker', function (string $dockerfile, ?string $expected) { + expect(dockerfileHealthcheckInstruction($dockerfile))->toBe($expected); +})->with([ + 'escape directive' => ["# escape=`\nFROM scratch\nHEALTHCHECK --interval=24s `\n --timeout=6s CMD /probe", 'HEALTHCHECK --interval=24s --timeout=6s CMD /probe'], + 'escape directive after syntax directive' => ["# syntax=docker/dockerfile:1\n# escape=`\nFROM scratch\nHEALTHCHECK --interval=49s `\n CMD /probe", 'HEALTHCHECK --interval=49s CMD /probe'], + 'escape comment after an instruction' => ["FROM scratch\n# escape=`\nHEALTHCHECK --interval=50s \\\n CMD /probe", 'HEALTHCHECK --interval=50s CMD /probe'], + 'backslash in command with backtick escape' => ["# escape=`\nFROM scratch\nHEALTHCHECK CMD c:\\probe.exe\\", 'HEALTHCHECK CMD c:\\probe.exe\\'], + 'continuation with trailing space' => ["FROM scratch\nHEALTHCHECK --interval=48s \\ \n CMD /probe", 'HEALTHCHECK --interval=48s CMD /probe'], + 'CRLF line endings' => ["FROM scratch\r\nHEALTHCHECK --interval=27s \\\r\n CMD /probe\r\n", 'HEALTHCHECK --interval=27s CMD /probe'], + 'NONE with flags' => ["FROM scratch\nHEALTHCHECK --interval=44s NONE", null], +]);