a73x

b7ac3a54

feat(agent): a raw image needs no conversion tool

a73x   2026-08-06 09:12

Commit message
feat(agent): a raw image needs no conversion tool

The image cache identifies what it downloaded by its magic bytes rather than
trusting the URL, which lies: both Ubuntu cloud images ship qcow2 behind a
`.img` suffix. An image that is already raw is materialised by renaming it into
place — the conversion step exists to change a format, and there is no format to
change.
docs/assumptions.md
Old New
@@ -38,6 +38,16 @@ running anything you care about on a machine that reboots.
38 boot a persistent guest was running again on the same address it held before, 38 boot a persistent guest was running again on the same address it held before,
39 its disk intact—while an ephemeral guest on the same host was failed by design. 39 its disk intact—while an ephemeral guest on the same host was failed by design.
40 40
41 ### An image we cannot name is safe to treat as raw
42
43 Sniffing recognizes gzip and qcow2 by magic number; everything else is called
44 raw and handed to the guest firmware to interpret. Underpins classifying by
45 bytes at all, rather than rejecting what we do not recognize.
46 **Unverified for the tail**: a VMDK or VHD would be called raw and materialized
47 as one, and the guest would fail at boot rather than the agent failing at
48 create. Raw has no magic of its own, so the alternative—an allowlist—would
49 reject legitimate raw images, which is worse.
50
41 ### Old agents tolerate raw images 51 ### Old agents tolerate raw images
42 52
43 `qemu-img convert -O raw` auto-detects its input, so a raw image is copied 53 `qemu-img convert -O raw` auto-detects its input, so a raw image is copied
internal/agent/imagecache/format.go
Old New
@@ -0,0 +1,63 @@
1 package imagecache
2
3 import (
4 "bytes"
5 "errors"
6 "fmt"
7 "io"
8 "os"
9 )
10
11 // imageFormat is what a download's first bytes say it is. Detection is by magic
12 // number rather than URL suffix because the suffix lies: the Ubuntu cloud
13 // images the fleet boots by default are named ".img" and are qcow2 inside.
14 type imageFormat string
15
16 const (
17 formatRaw imageFormat = "raw"
18 formatGzip imageFormat = "gzip"
19 formatQcow2 imageFormat = "qcow2"
20 )
21
22 var (
23 gzipMagic = []byte{0x1f, 0x8b}
24 qcow2Magic = []byte{'Q', 'F', 'I', 0xfb}
25 )
26
27 // magicLen is the longest magic number above: a download shorter than this
28 // cannot be anything but raw.
29 const magicLen = 4
30
31 // sniff classifies a download from its leading bytes. Anything unrecognized is
32 // raw — raw has no magic number of its own, and an image the agent cannot name
33 // is one the guest firmware gets to interpret rather than one the agent
34 // rejects.
35 func sniff(head []byte) imageFormat {
36 switch {
37 case bytes.HasPrefix(head, gzipMagic):
38 return formatGzip
39 case bytes.HasPrefix(head, qcow2Magic):
40 return formatQcow2
41 default:
42 return formatRaw
43 }
44 }
45
46 // sniffFile classifies the image file at path. A file shorter than magicLen is
47 // read in full and classified on what it has, so a truncated or empty image is
48 // raw rather than an error — the checksum has already vouched for the bytes by
49 // the time anything calls this.
50 func sniffFile(path string) (imageFormat, error) {
51 f, err := os.Open(path)
52 if err != nil {
53 return "", err
54 }
55 defer f.Close()
56
57 head := make([]byte, magicLen)
58 n, err := io.ReadFull(f, head)
59 if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) {
60 return "", fmt.Errorf("read image head: %w", err)
61 }
62 return sniff(head[:n]), nil
63 }
internal/agent/imagecache/format_test.go
Old New
@@ -0,0 +1,64 @@
1 package imagecache
2
3 import (
4 "os"
5 "path/filepath"
6 "testing"
7
8 "github.com/stretchr/testify/assert"
9 "github.com/stretchr/testify/require"
10 )
11
12 // TestSniffClassifiesByMagicNotSuffix pins the whole point of sniffing: the
13 // bytes decide, and anything without a magic number we recognize is raw.
14 func TestSniffClassifiesByMagicNotSuffix(t *testing.T) {
15 tests := []struct {
16 name string
17 head []byte
18 want imageFormat
19 }{
20 {"gzip", []byte{0x1f, 0x8b, 0x08, 0x00}, formatGzip},
21 {"qcow2", []byte{'Q', 'F', 'I', 0xfb}, formatQcow2},
22 {"raw with an MBR boot sector", []byte{0xeb, 0x63, 0x90, 0x10}, formatRaw},
23 {"raw of zeros", []byte{0x00, 0x00, 0x00, 0x00}, formatRaw},
24 {"empty", nil, formatRaw},
25 {"shorter than any magic", []byte{0x1f}, formatRaw},
26 {"gzip magic byte alone is not gzip", []byte{0x1f, 0x00}, formatRaw},
27 }
28 for _, tc := range tests {
29 t.Run(tc.name, func(t *testing.T) {
30 assert.Equal(t, tc.want, sniff(tc.head))
31 })
32 }
33 }
34
35 // TestSniffFileReadsOnlyTheHead pins that classification works off a file on
36 // disk (what Ensure has after a download) and does not need to read it all.
37 func TestSniffFileReadsOnlyTheHead(t *testing.T) {
38 dir := t.TempDir()
39 tests := []struct {
40 name string
41 body []byte
42 want imageFormat
43 }{
44 {"qcow2", append([]byte{'Q', 'F', 'I', 0xfb}, make([]byte, 4096)...), formatQcow2},
45 {"gzip", append([]byte{0x1f, 0x8b}, make([]byte, 4096)...), formatGzip},
46 {"raw", make([]byte, 4096), formatRaw},
47 {"one byte", []byte{0x1f}, formatRaw},
48 {"empty file", nil, formatRaw},
49 }
50 for _, tc := range tests {
51 t.Run(tc.name, func(t *testing.T) {
52 p := filepath.Join(dir, tc.name)
53 require.NoError(t, os.WriteFile(p, tc.body, 0o644))
54 got, err := sniffFile(p)
55 require.NoError(t, err)
56 assert.Equal(t, tc.want, got)
57 })
58 }
59 }
60
61 func TestSniffFileMissingFileErrors(t *testing.T) {
62 _, err := sniffFile(filepath.Join(t.TempDir(), "nope"))
63 assert.Error(t, err)
64 }
internal/agent/imagecache/imagecache.go
Old New
@@ -155,7 +155,27 @@ func (c *Cache) ensureOnce(ctx context.Context, url, sha string) (string, error)
155 if err != nil { 155 if err != nil {
156 return "", err 156 return "", err
157 } 157 }
158 // A no-op once the raw path has renamed tmp away; every other path needs it.
158 defer os.Remove(tmp) 159 defer os.Remove(tmp)
160
161 format, err := sniffFile(tmp)
162 if err != nil {
163 return "", fmt.Errorf("imagecache: identify %s: %w", url, err)
164 }
165 if format == formatRaw {
166 // The verified download IS the base image. Renaming it onto final is
167 // atomic (same directory) and costs neither a tool nor a second copy of a
168 // multi-gigabyte file. This is the path a host without qemu-img has.
169 if err := os.Chmod(tmp, 0o644); err != nil {
170 return "", fmt.Errorf("imagecache: chmod image: %w", err)
171 }
172 if err := os.Rename(tmp, final); err != nil {
173 return "", fmt.Errorf("imagecache: rename to final: %w", err)
174 }
175 c.evict(final)
176 return final, nil
177 }
178
159 // Convert to a UNIQUE temp file first, then rename onto final atomically so a 179 // Convert to a UNIQUE temp file first, then rename onto final atomically so a
160 // crash mid-convert cannot leave a corrupt file at the final path. The temp 180 // crash mid-convert cannot leave a corrupt file at the final path. The temp
161 // name is unique (not a fixed "<sha>.converting") so two concurrent Ensure 181 // name is unique (not a fixed "<sha>.converting") so two concurrent Ensure
internal/agent/imagecache/imagecache_test.go
Old New
@@ -35,6 +35,19 @@ func fakeRunner(t *testing.T) exec.Runner {
35 } 35 }
36 } 36 }
37 37
38 // noRunner fails the test if anything shells out. It is how "no tool was
39 // needed" is asserted rather than assumed: a raw image must be materialized by
40 // the cache itself, on a host that may have no qemu-img at all.
41 // It reports through Errorf and an error return rather than Fatalf: Ensure runs
42 // its body on a singleflight goroutine, and Fatalf's Goexit there would strand
43 // the caller waiting on a result that never arrives.
44 func noRunner(t *testing.T) exec.Runner {
45 return func(_ context.Context, name string, _ ...string) (string, error) {
46 t.Errorf("no subprocess expected, got %q", name)
47 return "", fmt.Errorf("unexpected subprocess %q", name)
48 }
49 }
50
38 func serve(t *testing.T, body []byte) (*httptest.Server, string) { 51 func serve(t *testing.T, body []byte) (*httptest.Server, string) {
39 t.Helper() 52 t.Helper()
40 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { 53 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -47,6 +60,48 @@ func serve(t *testing.T, body []byte) (*httptest.Server, string) {
47 60
48 func TestEnsureDownloadsVerifiesAndCaches(t *testing.T) { 61 func TestEnsureDownloadsVerifiesAndCaches(t *testing.T) {
49 body := []byte("pretend-qcow2-image") 62 body := []byte("pretend-qcow2-image")
63 var downloads atomic.Int64
64 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
65 downloads.Add(1)
66 w.Write(body)
67 }))
68 t.Cleanup(ts.Close)
69 sum := sha256.Sum256(body)
70 sha := hex.EncodeToString(sum[:])
71 c := New(t.TempDir(), noRunner(t))
72
73 p1, err := c.Ensure(context.Background(), ts.URL+"/img.qcow2", sha)
74 require.NoError(t, err)
75 assert.FileExists(t, p1)
76
77 p2, err := c.Ensure(context.Background(), ts.URL+"/img.qcow2", sha)
78 require.NoError(t, err)
79 assert.Equal(t, p1, p2)
80 assert.Equal(t, int64(1), downloads.Load(), "second Ensure must hit the cache, not re-fetch")
81 }
82
83 // TestEnsureMaterializesRawWithoutATool is the reason this path exists: vfkit
84 // wants a raw disk and macOS ships no qemu-img, so a raw image has to become
85 // the cache entry with nothing but a rename. noRunner turns "no tool" from a
86 // claim into an assertion.
87 func TestEnsureMaterializesRawWithoutATool(t *testing.T) {
88 body := bytes.Repeat([]byte{0x00, 0xeb, 0x63, 0x90}, 1024) // no magic number: raw
89 ts, sum := serve(t, body)
90 c := New(t.TempDir(), noRunner(t))
91
92 p, err := c.Ensure(context.Background(), ts.URL+"/disk.raw", sum)
93 require.NoError(t, err)
94
95 got, err := os.ReadFile(p)
96 require.NoError(t, err)
97 assert.Equal(t, body, got, "the verified download IS the base image")
98 assert.Equal(t, sum+".raw", filepath.Base(p))
99 }
100
101 // TestEnsureStillConvertsQcow2 pins that sniffing routes the other way too:
102 // qcow2 keeps the tool, so this change adds a path rather than replacing one.
103 func TestEnsureStillConvertsQcow2(t *testing.T) {
104 body := append([]byte{'Q', 'F', 'I', 0xfb}, bytes.Repeat([]byte{0x11}, 512)...)
50 ts, sum := serve(t, body) 105 ts, sum := serve(t, body)
51 var calls int 106 var calls int
52 c := New(t.TempDir(), func(ctx context.Context, name string, args ...string) (string, error) { 107 c := New(t.TempDir(), func(ctx context.Context, name string, args ...string) (string, error) {
@@ -54,14 +109,10 @@ func TestEnsureDownloadsVerifiesAndCaches(t *testing.T) {
54 return fakeRunner(t)(ctx, name, args...) 109 return fakeRunner(t)(ctx, name, args...)
55 }) 110 })
56 111
57 p1, err := c.Ensure(context.Background(), ts.URL+"/img.qcow2", sum) 112 p, err := c.Ensure(context.Background(), ts.URL+"/img.qcow2", sum)
58 require.NoError(t, err)
59 assert.FileExists(t, p1)
60
61 p2, err := c.Ensure(context.Background(), ts.URL+"/img.qcow2", sum)
62 require.NoError(t, err) 113 require.NoError(t, err)
63 assert.Equal(t, p1, p2) 114 assert.FileExists(t, p)
64 assert.Equal(t, 1, calls, "second Ensure must hit the cache, not re-convert") 115 assert.Equal(t, 1, calls, "qcow2 must still go through the converter")
65 } 116 }
66 117
67 func TestEnsureRejectsChecksumMismatch(t *testing.T) { 118 func TestEnsureRejectsChecksumMismatch(t *testing.T) {
@@ -97,7 +148,10 @@ func TestEnsureRejectsInvalidSha(t *testing.T) {
97 // --- I1: atomic conversion via temp file --- 148 // --- I1: atomic conversion via temp file ---
98 149
99 func TestEnsureAtomicConvert_LeftoverPartialIsIgnored(t *testing.T) { 150 func TestEnsureAtomicConvert_LeftoverPartialIsIgnored(t *testing.T) {
100 body := []byte("pretend-qcow2-image") 151 // Genuinely qcow2: only the converting path builds the temp this test is
152 // named for. A body without a magic number is raw, and raw renames the
153 // download straight onto final without ever creating one.
154 body := append([]byte{'Q', 'F', 'I', 0xfb}, []byte("pretend-qcow2-image")...)
101 ts, sum := serve(t, body) 155 ts, sum := serve(t, body)
102 dir := t.TempDir() 156 dir := t.TempDir()
103 157