diff --git a/pkg/distro/distro_test.go b/pkg/distro/distro_test.go index 55ac80d483..78b8244498 100644 --- a/pkg/distro/distro_test.go +++ b/pkg/distro/distro_test.go @@ -13,14 +13,24 @@ import ( "github.com/osbuild/images/pkg/blueprint" "github.com/osbuild/images/pkg/container" "github.com/osbuild/images/pkg/distro" - "github.com/osbuild/images/pkg/distro/distro_test_common" "github.com/osbuild/images/pkg/distrofactory" "github.com/osbuild/images/pkg/ostree" "github.com/osbuild/images/pkg/rpmmd" + testrepos "github.com/osbuild/images/test/data/repositories" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// listTestedDistros returns a list of distro names that are explicitly tested +func listTestedDistros(t *testing.T) []string { + testRepoRegistry, err := testrepos.New() + require.Nil(t, err) + require.NotEmpty(t, testRepoRegistry) + distros := testRepoRegistry.ListDistros() + require.NotEmpty(t, distros) + return distros +} + // Ensure all image types report the correct names for their pipelines. // Each image type contains a list of build and payload pipelines. They are // needed for knowing the names of pipelines from the static object without @@ -45,7 +55,7 @@ func TestImageTypePipelineNames(t *testing.T) { assert := assert.New(t) distroFactory := distrofactory.NewDefault() - distros := distro_test_common.ListTestedDistros(t) + distros := listTestedDistros(t) for _, distroName := range distros { d := distroFactory.GetDistro(distroName) for _, archName := range d.ListArches() { @@ -361,7 +371,7 @@ func TestPipelineRepositories(t *testing.T) { } distroFactory := distrofactory.NewDefault() - distros := distro_test_common.ListTestedDistros(t) + distros := listTestedDistros(t) for tName, tCase := range testCases { t.Run(tName, func(t *testing.T) { for _, distroName := range distros { @@ -529,7 +539,7 @@ func TestDistro_ManifestFIPSWarning(t *testing.T) { } distroFactory := distrofactory.NewDefault() - distros := distro_test_common.ListTestedDistros(t) + distros := listTestedDistros(t) for _, distroName := range distros { // FIPS blueprint customization is not supported for RHEL 7 images if strings.HasPrefix(distroName, "rhel-7") { @@ -578,7 +588,7 @@ func TestOSTreeOptionsErrorForNonOSTreeImgTypes(t *testing.T) { distroFactory := distrofactory.NewDefault() assert.NotNil(distroFactory) - distros := distro_test_common.ListTestedDistros(t) + distros := listTestedDistros(t) assert.NotEmpty(distros) for _, distroName := range distros { diff --git a/pkg/distro/distro_test_common/distro_test_common.go b/pkg/distro/distro_test_common/distro_test_common.go index 94a54c1768..a8d0a32f8f 100644 --- a/pkg/distro/distro_test_common/distro_test_common.go +++ b/pkg/distro/distro_test_common/distro_test_common.go @@ -9,9 +9,10 @@ import ( "github.com/stretchr/testify/require" "github.com/osbuild/images/pkg/blueprint" + "github.com/osbuild/images/pkg/disk" "github.com/osbuild/images/pkg/distro" "github.com/osbuild/images/pkg/ostree" - testrepos "github.com/osbuild/images/test/data/repositories" + "github.com/osbuild/images/pkg/platform" ) const RandomTestSeed = 0 @@ -476,12 +477,43 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) { } } -// ListTestedDistros returns a list of distro names that are explicitly tested -func ListTestedDistros(t *testing.T) []string { - testRepoRegistry, err := testrepos.New() - require.Nil(t, err) - require.NotEmpty(t, testRepoRegistry) - distros := testRepoRegistry.ListDistros() - require.NotEmpty(t, distros) - return distros +// TestESP checks whether all UEFI and hybrid images with a partition table have an ESP partition. +// It also checks the opposite, i.e. that legacy images don't have an ESP. This test is only +// performed on image types with a partition table, thus it doesn't run on e.g. installers and +// ostree commits. +// distros is a list of distros to test +// ptFunc is a function that returns an uncustomized partition table for a given image type. This +// proxy method is needed because the distro.ImageType interface doesn't provide a way to get a +// partition table. +func TestESP(t *testing.T, distros []distro.Distro, ptFunc func(i distro.ImageType) (*disk.PartitionTable, error)) { + for _, d := range distros { + for _, archName := range d.ListArches() { + a, err := d.GetArch(archName) + require.NoError(t, err) + + for _, itName := range a.ListImageTypes() { + i, err := a.GetImageType(itName) + require.NoError(t, err) + + // Skip image types that don't have a partition table. + if i.PartitionType() == disk.PT_NONE { + continue + } + + t.Run(fmt.Sprintf("%s/%s/%s", d.Name(), archName, itName), func(t *testing.T) { + pt, err := ptFunc(i) + require.NoError(t, err) + + switch i.BootMode() { + case platform.BOOT_HYBRID, platform.BOOT_UEFI: + require.NotNil(t, pt.FindMountable("/boot/efi")) + default: + require.Nil(t, pt.FindMountable("/boot/efi")) + } + + }) + } + } + + } } diff --git a/pkg/distro/fedora/distro_internal_test.go b/pkg/distro/fedora/distro_internal_test.go new file mode 100644 index 0000000000..59323fd9fa --- /dev/null +++ b/pkg/distro/fedora/distro_internal_test.go @@ -0,0 +1,28 @@ +package fedora + +import ( + "math/rand" + "testing" + + "github.com/osbuild/images/pkg/blueprint" + "github.com/osbuild/images/pkg/disk" + "github.com/osbuild/images/pkg/distro" + "github.com/osbuild/images/pkg/distro/distro_test_common" +) + +// math/rand is good enough in this case +/* #nosec G404 */ +var rng = rand.New(rand.NewSource(0)) + +func TestESP(t *testing.T) { + var distros []distro.Distro + for _, distroName := range []string{"fedora-40", "fedora-41", "fedora-42"} { + d := DistroFactory(distroName) + distros = append(distros, d) + } + + distro_test_common.TestESP(t, distros, func(i distro.ImageType) (*disk.PartitionTable, error) { + it := i.(*imageType) + return it.getPartitionTable(&blueprint.Customizations{}, distro.ImageOptions{}, rng) + }) +} diff --git a/pkg/distro/rhel/distro_test.go b/pkg/distro/rhel/distro_test.go new file mode 100644 index 0000000000..bc23f9e8c5 --- /dev/null +++ b/pkg/distro/rhel/distro_test.go @@ -0,0 +1,77 @@ +package rhel_test + +import ( + "fmt" + "math/rand" + "strings" + "testing" + + "github.com/osbuild/images/pkg/blueprint" + "github.com/osbuild/images/pkg/disk" + "github.com/osbuild/images/pkg/distro" + "github.com/osbuild/images/pkg/distro/distro_test_common" + "github.com/osbuild/images/pkg/distro/rhel" + "github.com/osbuild/images/pkg/distrofactory" + "github.com/osbuild/images/pkg/platform" + "github.com/stretchr/testify/require" +) + +// math/rand is good enough in this case +/* #nosec G404 */ +var rng = rand.New(rand.NewSource(0)) + +func TestESP(t *testing.T) { + var distros []distro.Distro + distroFactory := distrofactory.NewDefault() + for _, distroName := range []string{"rhel-7.9", "rhel-8.8", "rhel-8.9", "rhel-8.10", "centos-8", "rhel-9.0", "rhel-9.2", "rhel-9.4", "centos-9", "rhel-10.0", "centos-10"} { + distros = append(distros, distroFactory.GetDistro(distroName)) + } + + distro_test_common.TestESP(t, distros, func(i distro.ImageType) (*disk.PartitionTable, error) { + it := i.(*rhel.ImageType) + return it.GetPartitionTable([]blueprint.FilesystemCustomization{}, distro.ImageOptions{}, rng) + }) +} + +// TestAMIHybridBoot verifies that rhel 8.9 and 9.3 are the first RHEL versions +// that implemented hybrid boot for the ami and ec2* image types +func TestAMIHybridBoot(t *testing.T) { + testCases := []struct { + distro string + bootMode platform.BootMode + }{ + {"rhel-8.8", platform.BOOT_LEGACY}, + {"rhel-8.9", platform.BOOT_HYBRID}, + {"rhel-8.10", platform.BOOT_HYBRID}, + {"centos-8", platform.BOOT_HYBRID}, + {"rhel-9.0", platform.BOOT_LEGACY}, + {"rhel-9.2", platform.BOOT_LEGACY}, + {"rhel-9.3", platform.BOOT_HYBRID}, + {"rhel-9.4", platform.BOOT_HYBRID}, + {"centos-9", platform.BOOT_HYBRID}, + {"rhel-10.0", platform.BOOT_HYBRID}, + {"centos-10", platform.BOOT_HYBRID}, + } + + distroFactory := distrofactory.NewDefault() + + for _, tc := range testCases { + // test only x86_64. ami for aarch64 has always UEFI, other arches are not defined. + a, err := distroFactory.GetDistro(tc.distro).GetArch("x86_64") + require.NoError(t, err) + + for _, it := range a.ListImageTypes() { + // test only ami and ec2* image types + if it != "ami" && !strings.HasPrefix(it, "ec2") { + continue + } + t.Run(fmt.Sprintf("%s/%s", tc.distro, it), func(t *testing.T) { + it, err := a.GetImageType(it) + require.NoError(t, err) + + require.Equal(t, tc.bootMode, it.BootMode()) + }) + } + } + +} diff --git a/pkg/distro/rhel/rhel8/distro.go b/pkg/distro/rhel/rhel8/distro.go index f35f4500e3..4c4a1e3590 100644 --- a/pkg/distro/rhel/rhel8/distro.go +++ b/pkg/distro/rhel/rhel8/distro.go @@ -115,6 +115,15 @@ func newDistro(name string, minor int) *rhel.Distribution { ImageFormat: platform.FORMAT_RAW, }, } + + // Keep the RHEL EC2 x86_64 images before 8.9 BIOS-only for backward compatibility. + // RHEL-internal EC2 images and RHEL AMI images are kept intentionally in sync + // with regard to not supporting hybrid boot mode before RHEL version 8.9. + // The partitioning table for these reflects that and is also intentionally in sync. + if rd.IsRHEL() && common.VersionLessThan(rd.OsVersion(), "8.9") { + ec2X86Platform.UEFIVendor = "" + } + x86_64.AddImageTypes( ec2X86Platform, mkAmiImgTypeX86_64(), @@ -342,16 +351,6 @@ func newDistro(name string, minor int) *rhel.Distribution { mkAzureSapRhuiImgType(rd), ) - // keep the RHEL EC2 x86_64 images before 8.9 BIOS-only for backward compatibility - if common.VersionLessThan(rd.OsVersion(), "8.9") { - ec2X86Platform = &platform.X86{ - BIOS: true, - BasePlatform: platform.BasePlatform{ - ImageFormat: platform.FORMAT_RAW, - }, - } - } - // add ec2 image types to RHEL distro only x86_64.AddImageTypes( ec2X86Platform, diff --git a/pkg/distro/rhel/rhel9/distro.go b/pkg/distro/rhel/rhel9/distro.go index eb838c7bd4..06488134d2 100644 --- a/pkg/distro/rhel/rhel9/distro.go +++ b/pkg/distro/rhel/rhel9/distro.go @@ -202,6 +202,15 @@ func newDistro(name string, major, minor int) *rhel.Distribution { ImageFormat: platform.FORMAT_RAW, }, } + + // Keep the RHEL EC2 x86_64 images before 9.3 BIOS-only for backward compatibility. + // RHEL-internal EC2 images and RHEL AMI images are kept intentionally in sync + // with regard to not supporting hybrid boot mode before RHEL version 9.3. + // The partitioning table for these reflects that and is also intentionally in sync. + if rd.IsRHEL() && common.VersionLessThan(rd.OsVersion(), "9.3") { + ec2X86Platform.UEFIVendor = "" + } + x86_64.AddImageTypes( ec2X86Platform, mkAMIImgTypeX86_64(), @@ -337,16 +346,6 @@ func newDistro(name string, major, minor int) *rhel.Distribution { x86_64.AddImageTypes(azureX64Platform, mkAzureSapInternalImgType(rd)) - // keep the RHEL EC2 x86_64 images before 9.3 BIOS-only for backward compatibility - if common.VersionLessThan(rd.OsVersion(), "9.3") { - ec2X86Platform = &platform.X86{ - BIOS: true, - BasePlatform: platform.BasePlatform{ - ImageFormat: platform.FORMAT_RAW, - }, - } - } - // add ec2 image types to RHEL distro only x86_64.AddImageTypes(ec2X86Platform, mkEc2ImgTypeX86_64(), mkEc2HaImgTypeX86_64(), mkEC2SapImgTypeX86_64(rd.OsVersion())) diff --git a/pkg/distro/rhel/rhel9/partition_tables.go b/pkg/distro/rhel/rhel9/partition_tables.go index cb17cd210d..03959ea627 100644 --- a/pkg/distro/rhel/rhel9/partition_tables.go +++ b/pkg/distro/rhel/rhel9/partition_tables.go @@ -1,6 +1,8 @@ package rhel9 import ( + "strings" + "github.com/osbuild/images/internal/common" "github.com/osbuild/images/pkg/arch" "github.com/osbuild/images/pkg/datasizes" @@ -24,6 +26,47 @@ func defaultBasePartitionTables(t *rhel.ImageType) (disk.PartitionTable, bool) { switch t.Arch().Name() { case arch.ARCH_X86_64.String(): + // RHEL EC2 x86_64 images prior to 9.3 support only BIOS boot + if common.VersionLessThan(t.Arch().Distro().OsVersion(), "9.3") && t.IsRHEL() && (strings.HasPrefix(t.Name(), "ec2") || t.Name() == "ami") { + return disk.PartitionTable{ + UUID: "D209C89E-EA5E-4FBD-B161-B461CCE297E0", + Type: disk.PT_GPT, + Partitions: []disk.Partition{ + { + Size: 1 * datasizes.MebiByte, + Bootable: true, + Type: disk.BIOSBootPartitionGUID, + UUID: disk.BIOSBootPartitionUUID, + }, + { + Size: bootSize, + Type: disk.XBootLDRPartitionGUID, + UUID: disk.FilesystemDataUUID, + Payload: &disk.Filesystem{ + Type: "xfs", + Mountpoint: "/boot", + Label: "boot", + FSTabOptions: "defaults", + FSTabFreq: 0, + FSTabPassNo: 0, + }, + }, + { + Size: 2 * datasizes.GibiByte, + Type: disk.FilesystemDataGUID, + UUID: disk.RootPartitionUUID, + Payload: &disk.Filesystem{ + Type: "xfs", + Label: "root", + Mountpoint: "/", + FSTabOptions: "defaults", + FSTabFreq: 0, + FSTabPassNo: 0, + }, + }, + }, + }, true + } return disk.PartitionTable{ UUID: "D209C89E-EA5E-4FBD-B161-B461CCE297E0", Type: disk.PT_GPT,