diff --git a/config/common/errors.go b/config/common/errors.go index 0f82b400..1f0c8386 100644 --- a/config/common/errors.go +++ b/config/common/errors.go @@ -68,6 +68,8 @@ var ( // partition ErrReuseByLabel = errors.New("partitions cannot be reused by label; number must be specified except on boot disk (/dev/disk/by-id/coreos-boot-disk) or when wipe_table is true") ErrWrongPartitionNumber = errors.New("incorrect partition number; a new partition will be created using reserved label") + ErrRootTooSmall = errors.New("root should have 8GiB of space assigned") + ErrRootConstrained = errors.New("root is configured too small, and has no room to expand") // MachineConfigs ErrFieldElided = errors.New("field ignored in raw mode") diff --git a/config/fcos/v1_3/translate.go b/config/fcos/v1_3/translate.go index 748fb5e8..fec8d4fc 100644 --- a/config/fcos/v1_3/translate.go +++ b/config/fcos/v1_3/translate.go @@ -70,16 +70,70 @@ func (c Config) ToIgn3_2Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_3/translate_test.go b/config/fcos/v1_3/translate_test.go index 22dec8c8..63dcb7bc 100644 --- a/config/fcos/v1_3/translate_test.go +++ b/config/fcos/v1_3/translate_test.go @@ -1423,3 +1423,178 @@ func TestTranslateBootDevice(t *testing.T) { }) } } + +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_2Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} diff --git a/config/fcos/v1_4/translate.go b/config/fcos/v1_4/translate.go index 9723edaa..b4dd7b80 100644 --- a/config/fcos/v1_4/translate.go +++ b/config/fcos/v1_4/translate.go @@ -70,16 +70,70 @@ func (c Config) ToIgn3_3Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_4/translate_test.go b/config/fcos/v1_4/translate_test.go index 3640c900..21baa717 100644 --- a/config/fcos/v1_4/translate_test.go +++ b/config/fcos/v1_4/translate_test.go @@ -1423,3 +1423,178 @@ func TestTranslateBootDevice(t *testing.T) { }) } } + +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_3Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} diff --git a/config/fcos/v1_5/translate.go b/config/fcos/v1_5/translate.go index 473a0db3..a6070c86 100644 --- a/config/fcos/v1_5/translate.go +++ b/config/fcos/v1_5/translate.go @@ -71,16 +71,70 @@ func (c Config) ToIgn3_4Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_5/translate_test.go b/config/fcos/v1_5/translate_test.go index 78c2e7a1..1165ed7c 100644 --- a/config/fcos/v1_5/translate_test.go +++ b/config/fcos/v1_5/translate_test.go @@ -1506,6 +1506,181 @@ func TestTranslateBootDevice(t *testing.T) { } } +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_4Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} + // TestTranslateGrub tests translating the Butane config Grub section. func TestTranslateGrub(t *testing.T) { // Some tests below have the same translations diff --git a/config/fcos/v1_6/translate.go b/config/fcos/v1_6/translate.go index 2d24cc91..ea670b83 100644 --- a/config/fcos/v1_6/translate.go +++ b/config/fcos/v1_6/translate.go @@ -71,16 +71,70 @@ func (c Config) ToIgn3_5Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_6/translate_test.go b/config/fcos/v1_6/translate_test.go index 99c30253..8281d016 100644 --- a/config/fcos/v1_6/translate_test.go +++ b/config/fcos/v1_6/translate_test.go @@ -1506,6 +1506,181 @@ func TestTranslateBootDevice(t *testing.T) { } } +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_5Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} + // TestTranslateGrub tests translating the Butane config Grub section. func TestTranslateGrub(t *testing.T) { // Some tests below have the same translations diff --git a/config/fcos/v1_7_exp/translate.go b/config/fcos/v1_7_exp/translate.go index bd31358e..a380534c 100644 --- a/config/fcos/v1_7_exp/translate.go +++ b/config/fcos/v1_7_exp/translate.go @@ -71,16 +71,70 @@ func (c Config) ToIgn3_6Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_7_exp/translate_test.go b/config/fcos/v1_7_exp/translate_test.go index 380a80eb..ab9633d0 100644 --- a/config/fcos/v1_7_exp/translate_test.go +++ b/config/fcos/v1_7_exp/translate_test.go @@ -1506,6 +1506,181 @@ func TestTranslateBootDevice(t *testing.T) { } } +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_6Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} + // TestTranslateGrub tests translating the Butane config Grub section. func TestTranslateGrub(t *testing.T) { // Some tests below have the same translations diff --git a/docs/release-notes.md b/docs/release-notes.md index 5f4d4b66..53b442e8 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -14,6 +14,9 @@ nav_order: 9 ### Misc. changes +- Warn on root partition size is too small _(fcos 1.3.0-1.7.0-exp)_ +- Warn on root partition constrained by another partition _(fcos 1.3.0-1.7.0-exp)_ + ### Docs changes @@ -36,7 +39,6 @@ key](https://getfedora.org/security/). ### Bug fixes - Warn for `boot_device.layout` to be specified when using `boot_device.mirror` _(fcos 1.3.0-1.6.0)_ - ### Docs changes - Update `boot_device.mirror` examples to specify `boot_device.layout`