Speed up unit tests in CI (#5360)

* ci: drop gen-5 instance families from the test runner

m5a runners run the unit suite ~2.3x slower than m8id (15m vs 6.5m for `go test`), and when spot capacity for newer families dries up runs-on falls back to them. Keeping gen-6 through gen-8 leaves a wide spot pool without the slow tail.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* test: shrink decompression bomb fixtures

The bomb and heap-bound tests pushed 100MB-16GB through gzip and parsers, which under `-race` made them the slowest tests in the unit suite. Each now uses the smallest fixture that still separates the bounded path from the unbounded one.

- arch mtree caps are measured at a small cap and scaled to the shipped constants, so raising a cap still fails the budget
- snap and debian bombs expand to 32MB instead of 512MB (4x their 8MB budgets)
- golang ELF bombs still declare 256MB but deliver 64MB

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
Alex Goodman
2026-09-30 10:58:11 -04:00
committed by GitHub
parent 6ac7afb439
commit fdeb78fc19
5 changed files with 76 additions and 46 deletions
+1 -1
View File
@@ -27,7 +27,7 @@ jobs:
# enough; a 4 vCPU instance can't pull much more than that anyway. # enough; a 4 vCPU instance can't pull much more than that anyway.
# note: s3-cache intentionally omitted -- PR runs are untrusted and must not write to the # note: s3-cache intentionally omitted -- PR runs are untrusted and must not write to the
# shared cache backend that the trusted release workflow reads from (cache poisoning). # shared cache backend that the trusted release workflow reads from (cache poisoning).
runs-on: &test-runner "runs-on=${{ github.run_id }}/cpu=4+8/ram=16+32/family=m5+m6+m7+m8+c5+c6+c7+c8/spot=price-capacity-optimized/volume=100gb" runs-on: &test-runner "runs-on=${{ github.run_id }}/cpu=4+8/ram=16+32/family=m6+m7+m8+c6+c7+c8/spot=price-capacity-optimized/volume=100gb"
permissions: permissions:
contents: read contents: read
steps: steps:
+38 -19
View File
@@ -492,12 +492,16 @@ func Test_parseMtree_malformedInput(t *testing.T) {
// bytes still costs multiples of its own size in heap. Asserting the error would keep passing if the // bytes still costs multiples of its own size in heap. Asserting the error would keep passing if the
// line cap were removed, because the byte cap would still reject a large enough input eventually. // line cap were removed, because the byte cap would still reject a large enough input eventually.
func Test_parseMtree_lineCapBoundsHeap(t *testing.T) { func Test_parseMtree_lineCapBoundsHeap(t *testing.T) {
// newline-only lines are the worst case: minimum bytes per entry, so the most entries per byte // the property is linear in the cap, so it is measured at a small cap and scaled to the shipped
const payloadSize = 16 * intFile.MB // one. Running it at 300k lines means an uncapped parse costing ~16GB, which under -race is most
spec := bytes.Repeat([]byte{'\n'}, payloadSize) // of a CI minute for a test with no concurrency in it.
const (
lineCap = 4096
payloadSize = 64 * lineCap
)
// well inside the byte cap, so only the line cap can stop this // newline-only lines are the worst case: minimum bytes per entry, so the most entries per byte
require.Less(t, int64(payloadSize), int64(maxMtreeSize)) spec := bytes.Repeat([]byte{'\n'}, payloadSize)
unbounded := testutils.MeasureAlloc(t, func() { unbounded := testutils.MeasureAlloc(t, func() {
// the same listing with the line cap lifted, to prove the fixture is really a bomb // the same listing with the line cap lifted, to prove the fixture is really a bomb
@@ -507,19 +511,29 @@ func Test_parseMtree_lineCapBoundsHeap(t *testing.T) {
require.Greater(t, unbounded, uint64(payloadSize), require.Greater(t, unbounded, uint64(payloadSize),
"fixture did not actually cost more than its own size; it is no longer a bomb") "fixture did not actually cost more than its own size; it is no longer a bomb")
bounded := testutils.MeasureAlloc(t, func() { boundedAt := func(limit int) uint64 {
_, err := parseMtreeWithLimits(gzipOf(t, spec), maxMtreeSize, maxMtreeLines) gz := gzipOf(t, spec)
require.ErrorIs(t, err, errTooManyMtreeLines) return testutils.MeasureAlloc(t, func() {
}) _, err := parseMtreeWithLimits(gz, maxMtreeSize, limit)
require.ErrorIs(t, err, errTooManyMtreeLines)
})
}
bounded := boundedAt(lineCap)
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded) // the marginal cost between two caps, so fixed setup (gzip reader, parser) does not get scaled
perLine := (boundedAt(2*lineCap) - bounded) / lineCap
atShippedCap := perLine * maxMtreeLines
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes (%d per line, ~%d at the shipped cap)",
unbounded, bounded, perLine, atShippedCap)
// measured at ~238MB against ~16.5GB uncapped, so the cap is doing its job, but note what it assert.Less(t, bounded, unbounded/10,
// actually buys: 300k entries still cost a few hundred MB, and that is per concurrent cataloger.
// The budget is set to catch the cap being removed or raised by an order of magnitude, not to pin
// the exact figure.
assert.Less(t, bounded, uint64(512*intFile.MB),
"the line cap has to stop the parse before it builds an entry per line") "the line cap has to stop the parse before it builds an entry per line")
// measured at ~238MB for the shipped 300k lines, so the cap is doing its job, but note what it
// actually buys: that is still a few hundred MB, per concurrent cataloger. The budget is set to
// catch the cap being raised by an order of magnitude, not to pin the exact figure.
assert.Less(t, atShippedCap, uint64(512*intFile.MB),
"the shipped line cap allows more heap than budgeted")
} }
// Test_parseMtree_sizeCapBoundsHeap is the byte-cap counterpart to the line-cap budget above. // Test_parseMtree_sizeCapBoundsHeap is the byte-cap counterpart to the line-cap budget above.
@@ -532,9 +546,12 @@ func Test_parseMtree_lineCapBoundsHeap(t *testing.T) {
// be ordinary for the byte cap to be the thing under test, and there have to be few enough of them // be ordinary for the byte cap to be the thing under test, and there have to be few enough of them
// that the line cap does not fire first. // that the line cap does not fire first.
func Test_parseMtree_sizeCapBoundsHeap(t *testing.T) { func Test_parseMtree_sizeCapBoundsHeap(t *testing.T) {
// same reasoning as the line cap: measured at a small byte cap, since the shipped 64MB cap means
// gzipping and parsing ~200MB of fixture, which is slow under -race and proves nothing more.
const ( const (
sizeCap = intFile.MB
lineLen = 1024 lineLen = 1024
numLines = 96 * 1024 // ~96MB across ~98k lines: past the 64MB cap, well under the 300k line cap numLines = 2 * sizeCap / lineLen // twice the cap, and far under the line cap
) )
var raw bytes.Buffer var raw bytes.Buffer
@@ -542,7 +559,7 @@ func Test_parseMtree_sizeCapBoundsHeap(t *testing.T) {
for range numLines { for range numLines {
raw.Write(line) raw.Write(line)
} }
require.Greater(t, int64(raw.Len()), int64(maxMtreeSize), "fixture must exceed the byte cap") require.Greater(t, int64(raw.Len()), int64(sizeCap), "fixture must exceed the byte cap")
require.Less(t, numLines, maxMtreeLines, "and must not trip the line cap first") require.Less(t, numLines, maxMtreeLines, "and must not trip the line cap first")
unbounded := testutils.MeasureAlloc(t, func() { unbounded := testutils.MeasureAlloc(t, func() {
@@ -554,11 +571,13 @@ func Test_parseMtree_sizeCapBoundsHeap(t *testing.T) {
"fixture did not actually cost more than its own size; it is no longer a bomb") "fixture did not actually cost more than its own size; it is no longer a bomb")
bounded := testutils.MeasureAlloc(t, func() { bounded := testutils.MeasureAlloc(t, func() {
_, err := parseMtreeWithLimits(gzipOf(t, raw.Bytes()), maxMtreeSize, maxMtreeLines) _, err := parseMtreeWithLimits(gzipOf(t, raw.Bytes()), sizeCap, maxMtreeLines)
require.ErrorIs(t, err, errMtreeTooLarge) require.ErrorIs(t, err, errMtreeTooLarge)
}) })
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded) t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded)
assert.Less(t, bounded, uint64(512*intFile.MB),
// the old budget was 512MB against the 64MB cap, so 8x the cap
assert.Less(t, bounded, uint64(8*sizeCap),
"the size cap has to stop the read at the cap, not after the member is drained") "the size cap has to stop the read at the cap, not after the member is drained")
} }
@@ -373,9 +373,11 @@ func Test_processDataTar_capsCopyrightFiles(t *testing.T) {
// than after the fact. The existing bounds test uses a payload small enough that buffering the whole // than after the fact. The existing bounds test uses a payload small enough that buffering the whole
// thing would pass anyway; this one is large enough that the difference shows up as bytes. // thing would pass anyway; this one is large enough that the difference shows up as bytes.
func Test_decompressionStream_failsEarlyInsteadOfBuffering(t *testing.T) { func Test_decompressionStream_failsEarlyInsteadOfBuffering(t *testing.T) {
const expanded = 512 * intFile.MB // 4x the 8MB budget below is plenty to tell bounded from unbounded. Going bigger only makes the
// fixture slower to build and inflate, which is most of this test's time under -race.
const expanded = 32 * intFile.MB
// well under 1% of input expanding to 512MB, so heap is what separates a streaming bound from a // well under 1% of input expanding to 32MB, so heap is what separates a streaming bound from a
// bound checked after the read completes // bound checked after the read completes
var gz bytes.Buffer var gz bytes.Buffer
gw := gzip.NewWriter(&gz) gw := gzip.NewWriter(&gz)
@@ -20,19 +20,25 @@ import (
// Test_getBuildInfo_compressedSectionBomb covers the reason readBuildInfo exists: debug/buildinfo opens // Test_getBuildInfo_compressedSectionBomb covers the reason readBuildInfo exists: debug/buildinfo opens
// ELF files with debug/elf itself, and elf.NewFile expands the section-name string table as it parses, // ELF files with debug/elf itself, and elf.NewFile expands the section-name string table as it parses,
// so an oversized compression header there is an unbounded allocation on a path elfutil.NewFile never // so an oversized compression header there is an unbounded allocation on a path elfutil.NewFile never
// sees. The fixture is a real zlib stream, so it delivers every byte its header promises. // sees. The fixture is a real zlib stream, but it stops short of what its header promises: delivering
// the full 256MB only made it slower to build and inflate, mostly under -race.
func Test_getBuildInfo_compressedSectionBomb(t *testing.T) { func Test_getBuildInfo_compressedSectionBomb(t *testing.T) {
const declared = 256 << 20 // comfortably over elfutil's bound, small enough to allocate in a test const (
declared = 256 << 20 // comfortably over elfutil's bound, which only reads the header
// debug/elf allocates the declared size up front today, so delivery barely matters. Twice the
// guarded budget keeps the fixture a bomb if it ever switches to reading in chunks.
delivered = 64 << 20
)
bomb := elfWithCompressedNameTable(t, declared) bomb := elfWithCompressedNameTable(t, declared, delivered)
t.Logf("%d byte fixture declares a %d byte section name table", len(bomb), declared) t.Logf("%d byte fixture declares a %d byte section name table and delivers %d", len(bomb), declared, delivered)
// the unguarded path is the thing being defended against: prove the fixture really is a bomb // the unguarded path is the thing being defended against: prove the fixture really is a bomb
unguarded := testutils.MeasureAlloc(t, func() { unguarded := testutils.MeasureAlloc(t, func() {
_, err := buildinfo.Read(bytes.NewReader(bomb)) _, err := buildinfo.Read(bytes.NewReader(bomb))
t.Logf("buildinfo.Read err: %v", err) t.Logf("buildinfo.Read err: %v", err)
}) })
assert.Greater(t, unguarded, uint64(declared), "fixture did not actually deliver the declared bytes") assert.Greater(t, unguarded, uint64(delivered), "fixture did not actually deliver its bytes")
guarded := testutils.MeasureAlloc(t, func() { guarded := testutils.MeasureAlloc(t, func() {
_, err := getBuildInfo(bytes.NewReader(bomb)) _, err := getBuildInfo(bytes.NewReader(bomb))
@@ -47,17 +53,22 @@ func Test_getBuildInfo_compressedSectionBomb(t *testing.T) {
// goversion opens the file itself and reads .symtab plus the string table it links, and those are // goversion opens the file itself and reads .symtab plus the string table it links, and those are
// expanded lazily, so the section-name table bound getBuildInfo applies never reaches them. // expanded lazily, so the section-name table bound getBuildInfo applies never reaches them.
func Test_getCryptoInformation_compressedSymtabBomb(t *testing.T) { func Test_getCryptoInformation_compressedSymtabBomb(t *testing.T) {
const declared = 256 << 20 // comfortably over elfutil's bound, small enough to allocate in a test const (
declared = 256 << 20 // comfortably over elfutil's bound, which only reads the header
// debug/elf allocates the declared size up front today, so delivery barely matters. Twice the
// guarded budget keeps the fixture a bomb if it ever switches to reading in chunks.
delivered = 64 << 20
)
bomb := elfWithCompressedSymtab(t, declared) bomb := elfWithCompressedSymtab(t, declared, delivered)
t.Logf("%d byte fixture declares a %d byte symbol table", len(bomb), declared) t.Logf("%d byte fixture declares a %d byte symbol table and delivers %d", len(bomb), declared, delivered)
// the unguarded path is the thing being defended against: prove the fixture really is a bomb // the unguarded path is the thing being defended against: prove the fixture really is a bomb
unguarded := testutils.MeasureAlloc(t, func() { unguarded := testutils.MeasureAlloc(t, func() {
_, err := version.ReadExeFromReader(bytes.NewReader(bomb)) _, err := version.ReadExeFromReader(bytes.NewReader(bomb))
t.Logf("goversion err: %v", err) t.Logf("goversion err: %v", err)
}) })
assert.Greater(t, unguarded, uint64(declared), "fixture did not actually deliver the declared bytes") assert.Greater(t, unguarded, uint64(delivered), "fixture did not actually deliver its bytes")
guarded := testutils.MeasureAlloc(t, func() { guarded := testutils.MeasureAlloc(t, func() {
_, err := getCryptoInformation(bytes.NewReader(bomb)) _, err := getCryptoInformation(bytes.NewReader(bomb))
@@ -85,12 +96,12 @@ func Test_getCryptoInformation_passesThroughNonELF(t *testing.T) {
} }
// elfWithCompressedSymtab builds a minimal ELF64 whose .symtab is SHF_COMPRESSED, declaring `declared` // elfWithCompressedSymtab builds a minimal ELF64 whose .symtab is SHF_COMPRESSED, declaring `declared`
// decompressed bytes and genuinely delivering them. The name table is left uncompressed so the file gets // decompressed bytes and genuinely delivering `delivered` of them. The name table is left uncompressed so the file gets
// past the bound getBuildInfo already applies, which is the point: this is the section that bound misses. // past the bound getBuildInfo already applies, which is the point: this is the section that bound misses.
func elfWithCompressedSymtab(t *testing.T, declared uint64) []byte { func elfWithCompressedSymtab(t *testing.T, declared, delivered uint64) []byte {
t.Helper() t.Helper()
payload := make([]byte, declared) payload := make([]byte, delivered)
var compressed bytes.Buffer var compressed bytes.Buffer
zw := zlib.NewWriter(&compressed) zw := zlib.NewWriter(&compressed)
_, err := zw.Write(payload) _, err := zw.Write(payload)
@@ -150,12 +161,12 @@ func elfWithCompressedSymtab(t *testing.T, declared uint64) []byte {
} }
// elfWithCompressedNameTable builds a minimal ELF64 whose only real section is a SHF_COMPRESSED .shstrtab // elfWithCompressedNameTable builds a minimal ELF64 whose only real section is a SHF_COMPRESSED .shstrtab
// declaring `declared` decompressed bytes and genuinely delivering them. Only the test that runs the // declaring `declared` decompressed bytes and genuinely delivering `delivered` of them. Only the test that runs the
// unguarded path needs delivery; use elfDeclaringNameTable everywhere else, since building this one costs // unguarded path needs delivery; use elfDeclaringNameTable everywhere else, since building this one costs
// `declared` bytes of allocation plus a zlib compress of them. // `delivered` bytes of allocation plus a zlib compress of them.
func elfWithCompressedNameTable(t *testing.T, declared uint64) []byte { func elfWithCompressedNameTable(t *testing.T, declared, delivered uint64) []byte {
t.Helper() t.Helper()
return elfNameTableFixture(t, declared, true) return elfNameTableFixture(t, declared, delivered)
} }
// elfDeclaringNameTable declares `declared` bytes without delivering them. CheckSectionNameTable reads the // elfDeclaringNameTable declares `declared` bytes without delivering them. CheckSectionNameTable reads the
@@ -163,19 +174,15 @@ func elfWithCompressedNameTable(t *testing.T, declared uint64) []byte {
// bound itself never needs the stream to be real. // bound itself never needs the stream to be real.
func elfDeclaringNameTable(t *testing.T, declared uint64) []byte { func elfDeclaringNameTable(t *testing.T, declared uint64) []byte {
t.Helper() t.Helper()
return elfNameTableFixture(t, declared, false) return elfNameTableFixture(t, declared, 64)
} }
func elfNameTableFixture(t *testing.T, declared uint64, deliver bool) []byte { func elfNameTableFixture(t *testing.T, declared, delivered uint64) []byte {
t.Helper() t.Helper()
// the decompressed name table only has to start with the section names; the rest is padding that // the decompressed name table only has to start with the section names; the rest is padding that
// exists purely to make the declared size real // exists purely to make the declared size real
size := declared payload := make([]byte, delivered)
if !deliver {
size = 64
}
payload := make([]byte, size)
copy(payload, "\x00.shstrtab\x00") copy(payload, "\x00.shstrtab\x00")
var compressed bytes.Buffer var compressed bytes.Buffer
@@ -318,7 +318,9 @@ func TestParseKernelChangelog(t *testing.T) {
// thing holding this path, and it is incidental: switching to io.ReadAll or a bufio.Reader to pick up // thing holding this path, and it is incidental: switching to io.ReadAll or a bufio.Reader to pick up
// long lines would remove it silently, which is what this test is here to catch. // long lines would remove it silently, which is what this test is here to catch.
func TestParseKernelChangelog_gzipBombStaysBounded(t *testing.T) { func TestParseKernelChangelog_gzipBombStaysBounded(t *testing.T) {
const expanded = 512 * 1024 * 1024 // 4x the 8MB budget below is plenty to tell bounded from unbounded. Going bigger only makes the
// fixture slower to build and inflate, which is most of this test's time under -race.
const expanded = 32 * 1024 * 1024
// one enormous line with no newline, the worst case for a line scanner // one enormous line with no newline, the worst case for a line scanner
var gz bytes.Buffer var gz bytes.Buffer