From 13981c8ee494f3df40e7044425aa3b1ab755af6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Tue, 3 Dec 2024 10:42:04 +0100 Subject: [PATCH 1/6] distro: move listTestedDistros to distro_test It's not needed anywhere else, and it removes the dependency of distro_test_common on all distros (via repogistry) which helps with the subsequent commits. The function was also made private, because it's used in just one package. --- pkg/distro/distro_test.go | 20 ++++++++++++++----- .../distro_test_common/distro_test_common.go | 12 ----------- 2 files changed, 15 insertions(+), 17 deletions(-) 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..0d385541fe 100644 --- a/pkg/distro/distro_test_common/distro_test_common.go +++ b/pkg/distro/distro_test_common/distro_test_common.go @@ -6,12 +6,10 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/osbuild/images/pkg/blueprint" "github.com/osbuild/images/pkg/distro" "github.com/osbuild/images/pkg/ostree" - testrepos "github.com/osbuild/images/test/data/repositories" ) const RandomTestSeed = 0 @@ -475,13 +473,3 @@ 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 -} From 2c1b20a1ac440bbbd943d7240bc03cac3f218fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Tue, 3 Dec 2024 11:08:24 +0100 Subject: [PATCH 2/6] distro: add tests for ESP partition availability TestESP checks whether all images marked as UEFI or hybrid bootable have an ESP partition in their base partition table. The opposite is checked as well: legacy-only images shall not have an ESP partition. The code is deduplicated as much as possible. It meant adding a proxy function argument to the shared TestESP function because partition tables are not a public interface, but it's imho good enough. Maybe we can go one step further once the GetPartitionTable interface is unified across rhel and fedora, and just make it public. Maybe. --- .../distro_test_common/distro_test_common.go | 44 +++++++++++++++++++ pkg/distro/fedora/distro_internal_test.go | 28 ++++++++++++ pkg/distro/rhel/distro_test.go | 30 +++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 pkg/distro/fedora/distro_internal_test.go create mode 100644 pkg/distro/rhel/distro_test.go diff --git a/pkg/distro/distro_test_common/distro_test_common.go b/pkg/distro/distro_test_common/distro_test_common.go index 0d385541fe..a8d0a32f8f 100644 --- a/pkg/distro/distro_test_common/distro_test_common.go +++ b/pkg/distro/distro_test_common/distro_test_common.go @@ -6,10 +6,13 @@ import ( "testing" "github.com/stretchr/testify/assert" + "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" + "github.com/osbuild/images/pkg/platform" ) const RandomTestSeed = 0 @@ -473,3 +476,44 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) { } } } + +// 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..b54c72ee20 --- /dev/null +++ b/pkg/distro/rhel/distro_test.go @@ -0,0 +1,30 @@ +package rhel_test + +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" + "github.com/osbuild/images/pkg/distro/rhel" + "github.com/osbuild/images/pkg/distrofactory" +) + +// 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) + }) +} From 09507eeec946501d9ae00d35fd11a49109e80924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Tue, 3 Dec 2024 11:14:47 +0100 Subject: [PATCH 3/6] distro/rhel: add tests for ami/ec2* boot modes 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. --- pkg/distro/rhel/distro_test.go | 47 ++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pkg/distro/rhel/distro_test.go b/pkg/distro/rhel/distro_test.go index b54c72ee20..bc23f9e8c5 100644 --- a/pkg/distro/rhel/distro_test.go +++ b/pkg/distro/rhel/distro_test.go @@ -1,7 +1,9 @@ package rhel_test import ( + "fmt" "math/rand" + "strings" "testing" "github.com/osbuild/images/pkg/blueprint" @@ -10,6 +12,8 @@ import ( "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 @@ -28,3 +32,46 @@ func TestESP(t *testing.T) { 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()) + }) + } + } + +} From a16d65b2dded698e875ec3ba21af030f5679d14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 22 Nov 2024 21:29:26 +0100 Subject: [PATCH 4/6] distro/rhel8: fix ami being hybrid for 8.8 and older RHEL 8.8 and older doesn't support UEFI in AWS. The code correctly set it for EC2 images, but not for ami. This commit fixes that. --- pkg/distro/rhel/rhel8/distro.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) 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, From 117ae7f0dd6d5d9b54eae695dba1614bedcdee0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Mon, 2 Dec 2024 12:00:20 +0100 Subject: [PATCH 5/6] distro/rhel9: fix ami being hybrid for 9.2 and older RHEL 9.2 and older doesn't support UEFI in AWS. The code correctly set it for EC2 images, but not for ami. This commit fixes that. --- pkg/distro/rhel/rhel9/distro.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) 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())) From 419fee953be93340512ffdf199d6f607d765934f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Mon, 2 Dec 2024 12:00:32 +0100 Subject: [PATCH 6/6] distro/rhel9: remove ESP from non-UEFI AMI images RHEL 9.2 and older don't support UEFI on AWS. Let's remove also the ESP from there, as it's absolutely useless there. --- pkg/distro/rhel/rhel9/partition_tables.go | 43 +++++++++++++++++++++++ 1 file changed, 43 insertions(+) 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,