From 9aef45103fc59891ca29ef0a8a6144983645b36b Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 19 Apr 2024 15:46:37 +0200 Subject: [PATCH] Fix and document fixMissingSystemData Signed-off-by: Paul Mars --- internal/statemachine/state_machine.go | 27 +++--- internal/statemachine/state_machine_test.go | 100 +++++++++++++++++++- 2 files changed, 112 insertions(+), 15 deletions(-) diff --git a/internal/statemachine/state_machine.go b/internal/statemachine/state_machine.go index e3cf8903..37d59174 100644 --- a/internal/statemachine/state_machine.go +++ b/internal/statemachine/state_machine.go @@ -274,7 +274,7 @@ func (stateMachine *StateMachine) postProcessGadgetYaml() error { var rootfsSeen bool = false var farthestOffset quantity.Offset var lastOffset quantity.Offset - farthestOffsetUnknown := false + var farthestOffsetUnknown bool = false lastVolumeName := "" for _, volumeName := range stateMachine.VolumeOrder { @@ -392,22 +392,21 @@ func fixMissingContent(volume *gadget.Volume, structure *gadget.VolumeStructure, volume.Structure[structIndex] = *structure } -// fixMissingSystemData handles the case of unspecified system-data -// partition where we simply attach the rootfs at the end of the -// partition list. -// Since so far we have no knowledge of the rootfs contents, the -// size is set to 0, and will be calculated later +// fixMissingSystemData handles the case of unspecified system-data partition +// where we simply attach the rootfs at the end of the partition list. +// Since so far we have no knowledge of the rootfs contents, the size is set +// to 0, and will be calculated later func (stateMachine *StateMachine) fixMissingSystemData(lastVolumeName string, farthestOffset quantity.Offset, farthestOffsetUnknown bool, rootfsSeen bool) { - // For now we consider the main volume to be the last one unless it was previously found - volume := stateMachine.GadgetInfo.Volumes[lastVolumeName] - - if len(stateMachine.MainVolumeName) != 0 { - volume = stateMachine.GadgetInfo.Volumes[stateMachine.MainVolumeName] - } - - if !farthestOffsetUnknown || !rootfsSeen || len(stateMachine.GadgetInfo.Volumes) != 1 { + // We only add the structure if there is a single volume because we cannot + // be sure which volume is considered the "main one" by the user, even though + // we have a way to find it (see comment about the MainVolume field). We + // should revisit this if we want to be stricter in the future about that. + if farthestOffsetUnknown || rootfsSeen || len(stateMachine.GadgetInfo.Volumes) != 1 { return } + // So for now we consider the main volume to be the last one + volume := stateMachine.GadgetInfo.Volumes[lastVolumeName] + rootfsStructure := gadget.VolumeStructure{ Name: "", Label: "writable", diff --git a/internal/statemachine/state_machine_test.go b/internal/statemachine/state_machine_test.go index 273f09fd..3af5e822 100644 --- a/internal/statemachine/state_machine_test.go +++ b/internal/statemachine/state_machine_test.go @@ -658,6 +658,7 @@ func TestStateMachine_postProcessGadgetYaml(t *testing.T) { tests := []struct { name string gadgetYaml []byte + volumeOrder []string wantVolumes map[string]*gadget.Volume wantIsSeeded bool expectedErr string @@ -721,6 +722,7 @@ func TestStateMachine_postProcessGadgetYaml(t *testing.T) { type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4 size: 1G `), + volumeOrder: []string{"pc"}, wantIsSeeded: true, wantVolumes: map[string]*gadget.Volume{ "pc": { @@ -856,6 +858,7 @@ func TestStateMachine_postProcessGadgetYaml(t *testing.T) { update: edition: 2 `), + volumeOrder: []string{"pc"}, wantIsSeeded: true, wantVolumes: map[string]*gadget.Volume{ "pc": { @@ -909,6 +912,100 @@ func TestStateMachine_postProcessGadgetYaml(t *testing.T) { }, }, }, + { + name: "do not add a system-data structure if there is not exactly one", + gadgetYaml: []byte(`volumes: + pc: + bootloader: grub + structure: + - name: mbr + type: mbr + size: 440 + update: + edition: 1 + content: + - image: pc-boot.img + - name: ubuntu-seed + role: system-seed + filesystem: vfat + # UEFI will boot the ESP partition by default first + type: EF,C12A7328-F81F-11D2-BA4B-00A0C93EC93B + size: 1200M + update: + edition: 2 + pc2: + structure: + - name: mbr + type: mbr + size: 440 + update: + edition: 1 + content: + - image: pc-boot.img +`), + volumeOrder: []string{"pc", "pc2"}, + wantIsSeeded: true, + wantVolumes: map[string]*gadget.Volume{ + "pc": { + Schema: "gpt", + Bootloader: "grub", + Structure: []gadget.VolumeStructure{ + { + VolumeName: "pc", + Name: "mbr", + Offset: createOffsetPointer(0), + MinSize: 440, + Size: 440, + Type: "mbr", + Role: "mbr", + Content: []gadget.VolumeContent{ + { + Image: "pc-boot.img", + }, + }, + Update: gadget.VolumeUpdate{Edition: 1}, + }, + { + VolumeName: "pc", + Name: "ubuntu-seed", + Label: "ubuntu-seed", + Offset: createOffsetPointer(1048576), + MinSize: 1258291200, + Size: 1258291200, + Type: "EF,C12A7328-F81F-11D2-BA4B-00A0C93EC93B", + Role: "system-seed", + Filesystem: "vfat", + Content: []gadget.VolumeContent{}, + Update: gadget.VolumeUpdate{Edition: 2}, + YamlIndex: 1, + }, + }, + Name: "pc", + }, + "pc2": { + Schema: "gpt", + Bootloader: "", + Structure: []gadget.VolumeStructure{ + { + VolumeName: "pc2", + Name: "mbr", + Offset: createOffsetPointer(0), + MinSize: 440, + Size: 440, + Type: "mbr", + Role: "mbr", + Content: []gadget.VolumeContent{ + { + Image: "pc-boot.img", + }, + }, + Update: gadget.VolumeUpdate{Edition: 1}, + }, + }, + Name: "pc2", + }, + }, + }, { name: "error with invalid source path", gadgetYaml: []byte(`volumes: @@ -927,6 +1024,7 @@ func TestStateMachine_postProcessGadgetYaml(t *testing.T) { - source: ../grubx64.efi target: EFI/boot/grubx64.efi `), + volumeOrder: []string{"pc"}, wantIsSeeded: true, expectedErr: "filesystem content source \"../grubx64.efi\" contains \"../\". This is disallowed for security purposes", }, @@ -935,7 +1033,7 @@ func TestStateMachine_postProcessGadgetYaml(t *testing.T) { t.Run(tt.name, func(t *testing.T) { asserter := helper.Asserter{T: t} stateMachine := &StateMachine{ - VolumeOrder: []string{"pc"}, + VolumeOrder: tt.volumeOrder, } stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts()