Repository: osbuild/osbuild-composer
Pull request #1616 (2021-08-04 11:23:08)

[RHEL-8.5] ec2 images followup
This is a follow-up to #1607

Notable changes:
- image-info: mount partitions in correct order when analysing image
- composer: don't expose ec2 and ec2-ha RHEL-8.5 images via WeldrAPI
- docs/news: reflect EC2 image changes in the RHEL-8.5 images note
- rhel85: include dracut-config-generic in x86 UEFI boot package set

This pull request includes:

- [x] adequate testing for the new functionality or fixed issue
  - unit tests were extended / modified where it was appropriate
- [ ] adequate documentation informing people about the change such as
  - [x] create a file in [news/unreleased](https://github.com/osbuild/osbuild-composer/tree/main/docs/news/unreleased) directory if this change should be mentioned in the release news
    - extended an existing NEWS entry related to RHEL-8.5 image types
  - [ ] submit a PR for the [guides](https://github.com/osbuild/guides) repository if this PR changed any behavior described there: https://www.osbuild.org/guides/

<!--
Thanks for proposing a change to osbuild-composer!

Please don't remove the above check list. These are things that each pull
request must have before it is merged. It helps maintainers to not forget
anything.

If the reason for ticking any of the boxes is ambiguous, please add a short
note explaining why.

For user-visible changes, "adequate documentation" is an entry describing the
change for users in docs/news. Please refer to docs/news/README.md for details.

In addition, if this pull request fixes a downstream issue, please refer to
test/README.md and add these additional items:

- [ ] 1st commit of any `rhbz#` related PR contains bug reproducer; CI reports FAIL or
- [ ] PR contains automated tests for new functionality and
- [ ] QE has approved reproducer/new tests and
- [ ] Subsequent commits provide bug fixes without modifying the reproducer; CI reports PASS and
- [ ] QE approves this PR; RHBZ status is set to `MODIFIED + Verified=Tested`
-->



Patch:
diff --git a/cmd/osbuild-composer/config.go b/cmd/osbuild-composer/config.go
index 1cc737e1a..739a38ef2 100644
--- a/cmd/osbuild-composer/config.go
+++ b/cmd/osbuild-composer/config.go
@@ -28,9 +28,11 @@ type ComposerConfigFile struct {
 	ComposerAPI struct {
 		IdentityFilter []string `toml:"identity_filter"`
 	} `toml:"composer_api"`
-	WeldrAPI struct {
-		DistroConfigs map[string]WeldrDistroConfig `toml:"distros"`
-	} `toml:"weldr_api"`
+	WeldrAPI WeldrAPIConfig `toml:"weldr_api"`
+}
+
+type WeldrAPIConfig struct {
+	DistroConfigs map[string]WeldrDistroConfig `toml:"distros"`
 }
 
 type WeldrDistroConfig struct {
@@ -51,17 +53,35 @@ func (c *ComposerConfigFile) weldrDistrosImageTypeDenyList() map[string][]string
 	return distrosImageTypeDenyList
 }
 
+// GetDefaultConfig returns the default configuration of osbuild-composer
+// Defaults:
+// - 'ec2' and 'ec2-ha' image types on 'rhel-85' are not exposed via Weldr API
+func GetDefaultConfig() *ComposerConfigFile {
+	return &ComposerConfigFile{
+		WeldrAPI: WeldrAPIConfig{
+			map[string]WeldrDistroConfig{
+				"rhel-*": {
+					ImageTypeDenyList: []string{
+						"ec2",
+						"ec2-ha",
+					},
+				},
+			},
+		},
+	}
+}
+
 func LoadConfig(name string) (*ComposerConfigFile, error) {
-	var c ComposerConfigFile
-	_, err := toml.DecodeFile(name, &c)
+	c := GetDefaultConfig()
+	_, err := toml.DecodeFile(name, c)
 	if err != nil {
 		return nil, err
 	}
-	err = loadConfigFromEnv(&c)
+	err = loadConfigFromEnv(c)
 	if err != nil {
 		return nil, err
 	}
-	return &c, nil
+	return c, nil
 }
 
 func loadConfigFromEnv(intf interface{}) error {
diff --git a/docs/news/unreleased/rhel8.5-support.md b/docs/news/unreleased/rhel8.5-support.md
index bbbe3106f..a1419000d 100644
--- a/docs/news/unreleased/rhel8.5-support.md
+++ b/docs/news/unreleased/rhel8.5-support.md
@@ -1,8 +1,41 @@
 # Add support for RHEL 8.5 main image types
 
 OSBuild Composer can now build RHEL 8.5 images.  The following new image types
-are supported: qcow2, vhd, vmdk, openstack, and ami
+are supported:
 
+- `qcow2`
+- `vhd`
+- `vmdk`
+- `openstack`
+- `ami`
+- `ec2`
+- `ec2-ha`
+
+## RHEL-8.5 AWS images
+
+The `ami` image type have been redefined based on the official RHEL EC2 images.
+
+Notable changes compared to RHEL-8.4 are:
+
+- the default user created by cloud-init is `ec2-user`
+- NTP client configuration uses `169.254.169.123` NTP server by default
+- the boot mode was changed from hybrid to legacy only
+
+The `ec2` and `ec2-ha` images represent the official RHEL EC2 images, which are
+produced as part of RHEL release. These contain RHUI client packages, which are
+available only from within Red Hat internal network. For this reason, these
+image types are by default not exposed via Weldr API (in the on-premise use
+case) for all RHEL releases.
+
+This default configuration can be overridden by placing the following line in
+the osbuild-composer configuration `/etc/osbuild-composer/osbuild-composer.toml`:
+
+```toml
+[weldr_api.distros."rhel-*"]
+# no lines below this section
+```
+
+## Extended osbuild support
 To support these image types, the following new types were added to support the
 functionality in osbuild.
 
diff --git a/internal/distro/rhel85/package_sets.go b/internal/distro/rhel85/package_sets.go
index c5830af37..2e1caeeaa 100644
--- a/internal/distro/rhel85/package_sets.go
+++ b/internal/distro/rhel85/package_sets.go
@@ -74,7 +74,7 @@ func x8664LegacyBootPackageSet() rpmmd.PackageSet {
 // x86_64 UEFI arch-specific boot package set
 func x8664UEFIBootPackageSet() rpmmd.PackageSet {
 	return rpmmd.PackageSet{
-		Include: []string{"grub2-efi-x64", "shim-x64"},
+		Include: []string{"dracut-config-generic", "grub2-efi-x64", "shim-x64"},
 	}
 }
 
diff --git a/internal/osbuild2/files_input.go b/internal/osbuild2/files_input.go
index 785ca0eeb..bb2e0e70b 100644
--- a/internal/osbuild2/files_input.go
+++ b/internal/osbuild2/files_input.go
@@ -95,9 +95,9 @@ type FileReference struct {
 	File string `json:"file"`
 }
 
-func NewFilesInputReferencesPipeline(pieline, filename string) FilesInputReferences {
+func NewFilesInputReferencesPipeline(pipeline, filename string) FilesInputReferences {
 	ref := &FilesInputReferencesPipeline{
-		fmt.Sprintf("name:%s", pieline): {File: filename},
+		fmt.Sprintf("name:%s", pipeline): {File: filename},
 	}
 	return ref
 }
diff --git a/tools/image-info b/tools/image-info
index 225d80270..ba0a99b3d 100755
--- a/tools/image-info
+++ b/tools/image-info
@@ -7,6 +7,7 @@ import errno
 import functools
 import glob
 import mimetypes
+import operator
 import json
 import os
 import platform
@@ -555,7 +556,6 @@ def read_fstab(tree):
 
     An example return value:
     [
-        [],
         [
             "UUID=6d066eb4-e4c1-4472-91f9-d167097f48d1",
             "/",
@@ -569,7 +569,7 @@ def read_fstab(tree):
     result = []
     with contextlib.suppress(FileNotFoundError):
         with open(f"{tree}/etc/fstab") as f:
-            result = sorted([line.split() for line in f if line and not line.startswith("#")])
+            result = sorted([line.split() for line in f if line.strip() and not line.startswith("#")])
     return result
 
 
@@ -1813,44 +1813,55 @@ def append_filesystem(report, tree, *, is_ostree=False):
         print("EFI partition", file=sys.stderr)
 
 
-def partition_is_esp(partition):
-    return partition["type"] == "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
-
-
-def find_esp(partitions):
-    for i, p in enumerate(partitions):
-        if partition_is_esp(p):
-            return p, i
-    return None, 0
-
-
 def append_partitions(report, device, loctl):
     partitions = report["partitions"]
-    esp, esp_id = find_esp(partitions)
 
     with contextlib.ExitStack() as cm:
-
+        # open each partition as a loop device
         devices = {}
+        device_idx_by_part_uuid = {}
         for n, part in enumerate(partitions):
             start, size = part["start"], part["size"]
             dev = cm.enter_context(loop_open(loctl, device, offset=start, size=size))
             devices[n] = dev
             read_partition(dev, part)
+            if part["uuid"]:
+                device_idx_by_part_uuid[part["uuid"].upper()] = n
 
+        # find partition with fstab and read it
+        fstab = []
         for n, part in enumerate(partitions):
             if not part["fstype"]:
                 continue
-
             with mount(devices[n]) as tree:
-                if esp and os.path.exists(f"{tree}/boot/efi"):
-                    with mount_at(devices[esp_id], f"{tree}/boot/efi", options=['umask=077']):
-                        append_filesystem(report, tree)
-                # situation when /boot is on a separate partition
-                elif esp and os.path.exists(f"{tree}/efi"):
-                    with mount_at(devices[esp_id], f"{tree}/efi", options=['umask=077']):
-                        append_filesystem(report, tree)
-                else:
-                    append_filesystem(report, tree)
+                if os.path.exists(f"{tree}/etc/fstab"):
+                    fstab.extend(read_fstab(tree))
+                    break
+        # sort the fstab entries by the mountpoint
+        fstab = sorted(fstab, key=operator.itemgetter(1))
+
+        # mount all partitions to ther respective mount points
+        root_tree = ""
+        for n, fstab_entry in enumerate(fstab):
+            part_uuid = fstab_entry[0].split("=")[1].upper()
+            part_device = devices[device_idx_by_part_uuid[part_uuid]]
+            part_mountpoint = fstab_entry[1]
+            part_fstype = fstab_entry[2]
+            part_options = fstab_entry[3].split(",")
+
+            # the first mount point should be root
+            if n == 0:
+                if part_mountpoint != "/":
+                    raise RuntimeError("The first mountpoint in sorted fstab entries is not '/'")
+                root_tree = cm.enter_context(mount(part_device))
+                continue
+
+            cm.enter_context(mount_at(part_device, f"{root_tree}{part_mountpoint}", options=part_options, extra=["-t", part_fstype]))
+
+        if not root_tree:
+            raise RuntimeError("The root filesystem tree is not mounted")
+
+        append_filesystem(report, root_tree)
 
 
 def analyse_image(image):


Test patch:
diff --git a/cmd/osbuild-composer/config_test.go b/cmd/osbuild-composer/config_test.go
index 4e26a5aa8..b6a38faba 100644
--- a/cmd/osbuild-composer/config_test.go
+++ b/cmd/osbuild-composer/config_test.go
@@ -11,11 +11,7 @@ func TestEmpty(t *testing.T) {
 	config, err := LoadConfig("testdata/empty-config.toml")
 	require.NoError(t, err)
 	require.NotNil(t, config)
-	require.Empty(t, config.Koji.AllowedDomains)
-	require.Empty(t, config.Koji.CA)
-	require.Empty(t, config.Worker.AllowedDomains)
-	require.Empty(t, config.Worker.CA)
-	require.Empty(t, config.Worker.PGDatabase)
+	require.Equal(t, GetDefaultConfig(), config)
 }
 
 func TestNonExisting(t *testing.T) {
@@ -25,6 +21,26 @@ func TestNonExisting(t *testing.T) {
 	require.Nil(t, config)
 }
 
+func TestDefaultConfig(t *testing.T) {
+	defaultConfig := GetDefaultConfig()
+	require.Empty(t, defaultConfig.Koji)
+	require.Empty(t, defaultConfig.Worker)
+	require.Empty(t, defaultConfig.ComposerAPI)
+
+	expectedWeldrAPIConfig := WeldrAPIConfig{
+		DistroConfigs: map[string]WeldrDistroConfig{
+			"rhel-*": {
+				[]string{
+					"ec2",
+					"ec2-ha",
+				},
+			},
+		},
+	}
+
+	require.Equal(t, expectedWeldrAPIConfig, defaultConfig.WeldrAPI)
+}
+
 func TestConfig(t *testing.T) {
 	config, err := LoadConfig("testdata/test.toml")
 	require.NoError(t, err)
diff --git a/cmd/osbuild-composer/testdata/test.toml b/cmd/osbuild-composer/testdata/test.toml
index da6b3a977..fb415a83e 100644
--- a/cmd/osbuild-composer/testdata/test.toml
+++ b/cmd/osbuild-composer/testdata/test.toml
@@ -12,3 +12,6 @@ image_type_denylist = [ "qcow2", "vmdk" ]
 
 [weldr_api.distros.rhel-84]
 image_type_denylist = [ "qcow2" ]
+
+# overrides the default rhel-* configuration
+[weldr_api.distros."rhel-*"]
diff --git a/test/data/manifests/rhel_85-aarch64-ami-boot.json b/test/data/manifests/rhel_85-aarch64-ami-boot.json
index 8456943f5..431cc530b 100644
--- a/test/data/manifests/rhel_85-aarch64-ami-boot.json
+++ b/test/data/manifests/rhel_85-aarch64-ami-boot.json
@@ -10795,7 +10795,6 @@
     },
     "rpm-verify": {
       "changed": {
-        "/boot": ".M.......",
         "/etc/chrony.conf": "S.5....T.",
         "/etc/dnf/plugins/product-id.conf": "..5....T.",
         "/etc/dnf/plugins/subscription-manager.conf": "..5....T.",
@@ -10814,34 +10813,9 @@
         "/var/spool/anacron/cron.monthly": ".M.......",
         "/var/spool/anacron/cron.weekly": ".M......."
       },
-      "missing": [
-        "/boot/efi",
-        "/boot/efi/EFI",
-        "/boot/efi/EFI/BOOT",
-        "/boot/efi/EFI/BOOT/BOOTAA64.EFI",
-        "/boot/efi/EFI/BOOT/fbaa64.efi",
-        "/boot/efi/EFI/redhat",
-        "/boot/efi/EFI/redhat",
-        "/boot/efi/EFI/redhat/BOOTAA64.CSV",
-        "/boot/efi/EFI/redhat/fonts",
-        "/boot/efi/EFI/redhat/grubaa64.efi",
-        "/boot/efi/EFI/redhat/mmaa64.efi",
-        "/boot/efi/EFI/redhat/shim.efi",
-        "/boot/efi/EFI/redhat/shimaa64-redhat.efi",
-        "/boot/efi/EFI/redhat/shimaa64.efi",
-        "/boot/grub2",
-        "/boot/grub2/grubenv",
-        "/boot/loader/entries"
-      ]
+      "missing": []
     },
     "selinux": {
-      "context-mismatch": [
-        {
-          "actual": "unconfined_u:object_r:unlabeled_t:s0",
-          "expected": "system_u:object_r:boot_t:s0",
-          "filename": "/boot"
-        }
-      ],
       "policy": {
         "SELINUX": "enforcing",
         "SELINUXTYPE": "targeted"
diff --git a/test/data/manifests/rhel_85-aarch64-ec2-boot.json b/test/data/manifests/rhel_85-aarch64-ec2-boot.json
index 45609410e..9b1da0f7b 100644
--- a/test/data/manifests/rhel_85-aarch64-ec2-boot.json
+++ b/test/data/manifests/rhel_85-aarch64-ec2-boot.json
@@ -10835,7 +10835,6 @@
     },
     "rpm-verify": {
       "changed": {
-        "/boot": ".M.......",
         "/etc/chrony.conf": "S.5....T.",
         "/etc/dnf/plugins/product-id.conf": "..5....T.",
         "/etc/dnf/plugins/subscription-manager.conf": "..5....T.",
@@ -10857,34 +10856,10 @@
         "/var/spool/anacron/cron.weekly": ".M......."
       },
       "missing": [
-        "/boot/efi",
-        "/boot/efi/EFI",
-        "/boot/efi/EFI/BOOT",
-        "/boot/efi/EFI/BOOT/BOOTAA64.EFI",
-        "/boot/efi/EFI/BOOT/fbaa64.efi",
-        "/boot/efi/EFI/redhat",
-        "/boot/efi/EFI/redhat",
-        "/boot/efi/EFI/redhat/BOOTAA64.CSV",
-        "/boot/efi/EFI/redhat/fonts",
-        "/boot/efi/EFI/redhat/grubaa64.efi",
-        "/boot/efi/EFI/redhat/mmaa64.efi",
-        "/boot/efi/EFI/redhat/shim.efi",
-        "/boot/efi/EFI/redhat/shimaa64-redhat.efi",
-        "/boot/efi/EFI/redhat/shimaa64.efi",
-        "/boot/grub2",
-        "/boot/grub2/grubenv",
-        "/boot/loader/entries",
         "/etc/yum.repos.d/redhat-rhui.repo"
       ]
     },
     "selinux": {
-      "context-mismatch": [
-        {
-          "actual": "unconfined_u:object_r:unlabeled_t:s0",
-          "expected": "system_u:object_r:boot_t:s0",
-          "filename": "/boot"
-        }
-      ],
       "policy": {
         "SELINUX": "enforcing",
         "SELINUXTYPE": "targeted"