Skip to content

Commit b1232d2

Browse files
authored
Merge pull request #9 from frappe/fix/provision-preflight-sudo
fix(provision): sudo the image presence probe (0700 dir, boat user)
2 parents ea7adb8 + 1eca5a8 commit b1232d2

5 files changed

Lines changed: 52 additions & 15 deletions

File tree

internal/provision/clone_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func warmParams() Params {
2727
// warmHost has the golden's disk snapshot and both halves of its memory pair.
2828
func warmHost() *fakeCommands {
2929
return readyHost().
30+
exists("test -b " + snapshotLV).
3031
exists("sudo lvs --noheadings atlas/atlas-snap-golden").
3132
exists("sudo test -s " + warmDirectory + "/vmstate.bin").
3233
exists("sudo test -s " + warmDirectory + "/mem.bin")
@@ -51,7 +52,7 @@ func TestAWarmCloneStagesTheGoldenPairAndNeverTouchesTheDisk(t *testing.T) {
5152
}
5253

5354
assertTrace(t, fake,
54-
"? test -f "+testImage+"/rootfs.ext4",
55+
"? test -b "+snapshotLV,
5556
"? test -d /var/lib/atlas/virtual-machines",
5657
"sudo ls -1 /var/lib/atlas/virtual-machines",
5758
"install-dir 0700 "+testDirectory,
@@ -152,6 +153,7 @@ func TestAnUnconsumedMarkerLetsAReRunStageAgain(t *testing.T) {
152153
// back. The message names the file and the fix.
153154
func TestAnEmptyGoldenFileIsRefused(t *testing.T) {
154155
fake := readyHost().
156+
exists("test -b " + snapshotLV).
155157
exists("sudo lvs --noheadings atlas/atlas-snap-golden").
156158
exists("sudo test -s " + warmDirectory + "/vmstate.bin")
157159

internal/provision/fake_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func testParams() Params {
6868
// comes up as a block device. Each test then adds the scenario it is about.
6969
func readyHost() *fakeCommands {
7070
return newFakeCommands().
71-
exists("test -f "+testImage+"/rootfs.ext4").
71+
exists("test -b /dev/atlas/atlas-image-ubuntu-24.04").
7272
exists("test -d /var/lib/atlas/virtual-machines").
7373
exists("test -b "+testDevice).
7474
exists("test -b "+testDataDevice).

internal/provision/preflight.go

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"strings"
88

99
"github.com/frappe/boat/internal/paths"
10+
"github.com/frappe/boat/internal/thinpool"
1011
)
1112

1213
// preflight is steps 0 and 0b: refuse before anything is laid down.
@@ -21,21 +22,40 @@ func (provisioning *provisioning) preflight(ctx context.Context) error {
2122
// message so the operator knows to click Sync to Server before retrying — image
2223
// sync is multi-minute and is deliberately not auto-triggered from provision.
2324
//
24-
// The probe stays even when the rootfs comes from a snapshot (the clone path):
25-
// the KERNEL is hard-linked out of the same image directory regardless of where
26-
// the rootfs blocks come from.
27-
//
28-
// The Python stats the file in process; here it is `test -f` with no sudo, which
29-
// is the same reach — /var/lib/atlas/images is root-owned and world-readable, and
30-
// this verb runs as root.
25+
// It probes the image's BASE LV DEVICE NODE, not the rootfs FILE. The rootfs file
26+
// lives in /var/lib/atlas/images/<name>/ which sync.go / bootstrap.go create 0700
27+
// root-owned; provision-vm runs as the boat DAEMON user, so a bare `test -f` there
28+
// reads the root-only dir as "not present" and fails a fully-synced host (the
29+
// boat-user-vs-root idiom trap, spec/33 §3.4), while sudo would need a new grant.
30+
// The base LV node (/dev/atlas/atlas-image-<name>) is a world-visible symlink AND
31+
// is the durable artifact every per-VM disk snapshots from, so `test -b` on it —
32+
// no sudo, no new grant — is both reachable and a truer "is this image synced?"
33+
// check. The clone path relies on the same image dir for the hard-linked kernel;
34+
// a truly-absent image dir there fails loud at the link step.
3135
func (provisioning *provisioning) requireImage(ctx context.Context) error {
32-
rootfsImage := provisioning.imageDirectory + "/" + provisioning.params.RootfsFilename
33-
if provisioning.commands.OK(ctx, "test -f {}", rootfsImage) {
36+
// The ORIGIN the per-VM disk will snapshot from: the snapshot LV on the clone
37+
// path, else the base image LV — the same selection (and the same distinct
38+
// messages) resolveOrigin makes, so a clone never references the base image LV,
39+
// and this refuses BEFORE anything is laid down (resolveOrigin runs after the VM
40+
// dir exists). Probing the LV DEVICE NODE with `test -b` (a world-visible symlink)
41+
// needs no sudo and no new grant — unlike a `test -f` of the rootfs file, whose
42+
// 0700 root-owned dir the boat daemon user cannot stat.
43+
if provisioning.params.SnapshotRootfsPath != "" {
44+
origin := thinpool.NameFromDevice(provisioning.params.SnapshotRootfsPath)
45+
if provisioning.commands.OK(ctx, "test -b {}", thinpool.DevicePath(origin)) {
46+
return nil
47+
}
48+
return fmt.Errorf(
49+
"snapshot LV not found: %s (from %s)", origin, provisioning.params.SnapshotRootfsPath,
50+
)
51+
}
52+
node := thinpool.DevicePath(thinpool.BaseImageLV(provisioning.params.ImageName))
53+
if provisioning.commands.OK(ctx, "test -b {}", node) {
3454
return nil
3555
}
3656
return fmt.Errorf(
3757
"image '%s' not present on server (missing %s); run Sync to Server first",
38-
provisioning.params.ImageName, rootfsImage,
58+
provisioning.params.ImageName, node,
3959
)
4060
}
4161

internal/provision/provision_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestAColdProvisionRendersThePythonsSequence(t *testing.T) {
4040
assertTrace(t, fake,
4141
// 0. the image must be on the host; the kernel is hard-linked out of the
4242
// same directory whatever the rootfs source is.
43-
"? test -f "+testImage+"/rootfs.ext4",
43+
"? test -b /dev/atlas/atlas-image-ubuntu-24.04",
4444
// 0b. the per-VM uid collision guard, over the VMs this host already holds.
4545
"? test -d /var/lib/atlas/virtual-machines",
4646
"sudo ls -1 /var/lib/atlas/virtual-machines",
@@ -108,7 +108,9 @@ func TestTheGeneratedFilesLandWhereTheJailerLooksForThem(t *testing.T) {
108108
func TestACloneSnapshotsTheSnapshotLV(t *testing.T) {
109109
params := testParams()
110110
params.SnapshotRootfsPath = "/dev/atlas/atlas-snap-golden"
111-
fake := readyHost().exists("sudo lvs --noheadings atlas/atlas-snap-golden")
111+
fake := readyHost().
112+
exists("test -b /dev/atlas/atlas-snap-golden").
113+
exists("sudo lvs --noheadings atlas/atlas-snap-golden")
112114

113115
if _, err := Provision(context.Background(), fake, params, fake.recordInject()); err != nil {
114116
t.Fatalf("Provision: %v", err)
@@ -156,7 +158,7 @@ func TestAMissingImageDirectoryIsRefusedBeforeAnythingIsLaidDown(t *testing.T) {
156158
if err == nil || !strings.Contains(err.Error(), "not present on server") {
157159
t.Fatalf("a missing image gave %v", err)
158160
}
159-
assertTrace(t, fake, "? test -f "+testImage+"/rootfs.ext4")
161+
assertTrace(t, fake, "? test -b /dev/atlas/atlas-image-ubuntu-24.04")
160162
}
161163

162164
// TestANameThatIsNotAUUIDNeverBecomesAPath. The name is spliced into an LV

sudoers.d/boat

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,16 @@ Cmnd_Alias BOAT_VIRTUAL_MACHINE_NETWORK_DOWN = \
10261026
# --- atlas-data LV; ln/cp SOURCES bounded to /var/lib/atlas. Host-audit the
10271027
# --- source globs + chown uid classes with `sudo -u boat -n -l`.
10281028
Cmnd_Alias BOAT_PROVISION = \
1029+
/usr/bin/install -d -m 0700 /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f], \
1030+
/usr/bin/install -d -m 0700 /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/log, \
1031+
/usr/bin/install -d -m 0700 /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jail/firecracker/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/root, \
1032+
/usr/bin/install -d -m 0700 /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jail/firecracker/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/root/run, \
1033+
/usr/bin/install -m 0644 /var/lib/boat/spool/install /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jail/firecracker/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/root/firecracker.json, \
1034+
/usr/bin/install -m 0644 /var/lib/boat/spool/install /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/network.env, \
1035+
/usr/bin/install -m 0755 /var/lib/boat/spool/install /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jailer-launch.sh, \
1036+
/usr/bin/install -m 0644 /var/lib/boat/spool/install /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jail/firecracker/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/root/metadata.json, \
1037+
/usr/bin/test -s /var/lib/atlas/snapshots/*/vmstate.bin, \
1038+
/usr/bin/test -s /var/lib/atlas/snapshots/*/mem.bin, \
10291039
/usr/bin/ln -f /var/lib/atlas/images/*/* /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jail/firecracker/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/root/vmlinux, \
10301040
/usr/bin/ln -f /var/lib/atlas/*/* /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jail/firecracker/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/root/snapshot/*, \
10311041
/usr/bin/chown -R [0-9][0-9][0-9][0-9][0-9][0-9]\:[0-9][0-9][0-9][0-9][0-9][0-9] /var/lib/atlas/virtual-machines/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f]-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]/jail, \
@@ -1044,6 +1054,9 @@ Cmnd_Alias BOAT_WARM_SNAPSHOT = \
10441054
# --- Promote (promote-snapshot-image): hard-link a snapshot rootfs into the
10451055
# --- images tree. Both ends bounded to /var/lib/atlas; write end is images.
10461056
Cmnd_Alias BOAT_PROMOTE_IMAGE = \
1057+
/usr/bin/dd if=/dev/atlas/atlas-snap-* of=/dev/atlas/atlas-image-* bs=4M conv=fsync status=none, \
1058+
/usr/bin/test -f /var/lib/atlas/images/*/*, \
1059+
/usr/bin/install -m 0644 /var/lib/boat/spool/install /var/lib/atlas/images/*/*, \
10471060
/usr/bin/ln -f /var/lib/atlas/*/* /var/lib/atlas/images/*/*
10481061

10491062
# --- Sync image (sync-image): download + build a base rootfs. The write targets

0 commit comments

Comments
 (0)