From e501adb3ff09101f0036ac26b8ad2876d37b6750 Mon Sep 17 00:00:00 2001 From: Chris Gunn Date: Wed, 11 Sep 2024 12:14:33 -0700 Subject: [PATCH] Image Customizer: Allow omitting disk maxSize and partition start. (#10383) Allow the partition start to be inferred from the previous partition's end. Also, allow the disk's maxSize to be inferred from the size/end of the last partition. In addition, since the partition start can now be omitted, require the partitions to be specified in order. Fortunately, most users do this anyway. --- .../tools/imagecustomizerapi/config_test.go | 35 +++-- toolkit/tools/imagecustomizerapi/disk.go | 99 ++++++++---- toolkit/tools/imagecustomizerapi/disk_test.go | 148 ++++++++++++++---- toolkit/tools/imagecustomizerapi/partition.go | 15 +- .../imagecustomizerapi/partition_test.go | 24 +-- toolkit/tools/imagecustomizerapi/storage.go | 4 +- .../tools/imagecustomizerapi/storage_test.go | 50 +++--- .../customizepartitions_test.go | 63 +++++++- .../imagecustomizerlib/liveosisobuilder.go | 12 +- .../testdata/partitions-size-only-config.yaml | 35 +++++ .../pkg/imagecustomizerlib/typeConversion.go | 8 +- 11 files changed, 351 insertions(+), 142 deletions(-) create mode 100644 toolkit/tools/pkg/imagecustomizerlib/testdata/partitions-size-only-config.yaml diff --git a/toolkit/tools/imagecustomizerapi/config_test.go b/toolkit/tools/imagecustomizerapi/config_test.go index 5233e0cf500..1f559a1cace 100644 --- a/toolkit/tools/imagecustomizerapi/config_test.go +++ b/toolkit/tools/imagecustomizerapi/config_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/microsoft/azurelinux/toolkit/tools/imagegen/diskutils" + "github.com/microsoft/azurelinux/toolkit/tools/internal/ptrutils" "github.com/stretchr/testify/assert" ) @@ -15,11 +16,11 @@ func TestConfigIsValid(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Type: PartitionTypeESP, }, }, @@ -52,11 +53,11 @@ func TestConfigIsValidLegacy(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "boot", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Type: PartitionTypeBiosGrub, }, }, @@ -84,11 +85,11 @@ func TestConfigIsValidNoBootType(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 2 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), }, }, }}, @@ -109,11 +110,11 @@ func TestConfigIsValidMissingBootLoaderReset(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Type: PartitionTypeESP, }, }, @@ -145,11 +146,11 @@ func TestConfigIsValidMultipleDisks(t *testing.T) { Disks: []Disk{ { PartitionTableType: "gpt", - MaxSize: 1 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), }, { PartitionTableType: "gpt", - MaxSize: 1 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), }, }, BootType: "legacy", @@ -199,7 +200,7 @@ func TestConfigIsValidBadDisk(t *testing.T) { BootType: BootTypeEfi, Disks: []Disk{{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 0, + MaxSize: ptrutils.PtrTo(DiskSize(0)), }}, }, OS: &OS{ @@ -218,7 +219,7 @@ func TestConfigIsValidMissingEsp(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 2 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Partitions: []Partition{}, }}, BootType: "efi", @@ -239,7 +240,7 @@ func TestConfigIsValidMissingBiosBoot(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 2 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Partitions: []Partition{}, }}, BootType: "legacy", @@ -260,11 +261,11 @@ func TestConfigIsValidInvalidMountPoint(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Type: PartitionTypeESP, }, }, @@ -298,11 +299,11 @@ func TestConfigIsValidKernelCLI(t *testing.T) { Storage: &Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Type: PartitionTypeESP, }, }, diff --git a/toolkit/tools/imagecustomizerapi/disk.go b/toolkit/tools/imagecustomizerapi/disk.go index 794c7d69421..1ccb51bf06f 100644 --- a/toolkit/tools/imagecustomizerapi/disk.go +++ b/toolkit/tools/imagecustomizerapi/disk.go @@ -5,9 +5,9 @@ package imagecustomizerapi import ( "fmt" - "sort" "github.com/microsoft/azurelinux/toolkit/tools/imagegen/diskutils" + "github.com/microsoft/azurelinux/toolkit/tools/internal/ptrutils" ) const ( @@ -29,7 +29,8 @@ type Disk struct { PartitionTableType PartitionTableType `yaml:"partitionTableType"` // The virtual size of the disk. - MaxSize DiskSize `yaml:"maxSize"` + // Note: This value is filled in by IsValid(). + MaxSize *DiskSize `yaml:"maxSize"` // The partitions to allocate on the disk. Partitions []Partition `yaml:"partitions"` @@ -41,8 +42,10 @@ func (d *Disk) IsValid() error { return err } - if d.MaxSize <= 0 { - return fmt.Errorf("a disk's maxSize value (%d) must be a positive non-zero number", d.MaxSize) + if d.MaxSize != nil { + if *d.MaxSize <= 0 { + return fmt.Errorf("a disk's maxSize value (%d) must be a positive non-zero number", *d.MaxSize) + } } for i, partition := range d.Partitions { @@ -55,23 +58,41 @@ func (d *Disk) IsValid() error { gptHeaderSize := DiskSize(roundUp(GptHeaderSectorNum*DefaultSectorSize, DefaultPartitionAlignment)) gptFooterSize := DiskSize(roundUp(GptFooterSectorNum*DefaultSectorSize, DefaultPartitionAlignment)) - // Check for overlapping partitions. - // First, sort partitions by start index. - sortedPartitions := append([]Partition(nil), d.Partitions...) - sort.Slice(sortedPartitions, func(i, j int) bool { - return sortedPartitions[i].Start < sortedPartitions[j].Start - }) + // Auto-fill the start value from the previous partition's end value. + for i := range d.Partitions { + partition := &d.Partitions[i] + + if partition.Start == nil { + if i == 0 { + partition.Start = ptrutils.PtrTo(DiskSize(DefaultPartitionAlignment)) + } else { + prev := d.Partitions[i-1] + prevEnd, prevHasEnd := prev.GetEnd() + if !prevHasEnd { + return fmt.Errorf("partition (%s) omitted start value but previous partition (%s) has no size or end value", + partition.Id, prev.Id) + } + partition.Start = &prevEnd + } + } + + if partition.IsBiosBoot() { + if *partition.Start != diskutils.MiB { + return fmt.Errorf("BIOS boot partition must start at 1 MiB") + } + } + } - // Then, confirm each partition ends before the next starts. - for i := 0; i < len(sortedPartitions)-1; i++ { - a := &sortedPartitions[i] - b := &sortedPartitions[i+1] + // Confirm each partition ends before the next starts. + for i := 0; i < len(d.Partitions)-1; i++ { + a := d.Partitions[i] + b := d.Partitions[i+1] aEnd, aHasEnd := a.GetEnd() if !aHasEnd { return fmt.Errorf("partition (%s) is not last partition but size is set to \"grow\"", a.Id) } - if aEnd > b.Start { + if aEnd > *b.Start { bEnd, bHasEnd := b.GetEnd() bEndStr := "" if bHasEnd { @@ -82,31 +103,47 @@ func (d *Disk) IsValid() error { } } - if len(sortedPartitions) > 0 { + if d.MaxSize == nil && len(d.Partitions) <= 0 { + return fmt.Errorf("either disk must specify maxSize or last partition must have an end or size value") + } + + if len(d.Partitions) > 0 { // Make sure the first block isn't used. - firstPartition := sortedPartitions[0] - if firstPartition.Start < gptHeaderSize { + firstPartition := d.Partitions[0] + if *firstPartition.Start < gptHeaderSize { return fmt.Errorf("invalid partition (%s) start:\nfirst %s of disk is reserved for the GPT header", firstPartition.Id, gptHeaderSize.HumanReadable()) } - // Check that the disk is big enough for the partition layout. - lastPartition := sortedPartitions[len(sortedPartitions)-1] - + // Verify MaxSize value. + lastPartition := d.Partitions[len(d.Partitions)-1] lastPartitionEnd, lastPartitionHasEnd := lastPartition.GetEnd() - var requiredSize DiskSize - if !lastPartitionHasEnd { - requiredSize = lastPartition.Start + DefaultPartitionAlignment - } else { - requiredSize = lastPartitionEnd - } + switch { + case !lastPartitionHasEnd && d.MaxSize == nil: + return fmt.Errorf("either disk must specify maxSize or last partition (%s) must have an end or size value", + lastPartition.Id) + + case d.MaxSize == nil: + // Fill in the disk's size. + diskSize := lastPartitionEnd + gptFooterSize + d.MaxSize = &diskSize + + default: + // Check that the disk is big enough for the partition layout. + var requiredSize DiskSize + if !lastPartitionHasEnd { + requiredSize = *lastPartition.Start + DefaultPartitionAlignment + } else { + requiredSize = lastPartitionEnd + } - requiredSize += gptFooterSize + requiredSize += gptFooterSize - if requiredSize > d.MaxSize { - return fmt.Errorf("disk's partitions need %s but maxSize is only %s:\nGPT footer size is %s", - requiredSize.HumanReadable(), d.MaxSize.HumanReadable(), gptFooterSize.HumanReadable()) + if requiredSize > *d.MaxSize { + return fmt.Errorf("disk's partitions need %s but maxSize is only %s:\nGPT footer size is %s", + requiredSize.HumanReadable(), d.MaxSize.HumanReadable(), gptFooterSize.HumanReadable()) + } } } diff --git a/toolkit/tools/imagecustomizerapi/disk_test.go b/toolkit/tools/imagecustomizerapi/disk_test.go index 7757fdd1aa5..ebe12936f56 100644 --- a/toolkit/tools/imagecustomizerapi/disk_test.go +++ b/toolkit/tools/imagecustomizerapi/disk_test.go @@ -14,11 +14,11 @@ import ( func TestDiskIsValid(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), }, }, } @@ -30,11 +30,11 @@ func TestDiskIsValid(t *testing.T) { func TestDiskIsValidWithEnd(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), }, }, @@ -47,11 +47,11 @@ func TestDiskIsValidWithEnd(t *testing.T) { func TestDiskIsValidWithSize(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Size: PartitionSize{ Type: PartitionSizeTypeExplicit, Size: 1 * diskutils.MiB, @@ -67,11 +67,11 @@ func TestDiskIsValidWithSize(t *testing.T) { func TestDiskIsValidStartAt0(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), }, }, } @@ -85,11 +85,11 @@ func TestDiskIsValidStartAt0(t *testing.T) { func TestDiskIsValidInvalidTableType(t *testing.T) { disk := &Disk{ PartitionTableType: "a", - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), }, }, } @@ -102,11 +102,11 @@ func TestDiskIsValidInvalidTableType(t *testing.T) { func TestDiskIsValidInvalidPartition(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 2 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(0)), }, }, @@ -121,15 +121,15 @@ func TestDiskIsValidInvalidPartition(t *testing.T) { func TestDiskIsValidTwoExpanding(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), }, { Id: "b", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), }, }, } @@ -142,15 +142,15 @@ func TestDiskIsValidTwoExpanding(t *testing.T) { func TestDiskIsValidTwoExpandingGrow(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), }, { Id: "b", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Size: PartitionSize{ Type: PartitionSizeTypeGrow, }, @@ -166,16 +166,16 @@ func TestDiskIsValidTwoExpandingGrow(t *testing.T) { func TestDiskIsValidOverlaps(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), }, { Id: "b", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), }, }, @@ -189,16 +189,16 @@ func TestDiskIsValidOverlaps(t *testing.T) { func TestDiskIsValidOverlapsExpanding(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), }, { Id: "b", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), }, }, } @@ -211,16 +211,16 @@ func TestDiskIsValidOverlapsExpanding(t *testing.T) { func TestDiskIsValidTooSmall(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), }, { Id: "b", - Start: 3 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), }, }, @@ -235,16 +235,16 @@ func TestDiskIsValidTooSmall(t *testing.T) { func TestDiskIsValidTooSmallExpanding(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), }, { Id: "b", - Start: 3 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), }, }, } @@ -258,7 +258,7 @@ func TestDiskIsValidTooSmallExpanding(t *testing.T) { func TestDiskIsValidZeroSize(t *testing.T) { disk := &Disk{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 0, + MaxSize: ptrutils.PtrTo(DiskSize(0)), Partitions: []Partition{}, } @@ -266,3 +266,89 @@ func TestDiskIsValidZeroSize(t *testing.T) { assert.Error(t, err) assert.ErrorContains(t, err, "a disk's maxSize value (0) must be a positive non-zero number") } + +func TestDiskIsValidOmitMaxSizeAndPartitions(t *testing.T) { + disk := &Disk{ + PartitionTableType: PartitionTableTypeGpt, + MaxSize: nil, + Partitions: []Partition{}, + } + + err := disk.IsValid() + assert.Error(t, err) + assert.ErrorContains(t, err, "either disk must specify maxSize or last partition must have an end or size value") +} + +func TestDiskIsValidOmitMaxSizeAndGrow(t *testing.T) { + disk := &Disk{ + PartitionTableType: PartitionTableTypeGpt, + MaxSize: nil, + Partitions: []Partition{ + { + Id: "a", + Size: PartitionSize{ + Type: PartitionSizeTypeExplicit, + Size: 1 * diskutils.MiB, + }, + }, + { + Id: "b", + Size: PartitionSize{ + Type: PartitionSizeTypeGrow, + }, + }, + }, + } + + err := disk.IsValid() + assert.Error(t, err) + assert.ErrorContains(t, err, "either disk must specify maxSize or last partition (b) must have an end or size value") +} + +func TestDiskIsValidOmitMaxSizeAndStart(t *testing.T) { + disk := &Disk{ + PartitionTableType: PartitionTableTypeGpt, + MaxSize: nil, + Partitions: []Partition{ + { + Id: "a", + Size: PartitionSize{ + Type: PartitionSizeTypeExplicit, + Size: 1 * diskutils.MiB, + }, + }, + { + Id: "b", + Size: PartitionSize{ + Type: PartitionSizeTypeExplicit, + Size: 2 * diskutils.MiB, + }, + }, + }, + } + + err := disk.IsValid() + assert.NoError(t, err) +} + +func TestDiskIsValidOmitStartNoEnd(t *testing.T) { + disk := &Disk{ + PartitionTableType: PartitionTableTypeGpt, + MaxSize: nil, + Partitions: []Partition{ + { + Id: "a", + }, + { + Id: "b", + Size: PartitionSize{ + Type: PartitionSizeTypeExplicit, + Size: 2 * diskutils.MiB, + }, + }, + }, + } + + err := disk.IsValid() + assert.ErrorContains(t, err, "partition (b) omitted start value but previous partition (a) has no size or end value") +} diff --git a/toolkit/tools/imagecustomizerapi/partition.go b/toolkit/tools/imagecustomizerapi/partition.go index 2c4dfc51345..a86481d9386 100644 --- a/toolkit/tools/imagecustomizerapi/partition.go +++ b/toolkit/tools/imagecustomizerapi/partition.go @@ -6,8 +6,6 @@ package imagecustomizerapi import ( "fmt" "unicode" - - "github.com/microsoft/azurelinux/toolkit/tools/imagegen/diskutils" ) type Partition struct { @@ -16,7 +14,8 @@ type Partition struct { // Name is the label to assign to the partition. Label string `yaml:"label"` // Start is the offset where the partition begins (inclusive). - Start DiskSize `yaml:"start"` + // Note: When not provided, value is filled in by Disk.IsValid(). + Start *DiskSize `yaml:"start"` // End is the offset where the partition ends (exclusive). End *DiskSize `yaml:"end"` // Size is the size of the partition. @@ -35,7 +34,7 @@ func (p *Partition) IsValid() error { return fmt.Errorf("cannot specify both end and size on partition (%s)", p.Id) } - if (p.End != nil && p.Start >= *p.End) || (p.Size.Type == PartitionSizeTypeExplicit && p.Size.Size <= 0) { + if (p.End != nil && p.Start != nil && *p.Start >= *p.End) || (p.Size.Type == PartitionSizeTypeExplicit && p.Size.Size <= 0) { return fmt.Errorf("partition's (%s) size can't be 0 or negative", p.Id) } @@ -44,12 +43,6 @@ func (p *Partition) IsValid() error { return err } - if p.IsBiosBoot() { - if p.Start != diskutils.MiB { - return fmt.Errorf("BIOS boot partition must start at 1 MiB") - } - } - return nil } @@ -59,7 +52,7 @@ func (p *Partition) GetEnd() (DiskSize, bool) { } if p.Size.Type == PartitionSizeTypeExplicit { - return p.Start + p.Size.Size, true + return *p.Start + p.Size.Size, true } return 0, false diff --git a/toolkit/tools/imagecustomizerapi/partition_test.go b/toolkit/tools/imagecustomizerapi/partition_test.go index f64b3c9b70a..14c840ec281 100644 --- a/toolkit/tools/imagecustomizerapi/partition_test.go +++ b/toolkit/tools/imagecustomizerapi/partition_test.go @@ -14,7 +14,7 @@ import ( func TestPartitionIsValidExpanding(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), } err := partition.IsValid() @@ -24,7 +24,7 @@ func TestPartitionIsValidExpanding(t *testing.T) { func TestPartitionIsValidFixedSize(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), End: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), } @@ -35,7 +35,7 @@ func TestPartitionIsValidFixedSize(t *testing.T) { func TestPartitionIsValidZeroSize(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), End: ptrutils.PtrTo(DiskSize(0)), } @@ -48,7 +48,7 @@ func TestPartitionIsValidZeroSize(t *testing.T) { func TestPartitionIsValidZeroSizeV2(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), Size: PartitionSize{ Type: PartitionSizeTypeExplicit, Size: 0, @@ -63,7 +63,7 @@ func TestPartitionIsValidZeroSizeV2(t *testing.T) { func TestPartitionIsValidNegativeSize(t *testing.T) { partition := Partition{ Id: "a", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), } @@ -75,7 +75,7 @@ func TestPartitionIsValidNegativeSize(t *testing.T) { func TestPartitionIsValidBothEndAndSize(t *testing.T) { partition := Partition{ Id: "a", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Size: PartitionSize{ Type: PartitionSizeTypeExplicit, @@ -91,7 +91,7 @@ func TestPartitionIsValidBothEndAndSize(t *testing.T) { func TestPartitionIsValidEndAndGrow(t *testing.T) { partition := Partition{ Id: "a", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Size: PartitionSize{ Type: PartitionSizeTypeGrow, @@ -106,7 +106,7 @@ func TestPartitionIsValidEndAndGrow(t *testing.T) { func TestPartitionIsValidGoodName(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), End: nil, Label: "a", } @@ -118,7 +118,7 @@ func TestPartitionIsValidGoodName(t *testing.T) { func TestPartitionIsValidNameTooLong(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), End: nil, Label: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", } @@ -132,7 +132,7 @@ func TestPartitionIsValidNameTooLong(t *testing.T) { func TestPartitionIsValidNameNonASCII(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), End: nil, Label: "❤️", } @@ -146,7 +146,7 @@ func TestPartitionIsValidNameNonASCII(t *testing.T) { func TestPartitionIsValidGoodType(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), End: nil, Type: PartitionTypeESP, } @@ -158,7 +158,7 @@ func TestPartitionIsValidGoodType(t *testing.T) { func TestPartitionIsValidBadType(t *testing.T) { partition := Partition{ Id: "a", - Start: 0, + Start: ptrutils.PtrTo(DiskSize(0)), End: nil, Type: PartitionType("a"), } diff --git a/toolkit/tools/imagecustomizerapi/storage.go b/toolkit/tools/imagecustomizerapi/storage.go index 4184fd1ab1e..d6a7d633476 100644 --- a/toolkit/tools/imagecustomizerapi/storage.go +++ b/toolkit/tools/imagecustomizerapi/storage.go @@ -28,7 +28,9 @@ func (s *Storage) IsValid() error { return fmt.Errorf("defining multiple disks is not currently supported") } - for i, disk := range s.Disks { + for i := range s.Disks { + disk := &s.Disks[i] + err := disk.IsValid() if err != nil { return fmt.Errorf("invalid disk at index %d:\n%w", i, err) diff --git a/toolkit/tools/imagecustomizerapi/storage_test.go b/toolkit/tools/imagecustomizerapi/storage_test.go index 8e9b06f1f9a..7b31a27142e 100644 --- a/toolkit/tools/imagecustomizerapi/storage_test.go +++ b/toolkit/tools/imagecustomizerapi/storage_test.go @@ -15,11 +15,11 @@ func TestStorageIsValidDuplicatePartitionID(t *testing.T) { value := Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 2 * diskutils.GiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.GiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Type: PartitionTypeESP, }, }, @@ -52,11 +52,11 @@ func TestStorageIsValidUnsupportedFileSystem(t *testing.T) { storage := Storage{ Disks: []Disk{{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 2 * diskutils.GiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.GiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: nil, }, }, @@ -79,11 +79,11 @@ func TestStorageIsValidMissingFileSystemEntry(t *testing.T) { storage := Storage{ Disks: []Disk{{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 2 * diskutils.GiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.GiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: nil, Type: PartitionTypeESP, }, @@ -102,11 +102,11 @@ func TestStorageIsValidBadEspFsType(t *testing.T) { storage := Storage{ Disks: []Disk{{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 2 * diskutils.GiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.GiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: nil, Type: PartitionTypeESP, }, @@ -130,11 +130,11 @@ func TestStorageIsValidBadBiosBootFsType(t *testing.T) { storage := Storage{ Disks: []Disk{{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 2 * diskutils.GiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.GiB)), Partitions: []Partition{ { Id: "bios", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: nil, Type: PartitionTypeBiosGrub, }, @@ -158,11 +158,11 @@ func TestStorageIsValidBadBiosBootStart(t *testing.T) { storage := Storage{ Disks: []Disk{{ PartitionTableType: PartitionTableTypeGpt, - MaxSize: 2 * diskutils.GiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.GiB)), Partitions: []Partition{ { Id: "bios", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), End: nil, Type: PartitionTypeBiosGrub, }, @@ -186,11 +186,11 @@ func TestStorageIsValidBadDeviceId(t *testing.T) { value := Storage{ Disks: []Disk{{ PartitionTableType: "gpt", - MaxSize: 2 * diskutils.GiB, + MaxSize: ptrutils.PtrTo(DiskSize(2 * diskutils.GiB)), Partitions: []Partition{ { Id: "esp", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), Type: PartitionTypeESP, }, }, @@ -222,16 +222,16 @@ func TestStorageIsValidDuplicatePartitionId(t *testing.T) { Disks: []Disk{ { PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), }, { Id: "a", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), }, }, }, @@ -254,11 +254,11 @@ func TestStorageIsValidNoLabel(t *testing.T) { Disks: []Disk{ { PartitionTableType: PartitionTableTypeGpt, - MaxSize: 3 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(3 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Type: PartitionTypeESP, }, @@ -288,18 +288,18 @@ func TestStorageIsValidUniqueLabel(t *testing.T) { Disks: []Disk{ { PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Type: PartitionTypeESP, Label: "a", }, { Id: "b", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Label: "b", }, }, @@ -335,18 +335,18 @@ func TestStorageIsValidDuplicateLabel(t *testing.T) { Disks: []Disk{ { PartitionTableType: PartitionTableTypeGpt, - MaxSize: 4 * diskutils.MiB, + MaxSize: ptrutils.PtrTo(DiskSize(4 * diskutils.MiB)), Partitions: []Partition{ { Id: "a", - Start: 1 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(1 * diskutils.MiB)), End: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Type: PartitionTypeESP, Label: "a", }, { Id: "b", - Start: 2 * diskutils.MiB, + Start: ptrutils.PtrTo(DiskSize(2 * diskutils.MiB)), Label: "a", }, }, diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizepartitions_test.go b/toolkit/tools/pkg/imagecustomizerlib/customizepartitions_test.go index ab34f36762f..e0a1d6088c3 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizepartitions_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizepartitions_test.go @@ -104,9 +104,6 @@ func testCustomizeImagePartitionsToEfi(t *testing.T, testName string, imageType _, err = os.Stat(filepath.Join(imageConnection.Chroot().RootDir(), "/var/log")) assert.NoError(t, err, "check for /var/log") - partitions, err = getDiskPartitionsMap(imageConnection.Loopback().DevicePath()) - assert.NoError(t, err, "get disk partitions") - // Check that the fstab entries are correct. verifyFstabEntries(t, imageConnection, mountPoints, partitions) verifyBootloaderConfig(t, imageConnection, "console=tty0 console=ttyS0", @@ -115,6 +112,66 @@ func testCustomizeImagePartitionsToEfi(t *testing.T, testName string, imageType imageVersion) } +func TestCustomizeImagePartitionsSizeOnly(t *testing.T) { + baseImage := checkSkipForCustomizeImage(t, baseImageTypeCoreEfi, baseImageVersionDefault) + + testTmpDir := filepath.Join(tmpDir, "TestCustomizeImagePartitionsSizeOnly") + buildDir := filepath.Join(testTmpDir, "build") + configFile := filepath.Join(testDir, "partitions-size-only-config.yaml") + outImageFilePath := filepath.Join(testTmpDir, "image.raw") + + // Customize image. + err := CustomizeImageWithConfigFile(buildDir, configFile, baseImage, nil, outImageFilePath, "raw", "", + false /*useBaseImageRpmRepos*/, false /*enableShrinkFilesystems*/) + if !assert.NoError(t, err) { + return + } + + // Check output file type. + checkFileType(t, outImageFilePath, "raw") + + mountPoints := []mountPoint{ + { + PartitionNum: 2, + Path: "/", + FileSystemType: "ext4", + }, + { + PartitionNum: 1, + Path: "/boot/efi", + FileSystemType: "vfat", + }, + { + PartitionNum: 3, + Path: "/var", + FileSystemType: "ext4", + }, + } + + imageConnection, err := connectToImage(buildDir, outImageFilePath, false /*includeDefaultMounts*/, mountPoints) + if !assert.NoError(t, err) { + return + } + defer imageConnection.Close() + + // Check for key files/directories on the partitions. + _, err = os.Stat(filepath.Join(imageConnection.Chroot().RootDir(), "/usr/bin/bash")) + assert.NoError(t, err, "check for /usr/bin/bash") + + _, err = os.Stat(filepath.Join(imageConnection.Chroot().RootDir(), "/var/log")) + assert.NoError(t, err, "check for /var/log") + + partitions, err := getDiskPartitionsMap(imageConnection.Loopback().DevicePath()) + assert.NoError(t, err, "get disk partitions") + + // Check that the fstab entries are correct. + verifyFstabEntries(t, imageConnection, mountPoints, partitions) + verifyBootloaderConfig(t, imageConnection, "", + partitions[mountPoints[0].PartitionNum], + partitions[mountPoints[0].PartitionNum], + baseImageVersionDefault) +} + func TestCustomizeImagePartitionsEfiToLegacy(t *testing.T) { for _, version := range supportedAzureLinuxVersions { t.Run(string(version), func(t *testing.T) { diff --git a/toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go b/toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go index e6b3311a659..e93652e824e 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go +++ b/toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go @@ -1424,24 +1424,22 @@ func (b *LiveOSIsoBuilder) createWriteableImageFromSquashfs(buildDir, rawImageFi // define a disk layout with a boot partition and a rootfs partition maxDiskSizeMB := imagecustomizerapi.DiskSize(safeDiskSizeMB * diskutils.MiB) - var bootPartitionStart imagecustomizerapi.DiskSize - bootPartitionStart = imagecustomizerapi.DiskSize(1 * diskutils.MiB) - var bootPartitionEnd imagecustomizerapi.DiskSize - bootPartitionEnd = imagecustomizerapi.DiskSize(9 * diskutils.MiB) + bootPartitionStart := imagecustomizerapi.DiskSize(1 * diskutils.MiB) + bootPartitionEnd := imagecustomizerapi.DiskSize(9 * diskutils.MiB) diskConfig := imagecustomizerapi.Disk{ PartitionTableType: imagecustomizerapi.PartitionTableTypeGpt, - MaxSize: maxDiskSizeMB, + MaxSize: &maxDiskSizeMB, Partitions: []imagecustomizerapi.Partition{ { Id: "esp", - Start: bootPartitionStart, + Start: &bootPartitionStart, End: &bootPartitionEnd, Type: imagecustomizerapi.PartitionTypeESP, }, { Id: "rootfs", - Start: bootPartitionEnd, + Start: &bootPartitionEnd, }, }, } diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/partitions-size-only-config.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/partitions-size-only-config.yaml new file mode 100644 index 00000000000..8a69ceddce9 --- /dev/null +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/partitions-size-only-config.yaml @@ -0,0 +1,35 @@ +storage: + disks: + - partitionTableType: gpt + partitions: + - id: esp + type: esp + size: 8M + + - id: rootfs + size: 2G + + - id: var + size: 2G + + bootType: efi + + fileSystems: + - deviceId: esp + type: fat32 + mountPoint: + path: /boot/efi + options: umask=0077 + + - deviceId: rootfs + type: ext4 + mountPoint: + path: / + + - deviceId: var + type: ext4 + mountPoint: + path: /var + +os: + resetBootLoaderType: hard-reset diff --git a/toolkit/tools/pkg/imagecustomizerlib/typeConversion.go b/toolkit/tools/pkg/imagecustomizerlib/typeConversion.go index 0d421246c21..5d56773fd13 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/typeConversion.go +++ b/toolkit/tools/pkg/imagecustomizerlib/typeConversion.go @@ -37,8 +37,8 @@ func diskConfigToImager(diskConfig imagecustomizerapi.Disk, fileSystems []imagec return configuration.Disk{}, err } - imagerMaxSize := diskConfig.MaxSize / diskutils.MiB - if diskConfig.MaxSize%diskutils.MiB != 0 { + imagerMaxSize := *diskConfig.MaxSize / diskutils.MiB + if *diskConfig.MaxSize%diskutils.MiB != 0 { return configuration.Disk{}, fmt.Errorf("disk max size (%d) must be a multiple of 1 MiB", diskConfig.MaxSize) } @@ -87,8 +87,8 @@ func partitionToImager(partition imagecustomizerapi.Partition, fileSystems []ima return configuration.Partition{}, fmt.Errorf("failed to find filesystem entry with ID (%s)", partition.Id) } - imagerStart := partition.Start / diskutils.MiB - if partition.Start%diskutils.MiB != 0 { + imagerStart := *partition.Start / diskutils.MiB + if *partition.Start%diskutils.MiB != 0 { return configuration.Partition{}, fmt.Errorf("partition start (%d) must be a multiple of 1 MiB", partition.Start) }