Add bootloader metadata for COSI 1.1 #285
Conversation
| if err != nil { | ||
| return nil, fmt.Errorf("failed to extract systemd-boot entries:\n%w", err) | ||
| } | ||
| return &CosiBootloader{ |
There was a problem hiding this comment.
Shouldn't this fallthrough if the directory exists but is empty?
| line = strings.TrimSpace(line) | ||
| switch { | ||
| case strings.HasPrefix(line, "options"): | ||
| entry.Cmdline = strings.TrimSpace(strings.TrimPrefix(line, "options")) |
There was a problem hiding this comment.
This should append to entry.Cmdline rather than replace it. The spec says:
options shall contain kernel parameters to pass to the Linux kernel to spawn. This key is optional and may appear more than once in which case all specified parameters are combined in the order they are listed.
Also don't forget to include a space between concatenated items.
| if kernelVer, err := getKernelVersion(filepath.Base(linuxPath)); err == nil { | ||
| entry.Kernel = kernelVer | ||
| } | ||
| case strings.HasPrefix(line, "uki"): |
There was a problem hiding this comment.
The version of systemd-boot in AZL3 doesn't actually support this option. I think it is planned to land in systemd 258
Apparently Regarding |
| Images []FileSystem `json:"images"` | ||
| OsRelease string `json:"osRelease"` | ||
| Id string `json:"id,omitempty"` | ||
| Bootloader *CosiBootloader `json:"bootloader"` |
| type BootloaderType string | ||
|
|
||
| const ( | ||
| BootloaderGrub BootloaderType = "grub" |
There was a problem hiding this comment.
It is a good idea for enum values to start with the name of the enum. e.g. BootloaderTypeGrub. This makes it easier to see know the type of the value.
| SystemdBoot *SystemDBoot `json:"systemdBoot,omitempty"` | ||
| } | ||
|
|
||
| type Metadata struct { |
There was a problem hiding this comment.
Metadata doesn't seem to be used.
| Path: filepath.Join("boot", "loader", "entries", file.Name()), | ||
| } | ||
|
|
||
| lines := strings.Split(string(content), "\n") |
There was a problem hiding this comment.
Let's write a proper parser for the systemd-boot entry files. That way, the code will be reusable for when we add proper support for systemd-boot.
Also, the way you have written the code has a lot of code duplication (e.g. strings.TrimSpace(strings.TrimPrefix(, which would be good to remove.
The file format is documented here: Type #1 Boot Loader Specification Entries
| line = strings.TrimSpace(line) | ||
| switch { | ||
| case strings.HasPrefix(line, "options"): | ||
| entry.Cmdline = strings.TrimSpace(strings.TrimPrefix(line, "options")) |
There was a problem hiding this comment.
Any spaces at the end of the line are still considered to be part of the value. The only spaces that should be removed are the ones that are between the key and the value.
| return entries, nil | ||
| } | ||
|
|
||
| func GrubConfigSupportExists(installChroot safechroot.ChrootInterface) bool { |
There was a problem hiding this comment.
For Azure Linux, the best way to tell which bootloader is being used is to check if either the grub2-efi-binary or systemd-boot package is installed.
Add bootloader metadata extraction logic support grub-based boot and multiple types of UKI-based boot:
tested with uki image:
Checklist