Fix and document fixMissingSystemData

Signed-off-by: Paul Mars <[email protected]>
canonical · Apr 19, 2024 · 9aef451 · 9aef451
1 parent fa1ecef
commit 9aef451
Show file tree

Hide file tree

Showing 2 changed files with 112 additions and 15 deletions.
diff --git 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
@@ -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()