gadget: consider values of "partial" when validating gadget

2 files changed, 250 insertions, 33 deletions

diff --git a/gadget/gadget.go b/gadget/gadget.go
index a430edd7dd..b6d86b9aaf 100644
--- a/gadget/gadget.go
+++ b/gadget/gadget.go
@@ -156,6 +156,16 @@ type Volume struct {
	Name string `json:"-"`
 }
 
+// HasPartial checks if the volume has a partially defined part.
+func (v *Volume) HasPartial(pp PartialProperty) bool {
+	for _, vp := range v.Partial {
+	if vp == pp {
+	return true
+	}
+	}
+	return false
+}
+
 // MinSize returns the minimum size required by a volume, as implicitly
 // defined by the size structures. It assumes sorted structures.
 func (v *Volume) MinSize() quantity.Size {
@@ -251,6 +261,19 @@ func (vs *VolumeStructure) HasFilesystem() bool {
	return vs.Filesystem != "none" && vs.Filesystem != ""
 }
 
+// willHaveFilesystem considers also partially defined filesystem.
+func (vs *VolumeStructure) willHaveFilesystem(v *Volume) bool {
+	if vs.HasFilesystem() {
+	return true
+	}
+
+	if vs.Type == "bare" || vs.Type == "mbr" || !v.HasPartial(PartialFilesystem) {
+	return false
+	}
+
+	return true
+}
+
 // IsPartition returns true when the structure describes a partition in a block
 // device.
 func (vs *VolumeStructure) IsPartition() bool {
@@ -271,6 +294,15 @@ func (vs *VolumeStructure) IsFixedSize() bool {
	return vs.Size == vs.MinSize
 }
 
+// hasPartialSize tells us if the structure has partially defined size.
+func (vs *VolumeStructure) hasPartialSize(v *Volume) bool {
+	if !v.HasPartial(PartialSize) {
+	return false
+	}
+
+	return vs.Size == 0
+}
+
 // minStructureOffset works out the minimum start offset of an structure, which
 // depends on previous volume structures.
 func minStructureOffset(vss []VolumeStructure, idx int) quantity.Offset {
@@ -951,7 +983,7 @@ func asOffsetPtr(offs quantity.Offset) *quantity.Offset {
 
 func setImplicitForVolume(vol *Volume, model Model) error {
	rs := whichVolRuleset(model)
-	if vol.Schema == "" {
+	if vol.Schema == "" && !vol.HasPartial(PartialSchema) {
	// default for schema is gpt
	vol.Schema = schemaGPT
	}
@@ -1130,8 +1162,8 @@ func validateVolume(vol *Volume) error {
	return fmt.Errorf("invalid schema %q", vol.Schema)
	}
 
-	// named structures, for cross-referencing relative offset-write names
-	knownStructures := make(map[string]*VolumeStructure, len(vol.Structure))
+	// named structures, to check that names are not repeated
+	knownStructures := make(map[string]bool, len(vol.Structure))
 
	// TODO: should we also validate that if there is a system-recovery-select
	// role there should also be at least 2 system-recovery-image roles and
@@ -1174,11 +1206,11 @@ func validateVolume(vol *Volume) error {
	return fmt.Errorf("structure name %q is not unique", s.Name)
	}
	// keep track of named structures
-	knownStructures[s.Name] = &vol.Structure[idx]
+	knownStructures[s.Name] = true
	}
	}
 
-	return validateCrossVolumeStructure(vol.Structure, vol.MinSize())
+	return validateCrossVolumeStructure(vol)
 }
 
 // isMBR returns whether the structure is the MBR and can be used before setImplicitForVolume
@@ -1192,24 +1224,24 @@ func isMBR(vs *VolumeStructure) bool {
	return false
 }
 
-func validateCrossVolumeStructure(structures []VolumeStructure, volSize quantity.Size) error {
+func validateCrossVolumeStructure(vol *Volume) error {
	previousEnd := quantity.Offset(0)
	// cross structure validation:
	// - relative offsets that reference other structures by name
	// - structure overlap
-	for pidx, ps := range structures {
+	for pidx, ps := range vol.Structure {
	if isMBR(&ps) {
	if ps.Offset == nil || *(ps.Offset) != 0 {
	return fmt.Errorf(`structure %q has "mbr" role and must start at offset 0`, ps.Name)
	}
	}
-	if err := validateOffsetWrite(&ps, &structures[0], volSize); err != nil {
+	if err := validateOffsetWrite(&ps, &vol.Structure[0], vol.MinSize()); err != nil {
	return err
	}
	// We are assuming ordered structures
	if ps.Offset != nil {
	if *(ps.Offset) < previousEnd {
-	return fmt.Errorf("structure %q overlaps with the preceding structure %q", ps.Name, structures[pidx-1].Name)
+	return fmt.Errorf("structure %q overlaps with the preceding structure %q", ps.Name, vol.Structure[pidx-1].Name)
	}
	previousEnd = *(ps.Offset) + quantity.Offset(ps.Size)
	} else {
@@ -1249,12 +1281,14 @@ func validateOffsetWrite(s, firstStruct *VolumeStructure, volSize quantity.Size)
 }
 
 func validateVolumeStructure(vs *VolumeStructure, vol *Volume) error {
-	if vs.Size == 0 {
-	return errors.New("missing size")
-	}
-	if vs.MinSize > vs.Size {
-	return fmt.Errorf("min-size (%d) is bigger than size (%d)",
-	vs.MinSize, vs.Size)
+	if !vs.hasPartialSize(vol) {
+	if vs.Size == 0 {
+	return errors.New("missing size")
+	}
+	if vs.MinSize > vs.Size {
+	return fmt.Errorf("min-size (%d) is bigger than size (%d)",
+	vs.MinSize, vs.Size)
+	}
	}
	if err := validateStructureType(vs.Type, vol); err != nil {
	return fmt.Errorf("invalid type %q: %v", vs.Type, err)
@@ -1274,10 +1308,10 @@ func validateVolumeStructure(vs *VolumeStructure, vol *Volume) error {
 
	var contentChecker func(*VolumeContent) error
 
-	if !vs.HasFilesystem() {
-	contentChecker = validateBareContent
-	} else {
+	if vs.willHaveFilesystem(vol) {
	contentChecker = validateFilesystemContent
+	} else {
+	contentChecker = validateBareContent
	}
	for i, c := range vs.Content {
	if err := contentChecker(&c); err != nil {
@@ -1285,7 +1319,7 @@ func validateVolumeStructure(vs *VolumeStructure, vol *Volume) error {
	}
	}
 
-	if err := validateStructureUpdate(vs); err != nil {
+	if err := validateStructureUpdate(vs, vol); err != nil {
	return err
	}
 
@@ -1306,11 +1340,6 @@ func validateStructureType(s string, vol *Volume) error {
	// Hybrid ID is 2 hex digits of MBR type, followed by 36 GUUID
	// example: EF,C12A7328-F81F-11D2-BA4B-00A0C93EC93B
 
-	schema := vol.Schema
-	if schema == "" {
-	schema = schemaGPT
-	}
-
	if s == "" {
	return errors.New(`type is not specified`)
	}
@@ -1325,7 +1354,7 @@ func validateStructureType(s string, vol *Volume) error {
	return nil
	}
 
-	var isGPT, isMBR bool
+	var isGPT, isMBR, isHybrid bool
 
	idx := strings.IndexRune(s, ',')
	if idx == -1 {
@@ -1340,6 +1369,7 @@ func validateStructureType(s string, vol *Volume) error {
	}
	} else {
	// hybrid ID
+	isHybrid = true
	code := s[:idx]
	guid := s[idx+1:]
	if len(code) != 2 || len(guid) != 36 || !validTypeID.MatchString(code) || !validGUUID.MatchString(guid) {
@@ -1347,12 +1377,22 @@ func validateStructureType(s string, vol *Volume) error {
	}
	}
 
-	if schema != schemaGPT && isGPT {
-	// type: <uuid> is only valid for GPT volumes
-	return fmt.Errorf("GUID structure type with non-GPT schema %q", vol.Schema)
-	}
-	if schema != schemaMBR && isMBR {
-	return fmt.Errorf("MBR structure type with non-MBR schema %q", vol.Schema)
+	if vol.HasPartial(PartialSchema) {
+	if !isHybrid {
+	return fmt.Errorf("both MBR type and GUID structure type needs to be defined on partial schemas")
+	}
+	} else {
+	schema := vol.Schema
+	if schema == "" {
+	schema = schemaGPT
+	}
+	if schema != schemaGPT && isGPT {
+	// type: <uuid> is only valid for GPT volumes
+	return fmt.Errorf("GUID structure type with non-GPT schema %q", vol.Schema)
+	}
+	if schema != schemaMBR && isMBR {
+	return fmt.Errorf("MBR structure type with non-MBR schema %q", vol.Schema)
+	}
	}
 
	return nil
@@ -1425,8 +1465,8 @@ func validateFilesystemContent(vc *VolumeContent) error {
	return nil
 }
 
-func validateStructureUpdate(vs *VolumeStructure) error {
-	if !vs.HasFilesystem() && len(vs.Update.Preserve) > 0 {
+func validateStructureUpdate(vs *VolumeStructure, v *Volume) error {
+	if !vs.willHaveFilesystem(v) && len(vs.Update.Preserve) > 0 {
	return errors.New("preserving files during update is not supported for non-filesystem structures")
	}
 
diff --git a/gadget/gadget_test.go b/gadget/gadget_test.go
index 404685f2dd..cef54a1417 100644
--- a/gadget/gadget_test.go
+++ b/gadget/gadget_test.go
@@ -4517,3 +4517,180 @@ volumes:
	}
	}
 }
+
+func (s *gadgetYamlTestSuite) TestGadgetPartialSize(c *C) {
+	var yaml = []byte(`
+volumes:
+ frobinator-image:
+ partial: [size]
+ bootloader: u-boot
+ schema: gpt
+ structure:
+ - name: ubuntu-seed
+ filesystem: ext4
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-seed
+ - name: ubuntu-boot
+ filesystem: ext4
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-boot
+ - name: ubuntu-save
+ min-size: 1M
+ filesystem: ext4
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-save
+ - name: ubuntu-data
+ filesystem: ext4
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-data
+`)
+
+	// Not defining size in a structure is fine
+	_, err := gadget.InfoFromGadgetYaml(yaml, nil)
+	c.Assert(err, IsNil)
+
+	// but if defined, things are still checked
+	yaml = append(yaml, []byte(`
+ - name: ubuntu-data
+ filesystem: ext4
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-data
+ size: 1M
+ min-size: 2M
+`)...)
+	_, err = gadget.InfoFromGadgetYaml(yaml, nil)
+	c.Assert(err.Error(), Equals, `invalid volume "frobinator-image": invalid structure #4 ("ubuntu-data"): min-size (2097152) is bigger than size (1048576)`)
+}
+
+func (s *gadgetYamlTestSuite) TestGadgetPartialFilesystem(c *C) {
+	var yaml = []byte(`
+volumes:
+ frobinator-image:
+ partial: [filesystem]
+ bootloader: grub
+ schema: gpt
+ structure:
+ - name: mbr
+ type: mbr
+ size: 440
+ update:
+ edition: 1
+ content:
+ - image: mbr.img
+ - name: ubuntu-seed
+ filesystem: vfat
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-seed
+ - name: ubuntu-boot
+ filesystem: ext4
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-boot
+ - name: ubuntu-save
+ size: 1M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-save
+ content:
+ - source: splash.bmp
+ target: .
+ - name: ubuntu-data
+ size: 1000M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-data
+`)
+
+	// Not defining filesystem in a structure is fine
+	_, err := gadget.InfoFromGadgetYaml(yaml, nil)
+	c.Assert(err, IsNil)
+
+	// checks for bare still happen
+	yaml = append(yaml, []byte(`
+ - name: boot-fw
+ type: bare
+ size: 1M
+ content:
+ - source: splash.bmp
+ target: .
+`)...)
+	_, err = gadget.InfoFromGadgetYaml(yaml, nil)
+	c.Assert(err.Error(), Equals, `invalid volume "frobinator-image": invalid structure #5 ("boot-fw"): invalid content #0: cannot use non-image content for bare file system`)
+}
+
+func (s *gadgetYamlTestSuite) TestGadgetPartialSchema(c *C) {
+	var yaml = []byte(`
+volumes:
+ frobinator-image:
+ partial: [schema]
+ bootloader: u-boot
+ structure:
+ - name: ubuntu-seed
+ filesystem: vfat
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-seed
+ - name: ubuntu-boot
+ filesystem: ext4
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-boot
+ - name: ubuntu-save
+ size: 1M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-save
+ - name: ubuntu-data
+ size: 1000M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-data
+`)
+
+	// Not defining schema is fine
+	_, err := gadget.InfoFromGadgetYaml(yaml, nil)
+	c.Assert(err, IsNil)
+
+	// but fails if type does not contain both mbr type and gpt guid
+	yamlNoGPTGuid := append(yaml, []byte(`
+ - name: data
+ type: 83
+ size: 1M
+`)...)
+	_, err = gadget.InfoFromGadgetYaml(yamlNoGPTGuid, nil)
+	c.Assert(err.Error(), Equals, `invalid volume "frobinator-image": invalid structure #4 ("data"): invalid type "83": both MBR type and GUID structure type needs to be defined on partial schemas`)
+	yamlNoMBRType := append(yaml, []byte(`
+ - name: data
+ type: 0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ size: 1M
+`)...)
+	_, err = gadget.InfoFromGadgetYaml(yamlNoMBRType, nil)
+	c.Assert(err.Error(), Equals, `invalid volume "frobinator-image": invalid structure #4 ("data"): invalid type "0FC63DAF-8483-4772-8E79-3D69D8477DE4": both MBR type and GUID structure type needs to be defined on partial schemas`)
+}
+
+func (s *gadgetYamlTestSuite) TestGadgetPartialStructure(c *C) {
+	var yaml = []byte(`
+volumes:
+ frobinator-image:
+ partial: [structure]
+ bootloader: u-boot
+ schema: gpt
+ structure:
+ - name: ubuntu-seed
+ filesystem: ext4
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-seed
+ # Space for some unknown structure in the middle is left around
+ - name: ubuntu-boot
+ filesystem: ext4
+ offset: 1000M
+ size: 500M
+ type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
+ role: system-boot
+`)
+
+	// This test does not do a lot as this code does not check gaps
+	// between structures, but is left as a safeguard.
+	_, err := gadget.InfoFromGadgetYaml(yaml, nil)
+	c.Assert(err, IsNil)
+}