fix(api): drop a sub-1MB numeric mem_limit instead of reading it as unlimited - #466
Open
shani-singh1 wants to merge 1 commit into
Open
fix(api): drop a sub-1MB numeric mem_limit instead of reading it as unlimited#466shani-singh1 wants to merge 1 commit into
shani-singh1 wants to merge 1 commit into
Conversation
…nlimited
`parseComposeMemory`'s string path already refuses a value that rounds below
1 MB (`mb >= 1 ? … : undefined`), but the number path did not — it returned
`Math.floor(bytes / 1MB)`, which is `0` for anything under a megabyte:
parseComposeMemory(512) -> 0 parseComposeMemory("512") -> undefined
parseComposeMemory(1048575) -> 0 parseComposeMemory("1048575") -> undefined
`0` is not inert. Downstream, `memoryMb: 0` is UNLIMITED_RESOURCES and
`hasMemoryLimit` returns false for it (packages/core/resources.ts), and
`parseServiceResources` keeps `memoryMb: 0` (it only drops `undefined`). So a
service written as `mem_limit: 512` — almost always a typo for `512m` — parses
to *no memory limit at all* instead of a cap. That is the exact "a malformed
limit must not silently become a tiny cap" failure this parser's own doc-comment
and the oblien#333 tests set out to prevent; here it's worse, becoming no cap.
Apply the same sub-1MB guard on the number path so it agrees with the string
path. Values >= 1 MB are unchanged (e.g. the bare-bytes `1073741824` -> 1024 MB
case stays). Added regression tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
parseComposeMemory(apps/api/src/lib/compose-parser.ts) parses a service'smem_limit/deploy.resources.limits.memoryto MB. The string path already refuses a value that rounds below 1 MB (mb >= 1 ? … : undefined), but the number path did not:Math.floor(bytes / 1MB)is0for anything under a megabyte, so the two paths disagree for the same byte count:0is not inert here. Inpackages/core/src/resources.ts,UNLIMITED_RESOURCESis{ memoryMb: 0, … }andhasMemoryLimitreturns false formemoryMb: 0, andparseServiceResourceskeepsmemoryMb: 0(it only dropsundefined). So a service written as:parses to no memory limit at all instead of a cap. That's the exact "a malformed limit must not silently become a tiny cap" failure this parser's own doc-comment and the #333 resource-limit tests exist to prevent — here it's worse, silently becoming unlimited.
Fix
Apply the same sub-1MB guard on the number path so it agrees with the string path:
Values ≥ 1 MB are unchanged — the existing bare-bytes case
mem_limit: 1073741824 → 1024 MBstill passes, as do all suffix spellings and the swarm form. Only sub-1MB numeric byte values change (0→undefined), which then drops out ofparseServiceResourcesinstead of masquerading as unlimited.Tests
Added to the existing "service resource limits" block:
Verified against the function logic: before the fix
512/1048575return0; after, they returnundefined(matching the string path), while every value ≥ 1 MB (incl. the pinned1073741824 → 1024) is unchanged.