Repository: osbuild/osbuild-composer
Pull request #1619 (2021-08-05 06:21:55)

osbuild2/grub2.inst: always include location in stage options
Location property is actually required. If it's omitted from the stage
options, the stage just fails.

Also see: https://github.com/osbuild/osbuild/pull/759

Signed-off-by: Ondřej Budai <ondrej@budai.cz>


This pull request includes:

- [ ] adequate testing for the new functionality or fixed issue
- [ ] adequate documentation informing people about the change such as
  - [ ] 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
  - [ ] 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/internal/osbuild2/grub2_inst_stage.go b/internal/osbuild2/grub2_inst_stage.go
index 8019e26e9..fdd4a4c46 100644
--- a/internal/osbuild2/grub2_inst_stage.go
+++ b/internal/osbuild2/grub2_inst_stage.go
@@ -14,7 +14,7 @@ type Grub2InstStageOptions struct {
 	// Platform of the target system
 	Platform string `json:"platform"`
 
-	Location uint64 `json:"location,omitempty"`
+	Location uint64 `json:"location"`
 
 	// How to obtain the GRUB core image
 	Core CoreMkImage `json:"core"`
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"