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], +]);